Address review comments

Extract some logic to DesignUnsyncFinder

ee/app/finders/geo/design_unsynced_finder.rb

 # frozen_string_literal: true
-
-# Finder for retrieving unsynced designs that belong to a specific
-# shard using FDW queries.
-#
-# Basic usage:
 #
-#     Geo::DesignUnsyncedFinder
-#       .new(current_node: Gitlab::Geo.current_node, shard_name: 'default', batch_size: 1000)
-#       .execute.
+# rubocop:disable CodeReuse/ActiveRecord
+
 module Geo
+  # Finder for retrieving unsynced designs that belong to a specific
+  # shard using FDW queries.
+  #
+  # Basic usage:
+  #
+  #     Geo::DesignUnsyncedFinder
+  #       .new(shard_name: 'default', batch_size: 1000)
+  #       .execute.
   class DesignUnsyncedFinder
-    def initialize(current_node:, shard_name:, batch_size: nil)
-      @current_node = Geo::Fdw::GeoNode.find(current_node.id)
+    def initialize(scheduled_project_ids: [], shard_name:, batch_size: nil)
+      @current_node = Geo::Fdw::GeoNode.find(Gitlab::Geo.current_node.id)
+      @scheduled_project_ids = scheduled_project_ids
       @shard_name = shard_name
       @batch_size = batch_size
     end
 
-    # rubocop:disable CodeReuse/ActiveRecord
     def execute
       return Geo::Fdw::Project.none unless valid_shard?
 
-      relation = projects.with_designs.missing_design_registry.within_shards(shard_name)
+      relation = projects
+        .with_designs
+        .missing_design_registry
+        .within_shards(shard_name)
+        .id_not_in(scheduled_project_ids)
+        .reorder(last_repository_updated_at: :desc)
       relation = relation.limit(batch_size) unless batch_size.nil?
-      relation
+      relation.pluck_primary_key
     end
-    # rubocop:enable CodeReuse/ActiveRecord
 
     private
 
-    attr_reader :current_node, :shard_name, :batch_size
+    attr_reader :scheduled_project_ids, :current_node, :shard_name, :batch_size
 
     def projects
       return Geo::Fdw::Project.all if current_node.selective_sync_by_shards?

ee/app/finders/geo/design_updated_recently_finder.rb

 # frozen_string_literal: true
 
-# Finder for retrieving designs updated recently that belong to a specific
-# shard using FDW queries.
-#
-# Basic usage:
-#
-#     Geo::DesignUpdatedRecentlyFinder
-#       .new(current_node: Gitlab::Geo.current_node, shard_name: 'default', batch_size: 1000)
-#       .execute.
 module Geo
+  # Finder for retrieving designs updated recently that belong to a specific
+  # shard using FDW queries.
+  #
+  # Basic usage:
+  #
+  #     Geo::DesignUpdatedRecentlyFinder
+  #       .new(shard_name: 'default', batch_size: 1000)
+  #       .execute.
   class DesignUpdatedRecentlyFinder
-    def initialize(current_node:, shard_name:, batch_size: nil)
-      @current_node = Geo::Fdw::GeoNode.find(current_node.id)
+    def initialize(scheduled_project_ids: [], shard_name:, batch_size: nil)
+      @current_node = Geo::Fdw::GeoNode.find(Gitlab::Geo.current_node.id)
+      @scheduled_project_ids = scheduled_project_ids
       @shard_name = shard_name
       @batch_size = batch_size
     end
@@ -20,15 +21,20 @@ module Geo
     def execute
       return Geo::Fdw::Project.none unless valid_shard?
 
-      relation = projects.with_designs.recently_updated_designs.within_shards(shard_name)
+      relation = projects
+        .with_designs
+        .recently_updated_designs
+        .within_shards(shard_name)
+        .id_not_in(scheduled_project_ids)
+        .order('design_registry.last_synced_at ASC NULLS FIRST')
       relation = relation.limit(batch_size) unless batch_size.nil?
-      relation
+      relation.pluck_primary_key
     end
     # rubocop:enable CodeReuse/ActiveRecord
 
     private
 
-    attr_reader :current_node, :shard_name, :batch_size
+    attr_reader :scheduled_project_ids, :current_node, :shard_name, :batch_size
 
     def projects
       return Geo::Fdw::Project.all if current_node.selective_sync_by_shards?

ee/app/workers/geo/design_repository_shard_sync_worker.rb

@@ -10,33 +10,15 @@ module Geo
       { project_id: project_id, job_id: job_id } if job_id
     end
 
-    # rubocop: disable CodeReuse/ActiveRecord
     def find_project_ids_not_synced(batch_size:)
-      find_unsynced_projects(batch_size: batch_size)
-        .id_not_in(scheduled_project_ids)
-        .reorder(last_repository_updated_at: :desc)
-        .pluck_primary_key
-    end
-    # rubocop: enable CodeReuse/ActiveRecord
-
-    def find_unsynced_projects(batch_size:)
       Geo::DesignUnsyncedFinder
-        .new(current_node: current_node, shard_name: shard_name, batch_size: batch_size)
+        .new(scheduled_project_ids: scheduled_project_ids, shard_name: shard_name, batch_size: batch_size)
         .execute
     end
 
-    # rubocop: disable CodeReuse/ActiveRecord
     def find_project_ids_updated_recently(batch_size:)
-      find_projects_updated_recently(batch_size: batch_size)
-        .id_not_in(scheduled_project_ids)
-        .order('design_registry.last_synced_at ASC NULLS FIRST')
-        .pluck_primary_key
-    end
-    # rubocop: enable CodeReuse/ActiveRecord
-
-    def find_projects_updated_recently(batch_size:)
       Geo::DesignUpdatedRecentlyFinder
-        .new(current_node: current_node, shard_name: shard_name, batch_size: batch_size)
+        .new(scheduled_project_ids: scheduled_project_ids, shard_name: shard_name, batch_size: batch_size)
         .execute
     end
   end

ee/app/workers/geo/repository_sync_worker.rb

@@ -9,9 +9,7 @@ module Geo
         Geo::RepositoryShardSyncWorker.perform_async(shard_name)
       end
 
-      if Feature.enabled?(:enable_geo_design_sync)
       Geo::DesignRepositoryShardSyncWorker.perform_async(shard_name)
     end
   end
-  end
 end

ee/spec/finders/geo/design_unsynced_finder_spec.rb

@@ -3,6 +3,8 @@
 require 'spec_helper'
 
 describe Geo::DesignUnsyncedFinder, :geo, :geo_fdw do
+  include EE::GeoHelpers
+
   describe '#execute' do
     let(:node) { create(:geo_node) }
     let(:group_1) { create(:group) }
@@ -19,15 +21,17 @@ describe Geo::DesignUnsyncedFinder, :geo, :geo_fdw do
       create(:design, project: project_2)
       create(:design, project: project_3)
       create(:design, project: project_4)
+
+      stub_current_geo_node(node)
     end
 
-    subject { described_class.new(current_node: node, shard_name: 'default', batch_size: 100) }
+    subject { described_class.new(shard_name: 'default', batch_size: 100) }
 
     context 'without selective sync' do
       it 'returns designs without an entry on the tracking database' do
         create(:geo_design_registry, :synced, project: project_2)
 
-        expect(subject.execute).to match_ids(project_1, project_3)
+        expect(subject.execute).to match_array([project_1.id, project_3.id])
       end
     end
 
@@ -37,7 +41,7 @@ describe Geo::DesignUnsyncedFinder, :geo, :geo_fdw do
 
         node.update!(selective_sync_type: 'namespaces', namespaces: [group_1, nested_group_1])
 
-        expect(subject.execute).to match_ids(project_1, project_2)
+        expect(subject.execute).to match_array([project_1.id, project_2.id])
       end
     end
 
@@ -47,7 +51,7 @@ describe Geo::DesignUnsyncedFinder, :geo, :geo_fdw do
       end
 
       it 'does not return designs out of synced shards' do
-        subject = described_class.new(current_node: node, shard_name: 'default', batch_size: 100)
+        subject = described_class.new(shard_name: 'default', batch_size: 100)
 
         expect(subject.execute).to be_empty
       end
@@ -58,9 +62,9 @@ describe Geo::DesignUnsyncedFinder, :geo, :geo_fdw do
         create(:design, project: project_5)
         create(:geo_design_registry, :synced, project: project_4)
 
-        subject = described_class.new(current_node: node, shard_name: 'foo', batch_size: 100)
+        subject = described_class.new(shard_name: 'foo', batch_size: 100)
 
-        expect(subject.execute).to match_ids(project_5)
+        expect(subject.execute).to match_array([project_5.id])
       end
     end
   end

ee/spec/finders/geo/design_updated_recently_finder_spec.rb

@@ -3,6 +3,8 @@
 require 'spec_helper'
 
 describe Geo::DesignUpdatedRecentlyFinder, :geo, :geo_fdw do
+  include EE::GeoHelpers
+
   describe '#execute' do
     let(:node) { create(:geo_node) }
     let(:group_1) { create(:group) }
@@ -25,13 +27,15 @@ describe Geo::DesignUpdatedRecentlyFinder, :geo, :geo_fdw do
       create(:design, project: project_2)
       create(:design, project: project_3)
       create(:design, project: project_4)
+
+      stub_current_geo_node(node)
     end
 
-    subject { described_class.new(current_node: node, shard_name: 'default', batch_size: 100) }
+    subject { described_class.new(shard_name: 'default', batch_size: 100) }
 
     context 'without selective sync' do
       it 'returns desings with a dirty entry on the tracking database' do
-        expect(subject.execute).to match_ids(project_1, project_2)
+        expect(subject.execute).to match_array([project_1.id, project_2.id])
       end
     end
 
@@ -39,7 +43,7 @@ describe Geo::DesignUpdatedRecentlyFinder, :geo, :geo_fdw do
       it 'returns designs that belong to the namespaces with a dirty entry on the tracking database' do
         node.update!(selective_sync_type: 'namespaces', namespaces: [group_1])
 
-        expect(subject.execute).to match_ids(project_1, project_2)
+        expect(subject.execute).to match_array([project_1.id, project_2.id])
       end
     end
 
@@ -49,7 +53,7 @@ describe Geo::DesignUpdatedRecentlyFinder, :geo, :geo_fdw do
       end
 
       it 'does not return designs out of selected shard' do
-        subject = described_class.new(current_node: node, shard_name: 'default', batch_size: 100)
+        subject = described_class.new(shard_name: 'default', batch_size: 100)
 
         expect(subject.execute).to be_empty
       end
@@ -60,9 +64,9 @@ describe Geo::DesignUpdatedRecentlyFinder, :geo, :geo_fdw do
         create(:design, project: project_5)
         create(:geo_design_registry, :synced, project: project_5)
 
-        subject = described_class.new(current_node: node, shard_name: 'foo', batch_size: 100)
+        subject = described_class.new(shard_name: 'foo', batch_size: 100)
 
-        expect(subject.execute).to match_ids(project_4)
+        expect(subject.execute).to match_array([project_4.id])
       end
     end
   end

ee/spec/workers/geo/repository_sync_worker_spec.rb

@@ -108,23 +108,6 @@ describe Geo::RepositorySyncWorker, :geo, :clean_gitlab_redis_cache do
     include_examples '#perform', Geo::RepositoryShardSyncWorker
   end
 
-  context 'when enable_geo_design_sync flag is disabled' do
-    before do
-      stub_feature_flags(enable_geo_design_sync: false)
-    end
-
-    it 'skips calling Geo::DesignRepositoryShardSyncWorker' do
-      # Report that default shard is healthy
-      expect(Gitlab::HealthChecks::GitalyCheck).to receive(:readiness)
-        .and_return([result(true, healthy_shard_name)])
-
-      expect(Geo::Secondary::RepositoryBackfillWorker).to receive(:perform_async).with('default')
-      expect(design_worker).not_to receive(:perform_async).with('default')
-
-      subject.perform
-    end
-  end
-
   def result(success, shard)
     Gitlab::HealthChecks::Result.new('gitaly_check', success, nil, { shard: shard })
   end