diff options
Diffstat (limited to 'actionpack')
| -rw-r--r-- | actionpack/CHANGELOG.md | 7 | ||||
| -rw-r--r-- | actionpack/lib/action_dispatch/journey/router.rb | 3 | ||||
| -rw-r--r-- | actionpack/lib/action_dispatch/routing/mapper.rb | 16 | ||||
| -rw-r--r-- | actionpack/test/controller/resources_test.rb | 22 | ||||
| -rw-r--r-- | actionpack/test/dispatch/routing_test.rb | 16 |
5 files changed, 52 insertions, 12 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 90cf989100..740c6db06f 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,10 @@ +* Raise an error on root route naming conflicts. + + Raises an ArgumentError when multiple root routes are defined in the + same context instead of assigning nil names to subsequent roots. + + *Gannon McGibbon* + * Allow rescue from parameter parse errors: ``` diff --git a/actionpack/lib/action_dispatch/journey/router.rb b/actionpack/lib/action_dispatch/journey/router.rb index 30af3ff930..89a164f968 100644 --- a/actionpack/lib/action_dispatch/journey/router.rb +++ b/actionpack/lib/action_dispatch/journey/router.rb @@ -15,9 +15,6 @@ require "action_dispatch/journey/path/pattern" module ActionDispatch module Journey # :nodoc: class Router # :nodoc: - class RoutingError < ::StandardError # :nodoc: - end - attr_accessor :routes def initialize(routes) diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 99f3b4c2cd..421e2023c2 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -656,7 +656,7 @@ module ActionDispatch # Query if the following named route was already defined. def has_named_route?(name) - @set.named_routes.key? name + @set.named_routes.key?(name) end private @@ -1171,10 +1171,16 @@ module ActionDispatch end def actions + if @except + available_actions - Array(@except).map(&:to_sym) + else + available_actions + end + end + + def available_actions if @only Array(@only).map(&:to_sym) - elsif @except - default_actions - Array(@except).map(&:to_sym) else default_actions end @@ -1946,9 +1952,7 @@ module ActionDispatch end def match_root_route(options) - name = has_named_route?(name_for_action(:root, nil)) ? nil : :root - args = ["/", { as: name, via: :get }.merge!(options)] - + args = ["/", { as: :root, via: :get }.merge(options)] match(*args) end end diff --git a/actionpack/test/controller/resources_test.rb b/actionpack/test/controller/resources_test.rb index d336b96eff..d2146f12a5 100644 --- a/actionpack/test/controller/resources_test.rb +++ b/actionpack/test/controller/resources_test.rb @@ -853,6 +853,28 @@ class ResourcesTest < ActionController::TestCase end end + def test_resource_has_show_action_but_does_not_have_destroy_action + with_routing do |set| + set.draw do + resources :products, only: [:show, :destroy], except: :destroy + end + + assert_resource_allowed_routes("products", {}, { id: "1" }, :show, [:index, :new, :create, :edit, :update, :destroy]) + assert_resource_allowed_routes("products", { format: "xml" }, { id: "1" }, :show, [:index, :new, :create, :edit, :update, :destroy]) + end + end + + def test_singleton_resource_has_show_action_but_does_not_have_destroy_action + with_routing do |set| + set.draw do + resource :account, only: [:show, :destroy], except: :destroy + end + + assert_singleton_resource_allowed_routes("accounts", {}, :show, [:new, :create, :edit, :update, :destroy]) + assert_singleton_resource_allowed_routes("accounts", { format: "xml" }, :show, [:new, :create, :edit, :update, :destroy]) + end + end + def test_resource_has_only_create_action_and_named_route with_routing do |set| set.draw do diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index affc2d8497..4dffbd0db1 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -3698,15 +3698,25 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end end - def test_multiple_roots + def test_multiple_roots_raises_error + ex = assert_raises(ArgumentError) { + draw do + root "pages#index", constraints: { host: "www.example.com" } + root "admin/pages#index", constraints: { host: "admin.example.com" } + end + } + assert_match(/Invalid route name, already in use: 'root'/, ex.message) + end + + def test_multiple_named_roots draw do namespace :foo do root "pages#index", constraints: { host: "www.example.com" } - root "admin/pages#index", constraints: { host: "admin.example.com" } + root "admin/pages#index", constraints: { host: "admin.example.com" }, as: :admin_root end root "pages#index", constraints: { host: "www.example.com" } - root "admin/pages#index", constraints: { host: "admin.example.com" } + root "admin/pages#index", constraints: { host: "admin.example.com" }, as: :admin_root end get "http://www.example.com/foo" |