From bf9ce1f4cf5592f7aa1f761a5b394e7d500dc034 Mon Sep 17 00:00:00 2001
From: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
Date: Tue, 29 Jul 2014 12:11:16 +0300
Subject: [PATCH] Refactor compare logic for MR. Use satellites only for forks
for better performance
Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
---
app/services/compare_service.rb | 28 ++++++++++++++++++++
app/services/merge_requests/build_service.rb | 11 ++++----
lib/gitlab/compare_result.rb | 9 +++++++
lib/gitlab/satellite/compare_action.rb | 20 +++-----------
4 files changed, 46 insertions(+), 22 deletions(-)
create mode 100644 app/services/compare_service.rb
create mode 100644 lib/gitlab/compare_result.rb
diff --git a/app/services/compare_service.rb b/app/services/compare_service.rb
new file mode 100644
index 00000000000..e4c3bb1507d
--- /dev/null
+++ b/app/services/compare_service.rb
@@ -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
diff --git a/app/services/merge_requests/build_service.rb b/app/services/merge_requests/build_service.rb
index 81dd8887395..1475973e543 100644
--- a/app/services/merge_requests/build_service.rb
+++ b/app/services/merge_requests/build_service.rb
@@ -19,16 +19,15 @@ module MergeRequests
# Generate suggested MR title based on source branch name
merge_request.title = merge_request.source_branch.titleize.humanize
- # Try to compare branches to get commits list and diffs
- compare_action = Gitlab::Satellite::CompareAction.new(
+ compare_result = CompareService.new.execute(
current_user,
+ merge_request.source_project,
+ merge_request.source_branch,
merge_request.target_project,
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
# If we have at least one commit to merge -> creation allowed
@@ -38,7 +37,7 @@ module MergeRequests
merge_request.compare_failed = false
# Try to collect diff for merge request.
- diffs = compare_action.diffs
+ diffs = compare_result.diffs
if diffs.present?
merge_request.compare_diffs = diffs
diff --git a/lib/gitlab/compare_result.rb b/lib/gitlab/compare_result.rb
new file mode 100644
index 00000000000..d72391dade5
--- /dev/null
+++ b/lib/gitlab/compare_result.rb
@@ -0,0 +1,9 @@
+module Gitlab
+ class CompareResult
+ attr_reader :commits, :diffs
+
+ def initialize(compare)
+ @commits, @diffs = compare.commits, compare.diffs
+ end
+ end
+end
diff --git a/lib/gitlab/satellite/compare_action.rb b/lib/gitlab/satellite/compare_action.rb
index e5b62576330..4905b146a55 100644
--- a/lib/gitlab/satellite/compare_action.rb
+++ b/lib/gitlab/satellite/compare_action.rb
@@ -10,28 +10,16 @@ module Gitlab
@source_project, @source_branch = source_project, source_branch
end
- # Only show what is new in the source branch compared to the target branch, not the other way around.
- # The line below with merge_base is equivalent to diff with three dots (git diff branch1...branch2)
- # From the git documentation: "git diff A...B" is equivalent to "git diff $(git-merge-base A B) B"
- def diffs
+ # Compare 2 repositories and return Gitlab::CompareResult object
+ def result
in_locked_and_timed_satellite do |target_repo|
prepare_satellite!(target_repo)
update_satellite_source_and_target!(target_repo)
- compare(target_repo).diffs
- end
- rescue Grit::Git::CommandFailed => ex
- raise BranchesWithoutParent
- 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
+ Gitlab::CompareResult.new(compare(target_repo))
end
rescue Grit::Git::CommandFailed => ex
- handle_exception(ex)
+ raise BranchesWithoutParent
end
private
--
GitLab