Fix conflict resolution from corrupted upstream

I don't know why this happens exactly, but given an upstream and fork repository
from a customer, both of which required GC, resolving conflicts would corrupt
the fork so badly that it couldn't be cloned.

This isn't a perfect fix for that case, because the MR may still need to be
merged manually, but it does ensure that the repository is at least usable.

My best guess is that when we generate the index for the conflict
resolution (which we previously did in the target project), we obtain a
reference to an OID that doesn't exist in the source, even though we already
fetch the refs from the target into the source.

Explicitly setting the source project as the place to get the merge index from
seems to prevent repository corruption in this way.

7 files changed, 115 insertions, 101 deletions

diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index 207fbad7856..b99ccd453b8 100755
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -155,8 +155,8 @@ class Projects::MergeRequestsController < Projects::ApplicationController
       format.html { define_discussion_vars }
 
       format.json do
-        if @merge_request.conflicts_can_be_resolved_in_ui?
-          render json: @merge_request.conflicts
+        if @conflicts_list.can_be_resolved_in_ui?
+          render json: @conflicts_list
         elsif @merge_request.can_be_merged?
           render json: {
             message: 'The merge conflicts for this merge request have already been resolved. Please return to the merge request.',
@@ -173,9 +173,9 @@ class Projects::MergeRequestsController < Projects::ApplicationController
   end
 
   def conflict_for_path
-    return render_404 unless @merge_request.conflicts_can_be_resolved_in_ui?
+    return render_404 unless @conflicts_list.can_be_resolved_in_ui?
 
-    file = @merge_request.conflicts.file_for_path(params[:old_path], params[:new_path])
+    file = @conflicts_list.file_for_path(params[:old_path], params[:new_path])
 
     return render_404 unless file
 
@@ -183,7 +183,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
   end
 
   def resolve_conflicts
-    return render_404 unless @merge_request.conflicts_can_be_resolved_in_ui?
+    return render_404 unless @conflicts_list.can_be_resolved_in_ui?
 
     if @merge_request.can_be_merged?
       render status: :bad_request, json: { message: 'The merge conflicts for this merge request have already been resolved.' }
@@ -191,7 +191,9 @@ class Projects::MergeRequestsController < Projects::ApplicationController
     end
 
     begin
-      MergeRequests::ResolveService.new(@merge_request.source_project, current_user, params).execute(@merge_request)
+      MergeRequests::Conflicts::ResolveService.
+        new(merge_request).
+        execute(current_user, params)
 
       flash[:notice] = 'All merge conflicts were resolved. The merge request can now be merged.'
 
@@ -459,7 +461,9 @@ class Projects::MergeRequestsController < Projects::ApplicationController
   end
 
   def authorize_can_resolve_conflicts!
-    return render_404 unless @merge_request.conflicts_can_be_resolved_by?(current_user)
+    @conflicts_list = MergeRequests::Conflicts::ListService.new(@merge_request)
+
+    return render_404 unless @conflicts_list.can_be_resolved_by?(current_user)
   end
 
   def module_enabled
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 59736f70f24..5f65636ba63 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -899,34 +899,6 @@ class MergeRequest < ActiveRecord::Base
     project.repository.keep_around(self.merge_commit_sha)
   end
 
-  def conflicts
-    @conflicts ||= Gitlab::Conflict::FileCollection.new(self)
-  end
-
-  def conflicts_can_be_resolved_by?(user)
-    return false unless source_project
-
-    access = ::Gitlab::UserAccess.new(user, project: source_project)
-    access.can_push_to_branch?(source_branch)
-  end
-
-  def conflicts_can_be_resolved_in_ui?
-    return @conflicts_can_be_resolved_in_ui if defined?(@conflicts_can_be_resolved_in_ui)
-
-    return @conflicts_can_be_resolved_in_ui = false unless cannot_be_merged?
-    return @conflicts_can_be_resolved_in_ui = false unless has_complete_diff_refs?
-
-    begin
-      # Try to parse each conflict. If the MR's mergeable status hasn't been updated,
-      # ensure that we don't say there are conflicts to resolve when there are no conflict
-      # files.
-      conflicts.files.each(&:lines)
-      @conflicts_can_be_resolved_in_ui = conflicts.files.length > 0
-    rescue Rugged::OdbError, Gitlab::Conflict::Parser::UnresolvableError, Gitlab::Conflict::FileCollection::ConflictSideMissing
-      @conflicts_can_be_resolved_in_ui = false
-    end
-  end
-
   def has_commits?
     merge_request_diff && commits_count > 0
   end
diff --git a/app/presenters/merge_request_presenter.rb b/app/presenters/merge_request_presenter.rb
index 255f63db5c2..0db9e31031c 100644
--- a/app/presenters/merge_request_presenter.rb
+++ b/app/presenters/merge_request_presenter.rb
@@ -76,7 +76,7 @@ class MergeRequestPresenter < Gitlab::View::Presenter::Delegated
   end
 
   def conflict_resolution_path
-    if conflicts_can_be_resolved_in_ui? && conflicts_can_be_resolved_by?(current_user)
+    if conflicts.can_be_resolved_in_ui? && conflicts.can_be_resolved_by?(current_user)
       conflicts_namespace_project_merge_request_path(project.namespace, project, merge_request)
     end
   end
@@ -141,6 +141,10 @@ class MergeRequestPresenter < Gitlab::View::Presenter::Delegated
 
   private
 
+  def conflicts
+    @conflicts ||= MergeRequests::Conflicts::ListService.new(merge_request)
+  end
+
   def closing_issues
     @closing_issues ||= closes_issues(current_user)
   end
diff --git a/app/services/merge_requests/conflicts/base_service.rb b/app/services/merge_requests/conflicts/base_service.rb
new file mode 100644
index 00000000000..b50875347d9
--- /dev/null
+++ b/app/services/merge_requests/conflicts/base_service.rb
@@ -0,0 +1,11 @@
+module MergeRequests
+  module Conflicts
+    class BaseService
+      attr_reader :merge_request
+
+      def initialize(merge_request)
+        @merge_request = merge_request
+      end
+    end
+  end
+end
diff --git a/app/services/merge_requests/conflicts/list_service.rb b/app/services/merge_requests/conflicts/list_service.rb
new file mode 100644
index 00000000000..9bf82518643
--- /dev/null
+++ b/app/services/merge_requests/conflicts/list_service.rb
@@ -0,0 +1,35 @@
+module MergeRequests
+  module Conflicts
+    class ListService < MergeRequests::Conflicts::BaseService
+      delegate :file_for_path, :to_json, to: :conflicts
+
+      def can_be_resolved_by?(user)
+        return false unless merge_request.source_project
+
+        access = ::Gitlab::UserAccess.new(user, project: merge_request.source_project)
+        access.can_push_to_branch?(merge_request.source_branch)
+      end
+
+      def can_be_resolved_in_ui?
+        return @conflicts_can_be_resolved_in_ui if defined?(@conflicts_can_be_resolved_in_ui)
+
+        return @conflicts_can_be_resolved_in_ui = false unless merge_request.cannot_be_merged?
+        return @conflicts_can_be_resolved_in_ui = false unless merge_request.has_complete_diff_refs?
+
+        begin
+          # Try to parse each conflict. If the MR's mergeable status hasn't been
+          # updated, ensure that we don't say there are conflicts to resolve
+          # when there are no conflict files.
+          conflicts.files.each(&:lines)
+          @conflicts_can_be_resolved_in_ui = conflicts.files.length > 0
+        rescue Rugged::OdbError, Gitlab::Conflict::Parser::UnresolvableError, Gitlab::Conflict::FileCollection::ConflictSideMissing
+          @conflicts_can_be_resolved_in_ui = false
+        end
+      end
+
+      def conflicts
+        @conflicts ||= Gitlab::Conflict::FileCollection.read_only(merge_request)
+      end
+    end
+  end
+end
diff --git a/app/services/merge_requests/conflicts/resolve_service.rb b/app/services/merge_requests/conflicts/resolve_service.rb
new file mode 100644
index 00000000000..d74a82effd6
--- /dev/null
+++ b/app/services/merge_requests/conflicts/resolve_service.rb
@@ -0,0 +1,53 @@
+module MergeRequests
+  module Conflicts
+    class ResolveService < MergeRequests::Conflicts::BaseService
+      MissingFiles = Class.new(Gitlab::Conflict::ResolutionError)
+
+      def execute(current_user, params)
+        rugged = merge_request.source_project.repository.rugged
+
+        Gitlab::Conflict::FileCollection.for_resolution(merge_request) do |conflicts_for_resolution|
+          merge_index = conflicts_for_resolution.merge_index
+
+          params[:files].each do |file_params|
+            conflict_file = conflicts_for_resolution.file_for_path(file_params[:old_path], file_params[:new_path])
+
+            write_resolved_file_to_index(merge_index, rugged, conflict_file, file_params)
+          end
+
+          unless merge_index.conflicts.empty?
+            missing_files = merge_index.conflicts.map { |file| file[:ours][:path] }
+
+            raise MissingFiles, "Missing resolutions for the following files: #{missing_files.join(', ')}"
+          end
+
+          commit_params = {
+            message: params[:commit_message] || conflicts_for_resolution.default_commit_message,
+            parents: [conflicts_for_resolution.our_commit, conflicts_for_resolution.their_commit].map(&:oid),
+            tree: merge_index.write_tree(rugged)
+          }
+
+          conflicts_for_resolution.
+            project.
+            repository.
+            resolve_conflicts(current_user, merge_request.source_branch, commit_params)
+        end
+      end
+
+      private
+
+      def write_resolved_file_to_index(merge_index, rugged, file, params)
+        new_file = if params[:sections]
+                     file.resolve_lines(params[:sections]).map(&:text).join("\n")
+                   elsif params[:content]
+                     file.resolve_content(params[:content])
+                   end
+
+        our_path = file.our_path
+
+        merge_index.add(path: our_path, oid: rugged.write(new_file, :blob), mode: file.our_mode)
+        merge_index.conflict_remove(our_path)
+      end
+    end
+  end
+end
diff --git a/app/services/merge_requests/resolve_service.rb b/app/services/merge_requests/resolve_service.rb
deleted file mode 100644
index 82cd89d9a0b..00000000000
--- a/app/services/merge_requests/resolve_service.rb
+++ /dev/null
@@ -1,65 +0,0 @@
-module MergeRequests
-  class ResolveService < MergeRequests::BaseService
-    MissingFiles = Class.new(Gitlab::Conflict::ResolutionError)
-
-    attr_accessor :conflicts, :rugged, :merge_index, :merge_request
-
-    def execute(merge_request)
-      @conflicts = merge_request.conflicts
-      @rugged = project.repository.rugged
-      @merge_index = conflicts.merge_index
-      @merge_request = merge_request
-
-      fetch_their_commit!
-
-      params[:files].each do |file_params|
-        conflict_file = merge_request.conflicts.file_for_path(file_params[:old_path], file_params[:new_path])
-
-        write_resolved_file_to_index(conflict_file, file_params)
-      end
-
-      unless merge_index.conflicts.empty?
-        missing_files = merge_index.conflicts.map { |file| file[:ours][:path] }
-
-        raise MissingFiles, "Missing resolutions for the following files: #{missing_files.join(', ')}"
-      end
-
-      commit_params = {
-        message: params[:commit_message] || conflicts.default_commit_message,
-        parents: [conflicts.our_commit, conflicts.their_commit].map(&:oid),
-        tree: merge_index.write_tree(rugged)
-      }
-
-      project.repository.resolve_conflicts(current_user, merge_request.source_branch, commit_params)
-    end
-
-    def write_resolved_file_to_index(file, params)
-      new_file = if params[:sections]
-                   file.resolve_lines(params[:sections]).map(&:text).join("\n")
-                 elsif params[:content]
-                   file.resolve_content(params[:content])
-                 end
-
-      our_path = file.our_path
-
-      merge_index.add(path: our_path, oid: rugged.write(new_file, :blob), mode: file.our_mode)
-      merge_index.conflict_remove(our_path)
-    end
-
-    # If their commit (in the target project) doesn't exist in the source project, it
-    # can't be a parent for the merge commit we're about to create. If that's the case,
-    # fetch the target branch ref into the source project so the commit exists in both.
-    #
-    def fetch_their_commit!
-      return if rugged.include?(conflicts.their_commit.oid)
-
-      random_string = SecureRandom.hex
-
-      project.repository.fetch_ref(
-        merge_request.target_project.repository.path_to_repo,
-        "refs/heads/#{merge_request.target_branch}",
-        "refs/tmp/#{random_string}/head"
-      )
-    end
-  end
-end