Merge branch '323213-n-plus-one-query-on-project-pipeline-graphql' into 'master'

Eliminate some N+1 queries on project-pipeline GraphQL endpoint

See merge request gitlab-org/gitlab!64700

app/graphql/types/ci/job_type.rb

@@ -56,7 +56,7 @@ module Types
       field :short_sha, type: GraphQL::STRING_TYPE, null: false,
             description: 'Short SHA1 ID of the commit.'
       field :scheduling_type, GraphQL::STRING_TYPE, null: true,
-            description: 'Type of pipeline scheduling. Value is `dag` if the pipeline uses the `needs` keyword, and `stage` otherwise.'
+            description: 'Type of job scheduling. Value is `dag` if the job uses the `needs` keyword, and `stage` otherwise.'
       field :commit_path, GraphQL::STRING_TYPE, null: true,
             description: 'Path to the commit that triggered the job.'
       field :ref_name, GraphQL::STRING_TYPE, null: true,

app/graphql/types/ci/stage_type.rb

@@ -57,6 +57,7 @@ module Types
       # rubocop: disable CodeReuse/ActiveRecord
       def jobs_for_pipeline(pipeline, stage_ids, include_needs)
         results = pipeline.latest_statuses.where(stage_id: stage_ids)
+        results = results.preload(:project)
         results = results.preload(:needs) if include_needs
 
         results.group_by(&:stage_id)

doc/api/graphql/reference/index.md

@@ -7589,7 +7589,7 @@ Represents the total number of issues and their weights for a particular day.
 | <a id="cijobrefpath"></a>`refPath` | [`String`](#string) | Path to the ref. |
 | <a id="cijobretryable"></a>`retryable` | [`Boolean!`](#boolean) | Indicates the job can be retried. |
 | <a id="cijobscheduledat"></a>`scheduledAt` | [`Time`](#time) | Schedule for the build. |
-| <a id="cijobschedulingtype"></a>`schedulingType` | [`String`](#string) | Type of pipeline scheduling. Value is `dag` if the pipeline uses the `needs` keyword, and `stage` otherwise. |
+| <a id="cijobschedulingtype"></a>`schedulingType` | [`String`](#string) | Type of job scheduling. Value is `dag` if the job uses the `needs` keyword, and `stage` otherwise. |
 | <a id="cijobshortsha"></a>`shortSha` | [`String!`](#string) | Short SHA1 ID of the commit. |
 | <a id="cijobstage"></a>`stage` | [`CiStage`](#cistage) | Stage of the job. |
 | <a id="cijobstartedat"></a>`startedAt` | [`Time`](#time) | When the job was started. |

spec/requests/api/graphql/project/pipeline_spec.rb

@@ -8,9 +8,9 @@ RSpec.describe 'getting pipeline information nested in a project' do
   let_it_be(:project) { create(:project, :repository, :public) }
   let_it_be(:pipeline) { create(:ci_pipeline, project: project) }
   let_it_be(:current_user) { create(:user) }
-  let_it_be(:build_job) { create(:ci_build, :trace_with_sections, name: 'build-a', pipeline: pipeline) }
-  let_it_be(:failed_build) { create(:ci_build, :failed, name: 'failed-build', pipeline: pipeline) }
-  let_it_be(:bridge) { create(:ci_bridge, name: 'ci-bridge-example', pipeline: pipeline) }
+  let_it_be(:build_job) { create(:ci_build, :trace_with_sections, name: 'build-a', pipeline: pipeline, stage_idx: 0, stage: 'build') }
+  let_it_be(:failed_build) { create(:ci_build, :failed, name: 'failed-build', pipeline: pipeline, stage_idx: 0, stage: 'build') }
+  let_it_be(:bridge) { create(:ci_bridge, name: 'ci-bridge-example', pipeline: pipeline, stage_idx: 0, stage: 'build') }
 
   let(:path) { %i[project pipeline] }
   let(:pipeline_graphql_data) { graphql_data_at(*path) }
@@ -79,16 +79,6 @@ RSpec.describe 'getting pipeline information nested in a project' do
     end
   end
 
-  private
-
-  def build_query_to_find_pipeline_shas(*pipelines)
-    pipeline_fields = pipelines.map.each_with_index do |pipeline, idx|
-      "pipeline#{idx}: pipeline(iid: \"#{pipeline.iid}\") { sha }"
-    end.join(' ')
-
-    graphql_query_for('project', { 'fullPath' => project.full_path }, pipeline_fields)
-  end
-
   context 'when enough data is requested' do
     let(:fields) do
       query_graphql_field(:jobs, nil,
@@ -282,4 +272,69 @@ RSpec.describe 'getting pipeline information nested in a project' do
       end
     end
   end
+
+  context 'N+1 queries on stages jobs' do
+    let(:depth) { 5 }
+    let(:fields) do
+      <<~FIELDS
+      stages {
+        nodes {
+          name
+          groups {
+            nodes {
+              name
+              jobs {
+                nodes {
+                  name
+                  needs {
+                    nodes {
+                      name
+                    }
+                  }
+                  status: detailedStatus {
+                    tooltip
+                    hasDetails
+                    detailsPath
+                    action {
+                      buttonTitle
+                      path
+                      title
+                    }
+                  }
+                }
+              }
+            }
+          }
+        }
+      }
+      FIELDS
+    end
+
+    it 'does not generate N+1 queries', :request_store, :use_sql_query_cache do
+      # warm up
+      post_graphql(query, current_user: current_user)
+
+      control = ActiveRecord::QueryRecorder.new(skip_cached: false) do
+        post_graphql(query, current_user: current_user)
+      end
+
+      create(:ci_build, name: 'test-a', pipeline: pipeline, stage_idx: 1, stage: 'test')
+      create(:ci_build, name: 'test-b', pipeline: pipeline, stage_idx: 1, stage: 'test')
+      create(:ci_build, name: 'deploy-a', pipeline: pipeline, stage_idx: 2, stage: 'deploy')
+
+      expect do
+        post_graphql(query, current_user: current_user)
+      end.not_to exceed_all_query_limit(control)
+    end
+  end
+
+  private
+
+  def build_query_to_find_pipeline_shas(*pipelines)
+    pipeline_fields = pipelines.map.each_with_index do |pipeline, idx|
+      "pipeline#{idx}: pipeline(iid: \"#{pipeline.iid}\") { sha }"
+    end.join(' ')
+
+    graphql_query_for('project', { 'fullPath' => project.full_path }, pipeline_fields)
+  end
 end