sort at the right place in the code

Author: d-w-moore

irods/data_object.py

@@ -47,7 +47,7 @@ def __repr__(self):
 
 def _DEFAULT_SORT_KEY_FN(row):
     repl_status = int(row[DataObject.replica_status])
-   
+
     repl_status_rank = (
         _REPL_STATUSES.index(repl_status) if _REPL_STATUSES.count(repl_status)
         else sys.maxsize
@@ -61,6 +61,8 @@ def __init__(self, manager, parent=None, results=None, replica_sort_function=Non
         self.manager = manager
         if parent and results:
             self.collection = parent
+            if results:
+                results = sorted(results, key=(replica_sort_function or _DEFAULT_SORT_KEY_FN))
             for attr, value in DataObject.__dict__.items():
                 if not attr.startswith("_"):
                     try:
@@ -69,7 +71,6 @@ def __init__(self, manager, parent=None, results=None, replica_sort_function=Non
                         # backward compatibility with older schema versions
                         pass
             self.path = self.collection.path + "/" + self.name
-            replicas = sorted(results, key=(replica_sort_function or _DEFAULT_SORT_KEY_FN))
 
             # The status quo before iRODS 5
 
@@ -90,13 +91,13 @@ def __init__(self, manager, parent=None, results=None, replica_sort_function=Non
                         modify_time=r[DataObject.modify_time],
                     ),
                 )
-                for r in replicas
+                for r in results
             ]
 
             # Adjust for adding access_time in the iRODS 5 case.
 
             if self.manager.sess.server_version >= (5,):
-                for n, r in enumerate(replicas):
+                for n, r in enumerate(results):
                     replica_args[n][1]['access_time'] = r[DataObject.access_time]
             self.replicas = [iRODSReplica(*a, **k) for a, k in replica_args]
 

irods/test/data_obj_test.py

@@ -1251,7 +1251,7 @@ def test_replica_number(self):
         # refresh object
         obj = session.data_objects.get(obj_path)
 
-        # assertions on replicas 
+        # assertions on replicas
         self.assertEqual(len(obj.replicas), number_of_replicas)
         self.assertEqual(
             {repl.number for repl in obj.replicas},
@@ -2992,6 +2992,35 @@ def test_handling_of_termination_signals_during_multithread_put__issue_722(self)
 
         test_put__issue_722(self)
 
+    def test_default_sorting_of_replicas__issue_647(self):
+        basename = unique_name(my_function_name(), datetime.now()) + '_dataobj_647'
+        with self.create_simple_resc() as newResc1, self.create_simple_resc() as newResc2:
+            data = helpers.make_object(self.sess, f'{helpers.home_collection(self.sess)}/{basename}')
+
+            # Precondition for an eventual total of 3 replicas: initial data replica is not on either of the new resources.
+            self.assertFalse({repl.resource_name for repl in data.replicas} & {newResc1, newResc2})
+            try:
+                data.replicate(resource=newResc1)
+
+                # Ensure that one of the replicas is stale, to test proper sorting.
+                with data.open('a',**{kw.RESC_NAME_KW:newResc1}) as f:
+                    f.write(b'.')
+                time.sleep(2)
+
+                # Voting should ensure exactly two good replicas of the three.
+                data.replicate(resource=newResc2)
+
+                # refresh replicas
+                data = self.sess.data_objects.get(data.path)
+
+                # Test replica sorting.
+                self.assertEqual(data.replicas[0].status, '1')
+                self.assertEqual(data.replicas[0].modify_time, data.modify_time)
+                self.assertGreater(data.replicas[0].modify_time, data.replicas[1].modify_time)
+            finally:
+                if data:
+                    data.unlink(force=True)
+
 
 if __name__ == "__main__":
     # let the tests find the parent irods lib