diff options
-rw-r--r-- | app/services/tags/create_service.rb | 2 | ||||
-rw-r--r-- | lib/gitlab/git/repository.rb | 51 | ||||
-rw-r--r-- | lib/gitlab/gitaly_client/operation_service.rb | 21 | ||||
-rw-r--r-- | lib/gitlab/gitaly_client/ref_service.rb | 14 | ||||
-rw-r--r-- | lib/gitlab/gitaly_client/util.rb | 14 | ||||
-rw-r--r-- | spec/models/repository_spec.rb | 50 | ||||
-rw-r--r-- | spec/services/tags/create_service_spec.rb | 2 |
7 files changed, 101 insertions, 53 deletions
diff --git a/app/services/tags/create_service.rb b/app/services/tags/create_service.rb index b3f4a72d6fe..cc76d0df3a1 100644 --- a/app/services/tags/create_service.rb +++ b/app/services/tags/create_service.rb @@ -11,7 +11,7 @@ module Tags begin new_tag = repository.add_tag(current_user, tag_name, target, message) - rescue Rugged::TagError + rescue Gitlab::Git::Repository::TagExistsError return error("Tag #{tag_name} already exists") rescue Gitlab::Git::HooksService::PreReceiveError => ex return error(ex.message) diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 32370a8f0e1..6d710ffe2bb 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -20,6 +20,7 @@ module Gitlab GitError = Class.new(StandardError) DeleteBranchError = Class.new(StandardError) CreateTreeError = Class.new(StandardError) + TagExistsError = Class.new(StandardError) class << self # Unlike `new`, `create` takes the storage path, not the storage name @@ -646,24 +647,13 @@ module Gitlab end def add_tag(tag_name, user:, target:, message: nil) - target_object = Ref.dereference_object(lookup(target)) - raise InvalidRef.new("target not found: #{target}") unless target_object - - user = Gitlab::Git::User.from_gitlab(user) unless user.respond_to?(:gl_id) - - options = nil # Use nil, not the empty hash. Rugged cares about this. - if message - options = { - message: message, - tagger: Gitlab::Git.committer_hash(email: user.email, name: user.name) - } + gitaly_migrate(:operation_user_add_tag) do |is_enabled| + if is_enabled + gitaly_add_tag(tag_name, user: user, target: target, message: message) + else + rugged_add_tag(tag_name, user: user, target: target, message: message) + end end - - OperationService.new(user, self).add_tag(tag_name, target_object.oid, options) - - find_tag(tag_name) - rescue Rugged::ReferenceError => ex - raise InvalidRef, ex end def rm_branch(branch_name, user:) @@ -1392,6 +1382,33 @@ module Gitlab false end + def gitaly_add_tag(tag_name, user:, target:, message: nil) + gitaly_operations_client.add_tag(tag_name, user, target, message) + end + + def rugged_add_tag(tag_name, user:, target:, message: nil) + target_object = Ref.dereference_object(lookup(target)) + raise InvalidRef.new("target not found: #{target}") unless target_object + + user = Gitlab::Git::User.from_gitlab(user) unless user.respond_to?(:gl_id) + + options = nil # Use nil, not the empty hash. Rugged cares about this. + if message + options = { + message: message, + tagger: Gitlab::Git.committer_hash(email: user.email, name: user.name) + } + end + + Gitlab::Git::OperationService.new(user, self).add_tag(tag_name, target_object.oid, options) + + find_tag(tag_name) + rescue Rugged::ReferenceError => ex + raise InvalidRef, ex + rescue Rugged::TagError + raise TagExistsError + end + def rugged_create_branch(ref, start_point) rugged_ref = rugged.branches.create(ref, start_point) target_commit = Gitlab::Git::Commit.find(self, rugged_ref.target) diff --git a/lib/gitlab/gitaly_client/operation_service.rb b/lib/gitlab/gitaly_client/operation_service.rb index cdb37e96057..2d5440e7ea8 100644 --- a/lib/gitlab/gitaly_client/operation_service.rb +++ b/lib/gitlab/gitaly_client/operation_service.rb @@ -19,6 +19,27 @@ module Gitlab raise Gitlab::Git::HooksService::PreReceiveError, pre_receive_error end end + + def add_tag(tag_name, user, target, message) + request = Gitaly::UserCreateTagRequest.new( + repository: @gitaly_repo, + user: Util.gitaly_user(user), + tag_name: GitalyClient.encode(tag_name), + target_revision: GitalyClient.encode(target), + message: GitalyClient.encode(message.to_s) + ) + + response = GitalyClient.call(@repository.storage, :operation_service, :user_create_tag, request) + if pre_receive_error = response.pre_receive_error.presence + raise Gitlab::Git::HooksService::PreReceiveError, pre_receive_error + elsif response.exists + raise Gitlab::Git::Repository::TagExistsError + end + + Util.gitlab_tag_from_gitaly_tag(@repository, response.tag) + rescue GRPC::FailedPrecondition => e + raise Gitlab::Git::Repository::InvalidRef, e + end end end end diff --git a/lib/gitlab/gitaly_client/ref_service.rb b/lib/gitlab/gitaly_client/ref_service.rb index 8ef873d5848..b0c73395cb1 100644 --- a/lib/gitlab/gitaly_client/ref_service.rb +++ b/lib/gitlab/gitaly_client/ref_service.rb @@ -155,19 +155,7 @@ module Gitlab def consume_tags_response(response) response.flat_map do |message| - message.tags.map do |gitaly_tag| - if gitaly_tag.target_commit.present? - gitaly_commit = Gitlab::Git::Commit.decorate(@repository, gitaly_tag.target_commit) - end - - Gitlab::Git::Tag.new( - @repository, - encode!(gitaly_tag.name.dup), - gitaly_tag.id, - gitaly_commit, - encode!(gitaly_tag.message.chomp) - ) - end + message.tags.map { |gitaly_tag| Util.gitlab_tag_from_gitaly_tag(@repository, gitaly_tag) } end end diff --git a/lib/gitlab/gitaly_client/util.rb b/lib/gitlab/gitaly_client/util.rb index 554166a8a9e..2fb5875a7a2 100644 --- a/lib/gitlab/gitaly_client/util.rb +++ b/lib/gitlab/gitaly_client/util.rb @@ -20,6 +20,20 @@ module Gitlab email: GitalyClient.encode(gitlab_user.email) ) end + + def gitlab_tag_from_gitaly_tag(repository, gitaly_tag) + if gitaly_tag.target_commit.present? + commit = Gitlab::Git::Commit.decorate(repository, gitaly_tag.target_commit) + end + + Gitlab::Git::Tag.new( + repository, + Gitlab::EncodingHelper.encode!(gitaly_tag.name.dup), + gitaly_tag.id, + commit, + Gitlab::EncodingHelper.encode!(gitaly_tag.message.chomp) + ) + end end end end diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index e63977adec8..525c4027e5f 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -1617,27 +1617,41 @@ describe Repository do end describe '#add_tag' do - context 'with a valid target' do - let(:user) { build_stubbed(:user) } + let(:user) { build_stubbed(:user) } - it 'creates the tag using rugged' do - expect(repository.rugged.tags).to receive(:create) - .with('8.5', repository.commit('master').id, - hash_including(message: 'foo', - tagger: hash_including(name: user.name, email: user.email))) - .and_call_original + shared_examples 'adding tag' do + context 'with a valid target' do + it 'creates the tag' do + repository.add_tag(user, '8.5', 'master', 'foo') - repository.add_tag(user, '8.5', 'master', 'foo') - end + tag = repository.find_tag('8.5') + expect(tag).to be_present + expect(tag.message).to eq('foo') + expect(tag.dereferenced_target.id).to eq(repository.commit('master').id) + end - it 'returns a Gitlab::Git::Tag object' do - tag = repository.add_tag(user, '8.5', 'master', 'foo') + it 'returns a Gitlab::Git::Tag object' do + tag = repository.add_tag(user, '8.5', 'master', 'foo') + + expect(tag).to be_a(Gitlab::Git::Tag) + end + end - expect(tag).to be_a(Gitlab::Git::Tag) + context 'with an invalid target' do + it 'returns false' do + expect(repository.add_tag(user, '8.5', 'bar', 'foo')).to be false + end end + end - it 'passes commit SHA to pre-receive and update hooks,\ - and tag SHA to post-receive hook' do + context 'when Gitaly operation_user_add_tag feature is enabled' do + it_behaves_like 'adding tag' + end + + context 'when Gitaly operation_user_add_tag feature is disabled', skip_gitaly_mock: true do + it_behaves_like 'adding tag' + + it 'passes commit SHA to pre-receive and update hooks and tag SHA to post-receive hook' do pre_receive_hook = Gitlab::Git::Hook.new('pre-receive', project) update_hook = Gitlab::Git::Hook.new('update', project) post_receive_hook = Gitlab::Git::Hook.new('post-receive', project) @@ -1662,12 +1676,6 @@ describe Repository do .with(anything, anything, tag_sha, anything) end end - - context 'with an invalid target' do - it 'returns false' do - expect(repository.add_tag(user, '8.5', 'bar', 'foo')).to be false - end - end end describe '#rm_branch' do diff --git a/spec/services/tags/create_service_spec.rb b/spec/services/tags/create_service_spec.rb index 57013b54560..e7e9080b6b0 100644 --- a/spec/services/tags/create_service_spec.rb +++ b/spec/services/tags/create_service_spec.rb @@ -28,7 +28,7 @@ describe Tags::CreateService do it 'returns an error' do expect(repository).to receive(:add_tag) .with(user, 'v1.1.0', 'master', 'Foo') - .and_raise(Rugged::TagError) + .and_raise(Gitlab::Git::Repository::TagExistsError) response = service.execute('v1.1.0', 'master', 'Foo') |