From 4bddc06e83acecce662b4282159c5eb0096c4783 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Tue, 3 May 2011 00:37:40 +0200 Subject: Move most processing to load time for performance and improve test suite. --- .../lib/action_controller/metal/params_wrapper.rb | 74 ++++++++++++++-------- actionpack/test/controller/params_wrapper_test.rb | 39 ++++++++---- 2 files changed, 76 insertions(+), 37 deletions(-) diff --git a/actionpack/lib/action_controller/metal/params_wrapper.rb b/actionpack/lib/action_controller/metal/params_wrapper.rb index 7827ed598e..3262e24f67 100644 --- a/actionpack/lib/action_controller/metal/params_wrapper.rb +++ b/actionpack/lib/action_controller/metal/params_wrapper.rb @@ -1,4 +1,6 @@ require 'active_support/core_ext/class/attribute' +require 'active_support/core_ext/hash/slice' +require 'active_support/core_ext/array/wrap' require 'action_dispatch/http/mime_types' module ActionController @@ -96,23 +98,25 @@ module ActionController # ==== Options # * :format - The list of formats in which the parameters wrapper # will be enabled. - # * :only - The list of attribute names which parmeters wrapper + # * :only - The list of attribute names which parameters wrapper # will wrap into a nested hash. - # * :only - The list of attribute names which parmeters wrapper + # * :except - The list of attribute names which parameters wrapper # will exclude from a nested hash. def wrap_parameters(name_or_model_or_options, options = {}) - if !name_or_model_or_options.is_a? Hash - if name_or_model_or_options != false - options = options.merge(:name_or_model => name_or_model_or_options) - else - options = opions.merge(:format => []) - end - else + model = nil + + case name_or_model_or_options + when Hash options = name_or_model_or_options + when false + options = options.merge(:format => []) + when Symbol, String + options = options.merge(:name => name_or_model_or_options) + else + model = name_or_model_or_options end - options[:name_or_model] ||= _default_wrap_model - self._wrapper_options = self._wrapper_options.merge(options) + _set_wrapper_defaults(_wrapper_options.slice(:format).merge(options), model) end # Sets the default wrapper key or model which will be used to determine @@ -120,11 +124,13 @@ module ActionController # module is inherited. def inherited(klass) if klass._wrapper_options[:format].present? - klass._wrapper_options = klass._wrapper_options.merge(:name_or_model => klass._default_wrap_model) + klass._set_wrapper_defaults(klass._wrapper_options) end super end + protected + # Determine the wrapper model from the controller's name. By convention, # this could be done by trying to find the defined model that has the # same singularize name as the controller. For example, +UsersController+ @@ -142,6 +148,29 @@ module ActionController model_klass end + + def _set_wrapper_defaults(options, model=nil) + options = options.dup + + unless options[:only] || options[:except] + model ||= _default_wrap_model + if model.respond_to?(:column_names) + options[:only] = model.column_names + end + end + + unless options[:name] + model ||= _default_wrap_model + options[:name] = model ? model.to_s.demodulize.underscore : + controller_name.singularize + end + + options[:only] = Array.wrap(options[:only]).collect(&:to_s) if options[:only] + options[:except] = Array.wrap(options[:except]).collect(&:to_s) if options[:except] + options[:format] = Array.wrap(options[:format]) + + self._wrapper_options = options + end end # Performs parameters wrapping upon the request. Will be called automatically @@ -164,21 +193,15 @@ module ActionController private # Returns the wrapper key which will use to stored wrapped parameters. def _wrapper_key - @_wrapper_key ||= if _wrapper_options[:name_or_model] - _wrapper_options[:name_or_model].to_s.demodulize.underscore - else - self.class.controller_name.singularize - end + _wrapper_options[:name] end # Returns the list of parameters which will be selected for wrapped. def _wrapped_keys - @_wrapped_keys ||= if _wrapper_options[:only] - Array(_wrapper_options[:only]).collect(&:to_s) - elsif _wrapper_options[:except] - request.request_parameters.keys - Array(_wrapper_options[:except]).collect(&:to_s) - EXCLUDE_PARAMETERS - elsif _wrapper_options[:name_or_model].respond_to?(:column_names) - _wrapper_options[:name_or_model].column_names + @_wrapped_keys ||= if only = _wrapper_options[:only] + only + elsif except = _wrapper_options[:except] + request.request_parameters.keys - except - EXCLUDE_PARAMETERS else request.request_parameters.keys - EXCLUDE_PARAMETERS end @@ -186,12 +209,13 @@ module ActionController # Returns the list of enabled formats. def _wrapper_formats - Array(_wrapper_options[:format]) + _wrapper_options[:format] end # Checks if we should perform parameters wrapping. def _wrapper_enabled? - _wrapper_formats.any?{ |format| format == request.content_mime_type.try(:ref) } && request.request_parameters[_wrapper_key].nil? + ref = request.content_mime_type.try(:ref) + _wrapper_formats.any? { |format| format == ref } && !request.request_parameters[_wrapper_key] end end end diff --git a/actionpack/test/controller/params_wrapper_test.rb b/actionpack/test/controller/params_wrapper_test.rb index 2e5d096fcd..314b27cf47 100644 --- a/actionpack/test/controller/params_wrapper_test.rb +++ b/actionpack/test/controller/params_wrapper_test.rb @@ -80,6 +80,15 @@ class ParamsWrapperTest < ActionController::TestCase end end + def test_wrap_parameters_false + with_default_wrapper_options do + UsersController.wrap_parameters false + @request.env['CONTENT_TYPE'] = 'application/json' + post :test, { 'username' => 'sikachu', 'title' => 'Developer' } + assert_equal '{"username":"sikachu","title":"Developer"}', @response.body + end + end + def test_specify_format with_default_wrapper_options do UsersController.wrap_parameters :format => :xml @@ -115,10 +124,10 @@ class ParamsWrapperTest < ActionController::TestCase end def test_derived_wrapped_keys_from_matching_model - with_default_wrapper_options do - User.expects(:respond_to?).with(:column_names).returns(true) - User.expects(:column_names).returns(["username"]) + User.expects(:respond_to?).with(:column_names).returns(true) + User.expects(:column_names).returns(["username"]) + with_default_wrapper_options do @request.env['CONTENT_TYPE'] = 'application/json' post :test, { 'username' => 'sikachu', 'title' => 'Developer' } assert_equal '{"username":"sikachu","title":"Developer","user":{"username":"sikachu"}}', @response.body @@ -153,11 +162,13 @@ class NamespacedParamsWrapperTest < ActionController::TestCase render :json => params.except(:controller, :action) end end + end - class User; end + class Sample + def self.column_names + ["username"] + end end - class User; end - class Person; end tests Admin::UsersController @@ -169,12 +180,16 @@ class NamespacedParamsWrapperTest < ActionController::TestCase end end - def test_namespace_lookup_when_namespaced_model_available - with_default_wrapper_options do - Admin::User.expects(:respond_to?).with(:column_names).returns(false) - - @request.env['CONTENT_TYPE'] = 'application/json' - post :test, { 'username' => 'sikachu' } + def test_namespace_lookup_from_model + Admin.const_set(:User, Class.new(Sample)) + begin + with_default_wrapper_options do + @request.env['CONTENT_TYPE'] = 'application/json' + post :test, { 'username' => 'sikachu', 'title' => 'Developer' } + assert_equal '{"username":"sikachu","title":"Developer","user":{"username":"sikachu"}}', @response.body + end + ensure + Admin.send :remove_const, :User end end -- cgit v1.2.3