Merge branch 'eb-preload-artifacts-merge-request-widget' into 'master'

Preload job artifacts archive in pipeline details

See merge request gitlab-org/gitlab!25250

app/models/ci/build.rb

@@ -114,6 +114,7 @@ module Ci
     end
 
     scope :eager_load_job_artifacts, -> { includes(:job_artifacts) }
+    scope :eager_load_job_artifacts_archive, -> { includes(:job_artifacts_archive) }
 
     scope :eager_load_everything, -> do
       includes(

app/serializers/pipeline_details_entity.rb

@@ -8,7 +8,12 @@ class PipelineDetailsEntity < PipelineEntity
   end
 
   expose :details do
-    expose :artifacts, using: BuildArtifactEntity
+    expose :artifacts do |pipeline, options|
+      rel = pipeline.artifacts
+      rel = rel.eager_load_job_artifacts_archive if options.fetch(:preload_job_artifacts_archive, true)
+
+      BuildArtifactEntity.represent(rel, options)
+    end
     expose :manual_actions, using: BuildActionEntity
     expose :scheduled_actions, using: BuildActionEntity
   end

app/serializers/pipeline_serializer.rb

@@ -7,6 +7,10 @@ class PipelineSerializer < BaseSerializer
   # rubocop: disable CodeReuse/ActiveRecord
   def represent(resource, opts = {})
     if resource.is_a?(ActiveRecord::Relation)
+      # We don't want PipelineDetailsEntity to preload the job_artifacts_archive
+      # because we do it with preloaded_relations in a more optimal way
+      # if the given resource is a collection of multiple pipelines.
+      opts[:preload_job_artifacts_archive] = false
       resource = resource.preload(preloaded_relations)
     end
 

changelogs/unreleased/eb-preload-artifacts-merge-request-widget.yml

+---
+title: Fix N+1 queries caused by loading job artifacts archive in pipeline details
+  entity
+merge_request: 25250
+author:
+type: fixed

spec/controllers/projects/pipelines_controller_spec.rb

@@ -171,7 +171,17 @@ describe Projects::PipelinesController do
 
     def create_build(pipeline, stage, stage_idx, name, user = nil)
       status = %w[created running pending success failed canceled].sample
-      create(:ci_build, pipeline: pipeline, stage: stage, stage_idx: stage_idx, name: name, status: status, user: user)
+      create(
+        :ci_build,
+        :artifacts,
+        artifacts_expire_at: 2.days.from_now,
+        pipeline: pipeline,
+        stage: stage,
+        stage_idx: stage_idx,
+        name: name,
+        status: status,
+        user: user
+      )
     end
   end
 

spec/serializers/pipeline_details_entity_spec.rb

@@ -173,5 +173,44 @@ describe PipelineDetailsEntity do
         expect(subject[:triggered].first[:project]).not_to be_nil
       end
     end
+
+    context 'when pipeline has expiring archive artifacts' do
+      let(:pipeline) { create(:ci_empty_pipeline) }
+      let!(:build_1) { create(:ci_build, :artifacts, pipeline: pipeline, artifacts_expire_at: 2.days.from_now, name: 'build_1') }
+      let!(:build_2) { create(:ci_build, :artifacts, pipeline: pipeline, artifacts_expire_at: 2.days.from_now, name: 'build_2') }
+      let!(:build_3) { create(:ci_build, :artifacts, pipeline: pipeline, artifacts_expire_at: 2.days.from_now, name: 'build_3') }
+
+      let(:names) { subject[:details][:artifacts].map { |a| a[:name] } }
+
+      context 'and preload_job_artifacts_archive is not defined in the options' do
+        it 'defaults to true and eager loads the job_artifacts_archive' do
+          recorder = ActiveRecord::QueryRecorder.new do
+            expect(names).to match_array(%w[build_1 build_2 build_3])
+          end
+
+          expected_queries = Gitlab.ee? ? 42 : 29
+
+          # This makes only one query to fetch all job artifacts
+          expect(recorder.count).to eq(expected_queries)
+        end
+      end
+
+      context 'and preload_job_artifacts_archive is set to false' do
+        let(:entity) do
+          described_class.represent(pipeline, request: request, preload_job_artifacts_archive: false)
+        end
+
+        it 'does not eager load the job_artifacts_archive' do
+          recorder = ActiveRecord::QueryRecorder.new do
+            expect(names).to match_array(%w[build_1 build_2 build_3])
+          end
+
+          expected_queries = Gitlab.ee? ? 44 : 31
+
+          # This makes one query for each job artifact
+          expect(recorder.count).to eq(expected_queries)
+        end
+      end
+    end
   end
 end