Fix project deletion when there is a export available
Project deletions were failing with "Can't modify frozen hash" because: 1. Project#remove_exports was called in the after_destroy hook 2. This would remove the file and update ImportExportUpload 3. ImportExportUpload#save would attempt to write to a destroyed model To avoid this, we just check if ImportExportUpload has been destroyed before attempting to save it. This would have a side effect of not running after_commit hooks to delete the repository on disk, making it impossible to delete the project entirely. Closes #52362
This commit is contained in:
parent
2efbc75fd4
commit
fbb5953ec3
|
@ -1789,7 +1789,7 @@ class Project < ActiveRecord::Base
|
|||
return unless export_file_exists?
|
||||
|
||||
import_export_upload.remove_export_file!
|
||||
import_export_upload.save
|
||||
import_export_upload.save unless import_export_upload.destroyed?
|
||||
end
|
||||
|
||||
def export_file_exists?
|
||||
|
|
|
@ -0,0 +1,5 @@
|
|||
---
|
||||
title: Fix project deletion when there is a export available
|
||||
merge_request: 22276
|
||||
author:
|
||||
type: fixed
|
|
@ -65,10 +65,12 @@ describe Projects::DestroyService do
|
|||
|
||||
context 'Sidekiq inline' do
|
||||
before do
|
||||
# Run sidekiq immediatly to check that renamed repository will be removed
|
||||
# Run sidekiq immediately to check that renamed repository will be removed
|
||||
perform_enqueued_jobs { destroy_project(project, user, {}) }
|
||||
end
|
||||
|
||||
it_behaves_like 'deleting the project'
|
||||
|
||||
context 'when has remote mirrors' do
|
||||
let!(:project) do
|
||||
create(:project, :repository, namespace: user.namespace).tap do |project|
|
||||
|
@ -82,13 +84,28 @@ describe Projects::DestroyService do
|
|||
end
|
||||
end
|
||||
|
||||
it_behaves_like 'deleting the project'
|
||||
|
||||
it 'invalidates personal_project_count cache' do
|
||||
expect(user).to receive(:invalidate_personal_projects_count)
|
||||
|
||||
destroy_project(project, user)
|
||||
end
|
||||
|
||||
context 'when project has exports' do
|
||||
let!(:project_with_export) do
|
||||
create(:project, :repository, namespace: user.namespace).tap do |project|
|
||||
create(:import_export_upload,
|
||||
project: project,
|
||||
export_file: fixture_file_upload('spec/fixtures/project_export.tar.gz'))
|
||||
end
|
||||
end
|
||||
let!(:async) { true }
|
||||
|
||||
it 'destroys project and export' do
|
||||
expect { destroy_project(project_with_export, user) }.to change(ImportExportUpload, :count).by(-1)
|
||||
|
||||
expect(Project.all).not_to include(project_with_export)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
context 'Sidekiq fake' do
|
||||
|
|
Loading…
Reference in New Issue