diff options
author | José Valim <jose.valim@gmail.com> | 2010-07-02 19:13:00 +0200 |
---|---|---|
committer | José Valim <jose.valim@gmail.com> | 2010-07-02 19:13:00 +0200 |
commit | 9e6e64873243ee707ecad3d523ceb0dbae0617cd (patch) | |
tree | bc3cf0338ab01a3d6b74e6ee42d4dcc8203fa5ae | |
parent | 0189fb76e3c38ef9a0d292135683b0d135c4a369 (diff) | |
download | rails-9e6e64873243ee707ecad3d523ceb0dbae0617cd.tar.gz rails-9e6e64873243ee707ecad3d523ceb0dbae0617cd.tar.bz2 rails-9e6e64873243ee707ecad3d523ceb0dbae0617cd.zip |
Fix routes with :controller segment when namespaced [#5034 state:resolved]
-rw-r--r-- | actionpack/lib/action_dispatch/routing/mapper.rb | 2 | ||||
-rw-r--r-- | actionpack/lib/action_dispatch/routing/route_set.rb | 50 | ||||
-rw-r--r-- | actionpack/test/abstract_unit.rb | 17 | ||||
-rw-r--r-- | actionpack/test/dispatch/routing_test.rb | 24 |
4 files changed, 63 insertions, 30 deletions
diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 0d760ed23a..d71005a852 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -91,7 +91,7 @@ module ActionDispatch def app Constraints.new( - to.respond_to?(:call) ? to : Routing::RouteSet::Dispatcher.new(:defaults => defaults), + to.respond_to?(:call) ? to : Routing::RouteSet::Dispatcher.new(:defaults => defaults, :module => @scope[:module]), blocks, @set.request_class ) diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index 7248ed03f6..afa312889f 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -14,6 +14,7 @@ module ActionDispatch def initialize(options={}) @defaults = options[:defaults] @glob_param = options.delete(:glob) + @module = options.delete(:module) @controllers = {} end @@ -26,7 +27,7 @@ module ActionDispatch return [404, {'X-Cascade' => 'pass'}, []] end - controller.action(params[:action]).call(env) + dispatch(controller, params[:action], env) end def prepare_params!(params) @@ -34,29 +35,44 @@ module ActionDispatch split_glob_param!(params) if @glob_param end - def controller(params, raise_error=true) + # If this is a default_controller (i.e. a controller specified by the user) + # we should raise an error in case it's not found, because it usually means + # an user error. However, if the controller was retrieved through a dynamic + # segment, as in :controller(/:action), we should simply return nil and + # delegate the control back to Rack cascade. Besides, if this is not a default + # controller, it means we should respect the @scope[:module] parameter. + def controller(params, default_controller=true) if params && params.key?(:controller) - controller_param = params[:controller] - unless controller = @controllers[controller_param] - controller_name = "#{controller_param.camelize}Controller" - controller = @controllers[controller_param] = - ActiveSupport::Dependencies.ref(controller_name) - end - - controller.get + controller_param = @module && !default_controller ? + "#{@module}/#{params[:controller]}" : params[:controller] + controller_reference(controller_param) end rescue NameError => e - raise ActionController::RoutingError, e.message, e.backtrace if raise_error + raise ActionController::RoutingError, e.message, e.backtrace if default_controller end - private - def merge_default_action!(params) - params[:action] ||= 'index' - end + private - def split_glob_param!(params) - params[@glob_param] = params[@glob_param].split('/').map { |v| URI.unescape(v) } + def controller_reference(controller_param) + unless controller = @controllers[controller_param] + controller_name = "#{controller_param.camelize}Controller" + controller = @controllers[controller_param] = + ActiveSupport::Dependencies.ref(controller_name) end + controller.get + end + + def dispatch(controller, action, env) + controller.action(action).call(env) + end + + def merge_default_action!(params) + params[:action] ||= 'index' + end + + def split_glob_param!(params) + params[@glob_param] = params[@glob_param].split('/').map { |v| URI.unescape(v) } + end end # A NamedRouteCollection instance is a collection of named routes, and also diff --git a/actionpack/test/abstract_unit.rb b/actionpack/test/abstract_unit.rb index 89c4cb46a1..5be47f7c96 100644 --- a/actionpack/test/abstract_unit.rb +++ b/actionpack/test/abstract_unit.rb @@ -182,13 +182,16 @@ class ActionController::IntegrationTest < ActiveSupport::TestCase self.app = build_app - class StubDispatcher - def self.new(*args) - lambda { |env| - params = env['action_dispatch.request.path_parameters'] - controller, action = params[:controller], params[:action] - [200, {'Content-Type' => 'text/html'}, ["#{controller}##{action}"]] - } + # Stub Rails dispatcher so it does not get controller references and + # simply return the controller#action as Rack::Body. + class StubDispatcher < ::ActionDispatch::Routing::RouteSet::Dispatcher + protected + def controller_reference(controller_param) + controller_param + end + + def dispatch(controller, action, env) + [200, {'Content-Type' => 'text/html'}, ["#{controller}##{action}"]] end end diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index 26bd641cd6..1bfc92aa3d 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -310,11 +310,6 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest match '/' => 'mes#index' end - namespace :private do - root :to => redirect('/private/index') - match "index", :to => 'private#index' - end - get "(/:username)/followers" => "followers#index" get "/groups(/user/:username)" => "groups#index" get "(/user/:username)/photos" => "photos#index" @@ -348,6 +343,12 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end end + namespace :private do + root :to => redirect('/private/index') + match "index", :to => 'private#index' + match ":controller(/:action(/:id))" + end + match '/:locale/*file.:format', :to => 'files#show', :file => /path\/to\/existing\/file/ end end @@ -470,6 +471,19 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end end + def test_namespace_with_controller_segment + with_test_routes do + get '/private/foo' + assert_equal 'private/foo#index', @response.body + + get '/private/foo/bar' + assert_equal 'private/foo#bar', @response.body + + get '/private/foo/bar/1' + assert_equal 'private/foo#bar', @response.body + end + end + def test_session_singleton_resource with_test_routes do get '/session' |