Permalink
Browse files

Move most processing to load time for performance and improve test su…

…ite.
  • Loading branch information...
1 parent 3cca866 commit 4bddc06e83acecce662b4282159c5eb0096c4783 @josevalim josevalim committed May 2, 2011
View
74 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,35 +98,39 @@ module ClassMethods
# ==== Options
# * <tt>:format</tt> - The list of formats in which the parameters wrapper
# will be enabled.
- # * <tt>:only</tt> - The list of attribute names which parmeters wrapper
+ # * <tt>:only</tt> - The list of attribute names which parameters wrapper
# will wrap into a nested hash.
- # * <tt>:only</tt> - The list of attribute names which parmeters wrapper
+ # * <tt>:except</tt> - 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
# wrapper key and attribute names. Will be called automatically when the
# 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 @@ def _default_wrap_model
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,34 +193,29 @@ def process_action(*args)
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
end
# 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
View
39 actionpack/test/controller/params_wrapper_test.rb
@@ -80,6 +80,15 @@ def test_not_enabled_format
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 @@ def test_nested_params
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 @@ def test
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 @@ def test_derivered_name_from_controller
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

0 comments on commit 4bddc06

Please sign in to comment.