diff --git a/app/models/preloaders/commit_status_preloader.rb b/app/models/preloaders/commit_status_preloader.rb index 79c2549e37178d517961e4226c68f4c71b301bb7..9e25ee510b5a0491327b742797b18276ad1d4117 100644 --- a/app/models/preloaders/commit_status_preloader.rb +++ b/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 diff --git a/app/serializers/stage_entity.rb b/app/serializers/stage_entity.rb index aa570988010db4cacc6654b13802f88ae18cb715..49f7a04155c6fcee6094c5402ca975413af25b1e 100644 --- a/app/serializers/stage_entity.rb +++ b/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 diff --git a/config/feature_flags/gitlab_com_derisk/preload_ci_bridge_downstream_pipelines.yml b/config/feature_flags/gitlab_com_derisk/preload_ci_bridge_downstream_pipelines.yml new file mode 100644 index 0000000000000000000000000000000000000000..846e49291db9de0ddee9adc0507b37502937b800 --- /dev/null +++ b/config/feature_flags/gitlab_com_derisk/preload_ci_bridge_downstream_pipelines.yml @@ -0,0 +1,8 @@ +--- +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 diff --git a/spec/models/preloaders/commit_status_preloader_spec.rb b/spec/models/preloaders/commit_status_preloader_spec.rb index 0453b6267eda3bad50d0acd02d7855b59b2cb5a5..a028652577dde35ef8971ca321690ab781379b1b 100644 --- a/spec/models/preloaders/commit_status_preloader_spec.rb +++ b/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) diff --git a/spec/serializers/stage_entity_spec.rb b/spec/serializers/stage_entity_spec.rb index 81cb89d0c386c9983bf0b73a1d2ea1f086d8ae74..b3f08f2aee36179da9a43a655bcd8e67d97f4ade 100644 --- a/spec/serializers/stage_entity_spec.rb +++ b/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