From f4d68347866a9f2def248864be40f2249713b351 Mon Sep 17 00:00:00 2001 From: Jacob Vosmaer Date: Wed, 27 Jun 2018 14:29:03 +0200 Subject: [PATCH] Make Gitaly signature RPC's mandatory --- lib/gitlab/git/commit.rb | 49 +--------------------- lib/gitlab/gitaly_client/commit_service.rb | 4 ++ spec/lib/gitlab/git/commit_spec.rb | 30 ++----------- 3 files changed, 10 insertions(+), 73 deletions(-) diff --git a/lib/gitlab/git/commit.rb b/lib/gitlab/git/commit.rb index 74240cedc9d..b6822009e3a 100644 --- a/lib/gitlab/git/commit.rb +++ b/lib/gitlab/git/commit.rb @@ -220,19 +220,7 @@ module Gitlab end def shas_with_signatures(repository, shas) - GitalyClient.migrate(:filter_shas_with_signatures) do |is_enabled| - if is_enabled - Gitlab::GitalyClient::CommitService.new(repository).filter_shas_with_signatures(shas) - else - shas.select do |sha| - begin - Rugged::Commit.extract_signature(repository.rugged, sha) - rescue Rugged::OdbError - false - end - end - end - end + Gitlab::GitalyClient::CommitService.new(repository).filter_shas_with_signatures(shas) end # Only to be used when the object ids will not necessarily have a @@ -250,13 +238,7 @@ module Gitlab end def extract_signature(repository, commit_id) - repository.gitaly_migrate(:extract_commit_signature) do |is_enabled| - if is_enabled - repository.gitaly_commit_client.extract_signature(commit_id) - else - rugged_extract_signature(repository, commit_id) - end - end + repository.gitaly_commit_client.extract_signature(commit_id) end def extract_signature_lazily(repository, commit_id) @@ -276,36 +258,9 @@ module Gitlab end def batch_signature_extraction(repository, commit_ids) - repository.gitaly_migrate(:extract_commit_signature_in_batch) do |is_enabled| - if is_enabled - gitaly_batch_signature_extraction(repository, commit_ids) - else - rugged_batch_signature_extraction(repository, commit_ids) - end - end - end - - def gitaly_batch_signature_extraction(repository, commit_ids) repository.gitaly_commit_client.get_commit_signatures(commit_ids) end - def rugged_batch_signature_extraction(repository, commit_ids) - commit_ids.each_with_object({}) do |commit_id, signatures| - signature_data = rugged_extract_signature(repository, commit_id) - next unless signature_data - - signatures[commit_id] = signature_data - end - end - - def rugged_extract_signature(repository, commit_id) - begin - Rugged::Commit.extract_signature(repository.rugged, commit_id) - rescue Rugged::OdbError - nil - end - end - def get_message(repository, commit_id) BatchLoader.for({ repository: repository, commit_id: commit_id }).batch do |items, loader| items_by_repo = items.group_by { |i| i[:repository] } diff --git a/lib/gitlab/gitaly_client/commit_service.rb b/lib/gitlab/gitaly_client/commit_service.rb index 077297b9d6d..c9c414e5d33 100644 --- a/lib/gitlab/gitaly_client/commit_service.rb +++ b/lib/gitlab/gitaly_client/commit_service.rb @@ -324,6 +324,8 @@ module Gitlab return if signature.blank? && signed_text.blank? [signature, signed_text] + rescue GRPC::InvalidArgument => ex + raise ArgumentError, ex end def get_commit_signatures(commit_ids) @@ -341,6 +343,8 @@ module Gitlab end signatures + rescue GRPC::InvalidArgument => ex + raise ArgumentError, ex end def get_commit_messages(commit_ids) diff --git a/spec/lib/gitlab/git/commit_spec.rb b/spec/lib/gitlab/git/commit_spec.rb index ae69a362dda..0d7c930127c 100644 --- a/spec/lib/gitlab/git/commit_spec.rb +++ b/spec/lib/gitlab/git/commit_spec.rb @@ -309,7 +309,7 @@ describe Gitlab::Git::Commit, seed_helper: true do it { is_expected.not_to include(SeedRepo::FirstCommit::ID) } end - shared_examples '.shas_with_signatures' do + describe '.shas_with_signatures' do let(:signed_shas) { %w[5937ac0a7beb003549fc5fd26fc247adbce4a52e 570e7b2abdd848b95f2f578043fc23bd6f6fd24d] } let(:unsigned_shas) { %w[19e2e9b4ef76b422ce1154af39a91323ccc57434 c642fe9b8b9f28f9225d7ea953fe14e74748d53b] } let(:first_signed_shas) { %w[5937ac0a7beb003549fc5fd26fc247adbce4a52e c642fe9b8b9f28f9225d7ea953fe14e74748d53b] } @@ -330,14 +330,6 @@ describe Gitlab::Git::Commit, seed_helper: true do end end - describe '.shas_with_signatures with gitaly on' do - it_should_behave_like '.shas_with_signatures' - end - - describe '.shas_with_signatures with gitaly disabled', :disable_gitaly do - it_should_behave_like '.shas_with_signatures' - end - describe '.find_all' do shared_examples 'finding all commits' do it 'should return a return a collection of commits' do @@ -498,7 +490,7 @@ describe Gitlab::Git::Commit, seed_helper: true do end describe '.extract_signature_lazily' do - shared_examples 'loading signatures in batch once' do + describe 'loading signatures in batch once' do it 'fetches signatures in batch once' do commit_ids = %w[0b4bc9a49b562e85de7cc9e834518ea6828729b9 4b4918a572fa86f9771e5ba40fbd48e1eb03e2c6] signatures = commit_ids.map do |commit_id| @@ -516,27 +508,13 @@ describe Gitlab::Git::Commit, seed_helper: true do subject { described_class.extract_signature_lazily(repository, commit_id).itself } - context 'with Gitaly extract_commit_signature_in_batch feature enabled' do - it_behaves_like 'extracting commit signature' - it_behaves_like 'loading signatures in batch once' - end - - context 'with Gitaly extract_commit_signature_in_batch feature disabled', :disable_gitaly do - it_behaves_like 'extracting commit signature' - it_behaves_like 'loading signatures in batch once' - end + it_behaves_like 'extracting commit signature' end describe '.extract_signature' do subject { described_class.extract_signature(repository, commit_id) } - context 'with gitaly' do - it_behaves_like 'extracting commit signature' - end - - context 'without gitaly', :disable_gitaly do - it_behaves_like 'extracting commit signature' - end + it_behaves_like 'extracting commit signature' end end