diff options
| -rw-r--r-- | actionpack/lib/action_dispatch/routing/mapper.rb | 25 | ||||
| -rw-r--r-- | actionpack/lib/action_dispatch/routing/route_set.rb | 6 | ||||
| -rw-r--r-- | actionpack/test/dispatch/routing_test.rb | 36 |
3 files changed, 46 insertions, 21 deletions
diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index aab272f565..430f6fc5a3 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -65,6 +65,16 @@ module ActionDispatch end end + if path.match(':controller') + raise ArgumentError, ":controller segment is not allowed within a namespace block" if @scope[:module] + + # Add a default constraint for :controller path segments that matches namespaced + # controllers with default routes like :controller/:action/:id(.:format), e.g: + # GET /admin/products/show/1 + # => { :controller => 'admin/products', :action => 'show', :id => '1' } + options.reverse_merge!(:controller => /.+?/) + end + path = normalize_path(path) path_without_format = path.sub(/\(\.:format\)$/, '') @@ -93,7 +103,7 @@ module ActionDispatch def app Constraints.new( - to.respond_to?(:call) ? to : Routing::RouteSet::Dispatcher.new(:defaults => defaults, :module => @scope[:module]), + to.respond_to?(:call) ? to : Routing::RouteSet::Dispatcher.new(:defaults => defaults), blocks, @set.request_class ) @@ -135,8 +145,11 @@ module ActionDispatch defaults[:controller] ||= default_controller defaults[:action] ||= default_action - defaults.delete(:controller) if defaults[:controller].blank? - defaults.delete(:action) if defaults[:action].blank? + defaults.delete(:controller) if defaults[:controller].blank? || defaults[:controller].is_a?(Regexp) + defaults.delete(:action) if defaults[:action].blank? || defaults[:action].is_a?(Regexp) + + defaults[:controller] = defaults[:controller].to_s if defaults.key?(:controller) + defaults[:action] = defaults[:action].to_s if defaults.key?(:action) if defaults[:controller].blank? && segment_keys.exclude?("controller") raise ArgumentError, "missing :controller" @@ -185,15 +198,15 @@ module ActionDispatch def default_controller if @options[:controller] - @options[:controller].to_s + @options[:controller] elsif @scope[:controller] - @scope[:controller].to_s + @scope[:controller] end end def default_action if @options[:action] - @options[:action].to_s + @options[:action] end end end diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index 01a068a9f2..1b1a221c60 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -14,7 +14,6 @@ module ActionDispatch def initialize(options={}) @defaults = options[:defaults] @glob_param = options.delete(:glob) - @module = options.delete(:module) @controllers = {} end @@ -39,12 +38,11 @@ module ActionDispatch # 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 + # 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 = @module && !default_controller ? - "#{@module}/#{params[:controller]}" : params[:controller] + controller_param = params[:controller] controller_reference(controller_param) end rescue NameError => e diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index 90d05d4a67..2a014bf976 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -323,7 +323,7 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end end - match "whatever/:controller(/:action(/:id))" + match "whatever/:controller(/:action(/:id))", :id => /\d+/ resource :profile do get :settings @@ -349,7 +349,6 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest namespace :private do root :to => redirect('/private/index') match "index", :to => 'private#index' - match ":controller(/:action(/:id))" end scope :only => [:index, :show] do @@ -527,15 +526,14 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest 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 + assert_raise(ArgumentError) do + self.class.stub_controllers do |routes| + routes.draw do + namespace :admin do + match '/:controller(/:action(/:id(.:format)))' + end + end + end end end @@ -1354,6 +1352,22 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end end + def test_url_generator_for_namespaced_generic_route + with_test_routes do + get 'whatever/foo/bar/show' + assert_equal 'foo/bar#show', @response.body + + get 'whatever/foo/bar/show/1' + assert_equal 'foo/bar#show', @response.body + + assert_equal 'http://www.example.com/whatever/foo/bar/show', + url_for(:controller => "foo/bar", :action => "show") + + assert_equal 'http://www.example.com/whatever/foo/bar/show/1', + url_for(:controller => "foo/bar", :action => "show", :id => '1') + end + end + def test_assert_recognizes_account_overview with_test_routes do assert_recognizes({:controller => "account", :action => "overview"}, "/account/overview") |