Merge branch 'preload-downstream-pipeline-for-commit-statuses' into 'master'

Preload downstream_pipeline in CommitStatusPreloader

See merge request https://gitlab.com/gitlab-org/gitlab/-/merge_requests/142199



Merged-by: Allison Browne <abrowne@gitlab.com>
Approved-by: Allison Browne <abrowne@gitlab.com>
Reviewed-by: Allison Browne <abrowne@gitlab.com>
Reviewed-by: Max Fan <mfan@gitlab.com>
Co-authored-by: Hordur Freyr Yngvason <hfyngvason@gitlab.com>

app/models/preloaders/commit_status_preloader.rb

@@ -24,7 +24,24 @@ def objects(klass)
     end
 
     def associations(klass, relations)
-      klass.reflections.keys.map(&:to_sym) & relations.map(&:to_sym)
+      klass_reflections = klass.reflections.keys.map(&:to_sym).to_set
+
+      result = []
+      relations.each do |entry|
+        if entry.respond_to?(:to_sym)
+          result << entry.to_sym if klass_reflections.include?(entry.to_sym)
+        elsif entry.is_a?(Hash)
+          entry = entry.select do |key, _value|
+            klass_reflections.include?(key.to_sym)
+          end
+
+          result << entry if entry.present?
+        else
+          raise ArgumentError, "Invalid relation: #{entry.inspect}"
+        end
+      end
+
+      result
     end
   end
 end

app/serializers/stage_entity.rb

@@ -10,14 +10,14 @@ class StageEntity < Grape::Entity
   end
 
   expose :groups,
-    if: -> (_, opts) { opts[:grouped] },
+    if: ->(_, opts) { opts[:grouped] },
     with: JobGroupEntity
 
-  expose :latest_statuses, if: -> (_, opts) { opts[:details] }, with: Ci::JobEntity do |stage|
+  expose :latest_statuses, if: ->(_, opts) { opts[:details] }, with: Ci::JobEntity do |_stage|
     latest_statuses
   end
 
-  expose :retried, if: -> (_, opts) { opts[:retried] }, with: Ci::JobEntity do |stage|
+  expose :retried, if: ->(_, opts) { opts[:retried] }, with: Ci::JobEntity do |_stage|
     retried_statuses
   end
 
@@ -67,7 +67,12 @@ def grouped_retried_statuses
   end
 
   def preload_metadata(statuses)
-    Preloaders::CommitStatusPreloader.new(statuses).execute([:metadata, :pipeline])
+    relations = [:metadata, :pipeline]
+    if Feature.enabled?(:preload_ci_bridge_downstream_pipelines, stage.pipeline.project, type: :gitlab_com_derisk)
+      relations << { downstream_pipeline: [:user, { project: [:route, { namespace: :route }] }] }
+    end
+
+    Preloaders::CommitStatusPreloader.new(statuses).execute(relations)
 
     statuses
   end

config/feature_flags/gitlab_com_derisk/preload_ci_bridge_downstream_pipelines.yml

+---
+name: preload_ci_bridge_downstream_pipelines
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/142199
+rollout_issue_url: https://gitlab.com/gitlab-com/gl-infra/production/-/issues/17473
+milestone: '16.9'
+group: 'group::pipeline execution'
+type: gitlab_com_derisk
+default_enabled: false

spec/models/preloaders/commit_status_preloader_spec.rb

@@ -13,7 +13,7 @@
   let_it_be(:generic_commit_status2) { create(:generic_commit_status, pipeline: pipeline) }
 
   describe '#execute' do
-    let(:relations) { %i[pipeline metadata tags job_artifacts_archive downstream_pipeline] }
+    let(:relations) { %i[pipeline metadata tags job_artifacts_archive { downstream_pipeline: [:user] }] }
     let(:statuses) { CommitStatus.where(commit_id: pipeline.id).all }
 
     subject(:execute) { described_class.new(statuses).execute(relations) }
@@ -30,6 +30,12 @@
       end.to issue_same_number_of_queries_as(control)
     end
 
+    context 'when given an invalid relation' do
+      let(:relations) { [1] }
+
+      it { expect { execute }.to raise_error(ArgumentError, "Invalid relation: 1") }
+    end
+
     private
 
     def call_each_relation(statuses)

spec/serializers/stage_entity_spec.rb

@@ -96,5 +96,35 @@
         expect(subject[:status][:action]).to be_present
       end
     end
+
+    context 'when details: true' do
+      def serialize(stage)
+        described_class.new(stage, request: request, details: true).as_json
+      end
+
+      it 'avoids N+1 queries on latest_statuses', :use_sql_query_cache, :request_store do
+        pipeline = create(:ci_pipeline)
+        stage = create(:ci_stage, pipeline: pipeline, status: :success)
+
+        serialize(stage) # Warm up O(1) queries
+
+        # Prepare control
+        create(:ci_build, :tags, ci_stage: stage, pipeline: pipeline)
+        create(:ci_bridge, ci_stage: stage, pipeline: pipeline)
+        create(:generic_commit_status, ci_stage: stage, pipeline: pipeline)
+
+        control = ActiveRecord::QueryRecorder.new(skip_cached: false) do
+          serialize(stage)
+        end
+
+        # Prepare sample using a generous number to counteract any caches from
+        # the control
+        create_list(:ci_build, 10, :tags, ci_stage: stage, pipeline: pipeline)
+        create_list(:ci_bridge, 10, ci_stage: stage, pipeline: pipeline)
+        create_list(:generic_commit_status, 10, ci_stage: stage, pipeline: pipeline)
+
+        expect { serialize(stage) }.not_to exceed_query_limit(control)
+      end
+    end
   end
 end