Merge branch 'id-refresh-only-existing-mrs' into 'master'

Refresh only existing MRs on push

See merge request gitlab-org/gitlab!29420

app/services/git/branch_push_service.rb

@@ -36,6 +36,8 @@ module Git
     # Update merge requests that may be affected by this push. A new branch
     # could cause the last commit of a merge request to change.
     def enqueue_update_mrs
+      return if params[:merge_request_branches]&.exclude?(branch_name)
+
       UpdateMergeRequestsWorker.perform_async(
         project.id,
         current_user.id,

app/services/git/process_ref_changes_service.rb

@@ -42,6 +42,7 @@ module Git
       push_service_class = push_service_class_for(ref_type)
 
       create_bulk_push_event = changes.size > Gitlab::CurrentSettings.push_event_activities_limit
+      merge_request_branches = merge_request_branches_for(changes)
 
       changes.each do |change|
         push_service_class.new(
@@ -49,6 +50,7 @@ module Git
           current_user,
           change: change,
           push_options: params[:push_options],
+          merge_request_branches: merge_request_branches,
           create_pipelines: change[:index] < PIPELINE_PROCESS_LIMIT || Feature.enabled?(:git_push_create_all_pipelines, project),
           execute_project_hooks: execute_project_hooks,
           create_push_event: !create_bulk_push_event
@@ -71,5 +73,11 @@ module Git
 
       Git::BranchPushService
     end
+
+    def merge_request_branches_for(changes)
+      return if Feature.disabled?(:refresh_only_existing_merge_requests_on_push, default_enabled: true)
+
+      @merge_requests_branches ||= MergeRequests::PushedBranchesService.new(project, current_user, changes: changes).execute
+    end
   end
 end

app/services/merge_requests/pushed_branches_service.rb

+# frozen_string_literal: true
+
+module MergeRequests
+  class PushedBranchesService < MergeRequests::BaseService
+    include ::Gitlab::Utils::StrongMemoize
+
+    # Skip moving this logic into models since it's too specific
+    # rubocop: disable CodeReuse/ActiveRecord
+    def execute
+      return [] if branch_names.blank?
+
+      source_branches = project.source_of_merge_requests.opened
+        .from_source_branches(branch_names).pluck(:source_branch)
+
+      target_branches = project.merge_requests.opened
+        .by_target_branch(branch_names).distinct.pluck(:target_branch)
+
+      source_branches.concat(target_branches).to_set
+    end
+    # rubocop: enable CodeReuse/ActiveRecord
+
+    private
+
+    def branch_names
+      strong_memoize(:branch_names) do
+        params[:changes].map do |change|
+          Gitlab::Git.branch_name(change[:ref])
+        end.compact
+      end
+    end
+  end
+end

changelogs/unreleased/id-refresh-only-existing-mrs.yml

+---
+title: Refresh only existing MRs on push
+merge_request: 29420
+author:
+type: performance

spec/services/git/process_ref_changes_service_spec.rb

@@ -160,6 +160,49 @@ describe Git::ProcessRefChangesService do
     let(:ref_prefix) { 'refs/heads' }
 
     it_behaves_like 'service for processing ref changes', Git::BranchPushService
+
+    context 'when there are merge requests associated with branches' do
+      let(:tag_changes) do
+        [
+          { index: 0, oldrev: Gitlab::Git::BLANK_SHA, newrev: '789012', ref: "refs/tags/v10.0.0" }
+        ]
+      end
+      let(:branch_changes) do
+        [
+          { index: 0, oldrev: Gitlab::Git::BLANK_SHA, newrev: '789012', ref: "#{ref_prefix}/create1" },
+          { index: 1, oldrev: Gitlab::Git::BLANK_SHA, newrev: '789013', ref: "#{ref_prefix}/create2" },
+          { index: 2, oldrev: Gitlab::Git::BLANK_SHA, newrev: '789014', ref: "#{ref_prefix}/create3" }
+        ]
+      end
+      let(:git_changes) { double(branch_changes: branch_changes, tag_changes: tag_changes) }
+
+      it 'schedules job for existing merge requests' do
+        expect_next_instance_of(MergeRequests::PushedBranchesService) do |service|
+          expect(service).to receive(:execute).and_return(%w(create1 create2))
+        end
+
+        expect(UpdateMergeRequestsWorker).to receive(:perform_async)
+          .with(project.id, user.id, Gitlab::Git::BLANK_SHA, '789012', "#{ref_prefix}/create1").ordered
+        expect(UpdateMergeRequestsWorker).to receive(:perform_async)
+          .with(project.id, user.id, Gitlab::Git::BLANK_SHA, '789013', "#{ref_prefix}/create2").ordered
+        expect(UpdateMergeRequestsWorker).not_to receive(:perform_async)
+          .with(project.id, user.id, Gitlab::Git::BLANK_SHA, '789014', "#{ref_prefix}/create3").ordered
+
+        subject.execute
+      end
+
+      context 'refresh_only_existing_merge_requests_on_push disabled' do
+        before do
+          stub_feature_flags(refresh_only_existing_merge_requests_on_push: false)
+        end
+
+        it 'refreshes all merge requests' do
+          expect(UpdateMergeRequestsWorker).to receive(:perform_async).exactly(3).times
+
+          subject.execute
+        end
+      end
+    end
   end
 
   context 'tag changes' do

spec/services/merge_requests/pushed_branches_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe MergeRequests::PushedBranchesService do
+  let(:project) { create(:project) }
+  let!(:service) { described_class.new(project, nil, changes: pushed_branches) }
+
+  context 'when branches pushed' do
+    let(:pushed_branches) do
+      %w(branch1 branch2 extra1 extra2 extra3).map do |branch|
+        { ref: "refs/heads/#{branch}" }
+      end
+    end
+
+    it 'returns only branches which have a merge request' do
+      create(:merge_request, source_branch: 'branch1', source_project: project)
+      create(:merge_request, source_branch: 'branch2', source_project: project)
+      create(:merge_request, target_branch: 'branch2', source_project: project)
+      create(:merge_request, :closed, target_branch: 'extra1', source_project: project)
+      create(:merge_request, source_branch: 'extra2')
+
+      expect(service.execute).to contain_exactly('branch1', 'branch2')
+    end
+  end
+
+  context 'when tags pushed' do
+    let(:pushed_branches) do
+      %w(v10.0.0 v11.0.2 v12.1.0).map do |branch|
+        { ref: "refs/tags/#{branch}" }
+      end
+    end
+
+    it 'returns empty result without any SQL query performed' do
+      control_count = ActiveRecord::QueryRecorder.new do
+        expect(service.execute).to be_empty
+      end.count
+
+      expect(control_count).to be_zero
+    end
+  end
+end

spec/workers/post_receive_spec.rb

@@ -352,6 +352,9 @@ describe PostReceive do
 
     it "enqueues a UpdateMergeRequestsWorker job" do
       allow(Project).to receive(:find_by).and_return(project)
+      expect_next_instance_of(MergeRequests::PushedBranchesService) do |service|
+        expect(service).to receive(:execute).and_return(%w(tést))
+      end
 
       expect(UpdateMergeRequestsWorker).to receive(:perform_async).with(project.id, project.owner.id, any_args)