Make Template::Resolver always cache

All actionview caches are already cleared at the start of each request
(when Resolver.caching is false) by PerExecutionDigestCacheExpiry, which
calls LookupContext::DetailsKey.clear (which clears all caches).

Because caches are always cleared per-request in dev, we shouldn't need
this extra logic to compare mtimes and conditionally reload templates.

This should make templates slightly faster in development (particularly
multiple renders of the same template)

3 files changed, 4 insertions, 80 deletions

diff --git a/actionview/lib/action_view/template/resolver.rb b/actionview/lib/action_view/template/resolver.rb
index 1dc9c9919a..eeb7cce61e 100644
--- a/actionview/lib/action_view/template/resolver.rb
+++ b/actionview/lib/action_view/template/resolver.rb
@@ -63,26 +63,11 @@ module ActionView
 
       # Cache the templates returned by the block
       def cache(key, name, prefix, partial, locals)
-        if Resolver.caching?
-          @data[key][name][prefix][partial][locals] ||= canonical_no_templates(yield)
-        else
-          fresh_templates  = yield
-          cached_templates = @data[key][name][prefix][partial][locals]
-
-          if templates_have_changed?(cached_templates, fresh_templates)
-            @data[key][name][prefix][partial][locals] = canonical_no_templates(fresh_templates)
-          else
-            cached_templates || NO_TEMPLATES
-          end
-        end
+        @data[key][name][prefix][partial][locals] ||= canonical_no_templates(yield)
       end
 
       def cache_query(query) # :nodoc:
-        if Resolver.caching?
-          @query_cache[query] ||= canonical_no_templates(yield)
-        else
-          yield
-        end
+        @query_cache[query] ||= canonical_no_templates(yield)
       end
 
       def clear
@@ -112,19 +97,6 @@ module ActionView
         def canonical_no_templates(templates)
           templates.empty? ? NO_TEMPLATES : templates
         end
-
-        def templates_have_changed?(cached_templates, fresh_templates)
-          # if either the old or new template list is empty, we don't need to (and can't)
-          # compare modification times, and instead just check whether the lists are different
-          if cached_templates.blank? || fresh_templates.blank?
-            return fresh_templates.blank? != cached_templates.blank?
-          end
-
-          cached_templates_max_updated_at = cached_templates.map(&:updated_at).max
-
-          # if a template has changed, it will be now be newer than all the cached templates
-          fresh_templates.any? { |t| t.updated_at > cached_templates_max_updated_at }
-        end
     end
 
     cattr_accessor :caching, default: true
diff --git a/actionview/test/template/digestor_test.rb b/actionview/test/template/digestor_test.rb
index 7affd3c005..4515afdfff 100644
--- a/actionview/test/template/digestor_test.rb
+++ b/actionview/test/template/digestor_test.rb
@@ -326,12 +326,14 @@ class TemplateDigestorTest < ActionView::TestCase
 
     def assert_digest_difference(template_name, options = {})
       previous_digest = digest(template_name, options)
+      finder.view_paths.each(&:clear_cache)
       finder.digest_cache.clear
 
       yield
 
       assert_not_equal previous_digest, digest(template_name, options), "digest didn't change"
       finder.digest_cache.clear
+      finder.view_paths.each(&:clear_cache)
     end
 
     def digest(template_name, options = {})
diff --git a/actionview/test/template/lookup_context_test.rb b/actionview/test/template/lookup_context_test.rb
index 62935620e9..537f8ee163 100644
--- a/actionview/test/template/lookup_context_test.rb
+++ b/actionview/test/template/lookup_context_test.rb
@@ -235,56 +235,6 @@ class LookupContextTest < ActiveSupport::TestCase
   end
 end
 
-class LookupContextWithFalseCaching < ActiveSupport::TestCase
-  def setup
-    @resolver = ActionView::FixtureResolver.new("test/_foo.erb" => ["Foo", Time.utc(2000)])
-    @lookup_context = ActionView::LookupContext.new(@resolver, {})
-  end
-
-  test "templates are always found in the resolver but timestamp is checked before being compiled" do
-    ActionView::Resolver.stub(:caching?, false) do
-      template = @lookup_context.find("foo", %w(test), true)
-      assert_equal "Foo", template.source
-
-      # Now we are going to change the template, but it won't change the returned template
-      # since the timestamp is the same.
-      @resolver.data["test/_foo.erb"][0] = "Bar"
-      template = @lookup_context.find("foo", %w(test), true)
-      assert_equal "Foo", template.source
-
-      # Now update the timestamp.
-      @resolver.data["test/_foo.erb"][1] = Time.now.utc
-      template = @lookup_context.find("foo", %w(test), true)
-      assert_equal "Bar", template.source
-    end
-  end
-
-  test "if no template was found in the second lookup, with no cache, raise error" do
-    ActionView::Resolver.stub(:caching?, false) do
-      template = @lookup_context.find("foo", %w(test), true)
-      assert_equal "Foo", template.source
-
-      @resolver.data.clear
-      assert_raise ActionView::MissingTemplate do
-        @lookup_context.find("foo", %w(test), true)
-      end
-    end
-  end
-
-  test "if no template was cached in the first lookup, retrieval should work in the second call" do
-    ActionView::Resolver.stub(:caching?, false) do
-      @resolver.data.clear
-      assert_raise ActionView::MissingTemplate do
-        @lookup_context.find("foo", %w(test), true)
-      end
-
-      @resolver.data["test/_foo.erb"] = ["Foo", Time.utc(2000)]
-      template = @lookup_context.find("foo", %w(test), true)
-      assert_equal "Foo", template.source
-    end
-  end
-end
-
 class TestMissingTemplate < ActiveSupport::TestCase
   def setup
     @lookup_context = ActionView::LookupContext.new("/Path/to/views", {})