diff --git a/changelogs/unreleased/jprovazn-label-links-update.yml b/changelogs/unreleased/jprovazn-label-links-update.yml new file mode 100644 index 00000000000..75fb46ede6b --- /dev/null +++ b/changelogs/unreleased/jprovazn-label-links-update.yml @@ -0,0 +1,5 @@ +--- +title: Fix cross-project label references. +merge_request: +author: +type: fixed diff --git a/db/post_migrate/20180702120647_enqueue_fix_cross_project_label_links.rb b/db/post_migrate/20180702120647_enqueue_fix_cross_project_label_links.rb new file mode 100644 index 00000000000..59aa41adede --- /dev/null +++ b/db/post_migrate/20180702120647_enqueue_fix_cross_project_label_links.rb @@ -0,0 +1,30 @@ +class EnqueueFixCrossProjectLabelLinks < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + BATCH_SIZE = 100 + MIGRATION = 'FixCrossProjectLabelLinks' + DELAY_INTERVAL = 5.minutes + + disable_ddl_transaction! + + class Label < ActiveRecord::Base + self.table_name = 'labels' + end + + class Namespace < ActiveRecord::Base + self.table_name = 'namespaces' + + include ::EachBatch + + default_scope { where(type: 'Group', id: Label.where(type: 'GroupLabel').select('distinct group_id')) } + end + + def up + queue_background_migration_jobs_by_range_at_intervals(Namespace, MIGRATION, DELAY_INTERVAL, batch_size: BATCH_SIZE) + end + + def down + # noop + end +end diff --git a/db/schema.rb b/db/schema.rb index c9aaf80f059..8880ecf4f5c 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20180629191052) do +ActiveRecord::Schema.define(version: 20180702120647) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" diff --git a/lib/gitlab/background_migration/fix_cross_project_label_links.rb b/lib/gitlab/background_migration/fix_cross_project_label_links.rb new file mode 100644 index 00000000000..fa68ba5cca7 --- /dev/null +++ b/lib/gitlab/background_migration/fix_cross_project_label_links.rb @@ -0,0 +1,138 @@ +# frozen_string_literal: true +# rubocop:disable Style/Documentation + +module Gitlab + module BackgroundMigration + class FixCrossProjectLabelLinks + GROUP_NESTED_LEVEL = 10.freeze + + class Project < ActiveRecord::Base + self.table_name = 'projects' + end + + class Label < ActiveRecord::Base + self.table_name = 'labels' + end + + class LabelLink < ActiveRecord::Base + self.table_name = 'label_links' + end + + class Issue < ActiveRecord::Base + self.table_name = 'issues' + end + + class MergeRequest < ActiveRecord::Base + self.table_name = 'merge_requests' + end + + class Namespace < ActiveRecord::Base + self.table_name = 'namespaces' + + def self.groups_with_descendants_ids(start_id, stop_id) + # To isolate migration code, we avoid usage of + # Gitlab::GroupHierarchy#base_and_descendants which already + # does this job better + ids = Namespace.where(type: 'Group', id: Label.where(type: 'GroupLabel').select('distinct group_id')).where(id: start_id..stop_id).pluck(:id) + group_ids = ids + + GROUP_NESTED_LEVEL.times do + ids = Namespace.where(type: 'Group', parent_id: ids).pluck(:id) + break if ids.empty? + + group_ids += ids + end + + group_ids.uniq + end + end + + def perform(start_id, stop_id) + group_ids = Namespace.groups_with_descendants_ids(start_id, stop_id) + project_ids = Project.where(namespace_id: group_ids).select(:id) + + fix_issues(project_ids) + fix_merge_requests(project_ids) + end + + private + + # select IDs of issues which reference a label which is: + # a) a project label of a different project, or + # b) a group label of a different group than issue's project group + def fix_issues(project_ids) + issue_ids = Label + .joins('INNER JOIN label_links ON label_links.label_id = labels.id AND label_links.target_type = \'Issue\' + INNER JOIN issues ON issues.id = label_links.target_id + INNER JOIN projects ON projects.id = issues.project_id') + .where('issues.project_id in (?)', project_ids) + .where('(labels.project_id is not null and labels.project_id != issues.project_id) '\ + 'or (labels.group_id is not null and labels.group_id != projects.namespace_id)') + .select('distinct issues.id') + + Issue.where(id: issue_ids).find_each { |issue| check_resource_labels(issue, issue.project_id) } + end + + # select IDs of MRs which reference a label which is: + # a) a project label of a different project, or + # b) a group label of a different group than MR's project group + def fix_merge_requests(project_ids) + mr_ids = Label + .joins('INNER JOIN label_links ON label_links.label_id = labels.id AND label_links.target_type = \'MergeRequest\' + INNER JOIN merge_requests ON merge_requests.id = label_links.target_id + INNER JOIN projects ON projects.id = merge_requests.target_project_id') + .where('merge_requests.target_project_id in (?)', project_ids) + .where('(labels.project_id is not null and labels.project_id != merge_requests.target_project_id) '\ + 'or (labels.group_id is not null and labels.group_id != projects.namespace_id)') + .select('distinct merge_requests.id') + + MergeRequest.where(id: mr_ids).find_each { |merge_request| check_resource_labels(merge_request, merge_request.target_project_id) } + end + + def check_resource_labels(resource, project_id) + local_labels = available_labels(project_id) + + # get all label links for the given resource (issue/MR) + # which reference a label not included in avaiable_labels + # (other than its project labels and labels of ancestor groups) + cross_labels = LabelLink + .select('label_id, labels.title as title, labels.color as color, label_links.id as label_link_id') + .joins('INNER JOIN labels ON labels.id = label_links.label_id') + .where(target_type: resource.class.name.demodulize, target_id: resource.id) + .where('labels.id not in (?)', local_labels.select(:id)) + + cross_labels.each do |label| + matching_label = local_labels.find {|l| l.title == label.title && l.color == label.color} + + next unless matching_label + + Rails.logger.info "#{resource.class.name.demodulize} #{resource.id}: replacing #{label.label_id} with #{matching_label.id}" + LabelLink.update(label.label_link_id, label_id: matching_label.id) + end + end + + # get all labels available for the project (including + # group labels of ancestor groups) + def available_labels(project_id) + @labels ||= {} + @labels[project_id] ||= Label + .where("(type = 'GroupLabel' and group_id in (?)) or (type = 'ProjectLabel' and id = ?)", + project_group_ids(project_id), + project_id) + end + + def project_group_ids(project_id) + ids = [Project.find(project_id).namespace_id] + + GROUP_NESTED_LEVEL.times do + group = Namespace.find(ids.last) + break unless group.parent_id + + ids << group.parent_id + end + + ids + end + end + end +end diff --git a/spec/lib/gitlab/background_migration/fix_cross_project_label_links_spec.rb b/spec/lib/gitlab/background_migration/fix_cross_project_label_links_spec.rb new file mode 100644 index 00000000000..20af63bc6c8 --- /dev/null +++ b/spec/lib/gitlab/background_migration/fix_cross_project_label_links_spec.rb @@ -0,0 +1,109 @@ +require 'spec_helper' + +describe Gitlab::BackgroundMigration::FixCrossProjectLabelLinks, :migration, schema: 20180702120647 do + let(:namespaces_table) { table(:namespaces) } + let(:projects_table) { table(:projects) } + let(:issues_table) { table(:issues) } + let(:merge_requests_table) { table(:merge_requests) } + let(:labels_table) { table(:labels) } + let(:label_links_table) { table(:label_links) } + + let!(:group1) { namespaces_table.create(id: 10, type: 'Group', name: 'group1', path: 'group1') } + let!(:group2) { namespaces_table.create(id: 20, type: 'Group', name: 'group2', path: 'group2') } + + let!(:project1) { projects_table.create(id: 1, name: 'project1', path: 'group1/project1', namespace_id: 10) } + let!(:project2) { projects_table.create(id: 3, name: 'project2', path: 'group1/project2', namespace_id: 20) } + + let!(:label1) { labels_table.create(id: 1, title: 'bug', color: 'red', group_id: 10, type: 'GroupLabel') } + let!(:label2) { labels_table.create(id: 2, title: 'bug', color: 'red', group_id: 20, type: 'GroupLabel') } + + def create_merge_request(id, project_id) + merge_requests_table.create(id: id, + target_project_id: project_id, + target_branch: 'master', + source_project_id: project_id, + source_branch: 'mr name', + title: "mr name#{id}") + end + + def create_issue(id, project_id) + issues_table.create(id: id, title: "issue#{id}", project_id: project_id) + end + + def create_resource(target_type, id, project_id) + target_type == 'Issue' ? create_issue(id, project_id) : create_merge_request(id, project_id) + end + + shared_examples_for 'resource with cross-project labels' do + it 'updates only cross-project label links which exist in the local project or group' do + create_resource(target_type, 1, 1) + create_resource(target_type, 2, 3) + labels_table.create(id: 3, title: 'bug', color: 'red', project_id: 3, type: 'ProjectLabel') + link = label_links_table.create(label_id: 2, target_type: target_type, target_id: 1) + link2 = label_links_table.create(label_id: 3, target_type: target_type, target_id: 2) + + subject.perform(1, 100) + + expect(link.reload.label_id).to eq(1) + expect(link2.reload.label_id).to eq(3) + end + + it 'ignores cross-project label links if label color is different' do + labels_table.create(id: 3, title: 'bug', color: 'green', group_id: 20, type: 'GroupLabel') + create_resource(target_type, 1, 1) + link = label_links_table.create(label_id: 3, target_type: target_type, target_id: 1) + + subject.perform(1, 100) + + expect(link.reload.label_id).to eq(3) + end + + it 'ignores cross-project label links if label name is different' do + labels_table.create(id: 3, title: 'bug1', color: 'red', group_id: 20, type: 'GroupLabel') + create_resource(target_type, 1, 1) + link = label_links_table.create(label_id: 3, target_type: target_type, target_id: 1) + + subject.perform(1, 100) + + expect(link.reload.label_id).to eq(3) + end + + context 'with nested group' do + before do + namespaces_table.create(id: 11, type: 'Group', name: 'subgroup1', path: 'group1/subgroup1', parent_id: 10) + projects_table.create(id: 2, name: 'subproject1', path: 'group1/subgroup1/subproject1', namespace_id: 11) + create_resource(target_type, 1, 2) + end + + it 'ignores label links referencing ancestor group labels', :nested_groups do + labels_table.create(id: 4, title: 'bug', color: 'red', project_id: 2, type: 'ProjectLabel') + label_links_table.create(label_id: 4, target_type: target_type, target_id: 1) + link = label_links_table.create(label_id: 1, target_type: target_type, target_id: 1) + + subject.perform(1, 100) + + expect(link.reload.label_id).to eq(1) + end + + it 'checks also issues and MRs in subgroups', :nested_groups do + link = label_links_table.create(label_id: 2, target_type: target_type, target_id: 1) + + subject.perform(1, 100) + + expect(link.reload.label_id).to eq(1) + end + end + end + + context 'resource is Issue' do + it_behaves_like 'resource with cross-project labels' do + let(:target_type) { 'Issue' } + end + end + + context 'resource is Merge Request' do + it_behaves_like 'resource with cross-project labels' do + let(:target_type) { 'MergeRequest' } + end + end +end