Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Deprecate router generation "best match" sorting

  • Loading branch information...
commit ff643ce9679ad66a5cd79c90c096496d07d6efaf 1 parent ebb6606
@josh josh authored
View
4 actionpack/lib/action_controller/resources.rb
@@ -526,13 +526,13 @@ def map_resource(entities, options = {}, &block)
resource = Resource.new(entities, options)
with_options :controller => resource.controller do |map|
+ map_associations(resource, options)
+
map_collection_actions(map, resource)
map_default_collection_actions(map, resource)
map_new_actions(map, resource)
map_member_actions(map, resource)
- map_associations(resource, options)
-
if block_given?
with_options(options.slice(*INHERITABLE_OPTIONS).merge(:path_prefix => resource.nesting_path_prefix, :name_prefix => resource.nesting_name_prefix), &block)
end
View
32 actionpack/lib/action_controller/routing/route_set.rb
@@ -407,9 +407,24 @@ def generate(options, recall = {}, method=:generate)
# don't use the recalled keys when determining which routes to check
routes = routes_by_controller[controller][action][options.reject {|k,v| !v}.keys.sort_by { |x| x.object_id }]
- routes.each do |route|
+ routes[1].each_with_index do |route, index|
results = route.__send__(method, options, merged, expire_on)
- return results if results && (!results.is_a?(Array) || results.first)
+ if results && (!results.is_a?(Array) || results.first)
+
+ # Compare results with Rails 3.0 behavior
+ if routes[0][index] != route
+ routes[0].each do |route2|
+ new_results = route2.__send__(method, options, merged, expire_on)
+ if new_results && (!new_results.is_a?(Array) || new_results.first)
+ ActiveSupport::Deprecation.warn "The URL you generated will use the first matching route in routes.rb rather than the \"best\" match. " +
+ "In Rails 3.0 #{new_results} would of been generated instead of #{results}"
+ break
+ end
+ end
+ end
+
+ return results
+ end
end
end
@@ -448,7 +463,10 @@ def routes_by_controller
@routes_by_controller ||= Hash.new do |controller_hash, controller|
controller_hash[controller] = Hash.new do |action_hash, action|
action_hash[action] = Hash.new do |key_hash, keys|
- key_hash[keys] = routes_for_controller_and_action_and_keys(controller, action, keys)
+ key_hash[keys] = [
+ routes_for_controller_and_action_and_keys(controller, action, keys),
+ deprecated_routes_for_controller_and_action_and_keys(controller, action, keys)
+ ]
end
end
end
@@ -460,7 +478,7 @@ def routes_for(options, merged, expire_on)
merged = options if expire_on[:controller]
action = merged[:action] || 'index'
- routes_by_controller[controller][action][merged.keys]
+ routes_by_controller[controller][action][merged.keys][1]
end
def routes_for_controller_and_action(controller, action)
@@ -472,6 +490,12 @@ def routes_for_controller_and_action(controller, action)
end
def routes_for_controller_and_action_and_keys(controller, action, keys)
+ routes.select do |route|
+ route.matches_controller_and_action? controller, action
+ end
+ end
+
+ def deprecated_routes_for_controller_and_action_and_keys(controller, action, keys)
selected = routes.select do |route|
route.matches_controller_and_action? controller, action
end
View
2  actionpack/test/controller/caching_test.rb
@@ -52,7 +52,7 @@ def setup
ActionController::Base.perform_caching = true
ActionController::Routing::Routes.draw do |map|
- map.main '', :controller => 'posts'
+ map.main '', :controller => 'posts', :format => nil
map.formatted_posts 'posts.:format', :controller => 'posts'
map.resources :posts
map.connect ':controller/:action/:id'
View
8 actionpack/test/controller/routing_test.rb
@@ -1610,7 +1610,7 @@ def order_query_string(qs)
end
end
-class RouteSetTest < Test::Unit::TestCase
+class RouteSetTest < ActiveSupport::TestCase
def set
@set ||= ROUTING::RouteSet.new
end
@@ -2191,8 +2191,10 @@ def test_generate_finds_best_fit
map.connect "/ws/people", :controller => "people", :action => "index", :ws => true
end
- url = set.generate(:controller => "people", :action => "index", :ws => true)
- assert_equal "/ws/people", url
+ assert_deprecated {
+ url = set.generate(:controller => "people", :action => "index", :ws => true)
+ assert_equal "/ws/people", url
+ }
end
def test_generate_changes_controller_module
View
6 actionpack/test/controller/url_rewriter_test.rb
@@ -304,7 +304,7 @@ def test_path_generation_for_symbol_parameter_keys
def test_named_routes_with_nil_keys
ActionController::Routing::Routes.clear!
ActionController::Routing::Routes.draw do |map|
- map.main '', :controller => 'posts'
+ map.main '', :controller => 'posts', :format => nil
map.resources :posts
map.connect ':controller/:action/:id'
end
@@ -314,9 +314,9 @@ def test_named_routes_with_nil_keys
controller = kls.new
params = {:action => :index, :controller => :posts, :format => :xml}
- assert_equal("http://www.basecamphq.com/posts.xml", controller.send(:url_for, params))
+ assert_equal("http://www.basecamphq.com/posts.xml", controller.send(:url_for, params))
params[:format] = nil
- assert_equal("http://www.basecamphq.com/", controller.send(:url_for, params))
+ assert_equal("http://www.basecamphq.com/", controller.send(:url_for, params))
ensure
ActionController::Routing::Routes.load!
end
Please sign in to comment.
Something went wrong with that request. Please try again.