From b12471586b195a287c8792b3c35893c193490e98 Mon Sep 17 00:00:00 2001 From: Ahmad Sherif Date: Wed, 20 Sep 2017 12:11:51 +0200 Subject: Migrate Git::Repository#rm_tag to Gitaly Closes gitaly#562 --- GITALY_SERVER_VERSION | 2 +- Gemfile | 2 +- Gemfile.lock | 4 ++-- lib/gitlab/git/repository.rb | 12 +++++++++++- lib/gitlab/gitaly_client/operation_service.rb | 24 ++++++++++++++++++++++++ lib/gitlab/gitaly_client/util.rb | 10 ++++++++++ spec/features/tags/master_deletes_tag_spec.rb | 27 +++++++++++++++++++++------ spec/lib/gitlab/workhorse_spec.rb | 3 ++- spec/models/repository_spec.rb | 18 ++++++++++++++---- 9 files changed, 86 insertions(+), 16 deletions(-) create mode 100644 lib/gitlab/gitaly_client/operation_service.rb diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index 9b0025a7850..787ffc30a81 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -0.40.0 +0.42.0 diff --git a/Gemfile b/Gemfile index b58632d3542..b9b9b345dfe 100644 --- a/Gemfile +++ b/Gemfile @@ -398,7 +398,7 @@ group :ed25519 do end # Gitaly GRPC client -gem 'gitaly-proto', '~> 0.33.0', require: 'gitaly' +gem 'gitaly-proto', '~> 0.37.0', require: 'gitaly' gem 'toml-rb', '~> 0.3.15', require: false diff --git a/Gemfile.lock b/Gemfile.lock index 3313a687ff0..dbcf3177f6e 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -275,7 +275,7 @@ GEM po_to_json (>= 1.0.0) rails (>= 3.2.0) gherkin-ruby (0.3.2) - gitaly-proto (0.33.0) + gitaly-proto (0.37.0) google-protobuf (~> 3.1) grpc (~> 1.0) github-linguist (4.7.6) @@ -1025,7 +1025,7 @@ DEPENDENCIES gettext (~> 3.2.2) gettext_i18n_rails (~> 1.8.0) gettext_i18n_rails_js (~> 1.2.0) - gitaly-proto (~> 0.33.0) + gitaly-proto (~> 0.37.0) github-linguist (~> 4.7.0) gitlab-flowdock-git-hook (~> 1.0.1) gitlab-markup (~> 1.6.2) diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 616b075c087..82c43d37eeb 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -661,7 +661,13 @@ module Gitlab end def rm_tag(tag_name, user:) - OperationService.new(user, self).rm_tag(find_tag(tag_name)) + gitaly_migrate(:operation_user_delete_tag) do |is_enabled| + if is_enabled + gitaly_operations_client.rm_tag(tag_name, user) + else + OperationService.new(user, self).rm_tag(find_tag(tag_name)) + end + end end def find_tag(name) @@ -1033,6 +1039,10 @@ module Gitlab Gitlab::GitalyClient::Util.repository(@storage, @relative_path) end + def gitaly_operations_client + @gitaly_operations_client ||= Gitlab::GitalyClient::OperationService.new(self) + end + def gitaly_ref_client @gitaly_ref_client ||= Gitlab::GitalyClient::RefService.new(self) end diff --git a/lib/gitlab/gitaly_client/operation_service.rb b/lib/gitlab/gitaly_client/operation_service.rb new file mode 100644 index 00000000000..cdb37e96057 --- /dev/null +++ b/lib/gitlab/gitaly_client/operation_service.rb @@ -0,0 +1,24 @@ +module Gitlab + module GitalyClient + class OperationService + def initialize(repository) + @gitaly_repo = repository.gitaly_repository + @repository = repository + end + + def rm_tag(tag_name, user) + request = Gitaly::UserDeleteTagRequest.new( + repository: @gitaly_repo, + tag_name: GitalyClient.encode(tag_name), + user: Util.gitaly_user(user) + ) + + response = GitalyClient.call(@repository.storage, :operation_service, :user_delete_tag, 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/lib/gitlab/gitaly_client/util.rb b/lib/gitlab/gitaly_client/util.rb index 8fc937496af..172796c9998 100644 --- a/lib/gitlab/gitaly_client/util.rb +++ b/lib/gitlab/gitaly_client/util.rb @@ -10,6 +10,16 @@ module Gitlab git_alternate_object_directories: Array.wrap(Gitlab::Git::Env['GIT_ALTERNATE_OBJECT_DIRECTORIES']) ) end + + def gitaly_user(gitlab_user) + return unless gitlab_user + + Gitaly::User.new( + gl_id: Gitlab::GlId.gl_id(gitlab_user), + name: gitlab_user.name.b, + email: gitlab_user.email.b + ) + end end end end diff --git a/spec/features/tags/master_deletes_tag_spec.rb b/spec/features/tags/master_deletes_tag_spec.rb index d6a6b8fc7d5..80750c904b5 100644 --- a/spec/features/tags/master_deletes_tag_spec.rb +++ b/spec/features/tags/master_deletes_tag_spec.rb @@ -35,15 +35,30 @@ feature 'Master deletes tag' do end context 'when pre-receive hook fails', js: true do - before do - allow_any_instance_of(Gitlab::Git::HooksService).to receive(:execute) - .and_raise(Gitlab::Git::HooksService::PreReceiveError, 'Do not delete tags') + context 'when Gitaly operation_user_delete_tag feature is enabled' do + before do + allow_any_instance_of(Gitlab::GitalyClient::OperationService).to receive(:rm_tag) + .and_raise(Gitlab::Git::HooksService::PreReceiveError, 'Do not delete tags') + end + + scenario 'shows the error message' do + delete_first_tag + + expect(page).to have_content('Do not delete tags') + end end - scenario 'shows the error message' do - delete_first_tag + context 'when Gitaly operation_user_delete_tag feature is disabled', skip_gitaly_mock: true do + before do + allow_any_instance_of(Gitlab::Git::HooksService).to receive(:execute) + .and_raise(Gitlab::Git::HooksService::PreReceiveError, 'Do not delete tags') + end + + scenario 'shows the error message' do + delete_first_tag - expect(page).to have_content('Do not delete tags') + expect(page).to have_content('Do not delete tags') + end end end diff --git a/spec/lib/gitlab/workhorse_spec.rb b/spec/lib/gitlab/workhorse_spec.rb index 699184ad9fe..a333ae33972 100644 --- a/spec/lib/gitlab/workhorse_spec.rb +++ b/spec/lib/gitlab/workhorse_spec.rb @@ -218,7 +218,8 @@ describe Gitlab::Workhorse do storage_name: 'default', relative_path: project.full_path + '.git', git_object_directory: '', - git_alternate_object_directories: [] + git_alternate_object_directories: [], + gl_repository: '' } } expect(subject).to include(repo_param) diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 76bb658b10d..036545a5563 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -1682,12 +1682,22 @@ describe Repository, models: true do end describe '#rm_tag' do - it 'removes a tag' do - expect(repository).to receive(:before_remove_tag) + shared_examples 'removing tag' do + it 'removes a tag' do + expect(repository).to receive(:before_remove_tag) - repository.rm_tag(create(:user), 'v1.1.0') + repository.rm_tag(build_stubbed(:user), 'v1.1.0') - expect(repository.find_tag('v1.1.0')).to be_nil + expect(repository.find_tag('v1.1.0')).to be_nil + end + end + + context 'when Gitaly operation_user_delete_tag feature is enabled' do + it_behaves_like 'removing tag' + end + + context 'when Gitaly operation_user_delete_tag feature is disabled', skip_gitaly_mock: true do + it_behaves_like 'removing tag' end end -- cgit v1.2.1