Merge branch 'move-ci-pipeline-query-into-finder-235691' into 'master'

Promote ProjectPipelineResolver query to finder

See merge request gitlab-org/gitlab!47521

app/finders/ci/pipelines_finder.rb

@@ -18,7 +18,9 @@ module Ci
         return Ci::Pipeline.none
       end
 
-      items = pipelines.no_child
+      items = pipelines
+      items = items.no_child unless params[:iids].present?
+      items = by_iids(items)
       items = by_scope(items)
       items = by_status(items)
       items = by_ref(items)
@@ -52,6 +54,14 @@ module Ci
       project.repository.tag_names
     end
 
+    def by_iids(items)
+      if params[:iids].present?
+        items.for_iid(params[:iids])
+      else
+        items
+      end
+    end
+
     def by_scope(items)
       case params[:scope]
       when 'running'

app/graphql/resolvers/project_pipeline_resolver.rb

@@ -12,7 +12,9 @@ module Resolvers
 
     def resolve(iid:)
       BatchLoader::GraphQL.for(iid).batch(key: project) do |iids, loader, args|
-        args[:key].all_pipelines.for_iid(iids).each { |pl| loader.call(pl.iid.to_s, pl) }
+        finder = ::Ci::PipelinesFinder.new(project, context[:current_user], iids: iids)
+
+        finder.execute.each { |pipeline| loader.call(pipeline.iid.to_s, pipeline) }
       end
     end
   end

spec/finders/ci/pipelines_finder_spec.rb

@@ -72,7 +72,7 @@ RSpec.describe Ci::PipelinesFinder do
         create(:ci_sources_pipeline, pipeline: child_pipeline, source_pipeline: parent_pipeline)
       end
 
-      it 'filters out child pipelines and show only the parents' do
+      it 'filters out child pipelines and shows only the parents by default' do
         is_expected.to eq([parent_pipeline])
       end
     end
@@ -195,6 +195,21 @@ RSpec.describe Ci::PipelinesFinder do
       end
     end
 
+    context 'when iids filter is specified' do
+      let(:params) { { iids: [pipeline1.iid, pipeline3.iid] } }
+      let!(:pipeline1) { create(:ci_pipeline, project: project) }
+      let!(:pipeline2) { create(:ci_pipeline, project: project) }
+      let!(:pipeline3) { create(:ci_pipeline, project: project, source: :parent_pipeline) }
+
+      it 'returns matches pipelines' do
+        is_expected.to match_array([pipeline1, pipeline3])
+      end
+
+      it 'does not fitler out child pipelines' do
+        is_expected.to include(pipeline3)
+      end
+    end
+
     context 'when sha is specified' do
       let!(:pipeline) { create(:ci_pipeline, project: project, sha: '97de212e80737a608d939f648d959671fb0a0142') }
 

spec/graphql/resolvers/project_pipeline_resolver_spec.rb

@@ -18,6 +18,10 @@ RSpec.describe Resolvers::ProjectPipelineResolver do
     resolve(described_class, obj: project, args: args, ctx: { current_user: current_user })
   end
 
+  before do
+    project.add_developer(current_user)
+  end
+
   it 'resolves pipeline for the passed iid' do
     result = batch_sync do
       resolve_pipeline(project, { iid: '1234' })
@@ -26,6 +30,21 @@ RSpec.describe Resolvers::ProjectPipelineResolver do
     expect(result).to eq(pipeline)
   end
 
+  it 'keeps the queries under the threshold' do
+    create(:ci_pipeline, project: project, iid: '1235')
+
+    control = ActiveRecord::QueryRecorder.new do
+      batch_sync { resolve_pipeline(project, { iid: '1234' }) }
+    end
+
+    expect do
+      batch_sync do
+        resolve_pipeline(project, { iid: '1234' })
+        resolve_pipeline(project, { iid: '1235' })
+      end
+    end.not_to exceed_query_limit(control)
+  end
+
   it 'does not resolve a pipeline outside the project' do
     result = batch_sync do
       resolve_pipeline(other_pipeline.project, { iid: '1234' })

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

@@ -5,12 +5,12 @@ require 'spec_helper'
 RSpec.describe 'getting pipeline information nested in a project' do
   include GraphqlHelpers
 
-  let(:project) { create(:project, :repository, :public) }
-  let(:pipeline) { create(:ci_pipeline, project: project) }
-  let(:current_user) { create(:user) }
+  let!(:project) { create(:project, :repository, :public) }
+  let!(:pipeline) { create(:ci_pipeline, project: project) }
+  let!(:current_user) { create(:user) }
   let(:pipeline_graphql_data) { graphql_data['project']['pipeline'] }
 
-  let(:query) do
+  let!(:query) do
     graphql_query_for(
       'project',
       { 'fullPath' => project.full_path },
@@ -35,4 +35,45 @@ RSpec.describe 'getting pipeline information nested in a project' do
 
     expect(pipeline_graphql_data.dig('configSource')).to eq('UNKNOWN_SOURCE')
   end
+
+  context 'batching' do
+    let!(:pipeline2) { create(:ci_pipeline, project: project, user: current_user, builds: [create(:ci_build, :success)]) }
+    let!(:pipeline3) { create(:ci_pipeline, project: project, user: current_user, builds: [create(:ci_build, :success)]) }
+    let!(:query) { build_query_to_find_pipeline_shas(pipeline, pipeline2, pipeline3) }
+
+    it 'executes the finder once' do
+      mock = double(Ci::PipelinesFinder)
+      opts = { iids: [pipeline.iid, pipeline2.iid, pipeline3.iid].map(&:to_s) }
+
+      expect(Ci::PipelinesFinder).to receive(:new).once.with(project, current_user, opts).and_return(mock)
+      expect(mock).to receive(:execute).once.and_return(Ci::Pipeline.none)
+
+      post_graphql(query, current_user: current_user)
+    end
+
+    it 'keeps the queries under the threshold' do
+      control = ActiveRecord::QueryRecorder.new do
+        single_pipeline_query = build_query_to_find_pipeline_shas(pipeline)
+
+        post_graphql(single_pipeline_query, current_user: current_user)
+      end
+
+      aggregate_failures do
+        expect(response).to have_gitlab_http_status(:success)
+        expect do
+          post_graphql(query, current_user: current_user)
+        end.not_to exceed_query_limit(control)
+      end
+    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