Implement OperationService.UserAddBranch Gitaly RPC

lib/gitlab/git/repository.rb

@@ -656,13 +656,13 @@ module Gitlab
       end
 
       def add_branch(branch_name, user:, target:)
-        target_object = Ref.dereference_object(lookup(target))
-        raise InvalidRef.new("target not found: #{target}") unless target_object
-
-        OperationService.new(user, self).add_branch(branch_name, target_object.oid)
-        find_branch(branch_name)
-      rescue Rugged::ReferenceError => ex
-        raise InvalidRef, ex
+        gitaly_migrate(:operation_user_create_branch) do |is_enabled|
+          if is_enabled
+            gitaly_add_branch(branch_name, user, target)
+          else
+            rugged_add_branch(branch_name, user, target)
+          end
+        end
       end
 
       def add_tag(tag_name, user:, target:, message: nil)
@@ -1081,6 +1081,10 @@ module Gitlab
         @gitaly_repository_client ||= Gitlab::GitalyClient::RepositoryService.new(self)
       end
 
+      def gitaly_operation_client
+        @gitaly_operation_client ||= Gitlab::GitalyClient::OperationService.new(self)
+      end
+
       def gitaly_migrate(method, status: Gitlab::GitalyClient::MigrationStatus::OPT_IN, &block)
         Gitlab::GitalyClient.migrate(method, status: status, &block)
       rescue GRPC::NotFound => e
@@ -1472,6 +1476,22 @@ module Gitlab
           file.write(gitattributes_content)
         end
       end
+
+      def gitaly_add_branch(branch_name, user, target)
+        gitaly_operation_client.user_create_branch(branch_name, user, target)
+      rescue GRPC::FailedPrecondition => ex
+        raise InvalidRef, ex
+      end
+
+      def rugged_add_branch(branch_name, user, target)
+        target_object = Ref.dereference_object(lookup(target))
+        raise InvalidRef.new("target not found: #{target}") unless target_object
+
+        OperationService.new(user, self).add_branch(branch_name, target_object.oid)
+        find_branch(branch_name)
+      rescue Rugged::ReferenceError
+        raise InvalidRef, ex
+      end
     end
   end
 end

lib/gitlab/gitaly_client/operation_service.rb

@@ -40,6 +40,26 @@ module Gitlab
       rescue GRPC::FailedPrecondition => e
         raise Gitlab::Git::Repository::InvalidRef, e
       end
+
+      def user_create_branch(branch_name, user, start_point)
+        request = Gitaly::UserCreateBranchRequest.new(
+          repository: @gitaly_repo,
+          branch_name: GitalyClient.encode(branch_name),
+          user: Util.gitaly_user(user),
+          start_point: GitalyClient.encode(start_point)
+        )
+        response = GitalyClient.call(@repository.storage, :operation_service,
+          :user_create_branch, request)
+        if response.pre_receive_error.present?
+          raise Gitlab::Git::HooksService::PreReceiveError.new(response.pre_receive_error)
+        end
+
+        branch = response.branch
+        return nil unless branch
+
+        target_commit = Gitlab::Git::Commit.decorate(@repository, branch.target_commit)
+        Gitlab::Git::Branch.new(@repository, branch.name, target_commit.id, target_commit)
+      end
     end
   end
 end

spec/lib/gitlab/gitaly_client/operation_service_spec.rb

@@ -0,0 +1,55 @@
+require 'spec_helper'
+
+describe Gitlab::GitalyClient::OperationService do
+  let(:project) { create(:project) }
+  let(:repository) { project.repository.raw }
+  let(:client) { described_class.new(repository) }
+
+  describe '#user_create_branch' do
+    let(:user) { create(:user) }
+    let(:gitaly_user) { Gitlab::GitalyClient::Util.gitaly_user(user) }
+    let(:branch_name) { 'new' }
+    let(:start_point) { 'master' }
+    let(:request) do
+      Gitaly::UserCreateBranchRequest.new(
+        repository: repository.gitaly_repository,
+        branch_name: branch_name,
+        start_point: start_point,
+        user: gitaly_user
+      )
+    end
+    let(:gitaly_commit) { build(:gitaly_commit) }
+    let(:commit_id) { gitaly_commit.id }
+    let(:gitaly_branch) do
+      Gitaly::Branch.new(name: branch_name, target_commit: gitaly_commit)
+    end
+    let(:response) { Gitaly::UserCreateBranchResponse.new(branch: gitaly_branch) }
+    let(:commit) { Gitlab::Git::Commit.new(repository, gitaly_commit) }
+
+    subject { client.user_create_branch(branch_name, user, start_point) }
+
+    it 'sends a user_create_branch message and returns a Gitlab::git::Branch' do
+      expect_any_instance_of(Gitaly::OperationService::Stub)
+        .to receive(:user_create_branch).with(request, kind_of(Hash))
+        .and_return(response)
+
+      expect(subject.name).to eq(branch_name)
+      expect(subject.dereferenced_target).to eq(commit)
+    end
+
+    context "when pre_receive_error is present" do
+      let(:response) do
+        Gitaly::UserCreateBranchResponse.new(pre_receive_error: "something failed")
+      end
+
+      it "throws a PreReceive exception" do
+        expect_any_instance_of(Gitaly::OperationService::Stub)
+          .to receive(:user_create_branch).with(request, kind_of(Hash))
+          .and_return(response)
+
+        expect { subject }.to raise_error(
+          Gitlab::Git::HooksService::PreReceiveError, "something failed")
+      end
+    end
+  end
+end

spec/models/repository_spec.rb

@@ -815,45 +815,70 @@ describe Repository do
   end
 
   describe '#add_branch' do
-    context 'when pre hooks were successful' do
-      it 'runs without errors' do
-        hook = double(trigger: [true, nil])
-        expect(Gitlab::Git::Hook).to receive(:new).exactly(3).times.and_return(hook)
+    let(:branch_name) { 'new_feature' }
+    let(:target) { 'master' }
 
-        expect { repository.add_branch(user, 'new_feature', 'master') }.not_to raise_error
+    subject { repository.add_branch(user, branch_name, target) }
+
+    context 'with Gitaly enabled' do
+      it "calls Gitaly's OperationService" do
+        expect_any_instance_of(Gitlab::GitalyClient::OperationService)
+          .to receive(:user_create_branch).with(branch_name, user, target)
+          .and_return(nil)
+
+        subject
       end
 
-      it 'creates the branch' do
-        allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([true, nil])
-
-        branch = repository.add_branch(user, 'new_feature', 'master')
-
-        expect(branch.name).to eq('new_feature')
+      it 'creates_the_branch' do
+        expect(subject.name).to eq(branch_name)
+        expect(repository.find_branch(branch_name)).not_to be_nil
       end
 
-      it 'calls the after_create_branch hook' do
-        expect(repository).to receive(:after_create_branch)
+      context 'with a non-existing target' do
+        let(:target) { 'fake-target' }
 
-        repository.add_branch(user, 'new_feature', 'master')
+        it "returns false and doesn't create the branch" do
+          expect(subject).to be(false)
+          expect(repository.find_branch(branch_name)).to be_nil
+        end
       end
     end
 
-    context 'when pre hooks failed' do
-      it 'gets an error' do
-        allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([false, ''])
+    context 'with Gitaly disabled', skip_gitaly_mock: true do
+      context 'when pre hooks were successful' do
+        it 'runs without errors' do
+          hook = double(trigger: [true, nil])
+          expect(Gitlab::Git::Hook).to receive(:new).exactly(3).times.and_return(hook)
 
-        expect do
-          repository.add_branch(user, 'new_feature', 'master')
-        end.to raise_error(Gitlab::Git::HooksService::PreReceiveError)
+          expect { subject }.not_to raise_error
+        end
+
+        it 'creates the branch' do
+          allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([true, nil])
+
+          expect(subject.name).to eq(branch_name)
+        end
+
+        it 'calls the after_create_branch hook' do
+          expect(repository).to receive(:after_create_branch)
+
+          subject
+        end
       end
 
-      it 'does not create the branch' do
-        allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([false, ''])
+      context 'when pre hooks failed' do
+        it 'gets an error' do
+          allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([false, ''])
 
-        expect do
-          repository.add_branch(user, 'new_feature', 'master')
-        end.to raise_error(Gitlab::Git::HooksService::PreReceiveError)
-        expect(repository.find_branch('new_feature')).to be_nil
+          expect { subject }.to raise_error(Gitlab::Git::HooksService::PreReceiveError)
+        end
+
+        it 'does not create the branch' do
+          allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([false, ''])
+
+          expect { subject }.to raise_error(Gitlab::Git::HooksService::PreReceiveError)
+          expect(repository.find_branch(branch_name)).to be_nil
+        end
       end
     end
   end