From dad6c4a24cda23f169ade78c04324dd7579e75e2 Mon Sep 17 00:00:00 2001
From: Samuel Sciolla <ssciolla@umich.edu>
Date: Fri, 26 Apr 2024 15:33:51 -0400
Subject: [PATCH] Add explicit sorting; update/improve service tests

---
 lib/archivematica.rb       |  4 ++-
 test/test_archivematica.rb | 55 +++++++++++++++++++-------------------
 2 files changed, 31 insertions(+), 28 deletions(-)

diff --git a/lib/archivematica.rb b/lib/archivematica.rb
index 73c2a79..d34cdf5 100644
--- a/lib/archivematica.rb
+++ b/lib/archivematica.rb
@@ -184,7 +184,9 @@ def get_package_data_objects(stored_date:, package_filter: AllPackageFilter.new)
       logger.info("Archivematica instance: #{@name}")
       packages = @api.get_packages(location_uuid: @location_uuid, stored_date: stored_date)
       filtered_packages = package_filter.filter(packages)
-      filtered_packages.map { |package| create_package_data_object(package) }
+      filtered_packages
+        .map { |package| create_package_data_object(package) }
+        .sort_by(&:stored_time)
     end
   end
 end
diff --git a/test/test_archivematica.rb b/test/test_archivematica.rb
index 3f3379e..3a54175 100644
--- a/test/test_archivematica.rb
+++ b/test/test_archivematica.rb
@@ -200,20 +200,21 @@ def setup
     @location_uuid = SecureRandom.uuid
     @stored_date = Time.utc(2024, 2, 17)
 
-    @test_packages = [
-      Package.new(
-        uuid: "0948e2ae-eb24-4984-a71b-43bc440534d0",
-        path: "/storage/0948/e2ae/eb24/4984/a71b/43bc/4405/34d0/identifier-one-0948e2ae-eb24-4984-a71b-43bc440534d0",
-        size: 200000,
-        stored_date: "2024-02-18T00:00:00.000000"
-      ),
-      Package.new(
-        uuid: "0baa468e-dd42-49ff-ba90-5dedc30c8541",
-        path: "/storage/0baa/468e/dd42/49ff/ba90/5ded/c30c/8541/identifier-two-0baa468e-dd42-49ff-ba90-5dedc30c8541",
-        size: 500000000,
-        stored_date: "2024-02-19T00:00:00.000000"
-      )
-    ]
+    @first_package = Package.new(
+      uuid: "0948e2ae-eb24-4984-a71b-43bc440534d0",
+      path: "/storage/0948/e2ae/eb24/4984/a71b/43bc/4405/34d0/identifier-one-0948e2ae-eb24-4984-a71b-43bc440534d0",
+      size: 200000,
+      stored_date: "2024-02-18T00:00:00.000000"
+    )
+    @second_package = Package.new(
+      uuid: "0baa468e-dd42-49ff-ba90-5dedc30c8541",
+      path: "/storage/0baa/468e/dd42/49ff/ba90/5ded/c30c/8541/identifier-two-0baa468e-dd42-49ff-ba90-5dedc30c8541",
+      size: 500000000,
+      stored_date: "2024-02-19T00:00:00.000000"
+    )
+    # We expect packages to be in ascending order by stored date,
+    # but switching it here to ensure our code corrects it if another order occurs
+    @test_packages = [@second_package, @first_package]
 
     @service = ArchivematicaService.new(
       name: "test",
@@ -223,30 +224,31 @@ def setup
   end
 
   def test_get_package_data_objects_with_no_filter
-    @mock_api.expect(:get_packages, @test_packages, location_uuid: @location_uuid, stored_date: @stored_date)
+    @mock_api.expect(
+      :get_packages, @test_packages, location_uuid: @location_uuid, stored_date: @stored_date
+    )
     package_data_objs = @service.get_package_data_objects(stored_date: @stored_date)
     @mock_api.verify
 
     # No objects are filtered out
-    first_package, second_package = @test_packages
     expected = [
       RepositoryPackageData.new(
-        remote_path: first_package.path,
+        remote_path: @first_package.path,
         dir_name: "identifier-one-0948e2ae-eb24-4984-a71b-43bc440534d0",
         metadata: ObjectMetadata.new(
-          id: first_package.uuid,
-          title: "#{first_package.uuid} / identifier-one",
+          id: @first_package.uuid,
+          title: "#{@first_package.uuid} / identifier-one",
           creator: "Not available",
           description: "Not available"
         ),
         stored_time: Time.utc(2024, 2, 18)
       ),
       RepositoryPackageData.new(
-        remote_path: second_package.path,
+        remote_path: @second_package.path,
         dir_name: "identifier-two-0baa468e-dd42-49ff-ba90-5dedc30c8541",
         metadata: ObjectMetadata.new(
-          id: second_package.uuid,
-          title: "#{second_package.uuid} / identifier-two",
+          id: @second_package.uuid,
+          title: "#{@second_package.uuid} / identifier-two",
           creator: "Not available",
           description: "Not available"
         ),
@@ -266,16 +268,15 @@ def test_get_package_data_objects_with_size_filter
 
     # Larger object is filtered out
     assert_equal 1, package_data_objs.length
-    assert_equal package_data_objs[0].metadata.id, @test_packages[0].uuid
+    assert_equal @first_package.uuid, package_data_objs[0].metadata.id
   end
 
   def test_get_package_data_object_when_exists
-    first_package = @test_packages[0]
-    @mock_api.expect(:get_package, first_package, [first_package.uuid])
-    package_data_obj = @service.get_package_data_object(first_package.uuid)
+    @mock_api.expect(:get_package, @first_package, [@first_package.uuid])
+    package_data_obj = @service.get_package_data_object(@first_package.uuid)
     @mock_api.verify
     assert package_data_obj.is_a?(RepositoryPackageData)
-    assert_equal first_package.path, package_data_obj.remote_path
+    assert_equal @first_package.path, package_data_obj.remote_path
   end
 
   def test_get_package_data_object_when_does_not_exist