Move Gitlab::Git::Repository#add_branch to mandatory

Prior to this change, a feature flag could be used to disable this
feature. Now all requests go through Gitaly's OperationService.

Closes https://gitlab.com/gitlab-org/gitaly/issues/540

When vendoring `Gitlab::Git` again in Gitaly, this implemenation will be
gone, but this is readded there through:
https://gitlab.com/gitlab-org/gitaly/merge_requests/717

3 files changed, 22 insertions, 77 deletions

diff --git a/changelogs/unreleased/zj-add-branch-mandatory.yml b/changelogs/unreleased/zj-add-branch-mandatory.yml
new file mode 100644
index 00000000000..82712ce842d
--- /dev/null
+++ b/changelogs/unreleased/zj-add-branch-mandatory.yml
@@ -0,0 +1,5 @@
+---
+title: Adding branches through the WebUI is handled by Gitaly
+merge_request:
+author:
+type: other
diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb
index 29a3a35812c..03f137d7385 100644
--- a/lib/gitlab/git/repository.rb
+++ b/lib/gitlab/git/repository.rb
@@ -776,13 +776,9 @@ module Gitlab
       end
 
       def add_branch(branch_name, user:, target:)
-        gitaly_migrate(:operation_user_create_branch, status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled|
-          if is_enabled
-            gitaly_add_branch(branch_name, user, target)
-          else
-            rugged_add_branch(branch_name, user, target)
-          end
-        end
+        gitaly_operation_client.user_create_branch(branch_name, user, target)
+      rescue GRPC::FailedPrecondition => ex
+        raise InvalidRef, ex
       end
 
       def add_tag(tag_name, user:, target:, message: nil)
@@ -2232,22 +2228,6 @@ module Gitlab
         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 => ex
-        raise InvalidRef, ex
-      end
-
       def rugged_cherry_pick(user:, commit:, branch_name:, message:, start_branch_name:, start_repository:)
         OperationService.new(user, self).with_branch(
           branch_name,
diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb
index a7755a505d8..ac8d9a32d4e 100644
--- a/spec/models/repository_spec.rb
+++ b/spec/models/repository_spec.rb
@@ -990,65 +990,25 @@ describe Repository do
 
     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
-        expect(subject.name).to eq(branch_name)
-        expect(repository.find_branch(branch_name)).not_to be_nil
-      end
-
-      context 'with a non-existing target' do
-        let(:target) { 'fake-target' }
+    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)
 
-        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
+      subject
     end
 
-    context 'with Gitaly disabled', :disable_gitaly 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 { 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
-
-      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 { subject }.to raise_error(Gitlab::Git::HooksService::PreReceiveError)
-        end
+    it 'creates_the_branch' do
+      expect(subject.name).to eq(branch_name)
+      expect(repository.find_branch(branch_name)).not_to be_nil
+    end
 
-        it 'does not create the branch' do
-          allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([false, ''])
+    context 'with a non-existing target' do
+      let(:target) { 'fake-target' }
 
-          expect { subject }.to raise_error(Gitlab::Git::HooksService::PreReceiveError)
-          expect(repository.find_branch(branch_name)).to be_nil
-        end
+      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