Commit 470c675a authored by Grzegorz Bizon's avatar Grzegorz Bizon

Fix pipeline ref lazy commit retrieval with Gitaly

This commits fixes pipeline lazy ref retrieval with Gitaly, by encoding
ref names and using BatchLoader loading properly.
parent a2f63f4e
...@@ -572,7 +572,7 @@ module Ci ...@@ -572,7 +572,7 @@ module Ci
next unless project.repository_exists? next unless project.repository_exists?
project.repository.list_commits_by_ref_name(refs).then do |commits| 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 end
end end
......
...@@ -393,13 +393,13 @@ module Gitlab ...@@ -393,13 +393,13 @@ module Gitlab
def list_commits_by_ref_name(refs) def list_commits_by_ref_name(refs)
request = Gitaly::ListCommitsByRefNameRequest 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) response = GitalyClient.call(@repository.storage, :commit_service, :list_commits_by_ref_name, request, timeout: GitalyClient.medium_timeout)
commit_refs = response.flat_map do |message| commit_refs = response.flat_map do |message|
message.commit_refs.map do |commit_ref| 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
end end
......
...@@ -181,6 +181,7 @@ FactoryBot.define do ...@@ -181,6 +181,7 @@ FactoryBot.define do
transient do transient do
create_templates { nil } create_templates { nil }
create_branch { nil }
end end
after :create do |project, evaluator| after :create do |project, evaluator|
...@@ -206,6 +207,16 @@ FactoryBot.define do ...@@ -206,6 +207,16 @@ FactoryBot.define do
message: 'test 2', message: 'test 2',
branch_name: 'master') branch_name: 'master')
end 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
end end
......
...@@ -383,12 +383,16 @@ RSpec.describe Gitlab::GitalyClient::CommitService do ...@@ -383,12 +383,16 @@ RSpec.describe Gitlab::GitalyClient::CommitService do
end end
describe '#list_commits_by_ref_name' do 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 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('master').id).to eq 'b83d6e391c22777fca1ed3012fce84f633d7fed0'
expect(response.fetch('feature').id).to eq '0b4bc9a49b562e85de7cc9e834518ea6828729b9' expect(response.fetch('feature').id).to eq '0b4bc9a49b562e85de7cc9e834518ea6828729b9'
expect(response.fetch('v1.0.0').id).to eq '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9' 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' expect(response).not_to have_key 'nonexistent'
end end
end end
......
...@@ -1489,8 +1489,31 @@ RSpec.describe Ci::Pipeline, :mailer do ...@@ -1489,8 +1489,31 @@ RSpec.describe Ci::Pipeline, :mailer do
end end
describe '#lazy_ref_commit' do 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 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
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment