From aaa887febad442ecdb54680954006e63af70ac6c Mon Sep 17 00:00:00 2001 From: Andrew Newdigate Date: Mon, 14 Aug 2017 12:12:32 +0100 Subject: Client Implementation: RefService::RefExists --- GITALY_SERVER_VERSION | 2 +- Gemfile | 2 +- Gemfile.lock | 4 +- changelogs/unreleased/gitaly_ref_exists.yml | 4 ++ lib/gitlab/git/repository.rb | 51 +++++++++++++++++++---- lib/gitlab/gitaly_client/ref_service.rb | 8 ++++ spec/lib/gitlab/git/repository_spec.rb | 42 ++++++++++++++----- spec/lib/gitlab/gitaly_client/ref_service_spec.rb | 21 ++++++++-- 8 files changed, 106 insertions(+), 28 deletions(-) create mode 100644 changelogs/unreleased/gitaly_ref_exists.yml diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index 26bea73e811..9eb2aa3f109 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -0.31.0 +0.32.0 diff --git a/Gemfile b/Gemfile index 62d6b1e6446..466c399c8b1 100644 --- a/Gemfile +++ b/Gemfile @@ -401,7 +401,7 @@ group :ed25519 do end # Gitaly GRPC client -gem 'gitaly', '~> 0.27.0' +gem 'gitaly', '~> 0.29.0' gem 'toml-rb', '~> 0.3.15', require: false diff --git a/Gemfile.lock b/Gemfile.lock index ae1df783a80..c0a8294b20a 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 (0.27.0) + gitaly (0.29.0) google-protobuf (~> 3.1) grpc (~> 1.0) github-linguist (4.7.6) @@ -1019,7 +1019,7 @@ DEPENDENCIES gettext (~> 3.2.2) gettext_i18n_rails (~> 1.8.0) gettext_i18n_rails_js (~> 1.2.0) - gitaly (~> 0.27.0) + gitaly (~> 0.29.0) github-linguist (~> 4.7.0) gitlab-flowdock-git-hook (~> 1.0.1) gitlab-markup (~> 1.5.1) diff --git a/changelogs/unreleased/gitaly_ref_exists.yml b/changelogs/unreleased/gitaly_ref_exists.yml new file mode 100644 index 00000000000..f62b646e406 --- /dev/null +++ b/changelogs/unreleased/gitaly_ref_exists.yml @@ -0,0 +1,4 @@ +--- +title: Implement the Gitaly RefService::RefExists endpoint +merge_request: 13528 +author: Andrew Newdigate diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index aef7ae659fe..53aa5b12489 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -204,21 +204,26 @@ module Gitlab # # name - The name of the tag as a String. def tag_exists?(name) - !!rugged.tags[name] + gitaly_migrate(:ref_exists_tags) do |is_enabled| + if is_enabled + gitaly_ref_exists?("refs/tags/#{name}") + else + rugged_tag_exists?(name) + end + end end # Returns true if the given branch exists # # name - The name of the branch as a String. def branch_exists?(name) - rugged.branches.exists?(name) - - # If the branch name is invalid (e.g. ".foo") Rugged will raise an error. - # Whatever code calls this method shouldn't have to deal with that so - # instead we just return `false` (which is true since a branch doesn't - # exist when it has an invalid name). - rescue Rugged::ReferenceError - false + gitaly_migrate(:ref_exists_branches) do |is_enabled| + if is_enabled + gitaly_ref_exists?("refs/heads/#{name}") + else + rugged_branch_exists?(name) + end + end end # Returns an Array of branch and tag names @@ -995,6 +1000,34 @@ module Gitlab raw_output.compact end + # Returns true if the given ref name exists + # + # Ref names must start with `refs/`. + def gitaly_ref_exists?(ref_name) + gitaly_ref_client.ref_exists?(ref_name) + end + + # Returns true if the given tag exists + # + # name - The name of the tag as a String. + def rugged_tag_exists?(name) + !!rugged.tags[name] + end + + # Returns true if the given branch exists + # + # name - The name of the branch as a String. + def rugged_branch_exists?(name) + rugged.branches.exists?(name) + + # If the branch name is invalid (e.g. ".foo") Rugged will raise an error. + # Whatever code calls this method shouldn't have to deal with that so + # instead we just return `false` (which is true since a branch doesn't + # exist when it has an invalid name). + rescue Rugged::ReferenceError + false + end + def gitaly_copy_gitattributes(revision) gitaly_repository_client.apply_gitattributes(revision) end diff --git a/lib/gitlab/gitaly_client/ref_service.rb b/lib/gitlab/gitaly_client/ref_service.rb index 919fb68b8c7..cdcfed36740 100644 --- a/lib/gitlab/gitaly_client/ref_service.rb +++ b/lib/gitlab/gitaly_client/ref_service.rb @@ -70,6 +70,14 @@ module Gitlab consume_tags_response(response) end + def ref_exists?(ref_name) + request = Gitaly::RefExistsRequest.new(repository: @gitaly_repo, ref: ref_name) + response = GitalyClient.call(@storage, :ref_service, :ref_exists, request) + response.value + rescue GRPC::InvalidArgument => e + raise ArgumentError, e.message + end + private def consume_refs_response(response) diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index 4ef5d9070a2..a3ae0a4686d 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -1098,28 +1098,48 @@ describe Gitlab::Git::Repository, seed_helper: true do end describe '#tag_exists?' do - it 'returns true for an existing tag' do - tag = repository.tag_names.first + shared_examples 'checks the existence of tags' do + it 'returns true for an existing tag' do + tag = repository.tag_names.first - expect(repository.tag_exists?(tag)).to eq(true) + expect(repository.tag_exists?(tag)).to eq(true) + end + + it 'returns false for a non-existing tag' do + expect(repository.tag_exists?('v9000')).to eq(false) + end + end + + context 'when Gitaly ref_exists_tags feature is enabled' do + it_behaves_like 'checks the existence of tags' end - it 'returns false for a non-existing tag' do - expect(repository.tag_exists?('v9000')).to eq(false) + context 'when Gitaly ref_exists_tags feature is disabled', skip_gitaly_mock: true do + it_behaves_like 'checks the existence of tags' end end describe '#branch_exists?' do - it 'returns true for an existing branch' do - expect(repository.branch_exists?('master')).to eq(true) + shared_examples 'checks the existence of branches' do + it 'returns true for an existing branch' do + expect(repository.branch_exists?('master')).to eq(true) + end + + it 'returns false for a non-existing branch' do + expect(repository.branch_exists?('kittens')).to eq(false) + end + + it 'returns false when using an invalid branch name' do + expect(repository.branch_exists?('.bla')).to eq(false) + end end - it 'returns false for a non-existing branch' do - expect(repository.branch_exists?('kittens')).to eq(false) + context 'when Gitaly ref_exists_branches feature is enabled' do + it_behaves_like 'checks the existence of branches' end - it 'returns false when using an invalid branch name' do - expect(repository.branch_exists?('.bla')).to eq(false) + context 'when Gitaly ref_exists_branches feature is disabled', skip_gitaly_mock: true do + it_behaves_like 'checks the existence of branches' end end diff --git a/spec/lib/gitlab/gitaly_client/ref_service_spec.rb b/spec/lib/gitlab/gitaly_client/ref_service_spec.rb index 46efc1b18f0..6f59750b4da 100644 --- a/spec/lib/gitlab/gitaly_client/ref_service_spec.rb +++ b/spec/lib/gitlab/gitaly_client/ref_service_spec.rb @@ -1,10 +1,11 @@ require 'spec_helper' describe Gitlab::GitalyClient::RefService do - let(:project) { create(:project) } + let(:project) { create(:project, :repository) } let(:storage_name) { project.repository_storage } let(:relative_path) { project.disk_path + '.git' } - let(:client) { described_class.new(project.repository) } + let(:repository) { project.repository } + let(:client) { described_class.new(repository) } describe '#branches' do it 'sends a find_all_branches message' do @@ -84,11 +85,23 @@ describe Gitlab::GitalyClient::RefService do end describe '#find_ref_name', seed_helper: true do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH) } - let(:client) { described_class.new(repository) } subject { client.find_ref_name(SeedRepo::Commit::ID, 'refs/heads/master') } it { is_expected.to be_utf8 } it { is_expected.to eq('refs/heads/master') } end + + describe '#ref_exists?', seed_helper: true do + it 'finds the master branch ref' do + expect(client.ref_exists?('refs/heads/master')).to eq(true) + end + + it 'returns false for an illegal tag name ref' do + expect(client.ref_exists?('refs/tags/.this-tag-name-is-illegal')).to eq(false) + end + + it 'raises an argument error if the ref name parameter does not start with refs/' do + expect { client.ref_exists?('reXXXXX') }.to raise_error(ArgumentError) + end + end end -- cgit v1.2.1