Commit 9b696278 authored by Imre Farkas's avatar Imre Farkas

Merge branch 'id-fix-n-1-for-commits' into 'master'

Preload number of pipeline warnings for commits

See merge request gitlab-org/gitlab!37669
parents 613e0c94 0bae8d6f
...@@ -47,7 +47,10 @@ class CommitCollection ...@@ -47,7 +47,10 @@ class CommitCollection
pipelines = project.ci_pipelines.latest_pipeline_per_commit(map(&:id), ref) pipelines = project.ci_pipelines.latest_pipeline_per_commit(map(&:id), ref)
each do |commit| each do |commit|
commit.set_latest_pipeline_for_ref(ref, pipelines[commit.id]) pipeline = pipelines[commit.id]
pipeline&.number_of_warnings # preload number of warnings
commit.set_latest_pipeline_for_ref(ref, pipeline)
end end
self self
......
---
title: Preload number of pipeline warnings for commits
merge_request: 37669
author:
type: performance
...@@ -52,27 +52,38 @@ RSpec.describe CommitCollection do ...@@ -52,27 +52,38 @@ RSpec.describe CommitCollection do
end end
describe '#with_latest_pipeline' do describe '#with_latest_pipeline' do
let(:another_commit) { project.commit("60ecb67744cb56576c30214ff52294f8ce2def98") }
let!(:pipeline) do let!(:pipeline) do
create( create(:ci_empty_pipeline, ref: 'master', sha: commit.id, status: 'success', project: project)
:ci_empty_pipeline, end
ref: 'master',
sha: commit.id, let!(:another_pipeline) do
status: 'success', create(:ci_empty_pipeline, ref: 'master', sha: another_commit.id, status: 'success', project: project)
project: project
)
end end
let(:collection) { described_class.new(project, [commit]) }
let(:collection) { described_class.new(project, [commit, another_commit]) }
it 'sets the latest pipeline for every commit so no additional queries are necessary' do it 'sets the latest pipeline for every commit so no additional queries are necessary' do
commits = collection.with_latest_pipeline('master') commits = collection.with_latest_pipeline('master')
recorder = ActiveRecord::QueryRecorder.new do recorder = ActiveRecord::QueryRecorder.new do
expect(commits.map { |c| c.latest_pipeline('master') }) expect(commits.map { |c| c.latest_pipeline('master') })
.to eq([pipeline]) .to eq([pipeline, another_pipeline])
end end
expect(recorder.count).to be_zero expect(recorder.count).to be_zero
end end
it 'performs a single query to fetch pipeline warnings' do
recorder = ActiveRecord::QueryRecorder.new do
collection.with_latest_pipeline('master').each do |c|
c.latest_pipeline('master').number_of_warnings.itself
end
end
expect(recorder.count).to eq(2) # 1 for pipelines, 1 for warnings counts
end
end end
describe '#with_markdown_cache' do describe '#with_markdown_cache' do
......
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