Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

Optimize url helpers.

  • Loading branch information...
commit cd5dabab95924dfaf3af8c429454f1a46d9665c1 1 parent d7014bc
José Valim josevalim authored
17 actionpack/lib/action_dispatch/routing/route_set.rb
@@ -195,7 +195,7 @@ def define_url_helper(route, name, kind, options)
195 195 @module.module_eval <<-END_EVAL, __FILE__, __LINE__ + 1
196 196 remove_possible_method :#{selector}
197 197 def #{selector}(*args)
198   - if args.size == #{route.required_parts.size} && !args.last.is_a?(Hash) && _optimized_routes?
  198 + if args.size == #{route.required_parts.size} && !args.last.is_a?(Hash) && optimize_routes_generation?
199 199 options = #{options.inspect}.merge!(url_options)
200 200 options[:path] = "#{optimized_helper(route)}"
201 201 ActionDispatch::Http::URL.url_for(options)
@@ -216,14 +216,9 @@ def #{selector}(*args)
216 216 helpers << selector
217 217 end
218 218
219   - # If we are generating a path helper and we don't have a *path segment.
220   - # We can optimize the routes generation to a string interpolation if
221   - # it meets the appropriated runtime conditions.
222   - #
223   - # TODO We are enabling this only for path helpers, remove the
224   - # kind == :path and fix the failures to enable it for url as well.
  219 + # Clause check about when we need to generate an optimized helper.
225 220 def optimize_helper?(kind, route) #:nodoc:
226   - kind == :path && route.ast.grep(Journey::Nodes::Star).empty?
  221 + route.ast.grep(Journey::Nodes::Star).empty? && route.requirements.except(:controller, :action).empty?
227 222 end
228 223
229 224 # Generates the interpolation to be used in the optimized helper.
@@ -364,7 +359,7 @@ def url_helpers
364 359 # Rails.application.routes.url_helpers.url_for(args)
365 360 @_routes = routes
366 361 class << self
367   - delegate :url_for, :to => '@_routes'
  362 + delegate :url_for, :optimize_routes_generation?, :to => '@_routes'
368 363 end
369 364
370 365 # Make named_routes available in the module singleton
@@ -602,6 +597,10 @@ def mounted?
602 597 false
603 598 end
604 599
  600 + def optimize_routes_generation?
  601 + !mounted? && default_url_options.empty?
  602 + end
  603 +
605 604 def _generate_prefix(options = {})
606 605 nil
607 606 end
7 actionpack/lib/action_dispatch/routing/url_for.rb
@@ -102,6 +102,9 @@ def initialize(*)
102 102 super
103 103 end
104 104
  105 + # Hook overriden in controller to add request information
  106 + # with `default_url_options`. Application logic should not
  107 + # go into url_options.
105 108 def url_options
106 109 default_url_options
107 110 end
@@ -152,9 +155,9 @@ def url_for(options = nil)
152 155
153 156 protected
154 157
155   - def _optimized_routes?
  158 + def optimize_routes_generation?
156 159 return @_optimized_routes if defined?(@_optimized_routes)
157   - @_optimized_routes = default_url_options.empty? && !_routes.mounted? && _routes.default_url_options.empty?
  160 + @_optimized_routes = _routes.optimize_routes_generation? && default_url_options.empty?
158 161 end
159 162
160 163 def _with_routes(routes)
23 actionpack/lib/action_view/helpers/url_helper.rb
@@ -23,20 +23,25 @@ module UrlHelper
23 23 include ActionDispatch::Routing::UrlFor
24 24 include TagHelper
25 25
26   - def _routes_context
27   - controller
28   - end
  26 + # We need to override url_optoins, _routes_context
  27 + # and optimize_routes_generation? to consider the controller.
29 28
30   - # Need to map default url options to controller one.
31   - # def default_url_options(*args) #:nodoc:
32   - # controller.send(:default_url_options, *args)
33   - # end
34   - #
35   - def url_options
  29 + def url_options #:nodoc:
36 30 return super unless controller.respond_to?(:url_options)
37 31 controller.url_options
38 32 end
39 33
  34 + def _routes_context #:nodoc:
  35 + controller
  36 + end
  37 + protected :_routes_context
  38 +
  39 + def optimize_routes_generation? #:nodoc:
  40 + controller.respond_to?(:optimize_routes_generation?) ?
  41 + controller.optimize_routes_generation? : super
  42 + end
  43 + protected :optimize_routes_generation?
  44 +
40 45 # Returns the URL for the set of +options+ provided. This takes the
41 46 # same options as +url_for+ in Action Controller (see the
42 47 # documentation for <tt>ActionController::Base#url_for</tt>). Note that by default
8 actionpack/test/controller/base_test.rb
@@ -56,7 +56,7 @@ def from_view
56 56 end
57 57
58 58 def url_options
59   - super.merge(:host => 'www.override.com', :action => 'new', :locale => 'en')
  59 + super.merge(:host => 'www.override.com')
60 60 end
61 61 end
62 62
@@ -183,9 +183,9 @@ def test_url_options_override
183 183
184 184 get :from_view, :route => "from_view_url"
185 185
186   - assert_equal 'http://www.override.com/from_view?locale=en', @response.body
187   - assert_equal 'http://www.override.com/from_view?locale=en', @controller.send(:from_view_url)
188   - assert_equal 'http://www.override.com/default_url_options/new?locale=en', @controller.url_for(:controller => 'default_url_options')
  186 + assert_equal 'http://www.override.com/from_view', @response.body
  187 + assert_equal 'http://www.override.com/from_view', @controller.send(:from_view_url)
  188 + assert_equal 'http://www.override.com/default_url_options/index', @controller.url_for(:controller => 'default_url_options')
189 189 end
190 190 end
191 191
6 actionpack/test/controller/routing_test.rb
@@ -59,11 +59,11 @@ def test_route_generation_allows_passing_non_string_values_to_generated_helper
59 59 class MockController
60 60 def self.build(helpers)
61 61 Class.new do
62   - def url_for(options)
  62 + def url_options
  63 + options = super
63 64 options[:protocol] ||= "http"
64 65 options[:host] ||= "test.host"
65   -
66   - super(options)
  66 + options
67 67 end
68 68
69 69 include helpers
4 actionpack/test/template/sprockets_helper_with_routes_test.rb
@@ -17,7 +17,7 @@ class SprocketsHelperWithRoutesTest < ActionView::TestCase
17 17
18 18 def setup
19 19 super
20   - @controller = BasicController.new
  20 + @controller = BasicController.new
21 21
22 22 @assets = Sprockets::Environment.new
23 23 @assets.append_path(FIXTURES.join("sprockets/app/javascripts"))
@@ -34,7 +34,7 @@ def setup
34 34
35 35 test "namespace conflicts on a named route called asset_path" do
36 36 # Testing this for sanity - asset_path is now a named route!
37   - assert_match asset_path('test_asset'), '/assets/test_asset'
  37 + assert_equal asset_path('test_asset'), '/assets/test_asset'
38 38
39 39 assert_match %r{/assets/logo-[0-9a-f]+.png},
40 40 path_to_asset("logo.png")

0 comments on commit cd5daba

Please sign in to comment.
Something went wrong with that request. Please try again.