DEV: Allow `with_service` in jobs

This patch introduces a new `ServiceJob` class allowing the use of
`with_service` in jobs.

This way, it’s easier to use the chat service objects in jobs and
provides the same level of functionality than the one we have in
controllers.

plugins/chat/app/controllers/api_controller.rb

@@ -12,4 +12,18 @@ class Chat::Api < Chat::ChatBaseController
     raise Discourse::NotFound unless SiteSetting.chat_enabled
     guardian.ensure_can_chat!
   end
+
+  def default_actions_for_service
+    proc do
+      on_success { render(json: success_json) }
+      on_failure { render(json: failed_json, status: 422) }
+      on_failed_policy(:invalid_access) { raise Discourse::InvalidAccess }
+      on_failed_contract do
+        render(
+          json: failed_json.merge(errors: result[:"result.contract.default"].errors.full_messages),
+          status: 400,
+        )
+      end
+    end
+  end
 end

plugins/chat/app/helpers/with_service_helper.rb

@@ -6,32 +6,21 @@ module Chat
     end
 
     def with_service(service, default_actions: true, **dependencies, &block)
-      controller = self
+      object = self
       merged_block =
         proc do
-          instance_eval(&controller.default_actions_for_service) if default_actions
-          instance_eval(&(block || proc {}))
+          instance_exec(&object.method(:default_actions_for_service).call) if default_actions
+          instance_exec(&(block || proc {}))
         end
-      Chat::Endpoint.call(service, controller, **dependencies, &merged_block)
+      Chat::ServiceRunner.call(service, object, **dependencies, &merged_block)
     end
 
     def run_service(service, dependencies)
-      @_result = service.call(params.to_unsafe_h.merge(guardian: guardian, **dependencies.to_h))
+      @_result = service.call(params.to_unsafe_h.merge(guardian: guardian, **dependencies))
     end
 
     def default_actions_for_service
-      proc do
-        on_success { render(json: success_json) }
-        on_failure { render(json: failed_json, status: 422) }
-        on_failed_policy(:invalid_access) { raise Discourse::InvalidAccess }
-        on_failed_contract do
-          render(
-            json:
-              failed_json.merge(errors: result[:"result.contract.default"].errors.full_messages),
-            status: 400,
-          )
-        end
-      end
+      proc {}
     end
   end
 end

plugins/chat/app/jobs/service_job.rb

@@ -0,0 +1,9 @@
+# frozen_string_literal: true
+
+class ServiceJob < ::Jobs::Base
+  include Chat::WithServiceHelper
+
+  def run_service(service, dependencies)
+    @_result = service.call(dependencies)
+  end
+end

plugins/chat/lib/service_runner.rb

@@ -1,12 +1,12 @@
 # frozen_string_literal: true
 #
-# = Chat::Endpoint
+# = Chat::ServiceRunner
 #
-# This class is to be used via its helper +with_service+ in a controller. Its
+# This class is to be used via its helper +with_service+ in any class. Its
 # main purpose is to ease how actions can be run upon a service completion.
 # Since a service will likely return the same kind of things over and over,
 # this allows us to not have to repeat the same boilerplate code in every
-# controller.
+# object.
 #
 # There are several available actions and we can add new ones very easily:
 #
@@ -19,8 +19,7 @@
 # * +on_model_not_found(name)+: will execute the provided block if the service
 #   fails and its model is not present
 #
-# @example
-#   # in a controller
+# @example In a controller
 #   def create
 #     with_service MyService do
 #       on_success do
@@ -32,13 +31,21 @@
 #     end
 #   end
 #
+# @example In a job (inheriting from +ServiceJob+)
+#   def execute(args = {})
+#     with_service(MyService, **args) do
+#       on_success { Rails.logger.info "SUCCESS" }
+#       on_failure { Rails.logger.error "FAILURE" }
+#     end
+#   end
+#
 # The actions will be evaluated in the order they appear. So even if the
 # service will ultimately fail with a failed policy, in this example only the
 # +on_failed_policy+ action will be executed and not the +on_failure+ one.
 # The only exception to this being +on_failure+ as it will always be executed
 # last.
 #
-class Chat::Endpoint
+class Chat::ServiceRunner
   # @!visibility private
   NULL_RESULT = OpenStruct.new(failure?: false)
   # @!visibility private
@@ -51,14 +58,14 @@ class Chat::Endpoint
   }.with_indifferent_access.freeze
 
   # @!visibility private
-  attr_reader :service, :controller, :dependencies
+  attr_reader :service, :object, :dependencies
 
-  delegate :result, to: :controller
+  delegate :result, to: :object
 
   # @!visibility private
-  def initialize(service, controller, **dependencies)
+  def initialize(service, object, **dependencies)
     @service = service
-    @controller = controller
+    @object = object
     @dependencies = dependencies
     @actions = {}
   end
@@ -66,14 +73,14 @@ class Chat::Endpoint
   # @param service [Class] a class including {Chat::Service::Base}
   # @param block [Proc] a block containing the steps to match on
   # @return [void]
-  def self.call(service, controller, **dependencies, &block)
-    new(service, controller, **dependencies).call(&block)
+  def self.call(service, object, **dependencies, &block)
+    new(service, object, **dependencies).call(&block)
   end
 
   # @!visibility private
   def call(&block)
     instance_eval(&block)
-    controller.run_service(service, dependencies)
+    object.run_service(service, dependencies)
     # Always have `on_failure` as the last action
     (
       actions
@@ -88,13 +95,13 @@ class Chat::Endpoint
   attr_reader :actions
 
   def failure_for?(key)
-    (controller.result[key] || NULL_RESULT).failure?
+    (object.result[key] || NULL_RESULT).failure?
   end
 
   def add_action(name, *args, &block)
     actions[[name, *args].join("_").to_sym] = [
       -> { instance_exec(*args, &AVAILABLE_ACTIONS[name]) },
-      -> { controller.instance_eval(&block) },
+      -> { object.instance_eval(&block) },
     ]
   end
 

plugins/chat/plugin.rb

@@ -200,7 +200,7 @@ after_initialize do
   load File.expand_path("../lib/slack_compatibility.rb", __FILE__)
   load File.expand_path("../lib/post_notification_handler.rb", __FILE__)
   load File.expand_path("../lib/secure_uploads_compatibility.rb", __FILE__)
-  load File.expand_path("../lib/endpoint.rb", __FILE__)
+  load File.expand_path("../lib/service_runner.rb", __FILE__)
   load File.expand_path("../lib/steps_inspector.rb", __FILE__)
   load File.expand_path("../app/jobs/regular/auto_manage_channel_memberships.rb", __FILE__)
   load File.expand_path("../app/jobs/regular/auto_join_channel_batch.rb", __FILE__)
@@ -217,6 +217,7 @@ after_initialize do
   load File.expand_path("../app/jobs/scheduled/email_chat_notifications.rb", __FILE__)
   load File.expand_path("../app/jobs/scheduled/auto_join_users.rb", __FILE__)
   load File.expand_path("../app/jobs/scheduled/chat_periodical_updates.rb", __FILE__)
+  load File.expand_path("../app/jobs/service_job.rb", __FILE__)
   load File.expand_path("../app/services/chat_publisher.rb", __FILE__)
   load File.expand_path("../app/services/trash_channel.rb", __FILE__)
   load File.expand_path("../app/services/update_channel.rb", __FILE__)

plugins/chat/spec/lib/service_runner_spec.rb

@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-RSpec.describe Chat::Endpoint do
+RSpec.describe Chat::ServiceRunner do
   class SuccessService
     include Chat::Service::Base
   end
@@ -77,13 +77,13 @@ RSpec.describe Chat::Endpoint do
   end
 
   describe ".call(service, &block)" do
-    subject(:endpoint) { described_class.call(service, controller, &actions_block) }
+    subject(:runner) { described_class.call(service, object, &actions_block) }
 
-    let(:result) { controller.result }
-    let(:actions_block) { controller.instance_eval(actions) }
+    let(:result) { object.result }
+    let(:actions_block) { object.instance_eval(actions) }
     let(:service) { SuccessService }
     let(:actions) { "proc {}" }
-    let(:controller) do
+    let(:object) do
       Class
         .new(Chat::Api) do
           def request
@@ -101,7 +101,7 @@ RSpec.describe Chat::Endpoint do
     end
 
     it "runs the provided service in the context of a controller" do
-      endpoint
+      runner
       expect(result).to be_a Chat::Service::Base::Context
       expect(result).to be_a_success
     end
@@ -115,7 +115,7 @@ RSpec.describe Chat::Endpoint do
 
       context "when the service succeeds" do
         it "runs the provided block" do
-          expect(endpoint).to eq :success
+          expect(runner).to eq :success
         end
       end
 
@@ -123,7 +123,7 @@ RSpec.describe Chat::Endpoint do
         let(:service) { FailureService }
 
         it "does not run the provided block" do
-          expect(endpoint).not_to eq :success
+          expect(runner).not_to eq :success
         end
       end
     end
@@ -139,7 +139,7 @@ RSpec.describe Chat::Endpoint do
         let(:service) { FailureService }
 
         it "runs the provided block" do
-          expect(endpoint).to eq :fail
+          expect(runner).to eq :fail
         end
       end
 
@@ -147,7 +147,7 @@ RSpec.describe Chat::Endpoint do
         let(:service) { SuccessService }
 
         it "does not run the provided block" do
-          expect(endpoint).not_to eq :fail
+          expect(runner).not_to eq :fail
         end
       end
     end
@@ -163,7 +163,7 @@ RSpec.describe Chat::Endpoint do
         let(:service) { FailedPolicyService }
 
         it "runs the provided block" do
-          expect(endpoint).to eq :policy_failure
+          expect(runner).to eq :policy_failure
         end
       end
 
@@ -171,7 +171,7 @@ RSpec.describe Chat::Endpoint do
         let(:service) { SuccessPolicyService }
 
         it "does not run the provided block" do
-          expect(endpoint).not_to eq :policy_failure
+          expect(runner).not_to eq :policy_failure
         end
       end
     end
@@ -187,7 +187,7 @@ RSpec.describe Chat::Endpoint do
         let(:service) { FailedContractService }
 
         it "runs the provided block" do
-          expect(endpoint).to eq :contract_failure
+          expect(runner).to eq :contract_failure
         end
       end
 
@@ -195,7 +195,7 @@ RSpec.describe Chat::Endpoint do
         let(:service) { SuccessContractService }
 
         it "does not run the provided block" do
-          expect(endpoint).not_to eq :contract_failure
+          expect(runner).not_to eq :contract_failure
         end
       end
     end
@@ -211,7 +211,7 @@ RSpec.describe Chat::Endpoint do
         let(:service) { FailureWithModelService }
 
         it "runs the provided block" do
-          expect(endpoint).to eq :no_model
+          expect(runner).to eq :no_model
         end
       end
 
@@ -219,7 +219,7 @@ RSpec.describe Chat::Endpoint do
         let(:service) { SuccessWithModelService }
 
         it "does not run the provided block" do
-          expect(endpoint).not_to eq :no_model
+          expect(runner).not_to eq :no_model
         end
       end
     end
@@ -235,7 +235,21 @@ RSpec.describe Chat::Endpoint do
         BLOCK
 
       it "runs the first matching action" do
-        expect(endpoint).to eq :failure
+        expect(runner).to eq :failure
+      end
+    end
+
+    context "when running in the context of a job" do
+      let(:object) { Class.new(ServiceJob).new }
+      let(:actions) { <<-BLOCK }
+          proc do
+            on_success { :success }
+            on_failure { :failure }
+          end
+        BLOCK
+
+      it "runs properly" do
+        expect(runner).to eq :success
       end
     end
   end