Permalink
Browse files

Merge pull request #1090 from dchelimsky/issue-1089

3.1: params_wrapper raises error on anynomous controllers
  • Loading branch information...
2 parents f627da1 + 13950a8 commit f1e1e7658bbee698a75219b4c9d5e24a62220426 @josevalim josevalim committed May 17, 2011
@@ -140,6 +140,8 @@ def inherited(klass)
# This method also does namespace lookup. Foo::Bar::UsersController will
# try to find Foo::Bar::User, Foo::User and finally User.
def _default_wrap_model #:nodoc:
+ return nil if self.name.nil?
+
model_name = self.name.sub(/Controller$/, '').singularize
begin
@@ -168,7 +170,7 @@ def _set_wrapper_defaults(options, model=nil)
end
end
- unless options[:name]
+ unless options[:name] || self.name.nil?
model ||= _default_wrap_model
options[:name] = model ? model.to_s.demodulize.underscore :
controller_name.singularize
@@ -226,7 +228,7 @@ def _wrap_parameters(parameters)
# Checks if we should perform parameters wrapping.
def _wrapper_enabled?
ref = request.content_mime_type.try(:ref)
- _wrapper_formats.include?(ref) && !request.request_parameters[_wrapper_key]
+ _wrapper_formats.include?(ref) && _wrapper_key && !request.request_parameters[_wrapper_key]
end
end
end
@@ -413,7 +413,11 @@ def process(action, parameters = nil, session = nil, flash = nil, http_method =
@request.env['REQUEST_METHOD'] = http_method
parameters ||= {}
- @request.assign_parameters(@routes, @controller.class.name.underscore.sub(/_controller$/, ''), action.to_s, parameters)
+ controller_class_name = @controller.class.name ?
+ @controller.class.name.underscore.sub(/_controller$/, '') :
+ "anonymous_controller"
+
+ @request.assign_parameters(@routes, controller_class_name, action.to_s, parameters)
@request.session = ActionController::TestSession.new(session) if session
@request.session["flash"] = @request.flash.update(flash || {})
@@ -2,7 +2,21 @@
module Admin; class User; end; end
+module ParamsWrapperTestHelp
+ def with_default_wrapper_options(&block)
+ @controller.class._wrapper_options = {:format => [:json]}
+ @controller.class.inherited(@controller.class)
+ yield
+ end
+
+ def assert_parameters(expected)
+ assert_equal expected, self.class.controller_class.last_parameters
+ end
+end
+
class ParamsWrapperTest < ActionController::TestCase
+ include ParamsWrapperTestHelp
+
class UsersController < ActionController::Base
class << self
attr_accessor :last_parameters
@@ -166,20 +180,11 @@ def test_not_wrapping_abstract_model
assert_parameters({ 'username' => 'sikachu', 'title' => 'Developer', 'user' => { 'username' => 'sikachu', 'title' => 'Developer' }})
end
end
-
- private
- def with_default_wrapper_options(&block)
- @controller.class._wrapper_options = {:format => [:json]}
- @controller.class.inherited(@controller.class)
- yield
- end
-
- def assert_parameters(expected)
- assert_equal expected, UsersController.last_parameters
- end
end
class NamespacedParamsWrapperTest < ActionController::TestCase
+ include ParamsWrapperTestHelp
+
module Admin
module Users
class UsersController < ActionController::Base;
@@ -247,14 +252,36 @@ def test_hierarchy_namespace_lookup_from_model
end
end
- private
- def with_default_wrapper_options(&block)
- @controller.class._wrapper_options = {:format => [:json]}
- @controller.class.inherited(@controller.class)
- yield
+end
+
+class AnonymousControllerParamsWrapperTest < ActionController::TestCase
+ include ParamsWrapperTestHelp
+
+ tests(Class.new(ActionController::Base) do
+ class << self
+ attr_accessor :last_parameters
end
- def assert_parameters(expected)
- assert_equal expected, Admin::Users::UsersController.last_parameters
+ def parse
+ self.class.last_parameters = request.params.except(:controller, :action)
+ head :ok
+ end
+ end)
+
+ def test_does_not_implicitly_wrap_params
+ with_default_wrapper_options do
+ @request.env['CONTENT_TYPE'] = 'application/json'
+ post :parse, { 'username' => 'sikachu' }
+ assert_parameters({ 'username' => 'sikachu' })
end
+ end
+
+ def test_does_wrap_params_if_name_provided
+ with_default_wrapper_options do
+ @controller.class.wrap_parameters(:name => "guest")
+ @request.env['CONTENT_TYPE'] = 'application/json'
+ post :parse, { 'username' => 'sikachu' }
+ assert_parameters({ 'username' => 'sikachu', 'guest' => { 'username' => 'sikachu' }})
+ end
+ end
end

3 comments on commit f1e1e76

Owner

fxn commented on f1e1e76 May 17, 2011

Hey, #name behaves in a different way in 1.9 and 1.8 for anonymous classes or modules. AS provides Module#anonymous? as a portable and more declarative predicate to test that. Just a comment, I can revise that myself if you want.

Contributor

dchelimsky replied May 17, 2011

@fxn - that sounds great - I'd be happy to update it. @josevalim - would you accept one more patch on this?

Member

josevalim replied May 17, 2011

No need to worry, I have just pushed a fix. :D

Please sign in to comment.