Merge branch 'ajk-ci-job-needs-type' into 'master'

Change type of CiJob.needs [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!50192

app/graphql/types/ci/build_need_type.rb

+# frozen_string_literal: true
+
+module Types
+  module Ci
+    # rubocop: disable Graphql/AuthorizeTypes
+    # This type is only accessible from CiJob
+    class BuildNeedType < BaseObject
+      graphql_name 'CiBuildNeed'
+
+      field :name, GraphQL::STRING_TYPE, null: true,
+            description: 'Name of the job we need to complete.'
+    end
+  end
+end

app/graphql/types/ci/job_type.rb

@@ -10,8 +10,8 @@ module Types
             description: 'Pipeline the job belongs to'
       field :name, GraphQL::STRING_TYPE, null: true,
             description: 'Name of the job'
-      field :needs, JobType.connection_type, null: true,
-            description: 'Builds that must complete before the jobs run'
+      field :needs, BuildNeedType.connection_type, null: true,
+            description: 'References to builds that must complete before the jobs run'
       field :detailed_status, Types::Ci::DetailedStatusType, null: true,
             description: 'Detailed status of the job'
       field :scheduled_at, Types::TimeType, null: true,

app/graphql/types/ci/stage_type.rb

@@ -9,6 +9,7 @@ module Types
       field :name, GraphQL::STRING_TYPE, null: true,
         description: 'Name of the stage'
       field :groups, Ci::GroupType.connection_type, null: true,
+        extras: [:lookahead],
         description: 'Group of jobs for the stage'
       field :detailed_status, Types::Ci::DetailedStatusType, null: true,
             description: 'Detailed status of the stage'
@@ -18,24 +19,36 @@ module Types
       end
 
       # Issues one query per pipeline
-      def groups
-        BatchLoader::GraphQL.for([object.pipeline, object]).batch(default_value: []) do |keys, loader|
-          by_pipeline = keys.group_by(&:first)
+      def groups(lookahead:)
+        key = ::Gitlab::Graphql::BatchKey.new(object, lookahead, object_name: :stage)
+
+        BatchLoader::GraphQL.for(key).batch(default_value: []) do |keys, loader|
+          by_pipeline = keys.group_by(&:pipeline)
+          include_needs = keys.any? { |k| k.requires?(%i[nodes jobs nodes needs]) }
 
           by_pipeline.each do |pl, key_group|
             project = pl.project
-            stages = key_group.map(&:second).uniq
-            indexed = stages.index_by(&:id)
-            results = pl.latest_statuses.where(stage_id: stages.map(&:id)) # rubocop: disable CodeReuse/ActiveRecord
+            indexed = key_group.index_by(&:id)
 
-            results.group_by(&:stage_id).each do |stage_id, statuses|
-              stage = indexed[stage_id]
-              groups = ::Ci::Group.fabricate(project, stage, statuses)
-              loader.call([pl, stage], groups)
+            jobs_for_pipeline(pl, indexed.keys, include_needs).each do |stage_id, statuses|
+              key = indexed[stage_id]
+              groups = ::Ci::Group.fabricate(project, key.stage, statuses)
+              loader.call(key, groups)
+            end
           end
         end
       end
+
+      private
+
+      # 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(:needs) if include_needs
+
+        results.group_by(&:stage_id)
       end
+      # rubocop: enable CodeReuse/ActiveRecord
     end
   end
 end

changelogs/unreleased/ajk-ci-job-needs-type.yml

+---
+title: Change type of CiJob.needs
+merge_request: 50192
+author:
+type: fixed

doc/api/graphql/reference/gitlab_schema.graphql

@@ -2288,6 +2288,48 @@ type BurnupChartDailyTotals {
   scopeWeight: Int!
 }
 
+type CiBuildNeed {
+  """
+  Name of the job we need to complete.
+  """
+  name: String
+}
+
+"""
+The connection type for CiBuildNeed.
+"""
+type CiBuildNeedConnection {
+  """
+  A list of edges.
+  """
+  edges: [CiBuildNeedEdge]
+
+  """
+  A list of nodes.
+  """
+  nodes: [CiBuildNeed]
+
+  """
+  Information to aid in pagination.
+  """
+  pageInfo: PageInfo!
+}
+
+"""
+An edge in a connection.
+"""
+type CiBuildNeedEdge {
+  """
+  A cursor for use in pagination.
+  """
+  cursor: String!
+
+  """
+  The item at the end of the edge.
+  """
+  node: CiBuildNeed
+}
+
 type CiConfig {
   """
   Linting errors
@@ -2717,7 +2759,7 @@ type CiJob {
   name: String
 
   """
-  Builds that must complete before the jobs run
+  References to builds that must complete before the jobs run
   """
   needs(
     """
@@ -2739,7 +2781,7 @@ type CiJob {
     Returns the last _n_ elements from the list.
     """
     last: Int
-  ): CiJobConnection
+  ): CiBuildNeedConnection
 
   """
   Pipeline the job belongs to

doc/api/graphql/reference/gitlab_schema.json

@@ -6101,6 +6101,145 @@
           "enumValues": null,
           "possibleTypes": null
         },
+        {
+          "kind": "OBJECT",
+          "name": "CiBuildNeed",
+          "description": null,
+          "fields": [
+            {
+              "name": "name",
+              "description": "Name of the job we need to complete.",
+              "args": [
+
+              ],
+              "type": {
+                "kind": "SCALAR",
+                "name": "String",
+                "ofType": null
+              },
+              "isDeprecated": false,
+              "deprecationReason": null
+            }
+          ],
+          "inputFields": null,
+          "interfaces": [
+
+          ],
+          "enumValues": null,
+          "possibleTypes": null
+        },
+        {
+          "kind": "OBJECT",
+          "name": "CiBuildNeedConnection",
+          "description": "The connection type for CiBuildNeed.",
+          "fields": [
+            {
+              "name": "edges",
+              "description": "A list of edges.",
+              "args": [
+
+              ],
+              "type": {
+                "kind": "LIST",
+                "name": null,
+                "ofType": {
+                  "kind": "OBJECT",
+                  "name": "CiBuildNeedEdge",
+                  "ofType": null
+                }
+              },
+              "isDeprecated": false,
+              "deprecationReason": null
+            },
+            {
+              "name": "nodes",
+              "description": "A list of nodes.",
+              "args": [
+
+              ],
+              "type": {
+                "kind": "LIST",
+                "name": null,
+                "ofType": {
+                  "kind": "OBJECT",
+                  "name": "CiBuildNeed",
+                  "ofType": null
+                }
+              },
+              "isDeprecated": false,
+              "deprecationReason": null
+            },
+            {
+              "name": "pageInfo",
+              "description": "Information to aid in pagination.",
+              "args": [
+
+              ],
+              "type": {
+                "kind": "NON_NULL",
+                "name": null,
+                "ofType": {
+                  "kind": "OBJECT",
+                  "name": "PageInfo",
+                  "ofType": null
+                }
+              },
+              "isDeprecated": false,
+              "deprecationReason": null
+            }
+          ],
+          "inputFields": null,
+          "interfaces": [
+
+          ],
+          "enumValues": null,
+          "possibleTypes": null
+        },
+        {
+          "kind": "OBJECT",
+          "name": "CiBuildNeedEdge",
+          "description": "An edge in a connection.",
+          "fields": [
+            {
+              "name": "cursor",
+              "description": "A cursor for use in pagination.",
+              "args": [
+
+              ],
+              "type": {
+                "kind": "NON_NULL",
+                "name": null,
+                "ofType": {
+                  "kind": "SCALAR",
+                  "name": "String",
+                  "ofType": null
+                }
+              },
+              "isDeprecated": false,
+              "deprecationReason": null
+            },
+            {
+              "name": "node",
+              "description": "The item at the end of the edge.",
+              "args": [
+
+              ],
+              "type": {
+                "kind": "OBJECT",
+                "name": "CiBuildNeed",
+                "ofType": null
+              },
+              "isDeprecated": false,
+              "deprecationReason": null
+            }
+          ],
+          "inputFields": null,
+          "interfaces": [
+
+          ],
+          "enumValues": null,
+          "possibleTypes": null
+        },
         {
           "kind": "OBJECT",
           "name": "CiConfig",
@@ -7305,7 +7444,7 @@
             },
             {
               "name": "needs",
-              "description": "Builds that must complete before the jobs run",
+              "description": "References to builds that must complete before the jobs run",
               "args": [
                 {
                   "name": "after",
@@ -7350,7 +7489,7 @@
               ],
               "type": {
                 "kind": "OBJECT",
-                "name": "CiJobConnection",
+                "name": "CiBuildNeedConnection",
                 "ofType": null
               },
               "isDeprecated": false,

doc/api/graphql/reference/index.md

@@ -375,6 +375,12 @@ Represents the total number of issues and their weights for a particular day.
 | `scopeCount` | Int! | Number of issues as of this day |
 | `scopeWeight` | Int! | Total weight of issues as of this day |
 
+### CiBuildNeed
+
+| Field | Type | Description |
+| ----- | ---- | ----------- |
+| `name` | String | Name of the job we need to complete. |
+
 ### CiConfig
 
 | Field | Type | Description |
@@ -430,7 +436,7 @@ Represents the total number of issues and their weights for a particular day.
 | `artifacts` | CiJobArtifactConnection | Artifacts generated by the job |
 | `detailedStatus` | DetailedStatus | Detailed status of the job |
 | `name` | String | Name of the job |
-| `needs` | CiJobConnection | Builds that must complete before the jobs run |
+| `needs` | CiBuildNeedConnection | References to builds that must complete before the jobs run |
 | `pipeline` | Pipeline | Pipeline the job belongs to |
 | `scheduledAt` | Time | Schedule for the build |
 

lib/gitlab/graphql/batch_key.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Graphql
+    class BatchKey
+      attr_reader :object
+      delegate :hash, to: :object
+
+      def initialize(object, lookahead = nil, object_name: nil)
+        @object = object
+        @lookahead = lookahead
+        @object_name = object_name
+      end
+
+      def requires?(path)
+        return false unless @lookahead
+        return false unless path.present?
+
+        field = path.pop
+
+        path
+          .reduce(@lookahead) { |q, f| q.selection(f) }
+          .selects?(field)
+      end
+
+      def eql?(other)
+        other.is_a?(self.class) && object == other.object
+      end
+      alias_method :==, :eql?
+
+      def method_missing(method_name, *args, **kwargs)
+        return @object if method_name.to_sym == @object_name
+        return @object.public_send(method_name) if args.empty? && kwargs.empty? # rubocop: disable GitlabSecurity/PublicSend
+
+        super
+      end
+    end
+  end
+end

spec/lib/gitlab/graphql/batch_key_spec.rb

+# frozen_string_literal: true
+
+require 'fast_spec_helper'
+require 'test_prof/recipes/rspec/let_it_be'
+
+RSpec.describe ::Gitlab::Graphql::BatchKey do
+  let_it_be(:rect) { Struct.new(:len, :width) }
+  let_it_be(:circle) { Struct.new(:radius) }
+  let(:lookahead) { nil }
+  let(:object) { rect.new(2, 3) }
+
+  subject { described_class.new(object, lookahead, object_name: :rect) }
+
+  it 'is equal to keys of the same object, regardless of lookahead or object name' do
+    expect(subject).to eq(described_class.new(rect.new(2, 3)))
+    expect(subject).to eq(described_class.new(rect.new(2, 3), :anything))
+    expect(subject).to eq(described_class.new(rect.new(2, 3), lookahead, object_name: :does_not_matter))
+    expect(subject).not_to eq(described_class.new(rect.new(2, 4)))
+    expect(subject).not_to eq(described_class.new(circle.new(10)))
+  end
+
+  it 'delegates attribute lookup methods to the inner object' do
+    other = rect.new(2, 3)
+
+    expect(subject.hash).to eq(other.hash)
+    expect(subject.len).to eq(other.len)
+    expect(subject.width).to eq(other.width)
+  end
+
+  it 'allows the object to be named more meaningfully' do
+    expect(subject.object).to eq(object)
+    expect(subject.object).to eq(subject.rect)
+  end
+
+  it 'works as a hash key' do
+    h = { subject => :foo }
+
+    expect(h[described_class.new(object)]).to eq(:foo)
+  end
+
+  describe '#requires?' do
+    it 'returns false if the lookahead was not provided' do
+      expect(subject.requires?([:foo])).to be(false)
+    end
+
+    context 'lookahead was provided' do
+      let(:lookahead) { double(:Lookahead) }
+
+      before do
+        allow(lookahead).to receive(:selection).with(Symbol).and_return(lookahead)
+      end
+
+      it 'returns false if the path is empty' do
+        expect(subject.requires?([])).to be(false)
+      end
+
+      context 'it selects the field' do
+        before do
+          allow(lookahead).to receive(:selects?).with(Symbol).once.and_return(true)
+        end
+
+        it 'returns true' do
+          expect(subject.requires?(%i[foo bar baz])).to be(true)
+        end
+      end
+
+      context 'it does not select the field' do
+        before do
+          allow(lookahead).to receive(:selects?).with(Symbol).once.and_return(false)
+        end
+
+        it 'returns false' do
+          expect(subject.requires?(%i[foo bar baz])).to be(false)
+        end
+      end
+    end
+  end
+end

spec/requests/api/graphql/ci/jobs_spec.rb

@@ -15,7 +15,7 @@ RSpec.describe 'Query.project.pipeline' do
     let(:pipeline) do
       pipeline = create(:ci_pipeline, project: project, user: user)
       stage = create(:ci_stage_entity, project: project, pipeline: pipeline, name: 'first')
-      create(:commit_status, stage_id: stage.id, pipeline: pipeline, name: 'my test job')
+      create(:ci_build, stage_id: stage.id, pipeline: pipeline, name: 'my test job')
 
       pipeline
     end
@@ -42,6 +42,9 @@ RSpec.describe 'Query.project.pipeline' do
             jobs {
               nodes {
                 name
+                needs {
+                  nodes { #{all_graphql_fields_for('CiBuildNeed')} }
+                }
                 pipeline {
                   id
                 }
@@ -53,6 +56,27 @@ RSpec.describe 'Query.project.pipeline' do
       FIELDS
     end
 
+    context 'when there are build needs' do
+      before do
+        pipeline.statuses.each do |build|
+          create_list(:ci_build_need, 2, build: build)
+        end
+      end
+
+      it 'reports the build needs' do
+        post_graphql(query, current_user: user)
+
+        expect(jobs_graphql_data).to contain_exactly a_hash_including(
+          'needs' => a_hash_including(
+            'nodes' => contain_exactly(
+              a_hash_including('name' => String),
+              a_hash_including('name' => String)
+            )
+          )
+        )
+      end
+    end
+
     it 'returns the jobs of a pipeline stage' do
       post_graphql(query, current_user: user)