Merge branch 'fix-gitaly-not-found' into 'master'

Re-enable ref operations with gitaly after not-found fix

See merge request !10773

3 files changed, 92 insertions, 79 deletions

diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb
index c3f0de76d01..acd0037ee4f 100644
--- a/lib/gitlab/git/repository.rb
+++ b/lib/gitlab/git/repository.rb
@@ -45,17 +45,13 @@ module Gitlab
 
       # Default branch in the repository
       def root_ref
-        # NOTE: This feature is intentionally disabled until
-        # https://gitlab.com/gitlab-org/gitaly/issues/179 is resolved
-        # @root_ref ||= Gitlab::GitalyClient.migrate(:root_ref) do |is_enabled|
-        #   if is_enabled
-        #     gitaly_ref_client.default_branch_name
-        #   else
-        @root_ref ||= discover_default_branch
-        #   end
-        # end
-      rescue GRPC::BadStatus => e
-        raise CommandError.new(e)
+        @root_ref ||= gitaly_migrate(:root_ref) do |is_enabled|
+          if is_enabled
+            gitaly_ref_client.default_branch_name
+          else
+            discover_default_branch
+          end
+        end
       end
 
       # Alias to old method for compatibility
@@ -72,17 +68,13 @@ module Gitlab
       # Returns an Array of branch names
       # sorted by name ASC
       def branch_names
-        # Gitlab::GitalyClient.migrate(:branch_names) do |is_enabled|
-        #   NOTE: This feature is intentionally disabled until
-        #   https://gitlab.com/gitlab-org/gitaly/issues/179 is resolved
-        #   if is_enabled
-        #     gitaly_ref_client.branch_names
-        #   else
-        branches.map(&:name)
-        #   end
-        # end
-      rescue GRPC::BadStatus => e
-        raise CommandError.new(e)
+        gitaly_migrate(:branch_names) do |is_enabled|
+          if is_enabled
+            gitaly_ref_client.branch_names
+          else
+            branches.map(&:name)
+          end
+        end
       end
 
       # Returns an Array of Branches
@@ -152,17 +144,13 @@ module Gitlab
 
       # Returns an Array of tag names
       def tag_names
-        # Gitlab::GitalyClient.migrate(:tag_names) do |is_enabled|
-        #   NOTE: This feature is intentionally disabled until
-        #   https://gitlab.com/gitlab-org/gitaly/issues/179 is resolved
-        #   if is_enabled
-        #     gitaly_ref_client.tag_names
-        #   else
-        rugged.tags.map { |t| t.name }
-        #   end
-        # end
-      rescue GRPC::BadStatus => e
-        raise CommandError.new(e)
+        gitaly_migrate(:tag_names) do |is_enabled|
+          if is_enabled
+            gitaly_ref_client.tag_names
+          else
+            rugged.tags.map { |t| t.name }
+          end
+        end
       end
 
       # Returns an Array of Tags
@@ -1294,6 +1282,14 @@ module Gitlab
         @gitaly_commit_client ||= Gitlab::GitalyClient::Commit.new(self)
       end
 
+      def gitaly_migrate(method, &block)
+        Gitlab::GitalyClient.migrate(method, &block)
+      rescue GRPC::NotFound => e
+        raise NoRepository.new(e)
+      rescue GRPC::BadStatus => e
+        raise CommandError.new(e)
+      end
+
       # Returns the `Rugged` sorting type constant for a given
       # sort type key. Valid keys are `:none`, `:topo`, and `:date`
       def rugged_sort_type(key)
diff --git a/lib/gitlab/gitaly_client/ref.rb b/lib/gitlab/gitaly_client/ref.rb
index 2a5e8f73e55..f6c77ef1a3e 100644
--- a/lib/gitlab/gitaly_client/ref.rb
+++ b/lib/gitlab/gitaly_client/ref.rb
@@ -11,7 +11,9 @@ module Gitlab
 
       def default_branch_name
         request = Gitaly::FindDefaultBranchNameRequest.new(repository: @gitaly_repo)
-        stub.find_default_branch_name(request).name.gsub(/^refs\/heads\//, '')
+        branch_name = stub.find_default_branch_name(request).name
+
+        Gitlab::Git.branch_name(branch_name)
       end
 
       def branch_names
diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb
index 1b78910fa3c..ddedb7c3443 100644
--- a/spec/lib/gitlab/git/repository_spec.rb
+++ b/spec/lib/gitlab/git/repository_spec.rb
@@ -24,21 +24,26 @@ describe Gitlab::Git::Repository, seed_helper: true do
       end
     end
 
-    # TODO: Uncomment when feature is reenabled
-    # context 'with gitaly enabled' do
-    #   before { stub_gitaly }
-    #
-    #   it 'gets the branch name from GitalyClient' do
-    #     expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:default_branch_name)
-    #     repository.root_ref
-    #   end
-    #
-    #   it 'wraps GRPC exceptions' do
-    #     expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:default_branch_name).
-    #       and_raise(GRPC::Unknown)
-    #     expect { repository.root_ref }.to raise_error(Gitlab::Git::CommandError)
-    #   end
-    # end
+    context 'with gitaly enabled' do
+      before { stub_gitaly }
+
+      it 'gets the branch name from GitalyClient' do
+        expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:default_branch_name)
+        repository.root_ref
+      end
+
+      it 'wraps GRPC not found' do
+        expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:default_branch_name).
+          and_raise(GRPC::NotFound)
+        expect { repository.root_ref }.to raise_error(Gitlab::Git::Repository::NoRepository)
+      end
+
+      it 'wraps GRPC exceptions' do
+        expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:default_branch_name).
+          and_raise(GRPC::Unknown)
+        expect { repository.root_ref }.to raise_error(Gitlab::Git::CommandError)
+      end
+    end
   end
 
   describe "#rugged" do
@@ -113,21 +118,26 @@ describe Gitlab::Git::Repository, seed_helper: true do
     it { is_expected.to include("master") }
     it { is_expected.not_to include("branch-from-space") }
 
-    # TODO: Uncomment when feature is reenabled
-    # context 'with gitaly enabled' do
-    #   before { stub_gitaly }
-    #
-    #   it 'gets the branch names from GitalyClient' do
-    #     expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:branch_names)
-    #     subject
-    #   end
-    #
-    #   it 'wraps GRPC exceptions' do
-    #     expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:branch_names).
-    #       and_raise(GRPC::Unknown)
-    #     expect { subject }.to raise_error(Gitlab::Git::CommandError)
-    #   end
-    # end
+    context 'with gitaly enabled' do
+      before { stub_gitaly }
+
+      it 'gets the branch names from GitalyClient' do
+        expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:branch_names)
+        subject
+      end
+
+      it 'wraps GRPC not found' do
+        expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:branch_names).
+          and_raise(GRPC::NotFound)
+        expect { subject }.to raise_error(Gitlab::Git::Repository::NoRepository)
+      end
+
+      it 'wraps GRPC other exceptions' do
+        expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:branch_names).
+          and_raise(GRPC::Unknown)
+        expect { subject }.to raise_error(Gitlab::Git::CommandError)
+      end
+    end
   end
 
   describe '#tag_names' do
@@ -145,21 +155,26 @@ describe Gitlab::Git::Repository, seed_helper: true do
     it { is_expected.to include("v1.0.0") }
     it { is_expected.not_to include("v5.0.0") }
 
-    # TODO: Uncomment when feature is reenabled
-    # context 'with gitaly enabled' do
-    #   before { stub_gitaly }
-    #
-    #   it 'gets the tag names from GitalyClient' do
-    #     expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:tag_names)
-    #     subject
-    #   end
-    #
-    #   it 'wraps GRPC exceptions' do
-    #     expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:tag_names).
-    #       and_raise(GRPC::Unknown)
-    #     expect { subject }.to raise_error(Gitlab::Git::CommandError)
-    #   end
-    # end
+    context 'with gitaly enabled' do
+      before { stub_gitaly }
+
+      it 'gets the tag names from GitalyClient' do
+        expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:tag_names)
+        subject
+      end
+
+      it 'wraps GRPC not found' do
+        expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:tag_names).
+          and_raise(GRPC::NotFound)
+        expect { subject }.to raise_error(Gitlab::Git::Repository::NoRepository)
+      end
+
+      it 'wraps GRPC exceptions' do
+        expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:tag_names).
+          and_raise(GRPC::Unknown)
+        expect { subject }.to raise_error(Gitlab::Git::CommandError)
+      end
+    end
   end
 
   shared_examples 'archive check' do |extenstion|