diff --git a/Gemfile b/Gemfile index bfe16f7..d79cc2b 100644 --- a/Gemfile +++ b/Gemfile @@ -5,4 +5,5 @@ gemspec group :test do gem 'rake' + gem "factory_girl", "~> 4.0" end diff --git a/README.md b/README.md index 232f571..e79e900 100644 --- a/README.md +++ b/README.md @@ -16,6 +16,7 @@ gem 'grape-middleware-logger' ## Usage ```ruby class API < Grape::API + # @note Make sure this above you're first +mount+ use Grape::Middleware::Logger end ``` diff --git a/grape-middleware-logger.gemspec b/grape-middleware-logger.gemspec index 462fdc6..fed4ab4 100644 --- a/grape-middleware-logger.gemspec +++ b/grape-middleware-logger.gemspec @@ -16,7 +16,7 @@ Gem::Specification.new do |spec| spec.test_files = spec.files.grep(%r{^(test|spec|features)/}) spec.require_paths = ['lib'] - spec.add_dependency 'grape', '>= 0.12', '< 1' + spec.add_dependency 'grape', '>= 0.14', '< 1' spec.add_development_dependency 'bundler', '~> 1.7' spec.add_development_dependency 'rake', '~> 10.0' diff --git a/lib/grape/middleware/logger.rb b/lib/grape/middleware/logger.rb index 19bfc53..69cdbab 100644 --- a/lib/grape/middleware/logger.rb +++ b/lib/grape/middleware/logger.rb @@ -16,10 +16,11 @@ class Grape::Middleware::Logger < Grape::Middleware::Globals super # sets env['grape.*'] logger.info '' logger.info %Q(Started %s "%s" at %s) % [ - env['grape.request'].request_method, - env['grape.request'].path, + env[Grape::Env::GRAPE_REQUEST].request_method, + env[Grape::Env::GRAPE_REQUEST].path, start_time.to_s ] + logger.info %Q(Processing by #{processed_by}) logger.info %Q( Parameters: #{parameters}) end @@ -73,7 +74,7 @@ class Grape::Middleware::Logger < Grape::Middleware::Globals end def parameters - request_params = env['grape.request.params'].to_hash + request_params = env[Grape::Env::GRAPE_REQUEST_PARAMS].to_hash request_params.merge!(env['action_dispatch.request.request_parameters'] || {}) # for Rails if @options[:filter] @options[:filter].filter(request_params) @@ -86,6 +87,14 @@ class Grape::Middleware::Logger < Grape::Middleware::Globals @start_time ||= Time.now end + def processed_by + endpoint = env[Grape::Env::API_ENDPOINT] + parts = endpoint.options[:for].to_s + parts << endpoint.namespace if endpoint.namespace != '/' + parts << '#' << endpoint.options[:path].map { |path| path.to_s.sub('/', '') }.join('/') + parts + end + def default_logger default = Logger.new(STDOUT) default.formatter = LogFormatter.new diff --git a/spec/factories.rb b/spec/factories.rb new file mode 100644 index 0000000..8b043e1 --- /dev/null +++ b/spec/factories.rb @@ -0,0 +1,90 @@ +FactoryGirl.define do + class ExpectedEnv < Hash + attr_accessor :grape_request, :params, :post_params, :grape_endpoint + end + + class ParamFilter + def filter(opts) + opts.each_pair { |key, val| val[0..-1] = '[FILTERED]' if key == 'password' } + end + end + + class TestAPI + end + + class App + attr_accessor :response + + def call(_env) + response + end + end + + factory :param_filter + + require 'rack/rewindable_input' + + factory :expected_env do + grape_request { build :grape_request } + params { grape_request.params } + post_params { { 'name' => 'foo', 'password' => 'access' } } + grape_endpoint { build(:grape_endpoint) } + + initialize_with do + new.merge( + 'REQUEST_METHOD' => 'POST', + 'PATH_INFO' => '/api/1.0/users', + 'action_dispatch.request.request_parameters' => post_params, + Grape::Env::GRAPE_REQUEST => grape_request, + Grape::Env::GRAPE_REQUEST_PARAMS => params, + Grape::Env::API_ENDPOINT => grape_endpoint + ) + end + end + + factory :grape_endpoint, class: Grape::Endpoint do + settings { Grape::Util::InheritableSetting.new } + options { + { + path: [:users], + method: 'get', + for: TestAPI + } + } + + initialize_with { new(settings, options) } + + trait :complex do + options { + { + path: ['/users/:name/profile'], + method: 'put', + for: TestAPI + } + } + end + end + + factory :namespaced_endpoint, parent: :grape_endpoint do + initialize_with do + new(settings, options).tap do |me| + me.namespace_stackable(:namespace, Grape::Namespace.new('/admin', {})) + end + end + end + + factory :app_response, class: Rack::Response do + initialize_with { new('Hello World', 200, {}) } + end + + factory :grape_request, class: OpenStruct do + initialize_with { + new(request_method: 'POST', path: '/api/1.0/users', headers: {}, params: { 'id' => '101001' }) + } + end + + factory :app do + response { build :app_response } + end + +end diff --git a/spec/integration/lib/grape/middleware/logger_spec.rb b/spec/integration/lib/grape/middleware/logger_spec.rb new file mode 100644 index 0000000..ca70f17 --- /dev/null +++ b/spec/integration/lib/grape/middleware/logger_spec.rb @@ -0,0 +1,52 @@ +require 'spec_helper' +require 'grape/middleware/logger' + +describe Grape::Middleware::Logger, type: :integration do + let(:app) { build :app } + let(:options) { { filter: build(:param_filter), logger: Logger.new(Tempfile.new('logger')) } } + + subject { described_class.new(app, options) } + + let(:app_response) { build :app_response } + let(:grape_request) { build :grape_request } + let(:grape_endpoint) { build(:grape_endpoint) } + let(:env) { build(:expected_env, grape_endpoint: grape_endpoint) } + + it 'logs all parts of the request' do + expect(subject.logger).to receive(:info).with '' + expect(subject.logger).to receive(:info).with %Q(Started POST "/api/1.0/users" at #{subject.start_time}) + expect(subject.logger).to receive(:info).with %Q(Processing by TestAPI#users) + expect(subject.logger).to receive(:info).with %Q( Parameters: {"id"=>"101001", "name"=>"foo", "password"=>"[FILTERED]"}) + expect(subject.logger).to receive(:info).with /Completed 200 in \d.\d+ms/ + expect(subject.logger).to receive(:info).with '' + subject.call!(env) + end + + describe 'the "processing by" section' do + before { subject.call!(env) } + + context 'namespacing' do + let(:grape_endpoint) { build(:namespaced_endpoint) } + + it 'designates the namespace with a slash' do + expect(subject.processed_by).to eq 'TestAPI/admin#users' + end + + context 'with more complex route' do + let(:grape_endpoint) { build(:namespaced_endpoint, :complex) } + + it 'only escapes the first slash and leaves the rest of the untouched' do + expect(subject.processed_by).to eq 'TestAPI/admin#users/:name/profile' + end + end + end + + context 'with more complex route' do + let(:grape_endpoint) { build(:grape_endpoint, :complex) } + + it 'only escapes the first slash and leaves the rest of the untouched' do + expect(subject.processed_by).to eq 'TestAPI#users/:name/profile' + end + end + end +end diff --git a/spec/lib/grape/middleware/logger_spec.rb b/spec/lib/grape/middleware/logger_spec.rb index 0fcac14..95a0549 100644 --- a/spec/lib/grape/middleware/logger_spec.rb +++ b/spec/lib/grape/middleware/logger_spec.rb @@ -1,25 +1,14 @@ require 'spec_helper' -require 'grape/middleware/logger' describe Grape::Middleware::Logger do let(:app) { double('app') } - let(:options) { { filter: ParamFilter.new, logger: Object.new } } + let(:options) { { filter: build(:param_filter), logger: Object.new } } subject { described_class.new(app, options) } - let(:app_response) { Rack::Response.new 'Hello World', 200, {} } - let(:grape_request) { OpenStruct.new(request_method: 'POST', path: '/api/1.0/users', headers: {}, params: { 'id' => '101001' }) } - let(:env) { - { - 'grape.request' => grape_request, - 'grape.request.params' => grape_request.params, - 'action_dispatch.request.request_parameters' => { - 'name' => 'foo', - 'password' => 'access' - }, - 'rack.input' => OpenStruct.new - } - } + let(:app_response) { build :app_response } + let(:grape_request) { build :grape_request } + let(:env) { build(:expected_env) } describe '#call!' do context 'when calling the app results in an error response' do @@ -158,26 +147,4 @@ describe Grape::Middleware::Logger do end end end - - describe 'integration' do - it 'properly logs requests' do - expect(app).to receive(:call).with(env).and_return(app_response) - expect(Grape::Request).to receive(:new).and_return(grape_request) - expect(subject.logger).to receive(:info).with('') - expect(subject.logger).to receive(:info).with(%Q(Started POST "/api/1.0/users" at #{subject.start_time})) - expect(subject.logger).to receive(:info).with(%Q( Parameters: {"id"=>"101001", "name"=>"foo", "password"=>"[FILTERED]"})) - expect(subject.logger).to receive(:info).with(/Completed 200 in \d.\d+ms/) - expect(subject.logger).to receive(:info).with('') - subject.call!(env) - end - end - - # - # Test class - # - class ParamFilter - def filter(opts) - opts.each_pair { |key, val| val[0..-1] = '[FILTERED]' if key == 'password' } - end - end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 9e7d559..31bf9eb 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -2,7 +2,12 @@ $LOAD_PATH.unshift(File.dirname(__FILE__)) $LOAD_PATH.unshift(File.join(File.dirname(__FILE__), '..', 'lib')) require 'ostruct' +require 'factory_girl' +require 'grape/middleware/logger' + +FactoryGirl.find_definitions RSpec.configure do |config| config.raise_errors_for_deprecations! + config.include FactoryGirl::Syntax::Methods end