summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDouwe Maan <douwe@gitlab.com>2017-10-04 07:34:33 +0000
committerDouwe Maan <douwe@gitlab.com>2017-10-04 07:34:33 +0000
commitc6e5a77e5124a258933246faf886ddf61a9fe9c4 (patch)
tree5bcf6a74473e64866a371a794189e0391443c6c4
parent2ee93c486cfbdf5555677d570cc63d1542c159ba (diff)
parent79719cf003e21561d95e17e4922466a274c59a6b (diff)
downloadgitlab-ce-c6e5a77e5124a258933246faf886ddf61a9fe9c4.tar.gz
Merge branch 'gitaly-delete-branch' into 'master'
Implement OperationService.UserDeleteBranch Gitaly RPC See merge request gitlab-org/gitlab-ce!14603
-rw-r--r--.flayignore1
-rw-r--r--lib/gitlab/git/repository.rb8
-rw-r--r--lib/gitlab/gitaly_client/operation_service.rb14
-rw-r--r--spec/lib/gitlab/git/repository_spec.rb28
-rw-r--r--spec/lib/gitlab/gitaly_client/operation_service_spec.rb41
-rw-r--r--spec/models/repository_spec.rb113
6 files changed, 156 insertions, 49 deletions
diff --git a/.flayignore b/.flayignore
index b63ce4c4df0..acac0ce14c9 100644
--- a/.flayignore
+++ b/.flayignore
@@ -5,3 +5,4 @@ app/policies/project_policy.rb
app/models/concerns/relative_positioning.rb
app/workers/stuck_merge_jobs_worker.rb
lib/gitlab/redis/*.rb
+lib/gitlab/gitaly_client/operation_service.rb
diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb
index 22b735c6f7b..a76befe4b52 100644
--- a/lib/gitlab/git/repository.rb
+++ b/lib/gitlab/git/repository.rb
@@ -676,7 +676,13 @@ module Gitlab
end
def rm_branch(branch_name, user:)
- OperationService.new(user, self).rm_branch(find_branch(branch_name))
+ 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))
+ end
+ end
end
def rm_tag(tag_name, user:)
diff --git a/lib/gitlab/gitaly_client/operation_service.rb b/lib/gitlab/gitaly_client/operation_service.rb
index 46bd5c18603..81ddaf13e10 100644
--- a/lib/gitlab/gitaly_client/operation_service.rb
+++ b/lib/gitlab/gitaly_client/operation_service.rb
@@ -60,6 +60,20 @@ module Gitlab
target_commit = Gitlab::Git::Commit.decorate(@repository, branch.target_commit)
Gitlab::Git::Branch.new(@repository, branch.name, target_commit.id, target_commit)
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
diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb
index a0482e30a33..f405b2f2684 100644
--- a/spec/lib/gitlab/git/repository_spec.rb
+++ b/spec/lib/gitlab/git/repository_spec.rb
@@ -1444,6 +1444,34 @@ describe Gitlab::Git::Repository, seed_helper: true do
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)
source_branch = repository.branches.find { |branch| branch.name == source_branch_name }
rugged = repository.rugged
diff --git a/spec/lib/gitlab/gitaly_client/operation_service_spec.rb b/spec/lib/gitlab/gitaly_client/operation_service_spec.rb
index 769b14687ac..7bd6a7fa842 100644
--- a/spec/lib/gitlab/gitaly_client/operation_service_spec.rb
+++ b/spec/lib/gitlab/gitaly_client/operation_service_spec.rb
@@ -4,10 +4,10 @@ describe Gitlab::GitalyClient::OperationService do
let(:project) { create(:project) }
let(:repository) { project.repository.raw }
let(:client) { described_class.new(repository) }
+ let(:user) { create(:user) }
+ let(:gitaly_user) { Gitlab::GitalyClient::Util.gitaly_user(user) }
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
@@ -52,4 +52,41 @@ describe Gitlab::GitalyClient::OperationService do
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
diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb
index 8a4dcdc311e..bdcdad46390 100644
--- a/spec/models/repository_spec.rb
+++ b/spec/models/repository_spec.rb
@@ -901,47 +901,6 @@ describe Repository do
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
let(:old_rev) { '0b4bc9a49b562e85de7cc9e834518ea6828729b9' } # git rev-parse feature
let(:new_rev) { 'a74ae73c1ccde9b974a70e82b901588071dc142a' } # commit whose parent is old_rev
@@ -1744,13 +1703,75 @@ describe Repository do
end
describe '#rm_branch' do
- let(:user) { create(:user) }
+ shared_examples "user deleting a branch" do
+ it 'removes a branch' do
+ expect(repository).to receive(:before_remove_branch)
+ expect(repository).to receive(:after_remove_branch)
+
+ repository.rm_branch(user, 'feature')
+ end
+ end
- it 'removes a branch' do
- expect(repository).to receive(:before_remove_branch)
- expect(repository).to receive(:after_remove_branch)
+ context 'with gitaly enabled' do
+ it_behaves_like "user deleting a branch"
- repository.rm_branch(user, 'feature')
+ 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