Commit aaf382d5 authored by Lin Jen-Shin's avatar Lin Jen-Shin

Skip or retain project while deleting the project:

* Skip Ci::Build#update_project_statistics whenever there's no
  project (i.e. we're deleting the project!)
* Retain the unscoped_project before deleting the build,
  so that we could use the data to delete the artifacts.
  Note that carrierwave uses `after_commit` for this,
  so we need to retain it in the memory.

Closes #15005
parent 806b038a
...@@ -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
...@@ -559,6 +566,10 @@ module Ci ...@@ -559,6 +566,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 },
...@@ -597,6 +608,8 @@ module Ci ...@@ -597,6 +608,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:
require 'spec_helper' require 'spec_helper'
describe ProjectDestroyWorker do describe ProjectDestroyWorker do
let(:project) { create(:project) } let(:project) { create(:project, pending_delete: true) }
let(:path) { project.repository.path_to_repo } let(:path) { project.repository.path_to_repo }
subject { ProjectDestroyWorker.new } subject { ProjectDestroyWorker.new }
describe "#perform" do describe '#perform' do
it "deletes the project" do context 'with pipelines and builds' do
let!(:pipeline) { create(:ci_pipeline, project: project) }
let!(:build) { create(:ci_build, :artifacts, pipeline: pipeline) }
it 'deletes the project along with pipelines and builds' do
subject.perform(project.id, project.owner.id, {}) subject.perform(project.id, project.owner.id, {})
expect(Project.all).not_to include(project) expect(Project.all).not_to include(project)
expect(Ci::Pipeline.all).not_to include(pipeline)
expect(Ci::Build.all).not_to include(build)
expect(Dir.exist?(path)).to be_falsey expect(Dir.exist?(path)).to be_falsey
end end
end
it "deletes the project but skips repo deletion" do it 'deletes the project but skips repo deletion' do
subject.perform(project.id, project.owner.id, { "skip_repo" => true }) subject.perform(project.id, project.owner.id, { 'skip_repo' => true })
expect(Project.all).not_to include(project) expect(Project.all).not_to include(project)
expect(Dir.exist?(path)).to be_truthy expect(Dir.exist?(path)).to be_truthy
......
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