From 9c8568bc2b896c7ee531e6e904d41688b126631b Mon Sep 17 00:00:00 2001 From: Peter Boling Date: Tue, 13 Jan 2015 20:55:34 -0800 Subject: [PATCH 1/4] Maybe Globals could be functional? --- lib/grape/middleware/globals.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/grape/middleware/globals.rb b/lib/grape/middleware/globals.rb index ec3aa1695..a4e28b0a7 100644 --- a/lib/grape/middleware/globals.rb +++ b/lib/grape/middleware/globals.rb @@ -4,9 +4,9 @@ module Grape module Middleware class Globals < Base def before - @env['grape.request'] = Grape::Request.new(@env) + request = Grape::Request.new(@env) @env['grape.request.headers'] = request.headers - @env['grape.request.params'] = request.params if @env['rack.input'] + @env['grape.request.params'] = request.params end end end From 2c4c58ada126ef9af2a2300a5bda11c7d90fc54e Mon Sep 17 00:00:00 2001 From: Peter Boling Date: Tue, 13 Jan 2015 23:37:30 -0800 Subject: [PATCH 2/4] Fixed Globals Middleware; Added spec; Added example Logger middleware --- lib/grape/middleware/globals.rb | 9 ++++ lib/grape/middleware/logger.rb | 74 +++++++++++++++++++++++++++ spec/grape/middleware/globals_spec.rb | 37 ++++++++++++++ 3 files changed, 120 insertions(+) create mode 100644 lib/grape/middleware/logger.rb create mode 100644 spec/grape/middleware/globals_spec.rb diff --git a/lib/grape/middleware/globals.rb b/lib/grape/middleware/globals.rb index a4e28b0a7..40f892e88 100644 --- a/lib/grape/middleware/globals.rb +++ b/lib/grape/middleware/globals.rb @@ -1,8 +1,17 @@ +# This Middleware is not loaded by grape by default. +# If you intend to use it you must first: +# require 'grape/middleware/globals' +# See the spec for examples. +# require 'grape/middleware/base' module Grape module Middleware class Globals < Base + # NOTE: If you have Grape mounted as a Rack endpoint in a Rails stack action dispatch may have moved the params + # If your params are not showing up, + # then you may need to override this method and + # have it load params from @env['action_dispatch.request.request_parameters'] instead of request.params def before request = Grape::Request.new(@env) @env['grape.request.headers'] = request.headers diff --git a/lib/grape/middleware/logger.rb b/lib/grape/middleware/logger.rb new file mode 100644 index 000000000..f3ce725ea --- /dev/null +++ b/lib/grape/middleware/logger.rb @@ -0,0 +1,74 @@ +# This Middleware is not loaded by grape by default. +# If you intend to use it you must first: +# require 'grape/middleware/logger' +# See the spec for examples. +# +require 'grape/middleware/globals' + +module Grape + module Middleware + class Logger < Globals + + def before + super + logger.info "[api] Requested#{request_log}" if !request_log.blank? + logger.info "[api] HEADERS: #{@env['grape.request.headers']}" if !@env['grape.request.headers'].blank? + logger.info "[api] PARAMS: #{@env['grape.request.params']}" if !@env['grape.request.params'].blank? + end + + # Example of what you might do in a subclass in an after hook: + # def after + # response_body = JSON.parse(response.body.first) + # if response_body.is_a?(Hash) + # logger.debug "[api] RespType: #{response_body['response_type']}" unless response_body['response_type'].blank? + # logger.debug "[api] Response: #{response_body['response']}" unless response_body['response'].blank? + # logger.debug "[api] Backtrace:\n#{response_body['backtrace'].join("\n")}" if response_body['backtrace'] && response_body['backtrace'].any? + # end + # super + # end + + private + + # Override in a subclass to customize the logger (not affected by setting the logger helper in Grape::API) + # def logger + # @logger ||= Rails.logger # as an example + # end + def logger + @logger ||= Logger.new(STDOUT) + end + + def request_log + @request_log ||= begin + res = '' + res << " #{request_log_data}" if !request_log_data.blank? + res + end + end + + def request_log_data + rld = {} + + x_org = env['HTTP_X_ORGANIZATION'] + + rld[:user_id] = current_user.id if current_user + rld[:x_organization] = x_org if x_org + + rld + end + + # Override this method in a subclass and mount the subclass + # For example with Devise & Warden: + # def current_user + # @warden_user_for_log ||= begin + # woo = env['warden'].instance_variable_get(:'@users') + # woo[:user] if woo + # end + # end + # Also if Warden is later in your Rack Middleware stack + # then you can only render the current user data in the after hook, not the before hook. + def current_user + false + end + end + end +end diff --git a/spec/grape/middleware/globals_spec.rb b/spec/grape/middleware/globals_spec.rb new file mode 100644 index 000000000..b7d5d02f8 --- /dev/null +++ b/spec/grape/middleware/globals_spec.rb @@ -0,0 +1,37 @@ +require 'spec_helper' +require 'grape/middleware/globals' + +describe Grape::Middleware::Globals do + + let(:app) { lambda { |env| [200, env, 'Howdy Doody'] } } + subject { Grape::Middleware::Globals.new(app, {}) } + + context 'with params' do + it 'sets the params based on the params' do + env = Rack::MockRequest.env_for('/awesome', params: { 'swank' => 'bank' }) + expect(subject.call(env)[1]['grape.request.params']).to eq({ 'swank' => 'bank' }) + expect(subject.call(env)[1]['grape.request.headers']).to eq({}) + end + end + + context 'with headers' do + it 'sets the headers based on the headers' do + env = Rack::MockRequest.env_for('/awesome', params: { }) + env['HTTP_MONKEY'] = 'I_BANANA' + + expect(subject.call(env)[1]['grape.request.params']).to eq({}) + expect(subject.call(env)[1]['grape.request.headers']).to eq({ 'Monkey' => 'I_BANANA' }) + end + end + + context 'with headers and params' do + it 'sets the headers based on the headers' do + env = Rack::MockRequest.env_for('/awesome', params: { 'grapes' => 'wrath' }) + env['HTTP_MONKEY'] = 'I_BANANA' + + expect(subject.call(env)[1]['grape.request.params']).to eq({ 'grapes' => 'wrath' }) + expect(subject.call(env)[1]['grape.request.headers']).to eq({ 'Monkey' => 'I_BANANA' }) + end + end + +end From 286aa24176fb9c67c217d0d776e9826f52f16c50 Mon Sep 17 00:00:00 2001 From: Peter Boling Date: Wed, 14 Jan 2015 00:03:53 -0800 Subject: [PATCH 3/4] Remove railsisms, and fix offenses --- lib/grape/middleware/logger.rb | 9 ++++----- spec/grape/middleware/globals_spec.rb | 16 +++++++--------- 2 files changed, 11 insertions(+), 14 deletions(-) diff --git a/lib/grape/middleware/logger.rb b/lib/grape/middleware/logger.rb index f3ce725ea..ca30b6808 100644 --- a/lib/grape/middleware/logger.rb +++ b/lib/grape/middleware/logger.rb @@ -8,12 +8,11 @@ module Grape module Middleware class Logger < Globals - def before super - logger.info "[api] Requested#{request_log}" if !request_log.blank? - logger.info "[api] HEADERS: #{@env['grape.request.headers']}" if !@env['grape.request.headers'].blank? - logger.info "[api] PARAMS: #{@env['grape.request.params']}" if !@env['grape.request.params'].blank? + logger.info "[api] Requested#{request_log}" unless request_log.nil? || request_log.empty? + logger.info "[api] HEADERS: #{@env['grape.request.headers']}" unless @env['grape.request.headers'].nil? || @env['grape.request.headers'].empty? + logger.info "[api] PARAMS: #{@env['grape.request.params']}" unless @env['grape.request.params'].nil? || @env['grape.request.params'].empty? end # Example of what you might do in a subclass in an after hook: @@ -40,7 +39,7 @@ def logger def request_log @request_log ||= begin res = '' - res << " #{request_log_data}" if !request_log_data.blank? + res << " #{request_log_data}" unless request_log_data.nil? || request_log_data.empty? res end end diff --git a/spec/grape/middleware/globals_spec.rb b/spec/grape/middleware/globals_spec.rb index b7d5d02f8..7b9754d9f 100644 --- a/spec/grape/middleware/globals_spec.rb +++ b/spec/grape/middleware/globals_spec.rb @@ -2,36 +2,34 @@ require 'grape/middleware/globals' describe Grape::Middleware::Globals do - let(:app) { lambda { |env| [200, env, 'Howdy Doody'] } } subject { Grape::Middleware::Globals.new(app, {}) } context 'with params' do it 'sets the params based on the params' do - env = Rack::MockRequest.env_for('/awesome', params: { 'swank' => 'bank' }) - expect(subject.call(env)[1]['grape.request.params']).to eq({ 'swank' => 'bank' }) + env = Rack::MockRequest.env_for('/awesome', params: {'swank' => 'bank'}) + expect(subject.call(env)[1]['grape.request.params']).to eq({'swank' => 'bank'}) expect(subject.call(env)[1]['grape.request.headers']).to eq({}) end end context 'with headers' do it 'sets the headers based on the headers' do - env = Rack::MockRequest.env_for('/awesome', params: { }) + env = Rack::MockRequest.env_for('/awesome', params: {}) env['HTTP_MONKEY'] = 'I_BANANA' expect(subject.call(env)[1]['grape.request.params']).to eq({}) - expect(subject.call(env)[1]['grape.request.headers']).to eq({ 'Monkey' => 'I_BANANA' }) + expect(subject.call(env)[1]['grape.request.headers']).to eq({'Monkey' => 'I_BANANA'}) end end context 'with headers and params' do it 'sets the headers based on the headers' do - env = Rack::MockRequest.env_for('/awesome', params: { 'grapes' => 'wrath' }) + env = Rack::MockRequest.env_for('/awesome', params: {'grapes' => 'wrath'}) env['HTTP_MONKEY'] = 'I_BANANA' - expect(subject.call(env)[1]['grape.request.params']).to eq({ 'grapes' => 'wrath' }) - expect(subject.call(env)[1]['grape.request.headers']).to eq({ 'Monkey' => 'I_BANANA' }) + expect(subject.call(env)[1]['grape.request.params']).to eq({'grapes' => 'wrath'}) + expect(subject.call(env)[1]['grape.request.headers']).to eq({'Monkey' => 'I_BANANA'}) end end - end From 2b41a049a541fbc35a8a8a3f9693a4c8e4198b6f Mon Sep 17 00:00:00 2001 From: Peter Boling Date: Wed, 14 Jan 2015 02:24:08 -0800 Subject: [PATCH 4/4] All offenses fixed --- spec/grape/middleware/globals_spec.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/spec/grape/middleware/globals_spec.rb b/spec/grape/middleware/globals_spec.rb index 7b9754d9f..2d1794955 100644 --- a/spec/grape/middleware/globals_spec.rb +++ b/spec/grape/middleware/globals_spec.rb @@ -7,8 +7,8 @@ context 'with params' do it 'sets the params based on the params' do - env = Rack::MockRequest.env_for('/awesome', params: {'swank' => 'bank'}) - expect(subject.call(env)[1]['grape.request.params']).to eq({'swank' => 'bank'}) + env = Rack::MockRequest.env_for('/awesome', params: { 'swank' => 'bank' }) + expect(subject.call(env)[1]['grape.request.params']).to eq('swank' => 'bank') expect(subject.call(env)[1]['grape.request.headers']).to eq({}) end end @@ -19,17 +19,17 @@ env['HTTP_MONKEY'] = 'I_BANANA' expect(subject.call(env)[1]['grape.request.params']).to eq({}) - expect(subject.call(env)[1]['grape.request.headers']).to eq({'Monkey' => 'I_BANANA'}) + expect(subject.call(env)[1]['grape.request.headers']).to eq('Monkey' => 'I_BANANA') end end context 'with headers and params' do it 'sets the headers based on the headers' do - env = Rack::MockRequest.env_for('/awesome', params: {'grapes' => 'wrath'}) + env = Rack::MockRequest.env_for('/awesome', params: { 'grapes' => 'wrath' }) env['HTTP_MONKEY'] = 'I_BANANA' - expect(subject.call(env)[1]['grape.request.params']).to eq({'grapes' => 'wrath'}) - expect(subject.call(env)[1]['grape.request.headers']).to eq({'Monkey' => 'I_BANANA'}) + expect(subject.call(env)[1]['grape.request.params']).to eq('grapes' => 'wrath') + expect(subject.call(env)[1]['grape.request.headers']).to eq('Monkey' => 'I_BANANA') end end end