Commit dccc561b authored by Philip Cunningham's avatar Philip Cunningham Committed by Bob Van Landuyt

Promotes ProjectPipelineResolver query to finder

Moves query in ProjectPipelineResolver to Ci::PipelinesFinder.
parent a9f94a2f
...@@ -18,7 +18,9 @@ module Ci ...@@ -18,7 +18,9 @@ module Ci
return Ci::Pipeline.none return Ci::Pipeline.none
end 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_scope(items)
items = by_status(items) items = by_status(items)
items = by_ref(items) items = by_ref(items)
...@@ -52,6 +54,14 @@ module Ci ...@@ -52,6 +54,14 @@ module Ci
project.repository.tag_names project.repository.tag_names
end end
def by_iids(items)
if params[:iids].present?
items.for_iid(params[:iids])
else
items
end
end
def by_scope(items) def by_scope(items)
case params[:scope] case params[:scope]
when 'running' when 'running'
......
...@@ -12,7 +12,9 @@ module Resolvers ...@@ -12,7 +12,9 @@ module Resolvers
def resolve(iid:) def resolve(iid:)
BatchLoader::GraphQL.for(iid).batch(key: project) do |iids, loader, args| 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 end
end end
......
...@@ -72,7 +72,7 @@ RSpec.describe Ci::PipelinesFinder do ...@@ -72,7 +72,7 @@ RSpec.describe Ci::PipelinesFinder do
create(:ci_sources_pipeline, pipeline: child_pipeline, source_pipeline: parent_pipeline) create(:ci_sources_pipeline, pipeline: child_pipeline, source_pipeline: parent_pipeline)
end 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]) is_expected.to eq([parent_pipeline])
end end
end end
...@@ -195,6 +195,21 @@ RSpec.describe Ci::PipelinesFinder do ...@@ -195,6 +195,21 @@ RSpec.describe Ci::PipelinesFinder do
end end
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 context 'when sha is specified' do
let!(:pipeline) { create(:ci_pipeline, project: project, sha: '97de212e80737a608d939f648d959671fb0a0142') } let!(:pipeline) { create(:ci_pipeline, project: project, sha: '97de212e80737a608d939f648d959671fb0a0142') }
......
...@@ -18,6 +18,10 @@ RSpec.describe Resolvers::ProjectPipelineResolver do ...@@ -18,6 +18,10 @@ RSpec.describe Resolvers::ProjectPipelineResolver do
resolve(described_class, obj: project, args: args, ctx: { current_user: current_user }) resolve(described_class, obj: project, args: args, ctx: { current_user: current_user })
end end
before do
project.add_developer(current_user)
end
it 'resolves pipeline for the passed iid' do it 'resolves pipeline for the passed iid' do
result = batch_sync do result = batch_sync do
resolve_pipeline(project, { iid: '1234' }) resolve_pipeline(project, { iid: '1234' })
...@@ -26,6 +30,21 @@ RSpec.describe Resolvers::ProjectPipelineResolver do ...@@ -26,6 +30,21 @@ RSpec.describe Resolvers::ProjectPipelineResolver do
expect(result).to eq(pipeline) expect(result).to eq(pipeline)
end 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 it 'does not resolve a pipeline outside the project' do
result = batch_sync do result = batch_sync do
resolve_pipeline(other_pipeline.project, { iid: '1234' }) resolve_pipeline(other_pipeline.project, { iid: '1234' })
......
...@@ -5,12 +5,12 @@ require 'spec_helper' ...@@ -5,12 +5,12 @@ require 'spec_helper'
RSpec.describe 'getting pipeline information nested in a project' do RSpec.describe 'getting pipeline information nested in a project' do
include GraphqlHelpers include GraphqlHelpers
let(:project) { create(:project, :repository, :public) } let!(:project) { create(:project, :repository, :public) }
let(:pipeline) { create(:ci_pipeline, project: project) } let!(:pipeline) { create(:ci_pipeline, project: project) }
let(:current_user) { create(:user) } let!(:current_user) { create(:user) }
let(:pipeline_graphql_data) { graphql_data['project']['pipeline'] } let(:pipeline_graphql_data) { graphql_data['project']['pipeline'] }
let(:query) do let!(:query) do
graphql_query_for( graphql_query_for(
'project', 'project',
{ 'fullPath' => project.full_path }, { 'fullPath' => project.full_path },
...@@ -35,4 +35,45 @@ RSpec.describe 'getting pipeline information nested in a project' do ...@@ -35,4 +35,45 @@ RSpec.describe 'getting pipeline information nested in a project' do
expect(pipeline_graphql_data.dig('configSource')).to eq('UNKNOWN_SOURCE') expect(pipeline_graphql_data.dig('configSource')).to eq('UNKNOWN_SOURCE')
end 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 end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment