Commit e39ebfd5 authored by Fabio Pitino's avatar Fabio Pitino

Move domain logic to MergeTrain class

MergeTrains::RefreshMergeRequestService has a lot of domain
knowledge that belong to MergeTrain model. This refactoring
makes the model class richer and the service object more
focused on transactional logic.
parent 12f8692c
# frozen_string_literal: true # frozen_string_literal: true
class MergeTrain < ApplicationRecord class MergeTrain < ApplicationRecord
include Gitlab::Utils::StrongMemoize
include AfterCommitQueue include AfterCommitQueue
ACTIVE_STATUSES = %w[idle stale fresh].freeze ACTIVE_STATUSES = %w[idle stale fresh].freeze
...@@ -11,6 +12,8 @@ class MergeTrain < ApplicationRecord ...@@ -11,6 +12,8 @@ class MergeTrain < ApplicationRecord
belongs_to :user belongs_to :user
belongs_to :pipeline, class_name: 'Ci::Pipeline' belongs_to :pipeline, class_name: 'Ci::Pipeline'
alias_attribute :project, :target_project
after_destroy do |merge_train| after_destroy do |merge_train|
run_after_commit do run_after_commit do
merge_train.pipeline&.cancel_running(retries: 1) merge_train.pipeline&.cancel_running(retries: 1)
...@@ -148,12 +151,32 @@ class MergeTrain < ApplicationRecord ...@@ -148,12 +151,32 @@ class MergeTrain < ApplicationRecord
all_prev.count all_prev.count
end end
def first_in_train? def previous_ref
!follower_in_train? prev&.train_ref_path || merge_request.target_branch_ref
end
def previous_ref_sha
project.repository.commit(previous_ref)&.sha
end
def requires_new_pipeline?
!has_pipeline? || stale?
end
def pipeline_not_succeeded?
has_pipeline? && pipeline.complete? && !pipeline.success?
end end
def follower_in_train? def cancel_pipeline!(new_pipeline)
all_prev.exists? pipeline&.auto_cancel_running(new_pipeline, retries: 1)
rescue ActiveRecord::StaleObjectError
# Often the pipeline has already been canceled by the default cancellation
# mechanizm `Ci::CreatePipelineService#cancel_pending_pipelines`. In this
# case, we can ignore the exception as it's already canceled.
end
def mergeable?
has_pipeline? && pipeline&.success? && first_in_train?
end end
def cleanup_ref def cleanup_ref
...@@ -167,4 +190,14 @@ class MergeTrain < ApplicationRecord ...@@ -167,4 +190,14 @@ class MergeTrain < ApplicationRecord
def refresh_async def refresh_async
AutoMergeProcessWorker.perform_async(merge_request_id) AutoMergeProcessWorker.perform_async(merge_request_id)
end end
private
def has_pipeline?
pipeline_id.present? && pipeline
end
def first_in_train?
all_prev.none?
end
end end
...@@ -14,8 +14,8 @@ module MergeTrains ...@@ -14,8 +14,8 @@ module MergeTrains
@merge_request = merge_request @merge_request = merge_request
validate! validate!
pipeline_created = create_pipeline! if should_create_pipeline? pipeline_created = create_pipeline! if merge_train.requires_new_pipeline? || require_recreate?
merge! if should_merge? merge! if merge_train.mergeable?
success(pipeline_created: pipeline_created.present?) success(pipeline_created: pipeline_created.present?)
rescue ProcessError => e rescue ProcessError => e
...@@ -37,33 +37,26 @@ module MergeTrains ...@@ -37,33 +37,26 @@ module MergeTrains
raise ProcessError, 'merge request is not mergeable' raise ProcessError, 'merge request is not mergeable'
end end
unless previous_ref_exist? unless merge_train.previous_ref_sha.present?
raise ProcessError, 'previous ref does not exist' raise ProcessError, 'previous ref does not exist'
end end
if pipeline_for_merge_train if merge_train.pipeline_not_succeeded?
if pipeline_for_merge_train.complete? && !pipeline_for_merge_train.success? raise ProcessError, 'pipeline did not succeed'
raise ProcessError, 'pipeline did not succeed'
end
end end
end end
def should_create_pipeline?
pipeline_absent? || require_recreate? || stale_pipeline?
end
def create_pipeline! def create_pipeline!
result = MergeTrains::CreatePipelineService.new(merge_request.project, merge_user) result = MergeTrains::CreatePipelineService.new(merge_train.project, merge_train.user)
.execute(merge_request, previous_ref) .execute(merge_train.merge_request, merge_train.previous_ref)
raise ProcessError, result[:message] unless result[:status] == :success raise ProcessError, result[:message] unless result[:status] == :success
cancel_pipeline_for_merge_train(result[:pipeline]) pipeline = result[:pipeline]
update_pipeline_for_merge_train(result[:pipeline]) merge_train.cancel_pipeline!(pipeline)
end merge_train.refresh_pipeline!(pipeline.id)
def should_merge? pipeline
pipeline_for_merge_train&.success? && first_in_train?
end end
def merge! def merge!
...@@ -77,64 +70,14 @@ module MergeTrains ...@@ -77,64 +70,14 @@ module MergeTrains
merge_train.finish_merge! merge_train.finish_merge!
end end
def stale_pipeline?
merge_train.stale?
end
def pipeline_absent?
!pipeline_for_merge_train.present?
end
def merge_train def merge_train
merge_request.merge_train merge_request.merge_train
end end
def pipeline_for_merge_train
merge_train.pipeline
end
def cancel_pipeline_for_merge_train(new_pipeline)
pipeline_for_merge_train&.auto_cancel_running(new_pipeline, retries: 1)
rescue ActiveRecord::StaleObjectError
# Often the pipeline has already been canceled by the default cancelaltion
# mechanizm `Ci::CreatePipelineService#cancel_pending_pipelines`. In this
# case, we can ignore the exception as it's already canceled.
end
def update_pipeline_for_merge_train(pipeline)
merge_train.refresh_pipeline!(pipeline.id)
end
def merge_user def merge_user
merge_request.merge_user merge_request.merge_user
end end
def first_in_train?
strong_memoize(:is_first_in_train) do
merge_train.first_in_train?
end
end
def previous_ref_sha
strong_memoize(:previous_ref_sha) do
merge_request.project.repository.commit(previous_ref)&.sha
end
end
def previous_ref
previous_merge_request&.train_ref_path || merge_request.target_branch_ref
end
def previous_ref_exist?
previous_ref_sha.present?
end
def previous_merge_request
strong_memoize(:previous_merge_request) do
merge_request.merge_train.prev
end
end
def require_recreate? def require_recreate?
params[:require_recreate] params[:require_recreate]
end end
......
...@@ -434,38 +434,151 @@ RSpec.describe MergeTrain do ...@@ -434,38 +434,151 @@ RSpec.describe MergeTrain do
end end
end end
describe '#first_in_train?' do describe '#previous_ref' do
subject { merge_train.first_in_train? } subject { merge_train.previous_ref }
let(:merge_train) { merge_request.merge_train } let(:merge_train) { merge_request.merge_train }
let!(:merge_request) { create_merge_request_on_train } let!(:merge_request) { create_merge_request_on_train }
it { is_expected.to be_truthy } context 'when merge request is first on train' do
it 'returns the target branch' do
is_expected.to eq(merge_request.target_branch_ref)
end
end
context 'when the other merge request is on the merge train' do context 'when merge request is not first on train' do
let(:merge_train) { merge_request_2.merge_train } let(:merge_train) { merge_request_2.merge_train }
let!(:merge_request_2) { create_merge_request_on_train(source_branch: 'improve/awesome') } let!(:merge_request_2) { create_merge_request_on_train(source_branch: 'feature-2') }
it { is_expected.to be_falsy } it 'returns the ref of the previous merge request' do
is_expected.to eq(merge_request.train_ref_path)
end
end end
end end
describe '#follower_in_train?' do describe '#requires_new_pipeline?' do
subject { merge_train.follower_in_train? } subject { merge_train.requires_new_pipeline? }
let(:merge_train) { merge_request.merge_train } let(:merge_train) { merge_request.merge_train }
let!(:merge_request) { create_merge_request_on_train } let!(:merge_request) { create_merge_request_on_train }
it { is_expected.to be_falsy } context 'when merge train has a pipeline associated' do
before do
merge_train.update!(pipeline: create(:ci_pipeline, project: merge_train.project))
end
context 'when the other merge request is on the merge train' do it { is_expected.to be_falsey }
let(:merge_train) { merge_request_2.merge_train }
let!(:merge_request_2) { create_merge_request_on_train(source_branch: 'improve/awesome') } context 'when merge train is stale' do
before do
merge_train.update!(status: MergeTrain.state_machines[:status].states[:stale].value)
end
it { is_expected.to be_truthy }
end
end
context 'when merge train does not have a pipeline' do
before do
merge_train.update!(pipeline: nil)
end
it { is_expected.to be_truthy } it { is_expected.to be_truthy }
end end
end end
describe '#pipeline_not_succeeded?' do
subject { merge_train.pipeline_not_succeeded? }
let(:merge_train) { merge_request.merge_train }
let!(:merge_request) { create_merge_request_on_train }
context 'when merge train does not have a pipeline' do
it { is_expected.to be_falsey }
end
context 'when merge train has a pipeline' do
let(:pipeline) { create(:ci_pipeline, project: merge_train.project, status: status) }
before do
merge_train.update!(pipeline: pipeline)
end
context 'when pipeline failed' do
let(:status) { :failed }
it { is_expected.to be_truthy }
end
context 'when pipeline succeeded' do
let(:status) { :success }
it { is_expected.to be_falsey }
end
context 'when pipeline is running' do
let(:status) { :running }
it { is_expected.to be_falsey }
end
end
end
describe '#cancel_pipeline!' do
subject { merge_train.cancel_pipeline!(new_pipeline) }
let(:merge_train) { merge_request.merge_train }
let!(:merge_request) { create_merge_request_on_train }
let!(:pipeline) { create(:ci_pipeline, project: merge_train.project) }
let!(:new_pipeline) { create(:ci_pipeline, project: merge_train.project) }
before do
merge_train.update!(pipeline: pipeline)
end
it 'cancels the existing pipeline' do
expect(pipeline).to receive(:cancel_running).and_call_original
subject
expect(pipeline.reload.auto_canceled_by).to eq(new_pipeline)
end
end
describe '#mergeable?' do
subject { merge_train.mergeable? }
let(:merge_train) { merge_request.merge_train }
let!(:merge_request) { create_merge_request_on_train }
context 'when merge train has successful pipeline' do
before do
merge_train.update!(pipeline: create(:ci_pipeline, :success, project: merge_request.project))
end
context 'when merge request is first on train' do
it { is_expected.to be_truthy }
end
context 'when the other merge request is on the merge train' do
let(:merge_train) { merge_request_2.merge_train }
let!(:merge_request_2) { create_merge_request_on_train(source_branch: 'improve/awesome') }
it { is_expected.to be_falsy }
end
end
context 'when merge train has non successful pipeline' do
before do
merge_train.update!(pipeline: create(:ci_pipeline, :failed, project: merge_request.project))
end
context 'when merge request is first on train' do
it { is_expected.to be_falsey }
end
end
end
describe '#index' do describe '#index' do
subject { merge_train.index } subject { merge_train.index }
......
...@@ -138,7 +138,7 @@ RSpec.describe MergeTrains::RefreshMergeRequestService do ...@@ -138,7 +138,7 @@ RSpec.describe MergeTrains::RefreshMergeRequestService do
let(:previous_ref) { 'refs/tmp/test' } let(:previous_ref) { 'refs/tmp/test' }
before do before do
allow(service).to receive(:previous_ref) { previous_ref } allow(merge_request.merge_train).to receive(:previous_ref) { previous_ref }
end end
it_behaves_like 'drops the merge request from the merge train' do it_behaves_like 'drops the merge request from the merge train' do
......
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