diff --git a/app/finders/ci/pipelines_for_merge_request_finder.rb b/app/finders/ci/pipelines_for_merge_request_finder.rb index 839ec417cd402f6699b10874312f5baeadf9d445..b623a94541b236ab3233182f46c76e79ef7d744e 100644 --- a/app/finders/ci/pipelines_for_merge_request_finder.rb +++ b/app/finders/ci/pipelines_for_merge_request_finder.rb @@ -31,67 +31,27 @@ def execute # Fetch all pipelines without permission check. def all - ::Gitlab::Database.allow_cross_joins_across_databases(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/336891') do - strong_memoize(:all_pipelines) do - next Ci::Pipeline.none unless source_project + strong_memoize(:all_pipelines) do + next Ci::Pipeline.none unless source_project - pipelines = - if merge_request.persisted? - all_pipelines_for_merge_request - else - triggered_for_branch.for_sha(commit_shas) - end + pipelines = + if merge_request.persisted? + all_pipelines_for_merge_request + else + triggered_for_branch.for_sha(commit_shas) + end - pipelines = pipelines.allow_cross_joins_across_databases(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/336891') - - sort(pipelines) - end + sort(pipelines) end end private - # rubocop: disable CodeReuse/ActiveRecord - def pipelines_using_cte - sha_relation = merge_request.all_commits.select(:sha).distinct - - cte = Gitlab::SQL::CTE.new(:shas, sha_relation) - - pipelines_for_merge_requests = triggered_by_merge_request - pipelines_for_branch = filter_by_sha(triggered_for_branch, cte) - - Ci::Pipeline.with(cte.to_arel) # rubocop: disable CodeReuse/ActiveRecord - .from_union([pipelines_for_merge_requests, pipelines_for_branch]) - end - # rubocop: enable CodeReuse/ActiveRecord - - def filter_by_sha(pipelines, cte) - hex = Arel::Nodes::SqlLiteral.new("'hex'") - string_sha = Arel::Nodes::NamedFunction.new('encode', [cte.table[:sha], hex]) - join_condition = string_sha.eq(Ci::Pipeline.arel_table[:sha]) - - filter_by(pipelines, cte, join_condition) - end - - def filter_by(pipelines, cte, join_condition) - shas_table = - Ci::Pipeline.arel_table - .join(cte.table, Arel::Nodes::InnerJoin) - .on(join_condition) - .join_sources - - pipelines.joins(shas_table) # rubocop: disable CodeReuse/ActiveRecord - end - def all_pipelines_for_merge_request - if Feature.enabled?(:decomposed_ci_query_in_pipelines_for_merge_request_finder, target_project, default_enabled: :yaml) - pipelines_for_merge_request = triggered_by_merge_request - pipelines_for_branch = triggered_for_branch.for_sha(recent_diff_head_shas(COMMITS_LIMIT)) + pipelines_for_merge_request = triggered_by_merge_request + pipelines_for_branch = triggered_for_branch.for_sha(recent_diff_head_shas(COMMITS_LIMIT)) - Ci::Pipeline.from_union([pipelines_for_merge_request, pipelines_for_branch]) - else - pipelines_using_cte - end + Ci::Pipeline.from_union([pipelines_for_merge_request, pipelines_for_branch]) end # NOTE: this method returns only parent merge request pipelines. diff --git a/config/feature_flags/development/decomposed_ci_query_in_pipelines_for_merge_request_finder.yml b/config/feature_flags/development/decomposed_ci_query_in_pipelines_for_merge_request_finder.yml deleted file mode 100644 index 235b37dfb1dbb61f6f64755ee8149e2a307a90ec..0000000000000000000000000000000000000000 --- a/config/feature_flags/development/decomposed_ci_query_in_pipelines_for_merge_request_finder.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: decomposed_ci_query_in_pipelines_for_merge_request_finder -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/68549 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/341341 -milestone: '14.4' -type: development -group: group::pipeline execution -default_enabled: false diff --git a/spec/finders/ci/pipelines_for_merge_request_finder_spec.rb b/spec/finders/ci/pipelines_for_merge_request_finder_spec.rb index 8a802e9660bec8b1944e97f83ee21e962d72d741..a7cf041f553bde407c7cdea371479f86fd1e7866 100644 --- a/spec/finders/ci/pipelines_for_merge_request_finder_spec.rb +++ b/spec/finders/ci/pipelines_for_merge_request_finder_spec.rb @@ -135,86 +135,6 @@ end context 'when pipelines exist for the branch and merge request' do - shared_examples 'returns all pipelines for merge request' do - it 'returns merge request pipeline first' do - expect(subject.all).to eq([detached_merge_request_pipeline, branch_pipeline]) - end - - context 'when there are a branch pipeline and a merge request pipeline' do - let!(:branch_pipeline_2) do - create(:ci_pipeline, source: :push, project: project, - ref: source_ref, sha: shas.first) - end - - let!(:detached_merge_request_pipeline_2) do - create(:ci_pipeline, source: :merge_request_event, project: project, - ref: source_ref, sha: shas.first, merge_request: merge_request) - end - - it 'returns merge request pipelines first' do - expect(subject.all) - .to eq([detached_merge_request_pipeline_2, - detached_merge_request_pipeline, - branch_pipeline_2, - branch_pipeline]) - end - end - - context 'when there are multiple merge request pipelines from the same branch' do - let!(:branch_pipeline_2) do - create(:ci_pipeline, source: :push, project: project, - ref: source_ref, sha: shas.first) - end - - let!(:branch_pipeline_with_sha_not_belonging_to_merge_request) do - create(:ci_pipeline, source: :push, project: project, ref: source_ref) - end - - let!(:detached_merge_request_pipeline_2) do - create(:ci_pipeline, source: :merge_request_event, project: project, - ref: source_ref, sha: shas.first, merge_request: merge_request_2) - end - - let(:merge_request_2) do - create(:merge_request, source_project: project, source_branch: source_ref, - target_project: project, target_branch: 'stable') - end - - before do - shas.each.with_index do |sha, index| - create(:merge_request_diff_commit, - merge_request_diff: merge_request_2.merge_request_diff, - sha: sha, relative_order: index) - end - end - - it 'returns only related merge request pipelines' do - expect(subject.all) - .to eq([detached_merge_request_pipeline, - branch_pipeline_2, - branch_pipeline]) - - expect(described_class.new(merge_request_2, nil).all) - .to match_array([detached_merge_request_pipeline_2, branch_pipeline_2, branch_pipeline]) - end - end - - context 'when detached merge request pipeline is run on head ref of the merge request' do - let!(:detached_merge_request_pipeline) do - create(:ci_pipeline, source: :merge_request_event, project: project, - ref: merge_request.ref_path, sha: shas.second, merge_request: merge_request) - end - - it 'sets the head ref of the merge request to the pipeline ref' do - expect(detached_merge_request_pipeline.ref).to match(%r{refs/merge-requests/\d+/head}) - end - - it 'includes the detached merge request pipeline even though the ref is custom path' do - expect(merge_request.all_pipelines).to include(detached_merge_request_pipeline) - end - end - end - let(:source_ref) { 'feature' } let(:target_ref) { 'master' } @@ -240,20 +160,76 @@ let(:project) { create(:project, :repository) } let(:shas) { project.repository.commits(source_ref, limit: 2).map(&:id) } - context 'when `decomposed_ci_query_in_pipelines_for_merge_request_finder` feature flag enabled' do - before do - stub_feature_flags(decomposed_ci_query_in_pipelines_for_merge_request_finder: merge_request.target_project) + it 'returns merge request pipeline first' do + expect(subject.all).to match_array([detached_merge_request_pipeline, branch_pipeline]) + end + + context 'when there are a branch pipeline and a merge request pipeline' do + let!(:branch_pipeline_2) do + create(:ci_pipeline, source: :push, project: project, + ref: source_ref, sha: shas.first) + end + + let!(:detached_merge_request_pipeline_2) do + create(:ci_pipeline, source: :merge_request_event, project: project, + ref: source_ref, sha: shas.first, merge_request: merge_request) end - it_behaves_like 'returns all pipelines for merge request' + it 'returns merge request pipelines first' do + expect(subject.all) + .to match_array([detached_merge_request_pipeline_2, detached_merge_request_pipeline, branch_pipeline_2, branch_pipeline]) + end end - context 'when `decomposed_ci_query_in_pipelines_for_merge_request_finder` feature flag disabled' do + context 'when there are multiple merge request pipelines from the same branch' do + let!(:branch_pipeline_2) do + create(:ci_pipeline, source: :push, project: project, + ref: source_ref, sha: shas.first) + end + + let!(:branch_pipeline_with_sha_not_belonging_to_merge_request) do + create(:ci_pipeline, source: :push, project: project, ref: source_ref) + end + + let!(:detached_merge_request_pipeline_2) do + create(:ci_pipeline, source: :merge_request_event, project: project, + ref: source_ref, sha: shas.first, merge_request: merge_request_2) + end + + let(:merge_request_2) do + create(:merge_request, source_project: project, source_branch: source_ref, + target_project: project, target_branch: 'stable') + end + before do - stub_feature_flags(decomposed_ci_query_in_pipelines_for_merge_request_finder: false) + shas.each.with_index do |sha, index| + create(:merge_request_diff_commit, + merge_request_diff: merge_request_2.merge_request_diff, + sha: sha, relative_order: index) + end end - it_behaves_like 'returns all pipelines for merge request' + it 'returns only related merge request pipelines' do + expect(subject.all).to match_array([detached_merge_request_pipeline, branch_pipeline_2, branch_pipeline]) + + expect(described_class.new(merge_request_2, nil).all) + .to match_array([detached_merge_request_pipeline_2, branch_pipeline_2, branch_pipeline]) + end + end + + context 'when detached merge request pipeline is run on head ref of the merge request' do + let!(:detached_merge_request_pipeline) do + create(:ci_pipeline, source: :merge_request_event, project: project, + ref: merge_request.ref_path, sha: shas.second, merge_request: merge_request) + end + + it 'sets the head ref of the merge request to the pipeline ref' do + expect(detached_merge_request_pipeline.ref).to match(%r{refs/merge-requests/\d+/head}) + end + + it 'includes the detached merge request pipeline even though the ref is custom path' do + expect(merge_request.all_pipelines).to include(detached_merge_request_pipeline) + end end end end