Refactor compare and fetch logic

8 files changed, 36 insertions, 51 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
-        source_sha = merge_request.source_project.commit(source_branch).sha
+        # Update ref if merge request is from fork
+        merge_request.fetch_ref 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"
-        )
+        # Get latest sha of branch from source project
+        source_sha = merge_request.source_project.commit(source_branch).sha
 
-        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"
       )
+    end
 
-      source_sha = source_project.commit(source_branch).sha
-
-      Gitlab::CompareResult.new(
-        Gitlab::Git::Compare.new(
-          target_project.repository.raw_repository,
-          target_branch,
-          source_sha,
-        )
+    Gitlab::CompareResult.new(
+      Gitlab::Git::Compare.new(
+        target_project.repository.raw_repository,
+        target_branch,
+        source_sha,
       )
-    end
+    )
   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)