Merge branch 'jprovazn-label-links-update' into 'master'

Fix cross-project label references

Closes #45539

See merge request gitlab-org/gitlab-ce!20308

5 files changed, 283 insertions, 1 deletions

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