diff --git a/plugins/chat/app/controllers/api_controller.rb b/plugins/chat/app/controllers/api_controller.rb index 39f20e7c11a..70bf35dc60c 100644 --- a/plugins/chat/app/controllers/api_controller.rb +++ b/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 diff --git a/plugins/chat/app/helpers/with_service_helper.rb b/plugins/chat/app/helpers/with_service_helper.rb index 64e39787325..78b4d923ba7 100644 --- a/plugins/chat/app/helpers/with_service_helper.rb +++ b/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 diff --git a/plugins/chat/app/jobs/service_job.rb b/plugins/chat/app/jobs/service_job.rb new file mode 100644 index 00000000000..e2af50f8641 --- /dev/null +++ b/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 diff --git a/plugins/chat/lib/endpoint.rb b/plugins/chat/lib/service_runner.rb similarity index 78% rename from plugins/chat/lib/endpoint.rb rename to plugins/chat/lib/service_runner.rb index bddb987ef24..21084cedd42 100644 --- a/plugins/chat/lib/endpoint.rb +++ b/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 diff --git a/plugins/chat/plugin.rb b/plugins/chat/plugin.rb index b68743b24df..6a1a618943c 100644 --- a/plugins/chat/plugin.rb +++ b/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__) diff --git a/plugins/chat/spec/lib/endpoint_spec.rb b/plugins/chat/spec/lib/service_runner_spec.rb similarity index 80% rename from plugins/chat/spec/lib/endpoint_spec.rb rename to plugins/chat/spec/lib/service_runner_spec.rb index 26a00661d8d..8c601de838f 100644 --- a/plugins/chat/spec/lib/endpoint_spec.rb +++ b/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