Preserve base service abstraction for retry services

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