Commit ee43dcd5 authored by Douwe Maan's avatar Douwe Maan

Merge branch 'fix-deleting-project-again' into 'master'

Skip or retain project while deleting the project:

Closes #15005

See merge request !8960
parents 5ae946f2 ad59f123
...@@ -41,7 +41,7 @@ module Ci ...@@ -41,7 +41,7 @@ module Ci
before_save :update_artifacts_size, if: :artifacts_file_changed? before_save :update_artifacts_size, if: :artifacts_file_changed?
before_save :ensure_token before_save :ensure_token
before_destroy { project } before_destroy { unscoped_project }
after_create :execute_hooks after_create :execute_hooks
after_save :update_project_statistics, if: :artifacts_size_changed? after_save :update_project_statistics, if: :artifacts_size_changed?
...@@ -416,16 +416,23 @@ module Ci ...@@ -416,16 +416,23 @@ module Ci
# This method returns old path to artifacts only if it already exists. # This method returns old path to artifacts only if it already exists.
# #
def artifacts_path def artifacts_path
# We need the project even if it's soft deleted, because whenever
# we're really deleting the project, we'll also delete the builds,
# and in order to delete the builds, we need to know where to find
# the artifacts, which is depending on the data of the project.
# We need to retain the project in this case.
the_project = project || unscoped_project
old = File.join(created_at.utc.strftime('%Y_%m'), old = File.join(created_at.utc.strftime('%Y_%m'),
project.ci_id.to_s, the_project.ci_id.to_s,
id.to_s) id.to_s)
old_store = File.join(ArtifactUploader.artifacts_path, old) old_store = File.join(ArtifactUploader.artifacts_path, old)
return old if project.ci_id && File.directory?(old_store) return old if the_project.ci_id && File.directory?(old_store)
File.join( File.join(
created_at.utc.strftime('%Y_%m'), created_at.utc.strftime('%Y_%m'),
project.id.to_s, the_project.id.to_s,
id.to_s id.to_s
) )
end end
...@@ -560,6 +567,10 @@ module Ci ...@@ -560,6 +567,10 @@ module Ci
self.update(erased_by: user, erased_at: Time.now, artifacts_expire_at: nil) self.update(erased_by: user, erased_at: Time.now, artifacts_expire_at: nil)
end end
def unscoped_project
@unscoped_project ||= Project.unscoped.find_by(id: gl_project_id)
end
def predefined_variables def predefined_variables
variables = [ variables = [
{ key: 'CI', value: 'true', public: true }, { key: 'CI', value: 'true', public: true },
...@@ -598,6 +609,8 @@ module Ci ...@@ -598,6 +609,8 @@ module Ci
end end
def update_project_statistics def update_project_statistics
return unless project
ProjectCacheWorker.perform_async(project_id, [], [:build_artifacts_size]) ProjectCacheWorker.perform_async(project_id, [], [:build_artifacts_size])
end end
end end
......
---
title: Fix deleting projects with pipelines and builds
merge_request: 8960
author:
...@@ -9,12 +9,27 @@ describe Projects::DestroyService, services: true do ...@@ -9,12 +9,27 @@ describe Projects::DestroyService, services: true do
shared_examples 'deleting the project' do shared_examples 'deleting the project' do
it 'deletes the project' do it 'deletes the project' do
expect(Project.all).not_to include(project) expect(Project.unscoped.all).not_to include(project)
expect(Dir.exist?(path)).to be_falsey expect(Dir.exist?(path)).to be_falsey
expect(Dir.exist?(remove_path)).to be_falsey expect(Dir.exist?(remove_path)).to be_falsey
end end
end end
shared_examples 'deleting the project with pipeline and build' do
context 'with pipeline and build' do # which has optimistic locking
let!(:pipeline) { create(:ci_pipeline, project: project) }
let!(:build) { create(:ci_build, :artifacts, pipeline: pipeline) }
before do
perform_enqueued_jobs do
destroy_project(project, user, {})
end
end
it_behaves_like 'deleting the project'
end
end
context 'Sidekiq inline' do context 'Sidekiq inline' do
before do before do
# Run sidekiq immediatly to check that renamed repository will be removed # Run sidekiq immediatly to check that renamed repository will be removed
...@@ -35,9 +50,10 @@ describe Projects::DestroyService, services: true do ...@@ -35,9 +50,10 @@ describe Projects::DestroyService, services: true do
it { expect(Dir.exist?(remove_path)).to be_truthy } it { expect(Dir.exist?(remove_path)).to be_truthy }
end end
context 'async delete of project with private issue visibility' do context 'with async_execute' do
let!(:async) { true } let(:async) { true }
context 'async delete of project with private issue visibility' do
before do before do
project.project_feature.update_attribute("issues_access_level", ProjectFeature::PRIVATE) project.project_feature.update_attribute("issues_access_level", ProjectFeature::PRIVATE)
# Run sidekiq immediately to check that renamed repository will be removed # Run sidekiq immediately to check that renamed repository will be removed
...@@ -47,18 +63,11 @@ describe Projects::DestroyService, services: true do ...@@ -47,18 +63,11 @@ describe Projects::DestroyService, services: true do
it_behaves_like 'deleting the project' it_behaves_like 'deleting the project'
end end
context 'delete with pipeline' do # which has optimistic locking it_behaves_like 'deleting the project with pipeline and build'
let!(:pipeline) { create(:ci_pipeline, project: project) }
before do
expect(project).to receive(:destroy!).and_call_original
perform_enqueued_jobs do
destroy_project(project, user, {})
end
end end
it_behaves_like 'deleting the project' context 'with execute' do
it_behaves_like 'deleting the project with pipeline and build'
end end
context 'container registry' do context 'container registry' do
......
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