Client changes for Tag,BranchNamesContainingCommit

As part of gitlab-org/gitaly#884, this commit contains the client
implementation for both TagNamesContaintingCommit and
BranchNamesContainingCommit. The interface in the Repository model stays
the same, but the implementation on the serverside, e.g. Gitaly, uses
`for-each-ref`, as opposed to `branch` or `tag` which both aren't
plumbing command. The result stays the same.

On the serverside, we have the opportunity to limit the number of names
to return. However, this is not supported on the front end yet. My
proposal to use this ability: gitlab-org/gitlab-ce#42581. For now, this
ability is not used as that would change more behaviours on a feature
flag which might lead to unexpected changes on page refresh for example.

app/models/repository.rb

@@ -721,11 +721,11 @@ class Repository
   end
 
   def branch_names_contains(sha)
-    refs_contains_sha('branch', sha)
+    raw_repository.branch_names_contains_sha(sha)
   end
 
   def tag_names_contains(sha)
-    refs_contains_sha('tag', sha)
+    raw_repository.tag_names_contains_sha(sha)
   end
 
   def local_branches

lib/gitlab/git/branch.rb

@@ -1,13 +1,17 @@
-# Gitaly note: JV: no RPC's here.
-
 module Gitlab
   module Git
     class Branch < Ref
-      def self.find(repo, branch_name)
-        if branch_name.is_a?(Gitlab::Git::Branch)
-          branch_name
-        else
-          repo.find_branch(branch_name)
+      class << self
+        def find(repo, branch_name)
+          if branch_name.is_a?(Gitlab::Git::Branch)
+            branch_name
+          else
+            repo.find_branch(branch_name)
+          end
+        end
+
+        def names_contains_sha(repo, sha, limit: 0)
+          GitalyClient::RefService.new(repo).branch_names_contains_sha(sha)
         end
       end
 

lib/gitlab/git/repository.rb

@@ -1355,20 +1355,23 @@ module Gitlab
         raise CommandError.new(e)
       end
 
-      def refs_contains_sha(ref_type, sha)
-        args = %W(#{ref_type} --contains #{sha})
-        names = run_git(args).first
-
-        if names.respond_to?(:split)
-          names = names.split("\n").map(&:strip)
-
-          names.each do |name|
-            name.slice! '* '
+      def branch_names_contains_sha(sha)
+        gitaly_migrate(:branch_names_contains_sha) do |is_enabled|
+          if is_enabled
+            Gitlab::Git::Branch.names_contains_sha(self, sha)
+          else
+            refs_contains_sha(:branch, sha)
           end
+        end
+      end
 
-          names
-        else
-          []
+      def tag_names_contains_sha(sha)
+        gitaly_migrate(:tag_names_contains_sha) do |is_enabled|
+          if is_enabled
+            Gitlab::Git::Tag.names_contains_sha(self, sha)
+          else
+            refs_contains_sha(:tag, sha)
+          end
         end
       end
 
@@ -1446,6 +1449,21 @@ module Gitlab
         end
       end
 
+      def refs_contains_sha(ref_type, sha)
+        args = %W(#{ref_type} --contains #{sha})
+        names = run_git(args).first
+
+        return [] unless names.respond_to?(:split)
+
+        names = names.split("\n").map(&:strip)
+
+        names.each do |name|
+          name.slice! '* '
+        end
+
+        names
+      end
+
       def shell_write_ref(ref_path, ref, old_ref)
         raise ArgumentError, "invalid ref_path #{ref_path.inspect}" if ref_path.include?(' ')
         raise ArgumentError, "invalid ref #{ref.inspect}" if ref.include?("\x00")

lib/gitlab/git/tag.rb

@@ -1,8 +1,12 @@
-# Gitaly note: JV: no RPC's here.
-#
 module Gitlab
   module Git
     class Tag < Ref
+      class << self
+        def names_contains_sha(repo, sha)
+          GitalyClient::RefService.new(repo).branch_names_contains_sha(sha)
+        end
+      end
+
       attr_reader :object_sha
 
       def initialize(repository, name, target, target_commit, message = nil)

lib/gitlab/gitaly_client/ref_service.rb

@@ -145,6 +145,32 @@ module Gitlab
         raise Gitlab::Git::Repository::GitError, response.git_error if response.git_error.present?
       end
 
+      # Limit: 0 implies no limit, thus all tag names will be returned
+      def tag_names_containing(sha, limit: 0)
+        request = Gitaly::ListTagNamesContainingCommitRequest.new(
+          repository: @gitaly_repo,
+          commit_id: sha,
+          limit: limit
+        )
+
+        stream = GitalyClient.call(@repository.storage, :ref_service, :list_tag_names_containing_commit, request)
+
+        stream.each_with_object([]) { |response, array| array.concat(response.tag_names) }
+      end
+
+      # Limit: 0 implies no limit, thus all tag names will be returned
+      def branch_names_contains_sha(sha, limit: 0)
+        request = Gitaly::ListBranchNamesContainingCommitRequest.new(
+          repository: @gitaly_repo,
+          commit_id: sha,
+          limit: limit
+        )
+
+        stream = GitalyClient.call(@repository.storage, :ref_service, :list_branch_names_containing_commit, request)
+
+        stream.each_with_object([]) { |response, array| array.concat(response.branch_names) }
+      end
+
       private
 
       def consume_refs_response(response)

spec/models/repository_spec.rb

@@ -36,26 +36,49 @@ describe Repository do
   end
 
   describe '#branch_names_contains' do
-    subject { repository.branch_names_contains(sample_commit.id) }
+    shared_examples '#branch_names_contains' do
+      set(:project) { create(:project, :repository) }
+      let(:repository) { project.repository }
 
-    it { is_expected.to include('master') }
-    it { is_expected.not_to include('feature') }
-    it { is_expected.not_to include('fix') }
+      subject { repository.branch_names_contains(sample_commit.id) }
 
-    describe 'when storage is broken', :broken_storage  do
-      it 'should raise a storage error' do
-        expect_to_raise_storage_error do
-          broken_repository.branch_names_contains(sample_commit.id)
+      it { is_expected.to include('master') }
+      it { is_expected.not_to include('feature') }
+      it { is_expected.not_to include('fix') }
+
+      describe 'when storage is broken', :broken_storage  do
+        it 'should raise a storage error' do
+          expect_to_raise_storage_error do
+            broken_repository.branch_names_contains(sample_commit.id)
+          end
         end
       end
     end
+
+    context 'when gitaly is enabled' do
+      it_behaves_like '#branch_names_contains'
+    end
+
+    context 'when gitaly is disabled', :skip_gitaly_mock do
+      it_behaves_like '#branch_names_contains'
+    end
   end
 
   describe '#tag_names_contains' do
-    subject { repository.tag_names_contains(sample_commit.id) }
+    shared_examples '#tag_names_contains' do
+      subject { repository.tag_names_contains(sample_commit.id) }
 
-    it { is_expected.to include('v1.1.0') }
-    it { is_expected.not_to include('v1.0.0') }
+      it { is_expected.to include('v1.1.0') }
+      it { is_expected.not_to include('v1.0.0') }
+    end
+
+    context 'when gitaly is enabled' do
+      it_behaves_like '#tag_names_contains'
+    end
+
+    context 'when gitaly is enabled', :skip_gitaly_mock do
+      it_behaves_like '#tag_names_contains'
+    end
   end
 
   describe 'tags_sorted_by' do