Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Merge pull request #5242 from rails/opt_routes

Optimize routes generation in simple cases.

If you pass to the route helper the same amount of arguments
as the required segments, route generation will be optimized
as a string interpolation. After this commit, `post_path(post)`
is about 6.5 times faster, `post_url(post)` is about 5 times.
  • Loading branch information...
commit 1a046ab9bf7f0c3fa045a2b771a5a22d318f02ff 2 parents aeeebe1 + cd5daba
@josevalim josevalim authored
View
30 actionpack/lib/action_controller/metal/url_for.rb
@@ -1,7 +1,7 @@
-# Includes +url_for+ into the host class. The class has to provide a +RouteSet+ by implementing
+# Includes +url_for+ into the host class. The class has to provide a +RouteSet+ by implementing
# the <tt>_routes</tt> method. Otherwise, an exception will be raised.
#
-# In addition to <tt>AbstractController::UrlFor</tt>, this module accesses the HTTP layer to define
+# In addition to <tt>AbstractController::UrlFor</tt>, this module accesses the HTTP layer to define
# url options like the +host+. In order to do so, this module requires the host class
# to implement +env+ and +request+, which need to be a Rack-compatible.
#
@@ -18,7 +18,7 @@
# @url = root_path # named route from the application.
# end
# end
-#
+#
module ActionController
module UrlFor
extend ActiveSupport::Concern
@@ -26,22 +26,20 @@ module UrlFor
include AbstractController::UrlFor
def url_options
- @_url_options ||= super.reverse_merge(
- :host => request.host,
- :port => request.optional_port,
- :protocol => request.protocol,
- :_path_segments => request.symbolized_path_parameters
- ).freeze
+ @_url_options ||= begin
+ hash = super.reverse_merge(
+ :host => request.host,
+ :port => request.optional_port,
+ :protocol => request.protocol,
+ :_path_segments => request.symbolized_path_parameters
+ )
- if _routes.equal?(env["action_dispatch.routes"])
- @_url_options.dup.tap do |options|
- options[:script_name] = request.script_name.dup
- options.freeze
+ if _routes.equal?(env["action_dispatch.routes"])
+ hash[:script_name] = request.script_name.dup
end
- else
- @_url_options
+
+ hash.freeze
end
end
-
end
end
View
4 actionpack/lib/action_dispatch/http/url.rb
@@ -40,7 +40,9 @@ def url_for(options = {})
rewritten_url << ":#{options.delete(:port)}" if options[:port]
end
- path = options.delete(:path) || ''
+ path = ""
+ path << options.delete(:script_name).to_s.chomp("/")
+ path << options.delete(:path).to_s
params = options[:params] || {}
params.reject! {|k,v| v.to_param.nil? }
View
6 actionpack/lib/action_dispatch/routing/mapper.rb
@@ -446,7 +446,11 @@ def define_generate_prefix(app, name)
_route = @set.named_routes.routes[name.to_sym]
_routes = @set
app.routes.define_mounted_helper(name)
- app.routes.class_eval do
+ app.routes.singleton_class.class_eval do
+ define_method :mounted? do
+ true
+ end
+
define_method :_generate_prefix do |options|
prefix_options = options.slice(*_route.segment_keys)
# we must actually delete prefix segment keys to avoid passing them to next url_for
View
66 actionpack/lib/action_dispatch/routing/route_set.rb
@@ -191,14 +191,50 @@ def define_url_helper(route, name, kind, options)
selector = url_helper_name(name, kind)
hash_access_method = hash_access_name(name, kind)
- @module.module_eval <<-END_EVAL, __FILE__, __LINE__ + 1
- remove_possible_method :#{selector}
- def #{selector}(*args)
- url_for(#{hash_access_method}(*args))
- end
- END_EVAL
+ if optimize_helper?(kind, route)
+ @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) && optimize_routes_generation?
+ options = #{options.inspect}.merge!(url_options)
+ options[:path] = "#{optimized_helper(route)}"
+ ActionDispatch::Http::URL.url_for(options)
+ else
+ url_for(#{hash_access_method}(*args))
+ end
+ end
+ END_EVAL
+ else
+ @module.module_eval <<-END_EVAL, __FILE__, __LINE__ + 1
+ remove_possible_method :#{selector}
+ def #{selector}(*args)
+ url_for(#{hash_access_method}(*args))
+ end
+ END_EVAL
+ end
+
helpers << selector
end
+
+ # Clause check about when we need to generate an optimized helper.
+ def optimize_helper?(kind, route) #:nodoc:
+ route.ast.grep(Journey::Nodes::Star).empty? && route.requirements.except(:controller, :action).empty?
+ end
+
+ # Generates the interpolation to be used in the optimized helper.
+ def optimized_helper(route)
+ string_route = route.ast.to_s
+
+ while string_route.gsub!(/\([^\)]*\)/, "")
+ true
+ end
+
+ route.required_parts.each_with_index do |part, i|
+ string_route.gsub!(part.inspect, "\#{Journey::Router::Utils.escape_fragment(args[#{i}].to_param)}")
+ end
+
+ string_route
+ end
end
attr_accessor :formatter, :set, :named_routes, :default_scope, :router
@@ -323,7 +359,7 @@ def url_helpers
# 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
@@ -557,6 +593,14 @@ def generate(options, recall = {}, extras = false)
RESERVED_OPTIONS = [:host, :protocol, :port, :subdomain, :domain, :tld_length,
:trailing_slash, :anchor, :params, :only_path, :script_name]
+ def mounted?
+ false
+ end
+
+ def optimize_routes_generation?
+ !mounted? && default_url_options.empty?
+ end
+
def _generate_prefix(options = {})
nil
end
@@ -568,19 +612,17 @@ def url_for(options)
user, password = extract_authentication(options)
path_segments = options.delete(:_path_segments)
- script_name = options.delete(:script_name)
-
- path = (script_name.blank? ? _generate_prefix(options) : script_name.chomp('/')).to_s
+ script_name = options.delete(:script_name).presence || _generate_prefix(options)
path_options = options.except(*RESERVED_OPTIONS)
path_options = yield(path_options) if block_given?
- path_addition, params = generate(path_options, path_segments || {})
- path << path_addition
+ path, params = generate(path_options, path_segments || {})
params.merge!(options[:params] || {})
ActionDispatch::Http::URL.url_for(options.merge!({
:path => path,
+ :script_name => script_name,
:params => params,
:user => user,
:password => password
View
8 actionpack/lib/action_dispatch/routing/url_for.rb
@@ -102,6 +102,9 @@ def initialize(*)
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,6 +155,11 @@ def url_for(options = nil)
protected
+ def optimize_routes_generation?
+ return @_optimized_routes if defined?(@_optimized_routes)
+ @_optimized_routes = _routes.optimize_routes_generation? && default_url_options.empty?
+ end
+
def _with_routes(routes)
old_routes, @_routes = @_routes, routes
yield
View
23 actionpack/lib/action_view/helpers/url_helper.rb
@@ -23,20 +23,25 @@ module UrlHelper
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
View
8 actionpack/test/controller/base_test.rb
@@ -56,7 +56,7 @@ def from_view
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 @@ def test_url_options_override
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
View
6 actionpack/test/controller/routing_test.rb
@@ -59,11 +59,11 @@ def test_route_generation_allows_passing_non_string_values_to_generated_helper
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
View
4 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 @@ def setup
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")
Please sign in to comment.
Something went wrong with that request. Please try again.