Merge branch 'jprovazn-label-links-update' into 'master'
Fix cross-project label references Closes #45539 See merge request gitlab-org/gitlab-ce!20308
This commit is contained in:
commit
f5b12225d8
|
@ -0,0 +1,5 @@
|
|||
---
|
||||
title: Fix cross-project label references.
|
||||
merge_request:
|
||||
author:
|
||||
type: fixed
|
|
@ -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
|
|
@ -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"
|
||||
|
|
|
@ -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
|
|
@ -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
|
Loading…
Reference in New Issue