Merge branch '218987-fj-extrac-logic-from-project-into-has-repository-concern' into 'master'

Refactor repository move logic from the Project model into concern

See merge request gitlab-org/gitlab!49225

app/models/concerns/can_move_repository_storage.rb

+# frozen_string_literal: true
+
+module CanMoveRepositoryStorage
+  extend ActiveSupport::Concern
+
+  RepositoryReadOnlyError = Class.new(StandardError)
+
+  # Tries to set repository as read_only, checking for existing Git transfers in
+  # progress beforehand. Setting a repository read-only will fail if it is
+  # already in that state.
+  #
+  # @return nil. Failures will raise an exception
+  def set_repository_read_only!(skip_git_transfer_check: false)
+    with_lock do
+      raise RepositoryReadOnlyError, _('Git transfer in progress') if
+        !skip_git_transfer_check && git_transfer_in_progress?
+
+      raise RepositoryReadOnlyError, _('Repository already read-only') if
+        self.class.where(id: id).pick(:repository_read_only)
+
+      raise ActiveRecord::RecordNotSaved, _('Database update failed') unless
+        update_column(:repository_read_only, true)
+
+      nil
+    end
+  end
+
+  # Set repository as writable again. Unlike setting it read-only, this will
+  # succeed if the repository is already writable.
+  def set_repository_writable!
+    with_lock do
+      raise ActiveRecord::RecordNotSaved, _('Database update failed') unless
+        update_column(:repository_read_only, false)
+
+      nil
+    end
+  end
+
+  def git_transfer_in_progress?
+    reference_counter(type: repository.repo_type).value > 0
+  end
+
+  def reference_counter(type:)
+    Gitlab::ReferenceCounter.new(type.identifier_for_container(self))
+  end
+end

app/models/project.rb

@@ -19,6 +19,7 @@ class Project < ApplicationRecord
   include Presentable
   include HasRepository
   include HasWiki
+  include CanMoveRepositoryStorage
   include Routable
   include GroupDescendant
   include Gitlab::SQL::Pattern
@@ -2112,39 +2113,6 @@ class Project < ApplicationRecord
     (auto_devops || build_auto_devops)&.predefined_variables
   end
 
-  RepositoryReadOnlyError = Class.new(StandardError)
-
-  # Tries to set repository as read_only, checking for existing Git transfers in
-  # progress beforehand. Setting a repository read-only will fail if it is
-  # already in that state.
-  #
-  # @return nil. Failures will raise an exception
-  def set_repository_read_only!(skip_git_transfer_check: false)
-    with_lock do
-      raise RepositoryReadOnlyError, _('Git transfer in progress') if
-        !skip_git_transfer_check && git_transfer_in_progress?
-
-      raise RepositoryReadOnlyError, _('Repository already read-only') if
-        self.class.where(id: id).pick(:repository_read_only)
-
-      raise ActiveRecord::RecordNotSaved, _('Database update failed') unless
-        update_column(:repository_read_only, true)
-
-      nil
-    end
-  end
-
-  # Set repository as writable again. Unlike setting it read-only, this will
-  # succeed if the repository is already writable.
-  def set_repository_writable!
-    with_lock do
-      raise ActiveRecord::RecordNotSaved, _('Database update failed') unless
-        update_column(:repository_read_only, false)
-
-      nil
-    end
-  end
-
   def pushes_since_gc
     Gitlab::Redis::SharedState.with { |redis| redis.get(pushes_since_gc_redis_shared_state_key).to_i }
   end
@@ -2294,6 +2262,7 @@ class Project < ApplicationRecord
     end
   end
 
+  override :git_transfer_in_progress?
   def git_transfer_in_progress?
     GL_REPOSITORY_TYPES.any? do |type|
       reference_counter(type: type).value > 0
@@ -2306,10 +2275,6 @@ class Project < ApplicationRecord
     @storage = nil if storage_version_changed?
   end
 
-  def reference_counter(type: Gitlab::GlRepository::PROJECT)
-    Gitlab::ReferenceCounter.new(type.identifier_for_container(self))
-  end
-
   def badges
     return project_badges unless group
 

app/models/snippet.rb

@@ -15,6 +15,7 @@ class Snippet < ApplicationRecord
   include FromUnion
   include IgnorableColumns
   include HasRepository
+  include CanMoveRepositoryStorage
   include AfterCommitQueue
   extend ::Gitlab::Utils::Override
 

spec/models/project_spec.rb

@@ -147,6 +147,10 @@ RSpec.describe Project, factory_default: :keep do
       let(:container_without_wiki) { create(:project) }
     end
 
+    it_behaves_like 'can move repository storage' do
+      let_it_be(:container) { create(:project, :repository) }
+    end
+
     it 'has an inverse relationship with merge requests' do
       expect(described_class.reflect_on_association(:merge_requests).has_inverse?).to eq(:target_project)
     end
@@ -3040,50 +3044,6 @@ RSpec.describe Project, factory_default: :keep do
     end
   end
 
-  describe '#set_repository_read_only!' do
-    let(:project) { create(:project) }
-
-    it 'makes the repository read-only' do
-      expect { project.set_repository_read_only! }
-        .to change(project, :repository_read_only?)
-        .from(false)
-        .to(true)
-    end
-
-    it 'raises an error if the project is already read-only' do
-      project.set_repository_read_only!
-
-      expect { project.set_repository_read_only! }.to raise_error(described_class::RepositoryReadOnlyError, /already read-only/)
-    end
-
-    it 'raises an error when there is an existing git transfer in progress' do
-      allow(project).to receive(:git_transfer_in_progress?) { true }
-
-      expect { project.set_repository_read_only! }.to raise_error(described_class::RepositoryReadOnlyError, /in progress/)
-    end
-
-    context 'skip_git_transfer_check is true' do
-      it 'makes the project read-only when git transfers are in progress' do
-        allow(project).to receive(:git_transfer_in_progress?) { true }
-
-        expect { project.set_repository_read_only!(skip_git_transfer_check: true) }
-          .to change(project, :repository_read_only?)
-          .from(false)
-          .to(true)
-      end
-    end
-  end
-
-  describe '#set_repository_writable!' do
-    it 'sets repository_read_only to false' do
-      project = create(:project, :read_only)
-
-      expect { project.set_repository_writable! }
-        .to change(project, :repository_read_only)
-        .from(true).to(false)
-    end
-  end
-
   describe '#pushes_since_gc' do
     let(:project) { build_stubbed(:project) }
 

spec/models/snippet_spec.rb

@@ -769,4 +769,30 @@ RSpec.describe Snippet do
       it { is_expected.to be_falsey }
     end
   end
+
+  describe '#git_transfer_in_progress?' do
+    let(:snippet) { build(:snippet) }
+
+    subject { snippet.git_transfer_in_progress? }
+
+    it 'returns true when there are git transfers' do
+      allow(snippet).to receive(:reference_counter).with(type: Gitlab::GlRepository::SNIPPET) do
+        double(:reference_counter, value: 2)
+      end
+
+      expect(subject).to eq true
+    end
+
+    it 'returns false when there are not git transfers' do
+      allow(snippet).to receive(:reference_counter).with(type: Gitlab::GlRepository::SNIPPET) do
+        double(:reference_counter, value: 0)
+      end
+
+      expect(subject).to eq false
+    end
+  end
+
+  it_behaves_like 'can move repository storage' do
+    let_it_be(:container) { create(:snippet, :repository) }
+  end
 end

spec/support/shared_examples/models/concerns/can_move_repository_storage_shared_examples.rb

+# frozen_string_literal: true
+
+RSpec.shared_examples 'can move repository storage' do
+  let(:container) { raise NotImplementedError }
+
+  describe '#set_repository_read_only!' do
+    it 'makes the repository read-only' do
+      expect { container.set_repository_read_only! }
+        .to change(container, :repository_read_only?)
+        .from(false)
+        .to(true)
+    end
+
+    it 'raises an error if the project is already read-only' do
+      container.set_repository_read_only!
+
+      expect { container.set_repository_read_only! }.to raise_error(described_class::RepositoryReadOnlyError, /already read-only/)
+    end
+
+    it 'raises an error when there is an existing git transfer in progress' do
+      allow(container).to receive(:git_transfer_in_progress?) { true }
+
+      expect { container.set_repository_read_only! }.to raise_error(described_class::RepositoryReadOnlyError, /in progress/)
+    end
+
+    context 'skip_git_transfer_check is true' do
+      it 'makes the project read-only when git transfers are in progress' do
+        allow(container).to receive(:git_transfer_in_progress?) { true }
+
+        expect { container.set_repository_read_only!(skip_git_transfer_check: true) }
+          .to change(container, :repository_read_only?)
+          .from(false)
+          .to(true)
+      end
+    end
+  end
+
+  describe '#set_repository_writable!' do
+    it 'sets repository_read_only to false' do
+      expect { container.set_repository_writable! }
+        .to change(container, :repository_read_only)
+        .from(true).to(false)
+    end
+  end
+
+  describe '#reference_counter' do
+    it 'returns a Gitlab::ReferenceCounter object' do
+      expect(Gitlab::ReferenceCounter).to receive(:new).with(container.repository.gl_repository).and_call_original
+
+      result = container.reference_counter(type: container.repository.repo_type)
+
+      expect(result).to be_a Gitlab::ReferenceCounter
+    end
+  end
+end