From cf09c826a534d782de9cee4d28d3cdfa504099e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Rodr=C3=ADguez?= Date: Thu, 11 May 2017 17:26:51 -0300 Subject: Re-enable gitaly migration for ref_name_for_sha after bugfixes --- lib/gitlab/git/repository.rb | 26 +++++++++++++------------- lib/gitlab/gitaly_client/ref.rb | 2 +- spec/lib/gitlab/git/repository_spec.rb | 25 +++++++++++++++++++++++++ spec/models/repository_spec.rb | 33 ++++++++++++++------------------- 4 files changed, 53 insertions(+), 33 deletions(-) diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index d380c5021ee..a2c01ec4432 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -471,19 +471,19 @@ module Gitlab # Returns a RefName for a given SHA def ref_name_for_sha(ref_path, sha) - # NOTE: This feature is intentionally disabled until - # https://gitlab.com/gitlab-org/gitaly/issues/180 is resolved - # gitaly_migrate(:find_ref_name) do |is_enabled| - # if is_enabled - # gitaly_ref_client.find_ref_name(sha, ref_path) - # else - args = %W(#{Gitlab.config.git.bin_path} for-each-ref --count=1 #{ref_path} --contains #{sha}) - - # Not found -> ["", 0] - # Found -> ["b8d95eb4969eefacb0a58f6a28f6803f8070e7b9 commit\trefs/environments/production/77\n", 0] - Gitlab::Popen.popen(args, @path).first.split.last - # end - # end + raise ArgumentError, "sha can't be empty" unless sha.present? + + gitaly_migrate(:find_ref_name) do |is_enabled| + if is_enabled + gitaly_ref_client.find_ref_name(sha, ref_path) + else + args = %W(#{Gitlab.config.git.bin_path} for-each-ref --count=1 #{ref_path} --contains #{sha}) + + # Not found -> ["", 0] + # Found -> ["b8d95eb4969eefacb0a58f6a28f6803f8070e7b9 commit\trefs/environments/production/77\n", 0] + Gitlab::Popen.popen(args, @path).first.split.last + end + end end # Returns commits collection diff --git a/lib/gitlab/gitaly_client/ref.rb b/lib/gitlab/gitaly_client/ref.rb index bf04e1fa50b..53c43e28df8 100644 --- a/lib/gitlab/gitaly_client/ref.rb +++ b/lib/gitlab/gitaly_client/ref.rb @@ -28,7 +28,7 @@ module Gitlab def find_ref_name(commit_id, ref_prefix) request = Gitaly::FindRefNameRequest.new( - repository: @repository, + repository: @gitaly_repo, commit_id: commit_id, prefix: ref_prefix ) diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index fea186fd4f4..53d492b8f74 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -26,6 +26,7 @@ describe Gitlab::Git::Repository, seed_helper: true do context 'with gitaly enabled' do before { stub_gitaly } + after { Gitlab::GitalyClient.clear_stubs! } it 'gets the branch name from GitalyClient' do expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:default_branch_name) @@ -120,6 +121,7 @@ describe Gitlab::Git::Repository, seed_helper: true do context 'with gitaly enabled' do before { stub_gitaly } + after { Gitlab::GitalyClient.clear_stubs! } it 'gets the branch names from GitalyClient' do expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:branch_names) @@ -157,6 +159,7 @@ describe Gitlab::Git::Repository, seed_helper: true do context 'with gitaly enabled' do before { stub_gitaly } + after { Gitlab::GitalyClient.clear_stubs! } it 'gets the tag names from GitalyClient' do expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:tag_names) @@ -1046,6 +1049,28 @@ describe Gitlab::Git::Repository, seed_helper: true do end end + describe '#ref_name_for_sha' do + let(:ref_path) { 'refs/heads' } + let(:sha) { repository.find_branch('master').dereferenced_target.id } + let(:ref_name) { 'refs/heads/master' } + + it 'returns the ref name for the given sha' do + expect(repository.ref_name_for_sha(ref_path, sha)).to eq(ref_name) + end + + it "returns an empty name if the ref doesn't exist" do + expect(repository.ref_name_for_sha(ref_path, "000000")).to eq("") + end + + it "raise an exception if the ref is empty" do + expect { repository.ref_name_for_sha(ref_path, "") }.to raise_error(ArgumentError) + end + + it "raise an exception if the ref is nil" do + expect { repository.ref_name_for_sha(ref_path, nil) }.to raise_error(ArgumentError) + end + end + describe '#find_commits' do it 'should return a return a collection of commits' do commits = repository.find_commits diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 61b748429d7..718b7d5e86b 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -110,22 +110,11 @@ describe Repository, models: true do end describe '#ref_name_for_sha' do - context 'ref found' do - it 'returns the ref' do - allow_any_instance_of(Gitlab::Popen).to receive(:popen). - and_return(["b8d95eb4969eefacb0a58f6a28f6803f8070e7b9 commit\trefs/environments/production/77\n", 0]) + it 'returns the ref' do + allow(repository.raw_repository).to receive(:ref_name_for_sha). + and_return('refs/environments/production/77') - expect(repository.ref_name_for_sha('bla', '0' * 40)).to eq 'refs/environments/production/77' - end - end - - context 'ref not found' do - it 'returns nil' do - allow_any_instance_of(Gitlab::Popen).to receive(:popen). - and_return(["", 0]) - - expect(repository.ref_name_for_sha('bla', '0' * 40)).to eq nil - end + expect(repository.ref_name_for_sha('bla', '0' * 40)).to eq 'refs/environments/production/77' end end @@ -1917,12 +1906,18 @@ describe Repository, models: true do describe '#is_ancestor?' do context 'Gitaly is_ancestor feature enabled' do - it "asks Gitaly server if it's an ancestor" do - commit = repository.commit - expect(repository.raw_repository).to receive(:is_ancestor?).and_call_original + let(:commit) { repository.commit } + let(:ancestor) { commit.parents.first } + + before do + allow(Gitlab::GitalyClient).to receive(:enabled?).and_return(true) allow(Gitlab::GitalyClient).to receive(:feature_enabled?).with(:is_ancestor).and_return(true) + end + + it "asks Gitaly server if it's an ancestor" do + expect_any_instance_of(Gitlab::GitalyClient::Commit).to receive(:is_ancestor).with(ancestor.id, commit.id) - expect(repository.is_ancestor?(commit.id, commit.id)).to be true + repository.is_ancestor?(ancestor.id, commit.id) end end end -- cgit v1.2.3