Read about our upcoming Code of Conduct on this issue

Commit fb9e059a authored by Gabriel Mazetto's avatar Gabriel Mazetto
Browse files

Make sure repository's removal work for legacy and hashed storages

parent 5b407032aa4b
......@@ -1477,6 +1477,10 @@ def forks_count
Projects::ForksCountService.new(self).count
end
def legacy_storage?
self.storage_version.nil?
end
private
def storage
......
......@@ -13,7 +13,7 @@ def execute
# 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|
......
......@@ -35,16 +35,18 @@ def execute(user, options = {})
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!
......
......@@ -82,7 +82,7 @@
end
trait :hashed do
storage_version 1
storage_version Project::LATEST_STORAGE_VERSION
end
trait :access_requestable do
......
......@@ -2342,6 +2342,14 @@ def enable_lfs
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
......
......@@ -8,8 +8,8 @@
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 @@ def destroy_group(group, user, async)
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
......@@ -4,9 +4,10 @@
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 @@
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 @@
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
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