Merge branch 'gitaly-conflicts-prep' into 'master'

Simplify conflict resolver interface

See merge request gitlab-org/gitlab-ce!15897

lib/gitlab/conflict/file_collection.rb

@@ -4,11 +4,11 @@ module Gitlab
       attr_reader :merge_request, :resolver
 
       def initialize(merge_request)
-        source_repo = merge_request.source_project.repository.raw
         our_commit = merge_request.source_branch_head.raw
         their_commit = merge_request.target_branch_head.raw
         target_repo = merge_request.target_project.repository.raw
-        @resolver = Gitlab::Git::Conflict::Resolver.new(source_repo, our_commit, target_repo, their_commit)
+        @source_repo = merge_request.source_project.repository.raw
+        @resolver = Gitlab::Git::Conflict::Resolver.new(target_repo, our_commit.id, their_commit.id)
         @merge_request = merge_request
       end
 
@@ -18,7 +18,7 @@ module Gitlab
           target_branch: merge_request.target_branch,
           commit_message: commit_message || default_commit_message
         }
-        resolver.resolve_conflicts(user, files, args)
+        resolver.resolve_conflicts(@source_repo, user, files, args)
       ensure
         @merge_request.clear_memoized_shas
       end

lib/gitlab/git/conflict/resolver.rb

@@ -5,38 +5,31 @@ module Gitlab
         ConflictSideMissing = Class.new(StandardError)
         ResolutionError = Class.new(StandardError)
 
-        def initialize(repository, our_commit, target_repository, their_commit)
-          @repository = repository
-          @our_commit = our_commit.rugged_commit
+        def initialize(target_repository, our_commit_oid, their_commit_oid)
           @target_repository = target_repository
-          @their_commit = their_commit.rugged_commit
+          @our_commit_oid = our_commit_oid
+          @their_commit_oid = their_commit_oid
         end
 
         def conflicts
           @conflicts ||= begin
-            target_index = @target_repository.rugged.merge_commits(@our_commit, @their_commit)
+            target_index = @target_repository.rugged.merge_commits(@our_commit_oid, @their_commit_oid)
 
             # We don't need to do `with_repo_branch_commit` here, because the target
             # project always fetches source refs when creating merge request diffs.
-            target_index.conflicts.map do |conflict|
-              raise ConflictSideMissing unless conflict[:theirs] && conflict[:ours]
-
-              Gitlab::Git::Conflict::File.new(
-                @target_repository,
-                @our_commit.oid,
-                conflict,
-                target_index.merge_file(conflict[:ours][:path])[:data]
-              )
-            end
+            conflict_files(@target_repository, target_index)
           end
         end
 
-        def resolve_conflicts(user, files, source_branch:, target_branch:, commit_message:)
-          @repository.with_repo_branch_commit(@target_repository, target_branch) do
+        def resolve_conflicts(source_repository, user, files, source_branch:, target_branch:, commit_message:)
+          source_repository.with_repo_branch_commit(@target_repository, target_branch) do
+            index = source_repository.rugged.merge_commits(@our_commit_oid, @their_commit_oid)
+            conflicts = conflict_files(source_repository, index)
+
             files.each do |file_params|
-              conflict_file = conflict_for_path(file_params[:old_path], file_params[:new_path])
+              conflict_file = conflict_for_path(conflicts, file_params[:old_path], file_params[:new_path])
 
-              write_resolved_file_to_index(conflict_file, file_params)
+              write_resolved_file_to_index(source_repository, index, conflict_file, file_params)
             end
 
             unless index.conflicts.empty?
@@ -47,14 +40,14 @@ module Gitlab
 
             commit_params = {
               message: commit_message,
-              parents: [@our_commit, @their_commit].map(&:oid)
+              parents: [@our_commit_oid, @their_commit_oid]
             }
 
-            @repository.commit_index(user, source_branch, index, commit_params)
+            source_repository.commit_index(user, source_branch, index, commit_params)
           end
         end
 
-        def conflict_for_path(old_path, new_path)
+        def conflict_for_path(conflicts, old_path, new_path)
           conflicts.find do |conflict|
             conflict.their_path == old_path && conflict.our_path == new_path
           end
@@ -62,15 +55,20 @@ module Gitlab
 
         private
 
-        # We can only write when getting the merge index from the source
-        # project, because we will write to that project. We don't use this all
-        # the time because this fetches a ref into the source project, which
-        # isn't needed for reading.
-        def index
-          @index ||= @repository.rugged.merge_commits(@our_commit, @their_commit)
+        def conflict_files(repository, index)
+          index.conflicts.map do |conflict|
+            raise ConflictSideMissing unless conflict[:theirs] && conflict[:ours]
+
+            Gitlab::Git::Conflict::File.new(
+              repository,
+              @our_commit_oid,
+              conflict,
+              index.merge_file(conflict[:ours][:path])[:data]
+            )
+          end
         end
 
-        def write_resolved_file_to_index(file, params)
+        def write_resolved_file_to_index(repository, index, file, params)
           if params[:sections]
             resolved_lines = file.resolve_lines(params[:sections])
             new_file = resolved_lines.map { |line| line[:full_line] }.join("\n")
@@ -82,7 +80,8 @@ module Gitlab
 
           our_path = file.our_path
 
-          index.add(path: our_path, oid: @repository.rugged.write(new_file, :blob), mode: file.our_mode)
+          oid = repository.rugged.write(new_file, :blob)
+          index.add(path: our_path, oid: oid, mode: file.our_mode)
           index.conflict_remove(our_path)
         end
       end

spec/services/merge_requests/conflicts/resolve_service_spec.rb

@@ -213,7 +213,7 @@ describe MergeRequests::Conflicts::ResolveService do
         MergeRequests::Conflicts::ListService.new(merge_request).conflicts.resolver
       end
       let(:regex_conflict) do
-        resolver.conflict_for_path('files/ruby/regex.rb', 'files/ruby/regex.rb')
+        resolver.conflict_for_path(resolver.conflicts, 'files/ruby/regex.rb', 'files/ruby/regex.rb')
       end
 
       let(:invalid_params) do