Add background task to migrate projects and MR

Projects are migrated first, then its MRs are migrates,
to ensure approver overriding works.

ee/db/post_migrate/20181204040404_migrate_project_approvers.rb

+# frozen_string_literal: true
+
+class MigrateProjectApprovers < ActiveRecord::Migration[5.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+  BATCH_SIZE = 1000
+
+  class Project < ActiveRecord::Base
+    include ::EachBatch
+    self.table_name = 'projects'
+  end
+
+  def up
+    jobs = []
+    Project.each_batch(of: BATCH_SIZE) do |scope, _|
+      jobs << ['MigrateApproverToApprovalRulesInBatch', ['Project', scope.pluck(:id)]]
+    end
+    BackgroundMigration.bulk_perform_async(jobs)
+  end
+
+  def down
+  end
+end

ee/lib/gitlab/background_migration/migrate_approver_to_approval_rules.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module BackgroundMigration
+    # A Project/MergeRequest level migration, aiming to convert existing data
+    # (from approvers, approver_groups and approvals tables)
+    # to new rule based schema.
+    class MigrateApproverToApprovalRules
+      include Gitlab::Utils::StrongMemoize
+
+      class Approver < ActiveRecord::Base
+        self.table_name = 'approvers'
+      end
+
+      class ApproverGroup < ActiveRecord::Base
+        self.table_name = 'approver_groups'
+      end
+
+      class ApprovalMergeRequestRule < ApplicationRecord
+        self.table_name = 'approval_merge_request_rules'
+
+        include Gitlab::Utils::StrongMemoize
+
+        belongs_to :merge_request
+        scope :code_owner, -> { where(code_owner: true) }
+        scope :regular, -> { where(code_owner: false) } # Non code owner rule
+
+        has_and_belongs_to_many :users
+        has_and_belongs_to_many :groups, class_name: 'Group', join_table: :approval_merge_request_rules_groups
+        has_and_belongs_to_many :approvals # This is only populated after merge request is merged
+        has_many :approved_approvers, through: :approvals, source: :user
+
+        def project
+          merge_request.target_project
+        end
+
+        # Users who are eligible to approve, including specified group members.
+        # Excludes the author if 'self-approval' isn't explicitly
+        # enabled on project settings.
+        # @return [Array<User>]
+        def approvers
+          strong_memoize(:approvers) do
+            scope = User.from_union(
+              [
+                users,
+                User.joins(:group_members).where(members: { source_id: groups })
+              ]
+            )
+
+            if merge_request.author && !project.merge_requests_author_approval?
+              scope = scope.where.not(id: merge_request.author)
+            end
+
+            scope
+          end
+        end
+
+        def sync_approvals
+          # Before being merged, approvals are dynamically calculated instead of being persisted in the db.
+          return unless merge_request.merged?
+
+          self.approvals = merge_request.approvals.where(user_id: approvers.map(&:id))
+        end
+      end
+
+      class ApprovalProjectRule < ActiveRecord::Base
+        self.table_name = 'approval_project_rules'
+
+        belongs_to :project
+        has_and_belongs_to_many :users
+        has_and_belongs_to_many :groups, class_name: 'Group', join_table: :approval_project_rules_groups
+
+        scope :regular, -> { all }
+      end
+
+      class MergeRequest < ActiveRecord::Base
+        self.table_name = 'merge_requests'
+        include ::EachBatch
+
+        belongs_to :target_project, class_name: "Project"
+        belongs_to :author, class_name: "User"
+        has_many :approval_rules, class_name: 'ApprovalMergeRequestRule'
+        has_many :approvals
+
+        def approvers
+          Approver.where(target_type: 'MergeRequest', target_id: id)
+        end
+
+        def approver_groups
+          ApproverGroup.where(target_type: 'MergeRequest', target_id: id)
+        end
+
+        def merged?
+          state == 'merged'
+        end
+
+        def finalize_approvals
+          return unless merged?
+
+          copy_project_approval_rules unless approval_rules.regular.exists?
+
+          approval_rules.reload.each(&:sync_approvals)
+        end
+
+        private
+
+        def copy_project_approval_rules
+          target_project.approval_rules.each do |project_rule|
+            rule = approval_rules.create!(project_rule.attributes.slice('approvals_required', 'name'))
+            rule.users = project_rule.users
+            rule.groups = project_rule.groups
+          end
+        end
+      end
+
+      class Project < ActiveRecord::Base
+        self.table_name = 'projects'
+
+        has_many :merge_requests, foreign_key: 'target_project_id', inverse_of: :target_project
+        has_many :approval_rules, class_name: 'ApprovalProjectRule'
+
+        def approvers
+          Approver.where(target_type: 'Project', target_id: id)
+        end
+
+        def approver_groups
+          ApproverGroup.where(target_type: 'Project', target_id: id)
+        end
+      end
+
+      class User < ActiveRecord::Base
+        include FromUnion
+
+        self.table_name = 'users'
+
+        has_many :group_members, -> { where(requested_at: nil) }, source: 'GroupMember'
+      end
+
+      ALLOWED_TARGET_TYPES = %w{MergeRequest Project}.freeze
+
+      # @param target_type [String] class of target, either 'MergeRequest' or 'Project'
+      # @param target_id [Integer] id of target
+      def perform(target_type, target_id)
+        @target_type = target_type
+        @target_id = target_id
+
+        raise "Incorrect target_type #{target_type}" unless ALLOWED_TARGET_TYPES.include?(@target_type)
+
+        case target
+        when MergeRequest
+          handle_merge_request
+        when Project
+          handle_project
+        end
+      end
+
+      private
+
+      def handle_merge_request
+        ActiveRecord::Base.transaction do
+          sync_rule
+          target.finalize_approvals if target.merged?
+        end
+      end
+
+      def handle_project
+        ActiveRecord::Base.transaction do
+          sync_rule
+        end
+
+        schedule_to_migrate_merge_requests(target)
+      end
+
+      # Sync users and groups on rule
+      def sync_rule
+        unless approvers_exists?
+          target.approval_rules.regular.delete_all
+          return
+        end
+
+        rule = find_or_create_rule
+
+        rule.user_ids = target.approvers.pluck(:user_id)
+        rule.group_ids = target.approver_groups.pluck(:group_id)
+      end
+
+      def schedule_to_migrate_merge_requests(project)
+        jobs = []
+        project.merge_requests.each_batch do |scope, _|
+          jobs << ['MigrateApproverToApprovalRulesInBatch', ['MergeRequest', scope.pluck(:id)]]
+        end
+        BackgroundMigrationWorker.bulk_perform_async(jobs)
+      end
+
+      def target
+        strong_memoize(:target) do
+          case @target_type
+          when 'MergeRequest'
+            MergeRequest.find_by(id: @target_id)
+          when 'Project'
+            Project.find_by(id: @target_id)
+          end
+        end
+      end
+
+      def find_or_create_rule
+        rule = target.approval_rules.find_or_initialize_by(name: 'Default')
+
+        unless rule.persisted?
+          rule.approvals_required = target.approvals_before_merge || 0
+          rule.save!
+        end
+
+        rule
+      end
+
+      def approvers_exists?
+        target.approvers.to_a.any? || target.approver_groups.to_a.any?
+      end
+    end
+  end
+end

ee/lib/gitlab/background_migration/migrate_approver_to_approval_rules_in_batch.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module BackgroundMigration
+    class MigrateApproverToApprovalRulesInBatch
+      def perform(target_type, target_ids)
+        target_ids.each do |target_id|
+          MigrateApproverToApprovalRules.new.perform(target_type, target_id)
+        end
+      end
+    end
+  end
+end

ee/spec/lib/gitlab/background_migration/migrate_approver_to_approval_rules_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::BackgroundMigration::MigrateApproverToApprovalRules do
+  def create_skip_sync(*args)
+    build(*args) do |record|
+      allow(record).to receive(:schedule_approval_migration)
+      record.save!
+    end
+  end
+
+  def create_member_in(member, *populate_in)
+    if populate_in.include?(:old_schema)
+      case member
+      when User
+        create_skip_sync(:approver, target_type: target_type, target_id: target.id, user_id: member.id)
+      when Group
+        create_skip_sync(:approver_group, target_type: target_type, target_id: target.id, group_id: member.id)
+      end
+    end
+
+    if populate_in.include?(:new_schema)
+      approval_rule.add_member(member)
+    end
+  end
+
+  context 'sync approval rule and its members' do
+    shared_examples 'sync approval member' do
+      let(:user1) { create(:user) }
+      let(:user2) { create(:user) }
+      let(:group1) { create(:group) }
+      let(:group2) { create(:group) }
+
+      context 'when member in old schema but not in new schema' do
+        before do
+          create_member_in(user1, :old_schema)
+          create_member_in(group1, :old_schema)
+        end
+
+        it 'creates in new schema' do
+          expect do
+            described_class.new.perform(target_type, target.id)
+          end.to change { approval_rule.users.count }.by(1)
+            .and change { approval_rule.groups.count }.by(1)
+
+          approval_rule = target.approval_rules.first
+
+          expect(approval_rule.approvals_required).to eq(0)
+          expect(approval_rule.name).to eq('Default')
+
+          expect(approval_rule.users).to contain_exactly(user1)
+          expect(approval_rule.groups).to contain_exactly(group1)
+        end
+      end
+
+      context 'when member not in old schema but in new schema' do
+        before do
+          create_member_in(user1, :new_schema)
+          create_member_in(user2, :old_schema, :new_schema)
+          create_member_in(group1, :new_schema)
+          create_member_in(group2, :old_schema, :new_schema)
+        end
+
+        it 'removes in new schema' do
+          expect do
+            described_class.new.perform(target_type, target.id)
+          end.to change { approval_rule.users.count }.by(-1)
+            .and change { approval_rule.groups.count }.by(-1)
+
+          approval_rule = target.approval_rules.first
+
+          expect(approval_rule.users).to contain_exactly(user2)
+          expect(approval_rule.groups).to contain_exactly(group2)
+        end
+      end
+    end
+
+    context 'merge request' do
+      let(:target) { create(:merge_request) }
+      let(:target_type) { 'MergeRequest' }
+      let(:approval_rule) { create(:approval_merge_request_rule, merge_request: target) }
+
+      it_behaves_like 'sync approval member'
+
+      context 'when approver is no longer overwritten' do
+        before do
+          create_member_in(create(:user), :new_schema)
+          create_member_in(create(:group), :new_schema)
+        end
+
+        it 'removes rule' do
+          expect do
+            described_class.new.perform(target_type, target.id)
+          end.to change { approval_rule.users.count }.by(-1)
+            .and change { approval_rule.groups.count }.by(-1)
+
+          expect(target.approval_rules.exists?(approval_rule.id)).to eq(false)
+        end
+      end
+    end
+
+    context 'project' do
+      let(:target) { create(:project) }
+      let(:target_type) { 'Project' }
+      let(:approval_rule) { create(:approval_project_rule, project: target) }
+
+      it_behaves_like 'sync approval member'
+
+      context 'when project contains some merge requests' do
+        let!(:merge_request) { create(:merge_request, source_project: target, target_project: target) }
+
+        it 'schedules migrations for all its merge requests' do
+          expect(BackgroundMigrationWorker).to receive(:bulk_perform_async).with([['MigrateApproverToApprovalRulesInBatch', ["MergeRequest", [merge_request.id]]]])
+
+          described_class.new.perform(target.class.name, target.id)
+        end
+      end
+    end
+  end
+
+  # Copied and modified from merge_request_spec.rb
+  describe '#finalize_approvals' do
+    let(:project) { create(:project, :repository) }
+    subject(:merge_request) { create(:merge_request, source_project: project, target_project: project) }
+    let(:target) { merge_request }
+
+    let!(:member1) { create(:user) }
+    let!(:member2) { create(:user) }
+    let!(:member3) { create(:user) }
+    let!(:group1) { create(:group) }
+    let!(:group2) { create(:group) }
+    let!(:group1_member) { create(:user) }
+    let!(:group2_member) { create(:user) }
+    let!(:approval1) { create(:approval, merge_request: subject, user: member1) }
+    let!(:approval2) { create(:approval, merge_request: subject, user: member3) }
+    let!(:approval3) { create(:approval, merge_request: subject, user: group1_member) }
+    let!(:approval4) { create(:approval, merge_request: subject, user: group2_member) }
+
+    before do
+      group1.add_guest(group1_member)
+      group2.add_guest(group2_member)
+
+      rule = create(:approval_project_rule, project: project, name: 'foo', approvals_required: 12)
+
+      rule.users = [member1, member2]
+      rule.groups << group1
+    end
+
+    context 'when without MR rules (project rule not overwritten)' do
+      it 'does nothing if unmerged' do
+        expect do
+          described_class.new.perform(target.class.name, target.id)
+        end.not_to change { ApprovalMergeRequestRule.count }
+
+        expect(approval1.approval_rules).to be_empty
+        expect(approval2.approval_rules).to be_empty
+        expect(approval3.approval_rules).to be_empty
+        expect(approval4.approval_rules).to be_empty
+      end
+
+      context 'when merged' do
+        subject(:merge_request) { create(:merged_merge_request, source_project: project, target_project: project) }
+
+        it 'copies project rules to MR' do
+          expect do
+            described_class.new.perform(target.class.name, target.id)
+          end.to change { ApprovalMergeRequestRule.count }.by(1)
+
+          rule = subject.approval_rules.first
+
+          expect(rule.name).to eq('foo')
+          expect(rule.approvals_required).to eq(12)
+
+          expect(rule.users).to contain_exactly(member1, member2)
+          expect(rule.groups).to contain_exactly(group1)
+
+          rule = subject.approval_rules.first
+
+          expect(approval1.approval_rules).to contain_exactly(rule)
+          expect(approval2.approval_rules).to be_empty
+          expect(approval3.approval_rules).to contain_exactly(rule)
+          expect(approval4.approval_rules).to be_empty
+        end
+      end
+    end
+
+    context 'when with MR approver exists (project rule overwritten)' do
+      before do
+        create_skip_sync(:approver, target: subject, user: member2)
+        create_skip_sync(:approver, target: subject, user: member3)
+        create_skip_sync(:approver_group, target: subject, group: group2)
+
+        merge_request.update(approvals_before_merge: 32)
+      end
+
+      it 'does not call finalize_approvals if unmerged' do
+        expect do
+          described_class.new.perform(target.class.name, target.id)
+        end.to change { ApprovalMergeRequestRule.count }.by(1)
+
+        expect(approval1.approval_rules).to be_empty
+        expect(approval2.approval_rules).to be_empty
+        expect(approval3.approval_rules).to be_empty
+        expect(approval4.approval_rules).to be_empty
+      end
+
+      context 'when merged' do
+        subject(:merge_request) { create(:merged_merge_request, source_project: project, target_project: project) }
+
+        it 'does not copy project rules, and updates approval mapping with MR rules' do
+          expect(subject).not_to receive(:copy_project_approval_rules)
+
+          expect do
+            described_class.new.perform(target.class.name, target.id)
+          end.to change { ApprovalMergeRequestRule.count }.by(1)
+
+          rule = subject.approval_rules.first
+
+          expect(rule.name).to eq('Default')
+          expect(rule.approvals_required).to eq(32)
+
+          expect(rule.users).to contain_exactly(member2, member3)
+          expect(rule.groups).to contain_exactly(group2)
+
+          expect(approval1.approval_rules).to be_empty
+          expect(approval2.approval_rules).to contain_exactly(rule)
+          expect(approval3.approval_rules).to be_empty
+          expect(approval4.approval_rules).to contain_exactly(rule)
+        end
+      end
+    end
+  end
+end

ee/spec/migrations/migrate_project_approvers_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+require Rails.root.join('ee', 'db', 'post_migrate', '20181204040404_migrate_project_approvers.rb')
+
+describe MigrateProjectApprovers, :migration do
+  let(:migration) { described_class.new }
+
+  describe '#up' do
+    let(:projects) { table(:projects) }
+    let(:namespaces) { table(:namespaces) }
+    let(:approvers) { table(:approvers) }
+    let(:approval_project_rules) { table(:approval_project_rules) }
+    let(:approval_project_rules_users) { table(:approval_project_rules_users) }
+    let(:users) { table(:users) }
+
+    before do
+      namespaces.create(id: 1, name: 'gitlab-org', path: 'gitlab-org')
+      projects.create!(id: 123, name: 'gitlab1', path: 'gitlab1', namespace_id: 1)
+      projects.create!(id: 124, name: 'gitlab2', path: 'gitlab2', namespace_id: 1)
+
+      users.create(id: 1, name: 'user1', email: 'user1@example.com', projects_limit: 0)
+      users.create(id: 2, name: 'user2', email: 'user2@example.com', projects_limit: 0)
+
+      approvers.create!(target_id: 123, target_type: 'Project', user_id: 1)
+      approvers.create!(target_id: 124, target_type: 'Project', user_id: 2)
+    end
+
+    it 'creates approval rules and its associations' do
+      migrate!
+
+      expect(approval_project_rules.pluck(:project_id)).to eq([123, 124])
+
+      rule_ids = approval_project_rules.pluck(:id)
+
+      expect(approval_project_rules_users.where(approval_project_rule_id: rule_ids.first).pluck(:user_id)).to contain_exactly(1)
+      expect(approval_project_rules_users.where(approval_project_rule_id: rule_ids.last).pluck(:user_id)).to contain_exactly(2)
+    end
+  end
+end