Skip to content

G
gitlab-ce
Commit f126baa5 authored by Thong Kuah's avatar Thong Kuah
Browse files
Options

Merge branch 'revert-92896973' into 'master' 

Prevent further deletion of epic-issue relationships

See merge request gitlab-org/gitlab!76815
parents b558ba51 34f8f6da
Show whitespace changes
Inline Side-by-side
Showing with 4 additions and 227 deletions
db/post_migrate/20211023102243_schedule_delete_invalid_epic_issues.rb
View file @ f126baa5
# frozen_string_literal: true
class ScheduleDeleteInvalidEpicIssues < Gitlab::Database::Migration[1.0]
MIGRATION = 'DeleteInvalidEpicIssues'
DELAY_INTERVAL = 2.minutes
BATCH_SIZE = 10_000
disable_ddl_transaction!
# This is a now a no-op
# See https://gitlab.com/gitlab-org/gitlab/-/issues/348477
def up
queue_background_migration_jobs_by_range_at_intervals(
define_batchable_model('epics'),
MIGRATION,
DELAY_INTERVAL,
batch_size: BATCH_SIZE,
track_jobs: true
)
# no-op
end
def down
# also no-op
end
end
ee/lib/ee/gitlab/background_migration/delete_invalid_epic_issues.rb deleted 100644 → 0
View file @ b558ba51
# frozen_string_literal: true
module EE
module Gitlab
module BackgroundMigration
module DeleteInvalidEpicIssues
extend ::Gitlab::Utils::Override
class Namespace < ActiveRecord::Base
self.table_name = 'namespaces'
has_many :epics
has_many :projects
end
class Project < ActiveRecord::Base
self.table_name = 'projects'
has_many :issues
belongs_to :group
end
class Epic < ActiveRecord::Base
include EachBatch
self.table_name = 'epics'
has_many :epic_issues
belongs_to :group
end
class Issue < ActiveRecord::Base
self.table_name = 'issues'
has_many :epic_issues
belongs_to :project
end
class EpicIssue < ActiveRecord::Base
self.table_name = 'epic_issues'
belongs_to :epic
belongs_to :issue
end
override :perform
def perform(start_id, stop_id)
to_delete = []
epics = Epic.where(id: start_id..stop_id)
.includes(epic_issues: { issue: :project })
.order(:group_id)
epics.find_in_batches do |batch|
batch.group_by { |epic| epic.group_id }.each do |group_id, group_epics|
groups = group_and_hierarchy(group_id)
group_epics.each do |epic|
next if epic.epic_issues.empty?
epic.epic_issues.each do |epic_issue|
to_delete << epic_issue.id unless groups.include?(epic_issue.issue.project.namespace_id)
end
end
end
EpicIssue.where(id: to_delete).delete_all if to_delete.present?
end
end
def group_and_hierarchy(id)
::Gitlab::ObjectHierarchy
.new(Namespace.where(id: id))
.base_and_ancestors.pluck(:id)
end
end
end
end
end
ee/spec/lib/ee/gitlab/background_migration/delete_invalid_epic_issues_spec.rb deleted 100644 → 0
View file @ b558ba51
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Gitlab::BackgroundMigration::DeleteInvalidEpicIssues do
# rubocop:disable RSpec/MultipleMemoizedHelpers
let!(:users) { table(:users) }
let!(:namespaces) { table(:namespaces) }
let!(:projects) { table(:projects) }
let!(:epics) { table(:epics) }
let!(:issues) { table(:issues) }
let!(:epic_issues) { table(:epic_issues) }
let!(:user) { users.create!(name: 'test', email: 'test@example.com', projects_limit: 5) }
let!(:group) { namespaces.create!(name: 'test 1', path: 'test1') }
let!(:sub_group) { namespaces.create!(name: 'test 2', path: 'test2', parent_id: group.id) }
let!(:other_group) { namespaces.create!(name: 'test 3', path: 'test3') }
let!(:project) { projects.create!(namespace_id: group.id, name: 'test 1', path: 'test1') }
let!(:project_sub) { projects.create!(namespace_id: sub_group.id, name: 'test 1', path: 'test1') }
let!(:project_other) { projects.create!(namespace_id: other_group.id, name: 'test 1', path: 'test1') }
describe '#perform' do
let!(:epic_before) { epics.create!(iid: 1, title: 'test 1', title_html: 'test 1', group_id: group.id, author_id: user.id) }
let!(:epic) { epics.create!(iid: 2, title: 'test 2', title_html: 'test 2', group_id: group.id, author_id: user.id) }
let!(:epic_sub) { epics.create!(iid: 3, title: 'test 3', title_html: 'test 3', group_id: sub_group.id, author_id: user.id) }
let!(:epic_other) { epics.create!(iid: 4, title: 'test 4', title_html: 'test 4', group_id: other_group.id, author_id: user.id) }
let!(:epic_last) { epics.create!(iid: 5, title: 'test 5', title_html: 'test 5', group_id: group.id, author_id: user.id) }
let!(:issue) { issues.create!(iid: 1, project_id: project.id, title: 'issue 1', title_html: 'issue 1', author_id: user.id) }
let!(:issue2) { issues.create!(iid: 2, project_id: project.id, title: 'issue 2', title_html: 'issue 2', author_id: user.id) }
let!(:issue3) { issues.create!(iid: 6, project_id: project.id, title: 'issue 3', title_html: 'issue 3', author_id: user.id) }
let!(:issue4) { issues.create!(iid: 7, project_id: project.id, title: 'issue 4', title_html: 'issue 4', author_id: user.id) }
let!(:issue5) { issues.create!(iid: 8, project_id: project.id, title: 'issue 5', title_html: 'issue 5', author_id: user.id) }
let!(:issue_sub) { issues.create!(iid: 3, project_id: project_sub.id, title: 'issue 4', title_html: 'issue 4', author_id: user.id) }
let!(:issue_other) { issues.create!(iid: 4, project_id: project_other.id, title: 'issue 5', title_html: 'issue 5', author_id: user.id) }
let!(:issue_other_2) { issues.create!(iid: 5, project_id: project_other.id, title: 'issue 6', title_html: 'issue 6', author_id: user.id) }
let!(:issue_other_3) { issues.create!(iid: 6, project_id: project_other.id, title: 'issue 7', title_html: 'issue 7', author_id: user.id) }
let!(:valid_and_invalid_epic_issues) do
invalid_epic_issues = []
valid_epic_issues = []
valid_epic_issues << epic_issues.create!(issue_id: issue_other_3.id, epic_id: epic_before.id)
valid_epic_issues << epic_issues.create!(issue_id: issue.id, epic_id: epic_sub.id)
valid_epic_issues << epic_issues.create!(issue_id: issue_sub.id, epic_id: epic_sub.id)
invalid_epic_issues << epic_issues.create!(issue_id: issue_other.id, epic_id: epic_sub.id)
valid_epic_issues << epic_issues.create!(issue_id: issue_other_2.id, epic_id: epic_other.id)
invalid_epic_issues << epic_issues.create!(issue_id: issue2.id, epic_id: epic_other.id)
valid_epic_issues << epic_issues.create!(issue_id: issue3.id, epic_id: epic.id)
valid_epic_issues << epic_issues.create!(issue_id: issue5.id, epic_id: epic_last.id)
{ valid: valid_epic_issues, invalid: invalid_epic_issues }
end
let(:valid_epic_issues) { valid_and_invalid_epic_issues[:valid] }
let(:invalid_epic_issues) { valid_and_invalid_epic_issues[:invalid] }
it 'removes invalid epic issues' do
expect { described_class.new.perform(epic.id, epic_last.id) }.to change { epic_issues.count }.from(8).to(6)
expect(epic_issues.all).to match_array(valid_epic_issues)
end
it 'searches the group hierarchy only once per epics in the same group' do
service = described_class.new
expect(service).to receive(:group_and_hierarchy).exactly(3).times.and_call_original
service.perform(epic.id, epic_last.id)
end
it 'prevents N+1 queries' do
control = ActiveRecord::QueryRecorder.new(skip_cached: false) do
described_class.new.perform(epic.id, epic_other.id)
end
# recreate deleted records
epic_issues.create!(issue_id: issue_other.id, epic_id: epic_sub.id)
epic_issues.create!(issue_id: issue2.id, epic_id: epic_other.id)
# create new records to delete
issue_9 = issues.create!(iid: 9, project_id: project.id, title: 'issue 7', title_html: 'issue 7', author_id: user.id)
issue_10 = issues.create!(iid: 10, project_id: project.id, title: 'issue 8', title_html: 'issue 8', author_id: user.id)
epic_issues.create!(issue_id: issue_9.id, epic_id: epic_other.id)
epic_issues.create!(issue_id: issue_10.id, epic_id: epic_other.id)
expect { described_class.new.perform(epic.id, epic_other.id) }.not_to exceed_all_query_limit(control)
end
end
end
ee/spec/migrations/schedule_delete_invalid_epic_issues_spec.rb deleted 100644 → 0
View file @ b558ba51
# frozen_string_literal: true
require 'spec_helper'
require_migration!
RSpec.describe ScheduleDeleteInvalidEpicIssues do
let(:migration) { described_class::MIGRATION }
let(:users) { table(:users) }
let(:namespaces) { table(:namespaces) }
let(:epics) { table(:epics) }
let(:user) { users.create!(name: 'test', email: 'test@example.com', projects_limit: 5) }
let!(:group) { namespaces.create!(name: 'test 1', path: 'test1') }
let!(:epic1) { epics.create!(iid: 1, title: 'test 1', title_html: 'test 1', group_id: group.id, author_id: user.id) }
let!(:epic2) { epics.create!(iid: 2, title: 'test 2', title_html: 'test 2', group_id: group.id, author_id: user.id) }
let!(:epic3) { epics.create!(iid: 3, title: 'test 3', title_html: 'test 3', group_id: group.id, author_id: user.id) }
it 'correctly schedules background migrations' do
stub_const("#{described_class.name}::BATCH_SIZE", 2)
Sidekiq::Testing.fake! do
freeze_time do
migrate!
expect(BackgroundMigrationWorker.jobs.size).to eq(2)
expect(migration).to be_scheduled_delayed_migration(2.minutes, epic1.id, epic2.id)
expect(migration).to be_scheduled_delayed_migration(4.minutes, epic3.id, epic3.id)
end
end
end
end
lib/gitlab/background_migration/delete_invalid_epic_issues.rb deleted 100644 → 0
View file @ b558ba51
# frozen_string_literal: true
module Gitlab
module BackgroundMigration
# rubocop: disable Style/Documentation
class DeleteInvalidEpicIssues
def perform(start_id, stop_id)
end
end
end
end
Gitlab::BackgroundMigration::DeleteInvalidEpicIssues.prepend_mod_with('Gitlab::BackgroundMigration::DeleteInvalidEpicIssues')
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
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7