Skip to content

Commit

Permalink
Avoid excessive AST traversal for Routing
Browse files Browse the repository at this point in the history
  • Loading branch information
methodmissing committed Jul 10, 2008
1 parent 5e8a69a commit 19bb601
Show file tree
Hide file tree
Showing 5 changed files with 93 additions and 91 deletions.
10 changes: 5 additions & 5 deletions actionpack/lib/action_controller/routing/builder.rb
Expand Up @@ -4,16 +4,16 @@ class RouteBuilder #:nodoc:
attr_accessor :separators, :optional_separators

def initialize
self.separators = Routing::SEPARATORS
self.optional_separators = %w( / )
@separators = Routing::SEPARATORS
@optional_separators = %w( / )
end

def separator_pattern(inverted = false)
"[#{'^' if inverted}#{Regexp.escape(separators.join)}]"
"[#{'^' if inverted}#{Regexp.escape(@separators.join)}]"
end

def interval_regexp
Regexp.new "(.*?)(#{separators.source}|$)"
Regexp.new "(.*?)(#{@separators.source}|$)"
end

def multiline_regexp?(expression)
Expand Down Expand Up @@ -54,7 +54,7 @@ def segment_for(string)
when /\A(#{separator_pattern(:inverted)}+)/ then StaticSegment.new($1)
when Regexp.new(separator_pattern) then
returning segment = DividerSegment.new($&) do
segment.is_optional = (optional_separators.include? $&)
segment.is_optional = (@optional_separators.include? $&)
end
end
[segment, $~.post_match]
Expand Down
10 changes: 5 additions & 5 deletions actionpack/lib/action_controller/routing/optimisations.rb
Expand Up @@ -57,7 +57,7 @@ def applicable?
# rather than triggering the expensive logic in +url_for+.
class PositionalArguments < Optimiser
def guard_condition
number_of_arguments = route.segment_keys.size
number_of_arguments = @route.segment_keys.size
# if they're using foo_url(:id=>2) it's one
# argument, but we don't want to generate /foos/id2
if number_of_arguments == 1
Expand All @@ -71,7 +71,7 @@ def generation_code
elements = []
idx = 0

if kind == :url
if @kind == :url
elements << '#{request.protocol}'
elements << '#{request.host_with_port}'
end
Expand All @@ -81,7 +81,7 @@ def generation_code
# The last entry in <tt>route.segments</tt> appears to *always* be a
# 'divider segment' for '/' but we have assertions to ensure that
# we don't include the trailing slashes, so skip them.
(route.segments.size == 1 ? route.segments : route.segments[0..-2]).each do |segment|
(@route.segments.size == 1 ? @route.segments : @route.segments[0..-2]).each do |segment|
if segment.is_a?(DynamicSegment)
elements << segment.interpolation_chunk("args[#{idx}].to_param")
idx += 1
Expand All @@ -98,7 +98,7 @@ def generation_code
# argument
class PositionalArgumentsWithAdditionalParams < PositionalArguments
def guard_condition
"(!defined?(default_url_options) || default_url_options.blank?) && defined?(request) && request && args.size == #{route.segment_keys.size + 1} && !args.last.has_key?(:anchor) && !args.last.has_key?(:port) && !args.last.has_key?(:host)"
"(!defined?(default_url_options) || default_url_options.blank?) && defined?(request) && request && args.size == #{@route.segment_keys.size + 1} && !args.last.has_key?(:anchor) && !args.last.has_key?(:port) && !args.last.has_key?(:host)"
end

# This case uses almost the same code as positional arguments,
Expand All @@ -110,7 +110,7 @@ def generation_code
# To avoid generating "http://localhost/?host=foo.example.com" we
# can't use this optimisation on routes without any segments
def applicable?
super && route.segment_keys.size > 0
super && @route.segment_keys.size > 0
end
end

Expand Down
32 changes: 16 additions & 16 deletions actionpack/lib/action_controller/routing/route.rb
Expand Up @@ -17,7 +17,7 @@ def optimise?
end

def segment_keys
segments.collect do |segment|
@segments.collect do |segment|
segment.key if segment.respond_to? :key
end.compact
end
Expand Down Expand Up @@ -52,15 +52,15 @@ def write_generation
# Build several lines of code that extract values from the options hash. If any
# of the values are missing or rejected then a return will be executed.
def generation_extraction
segments.collect do |segment|
@segments.collect do |segment|
segment.extraction_code
end.compact * "\n"
end

# Produce a condition expression that will check the requirements of this route
# upon generation.
def generation_requirements
requirement_conditions = requirements.collect do |key, req|
requirement_conditions = @requirements.collect do |key, req|
if req.is_a? Regexp
value_regexp = Regexp.new "\\A#{req.to_s}\\Z"
"hash[:#{key}] && #{value_regexp.inspect} =~ options[:#{key}]"
Expand All @@ -72,7 +72,7 @@ def generation_requirements
end

def generation_structure
segments.last.string_structure segments[0..-2]
@segments.last.string_structure @segments[0..-2]
end

# Write and compile a +recognize+ method for this Route.
Expand All @@ -92,14 +92,14 @@ def write_recognition
# recognition, not generation.
def recognition_conditions
result = ["(match = #{Regexp.new(recognition_pattern).inspect}.match(path))"]
result << "conditions[:method] === env[:method]" if conditions[:method]
result << "@conditions[:method] === env[:method]" if @conditions[:method]
result
end

# Build the regular expression pattern that will match this route.
def recognition_pattern(wrap = true)
pattern = ''
segments.reverse_each do |segment|
@segments.reverse_each do |segment|
pattern = segment.build_pattern pattern
end
wrap ? ("\\A" + pattern + "\\Z") : pattern
Expand All @@ -108,7 +108,7 @@ def recognition_pattern(wrap = true)
# Write the code to extract the parameters from a matched route.
def recognition_extraction
next_capture = 1
extraction = segments.collect do |segment|
extraction = @segments.collect do |segment|
x = segment.match_extraction(next_capture)
next_capture += Regexp.new(segment.regexp_chunk).number_of_captures
x
Expand Down Expand Up @@ -176,7 +176,7 @@ def recognize(path, environment={})
#
def parameter_shell
@parameter_shell ||= returning({}) do |shell|
requirements.each do |key, requirement|
@requirements.each do |key, requirement|
shell[key] = requirement unless requirement.is_a? Regexp
end
end
Expand All @@ -187,8 +187,8 @@ def parameter_shell
# placed upon them.
def significant_keys
@significant_keys ||= returning [] do |sk|
segments.each { |segment| sk << segment.key if segment.respond_to? :key }
sk.concat requirements.keys
@segments.each { |segment| sk << segment.key if segment.respond_to? :key }
sk.concat @requirements.keys
sk.uniq!
end
end
Expand All @@ -197,11 +197,11 @@ def significant_keys
# have defaults, or which are specified by non-regexp requirements.
def defaults
@defaults ||= returning({}) do |hash|
segments.each do |segment|
@segments.each do |segment|
next unless segment.respond_to? :default
hash[segment.key] = segment.default unless segment.default.nil?
end
requirements.each do |key,req|
@requirements.each do |key,req|
next if Regexp === req || req.nil?
hash[key] = req
end
Expand All @@ -221,15 +221,15 @@ def matches_controller_and_action?(controller, action)

def to_s
@to_s ||= begin
segs = segments.inject("") { |str,s| str << s.to_s }
"%-6s %-40s %s" % [(conditions[:method] || :any).to_s.upcase, segs, requirements.inspect]
segs = @segments.inject("") { |str,s| str << s.to_s }
"%-6s %-40s %s" % [(@conditions[:method] || :any).to_s.upcase, segs, @requirements.inspect]
end
end

protected
def requirement_for(key)
return requirements[key] if requirements.key? key
segments.each do |segment|
return @requirements[key] if @requirements.key? key
@segments.each do |segment|
return segment.regexp if segment.respond_to?(:key) && segment.key == key
end
nil
Expand Down
52 changes: 26 additions & 26 deletions actionpack/lib/action_controller/routing/route_set.rb
Expand Up @@ -79,33 +79,33 @@ def clear!
end

def add(name, route)
routes[name.to_sym] = route
@routes[name.to_sym] = route
define_named_route_methods(name, route)
end

def get(name)
routes[name.to_sym]
@routes[name.to_sym]
end

alias []= add
alias [] get
alias clear clear!

def each
routes.each { |name, route| yield name, route }
@routes.each { |name, route| yield name, route }
self
end

def names
routes.keys
@routes.keys
end

def length
routes.length
@routes.length
end

def reset!
old_routes = routes.dup
old_routes = @routes.dup
clear!
old_routes.each do |name, route|
add(name, route)
Expand Down Expand Up @@ -144,7 +144,7 @@ def #{selector}(options = nil)
end
protected :#{selector}
end_eval
helpers << selector
@helpers << selector
end

def define_url_helper(route, name, kind, options)
Expand Down Expand Up @@ -185,15 +185,15 @@ def #{selector}(*args)
end
protected :#{selector}
end_eval
helpers << selector
@helpers << selector
end
end

attr_accessor :routes, :named_routes, :configuration_file

def initialize
self.routes = []
self.named_routes = NamedRouteCollection.new
@routes = []
@named_routes = NamedRouteCollection.new
end

# Subclasses and plugins may override this method to specify a different
Expand All @@ -209,8 +209,8 @@ def draw
end

def clear!
routes.clear
named_routes.clear
@routes.clear
@named_routes.clear
@combined_regexp = nil
@routes_by_controller = nil
# This will force routing/recognition_optimization.rb
Expand All @@ -220,11 +220,11 @@ def clear!

def install_helpers(destinations = [ActionController::Base, ActionView::Base], regenerate_code = false)
Array(destinations).each { |d| d.module_eval { include Helpers } }
named_routes.install(destinations, regenerate_code)
@named_routes.install(destinations, regenerate_code)
end

def empty?
routes.empty?
@routes.empty?
end

def load!
Expand All @@ -238,8 +238,8 @@ def load!
alias reload! load!

def reload
if @routes_last_modified && configuration_file
mtime = File.stat(configuration_file).mtime
if @routes_last_modified && @configuration_file
mtime = File.stat(@configuration_file).mtime
# if it hasn't been changed, then just return
return if mtime == @routes_last_modified
# if it has changed then record the new time and fall to the load! below
Expand All @@ -249,24 +249,24 @@ def reload
end

def load_routes!
if configuration_file
load configuration_file
@routes_last_modified = File.stat(configuration_file).mtime
if @configuration_file
load @configuration_file
@routes_last_modified = File.stat(@configuration_file).mtime
else
add_route ":controller/:action/:id"
end
end

def add_route(path, options = {})
route = builder.build(path, options)
routes << route
@routes << route
route
end

def add_named_route(name, path, options = {})
# TODO - is options EVER used?
name = options[:name_prefix] + name.to_s if options[:name_prefix]
named_routes[name.to_sym] = add_route(path, options)
@named_routes[name.to_sym] = add_route(path, options)
end

def options_as_params(options)
Expand Down Expand Up @@ -308,7 +308,7 @@ def generate(options, recall = {}, method=:generate)
named_route_name = options.delete(:use_route)
generate_all = options.delete(:generate_all)
if named_route_name
named_route = named_routes[named_route_name]
named_route = @named_routes[named_route_name]
options = named_route.parameter_shell.merge(options)
end

Expand Down Expand Up @@ -351,7 +351,7 @@ def generate(options, recall = {}, method=:generate)

if generate_all
# Used by caching to expire all paths for a resource
return routes.collect do |route|
return @routes.collect do |route|
route.send!(method, options, merged, expire_on)
end.compact
end
Expand Down Expand Up @@ -410,14 +410,14 @@ def routes_for(options, merged, expire_on)
end

def routes_for_controller_and_action(controller, action)
selected = routes.select do |route|
selected = @routes.select do |route|
route.matches_controller_and_action? controller, action
end
(selected.length == routes.length) ? routes : selected
(selected.length == @routes.length) ? @routes : selected
end

def routes_for_controller_and_action_and_keys(controller, action, keys)
selected = routes.select do |route|
selected = @routes.select do |route|
route.matches_controller_and_action? controller, action
end
selected.sort_by do |route|
Expand Down

0 comments on commit 19bb601

Please sign in to comment.