aboutsummaryrefslogtreecommitdiffstats
path: root/actionview
diff options
context:
space:
mode:
authorAaron Patterson <aaron.patterson@gmail.com>2019-02-15 17:10:28 -0800
committerAaron Patterson <aaron.patterson@gmail.com>2019-02-15 17:27:33 -0800
commit1581cab9ff26731ed03a17f7ddec3c85d536988a (patch)
tree3691cb09659fcd6b3e140926b07ddd589dad4a41 /actionview
parent3aa3c0684d3bd748be9e85d25616a8a7a1ab7755 (diff)
downloadrails-1581cab9ff26731ed03a17f7ddec3c85d536988a.tar.gz
rails-1581cab9ff26731ed03a17f7ddec3c85d536988a.tar.bz2
rails-1581cab9ff26731ed03a17f7ddec3c85d536988a.zip
Pass the template format to the digestor
This commit passes the template format to the digestor in order to come up with a key. Before this commit, the digestor would depend on the side effect of the template renderer setting the rendered_format on the lookup context. I would like to remove that mutation, so I've changed this to pass the template format in to the digestor. I've introduced a new instance variable that will be alive during a template render. When the template is being rendered, it pushes the current template on to a stack, setting `@current_template` to the template currently being rendered. When the cache helper asks the digestor for a key, it uses the format of the template currently on the stack.
Diffstat (limited to 'actionview')
-rw-r--r--actionview/lib/action_view/base.rb8
-rw-r--r--actionview/lib/action_view/digestor.rb10
-rw-r--r--actionview/lib/action_view/helpers/cache_helper.rb10
-rw-r--r--actionview/lib/action_view/renderer/partial_renderer/collection_caching.rb2
-rw-r--r--actionview/lib/action_view/template.rb2
-rw-r--r--actionview/lib/action_view/template/handlers/erb/erubi.rb2
-rw-r--r--actionview/test/activerecord/relation_cache_test.rb3
-rw-r--r--actionview/test/template/digestor_test.rb25
-rw-r--r--actionview/test/template/render_test.rb2
9 files changed, 33 insertions, 31 deletions
diff --git a/actionview/lib/action_view/base.rb b/actionview/lib/action_view/base.rb
index 712e5d251e..56e0e3275c 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..ec75d861bb 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
@@ -73,9 +73,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 cda8c942d8..5c00596b41 100644
--- a/actionview/test/template/render_test.rb
+++ b/actionview/test/template/render_test.rb
@@ -783,7 +783,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