Bugfix, logic optimization, correct code format

- Fix typo in README
- Replace tab character with spaces and correct wrong indents.
- Bugfix: headers are not sorted by keys when the option is `:all`
- Optimize filtering logic

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)
 

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|
+    result = {}
-      headers_needed.any? do |need|
+    headers_needed.each do |need|
-        need.to_s.casecmp(key).zero?
+      result.merge!(request_headers.select { |key, value| need.to_s.casecmp(key).zero? })
-      end
     end
+    Hash[result.sort]
   end
 
   def start_time

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

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),
+      filter: build(:param_filter),
-        headers: :all,
+      headers: :all,
-        logger: Logger.new(Tempfile.new('logger'))
+      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),
+      filter: build(:param_filter),
-        headers: %w(User-Agent Cache-Control),
+      headers: %w(User-Agent Cache-Control),
-        logger: Logger.new(Tempfile.new('logger'))
+      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),
+      filter: build(:param_filter),
-        headers: 'Cache-Control',
+      headers: 'Cache-Control',
-        logger: Logger.new(Tempfile.new('logger'))
+      logger: Logger.new(Tempfile.new('logger'))
     } }
     it 'only Cache-Control header will be shown' do
       expect(subject.logger).to receive(:info).with ''

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(:grape_request) { build :grape_request, :basic_headers }
-  	let(:env) { build :expected_env, grape_request: grape_request }
+    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