Refactor compare logic for MR. Use satellites only for forks for better performance
Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
This commit is contained in:
parent
d30454e112
commit
bf9ce1f4cf
|
@ -0,0 +1,28 @@
|
||||||
|
# Compare 2 branches for one repo or between repositories
|
||||||
|
# and return Gitlab::CompareResult object that responds to commits and diffs
|
||||||
|
class CompareService
|
||||||
|
def execute(current_user, source_project, source_branch, target_project, target_branch)
|
||||||
|
# Try to compare branches to get commits list and diffs
|
||||||
|
#
|
||||||
|
# Note: Use satellite only when need to compare between to repos
|
||||||
|
# because satellites are slower then operations on bare repo
|
||||||
|
if target_project == source_project
|
||||||
|
Gitlab::CompareResult.new(
|
||||||
|
Gitlab::Git::Compare.new(
|
||||||
|
target_project.repository.raw_repository,
|
||||||
|
target_branch,
|
||||||
|
source_branch,
|
||||||
|
10000
|
||||||
|
)
|
||||||
|
)
|
||||||
|
else
|
||||||
|
Gitlab::Satellite::CompareAction.new(
|
||||||
|
current_user,
|
||||||
|
target_project,
|
||||||
|
target_branch,
|
||||||
|
source_project,
|
||||||
|
source_branch
|
||||||
|
).result
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
|
@ -19,16 +19,15 @@ module MergeRequests
|
||||||
# Generate suggested MR title based on source branch name
|
# Generate suggested MR title based on source branch name
|
||||||
merge_request.title = merge_request.source_branch.titleize.humanize
|
merge_request.title = merge_request.source_branch.titleize.humanize
|
||||||
|
|
||||||
# Try to compare branches to get commits list and diffs
|
compare_result = CompareService.new.execute(
|
||||||
compare_action = Gitlab::Satellite::CompareAction.new(
|
|
||||||
current_user,
|
current_user,
|
||||||
|
merge_request.source_project,
|
||||||
|
merge_request.source_branch,
|
||||||
merge_request.target_project,
|
merge_request.target_project,
|
||||||
merge_request.target_branch,
|
merge_request.target_branch,
|
||||||
merge_request.source_project,
|
|
||||||
merge_request.source_branch
|
|
||||||
)
|
)
|
||||||
|
|
||||||
commits = compare_action.commits
|
commits = compare_result.commits
|
||||||
|
|
||||||
# At this point we decide if merge request can be created
|
# At this point we decide if merge request can be created
|
||||||
# If we have at least one commit to merge -> creation allowed
|
# If we have at least one commit to merge -> creation allowed
|
||||||
|
@ -38,7 +37,7 @@ module MergeRequests
|
||||||
merge_request.compare_failed = false
|
merge_request.compare_failed = false
|
||||||
|
|
||||||
# Try to collect diff for merge request.
|
# Try to collect diff for merge request.
|
||||||
diffs = compare_action.diffs
|
diffs = compare_result.diffs
|
||||||
|
|
||||||
if diffs.present?
|
if diffs.present?
|
||||||
merge_request.compare_diffs = diffs
|
merge_request.compare_diffs = diffs
|
||||||
|
|
|
@ -0,0 +1,9 @@
|
||||||
|
module Gitlab
|
||||||
|
class CompareResult
|
||||||
|
attr_reader :commits, :diffs
|
||||||
|
|
||||||
|
def initialize(compare)
|
||||||
|
@commits, @diffs = compare.commits, compare.diffs
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
|
@ -10,30 +10,18 @@ module Gitlab
|
||||||
@source_project, @source_branch = source_project, source_branch
|
@source_project, @source_branch = source_project, source_branch
|
||||||
end
|
end
|
||||||
|
|
||||||
# Only show what is new in the source branch compared to the target branch, not the other way around.
|
# Compare 2 repositories and return Gitlab::CompareResult object
|
||||||
# The line below with merge_base is equivalent to diff with three dots (git diff branch1...branch2)
|
def result
|
||||||
# From the git documentation: "git diff A...B" is equivalent to "git diff $(git-merge-base A B) B"
|
|
||||||
def diffs
|
|
||||||
in_locked_and_timed_satellite do |target_repo|
|
in_locked_and_timed_satellite do |target_repo|
|
||||||
prepare_satellite!(target_repo)
|
prepare_satellite!(target_repo)
|
||||||
update_satellite_source_and_target!(target_repo)
|
update_satellite_source_and_target!(target_repo)
|
||||||
compare(target_repo).diffs
|
|
||||||
|
Gitlab::CompareResult.new(compare(target_repo))
|
||||||
end
|
end
|
||||||
rescue Grit::Git::CommandFailed => ex
|
rescue Grit::Git::CommandFailed => ex
|
||||||
raise BranchesWithoutParent
|
raise BranchesWithoutParent
|
||||||
end
|
end
|
||||||
|
|
||||||
# Retrieve an array of commits between the source and the target
|
|
||||||
def commits
|
|
||||||
in_locked_and_timed_satellite do |target_repo|
|
|
||||||
prepare_satellite!(target_repo)
|
|
||||||
update_satellite_source_and_target!(target_repo)
|
|
||||||
compare(target_repo).commits
|
|
||||||
end
|
|
||||||
rescue Grit::Git::CommandFailed => ex
|
|
||||||
handle_exception(ex)
|
|
||||||
end
|
|
||||||
|
|
||||||
private
|
private
|
||||||
|
|
||||||
# Assumes a satellite exists that is a fresh clone of the projects repo, prepares satellite for diffs
|
# Assumes a satellite exists that is a fresh clone of the projects repo, prepares satellite for diffs
|
||||||
|
|
Loading…
Reference in New Issue