Refactor find_never_synced_registries method

Refactor code to make the finder class
closer to a Geo replicator.

ee/app/finders/geo/project_registry_finder.rb

@@ -19,11 +19,9 @@ module Geo
     # @param [Integer] batch_size used to limit the results returned
     # @param [Array<Integer>] except_ids ids that will be ignored from the query
     # rubocop:disable CodeReuse/ActiveRecord
-    def find_never_synced_registries(batch_size:, except_ids: [])
+    def find_unsynced_registries(batch_size:, except_ids: [])
       registry_class
-        .never_synced
-        .model_id_not_in(except_ids)
-        .limit(batch_size)
+        .find_unsynced_registries(batch_size: batch_size, except_ids: except_ids)
     end
     # rubocop:enable CodeReuse/ActiveRecord
 

ee/app/finders/geo/registry_finder.rb

@@ -38,7 +38,7 @@ module Geo
       [untracked_ids, unused_tracked_ids]
     end
 
-    # @!method find_never_synced_registries
+    # @!method find_unsynced_registries
     #    Return an ActiveRecord::Relation of the registry records for the
     #    tracked ype that have never been synced.
     #
@@ -58,11 +58,9 @@ module Geo
     # @param [Array<Integer>] except_ids ids that will be ignored from the query
     #
     # rubocop:disable CodeReuse/ActiveRecord
-    def find_never_synced_registries(batch_size:, except_ids: [])
+    def find_unsynced_registries(batch_size:, except_ids: [])
       registry_class
-        .never_synced
-        .model_id_not_in(except_ids)
-        .limit(batch_size)
+        .find_unsynced_registries(batch_size: batch_size, except_ids: except_ids)
     end
     # rubocop:enable CodeReuse/ActiveRecord
 

ee/app/models/concerns/geo/syncable.rb

@@ -6,7 +6,7 @@ module Geo::Syncable
   included do
     scope :failed, -> { where(success: false).where.not(retry_count: nil) }
     scope :missing_on_primary, -> { where(missing_on_primary: true) }
-    scope :never_synced, -> { where(success: false, retry_count: nil) }
+    scope :pending, -> { where(success: false, retry_count: nil) }
     scope :retry_due, -> { where('retry_at is NULL OR retry_at < ?', Time.current) }
     scope :retryable, -> { failed.retry_due }
     scope :synced, -> { where(success: true) }

ee/app/models/geo/container_repository_registry.rb

@@ -8,11 +8,11 @@ class Geo::ContainerRepositoryRegistry < Geo::BaseRegistry
 
   belongs_to :container_repository
 
-  scope :never_synced, -> { with_state(:pending).where(last_synced_at: nil) }
   scope :failed, -> { with_state(:failed) }
-  scope :synced, -> { with_state(:synced) }
+  scope :pending, -> { with_state(:pending).where(last_synced_at: nil) }
   scope :retry_due, -> { where(arel_table[:retry_at].eq(nil).or(arel_table[:retry_at].lt(Time.current))) }
   scope :retryable, -> { failed.retry_due }
+  scope :synced, -> { with_state(:synced) }
 
   state_machine :state, initial: :pending do
     state :started

ee/app/models/geo/design_registry.rb

@@ -10,7 +10,7 @@ class Geo::DesignRegistry < Geo::BaseRegistry
 
   belongs_to :project
 
-  scope :never_synced, -> { with_state(:pending).where(last_synced_at: nil) }
+  scope :never, -> { with_state(:pending).where(last_synced_at: nil) }
   scope :pending, -> { with_state(:pending) }
   scope :failed, -> { with_state(:failed) }
   scope :synced, -> { with_state(:synced) }
@@ -59,6 +59,12 @@ class Geo::DesignRegistry < Geo::BaseRegistry
     finder_class.new(current_node_id: Gitlab::Geo.current_node.id).find_registry_differences(range)
   end
 
+  def self.find_unsynced_registries(batch_size:, except_ids: [])
+    never
+      .model_id_not_in(except_ids)
+      .limit(batch_size)
+  end
+
   def self.find_failed_registries(batch_size:, except_ids: [])
     super
       .order(Gitlab::Database.nulls_first_order(:last_synced_at))

ee/app/models/geo/project_registry.rb

@@ -23,7 +23,7 @@ class Geo::ProjectRegistry < Geo::BaseRegistry
 
   scope :dirty, -> { where(arel_table[:resync_repository].eq(true).or(arel_table[:resync_wiki].eq(true))) }
   scope :retryable, -> { dirty.retry_due }
-  scope :never_synced, -> { where(last_repository_synced_at: nil) }
+  scope :pending, -> { where(last_repository_synced_at: nil) }
   scope :synced_repos, -> { where(resync_repository: false) }
   scope :synced_wikis, -> { where(resync_wiki: false) }
   scope :failed_repos, -> { where(arel_table[:repository_retry_count].gt(0)) }

ee/app/models/geo/upload_registry.rb

@@ -48,11 +48,8 @@ class Geo::UploadRegistry < Geo::BaseRegistry
 
   def self.with_status(status)
     case status
-    when 'synced', 'failed'
+    when 'synced', 'failed', 'pending'
       self.public_send(status) # rubocop: disable GitlabSecurity/PublicSend
-    # Explained via: https://gitlab.com/gitlab-org/gitlab/-/issues/216049
-    when 'pending'
-      self.never_synced
     else
       all
     end

ee/app/workers/geo/container_repository_sync_dispatch_worker.rb

@@ -48,7 +48,7 @@ module Geo
 
     def find_container_repository_ids_not_synced(batch_size:)
       registry_finder
-        .find_never_synced_registries(batch_size: batch_size, except_ids: scheduled_repository_ids)
+        .find_unsynced_registries(batch_size: batch_size, except_ids: scheduled_repository_ids)
         .pluck_model_foreign_key
     end
 

ee/app/workers/geo/design_repository_shard_sync_worker.rb

@@ -14,7 +14,7 @@ module Geo
     def find_project_ids_not_synced(except_ids:, batch_size:)
       project_ids =
         registry_finder
-          .find_never_synced_registries(batch_size: batch_size, except_ids: except_ids)
+          .find_unsynced_registries(batch_size: batch_size, except_ids: except_ids)
           .pluck_model_foreign_key
 
       find_project_ids_within_shard(project_ids, direction: :desc)

ee/app/workers/geo/file_download_dispatch_worker/job_finder.rb

@@ -23,7 +23,7 @@ module Geo
 
       def find_unsynced_jobs(batch_size:)
         convert_registry_relation_to_job_args(
-          registry_finder.find_never_synced_registries(find_batch_params(batch_size))
+          registry_finder.find_unsynced_registries(find_batch_params(batch_size))
         )
       end
 

ee/app/workers/geo/repository_shard_sync_worker.rb

@@ -78,7 +78,7 @@ module Geo
     def find_project_ids_not_synced(except_ids:, batch_size:)
       project_ids =
         registry_finder
-          .find_never_synced_registries(batch_size: batch_size, except_ids: except_ids)
+          .find_unsynced_registries(batch_size: batch_size, except_ids: except_ids)
           .pluck_model_foreign_key
 
       find_project_ids_within_shard(project_ids, direction: :desc)

ee/spec/finders/geo/attachment_registry_finder_spec.rb

@@ -121,7 +121,7 @@ RSpec.describe Geo::AttachmentRegistryFinder, :geo do
     end
   end
 
-  describe '#find_never_synced_registries' do
+  describe '#find_unsynced_registries' do
     it 'returns registries for uploads that have never been synced' do
       create(:geo_upload_registry, :attachment, :failed, file_id: upload_1.id)
       create(:geo_upload_registry, :attachment, file_id: upload_2.id, missing_on_primary: true)
@@ -132,7 +132,7 @@ RSpec.describe Geo::AttachmentRegistryFinder, :geo do
       create(:geo_upload_registry, :attachment, file_id: upload_7.id, missing_on_primary: true)
       registry_upload_8 = create(:geo_upload_registry, :attachment, :never_synced, file_id: upload_8.id)
 
-      registries = subject.find_never_synced_registries(batch_size: 10)
+      registries = subject.find_unsynced_registries(batch_size: 10)
 
       expect(registries).to match_ids(registry_upload_3, registry_upload_8)
     end
@@ -147,7 +147,7 @@ RSpec.describe Geo::AttachmentRegistryFinder, :geo do
       create(:geo_upload_registry, :attachment, file_id: upload_7.id, missing_on_primary: true)
       registry_upload_8 = create(:geo_upload_registry, :attachment, :never_synced, file_id: upload_8.id)
 
-      registries = subject.find_never_synced_registries(batch_size: 10, except_ids: [upload_3.id])
+      registries = subject.find_unsynced_registries(batch_size: 10, except_ids: [upload_3.id])
 
       expect(registries).to match_ids(registry_upload_8)
     end

ee/spec/finders/geo/container_repository_registry_finder_spec.rb

@@ -185,7 +185,7 @@ RSpec.describe Geo::ContainerRepositoryRegistryFinder, :geo do
     end
   end
 
-  describe '#find_never_synced_registries' do
+  describe '#find_unsynced_registries' do
     let_it_be(:registry_container_registry_1) { create(:container_repository_registry, :synced, container_repository_id: container_repository_1.id) }
     let_it_be(:registry_container_registry_2) { create(:container_repository_registry, :sync_failed, container_repository_id: container_repository_2.id) }
     let_it_be(:registry_container_registry_3) { create(:container_repository_registry, container_repository_id: container_repository_3.id, last_synced_at: nil) }
@@ -194,13 +194,13 @@ RSpec.describe Geo::ContainerRepositoryRegistryFinder, :geo do
     let_it_be(:registry_container_registry_6) { create(:container_repository_registry, container_repository_id: container_repository_6.id, last_synced_at: nil) }
 
     it 'returns registries for projects that have never been synced' do
-      registries = subject.find_never_synced_registries(batch_size: 10)
+      registries = subject.find_unsynced_registries(batch_size: 10)
 
       expect(registries).to match_ids(registry_container_registry_3, registry_container_registry_6)
     end
 
     it 'excludes except_ids' do
-      registries = subject.find_never_synced_registries(batch_size: 10, except_ids: [container_repository_3.id])
+      registries = subject.find_unsynced_registries(batch_size: 10, except_ids: [container_repository_3.id])
 
       expect(registries).to match_ids(registry_container_registry_6)
     end

ee/spec/finders/geo/design_registry_finder_spec.rb

@@ -199,7 +199,7 @@ RSpec.describe Geo::DesignRegistryFinder, :geo do
     end
   end
 
-  describe '#find_never_synced_registries' do
+  describe '#find_unsynced_registries' do
     let!(:registry_project_1) { create(:geo_design_registry, :synced, project_id: project_1.id) }
     let!(:registry_project_2) { create(:geo_design_registry, :sync_failed, project_id: project_2.id) }
     let!(:registry_project_3) { create(:geo_design_registry, project_id: project_3.id, last_synced_at: nil) }
@@ -208,13 +208,13 @@ RSpec.describe Geo::DesignRegistryFinder, :geo do
     let!(:registry_project_6) { create(:geo_design_registry, project_id: project_6.id, last_synced_at: nil) }
 
     it 'returns registries for projects that have never been synced' do
-      registries = subject.find_never_synced_registries(batch_size: 10)
+      registries = subject.find_unsynced_registries(batch_size: 10)
 
       expect(registries).to match_ids(registry_project_3, registry_project_6)
     end
 
     it 'excludes except_ids' do
-      registries = subject.find_never_synced_registries(batch_size: 10, except_ids: [project_3.id])
+      registries = subject.find_unsynced_registries(batch_size: 10, except_ids: [project_3.id])
 
       expect(registries).to match_ids(registry_project_6)
     end

ee/spec/finders/geo/job_artifact_registry_finder_spec.rb

@@ -306,7 +306,7 @@ RSpec.describe Geo::JobArtifactRegistryFinder, :geo do
     end
   end
 
-  describe '#find_never_synced_registries' do
+  describe '#find_unsynced_registries' do
     it 'returns registries for job artifacts that have never been synced' do
       create(:geo_job_artifact_registry, :failed, artifact_id: ci_job_artifact_1.id)
       create(:geo_job_artifact_registry, artifact_id: ci_job_artifact_2.id, missing_on_primary: true)
@@ -317,7 +317,7 @@ RSpec.describe Geo::JobArtifactRegistryFinder, :geo do
       create(:geo_job_artifact_registry, artifact_id: ci_job_artifact_remote_2.id, missing_on_primary: true)
       registry_ci_job_artifact_remote_3 = create(:geo_job_artifact_registry, :never_synced, artifact_id: ci_job_artifact_remote_3.id)
 
-      registries = subject.find_never_synced_registries(batch_size: 10)
+      registries = subject.find_unsynced_registries(batch_size: 10)
 
       expect(registries).to match_ids(registry_ci_job_artifact_3, registry_ci_job_artifact_remote_3)
     end
@@ -332,7 +332,7 @@ RSpec.describe Geo::JobArtifactRegistryFinder, :geo do
       create(:geo_job_artifact_registry, artifact_id: ci_job_artifact_remote_2.id, missing_on_primary: true)
       registry_ci_job_artifact_remote_3 = create(:geo_job_artifact_registry, :never_synced, artifact_id: ci_job_artifact_remote_3.id)
 
-      registries = subject.find_never_synced_registries(batch_size: 10, except_ids: [ci_job_artifact_3.id])
+      registries = subject.find_unsynced_registries(batch_size: 10, except_ids: [ci_job_artifact_3.id])
 
       expect(registries).to match_ids(registry_ci_job_artifact_remote_3)
     end

ee/spec/finders/geo/lfs_object_registry_finder_spec.rb

@@ -253,7 +253,7 @@ RSpec.describe Geo::LfsObjectRegistryFinder, :geo do
     end
   end
 
-  describe '#find_never_synced_registries' do
+  describe '#find_unsynced_registries' do
     it 'returns registries for LFS objects that have never been synced' do
       create(:geo_lfs_object_registry, :failed, lfs_object_id: lfs_object_1.id)
       create(:geo_lfs_object_registry, lfs_object_id: lfs_object_2.id, missing_on_primary: true)
@@ -264,7 +264,7 @@ RSpec.describe Geo::LfsObjectRegistryFinder, :geo do
       create(:geo_lfs_object_registry, lfs_object_id: lfs_object_remote_2.id, missing_on_primary: true)
       registry_lfs_object_remote_3 = create(:geo_lfs_object_registry, :never_synced, lfs_object_id: lfs_object_remote_3.id)
 
-      registries = subject.find_never_synced_registries(batch_size: 10)
+      registries = subject.find_unsynced_registries(batch_size: 10)
 
       expect(registries).to match_ids(registry_lfs_object_3, registry_lfs_object_remote_3)
     end
@@ -279,7 +279,7 @@ RSpec.describe Geo::LfsObjectRegistryFinder, :geo do
       create(:geo_lfs_object_registry, lfs_object_id: lfs_object_remote_2.id, missing_on_primary: true)
       registry_lfs_object_remote_3 = create(:geo_lfs_object_registry, :never_synced, lfs_object_id: lfs_object_remote_3.id)
 
-      registries = subject.find_never_synced_registries(batch_size: 10, except_ids: [lfs_object_3.id])
+      registries = subject.find_unsynced_registries(batch_size: 10, except_ids: [lfs_object_3.id])
 
       expect(registries).to match_ids(registry_lfs_object_remote_3)
     end

ee/spec/finders/geo/project_registry_finder_spec.rb

@@ -17,15 +17,15 @@ RSpec.describe Geo::ProjectRegistryFinder, :geo do
   let_it_be(:registry_project_5) { create(:geo_project_registry, :wiki_dirty, project_id: project_5.id, last_repository_synced_at: 5.days.ago) }
   let_it_be(:registry_project_6) { create(:geo_project_registry, project_id: project_6.id) }
 
-  describe '#find_never_synced_registries' do
+  describe '#find_unsynced_registries' do
     it 'returns registries for projects that have never been synced' do
-      registries = subject.find_never_synced_registries(batch_size: 10)
+      registries = subject.find_unsynced_registries(batch_size: 10)
 
       expect(registries).to match_ids(registry_project_3, registry_project_6)
     end
 
     it 'excludes except_ids' do
-      registries = subject.find_never_synced_registries(batch_size: 10, except_ids: [project_3.id])
+      registries = subject.find_unsynced_registries(batch_size: 10, except_ids: [project_3.id])
 
       expect(registries).to match_ids(registry_project_6)
     end

ee/spec/models/geo/upload_registry_spec.rb

@@ -3,8 +3,6 @@
 require 'spec_helper'
 
 RSpec.describe Geo::UploadRegistry, :geo do
-  include EE::GeoHelpers
-
   let!(:failed) { create(:geo_upload_registry, :failed) }
   let!(:synced) { create(:geo_upload_registry) }
 
@@ -40,11 +38,11 @@ RSpec.describe Geo::UploadRegistry, :geo do
     end
   end
 
-  describe '.never' do
+  describe '.pending' do
     it 'returns registries that are never synced' do
-      never = create(:geo_upload_registry, retry_count: nil, success: false)
+      pending = create(:geo_upload_registry, retry_count: nil, success: false)
 
-      expect(described_class.never).to match_ids([never])
+      expect(described_class.pending).to match_ids([pending])
     end
   end
 
@@ -55,12 +53,12 @@ RSpec.describe Geo::UploadRegistry, :geo do
       described_class.with_status('synced')
     end
 
-    # Explained via: https://gitlab.com/gitlab-org/gitlab/-/issues/216049
-    it 'finds the registries with status "never" when filter is set to "pending"' do
-      expect(described_class).to receive(:never)
+    it 'finds the registries with status "pending" when filter is set to "pending"' do
+      expect(described_class).to receive(:pending)
 
       described_class.with_status('pending')
     end
+
     it 'finds the registries with status "failed"' do
       expect(described_class).to receive(:failed)