Incorporate Gitaly's OperationService.UserFFBranch RPCgitaly-ff-merge

5 files changed, 115 insertions, 30 deletions

diff --git a/lib/gitlab/git/commit.rb b/lib/gitlab/git/commit.rb
index 23ae37ff71e..d5518814483 100644
--- a/lib/gitlab/git/commit.rb
+++ b/lib/gitlab/git/commit.rb
@@ -73,7 +73,7 @@ module Gitlab
           decorate(repo, commit) if commit
         rescue Rugged::ReferenceError, Rugged::InvalidError, Rugged::ObjectError,
                Gitlab::Git::CommandError, Gitlab::Git::Repository::NoRepository,
-               Rugged::OdbError, Rugged::TreeError
+               Rugged::OdbError, Rugged::TreeError, ArgumentError
           nil
         end
 
diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb
index fc8af38d4d9..6e57061b856 100644
--- a/lib/gitlab/git/repository.rb
+++ b/lib/gitlab/git/repository.rb
@@ -750,13 +750,13 @@ module Gitlab
       end
 
       def ff_merge(user, source_sha, target_branch)
-        OperationService.new(user, self).with_branch(target_branch) do |our_commit|
-          raise ArgumentError, 'Invalid merge target' unless our_commit
-
-          source_sha
+        gitaly_migrate(:operation_user_ff_branch) do |is_enabled|
+          if is_enabled
+            gitaly_ff_merge(user, source_sha, target_branch)
+          else
+            rugged_ff_merge(user, source_sha, target_branch)
+          end
         end
-      rescue Rugged::ReferenceError
-        raise ArgumentError, 'Invalid merge source'
       end
 
       def revert(user:, commit:, branch_name:, message:, start_branch_name:, start_repository:)
@@ -1169,10 +1169,10 @@ module Gitlab
         Gitlab::GitalyClient.migrate(method, status: status, &block)
       rescue GRPC::NotFound => e
         raise NoRepository.new(e)
-      rescue GRPC::BadStatus => e
-        raise CommandError.new(e)
       rescue GRPC::InvalidArgument => e
         raise ArgumentError.new(e)
+      rescue GRPC::BadStatus => e
+        raise CommandError.new(e)
       end
 
       private
@@ -1614,6 +1614,22 @@ module Gitlab
 
         run_git(args, env: env)
       end
+
+      def gitaly_ff_merge(user, source_sha, target_branch)
+        gitaly_operations_client.user_ff_branch(user, source_sha, target_branch)
+      rescue GRPC::FailedPrecondition => e
+        raise CommitError, e
+      end
+
+      def rugged_ff_merge(user, source_sha, target_branch)
+        OperationService.new(user, self).with_branch(target_branch) do |our_commit|
+          raise ArgumentError, 'Invalid merge target' unless our_commit
+
+          source_sha
+        end
+      rescue Rugged::ReferenceError
+        raise ArgumentError, 'Invalid merge source'
+      end
     end
   end
 end
diff --git a/lib/gitlab/gitaly_client/operation_service.rb b/lib/gitlab/gitaly_client/operation_service.rb
index adaf255f24b..526d44a8b77 100644
--- a/lib/gitlab/gitaly_client/operation_service.rb
+++ b/lib/gitlab/gitaly_client/operation_service.rb
@@ -105,6 +105,23 @@ module Gitlab
       ensure
         request_enum.close
       end
+
+      def user_ff_branch(user, source_sha, target_branch)
+        request = Gitaly::UserFFBranchRequest.new(
+          repository: @gitaly_repo,
+          user: Gitlab::Git::User.from_gitlab(user).to_gitaly,
+          commit_id: source_sha,
+          branch: GitalyClient.encode(target_branch)
+        )
+
+        branch_update = GitalyClient.call(
+          @repository.storage,
+          :operation_service,
+          :user_ff_branch,
+          request
+        ).branch_update
+        Gitlab::Git::OperationService::BranchUpdate.from_gitaly(branch_update)
+      end
     end
   end
 end
diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb
index b161d25b96c..3218e610ac6 100644
--- a/spec/lib/gitlab/git/repository_spec.rb
+++ b/spec/lib/gitlab/git/repository_spec.rb
@@ -1609,38 +1609,56 @@ describe Gitlab::Git::Repository, seed_helper: true do
 
     subject { repository.ff_merge(user, source_sha, target_branch) }
 
-    it 'performs a ff_merge' do
-      expect(subject.newrev).to eq(source_sha)
-      expect(subject.repo_created).to be(false)
-      expect(subject.branch_created).to be(false)
+    shared_examples '#ff_merge' do
+      it 'performs a ff_merge' do
+        expect(subject.newrev).to eq(source_sha)
+        expect(subject.repo_created).to be(false)
+        expect(subject.branch_created).to be(false)
 
-      expect(repository.commit(target_branch).id).to eq(source_sha)
-    end
+        expect(repository.commit(target_branch).id).to eq(source_sha)
+      end
 
-    context 'with a non-existing target branch' do
-      subject { repository.ff_merge(user, source_sha, 'this-isnt-real') }
+      context 'with a non-existing target branch' do
+        subject { repository.ff_merge(user, source_sha, 'this-isnt-real') }
 
-      it 'throws an ArgumentError' do
-        expect { subject }.to raise_error(ArgumentError)
+        it 'throws an ArgumentError' do
+          expect { subject }.to raise_error(ArgumentError)
+        end
       end
-    end
 
-    context 'with a non-existing source commit' do
-      let(:source_sha) { 'f001' }
+      context 'with a non-existing source commit' do
+        let(:source_sha) { 'f001' }
 
-      it 'throws an ArgumentError' do
-        expect { subject }.to raise_error(ArgumentError)
+        it 'throws an ArgumentError' do
+          expect { subject }.to raise_error(ArgumentError)
+        end
       end
-    end
 
-    context 'when the source sha is not a descendant of the branch head' do
-      let(:source_sha) { '1a0b36b3cdad1d2ee32457c102a8c0b7056fa863' }
+      context 'when the source sha is not a descendant of the branch head' do
+        let(:source_sha) { '1a0b36b3cdad1d2ee32457c102a8c0b7056fa863' }
+
+        it "doesn't perform the ff_merge" do
+          expect { subject }.to raise_error(Gitlab::Git::CommitError)
+
+          expect(repository.commit(target_branch).id).to eq(branch_head)
+        end
+      end
+    end
 
-      it "doesn't perform the ff_merge" do
-        expect { subject }.to raise_error(Gitlab::Git::CommitError)
+    context 'with gitaly' do
+      it "calls Gitaly's OperationService" do
+        expect_any_instance_of(Gitlab::GitalyClient::OperationService)
+          .to receive(:user_ff_branch).with(user, source_sha, target_branch)
+          .and_return(nil)
 
-        expect(repository.commit(target_branch).id).to eq(branch_head)
+        subject
       end
+
+      it_behaves_like '#ff_merge'
+    end
+
+    context 'without gitaly', :skip_gitaly_mock do
+      it_behaves_like '#ff_merge'
     end
   end
 
diff --git a/spec/lib/gitlab/gitaly_client/operation_service_spec.rb b/spec/lib/gitlab/gitaly_client/operation_service_spec.rb
index e144e28b5d8..d9ec28ab02e 100644
--- a/spec/lib/gitlab/gitaly_client/operation_service_spec.rb
+++ b/spec/lib/gitlab/gitaly_client/operation_service_spec.rb
@@ -89,4 +89,38 @@ describe Gitlab::GitalyClient::OperationService do
       end
     end
   end
+
+  describe '#user_ff_branch' do
+    let(:target_branch) { 'my-branch' }
+    let(:source_sha) { 'cfe32cf61b73a0d5e9f13e774abde7ff789b1660' }
+    let(:request) do
+      Gitaly::UserFFBranchRequest.new(
+        repository: repository.gitaly_repository,
+        branch: target_branch,
+        commit_id: source_sha,
+        user: gitaly_user
+      )
+    end
+    let(:branch_update) do
+      Gitaly::OperationBranchUpdate.new(
+        commit_id: source_sha,
+        repo_created: false,
+        branch_created: false
+      )
+    end
+    let(:response) { Gitaly::UserFFBranchResponse.new(branch_update: branch_update) }
+
+    subject { client.user_ff_branch(user, source_sha, target_branch) }
+
+    it 'sends a user_ff_branch message and returns a BranchUpdate object' do
+      expect_any_instance_of(Gitaly::OperationService::Stub)
+        .to receive(:user_ff_branch).with(request, kind_of(Hash))
+        .and_return(response)
+
+      expect(subject).to be_a(Gitlab::Git::OperationService::BranchUpdate)
+      expect(subject.newrev).to eq(source_sha)
+      expect(subject.repo_created).to be(false)
+      expect(subject.branch_created).to be(false)
+    end
+  end
 end