From eeeb96c9d0cab9c5da850809a9614e2a01fdb7d2 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Wed, 28 Sep 2016 17:22:06 +0800
Subject: [PATCH] Change service to be a worker, feedback:

https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/6342#note_16118195
---
 app/models/ci/pipeline.rb                           |  4 ++--
 .../project_services/pipelines_email_service.rb     |  4 ++--
 .../ci/send_pipeline_notification_service.rb        | 13 -------------
 app/workers/send_pipeline_notification_worker.rb    |  9 +++++++++
 spec/models/ci/pipeline_spec.rb                     |  6 ++++--
 .../send_pipeline_notification_worker_spec.rb}      |  4 ++--
 6 files changed, 19 insertions(+), 21 deletions(-)
 delete mode 100644 app/services/ci/send_pipeline_notification_service.rb
 create mode 100644 app/workers/send_pipeline_notification_worker.rb
 rename spec/{services/ci/send_pipeline_notification_service_spec.rb => workers/send_pipeline_notification_worker_spec.rb} (96%)

diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index 61bb89a0a2..d534fdc056 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -83,8 +83,8 @@ module Ci
         end
       end
 
-      after_transition any => [:success, :failed] do |pipeline, transition|
-        SendPipelineNotificationService.new(pipeline).execute
+      after_transition any => [:success, :failed] do |pipeline|
+        SendPipelineNotificationWorker.perform_async(pipeline.id)
       end
     end
 
diff --git a/app/models/project_services/pipelines_email_service.rb b/app/models/project_services/pipelines_email_service.rb
index ec27aa88f1..4cebb9e7f9 100644
--- a/app/models/project_services/pipelines_email_service.rb
+++ b/app/models/project_services/pipelines_email_service.rb
@@ -31,8 +31,8 @@ class PipelinesEmailService < Service
 
     return unless all_recipients.any?
 
-    pipeline = Ci::Pipeline.find(data[:object_attributes][:id])
-    Ci::SendPipelineNotificationService.new(pipeline).execute(all_recipients)
+    pipeline_id = data[:object_attributes][:id]
+    SendPipelineNotificationWorker.perform_async(pipeline_id, all_recipients)
   end
 
   def can_test?
diff --git a/app/services/ci/send_pipeline_notification_service.rb b/app/services/ci/send_pipeline_notification_service.rb
deleted file mode 100644
index d330fa1a73..0000000000
--- a/app/services/ci/send_pipeline_notification_service.rb
+++ /dev/null
@@ -1,13 +0,0 @@
-module Ci
-  class SendPipelineNotificationService < BaseService
-    attr_reader :pipeline
-
-    def initialize(new_pipeline)
-      @pipeline = new_pipeline
-    end
-
-    def execute(recipients = nil)
-      notification_service.pipeline_finished(pipeline, recipients)
-    end
-  end
-end
diff --git a/app/workers/send_pipeline_notification_worker.rb b/app/workers/send_pipeline_notification_worker.rb
new file mode 100644
index 0000000000..d4837d815a
--- /dev/null
+++ b/app/workers/send_pipeline_notification_worker.rb
@@ -0,0 +1,9 @@
+class SendPipelineNotificationWorker
+  include Sidekiq::Worker
+
+  def perform(pipeline_id, recipients = nil)
+    pipeline = Ci::Pipeline.find(pipeline_id)
+
+    NotificationService.new.pipeline_finished(pipeline, recipients)
+  end
+end
diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb
index 787042cf05..6ea712ae1b 100644
--- a/spec/models/ci/pipeline_spec.rb
+++ b/spec/models/ci/pipeline_spec.rb
@@ -537,8 +537,10 @@ describe Ci::Pipeline, models: true do
     before do
       reset_delivered_emails!
 
-      pipeline.enqueue
-      pipeline.run
+      perform_enqueued_jobs do
+        pipeline.enqueue
+        pipeline.run
+      end
     end
 
     shared_examples 'sending a notification' do
diff --git a/spec/services/ci/send_pipeline_notification_service_spec.rb b/spec/workers/send_pipeline_notification_worker_spec.rb
similarity index 96%
rename from spec/services/ci/send_pipeline_notification_service_spec.rb
rename to spec/workers/send_pipeline_notification_worker_spec.rb
index 110e16410c..0670d67501 100644
--- a/spec/services/ci/send_pipeline_notification_service_spec.rb
+++ b/spec/workers/send_pipeline_notification_worker_spec.rb
@@ -1,6 +1,6 @@
 require 'spec_helper'
 
-describe Ci::SendPipelineNotificationService, services: true do
+describe SendPipelineNotificationWorker, services: true do
   let(:pipeline) do
     create(:ci_pipeline,
            project: project,
@@ -23,7 +23,7 @@ describe Ci::SendPipelineNotificationService, services: true do
     shared_examples 'sending emails' do
       it 'sends emails' do
         perform_enqueued_jobs do
-          subject.execute
+          subject.perform(pipeline.id)
         end
 
         expected_receivers = [pusher, watcher].uniq.sort_by(&:email)
-- 
2.30.9