Merge branch 'feature/process-pipeline-hooks-asynchronously' into 'master'

app/models/ci/build.rb

 module Ci
   class Build < CommitStatus
     include TokenAuthenticatable
+    include AfterCommitQueue
 
     belongs_to :runner, class_name: 'Ci::Runner'
     belongs_to :trigger_request, class_name: 'Ci::TriggerRequest'
@@ -75,25 +76,20 @@ module Ci
 
     state_machine :status do
       after_transition pending: :running do |build|
-        build.execute_hooks
+        build.run_after_commit do
+          BuildHooksWorker.perform_async(id)
+        end
       end
 
       after_transition any => [:success, :failed, :canceled] do |build|
-        build.update_coverage
-        build.execute_hooks
+        build.run_after_commit do
+          BuildFinishedWorker.perform_async(id)
+        end
       end
 
       after_transition any => [:success] do |build|
-        if build.environment.present?
-          service = CreateDeploymentService.new(
-            build.project, build.user,
-            environment: build.environment,
-            sha: build.sha,
-            ref: build.ref,
-            tag: build.tag,
-            options: build.options.to_h[:environment],
-            variables: build.variables)
-          service.execute(build)
+        build.run_after_commit do
+          BuildSuccessWorker.perform_async(id)
         end
       end
     end

app/models/ci/pipeline.rb

@@ -76,7 +76,11 @@ module Ci
       end
 
       after_transition do |pipeline, transition|
-        pipeline.execute_hooks unless transition.loopback?
+        next if transition.loopback?
+
+        pipeline.run_after_commit do
+          PipelineHooksWorker.perform_async(id)
+        end
       end
     end
 

app/services/create_deployment_service.rb

@@ -2,25 +2,35 @@ require_relative 'base_service'
 
 class CreateDeploymentService < BaseService
   def execute(deployable = nil)
-    environment = find_or_create_environment
+    return unless executable?
 
-    deployment = project.deployments.create(
-      environment: environment,
+    ActiveRecord::Base.transaction do
+      @deployable = deployable
+      @environment = prepare_environment
+
+      deploy.tap do |deployment|
+        deployment.update_merge_request_metrics!
+      end
+    end
+  end
+
+  private
+
+  def executable?
+    project && name.present?
+  end
+
+  def deploy
+    project.deployments.create(
+      environment: @environment,
       ref: params[:ref],
       tag: params[:tag],
       sha: params[:sha],
       user: current_user,
-      deployable: deployable
-    )
-
-    deployment.update_merge_request_metrics!
-
-    deployment
+      deployable: @deployable)
   end
 
-  private
-
-  def find_or_create_environment
+  def prepare_environment
     project.environments.find_or_create_by(name: expanded_name) do |environment|
       environment.external_url = expanded_url
     end

app/workers/build_coverage_worker.rb

+class BuildCoverageWorker
+  include Sidekiq::Worker
+  sidekiq_options queue: :default
+
+  def perform(build_id)
+    Ci::Build.find_by(id: build_id)
+      .try(:update_coverage)
+  end
+end

app/workers/build_finished_worker.rb

+class BuildFinishedWorker
+  include Sidekiq::Worker
+
+  def perform(build_id)
+    Ci::Build.find_by(id: build_id).try do |build|
+      BuildCoverageWorker.new.perform(build.id)
+      BuildHooksWorker.new.perform(build.id)
+    end
+  end
+end

app/workers/build_hooks_worker.rb

+class BuildHooksWorker
+  include Sidekiq::Worker
+  sidekiq_options queue: :default
+
+  def perform(build_id)
+    Ci::Build.find_by(id: build_id)
+      .try(:execute_hooks)
+  end
+end

app/workers/build_success_worker.rb

+class BuildSuccessWorker
+  include Sidekiq::Worker
+  sidekiq_options queue: :default
+
+  def perform(build_id)
+    Ci::Build.find_by(id: build_id).try do |build|
+      create_deployment(build)
+    end
+  end
+
+  private
+
+  def create_deployment(build)
+    return if build.environment.blank?
+
+    service = CreateDeploymentService.new(
+      build.project, build.user,
+      environment: build.environment,
+      sha: build.sha,
+      ref: build.ref,
+      tag: build.tag,
+      options: build.options.to_h[:environment],
+      variables: build.variables)
+
+    service.execute(build)
+  end
+end

app/workers/pipeline_hooks_worker.rb

+class PipelineHooksWorker
+  include Sidekiq::Worker
+  sidekiq_options queue: :default
+
+  def perform(pipeline_id)
+    Ci::Pipeline.find_by(id: pipeline_id)
+      .try(:execute_hooks)
+  end
+end

spec/services/create_deployment_service_spec.rb

@@ -84,11 +84,22 @@ describe CreateDeploymentService, services: true do
         expect(subject).to be_persisted
       end
     end
+
+    context 'when project was removed' do
+      let(:project) { nil }
+
+      it 'does not create deployment or environment' do
+        expect { subject }.not_to raise_error
+
+        expect(Environment.count).to be_zero
+        expect(Deployment.count).to be_zero
+      end
+    end
   end
 
   describe 'processing of builds' do
     let(:environment) { nil }
-    
+
     shared_examples 'does not create environment and deployment' do
       it 'does not create a new environment' do
         expect { subject }.not_to change { Environment.count }
@@ -133,12 +144,12 @@ describe CreateDeploymentService, services: true do
 
     context 'without environment specified' do
       let(:build) { create(:ci_build, project: project) }
-      
+
       it_behaves_like 'does not create environment and deployment' do
         subject { build.success }
       end
     end
-    
+
     context 'when environment is specified' do
       let(:pipeline) { create(:ci_pipeline, project: project) }
       let(:build) { create(:ci_build, pipeline: pipeline, environment: 'production', options: options) }

spec/workers/build_coverage_worker_spec.rb

+require 'spec_helper'
+
+describe BuildCoverageWorker do
+  describe '#perform' do
+    context 'when build exists' do
+      let!(:build) { create(:ci_build) }
+
+      it 'updates code coverage' do
+        expect_any_instance_of(Ci::Build)
+          .to receive(:update_coverage)
+
+        described_class.new.perform(build.id)
+      end
+    end
+
+    context 'when build does not exist' do
+      it 'does not raise exception' do
+        expect { described_class.new.perform(123) }
+          .not_to raise_error
+      end
+    end
+  end
+end

spec/workers/build_finished_worker_spec.rb

+require 'spec_helper'
+
+describe BuildFinishedWorker do
+  describe '#perform' do
+    context 'when build exists' do
+      let(:build) { create(:ci_build) }
+
+      it 'calculates coverage and calls hooks' do
+        expect(BuildCoverageWorker)
+          .to receive(:new).ordered.and_call_original
+        expect(BuildHooksWorker)
+          .to receive(:new).ordered.and_call_original
+
+        expect_any_instance_of(BuildCoverageWorker)
+          .to receive(:perform)
+        expect_any_instance_of(BuildHooksWorker)
+          .to receive(:perform)
+
+        described_class.new.perform(build.id)
+      end
+    end
+
+    context 'when build does not exist' do
+      it 'does not raise exception' do
+        expect { described_class.new.perform(123) }
+          .not_to raise_error
+      end
+    end
+  end
+end

spec/workers/build_hooks_worker_spec.rb

+require 'spec_helper'
+
+describe BuildHooksWorker do
+  describe '#perform' do
+    context 'when build exists' do
+      let!(:build) { create(:ci_build) }
+
+      it 'calls build hooks' do
+        expect_any_instance_of(Ci::Build)
+          .to receive(:execute_hooks)
+
+        described_class.new.perform(build.id)
+      end
+    end
+
+    context 'when build does not exist' do
+      it 'does not raise exception' do
+        expect { described_class.new.perform(123) }
+          .not_to raise_error
+      end
+    end
+  end
+end

spec/workers/build_success_worker_spec.rb

+require 'spec_helper'
+
+describe BuildSuccessWorker do
+  describe '#perform' do
+    context 'when build exists' do
+      context 'when build belogs to the environment' do
+        let!(:build) { create(:ci_build, environment: 'production') }
+
+        it 'executes deployment service' do
+          expect_any_instance_of(CreateDeploymentService)
+            .to receive(:execute)
+
+          described_class.new.perform(build.id)
+        end
+      end
+
+      context 'when build is not associated with project' do
+        let!(:build) { create(:ci_build, project: nil) }
+
+        it 'does not create deployment' do
+          expect_any_instance_of(CreateDeploymentService)
+            .not_to receive(:execute)
+
+          described_class.new.perform(build.id)
+        end
+      end
+    end
+
+    context 'when build does not exist' do
+      it 'does not raise exception' do
+        expect { described_class.new.perform(123) }
+          .not_to raise_error
+      end
+    end
+  end
+end

spec/workers/pipeline_hooks_worker_spec.rb

+require 'spec_helper'
+
+describe PipelineHooksWorker do
+  describe '#perform' do
+    context 'when pipeline exists' do
+      let(:pipeline) { create(:ci_pipeline) }
+
+      it 'executes hooks for the pipeline' do
+        expect_any_instance_of(Ci::Pipeline)
+          .to receive(:execute_hooks)
+
+        described_class.new.perform(pipeline.id)
+      end
+    end
+
+    context 'when pipeline does not exist' do
+      it 'does not raise exception' do
+        expect { described_class.new.perform(123) }
+          .not_to raise_error
+      end
+    end
+  end
+end