Merge pull request #35623 from jhawthorn/actionview_cache

Make Template::Resolver always cache

actionview/lib/action_view/file_template.rb

@@ -22,11 +22,11 @@ def refresh(_)
     # to ensure that references to the template object can be marshalled as well. This means forgoing
     # the marshalling of the compiler mutex and instantiating that again on unmarshalling.
     def marshal_dump # :nodoc:
-      [ @identifier, @handler, @compiled, @locals, @virtual_path, @updated_at, @format, @variant ]
+      [ @identifier, @handler, @compiled, @locals, @virtual_path, @format, @variant ]
     end
 
     def marshal_load(array) # :nodoc:
-      @identifier, @handler, @compiled, @locals, @virtual_path, @updated_at, @format, @variant = *array
+      @identifier, @handler, @compiled, @locals, @virtual_path, @format, @variant = *array
       @compile_mutex = Mutex.new
     end
   end

actionview/lib/action_view/template.rb

@@ -122,10 +122,10 @@ def self.finalize_compiled_template_methods=(_)
 
     extend Template::Handlers
 
-    attr_reader :source, :identifier, :handler, :original_encoding, :updated_at
+    attr_reader :source, :identifier, :handler, :original_encoding
     attr_reader :variable, :format, :variant, :locals, :virtual_path
 
-    def initialize(source, identifier, handler, format: nil, variant: nil, locals: nil, virtual_path: nil, updated_at: Time.now)
+    def initialize(source, identifier, handler, format: nil, variant: nil, locals: nil, virtual_path: nil)
       unless locals
         ActiveSupport::Deprecation.warn "ActionView::Template#initialize requires a locals parameter"
         locals = []
@@ -144,7 +144,6 @@ def initialize(source, identifier, handler, format: nil, variant: nil, locals: n
         $1.to_sym
       end
 
-      @updated_at        = updated_at
       @format            = format
       @variant           = variant
       @compile_mutex     = Mutex.new
@@ -261,11 +260,11 @@ def encode!
     # to ensure that references to the template object can be marshalled as well. This means forgoing
     # the marshalling of the compiler mutex and instantiating that again on unmarshalling.
     def marshal_dump # :nodoc:
-      [ @source, @identifier, @handler, @compiled, @locals, @virtual_path, @updated_at, @format, @variant ]
+      [ @source, @identifier, @handler, @compiled, @locals, @virtual_path, @format, @variant ]
     end
 
     def marshal_load(array) # :nodoc:
-      @source, @identifier, @handler, @compiled, @locals, @virtual_path, @updated_at, @format, @variant = *array
+      @source, @identifier, @handler, @compiled, @locals, @virtual_path, @format, @variant = *array
       @compile_mutex = Mutex.new
     end
 

actionview/lib/action_view/template/resolver.rb

@@ -63,26 +63,11 @@ def inspect
 
       # 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 @@ def size # :nodoc:
         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
@@ -218,8 +190,7 @@ def query(path, details, formats, outside_app_allowed, locals)
             virtual_path: path.virtual,
             format: format,
             variant: variant,
-            locals: locals,
-            updated_at: mtime(template)
+            locals: locals
           )
         end
       end
@@ -272,11 +243,6 @@ def escape_entry(entry)
         entry.gsub(/[*?{}\[\]]/, '\\\\\\&')
       end
 
-      # Returns the file mtime from the filesystem.
-      def mtime(p)
-        File.mtime(p)
-      end
-
       # Extract handler, formats and variant from path. If a format cannot be found neither
       # from the path, or the handler, we should return the array of formats given
       # to the resolver.

actionview/lib/action_view/testing/resolvers.rb

@@ -31,16 +31,14 @@ def query(path, exts, _, _, locals)
         query = /^(#{Regexp.escape(path)})#{query}$/
 
         templates = []
-        @hash.each do |_path, array|
-          source, updated_at = array
+        @hash.each do |_path, source|
           next unless query.match?(_path)
           handler, format, variant = extract_handler_and_format_and_variant(_path)
           templates << Template.new(source, _path, handler,
             virtual_path: path.virtual,
             format: format,
             variant: variant,
-            locals: locals,
-            updated_at: updated_at
+            locals: locals
           )
         end
 

actionview/test/template/digestor_test.rb

@@ -326,12 +326,14 @@ def assert_logged(message)
 
     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 = {})

actionview/test/template/lookup_context_test.rb

@@ -235,56 +235,6 @@ def teardown
   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", {})