[ 7498] Deprecate AGE_KW usage in trim API

Adds a deprecation message to the rError stack of the RsComm
object in the trim API when the AGE_KW is provided by the client.

This commit fixes an issue where the deprecation messages are not being
printed by itrim due to a client call occurring after rcDataObjTrim. This
causes the rError stack to be freed and set to nullptr in the RcComm which
means that the deprecation messages are lost when rcObjStat is called. The
issue would only manifest when a trim actually occurs such that the
rcObjStat call would be necessary. At the expense of a possibly unnecessary
API invocation, rcObjStat is now called before performing the trim so that
the messages from the salient API call (namely, rcDataObjTrim) can be
returned to the itrim client.

Also also, fixes a potential memory leak in trimUtil.

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++;

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'

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...