Refactor legacy finder to find registries that have been synced

These changes use the new finder when FDW is enabled without selective
sync to avoid code duplication.

ee/app/finders/geo/legacy_project_registry_synced_finder.rb

 # frozen_string_literal: true
 
 # Finder for retrieving project registries that have been synced
-# scoped to a type (repository or wiki) using cross-database joins
-# for selective sync.
+# scoped to a type (repository or wiki) using cross-database joins.
 #
 # Basic usage:
 #
@@ -22,30 +21,16 @@ module Geo
     end
 
     def execute
-      if selective_sync?
-        synced_registries_for_selective_sync
-      else
-        synced_registries
-      end
-    end
-
-    private
-
-    attr_reader :type
-
-    def synced_registries
-      Geo::ProjectRegistry.synced(type)
-    end
-
-    # rubocop: disable CodeReuse/ActiveRecord
-    def synced_registries_for_selective_sync
       legacy_inner_join_registry_ids(
-        synced_registries,
-        current_node.projects.pluck(:id),
+        Geo::ProjectRegistry.synced(type),
+        current_node.projects.pluck_primary_key,
         Geo::ProjectRegistry,
         foreign_key: :project_id
       )
     end
-    # rubocop: enable CodeReuse/ActiveRecord
+
+    private
+
+    attr_reader :type
   end
 end

ee/app/finders/geo/project_registry_finder.rb

@@ -168,10 +168,10 @@ module Geo
     end
 
     def finder_klass_for_synced_registries
-      if Gitlab::Geo::Fdw.enabled_for_selective_sync?
-        Geo::ProjectRegistrySyncedFinder
-      else
+      if !Gitlab::Geo::Fdw.enabled? || use_legacy_queries_for_selective_sync?
         Geo::LegacyProjectRegistrySyncedFinder
+      else
+        Geo::ProjectRegistrySyncedFinder
       end
     end
 

ee/spec/finders/geo/legacy_project_registry_synced_finder_spec.rb

@@ -22,84 +22,68 @@ describe Geo::LegacyProjectRegistrySyncedFinder, :geo do
     let!(:registry_repository_dirty_broken_shard) { create(:geo_project_registry, :synced, :repository_dirty, project: project_5) }
     let!(:registry_sync_failed) { create(:geo_project_registry, :sync_failed) }
 
-    shared_examples 'finds synced registries' do
-      context 'with repository type' do
-        subject { described_class.new(current_node: node, type: :repository) }
-
-        context 'without selective sync' do
-          it 'returns all synced registries' do
-            expect(subject.execute).to match_array([registry_synced, registry_wiki_dirty, registry_wiki_dirty_broken_shard])
-          end
-        end
+    before do
+      stub_fdw_disabled
+    end
 
-        context 'with selective sync by namespace' do
-          it 'returns synced registries where projects belongs to the namespaces' do
-            node.update!(selective_sync_type: 'namespaces', namespaces: [group_1, nested_group_1])
+    context 'with repository type' do
+      subject { described_class.new(current_node: node, type: :repository) }
 
-            expect(subject.execute).to match_array([registry_synced, registry_wiki_dirty])
-          end
+      context 'without selective sync' do
+        it 'returns all synced registries' do
+          expect(subject.execute).to match_array([registry_synced, registry_wiki_dirty, registry_wiki_dirty_broken_shard])
         end
+      end
 
-        context 'with selective sync by shard' do
-          it 'returns synced registries where projects belongs to the shards' do
-            node.update!(selective_sync_type: 'shards', selective_sync_shards: ['broken'])
+      context 'with selective sync by namespace' do
+        it 'returns synced registries where projects belongs to the namespaces' do
+          node.update!(selective_sync_type: 'namespaces', namespaces: [group_1, nested_group_1])
 
-            expect(subject.execute).to match_array([registry_wiki_dirty_broken_shard])
-          end
+          expect(subject.execute).to match_array([registry_synced, registry_wiki_dirty])
         end
       end
 
-      context 'with wiki type' do
-        subject { described_class.new(current_node: node, type: :wiki) }
+      context 'with selective sync by shard' do
+        it 'returns synced registries where projects belongs to the shards' do
+          node.update!(selective_sync_type: 'shards', selective_sync_shards: ['broken'])
 
-        context 'without selective sync' do
-          it 'returns all synced registries' do
-            expect(subject.execute).to match_array([registry_synced, registry_repository_dirty, registry_repository_dirty_broken_shard])
-          end
+          expect(subject.execute).to match_array([registry_wiki_dirty_broken_shard])
         end
+      end
+    end
 
-        context 'with selective sync by namespace' do
-          it 'returns synced registries where projects belongs to the namespaces' do
-            node.update!(selective_sync_type: 'namespaces', namespaces: [group_1, nested_group_1])
+    context 'with wiki type' do
+      subject { described_class.new(current_node: node, type: :wiki) }
 
-            expect(subject.execute).to match_array([registry_synced, registry_repository_dirty])
-          end
+      context 'without selective sync' do
+        it 'returns all synced registries' do
+          expect(subject.execute).to match_array([registry_synced, registry_repository_dirty, registry_repository_dirty_broken_shard])
         end
+      end
 
-        context 'with selective sync by shard' do
-          it 'returns synced registries where projects belongs to the shards' do
-            node.update!(selective_sync_type: 'shards', selective_sync_shards: ['broken'])
+      context 'with selective sync by namespace' do
+        it 'returns synced registries where projects belongs to the namespaces' do
+          node.update!(selective_sync_type: 'namespaces', namespaces: [group_1, nested_group_1])
 
-            expect(subject.execute).to match_array([registry_repository_dirty_broken_shard])
-          end
+          expect(subject.execute).to match_array([registry_synced, registry_repository_dirty])
         end
       end
 
-      context 'with invalid type' do
-        subject { described_class.new(current_node: node, type: :invalid) }
+      context 'with selective sync by shard' do
+        it 'returns synced registries where projects belongs to the shards' do
+          node.update!(selective_sync_type: 'shards', selective_sync_shards: ['broken'])
 
-        it 'returns nothing' do
-          expect(subject.execute).to be_empty
+          expect(subject.execute).to match_array([registry_repository_dirty_broken_shard])
         end
       end
     end
 
-    # Disable transactions via :delete method because a foreign table
-    # can't see changes inside a transaction of a different connection.
-    context 'FDW', :delete do
-      before do
-        skip('FDW is not configured') unless Gitlab::Geo::Fdw.enabled?
-      end
+    context 'with invalid type' do
+      subject { described_class.new(current_node: node, type: :invalid) }
 
-      include_examples 'finds synced registries'
-    end
-
-    context 'Legacy' do
-      before do
-        stub_fdw_disabled
+      it 'returns nothing' do
+        expect(subject.execute).to be_empty
       end
-
-      include_examples 'finds synced registries'
     end
   end
 end

ee/spec/finders/geo/project_registry_finder_spec.rb

@@ -28,27 +28,34 @@ describe Geo::ProjectRegistryFinder, :geo do
 
   shared_examples 'counts all the things' do |method_prefix|
     describe '#count_synced_repositories' do
-      it 'counts repositories that have been synced' do
-        create(:geo_project_registry, :sync_failed)
+      before do
+        project_1_in_synced_group = create(:project, group: synced_group)
+        project_2_in_synced_group = create(:project, group: synced_group)
+        project_3_in_synced_group = create(:project, group: synced_group)
+        project_4_broken_storage = create(:project, :broken_storage)
+
         create(:geo_project_registry, :synced, project: project_synced)
-        create(:geo_project_registry, :synced, :repository_dirty, project: project_repository_dirty)
-        create(:geo_project_registry, :synced, :wiki_dirty, project: project_wiki_dirty)
+        create(:geo_project_registry, :synced, :repository_dirty, project: project_1_in_synced_group)
+        create(:geo_project_registry, :synced, :wiki_dirty, project: project_2_in_synced_group)
+        create(:geo_project_registry, :sync_failed, project: project_3_in_synced_group)
+        create(:geo_project_registry, :synced, :wiki_dirty, project: project_4_broken_storage)
+      end
 
-        expect(subject.count_synced_repositories).to eq 2
+      it 'counts registries that repository have been synced' do
+        expect(subject.count_synced_repositories).to eq 3
       end
 
-      context 'with selective sync' do
-        before do
+      context 'with selective sync by namespace' do
+        it 'counts registries that repository have been synced where projects belongs to the namespaces' do
           secondary.update!(selective_sync_type: 'namespaces', namespaces: [synced_group])
-        end
 
-        it 'counts projects that has been synced' do
-          project_1_in_synced_group = create(:project, group: synced_group)
-          project_2_in_synced_group = create(:project, group: synced_group)
+          expect(subject.count_synced_repositories).to eq 1
+        end
+      end
 
-          create(:geo_project_registry, :synced, project: project_synced)
-          create(:geo_project_registry, :synced, project: project_1_in_synced_group)
-          create(:geo_project_registry, :sync_failed, project: project_2_in_synced_group)
+      context 'with selective sync by shard' do
+        it 'counts registries that repository have been synced where projects belongs to the shards' do
+          secondary.update!(selective_sync_type: 'shards', selective_sync_shards: ['broken'])
 
           expect(subject.count_synced_repositories).to eq 1
         end
@@ -56,35 +63,34 @@ describe Geo::ProjectRegistryFinder, :geo do
     end
 
     describe '#count_synced_wikis' do
-      it 'counts wiki that have been synced' do
-        create(:geo_project_registry, :sync_failed)
-        create(:geo_project_registry, :synced, project: project_synced)
-        create(:geo_project_registry, :synced, :repository_dirty, project: project_repository_dirty)
-        create(:geo_project_registry, :synced, :wiki_dirty, project: project_wiki_dirty)
-
-        expect(subject.count_synced_wikis).to eq 2
-      end
-
-      it 'counts synced wikis with nil wiki_access_level (which means enabled wiki)' do
-        project_synced.project_feature.update!(wiki_access_level: nil)
+      before do
+        project_1_in_synced_group = create(:project, group: synced_group)
+        project_2_in_synced_group = create(:project, group: synced_group)
+        project_3_in_synced_group = create(:project, group: synced_group)
+        project_4_broken_storage = create(:project, :broken_storage)
 
         create(:geo_project_registry, :synced, project: project_synced)
+        create(:geo_project_registry, :synced, :wiki_dirty, project: project_1_in_synced_group)
+        create(:geo_project_registry, :synced, :repository_dirty, project: project_2_in_synced_group)
+        create(:geo_project_registry, :sync_failed, project: project_3_in_synced_group)
+        create(:geo_project_registry, :synced, :repository_dirty, project: project_4_broken_storage)
+      end
 
-        expect(subject.count_synced_wikis).to eq 1
+      it 'counts registries that wiki have been synced' do
+        expect(subject.count_synced_wikis).to eq 3
       end
 
-      context 'with selective sync' do
-        before do
+      context 'with selective sync by namespace' do
+        it 'counts registries that wiki have been synced where projects belongs to the namespaces' do
           secondary.update!(selective_sync_type: 'namespaces', namespaces: [synced_group])
-        end
 
-        it 'counts projects that has been synced' do
-          project_1_in_synced_group = create(:project, group: synced_group)
-          project_2_in_synced_group = create(:project, group: synced_group)
+          expect(subject.count_synced_wikis).to eq 1
+        end
+      end
 
-          create(:geo_project_registry, :synced, project: project_synced)
-          create(:geo_project_registry, :synced, project: project_1_in_synced_group)
-          create(:geo_project_registry, :sync_failed, project: project_2_in_synced_group)
+      context 'with selective sync by shard' do
+        it 'counts registries that wiki have been synced where projects belongs to the shards' do
+          secondary.update!(selective_sync_type: 'shards', selective_sync_shards: ['broken'])
 
           expect(subject.count_synced_wikis).to eq 1
         end