Permalink
Browse files

Raise an ArgumentError when a clashing named route is defined

  • Loading branch information...
1 parent e61cdf0 commit a2b7c0e69d671294067b625c749be6fdbec7b433 @trevorturk trevorturk committed Mar 20, 2013
View
4 actionpack/CHANGELOG.md
@@ -1,5 +1,9 @@
## Rails 4.0.0 (unreleased) ##
+* Raise an `ArgumentError` when a clashing named route is defined.
+
+ *Trevor Turk*
+
* Allow default url options to accept host with protocol such as `http://`
config.action_mailer.default_url_options = { host: "http://mydomain.com" }
View
9 actionpack/lib/action_dispatch/routing/route_set.rb
@@ -403,11 +403,18 @@ def empty?
def add_route(app, conditions = {}, requirements = {}, defaults = {}, name = nil, anchor = true)
raise ArgumentError, "Invalid route name: '#{name}'" unless name.blank? || name.to_s.match(/^[_a-z]\w*$/i)
+ if name && named_routes[name]
+ raise ArgumentError, "Invalid route name, already in use: '#{name}' \n" \
+ "This conflict might arise if your routes contain `resources :#{name.pluralize}`. " \
+ "If so, you can restrict the routes created with `resources` as explained here: \n" \
+ "http://guides.rubyonrails.org/routing.html#restricting-the-routes-created"
+ end
+
path = build_path(conditions.delete(:path_info), requirements, SEPARATORS, anchor)
conditions = build_conditions(conditions, path.names.map { |x| x.to_sym })
route = @set.add_route(app, path, conditions, defaults, name)
- named_routes[name] = route if name && !named_routes[name]
+ named_routes[name] = route if name
route
end
View
37 actionpack/test/dispatch/routing_test.rb
@@ -2577,22 +2577,6 @@ def test_nested_resource_constraints
assert_raises(ActionController::UrlGenerationError){ list_todo_path(:list_id => '2', :id => '1') }
end
- def test_named_routes_collision_is_avoided_unless_explicitly_given_as
- draw do
- scope :as => "routes" do
- get "/c/:id", :as => :collision, :to => "collision#show"
- get "/collision", :to => "collision#show"
- get "/no_collision", :to => "collision#show", :as => nil
-
- get "/fc/:id", :as => :forced_collision, :to => "forced_collision#show"
- get "/forced_collision", :as => :forced_collision, :to => "forced_collision#show"
- end
- end
-
- assert_equal "/c/1", routes_collision_path(1)
- assert_equal "/fc/1", routes_forced_collision_path(1)
- end
-
def test_redirect_argument_error
routes = Class.new { include ActionDispatch::Routing::Redirection }.new
assert_raises(ArgumentError) { routes.redirect Object.new }
@@ -2604,9 +2588,6 @@ def test_explicitly_avoiding_the_named_route
get "/c/:id", :as => :collision, :to => "collision#show"
get "/collision", :to => "collision#show"
get "/no_collision", :to => "collision#show", :as => nil
-
- get "/fc/:id", :as => :forced_collision, :to => "forced_collision#show"
- get "/forced_collision", :as => :forced_collision, :to => "forced_collision#show"
end
end
@@ -2657,6 +2638,24 @@ def test_invalid_route_name_raises_error
end
end
+ def test_duplicate_route_name_raises_error
+ assert_raise(ArgumentError) do
+ draw do
+ get '/collision', :to => 'collision#show', :as => 'collision'
+ get '/duplicate', :to => 'duplicate#show', :as => 'collision'
+ end
+ end
+ end
+
+ def test_duplicate_route_name_via_resources_raises_error
+ assert_raise(ArgumentError) do
+ draw do
+ resources :collisions
+ get '/collision', :to => 'collision#show', :as => 'collision'
+ end
+ end
+ end
+
def test_nested_route_in_nested_resource
draw do
resources :posts, :only => [:index, :show] do
View
18 guides/source/upgrading_ruby_on_rails.md
@@ -103,32 +103,20 @@ Rails 4.0 extracted Active Resource to its own gem. If you still need the featur
* Rails 4.0 changed how `assert_generates`, `assert_recognizes`, and `assert_routing` work. Now all these assertions raise `Assertion` instead of `ActionController::RoutingError`.
-* Rails 4.0 correctly prefers the first named route defined in `config/routes.rb` if a clashing route is found later. Check the output of `rake routes` before upgrading and remove unused named routes to avoid issues.
+* Rails 4.0 raises an `ArgumentError` if clashing named routes are defined. This can be triggered by explicitly defined named routes or by the `resources` method. Here are two examples that clash with routes named `example_path`:
```ruby
- # config/routes.rb
get 'one' => 'test#example', as: :example
get 'two' => 'test#example', as: :example
-
- # Rails 3
- <%= example_path %> # => '/two'
-
- # Rails 4
- <%= example_path %> # => '/one'
```
```ruby
- # config/routes.rb
resources :examples
get 'clashing/:id' => 'test#example', as: :example
-
- # Rails 3
- <%= example_path(1) %> # => '/clashing/1'
-
- # Rails 4
- <%= example_path(1) %> # => '/examples/1'
```
+In the first case, you can simply avoid using the same name for multiple routes. In the second, you can use the `only` or `except` options provided by the `resources` method to restrict the routes created as detailed in the [Routing Guide](http://guides.rubyonrails.org/routing.html#restricting-the-routes-created).
+
* Rails 4.0 also changed the way unicode character routes are drawn. Now you can draw unicode character routes directly. If you already draw such routes, you must change them, for example:
```ruby

0 comments on commit a2b7c0e

Please sign in to comment.