diff options
| -rw-r--r-- | app/models/project.rb | 4 | ||||
| -rw-r--r-- | app/services/groups/destroy_service.rb | 2 | ||||
| -rw-r--r-- | app/services/users/destroy_service.rb | 6 | ||||
| -rw-r--r-- | spec/factories/projects.rb | 2 | ||||
| -rw-r--r-- | spec/models/project_spec.rb | 8 | ||||
| -rw-r--r-- | spec/services/groups/destroy_service_spec.rb | 26 | ||||
| -rw-r--r-- | spec/services/users/destroy_service_spec.rb | 27 |
7 files changed, 67 insertions, 8 deletions
diff --git a/app/models/project.rb b/app/models/project.rb index a13ad9dceec..37f4dd08355 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1477,6 +1477,10 @@ class Project < ActiveRecord::Base Projects::ForksCountService.new(self).count end + def legacy_storage? + self.storage_version.nil? + end + private def storage diff --git a/app/services/groups/destroy_service.rb b/app/services/groups/destroy_service.rb index f565612a89d..e3f9d9ee95d 100644 --- a/app/services/groups/destroy_service.rb +++ b/app/services/groups/destroy_service.rb @@ -13,7 +13,7 @@ module Groups # Execute the destruction of the models immediately to ensure atomic cleanup. # Skip repository removal because we remove directory with namespace # that contain all these repositories - ::Projects::DestroyService.new(project, current_user, skip_repo: true).execute + ::Projects::DestroyService.new(project, current_user, skip_repo: project.legacy_storage?).execute end group.children.each do |group| diff --git a/app/services/users/destroy_service.rb b/app/services/users/destroy_service.rb index 9d7237c2fbb..8e20de8dfa5 100644 --- a/app/services/users/destroy_service.rb +++ b/app/services/users/destroy_service.rb @@ -35,16 +35,18 @@ module Users Groups::DestroyService.new(group, current_user).execute end + namespace = user.namespace + namespace.prepare_for_destroy + user.personal_projects.each do |project| # Skip repository removal because we remove directory with namespace # that contain all this repositories - ::Projects::DestroyService.new(project, current_user, skip_repo: true).execute + ::Projects::DestroyService.new(project, current_user, skip_repo: project.legacy_storage?).execute end MigrateToGhostUserService.new(user).execute unless options[:hard_delete] # Destroy the namespace after destroying the user since certain methods may depend on the namespace existing - namespace = user.namespace user_data = user.destroy namespace.really_destroy! diff --git a/spec/factories/projects.rb b/spec/factories/projects.rb index c6f1da82f3c..9ebda0ba03b 100644 --- a/spec/factories/projects.rb +++ b/spec/factories/projects.rb @@ -82,7 +82,7 @@ FactoryGirl.define do end trait :hashed do - storage_version 1 + storage_version Project::LATEST_STORAGE_VERSION end trait :access_requestable do diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index c7d2f2063af..2e613c44357 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -2342,6 +2342,14 @@ describe Project do end end + describe '#legacy_storage?' do + it 'returns true when storage_version is nil' do + project = build(:project) + + expect(project.legacy_storage?).to be_truthy + end + end + describe '#rename_repo' do before do # Project#gitlab_shell returns a new instance of Gitlab::Shell on every diff --git a/spec/services/groups/destroy_service_spec.rb b/spec/services/groups/destroy_service_spec.rb index 1b2ce3cd03e..ac4b9c02ba7 100644 --- a/spec/services/groups/destroy_service_spec.rb +++ b/spec/services/groups/destroy_service_spec.rb @@ -8,8 +8,8 @@ describe Groups::DestroyService do let!(:nested_group) { create(:group, parent: group) } let!(:project) { create(:project, namespace: group) } let!(:notification_setting) { create(:notification_setting, source: group)} - let!(:gitlab_shell) { Gitlab::Shell.new } - let!(:remove_path) { group.path + "+#{group.id}+deleted" } + let(:gitlab_shell) { Gitlab::Shell.new } + let(:remove_path) { group.path + "+#{group.id}+deleted" } before do group.add_user(user, Gitlab::Access::OWNER) @@ -134,4 +134,26 @@ describe Groups::DestroyService do it_behaves_like 'group destruction', false end + + describe 'repository removal' do + before do + destroy_group(group, user, false) + end + + context 'legacy storage' do + let!(:project) { create(:project, :empty_repo, namespace: group) } + + it 'removes repository' do + expect(gitlab_shell.exists?(project.repository_storage_path, "#{project.disk_path}.git")).to be_falsey + end + end + + context 'hashed storage' do + let!(:project) { create(:project, :hashed, :empty_repo, namespace: group) } + + it 'removes repository' do + expect(gitlab_shell.exists?(project.repository_storage_path, "#{project.disk_path}.git")).to be_falsey + end + end + end end diff --git a/spec/services/users/destroy_service_spec.rb b/spec/services/users/destroy_service_spec.rb index a82567f6f43..58a5bede3de 100644 --- a/spec/services/users/destroy_service_spec.rb +++ b/spec/services/users/destroy_service_spec.rb @@ -4,9 +4,10 @@ describe Users::DestroyService do describe "Deletes a user and all their personal projects" do let!(:user) { create(:user) } let!(:admin) { create(:admin) } - let!(:namespace) { create(:namespace, owner: user) } + let!(:namespace) { user.namespace } let!(:project) { create(:project, namespace: namespace) } let(:service) { described_class.new(admin) } + let(:gitlab_shell) { Gitlab::Shell.new } context 'no options are given' do it 'deletes the user' do @@ -14,7 +15,7 @@ describe Users::DestroyService do expect { user_data['email'].to eq(user.email) } expect { User.find(user.id) }.to raise_error(ActiveRecord::RecordNotFound) - expect { Namespace.with_deleted.find(user.namespace.id) }.to raise_error(ActiveRecord::RecordNotFound) + expect { Namespace.with_deleted.find(namespace.id) }.to raise_error(ActiveRecord::RecordNotFound) end it 'will delete the project' do @@ -165,5 +166,27 @@ describe Users::DestroyService do expect(Issue.exists?(issue.id)).to be_falsy end end + + describe "user personal's repository removal" do + before do + Sidekiq::Testing.inline! { service.execute(user) } + end + + context 'legacy storage' do + let!(:project) { create(:project, :empty_repo, namespace: user.namespace) } + + it 'removes repository' do + expect(gitlab_shell.exists?(project.repository_storage_path, "#{project.disk_path}.git")).to be_falsey + end + end + + context 'hashed storage' do + let!(:project) { create(:project, :empty_repo, :hashed, namespace: user.namespace) } + + it 'removes repository' do + expect(gitlab_shell.exists?(project.repository_storage_path, "#{project.disk_path}.git")).to be_falsey + end + end + end end end |