Migrate merged_branch_names to Gitalyfeature/migrate-merged-branch-names-to-gitaly

Closes gitaly#851

3 files changed, 85 insertions, 45 deletions

diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb
index 283134e043e..8f532a5d604 100644
--- a/lib/gitlab/git/repository.rb
+++ b/lib/gitlab/git/repository.rb
@@ -571,7 +571,21 @@ module Gitlab
       end
 
       def merged_branch_names(branch_names = [])
-        Set.new(git_merged_branch_names(branch_names))
+        return [] unless root_ref
+
+        root_sha = find_branch(root_ref)&.target
+
+        return [] unless root_sha
+
+        branches = gitaly_migrate(:merged_branch_names) do |is_enabled|
+          if is_enabled
+            gitaly_merged_branch_names(branch_names, root_sha)
+          else
+            git_merged_branch_names(branch_names, root_sha)
+          end
+        end
+
+        Set.new(branches)
       end
 
       # Return an array of Diff objects that represent the diff
@@ -1475,14 +1489,7 @@ module Gitlab
         sort_branches(branches, sort_by)
       end
 
-      # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/695
-      def git_merged_branch_names(branch_names = [])
-        return [] unless root_ref
-
-        root_sha = find_branch(root_ref)&.target
-
-        return [] unless root_sha
-
+      def git_merged_branch_names(branch_names, root_sha)
         git_arguments =
           %W[branch --merged #{root_sha}
              --format=%(refname:short)\ %(objectname)] + branch_names
@@ -1496,6 +1503,14 @@ module Gitlab
         end
       end
 
+      def gitaly_merged_branch_names(branch_names, root_sha)
+        qualified_branch_names = branch_names.map { |b| "refs/heads/#{b}" }
+
+        gitaly_ref_client.merged_braches(qualified_branch_names)
+          .reject { |b| b.target == root_sha }
+          .map(&:name)
+      end
+
       def process_count_commits_options(options)
         if options[:from] || options[:to]
           ref =
diff --git a/lib/gitlab/gitaly_client/ref_service.rb b/lib/gitlab/gitaly_client/ref_service.rb
index 5bce1009878..f8e2a27f3fe 100644
--- a/lib/gitlab/gitaly_client/ref_service.rb
+++ b/lib/gitlab/gitaly_client/ref_service.rb
@@ -14,12 +14,18 @@ module Gitlab
         request = Gitaly::FindAllBranchesRequest.new(repository: @gitaly_repo)
         response = GitalyClient.call(@storage, :ref_service, :find_all_branches, request)
 
-        response.flat_map do |message|
-          message.branches.map do |branch|
-            target_commit = Gitlab::Git::Commit.decorate(@repository, branch.target)
-            Gitlab::Git::Branch.new(@repository, branch.name, branch.target.id, target_commit)
-          end
-        end
+        consume_find_all_branches_response(response)
+      end
+
+      def merged_branches(branch_names = [])
+        request = Gitaly::FindAllBranchesRequest.new(
+          repository: @gitaly_repo,
+          merged_only: true,
+          merged_branches: branch_names.map { |s| encode_binary(s) }
+        )
+        response = GitalyClient.call(@storage, :ref_service, :find_all_branches, request)
+
+        consume_find_all_branches_response(response)
       end
 
       def default_branch_name
@@ -62,7 +68,7 @@ module Gitlab
         request = Gitaly::FindLocalBranchesRequest.new(repository: @gitaly_repo)
         request.sort_by = sort_by_param(sort_by) if sort_by
         response = GitalyClient.call(@storage, :ref_service, :find_local_branches, request)
-        consume_branches_response(response)
+        consume_find_local_branches_response(response)
       end
 
       def tags
@@ -151,7 +157,7 @@ module Gitlab
         enum_value
       end
 
-      def consume_branches_response(response)
+      def consume_find_local_branches_response(response)
         response.flat_map do |message|
           message.branches.map do |gitaly_branch|
             Gitlab::Git::Branch.new(
@@ -164,6 +170,15 @@ module Gitlab
         end
       end
 
+      def consume_find_all_branches_response(response)
+        response.flat_map do |message|
+          message.branches.map do |branch|
+            target_commit = Gitlab::Git::Commit.decorate(@repository, branch.target)
+            Gitlab::Git::Branch.new(@repository, branch.name, branch.target.id, target_commit)
+          end
+        end
+      end
+
       def consume_tags_response(response)
         response.flat_map do |message|
           message.tags.map { |gitaly_tag| Util.gitlab_tag_from_gitaly_tag(@repository, gitaly_tag) }
diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb
index f346a345f00..f4e781c599e 100644
--- a/spec/lib/gitlab/git/repository_spec.rb
+++ b/spec/lib/gitlab/git/repository_spec.rb
@@ -1283,48 +1283,58 @@ describe Gitlab::Git::Repository, seed_helper: true do
   end
 
   describe '#merged_branch_names' do
-    context 'when branch names are passed' do
-      it 'only returns the names we are asking' do
-        names = repository.merged_branch_names(%w[merge-test])
+    shared_examples 'finding merged branch names' do
+      context 'when branch names are passed' do
+        it 'only returns the names we are asking' do
+          names = repository.merged_branch_names(%w[merge-test])
 
-        expect(names).to contain_exactly('merge-test')
-      end
+          expect(names).to contain_exactly('merge-test')
+        end
 
-      it 'does not return unmerged branch names' do
-        names = repository.merged_branch_names(%w[feature])
+        it 'does not return unmerged branch names' do
+          names = repository.merged_branch_names(%w[feature])
 
-        expect(names).to be_empty
+          expect(names).to be_empty
+        end
       end
-    end
 
-    context 'when no root ref is available' do
-      it 'returns empty list' do
-        project = create(:project, :empty_repo)
+      context 'when no root ref is available' do
+        it 'returns empty list' do
+          project = create(:project, :empty_repo)
 
-        names = project.repository.merged_branch_names(%w[feature])
+          names = project.repository.merged_branch_names(%w[feature])
 
-        expect(names).to be_empty
+          expect(names).to be_empty
+        end
       end
-    end
 
-    context 'when no branch names are specified' do
-      before do
-        repository.create_branch('identical', 'master')
-      end
+      context 'when no branch names are specified' do
+        before do
+          repository.create_branch('identical', 'master')
+        end
 
-      after do
-        ensure_seeds
-      end
+        after do
+          ensure_seeds
+        end
 
-      it 'returns all merged branch names except for identical one' do
-        names = repository.merged_branch_names
+        it 'returns all merged branch names except for identical one' do
+          names = repository.merged_branch_names
 
-        expect(names).to include('merge-test')
-        expect(names).to include('fix-mode')
-        expect(names).not_to include('feature')
-        expect(names).not_to include('identical')
+          expect(names).to include('merge-test')
+          expect(names).to include('fix-mode')
+          expect(names).not_to include('feature')
+          expect(names).not_to include('identical')
+        end
       end
     end
+
+    context 'when Gitaly merged_branch_names feature is enabled' do
+      it_behaves_like 'finding merged branch names'
+    end
+
+    context 'when Gitaly merged_branch_names feature is disabled', :disable_gitaly do
+      it_behaves_like 'finding merged branch names'
+    end
   end
 
   describe "#ls_files" do