diff --git a/app/graphql/resolvers/ci/project_pipeline_resolver.rb b/app/graphql/resolvers/ci/project_pipeline_resolver.rb index 885839d131b8bde03944912345bb5322ee1a6cdb..ce02e1a1d633b4ae4b88c2c79a53ccb62e3518fa 100644 --- a/app/graphql/resolvers/ci/project_pipeline_resolver.rb +++ b/app/graphql/resolvers/ci/project_pipeline_resolver.rb @@ -42,7 +42,12 @@ def resolve(id: nil, iid: nil, sha: nil, **args) end elsif iid BatchLoader::GraphQL.for(iid).batch(key: project) do |iids, loader| - finder = ::Ci::PipelinesFinder.new(project, current_user, iids: iids) + # This is a temporary workaround for some customers until + # https://gitlab.com/gitlab-org/gitlab/-/issues/545167 is addressed + args = { iids: iids } + args[:sort] = :asc if Feature.enabled?(:single_pipeline_for_resolver, project) + + finder = ::Ci::PipelinesFinder.new(project, current_user, args) apply_lookahead(finder.execute).each { |pipeline| loader.call(pipeline.iid.to_s, pipeline) } end diff --git a/config/feature_flags/gitlab_com_derisk/single_pipeline_for_resolver.yml b/config/feature_flags/gitlab_com_derisk/single_pipeline_for_resolver.yml new file mode 100644 index 0000000000000000000000000000000000000000..f9299fbf11ad997ab573224806f6349dc81b0636 --- /dev/null +++ b/config/feature_flags/gitlab_com_derisk/single_pipeline_for_resolver.yml @@ -0,0 +1,10 @@ +--- +name: single_pipeline_for_resolver +description: Temp workaround FF to only return one result on project pipeline resolver +feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/461470 +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/192174 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/544930 +milestone: '18.1' +group: group::pipeline execution +type: gitlab_com_derisk +default_enabled: false diff --git a/spec/graphql/resolvers/ci/project_pipeline_resolver_spec.rb b/spec/graphql/resolvers/ci/project_pipeline_resolver_spec.rb index 0834c24675488a7148f951ec19aff48b31f20320..f17c6812959e87adbbb2e125a33b6c6f32d7eb6e 100644 --- a/spec/graphql/resolvers/ci/project_pipeline_resolver_spec.rb +++ b/spec/graphql/resolvers/ci/project_pipeline_resolver_spec.rb @@ -37,7 +37,7 @@ def resolve_pipeline(project, args) it 'resolves pipeline for the passed iid' do expect(Ci::PipelinesFinder) .to receive(:new) - .with(project, current_user, iids: [project_pipeline_1.iid.to_s]) + .with(project, current_user, { iids: [project_pipeline_1.iid.to_s], sort: :asc }) .and_call_original result = batch_sync do @@ -47,6 +47,25 @@ def resolve_pipeline(project, args) expect(result).to eq(project_pipeline_1) end + context 'with FF single_pipeline_for_resolver disabled' do + before do + stub_feature_flags(single_pipeline_for_resolver: false) + end + + it 'resolves pipeline for the passed iid' do + expect(Ci::PipelinesFinder) + .to receive(:new) + .with(project, current_user, { iids: [project_pipeline_1.iid.to_s] }) + .and_call_original + + result = batch_sync do + resolve_pipeline(project, { iid: project_pipeline_1.iid.to_s }) + end + + expect(result).to eq(project_pipeline_1) + end + end + it 'resolves pipeline for the passed sha' do expect(Ci::PipelinesFinder) .to receive(:new) @@ -60,6 +79,21 @@ def resolve_pipeline(project, args) expect(result).to eq(project_pipeline_2) end + it 'only calls the finder once for all parameters' do + expect(Ci::PipelinesFinder) + .to receive(:new) + .with(project, current_user, sha: %w[sha sha1 sha2]) + .and_call_original + + result = batch_sync do + resolve_pipeline(project, { sha: 'sha' }) + resolve_pipeline(project, { sha: 'sha1' }) + resolve_pipeline(project, { sha: 'sha2' }) + end + + expect(result).to eq(project_pipeline_3) + end + it 'keeps the queries under the threshold for id' do control = ActiveRecord::QueryRecorder.new do batch_sync { resolve_pipeline(project, { id: project_pipeline_1.to_global_id }) } diff --git a/spec/requests/api/graphql/project/pipeline_spec.rb b/spec/requests/api/graphql/project/pipeline_spec.rb index bd454fdbbdf5effe17115504ccc2308f645548c8..a3b1942153402aff82a2ccc95f5131a3bb93af58 100644 --- a/spec/requests/api/graphql/project/pipeline_spec.rb +++ b/spec/requests/api/graphql/project/pipeline_spec.rb @@ -56,7 +56,7 @@ def successful_pipeline it 'executes the finder once' do mock = double(Ci::PipelinesFinder) - opts = { iids: [pipeline.iid, pipeline2.iid, pipeline3.iid].map(&:to_s) } + opts = { iids: [pipeline.iid, pipeline2.iid, pipeline3.iid].map(&:to_s), sort: :asc } 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)