Merge branch 'mk/refactor-replicables-method' into 'master'

Geo: Refactor replicables_for_geo_node method

See merge request gitlab-org/gitlab!44938

doc/development/geo/framework.md

@@ -199,7 +199,7 @@ For example, to add support for files referenced by a `Widget` model with a
 
      # @param primary_key_in [Range, Widget] arg to pass to primary_key_in scope
      # @return [ActiveRecord::Relation<Widget>] everything that should be synced to this node, restricted by primary key
-     def self.replicables_for_geo_node(primary_key_in)
+     def self.replicables_for_current_secondary(primary_key_in)
        # Should be implemented. The idea of the method is to restrict
        # the set of synced items depending on synchronization settings
      end

ee/app/models/concerns/geo/blob_replicator_strategy.rb

@@ -22,7 +22,7 @@ module Geo
 
     # Called by Gitlab::Geo::Replicator#consume
     def consume_event_created(**params)
-      return unless in_replicables_for_geo_node?
+      return unless in_replicables_for_current_secondary?
 
       download
     end

ee/app/models/concerns/geo/repository_replicator_strategy.rb

@@ -15,7 +15,7 @@ module Geo
 
     # Called by Gitlab::Geo::Replicator#consume
     def consume_event_updated(**params)
-      return unless in_replicables_for_geo_node?
+      return unless in_replicables_for_current_secondary?
 
       sync_repository
     end

ee/app/models/ee/ci/job_artifact.rb

@@ -83,9 +83,10 @@ module EE
       end
 
       # @param primary_key_in [Range, Ci::JobArtifact] arg to pass to primary_key_in scope
-      # @param node [GeoNode] defaults to ::Gitlab::Geo.current_node
       # @return [ActiveRecord::Relation<Ci::JobArtifact>] everything that should be synced to this node, restricted by primary key
-      def replicables_for_geo_node(primary_key_in, node = ::Gitlab::Geo.current_node)
+      def replicables_for_current_secondary(primary_key_in)
+        node = ::Gitlab::Geo.current_node
+
         not_expired
           .primary_key_in(primary_key_in)
           .merge(selective_sync_scope(node))

ee/app/models/ee/container_repository.rb

@@ -10,9 +10,10 @@ module EE
 
     class_methods do
       # @param primary_key_in [Range, ContainerRepository] arg to pass to primary_key_in scope
-      # @param node [GeoNode] defaults to ::Gitlab::Geo.current_node
       # @return [ActiveRecord::Relation<ContainerRepository>] everything that should be synced to this node, restricted by primary key
-      def replicables_for_geo_node(primary_key_in, node = ::Gitlab::Geo.current_node)
+      def replicables_for_current_secondary(primary_key_in)
+        node = ::Gitlab::Geo.current_node
+
         node.container_repositories.primary_key_in(primary_key_in)
       end
     end

ee/app/models/ee/lfs_object.rb

@@ -18,9 +18,9 @@ module EE
 
     class_methods do
       # @param primary_key_in [Range, LfsObject] arg to pass to primary_key_in scope
-      # @param node [GeoNode] defaults to ::Gitlab::Geo.current_node
       # @return [ActiveRecord::Relation<LfsObject>] everything that should be synced to this node, restricted by primary key
-      def replicables_for_geo_node(primary_key_in, node = ::Gitlab::Geo.current_node)
+      def replicables_for_current_secondary(primary_key_in)
+        node = ::Gitlab::Geo.current_node
         local_storage_only = !node.sync_object_storage
 
         scope = node.lfs_objects(primary_key_in: primary_key_in)

ee/app/models/ee/merge_request_diff.rb

@@ -30,9 +30,10 @@ module EE
 
     class_methods do
       # @param primary_key_in [Range, MergeRequestDiff] arg to pass to primary_key_in scope
-      # @param node [GeoNode] defaults to ::Gitlab::Geo.current_node
       # @return [ActiveRecord::Relation<MergeRequestDiff>] everything that should be synced to this node, restricted by primary key
-      def replicables_for_geo_node(primary_key_in, node = ::Gitlab::Geo.current_node)
+      def replicables_for_current_secondary(primary_key_in)
+        node = ::Gitlab::Geo.current_node
+
         has_external_diffs.primary_key_in(primary_key_in)
                           .merge(selective_sync_scope(node))
                           .merge(object_storage_scope(node))

ee/app/models/ee/packages/package_file.rb

@@ -13,7 +13,7 @@ module EE
       class_methods do
         # @param primary_key_in [Range, Packages::PackageFile] arg to pass to primary_key_in scope
         # @return [ActiveRecord::Relation<LfsObject>] everything that should be synced to this node, restricted by primary key
-        def replicables_for_geo_node(primary_key_in)
+        def replicables_for_current_secondary(primary_key_in)
           primary_key_in(primary_key_in)
             .merge(selective_sync_scope)
             .merge(object_storage_scope)

ee/app/models/ee/project.rb

@@ -205,9 +205,10 @@ module EE
       extend ::Gitlab::Utils::Override
 
       # @param primary_key_in [Range, Project] arg to pass to primary_key_in scope
-      # @param node [GeoNode] defaults to ::Gitlab::Geo.current_node
       # @return [ActiveRecord::Relation<Project>] everything that should be synced to this node, restricted by primary key
-      def replicables_for_geo_node(primary_key_in, node = ::Gitlab::Geo.current_node)
+      def replicables_for_current_secondary(primary_key_in)
+        node = ::Gitlab::Geo.current_node
+
         node.projects.primary_key_in(primary_key_in)
       end
 

ee/app/models/ee/snippet_repository.rb

@@ -12,9 +12,8 @@ module EE
 
     class_methods do
       # @param primary_key_in [Range, SnippetRepository] arg to pass to primary_key_in scope
-      # @param node [GeoNode] defaults to ::Gitlab::Geo.current_node
       # @return [ActiveRecord::Relation<SnippetRepository>] everything that should be synced to this node, restricted by primary key
-      def replicables_for_geo_node(primary_key_in, node = ::Gitlab::Geo.current_node)
+      def replicables_for_current_secondary(primary_key_in)
         # Not implemented yet. Should be responsible for selective sync
         all
       end

ee/app/models/ee/terraform/state_version.rb

@@ -15,9 +15,10 @@ module EE
 
       class_methods do
         # @param primary_key_in [Range, Terraform::StateVersion] arg to pass to primary_key_in scope
-        # @param node [GeoNode] defaults to ::Gitlab::Geo.current_node
         # @return [ActiveRecord::Relation<Terraform::StateVersion>] everything that should be synced to this node, restricted by primary key
-        def replicables_for_geo_node(primary_key_in, node = ::Gitlab::Geo.current_node)
+        def replicables_for_current_secondary(primary_key_in)
+          node = ::Gitlab::Geo.current_node
+
           primary_key_in(primary_key_in)
             .merge(selective_sync_scope(node))
             .merge(object_storage_scope(node))

ee/app/models/ee/upload.rb

@@ -19,9 +19,10 @@ module EE
 
     class_methods do
       # @param primary_key_in [Range, Upload] arg to pass to primary_key_in scope
-      # @param node [GeoNode] defaults to ::Gitlab::Geo.current_node
       # @return [ActiveRecord::Relation<Upload>] everything that should be synced to this node, restricted by primary key
-      def replicables_for_geo_node(primary_key_in, node = ::Gitlab::Geo.current_node)
+      def replicables_for_current_secondary(primary_key_in)
+        node = ::Gitlab::Geo.current_node
+
         primary_key_in(primary_key_in)
           .merge(selective_sync_scope(node))
           .merge(object_storage_scope(node))

ee/app/models/geo/base_registry.rb

@@ -53,7 +53,7 @@ class Geo::BaseRegistry < Geo::TrackingBase
     model_primary_key = self::MODEL_CLASS.primary_key.to_sym
 
     source_ids = self::MODEL_CLASS
-                  .replicables_for_geo_node(range)
+                  .replicables_for_current_secondary(range)
                   .pluck(self::MODEL_CLASS.arel_table[model_primary_key])
 
     tracked_ids = self.pluck_model_ids_in_range(range)

ee/app/models/geo/upload_registry.rb

@@ -34,7 +34,7 @@ class Geo::UploadRegistry < Geo::BaseRegistry
   #                 For example: [[[1, 'avatar'], [5, 'file']], [[3, 'attachment']]]
   def self.find_registry_differences(range)
     source =
-      self::MODEL_CLASS.replicables_for_geo_node(range)
+      self::MODEL_CLASS.replicables_for_current_secondary(range)
           .pluck(self::MODEL_CLASS.arel_table[:id], self::MODEL_CLASS.arel_table[:uploader])
           .map! { |id, uploader| [id, uploader.sub(/Uploader\z/, '').underscore] }
 

ee/lib/gitlab/geo/replicable_model.rb

@@ -83,8 +83,8 @@ module Gitlab
         end
       end
 
-      def in_replicables_for_geo_node?
-        self.class.replicables_for_geo_node(self).exists?
+      def in_replicables_for_current_secondary?
+        self.class.replicables_for_current_secondary(self).exists?
       end
     end
   end

ee/lib/gitlab/geo/replicator.rb

@@ -20,7 +20,7 @@ module Gitlab
 
       delegate :model, to: :class
       delegate :replication_enabled_feature_key, to: :class
-      delegate :in_replicables_for_geo_node?, to: :model_record
+      delegate :in_replicables_for_current_secondary?, to: :model_record
 
       class << self
         delegate :find_registries_never_attempted_sync, :find_registries_needs_sync_again, to: :registry_class

ee/spec/lib/gitlab/geo/replicable_model_spec.rb

@@ -43,11 +43,11 @@ RSpec.describe Gitlab::Geo::ReplicableModel do
     end
   end
 
-  describe '#in_replicables_for_geo_node?' do
-    it 'reuses replicables_for_geo_node' do
-      expect(DummyModel).to receive(:replicables_for_geo_node).once.with(subject).and_call_original
+  describe '#in_replicables_for_current_secondary?' do
+    it 'reuses replicables_for_current_secondary' do
+      expect(DummyModel).to receive(:replicables_for_current_secondary).once.with(subject).and_call_original
 
-      subject.in_replicables_for_geo_node?
+      subject.in_replicables_for_current_secondary?
     end
   end
 end

ee/spec/lib/gitlab/geo/replicator_spec.rb

@@ -171,7 +171,7 @@ RSpec.describe Gitlab::Geo::Replicator do
     end
   end
 
-  describe '#in_replicables_for_geo_node?' do
-    it { is_expected.to delegate_method(:in_replicables_for_geo_node?).to(:model_record) }
+  describe '#in_replicables_for_current_secondary?' do
+    it { is_expected.to delegate_method(:in_replicables_for_current_secondary?).to(:model_record) }
   end
 end

ee/spec/models/ee/ci/job_artifact_spec.rb

@@ -133,7 +133,7 @@ RSpec.describe Ci::JobArtifact do
     end
   end
 
-  describe '#replicables_for_geo_node' do
+  describe '#replicables_for_current_secondary' do
     # Selective sync is configured relative to the job artifact's project.
     #
     # Permutations of sync_object_storage combined with object-stored-artifacts
@@ -162,7 +162,7 @@ RSpec.describe Ci::JobArtifact do
     end
 
     with_them do
-      subject(:job_artifact_included) { described_class.replicables_for_geo_node(ci_job_artifact).exists? }
+      subject(:job_artifact_included) { described_class.replicables_for_current_secondary(ci_job_artifact).exists? }
 
       let(:project) { create(*project_factory) }
       let(:ci_build) { create(:ci_build, project: project) }

ee/spec/models/ee/merge_request_diff_spec.rb

@@ -61,7 +61,7 @@ RSpec.describe MergeRequestDiff do
     end
   end
 
-  describe '.replicables_for_geo_node' do
+  describe '.replicables_for_current_secondary' do
     context 'without selective sync or object storage' do
       let(:secondary) { create(:geo_node) }
 
@@ -74,13 +74,13 @@ RSpec.describe MergeRequestDiff do
 
         create(:merge_request, source_project: project)
 
-        expect(described_class.replicables_for_geo_node(1..described_class.last.id)).to be_empty
+        expect(described_class.replicables_for_current_secondary(1..described_class.last.id)).to be_empty
       end
 
       it 'excludes empty diffs' do
         create(:merge_request, source_project: create(:project))
 
-        expect(described_class.replicables_for_geo_node(1..described_class.last.id)).to be_empty
+        expect(described_class.replicables_for_current_secondary(1..described_class.last.id)).to be_empty
       end
     end
 
@@ -119,7 +119,7 @@ RSpec.describe MergeRequestDiff do
       end
 
       it 'returns the proper number of merge request diff states' do
-        expect(described_class.replicables_for_geo_node(1..described_class.last.id)).to have_attributes(count: synced_states)
+        expect(described_class.replicables_for_current_secondary(1..described_class.last.id)).to have_attributes(count: synced_states)
       end
     end
   end

ee/spec/models/ee/terraform/state_version_spec.rb

@@ -25,7 +25,7 @@ RSpec.describe Terraform::StateVersion do
     end
   end
 
-  describe '.replicables_for_geo_node' do
+  describe '.replicables_for_current_secondary' do
     where(:selective_sync_enabled, :object_storage_sync_enabled, :terraform_object_storage_enabled, :synced_states) do
       true  | true  | true  | 5
       true  | true  | false | 5
@@ -60,7 +60,7 @@ RSpec.describe Terraform::StateVersion do
       end
 
       it 'returns the proper number of terraform states' do
-        expect(described_class.replicables_for_geo_node(1..described_class.last.id).count).to eq(synced_states)
+        expect(described_class.replicables_for_current_secondary(1..described_class.last.id).count).to eq(synced_states)
       end
     end
   end

ee/spec/models/packages/package_file_spec.rb

@@ -36,8 +36,8 @@ RSpec.describe Packages::PackageFile, type: :model do
     end
   end
 
-  describe '.replicables_for_geo_node' do
-    subject { described_class.replicables_for_geo_node(1..described_class.last.id) }
+  describe '.replicables_for_current_secondary' do
+    subject { described_class.replicables_for_current_secondary(1..described_class.last.id) }
 
     it 'returns a package files scope' do
       secondary = create(:geo_node)

ee/spec/models/upload_spec.rb

@@ -6,7 +6,7 @@ RSpec.describe Upload do
   include EE::GeoHelpers
   using RSpec::Parameterized::TableSyntax
 
-  describe '.replicables_for_geo_node' do
+  describe '.replicables_for_current_secondary' do
     # Selective sync is configured relative to the upload's model. Take care not
     # to specify a model_factory that contradicts factory.
     #
@@ -37,7 +37,7 @@ RSpec.describe Upload do
     end
 
     with_them do
-      subject(:upload_included) { described_class.replicables_for_geo_node(upload).exists? }
+      subject(:upload_included) { described_class.replicables_for_current_secondary(upload).exists? }
 
       let(:model) { create(*model_factory) }
       let(:node) do

ee/spec/support/helpers/ee/geo_helpers.rb

@@ -83,7 +83,7 @@ module EE
 
         with_replicator Geo::DummyReplicator
 
-        def self.replicables_for_geo_node(primary_key_in)
+        def self.replicables_for_current_secondary(primary_key_in)
           self.primary_key_in(primary_key_in)
         end
       end

ee/spec/support/shared_examples/models/concerns/blob_replicator_strategy_shared_examples.rb

@@ -109,7 +109,7 @@ RSpec.shared_examples 'a blob replicator' do
   describe 'created event consumption' do
     context "when the blob's project is in replicables for this geo node" do
       it 'invokes Geo::BlobDownloadService' do
-        expect(replicator).to receive(:in_replicables_for_geo_node?).and_return(true)
+        expect(replicator).to receive(:in_replicables_for_current_secondary?).and_return(true)
         service = double(:service)
 
         expect(service).to receive(:execute)
@@ -121,7 +121,7 @@ RSpec.shared_examples 'a blob replicator' do
 
     context "when the blob's project is not in replicables for this geo node" do
       it 'does not invoke Geo::BlobDownloadService' do
-        expect(replicator).to receive(:in_replicables_for_geo_node?).and_return(false)
+        expect(replicator).to receive(:in_replicables_for_current_secondary?).and_return(false)
 
         expect(::Geo::BlobDownloadService).not_to receive(:new)
 

ee/spec/support/shared_examples/models/concerns/replicable_model_shared_examples.rb

@@ -28,7 +28,7 @@ RSpec.shared_examples 'a replicable model' do
     model_record.save!
   end
 
-  describe '.replicables_for_geo_node' do
+  describe '.replicables_for_current_secondary' do
     let_it_be(:secondary) { create(:geo_node) }
 
     before do
@@ -36,7 +36,7 @@ RSpec.shared_examples 'a replicable model' do
     end
 
     it 'is implemented' do
-      expect(model_record.class.replicables_for_geo_node(model_record.id)).to be_an(ActiveRecord::Relation)
+      expect(model_record.class.replicables_for_current_secondary(model_record.id)).to be_an(ActiveRecord::Relation)
     end
   end
 end

ee/spec/support/shared_examples/models/concerns/repository_replicator_strategy_shared_examples.rb

@@ -71,7 +71,7 @@ RSpec.shared_examples 'a repository replicator' do
   end
 
   describe 'updated event consumption' do
-    context 'in replicables_for_geo_node list' do
+    context 'in replicables_for_current_secondary list' do
       it 'runs SnippetRepositorySyncService service' do
         model_record.save!
 
@@ -87,7 +87,7 @@ RSpec.shared_examples 'a repository replicator' do
       end
     end
 
-    context 'not in replicables_for_geo_node list' do
+    context 'not in replicables_for_current_secondary list' do
       it 'runs SnippetRepositorySyncService service' do
         expect(::Geo::FrameworkRepositorySyncService)
           .not_to receive(:new)