From a55f2de0c5baae589b1730df1e4068f0cd1474ce Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jos=C3=A9=20Valim?= <jose.valim@gmail.com>
Date: Tue, 3 May 2011 01:03:21 +0200
Subject: Improve performance for filtered parameters and add tests.

---
 actionpack/lib/action_controller/base.rb                  |  5 ++++-
 actionpack/lib/action_controller/metal/instrumentation.rb |  2 +-
 actionpack/lib/action_controller/metal/params_wrapper.rb  |  5 ++---
 actionpack/lib/action_dispatch/http/filter_parameters.rb  |  5 +++++
 actionpack/test/controller/log_subscriber_test.rb         | 11 +++++++++++
 5 files changed, 23 insertions(+), 5 deletions(-)

(limited to 'actionpack')

diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb
index ccf1632a14..c03c77cb4a 100644
--- a/actionpack/lib/action_controller/base.rb
+++ b/actionpack/lib/action_controller/base.rb
@@ -194,7 +194,6 @@ module ActionController
       Caching,
       MimeResponds,
       ImplicitRender,
-      ParamsWrapper,
 
       Cookies,
       Flash,
@@ -215,6 +214,10 @@ module ActionController
       # all the methods properly.
       Instrumentation,
 
+      # Params wrapper should come before instrumentation so they are
+      # properly showed in logs
+      ParamsWrapper,
+
       # The same with rescue, append it at the end to wrap as much as possible.
       Rescue
     ]
diff --git a/actionpack/lib/action_controller/metal/instrumentation.rb b/actionpack/lib/action_controller/metal/instrumentation.rb
index dc3ea939e6..4e54c2ad88 100644
--- a/actionpack/lib/action_controller/metal/instrumentation.rb
+++ b/actionpack/lib/action_controller/metal/instrumentation.rb
@@ -14,7 +14,7 @@ module ActionController
 
     attr_internal :view_runtime
 
-    def process_action(action, *args)
+    def process_action(*args)
       raw_payload = {
         :controller => self.class.name,
         :action     => self.action_name,
diff --git a/actionpack/lib/action_controller/metal/params_wrapper.rb b/actionpack/lib/action_controller/metal/params_wrapper.rb
index 3262e24f67..d128f6d03c 100644
--- a/actionpack/lib/action_controller/metal/params_wrapper.rb
+++ b/actionpack/lib/action_controller/metal/params_wrapper.rb
@@ -178,14 +178,13 @@ module ActionController
     def process_action(*args)
       if _wrapper_enabled?
         wrapped_hash = { _wrapper_key => request.request_parameters.slice(*_wrapped_keys) }
-        wrapped_filtered_hash = { _wrapper_key => request.filtered_parameters.slice(*_wrapped_keys) }
 
         # This will make the wrapped hash accessible from controller and view
         request.parameters.merge! wrapped_hash
         request.request_parameters.merge! wrapped_hash
 
         # This will make the wrapped hash displayed in the log file
-        request.filtered_parameters.merge! wrapped_filtered_hash
+        request.clear_filtered_parameters
       end
       super
     end
@@ -215,7 +214,7 @@ module ActionController
       # Checks if we should perform parameters wrapping.
       def _wrapper_enabled?
         ref = request.content_mime_type.try(:ref)
-        _wrapper_formats.any? { |format| format == ref } && !request.request_parameters[_wrapper_key]
+        _wrapper_formats.include?(ref) && !request.request_parameters[_wrapper_key]
       end
   end
 end
diff --git a/actionpack/lib/action_dispatch/http/filter_parameters.rb b/actionpack/lib/action_dispatch/http/filter_parameters.rb
index 8dd1af7f3d..9c5b6a6b88 100644
--- a/actionpack/lib/action_dispatch/http/filter_parameters.rb
+++ b/actionpack/lib/action_dispatch/http/filter_parameters.rb
@@ -33,6 +33,11 @@ module ActionDispatch
         @filtered_parameters ||= parameter_filter.filter(parameters)
       end
 
+      # Clear any filtered parameters forcing them to be filtered again.
+      def clear_filtered_parameters
+        @filtered_parameters = nil
+      end
+
       # Return a hash of request.env with all sensitive data replaced.
       def filtered_env
         @filtered_env ||= env_filter.filter(@env)
diff --git a/actionpack/test/controller/log_subscriber_test.rb b/actionpack/test/controller/log_subscriber_test.rb
index ddfa3df552..5d7a51e902 100644
--- a/actionpack/test/controller/log_subscriber_test.rb
+++ b/actionpack/test/controller/log_subscriber_test.rb
@@ -4,6 +4,8 @@ require "action_controller/log_subscriber"
 
 module Another
   class LogSubscribersController < ActionController::Base
+    wrap_parameters :person, :only => :name, :format => :json
+
     def show
       render :nothing => true
     end
@@ -95,6 +97,15 @@ class ACLogSubscriberTest < ActionController::TestCase
     assert_equal 'Parameters: {"id"=>"10"}', logs[1]
   end
 
+  def test_process_action_with_wrapped_parameters
+    @request.env['CONTENT_TYPE'] = 'application/json'
+    post :show, :id => '10', :name => 'jose'
+    wait
+
+    assert_equal 3, logs.size
+    assert_match '"person"=>{"name"=>"jose"}', logs[1]
+  end
+
   def test_process_action_with_view_runtime
     get :show
     wait
-- 
cgit v1.2.3