Merge branch 'fix/gb/fix-pipeline-lazy-ref' into 'master'

Fix pipeline ref lazy commit retrieval with Gitaly

Closes #224523

See merge request gitlab-org/gitlab!35508

app/models/ci/pipeline.rb

@@ -566,7 +566,7 @@ module Ci
         next unless project.repository_exists?
 
         project.repository.list_commits_by_ref_name(refs).then do |commits|
-          loader.call(ref, commits[ref])
+          commits.each { |key, commit| loader.call(key, commits[key]) }
         end
       end
     end

lib/gitlab/gitaly_client/commit_service.rb

@@ -396,13 +396,13 @@ module Gitlab
 
       def list_commits_by_ref_name(refs)
         request = Gitaly::ListCommitsByRefNameRequest
-          .new(repository: @gitaly_repo, ref_names: refs)
+          .new(repository: @gitaly_repo, ref_names: refs.map { |ref| encode_binary(ref) })
 
         response = GitalyClient.call(@repository.storage, :commit_service, :list_commits_by_ref_name, request, timeout: GitalyClient.medium_timeout)
 
         commit_refs = response.flat_map do |message|
           message.commit_refs.map do |commit_ref|
-            [commit_ref.ref_name, Gitlab::Git::Commit.new(@repository, commit_ref.commit)]
+            [encode_utf8(commit_ref.ref_name), Gitlab::Git::Commit.new(@repository, commit_ref.commit)]
           end
         end
 

spec/factories/projects.rb

@@ -181,6 +181,7 @@ FactoryBot.define do
 
       transient do
         create_templates { nil }
+        create_branch { nil }
       end
 
       after :create do |project, evaluator|
@@ -206,6 +207,16 @@ FactoryBot.define do
             message: 'test 2',
             branch_name: 'master')
         end
+
+        if evaluator.create_branch
+          project.repository.create_file(
+            project.creator,
+            'README.md',
+            "README on branch #{evaluator.create_branch}",
+            message: 'Add README.md',
+            branch_name: evaluator.create_branch)
+
+        end
       end
     end
 

spec/lib/gitlab/gitaly_client/commit_service_spec.rb

@@ -387,12 +387,16 @@ RSpec.describe Gitlab::GitalyClient::CommitService do
   end
 
   describe '#list_commits_by_ref_name' do
+    let(:project) { create(:project, :repository, create_branch: 'ü/unicode/multi-byte') }
+
     it 'lists latest commits grouped by a ref name' do
-      response = client.list_commits_by_ref_name(%w[master feature v1.0.0 nonexistent])
+      response = client.list_commits_by_ref_name(%w[master feature v1.0.0 nonexistent ü/unicode/multi-byte])
 
+      expect(response.keys.count).to eq 4
       expect(response.fetch('master').id).to eq 'b83d6e391c22777fca1ed3012fce84f633d7fed0'
       expect(response.fetch('feature').id).to eq '0b4bc9a49b562e85de7cc9e834518ea6828729b9'
       expect(response.fetch('v1.0.0').id).to eq '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9'
+      expect(response.fetch('ü/unicode/multi-byte')).to be_present
       expect(response).not_to have_key 'nonexistent'
     end
   end

spec/models/ci/pipeline_spec.rb

@@ -1489,8 +1489,31 @@ RSpec.describe Ci::Pipeline, :mailer do
     end
 
     describe '#lazy_ref_commit' do
+      let(:another) do
+        create(:ci_pipeline,
+               project: project,
+               ref: 'feature',
+               sha: project.commit('feature').sha)
+      end
+
+      let(:unicode) do
+        create(:ci_pipeline,
+               project: project,
+               ref: 'ü/unicode/multi-byte')
+      end
+
       it 'returns the latest commit for a ref lazily' do
-        expect(pipeline.lazy_ref_commit.id).to eq project.commit(pipeline.ref).id
+        expect(project.repository)
+          .to receive(:list_commits_by_ref_name).once
+          .and_call_original
+
+        pipeline.lazy_ref_commit
+        another.lazy_ref_commit
+        unicode.lazy_ref_commit
+
+        expect(pipeline.lazy_ref_commit.id).to eq pipeline.sha
+        expect(another.lazy_ref_commit.id).to eq another.sha
+        expect(unicode.lazy_ref_commit).to be_nil
       end
     end