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

Pass the template format to the digestor

12 files changed, 53 insertions, 50 deletions

diff --git a/actionmailer/test/caching_test.rb b/actionmailer/test/caching_test.rb
index 22f310f39f..b658c96ec7 100644
--- a/actionmailer/test/caching_test.rb
+++ b/actionmailer/test/caching_test.rb
@@ -124,7 +124,7 @@ class FunctionalFragmentCachingTest < BaseCachingTest
 
     assert_match expected_body, email.body.encoded
     assert_match expected_body,
-      @store.read("views/caching_mailer/fragment_cache:#{template_digest("caching_mailer/fragment_cache")}/caching")
+      @store.read("views/caching_mailer/fragment_cache:#{template_digest("caching_mailer/fragment_cache", "html")}/caching")
   end
 
   def test_fragment_caching_in_partials
@@ -133,7 +133,7 @@ class FunctionalFragmentCachingTest < BaseCachingTest
     assert_match(expected_body, email.body.encoded)
 
     assert_match(expected_body,
-      @store.read("views/caching_mailer/_partial:#{template_digest("caching_mailer/_partial")}/caching"))
+      @store.read("views/caching_mailer/_partial:#{template_digest("caching_mailer/_partial", "html")}/caching"))
   end
 
   def test_skip_fragment_cache_digesting
@@ -183,15 +183,15 @@ class FunctionalFragmentCachingTest < BaseCachingTest
     end
 
     assert_equal "caching_mailer", payload[:mailer]
-    assert_equal [ :views, "caching_mailer/fragment_cache:#{template_digest("caching_mailer/fragment_cache")}", :caching ], payload[:key]
+    assert_equal [ :views, "caching_mailer/fragment_cache:#{template_digest("caching_mailer/fragment_cache", "html")}", :caching ], payload[:key]
   ensure
     @mailer.enable_fragment_cache_logging = true
   end
 
   private
 
-    def template_digest(name)
-      ActionView::Digestor.digest(name: name, finder: @mailer.lookup_context)
+    def template_digest(name, format)
+      ActionView::Digestor.digest(name: name, format: format, finder: @mailer.lookup_context)
     end
 end
 
diff --git a/actionpack/lib/action_controller/metal/etag_with_template_digest.rb b/actionpack/lib/action_controller/metal/etag_with_template_digest.rb
index 640c75536e..2f1544c69c 100644
--- a/actionpack/lib/action_controller/metal/etag_with_template_digest.rb
+++ b/actionpack/lib/action_controller/metal/etag_with_template_digest.rb
@@ -51,7 +51,7 @@ module ActionController
       end
 
       def lookup_and_digest_template(template)
-        ActionView::Digestor.digest name: template, finder: lookup_context
+        ActionView::Digestor.digest name: template, format: nil, finder: lookup_context
       end
   end
 end
diff --git a/actionpack/test/controller/caching_test.rb b/actionpack/test/controller/caching_test.rb
index 5543f9120f..f09e812147 100644
--- a/actionpack/test/controller/caching_test.rb
+++ b/actionpack/test/controller/caching_test.rb
@@ -212,7 +212,7 @@ CACHED
     assert_equal expected_body, @response.body
 
     assert_equal "This bit's fragment cached",
-      @store.read("views/functional_caching/fragment_cached:#{template_digest("functional_caching/fragment_cached")}/fragment")
+      @store.read("views/functional_caching/fragment_cached:#{template_digest("functional_caching/fragment_cached", "html")}/fragment")
   end
 
   def test_fragment_caching_in_partials
@@ -221,7 +221,7 @@ CACHED
     assert_match(/Old fragment caching in a partial/, @response.body)
 
     assert_match("Old fragment caching in a partial",
-      @store.read("views/functional_caching/_partial:#{template_digest("functional_caching/_partial")}/test.host/functional_caching/html_fragment_cached_with_partial"))
+      @store.read("views/functional_caching/_partial:#{template_digest("functional_caching/_partial", "html")}/test.host/functional_caching/html_fragment_cached_with_partial"))
   end
 
   def test_skipping_fragment_cache_digesting
@@ -251,7 +251,7 @@ CACHED
     assert_match(/Some inline content/, @response.body)
     assert_match(/Some cached content/, @response.body)
     assert_match("Some cached content",
-      @store.read("views/functional_caching/inline_fragment_cached:#{template_digest("functional_caching/inline_fragment_cached")}/test.host/functional_caching/inline_fragment_cached"))
+      @store.read("views/functional_caching/inline_fragment_cached:#{template_digest("functional_caching/inline_fragment_cached", "html")}/test.host/functional_caching/inline_fragment_cached"))
   end
 
   def test_fragment_cache_instrumentation
@@ -271,36 +271,39 @@ CACHED
   end
 
   def test_html_formatted_fragment_caching
-    get :formatted_fragment_cached, format: "html"
+    format = "html"
+    get :formatted_fragment_cached, format: format
     assert_response :success
     expected_body = "<body>\n<p>ERB</p>\n</body>\n"
 
     assert_equal expected_body, @response.body
 
     assert_equal "<p>ERB</p>",
-      @store.read("views/functional_caching/formatted_fragment_cached:#{template_digest("functional_caching/formatted_fragment_cached")}/fragment")
+      @store.read("views/functional_caching/formatted_fragment_cached:#{template_digest("functional_caching/formatted_fragment_cached", format)}/fragment")
   end
 
   def test_xml_formatted_fragment_caching
-    get :formatted_fragment_cached, format: "xml"
+    format = "xml"
+    get :formatted_fragment_cached, format: format
     assert_response :success
     expected_body = "<body>\n  <p>Builder</p>\n</body>\n"
 
     assert_equal expected_body, @response.body
 
     assert_equal "  <p>Builder</p>\n",
-      @store.read("views/functional_caching/formatted_fragment_cached:#{template_digest("functional_caching/formatted_fragment_cached")}/fragment")
+      @store.read("views/functional_caching/formatted_fragment_cached:#{template_digest("functional_caching/formatted_fragment_cached", format)}/fragment")
   end
 
   def test_fragment_caching_with_variant
-    get :formatted_fragment_cached_with_variant, format: "html", params: { v: :phone }
+    format = "html"
+    get :formatted_fragment_cached_with_variant, format: format, params: { v: :phone }
     assert_response :success
     expected_body = "<body>\n<p>PHONE</p>\n</body>\n"
 
     assert_equal expected_body, @response.body
 
     assert_equal "<p>PHONE</p>",
-      @store.read("views/functional_caching/formatted_fragment_cached_with_variant:#{template_digest("functional_caching/formatted_fragment_cached_with_variant")}/fragment")
+      @store.read("views/functional_caching/formatted_fragment_cached_with_variant:#{template_digest("functional_caching/formatted_fragment_cached_with_variant", format)}/fragment")
   end
 
   def test_fragment_caching_with_html_partials_in_xml
@@ -309,8 +312,8 @@ CACHED
   end
 
   private
-    def template_digest(name)
-      ActionView::Digestor.digest(name: name, finder: @controller.lookup_context)
+    def template_digest(name, format)
+      ActionView::Digestor.digest(name: name, format: format, finder: @controller.lookup_context)
     end
 end
 
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