From 26f5d6047d6e21a5c65a4276266648f1e69aac4a Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Wed, 15 Jul 2015 17:28:09 +0200 Subject: [PATCH] Refactor compare and fetch logic --- .../projects/compare_controller.rb | 9 ++--- .../projects/merge_requests_controller.rb | 3 +- app/models/merge_request.rb | 8 +++++ app/models/merge_request_diff.rb | 22 ++++++------ app/models/repository.rb | 2 ++ app/services/compare_service.rb | 35 ++++++++----------- app/services/merge_requests/build_service.rb | 1 - app/services/merge_requests/create_service.rb | 9 ----- 8 files changed, 37 insertions(+), 52 deletions(-) diff --git a/app/controllers/projects/compare_controller.rb b/app/controllers/projects/compare_controller.rb index c5f085c236f..d9b3adae95b 100644 --- a/app/controllers/projects/compare_controller.rb +++ b/app/controllers/projects/compare_controller.rb @@ -13,13 +13,8 @@ class Projects::CompareController < Projects::ApplicationController base_ref = Addressable::URI.unescape(params[:from]) @ref = head_ref = Addressable::URI.unescape(params[:to]) - compare_result = CompareService.new.execute( - current_user, - @project, - head_ref, - @project, - base_ref - ) + compare_result = CompareService.new. + execute(@project, head_ref, @project, base_ref) @commits = compare_result.commits @diffs = compare_result.diffs diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index fbf2d8fa3ee..0ef9e99123d 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -149,8 +149,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController return access_denied! unless @merge_request.can_be_merged_by?(current_user) if @merge_request.automergeable? - #AutoMergeWorker.perform_async(@merge_request.id, current_user.id, params) - @merge_request.automerge!(current_user, params[:commit_message]) + AutoMergeWorker.perform_async(@merge_request.id, current_user.id, params) @status = true else @status = false diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index e32b224eb77..4dcde029efa 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -437,4 +437,12 @@ class MergeRequest < ActiveRecord::Base def source_sha commits.first.sha end + + def fetch_ref + target_project.repository.fetch_ref( + source_project.repository.path_to_repo, + "refs/heads/#{source_branch}", + "refs/merge-requests/#{id}/head" + ) + end end diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb index 8a48f78dbd4..6f01a589d1c 100644 --- a/app/models/merge_request_diff.rb +++ b/app/models/merge_request_diff.rb @@ -162,20 +162,18 @@ class MergeRequestDiff < ActiveRecord::Base def compare_result @compare_result ||= begin + # Update ref if merge request is from fork + merge_request.fetch_ref if merge_request.for_fork? + + # Get latest sha of branch from source project source_sha = merge_request.source_project.commit(source_branch).sha - merge_request.target_project.repository.fetch_ref( - merge_request.source_project.repository.path_to_repo, - "refs/heads/#{merge_request.source_branch}", - "refs/merge-requests/#{merge_request.id}/head" - ) - - CompareService.new.execute( - merge_request.author, - merge_request.target_project, - source_sha, - merge_request.target_project, - merge_request.target_branch, + Gitlab::CompareResult.new( + Gitlab::Git::Compare.new( + merge_request.target_project.repository.raw_repository, + merge_request.target_branch, + source_sha, + ) ) end end diff --git a/app/models/repository.rb b/app/models/repository.rb index 1924e7ee072..41666bd6e38 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -428,6 +428,8 @@ class Repository if our_commit && their_commit !rugged.merge_commits(our_commit, their_commit).conflicts? + else + false end end diff --git a/app/services/compare_service.rb b/app/services/compare_service.rb index 4c8b35edff8..70f642baaaa 100644 --- a/app/services/compare_service.rb +++ b/app/services/compare_service.rb @@ -3,33 +3,26 @@ require 'securerandom' # 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 - if target_project == source_project - Gitlab::CompareResult.new( - Gitlab::Git::Compare.new( - target_project.repository.raw_repository, - target_branch, - source_branch, - ) - ) - else + def execute(source_project, source_branch, target_project, target_branch) + source_sha = source_project.commit(source_branch).sha + + # If compare with other project we need to fetch ref first + unless target_project == source_project random_string = SecureRandom.hex + target_project.repository.fetch_ref( source_project.repository.path_to_repo, "refs/heads/#{source_branch}", "refs/tmp/#{random_string}/head" ) - - source_sha = source_project.commit(source_branch).sha - - Gitlab::CompareResult.new( - Gitlab::Git::Compare.new( - target_project.repository.raw_repository, - target_branch, - source_sha, - ) - ) end + + Gitlab::CompareResult.new( + Gitlab::Git::Compare.new( + target_project.repository.raw_repository, + target_branch, + source_sha, + ) + ) end end diff --git a/app/services/merge_requests/build_service.rb b/app/services/merge_requests/build_service.rb index 5ef1cd2caa0..3ff5374b353 100644 --- a/app/services/merge_requests/build_service.rb +++ b/app/services/merge_requests/build_service.rb @@ -17,7 +17,6 @@ module MergeRequests end compare_result = CompareService.new.execute( - current_user, merge_request.source_project, merge_request.source_branch, merge_request.target_project, diff --git a/app/services/merge_requests/create_service.rb b/app/services/merge_requests/create_service.rb index 8389f29a53a..f431c5d5534 100644 --- a/app/services/merge_requests/create_service.rb +++ b/app/services/merge_requests/create_service.rb @@ -9,15 +9,6 @@ module MergeRequests merge_request.author = current_user if merge_request.save - # Fetch fork branch into hidden ref of target repository - if merge_request.for_fork? - merge_request.target_project.repository.fetch_ref( - merge_request.source_project.repository.path_to_repo, - "refs/heads/#{merge_request.source_branch}", - "refs/merge-requests/#{merge_request.id}/head" - ) - end - merge_request.update_attributes(label_ids: label_params) event_service.open_mr(merge_request, current_user) notification_service.new_merge_request(merge_request, current_user)