Merge pull request #35293 from rails/remove-rendered-format-from-cache

Pass the template format to the digestor

9 files changed, 33 insertions, 33 deletions

diff --git a/actionview/lib/action_view/base.rb b/actionview/lib/action_view/base.rb
index c0d2d258c5..556a5ee502 100644
--- a/actionview/lib/action_view/base.rb
+++ b/actionview/lib/action_view/base.rb
@@ -257,6 +257,7 @@ module ActionView #:nodoc:
       end
 
       @view_renderer = ActionView::Renderer.new @lookup_context
+      @current_template = nil
 
       @cache_hit = {}
       assign(assigns)
@@ -264,12 +265,13 @@ module ActionView #:nodoc:
       _prepare_context
     end
 
-    def run(method, locals, buffer, &block)
-      _old_output_buffer, _old_virtual_path = @output_buffer, @virtual_path
+    def run(method, template, locals, buffer, &block)
+      _old_output_buffer, _old_virtual_path, _old_template = @output_buffer, @virtual_path, @current_template
+      @current_template = template
       @output_buffer = buffer
       send(method, locals, buffer, &block)
     ensure
-      @output_buffer, @virtual_path = _old_output_buffer, _old_virtual_path
+      @output_buffer, @virtual_path, @current_template = _old_output_buffer, _old_virtual_path, _old_template
     end
 
     def compiled_method_container
diff --git a/actionview/lib/action_view/digestor.rb b/actionview/lib/action_view/digestor.rb
index 6d2e471a44..9fa8d7eab1 100644
--- a/actionview/lib/action_view/digestor.rb
+++ b/actionview/lib/action_view/digestor.rb
@@ -18,11 +18,11 @@ module ActionView
       # * <tt>name</tt>   - Template name
       # * <tt>finder</tt>  - An instance of <tt>ActionView::LookupContext</tt>
       # * <tt>dependencies</tt>  - An array of dependent views
-      def digest(name:, finder:, dependencies: nil)
+      def digest(name:, format:, finder:, dependencies: nil)
         if dependencies.nil? || dependencies.empty?
-          cache_key = "#{name}.#{finder.rendered_format}"
+          cache_key = "#{name}.#{format}"
         else
-          cache_key = [ name, finder.rendered_format, dependencies ].flatten.compact.join(".")
+          cache_key = [ name, format, dependencies ].flatten.compact.join(".")
         end
 
         # this is a correctly done double-checked locking idiom
@@ -48,8 +48,6 @@ module ActionView
         logical_name = name.gsub(%r|/_|, "/")
 
         if template = find_template(finder, logical_name, [], partial, [])
-          finder.rendered_format ||= template.formats.first
-
           if node = seen[template.identifier] # handle cycles in the tree
             node
           else
@@ -73,9 +71,7 @@ module ActionView
       private
         def find_template(finder, name, prefixes, partial, keys)
           finder.disable_cache do
-            format = finder.rendered_format
-            result = finder.find_all(name, prefixes, partial, keys, formats: [format]).first if format
-            result || finder.find_all(name, prefixes, partial, keys).first
+            finder.find_all(name, prefixes, partial, keys).first
           end
         end
     end
diff --git a/actionview/lib/action_view/helpers/cache_helper.rb b/actionview/lib/action_view/helpers/cache_helper.rb
index b1a14250c3..6b69b71947 100644
--- a/actionview/lib/action_view/helpers/cache_helper.rb
+++ b/actionview/lib/action_view/helpers/cache_helper.rb
@@ -216,13 +216,13 @@ module ActionView
         end
       end
 
-      def digest_path_from_virtual(virtual_path) # :nodoc:
-        digest = Digestor.digest(name: virtual_path, finder: lookup_context, dependencies: view_cache_dependencies)
+      def digest_path_from_template(template) # :nodoc:
+        digest = Digestor.digest(name: template.virtual_path, format: template.formats.first, finder: lookup_context, dependencies: view_cache_dependencies)
 
         if digest.present?
-          "#{virtual_path}:#{digest}"
+          "#{template.virtual_path}:#{digest}"
         else
-          virtual_path
+          template.virtual_path
         end
       end
 
@@ -234,7 +234,7 @@ module ActionView
         if virtual_path || digest_path
           name = controller.url_for(name).split("://").last if name.is_a?(Hash)
 
-          digest_path ||= digest_path_from_virtual(virtual_path)
+          digest_path ||= digest_path_from_template(@current_template)
 
           [ digest_path, name ]
         else
diff --git a/actionview/lib/action_view/renderer/partial_renderer/collection_caching.rb b/actionview/lib/action_view/renderer/partial_renderer/collection_caching.rb
index 388f9e5e56..9f1de5a397 100644
--- a/actionview/lib/action_view/renderer/partial_renderer/collection_caching.rb
+++ b/actionview/lib/action_view/renderer/partial_renderer/collection_caching.rb
@@ -54,7 +54,7 @@ module ActionView
       def collection_by_cache_keys(view, template)
         seed = callable_cache_key? ? @options[:cached] : ->(i) { i }
 
-        digest_path = view.digest_path_from_virtual(template.virtual_path)
+        digest_path = view.digest_path_from_template(template)
 
         @collection.each_with_object({}) do |item, hash|
           hash[expanded_cache_key(seed.call(item), view, template, digest_path)] = item
diff --git a/actionview/lib/action_view/template.rb b/actionview/lib/action_view/template.rb
index 37f476e554..907b322c8a 100644
--- a/actionview/lib/action_view/template.rb
+++ b/actionview/lib/action_view/template.rb
@@ -165,7 +165,7 @@ module ActionView
     def render(view, locals, buffer = ActionView::OutputBuffer.new, &block)
       instrument_render_template do
         compile!(view)
-        view.run(method_name, locals, buffer, &block)
+        view.run(method_name, self, locals, buffer, &block)
       end
     rescue => e
       handle_render_error(view, e)
diff --git a/actionview/lib/action_view/template/handlers/erb/erubi.rb b/actionview/lib/action_view/template/handlers/erb/erubi.rb
index 20510c3062..247b6d75d1 100644
--- a/actionview/lib/action_view/template/handlers/erb/erubi.rb
+++ b/actionview/lib/action_view/template/handlers/erb/erubi.rb
@@ -27,7 +27,7 @@ module ActionView
               include action_view_erb_handler_context._routes.url_helpers
               class_eval("define_method(:_template) { |local_assigns, output_buffer| #{src} }", @filename || "(erubi)", 0)
             }.empty
-            view.run(:_template, {}, ActionView::OutputBuffer.new)
+            view.run(:_template, nil, {}, ActionView::OutputBuffer.new)
           end
 
         private
diff --git a/actionview/test/activerecord/relation_cache_test.rb b/actionview/test/activerecord/relation_cache_test.rb
index a6befc3ee5..6fe83dcb9a 100644
--- a/actionview/test/activerecord/relation_cache_test.rb
+++ b/actionview/test/activerecord/relation_cache_test.rb
@@ -11,13 +11,14 @@ class RelationCacheTest < ActionView::TestCase
     lookup_context = ActionView::LookupContext.new(view_paths, {}, ["test"])
     @view_renderer = ActionView::Renderer.new(lookup_context)
     @virtual_path  = "path"
+    @current_template = lookup_context.find "test/hello_world"
 
     controller.cache_store = ActiveSupport::Cache::MemoryStore.new
   end
 
   def test_cache_relation_other
     cache(Project.all) { concat("Hello World") }
-    assert_equal "Hello World", controller.cache_store.read("views/path/projects-#{Project.count}")
+    assert_equal "Hello World", controller.cache_store.read("views/test/hello_world:fa9482a68ce25bf7589b8eddad72f736/projects-#{Project.count}")
   end
 
   def view_cache_dependencies; []; end
diff --git a/actionview/test/template/digestor_test.rb b/actionview/test/template/digestor_test.rb
index ddaa7febb3..91861edf11 100644
--- a/actionview/test/template/digestor_test.rb
+++ b/actionview/test/template/digestor_test.rb
@@ -7,9 +7,8 @@ require "action_view/dependency_tracker"
 class FixtureFinder < ActionView::LookupContext
   FIXTURES_DIR = File.expand_path("../fixtures/digestor", __dir__)
 
-  def initialize(details = {})
-    super(ActionView::PathSet.new(["digestor", "digestor/api"]), details, [])
-    @rendered_format = :html
+  def self.build(details = {})
+    new(ActionView::PathSet.new(["digestor", "digestor/api"]), details, [])
   end
 end
 
@@ -146,13 +145,12 @@ class TemplateDigestorTest < ActionView::TestCase
   end
 
   def test_nested_template_deps_with_non_default_rendered_format
-    finder.rendered_format = nil
     nested_deps = [{ "comments/comments" => ["comments/comment"] }]
     assert_equal nested_deps, nested_dependencies("messages/thread")
   end
 
   def test_template_formats_of_nested_deps_with_non_default_rendered_format
-    finder.rendered_format = nil
+    @finder = finder.with_prepended_formats([:json])
     assert_equal [:json], tree_template_formats("messages/thread").uniq
   end
 
@@ -161,12 +159,10 @@ class TemplateDigestorTest < ActionView::TestCase
   end
 
   def test_template_dependencies_with_fallback_from_js_to_html_format
-    finder.rendered_format = :js
     assert_equal ["comments/comment"], dependencies("comments/show")
   end
 
   def test_template_digest_with_fallback_from_js_to_html_format
-    finder.rendered_format = :js
     assert_digest_difference("comments/show") do
       change_template("comments/_comment")
     end
@@ -219,14 +215,14 @@ class TemplateDigestorTest < ActionView::TestCase
 
   def test_details_are_included_in_cache_key
     # Cache the template digest.
-    @finder = FixtureFinder.new(formats: [:html])
+    @finder = FixtureFinder.build(formats: [:html])
     old_digest = digest("events/_event")
 
     # Change the template; the cached digest remains unchanged.
     change_template("events/_event")
 
     # The details are changed, so a new cache key is generated.
-    @finder = FixtureFinder.new
+    @finder = FixtureFinder.build
 
     # The cache is busted.
     assert_not_equal old_digest, digest("events/_event")
@@ -343,9 +339,14 @@ class TemplateDigestorTest < ActionView::TestCase
       finder_options = options.extract!(:variants, :format)
 
       finder.variants = finder_options[:variants] || []
-      finder.rendered_format = finder_options[:format] if finder_options[:format]
 
-      ActionView::Digestor.digest(name: template_name, finder: finder, dependencies: (options[:dependencies] || []))
+      finder_with_formats = if finder_options[:format]
+        finder.with_prepended_formats(Array(finder_options[:format]))
+      else
+        finder
+      end
+
+      ActionView::Digestor.digest(name: template_name, format: finder_options[:format], finder: finder_with_formats, dependencies: (options[:dependencies] || []))
     end
 
     def dependencies(template_name)
@@ -371,7 +372,7 @@ class TemplateDigestorTest < ActionView::TestCase
     end
 
     def finder
-      @finder ||= FixtureFinder.new
+      @finder ||= FixtureFinder.build
     end
 
     def change_template(template_name, variant = nil)
diff --git a/actionview/test/template/render_test.rb b/actionview/test/template/render_test.rb
index 543df7a71a..5068e00c7d 100644
--- a/actionview/test/template/render_test.rb
+++ b/actionview/test/template/render_test.rb
@@ -790,7 +790,7 @@ class CachedCollectionViewRenderTest < ActiveSupport::TestCase
 
   private
     def cache_key(*names, virtual_path)
-      digest = ActionView::Digestor.digest name: virtual_path, finder: @view.lookup_context, dependencies: []
+      digest = ActionView::Digestor.digest name: virtual_path, format: :html, finder: @view.lookup_context, dependencies: []
       @view.combined_fragment_cache_key([ "#{virtual_path}:#{digest}", *names ])
     end
 end