From c0db4400f4d838e496090a6cd52a65bf2cd2051c Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 14 Feb 2017 10:38:17 +0100 Subject: Preserve base service abstraction for retry services --- app/models/ci/build.rb | 4 +- app/services/ci/retry_build_service.rb | 61 ++++++++++++------------- app/services/ci/retry_pipeline_service.rb | 30 ++++++------ spec/services/ci/retry_build_service_spec.rb | 16 +++---- spec/services/ci/retry_pipeline_service_spec.rb | 12 ++--- 5 files changed, 62 insertions(+), 61 deletions(-) diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 0b4b7b33ecd..e018f8e7c4e 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -63,7 +63,9 @@ module Ci end def retry(build, current_user) - Ci::RetryBuildService.new(build, current_user).retry! + Ci::RetryBuildService + .new(build.project, current_user) + .execute(build) end end diff --git a/app/services/ci/retry_build_service.rb b/app/services/ci/retry_build_service.rb index b7a9a55dd9e..e470bd6ee6a 100644 --- a/app/services/ci/retry_build_service.rb +++ b/app/services/ci/retry_build_service.rb @@ -1,47 +1,44 @@ module Ci - class RetryBuildService - include Gitlab::Allowable + class RetryBuildService < ::BaseService + def execute(build) + self.retry(build).tap do |new_build| + MergeRequests::AddTodoWhenBuildFailsService + .new(build.project, current_user) + .close(new_build) - def initialize(build, user) - @build = build - @user = user - @pipeline = build.pipeline + build.pipeline + .mark_as_processable_after_stage(build.stage_idx) + end end - def retry! - reprocess!.tap do |new_build| + def retry(build) + self.reprocess(build).tap do |new_build| new_build.enqueue! - - MergeRequests::AddTodoWhenBuildFailsService - .new(@build.project, @user) - .close(new_build) - - @pipeline.mark_as_processable_after_stage(@build.stage_idx) end end - def reprocess! - unless can?(@user, :update_build, @build) + def reprocess(build) + unless can?(current_user, :update_build, build) raise Gitlab::Access::AccessDeniedError end Ci::Build.create( - ref: @build.ref, - tag: @build.tag, - options: @build.options, - commands: @build.commands, - tag_list: @build.tag_list, - project: @build.project, - pipeline: @build.pipeline, - name: @build.name, - allow_failure: @build.allow_failure, - stage: @build.stage, - stage_idx: @build.stage_idx, - trigger_request: @build.trigger_request, - yaml_variables: @build.yaml_variables, - when: @build.when, - environment: @build.environment, - user: @user) + ref: build.ref, + tag: build.tag, + options: build.options, + commands: build.commands, + tag_list: build.tag_list, + project: build.project, + pipeline: build.pipeline, + name: build.name, + allow_failure: build.allow_failure, + stage: build.stage, + stage_idx: build.stage_idx, + trigger_request: build.trigger_request, + yaml_variables: build.yaml_variables, + when: build.when, + environment: build.environment, + user: current_user) end end end diff --git a/app/services/ci/retry_pipeline_service.rb b/app/services/ci/retry_pipeline_service.rb index 801eaa59cd6..fd180b2c624 100644 --- a/app/services/ci/retry_pipeline_service.rb +++ b/app/services/ci/retry_pipeline_service.rb @@ -1,34 +1,36 @@ module Ci - class RetryPipelineService - include Gitlab::Allowable - - def initialize(pipeline, user) + class RetryPipelineService < ::BaseService + def execute(pipeline) @pipeline = pipeline - @user = user - end - def execute - unless can?(@user, :update_pipeline, @pipeline) + unless can?(current_user, :update_pipeline, pipeline) raise Gitlab::Access::AccessDeniedError end ## - # Reprocess builds in subsequent stages if any - # - # TODO, refactor. + # Reprocess builds in subsequent stages # - @pipeline.builds + pipeline.builds .where('stage_idx > ?', resume_stage.index) .failed_or_canceled.find_each do |build| - Ci::RetryBuildService.new(build, @user).reprocess! + Ci::RetryBuildService + .new(project, current_user) + .reprocess(build) end ## # Retry builds in the first unsuccessful stage # resume_stage.builds.failed_or_canceled.find_each do |build| - Ci::Build.retry(build, @user) + Ci::RetryBuildService + .new(project, current_user) + .retry(build) end + + ## + # Mark skipped builds as processable again + # + pipeline.mark_as_processable_after_stage(resume_stage.index) end private diff --git a/spec/services/ci/retry_build_service_spec.rb b/spec/services/ci/retry_build_service_spec.rb index 0815469f2cb..9cd1da59c54 100644 --- a/spec/services/ci/retry_build_service_spec.rb +++ b/spec/services/ci/retry_build_service_spec.rb @@ -7,13 +7,13 @@ describe Ci::RetryBuildService, :services do let(:build) { create(:ci_build, pipeline: pipeline) } let(:service) do - described_class.new(build, user) + described_class.new(project, user) end - describe '#retry!' do - let(:new_build) { service.retry! } + describe '#execute' do + let(:new_build) { service.execute(build) } - context 'when user has ability to retry build' do + context 'when user has ability to execute build' do before do project.team << [user, :developer] end @@ -30,7 +30,7 @@ describe Ci::RetryBuildService, :services do expect(MergeRequests::AddTodoWhenBuildFailsService) .to receive_message_chain(:new, :close) - service.retry! + service.execute(build) end context 'when there are subsequent builds that are skipped' do @@ -39,16 +39,16 @@ describe Ci::RetryBuildService, :services do end it 'resumes pipeline processing in subsequent stages' do - service.retry! + service.execute(build) expect(subsequent_build.reload).to be_created end end end - context 'when user does not have ability to retry build' do + context 'when user does not have ability to execute build' do it 'raises an error' do - expect { service.retry! } + expect { service.execute(build) } .to raise_error Gitlab::Access::AccessDeniedError end end diff --git a/spec/services/ci/retry_pipeline_service_spec.rb b/spec/services/ci/retry_pipeline_service_spec.rb index aeb3ee228ad..d7afca538aa 100644 --- a/spec/services/ci/retry_pipeline_service_spec.rb +++ b/spec/services/ci/retry_pipeline_service_spec.rb @@ -4,7 +4,7 @@ describe Ci::RetryPipelineService, '#execute', :services do let(:user) { create(:user) } let(:project) { create(:empty_project) } let(:pipeline) { create(:ci_pipeline, project: project) } - let(:service) { described_class.new(pipeline, user) } + let(:service) { described_class.new(project, user) } context 'when user has ability to modify pipeline' do let(:user) { create(:admin) } @@ -17,7 +17,7 @@ describe Ci::RetryPipelineService, '#execute', :services do end it 'enqueues all builds in the last stage' do - service.execute + service.execute(pipeline) expect(build('rspec 2')).to be_pending expect(build('rspec 3')).to be_pending @@ -34,7 +34,7 @@ describe Ci::RetryPipelineService, '#execute', :services do end it 'retries builds failed builds and marks subsequent for processing' do - service.execute + service.execute(pipeline) expect(build('rspec 1')).to be_pending expect(build('rspec 2')).to be_pending @@ -53,7 +53,7 @@ describe Ci::RetryPipelineService, '#execute', :services do end it 'retries builds failed builds and marks subsequent for processing' do - service.execute + service.execute(pipeline) expect(build('rspec 1')).to be_pending expect(build('rspec 2')).to be_pending @@ -63,7 +63,7 @@ describe Ci::RetryPipelineService, '#execute', :services do end it 'creates a new job for report job in this case' do - service.execute + service.execute(pipeline) expect(statuses.where(name: 'report 1').count).to eq 2 end @@ -72,7 +72,7 @@ describe Ci::RetryPipelineService, '#execute', :services do context 'when user is not allowed to retry pipeline' do it 'raises an error' do - expect { service.execute } + expect { service.execute(pipeline) } .to raise_error Gitlab::Access::AccessDeniedError end end -- cgit v1.2.1