Improved performance of Routes generation by a factor of 5 #1434 [Nicholas...

Improved performance of Routes generation by a factor of 5 #1434 [Nicholas Seckar] Added named routes (NEEDS BETTER DESCRIPTION) #1434 [Nicholas Seckar]

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@1496 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

actionpack/CHANGELOG

 *SVN*
 
-* Improved AbstractRequest documentation.  #1483 [court3nay@gmail.com]
+* Improved performance of Routes generation by a factor of 5 #1434 [Nicholas Seckar] 
 
-* Added ActionController::Base.allow_concurrency to control whether the application is thread-safe, so multi-threaded servers like WEBrick knows whether to apply a mutex around the performance of each action. Action Pack and Active Record are by default thread-safe, but many applications may not be. Turned off by default.
+* Added named routes (NEEDS BETTER DESCRIPTION) #1434 [Nicholas Seckar]
+
+* Improved AbstractRequest documentation #1483 [court3nay@gmail.com]
+
+* Added ActionController::Base.allow_concurrency to control whether the application is thread-safe, so multi-threaded servers like WEBrick knows whether to apply a mutex around the performance of each action. Turned off by default. EXPERIMENTAL FEATURE.
 
 * Added TextHelper#word_wrap(text, line_length = 80) #1449 [tuxie@dekadance.se]
 

actionpack/lib/action_controller/base.rb

 require 'action_controller/request'
 require 'action_controller/response'
 require 'action_controller/routing'
+require 'action_controller/code_generation'
 require 'action_controller/url_rewriter'
 require 'drb'
 
@@ -11,7 +12,7 @@ class SessionRestoreError < ActionControllerError #:nodoc:
   end
   class MissingTemplate < ActionControllerError #:nodoc:
   end
-  class RoutingError < ActionControllerError#:nodoc:
+  class RoutingError < ActionControllerError #:nodoc:
     attr_reader :failures
     def initialize(message, failures=[])
       super(message)

actionpack/lib/action_controller/code_generation.rb

+module ActionController
+  module CodeGeneration #:nodoc
+    class GenerationError < StandardError
+    end
+  
+    class Source
+      attr_reader :lines, :indentation_level
+      IndentationString = '  '
+      def initialize
+        @lines, @indentation_level = [], 0
+      end
+      def line(line)
+        @lines << (IndentationString * @indentation_level + line)
+      end
+      alias :<< :line
+    
+      def indent
+        @indentation_level += 1
+        yield
+        ensure
+        @indentation_level -= 1
+      end
+    
+      def to_s() lines.join("\n") end
+    end
+
+    class CodeGenerator
+      attr_accessor :source, :locals
+      def initialize(source = nil)
+        @locals = []
+        @source = source || Source.new
+      end
+    
+      BeginKeywords = %w(if unless begin until while def).collect {|kw| kw.to_sym}
+      ResumeKeywords = %w(elsif else rescue).collect {|kw| kw.to_sym}
+      Keywords = BeginKeywords + ResumeKeywords
+    
+      def method_missing(keyword, *text)
+        if Keywords.include? keyword
+          if ResumeKeywords.include? keyword
+            raise GenerationError, "Can only resume with #{keyword} immediately after an end" unless source.lines.last =~ /^\s*end\s*$/ 
+            source.lines.pop # Remove the 'end'
+          end
+      
+          line "#{keyword} #{text.join ' '}"
+          begin source.indent { yield(self.dup) }
+          ensure line 'end'
+          end
+        else
+          super(keyword, *text)
+        end
+      end
+    
+      def line(*args) self.source.line(*args) end
+      alias :<< :line
+      def indent(*args, &block) source(*args, &block) end
+      def to_s() source.to_s end
+    
+      def share_locals_with(other)
+        other.locals = self.locals = (other.locals | locals) 
+      end
+    
+      FieldsToDuplicate = [:locals]
+      def dup
+        copy = self.class.new(source)
+        self.class::FieldsToDuplicate.each do |sym|
+          value = self.send(sym)
+          value = value.dup unless value.nil? || value.is_a?(Numeric)
+          copy.send("#{sym}=", value)
+        end
+        return copy
+      end
+    end
+
+    class RecognitionGenerator < CodeGenerator
+      Attributes = [:after, :before, :current, :results, :constants, :depth, :move_ahead, :finish_statement]
+      attr_accessor(*Attributes)
+      FieldsToDuplicate = CodeGenerator::FieldsToDuplicate + Attributes
+    
+      def initialize(*args)
+        super(*args)
+        @after, @before = [], []
+        @current = nil
+        @results, @constants = {}, {}
+        @depth = 0
+        @move_ahead = nil
+        @finish_statement = Proc.new {|hash_expr| hash_expr}
+      end
+    
+      def if_next_matches(string, &block)
+        test = Routing.test_condition(next_segment(true), string)
+        self.if(test, &block)
+      end
+    
+      def move_forward(places = 1)
+        dup = self.dup
+        dup.depth += 1
+        dup.move_ahead = places
+        yield dup
+      end
+    
+      def next_segment(assign_inline = false, default = nil)
+        if locals.include?(segment_name)
+          code = segment_name
+        else
+          code = "#{segment_name} = #{path_name}[#{index_name}]"
+          if assign_inline
+            code = "(#{code})"
+          else
+            line(code)
+            code = segment_name
+          end
+        
+          locals << segment_name
+        end
+        code = "(#{code} || #{default.inspect})" if default 
+      
+        return code
+      end
+    
+      def segment_name() "segment#{depth}".to_sym end
+      def path_name() :path end
+      def index_name
+        move_ahead, @move_ahead = @move_ahead, nil
+        move_ahead ? "index += #{move_ahead}" : 'index'
+      end
+    
+      def continue
+        dup = self.dup
+        dup.before << dup.current
+        dup.current = dup.after.shift
+        dup.go
+      end
+    
+      def go
+        if current then current.write_recognition(self)
+        else self.finish
+        end
+      end 
+    
+      def result(key, expression, delay = false)
+        unless delay
+          line "#{key}_value = #{expression}"
+          expression = "#{key}_value"
+        end
+        results[key] = expression
+      end
+      def constant_result(key, object)
+        constants[key] = object
+      end
+  
+      def finish(ensure_traversal_finished = true)
+        pairs = [] 
+        (results.keys + constants.keys).uniq.each do |key|
+          pairs << "#{key.to_s.inspect} => #{results[key] ? results[key] : constants[key].inspect}"
+        end
+        hash_expr = "{#{pairs.join(', ')}}"
+      
+        statement = finish_statement.call(hash_expr)
+        if ensure_traversal_finished then self.if("! #{next_segment(true)}") {|gp| gp << statement}
+        else self << statement
+        end
+      end
+    end
+  
+    class GenerationGenerator < CodeGenerator
+      Attributes = [:after, :before, :current, :segments]
+      attr_accessor(*Attributes)
+      FieldsToDuplicate = CodeGenerator::FieldsToDuplicate + Attributes
+    
+      def initialize(*args)
+        super(*args)
+        @after, @before = [], []
+        @current = nil
+        @segments = []
+      end
+    
+      def hash_name() 'hash' end
+      def local_name(key) "#{key}_value" end
+    
+      def hash_value(key, assign = true, default = nil)
+        if locals.include?(local_name(key)) then code = local_name(key)
+        else
+          code = "hash[#{key.to_sym.inspect}]"
+          if assign
+            code = "(#{local_name(key)} = #{code})"
+            locals << local_name(key)
+          end
+        end
+        code = "(#{code} || #{default.inspect})" if default
+        return code
+      end 
+    
+      def expire_for_keys(*keys)
+        return if keys.empty?
+        conds = keys.collect {|key| "expire_on[#{key.to_sym.inspect}]"}
+        line "not_expired, #{hash_name} = false, options if not_expired && #{conds.join(' && ')}"
+      end
+    
+      def add_segment(*segments)
+        d = dup
+        d.segments.concat segments
+        yield d
+      end
+    
+      def go
+        if current then current.write_generation(self)
+        else self.finish
+        end
+      end
+    
+      def continue
+        d = dup
+        d.before << d.current
+        d.current = d.after.shift
+        d.go
+      end
+    
+      def finish
+        line %("#{segments.join('/')}")
+      end
+
+      def check_conditions(conditions)
+        tests = []
+        generator = nil
+        conditions.each do |key, condition|
+          tests << (generator || self).hash_value(key, true) if condition.is_a? Regexp
+          tests << Routing.test_condition((generator || self).hash_value(key, false), condition)
+          generator = self.dup unless generator
+        end
+        return tests.join(' && ')
+      end
+    end
+  end
+end

actionpack/lib/action_controller/request.rb

@@ -38,7 +38,6 @@ def head?
       method == :head
     end
 
-
     # Determine whether the body of a POST request is URL-encoded (default),
     # XML, or YAML by checking the Content-Type HTTP header:
     #
@@ -78,9 +77,9 @@ def yaml_post?
       post_format == :yaml && post?
     end
 
-    # Is the X-Requested-With HTTP header present and does it contain the
-    # string "XMLHttpRequest"?.  The Prototype Javascript library sends this
-    # header with every Ajax request.
+    # Returns true if the request's "X-Requested-With" header contains
+    # "XMLHttpRequest". (The Prototype Javascript library sends this header with
+    # every Ajax request.)
     def xml_http_request?
       not /XMLHttpRequest/i.match(env['HTTP_X_REQUESTED_WITH']).nil?
     end
@@ -186,7 +185,11 @@ def host_with_port
   
     def path_parameters=(parameters)
       @path_parameters = parameters
-      @parameters = nil
+      @symbolized_path_parameters = @parameters = nil
+    end
+    
+    def symbolized_path_parameters
+      @symbolized_path_parameters ||= path_parameters.symbolize_keys
     end
 
     def path_parameters

actionpack/lib/action_controller/url_rewriter.rb

@@ -12,7 +12,7 @@ def rewrite(options = {})
     end
 
     def to_str
-  		"#{@request.protocol}, #{@request.host_with_port}, #{@request.path}, #{@parameters[:controller]}, #{@parameters[:action]}, #{@request.parameters.inspect}"
+      "#{@request.protocol}, #{@request.host_with_port}, #{@request.path}, #{@parameters[:controller]}, #{@parameters[:action]}, #{@request.parameters.inspect}"
     end
 
     alias_method :to_s, :to_str
@@ -28,7 +28,7 @@ def rewrite_url(path, options)
         rewritten_url << '/' if options[:trailing_slash]
         rewritten_url << "##{options[:anchor]}" if options[:anchor]
 
-        return rewritten_url
+        rewritten_url
       end
 
       def rewrite_path(options)
@@ -38,17 +38,16 @@ def rewrite_path(options)
         path, extras = Routing::Routes.generate(options, @request)
 
         if extras[:overwrite_params]
-          params_copy = @request.parameters.reject { |k,v| ["controller","action"].include? k }
+          params_copy = @request.parameters.reject { |k,v| %w(controller action).include? k }
           params_copy.update extras[:overwrite_params]
           extras.delete(:overwrite_params)
           extras.update(params_copy)
         end
 
-        path = "/#{path.join('/')}".chomp '/'
-        path = '/' if path.empty?
-        path += build_query_string(extras)
+        path  = "/#{path}"
+        path <<  build_query_string(extras) unless extras.empty?
         
-        return path
+        path
       end
 
       # Returns a query string with escaped keys and values from the passed hash. If the passed hash contains an "id" it'll
@@ -58,15 +57,14 @@ def build_query_string(hash)
         query_string = ""
         
         hash.each do |key, value|
-          key = key.to_s
-          key = CGI.escape key
-          key += '[]' if value.class == Array
+          key = CGI.escape key.to_s
+          key <<  '[]' if value.class == Array
           value = [ value ] unless value.class == Array
           value.each { |val| elements << "#{key}=#{Routing.extract_parameter_value(val)}" }
         end
         
         query_string << ("?" + elements.join("&")) unless elements.empty?
-        return query_string
+        query_string
       end
   end
 end

actionpack/test/abstract_unit.rb

@@ -9,4 +9,4 @@
 
 ActionController::Base.logger = nil
 ActionController::Base.ignore_missing_templates = true
-ActionController::Routing::Routes.reload
+ActionController::Routing::Routes.reload rescue nil
\ No newline at end of file

activesupport/lib/active_support/core_ext/array.rb

+require File.dirname(__FILE__) + '/array/to_param'
+
+class Array #:nodoc:
+  include ActiveSupport::CoreExtensions::Array::ToParam
+end

activesupport/lib/active_support/core_ext/array/to_param.rb

+module ActiveSupport #:nodoc:
+  module CoreExtensions #:nodoc:
+    module Array #:nodoc:
+      module ToParam #:nodoc:
+        # When an array is given to url_for, it is converted to a slash separated string.
+        def to_param
+          join '/'
+        end
+      end
+    end
+  end
+end

activesupport/lib/active_support/core_ext/cgi.rb

+require File.dirname(__FILE__) + '/cgi/escape_skipping_slashes'
+
+class CGI
+  extend(ActiveSupport::CoreExtensions::CGI::EscapeSkippingSlashes)
+end

activesupport/lib/active_support/core_ext/cgi/escape_skipping_slashes.rb

+module ActiveSupport #:nodoc:
+  module CoreExtensions #:nodoc:
+    module CGI #:nodoc:
+      module EscapeSkippingSlashes #:nodoc:
+        def escape_skipping_slashes(str)
+          str = str.join('/') if str.respond_to? :join
+          str.gsub(/([^ \/a-zA-Z0-9_.-])/n) do
+            "%#{$1.unpack('H2').first.upcase}"
+          end.tr(' ', '+')
+        end
+      end
+    end
+  end
+end

activesupport/lib/active_support/dependencies.rb

@@ -14,7 +14,7 @@ def load?
   end
   
   def depend_on(file_name, swallow_load_errors = false)
-    if !loaded.include?(file_name)
+    unless loaded.include?(file_name)
       loaded << file_name
 
       begin
@@ -34,7 +34,7 @@ def clear
   end
   
   def require_or_load(file_name)
-    file_name = "#{file_name}.rb" unless ! load? || /\.rb$/ =~ file_name
+    file_name = "#{file_name}.rb" unless ! load? || file_name[-3..-1] == '.rb'
     load? ? load(file_name) : require(file_name)
   end
   
@@ -52,8 +52,10 @@ class LoadingModule < Module
     attr_reader :path
     attr_reader :root
     
-    def self.root(*load_paths)
-      RootLoadingModule.new(*load_paths)
+    class << self
+      def root(*load_paths)
+        RootLoadingModule.new(*load_paths)
+      end
     end
     
     def initialize(root, path=[])
@@ -61,7 +63,7 @@ def initialize(root, path=[])
       @root = root
     end
     
-    def root?() self.root == self end
+    def root?()      self.root == self    end
     def load_paths() self.root.load_paths end
     
     # Load missing constants if possible.
@@ -71,13 +73,15 @@ def const_missing(name)
   
     # Load the controller class or a parent module.
     def const_load!(name, file_name = nil)
+      file_name ||= 'application' if root? && name.to_s == 'ApplicationController'
       path = self.path + [file_name || name]
 
       load_paths.each do |load_path|
         fs_path = load_path.filesystem_path(path)
         next unless fs_path
 
-        if File.directory?(fs_path)
+        case 
+        when File.directory?(fs_path)
           new_module = LoadingModule.new(self.root, self.path + [name])
           self.const_set name, new_module
           if self.root?
@@ -88,7 +92,7 @@ def const_load!(name, file_name = nil)
             Object.const_set(name, new_module)
           end
           break
-        elsif File.file?(fs_path)
+        when File.file?(fs_path)
           self.root.load_file!(fs_path)
           
           # Import the loaded constant from Object provided we are the root node.
@@ -97,7 +101,7 @@ def const_load!(name, file_name = nil)
         end
       end
       
-      return self.const_defined?(name)
+      self.const_defined?(name)
     end
     
     # Is this name present or loadable?
@@ -141,7 +145,7 @@ def initialize(root) @root = root end
     # if the path leads to a module, or the path to a file if it leads to an object.
     def filesystem_path(path, allow_module=true)
       fs_path = [@root]
-      fs_path += path[0..-2].collect {|name| const_name_to_module_name name}
+      fs_path += path[0..-2].map {|name| const_name_to_module_name name}
 
       if allow_module
         result = File.join(fs_path, const_name_to_module_name(path.last))
@@ -150,7 +154,7 @@ def filesystem_path(path, allow_module=true)
 
       result = File.join(fs_path, const_name_to_file_name(path.last))
 
-      return File.file?(result) ? result : nil
+      File.file?(result) ? result : nil
     end
     
     def const_name_to_file_name(name)
@@ -164,8 +168,8 @@ def const_name_to_module_name(name)
 end
 
 Object.send(:define_method, :require_or_load)     { |file_name| Dependencies.require_or_load(file_name) } unless Object.respond_to?(:require_or_load)
-Object.send(:define_method, :require_dependency)  { |file_name| Dependencies.depend_on(file_name) } unless Object.respond_to?(:require_dependency)
-Object.send(:define_method, :require_association) { |file_name| Dependencies.associate_with(file_name) } unless Object.respond_to?(:require_association)
+Object.send(:define_method, :require_dependency)  { |file_name| Dependencies.depend_on(file_name) }       unless Object.respond_to?(:require_dependency)
+Object.send(:define_method, :require_association) { |file_name| Dependencies.associate_with(file_name) }  unless Object.respond_to?(:require_association)
 
 class Module #:nodoc:
   # Use const_missing to autoload associations so we don't have to
@@ -186,19 +190,17 @@ def const_missing(class_id)
 
 class Object #:nodoc:
   def load(file, *extras)
-    begin super(file, *extras)
-    rescue Object => exception
-      exception.blame_file! file
-      raise
-    end
+    super(file, *extras)
+  rescue Object => exception
+    exception.blame_file! file
+    raise
   end
 
   def require(file, *extras)
-    begin super(file, *extras)
-    rescue Object => exception
-      exception.blame_file! file
-      raise
-    end
+    super(file, *extras)
+  rescue Object => exception
+    exception.blame_file! file
+    raise
   end
 end
 

activesupport/test/core_ext/array_ext_test.rb

+require 'test/unit'
+require File.dirname(__FILE__) + '/../../lib/active_support/core_ext/array'
+
+class ArrayExtToParamTests < Test::Unit::TestCase
+  def test_string_array
+    assert_equal '', %w().to_param
+    assert_equal 'hello/world', %w(hello world).to_param
+    assert_equal 'hello/10', %w(hello 10).to_param
+  end
+  
+  def test_number_array
+    assert_equal '10/20', [10, 20].to_param
+  end
+end

activesupport/test/core_ext/cgi_ext_test.rb

+require 'test/unit'
+require File.dirname(__FILE__) + '/../../lib/active_support/core_ext/cgi'
+
+class EscapeSkippingSlashesTest < Test::Unit::TestCase
+  def test_array
+    assert_equal 'hello/world', CGI.escape_skipping_slashes(%w(hello world))
+    assert_equal 'hello+world/how/are/you', CGI.escape_skipping_slashes(['hello world', 'how', 'are', 'you'])
+  end
+
+  def test_typical
+    assert_equal 'hi', CGI.escape_skipping_slashes('hi')
+    assert_equal 'hi/world', CGI.escape_skipping_slashes('hi/world')
+    assert_equal 'hi/world+you+funky+thing', CGI.escape_skipping_slashes('hi/world you funky thing')
+  end
+end