Commit aad9f49b authored by Mike Kozono's avatar Mike Kozono

Use `replicables_for_geo_node` as SSOT

...to answer the question: "Which records should be synced?".
parent 62848d57
...@@ -21,7 +21,7 @@ module Geo ...@@ -21,7 +21,7 @@ module Geo
# Called by Gitlab::Geo::Replicator#consume # Called by Gitlab::Geo::Replicator#consume
def consume_event_created(**params) def consume_event_created(**params)
return if excluded_by_selective_sync? return unless in_replicables_for_geo_node?
download download
end end
...@@ -34,7 +34,7 @@ module Geo ...@@ -34,7 +34,7 @@ module Geo
# Called by Gitlab::Geo::Replicator#consume # Called by Gitlab::Geo::Replicator#consume
def consume_event_deleted(**params) def consume_event_deleted(**params)
return if excluded_by_selective_sync? return unless in_replicables_for_geo_node?
replicate_destroy(params) replicate_destroy(params)
end end
......
...@@ -82,6 +82,10 @@ module Gitlab ...@@ -82,6 +82,10 @@ module Gitlab
replicator.carrierwave_uploader.exists? replicator.carrierwave_uploader.exists?
end end
end end
def in_replicables_for_geo_node?
self.class.replicables_for_geo_node.id_in(self).exists?
end
end end
end end
end end
...@@ -19,6 +19,7 @@ module Gitlab ...@@ -19,6 +19,7 @@ module Gitlab
attr_reader :model_record_id attr_reader :model_record_id
delegate :model, to: :class delegate :model, to: :class
delegate :in_replicables_for_geo_node?, to: :model_record
class << self class << self
delegate :find_unsynced_registries, :find_failed_registries, to: :registry_class delegate :find_unsynced_registries, :find_failed_registries, to: :registry_class
...@@ -275,26 +276,6 @@ module Gitlab ...@@ -275,26 +276,6 @@ module Gitlab
registry.verification_checksum registry.verification_checksum
end end
# This method does not yet cover resources that are owned by a namespace
# but not a project, because we do not have that use-case...yet.
# E.g. GroupWikis will need it.
def excluded_by_selective_sync?
# If the replicable is not owned by a project or namespace, then selective sync cannot apply to it.
return false unless parent_project_id
!current_node.projects_include?(parent_project_id)
end
def parent_project_id
strong_memoize(:parent_project_id) do
# We should never see this at runtime. All Replicators should be tested
# by `it_behaves_like 'a replicator'`, which would reveal this problem.
selective_sync_not_implemented_error(__method__) unless model_record.respond_to?(:project_id)
model_record.project_id
end
end
# Return exactly the data needed by `for_replicable_params` to # Return exactly the data needed by `for_replicable_params` to
# reinstantiate this Replicator elsewhere. # reinstantiate this Replicator elsewhere.
# #
...@@ -335,11 +316,6 @@ module Gitlab ...@@ -335,11 +316,6 @@ module Gitlab
def current_node def current_node
Gitlab::Geo.current_node Gitlab::Geo.current_node
end end
def selective_sync_not_implemented_error(method_name)
raise NotImplementedError,
"#{self.class} does not implement #{method_name}. If selective sync is not applicable, just return nil."
end
end end
end end
end end
...@@ -179,68 +179,6 @@ RSpec.describe Gitlab::Geo::Replicator do ...@@ -179,68 +179,6 @@ RSpec.describe Gitlab::Geo::Replicator do
end end
end end
describe '#excluded_by_selective_sync?' do
subject(:replicator) { Geo::DummyReplicator.new }
before do
stub_current_geo_node(secondary_node)
end
context 'when parent_project_id is not nil' do
before do
allow(replicator).to receive(:parent_project_id).and_return(123456)
end
context 'when the current Geo node excludes the parent_project due to selective sync' do
it 'returns true' do
expect(secondary_node).to receive(:projects_include?).with(123456).and_return(false)
expect(replicator.excluded_by_selective_sync?).to eq(true)
end
end
context 'when the current Geo node does not exclude the parent_project due to selective sync' do
it 'returns false' do
expect(secondary_node).to receive(:projects_include?).with(123456).and_return(true)
expect(replicator.excluded_by_selective_sync?).to eq(false)
end
end
end
context 'when parent_project_id is nil' do
before do
expect(replicator).to receive(:parent_project_id).and_return(nil)
end
it 'returns false' do
expect(replicator.excluded_by_selective_sync?).to eq(false)
end
end
end
describe '#parent_project_id' do
subject(:replicator) { Geo::DummyReplicator.new(model_record: model_record) }
# We cannot infer parent project, so parent_project_id should be overridden.
context 'when model_record does not respond to project_id' do
let(:model_record) { double(:model_record, id: 555) }
it 'raises NotImplementedError' do
expect { replicator.parent_project_id }.to raise_error(NotImplementedError)
end
end
# We assume project_id to be the parent project.
context 'when model_record responds to project_id' do
let(:model_record) { double(:model_record, id: 555, project_id: 1234) }
it 'does not error' do
expect(replicator.parent_project_id).to eq(1234)
end
end
end
describe '.for_replicable_params' do describe '.for_replicable_params' do
it 'returns the corresponding Replicator instance' do it 'returns the corresponding Replicator instance' do
replicator = described_class.for_replicable_params(replicable_name: 'dummy', replicable_id: 123456) replicator = described_class.for_replicable_params(replicable_name: 'dummy', replicable_id: 123456)
...@@ -295,5 +233,9 @@ RSpec.describe Gitlab::Geo::Replicator do ...@@ -295,5 +233,9 @@ RSpec.describe Gitlab::Geo::Replicator do
end end
end end
end end
describe '#in_replicables_for_geo_node?' do
it { is_expected.to delegate_method(:in_replicables_for_geo_node?).to(:model_record) }
end
end end
end end
...@@ -104,9 +104,9 @@ RSpec.shared_examples 'a blob replicator' do ...@@ -104,9 +104,9 @@ RSpec.shared_examples 'a blob replicator' do
end end
describe '#consume_event_created' do describe '#consume_event_created' do
context "when the blob's project is not excluded by selective sync" do context "when the blob's project is in replicables for this geo node" do
it 'invokes Geo::BlobDownloadService' do it 'invokes Geo::BlobDownloadService' do
expect(replicator).to receive(:excluded_by_selective_sync?).and_return(false) expect(replicator).to receive(:in_replicables_for_geo_node?).and_return(true)
service = double(:service) service = double(:service)
expect(service).to receive(:execute) expect(service).to receive(:execute)
...@@ -116,9 +116,9 @@ RSpec.shared_examples 'a blob replicator' do ...@@ -116,9 +116,9 @@ RSpec.shared_examples 'a blob replicator' do
end end
end end
context "when the blob's project is excluded by selective sync" do context "when the blob's project is not in replicables for this geo node" do
it 'does not invoke Geo::BlobDownloadService' do it 'does not invoke Geo::BlobDownloadService' do
expect(replicator).to receive(:excluded_by_selective_sync?).and_return(true) expect(replicator).to receive(:in_replicables_for_geo_node?).and_return(false)
expect(::Geo::BlobDownloadService).not_to receive(:new) expect(::Geo::BlobDownloadService).not_to receive(:new)
...@@ -128,9 +128,9 @@ RSpec.shared_examples 'a blob replicator' do ...@@ -128,9 +128,9 @@ RSpec.shared_examples 'a blob replicator' do
end end
describe '#consume_event_deleted' do describe '#consume_event_deleted' do
context "when the blob's project is not excluded by selective sync" do context "when the blob's project is in replicables for this geo node" do
it 'invokes Geo::FileRegistryRemovalService' do it 'invokes Geo::FileRegistryRemovalService' do
expect(replicator).to receive(:excluded_by_selective_sync?).and_return(false) expect(replicator).to receive(:in_replicables_for_geo_node?).and_return(true)
service = double(:service) service = double(:service)
expect(service).to receive(:execute) expect(service).to receive(:execute)
...@@ -141,9 +141,9 @@ RSpec.shared_examples 'a blob replicator' do ...@@ -141,9 +141,9 @@ RSpec.shared_examples 'a blob replicator' do
end end
end end
context "when the blob's project is excluded by selective sync" do context "when the blob's project is not in replicables for this geo node" do
it 'does not invoke Geo::FileRegistryRemovalService' do it 'does not invoke Geo::FileRegistryRemovalService' do
expect(replicator).to receive(:excluded_by_selective_sync?).and_return(true) expect(replicator).to receive(:in_replicables_for_geo_node?).and_return(false)
expect(::Geo::FileRegistryRemovalService).not_to receive(:new) expect(::Geo::FileRegistryRemovalService).not_to receive(:new)
......
...@@ -13,12 +13,6 @@ ...@@ -13,12 +13,6 @@
RSpec.shared_examples 'a replicator' do RSpec.shared_examples 'a replicator' do
include EE::GeoHelpers include EE::GeoHelpers
describe '#parent_project_id' do
it 'is implemented if needed' do
expect { replicator.parent_project_id }.not_to raise_error
end
end
context 'Geo node status' do context 'Geo node status' do
context 'on a secondary node' do context 'on a secondary node' do
let_it_be(:registry_factory) { registry_factory_name(described_class.registry_class) } let_it_be(:registry_factory) { registry_factory_name(described_class.registry_class) }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment