Commit 79719cf0 authored by Alejandro Rodríguez's avatar Alejandro Rodríguez

Add OperationService.UserDeleteBranch Gitaly RPC

parent b40192a9
...@@ -5,3 +5,4 @@ app/policies/project_policy.rb ...@@ -5,3 +5,4 @@ app/policies/project_policy.rb
app/models/concerns/relative_positioning.rb app/models/concerns/relative_positioning.rb
app/workers/stuck_merge_jobs_worker.rb app/workers/stuck_merge_jobs_worker.rb
lib/gitlab/redis/*.rb lib/gitlab/redis/*.rb
lib/gitlab/gitaly_client/operation_service.rb
...@@ -676,8 +676,14 @@ module Gitlab ...@@ -676,8 +676,14 @@ module Gitlab
end end
def rm_branch(branch_name, user:) def rm_branch(branch_name, user:)
gitaly_migrate(:operation_user_delete_branch) do |is_enabled|
if is_enabled
gitaly_operations_client.user_delete_branch(branch_name, user)
else
OperationService.new(user, self).rm_branch(find_branch(branch_name)) OperationService.new(user, self).rm_branch(find_branch(branch_name))
end end
end
end
def rm_tag(tag_name, user:) def rm_tag(tag_name, user:)
gitaly_migrate(:operation_user_delete_tag) do |is_enabled| gitaly_migrate(:operation_user_delete_tag) do |is_enabled|
......
...@@ -60,6 +60,20 @@ module Gitlab ...@@ -60,6 +60,20 @@ module Gitlab
target_commit = Gitlab::Git::Commit.decorate(@repository, branch.target_commit) target_commit = Gitlab::Git::Commit.decorate(@repository, branch.target_commit)
Gitlab::Git::Branch.new(@repository, branch.name, target_commit.id, target_commit) Gitlab::Git::Branch.new(@repository, branch.name, target_commit.id, target_commit)
end end
def user_delete_branch(branch_name, user)
request = Gitaly::UserDeleteBranchRequest.new(
repository: @gitaly_repo,
branch_name: GitalyClient.encode(branch_name),
user: Util.gitaly_user(user)
)
response = GitalyClient.call(@repository.storage, :operation_service, :user_delete_branch, request)
if pre_receive_error = response.pre_receive_error.presence
raise Gitlab::Git::HooksService::PreReceiveError, pre_receive_error
end
end
end end
end end
end end
...@@ -1444,6 +1444,34 @@ describe Gitlab::Git::Repository, seed_helper: true do ...@@ -1444,6 +1444,34 @@ describe Gitlab::Git::Repository, seed_helper: true do
end end
end end
describe '#rm_branch' do
shared_examples "user deleting a branch" do
let(:project) { create(:project, :repository) }
let(:repository) { project.repository.raw }
let(:user) { create(:user) }
let(:branch_name) { "to-be-deleted-soon" }
before do
project.team << [user, :developer]
repository.create_branch(branch_name)
end
it "removes the branch from the repo" do
repository.rm_branch(branch_name, user: user)
expect(repository.rugged.branches[branch_name]).to be_nil
end
end
context "when Gitaly user_delete_branch is enabled" do
it_behaves_like "user deleting a branch"
end
context "when Gitaly user_delete_branch is disabled", skip_gitaly_mock: true do
it_behaves_like "user deleting a branch"
end
end
def create_remote_branch(repository, remote_name, branch_name, source_branch_name) def create_remote_branch(repository, remote_name, branch_name, source_branch_name)
source_branch = repository.branches.find { |branch| branch.name == source_branch_name } source_branch = repository.branches.find { |branch| branch.name == source_branch_name }
rugged = repository.rugged rugged = repository.rugged
......
...@@ -4,10 +4,10 @@ describe Gitlab::GitalyClient::OperationService do ...@@ -4,10 +4,10 @@ describe Gitlab::GitalyClient::OperationService do
let(:project) { create(:project) } let(:project) { create(:project) }
let(:repository) { project.repository.raw } let(:repository) { project.repository.raw }
let(:client) { described_class.new(repository) } let(:client) { described_class.new(repository) }
describe '#user_create_branch' do
let(:user) { create(:user) } let(:user) { create(:user) }
let(:gitaly_user) { Gitlab::GitalyClient::Util.gitaly_user(user) } let(:gitaly_user) { Gitlab::GitalyClient::Util.gitaly_user(user) }
describe '#user_create_branch' do
let(:branch_name) { 'new' } let(:branch_name) { 'new' }
let(:start_point) { 'master' } let(:start_point) { 'master' }
let(:request) do let(:request) do
...@@ -52,4 +52,41 @@ describe Gitlab::GitalyClient::OperationService do ...@@ -52,4 +52,41 @@ describe Gitlab::GitalyClient::OperationService do
end end
end end
end end
describe '#user_delete_branch' do
let(:branch_name) { 'my-branch' }
let(:request) do
Gitaly::UserDeleteBranchRequest.new(
repository: repository.gitaly_repository,
branch_name: branch_name,
user: gitaly_user
)
end
let(:response) { Gitaly::UserDeleteBranchResponse.new }
subject { client.user_delete_branch(branch_name, user) }
it 'sends a user_delete_branch message' do
expect_any_instance_of(Gitaly::OperationService::Stub)
.to receive(:user_delete_branch).with(request, kind_of(Hash))
.and_return(response)
subject
end
context "when pre_receive_error is present" do
let(:response) do
Gitaly::UserDeleteBranchResponse.new(pre_receive_error: "something failed")
end
it "throws a PreReceive exception" do
expect_any_instance_of(Gitaly::OperationService::Stub)
.to receive(:user_delete_branch).with(request, kind_of(Hash))
.and_return(response)
expect { subject }.to raise_error(
Gitlab::Git::HooksService::PreReceiveError, "something failed")
end
end
end
end end
...@@ -901,47 +901,6 @@ describe Repository do ...@@ -901,47 +901,6 @@ describe Repository do
end end
end end
describe '#rm_branch' do
let(:old_rev) { '0b4bc9a49b562e85de7cc9e834518ea6828729b9' } # git rev-parse feature
let(:blank_sha) { '0000000000000000000000000000000000000000' }
context 'when pre hooks were successful' do
it 'runs without errors' do
expect_any_instance_of(Gitlab::Git::HooksService).to receive(:execute)
.with(git_user, repository.raw_repository, old_rev, blank_sha, 'refs/heads/feature')
expect { repository.rm_branch(user, 'feature') }.not_to raise_error
end
it 'deletes the branch' do
allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([true, nil])
expect { repository.rm_branch(user, 'feature') }.not_to raise_error
expect(repository.find_branch('feature')).to be_nil
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 do
repository.rm_branch(user, 'feature')
end.to raise_error(Gitlab::Git::HooksService::PreReceiveError)
end
it 'does not delete the branch' do
allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([false, ''])
expect do
repository.rm_branch(user, 'feature')
end.to raise_error(Gitlab::Git::HooksService::PreReceiveError)
expect(repository.find_branch('feature')).not_to be_nil
end
end
end
describe '#update_branch_with_hooks' do describe '#update_branch_with_hooks' do
let(:old_rev) { '0b4bc9a49b562e85de7cc9e834518ea6828729b9' } # git rev-parse feature let(:old_rev) { '0b4bc9a49b562e85de7cc9e834518ea6828729b9' } # git rev-parse feature
let(:new_rev) { 'a74ae73c1ccde9b974a70e82b901588071dc142a' } # commit whose parent is old_rev let(:new_rev) { 'a74ae73c1ccde9b974a70e82b901588071dc142a' } # commit whose parent is old_rev
...@@ -1716,8 +1675,7 @@ describe Repository do ...@@ -1716,8 +1675,7 @@ describe Repository do
end end
describe '#rm_branch' do describe '#rm_branch' do
let(:user) { create(:user) } shared_examples "user deleting a branch" do
it 'removes a branch' do it 'removes a branch' do
expect(repository).to receive(:before_remove_branch) expect(repository).to receive(:before_remove_branch)
expect(repository).to receive(:after_remove_branch) expect(repository).to receive(:after_remove_branch)
...@@ -1726,6 +1684,69 @@ describe Repository do ...@@ -1726,6 +1684,69 @@ describe Repository do
end end
end end
context 'with gitaly enabled' do
it_behaves_like "user deleting a branch"
context 'when pre hooks failed' do
before do
allow_any_instance_of(Gitlab::GitalyClient::OperationService)
.to receive(:user_delete_branch).and_raise(Gitlab::Git::HooksService::PreReceiveError)
end
it 'gets an error and does not delete the branch' do
expect do
repository.rm_branch(user, 'feature')
end.to raise_error(Gitlab::Git::HooksService::PreReceiveError)
expect(repository.find_branch('feature')).not_to be_nil
end
end
end
context 'with gitaly disabled', skip_gitaly_mock: true do
it_behaves_like "user deleting a branch"
let(:old_rev) { '0b4bc9a49b562e85de7cc9e834518ea6828729b9' } # git rev-parse feature
let(:blank_sha) { '0000000000000000000000000000000000000000' }
context 'when pre hooks were successful' do
it 'runs without errors' do
expect_any_instance_of(Gitlab::Git::HooksService).to receive(:execute)
.with(git_user, repository.raw_repository, old_rev, blank_sha, 'refs/heads/feature')
expect { repository.rm_branch(user, 'feature') }.not_to raise_error
end
it 'deletes the branch' do
allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([true, nil])
expect { repository.rm_branch(user, 'feature') }.not_to raise_error
expect(repository.find_branch('feature')).to be_nil
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 do
repository.rm_branch(user, 'feature')
end.to raise_error(Gitlab::Git::HooksService::PreReceiveError)
end
it 'does not delete the branch' do
allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([false, ''])
expect do
repository.rm_branch(user, 'feature')
end.to raise_error(Gitlab::Git::HooksService::PreReceiveError)
expect(repository.find_branch('feature')).not_to be_nil
end
end
end
end
describe '#rm_tag' do describe '#rm_tag' do
shared_examples 'removing tag' do shared_examples 'removing tag' do
it 'removes a tag' do it 'removes a tag' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment