Simplify conflict resolution interface and code

- Add a Gitlab::Git::Conflict::Resolution class to encapsulate
resolution data
- Simplify conflict file collection assembly

lib/gitlab/conflict/file_collection.rb

@@ -13,12 +13,13 @@ module Gitlab
       end
       def resolve(user, commit_message, files)
+        msg = commit_message || default_commit_message
+        resolution = Gitlab::Git::Conflict::Resolution.new(user, files, msg)
         args = {
           source_branch: merge_request.source_branch,
-          target_branch: merge_request.target_branch,
-          commit_message: commit_message || default_commit_message
+          target_branch: merge_request.target_branch
         }
-        resolver.resolve_conflicts(@source_repo, user, files, args)
+        resolver.resolve_conflicts(@source_repo, resolution, args)
       ensure
         @merge_request.clear_memoized_shas
       end

lib/gitlab/git/conflict/file.rb

@@ -2,7 +2,9 @@ module Gitlab
   module Git
     module Conflict
       class File
-        attr_reader :content, :their_path, :our_path, :our_mode, :repository, :commit_oid
+        attr_reader :their_path, :our_path, :our_mode, :repository, :commit_oid
+
+        attr_accessor :content
         def initialize(repository, commit_oid, conflict, content)
           @repository = repository

lib/gitlab/git/conflict/resolution.rb

+module Gitlab
+  module Git
+    module Conflict
+      class Resolution
+        attr_reader :user, :files, :commit_message
+
+        def initialize(user, files, commit_message)
+          @user = user
+          @files = files
+          @commit_message = commit_message
+        end
+      end
+    end
+  end
+end

lib/gitlab/git/conflict/resolver.rb

@@ -27,12 +27,12 @@ module Gitlab
           raise Gitlab::Git::CommandError.new(e)
         end
-        def resolve_conflicts(source_repository, user, files, source_branch:, target_branch:, commit_message:)
+        def resolve_conflicts(source_repository, resolution, source_branch:, target_branch:)
           source_repository.gitaly_migrate(:conflicts_resolve_conflicts) do |is_enabled|
             if is_enabled
-              gitaly_conflicts_client(source_repository).resolve_conflicts(@target_repository, user, files, source_branch, target_branch, commit_message)
+              gitaly_conflicts_client(source_repository).resolve_conflicts(@target_repository, resolution, source_branch, target_branch)
             else
-              rugged_resolve_conflicts(source_repository, user, files, source_branch, target_branch, commit_message)
+              rugged_resolve_conflicts(source_repository, resolution, source_branch, target_branch)
             end
           end
         end
@@ -87,12 +87,12 @@ module Gitlab
           conflict_files(@target_repository, target_index)
         end
-        def rugged_resolve_conflicts(source_repository, user, files, source_branch, target_branch, commit_message)
+        def rugged_resolve_conflicts(source_repository, resolution, source_branch, target_branch)
           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|
+            resolution.files.each do |file_params|
               conflict_file = conflict_for_path(conflicts, file_params[:old_path], file_params[:new_path])
               write_resolved_file_to_index(source_repository, index, conflict_file, file_params)
@@ -105,11 +105,11 @@ module Gitlab
             end
             commit_params = {
-              message: commit_message,
+              message: resolution.commit_message,
               parents: [@our_commit_oid, @their_commit_oid]
             }
-            source_repository.commit_index(user, source_branch, index, commit_params)
+            source_repository.commit_index(resolution.user, source_branch, index, commit_params)
           end
         end
       end

lib/gitlab/gitaly_client/conflicts_service.rb

@@ -17,36 +17,15 @@ module Gitlab
           their_commit_oid: @their_commit_oid
         )
         response = GitalyClient.call(@repository.storage, :conflicts_service, :list_conflict_files, request)
-        files = []
-        header = nil
-        content = nil
-
-        response.each do |msg|
-          msg.files.each do |gitaly_file|
-            if gitaly_file.header
-              # Add previous file to the collection, except on first iteration
-              files << conflict_file_from_gitaly(header, content) if header
-
-              header = gitaly_file.header
-              content = ""
-            else
-              # Append content to curret file
-              content << gitaly_file.content
-            end
-          end
-        end
-
-        # Add leftover file, if any
-        files << conflict_file_from_gitaly(header, content) if header
-        files
+        files_from_response(response).to_a
       end
-      def resolve_conflicts(target_repository, user, files, source_branch, target_branch, commit_message)
-        reader = GitalyClient.binary_stringio(files.to_json)
+      def resolve_conflicts(target_repository, resolution, source_branch, target_branch)
+        reader = GitalyClient.binary_stringio(resolution.files.to_json)
         req_enum = Enumerator.new do |y|
-          header = resolve_conflicts_request_header(target_repository, user, source_branch, target_branch, commit_message)
+          header = resolve_conflicts_request_header(target_repository, resolution, source_branch, target_branch)
           y.yield Gitaly::ResolveConflictsRequest.new(header: header)
           until reader.eof?
@@ -65,12 +44,41 @@ module Gitlab
       private
-      def conflict_file_from_gitaly(header, content)
+      def resolve_conflicts_request_header(target_repository, resolution, source_branch, target_branch)
+        Gitaly::ResolveConflictsRequestHeader.new(
+          repository: @gitaly_repo,
+          our_commit_oid: @our_commit_oid,
+          target_repository: target_repository.gitaly_repository,
+          their_commit_oid: @their_commit_oid,
+          source_branch: source_branch,
+          target_branch: target_branch,
+          commit_message: resolution.commit_message,
+          user: Gitlab::Git::User.from_gitlab(resolution.user).to_gitaly
+        )
+      end
+
+      def files_from_response(response)
+        files = []
+
+        response.each do |msg|
+          msg.files.each do |gitaly_file|
+            if gitaly_file.header
+              files << file_from_gitaly_header(gitaly_file.header)
+            else
+              files.last.content << gitaly_file.content
+            end
+          end
+        end
+
+        files
+      end
+
+      def file_from_gitaly_header(header)
         Gitlab::Git::Conflict::File.new(
           Gitlab::GitalyClient::Util.git_repository(header.repository),
           header.commit_oid,
           conflict_from_gitaly_file_header(header),
-          content
+          ''
         )
       end
@@ -80,19 +88,6 @@ module Gitlab
           theirs: { path: header.their_path }
         }
       end
-
-      def resolve_conflicts_request_header(target_repository, user, source_branch, target_branch, commit_message)
-        Gitaly::ResolveConflictsRequestHeader.new(
-          repository: @gitaly_repo,
-          our_commit_oid: @our_commit_oid,
-          target_repository: target_repository.gitaly_repository,
-          their_commit_oid: @their_commit_oid,
-          source_branch: source_branch,
-          target_branch: target_branch,
-          commit_message: commit_message,
-          user: Gitlab::Git::User.from_gitlab(user).to_gitaly
-        )
-      end
     end
   end
 end

spec/lib/gitlab/gitaly_client/conflicts_service_spec.rb

@@ -65,9 +65,12 @@ describe Gitlab::GitalyClient::ConflictsService do
     let(:source_branch) { 'master' }
     let(:target_branch) { 'feature' }
     let(:commit_message) { 'Solving conflicts' }
+    let(:resolution) do
+      Gitlab::Git::Conflict::Resolution.new(user, files, commit_message)
+    end
     subject do
-      client.resolve_conflicts(source_repository, user, files, source_branch, target_branch, commit_message)
+      client.resolve_conflicts(source_repository, resolution, source_branch, target_branch)
     end
     it 'sends an RPC request' do