Major backward-incompatible changes

rwojsznis · Apr 20, 2015 · deec704 · deec704
1 parent 2d131f4
commit deec704
Show file tree

Hide file tree

Showing 5 changed files with 48 additions and 14 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,8 @@
+## Unreleased
+
+- store logged headers under `headers` key (see `Storage::Unit`)
+- don't log not-existing headers unless _truthy_ fallback value is provided
+
 ## 0.0.5alpha
 
 - log specified headers feature (backend) #3 (@michaldarda)

diff --git a/lib/rack/request_police/middleware.rb b/lib/rack/request_police/middleware.rb
@@ -26,7 +26,8 @@ def call(env)
               request_params.merge!('data' => utf8_input(env))
             end
 
-            request_params.merge!(request_headers(env))
+            headers = request_headers(env)
+            request_params.merge!('headers' => headers) unless headers.empty?
 
             ::Rack::RequestPolice.storage.log_request(request_params)
           end
@@ -49,7 +50,7 @@ def request_headers(env)
           header_value = transformation.call(header_value) if header_value
           header_value ||= fallback_value
 
-          result[storage_name] = header_value
+          result[storage_name] = header_value if header_value
         end
       end
 

diff --git a/lib/rack/request_police/storage/redis.rb b/lib/rack/request_police/storage/redis.rb
@@ -26,7 +26,7 @@ def page(pageidx = 1, page_size = 25)
           total_size = redis.llen(REDIS_KEY)
           items      = redis.lrange(REDIS_KEY, starting, ending).map do |json|
             hash = parser.load(json)
-            Unit.new(hash['method'], hash['ip'], hash['url'], Time.at(hash['time']), hash['data'])
+            Unit.new(hash['method'], hash['ip'], hash['url'], Time.at(hash['time']), hash['data'], hash['headers'])
           end
 
           [current_page, total_size, items]

diff --git a/lib/rack/request_police/storage/unit.rb b/lib/rack/request_police/storage/unit.rb
@@ -1,7 +1,7 @@
 module Rack
   module RequestPolice
     module Storage
-      class Unit < Struct.new(:method, :ip, :url, :time, :data)
+      class Unit < Struct.new(:method, :ip, :url, :time, :data, :headers)
       end
     end
   end

diff --git a/spec/middleware_spec.rb b/spec/middleware_spec.rb
@@ -260,7 +260,7 @@
           "ip" => "127.0.0.1",
           "method" => "get",
           "time" => Time.now.to_i,
-          "HTTP_MY_HEADER" => "MY%HEADER%VALUE"
+          "headers" => { "HTTP_MY_HEADER" => "MY%HEADER%VALUE" }
         })
 
         get '/user', {}, { 'HTTP_MY_HEADER' => 'MY%HEADER%VALUE' }
@@ -288,7 +288,7 @@
           "ip" => "127.0.0.1",
           "method" => "get",
           "time" => Time.now.to_i,
-          "my_header" => "my_header_value"
+          "headers" => { "my_header" => "my_header_value" }
         })
 
         get '/user', {}, { 'HTTP_MY_HEADER' => 'MY%HEADER%VALUE' }
@@ -319,7 +319,7 @@
           "ip" => "127.0.0.1",
           "method" => "get",
           "time" => Time.now.to_i,
-          'my_header' => 'HEADER_MISSING'
+          "headers" => { 'my_header' => 'HEADER_MISSING' }
         })
 
         get '/user', {}, {}
@@ -340,14 +340,13 @@
         end
       end
 
-      it "logs header as custom name and transforms it" do
+      it "ignores not existing header" do
         expect(Rack::RequestPolice.storage).to receive(:log_request)
           .with({
           "url" => "http://example.org/user",
           "ip" => "127.0.0.1",
           "method" => "get",
-          "time" => Time.now.to_i,
-          "my_header" => nil
+          "time" => Time.now.to_i
         })
 
         get '/user', {}
@@ -375,7 +374,7 @@
           "ip" => "127.0.0.1",
           "method" => "get",
           "time" => Time.now.to_i,
-          "my_header" => 'not found'
+          "headers" => { "my_header" => 'not found' }
         })
 
         get '/user', {}
@@ -396,20 +395,49 @@
         end
       end
 
-      it "logs header as custom name and falls-back to default value" do
+      it "ignores not existing header" do
         expect(Rack::RequestPolice.storage).to receive(:log_request)
           .with({
           "url" => "http://example.org/user",
           "ip" => "127.0.0.1",
           "method" => "get",
-          "time" => Time.now.to_i,
-          "HTTP_MY_HEADER" => nil
+          "time" => Time.now.to_i
         })
 
         get '/user', {}
 
         expect(last_response.status).to eq 200
       end
     end
+
+    context 'logging multiple headers' do
+      before do
+        Rack::RequestPolice.configure do |c|
+          c.storage = DummyStorage.new
+          c.regex = nil
+
+          c.headers = [
+            'HTTP_HEADER_1',
+            'HTTP_HEADER_2',
+            'HTTP_HEADER_3'
+          ]
+        end
+      end
+
+      it 'logs all declared headers' do
+        expect(Rack::RequestPolice.storage).to receive(:log_request)
+          .with({
+          "url" => "http://example.org/user",
+          "ip" => "127.0.0.1",
+          "method" => "get",
+          "time" => Time.now.to_i,
+          "headers" => { 'HTTP_HEADER_1' => "one", 'HTTP_HEADER_3' => 'two' }
+        })
+
+        get '/user', {}, { 'HTTP_HEADER_1' => 'one', 'HTTP_HEADER_3' => 'two' }
+
+        expect(last_response.status).to eq 200
+      end
+    end
   end
 end