Merge branch '198323-migrate-snippet-mentions-to-db-table' into 'master'

Migrate mentions for snippets and snippet notes to DB table

See merge request gitlab-org/gitlab!23783

changelogs/unreleased/198323-migrate-snippet-mentions-to-db-table.yml

+---
+title: Migrate mentions for snippet and snippet notes to snippet_user_mentions DB
+  table
+merge_request: 23783
+author:
+type: changed

db/post_migrate/20200127111953_cleanup_empty_snippet_user_mentions.rb

+# frozen_string_literal: true
+
+class CleanupEmptySnippetUserMentions < ActiveRecord::Migration[5.2]
+  DOWNTIME = false
+  BATCH_SIZE = 10_000
+
+  class SnippetUserMention < ActiveRecord::Base
+    include EachBatch
+
+    self.table_name = 'snippet_user_mentions'
+  end
+
+  def up
+    # cleanup snippet user mentions with no actual mentions,
+    # re https://gitlab.com/gitlab-org/gitlab/-/merge_requests/24586#note_285982468
+    SnippetUserMention
+      .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/20200127131953_migrate_snippet_mentions_to_db.rb

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

db/post_migrate/20200127141953_add_temporary_snippet_notes_with_mentions_index.rb

+# frozen_string_literal: true
+
+class AddTemporarySnippetNotesWithMentionsIndex < ActiveRecord::Migration[5.2]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+  INDEX_NAME = 'snippet_mentions_temp_index'
+  INDEX_CONDITION = "note LIKE '%@%'::text AND notes.noteable_type = 'Snippet'"
+
+  disable_ddl_transaction!
+
+  def up
+    # create temporary index for notes with mentions, may take well over 1h
+    add_concurrent_index(:notes, :id, where: INDEX_CONDITION, name: INDEX_NAME)
+  end
+
+  def down
+    remove_concurrent_index(:notes, :id, where: INDEX_CONDITION, name: INDEX_NAME)
+  end
+end

db/post_migrate/20200127151953_migrate_snippet_notes_mentions_to_db.rb

+# frozen_string_literal: true
+
+class MigrateSnippetNotesMentionsToDb < ActiveRecord::Migration[5.2]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+  DELAY = 2.minutes.to_i
+  BATCH_SIZE = 10_000
+  MIGRATION = 'UserMentions::CreateResourceUserMention'
+
+  INDEX_CONDITION = "note LIKE '%@%'::text AND notes.noteable_type = 'Snippet'"
+  QUERY_CONDITIONS = "#{INDEX_CONDITION} AND snippet_user_mentions.snippet_id IS NULL"
+  JOIN = 'INNER JOIN snippets ON snippets.id = notes.noteable_id LEFT JOIN snippet_user_mentions ON notes.id = snippet_user_mentions.note_id'
+
+  disable_ddl_transaction!
+
+  class Note < ActiveRecord::Base
+    include EachBatch
+
+    self.table_name = 'notes'
+  end
+
+  def up
+    Note
+      .joins(JOIN)
+      .where(QUERY_CONDITIONS)
+      .each_batch(of: BATCH_SIZE) do |batch, index|
+      range = batch.pluck(Arel.sql('MIN(notes.id)'), Arel.sql('MAX(notes.id)')).first
+      migrate_in(index * DELAY, MIGRATION, ['Snippet', JOIN, QUERY_CONDITIONS, true, *range])
+    end
+  end
+
+  def down
+    # no-op
+    # temporary index is to be dropped in a different migration in an upcoming release:
+    # https://gitlab.com/gitlab-org/gitlab/issues/196842
+  end
+end

db/schema.rb

@@ -2824,6 +2824,7 @@ ActiveRecord::Schema.define(version: 2020_02_26_162723) do
     t.index ["discussion_id"], name: "index_notes_on_discussion_id"
     t.index ["id"], name: "design_mentions_temp_index", where: "((note ~~ '%@%'::text) AND ((noteable_type)::text = 'DesignManagement::Design'::text))"
     t.index ["id"], name: "epic_mentions_temp_index", where: "((note ~~ '%@%'::text) AND ((noteable_type)::text = 'Epic'::text))"
+    t.index ["id"], name: "snippet_mentions_temp_index", where: "((note ~~ '%@%'::text) AND ((noteable_type)::text = 'Snippet'::text))"
     t.index ["line_code"], name: "index_notes_on_line_code"
     t.index ["note"], name: "index_notes_on_note_trigram", opclass: :gin_trgm_ops, using: :gin
     t.index ["note"], name: "tmp_idx_on_promoted_notes", where: "(((noteable_type)::text = 'Issue'::text) AND (system IS TRUE) AND (note ~~ 'promoted to epic%'::text))"

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

@@ -7,7 +7,7 @@ require './db/post_migrate/20200214174519_remigrate_epic_mentions_to_db'
 require './db/post_migrate/20200214174607_remigrate_epic_notes_mentions_to_db'
 require './db/post_migrate/20200124110831_migrate_design_notes_mentions_to_db'
 
-describe Gitlab::BackgroundMigration::UserMentions::CreateResourceUserMention do
+describe Gitlab::BackgroundMigration::UserMentions::CreateResourceUserMention, schema: 20200214174607 do
   include MigrationsHelpers
 
   context 'when migrating data' do

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

+# frozen_string_literal: true
+# rubocop:disable Style/Documentation
+
+module Gitlab
+  module BackgroundMigration
+    module UserMentions
+      module Models
+        class Snippet < ActiveRecord::Base
+          include IsolatedMentionable
+          include CacheMarkdownField
+          include MentionableMigrationMethods
+
+          attr_mentionable :title, pipeline: :single_line
+          attr_mentionable :description
+          cache_markdown_field :title, pipeline: :single_line
+          cache_markdown_field :description
+
+          self.table_name = 'snippets'
+          self.inheritance_column = :_type_disabled
+
+          belongs_to :author, class_name: "User"
+          belongs_to :project
+
+          def self.user_mention_model
+            Gitlab::BackgroundMigration::UserMentions::Models::SnippetUserMention
+          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/snippet_user_mention.rb

+# frozen_string_literal: true
+# rubocop:disable Style/Documentation
+
+module Gitlab
+  module BackgroundMigration
+    module UserMentions
+      module Models
+        class SnippetUserMention < ActiveRecord::Base
+          self.table_name = 'snippet_user_mentions'
+
+          def self.resource_foreign_key
+            :snippet_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/20200127131953_migrate_snippet_mentions_to_db'
+require './db/post_migrate/20200127151953_migrate_snippet_notes_mentions_to_db'
+
+describe Gitlab::BackgroundMigration::UserMentions::CreateResourceUserMention, schema: 20200127151953 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 snippet mentions' do
+      let(:snippets) { table(:snippets) }
+      let(:snippet_user_mentions) { table(:snippet_user_mentions) }
+
+      let!(:snippet1) { snippets.create!(project_id: project.id, author_id: author.id, title: 'title1', description: description_mentions) }
+      let!(:snippet2) { snippets.create!(project_id: project.id, author_id: author.id, title: 'title2', description: 'some description') }
+      let!(:snippet3) { snippets.create!(project_id: project.id, author_id: author.id, title: 'title3', description: 'description with an email@example.com and some other @ char here.') }
+
+      let(:user_mentions) { snippet_user_mentions }
+      let(:resource) { snippet1 }
+
+      it_behaves_like 'resource mentions migration', MigrateSnippetMentionsToDb, Snippet
+
+      context 'mentions in note' do
+        let!(:note1) { notes.create!(noteable_id: snippet1.id, noteable_type: 'Snippet', project_id: project.id, author_id: author.id, note: description_mentions) }
+        let!(:note2) { notes.create!(noteable_id: snippet1.id, noteable_type: 'Snippet', project_id: project.id, author_id: author.id, note: 'sample note') }
+        let!(:note3) { notes.create!(noteable_id: snippet1.id, noteable_type: 'Snippet', project_id: project.id, author_id: author.id, note: description_mentions, system: true) }
+        # this not does not have actual mentions
+        let!(:note4) { notes.create!(noteable_id: snippet1.id, noteable_type: 'Snippet', project_id: project.id, author_id: author.id, note: 'note3 for an email@somesite.com and some other rando @ ref' ) }
+        # this note points to an innexistent noteable record in snippets table
+        let!(:note5) { notes.create!(noteable_id: snippets.maximum(:id) + 10, noteable_type: 'Snippet', project_id: project.id, author_id: author.id, note: description_mentions) }
+
+        it_behaves_like 'resource notes mentions migration', MigrateSnippetNotesMentionsToDb, Snippet
+      end
+    end
+  end
+
+  context 'checks no_quote_columns' do
+    it 'has correct no_quote_columns' do
+      expect(Gitlab::BackgroundMigration::UserMentions::Models::Snippet.no_quote_columns).to match([:note_id, :snippet_id])
+    end
+  end
+end

spec/migrations/cleanup_empty_snippet_user_mentions_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+require Rails.root.join('db', 'post_migrate', '20200127111953_cleanup_empty_snippet_user_mentions')
+
+describe CleanupEmptySnippetUserMentions, :migration, :sidekiq do
+  let(:users) { table(:users) }
+  let(:projects) { table(:projects) }
+  let(:namespaces) { table(:namespaces) }
+  let(:snippets) { table(:snippets) }
+  let(:snippet_user_mentions) { table(:snippet_user_mentions) }
+  let(:notes) { table(:notes) }
+
+  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) }
+  let(:project) { projects.create!(name: 'gitlab1', path: 'gitlab1', namespace_id: group.id, visibility_level: 0) }
+  let(:snippet) { snippets.create!(title: "title1", title_html: 'title1', description: 'snippet description with @root mention', project_id: project.id, author_id: user.id) }
+
+  let!(:resource1) { notes.create!(note: 'note for @root to check', noteable_id: snippet.id, noteable_type: 'Snippet') }
+  let!(:resource2) { notes.create!(note: 'note for @root to check', noteable_id: snippet.id, noteable_type: 'Snippet', system: true) }
+  let!(:resource3) { notes.create!(note: 'note for @root to check', noteable_id: snippet.id, noteable_type: 'Snippet') }
+
+  # non-migrateable resources
+  # this note is already migrated, as it has a record in the snippet_user_mentions table
+  let!(:resource4) { notes.create!(note: 'note for @root to check', noteable_id: snippet.id, noteable_type: 'Snippet') }
+  let!(:user_mention) { snippet_user_mentions.create!(snippet_id: snippet.id, note_id: resource4.id, mentioned_users_ids: [1]) }
+  # this note points to an innexistent noteable record
+  let!(:resource5) { notes.create!(note: 'note for @root to check', noteable_id: snippets.maximum(:id) + 10, noteable_type: 'Snippet') }
+
+  # these should get cleanup, by the migration
+  let!(:blank_snippet_user_mention1) { snippet_user_mentions.create!(snippet_id: snippet.id, note_id: resource1.id)}
+  let!(:blank_snippet_user_mention2) { snippet_user_mentions.create!(snippet_id: snippet.id, note_id: resource2.id)}
+  let!(:blank_snippet_user_mention3) { snippet_user_mentions.create!(snippet_id: snippet.id, note_id: resource3.id)}
+
+  it 'cleanups blank user mentions' do
+    expect(snippet_user_mentions.count).to eq 4
+
+    migrate!
+
+    expect(snippet_user_mentions.count).to eq 1
+  end
+end

spec/migrations/migrate_snippet_mentions_to_db_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+require Rails.root.join('db', 'post_migrate', '20200127131953_migrate_snippet_mentions_to_db')
+
+describe MigrateSnippetMentionsToDb, :migration, :sidekiq do
+  let(:users) { table(:users) }
+  let(:namespaces) { table(:namespaces) }
+  let(:projects) { table(:projects) }
+  let(:snippets) { table(:snippets) }
+  let(:snippet_user_mentions) { table(:snippet_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) }
+  let(:project) { projects.create!(name: 'gitlab1', path: 'gitlab1', namespace_id: group.id, visibility_level: 0) }
+  let!(:resource1) { snippets.create!(title: "title1", title_html: 'title1', description: 'snippet description with @root mention', project_id: project.id, author_id: user.id) }
+  let!(:resource2) { snippets.create!(title: "title2", title_html: "title2", description: 'snippet description with @group mention', project_id: project.id, author_id: user.id) }
+  let!(:resource3) { snippets.create!(title: "title3", title_html: "title3", description: 'snippet description with @project mention', project_id: project.id, author_id: user.id) }
+
+  # non-migrateable resources
+  # this snippet is already migrated, as it has a record in the snippet_user_mentions table
+  let!(:resource4) { snippets.create!(title: "title4", title_html: "title4", description: 'snippet description with @project mention', project_id: project.id, author_id: user.id) }
+  let!(:user_mention) { snippet_user_mentions.create!(snippet_id: resource4.id, mentioned_users_ids: [1]) }
+  # this snippet has no mentions so should be filtered out
+  let!(:resource5) { snippets.create!(title: "title5", title_html: "title5", description: 'snippet description with no mention', project_id: project.id, author_id: user.id) }
+
+  it_behaves_like 'schedules resource mentions migration', Snippet, false
+end

spec/migrations/migrate_snippet_notes_mentions_to_db_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+require Rails.root.join('db', 'post_migrate', '20200127151953_migrate_snippet_notes_mentions_to_db')
+
+describe MigrateSnippetNotesMentionsToDb, :migration, :sidekiq do
+  let(:users) { table(:users) }
+  let(:projects) { table(:projects) }
+  let(:namespaces) { table(:namespaces) }
+  let(:snippets) { table(:snippets) }
+  let(:snippet_user_mentions) { table(:snippet_user_mentions) }
+  let(:notes) { table(:notes) }
+
+  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) }
+  let(:project) { projects.create!(name: 'gitlab1', path: 'gitlab1', namespace_id: group.id, visibility_level: 0) }
+  let(:snippet) { snippets.create!(title: "title1", title_html: 'title1', description: 'snippet description with @root mention', project_id: project.id, author_id: user.id) }
+
+  let!(:resource1) { notes.create!(note: 'note for @root to check', noteable_id: snippet.id, noteable_type: 'Snippet') }
+  let!(:resource2) { notes.create!(note: 'note for @root to check', noteable_id: snippet.id, noteable_type: 'Snippet', system: true) }
+  let!(:resource3) { notes.create!(note: 'note for @root to check', noteable_id: snippet.id, noteable_type: 'Snippet') }
+
+  # non-migrateable resources
+  # this note is already migrated, as it has a record in the snippet_user_mentions table
+  let!(:resource4) { notes.create!(note: 'note for @root to check', noteable_id: snippet.id, noteable_type: 'Snippet') }
+  let!(:user_mention) { snippet_user_mentions.create!(snippet_id: snippet.id, note_id: resource4.id, mentioned_users_ids: [1]) }
+  # this note points to an innexistent noteable record
+  let!(:resource5) { notes.create!(note: 'note for @root to check', noteable_id: snippets.maximum(:id) + 10, noteable_type: 'Snippet') }
+
+  it_behaves_like 'schedules resource mentions migration', Snippet, true
+end

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

@@ -72,7 +72,7 @@ shared_examples 'schedules resource mentions migration' do |resource_class, is_f
   it 'schedules background migrations' do
     Sidekiq::Testing.fake! do
       Timecop.freeze do
-        resource_count = is_for_notes ? Note.count : resource_class.count
+        resource_count = is_for_notes ? Note.where(noteable_type: resource_class.to_s).count : resource_class.count
         expect(resource_count).to eq 5
 
         migrate!