Add CanMoveRepositoryStorage concern to Group

In this commit we add the necessary functionality to groups
to interact with the `repository_read_only` column.

Groups doesn't have the column in its table but in the
`namespace_settings` one. That's why we need to override
some methods and use them as proxy for this operation.
parent a6080c29
...@@ -16,10 +16,10 @@ module CanMoveRepositoryStorage ...@@ -16,10 +16,10 @@ module CanMoveRepositoryStorage
!skip_git_transfer_check && git_transfer_in_progress? !skip_git_transfer_check && git_transfer_in_progress?
raise RepositoryReadOnlyError, _('Repository already read-only') if raise RepositoryReadOnlyError, _('Repository already read-only') if
self.class.where(id: id).pick(:repository_read_only) _safe_read_repository_read_only_column
raise ActiveRecord::RecordNotSaved, _('Database update failed') unless raise ActiveRecord::RecordNotSaved, _('Database update failed') unless
update_column(:repository_read_only, true) _update_repository_read_only_column(true)
nil nil
end end
...@@ -30,7 +30,7 @@ module CanMoveRepositoryStorage ...@@ -30,7 +30,7 @@ module CanMoveRepositoryStorage
def set_repository_writable! def set_repository_writable!
with_lock do with_lock do
raise ActiveRecord::RecordNotSaved, _('Database update failed') unless raise ActiveRecord::RecordNotSaved, _('Database update failed') unless
update_column(:repository_read_only, false) _update_repository_read_only_column(false)
nil nil
end end
...@@ -43,4 +43,19 @@ module CanMoveRepositoryStorage ...@@ -43,4 +43,19 @@ module CanMoveRepositoryStorage
def reference_counter(type:) def reference_counter(type:)
Gitlab::ReferenceCounter.new(type.identifier_for_container(self)) Gitlab::ReferenceCounter.new(type.identifier_for_container(self))
end end
private
# Not all resources that can move repositories have the `repository_read_only`
# in their table, for example groups. We need these methods to override the
# behavior in those classes in order to access the column.
def _safe_read_repository_read_only_column
# This was added originally this way because of
# https://gitlab.com/gitlab-org/gitlab/-/commit/43f9b98302d3985312c9f8b66018e2835d8293d2
self.class.where(id: id).pick(:repository_read_only)
end
def _update_repository_read_only_column(value)
update_column(:repository_read_only, value)
end
end end
...@@ -14,6 +14,7 @@ module EE ...@@ -14,6 +14,7 @@ module EE
include InsightsFeature include InsightsFeature
include HasTimelogsReport include HasTimelogsReport
include HasWiki include HasWiki
include CanMoveRepositoryStorage
add_authentication_token_field :saml_discovery_token, unique: false, token_generator: -> { Devise.friendly_token(8) } add_authentication_token_field :saml_discovery_token, unique: false, token_generator: -> { Devise.friendly_token(8) }
...@@ -49,6 +50,7 @@ module EE ...@@ -49,6 +50,7 @@ module EE
has_one :deletion_schedule, class_name: 'GroupDeletionSchedule' has_one :deletion_schedule, class_name: 'GroupDeletionSchedule'
delegate :deleting_user, :marked_for_deletion_on, to: :deletion_schedule, allow_nil: true delegate :deleting_user, :marked_for_deletion_on, to: :deletion_schedule, allow_nil: true
delegate :enforced_group_managed_accounts?, :enforced_sso?, to: :saml_provider, allow_nil: true delegate :enforced_group_managed_accounts?, :enforced_sso?, to: :saml_provider, allow_nil: true
delegate :repository_read_only, :repository_read_only?, to: :namespace_settings, allow_nil: true
has_one :group_wiki_repository has_one :group_wiki_repository
has_many :repository_storage_moves, class_name: 'Groups::RepositoryStorageMove', inverse_of: :container has_many :repository_storage_moves, class_name: 'Groups::RepositoryStorageMove', inverse_of: :container
...@@ -455,6 +457,11 @@ module EE ...@@ -455,6 +457,11 @@ module EE
end end
end end
override :git_transfer_in_progress?
def git_transfer_in_progress?
reference_counter(type: ::Gitlab::GlRepository::WIKI).value > 0
end
private private
def custom_project_templates_group_allowed def custom_project_templates_group_allowed
...@@ -517,5 +524,17 @@ module EE ...@@ -517,5 +524,17 @@ module EE
def invited_or_shared_group_members(groups) def invited_or_shared_group_members(groups)
::GroupMember.active_without_invites_and_requests.where(source_id: ::Gitlab::ObjectHierarchy.new(groups).base_and_ancestors) ::GroupMember.active_without_invites_and_requests.where(source_id: ::Gitlab::ObjectHierarchy.new(groups).base_and_ancestors)
end end
override :_safe_read_repository_read_only_column
def _safe_read_repository_read_only_column
::NamespaceSetting.where(namespace: self).pick(:repository_read_only)
end
override :_update_repository_read_only_column
def _update_repository_read_only_column(value)
settings = namespace_settings || create_namespace_settings
settings.update_column(:repository_read_only, value)
end
end end
end end
...@@ -8,7 +8,7 @@ module EE ...@@ -8,7 +8,7 @@ module EE
override :identifier_for_container override :identifier_for_container
def identifier_for_container(container) def identifier_for_container(container)
if container.is_a?(GroupWiki) if container.is_a?(GroupWiki) || (wiki? && container.is_a?(Group))
"group-#{container.id}-#{name}" "group-#{container.id}-#{name}"
else else
super super
......
...@@ -14,6 +14,12 @@ RSpec.describe Gitlab::GlRepository::RepoType do ...@@ -14,6 +14,12 @@ RSpec.describe Gitlab::GlRepository::RepoType do
let(:expected_container) { wiki } let(:expected_container) { wiki }
let(:expected_repository) { ::Repository.new(wiki.full_path, wiki, shard: wiki.repository_storage, disk_path: wiki.disk_path, repo_type: Gitlab::GlRepository::WIKI) } let(:expected_repository) { ::Repository.new(wiki.full_path, wiki, shard: wiki.repository_storage, disk_path: wiki.disk_path, repo_type: Gitlab::GlRepository::WIKI) }
end end
describe '#identifier_for_container' do
subject { described_class.identifier_for_container(wiki.group) }
it { is_expected.to eq("group-#{wiki.group.id}-wiki") }
end
end end
end end
end end
...@@ -1263,4 +1263,10 @@ RSpec.describe Group do ...@@ -1263,4 +1263,10 @@ RSpec.describe Group do
end end
end end
end end
it_behaves_like 'can move repository storage' do
let_it_be(:container) { create(:group, :wiki_repo) }
let(:repository) { container.wiki.repository }
end
end end
...@@ -2,11 +2,12 @@ ...@@ -2,11 +2,12 @@
RSpec.shared_examples 'can move repository storage' do RSpec.shared_examples 'can move repository storage' do
let(:container) { raise NotImplementedError } let(:container) { raise NotImplementedError }
let(:repository) { container.repository }
describe '#set_repository_read_only!' do describe '#set_repository_read_only!' do
it 'makes the repository read-only' do it 'makes the repository read-only' do
expect { container.set_repository_read_only! } expect { container.set_repository_read_only! }
.to change(container, :repository_read_only?) .to change { container.repository_read_only? }
.from(false) .from(false)
.to(true) .to(true)
end end
...@@ -28,7 +29,7 @@ RSpec.shared_examples 'can move repository storage' do ...@@ -28,7 +29,7 @@ RSpec.shared_examples 'can move repository storage' do
allow(container).to receive(:git_transfer_in_progress?) { true } allow(container).to receive(:git_transfer_in_progress?) { true }
expect { container.set_repository_read_only!(skip_git_transfer_check: true) } expect { container.set_repository_read_only!(skip_git_transfer_check: true) }
.to change(container, :repository_read_only?) .to change { container.repository_read_only? }
.from(false) .from(false)
.to(true) .to(true)
end end
...@@ -38,16 +39,16 @@ RSpec.shared_examples 'can move repository storage' do ...@@ -38,16 +39,16 @@ RSpec.shared_examples 'can move repository storage' do
describe '#set_repository_writable!' do describe '#set_repository_writable!' do
it 'sets repository_read_only to false' do it 'sets repository_read_only to false' do
expect { container.set_repository_writable! } expect { container.set_repository_writable! }
.to change(container, :repository_read_only) .to change { container.repository_read_only? }
.from(true).to(false) .from(true).to(false)
end end
end end
describe '#reference_counter' do describe '#reference_counter' do
it 'returns a Gitlab::ReferenceCounter object' do it 'returns a Gitlab::ReferenceCounter object' do
expect(Gitlab::ReferenceCounter).to receive(:new).with(container.repository.gl_repository).and_call_original expect(Gitlab::ReferenceCounter).to receive(:new).with(repository.gl_repository).and_call_original
result = container.reference_counter(type: container.repository.repo_type) result = container.reference_counter(type: repository.repo_type)
expect(result).to be_a Gitlab::ReferenceCounter expect(result).to be_a Gitlab::ReferenceCounter
end end
......
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