Fixed template recompile logic [#630 state:resolved]

Signed-off-by: Joshua Peek <josh@joshpeek.com>

3 files changed, 73 insertions, 22 deletions

diff --git a/actionpack/lib/action_view/renderable.rb b/actionpack/lib/action_view/renderable.rb
index badb2b171c..46193670f3 100644
--- a/actionpack/lib/action_view/renderable.rb
+++ b/actionpack/lib/action_view/renderable.rb
@@ -9,6 +9,10 @@ module ActionView
 
     include ActiveSupport::Memoizable
 
+    def filename
+      'compiled-template'
+    end
+
     def handler
       Template.handler_class_for_extension(extension)
     end
@@ -35,35 +39,41 @@ module ActionView
     end
 
     private
-      # Compile and evaluate the template's code
+      # Compile and evaluate the template's code (if necessary)
       def compile(local_assigns)
         render_symbol = method(local_assigns)
 
         @@mutex.synchronize do
-          return false unless recompile?(render_symbol)
+          if recompile?(render_symbol)
+            compile!(render_symbol, local_assigns)
+          end
+        end
+      end
 
-          locals_code = local_assigns.keys.map { |key| "#{key} = local_assigns[:#{key}];" }.join
+      def compile!(render_symbol, local_assigns)
+        locals_code = local_assigns.keys.map { |key| "#{key} = local_assigns[:#{key}];" }.join
 
-          source = <<-end_src
-            def #{render_symbol}(local_assigns)
-              old_output_buffer = output_buffer;#{locals_code};#{compiled_source}
-            ensure
-              self.output_buffer = old_output_buffer
-            end
-          end_src
+        source = <<-end_src
+          def #{render_symbol}(local_assigns)
+            old_output_buffer = output_buffer;#{locals_code};#{compiled_source}
+          ensure
+            self.output_buffer = old_output_buffer
+          end
+        end_src
 
-          begin
-            file_name = respond_to?(:filename) ? filename : 'compiled-template'
-            ActionView::Base::CompiledTemplates.module_eval(source, file_name, 0)
-          rescue Exception => e # errors from template code
-            if logger = ActionController::Base.logger
-              logger.debug "ERROR: compiling #{render_symbol} RAISED #{e}"
-              logger.debug "Function body: #{source}"
-              logger.debug "Backtrace: #{e.backtrace.join("\n")}"
-            end
+        begin
+          logger = ActionController::Base.logger
+          logger.debug "Compiling template #{render_symbol}" if logger
 
-            raise ActionView::TemplateError.new(self, {}, e)
+          ActionView::Base::CompiledTemplates.module_eval(source, filename, 0)
+        rescue Exception => e # errors from template code
+          if logger
+            logger.debug "ERROR: compiling #{render_symbol} RAISED #{e}"
+            logger.debug "Function body: #{source}"
+            logger.debug "Backtrace: #{e.backtrace.join("\n")}"
           end
+
+          raise ActionView::TemplateError.new(self, {}, e)
         end
       end
 
@@ -71,8 +81,7 @@ module ActionView
       # The template will be compiled if the file has not been compiled yet, or
       # if local_assigns has a new key, which isn't supported by the compiled code yet.
       def recompile?(symbol)
-        meth = Base::CompiledTemplates.instance_method(template.method) rescue nil
-        !(meth && frozen?)
+        !(frozen? && Base::CompiledTemplates.method_defined?(symbol))
       end
   end
 end
diff --git a/actionpack/test/abstract_unit.rb b/actionpack/test/abstract_unit.rb
index 520379fe82..9db4cddd6a 100644
--- a/actionpack/test/abstract_unit.rb
+++ b/actionpack/test/abstract_unit.rb
@@ -23,6 +23,7 @@ ActionController::Base.logger = nil
 ActionController::Routing::Routes.reload rescue nil
 
 FIXTURE_LOAD_PATH = File.join(File.dirname(__FILE__), 'fixtures')
+ActionView::PathSet::Path.eager_load_templates!
 ActionController::Base.view_paths = FIXTURE_LOAD_PATH
 
 # Wrap tests that use Mocha and skip if unavailable.
diff --git a/actionpack/test/template/compiled_templates_test.rb b/actionpack/test/template/compiled_templates_test.rb
new file mode 100644
index 0000000000..4b34827f91
--- /dev/null
+++ b/actionpack/test/template/compiled_templates_test.rb
@@ -0,0 +1,41 @@
+require 'abstract_unit'
+require 'controller/fake_models'
+
+uses_mocha 'TestTemplateRecompilation' do
+  class CompiledTemplatesTest < Test::Unit::TestCase
+    def setup
+      @view = ActionView::Base.new(ActionController::Base.view_paths, {})
+      @compiled_templates = ActionView::Base::CompiledTemplates
+      @compiled_templates.instance_methods.each do |m|
+        @compiled_templates.send(:remove_method, m) if m =~ /^_run_/
+      end
+    end
+
+    def test_template_gets_compiled
+      assert_equal 0, @compiled_templates.instance_methods.size
+      assert_equal "Hello world!", @view.render("test/hello_world.erb")
+      assert_equal 1, @compiled_templates.instance_methods.size
+    end
+
+    def test_template_gets_recompiled_when_using_different_keys_in_local_assigns
+      assert_equal 0, @compiled_templates.instance_methods.size
+      assert_equal "Hello world!", @view.render("test/hello_world.erb")
+      assert_equal "Hello world!", @view.render("test/hello_world.erb", {:foo => "bar"})
+      assert_equal 2, @compiled_templates.instance_methods.size
+    end
+
+    def test_compiled_template_will_not_be_recompiled_when_rendered_with_identical_local_assigns
+      assert_equal 0, @compiled_templates.instance_methods.size
+      assert_equal "Hello world!", @view.render("test/hello_world.erb")
+      ActionView::Template.any_instance.expects(:compile!).never
+      assert_equal "Hello world!", @view.render("test/hello_world.erb")
+    end
+
+    def test_compiled_template_will_always_be_recompiled_when_rendered_if_template_is_outside_cache
+      assert_equal 0, @compiled_templates.instance_methods.size
+      assert_equal "Hello world!", @view.render("#{FIXTURE_LOAD_PATH}/test/hello_world.erb")
+      ActionView::Template.any_instance.expects(:compile!).times(3)
+      3.times { assert_equal "Hello world!", @view.render("#{FIXTURE_LOAD_PATH}/test/hello_world.erb") }
+    end
+  end
+end