diff options
| -rw-r--r-- | actionpack/lib/action_controller/routing.rb | 20 | ||||
| -rw-r--r-- | actionpack/lib/action_controller/routing_optimisation.rb | 22 | ||||
| -rw-r--r-- | actionpack/lib/action_controller/url_rewriter.rb | 6 | ||||
| -rw-r--r-- | actionpack/test/controller/routing_test.rb | 7 | ||||
| -rw-r--r-- | actionpack/test/controller/url_rewriter_test.rb | 4 |
5 files changed, 47 insertions, 12 deletions
diff --git a/actionpack/lib/action_controller/routing.rb b/actionpack/lib/action_controller/routing.rb index adeb70235c..faaecb8964 100644 --- a/actionpack/lib/action_controller/routing.rb +++ b/actionpack/lib/action_controller/routing.rb @@ -342,7 +342,7 @@ module ActionController # Indicates whether the routes should be optimised with the string interpolation # version of the named routes methods. def optimise? - @optimise + @optimise && ActionController::Routing::optimise_named_routes end def segment_keys @@ -1004,8 +1004,7 @@ module ActionController # Routes cannot use the current string interpolation method # if there are user-supplied :requirements as the interpolation # code won't raise RoutingErrors when generating - route.optimise = !options.key?(:requirements) && ActionController::Routing.optimise_named_routes - + route.optimise = !options.key?(:requirements) if !route.significant_keys.include?(:action) && !route.requirements[:action] route.requirements[:action] = "index" route.significant_keys << :action @@ -1119,7 +1118,16 @@ module ActionController routes.length end - def install(destinations = [ActionController::Base, ActionView::Base]) + def reset! + old_routes = routes.dup + clear! + old_routes.each do |name, route| + add(name, route) + end + end + + def install(destinations = [ActionController::Base, ActionView::Base], regenerate = false) + reset! if regenerate Array(destinations).each { |dest| dest.send :include, @module } end @@ -1219,9 +1227,9 @@ module ActionController @routes_by_controller = nil end - def install_helpers(destinations = [ActionController::Base, ActionView::Base]) + def install_helpers(destinations = [ActionController::Base, ActionView::Base], regenerate_code = false) Array(destinations).each { |d| d.send :include, Helpers } - named_routes.install(destinations) + named_routes.install(destinations, regenerate_code) end def empty? diff --git a/actionpack/lib/action_controller/routing_optimisation.rb b/actionpack/lib/action_controller/routing_optimisation.rb index eaff1869dd..7cf85c9b9a 100644 --- a/actionpack/lib/action_controller/routing_optimisation.rb +++ b/actionpack/lib/action_controller/routing_optimisation.rb @@ -13,8 +13,8 @@ module ActionController def generate_optimisation_block(route, kind) return "" unless route.optimise? OPTIMISERS.inject("") do |memo, klazz| - optimiser = klazz.new(route, kind) - memo << "return #{optimiser.generation_code} if #{optimiser.guard_condition}\n" + memo << klazz.new(route, kind).source_code + memo end end @@ -32,6 +32,18 @@ module ActionController def generation_code 'nil' end + + def source_code + if applicable? + "return #{generation_code} if #{guard_condition}\n" + else + "\n" + end + end + + def applicable? + true + end end # Given a route: @@ -89,6 +101,12 @@ module ActionController def generation_code super.insert(-2, '?#{args.last.to_query}') end + + # To avoid generating http://localhost/?host=foo.example.com we + # can't use this optimisation on routes without any segments + def applicable? + route.segment_keys.size > 0 + end end OPTIMISERS = [PositionalArguments, PositionalArgumentsWithAdditionalParams] diff --git a/actionpack/lib/action_controller/url_rewriter.rb b/actionpack/lib/action_controller/url_rewriter.rb index 8248696431..7aae619681 100644 --- a/actionpack/lib/action_controller/url_rewriter.rb +++ b/actionpack/lib/action_controller/url_rewriter.rb @@ -21,9 +21,13 @@ module ActionController self.default_url_options = {} def self.included(base) #:nodoc: - ActionController::Routing::Routes.install_helpers base + original_optimise = ActionController::Routing.optimise_named_routes + ActionController::Routing.optimise_named_routes = false + ActionController::Routing::Routes.install_helpers base, :regenerate base.mattr_accessor :default_url_options base.default_url_options ||= default_url_options + ensure + ActionController::Routing.optimise_named_routes = original_optimise end # Generate a url based on the options provided, default_url_options and the diff --git a/actionpack/test/controller/routing_test.rb b/actionpack/test/controller/routing_test.rb index 793e1d8cdf..c7b7f40e0d 100644 --- a/actionpack/test/controller/routing_test.rb +++ b/actionpack/test/controller/routing_test.rb @@ -189,7 +189,7 @@ class LegacyRouteSetTests < Test::Unit::TestCase end def test_named_route_with_nested_controller - rs.add_named_route :users, 'admin/user', :controller => '/admin/user', :action => 'index' + rs.add_named_route :users, '/admin/user', :controller => '/admin/user', :action => 'index' x = setup_for_named_route assert_equal("http://named.route.test/admin/user", x.send(:users_url)) @@ -198,12 +198,13 @@ class LegacyRouteSetTests < Test::Unit::TestCase uses_mocha "named route optimisation" do def test_optimised_named_route_call_never_uses_url_for rs.add_named_route :users, 'admin/user', :controller => '/admin/user', :action => 'index' + rs.add_named_route :user, 'admin/user/:id', :controller=>'/admin/user', :action=>'show' x = setup_for_named_route x.expects(:url_for).never x.send(:users_url) x.send(:users_path) - x.send(:users_url, :some_arg=>"some_value") - x.send(:users_path, :some_arg=>"some_value") + x.send(:user_url, 2, :foo=>"bar") + x.send(:user_path, 3, :bar=>"foo") end end diff --git a/actionpack/test/controller/url_rewriter_test.rb b/actionpack/test/controller/url_rewriter_test.rb index a49aaa822a..3c1d584d89 100644 --- a/actionpack/test/controller/url_rewriter_test.rb +++ b/actionpack/test/controller/url_rewriter_test.rb @@ -151,6 +151,7 @@ class UrlWriterTests < Test::Unit::TestCase def test_named_route ActionController::Routing::Routes.draw do |map| + map.no_args '/this/is/verbose', :controller => 'home', :action => 'index' map.home '/home/sweet/home/:user', :controller => 'home', :action => 'index' map.connect ':controller/:action/:id' end @@ -163,6 +164,8 @@ class UrlWriterTests < Test::Unit::TestCase controller.send(:home_url, :host => 'www.basecamphq.com', :user => 'again') assert_equal("/home/sweet/home/alabama", controller.send(:home_path, :user => 'alabama', :host => 'unused')) + assert_equal("http://www.basecamphq.com/home/sweet/home/alabama", controller.send(:home_url, :user => 'alabama', :host => 'www.basecamphq.com')) + assert_equal("http://www.basecamphq.com/this/is/verbose", controller.send(:no_args_url, :host=>'www.basecamphq.com')) ensure ActionController::Routing::Routes.load! end @@ -181,6 +184,7 @@ class UrlWriterTests < Test::Unit::TestCase controller.send(:url_for, :controller => 'brave', :action => 'new', :id => 'world', :only_path => true) assert_equal("/home/sweet/home/alabama", controller.send(:home_url, :user => 'alabama', :host => 'unused', :only_path => true)) + assert_equal("/home/sweet/home/alabama", controller.send(:home_path, 'alabama')) ensure ActionController::Routing::Routes.load! end |