Incorporate ConflictsService.ResolveConflicts Gitaly RPC

lib/gitlab/git/conflict/resolver.rb

@@ -15,7 +15,7 @@ module Gitlab
           @conflicts ||= begin
             @target_repository.gitaly_migrate(:conflicts_list_conflict_files) do |is_enabled|
               if is_enabled
-                @target_repository.gitaly_conflicts_client.list_conflict_files(@our_commit_oid, @their_commit_oid)
+                gitaly_conflicts_client(@target_repository).list_conflict_files
               else
                 rugged_list_conflict_files
               end
@@ -28,28 +28,12 @@ module Gitlab
         end
         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(conflicts, file_params[:old_path], file_params[:new_path])
-
-              write_resolved_file_to_index(source_repository, index, conflict_file, file_params)
-            end
-
-            unless index.conflicts.empty?
-              missing_files = index.conflicts.map { |file| file[:ours][:path] }
-
-              raise ResolutionError, "Missing resolutions for the following files: #{missing_files.join(', ')}"
+          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)
+            else
+              rugged_resolve_conflicts(source_repository, user, files, source_branch, target_branch, commit_message)
             end
-
-            commit_params = {
-              message: commit_message,
-              parents: [@our_commit_oid, @their_commit_oid]
-            }
-
-            source_repository.commit_index(user, source_branch, index, commit_params)
           end
         end
@@ -74,6 +58,10 @@ module Gitlab
           end
         end
+        def gitaly_conflicts_client(repository)
+          repository.gitaly_conflicts_client(@our_commit_oid, @their_commit_oid)
+        end
+
         def write_resolved_file_to_index(repository, index, file, params)
           if params[:sections]
             resolved_lines = file.resolve_lines(params[:sections])
@@ -98,6 +86,32 @@ module Gitlab
           # project always fetches source refs when creating merge request diffs.
           conflict_files(@target_repository, target_index)
         end
+
+        def rugged_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(conflicts, file_params[:old_path], file_params[:new_path])
+
+              write_resolved_file_to_index(source_repository, index, conflict_file, file_params)
+            end
+
+            unless index.conflicts.empty?
+              missing_files = index.conflicts.map { |file| file[:ours][:path] }
+
+              raise ResolutionError, "Missing resolutions for the following files: #{missing_files.join(', ')}"
+            end
+
+            commit_params = {
+              message: commit_message,
+              parents: [@our_commit_oid, @their_commit_oid]
+            }
+
+            source_repository.commit_index(user, source_branch, index, commit_params)
+          end
+        end
       end
     end
   end

lib/gitlab/git/repository.rb

@@ -1294,8 +1294,8 @@ module Gitlab
         @gitaly_remote_client ||= Gitlab::GitalyClient::RemoteService.new(self)
       end
-      def gitaly_conflicts_client
-        @gitaly_conflicts_client ||= Gitlab::GitalyClient::ConflictsService.new(self)
+      def gitaly_conflicts_client(our_commit_oid, their_commit_oid)
+        Gitlab::GitalyClient::ConflictsService.new(self, our_commit_oid, their_commit_oid)
       end
       def gitaly_migrate(method, status: Gitlab::GitalyClient::MigrationStatus::OPT_IN, &block)

lib/gitlab/gitaly_client/conflicts_service.rb

 module Gitlab
   module GitalyClient
     class ConflictsService
-      def initialize(repository)
+      MAX_MSG_SIZE = 128.kilobytes.freeze
+
+      def initialize(repository, our_commit_oid, their_commit_oid)
         @gitaly_repo = repository.gitaly_repository
         @repository = repository
+        @our_commit_oid = our_commit_oid
+        @their_commit_oid = their_commit_oid
       end
-      def list_conflict_files(our_commit_oid, their_commit_oid)
+      def list_conflict_files
         request = Gitaly::ListConflictFilesRequest.new(
           repository: @gitaly_repo,
-          our_commit_oid: our_commit_oid,
-          their_commit_oid: their_commit_oid
+          our_commit_oid: @our_commit_oid,
+          their_commit_oid: @their_commit_oid
         )
         response = GitalyClient.call(@repository.storage, :conflicts_service, :list_conflict_files, request)
         files = []
@@ -38,6 +42,27 @@ module Gitlab
         files
       end
+      def resolve_conflicts(target_repository, user, files, source_branch, target_branch, commit_message)
+        reader = GitalyClient.binary_stringio(files.to_json)
+
+        req_enum = Enumerator.new do |y|
+          header = resolve_conflicts_request_header(target_repository, user, source_branch, target_branch, commit_message)
+          y.yield Gitaly::ResolveConflictsRequest.new(header: header)
+
+          until reader.eof?
+            chunk = reader.read(MAX_MSG_SIZE)
+
+            y.yield Gitaly::ResolveConflictsRequest.new(files_json: chunk)
+          end
+        end
+
+        response = GitalyClient.call(@repository.storage, :conflicts_service, :resolve_conflicts, req_enum, remote_storage: target_repository.storage)
+
+        if response.resolution_error.present?
+          raise Gitlab::Git::Conflict::Resolver::ResolutionError, response.resolution_error
+        end
+      end
+
       private
       def conflict_file_from_gitaly(header, content)
@@ -55,6 +80,19 @@ 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

@@ -3,10 +3,14 @@ require 'spec_helper'
 describe Gitlab::GitalyClient::ConflictsService do
   let(:project) { create(:project, :repository) }
   let(:target_project) { create(:project, :repository) }
-  let(:repository) { project.repository }
-  let(:gitaly_repositoy) { repository.gitaly_repository }
-  let(:target_repository) { target_project.repository }
+  let(:source_repository) { project.repository.raw }
+  let(:target_repository) { target_project.repository.raw }
   let(:target_gitaly_repository) { target_repository.gitaly_repository }
+  let(:our_commit_oid) { 'f00' }
+  let(:their_commit_oid) { 'f44' }
+  let(:client) do
+    described_class.new(target_repository, our_commit_oid, their_commit_oid)
+  end
   describe '#list_conflict_files' do
     let(:request) do
@@ -15,8 +19,6 @@ describe Gitlab::GitalyClient::ConflictsService do
         their_commit_oid: their_commit_oid
       )
     end
-    let(:our_commit_oid) { 'f00' }
-    let(:their_commit_oid) { 'f44' }
     let(:our_path) { 'our/path' }
     let(:their_path) { 'their/path' }
     let(:our_mode) { 0744 }
@@ -31,9 +33,8 @@ describe Gitlab::GitalyClient::ConflictsService do
       ]
     end
     let(:file) { subject[0] }
-    let(:client) { described_class.new(target_repository) }
-    subject { client.list_conflict_files(our_commit_oid, their_commit_oid) }
+    subject { client.list_conflict_files }
     it 'sends an RPC request' do
       expect_any_instance_of(Gitaly::ConflictsService::Stub).to receive(:list_conflict_files)
@@ -55,4 +56,32 @@ describe Gitlab::GitalyClient::ConflictsService do
       expect(file.commit_oid).to eq(our_commit_oid)
     end
   end
+
+  describe '#resolve_conflicts' do
+    let(:user) { create(:user) }
+    let(:files) do
+      [{ old_path: 'some/path', new_path: 'some/path', content: '' }]
+    end
+    let(:source_branch) { 'master' }
+    let(:target_branch) { 'feature' }
+    let(:commit_message) { 'Solving conflicts' }
+
+    subject do
+      client.resolve_conflicts(source_repository, user, files, source_branch, target_branch, commit_message)
+    end
+
+    it 'sends an RPC request' do
+      expect_any_instance_of(Gitaly::ConflictsService::Stub).to receive(:resolve_conflicts)
+        .with(kind_of(Enumerator), kind_of(Hash)).and_return(double(resolution_error: ""))
+
+      subject
+    end
+
+    it 'raises a relevant exception if resolution_error is present' do
+      expect_any_instance_of(Gitaly::ConflictsService::Stub).to receive(:resolve_conflicts)
+        .with(kind_of(Enumerator), kind_of(Hash)).and_return(double(resolution_error: "something happened"))
+
+      expect { subject }.to raise_error(Gitlab::Git::Conflict::Resolver::ResolutionError)
+    end
+  end
 end