Merge branch 'sh-optimize-jira-ref-retrieval' into 'master'

Optimize JIRA ref lookup

See merge request gitlab-org/gitlab!72739

Gemfile

@@ -476,7 +476,7 @@ end
 gem 'spamcheck', '~> 0.1.0'
 
 # Gitaly GRPC protocol definitions
-gem 'gitaly', '~> 14.3.0.pre.rc2'
+gem 'gitaly', '~> 14.4.0.pre.rc43'
 
 # KAS GRPC protocol definitions
 gem 'kas-grpc', '~> 0.0.2'

Gemfile.lock

@@ -450,7 +450,7 @@ GEM
       rails (>= 3.2.0)
     git (1.7.0)
       rchardet (~> 1.8)
-    gitaly (14.3.0.pre.rc2)
+    gitaly (14.4.0.pre.rc43)
       grpc (~> 1.0)
     github-markup (1.7.0)
     gitlab (4.16.1)
@@ -1457,7 +1457,7 @@ DEPENDENCIES
   gettext (~> 3.3)
   gettext_i18n_rails (~> 1.8.0)
   gettext_i18n_rails_js (~> 1.3)
-  gitaly (~> 14.3.0.pre.rc2)
+  gitaly (~> 14.4.0.pre.rc43)
   github-markup (~> 1.7.0)
   gitlab-chronic (~> 0.10.5)
   gitlab-dangerfiles (~> 2.3.1)

app/models/integrations/jira.rb

@@ -302,6 +302,14 @@ module Integrations
 
     private
 
+    def branch_name(noteable)
+      if Feature.enabled?(:jira_use_first_ref_by_oid, project, default_enabled: :yaml)
+        noteable.first_ref_by_oid(project.repository)
+      else
+        noteable.ref_names(project.repository).first
+      end
+    end
+
     def server_info
       strong_memoize(:server_info) do
         client_url.present? ? jira_request { client.ServerInfo.all.attrs } : nil
@@ -495,7 +503,7 @@ module Integrations
         {
           id: noteable.short_id,
           description: noteable.safe_message,
-          branch: noteable.ref_names(project.repository).first
+          branch: branch_name(noteable)
         }
       elsif noteable.is_a?(MergeRequest)
         {

config/feature_flags/development/jira_use_first_ref_by_oid.yml

+---
+name: jira_use_first_ref_by_oid
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/72739
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/343585
+milestone: '14.5'
+type: development
+group: group::integrations
+default_enabled: false

lib/gitlab/git/commit.rb

@@ -315,10 +315,18 @@ module Gitlab
       #
       def ref_names(repo)
         refs(repo).map do |ref|
-          ref.sub(%r{^refs/(heads|remotes|tags)/}, "")
+          strip_ref_prefix(ref)
         end
       end
 
+      def first_ref_by_oid(repo)
+        ref = repo.refs_by_oid(oid: id, limit: 1)&.first
+
+        return unless ref
+
+        strip_ref_prefix(ref)
+      end
+
       def message
         encode! @message
       end
@@ -466,6 +474,10 @@ module Gitlab
             commit_id.match?(/\s/)
         )
       end
+
+      def strip_ref_prefix(ref)
+        ref.sub(%r{^refs/(heads|remotes|tags)/}, "")
+      end
     end
   end
 end

lib/gitlab/git/repository.rb

@@ -519,6 +519,17 @@ module Gitlab
         @refs_hash
       end
 
+      # Returns matching refs for OID
+      #
+      # Limit of 0 means there is no limit.
+      def refs_by_oid(oid:, limit: 0)
+        wrapped_gitaly_errors do
+          gitaly_ref_client.find_refs_by_oid(oid: oid, limit: limit)
+        end
+      rescue CommandError, TypeError, NoRepository
+        nil
+      end
+
       # Returns url for submodule
       #
       # Ex.

lib/gitlab/gitaly_client/ref_service.rb

@@ -210,6 +210,13 @@ module Gitlab
         GitalyClient.call(@storage, :ref_service, :pack_refs, request, timeout: GitalyClient.long_timeout)
       end
 
+      def find_refs_by_oid(oid:, limit:)
+        request = Gitaly::FindRefsByOIDRequest.new(repository: @gitaly_repo, sort_field: :refname, oid: oid, limit: limit)
+
+        response = GitalyClient.call(@storage, :ref_service, :find_refs_by_oid, request, timeout: GitalyClient.medium_timeout)
+        response&.refs&.to_a
+      end
+
       private
 
       def consume_refs_response(response)

spec/lib/gitlab/git/commit_spec.rb

@@ -715,6 +715,14 @@ RSpec.describe Gitlab::Git::Commit, :seed_helper do
     it { is_expected.not_to include("feature") }
   end
 
+  describe '#first_ref_by_oid' do
+    let(:commit) { described_class.find(repository, 'master') }
+
+    subject { commit.first_ref_by_oid(repository) }
+
+    it { is_expected.to eq("master") }
+  end
+
   describe '.get_message' do
     let(:commit_ids) { %w[6d394385cf567f80a8fd85055db1ab4c5295806f cfe32cf61b73a0d5e9f13e774abde7ff789b1660] }
 

spec/lib/gitlab/git/repository_spec.rb

@@ -1900,6 +1900,32 @@ RSpec.describe Gitlab::Git::Repository, :seed_helper do
     end
   end
 
+  describe '#refs_by_oid' do
+    it 'returns a list of refs from a OID' do
+      refs = repository.refs_by_oid(oid: repository.commit.id)
+
+      expect(refs).to be_an(Array)
+      expect(refs).to include(Gitlab::Git::BRANCH_REF_PREFIX + repository.root_ref)
+    end
+
+    it 'returns a single ref from a OID' do
+      refs = repository.refs_by_oid(oid: repository.commit.id, limit: 1)
+
+      expect(refs).to be_an(Array)
+      expect(refs).to eq([Gitlab::Git::BRANCH_REF_PREFIX + repository.root_ref])
+    end
+
+    it 'returns empty for unknown ID' do
+      expect(repository.refs_by_oid(oid: Gitlab::Git::BLANK_SHA, limit: 0)).to eq([])
+    end
+
+    it 'returns nil for an empty repo' do
+      project = create(:project)
+
+      expect(project.repository.refs_by_oid(oid: SeedRepo::Commit::ID, limit: 0)).to be_nil
+    end
+  end
+
   describe '#set_full_path' do
     before do
       repository_rugged.config["gitlab.fullpath"] = repository_path

spec/lib/gitlab/gitaly_client/ref_service_spec.rb

@@ -282,4 +282,19 @@ RSpec.describe Gitlab::GitalyClient::RefService do
       client.pack_refs
     end
   end
+
+  describe '#find_refs_by_oid' do
+    let(:oid) { project.repository.commit.id }
+
+    it 'sends a find_refs_by_oid message' do
+      expect_any_instance_of(Gitaly::RefService::Stub)
+        .to receive(:find_refs_by_oid)
+        .with(gitaly_request_with_params(sort_field: 'refname', oid: oid, limit: 1), kind_of(Hash))
+        .and_call_original
+
+      refs = client.find_refs_by_oid(oid: oid, limit: 1)
+
+      expect(refs.to_a).to eq([Gitlab::Git::BRANCH_REF_PREFIX + project.repository.root_ref])
+    end
+  end
 end

spec/models/integrations/jira_spec.rb

@@ -876,10 +876,24 @@ RSpec.describe Integrations::Jira do
         subject
 
         expect(WebMock).to have_requested(:post, comment_url).with(
-          body: /mentioned this issue in/
+          body: /mentioned this issue.*on branch \[master/
         ).once
       end
 
+      context 'with jira_use_first_ref_by_oid feature flag disabled' do
+        before do
+          stub_feature_flags(jira_use_first_ref_by_oid: false)
+        end
+
+        it 'creates a comment on Jira' do
+          subject
+
+          expect(WebMock).to have_requested(:post, comment_url).with(
+            body: /mentioned this issue.*on branch \[master/
+          ).once
+        end
+      end
+
       it 'tracks usage' do
         expect(Gitlab::UsageDataCounters::HLLRedisCounter)
           .to receive(:track_event)