diff --git a/lib/gitlab/git/commit.rb b/lib/gitlab/git/commit.rb index 23ae37ff71e..d5518814483 100644 --- a/lib/gitlab/git/commit.rb +++ b/lib/gitlab/git/commit.rb @@ -73,7 +73,7 @@ module Gitlab decorate(repo, commit) if commit rescue Rugged::ReferenceError, Rugged::InvalidError, Rugged::ObjectError, Gitlab::Git::CommandError, Gitlab::Git::Repository::NoRepository, - Rugged::OdbError, Rugged::TreeError + Rugged::OdbError, Rugged::TreeError, ArgumentError nil end diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index fc8af38d4d9..6e57061b856 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -750,13 +750,13 @@ module Gitlab end def ff_merge(user, source_sha, target_branch) - OperationService.new(user, self).with_branch(target_branch) do |our_commit| - raise ArgumentError, 'Invalid merge target' unless our_commit - - source_sha + gitaly_migrate(:operation_user_ff_branch) do |is_enabled| + if is_enabled + gitaly_ff_merge(user, source_sha, target_branch) + else + rugged_ff_merge(user, source_sha, target_branch) + end end - rescue Rugged::ReferenceError - raise ArgumentError, 'Invalid merge source' end def revert(user:, commit:, branch_name:, message:, start_branch_name:, start_repository:) @@ -1169,10 +1169,10 @@ module Gitlab Gitlab::GitalyClient.migrate(method, status: status, &block) rescue GRPC::NotFound => e raise NoRepository.new(e) - rescue GRPC::BadStatus => e - raise CommandError.new(e) rescue GRPC::InvalidArgument => e raise ArgumentError.new(e) + rescue GRPC::BadStatus => e + raise CommandError.new(e) end private @@ -1614,6 +1614,22 @@ module Gitlab run_git(args, env: env) end + + def gitaly_ff_merge(user, source_sha, target_branch) + gitaly_operations_client.user_ff_branch(user, source_sha, target_branch) + rescue GRPC::FailedPrecondition => e + raise CommitError, e + end + + def rugged_ff_merge(user, source_sha, target_branch) + OperationService.new(user, self).with_branch(target_branch) do |our_commit| + raise ArgumentError, 'Invalid merge target' unless our_commit + + source_sha + end + rescue Rugged::ReferenceError + raise ArgumentError, 'Invalid merge source' + end end end end diff --git a/lib/gitlab/gitaly_client/operation_service.rb b/lib/gitlab/gitaly_client/operation_service.rb index adaf255f24b..526d44a8b77 100644 --- a/lib/gitlab/gitaly_client/operation_service.rb +++ b/lib/gitlab/gitaly_client/operation_service.rb @@ -105,6 +105,23 @@ module Gitlab ensure request_enum.close end + + def user_ff_branch(user, source_sha, target_branch) + request = Gitaly::UserFFBranchRequest.new( + repository: @gitaly_repo, + user: Gitlab::Git::User.from_gitlab(user).to_gitaly, + commit_id: source_sha, + branch: GitalyClient.encode(target_branch) + ) + + branch_update = GitalyClient.call( + @repository.storage, + :operation_service, + :user_ff_branch, + request + ).branch_update + Gitlab::Git::OperationService::BranchUpdate.from_gitaly(branch_update) + end end end end diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index b161d25b96c..3218e610ac6 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -1609,38 +1609,56 @@ describe Gitlab::Git::Repository, seed_helper: true do subject { repository.ff_merge(user, source_sha, target_branch) } - it 'performs a ff_merge' do - expect(subject.newrev).to eq(source_sha) - expect(subject.repo_created).to be(false) - expect(subject.branch_created).to be(false) + shared_examples '#ff_merge' do + it 'performs a ff_merge' do + expect(subject.newrev).to eq(source_sha) + expect(subject.repo_created).to be(false) + expect(subject.branch_created).to be(false) - expect(repository.commit(target_branch).id).to eq(source_sha) - end + expect(repository.commit(target_branch).id).to eq(source_sha) + end - context 'with a non-existing target branch' do - subject { repository.ff_merge(user, source_sha, 'this-isnt-real') } + context 'with a non-existing target branch' do + subject { repository.ff_merge(user, source_sha, 'this-isnt-real') } - it 'throws an ArgumentError' do - expect { subject }.to raise_error(ArgumentError) + it 'throws an ArgumentError' do + expect { subject }.to raise_error(ArgumentError) + end + end + + context 'with a non-existing source commit' do + let(:source_sha) { 'f001' } + + it 'throws an ArgumentError' do + expect { subject }.to raise_error(ArgumentError) + end + end + + context 'when the source sha is not a descendant of the branch head' do + let(:source_sha) { '1a0b36b3cdad1d2ee32457c102a8c0b7056fa863' } + + it "doesn't perform the ff_merge" do + expect { subject }.to raise_error(Gitlab::Git::CommitError) + + expect(repository.commit(target_branch).id).to eq(branch_head) + end end end - context 'with a non-existing source commit' do - let(:source_sha) { 'f001' } + context 'with gitaly' do + it "calls Gitaly's OperationService" do + expect_any_instance_of(Gitlab::GitalyClient::OperationService) + .to receive(:user_ff_branch).with(user, source_sha, target_branch) + .and_return(nil) - it 'throws an ArgumentError' do - expect { subject }.to raise_error(ArgumentError) + subject end + + it_behaves_like '#ff_merge' end - context 'when the source sha is not a descendant of the branch head' do - let(:source_sha) { '1a0b36b3cdad1d2ee32457c102a8c0b7056fa863' } - - it "doesn't perform the ff_merge" do - expect { subject }.to raise_error(Gitlab::Git::CommitError) - - expect(repository.commit(target_branch).id).to eq(branch_head) - end + context 'without gitaly', :skip_gitaly_mock do + it_behaves_like '#ff_merge' end end diff --git a/spec/lib/gitlab/gitaly_client/operation_service_spec.rb b/spec/lib/gitlab/gitaly_client/operation_service_spec.rb index e144e28b5d8..d9ec28ab02e 100644 --- a/spec/lib/gitlab/gitaly_client/operation_service_spec.rb +++ b/spec/lib/gitlab/gitaly_client/operation_service_spec.rb @@ -89,4 +89,38 @@ describe Gitlab::GitalyClient::OperationService do end end end + + describe '#user_ff_branch' do + let(:target_branch) { 'my-branch' } + let(:source_sha) { 'cfe32cf61b73a0d5e9f13e774abde7ff789b1660' } + let(:request) do + Gitaly::UserFFBranchRequest.new( + repository: repository.gitaly_repository, + branch: target_branch, + commit_id: source_sha, + user: gitaly_user + ) + end + let(:branch_update) do + Gitaly::OperationBranchUpdate.new( + commit_id: source_sha, + repo_created: false, + branch_created: false + ) + 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 + expect_any_instance_of(Gitaly::OperationService::Stub) + .to receive(:user_ff_branch).with(request, kind_of(Hash)) + .and_return(response) + + 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 + end end