Merge branch...

Merge branch '214275-fix-geo-attachmentregistry-finder-find_registry_differences-returns-wrong-file-type' into 'master'

Geo - Fix wrong file type for source uploads in Geo::AttachmentRegistryFinder#find_registry_differences

Closes #214275

See merge request gitlab-org/gitlab!29449

ee/app/finders/geo/attachment_registry_finder.rb

@@ -34,13 +34,14 @@ module Geo
       Upload
     end
 
-    # Returns untracked IDs as well as tracked IDs that are unused.
+    # Returns untracked uploads as well as tracked uploads that are unused.
     #
-    # Untracked IDs are model IDs that are supposed to be synced but don't yet
-    # have a registry entry.
+    # Untracked uploads is an array where each item is a tuple of [id, file_type]
+    # that is supposed supposed to be synced but don't yet have a registry entry.
     #
-    # Unused tracked IDs are model IDs that are not supposed to be synced but
-    # already have a registry entry. For example:
+    # Unused uploads is an array where each item is a tuple of [id, file_type]
+    # that is not supposed to be synced but already have a registry entry. For
+    # example:
     #
     #   - orphaned registries
     #   - records that became excluded from selective sync
@@ -50,10 +51,22 @@ module Geo
     # We compute both sets in this method to reduce the number of DB queries
     # performed.
     #
-    # @return [Array] the first element is an Array of untracked IDs, and the second element is an Array of tracked IDs that are unused
+    # @return [Array] the first element is an Array of untracked uploads, and the
+    #                 second element is an Array of tracked uploads that are unused.
+    #                 For example: [[[1, 'avatar'], [5, 'file']], [[3, 'attachment']]]
     def find_registry_differences(range)
-      source = attachments(fdw: false).where(id: range).pluck(::Upload.arel_table[:id], ::Upload.arel_table[:uploader]) # rubocop:disable CodeReuse/ActiveRecord
-      tracked = Geo::UploadRegistry.where(file_id: range).pluck(:file_id, :file_type) # rubocop:disable CodeReuse/ActiveRecord
+      # rubocop:disable CodeReuse/ActiveRecord
+      source =
+        attachments(fdw: false)
+            .id_in(range)
+            .pluck(::Upload.arel_table[:id], ::Upload.arel_table[:uploader])
+            .map! { |id, uploader| [id, uploader.sub(/Uploader\z/, '').underscore] }
+
+      tracked =
+        Geo::UploadRegistry
+            .model_id_in(range)
+            .pluck(:file_id, :file_type)
+      # rubocop:enable CodeReuse/ActiveRecord
 
       untracked = source - tracked
       unused_tracked = tracked - source

ee/spec/finders/geo/attachment_registry_finder_spec.rb

@@ -339,5 +339,36 @@ describe Geo::AttachmentRegistryFinder, :geo, :geo_fdw do
         end
       end
     end
+
+    describe '#find_registry_differences' do
+      it 'returns untracked IDs as well as tracked IDs that are unused', :aggregate_failures do
+        max_id = Upload.maximum(:id)
+        create(:geo_upload_registry, :avatar, file_id: upload_synced_group.id)
+        create(:geo_upload_registry, :file, file_id: upload_issuable_synced_nested_project.id)
+        create(:geo_upload_registry, :avatar, file_id: upload_synced_project.id)
+        create(:geo_upload_registry, :personal_file, file_id: upload_personal_snippet.id)
+        create(:geo_upload_registry, :avatar, file_id: upload_remote_synced_project.id)
+        unused_registry_1 = create(:geo_upload_registry, :attachment, file_id: max_id + 1)
+        unused_registry_2 = create(:geo_upload_registry, :personal_file, file_id: max_id + 2)
+        range = 1..(max_id + 2)
+
+        untracked, unused = subject.find_registry_differences(range)
+
+        expected_untracked = [
+          [upload_unsynced_group.id, 'avatar'],
+          [upload_unsynced_project.id, 'avatar'],
+          [upload_remote_unsynced_project.id, 'avatar'],
+          [upload_remote_synced_group.id, 'avatar']
+        ]
+
+        expected_unused = [
+          [unused_registry_1.file_id, 'attachment'],
+          [unused_registry_2.file_id, 'personal_file']
+        ]
+
+        expect(untracked).to match_array(expected_untracked)
+        expect(unused).to match_array(expected_unused)
+      end
+    end
   end
 end