diff options
author | Rémy Coutable <remy@rymai.me> | 2018-07-19 20:48:58 +0300 |
---|---|---|
committer | Rémy Coutable <remy@rymai.me> | 2018-07-19 20:48:58 +0300 |
commit | 1df0a17a1dba03d11830da2560881374c726d1fc (patch) | |
tree | fb9a6ab224fbb3eccbbc733074aee46385ba686d | |
parent | be2410ab18136df8ffc81d667879b4fefa61b732 (diff) | |
parent | 805de7fb8d35a4bebb2f49f97584fe05a8bdd25a (diff) |
Merge branch 'gitaly-ff-branch-nil' into 'master'
Add missing Gitaly branch_update nil checks
Closes #49263
See merge request gitlab-org/gitlab-ce!20711
-rw-r--r-- | changelogs/unreleased/gitaly-ff-branch-nil.yml | 5 | ||||
-rw-r--r-- | lib/gitlab/git/operation_service.rb | 2 | ||||
-rw-r--r-- | lib/gitlab/gitaly_client/operation_service.rb | 11 | ||||
-rw-r--r-- | spec/lib/gitlab/gitaly_client/operation_service_spec.rb | 134 |
4 files changed, 141 insertions, 11 deletions
diff --git a/changelogs/unreleased/gitaly-ff-branch-nil.yml b/changelogs/unreleased/gitaly-ff-branch-nil.yml new file mode 100644 index 00000000000..e7e689e6053 --- /dev/null +++ b/changelogs/unreleased/gitaly-ff-branch-nil.yml @@ -0,0 +1,5 @@ +--- +title: Add missing Gitaly branch_update nil checks +merge_request: 20711 +author: +type: fixed diff --git a/lib/gitlab/git/operation_service.rb b/lib/gitlab/git/operation_service.rb index 280def182d5..57d748343be 100644 --- a/lib/gitlab/git/operation_service.rb +++ b/lib/gitlab/git/operation_service.rb @@ -8,6 +8,8 @@ module Gitlab alias_method :branch_created?, :branch_created def self.from_gitaly(branch_update) + return if branch_update.nil? + new( branch_update.commit_id, branch_update.repo_created, diff --git a/lib/gitlab/gitaly_client/operation_service.rb b/lib/gitlab/gitaly_client/operation_service.rb index 555733d1834..54c78fdb680 100644 --- a/lib/gitlab/gitaly_client/operation_service.rb +++ b/lib/gitlab/gitaly_client/operation_service.rb @@ -144,13 +144,14 @@ module Gitlab branch: encode_binary(target_branch) ) - branch_update = GitalyClient.call( + response = GitalyClient.call( @repository.storage, :operation_service, :user_ff_branch, request - ).branch_update - Gitlab::Git::OperationService::BranchUpdate.from_gitaly(branch_update) + ) + + Gitlab::Git::OperationService::BranchUpdate.from_gitaly(response.branch_update) rescue GRPC::FailedPrecondition => e raise Gitlab::Git::CommitError, e end @@ -306,9 +307,9 @@ module Gitlab raise Gitlab::Git::CommitError, response.commit_error elsif response.create_tree_error.presence raise Gitlab::Git::Repository::CreateTreeError, response.create_tree_error - else - Gitlab::Git::OperationService::BranchUpdate.from_gitaly(response.branch_update) end + + Gitlab::Git::OperationService::BranchUpdate.from_gitaly(response.branch_update) end def user_commit_files_request_header( diff --git a/spec/lib/gitlab/gitaly_client/operation_service_spec.rb b/spec/lib/gitlab/gitaly_client/operation_service_spec.rb index 031d1e87dc1..eaf64e3c9b4 100644 --- a/spec/lib/gitlab/gitaly_client/operation_service_spec.rb +++ b/spec/lib/gitlab/gitaly_client/operation_service_spec.rb @@ -1,10 +1,10 @@ require 'spec_helper' describe Gitlab::GitalyClient::OperationService do - let(:project) { create(:project) } + set(:project) { create(:project, :repository) } let(:repository) { project.repository.raw } let(:client) { described_class.new(repository) } - let(:user) { create(:user) } + set(:user) { create(:user) } let(:gitaly_user) { Gitlab::Git::User.from_gitlab(user).to_gitaly } describe '#user_create_branch' do @@ -151,18 +151,104 @@ describe Gitlab::GitalyClient::OperationService do end let(:response) { Gitaly::UserFFBranchResponse.new(branch_update: branch_update) } - subject { client.user_ff_branch(user, source_sha, target_branch) } - - it 'sends a user_ff_branch message and returns a BranchUpdate object' do + before do expect_any_instance_of(Gitaly::OperationService::Stub) .to receive(:user_ff_branch).with(request, kind_of(Hash)) .and_return(response) + end + subject { client.user_ff_branch(user, source_sha, target_branch) } + + it 'sends a user_ff_branch message and returns a BranchUpdate object' do expect(subject).to be_a(Gitlab::Git::OperationService::BranchUpdate) expect(subject.newrev).to eq(source_sha) expect(subject.repo_created).to be(false) expect(subject.branch_created).to be(false) end + + context 'when the response has no branch_update' do + let(:response) { Gitaly::UserFFBranchResponse.new } + + it { expect(subject).to be_nil } + end + end + + shared_examples 'cherry pick and revert errors' do + context 'when a pre_receive_error is present' do + let(:response) { response_class.new(pre_receive_error: "something failed") } + + it 'raises a PreReceiveError' do + expect { subject }.to raise_error(Gitlab::Git::PreReceiveError, "something failed") + end + end + + context 'when a commit_error is present' do + let(:response) { response_class.new(commit_error: "something failed") } + + it 'raises a CommitError' do + expect { subject }.to raise_error(Gitlab::Git::CommitError, "something failed") + end + end + + context 'when a create_tree_error is present' do + let(:response) { response_class.new(create_tree_error: "something failed") } + + it 'raises a CreateTreeError' do + expect { subject }.to raise_error(Gitlab::Git::Repository::CreateTreeError, "something failed") + end + end + + context 'when branch_update is nil' do + let(:response) { response_class.new } + + it { expect(subject).to be_nil } + end + end + + describe '#user_cherry_pick' do + let(:response_class) { Gitaly::UserCherryPickResponse } + + subject do + client.user_cherry_pick( + user: user, + commit: repository.commit, + branch_name: 'master', + message: 'Cherry-pick message', + start_branch_name: 'master', + start_repository: repository + ) + end + + before do + expect_any_instance_of(Gitaly::OperationService::Stub) + .to receive(:user_cherry_pick).with(kind_of(Gitaly::UserCherryPickRequest), kind_of(Hash)) + .and_return(response) + end + + it_behaves_like 'cherry pick and revert errors' + end + + describe '#user_revert' do + let(:response_class) { Gitaly::UserRevertResponse } + + subject do + client.user_revert( + user: user, + commit: repository.commit, + branch_name: 'master', + message: 'Revert message', + start_branch_name: 'master', + start_repository: repository + ) + end + + before do + expect_any_instance_of(Gitaly::OperationService::Stub) + .to receive(:user_revert).with(kind_of(Gitaly::UserRevertRequest), kind_of(Hash)) + .and_return(response) + end + + it_behaves_like 'cherry pick and revert errors' end describe '#user_squash' do @@ -203,7 +289,7 @@ describe Gitlab::GitalyClient::OperationService do Gitaly::UserSquashResponse.new(git_error: "something failed") end - it "throws a PreReceive exception" do + it "raises a GitError exception" do expect_any_instance_of(Gitaly::OperationService::Stub) .to receive(:user_squash).with(request, kind_of(Hash)) .and_return(response) @@ -212,5 +298,41 @@ describe Gitlab::GitalyClient::OperationService do Gitlab::Git::Repository::GitError, "something failed") end end + + describe '#user_commit_files' do + subject do + client.user_commit_files( + gitaly_user, 'my-branch', 'Commit files message', [], 'janedoe@example.com', 'Jane Doe', + 'master', repository) + end + + before do + expect_any_instance_of(Gitaly::OperationService::Stub) + .to receive(:user_commit_files).with(kind_of(Enumerator), kind_of(Hash)) + .and_return(response) + end + + context 'when a pre_receive_error is present' do + let(:response) { Gitaly::UserCommitFilesResponse.new(pre_receive_error: "something failed") } + + it 'raises a PreReceiveError' do + expect { subject }.to raise_error(Gitlab::Git::PreReceiveError, "something failed") + end + end + + context 'when an index_error is present' do + let(:response) { Gitaly::UserCommitFilesResponse.new(index_error: "something failed") } + + it 'raises a PreReceiveError' do + expect { subject }.to raise_error(Gitlab::Git::Index::IndexError, "something failed") + end + end + + context 'when branch_update is nil' do + let(:response) { Gitaly::UserCommitFilesResponse.new } + + it { expect(subject).to be_nil } + end + end end end |