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
This commit is contained in:
parent
40683268b2
commit
e012485a99
|
@ -0,0 +1,5 @@
|
||||||
|
---
|
||||||
|
title: Adding branches through the WebUI is handled by Gitaly
|
||||||
|
merge_request:
|
||||||
|
author:
|
||||||
|
type: other
|
|
@ -776,13 +776,9 @@ module Gitlab
|
||||||
end
|
end
|
||||||
|
|
||||||
def add_branch(branch_name, user:, target:)
|
def add_branch(branch_name, user:, target:)
|
||||||
gitaly_migrate(:operation_user_create_branch, status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled|
|
gitaly_operation_client.user_create_branch(branch_name, user, target)
|
||||||
if is_enabled
|
rescue GRPC::FailedPrecondition => ex
|
||||||
gitaly_add_branch(branch_name, user, target)
|
raise InvalidRef, ex
|
||||||
else
|
|
||||||
rugged_add_branch(branch_name, user, target)
|
|
||||||
end
|
|
||||||
end
|
|
||||||
end
|
end
|
||||||
|
|
||||||
def add_tag(tag_name, user:, target:, message: nil)
|
def add_tag(tag_name, user:, target:, message: nil)
|
||||||
|
@ -2232,22 +2228,6 @@ module Gitlab
|
||||||
end
|
end
|
||||||
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:)
|
def rugged_cherry_pick(user:, commit:, branch_name:, message:, start_branch_name:, start_repository:)
|
||||||
OperationService.new(user, self).with_branch(
|
OperationService.new(user, self).with_branch(
|
||||||
branch_name,
|
branch_name,
|
||||||
|
|
|
@ -990,65 +990,25 @@ describe Repository do
|
||||||
|
|
||||||
subject { repository.add_branch(user, branch_name, target) }
|
subject { repository.add_branch(user, branch_name, target) }
|
||||||
|
|
||||||
context 'with Gitaly enabled' do
|
it "calls Gitaly's OperationService" do
|
||||||
it "calls Gitaly's OperationService" do
|
expect_any_instance_of(Gitlab::GitalyClient::OperationService)
|
||||||
expect_any_instance_of(Gitlab::GitalyClient::OperationService)
|
.to receive(:user_create_branch).with(branch_name, user, target)
|
||||||
.to receive(:user_create_branch).with(branch_name, user, target)
|
.and_return(nil)
|
||||||
.and_return(nil)
|
|
||||||
|
|
||||||
subject
|
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 "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
|
end
|
||||||
|
|
||||||
context 'with Gitaly disabled', :disable_gitaly do
|
it 'creates_the_branch' do
|
||||||
context 'when pre hooks were successful' do
|
expect(subject.name).to eq(branch_name)
|
||||||
it 'runs without errors' do
|
expect(repository.find_branch(branch_name)).not_to be_nil
|
||||||
hook = double(trigger: [true, nil])
|
end
|
||||||
expect(Gitlab::Git::Hook).to receive(:new).exactly(3).times.and_return(hook)
|
|
||||||
|
|
||||||
expect { subject }.not_to raise_error
|
context 'with a non-existing target' do
|
||||||
end
|
let(:target) { 'fake-target' }
|
||||||
|
|
||||||
it 'creates the branch' do
|
it "returns false and doesn't create the branch" do
|
||||||
allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([true, nil])
|
expect(subject).to be(false)
|
||||||
|
expect(repository.find_branch(branch_name)).to be_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 '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
|
end
|
||||||
end
|
end
|
||||||
|
|
Loading…
Reference in New Issue