diff --git a/.rubocop.yml b/.rubocop.yml index 26c7c27..bc946a5 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -32,3 +32,6 @@ FactoryBot/StaticAttributeDefinedDynamically: RSpec/ExpectChange: EnforcedStyle: block + +Style/RescueStandardError: + EnforcedStyle: implicit diff --git a/app/controllers/logux_controller.rb b/app/controllers/logux_controller.rb index ac68b55..dc9364a 100644 --- a/app/controllers/logux_controller.rb +++ b/app/controllers/logux_controller.rb @@ -3,7 +3,6 @@ class LoguxController < ActionController::Base include ActionController::Live - # rubocop:disable Style/RescueStandardError def create Logux.verify_request_meta_data(meta_params) logux_stream.write('[') @@ -13,13 +12,12 @@ def create Logux.configuration.on_error.call(e) Logux.logger.error("#{e}\n#{e.backtrace.join("\n")}") ensure - logux_stream.write([:error].to_json) + logux_stream.write(Logux::ErrorRenderer.new(e).message) end ensure logux_stream.write(']') logux_stream.close end - # rubocop:enable Style/RescueStandardError private diff --git a/lib/logux.rb b/lib/logux.rb index cae414b..3d26cf4 100644 --- a/lib/logux.rb +++ b/lib/logux.rb @@ -13,8 +13,18 @@ module Logux extend ActiveSupport::Autoload include Configurations - class NoPolicyError < StandardError; end - class NoActionError < StandardError; end + class WithMetaError < StandardError + attr_reader :meta + + def initialize(msg, meta: nil) + @meta = meta + super(msg) + end + end + + UnknownActionError = Class.new(WithMetaError) + UnknownChannelError = Class.new(WithMetaError) + UnauthorizedError = Class.new(StandardError) autoload :Client, 'logux/client' autoload :Meta, 'logux/meta' @@ -35,6 +45,7 @@ class NoActionError < StandardError; end autoload :Logger, 'logux/logger' autoload :Version, 'logux/version' autoload :Test, 'logux/test' + autoload :ErrorRenderer, 'logux/error_renderer' configurable :logux_host, :verify_authorized, :password, :logger, @@ -64,13 +75,13 @@ def self.undo(meta: Logux::Meta.new({}), reason: nil) def self.verify_request_meta_data(meta_params) if Logux.configuration.password.nil? - logger.warn(%(Please, add passoword for logux server: + logger.warn(%(Please, add password for logux server: Logux.configure do |c| c.password = 'your-password' end)) end auth = Logux.configuration.password == meta_params&.dig(:password) - raise unless auth + raise Logux::UnauthorizedError, 'Incorrect password' unless auth end def self.process_batch(stream:, batch:) diff --git a/lib/logux/action_caller.rb b/lib/logux/action_caller.rb index 3cd77d5..4781844 100644 --- a/lib/logux/action_caller.rb +++ b/lib/logux/action_caller.rb @@ -15,6 +15,9 @@ def call! action_class = class_finder.find_action_class @action_controller = action_class.new(action: action, meta: meta) format(action_controller.public_send(action.action_type)) + rescue Logux::UnknownActionError, Logux::UnknownChannelError => e + Logux.logger.warn(e) + format(nil) end private @@ -26,7 +29,7 @@ def format(response) end def class_finder - @class_finder ||= Logux::ClassFinder.new(action) + @class_finder ||= Logux::ClassFinder.new(action: action, meta: meta) end end end diff --git a/lib/logux/class_finder.rb b/lib/logux/class_finder.rb index 6b0feda..30012ed 100644 --- a/lib/logux/class_finder.rb +++ b/lib/logux/class_finder.rb @@ -2,41 +2,56 @@ module Logux class ClassFinder - attr_reader :params + attr_reader :action, :meta - def initialize(params) - @params = params + def initialize(action:, meta:) + @action = action + @meta = meta end def find_action_class "#{class_namespace}::#{class_name}".constantize rescue NameError - raise Logux::NoActionError, %( + message = %( Unable to find action #{class_name.camelize} - Should be in app/logux/actions/#{class_path}.rb + Should be in app/logux/#{class_namespace.downcase}/#{class_path}.rb ) + raise Logux::UnknownActionError.new(message, meta: meta) if action? + raise Logux::UnknownChannelError.new(message, meta: meta) end def find_policy_class "Policies::#{class_namespace}::#{class_name}".constantize rescue NameError - raise Logux::NoPolicyError, %( - Unable to find policy #{class_name.camelize} - Should be in app/logux/policies/#{class_path}.rb + message = %( + Unable to find action policy #{class_name.camelize} + Should be in app/logux/policies/#{class_namespace.downcase}/#{class_path}.rb ) + raise Logux::UnknownActionError.new(message, meta: meta) if action? + raise Logux::UnknownChannelError.new(message, meta: meta) end + def class_name + if subscribe? + action.channel_name.camelize + else + action.type.split('/')[0..-2].map(&:camelize).join('::') + end + end + + private + def class_namespace - return 'Channels' if params.type == 'logux/subscribe' + return 'Channels' if subscribe? 'Actions' end - def class_name - if params.type == 'logux/subscribe' - params.channel_name.camelize - else - params.type.split('/')[0..-2].map(&:camelize).join('::') - end + def subscribe? + action.type == 'logux/subscribe' + end + + def action? + !subscribe? end def class_path diff --git a/lib/logux/error_renderer.rb b/lib/logux/error_renderer.rb new file mode 100644 index 0000000..fcecf4f --- /dev/null +++ b/lib/logux/error_renderer.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +module Logux + class ErrorRenderer + attr_reader :exception + + def initialize(exception) + @exception = exception + end + + def message + case exception + when Logux::WithMetaError + build_message(exception, exception.meta.id) + when Logux::UnauthorizedError + build_message(exception, exception.message) + when StandardError + # some runtime error that should be fixed + ['error', 'Please look server logs for more information'] + end + end + + private + + def build_message(exception, additional_info) + [ + exception.class.name.demodulize.camelize(:lower).gsub(/Error/, ''), + additional_info + ] + end + end +end diff --git a/lib/logux/policy_caller.rb b/lib/logux/policy_caller.rb index f363fc0..a577fca 100644 --- a/lib/logux/policy_caller.rb +++ b/lib/logux/policy_caller.rb @@ -15,13 +15,13 @@ def call! policy_class = class_finder.find_policy_class @policy = policy_class.new(action: action, meta: meta) policy.public_send("#{action.action_type}?") - rescue Logux::NoPolicyError => e + rescue Logux::UnknownActionError, Logux::UnknownChannelError => e raise e if Logux.configuration.verify_authorized Logux.logger.warn(e) end def class_finder - @class_finder ||= Logux::ClassFinder.new(action) + @class_finder ||= Logux::ClassFinder.new(action: action, meta: meta) end end end diff --git a/spec/dummy/app/logux/policies/actions/policy_without_action.rb b/spec/dummy/app/logux/policies/actions/policy_without_action.rb new file mode 100644 index 0000000..94fba45 --- /dev/null +++ b/spec/dummy/app/logux/policies/actions/policy_without_action.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +module Policies + module Actions + class PolicyWithoutAction < Logux::Policy + def create? + true + end + end + end +end diff --git a/spec/dummy/app/logux/policies/channels/policy_without_channel.rb b/spec/dummy/app/logux/policies/channels/policy_without_channel.rb new file mode 100644 index 0000000..d7b9f5d --- /dev/null +++ b/spec/dummy/app/logux/policies/channels/policy_without_channel.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +module Policies + module Channels + class PolicyWithoutChannel < Logux::Policy + def subscribe? + true + end + end + end +end diff --git a/spec/factories/logux_actions_factory.rb b/spec/factories/logux_actions_factory.rb index d11c0d5..c01244a 100644 --- a/spec/factories/logux_actions_factory.rb +++ b/spec/factories/logux_actions_factory.rb @@ -18,5 +18,14 @@ key { 'name' } value { 'test1' } end + + factory :logux_actions_unknown do + type 'unknown/action' + end + + factory :logux_actions_unknown_subscribe do + type 'logux/subscribe' + channel 'unknown/channel' + end end end diff --git a/spec/logux/action_caller_spec.rb b/spec/logux/action_caller_spec.rb index 8757256..dc68bc5 100644 --- a/spec/logux/action_caller_spec.rb +++ b/spec/logux/action_caller_spec.rb @@ -8,10 +8,6 @@ let(:meta) { create(:logux_meta) } describe '#call!' do - it 'raise error' do - expect { action_caller.call! }.to raise_error(Logux::NoActionError) - end - context 'when action defined' do subject(:result) { action_caller.call! } diff --git a/spec/logux/class_finder_spec.rb b/spec/logux/class_finder_spec.rb index fc0f919..2ee996a 100644 --- a/spec/logux/class_finder_spec.rb +++ b/spec/logux/class_finder_spec.rb @@ -3,12 +3,13 @@ require 'spec_helper' describe Logux::ClassFinder do - let(:finder) { described_class.new(params) } + let(:finder) { described_class.new(action: action, meta: meta) } + let(:meta) { create(:logux_meta) } describe '#class_name' do subject(:class_name) { finder.class_name } - let(:params) { create(:logux_actions_add, type: 'test/test/name/try/user/add') } + let(:action) { create(:logux_actions_add, type: 'test/test/name/try/user/add') } it 'returns nested classes' do expect(class_name).to eq('Test::Test::Name::Try::User') @@ -18,10 +19,40 @@ describe '#find_policy_class' do subject(:policy_class) { finder.find_policy_class } - let(:params) { create(:logux_actions_add, type: 'test/test/name/try/user/add') } + context 'with unknown action' do + let(:action) { create(:logux_actions_unknown) } - it 'raise an error' do - expect { policy_class }.to raise_error(Logux::NoPolicyError) + it 'raise an error for unknown action error' do + expect { policy_class }.to raise_error(Logux::UnknownActionError) + end + end + + context 'with unknown subscribe' do + let(:action) { create(:logux_actions_unknown_subscribe) } + + it 'raise an error for unknown action error' do + expect { policy_class }.to raise_error(Logux::UnknownChannelError) + end + end + end + + describe '#find_action_class' do + subject(:action_class) { finder.find_action_class } + + context 'with unknown action' do + let(:action) { create(:logux_actions_unknown) } + + it 'raise an error for unknown action error' do + expect { action_class }.to raise_error(Logux::UnknownActionError) + end + end + + context 'with unknown subscribe' do + let(:action) { create(:logux_actions_unknown_subscribe) } + + it 'raise an error for unknown action error' do + expect { action_class }.to raise_error(Logux::UnknownChannelError) + end end end end diff --git a/spec/logux/error_renderer_spec.rb b/spec/logux/error_renderer_spec.rb new file mode 100644 index 0000000..311d315 --- /dev/null +++ b/spec/logux/error_renderer_spec.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Logux::ErrorRenderer do + let(:meta) { create(:logux_meta, id: 123) } + + describe '#message' do + def build_message(exception) + described_class.new(exception).message + end + + it 'returns correct error message for UnknownActionError' do + exception = Logux::UnknownActionError.new('test', meta: meta) + + expect(build_message(exception)).to eq(['unknownAction', 123]) + end + + it 'returns correct error message for UnknownChannelError' do + exception = Logux::UnknownChannelError.new('test', meta: meta) + + expect(build_message(exception)).to eq(['unknownChannel', 123]) + end + + it 'returns correct error message for UnauthorizedError' do + exception = Logux::UnauthorizedError.new('test') + + expect(build_message(exception)).to eq(%w[unauthorized test]) + end + + it 'returns correct error message for some unknown error' do + exception = StandardError.new + + expect(build_message(exception)).to eq( + ['error', 'Please look server logs for more information'] + ) + end + end +end diff --git a/spec/logux/policy_caller_spec.rb b/spec/logux/policy_caller_spec.rb index a6b1450..75e63d3 100644 --- a/spec/logux/policy_caller_spec.rb +++ b/spec/logux/policy_caller_spec.rb @@ -3,34 +3,46 @@ require 'spec_helper' describe Logux::PolicyCaller do + subject(:call!) { policy_caller.call! } + let(:policy_caller) { described_class.new(action: action, meta: meta) } + let(:meta) { {} } - describe '.call!' do - subject(:call!) { policy_caller.call! } + context 'when request is not verified' do + let(:action) { create(:logux_actions_unknown) } - let(:action) { Logux::Actions.new(type: 'test/test') } - let(:meta) { {} } + before do + Logux.configuration.verify_authorized = false + allow(Logux.logger).to receive(:warn) + call! + Logux.configuration.verify_authorized = true + end - context 'when request is not verified' do - before do - allow(Logux.logger).to receive(:warn) - call! - end + it 'doesn\'t raise an error' do + expect(Logux.logger).to have_received(:warn).once + end + end - it 'doesn\'t raise an error' do - expect(Logux.logger).to have_received(:warn).once - end + context 'when verify_authorized' do + around do |example| + Logux.configuration.verify_authorized = true + example.call + Logux.configuration.verify_authorized = false end - context 'when verify_authorized' do - around do |example| - Logux.configuration.verify_authorized = true - example.call - Logux.configuration.verify_authorized = true + context 'with unknown action' do + let(:action) { create(:logux_actions_unknown) } + + it 'raises an unknownActionError' do + expect { call! }.to raise_error(Logux::UnknownActionError) end + end + + context 'with unknown subscribe' do + let(:action) { create(:logux_actions_unknown_subscribe) } - it 'raises an error' do - expect { call! }.to raise_error(Logux::NoPolicyError) + it 'raises an unknownActionError' do + expect { call! }.to raise_error(Logux::UnknownChannelError) end end end diff --git a/spec/requests/request_from_logux_server_spec.rb b/spec/requests/request_from_logux_server_spec.rb index 7736771..a25b461 100644 --- a/spec/requests/request_from_logux_server_spec.rb +++ b/spec/requests/request_from_logux_server_spec.rb @@ -53,7 +53,7 @@ ['forbidden', '219_856_768 clientid 0'] end - it 'does return correct body' do + it 'returns correct body' do expect(response.stream).to have_chunk(logux_response) end end @@ -63,8 +63,10 @@ let(:password) { '12345' } - it 'does return error' do - expect(response.stream).to start_from_chunk([:error]) + it 'returns error' do + expect(response.stream).to start_from_chunk( + ['unauthorized', 'Incorrect password'] + ) end end @@ -87,4 +89,109 @@ expect { request_logux }.to change { logux_store.size }.by(1) end end + + # rubocop: disable RSpec/NestedGroups + # TODO: refactoring; may be move this cases to separated file + context 'with unknown action' do + let(:logux_params) do + { version: 0, + password: password, + commands: [ + ['action', + { type: action, key: 'text', value: 'hi' }, + { time: Time.now.to_i, id: '219_856_768 clientid 0', userId: 1 }] + ] } + end + + context 'when verify_authorized=true' do + before { Logux.configuration.verify_authorized = true } + + context 'when policy not exists' do + let(:action) { 'notexists/create' } + + it 'returns unknownAction' do + request_logux + expect(response.stream).to have_chunk( + ['unknownAction', '219_856_768 clientid 0'] + ) + end + end + + context 'when policy is exists' do + let(:action) { 'policy_without_action/create' } + + it 'returns processed' do + request_logux + expect(response.stream).to have_chunk( + ['processed', '219_856_768 clientid 0'] + ) + end + end + end + + context 'when verify_authorized=false' do + let(:action) { 'notexists/create' } + + before { Logux.configuration.verify_authorized = false } + + it 'returns processed' do + request_logux + expect(response.stream).to have_chunk( + ['processed', '219_856_768 clientid 0'] + ) + end + end + end + + context 'with unknown subscribe' do + let(:logux_params) do + { version: 0, + password: password, + commands: [ + ['action', + { type: 'logux/subscribe', channel: channel }, + { time: Time.now.to_i, id: '219_856_768 clientid 0', userId: 1 }] + ] } + end + + context 'when verify_authorized=true' do + before { Logux.configuration.verify_authorized = true } + + context 'when policy not exists' do + let(:channel) { 'notexists/123' } + + it 'returns unknownChannel' do + request_logux + expect(response.stream).to have_chunk( + ['unknownChannel', '219_856_768 clientid 0'] + ) + end + end + + context 'when policy is exists' do + let(:channel) { 'policy_without_channel/123' } + + it 'returns processed' do + request_logux + expect(response.stream).to have_chunk( + ['processed', '219_856_768 clientid 0'] + ) + end + end + end + + context 'when verify_authorized=false' do + let(:channel) { 'notexists/123' } + + before { Logux.configuration.verify_authorized = false } + + it 'returns processed' do + request_logux + expect(response.stream).to have_chunk( + ['processed', '219_856_768 clientid 0'] + ) + end + end + end + # rubocop: enable RSpec/NestedGroups end