Commit 7c96e4d4 authored by Maxime Orefice's avatar Maxime Orefice Committed by Thong Kuah

Fix object storage orphans when destroying project

This commit prevents project from being destroyed
if any child objects with object storage are not
destroyed first.
parent 6fb1784e
...@@ -352,15 +352,15 @@ class Project < ApplicationRecord ...@@ -352,15 +352,15 @@ class Project < ApplicationRecord
has_many :pending_builds, class_name: 'Ci::PendingBuild' has_many :pending_builds, class_name: 'Ci::PendingBuild'
has_many :builds, class_name: 'Ci::Build', inverse_of: :project has_many :builds, class_name: 'Ci::Build', inverse_of: :project
has_many :processables, class_name: 'Ci::Processable', inverse_of: :project has_many :processables, class_name: 'Ci::Processable', inverse_of: :project
has_many :build_trace_chunks, class_name: 'Ci::BuildTraceChunk', through: :builds, source: :trace_chunks has_many :build_trace_chunks, class_name: 'Ci::BuildTraceChunk', through: :builds, source: :trace_chunks, dependent: :restrict_with_error
has_many :build_report_results, class_name: 'Ci::BuildReportResult', inverse_of: :project has_many :build_report_results, class_name: 'Ci::BuildReportResult', inverse_of: :project
has_many :job_artifacts, class_name: 'Ci::JobArtifact' has_many :job_artifacts, class_name: 'Ci::JobArtifact', dependent: :restrict_with_error
has_many :pipeline_artifacts, class_name: 'Ci::PipelineArtifact', inverse_of: :project has_many :pipeline_artifacts, class_name: 'Ci::PipelineArtifact', inverse_of: :project, dependent: :restrict_with_error
has_many :runner_projects, class_name: 'Ci::RunnerProject', inverse_of: :project has_many :runner_projects, class_name: 'Ci::RunnerProject', inverse_of: :project
has_many :runners, through: :runner_projects, source: :runner, class_name: 'Ci::Runner' has_many :runners, through: :runner_projects, source: :runner, class_name: 'Ci::Runner'
has_many :variables, class_name: 'Ci::Variable' has_many :variables, class_name: 'Ci::Variable'
has_many :triggers, class_name: 'Ci::Trigger' has_many :triggers, class_name: 'Ci::Trigger'
has_many :secure_files, class_name: 'Ci::SecureFile' has_many :secure_files, class_name: 'Ci::SecureFile', dependent: :restrict_with_error
has_many :environments has_many :environments
has_many :environments_for_dashboard, -> { from(with_rank.unfoldered.available, :environments).where('rank <= 3') }, class_name: 'Environment' has_many :environments_for_dashboard, -> { from(with_rank.unfoldered.available, :environments).where('rank <= 3') }, class_name: 'Environment'
has_many :deployments has_many :deployments
......
...@@ -134,7 +134,7 @@ RSpec.describe Project, factory_default: :keep do ...@@ -134,7 +134,7 @@ RSpec.describe Project, factory_default: :keep do
it { is_expected.to have_many(:packages).class_name('Packages::Package') } it { is_expected.to have_many(:packages).class_name('Packages::Package') }
it { is_expected.to have_many(:package_files).class_name('Packages::PackageFile') } it { is_expected.to have_many(:package_files).class_name('Packages::PackageFile') }
it { is_expected.to have_many(:debian_distributions).class_name('Packages::Debian::ProjectDistribution').dependent(:destroy) } it { is_expected.to have_many(:debian_distributions).class_name('Packages::Debian::ProjectDistribution').dependent(:destroy) }
it { is_expected.to have_many(:pipeline_artifacts) } it { is_expected.to have_many(:pipeline_artifacts).dependent(:restrict_with_error) }
it { is_expected.to have_many(:terraform_states).class_name('Terraform::State').inverse_of(:project) } it { is_expected.to have_many(:terraform_states).class_name('Terraform::State').inverse_of(:project) }
it { is_expected.to have_many(:timelogs) } it { is_expected.to have_many(:timelogs) }
it { is_expected.to have_many(:error_tracking_errors).class_name('ErrorTracking::Error') } it { is_expected.to have_many(:error_tracking_errors).class_name('ErrorTracking::Error') }
...@@ -142,6 +142,9 @@ RSpec.describe Project, factory_default: :keep do ...@@ -142,6 +142,9 @@ RSpec.describe Project, factory_default: :keep do
it { is_expected.to have_many(:pending_builds).class_name('Ci::PendingBuild') } it { is_expected.to have_many(:pending_builds).class_name('Ci::PendingBuild') }
it { is_expected.to have_many(:ci_feature_usages).class_name('Projects::CiFeatureUsage') } it { is_expected.to have_many(:ci_feature_usages).class_name('Projects::CiFeatureUsage') }
it { is_expected.to have_many(:bulk_import_exports).class_name('BulkImports::Export') } it { is_expected.to have_many(:bulk_import_exports).class_name('BulkImports::Export') }
it { is_expected.to have_many(:job_artifacts).dependent(:restrict_with_error) }
it { is_expected.to have_many(:build_trace_chunks).through(:builds).dependent(:restrict_with_error) }
it { is_expected.to have_many(:secure_files).class_name('Ci::SecureFile').dependent(:restrict_with_error) }
# GitLab Pages # GitLab Pages
it { is_expected.to have_many(:pages_domains) } it { is_expected.to have_many(:pages_domains) }
...@@ -202,6 +205,35 @@ RSpec.describe Project, factory_default: :keep do ...@@ -202,6 +205,35 @@ RSpec.describe Project, factory_default: :keep do
end end
end end
context 'when project has object storage attached to it' do
let_it_be(:project) { create(:project) }
before do
create(:ci_job_artifact, project: project)
end
context 'when associated object storage object is not deleted before the project' do
it 'adds an error to project', :aggregate_failures do
expect { project.destroy! }.to raise_error(ActiveRecord::RecordNotDestroyed)
expect(project.errors).not_to be_empty
expect(project.errors.first.message).to eq("Cannot delete record because dependent job artifacts exist")
end
end
context 'when associated object storage object is deleted before the project' do
before do
project.job_artifacts.first.destroy!
end
it 'deletes the project' do
project.destroy!
expect { project.reload }.to raise_error(ActiveRecord::RecordNotFound)
end
end
end
context 'when creating a new project' do context 'when creating a new project' do
let_it_be(:project) { create(:project) } let_it_be(:project) { create(:project) }
......
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