diff options
Diffstat (limited to 'app/services/merge_requests')
| -rw-r--r-- | app/services/merge_requests/base_service.rb | 38 | ||||
| -rw-r--r-- | app/services/merge_requests/create_ref_service.rb | 130 | ||||
| -rw-r--r-- | app/services/merge_requests/merge_base_service.rb | 7 | ||||
| -rw-r--r-- | app/services/merge_requests/merge_service.rb | 63 | ||||
| -rw-r--r-- | app/services/merge_requests/merge_strategies/from_source_branch.rb | 112 | ||||
| -rw-r--r-- | app/services/merge_requests/merge_strategies/strategy_error.rb | 7 | ||||
| -rw-r--r-- | app/services/merge_requests/merge_to_ref_service.rb | 19 | ||||
| -rw-r--r-- | app/services/merge_requests/squash_service.rb | 20 |
8 files changed, 342 insertions, 54 deletions
diff --git a/app/services/merge_requests/base_service.rb b/app/services/merge_requests/base_service.rb index aaa91548d19..0fc85675e49 100644 --- a/app/services/merge_requests/base_service.rb +++ b/app/services/merge_requests/base_service.rb @@ -4,6 +4,7 @@ module MergeRequests class BaseService < ::IssuableBaseService extend ::Gitlab::Utils::Override include MergeRequests::AssignsMergeParams + include MergeRequests::ErrorLogger delegate :repository, to: :project @@ -175,10 +176,21 @@ module MergeRequests params.delete(:allow_collaboration) end + filter_locked_labels(merge_request) filter_reviewer(merge_request) filter_suggested_reviewers end + # Filter out any locked labels that are requested to be removed. + # Only supported for merged MRs. + def filter_locked_labels(merge_request) + return unless params[:remove_label_ids].present? + return unless merge_request.merged? + return unless Feature.enabled?(:enforce_locked_labels_on_merge, merge_request.project, type: :ops) + + params[:remove_label_ids] -= labels_service.filter_locked_labels_ids_in_param(:remove_label_ids) + end + def filter_reviewer(merge_request) return if params[:reviewer_ids].blank? @@ -260,32 +272,6 @@ module MergeRequests end end - def log_error(exception:, message:, save_message_on_model: false) - reference = merge_request.to_reference(full: true) - data = { - class: self.class.name, - message: message, - merge_request_id: merge_request.id, - merge_request: reference, - save_message_on_model: save_message_on_model - } - - if exception - Gitlab::ApplicationContext.with_context(user: current_user) do - Gitlab::ErrorTracking.track_exception(exception, data) - end - - data[:"exception.message"] = exception.message - end - - # TODO: Deprecate Gitlab::GitLogger since ErrorTracking should suffice: - # https://gitlab.com/gitlab-org/gitlab/-/issues/216379 - data[:message] = "#{self.class.name} error (#{reference}): #{message}" - Gitlab::GitLogger.error(data) - - merge_request.update(merge_error: message) if save_message_on_model - end - def trigger_merge_request_reviewers_updated(merge_request) GraphqlTriggers.merge_request_reviewers_updated(merge_request) end diff --git a/app/services/merge_requests/create_ref_service.rb b/app/services/merge_requests/create_ref_service.rb new file mode 100644 index 00000000000..e0f10183bac --- /dev/null +++ b/app/services/merge_requests/create_ref_service.rb @@ -0,0 +1,130 @@ +# frozen_string_literal: true + +module MergeRequests + # CreateRefService creates or overwrites a ref under "refs/merge-requests/" + # with a commit for the merged result. + class CreateRefService + include Gitlab::Utils::StrongMemoize + + CreateRefError = Class.new(StandardError) + + def initialize( + current_user:, merge_request:, target_ref:, first_parent_ref:, + source_sha: nil, merge_commit_message: nil) + + @current_user = current_user + @merge_request = merge_request + @initial_source_sha = source_sha + @target_ref = target_ref + @merge_commit_message = merge_commit_message + @first_parent_sha = target_project.commit(first_parent_ref)&.sha + end + + def execute + commit_sha = initial_source_sha # the SHA to be at HEAD of target_ref + source_sha = initial_source_sha # the SHA to be the merged result of the source (minus the merge commit) + expected_old_oid = "" # the SHA we expect target_ref to be at prior to an update (an optimistic lock) + + # TODO: Update this message with the removal of FF merge_trains_create_ref_service and update tests + # This is for compatibility with MergeToRefService during the rollout. + return ServiceResponse.error(message: '3:Invalid merge source') unless first_parent_sha.present? + + commit_sha, source_sha, expected_old_oid = maybe_squash!(commit_sha, source_sha, expected_old_oid) + commit_sha, source_sha, expected_old_oid = maybe_rebase!(commit_sha, source_sha, expected_old_oid) + commit_sha, source_sha = maybe_merge!(commit_sha, source_sha, expected_old_oid) + + ServiceResponse.success( + payload: { + commit_sha: commit_sha, + target_sha: first_parent_sha, + source_sha: source_sha + } + ) + rescue CreateRefError => error + ServiceResponse.error(message: error.message) + end + + private + + attr_reader :current_user, :merge_request, :target_ref, :first_parent_sha, :initial_source_sha + + delegate :target_project, to: :merge_request + delegate :repository, to: :target_project + + def maybe_squash!(commit_sha, source_sha, expected_old_oid) + if merge_request.squash_on_merge? + squash_result = MergeRequests::SquashService.new( + merge_request: merge_request, + current_user: current_user, + commit_message: squash_commit_message + ).execute + raise CreateRefError, squash_result[:message] if squash_result[:status] == :error + + commit_sha = squash_result[:squash_sha] + source_sha = commit_sha + end + + # squash does not overwrite target_ref, so expected_old_oid remains the same + [commit_sha, source_sha, expected_old_oid] + end + + def maybe_rebase!(commit_sha, source_sha, expected_old_oid) + if target_project.ff_merge_must_be_possible? + commit_sha = safe_gitaly_operation do + repository.rebase_to_ref( + current_user, + source_sha: source_sha, + target_ref: target_ref, + first_parent_ref: first_parent_sha + ) + end + + source_sha = commit_sha + expected_old_oid = commit_sha + end + + [commit_sha, source_sha, expected_old_oid] + end + + def maybe_merge!(commit_sha, source_sha, expected_old_oid) + unless target_project.merge_requests_ff_only_enabled + commit_sha = safe_gitaly_operation do + repository.merge_to_ref( + current_user, + source_sha: source_sha, + target_ref: target_ref, + message: merge_commit_message, + first_parent_ref: first_parent_sha, + branch: nil, + expected_old_oid: expected_old_oid + ) + end + commit = target_project.commit(commit_sha) + _, source_sha = commit.parent_ids + end + + [commit_sha, source_sha] + end + + def safe_gitaly_operation + yield + rescue Gitlab::Git::PreReceiveError, Gitlab::Git::CommandError, ArgumentError => error + raise CreateRefError, error.message + end + + def squash_commit_message + merge_request.merge_params['squash_commit_message'].presence || + merge_request.default_squash_commit_message(user: current_user) + end + strong_memoize_attr :squash_commit_message + + def merge_commit_message + return @merge_commit_message if @merge_commit_message.present? + + @merge_commit_message = ( + merge_request.merge_params['commit_message'].presence || + merge_request.default_merge_commit_message(user: current_user) + ) + end + end +end diff --git a/app/services/merge_requests/merge_base_service.rb b/app/services/merge_requests/merge_base_service.rb index 2a3c1e8bc26..fa0a4f808e2 100644 --- a/app/services/merge_requests/merge_base_service.rb +++ b/app/services/merge_requests/merge_base_service.rb @@ -60,8 +60,11 @@ module MergeRequests end def squash_sha! - params[:merge_request] = merge_request - squash_result = ::MergeRequests::SquashService.new(project: project, current_user: current_user, params: params).execute + squash_result = ::MergeRequests::SquashService.new( + merge_request: merge_request, + current_user: current_user, + commit_message: params[:squash_commit_message] + ).execute case squash_result[:status] when :success diff --git a/app/services/merge_requests/merge_service.rb b/app/services/merge_requests/merge_service.rb index 5e41375e7a0..1398a6dbb67 100644 --- a/app/services/merge_requests/merge_service.rb +++ b/app/services/merge_requests/merge_service.rb @@ -16,6 +16,8 @@ module MergeRequests delegate :merge_jid, :state, to: :@merge_request def execute(merge_request, options = {}) + return execute_v2(merge_request, options) if Feature.enabled?(:refactor_merge_service, project) + if project.merge_requests_ff_only_enabled && !self.is_a?(FfMergeService) FfMergeService.new(project: project, current_user: current_user, params: params).execute(merge_request) return @@ -45,11 +47,40 @@ module MergeRequests exclusive_lease(merge_request.id).cancel end + def execute_v2(merge_request, options = {}) + return if merge_request.merged? + return unless exclusive_lease(merge_request.id).try_obtain + + merge_strategy_class = options[:merge_strategy] || MergeRequests::MergeStrategies::FromSourceBranch + @merge_strategy = merge_strategy_class.new(merge_request, current_user, merge_params: params, options: options) + + @merge_request = merge_request + @options = options + jid = merge_jid + + validate! + + merge_request.in_locked_state do + if commit_v2 + after_merge + clean_merge_jid + success + end + end + + log_info("Merge process finished on JID #{jid} with state #{state}") + rescue MergeError, MergeRequests::MergeStrategies::StrategyError => e + handle_merge_error(log_message: e.message, save_message_on_model: true) + ensure + exclusive_lease(merge_request.id).cancel + end + private def validate! authorization_check! error_check! + validate_strategy! updated_check! end @@ -59,15 +90,18 @@ module MergeRequests end end + # Can remove this entire method when :refactor_merge_service is enabled def error_check! super + return if Feature.enabled?(:refactor_merge_service, project) + check_source error = if @merge_request.should_be_rebased? 'Only fast-forward merge is allowed for your project. Please update your source branch' - elsif !@merge_request.mergeable?(skip_discussions_check: @options[:skip_discussions_check]) + elsif !@merge_request.mergeable?(skip_discussions_check: @options[:skip_discussions_check], check_mergeability_retry_lease: @options[:check_mergeability_retry_lease]) 'Merge request is not mergeable' elsif !@merge_request.squash && project.squash_always? 'This project requires squashing commits when merge requests are accepted.' @@ -76,6 +110,10 @@ module MergeRequests raise_error(error) if error end + def validate_strategy! + @merge_strategy.validate! if Feature.enabled?(:refactor_merge_service, project) + end + def updated_check! unless source_matches? raise_error('Branch has been updated since the merge was requested. '\ @@ -83,9 +121,28 @@ module MergeRequests end end + def commit_v2 + log_info("Git merge started on JID #{merge_jid}") + + merge_result = try_merge { @merge_strategy.execute_git_merge! } + + commit_sha = merge_result[:commit_sha] + raise_error(GENERIC_ERROR_MESSAGE) unless commit_sha + + log_info("Git merge finished on JID #{merge_jid} commit #{commit_sha}") + + new_merge_request_attributes = merge_result.slice(:merge_commit_sha, :squash_commit_sha) + merge_request.update!(new_merge_request_attributes) if new_merge_request_attributes.present? + + commit_sha + ensure + merge_request.update_and_mark_in_progress_merge_commit_sha(nil) + log_info("Merge request marked in progress") + end + def commit log_info("Git merge started on JID #{merge_jid}") - commit_id = try_merge + commit_id = try_merge { execute_git_merge } if commit_id log_info("Git merge finished on JID #{merge_jid} commit #{commit_id}") @@ -113,7 +170,7 @@ module MergeRequests end def try_merge - execute_git_merge + yield rescue Gitlab::Git::PreReceiveError => e raise MergeError, "Something went wrong during merge pre-receive hook. #{e.message}".strip rescue StandardError => e diff --git a/app/services/merge_requests/merge_strategies/from_source_branch.rb b/app/services/merge_requests/merge_strategies/from_source_branch.rb new file mode 100644 index 00000000000..9fe5fc5160b --- /dev/null +++ b/app/services/merge_requests/merge_strategies/from_source_branch.rb @@ -0,0 +1,112 @@ +# frozen_string_literal: true + +module MergeRequests + module MergeStrategies + # FromSourceBranch performs a git merge from a merge request's source branch + # to the target branch, including a squash if needed. + class FromSourceBranch + include Gitlab::Utils::StrongMemoize + + delegate :repository, to: :project + + def initialize(merge_request, current_user, merge_params: {}, options: {}) + @merge_request = merge_request + @current_user = current_user + @project = merge_request.project + @merge_params = merge_params + @options = options + end + + def validate! + error_message = + if source_sha.blank? + 'No source for merge' + elsif merge_request.should_be_rebased? + 'Only fast-forward merge is allowed for your project. Please update your source branch' + elsif !merge_request.mergeable?( + skip_discussions_check: @options[:skip_discussions_check], + check_mergeability_retry_lease: @options[:check_mergeability_retry_lease] + ) + 'Merge request is not mergeable' + elsif !merge_request.squash && project.squash_always? + 'This project requires squashing commits when merge requests are accepted.' + end + + raise_error(error_message) if error_message + end + + def execute_git_merge! + result = + if project.merge_requests_ff_only_enabled + fast_forward! + else + merge_commit! + end + + result[:squash_commit_sha] = source_sha if merge_request.squash_on_merge? + + result + end + + private + + attr_reader :merge_request, :current_user, :merge_params, :options, :project + + def source_sha + if merge_request.squash_on_merge? + squash_sha! + else + merge_request.diff_head_sha + end + end + strong_memoize_attr :source_sha + + def squash_sha! + squash_result = ::MergeRequests::SquashService + .new( + merge_request: merge_request, + current_user: current_user, + commit_message: merge_params[:squash_commit_message] + ).execute + + case squash_result[:status] + when :success + squash_result[:squash_sha] + when :error + raise_error(squash_result[:message]) + end + end + + def fast_forward! + commit_sha = repository.ff_merge( + current_user, + source_sha, + merge_request.target_branch, + merge_request: merge_request + ) + + { commit_sha: commit_sha } + end + + def merge_commit! + commit_sha = repository.merge( + current_user, + source_sha, + merge_request, + merge_commit_message + ) + + { commit_sha: commit_sha, merge_commit_sha: commit_sha } + end + + def merge_commit_message + merge_params[:commit_message] || + merge_request.default_merge_commit_message(user: current_user) + end + + def raise_error(message) + raise ::MergeRequests::MergeStrategies::StrategyError, message + end + end + end +end diff --git a/app/services/merge_requests/merge_strategies/strategy_error.rb b/app/services/merge_requests/merge_strategies/strategy_error.rb new file mode 100644 index 00000000000..144860f5c93 --- /dev/null +++ b/app/services/merge_requests/merge_strategies/strategy_error.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +module MergeRequests + module MergeStrategies + StrategyError = Class.new(StandardError) + end +end diff --git a/app/services/merge_requests/merge_to_ref_service.rb b/app/services/merge_requests/merge_to_ref_service.rb index acd3bc36e1d..8b79feb5e0f 100644 --- a/app/services/merge_requests/merge_to_ref_service.rb +++ b/app/services/merge_requests/merge_to_ref_service.rb @@ -13,13 +13,12 @@ module MergeRequests class MergeToRefService < MergeRequests::MergeBaseService extend ::Gitlab::Utils::Override - def execute(merge_request, cache_merge_to_ref_calls = false) + def execute(merge_request) @merge_request = merge_request error_check! - commit_id = commit(cache_merge_to_ref_calls) - + commit_id = extracted_merge_to_ref raise_error('Conflicts detected during merge') unless commit_id commit = project.commit(commit_id) @@ -56,16 +55,6 @@ module MergeRequests params[:first_parent_ref] || merge_request.target_branch_ref end - def commit(cache_merge_to_ref_calls = false) - if cache_merge_to_ref_calls - Rails.cache.fetch(cache_key, expires_in: 1.day) do - extracted_merge_to_ref - end - else - extracted_merge_to_ref - end - end - def extracted_merge_to_ref repository.merge_to_ref(current_user, source_sha: source, @@ -76,9 +65,5 @@ module MergeRequests rescue Gitlab::Git::PreReceiveError, Gitlab::Git::CommandError => error raise MergeError, error.message end - - def cache_key - [:merge_to_ref_service, project.full_path, merge_request.target_branch_sha, merge_request.source_branch_sha] - end end end diff --git a/app/services/merge_requests/squash_service.rb b/app/services/merge_requests/squash_service.rb index f04682bf08a..0b1aefb30d7 100644 --- a/app/services/merge_requests/squash_service.rb +++ b/app/services/merge_requests/squash_service.rb @@ -1,7 +1,17 @@ # frozen_string_literal: true module MergeRequests - class SquashService < MergeRequests::BaseService + class SquashService + include BaseServiceUtility + include MergeRequests::ErrorLogger + + def initialize(merge_request:, current_user:, commit_message:) + @merge_request = merge_request + @target_project = merge_request.target_project + @current_user = current_user + @commit_message = commit_message + end + def execute # If performing a squash would result in no change, then # immediately return a success message without performing a squash @@ -16,6 +26,8 @@ module MergeRequests private + attr_reader :merge_request, :target_project, :current_user, :commit_message + def squash! squash_sha = repository.squash(current_user, merge_request, message) @@ -34,12 +46,8 @@ module MergeRequests target_project.repository end - def merge_request - params[:merge_request] - end - def message - params[:squash_commit_message].presence || merge_request.default_squash_commit_message(user: current_user) + commit_message.presence || merge_request.default_squash_commit_message(user: current_user) end end end |