Fix some edge cases when the same template is called with different local assigns

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

actionpack/lib/action_view/renderable.rb

@@ -16,8 +16,18 @@ module ActionView
     memoize :handler
 
     def compiled_source
+      @compiled_at = Time.now
       handler.call(self)
     end
+    memoize :compiled_source
+
+    def compiled_at
+      @compiled_at
+    end
+
+    def defined_at
+      @defined_at ||= {}
+    end
 
     def method_name_without_locals
       ['_run', extension, method_segment].compact.join('_')
@@ -61,8 +71,12 @@ module ActionView
       def compile(local_assigns)
         render_symbol = method_name(local_assigns)
 
-        if !Base::CompiledTemplates.method_defined?(render_symbol) || recompile?
+        if self.is_a?(InlineTemplate)
           compile!(render_symbol, local_assigns)
+        else
+          if !Base::CompiledTemplates.method_defined?(render_symbol) || recompile?(render_symbol)
+            recompile!(render_symbol, local_assigns)
+          end
         end
       end
 
@@ -79,6 +93,7 @@ module ActionView
 
         begin
           ActionView::Base::CompiledTemplates.module_eval(source, filename, 0)
+          defined_at[render_symbol] = Time.now if respond_to?(:reloadable?) && reloadable?
         rescue Errno::ENOENT => e
           raise e # Missing template file, re-raise for Base to rescue
         rescue Exception => e # errors from template code
@@ -91,5 +106,18 @@ module ActionView
           raise ActionView::TemplateError.new(self, {}, e)
         end
       end
+
+      def recompile?(render_symbol)
+        !cached? || redefine?(render_symbol) || stale?
+      end
+
+      def recompile!(render_symbol, local_assigns)
+        compiled_source(:reload) if compiled_at.nil? || compiled_at < mtime
+        compile!(render_symbol, local_assigns)
+      end
+
+      def redefine?(render_symbol)
+        compiled_at && defined_at[render_symbol] && compiled_at > defined_at[render_symbol]
+      end
   end
 end

actionpack/lib/action_view/template.rb

@@ -7,7 +7,9 @@ module ActionView #:nodoc:
       def initialize(path)
         raise ArgumentError, "path already is a Path class" if path.is_a?(Path)
         @path = path.freeze
+      end
 
+      def load!
         @paths = {}
         templates_in_path do |template|
           load_template(template)
@@ -44,6 +46,7 @@ module ActionView #:nodoc:
       # etc. A format must be supplied to match a formated file. +hello/index+
       # will never match +hello/index.html.erb+.
       def [](path)
+        load! if @paths.nil?
         @paths[path] || find_template(path)
       end
 
@@ -197,26 +200,22 @@ module ActionView #:nodoc:
     end
 
     def stale?
-      !frozen? && mtime < mtime(:reload)
+      reloadable? && (mtime < mtime(:reload))
     end
 
     def load!
       reloadable? ? memoize_all : freeze
     end
 
+    def reloadable?
+      !(Base.cache_template_loading || ActionController::Base.allow_concurrency)
+    end
+
+    def cached?
+      ActionController::Base.perform_caching || !reloadable?
+    end
+
     private
-      def cached?
-        Base.cache_template_loading || ActionController::Base.allow_concurrency
-      end
-
-      def reloadable?
-        !cached?
-      end
-
-      def recompile?
-        reloadable? ? stale? : false
-      end
-
       def valid_extension?(extension)
         !Template.registered_template_handler(extension).nil?
       end

actionpack/test/template/compiled_templates_test.rb

@@ -10,52 +10,64 @@ class CompiledTemplatesTest < Test::Unit::TestCase
   end
 
   def test_template_gets_compiled
-    assert_equal 0, @compiled_templates.instance_methods.size
+    with_caching(true) do
-    assert_equal "Hello world!", render(:file => "test/hello_world.erb")
+      assert_equal 0, @compiled_templates.instance_methods.size
-    assert_equal 1, @compiled_templates.instance_methods.size
+      assert_equal "Hello world!", render(:file => "test/hello_world.erb")
+      assert_equal 1, @compiled_templates.instance_methods.size
+    end
   end
 
   def test_template_gets_recompiled_when_using_different_keys_in_local_assigns
-    assert_equal 0, @compiled_templates.instance_methods.size
+    with_caching(true) do
-    assert_equal "Hello world!", render(:file => "test/hello_world.erb")
+      assert_equal 0, @compiled_templates.instance_methods.size
-    assert_equal "Hello world!", render(:file => "test/hello_world.erb", :locals => {:foo => "bar"})
+      assert_equal "Hello world!", render(:file => "test/hello_world.erb")
-    assert_equal 2, @compiled_templates.instance_methods.size
+      assert_equal "Hello world!", render(:file => "test/hello_world.erb", :locals => {:foo => "bar"})
+      assert_equal 2, @compiled_templates.instance_methods.size
+    end
   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!", render(:file => "test/hello_world.erb")
-    ActionView::Template.any_instance.expects(:compile!).never
-    assert_equal "Hello world!", render(:file => "test/hello_world.erb")
-  end
-
-  def test_compiled_template_will_always_be_recompiled_when_template_is_not_cached
-    ActionView::Template.any_instance.expects(:recompile?).times(3).returns(true)
-    assert_equal 0, @compiled_templates.instance_methods.size
-    assert_equal "Hello world!", render(:file => "#{FIXTURE_LOAD_PATH}/test/hello_world.erb")
-    ActionView::Template.any_instance.expects(:compile!).times(3)
-    3.times { assert_equal "Hello world!", render(:file => "#{FIXTURE_LOAD_PATH}/test/hello_world.erb") }
-    assert_equal 1, @compiled_templates.instance_methods.size
-  end
-
-  def test_template_changes_are_not_reflected_with_cached_templates
     with_caching(true) do
+      assert_equal 0, @compiled_templates.instance_methods.size
       assert_equal "Hello world!", render(:file => "test/hello_world.erb")
-      modify_template "test/hello_world.erb", "Goodbye world!" do
+      ActionView::Template.any_instance.expects(:compile!).never
-        assert_equal "Hello world!", render(:file => "test/hello_world.erb")
-      end
       assert_equal "Hello world!", render(:file => "test/hello_world.erb")
     end
   end
 
-  def test_template_changes_are_reflected_without_cached_templates
+  def test_compiled_template_will_always_be_recompiled_when_template_is_not_cached
     with_caching(false) do
-      assert_equal "Hello world!", render(:file => "test/hello_world.erb")
+      ActionView::Template.any_instance.expects(:recompile?).times(3).returns(true)
-      modify_template "test/hello_world.erb", "Goodbye world!" do
+      assert_equal 0, @compiled_templates.instance_methods.size
-        assert_equal "Goodbye world!", render(:file => "test/hello_world.erb")
+      assert_equal "Hello world!", render(:file => "#{FIXTURE_LOAD_PATH}/test/hello_world.erb")
-        sleep(1) # Need to sleep so that the timestamp actually changes
+      ActionView::Template.any_instance.expects(:compile!).times(3)
+      3.times { assert_equal "Hello world!", render(:file => "#{FIXTURE_LOAD_PATH}/test/hello_world.erb") }
+      assert_equal 1, @compiled_templates.instance_methods.size
+    end
+  end
+
+  def test_template_changes_are_not_reflected_with_cached_template_loading
+    with_caching(true) do
+      with_reloading(false) do
+        assert_equal "Hello world!", render(:file => "test/hello_world.erb")
+        modify_template "test/hello_world.erb", "Goodbye world!" do
+          assert_equal "Hello world!", render(:file => "test/hello_world.erb")
+        end
+        assert_equal "Hello world!", render(:file => "test/hello_world.erb")
+      end
+    end
+  end
+
+  def test_template_changes_are_reflected_without_cached_template_loading
+    with_caching(true) do
+      with_reloading(true) do
+        assert_equal "Hello world!", render(:file => "test/hello_world.erb")
+        modify_template "test/hello_world.erb", "Goodbye world!" do
+          assert_equal "Goodbye world!", render(:file => "test/hello_world.erb")
+          sleep(1) # Need to sleep so that the timestamp actually changes
+        end
+        assert_equal "Hello world!", render(:file => "test/hello_world.erb")
       end
-      assert_equal "Hello world!", render(:file => "test/hello_world.erb")
     end
   end
 
@@ -77,13 +89,23 @@ class CompiledTemplatesTest < Test::Unit::TestCase
       end
     end
 
-    def with_caching(caching_enabled)
+    def with_caching(perform_caching)
-      old_caching_enabled = ActionView::Base.cache_template_loading
+      old_perform_caching = ActionController::Base.perform_caching
       begin
-        ActionView::Base.cache_template_loading = caching_enabled
+        ActionController::Base.perform_caching = perform_caching
         yield
       ensure
-        ActionView::Base.cache_template_loading = old_caching_enabled
+        ActionController::Base.perform_caching = old_perform_caching
+      end
+    end
+
+    def with_reloading(reload_templates)
+      old_cache_template_loading = ActionView::Base.cache_template_loading
+      begin
+        ActionView::Base.cache_template_loading = !reload_templates
+        yield
+      ensure
+        ActionView::Base.cache_template_loading = old_cache_template_loading
       end
     end
 end

railties/lib/initializer.rb

@@ -369,10 +369,8 @@ Run `rake gems:install` to install the missing gems.
 
     def load_view_paths
       if configuration.frameworks.include?(:action_view)
-        if configuration.cache_classes
+        ActionController::Base.view_paths.each { |path| path.load! } if configuration.frameworks.include?(:action_controller)
-          ActionController::Base.view_paths = configuration.view_path if configuration.frameworks.include?(:action_controller)
+        ActionMailer::Base.template_root.load! if configuration.frameworks.include?(:action_mailer)
-          ActionMailer::Base.template_root = configuration.view_path if configuration.frameworks.include?(:action_mailer)
-        end
       end
     end