diff --git a/README.md b/README.md index 5821fde..d0dfae2 100644 --- a/README.md +++ b/README.md @@ -53,7 +53,7 @@ The middleware logger can be customized with the following options: * The `:logger` option can be any object that responds to `.info(String)` * The `:filter` option can be any object that responds to `.filter(Hash)` and returns a hash. -* The `:headers` option can be eather `:all` or array of strings. +* The `:headers` option can be either `:all` or array of strings. + If `:all`, all request headers will be output. + If array, output will be filtered by names in the array. (case-insensitive) diff --git a/lib/grape/middleware/logger.rb b/lib/grape/middleware/logger.rb index f950ab1..38e9d29 100644 --- a/lib/grape/middleware/logger.rb +++ b/lib/grape/middleware/logger.rb @@ -95,14 +95,14 @@ class Grape::Middleware::Logger < Grape::Middleware::Globals def headers request_headers = env[Grape::Env::GRAPE_REQUEST_HEADERS].to_hash - return request_headers if @options[:headers] == :all + return Hash[request_headers.sort] if @options[:headers] == :all headers_needed = Array(@options[:headers]) - Hash[request_headers.sort].select do |key, value| - headers_needed.any? do |need| - need.to_s.casecmp(key).zero? - end + result = {} + headers_needed.each do |need| + result.merge!(request_headers.select { |key, value| need.to_s.casecmp(key).zero? }) end + Hash[result.sort] end def start_time diff --git a/spec/factories.rb b/spec/factories.rb index cae0c95..b17fab8 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -52,8 +52,10 @@ FactoryGirl.define do trait :prefixed_basic_headers do other_env_params { { + 'HTTP_VERSION' => 'HTTP/1.1', 'HTTP_CACHE_CONTROL' => 'max-age=0', - 'HTTP_USER_AGENT' => 'Mozilla/5.0' + 'HTTP_USER_AGENT' => 'Mozilla/5.0', + 'HTTP_ACCEPT_LANGUAGE' => 'en-US' } } end @@ -103,8 +105,10 @@ FactoryGirl.define do trait :basic_headers do headers { { + 'Version' => 'HTTP/1.1', 'Cache-Control' => 'max-age=0', - 'User-Agent' => 'Mozilla/5.0' + 'User-Agent' => 'Mozilla/5.0', + 'Accept-Language' => 'en-US' } } end end diff --git a/spec/integration/lib/grape/middleware/headers_option_spec.rb b/spec/integration/lib/grape/middleware/headers_option_spec.rb index af9ed61..f08de29 100644 --- a/spec/integration/lib/grape/middleware/headers_option_spec.rb +++ b/spec/integration/lib/grape/middleware/headers_option_spec.rb @@ -10,16 +10,16 @@ describe Grape::Middleware::Logger, type: :integration do context ':all option is set to option headers' do let(:options) { { - filter: build(:param_filter), - headers: :all, - logger: Logger.new(Tempfile.new('logger')) + filter: build(:param_filter), + headers: :all, + logger: Logger.new(Tempfile.new('logger')) } } it 'all headers will be shown, headers will be sorted by name' 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", "secret"=>"[FILTERED]", "customer"=>[], "name"=>"foo", "password"=>"[FILTERED]"}) - expect(subject.logger).to receive(:info).with %Q( Headers: {"Cache-Control"=>"max-age=0", "User-Agent"=>"Mozilla/5.0"}) + expect(subject.logger).to receive(:info).with %Q( Headers: {"Accept-Language"=>"en-US", "Cache-Control"=>"max-age=0", "User-Agent"=>"Mozilla/5.0", "Version"=>"HTTP/1.1"}) expect(subject.logger).to receive(:info).with /Completed 200 in \d+.\d+ms/ expect(subject.logger).to receive(:info).with '' subject.call!(env) @@ -28,9 +28,9 @@ describe Grape::Middleware::Logger, type: :integration do context 'list of names ["User-Agent", "Cache-Control"] is set to option headers' do let(:options) { { - filter: build(:param_filter), - headers: %w(User-Agent Cache-Control), - logger: Logger.new(Tempfile.new('logger')) + filter: build(:param_filter), + headers: %w(User-Agent Cache-Control), + logger: Logger.new(Tempfile.new('logger')) } } it 'two headers will be shown' do expect(subject.logger).to receive(:info).with '' @@ -46,9 +46,9 @@ describe Grape::Middleware::Logger, type: :integration do context 'a single string "Cache-Control" is set to option headers' do let(:options) { { - filter: build(:param_filter), - headers: 'Cache-Control', - logger: Logger.new(Tempfile.new('logger')) + filter: build(:param_filter), + headers: 'Cache-Control', + logger: Logger.new(Tempfile.new('logger')) } } it 'only Cache-Control header will be shown' do expect(subject.logger).to receive(:info).with '' diff --git a/spec/lib/grape/middleware/headers_option_spec.rb b/spec/lib/grape/middleware/headers_option_spec.rb index dd64b52..089282c 100644 --- a/spec/lib/grape/middleware/headers_option_spec.rb +++ b/spec/lib/grape/middleware/headers_option_spec.rb @@ -6,16 +6,18 @@ describe Grape::Middleware::Logger do subject { described_class.new(app, options) } describe '#headers' do - let(:grape_request) { build :grape_request, :basic_headers } - let(:env) { build :expected_env, grape_request: grape_request } - + let(:grape_request) { build :grape_request, :basic_headers } + let(:env) { build :expected_env, grape_request: grape_request } + before { subject.instance_variable_set(:@env, env) } context 'when @options[:headers] has a symbol :all' do let(:options) { { headers: :all, logger: Object.new } } it 'all request headers should be retrieved' do + expect(subject.headers.fetch('Accept-Language')).to eq('en-US') expect(subject.headers.fetch('Cache-Control')).to eq('max-age=0') expect(subject.headers.fetch('User-Agent')).to eq('Mozilla/5.0') + expect(subject.headers.fetch('Version')).to eq('HTTP/1.1') end end @@ -40,7 +42,7 @@ describe Grape::Middleware::Logger do end describe '#headers if no request header' do - let(:env) { build :expected_env } + let(:env) { build :expected_env } before { subject.instance_variable_set(:@env, env) } context 'when @options[:headers] is set, but no request header is there' do