diff options
| -rw-r--r-- | actionpack/lib/action_dispatch/routing/route_set.rb | 17 | ||||
| -rw-r--r-- | actionpack/lib/action_dispatch/routing/url_for.rb | 7 | ||||
| -rw-r--r-- | actionpack/lib/action_view/helpers/url_helper.rb | 23 | ||||
| -rw-r--r-- | actionpack/test/controller/base_test.rb | 8 | ||||
| -rw-r--r-- | actionpack/test/controller/routing_test.rb | 6 | ||||
| -rw-r--r-- | actionpack/test/template/sprockets_helper_with_routes_test.rb | 4 |
6 files changed, 36 insertions, 29 deletions
diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index 0e5bf46ee4..c4d87ea3d9 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -195,7 +195,7 @@ module ActionDispatch @module.module_eval <<-END_EVAL, __FILE__, __LINE__ + 1 remove_possible_method :#{selector} def #{selector}(*args) - if args.size == #{route.required_parts.size} && !args.last.is_a?(Hash) && _optimized_routes? + if args.size == #{route.required_parts.size} && !args.last.is_a?(Hash) && optimize_routes_generation? options = #{options.inspect}.merge!(url_options) options[:path] = "#{optimized_helper(route)}" ActionDispatch::Http::URL.url_for(options) @@ -216,14 +216,9 @@ module ActionDispatch helpers << selector end - # If we are generating a path helper and we don't have a *path segment. - # We can optimize the routes generation to a string interpolation if - # it meets the appropriated runtime conditions. - # - # TODO We are enabling this only for path helpers, remove the - # kind == :path and fix the failures to enable it for url as well. + # Clause check about when we need to generate an optimized helper. def optimize_helper?(kind, route) #:nodoc: - kind == :path && route.ast.grep(Journey::Nodes::Star).empty? + route.ast.grep(Journey::Nodes::Star).empty? && route.requirements.except(:controller, :action).empty? end # Generates the interpolation to be used in the optimized helper. @@ -364,7 +359,7 @@ module ActionDispatch # Rails.application.routes.url_helpers.url_for(args) @_routes = routes class << self - delegate :url_for, :to => '@_routes' + delegate :url_for, :optimize_routes_generation?, :to => '@_routes' end # Make named_routes available in the module singleton @@ -602,6 +597,10 @@ module ActionDispatch false end + def optimize_routes_generation? + !mounted? && default_url_options.empty? + end + def _generate_prefix(options = {}) nil end diff --git a/actionpack/lib/action_dispatch/routing/url_for.rb b/actionpack/lib/action_dispatch/routing/url_for.rb index 191a2cb995..94db36ce1f 100644 --- a/actionpack/lib/action_dispatch/routing/url_for.rb +++ b/actionpack/lib/action_dispatch/routing/url_for.rb @@ -102,6 +102,9 @@ module ActionDispatch super end + # Hook overriden in controller to add request information + # with `default_url_options`. Application logic should not + # go into url_options. def url_options default_url_options end @@ -152,9 +155,9 @@ module ActionDispatch protected - def _optimized_routes? + def optimize_routes_generation? return @_optimized_routes if defined?(@_optimized_routes) - @_optimized_routes = default_url_options.empty? && !_routes.mounted? && _routes.default_url_options.empty? + @_optimized_routes = _routes.optimize_routes_generation? && default_url_options.empty? end def _with_routes(routes) diff --git a/actionpack/lib/action_view/helpers/url_helper.rb b/actionpack/lib/action_view/helpers/url_helper.rb index 8d7417809b..f4946e65b5 100644 --- a/actionpack/lib/action_view/helpers/url_helper.rb +++ b/actionpack/lib/action_view/helpers/url_helper.rb @@ -23,20 +23,25 @@ module ActionView include ActionDispatch::Routing::UrlFor include TagHelper - def _routes_context - controller - end + # We need to override url_optoins, _routes_context + # and optimize_routes_generation? to consider the controller. - # Need to map default url options to controller one. - # def default_url_options(*args) #:nodoc: - # controller.send(:default_url_options, *args) - # end - # - def url_options + def url_options #:nodoc: return super unless controller.respond_to?(:url_options) controller.url_options end + def _routes_context #:nodoc: + controller + end + protected :_routes_context + + def optimize_routes_generation? #:nodoc: + controller.respond_to?(:optimize_routes_generation?) ? + controller.optimize_routes_generation? : super + end + protected :optimize_routes_generation? + # Returns the URL for the set of +options+ provided. This takes the # same options as +url_for+ in Action Controller (see the # documentation for <tt>ActionController::Base#url_for</tt>). Note that by default diff --git a/actionpack/test/controller/base_test.rb b/actionpack/test/controller/base_test.rb index 791edb9069..7d0609751f 100644 --- a/actionpack/test/controller/base_test.rb +++ b/actionpack/test/controller/base_test.rb @@ -56,7 +56,7 @@ class UrlOptionsController < ActionController::Base end def url_options - super.merge(:host => 'www.override.com', :action => 'new', :locale => 'en') + super.merge(:host => 'www.override.com') end end @@ -183,9 +183,9 @@ class UrlOptionsTest < ActionController::TestCase get :from_view, :route => "from_view_url" - assert_equal 'http://www.override.com/from_view?locale=en', @response.body - assert_equal 'http://www.override.com/from_view?locale=en', @controller.send(:from_view_url) - assert_equal 'http://www.override.com/default_url_options/new?locale=en', @controller.url_for(:controller => 'default_url_options') + assert_equal 'http://www.override.com/from_view', @response.body + assert_equal 'http://www.override.com/from_view', @controller.send(:from_view_url) + assert_equal 'http://www.override.com/default_url_options/index', @controller.url_for(:controller => 'default_url_options') end end diff --git a/actionpack/test/controller/routing_test.rb b/actionpack/test/controller/routing_test.rb index 843ae1a813..2a7c1f86c6 100644 --- a/actionpack/test/controller/routing_test.rb +++ b/actionpack/test/controller/routing_test.rb @@ -59,11 +59,11 @@ end class MockController def self.build(helpers) Class.new do - def url_for(options) + def url_options + options = super options[:protocol] ||= "http" options[:host] ||= "test.host" - - super(options) + options end include helpers diff --git a/actionpack/test/template/sprockets_helper_with_routes_test.rb b/actionpack/test/template/sprockets_helper_with_routes_test.rb index bcbd81a7dd..89b9940eb7 100644 --- a/actionpack/test/template/sprockets_helper_with_routes_test.rb +++ b/actionpack/test/template/sprockets_helper_with_routes_test.rb @@ -17,7 +17,7 @@ class SprocketsHelperWithRoutesTest < ActionView::TestCase def setup super - @controller = BasicController.new + @controller = BasicController.new @assets = Sprockets::Environment.new @assets.append_path(FIXTURES.join("sprockets/app/javascripts")) @@ -34,7 +34,7 @@ class SprocketsHelperWithRoutesTest < ActionView::TestCase test "namespace conflicts on a named route called asset_path" do # Testing this for sanity - asset_path is now a named route! - assert_match asset_path('test_asset'), '/assets/test_asset' + assert_equal asset_path('test_asset'), '/assets/test_asset' assert_match %r{/assets/logo-[0-9a-f]+.png}, path_to_asset("logo.png") |