Merge branch 'sync-merge-ref-upon-mergeability-check' into 'master'

Automatically update MR merge-ref along merge status

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

3 files changed, 137 insertions, 18 deletions

diff --git a/app/services/merge_requests/merge_to_ref_service.rb b/app/services/merge_requests/merge_to_ref_service.rb
index 87147d90c32..efe4dcd6255 100644
--- a/app/services/merge_requests/merge_to_ref_service.rb
+++ b/app/services/merge_requests/merge_to_ref_service.rb
@@ -11,6 +11,8 @@ module MergeRequests
   # be executed regardless of the `target_ref` current state).
   #
   class MergeToRefService < MergeRequests::MergeBaseService
+    extend ::Gitlab::Utils::Override
+
     def execute(merge_request)
       @merge_request = merge_request
 
@@ -26,14 +28,18 @@ module MergeRequests
       success(commit_id: commit.id,
               target_id: target_id,
               source_id: source_id)
-    rescue MergeError => error
+    rescue MergeError, ArgumentError => error
       error(error.message)
     end
 
     private
 
+    override :source
+    def source
+      merge_request.diff_head_sha
+    end
+
     def validate!
-      authorization_check!
       error_check!
     end
 
@@ -43,21 +49,13 @@ module MergeRequests
       error =
         if !hooks_validation_pass?(merge_request)
           hooks_validation_error(merge_request)
-        elsif !@merge_request.mergeable_to_ref?
-          "Merge request is not mergeable to #{target_ref}"
-        elsif !source
+        elsif source.blank?
           'No source for merge'
         end
 
       raise_error(error) if error
     end
 
-    def authorization_check!
-      unless Ability.allowed?(current_user, :admin_merge_request, project)
-        raise_error("You are not allowed to merge to this ref")
-      end
-    end
-
     def target_ref
       merge_request.merge_ref_path
     end
diff --git a/app/services/merge_requests/mergeability_check_service.rb b/app/services/merge_requests/mergeability_check_service.rb
new file mode 100644
index 00000000000..9fa50c9448f
--- /dev/null
+++ b/app/services/merge_requests/mergeability_check_service.rb
@@ -0,0 +1,120 @@
+# frozen_string_literal: true
+
+module MergeRequests
+  class MergeabilityCheckService < ::BaseService
+    include Gitlab::Utils::StrongMemoize
+
+    delegate :project, to: :@merge_request
+    delegate :repository, to: :project
+
+    def initialize(merge_request)
+      @merge_request = merge_request
+    end
+
+    # Updates the MR merge_status. Whenever it switches to a can_be_merged state,
+    # the merge-ref is refreshed.
+    #
+    # recheck - When given, it'll enforce a merge-ref refresh if the current merge_status is
+    # can_be_merged or cannot_be_merged and merge-ref is outdated.
+    # Given MergeRequests::RefreshService is called async, it might happen that the target
+    # branch gets updated, but the MergeRequest#merge_status lags behind. So in scenarios
+    # where we need the current state of the merge ref in repository, the `recheck`
+    # argument is required.
+    #
+    # Returns a ServiceResponse indicating merge_status is/became can_be_merged
+    # and the merge-ref is synced. Success in case of being/becoming mergeable,
+    # error otherwise.
+    def execute(recheck: false)
+      return ServiceResponse.error(message: 'Invalid argument') unless merge_request
+      return ServiceResponse.error(message: 'Unsupported operation') if Gitlab::Database.read_only?
+
+      recheck! if recheck
+      update_merge_status
+
+      unless merge_request.can_be_merged?
+        return ServiceResponse.error(message: 'Merge request is not mergeable')
+      end
+
+      unless merge_ref_auto_sync_enabled?
+        return ServiceResponse.error(message: 'Merge ref is outdated due to disabled feature')
+      end
+
+      unless payload.fetch(:merge_ref_head)
+        return ServiceResponse.error(message: 'Merge ref cannot be updated')
+      end
+
+      ServiceResponse.success(payload: payload)
+    end
+
+    private
+
+    attr_reader :merge_request
+
+    def payload
+      strong_memoize(:payload) do
+        {
+          merge_ref_head: merge_ref_head_payload
+        }
+      end
+    end
+
+    def merge_ref_head_payload
+      commit = merge_request.merge_ref_head
+
+      return unless commit
+
+      target_id, source_id = commit.parent_ids
+
+      {
+        commit_id: commit.id,
+        source_id: source_id,
+        target_id: target_id
+      }
+    end
+
+    def update_merge_status
+      return unless merge_request.recheck_merge_status?
+
+      if can_git_merge? && merge_to_ref
+        merge_request.mark_as_mergeable
+      else
+        merge_request.mark_as_unmergeable
+      end
+    end
+
+    def recheck!
+      if !merge_request.recheck_merge_status? && outdated_merge_ref?
+        merge_request.mark_as_unchecked
+      end
+    end
+
+    # Checks if the existing merge-ref is synced with the target branch.
+    #
+    # Returns true if the merge-ref does not exists or is out of sync.
+    def outdated_merge_ref?
+      return false unless merge_ref_auto_sync_enabled?
+      return false unless merge_request.open?
+
+      return true unless ref_head = merge_request.merge_ref_head
+      return true unless target_sha = merge_request.target_branch_sha
+      return true unless source_sha = merge_request.source_branch_sha
+
+      ref_head.parent_ids != [target_sha, source_sha]
+    end
+
+    def can_git_merge?
+      !merge_request.broken? && repository.can_be_merged?(merge_request.diff_head_sha, merge_request.target_branch)
+    end
+
+    def merge_to_ref
+      return true unless merge_ref_auto_sync_enabled?
+
+      result = MergeRequests::MergeToRefService.new(project, merge_request.author).execute(merge_request)
+      result[:status] == :success
+    end
+
+    def merge_ref_auto_sync_enabled?
+      Feature.enabled?(:merge_ref_auto_sync, project, default_enabled: true)
+    end
+  end
+end
diff --git a/app/services/service_response.rb b/app/services/service_response.rb
index 1de30e68d87..f3437ba16de 100644
--- a/app/services/service_response.rb
+++ b/app/services/service_response.rb
@@ -1,19 +1,20 @@
 # frozen_string_literal: true
 
 class ServiceResponse
-  def self.success(message: nil)
-    new(status: :success, message: message)
+  def self.success(message: nil, payload: {})
+    new(status: :success, message: message, payload: payload)
   end
 
-  def self.error(message:, http_status: nil)
-    new(status: :error, message: message, http_status: http_status)
+  def self.error(message:, payload: {}, http_status: nil)
+    new(status: :error, message: message, payload: payload, http_status: http_status)
   end
 
-  attr_reader :status, :message, :http_status
+  attr_reader :status, :message, :http_status, :payload
 
-  def initialize(status:, message: nil, http_status: nil)
+  def initialize(status:, message: nil, payload: {}, http_status: nil)
     self.status = status
     self.message = message
+    self.payload = payload
     self.http_status = http_status
   end
 
@@ -27,5 +28,5 @@ class ServiceResponse
 
   private
 
-  attr_writer :status, :message, :http_status
+  attr_writer :status, :message, :http_status, :payload
 end