Commit f0d7445b authored by Grzegorz Bizon's avatar Grzegorz Bizon

Reduce pipeline serialization queries when preloaded

parent bc9a0e10
...@@ -3,8 +3,11 @@ class PipelineSerializer < BaseSerializer ...@@ -3,8 +3,11 @@ class PipelineSerializer < BaseSerializer
entity PipelineDetailsEntity entity PipelineDetailsEntity
def represent(resource, opts = {}) def represent(resource, opts = {})
if resource.is_a?(ActiveRecord::Relation) if paginated? && !resource.respond_to?(:page)
raise Gitlab::Serializer::Pagination::InvalidResourceError
end
if resource.is_a?(ActiveRecord::Relation)
resource = resource.preload([ resource = resource.preload([
:stages, :stages,
:retryable_builds, :retryable_builds,
...@@ -18,7 +21,6 @@ class PipelineSerializer < BaseSerializer ...@@ -18,7 +21,6 @@ class PipelineSerializer < BaseSerializer
end end
if opts.delete(:preload) if opts.delete(:preload)
resource = @paginator.paginate(resource) if paginated?
resource = Gitlab::Ci::Pipeline::Preloader.preload!(resource) resource = Gitlab::Ci::Pipeline::Preloader.preload!(resource)
end end
...@@ -35,7 +37,7 @@ class PipelineSerializer < BaseSerializer ...@@ -35,7 +37,7 @@ class PipelineSerializer < BaseSerializer
def represent_stages(resource) def represent_stages(resource)
return {} unless resource.present? return {} unless resource.present?
data = represent(resource, { only: [{ details: [:stages] }] }) data = represent(resource, { only: [{ details: [:stages] }], preload: true })
data.dig(:details, :stages) || [] data.dig(:details, :stages) || []
end end
end end
...@@ -99,7 +99,8 @@ describe PipelineSerializer do ...@@ -99,7 +99,8 @@ describe PipelineSerializer do
end end
end end
context 'number of queries' do describe 'number of queries when preloaded' do
subject { serializer.represent(resource, preload: true) }
let(:resource) { Ci::Pipeline.all } let(:resource) { Ci::Pipeline.all }
before do before do
...@@ -120,7 +121,7 @@ describe PipelineSerializer do ...@@ -120,7 +121,7 @@ describe PipelineSerializer do
it 'verifies number of queries', :request_store do it 'verifies number of queries', :request_store do
recorded = ActiveRecord::QueryRecorder.new { subject } recorded = ActiveRecord::QueryRecorder.new { subject }
expect(recorded.count).to be_within(1).of(38) expect(recorded.count).to be_within(1).of(31)
expect(recorded.cached_count).to eq(0) expect(recorded.cached_count).to eq(0)
end end
end end
...@@ -139,7 +140,7 @@ describe PipelineSerializer do ...@@ -139,7 +140,7 @@ describe PipelineSerializer do
# pipeline. With the same ref this check is cached but if refs are # pipeline. With the same ref this check is cached but if refs are
# different then there is an extra query per ref # different then there is an extra query per ref
# https://gitlab.com/gitlab-org/gitlab-ce/issues/46368 # https://gitlab.com/gitlab-org/gitlab-ce/issues/46368
expect(recorded.count).to be_within(1).of(45) expect(recorded.count).to be_within(1).of(38)
expect(recorded.cached_count).to eq(0) expect(recorded.cached_count).to eq(0)
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