diff options
Diffstat (limited to 'actionpack')
18 files changed, 125 insertions, 80 deletions
diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb index 260e5da336..f86a61d791 100644 --- a/actionpack/lib/action_controller/base.rb +++ b/actionpack/lib/action_controller/base.rb @@ -26,6 +26,7 @@ module ActionController include ActionController::Compatibility include ActionController::Cookies + include ActionController::FilterParameterLogging include ActionController::Flash include ActionController::Verification include ActionController::RequestForgeryProtection @@ -37,7 +38,6 @@ module ActionController # Add instrumentations hooks at the bottom, to ensure they instrument # all the methods properly. include ActionController::Instrumentation - include ActionController::FilterParameterLogging # TODO: Extract into its own module # This should be moved together with other normalizing behavior diff --git a/actionpack/lib/action_controller/metal/filter_parameter_logging.rb b/actionpack/lib/action_controller/metal/filter_parameter_logging.rb index 0b1e1ee6ab..9e03f50759 100644 --- a/actionpack/lib/action_controller/metal/filter_parameter_logging.rb +++ b/actionpack/lib/action_controller/metal/filter_parameter_logging.rb @@ -58,11 +58,6 @@ module ActionController protected - def append_info_to_payload(payload) - super - payload[:params] = filter_parameters(request.params) - end - def filter_parameters(params) params.dup.except!(*INTERNAL_PARAMS) end diff --git a/actionpack/lib/action_controller/metal/helpers.rb b/actionpack/lib/action_controller/metal/helpers.rb index d0402e5bad..cdd14560e1 100644 --- a/actionpack/lib/action_controller/metal/helpers.rb +++ b/actionpack/lib/action_controller/metal/helpers.rb @@ -100,7 +100,7 @@ module ActionController module_path = module_name.underscore helper module_path rescue MissingSourceFile => e - raise e unless e.is_missing? "#{module_path}_helper" + raise e unless e.is_missing? "helpers/#{module_path}_helper" rescue NameError => e raise e unless e.missing_name? "#{module_name}Helper" end diff --git a/actionpack/lib/action_controller/metal/instrumentation.rb b/actionpack/lib/action_controller/metal/instrumentation.rb index 876f778751..7b2b037c67 100644 --- a/actionpack/lib/action_controller/metal/instrumentation.rb +++ b/actionpack/lib/action_controller/metal/instrumentation.rb @@ -9,18 +9,24 @@ module ActionController module Instrumentation extend ActiveSupport::Concern - included do - include AbstractController::Logger - end + include AbstractController::Logger + include ActionController::FilterParameterLogging attr_internal :view_runtime def process_action(action, *args) - ActiveSupport::Notifications.instrument("action_controller.process_action") do |payload| + raw_payload = { + :controller => self.class.name, + :action => self.action_name, + :params => filter_parameters(params), + :formats => request.formats.map(&:to_sym) + } + + ActiveSupport::Notifications.instrument("action_controller.start_processing", raw_payload.dup) + + ActiveSupport::Notifications.instrument("action_controller.process_action", raw_payload) do |payload| result = super - payload[:controller] = self.class.name - payload[:action] = self.action_name - payload[:status] = response.status + payload[:status] = response.status append_info_to_payload(payload) result end diff --git a/actionpack/lib/action_controller/railties/subscriber.rb b/actionpack/lib/action_controller/railties/subscriber.rb index 6659e5df47..d257d6ac2c 100644 --- a/actionpack/lib/action_controller/railties/subscriber.rb +++ b/actionpack/lib/action_controller/railties/subscriber.rb @@ -1,15 +1,19 @@ module ActionController module Railties class Subscriber < Rails::Subscriber - def process_action(event) + def start_processing(event) payload = event.payload + info " Processing by #{payload[:controller]}##{payload[:action]} as #{payload[:formats].first.to_s.upcase}" info " Parameters: #{payload[:params].inspect}" unless payload[:params].blank? + end + def process_action(event) + payload = event.payload additions = ActionController::Base.log_process_action(payload) message = "Completed in %.0fms" % event.duration message << " (#{additions.join(" | ")})" unless additions.blank? - message << " by #{payload[:controller]}##{payload[:action]} [#{payload[:status]}]" + message << " with #{payload[:status]}" info(message) end diff --git a/actionpack/lib/action_dispatch/middleware/notifications.rb b/actionpack/lib/action_dispatch/middleware/notifications.rb index 01d2cbb435..c3776d53a8 100644 --- a/actionpack/lib/action_dispatch/middleware/notifications.rb +++ b/actionpack/lib/action_dispatch/middleware/notifications.rb @@ -8,17 +8,24 @@ module ActionDispatch @app = app end - def call(stack_env) - env = stack_env.dup - ActiveSupport::Notifications.instrument("action_dispatch.before_dispatch", :env => env) + def call(env) + payload = retrieve_payload_from_env(env) + ActiveSupport::Notifications.instrument("action_dispatch.before_dispatch", payload) - ActiveSupport::Notifications.instrument!("action_dispatch.after_dispatch", :env => env) do - @app.call(stack_env) + ActiveSupport::Notifications.instrument!("action_dispatch.after_dispatch", payload) do + @app.call(env) end rescue Exception => exception ActiveSupport::Notifications.instrument('action_dispatch.exception', - :env => stack_env, :exception => exception) + :env => env, :exception => exception) raise exception end + + protected + + # Remove any rack related constants from the env, like rack.input. + def retrieve_payload_from_env(env) + Hash[:env => env.except(*env.keys.select { |k| k.to_s.index("rack.") == 0 })] + end end end
\ No newline at end of file diff --git a/actionpack/lib/action_dispatch/middleware/params_parser.rb b/actionpack/lib/action_dispatch/middleware/params_parser.rb index 534390d4aa..522982e202 100644 --- a/actionpack/lib/action_dispatch/middleware/params_parser.rb +++ b/actionpack/lib/action_dispatch/middleware/params_parser.rb @@ -35,14 +35,14 @@ module ActionDispatch when Proc strategy.call(request.raw_post) when :xml_simple, :xml_node - request.body.size == 0 ? {} : Hash.from_xml(request.body).with_indifferent_access + request.body.size == 0 ? {} : Hash.from_xml(request.raw_post).with_indifferent_access when :yaml - YAML.load(request.body) + YAML.load(request.raw_post) when :json if request.body.size == 0 {} else - data = ActiveSupport::JSON.decode(request.body) + data = ActiveSupport::JSON.decode(request.raw_post) data = {:_json => data} unless data.is_a?(Hash) data.with_indifferent_access end diff --git a/actionpack/lib/action_dispatch/middleware/stack.rb b/actionpack/lib/action_dispatch/middleware/stack.rb index 24be4fee55..0dc1d70e37 100644 --- a/actionpack/lib/action_dispatch/middleware/stack.rb +++ b/actionpack/lib/action_dispatch/middleware/stack.rb @@ -60,9 +60,7 @@ module ActionDispatch end def inspect - str = klass.to_s - args.each { |arg| str += ", #{build_args.inspect}" } - str + klass.to_s end def build(app) diff --git a/actionpack/lib/action_dispatch/railties/subscriber.rb b/actionpack/lib/action_dispatch/railties/subscriber.rb index c08a844c6a..cdb1162eac 100644 --- a/actionpack/lib/action_dispatch/railties/subscriber.rb +++ b/actionpack/lib/action_dispatch/railties/subscriber.rb @@ -5,8 +5,8 @@ module ActionDispatch request = Request.new(event.payload[:env]) path = request.request_uri.inspect rescue "unknown" - info "\n\nProcessing #{path} to #{request.formats.join(', ')} " << - "(for #{request.remote_ip} at #{event.time.to_s(:db)}) [#{request.method.to_s.upcase}]" + info "\n\nStarted #{request.method.to_s.upcase} #{path} " << + "for #{request.remote_ip} at #{event.time.to_s(:db)}" end def logger diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 811c287355..fcbb70749f 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -375,6 +375,15 @@ module ActionDispatch end end + def action_type(action) + case action + when :index, :create + :collection + when :show, :update, :destroy + :member + end + end + def name options[:as] || plural end @@ -391,6 +400,15 @@ module ActionDispatch plural end + def name_for_action(action) + case action_type(action) + when :collection + collection_name + when :member + member_name + end + end + def id_segment ":#{singular}_id" end @@ -405,6 +423,13 @@ module ActionDispatch super end + def action_type(action) + case action + when :show, :create, :update, :destroy + :member + end + end + def name options[:as] || singular end @@ -428,7 +453,7 @@ module ActionDispatch with_scope_level(:resource, resource) do yield if block_given? - get :show, :as => resource.member_name if resource.actions.include?(:show) + get :show if resource.actions.include?(:show) post :create if resource.actions.include?(:create) put :update if resource.actions.include?(:update) delete :destroy if resource.actions.include?(:destroy) @@ -454,14 +479,14 @@ module ActionDispatch yield if block_given? with_scope_level(:collection) do - get :index, :as => resource.collection_name if resource.actions.include?(:index) + get :index if resource.actions.include?(:index) post :create if resource.actions.include?(:create) get :new, :as => resource.singular if resource.actions.include?(:new) end with_scope_level(:member) do scope(':id') do - get :show, :as => resource.member_name if resource.actions.include?(:show) + get :show if resource.actions.include?(:show) put :update if resource.actions.include?(:update) delete :destroy if resource.actions.include?(:destroy) get :edit, :as => resource.singular if resource.actions.include?(:edit) @@ -525,7 +550,10 @@ module ActionDispatch begin old_path = @scope[:path] @scope[:path] = "#{@scope[:path]}(.:format)" - return match(options.reverse_merge(:to => action)) + return match(options.reverse_merge( + :to => action, + :as => parent_resource.name_for_action(action) + )) ensure @scope[:path] = old_path end diff --git a/actionpack/lib/action_dispatch/testing/integration.rb b/actionpack/lib/action_dispatch/testing/integration.rb index 4ec47d146c..d4c9df7ebd 100644 --- a/actionpack/lib/action_dispatch/testing/integration.rb +++ b/actionpack/lib/action_dispatch/testing/integration.rb @@ -259,7 +259,9 @@ module ActionDispatch "HTTP_HOST" => host, "REMOTE_ADDR" => remote_addr, "CONTENT_TYPE" => "application/x-www-form-urlencoded", - "HTTP_ACCEPT" => accept + "HTTP_ACCEPT" => accept, + + "action_dispatch.show_exceptions" => false } (rack_environment || {}).each do |key, value| diff --git a/actionpack/test/activerecord/controller_runtime_test.rb b/actionpack/test/activerecord/controller_runtime_test.rb index d6f7cd80ab..37c7738301 100644 --- a/actionpack/test/activerecord/controller_runtime_test.rb +++ b/actionpack/test/activerecord/controller_runtime_test.rb @@ -37,8 +37,8 @@ module ControllerRuntimeSubscriberTest get :show wait - assert_equal 1, @logger.logged(:info).size - assert_match /\(Views: [\d\.]+ms | ActiveRecord: [\d\.]+ms\)/, @logger.logged(:info)[0] + assert_equal 2, @logger.logged(:info).size + assert_match /\(Views: [\d\.]+ms | ActiveRecord: [\d\.]+ms\)/, @logger.logged(:info)[1] end class SyncSubscriberTest < ActionController::TestCase diff --git a/actionpack/test/controller/new_base/base_test.rb b/actionpack/test/controller/new_base/base_test.rb index 964780eaf2..0b40f8ce95 100644 --- a/actionpack/test/controller/new_base/base_test.rb +++ b/actionpack/test/controller/new_base/base_test.rb @@ -22,7 +22,7 @@ module Dispatching end def show_actions - render :text => "actions: #{action_methods.to_a.join(', ')}" + render :text => "actions: #{action_methods.to_a.sort.join(', ')}" end protected @@ -77,9 +77,9 @@ module Dispatching test "action methods" do assert_equal Set.new(%w( + index modify_response_headers modify_response_body_twice - index modify_response_body show_actions )), SimpleController.action_methods @@ -88,7 +88,7 @@ module Dispatching assert_equal Set.new, Submodule::ContainedEmptyController.action_methods get "/dispatching/simple/show_actions" - assert_body "actions: modify_response_headers, modify_response_body_twice, index, modify_response_body, show_actions" + assert_body "actions: index, modify_response_body, modify_response_body_twice, modify_response_headers, show_actions" end end end diff --git a/actionpack/test/controller/subscriber_test.rb b/actionpack/test/controller/subscriber_test.rb index 24132ee928..950eecaf6f 100644 --- a/actionpack/test/controller/subscriber_test.rb +++ b/actionpack/test/controller/subscriber_test.rb @@ -63,13 +63,19 @@ module ActionControllerSubscriberTest ActionController::Base.logger = logger end + def test_start_processing + get :show + wait + assert_equal 2, logs.size + assert_equal "Processing by Another::SubscribersController#show as HTML", logs.first + end + def test_process_action get :show wait - assert_equal 1, logs.size - assert_match /Completed/, logs.first - assert_match /\[200\]/, logs.first - assert_match /Another::SubscribersController#show/, logs.first + assert_equal 2, logs.size + assert_match /Completed/, logs.last + assert_match /with 200/, logs.last end def test_process_action_without_parameters @@ -82,14 +88,14 @@ module ActionControllerSubscriberTest get :show, :id => '10' wait - assert_equal 2, logs.size - assert_equal 'Parameters: {"id"=>"10"}', logs[0] + assert_equal 3, logs.size + assert_equal 'Parameters: {"id"=>"10"}', logs[1] end def test_process_action_with_view_runtime get :show wait - assert_match /\(Views: [\d\.]+ms\)/, logs[0] + assert_match /\(Views: [\d\.]+ms\)/, logs[1] end def test_process_action_with_filter_parameters @@ -98,7 +104,7 @@ module ActionControllerSubscriberTest get :show, :lifo => 'Pratik', :amount => '420', :step => '1' wait - params = logs[0] + params = logs[1] assert_match /"amount"=>"\[FILTERED\]"/, params assert_match /"lifo"=>"\[FILTERED\]"/, params assert_match /"step"=>"1"/, params @@ -108,34 +114,34 @@ module ActionControllerSubscriberTest get :redirector wait - assert_equal 2, logs.size - assert_equal "Redirected to http://foo.bar/", logs[0] + assert_equal 3, logs.size + assert_equal "Redirected to http://foo.bar/", logs[1] end def test_send_data get :data_sender wait - assert_equal 2, logs.size - assert_match /Sent data omg\.txt/, logs[0] + assert_equal 3, logs.size + assert_match /Sent data omg\.txt/, logs[1] end def test_send_file get :file_sender wait - assert_equal 2, logs.size - assert_match /Sent file/, logs[0] - assert_match /test\/fixtures\/company\.rb/, logs[0] + assert_equal 3, logs.size + assert_match /Sent file/, logs[1] + assert_match /test\/fixtures\/company\.rb/, logs[1] end def test_send_xfile get :xfile_sender wait - assert_equal 2, logs.size - assert_match /Sent X\-Sendfile header/, logs[0] - assert_match /test\/fixtures\/company\.rb/, logs[0] + assert_equal 3, logs.size + assert_match /Sent X\-Sendfile header/, logs[1] + assert_match /test\/fixtures\/company\.rb/, logs[1] end def test_with_fragment_cache @@ -143,9 +149,9 @@ module ActionControllerSubscriberTest get :with_fragment_cache wait - assert_equal 3, logs.size - assert_match /Exist fragment\? views\/foo/, logs[0] - assert_match /Write fragment views\/foo/, logs[1] + assert_equal 4, logs.size + assert_match /Exist fragment\? views\/foo/, logs[1] + assert_match /Write fragment views\/foo/, logs[2] ensure ActionController::Base.perform_caching = true end @@ -155,9 +161,9 @@ module ActionControllerSubscriberTest get :with_page_cache wait - assert_equal 2, logs.size - assert_match /Write page/, logs[0] - assert_match /\/index\.html/, logs[0] + assert_equal 3, logs.size + assert_match /Write page/, logs[1] + assert_match /\/index\.html/, logs[1] ensure ActionController::Base.perform_caching = true end diff --git a/actionpack/test/dispatch/request/json_params_parsing_test.rb b/actionpack/test/dispatch/request/json_params_parsing_test.rb index d3308f73cc..0faa99a912 100644 --- a/actionpack/test/dispatch/request/json_params_parsing_test.rb +++ b/actionpack/test/dispatch/request/json_params_parsing_test.rb @@ -35,7 +35,7 @@ class JsonParamsParsingTest < ActionController::IntegrationTest begin $stderr = StringIO.new json = "[\"person]\": {\"name\": \"David\"}}" - post "/parse", json, {'CONTENT_TYPE' => 'application/json'} + post "/parse", json, {'CONTENT_TYPE' => 'application/json', 'action_dispatch.show_exceptions' => true} assert_response :error $stderr.rewind && err = $stderr.read assert err =~ /Error occurred while parsing request parameters/ diff --git a/actionpack/test/dispatch/request/xml_params_parsing_test.rb b/actionpack/test/dispatch/request/xml_params_parsing_test.rb index 96189e4ca2..488799ac2a 100644 --- a/actionpack/test/dispatch/request/xml_params_parsing_test.rb +++ b/actionpack/test/dispatch/request/xml_params_parsing_test.rb @@ -43,7 +43,7 @@ class XmlParamsParsingTest < ActionController::IntegrationTest begin $stderr = StringIO.new xml = "<person><name>David</name><avatar type='file' name='me.jpg' content_type='image/jpg'>#{ActiveSupport::Base64.encode64('ABC')}</avatar></pineapple>" - post "/parse", xml, default_headers + post "/parse", xml, default_headers.merge('action_dispatch.show_exceptions' => true) assert_response :error $stderr.rewind && err = $stderr.read assert err =~ /Error occurred while parsing request parameters/ diff --git a/actionpack/test/dispatch/show_exceptions_test.rb b/actionpack/test/dispatch/show_exceptions_test.rb index def86c8323..97da680f17 100644 --- a/actionpack/test/dispatch/show_exceptions_test.rb +++ b/actionpack/test/dispatch/show_exceptions_test.rb @@ -38,15 +38,15 @@ class ShowExceptionsTest < ActionController::IntegrationTest @app = ProductionApp self.remote_addr = '208.77.188.166' - get "/" + get "/", {}, {'action_dispatch.show_exceptions' => true} assert_response 500 assert_equal "500 error fixture\n", body - get "/not_found" + get "/not_found", {}, {'action_dispatch.show_exceptions' => true} assert_response 404 assert_equal "404 error fixture\n", body - get "/method_not_allowed" + get "/method_not_allowed", {}, {'action_dispatch.show_exceptions' => true} assert_response 405 assert_equal "", body end @@ -56,15 +56,15 @@ class ShowExceptionsTest < ActionController::IntegrationTest ['127.0.0.1', '::1'].each do |ip_address| self.remote_addr = ip_address - get "/" + get "/", {}, {'action_dispatch.show_exceptions' => true} assert_response 500 assert_match /puke/, body - get "/not_found" + get "/not_found", {}, {'action_dispatch.show_exceptions' => true} assert_response 404 assert_match /#{ActionController::UnknownAction.name}/, body - get "/method_not_allowed" + get "/method_not_allowed", {}, {'action_dispatch.show_exceptions' => true} assert_response 405 assert_match /ActionController::MethodNotAllowed/, body end @@ -78,11 +78,11 @@ class ShowExceptionsTest < ActionController::IntegrationTest @app = ProductionApp self.remote_addr = '208.77.188.166' - get "/" + get "/", {}, {'action_dispatch.show_exceptions' => true} assert_response 500 assert_equal "500 localized error fixture\n", body - get "/not_found" + get "/not_found", {}, {'action_dispatch.show_exceptions' => true} assert_response 404 assert_equal "404 error fixture\n", body ensure @@ -94,15 +94,15 @@ class ShowExceptionsTest < ActionController::IntegrationTest @app = DevelopmentApp self.remote_addr = '208.77.188.166' - get "/" + get "/", {}, {'action_dispatch.show_exceptions' => true} assert_response 500 assert_match /puke/, body - get "/not_found" + get "/not_found", {}, {'action_dispatch.show_exceptions' => true} assert_response 404 assert_match /#{ActionController::UnknownAction.name}/, body - get "/method_not_allowed" + get "/method_not_allowed", {}, {'action_dispatch.show_exceptions' => true} assert_response 405 assert_match /ActionController::MethodNotAllowed/, body end diff --git a/actionpack/test/dispatch/subscriber_test.rb b/actionpack/test/dispatch/subscriber_test.rb index a7f1a2659a..3096c132e7 100644 --- a/actionpack/test/dispatch/subscriber_test.rb +++ b/actionpack/test/dispatch/subscriber_test.rb @@ -78,9 +78,8 @@ module DispatcherSubscriberTest log = @logger.logged(:info).first assert_equal 1, @logger.logged(:info).size - assert_match %r{^Processing "/" to text/html}, log - assert_match %r{\(for 127\.0\.0\.1}, log - assert_match %r{\[GET\]}, log + assert_match %r{^Started GET "/"}, log + assert_match %r{for 127\.0\.0\.1}, log end def test_subscriber_has_its_logged_flushed_after_request |