diff --git a/lib/core/src/trimUtil.cpp b/lib/core/src/trimUtil.cpp index 3bc93ee296..7ed5370c63 100644 --- a/lib/core/src/trimUtil.cpp +++ b/lib/core/src/trimUtil.cpp @@ -9,6 +9,7 @@ #include "irods/miscUtil.h" #include "irods/trimUtil.h" #include "irods/rcGlobalExtern.h" +#include "irods/irods_at_scope_exit.hpp" rodsLong_t TotalSizeTrimmed = 0; int TotalTrimmed = 0; @@ -75,7 +76,6 @@ int trimDataObjUtil( rcComm_t *conn, char *srcPath, rodsArguments_t *rodsArgs, dataObjInp_t *dataObjInp ) { int status = 0; - rodsObjStat_t *rodsObjStatOut = NULL; if ( srcPath == NULL ) { rodsLog( LOG_ERROR, @@ -85,6 +85,22 @@ trimDataObjUtil( rcComm_t *conn, char *srcPath, rstrcpy( dataObjInp->objPath, srcPath, MAX_NAME_LEN ); + // rcObjStat results will only be used if rcDataObjTrim is successful and actually trims at least one replica from + // the data object. The invocation occurs before the rcDataObjTrim call because the API call will cause the rError + // stack in the RcComm struct to be freed and set to nullptr (in successful cases), losing any rError messages + // which might have been added in rcDataObjTrim. Specifically, a successful trim might include messages warning the + // client that a deprecated option has been used and we want to ensure that these are returned to the client. + rodsObjStat_t* rodsObjStatOut = nullptr; + const auto free_obj_stat = irods::at_scope_exit{[&rodsObjStatOut] { + if (rodsObjStatOut) { + freeRodsObjStat(rodsObjStatOut); + } + }}; + const int objStatus = rcObjStat(conn, dataObjInp, &rodsObjStatOut); + if (objStatus < 0) { + return objStatus; + } + status = rcDataObjTrim( conn, dataObjInp ); if ( status < 0 ) { @@ -102,10 +118,6 @@ trimDataObjUtil( rcComm_t *conn, char *srcPath, } if ( status > 0 ) { - const int objStatus = rcObjStat(conn, dataObjInp, &rodsObjStatOut); - if ( objStatus < 0 ) { - return objStatus; - } if ( objStatus == DATA_OBJ_T ) { TotalSizeTrimmed += rodsObjStatOut->objSize; TotalTrimmed++; diff --git a/scripts/irods/test/test_itrim.py b/scripts/irods/test/test_itrim.py index 42f80d1f67..7bca5f1c65 100644 --- a/scripts/irods/test/test_itrim.py +++ b/scripts/irods/test/test_itrim.py @@ -124,6 +124,43 @@ def test_itrim_option_N_is_deprecated__issue_4860(self): lib.remove_resource(self.admin, resc1) lib.remove_resource(self.admin, resc2) + def test_itrim_option_age_is_deprecated__issue_7498(self): + resc1 = 'issue_7498_1' + resc2 = 'issue_7498_2' + filename = 'test_itrim_option_age_is_deprecated__issue_7498' + logical_path = os.path.join(self.admin.session_collection, filename) + + try: + lib.create_ufs_resource(self.admin, resc1, test.settings.HOSTNAME_2) + lib.create_ufs_resource(self.admin, resc2, test.settings.HOSTNAME_3) + + # Create 3 replicas so that a trim will actually occur. + self.admin.assert_icommand(['itouch', logical_path]) + self.admin.assert_icommand(['irepl', '-R', resc1, logical_path]) + self.admin.assert_icommand(['irepl', '-R', resc2, logical_path]) + self.assertTrue(lib.replica_exists_on_resource(self.admin, logical_path, self.admin.default_resource)) + self.assertTrue(lib.replica_exists_on_resource(self.admin, logical_path, resc1)) + self.assertTrue(lib.replica_exists_on_resource(self.admin, logical_path, resc2)) + + # Ensure the deprecation message appears when a replica is not trimmed. + self.admin.assert_icommand( + ['itrim', '--age', '9001', logical_path], + 'STDOUT_MULTILINE', + ['Specifying a minimum age of replicas to trim is deprecated.', 'files trimmed = 0']) + + # Ensure the deprecation message appears when a replica is trimmed. + self.admin.assert_icommand( + ['itrim', '--age', '0', logical_path], + 'STDOUT_MULTILINE', + ['Specifying a minimum age of replicas to trim is deprecated.', 'files trimmed = 1']) + + finally: + self.admin.assert_icommand(['ils', '-l', logical_path], 'STDOUT', filename) # debugging + self.admin.run_icommand(['irm', '-f', logical_path]) + lib.remove_resource(self.admin, resc1) + lib.remove_resource(self.admin, resc2) + + @unittest.skipIf(test.settings.RUN_IN_TOPOLOGY, "Skip for Topology Testing") def test_itrim_removes_in_vault_registered_replicas_from_disk__issue_5362(self): resc_name = 'issue_5362_resc' diff --git a/server/api/src/rsDataObjTrim.cpp b/server/api/src/rsDataObjTrim.cpp index 72f7f10c1d..4d779c62c0 100644 --- a/server/api/src/rsDataObjTrim.cpp +++ b/server/api/src/rsDataObjTrim.cpp @@ -305,13 +305,16 @@ int rsDataObjTrim( // The native rule engine may erase all messages in the rError array. // The only way to guarantee that messages are received by the client // is to add them to the rError array when the function returns. - irods::at_scope_exit at_scope_exit{[&] { - // itrim -N + const auto add_warning_messages_for_deprecated_parameters = irods::at_scope_exit{[&] { if (getValByKey(&dataObjInp->condInput, COPIES_KW)) { addRErrorMsg(&rsComm->rError, DEPRECATED_PARAMETER, "Specifying a minimum number of replicas to keep is deprecated."); } + if (getValByKey(&dataObjInp->condInput, AGE_KW)) { + addRErrorMsg( + &rsComm->rError, DEPRECATED_PARAMETER, "Specifying a minimum age of replicas to trim is deprecated."); + } }}; // -S and -n are incompatible...