Merge branch '198338-migrate-mr-mentions-to-db-table' into 'master'

Migrate merge requests mentions to DB table

Closes #198338

See merge request gitlab-org/gitlab!25826

changelogs/unreleased/198338-migrate-mr-mentions-to-db-table.yml

+---
+title: Migrate mentions for merge requests to DB table
+merge_request: 25826
+author:
+type: changed

db/post_migrate/20200211155000_cleanup_empty_merge_request_mentions.rb

+# frozen_string_literal: true
+
+class CleanupEmptyMergeRequestMentions < ActiveRecord::Migration[5.2]
+  DOWNTIME = false
+  BATCH_SIZE = 1_000
+
+  class MergeRequestUserMention < ActiveRecord::Base
+    include EachBatch
+
+    self.table_name = 'merge_request_user_mentions'
+  end
+
+  def up
+    # cleanup merge request user mentions with no actual mentions,
+    # re https://gitlab.com/gitlab-org/gitlab/-/merge_requests/24586#note_285982468
+    MergeRequestUserMention
+      .where(mentioned_users_ids: nil)
+      .where(mentioned_groups_ids: nil)
+      .where(mentioned_projects_ids: nil).each_batch(of: BATCH_SIZE) do |batch|
+      batch.delete_all
+    end
+  end
+
+  def down
+    # no-op
+  end
+end

db/post_migrate/20200211155100_add_temporary_merge_request_with_mentions_index.rb

+# frozen_string_literal: true
+
+class AddTemporaryMergeRequestWithMentionsIndex < ActiveRecord::Migration[5.2]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+  INDEX_CONDITION = "description like '%@%' OR title like '%@%'"
+  INDEX_NAME = 'merge_request_mentions_temp_index'
+
+  disable_ddl_transaction!
+
+  def up
+    # create temporary index for notes with mentions, may take well over 1h
+    add_concurrent_index(:merge_requests, :id, where: INDEX_CONDITION, name: INDEX_NAME)
+  end
+
+  def down
+    remove_concurrent_index(:merge_requests, :id, where: INDEX_CONDITION, name: INDEX_NAME)
+  end
+end

db/post_migrate/20200211155539_migrate_merge_request_mentions_to_db.rb

+# frozen_string_literal: true
+
+class MigrateMergeRequestMentionsToDb < ActiveRecord::Migration[5.2]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+  DELAY = 3.minutes.to_i
+  BATCH_SIZE = 1_000
+  MIGRATION = 'UserMentions::CreateResourceUserMention'
+
+  JOIN = "LEFT JOIN merge_request_user_mentions on merge_requests.id = merge_request_user_mentions.merge_request_id"
+  QUERY_CONDITIONS = "(description like '%@%' OR title like '%@%') AND merge_request_user_mentions.merge_request_id IS NULL"
+
+  disable_ddl_transaction!
+
+  class MergeRequest < ActiveRecord::Base
+    include EachBatch
+
+    self.table_name = 'merge_requests'
+  end
+
+  def up
+    MergeRequest
+      .joins(JOIN)
+      .where(QUERY_CONDITIONS)
+      .each_batch(of: BATCH_SIZE) do |batch, index|
+      range = batch.pluck(Arel.sql('MIN(merge_requests.id)'), Arel.sql('MAX(merge_requests.id)')).first
+      migrate_in(index * DELAY, MIGRATION, ['MergeRequest', JOIN, QUERY_CONDITIONS, false, *range])
+    end
+  end
+
+  def down
+    # no-op
+  end
+end

db/schema.rb

@@ -2634,6 +2634,7 @@ ActiveRecord::Schema.define(version: 2020_03_11_165635) do
     t.index ["head_pipeline_id"], name: "index_merge_requests_on_head_pipeline_id"
     t.index ["id", "merge_jid"], name: "idx_merge_requests_on_id_and_merge_jid", where: "((merge_jid IS NOT NULL) AND (state_id = 4))"
     t.index ["id", "merge_jid"], name: "index_merge_requests_on_id_and_merge_jid", where: "((merge_jid IS NOT NULL) AND ((state)::text = 'locked'::text))"
+    t.index ["id"], name: "merge_request_mentions_temp_index", where: "((description ~~ '%@%'::text) OR ((title)::text ~~ '%@%'::text))"
     t.index ["latest_merge_request_diff_id"], name: "index_merge_requests_on_latest_merge_request_diff_id"
     t.index ["lock_version"], name: "index_merge_requests_on_lock_version", where: "(lock_version IS NULL)"
     t.index ["merge_user_id"], name: "index_merge_requests_on_merge_user_id", where: "(merge_user_id IS NOT NULL)"

ee/spec/lib/gitlab/background_migration/user_mentions/create_resource_user_mention_spec.rb

@@ -59,7 +59,7 @@ describe Gitlab::BackgroundMigration::UserMentions::CreateResourceUserMention, s
                         title_html: "epic title", description: 'simple description')
         end
         let!(:epic3) do
-          epics.create!(iid: 2, group_id: group.id, author_id: author.id, title: "epic title}",
+          epics.create!(iid: 3, group_id: group.id, author_id: author.id, title: "epic title}",
                         title_html: "epic title", description: 'description with an email@example.com and some other @ char here.')
         end
 

lib/gitlab/background_migration/user_mentions/create_resource_user_mention.rb

@@ -8,7 +8,7 @@ module Gitlab
         # Resources that have mentions to be migrated:
         # issue, merge_request, epic, commit, snippet, design
 
-        BULK_INSERT_SIZE = 5000
+        BULK_INSERT_SIZE = 1_000
         ISOLATION_MODULE = 'Gitlab::BackgroundMigration::UserMentions::Models'
 
         def perform(resource_model, join, conditions, with_notes, start_id, end_id)

lib/gitlab/background_migration/user_mentions/models/merge_request.rb

+# frozen_string_literal: true
+# rubocop:disable Style/Documentation
+
+module Gitlab
+  module BackgroundMigration
+    module UserMentions
+      module Models
+        class MergeRequest < ActiveRecord::Base
+          include Concerns::IsolatedMentionable
+          include CacheMarkdownField
+          include Concerns::MentionableMigrationMethods
+
+          attr_mentionable :title, pipeline: :single_line
+          attr_mentionable :description
+          cache_markdown_field :title, pipeline: :single_line
+          cache_markdown_field :description, issuable_state_filter_enabled: true
+
+          self.table_name = 'merge_requests'
+
+          belongs_to :author, class_name: "User"
+          belongs_to :target_project, class_name: "Project"
+          belongs_to :source_project, class_name: "Project"
+
+          alias_attribute :project, :target_project
+
+          def self.user_mention_model
+            Gitlab::BackgroundMigration::UserMentions::Models::MergeRequestUserMention
+          end
+
+          def user_mention_model
+            self.class.user_mention_model
+          end
+
+          def user_mention_resource_id
+            id
+          end
+
+          def user_mention_note_id
+            'NULL'
+          end
+        end
+      end
+    end
+  end
+end

lib/gitlab/background_migration/user_mentions/models/merge_request_user_mention.rb

+# frozen_string_literal: true
+# rubocop:disable Style/Documentation
+
+module Gitlab
+  module BackgroundMigration
+    module UserMentions
+      module Models
+        class MergeRequestUserMention < ActiveRecord::Base
+          self.table_name = 'merge_request_user_mentions'
+
+          def self.resource_foreign_key
+            :merge_request_id
+          end
+        end
+      end
+    end
+  end
+end

spec/lib/gitlab/background_migration/user_mentions/create_resource_user_mention_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+require './db/post_migrate/20200211155539_migrate_merge_request_mentions_to_db'
+
+describe Gitlab::BackgroundMigration::UserMentions::CreateResourceUserMention, schema: 20200211155539 do
+  include MigrationsHelpers
+
+  context 'when migrating data' do
+    let(:users) { table(:users) }
+    let(:namespaces) { table(:namespaces) }
+    let(:projects) { table(:projects) }
+    let(:notes) { table(:notes) }
+
+    let(:author) { users.create!(email: 'author@example.com', notification_email: 'author@example.com', name: 'author', username: 'author', projects_limit: 10, state: 'active') }
+    let(:member) { users.create!(email: 'member@example.com', notification_email: 'member@example.com', name: 'member', username: 'member', projects_limit: 10, state: 'active') }
+    let(:admin) { users.create!(email: 'administrator@example.com', notification_email: 'administrator@example.com', name: 'administrator', username: 'administrator', admin: 1, projects_limit: 10, state: 'active') }
+    let(:john_doe) { users.create!(email: 'john_doe@example.com', notification_email: 'john_doe@example.com', name: 'john_doe', username: 'john_doe', projects_limit: 10, state: 'active') }
+    let(:skipped) { users.create!(email: 'skipped@example.com', notification_email: 'skipped@example.com', name: 'skipped', username: 'skipped', projects_limit: 10, state: 'active') }
+
+    let(:mentioned_users) { [author, member, admin, john_doe, skipped] }
+    let(:mentioned_users_refs) { mentioned_users.map { |u| "@#{u.username}" }.join(' ') }
+
+    let(:group) { namespaces.create!(name: 'test1', path: 'test1', runners_token: 'my-token1', project_creation_level: 1, visibility_level: 20, type: 'Group') }
+    let(:inaccessible_group) { namespaces.create!(name: 'test2', path: 'test2', runners_token: 'my-token2', project_creation_level: 1, visibility_level: 0, type: 'Group') }
+    let(:project) { projects.create!(name: 'gitlab1', path: 'gitlab1', namespace_id: group.id, visibility_level: 0) }
+
+    let(:mentioned_groups) { [group, inaccessible_group] }
+    let(:group_mentions) { [group, inaccessible_group].map { |gr| "@#{gr.path}" }.join(' ') }
+    let(:description_mentions) { "description with mentions #{mentioned_users_refs} and #{group_mentions}" }
+
+    before do
+      # build personal namespaces and routes for users
+      mentioned_users.each { |u| u.becomes(User).save! }
+
+      # build namespaces and routes for groups
+      mentioned_groups.each do |gr|
+        gr.name += '-org'
+        gr.path += '-org'
+        gr.becomes(Namespace).save!
+      end
+    end
+
+    context 'migrate merge request mentions' do
+      let(:merge_requests) { table(:merge_requests) }
+      let(:merge_request_user_mentions) { table(:merge_request_user_mentions) }
+
+      let!(:mr1) do
+        merge_requests.create!(
+          title: "title 1", state_id: 1, target_branch: 'feature1', source_branch: 'master',
+          source_project_id: project.id, target_project_id: project.id, author_id: author.id,
+          description: description_mentions
+        )
+      end
+
+      let!(:mr2) do
+        merge_requests.create!(
+          title: "title 2", state_id: 1, target_branch: 'feature2', source_branch: 'master',
+          source_project_id: project.id, target_project_id: project.id, author_id: author.id,
+          description: 'some description'
+        )
+      end
+
+      let!(:mr3) do
+        merge_requests.create!(
+          title: "title 3", state_id: 1, target_branch: 'feature3', source_branch: 'master',
+          source_project_id: project.id, target_project_id: project.id, author_id: author.id,
+          description: 'description with an email@example.com and some other @ char here.')
+      end
+
+      let(:user_mentions) { merge_request_user_mentions }
+      let(:resource) { merge_request }
+
+      it_behaves_like 'resource mentions migration', MigrateMergeRequestMentionsToDb, MergeRequest
+    end
+  end
+
+  context 'checks no_quote_columns' do
+    it 'has correct no_quote_columns' do
+      expect(Gitlab::BackgroundMigration::UserMentions::Models::MergeRequest.no_quote_columns).to match([:note_id, :merge_request_id])
+    end
+  end
+end

spec/migrations/migrate_merge_request_mentions_to_db_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+require Rails.root.join('db', 'post_migrate', '20200211155539_migrate_merge_request_mentions_to_db')
+
+describe MigrateMergeRequestMentionsToDb, :migration do
+  let(:users) { table(:users) }
+  let(:projects) { table(:projects) }
+  let(:namespaces) { table(:namespaces) }
+  let(:merge_requests) { table(:merge_requests) }
+  let(:merge_request_user_mentions) { table(:merge_request_user_mentions) }
+
+  let(:user) { users.create!(name: 'root', email: 'root@example.com', username: 'root', projects_limit: 0) }
+  let(:group) { namespaces.create!(name: 'group1', path: 'group1', owner_id: user.id, type: 'Group') }
+  let(:project) { projects.create!(name: 'gitlab1', path: 'gitlab1', namespace_id: group.id, visibility_level: 0) }
+
+  # migrateable resources
+  let(:common_args) { { source_branch: 'master', source_project_id: project.id, target_project_id: project.id, author_id: user.id, description: 'mr description with @root mention' } }
+  let!(:resource1) { merge_requests.create!(common_args.merge(title: "title 1", state_id: 1, target_branch: 'feature1')) }
+  let!(:resource2) { merge_requests.create!(common_args.merge(title: "title 2", state_id: 1, target_branch: 'feature2')) }
+  let!(:resource3) { merge_requests.create!(common_args.merge(title: "title 3", state_id: 1, target_branch: 'feature3')) }
+
+  # non-migrateable resources
+  # this merge request is already migrated, as it has a record in the merge_request_user_mentions table
+  let!(:resource4) { merge_requests.create!(common_args.merge(title: "title 3", state_id: 1, target_branch: 'feature3')) }
+  let!(:user_mention) { merge_request_user_mentions.create!(merge_request_id: resource4.id, mentioned_users_ids: [1]) }
+
+  let!(:resource5) { merge_requests.create!(common_args.merge(title: "title 3", description: 'description with no mention', state_id: 1, target_branch: 'feature3')) }
+
+  it_behaves_like 'schedules resource mentions migration', MergeRequest, false
+end

spec/support/shared_examples/lib/gitlab/background_migration/mentions_migration_shared_examples.rb

@@ -80,10 +80,11 @@ shared_examples 'schedules resource mentions migration' do |resource_class, is_f
         migration = described_class::MIGRATION
         join = described_class::JOIN
         conditions = described_class::QUERY_CONDITIONS
+        delay = described_class::DELAY
 
-        expect(migration).to be_scheduled_delayed_migration(2.minutes, resource_class.name, join, conditions, is_for_notes, resource1.id, resource1.id)
-        expect(migration).to be_scheduled_delayed_migration(4.minutes, resource_class.name, join, conditions, is_for_notes, resource2.id, resource2.id)
-        expect(migration).to be_scheduled_delayed_migration(6.minutes, resource_class.name, join, conditions, is_for_notes, resource3.id, resource3.id)
+        expect(migration).to be_scheduled_delayed_migration(1 * delay, resource_class.name, join, conditions, is_for_notes, resource1.id, resource1.id)
+        expect(migration).to be_scheduled_delayed_migration(2 * delay, resource_class.name, join, conditions, is_for_notes, resource2.id, resource2.id)
+        expect(migration).to be_scheduled_delayed_migration(3 * delay, resource_class.name, join, conditions, is_for_notes, resource3.id, resource3.id)
         expect(BackgroundMigrationWorker.jobs.size).to eq 3
       end
     end