Skip to content

G
gitlab-ce
Commit eafcfe2d authored by Igor Drozdov's avatar Igor Drozdov
Browse files
Options

Extract MergeRequest#all_pipelines into a separate class 

It uses scopes from Ci::Pipeline, which are public and used once,
what about just moving them in this class and make them private?
parent f7f91373
Hide whitespace changes
Inline Side-by-side
Showing with 221 additions and 389 deletions
app/models/ci/pipeline.rb
View file @ eafcfe2d
......@@ -205,14 +205,6 @@ module Ci
scope :internal, -> { where(source: internal_sources) }
scope :ci_sources, -> { where(config_source: ci_sources_values) }
scope :sort_by_merge_request_pipelines, -> do
sql = 'CASE ci_pipelines.source WHEN (?) THEN 0 ELSE 1 END, ci_pipelines.id DESC'
query = ApplicationRecord.send(:sanitize_sql_array, [sql, sources[:merge_request_event]]) # rubocop:disable GitlabSecurity/PublicSend
order(Arel.sql(query))
end
scope :for_user, -> (user) { where(user: user) }
scope :for_sha, -> (sha) { where(sha: sha) }
scope :for_source_sha, -> (source_sha) { where(source_sha: source_sha) }
......@@ -221,22 +213,6 @@ module Ci
scope :for_id, -> (id) { where(id: id) }
scope :created_after, -> (time) { where('ci_pipelines.created_at > ?', time) }
scope :triggered_by_merge_request, -> (merge_request) do
where(source: :merge_request_event, merge_request: merge_request)
end
scope :detached_merge_request_pipelines, -> (merge_request, sha) do
triggered_by_merge_request(merge_request).for_sha(sha)
end
scope :merge_request_pipelines, -> (merge_request, source_sha) do
triggered_by_merge_request(merge_request).for_source_sha(source_sha)
end
scope :triggered_for_branch, -> (ref) do
where(source: branch_pipeline_sources).where(ref: ref, tag: false)
end
scope :with_reports, -> (reports_scope) do
where('EXISTS (?)', ::Ci::Build.latest.with_reports(reports_scope).where('ci_pipelines.id=ci_builds.commit_id').select(1))
end
......@@ -344,10 +320,6 @@ module Ci
sources.reject { |source| source == "external" }.values
end
def self.branch_pipeline_sources
@branch_pipeline_sources ||= sources.reject { |source| source == 'merge_request_event' }.values
end
def self.ci_sources_values
config_sources.values_at(:repository_source, :auto_devops_source, :unknown_source)
end
......
app/models/merge_request.rb
View file @ eafcfe2d
......@@ -1245,16 +1245,8 @@ class MergeRequest < ApplicationRecord
end
def all_pipelines
return Ci::Pipeline.none unless source_project
shas = all_commit_shas
strong_memoize(:all_pipelines) do
Ci::Pipeline.from_union(
[source_project.ci_pipelines.merge_request_pipelines(self, shas),
source_project.ci_pipelines.detached_merge_request_pipelines(self, shas),
source_project.ci_pipelines.triggered_for_branch(source_branch).for_sha(shas)],
remove_duplicates: false).sort_by_merge_request_pipelines
MergeRequest::Pipelines.new(self).all
end
end
......
app/models/merge_request/pipelines.rb 0 → 100644
View file @ eafcfe2d
# frozen_string_literal: true
# A state object to centralize logic related to merge request pipelines
class MergeRequest::Pipelines
include Gitlab::Utils::StrongMemoize
EVENT = 'merge_request_event'
def initialize(merge_request)
@merge_request = merge_request
end
attr_reader :merge_request
delegate :all_commit_shas, :source_project, :source_branch, to: :merge_request
def all
return Ci::Pipeline.none unless source_project
strong_memoize(:all_pipelines) do
pipelines = Ci::Pipeline.from_union(
[source_pipelines, detached_pipelines, triggered_for_branch],
remove_duplicates: false)
sort(pipelines)
end
end
private
def triggered_by_merge_request
source_project.ci_pipelines
.where(source: :merge_request_event, merge_request: merge_request)
end
def detached_pipelines
triggered_by_merge_request.for_sha(all_commit_shas)
end
def source_pipelines
triggered_by_merge_request.for_source_sha(all_commit_shas)
end
def triggered_for_branch
source_project.ci_pipelines
.where(source: branch_pipeline_sources, ref: source_branch, tag: false)
.for_sha(all_commit_shas)
end
def sources
::Ci::Pipeline.sources
end
def branch_pipeline_sources
strong_memoize(:branch_pipeline_sources) do
sources.reject { |source| source == EVENT }.values
end
end
def sort(pipelines)
sql = 'CASE ci_pipelines.source WHEN (?) THEN 0 ELSE 1 END, ci_pipelines.id DESC'
query = ApplicationRecord.send(:sanitize_sql_array, [sql, sources[:merge_request_event]]) # rubocop:disable GitlabSecurity/PublicSend
pipelines.order(Arel.sql(query))
end
end
spec/models/ci/pipeline_spec.rb
View file @ eafcfe2d
......@@ -75,71 +75,6 @@ describe Ci::Pipeline, :mailer do
end
end
describe '.sort_by_merge_request_pipelines' do
subject { described_class.sort_by_merge_request_pipelines }
context 'when branch pipelines exist' do
let!(:branch_pipeline_1) { create(:ci_pipeline, source: :push) }
let!(:branch_pipeline_2) { create(:ci_pipeline, source: :push) }
it 'returns pipelines order by id' do
expect(subject).to eq([branch_pipeline_2,
branch_pipeline_1])
end
end
context 'when merge request pipelines exist' do
let!(:merge_request_pipeline_1) do
create(:ci_pipeline, source: :merge_request_event, merge_request: merge_request)
end
let!(:merge_request_pipeline_2) do
create(:ci_pipeline, source: :merge_request_event, merge_request: merge_request)
end
let(:merge_request) do
create(:merge_request,
source_project: project,
source_branch: 'feature',
target_project: project,
target_branch: 'master')
end
it 'returns pipelines order by id' do
expect(subject).to eq([merge_request_pipeline_2,
merge_request_pipeline_1])
end
end
context 'when both branch pipeline and merge request pipeline exist' do
let!(:branch_pipeline_1) { create(:ci_pipeline, source: :push) }
let!(:branch_pipeline_2) { create(:ci_pipeline, source: :push) }
let!(:merge_request_pipeline_1) do
create(:ci_pipeline, source: :merge_request_event, merge_request: merge_request)
end
let!(:merge_request_pipeline_2) do
create(:ci_pipeline, source: :merge_request_event, merge_request: merge_request)
end
let(:merge_request) do
create(:merge_request,
source_project: project,
source_branch: 'feature',
target_project: project,
target_branch: 'master')
end
it 'returns merge request pipeline first' do
expect(subject).to eq([merge_request_pipeline_2,
merge_request_pipeline_1,
branch_pipeline_2,
branch_pipeline_1])
end
end
end
describe '.for_sha' do
subject { described_class.for_sha(sha) }
......@@ -226,39 +161,6 @@ describe Ci::Pipeline, :mailer do
end
end
describe '.detached_merge_request_pipelines' do
subject { described_class.detached_merge_request_pipelines(merge_request, sha) }
let!(:pipeline) do
create(:ci_pipeline, source: :merge_request_event, merge_request: merge_request, sha: merge_request.diff_head_sha)
end
let(:merge_request) { create(:merge_request) }
let(:sha) { merge_request.diff_head_sha }
it 'returns detached merge request pipelines' do
is_expected.to eq([pipeline])
end
context 'when sha does not exist' do
let(:sha) { 'abc' }
it 'returns empty array' do
is_expected.to be_empty
end
end
context 'when pipeline is merge request pipeline' do
let!(:pipeline) do
create(:ci_pipeline, source: :merge_request_event, merge_request: merge_request, source_sha: merge_request.diff_head_sha)
end
it 'returns empty array' do
is_expected.to be_empty
end
end
end
describe '#detached_merge_request_pipeline?' do
subject { pipeline.detached_merge_request_pipeline? }
......@@ -278,39 +180,6 @@ describe Ci::Pipeline, :mailer do
end
end
describe '.merge_request_pipelines' do
subject { described_class.merge_request_pipelines(merge_request, source_sha) }
let!(:pipeline) do
create(:ci_pipeline, source: :merge_request_event, merge_request: merge_request, source_sha: merge_request.diff_head_sha)
end
let(:merge_request) { create(:merge_request) }
let(:source_sha) { merge_request.diff_head_sha }
it 'returns merge pipelines' do
is_expected.to eq([pipeline])
end
context 'when source sha is empty' do
let(:source_sha) { nil }
it 'returns empty array' do
is_expected.to be_empty
end
end
context 'when pipeline is detached merge request pipeline' do
let!(:pipeline) do
create(:ci_pipeline, source: :merge_request_event, merge_request: merge_request, sha: merge_request.diff_head_sha)
end
it 'returns empty array' do
is_expected.to be_empty
end
end
end
describe '#merge_request_pipeline?' do
subject { pipeline.merge_request_pipeline? }
......@@ -499,50 +368,6 @@ describe Ci::Pipeline, :mailer do
end
end
describe '.triggered_for_branch' do
subject { described_class.triggered_for_branch(ref) }
let(:project) { create(:project, :repository) }
let(:ref) { 'feature' }
let!(:pipeline) { create(:ci_pipeline, ref: ref) }
it 'returns the pipeline' do
is_expected.to eq([pipeline])
end
context 'when sha is not specified' do
it 'returns the pipeline' do
expect(described_class.triggered_for_branch(ref)).to eq([pipeline])
end
end
context 'when pipeline is triggered for tag' do
let(:ref) { 'v1.1.0' }
let!(:pipeline) { create(:ci_pipeline, ref: ref, tag: true) }
it 'does not return the pipeline' do
is_expected.to be_empty
end
end
context 'when pipeline is triggered for merge_request' do
let!(:merge_request) do
create(:merge_request,
:with_merge_request_pipeline,
source_project: project,
source_branch: ref,
target_project: project,
target_branch: 'master')
end
let(:pipeline) { merge_request.pipelines_for_merge_request.first }
it 'does not return the pipeline' do
is_expected.to be_empty
end
end
end
describe '.with_reports' do
subject { described_class.with_reports(Ci::JobArtifact.test_reports) }
......
spec/models/merge_request/pipelines_spec.rb 0 → 100644
View file @ eafcfe2d
# frozen_string_literal: true
require 'spec_helper'
describe MergeRequest::Pipelines do
describe '#all' do
let(:merge_request) { create(:merge_request) }
let(:project) { merge_request.source_project }
subject { described_class.new(merge_request) }
shared_examples 'returning pipelines with proper ordering' do
let!(:all_pipelines) do
merge_request.all_commit_shas.map do |sha|
create(:ci_empty_pipeline,
project: project, sha: sha, ref: merge_request.source_branch)
end
end
it 'returns all pipelines' do
expect(subject.all).not_to be_empty
expect(subject.all).to eq(all_pipelines.reverse)
end
end
context 'with single merge_request_diffs' do
it_behaves_like 'returning pipelines with proper ordering'
end
context 'with multiple irrelevant merge_request_diffs' do
before do
merge_request.update(target_branch: 'v1.0.0')
end
it_behaves_like 'returning pipelines with proper ordering'
end
context 'with unsaved merge request' do
let(:merge_request) { build(:merge_request) }
let!(:pipeline) do
create(:ci_empty_pipeline, project: project,
sha: merge_request.diff_head_sha, ref: merge_request.source_branch)
end
it 'returns pipelines from diff_head_sha' do
expect(subject.all).to contain_exactly(pipeline)
end
end
context 'when pipelines exist for the branch and merge request' do
let(:source_ref) { 'feature' }
let(:target_ref) { 'master' }
let!(:branch_pipeline) do
create(:ci_pipeline, source: :push, project: project,
ref: source_ref, sha: shas.second)
end
let!(:tag_pipeline) do
create(:ci_pipeline, project: project, ref: source_ref, tag: true)
end
let!(:detached_merge_request_pipeline) do
create(:ci_pipeline, source: :merge_request_event, project: project,
ref: source_ref, sha: shas.second, merge_request: merge_request)
end
let(:merge_request) do
create(:merge_request, source_project: project, source_branch: source_ref,
target_project: project, target_branch: target_ref)
end
let(:project) { create(:project, :repository) }
let(:shas) { project.repository.commits(source_ref, limit: 2).map(&:id) }
before do
allow(merge_request).to receive(:all_commit_shas) { shas }
end
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!(: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
allow(merge_request_2).to receive(:all_commit_shas) { shas }
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).all)
.to eq([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
end
spec/models/merge_request_spec.rb
View file @ eafcfe2d
......@@ -1441,183 +1441,6 @@ describe MergeRequest do
end
end
describe '#all_pipelines' do
shared_examples 'returning pipelines with proper ordering' do
let!(:all_pipelines) do
subject.all_commit_shas.map do |sha|
create(:ci_empty_pipeline,
project: subject.source_project,
sha: sha,
ref: subject.source_branch)
end
end
it 'returns all pipelines' do
expect(subject.all_pipelines).not_to be_empty
expect(subject.all_pipelines).to eq(all_pipelines.reverse)
end
end
context 'with single merge_request_diffs' do
it_behaves_like 'returning pipelines with proper ordering'
end
context 'with multiple irrelevant merge_request_diffs' do
before do
subject.update(target_branch: 'v1.0.0')
end
it_behaves_like 'returning pipelines with proper ordering'
end
context 'with unsaved merge request' do
subject { build(:merge_request) }
let!(:pipeline) do
create(:ci_empty_pipeline,
project: subject.project,
sha: subject.diff_head_sha,
ref: subject.source_branch)
end
it 'returns pipelines from diff_head_sha' do
expect(subject.all_pipelines).to contain_exactly(pipeline)
end
end
context 'when pipelines exist for the branch and merge request' do
let(:source_ref) { 'feature' }
let(:target_ref) { 'master' }
let!(:branch_pipeline) do
create(:ci_pipeline,
source: :push,
project: project,
ref: source_ref,
sha: shas.second)
end
let!(:detached_merge_request_pipeline) do
create(:ci_pipeline,
source: :merge_request_event,
project: project,
ref: source_ref,
sha: shas.second,
merge_request: merge_request)
end
let(:merge_request) do
create(:merge_request,
source_project: project,
source_branch: source_ref,
target_project: project,
target_branch: target_ref)
end
let(:project) { create(:project, :repository) }
let(:shas) { project.repository.commits(source_ref, limit: 2).map(&:id) }
before do
allow(merge_request).to receive(:all_commit_shas) { shas }
end
it 'returns merge request pipeline first' do
expect(merge_request.all_pipelines)
.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(merge_request.all_pipelines)
.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!(: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
allow(merge_request_2).to receive(:all_commit_shas) { shas }
end
it 'returns only related merge request pipelines' do
expect(merge_request.all_pipelines)
.to eq([detached_merge_request_pipeline,
branch_pipeline_2,
branch_pipeline])
expect(merge_request_2.all_pipelines)
.to eq([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
describe '#update_head_pipeline' do
subject { merge_request.update_head_pipeline }
......
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
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7