Refactor template compilation from AV::Base into the template handlers. Closes #10888 [lifofifo]

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

8 files changed, 184 insertions, 170 deletions

diff --git a/actionpack/lib/action_view.rb b/actionpack/lib/action_view.rb
index 55aeb25640..d2d765787c 100644
--- a/actionpack/lib/action_view.rb
+++ b/actionpack/lib/action_view.rb
@@ -22,6 +22,7 @@
 #++
 
 require 'action_view/template_handler'
+require 'action_view/template_handlers/compilable'
 require 'action_view/template_handlers/builder'
 require 'action_view/template_handlers/erb'
 require 'action_view/template_handlers/rjs'
diff --git a/actionpack/lib/action_view/base.rb b/actionpack/lib/action_view/base.rb
index 9fca77db9d..cb53fbafc8 100644
--- a/actionpack/lib/action_view/base.rb
+++ b/actionpack/lib/action_view/base.rb
@@ -196,14 +196,11 @@ module ActionView #:nodoc:
     end
     include CompiledTemplates
 
-    # Maps inline templates to their method names 
+    # Maps inline templates to their method names
+    cattr_accessor :method_names
     @@method_names = {}
-    # Map method names to their compile time
-    @@compile_time = {}
     # Map method names to the names passed in local assigns so far
     @@template_args = {}
-    # Count the number of inline templates
-    @@inline_template_count = 0
 
     # Cache public asset paths
     cattr_reader :computed_public_paths
@@ -365,7 +362,7 @@ If you are rendering a subtemplate, you must now use controller-like partial syn
       if handler.compilable?
         compile_and_render_template(handler, template, file_path, local_assigns)
       else
-        template ||= read_template_file(file_path, template_extension) # Make sure that a lazyily-read template is loaded.
+        template ||= handler.read_template_file(file_path, template_extension) # Make sure that a lazyily-read template is loaded.
         handler.render(template, local_assigns)
       end
     end
@@ -389,11 +386,6 @@ If you are rendering a subtemplate, you must now use controller-like partial syn
         returning(yield) { @content_for_layout = original_content_for_layout }
       end
 
-      # This method reads a template file.
-      def read_template_file(template_path, extension)
-        File.read(template_path)
-      end
-
       # Evaluate the local assigns and pushes them to the view.
       def evaluate_assigns
         unless @assigns_added
@@ -407,99 +399,6 @@ If you are rendering a subtemplate, you must now use controller-like partial syn
         @assigns.each { |key, value| instance_variable_set("@#{key}", value) }
       end
 
-
-      # Return true if the given template was compiled for a superset of the keys in local_assigns
-      def supports_local_assigns?(render_symbol, local_assigns)
-        local_assigns.empty? ||
-          ((args = @@template_args[render_symbol]) && local_assigns.all? { |k,_| args.has_key?(k) })
-      end
-
-      # Method to check whether template compilation is necessary.
-      # The template will be compiled if the inline template or file has not been compiled yet,
-      # if local_assigns has a new key, which isn't supported by the compiled code yet,
-      # or if the file has changed on disk and checking file mods hasn't been disabled.
-      def compile_template?(template, file_name, local_assigns)
-        method_key    = file_name || template
-        render_symbol = @@method_names[method_key]
-
-        compile_time = @@compile_time[render_symbol]
-        if compile_time && supports_local_assigns?(render_symbol, local_assigns)
-          if file_name && !@@cache_template_loading
-            template_changed_since?(file_name, compile_time)
-          end
-        else
-          true
-        end
-      end
-
-      # Method to handle checking a whether a template has changed since last compile; isolated so that templates
-      # not stored on the file system can hook and extend appropriately.
-      def template_changed_since?(file_name, compile_time)
-        lstat = File.lstat(file_name)
-        compile_time < lstat.mtime || 
-          (lstat.symlink? && compile_time < File.stat(file_name).mtime)
-      end
-
-      # Method to create the source code for a given template.
-      def create_template_source(handler, template, render_symbol, locals)
-        body = handler.compile(template)
-
-        @@template_args[render_symbol] ||= {}
-        locals_keys = @@template_args[render_symbol].keys | locals
-        @@template_args[render_symbol] = locals_keys.inject({}) { |h, k| h[k] = true; h }
-
-        locals_code = ""
-        locals_keys.each do |key|
-          locals_code << "#{key} = local_assigns[:#{key}]\n"
-        end
-
-        "def #{render_symbol}(local_assigns)\n#{locals_code}#{body}\nend"
-      end
-
-      def assign_method_name(handler, template, file_name)
-        method_key = file_name || template
-        @@method_names[method_key] ||= compiled_method_name(handler, template, file_name)
-      end
-
-      def compiled_method_name(handler, template, file_name)
-        ['_run', handler.class.to_s.demodulize.underscore, compiled_method_name_file_path_segment(file_name)].compact.join('_').to_sym
-      end
-
-      def compiled_method_name_file_path_segment(file_name)
-        if file_name
-          s = File.expand_path(file_name)
-          s.sub!(/^#{Regexp.escape(File.expand_path(RAILS_ROOT))}/, '') if defined?(RAILS_ROOT)
-          s.gsub!(/([^a-zA-Z0-9_])/) { $1.ord }
-          s
-        else
-          (@@inline_template_count += 1).to_s
-        end
-      end
-
-      # Compile and evaluate the template's code
-      def compile_template(handler, template, file_name, local_assigns)
-        render_symbol = assign_method_name(handler, template, file_name)
-        render_source = create_template_source(handler, template, render_symbol, local_assigns.keys)
-        line_offset   = @@template_args[render_symbol].size + handler.line_offset
-
-        begin
-          file_name = 'compiled-template' if file_name.blank?
-          CompiledTemplates.module_eval(render_source, file_name, -line_offset)
-        rescue Exception => e  # errors from template code
-          if logger
-            logger.debug "ERROR: compiling #{render_symbol} RAISED #{e}"
-            logger.debug "Function body: #{render_source}"
-            logger.debug "Backtrace: #{e.backtrace.join("\n")}"
-          end
-
-          raise TemplateError.new(@finder.extract_base_path_from(file_name) ||
-                @finder.view_paths.first, file_name || template, @assigns, template, e)
-        end
-
-        @@compile_time[render_symbol] = Time.now
-        # logger.debug "Compiled template #{file_name || template}\n  ==> #{render_symbol}" if logger
-      end
-
       # Render the provided template with the given local assigns. If the template has not been rendered with the provided
       # local assigns yet, or if the template has been updated on disk, then the template will be compiled to a method.
       #
@@ -511,10 +410,7 @@ If you are rendering a subtemplate, you must now use controller-like partial syn
         local_assigns = local_assigns.symbolize_keys if @@local_assigns_support_string_keys
 
         # compile the given template, if necessary
-        if compile_template?(template, file_path, local_assigns)
-          template ||= read_template_file(file_path, nil)
-          compile_template(handler, template, file_path, local_assigns)
-        end
+        handler.compile_template(template, file_path, local_assigns)
 
         # Get the method name for this template and run it
         method_name = @@method_names[file_path || template]
diff --git a/actionpack/lib/action_view/template_handler.rb b/actionpack/lib/action_view/template_handler.rb
index b2c6ff3e36..d4892eccd3 100644
--- a/actionpack/lib/action_view/template_handler.rb
+++ b/actionpack/lib/action_view/template_handler.rb
@@ -1,5 +1,17 @@
 module ActionView
   class TemplateHandler
+    # Map method names to their compile time
+    cattr_accessor :compile_time
+    @@compile_time = {}
+
+    # Map method names to the names passed in local assigns so far
+    cattr_accessor :template_args
+    @@template_args = {}
+
+    # Count the number of inline templates
+    cattr_accessor :inline_template_count
+    @@inline_template_count = 0
+
     def self.line_offset
       0
     end
@@ -29,5 +41,10 @@ module ActionView
     # Called by CacheHelper#cache
     def cache_fragment(block, name = {}, options = nil)
     end
+
+    # This method reads a template file.
+    def read_template_file(template_path, extension)
+      File.read(template_path)
+    end
   end
 end
diff --git a/actionpack/lib/action_view/template_handlers/builder.rb b/actionpack/lib/action_view/template_handlers/builder.rb
index 098c90f5c7..cff9e6beb8 100644
--- a/actionpack/lib/action_view/template_handlers/builder.rb
+++ b/actionpack/lib/action_view/template_handlers/builder.rb
@@ -3,18 +3,16 @@ require 'builder'
 module ActionView
   module TemplateHandlers
     class Builder < TemplateHandler
+      include Compilable
+
       def self.line_offset
         2
       end
 
-      def self.compilable?
-        true
-      end
-
       def compile(template)
         content_type_handler = (@view.send!(:controller).respond_to?(:response) ? "controller.response" : "controller")
         "#{content_type_handler}.content_type ||= Mime::XML\n" +
-        "xml = Builder::XmlMarkup.new(:indent => 2)\n" +
+        "xml = ::Builder::XmlMarkup.new(:indent => 2)\n" +
         template +
         "\nxml.target!\n"
       end
diff --git a/actionpack/lib/action_view/template_handlers/compilable.rb b/actionpack/lib/action_view/template_handlers/compilable.rb
new file mode 100644
index 0000000000..2446436530
--- /dev/null
+++ b/actionpack/lib/action_view/template_handlers/compilable.rb
@@ -0,0 +1,116 @@
+module ActionView
+  module TemplateHandlers
+    module Compilable
+
+      def self.included(base)
+        base.extend ClassMethod
+      end
+
+      module ClassMethod
+        # If a handler is mixin this module, set compilable to true
+        def compilable?
+          true
+        end
+      end
+
+      # Compile and evaluate the template's code
+      def compile_template(template, file_name, local_assigns)
+        return unless compile_template?(template, file_name, local_assigns)
+
+        template ||= read_template_file(file_name, nil)
+
+        render_symbol = assign_method_name(template, file_name)
+        render_source = create_template_source(template, render_symbol, local_assigns.keys)
+        line_offset   = self.template_args[render_symbol].size + self.line_offset
+
+        begin
+          file_name = 'compiled-template' if file_name.blank?
+          ActionView::Base::CompiledTemplates.module_eval(render_source, file_name, line_offset)
+        rescue Exception => e  # errors from template code
+          if @view.logger
+            @view.logger.debug "ERROR: compiling #{render_symbol} RAISED #{e}"
+            @view.logger.debug "Function body: #{render_source}"
+            @view.logger.debug "Backtrace: #{e.backtrace.join("\n")}"
+          end
+
+          raise ActionView::TemplateError.new(@view.finder.extract_base_path_from(file_name) ||
+                @view.finder.view_paths.first, file_name || template, @view.assigns, template, e)
+        end
+
+        self.compile_time[render_symbol] = Time.now
+        # logger.debug "Compiled template #{file_name || template}\n  ==> #{render_symbol}" if logger
+      end
+
+      private
+
+      # Method to check whether template compilation is necessary.
+      # The template will be compiled if the inline template or file has not been compiled yet,
+      # if local_assigns has a new key, which isn't supported by the compiled code yet,
+      # or if the file has changed on disk and checking file mods hasn't been disabled.
+      def compile_template?(template, file_name, local_assigns)
+        method_key    = file_name || template
+        render_symbol = @view.method_names[method_key]
+
+        compile_time = self.compile_time[render_symbol]
+        if compile_time && supports_local_assigns?(render_symbol, local_assigns)
+          if file_name && !@view.cache_template_loading
+            template_changed_since?(file_name, compile_time)
+          end
+        else
+          true
+        end
+      end
+
+      def assign_method_name(template, file_name)
+        method_key = file_name || template
+        @view.method_names[method_key] ||= compiled_method_name(template, file_name)
+      end
+
+      def compiled_method_name(template, file_name)
+        ['_run', self.class.to_s.demodulize.underscore, compiled_method_name_file_path_segment(file_name)].compact.join('_').to_sym
+      end
+
+      def compiled_method_name_file_path_segment(file_name)
+        if file_name
+          s = File.expand_path(file_name)
+          s.sub!(/^#{Regexp.escape(File.expand_path(RAILS_ROOT))}/, '') if defined?(RAILS_ROOT)
+          s.gsub!(/([^a-zA-Z0-9_])/) { $1.ord }
+          s
+        else
+          (self.inline_template_count += 1).to_s
+        end
+      end
+
+      # Method to create the source code for a given template.
+      def create_template_source(template, render_symbol, locals)
+        body = compile(template)
+
+        self.template_args[render_symbol] ||= {}
+        locals_keys = self.template_args[render_symbol].keys | locals
+        self.template_args[render_symbol] = locals_keys.inject({}) { |h, k| h[k] = true; h }
+
+        locals_code = ""
+        locals_keys.each do |key|
+          locals_code << "#{key} = local_assigns[:#{key}]\n"
+        end
+
+        "def #{render_symbol}(local_assigns)\n#{locals_code}#{body}\nend"
+      end
+
+      # Return true if the given template was compiled for a superset of the keys in local_assigns
+      def supports_local_assigns?(render_symbol, local_assigns)
+        local_assigns.empty? ||
+          ((args = self.template_args[render_symbol]) && local_assigns.all? { |k,_| args.has_key?(k) })
+      end
+
+      # Method to handle checking a whether a template has changed since last compile; isolated so that templates
+      # not stored on the file system can hook and extend appropriately.
+      def template_changed_since?(file_name, compile_time)
+        lstat = File.lstat(file_name)
+        compile_time < lstat.mtime ||
+          (lstat.symlink? && compile_time < File.stat(file_name).mtime)
+      end
+
+    end
+  end
+end
\ No newline at end of file
diff --git a/actionpack/lib/action_view/template_handlers/erb.rb b/actionpack/lib/action_view/template_handlers/erb.rb
index ffc1ab0e97..3389c124eb 100644
--- a/actionpack/lib/action_view/template_handlers/erb.rb
+++ b/actionpack/lib/action_view/template_handlers/erb.rb
@@ -22,14 +22,12 @@ end
 module ActionView
   module TemplateHandlers
     class ERB < TemplateHandler
+      include Compilable
+
       def compile(template)
         ::ERB.new(template, nil, @view.erb_trim_mode).src
       end
 
-      def self.compilable?
-        true
-      end
-
       def cache_fragment(block, name = {}, options = nil) #:nodoc:
         @view.fragment_for(block, name, options) do
           eval(ActionView::Base.erb_variable, block.binding)
diff --git a/actionpack/lib/action_view/template_handlers/rjs.rb b/actionpack/lib/action_view/template_handlers/rjs.rb
index e4b3d0c2a6..e0f95205de 100644
--- a/actionpack/lib/action_view/template_handlers/rjs.rb
+++ b/actionpack/lib/action_view/template_handlers/rjs.rb
@@ -1,6 +1,8 @@
 module ActionView
   module TemplateHandlers
     class RJS < TemplateHandler
+      include Compilable
+
       def self.line_offset
         2
       end
@@ -10,10 +12,6 @@ module ActionView
         "update_page do |page|\n#{template}\nend"
       end
 
-      def self.compilable?
-        true
-      end
-
       def cache_fragment(block, name = {}, options = nil) #:nodoc:
         @view.fragment_for(block, name, options) do
           begin
diff --git a/actionpack/test/template/compiled_templates_test.rb b/actionpack/test/template/compiled_templates_test.rb
index 4998eb198c..c46eed0775 100644
--- a/actionpack/test/template/compiled_templates_test.rb
+++ b/actionpack/test/template/compiled_templates_test.rb
@@ -87,21 +87,22 @@ class CompiledTemplateTests < Test::Unit::TestCase
       v.base_path = '.'
       v.cache_template_loading = false
 
+      @handler_class = ActionView::Base.handler_class_for_extension(:rhtml)
+      @handler       = @handler_class.new(v)
+
       # All templates were created at t+1
       File::Stat.any_instance.expects(:mtime).times(windows ? 2 : 3).returns(t + 1.second)
 
       # private methods template_changed_since? and compile_template?
       # should report true for all since they have not been compiled
-      assert v.send(:template_changed_since?, @a, t)
-      assert v.send(:template_changed_since?, @b, t)
-      assert v.send(:template_changed_since?, @s, t) unless windows
+      assert @handler.send(:template_changed_since?, @a, t)
+      assert @handler.send(:template_changed_since?, @b, t)
+      assert @handler.send(:template_changed_since?, @s, t) unless windows
 
-      assert v.send(:compile_template?, nil, @a, {})
-      assert v.send(:compile_template?, nil, @b, {})
-      assert v.send(:compile_template?, nil, @s, {}) unless windows
+      assert @handler.send(:compile_template?, nil, @a, {})
+      assert @handler.send(:compile_template?, nil, @b, {})
+      assert @handler.send(:compile_template?, nil, @s, {}) unless windows
 
-      @handler_class = ActionView::Base.handler_class_for_extension(:rhtml)
-      @handler       = @handler_class.new(v)
       # All templates are rendered at t+2
       Time.expects(:now).times(windows ? 2 : 3).returns(t + 2.seconds)
       v.send(:compile_and_render_template, @handler, '', @a)
@@ -111,26 +112,26 @@ class CompiledTemplateTests < Test::Unit::TestCase
       b_n = v.method_names[@b]
       s_n = v.method_names[@s] unless windows
       # all of the files have changed since last compile
-      assert v.compile_time[a_n] > t
-      assert v.compile_time[b_n] > t
-      assert v.compile_time[s_n] > t unless windows
+      assert @handler.compile_time[a_n] > t
+      assert @handler.compile_time[b_n] > t
+      assert @handler.compile_time[s_n] > t unless windows
 
       # private methods template_changed_since? and compile_template?
       # should report false for all since none have changed since compile
       File::Stat.any_instance.expects(:mtime).times(windows ? 6 : 12).returns(t + 1.second)
-      assert !v.send(:template_changed_since?, @a, v.compile_time[a_n])
-      assert !v.send(:template_changed_since?, @b, v.compile_time[b_n])
-      assert !v.send(:template_changed_since?, @s, v.compile_time[s_n]) unless windows
-      assert !v.send(:compile_template?, nil, @a, {})
-      assert !v.send(:compile_template?, nil, @b, {})
-      assert !v.send(:compile_template?, nil, @s, {}) unless windows
+      assert !@handler.send(:template_changed_since?, @a, @handler.compile_time[a_n])
+      assert !@handler.send(:template_changed_since?, @b, @handler.compile_time[b_n])
+      assert !@handler.send(:template_changed_since?, @s, @handler.compile_time[s_n]) unless windows
+      assert !@handler.send(:compile_template?, nil, @a, {})
+      assert !@handler.send(:compile_template?, nil, @b, {})
+      assert !@handler.send(:compile_template?, nil, @s, {}) unless windows
       v.send(:compile_and_render_template, @handler, '', @a)
       v.send(:compile_and_render_template, @handler, '', @b)
       v.send(:compile_and_render_template, @handler, '', @s)  unless windows
       # none of the files have changed since last compile
-      assert v.compile_time[a_n] < t + 3.seconds
-      assert v.compile_time[b_n] < t + 3.seconds
-      assert v.compile_time[s_n] < t + 3.seconds  unless windows
+      assert @handler.compile_time[a_n] < t + 3.seconds
+      assert @handler.compile_time[b_n] < t + 3.seconds
+      assert @handler.compile_time[s_n] < t + 3.seconds  unless windows
 
       `rm #{@s}; ln -s #{@b} #{@s}` unless windows
       # private methods template_changed_since? and compile_template?
@@ -140,12 +141,12 @@ class CompiledTemplateTests < Test::Unit::TestCase
       File::Stat.any_instance.expects(:mtime).times(windows ? 6 : 9).returns(
         *(windows ? [ t + 1.second, t + 1.second ] :
           [ t + 1.second, t + 1.second, t + 3.second ]) * 3)
-      assert !v.send(:template_changed_since?, @a, v.compile_time[a_n])
-      assert !v.send(:template_changed_since?, @b, v.compile_time[b_n])
-      assert v.send(:template_changed_since?, @s, v.compile_time[s_n]) unless windows
-      assert !v.send(:compile_template?, nil, @a, {})
-      assert !v.send(:compile_template?, nil, @b, {})
-      assert v.send(:compile_template?, nil, @s, {}) unless windows
+      assert !@handler.send(:template_changed_since?, @a, @handler.compile_time[a_n])
+      assert !@handler.send(:template_changed_since?, @b, @handler.compile_time[b_n])
+      assert @handler.send(:template_changed_since?, @s, @handler.compile_time[s_n]) unless windows
+      assert !@handler.send(:compile_template?, nil, @a, {})
+      assert !@handler.send(:compile_template?, nil, @b, {})
+      assert @handler.send(:compile_template?, nil, @s, {}) unless windows
 
       # Only the symlink template gets rendered at t+3
       Time.stubs(:now).returns(t + 3.seconds) unless windows
@@ -153,9 +154,9 @@ class CompiledTemplateTests < Test::Unit::TestCase
       v.send(:compile_and_render_template, @handler, '', @b)
       v.send(:compile_and_render_template, @handler, '', @s) unless windows
       # the symlink has changed since last compile
-      assert v.compile_time[a_n] < t + 3.seconds
-      assert v.compile_time[b_n] < t + 3.seconds
-      assert_equal v.compile_time[s_n], t + 3.seconds unless windows
+      assert @handler.compile_time[a_n] < t + 3.seconds
+      assert @handler.compile_time[b_n] < t + 3.seconds
+      assert_equal @handler.compile_time[s_n], t + 3.seconds unless windows
 
       FileUtils.touch @b
       # private methods template_changed_since? and compile_template?
@@ -166,12 +167,12 @@ class CompiledTemplateTests < Test::Unit::TestCase
       File::Stat.any_instance.expects(:mtime).times(windows ? 6 : 12).returns(
         *(windows ? [ t + 1.second, t + 4.seconds ] :
           [ t + 1.second, t + 4.seconds, t + 3.second, t + 4.seconds ]) * 3)
-      assert !v.send(:template_changed_since?, @a, v.compile_time[a_n])
-      assert v.send(:template_changed_since?, @b, v.compile_time[b_n])
-      assert v.send(:template_changed_since?, @s, v.compile_time[s_n]) unless windows
-      assert !v.send(:compile_template?, nil, @a, {})
-      assert v.send(:compile_template?, nil, @b, {})
-      assert v.send(:compile_template?, nil, @s, {}) unless windows
+      assert !@handler.send(:template_changed_since?, @a, @handler.compile_time[a_n])
+      assert @handler.send(:template_changed_since?, @b, @handler.compile_time[b_n])
+      assert @handler.send(:template_changed_since?, @s, @handler.compile_time[s_n]) unless windows
+      assert !@handler.send(:compile_template?, nil, @a, {})
+      assert @handler.send(:compile_template?, nil, @b, {})
+      assert @handler.send(:compile_template?, nil, @s, {}) unless windows
 
       Time.expects(:now).times(windows ? 1 : 2).returns(t + 5.seconds)
       v.send(:compile_and_render_template, @handler, '', @a)
@@ -179,20 +180,9 @@ class CompiledTemplateTests < Test::Unit::TestCase
       v.send(:compile_and_render_template, @handler, '', @s) unless windows
       # the file at the end of the symlink has changed since last compile
       # both the symlink and the file at the end of it should be recompiled
-      assert v.compile_time[a_n] < t + 5.seconds
-      assert_equal v.compile_time[b_n], t + 5.seconds
-      assert_equal v.compile_time[s_n], t + 5.seconds unless windows
-    end
-  end
-end
-
-module ActionView
-  class Base
-    def compile_time
-      @@compile_time
-    end
-    def method_names
-      @@method_names
+      assert @handler.compile_time[a_n] < t + 5.seconds
+      assert_equal @handler.compile_time[b_n], t + 5.seconds
+      assert_equal @handler.compile_time[s_n], t + 5.seconds unless windows
     end
   end
 end