Commit c2b2e249 authored by Maxime Orefice's avatar Maxime Orefice Committed by Kamil Trzciński

Remove dependent destroy for project builds

This commit removes the ActiveRecord callback and leverages
the foreign key when deleting builds attached to a project.
parent 1cdc91a9
...@@ -341,12 +341,8 @@ class Project < ApplicationRecord ...@@ -341,12 +341,8 @@ class Project < ApplicationRecord
has_many :stages, class_name: 'Ci::Stage', inverse_of: :project has_many :stages, class_name: 'Ci::Stage', inverse_of: :project
has_many :ci_refs, class_name: 'Ci::Ref', inverse_of: :project has_many :ci_refs, class_name: 'Ci::Ref', inverse_of: :project
# Ci::Build objects store data on the file system such as artifact files and
# build traces. Currently there's no efficient way of removing this data in
# bulk that doesn't involve loading the rows into memory. As a result we're
# still using `dependent: :destroy` here.
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, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent 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
has_many :build_report_results, class_name: 'Ci::BuildReportResult', inverse_of: :project has_many :build_report_results, class_name: 'Ci::BuildReportResult', inverse_of: :project
......
...@@ -6,10 +6,10 @@ RSpec.describe BatchDestroyDependentAssociations do ...@@ -6,10 +6,10 @@ RSpec.describe BatchDestroyDependentAssociations do
class TestProject < ActiveRecord::Base class TestProject < ActiveRecord::Base
self.table_name = 'projects' self.table_name = 'projects'
has_many :builds, dependent: :destroy has_many :builds
has_many :notification_settings, as: :source, dependent: :delete_all has_many :notification_settings, as: :source, dependent: :delete_all
has_many :pages_domains has_many :pages_domains
has_many :todos has_many :todos, dependent: :destroy
include BatchDestroyDependentAssociations include BatchDestroyDependentAssociations
end end
...@@ -18,7 +18,7 @@ RSpec.describe BatchDestroyDependentAssociations do ...@@ -18,7 +18,7 @@ RSpec.describe BatchDestroyDependentAssociations do
let_it_be(:project) { TestProject.new } let_it_be(:project) { TestProject.new }
it 'returns the right associations' do it 'returns the right associations' do
expect(project.dependent_associations_to_destroy.map(&:name)).to match_array([:builds]) expect(project.dependent_associations_to_destroy.map(&:name)).to match_array([:todos])
end end
end end
...@@ -26,36 +26,35 @@ RSpec.describe BatchDestroyDependentAssociations do ...@@ -26,36 +26,35 @@ RSpec.describe BatchDestroyDependentAssociations do
let_it_be(:project) { create(:project) } let_it_be(:project) { create(:project) }
let_it_be(:build) { create(:ci_build, project: project) } let_it_be(:build) { create(:ci_build, project: project) }
let_it_be(:notification_setting) { create(:notification_setting, project: project) } let_it_be(:notification_setting) { create(:notification_setting, project: project) }
let_it_be(:note) { create(:note, project: project) }
let!(:todos) { create(:todo, project: project) } it 'destroys multiple notes' do
create(:note, project: project)
it 'destroys multiple builds' do expect(Note.count).to eq(2)
create(:ci_build, project: project)
expect(Ci::Build.count).to eq(2)
project.destroy_dependent_associations_in_batches project.destroy_dependent_associations_in_batches
expect(Ci::Build.count).to eq(0) expect(Note.count).to eq(0)
end end
it 'destroys builds in batches' do it 'destroys note in batches' do
expect(project).to receive_message_chain(:builds, :find_each).and_yield(build) expect(project).to receive_message_chain(:notes, :find_each).and_yield(note)
expect(build).to receive(:destroy).and_call_original expect(note).to receive(:destroy).and_call_original
project.destroy_dependent_associations_in_batches project.destroy_dependent_associations_in_batches
expect(Ci::Build.count).to eq(0) expect(Ci::Build.count).to eq(1)
expect(Todo.count).to eq(1) expect(Note.count).to eq(0)
expect(User.count).to be > 0 expect(User.count).to be > 0
expect(NotificationSetting.count).to eq(User.count) expect(NotificationSetting.count).to eq(User.count)
end end
it 'excludes associations' do it 'excludes associations' do
project.destroy_dependent_associations_in_batches(exclude: [:builds]) project.destroy_dependent_associations_in_batches(exclude: [:notes])
expect(Note.count).to eq(1)
expect(Ci::Build.count).to eq(1) expect(Ci::Build.count).to eq(1)
expect(Todo.count).to eq(1)
expect(User.count).to be > 0 expect(User.count).to be > 0
expect(NotificationSetting.count).to eq(User.count) expect(NotificationSetting.count).to eq(User.count)
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