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

Pass the template format to the digestor
This commit is contained in:
Aaron Patterson 2019-02-19 13:45:30 -08:00 committed by GitHub
commit ff6b713f5e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 53 additions and 50 deletions

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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)

View File

@ -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

View File

@ -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

View File

@ -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)

View File

@ -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