Merge branch '11922-job-artifact-registry-finder' into 'master'

Geo - Remove legacy queries from Geo::JobArtifactRegistryFinder

See merge request gitlab-org/gitlab-ee!14289

ee/app/finders/geo/job_artifact_registry_finder.rb

@@ -2,31 +2,33 @@
 
 module Geo
   class JobArtifactRegistryFinder < RegistryFinder
+    def initialize(current_node:)
+      @current_node = Geo::Fdw::GeoNode.find(current_node.id)
+    end
+
+    def count_registry
+      Geo::JobArtifactRegistry.count
+    end
+
     def count_syncable
       syncable.count
     end
 
     def count_synced
-      job_artifacts_synced.count
+      registries_for_job_artifacts.merge(Geo::JobArtifactRegistry.synced).count
     end
 
     def count_failed
-      job_artifacts_failed.count
+      registries_for_job_artifacts.merge(Geo::JobArtifactRegistry.failed).count
     end
 
     def count_synced_missing_on_primary
-      job_artifacts_synced_missing_on_primary.count
-    end
-
-    def count_registry
-      Geo::JobArtifactRegistry.count
+      registries_for_job_artifacts.merge(Geo::JobArtifactRegistry.synced.missing_on_primary).count
     end
 
     def syncable
-      if use_legacy_queries_for_selective_sync?
-        legacy_finder.syncable
-      elsif selective_sync?
-        fdw_geo_node.job_artifacts.syncable
+      if selective_sync?
+        job_artifacts.syncable
       else
         Ci::JobArtifact.syncable
       end
@@ -44,27 +46,21 @@ module Geo
     # @param [Array<Integer>] except_artifact_ids ids that will be ignored from the query
     # rubocop: disable CodeReuse/ActiveRecord
     def find_unsynced(batch_size:, except_artifact_ids: [])
-      relation =
-        if use_legacy_queries_for_selective_sync?
-          legacy_finder.job_artifacts_unsynced(except_artifact_ids: except_artifact_ids)
-        else
-          job_artifacts_unsynced(except_artifact_ids: except_artifact_ids)
-        end
-
-      relation.limit(batch_size)
+      job_artifacts
+        .syncable
+        .missing_job_artifact_registry
+        .id_not_in(except_artifact_ids)
+        .limit(batch_size)
     end
     # rubocop: enable CodeReuse/ActiveRecord
 
     # rubocop: disable CodeReuse/ActiveRecord
     def find_migrated_local(batch_size:, except_artifact_ids: [])
-      relation =
-        if use_legacy_queries_for_selective_sync?
-          legacy_finder.job_artifacts_migrated_local(except_artifact_ids: except_artifact_ids)
-        else
-          job_artifacts_migrated_local(except_artifact_ids: except_artifact_ids)
-        end
-
-      relation.limit(batch_size)
+      job_artifacts
+        .inner_join_job_artifact_registry
+        .with_files_stored_remotely
+        .id_not_in(except_artifact_ids)
+        .limit(batch_size)
     end
     # rubocop: enable CodeReuse/ActiveRecord
 
@@ -91,65 +87,14 @@ module Geo
 
     private
 
-    # rubocop:disable CodeReuse/Finder
-    def legacy_finder
-      @legacy_finder ||= Geo::LegacyJobArtifactRegistryFinder.new(current_node: current_node)
-    end
-    # rubocop:enable CodeReuse/Finder
-
-    def fdw_geo_node
-      @fdw_geo_node ||= Geo::Fdw::GeoNode.find(current_node.id)
+    def job_artifacts
+      current_node.job_artifacts
     end
 
     def registries_for_job_artifacts
-      if use_legacy_queries_for_selective_sync?
-        legacy_finder.registries_for_job_artifacts
-      else
-        fdw_geo_node
-          .job_artifacts
-          .inner_join_job_artifact_registry
-          .syncable
-      end
-    end
-
-    def job_artifacts_synced
-      if use_legacy_queries_for_selective_sync?
-        legacy_finder.job_artifacts_synced
-      else
-        registries_for_job_artifacts.merge(Geo::JobArtifactRegistry.synced)
-      end
-    end
-
-    def job_artifacts_failed
-      if use_legacy_queries_for_selective_sync?
-        legacy_finder.job_artifacts_failed
-      else
-        registries_for_job_artifacts.merge(Geo::JobArtifactRegistry.failed)
-      end
-    end
-
-    def job_artifacts_synced_missing_on_primary
-      if use_legacy_queries_for_selective_sync?
-        legacy_finder.job_artifacts_synced_missing_on_primary
-      else
-        registries_for_job_artifacts.merge(Geo::JobArtifactRegistry.synced.missing_on_primary)
-      end
-    end
-
-    def job_artifacts_unsynced(except_artifact_ids:)
-      fdw_geo_node
-        .job_artifacts
-        .syncable
-        .missing_job_artifact_registry
-        .id_not_in(except_artifact_ids)
-    end
-
-    def job_artifacts_migrated_local(except_artifact_ids:)
-      fdw_geo_node
-        .job_artifacts
+      job_artifacts
         .inner_join_job_artifact_registry
-        .with_files_stored_remotely
-        .id_not_in(except_artifact_ids)
+        .syncable
     end
   end
 end

ee/app/finders/geo/legacy_job_artifact_registry_finder.rb

-# frozen_string_literal: true
-
-module Geo
-  class LegacyJobArtifactRegistryFinder < RegistryFinder
-    def syncable
-      current_node.job_artifacts.syncable
-    end
-
-    def job_artifacts_synced
-      legacy_inner_join_registry_ids(
-        syncable,
-        Geo::JobArtifactRegistry.synced.pluck_artifact_key,
-        Ci::JobArtifact
-      )
-    end
-
-    def job_artifacts_failed
-      legacy_inner_join_registry_ids(
-        syncable,
-        Geo::JobArtifactRegistry.failed.pluck_artifact_key,
-        Ci::JobArtifact
-      )
-    end
-
-    def job_artifacts_synced_missing_on_primary
-      legacy_inner_join_registry_ids(
-        syncable,
-        Geo::JobArtifactRegistry.synced.missing_on_primary.pluck_artifact_key,
-        Ci::JobArtifact
-      )
-    end
-
-    def job_artifacts_unsynced(except_artifact_ids: [])
-      registry_artifact_ids = Geo::JobArtifactRegistry.pluck_artifact_key | except_artifact_ids
-
-      legacy_left_outer_join_registry_ids(
-        syncable,
-        registry_artifact_ids,
-        Ci::JobArtifact
-      )
-    end
-
-    def job_artifacts_migrated_local(except_artifact_ids: [])
-      registry_artifact_ids = Geo::JobArtifactRegistry.pluck_artifact_key - except_artifact_ids
-
-      legacy_inner_join_registry_ids(
-        current_node.job_artifacts.with_files_stored_remotely,
-        registry_artifact_ids,
-        Ci::JobArtifact
-      )
-    end
-
-    def registries_for_job_artifacts
-      return Geo::JobArtifactRegistry.all unless selective_sync?
-
-      legacy_inner_join_registry_ids(
-        Geo::JobArtifactRegistry.all,
-        current_node.job_artifacts.pluck_primary_key,
-        Geo::JobArtifactRegistry,
-        foreign_key: :artifact_id
-      )
-    end
-  end
-end

ee/spec/finders/geo/job_artifact_registry_finder_spec.rb

 require 'spec_helper'
 
-describe Geo::JobArtifactRegistryFinder, :geo do
+describe Geo::JobArtifactRegistryFinder, :geo_fdw do
   include ::EE::GeoHelpers
 
   # Using let() instead of set() because set() does not work properly
@@ -563,5 +563,25 @@ describe Geo::JobArtifactRegistryFinder, :geo do
     end
   end
 
-  it_behaves_like 'a file registry finder'
+  it 'responds to file registry finder methods' do
+    file_registry_finder_methods = %i{
+      syncable
+      count_syncable
+      count_synced
+      count_failed
+      count_synced_missing_on_primary
+      count_registry
+      find_unsynced
+      find_migrated_local
+      find_retryable_failed_registries
+      find_retryable_synced_missing_on_primary_registries
+    }
+
+    file_registry_finder_methods.each do |method|
+      expect(subject).to respond_to(method)
+    end
+  end
+
+  include_examples 'counts all the things'
+  include_examples 'finds all the things'
 end