diff --git a/Gemfile b/Gemfile index 11bef61d4875d7bbd84cdd1eca825a0ff42e430e..3a04f1e1395b7f694911e157f11bdb8c84f4733c 100644 --- a/Gemfile +++ b/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' diff --git a/Gemfile.lock b/Gemfile.lock index 40b75863c650785dc1423462f1c870613fbb00a0..7497734d6dcffd96cd7f0763d5bb62ba7cfdf5bb 100644 --- a/Gemfile.lock +++ b/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) diff --git a/app/models/integrations/jira.rb b/app/models/integrations/jira.rb index a23c6e18d71ccae748707c961cda630ea4ad8677..26a1d1aed190f3ef74891144d2d7210895412d92 100644 --- a/app/models/integrations/jira.rb +++ b/app/models/integrations/jira.rb @@ -302,6 +302,14 @@ def issue_transition_enabled? 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 @@ def build_entity_meta(noteable) { id: noteable.short_id, description: noteable.safe_message, - branch: noteable.ref_names(project.repository).first + branch: branch_name(noteable) } elsif noteable.is_a?(MergeRequest) { diff --git a/config/feature_flags/development/jira_use_first_ref_by_oid.yml b/config/feature_flags/development/jira_use_first_ref_by_oid.yml new file mode 100644 index 0000000000000000000000000000000000000000..10e2ad1b8ada6a929f26a3e0f6eca1ca473bff52 --- /dev/null +++ b/config/feature_flags/development/jira_use_first_ref_by_oid.yml @@ -0,0 +1,8 @@ +--- +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 diff --git a/lib/gitlab/git/commit.rb b/lib/gitlab/git/commit.rb index 6605e896ef1d4306dead127d2f0cfc513a5e204a..267107e04e6959d8e9721a96a3645a2029d6419f 100644 --- a/lib/gitlab/git/commit.rb +++ b/lib/gitlab/git/commit.rb @@ -315,10 +315,18 @@ def stats # 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 @@ def self.valid?(commit_id) commit_id.match?(/\s/) ) end + + def strip_ref_prefix(ref) + ref.sub(%r{^refs/(heads|remotes|tags)/}, "") + end end end end diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index dde9226a93efca8dd8ef01f52a636395ba7209e8..c5f3f224a3d46e0d1431a528e1b67ab68fdea996 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -519,6 +519,17 @@ def refs_hash @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. diff --git a/lib/gitlab/gitaly_client/ref_service.rb b/lib/gitlab/gitaly_client/ref_service.rb index aa4626029b539d044b100620fa64b8a5e3bcd5be..c28abda3843e1f8ea00b12fa7ea7ffcabdda5cfd 100644 --- a/lib/gitlab/gitaly_client/ref_service.rb +++ b/lib/gitlab/gitaly_client/ref_service.rb @@ -210,6 +210,13 @@ def pack_refs 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) diff --git a/spec/lib/gitlab/git/commit_spec.rb b/spec/lib/gitlab/git/commit_spec.rb index f4dba5e8d586978352f16237f3df7fec4f2c6124..11510daf9c0b1ab1e936d4a314304c4eac3dd2fa 100644 --- a/spec/lib/gitlab/git/commit_spec.rb +++ b/spec/lib/gitlab/git/commit_spec.rb @@ -715,6 +715,14 @@ 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] } diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index 8d28515d1af8697d35d6a0c4f1bccb842e26ee9d..1004947e368bee4123736f1d5d878dc9a26a5bce 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -1900,6 +1900,32 @@ def create_commit(blobs) 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 diff --git a/spec/lib/gitlab/gitaly_client/ref_service_spec.rb b/spec/lib/gitlab/gitaly_client/ref_service_spec.rb index b4355b56bac7ce379fa48a9f6ae62b9fdcaffb3d..d013bb2bd55868912093fbc0a2adba0112e8035d 100644 --- a/spec/lib/gitlab/gitaly_client/ref_service_spec.rb +++ b/spec/lib/gitlab/gitaly_client/ref_service_spec.rb @@ -282,4 +282,19 @@ 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 diff --git a/spec/models/integrations/jira_spec.rb b/spec/models/integrations/jira_spec.rb index 76220987f18d6aec1aaf966c3f9264f4452b84c5..6401cd3acbec41920819e9415e51bead9d6ea431 100644 --- a/spec/models/integrations/jira_spec.rb +++ b/spec/models/integrations/jira_spec.rb @@ -876,10 +876,24 @@ def close_issue 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)