7128.43s.secret

plugins/api/src/replica_close.cpp

@@ -360,6 +360,17 @@ namespace
                 return BAD_INPUT_DESC_INDEX;
             }
 
+            const auto compute_checksum_iter = json_input.find("compute_checksum");
+            const auto compute_checksum =
+                (json_input.end() != compute_checksum_iter) && compute_checksum_iter->get<bool>();
+            if (compute_checksum && !update_status) {
+                log::api::error("{}: Incompatible parameters for closing [{}]: Checksum cannot be computed without "
+                                "updating replica status.",
+                                __func__,
+                                l1desc.dataObjInfo->objPath);
+                return USER_INCOMPATIBLE_PARAMS;
+            }
+
             // Redirect to the federated zone if the local L1 descriptor references a remote zone.
             if (l1desc.oprType == REMOTE_ZONE_OPR && l1desc.remoteZoneHost) {
                 auto* conn = l1desc.remoteZoneHost->conn;
@@ -388,8 +399,6 @@ namespace
             }};
 
             const auto update_size = !json_input.contains("update_size") || json_input.at("update_size").get<bool>();
-            const auto compute_checksum =
-                json_input.contains("compute_checksum") && json_input.at("compute_checksum").get<bool>();
 
             // Close the underlying file object.
             if (const auto ec = close_physical_object(*_comm, l1desc.l3descInx); ec != 0) {

scripts/irods/test/test_istream.py

@@ -1,13 +1,10 @@
+import copy
 import os
+import socket
 import sys
 import tempfile
-import socket
-import copy
-
-if sys.version_info < (2, 7):
-    import unittest2 as unittest
-else:
-    import unittest
+import time
+import unittest
 
 from . import session
 from .. import lib
@@ -523,3 +520,48 @@ def do_test(offset, count):
         finally:
             if os.path.exists(local_filename):
                 os.unlink(local_filename)
+
+    def test_istream_write_with_various_options_on_empty_replica_with_effects_on_mtime__issue_7128(self):
+        logical_path = "/".join([self.user.session_collection, "emptiness"])
+
+        try:
+            # Create an empty data object.
+            self.user.assert_icommand(["itouch", logical_path])
+
+            with self.subTest("with truncate"):
+                # Get the current mtime for comparison.
+                original_mtime = lib.get_replica_mtime(self.user, logical_path, 0)
+
+                # Sleep for one second so that the modify time has a chance to change from the original.
+                time.sleep(1)
+
+                # Open it for write, but don't write anything. istream truncates by default, so mtime should be updated.
+                self.user.assert_icommand(["istream", "write", logical_path])
+                self.assertLess(original_mtime, lib.get_replica_mtime(self.user, logical_path, 0))
+
+            with self.subTest("with no-trunc"):
+                # Get the current mtime for comparison.
+                original_mtime = lib.get_replica_mtime(self.user, logical_path, 0)
+
+                # Sleep for one second so that the modify time has a chance to change from the original.
+                time.sleep(1)
+
+                # Open it for write with no-trunc, but don't write anything. Opening for write without truncating should
+                # not update the mtime
+                self.user.assert_icommand(["istream", "--no-trunc", "write", logical_path])
+                self.assertEqual(original_mtime, lib.get_replica_mtime(self.user, logical_path, 0))
+
+            with self.subTest("with append"):
+                # Get the current mtime for comparison.
+                original_mtime = lib.get_replica_mtime(self.user, logical_path, 0)
+
+                # Sleep for one second so that the modify time has a chance to change from the original.
+                time.sleep(1)
+
+                # Open it for write with append, but don't write anything. append implies no-trunc, so mtime should not
+                # be updated.
+                self.user.assert_icommand(["istream", "--append", "write", logical_path])
+                self.assertEqual(original_mtime, lib.get_replica_mtime(self.user, logical_path, 0))
+
+        finally:
+            self.user.run_icommand(["irm", "-f", logical_path])

server/api/src/rsDataObjOpen.cpp

@@ -687,8 +687,8 @@ namespace
             return l1_index;
         }
 
-        // Update the catalog to reflect the truncation (i.e. set the data size to zero and erase
-        // the checksum). It is important that the catalog reflect truncation immediately because
+        // Update the catalog to reflect the truncation (i.e. set the data size to zero, erase the checksum, and set
+        // the mtime to the current time). It is important that the catalog reflect truncation immediately because
         // operations following the open may depend on the size and checksum of the replica.
         //
         // TODO: optimization... do not touch the catalog -- update the structure and use this in lock_data_object
@@ -703,7 +703,20 @@ namespace
                 replica.size(0);
                 replica.checksum("");
 
-                rst::update(replica.data_id(), replica.replica_number(), {{"data_size", "0"}, {"data_checksum", ""}});
+                // Manually get current time and format the string because using SET_TIME_TO_NOW_KW causes the mtime to
+                // update again on close since the SET_TIME_TO_NOW_KW persists as the mtime value in the "after"
+                // replica state table entry.
+                {
+                    using object_time_type = std::chrono::time_point<std::chrono::system_clock, std::chrono::seconds>;
+                    using clock_type = object_time_type::clock;
+                    using duration_type = object_time_type::duration;
+                    const auto now = std::chrono::time_point_cast<duration_type>(clock_type::now());
+                    replica.mtime(fmt::format("{:011}", now.time_since_epoch().count()));
+                }
+
+                rst::update(replica.data_id(),
+                            replica.replica_number(),
+                            {{"data_size", "0"}, {"data_checksum", ""}, {"modify_ts", replica.mtime()}});
 
                 if (const auto [ret, ec] = rst::publish::to_catalog(_comm, {replica, admin_operation}); ec < 0) {
                     return ec;