Refactoring Geo::ProjectRegistryFinder

app/models/geo_node_status.rb

@@ -72,8 +72,8 @@ class GeoNodeStatus < ActiveRecord::Base
       self.db_replication_lag_seconds = Gitlab::Geo::HealthCheck.db_replication_lag_seconds
       self.cursor_last_event_id = Geo::EventLogState.last_processed&.event_id
       self.cursor_last_event_date = Geo::EventLog.find_by(id: self.cursor_last_event_id)&.created_at
-      self.repositories_synced_count = projects_finder.count_synced_projects
-      self.repositories_failed_count = projects_finder.count_failed_projects
+      self.repositories_synced_count = projects_finder.count_synced_project_registries
+      self.repositories_failed_count = projects_finder.count_failed_project_registries
       self.lfs_objects_synced_count = lfs_objects_finder.count_synced_lfs_objects
       self.lfs_objects_failed_count = lfs_objects_finder.count_failed_lfs_objects
       self.attachments_synced_count = attachments_finder.find_synced_attachments.count

ee/app/finders/geo/project_registry_finder.rb

 module Geo
   class ProjectRegistryFinder < RegistryFinder
-    def count_synced_projects
+    def count_synced_project_registries
       relation =
         if selective_sync?
-          legacy_find_synced_projects
+          legacy_find_synced_project_registries
         else
-          find_synced_projects_registries
+          find_synced_project_registries
         end
 
       relation.count
     end
 
-    def count_failed_projects
-      relation =
-        if selective_sync?
-          legacy_find_failed_projects
-        else
-          find_failed_projects_registries
-        end
-
-      relation.count
+    def count_failed_project_registries
+      find_failed_project_registries.count
     end
 
     def find_failed_project_registries(type = nil)
       relation =
         if selective_sync?
-          legacy_find_failed_project_registries(type)
+          legacy_find_filtered_failed_project_registries(type)
         else
-          find_failed_projects_registries(type)
+          find_filtered_failed_project_registries(type)
         end
 
       relation
@@ -57,11 +50,11 @@ module Geo
 
     protected
 
-    def find_synced_projects_registries
+    def find_synced_project_registries
       Geo::ProjectRegistry.synced
     end
 
-    def find_failed_projects_registries(type = nil)
+    def find_filtered_failed_project_registries(type = nil)
       case type
       when 'repository'
         Geo::ProjectRegistry.failed_repos
@@ -117,16 +110,18 @@ module Geo
       legacy_find_projects(Geo::ProjectRegistry.dirty.retry_due.pluck(:project_id))
     end
 
-    # @return [ActiveRecord::Relation<Project>] list of synced projects
-    def legacy_find_synced_projects
-      legacy_find_projects(Geo::ProjectRegistry.synced.pluck(:project_id))
+    # @return [ActiveRecord::Relation<Geo::ProjectRegistry>] list of synced projects
+    def legacy_find_synced_project_registries
+      legacy_find_project_registries(Geo::ProjectRegistry.synced)
     end
 
-    # @return [ActiveRecord::Relation<Project>] list of projects that sync has failed
-    def legacy_find_failed_projects
-      legacy_find_projects(Geo::ProjectRegistry.failed.pluck(:project_id))
+    # @return [ActiveRecord::Relation<Geo::ProjectRegistry>] list of projects that sync has failed
+    def legacy_find_filtered_failed_project_registries(type = nil)
+      project_registries = find_filtered_failed_project_registries(type)
+      legacy_find_project_registries(project_registries)
     end
 
+    # @return [ActiveRecord::Relation<Project>]
     def legacy_find_projects(registry_project_ids)
       return Project.none if registry_project_ids.empty?
 
@@ -140,15 +135,15 @@ module Geo
       joined_relation
     end
 
-    def legacy_find_failed_project_registries(type)
-      project_registries = find_failed_projects_registries(type)
+    # @return [ActiveRecord::Relation<Geo::ProjectRegistry>]
+    def legacy_find_project_registries(project_registries)
       return Geo::ProjectRegistry.none if project_registries.empty?
 
       joined_relation = project_registries.joins(<<~SQL)
         INNER JOIN
         (VALUES #{current_node.projects.pluck(:id).map { |id| "(#{id})" }.join(',')})
-        projects(project_id)
-        ON #{Geo::ProjectRegistry.table_name}.id = projects.project_id
+        projects(id)
+        ON #{Geo::ProjectRegistry.table_name}.project_id = projects.id
       SQL
 
       joined_relation

spec/ee/spec/finders/geo/project_registry_finder_spec.rb

@@ -18,11 +18,11 @@ describe Geo::ProjectRegistryFinder, :geo, :truncate do
     stub_current_geo_node(secondary)
   end
 
-  describe '#count_synced_projects' do
-    it 'delegates to #find_synced_projects_registries' do
-      expect(subject).to receive(:find_synced_projects_registries).and_call_original
+  describe '#count_synced_project_registries' do
+    it 'delegates to #find_synced_project_registries' do
+      expect(subject).to receive(:find_synced_project_registries).and_call_original
 
-      subject.count_synced_projects
+      subject.count_synced_project_registries
     end
 
     it 'counts projects that has been synced' do
@@ -31,7 +31,7 @@ describe Geo::ProjectRegistryFinder, :geo, :truncate do
       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_projects).to eq 1
+      expect(subject.count_synced_project_registries).to eq 1
     end
 
     context 'with selective sync' do
@@ -39,10 +39,10 @@ describe Geo::ProjectRegistryFinder, :geo, :truncate do
         secondary.update_attribute(:namespaces, [synced_group])
       end
 
-      it 'delegates to #legacy_find_synced_projects' do
-        expect(subject).to receive(:legacy_find_synced_projects).and_call_original
+      it 'delegates to #legacy_find_synced_project_registries' do
+        expect(subject).to receive(:legacy_find_synced_project_registries).and_call_original
 
-        subject.count_synced_projects
+        subject.count_synced_project_registries
       end
 
       it 'counts projects that has been synced' do
@@ -53,16 +53,16 @@ describe Geo::ProjectRegistryFinder, :geo, :truncate do
         create(:geo_project_registry, :synced, project: project_1_in_synced_group)
         create(:geo_project_registry, :sync_failed, project: project_2_in_synced_group)
 
-        expect(subject.count_synced_projects).to eq 1
+        expect(subject.count_synced_project_registries).to eq 1
       end
     end
   end
 
-  describe '#count_failed_projects' do
-    it 'delegates to #find_failed_projects_registries' do
-      expect(subject).to receive(:find_failed_projects_registries).and_call_original
+  describe '#count_failed_project_registries' do
+    it 'delegates to #find_failed_project_registries' do
+      expect(subject).to receive(:find_failed_project_registries).and_call_original
 
-      subject.count_failed_projects
+      subject.count_failed_project_registries
     end
 
     it 'counts projects that sync has failed' do
@@ -71,7 +71,7 @@ describe Geo::ProjectRegistryFinder, :geo, :truncate do
       create(:geo_project_registry, :repository_sync_failed, project: project_repository_dirty)
       create(:geo_project_registry, :wiki_sync_failed, project: project_wiki_dirty)
 
-      expect(subject.count_failed_projects).to eq 3
+      expect(subject.count_failed_project_registries).to eq 3
     end
 
     context 'with selective sync' do
@@ -79,10 +79,10 @@ describe Geo::ProjectRegistryFinder, :geo, :truncate do
         secondary.update_attribute(:namespaces, [synced_group])
       end
 
-      it 'delegates to #legacy_find_failed_projects' do
-        expect(subject).to receive(:legacy_find_failed_projects).and_call_original
+      it 'delegates to #find_failed_project_registries' do
+        expect(subject).to receive(:find_failed_project_registries).and_call_original
 
-        subject.count_failed_projects
+        subject.count_failed_project_registries
       end
 
       it 'counts projects that sync has failed' do
@@ -93,7 +93,63 @@ describe Geo::ProjectRegistryFinder, :geo, :truncate do
         create(:geo_project_registry, :repository_sync_failed, project: project_1_in_synced_group)
         create(:geo_project_registry, :synced, project: project_2_in_synced_group)
 
-        expect(subject.count_failed_projects).to eq 1
+        expect(subject.count_failed_project_registries).to eq 1
+      end
+    end
+  end
+
+  describe '#find_failed_project_registries' do
+    let(:project_1_in_synced_group) { create(:project, group: synced_group) }
+    let(:project_2_in_synced_group) { create(:project, group: synced_group) }
+
+    let!(:synced) { create(:geo_project_registry, :synced) }
+    let!(:sync_failed) { create(:geo_project_registry, :sync_failed, project: project_synced) }
+    let!(:repository_sync_failed) { create(:geo_project_registry, :repository_sync_failed, project: project_1_in_synced_group) }
+    let!(:wiki_sync_failed) { create(:geo_project_registry, :wiki_sync_failed, project: project_2_in_synced_group) }
+
+    it 'delegates to #find_filtered_failed_project_registries' do
+      expect(subject).to receive(:find_filtered_failed_project_registries).and_call_original
+
+      subject.find_failed_project_registries
+    end
+
+    it 'returns project registries that sync has failed' do
+      expect(subject.find_failed_project_registries).to match_array([sync_failed, repository_sync_failed, wiki_sync_failed])
+    end
+
+    it 'returns only project registries that repository sync has failed' do
+      expect(subject.find_failed_project_registries('repository')).to match_array([sync_failed, repository_sync_failed])
+    end
+
+    it 'returns only project registries that wiki sync has failed' do
+      expect(subject.find_failed_project_registries('wiki')).to match_array([sync_failed, wiki_sync_failed])
+    end
+
+    context 'with selective sync' do
+      before do
+        secondary.update_attribute(:namespaces, [synced_group])
+      end
+
+      it 'delegates to #legacy_find_filtered_failed_project_registries' do
+        expect(subject).to receive(:legacy_find_filtered_failed_project_registries).and_call_original
+
+        subject.find_failed_project_registries
+      end
+
+      it 'returns project registries that sync has failed' do
+        expect(subject.find_failed_project_registries).to match_array([repository_sync_failed, wiki_sync_failed])
+      end
+
+      it 'returns only project registries that repository sync has failed' do
+        create(:geo_project_registry, :repository_sync_failed)
+
+        expect(subject.find_failed_project_registries('repository')).to match_array([repository_sync_failed])
+      end
+
+      it 'returns only project registries that wiki sync has failed' do
+        create(:geo_project_registry, :wiki_sync_failed)
+
+        expect(subject.find_failed_project_registries('wiki')).to match_array([wiki_sync_failed])
       end
     end
   end

spec/factories/geo/project_registry.rb

@@ -40,6 +40,7 @@ FactoryGirl.define do
       resync_repository true
       resync_wiki true
       repository_retry_count 1
+      wiki_retry_count 1
     end
 
     trait :repository_sync_failed do