From 92ce332ab30725da1b7136cfda420598cfede131 Mon Sep 17 00:00:00 2001 From: Michael Kozono Date: Thu, 4 Sep 2025 12:16:11 -1000 Subject: [PATCH 1/3] Geo: Refactor: Raise if selective sync type unknown --- ee/app/models/concerns/geo/selective_sync.rb | 15 +++++++++++++++ ee/app/models/geo/errors.rb | 12 ++++++++++++ ee/app/models/geo_node.rb | 2 -- ee/spec/models/ee/project_spec.rb | 4 ++-- ee/spec/models/geo_node_spec.rb | 14 +++++++++++++- ee/spec/models/group_wiki_repository_spec.rb | 5 +++-- ee/spec/models/snippet_repository_spec.rb | 5 +++-- 7 files changed, 48 insertions(+), 9 deletions(-) diff --git a/ee/app/models/concerns/geo/selective_sync.rb b/ee/app/models/concerns/geo/selective_sync.rb index ef56565258779d..bc6706c1663929 100644 --- a/ee/app/models/concerns/geo/selective_sync.rb +++ b/ee/app/models/concerns/geo/selective_sync.rb @@ -4,18 +4,33 @@ module Geo module SelectiveSync extend ActiveSupport::Concern + SELECTIVE_SYNC_TYPES = %w[namespaces shards].freeze + def selective_sync? + validate_selective_sync_type! + selective_sync_type.present? end def selective_sync_by_namespaces? + validate_selective_sync_type! + selective_sync_type == 'namespaces' end def selective_sync_by_shards? + validate_selective_sync_type! + selective_sync_type == 'shards' end + def validate_selective_sync_type! + return if selective_sync_type.blank? + return if SELECTIVE_SYNC_TYPES.include?(selective_sync_type) + + raise ::Geo::Errors::UnknownSelectiveSyncType.new(selective_sync_type: selective_sync_type) + end + # This method should only be used when: # # - Selective sync is enabled diff --git a/ee/app/models/geo/errors.rb b/ee/app/models/geo/errors.rb index bf027fd4e19610..58b1688140fe32 100644 --- a/ee/app/models/geo/errors.rb +++ b/ee/app/models/geo/errors.rb @@ -8,5 +8,17 @@ def message "Generating Geo node status is taking too long" end end + + class UnknownSelectiveSyncType < BaseError + attr_reader :selective_sync_type + + def initialize(selective_sync_type:) + @selective_sync_type = selective_sync_type + end + + def message + "Selective sync type is not known: #{selective_sync_type}" + end + end end end diff --git a/ee/app/models/geo_node.rb b/ee/app/models/geo_node.rb index 343b382144def5..48ea4257235a12 100644 --- a/ee/app/models/geo_node.rb +++ b/ee/app/models/geo_node.rb @@ -6,8 +6,6 @@ class GeoNode < ApplicationRecord include StripAttribute include Gitlab::EncryptedAttribute - SELECTIVE_SYNC_TYPES = %w[namespaces shards].freeze - # Array of repository storages to synchronize for selective sync by shards serialize :selective_sync_shards, type: Array diff --git a/ee/spec/models/ee/project_spec.rb b/ee/spec/models/ee/project_spec.rb index 2052a0fac5836a..c8c26096975afb 100644 --- a/ee/spec/models/ee/project_spec.rb +++ b/ee/spec/models/ee/project_spec.rb @@ -5452,10 +5452,10 @@ def stub_default_url_options(host) expect(described_class.selective_sync_scope(node)).to match_array([project_1, project_2]) end - it 'returns nothing if an unrecognised selective sync type is used' do + it 'raises if an unrecognised selective sync type is used' do node.update_attribute(:selective_sync_type, 'unknown') - expect(described_class.selective_sync_scope(node)).to be_empty + expect { described_class.selective_sync_scope(node) }.to raise_error(Geo::Errors::UnknownSelectiveSyncType) end end diff --git a/ee/spec/models/geo_node_spec.rb b/ee/spec/models/geo_node_spec.rb index 9926a70b4d6ab0..671993b5863de8 100644 --- a/ee/spec/models/geo_node_spec.rb +++ b/ee/spec/models/geo_node_spec.rb @@ -734,7 +734,7 @@ end describe '#selective_sync?' do - subject { node.selective_sync? } + subject(:selective_sync?) { node.selective_sync? } it 'returns true when selective sync is by namespaces' do node.update!(selective_sync_type: 'namespaces') @@ -757,6 +757,18 @@ is_expected.to be_falsy end + + it 'returns false when selective_sync_type is nil' do + node.update!(selective_sync_type: nil) + + is_expected.to be_falsy + end + + it 'raises when selective_sync_type is invalid' do + node.selective_sync_type = 'invalid_type' + + expect { selective_sync? }.to raise_error(Geo::Errors::UnknownSelectiveSyncType) + end end describe '#namespaces_for_group_owned_replicables' do diff --git a/ee/spec/models/group_wiki_repository_spec.rb b/ee/spec/models/group_wiki_repository_spec.rb index 2209d44b9711bd..18cf9450cf0864 100644 --- a/ee/spec/models/group_wiki_repository_spec.rb +++ b/ee/spec/models/group_wiki_repository_spec.rb @@ -125,10 +125,11 @@ end end - it 'returns nothing if an unrecognised selective sync type is used' do + it 'raises if an unrecognised selective sync type is used' do node.update_attribute(:selective_sync_type, 'unknown') - expect(described_class.replicables_for_current_secondary(1..described_class.last.id)).to be_empty + expect { described_class.replicables_for_current_secondary(1..described_class.last.id) } + .to raise_error(Geo::Errors::UnknownSelectiveSyncType) end end end diff --git a/ee/spec/models/snippet_repository_spec.rb b/ee/spec/models/snippet_repository_spec.rb index 6d7c2a6ab959ce..10d820dbb7d5c3 100644 --- a/ee/spec/models/snippet_repository_spec.rb +++ b/ee/spec/models/snippet_repository_spec.rb @@ -127,10 +127,11 @@ end end - it 'returns nothing if an unrecognised selective sync type is used' do + it 'raises if an unrecognised selective sync type is used' do secondary.update_attribute(:selective_sync_type, 'unknown') - expect(described_class.replicables_for_current_secondary(1..described_class.last.id)).to be_empty + expect { described_class.replicables_for_current_secondary(1..described_class.last.id) } + .to raise_error(Geo::Errors::UnknownSelectiveSyncType) end end end -- GitLab From b4361f62bf89f83605ba4426e6c4cd3afd86a750 Mon Sep 17 00:00:00 2001 From: Mike Kozono Date: Thu, 28 Aug 2025 17:12:09 -1000 Subject: [PATCH 2/3] Fix leaky test setup --- ee/spec/services/geo/node_update_service_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/spec/services/geo/node_update_service_spec.rb b/ee/spec/services/geo/node_update_service_spec.rb index 5614ff1b85396c..dcbf0f2bcf8d73 100644 --- a/ee/spec/services/geo/node_update_service_spec.rb +++ b/ee/spec/services/geo/node_update_service_spec.rb @@ -6,7 +6,7 @@ include EE::GeoHelpers let_it_be(:primary, reload: true) { create(:geo_node, :primary) } - let_it_be(:geo_node) { create(:geo_node) } + let!(:geo_node) { create(:geo_node) } before do stub_current_geo_node(primary) -- GitLab From 10e473f5b249fdfe5d4fbdf8dc885010ecbd449b Mon Sep 17 00:00:00 2001 From: Michael Kozono Date: Tue, 9 Sep 2025 13:42:00 -1000 Subject: [PATCH 3/3] Add test coverage for error message --- ee/spec/models/geo_node_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/spec/models/geo_node_spec.rb b/ee/spec/models/geo_node_spec.rb index 671993b5863de8..42a0c738c0f14c 100644 --- a/ee/spec/models/geo_node_spec.rb +++ b/ee/spec/models/geo_node_spec.rb @@ -767,7 +767,7 @@ it 'raises when selective_sync_type is invalid' do node.selective_sync_type = 'invalid_type' - expect { selective_sync? }.to raise_error(Geo::Errors::UnknownSelectiveSyncType) + expect { selective_sync? }.to raise_error(Geo::Errors::UnknownSelectiveSyncType, /Selective sync type is not known: invalid_type/) end end -- GitLab