Add latest changes from gitlab-org/gitlab@master

2 files changed, 119 insertions, 122 deletions

diff --git a/spec/requests/api/graphql/ci/jobs_spec.rb b/spec/requests/api/graphql/ci/jobs_spec.rb
index 19954c4e52f..045dcb532d8 100644
--- a/spec/requests/api/graphql/ci/jobs_spec.rb
+++ b/spec/requests/api/graphql/ci/jobs_spec.rb
@@ -7,48 +7,50 @@ RSpec.describe 'Query.project.pipeline' do
   let_it_be(:project) { create(:project, :repository, :public) }
   let_it_be(:user) { create(:user) }
 
-  def first(field)
-    [field.pluralize, 'nodes', 0]
+  def all(*fields)
+    fields.flat_map { |f| [f, :nodes] }
   end
 
   describe '.stages.groups.jobs' do
     let(:pipeline) do
       pipeline = create(:ci_pipeline, project: project, user: user)
-      stage = create(:ci_stage_entity, pipeline: pipeline, name: 'first')
+      stage = create(:ci_stage_entity, project: project, pipeline: pipeline, name: 'first')
       create(:commit_status, stage_id: stage.id, pipeline: pipeline, name: 'my test job')
 
       pipeline
     end
 
-    let(:jobs_graphql_data) { graphql_data.dig(*%w[project pipeline], *first('stage'), *first('group'), 'jobs', 'nodes') }
+    let(:jobs_graphql_data) { graphql_data_at(:project, :pipeline, *all(:stages, :groups, :jobs)) }
+
+    let(:first_n) { var('Int') }
 
     let(:query) do
-      %(
-        query {
-          project(fullPath: "#{project.full_path}") {
-            pipeline(iid: "#{pipeline.iid}") {
-              stages {
-                nodes {
-                  name
-                  groups {
-                    nodes {
-                      name
-                      jobs {
-                        nodes {
-                          name
-                          pipeline {
-                            id
-                          }
-                        }
-                      }
-                    }
-                  }
+      with_signature([first_n], wrap_fields(query_graphql_path([
+        [:project,  { full_path: project.full_path }],
+        [:pipeline, { iid: pipeline.iid.to_s }],
+        [:stages,   { first: first_n }]
+      ], stage_fields)))
+    end
+
+    let(:stage_fields) do
+      <<~FIELDS
+      nodes {
+        name
+        groups {
+          nodes {
+            name
+            jobs {
+              nodes {
+                name
+                pipeline {
+                  id
                 }
               }
             }
           }
         }
-      )
+      }
+      FIELDS
     end
 
     it 'returns the jobs of a pipeline stage' do
@@ -57,60 +59,43 @@ RSpec.describe 'Query.project.pipeline' do
       expect(jobs_graphql_data).to contain_exactly(a_hash_including('name' => 'my test job'))
     end
 
-    it 'avoids N+1 queries', :aggregate_failures do
-      control_count = ActiveRecord::QueryRecorder.new do
-        post_graphql(query, current_user: user)
+    describe 'performance' do
+      before do
+        build_stage = create(:ci_stage_entity, position: 2, name: 'build', project: project, pipeline: pipeline)
+        test_stage = create(:ci_stage_entity, position: 3, name: 'test', project: project, pipeline: pipeline)
+        create(:commit_status, pipeline: pipeline, stage_id: build_stage.id, name: 'docker 1 2')
+        create(:commit_status, pipeline: pipeline, stage_id: build_stage.id, name: 'docker 2 2')
+        create(:commit_status, pipeline: pipeline, stage_id: test_stage.id, name: 'rspec 1 2')
+        create(:commit_status, pipeline: pipeline, stage_id: test_stage.id, name: 'rspec 2 2')
       end
 
-      build_stage = create(:ci_stage_entity, name: 'build', pipeline: pipeline)
-      test_stage = create(:ci_stage_entity, name: 'test', pipeline: pipeline)
-      create(:commit_status, pipeline: pipeline, stage_id: build_stage.id, name: 'docker 1 2')
-      create(:commit_status, pipeline: pipeline, stage_id: build_stage.id, name: 'docker 2 2')
-      create(:commit_status, pipeline: pipeline, stage_id: test_stage.id, name: 'rspec 1 2')
-      create(:commit_status, pipeline: pipeline, stage_id: test_stage.id, name: 'rspec 2 2')
+      it 'can find the first stage' do
+        post_graphql(query, current_user: user, variables: first_n.with(1))
 
-      expect do
-        post_graphql(query, current_user: user)
-      end.not_to exceed_query_limit(control_count)
+        expect(jobs_graphql_data).to contain_exactly(a_hash_including('name' => 'my test job'))
+      end
 
-      expect(response).to have_gitlab_http_status(:ok)
+      it 'can find all stages' do
+        post_graphql(query, current_user: user, variables: first_n.with(3))
 
-      build_stage = graphql_data.dig('project', 'pipeline', 'stages', 'nodes').find do |stage|
-        stage['name'] == 'build'
+        expect(jobs_graphql_data).to contain_exactly(
+          a_hash_including('name' => 'my test job'),
+          a_hash_including('name' => 'docker 1 2'),
+          a_hash_including('name' => 'docker 2 2'),
+          a_hash_including('name' => 'rspec 1 2'),
+          a_hash_including('name' => 'rspec 2 2')
+        )
       end
-      test_stage = graphql_data.dig('project', 'pipeline', 'stages', 'nodes').find do |stage|
-        stage['name'] == 'test'
+
+      it 'avoids N+1 queries' do
+        control_count = ActiveRecord::QueryRecorder.new do
+          post_graphql(query, current_user: user, variables: first_n.with(1))
+        end
+
+        expect do
+          post_graphql(query, current_user: user, variables: first_n.with(3))
+        end.not_to exceed_query_limit(control_count)
       end
-      docker_group = build_stage.dig('groups', 'nodes').first
-      rspec_group = test_stage.dig('groups', 'nodes').first
-
-      expect(docker_group['name']).to eq('docker')
-      expect(rspec_group['name']).to eq('rspec')
-
-      docker_jobs = docker_group.dig('jobs', 'nodes')
-      rspec_jobs = rspec_group.dig('jobs', 'nodes')
-
-      expect(docker_jobs).to eq([
-        {
-          'name' => 'docker 1 2',
-          'pipeline' => { 'id' => pipeline.to_global_id.to_s }
-        },
-        {
-          'name' => 'docker 2 2',
-          'pipeline' => { 'id' => pipeline.to_global_id.to_s }
-        }
-      ])
-
-      expect(rspec_jobs).to eq([
-        {
-          'name' => 'rspec 1 2',
-          'pipeline' => { 'id' => pipeline.to_global_id.to_s }
-        },
-        {
-          'name' => 'rspec 2 2',
-          'pipeline' => { 'id' => pipeline.to_global_id.to_s }
-        }
-      ])
     end
   end
 
diff --git a/spec/requests/api/graphql/ci/pipelines_spec.rb b/spec/requests/api/graphql/ci/pipelines_spec.rb
index 414ddabbac9..7933251b8e9 100644
--- a/spec/requests/api/graphql/ci/pipelines_spec.rb
+++ b/spec/requests/api/graphql/ci/pipelines_spec.rb
@@ -6,53 +6,59 @@ RSpec.describe 'Query.project(fullPath).pipelines' do
   include GraphqlHelpers
 
   let_it_be(:project) { create(:project, :repository, :public) }
-  let_it_be(:first_user) { create(:user) }
-  let_it_be(:second_user) { create(:user) }
+  let_it_be(:user) { create(:user) }
 
   describe '.jobs' do
-    let_it_be(:query) do
-      %(
-        query {
-          project(fullPath: "#{project.full_path}") {
-            pipelines {
-              nodes {
-                jobs {
-                  nodes {
-                    name
-                  }
-                }
-              }
-            }
-          }
-        }
-      )
+    let(:first_n) { var('Int') }
+    let(:query_path) do
+      [
+        [:project, { full_path: project.full_path }],
+        [:pipelines, { first: first_n }],
+        [:nodes],
+        [:jobs],
+        [:nodes]
+      ]
     end
 
-    it 'fetches the jobs without an N+1' do
+    let(:query) do
+      with_signature([first_n], wrap_fields(query_graphql_path(query_path, :name)))
+    end
+
+    before_all do
       pipeline = create(:ci_pipeline, project: project)
       create(:ci_build, pipeline: pipeline, name: 'Job 1')
-
-      control_count = ActiveRecord::QueryRecorder.new do
-        post_graphql(query, current_user: first_user)
-      end
-
       pipeline = create(:ci_pipeline, project: project)
       create(:ci_build, pipeline: pipeline, name: 'Job 2')
+    end
 
-      expect do
-        post_graphql(query, current_user: second_user)
-      end.not_to exceed_query_limit(control_count)
+    it 'limits the results' do
+      post_graphql(query, current_user: user, variables: first_n.with(1))
 
-      expect(response).to have_gitlab_http_status(:ok)
+      expect(graphql_data_at(*query_path.map(&:first))).to contain_exactly a_hash_including(
+        'name' => 'Job 2'
+      )
+    end
 
-      pipelines_data = graphql_data.dig('project', 'pipelines', 'nodes')
+    it 'fetches all results' do
+      post_graphql(query, current_user: user)
 
-      job_names = pipelines_data.map do |pipeline_data|
-        jobs_data = pipeline_data.dig('jobs', 'nodes')
-        jobs_data.map { |job_data| job_data['name'] }
-      end.flatten
+      expect(graphql_data_at(*query_path.map(&:first))).to contain_exactly(
+        a_hash_including('name' => 'Job 1'),
+        a_hash_including('name' => 'Job 2')
+      )
+    end
+
+    it 'fetches the jobs without an N+1' do
+      first_user = create(:personal_access_token).user
+      second_user = create(:personal_access_token).user
+
+      control_count = ActiveRecord::QueryRecorder.new do
+        post_graphql(query, current_user: first_user, variables: first_n.with(1))
+      end
 
-      expect(job_names).to contain_exactly('Job 1', 'Job 2')
+      expect do
+        post_graphql(query, current_user: second_user)
+      end.not_to exceed_query_limit(control_count)
     end
   end
 
@@ -80,7 +86,7 @@ RSpec.describe 'Query.project(fullPath).pipelines' do
       create(:ci_build, :dast, name: 'DAST Job 1', pipeline: pipeline)
       create(:ci_build, :sast, name: 'SAST Job 1', pipeline: pipeline)
 
-      post_graphql(query, current_user: first_user)
+      post_graphql(query, current_user: user)
 
       expect(response).to have_gitlab_http_status(:ok)
 
@@ -96,9 +102,9 @@ RSpec.describe 'Query.project(fullPath).pipelines' do
   end
 
   describe 'upstream' do
-    let_it_be(:pipeline) { create(:ci_pipeline, project: project, user: first_user) }
+    let_it_be(:pipeline) { create(:ci_pipeline, project: project, user: user) }
     let_it_be(:upstream_project) { create(:project, :repository, :public) }
-    let_it_be(:upstream_pipeline) { create(:ci_pipeline, project: upstream_project, user: first_user) }
+    let_it_be(:upstream_pipeline) { create(:ci_pipeline, project: upstream_project, user: user) }
     let(:upstream_pipelines_graphql_data) { graphql_data.dig(*%w[project pipelines nodes]).first['upstream'] }
 
     let(:query) do
@@ -120,7 +126,7 @@ RSpec.describe 'Query.project(fullPath).pipelines' do
     before do
       create(:ci_sources_pipeline, source_pipeline: upstream_pipeline, pipeline: pipeline )
 
-      post_graphql(query, current_user: first_user)
+      post_graphql(query, current_user: user)
     end
 
     it_behaves_like 'a working graphql query'
@@ -131,15 +137,18 @@ RSpec.describe 'Query.project(fullPath).pipelines' do
 
     context 'when fetching the upstream pipeline from the pipeline' do
       it 'avoids N+1 queries' do
+        first_user = create(:user)
+        second_user = create(:user)
+
         control_count = ActiveRecord::QueryRecorder.new do
           post_graphql(query, current_user: first_user)
         end
 
-        pipeline_2 = create(:ci_pipeline, project: project, user: first_user)
-        upstream_pipeline_2 = create(:ci_pipeline, project: upstream_project, user: first_user)
+        pipeline_2 = create(:ci_pipeline, project: project, user: user)
+        upstream_pipeline_2 = create(:ci_pipeline, project: upstream_project, user: user)
         create(:ci_sources_pipeline, source_pipeline: upstream_pipeline_2, pipeline: pipeline_2 )
-        pipeline_3 = create(:ci_pipeline, project: project, user: first_user)
-        upstream_pipeline_3 = create(:ci_pipeline, project: upstream_project, user: first_user)
+        pipeline_3 = create(:ci_pipeline, project: project, user: user)
+        upstream_pipeline_3 = create(:ci_pipeline, project: upstream_project, user: user)
         create(:ci_sources_pipeline, source_pipeline: upstream_pipeline_3, pipeline: pipeline_3 )
 
         expect do
@@ -152,12 +161,12 @@ RSpec.describe 'Query.project(fullPath).pipelines' do
   end
 
   describe 'downstream' do
-    let_it_be(:pipeline) { create(:ci_pipeline, project: project, user: first_user) }
-    let(:pipeline_2) { create(:ci_pipeline, project: project, user: first_user) }
+    let_it_be(:pipeline) { create(:ci_pipeline, project: project, user: user) }
+    let(:pipeline_2) { create(:ci_pipeline, project: project, user: user) }
 
     let_it_be(:downstream_project) { create(:project, :repository, :public) }
-    let_it_be(:downstream_pipeline_a) { create(:ci_pipeline, project: downstream_project, user: first_user) }
-    let_it_be(:downstream_pipeline_b) { create(:ci_pipeline, project: downstream_project, user: first_user) }
+    let_it_be(:downstream_pipeline_a) { create(:ci_pipeline, project: downstream_project, user: user) }
+    let_it_be(:downstream_pipeline_b) { create(:ci_pipeline, project: downstream_project, user: user) }
 
     let(:pipelines_graphql_data) { graphql_data.dig(*%w[project pipelines nodes]) }
 
@@ -183,7 +192,7 @@ RSpec.describe 'Query.project(fullPath).pipelines' do
       create(:ci_sources_pipeline, source_pipeline: pipeline, pipeline: downstream_pipeline_a)
       create(:ci_sources_pipeline, source_pipeline: pipeline_2, pipeline: downstream_pipeline_b)
 
-      post_graphql(query, current_user: first_user)
+      post_graphql(query, current_user: user)
     end
 
     it_behaves_like 'a working graphql query'
@@ -198,16 +207,19 @@ RSpec.describe 'Query.project(fullPath).pipelines' do
 
     context 'when fetching the downstream pipelines from the pipeline' do
       it 'avoids N+1 queries' do
+        first_user = create(:user)
+        second_user = create(:user)
+
         control_count = ActiveRecord::QueryRecorder.new do
           post_graphql(query, current_user: first_user)
         end
 
-        downstream_pipeline_2a = create(:ci_pipeline, project: downstream_project, user: first_user)
+        downstream_pipeline_2a = create(:ci_pipeline, project: downstream_project, user: user)
         create(:ci_sources_pipeline, source_pipeline: pipeline, pipeline: downstream_pipeline_2a)
-        downsteam_pipeline_3a = create(:ci_pipeline, project: downstream_project, user: first_user)
+        downsteam_pipeline_3a = create(:ci_pipeline, project: downstream_project, user: user)
         create(:ci_sources_pipeline, source_pipeline: pipeline, pipeline: downsteam_pipeline_3a)
 
-        downstream_pipeline_2b = create(:ci_pipeline, project: downstream_project, user: first_user)
+        downstream_pipeline_2b = create(:ci_pipeline, project: downstream_project, user: user)
         create(:ci_sources_pipeline, source_pipeline: pipeline_2, pipeline: downstream_pipeline_2b)
         downsteam_pipeline_3b = create(:ci_pipeline, project: downstream_project, user: first_user)
         create(:ci_sources_pipeline, source_pipeline: pipeline_2, pipeline: downsteam_pipeline_3b)