diff --git a/actionview/CHANGELOG.md b/actionview/CHANGELOG.md index 4197c4c021..50b9e5be44 100644 --- a/actionview/CHANGELOG.md +++ b/actionview/CHANGELOG.md @@ -1,3 +1,12 @@ +* Ensure cache fragment digests include all relevant template dependencies when + fragments are contained in a block passed to the render helper. Remove the + virtual_path keyword arguments found in CacheHelper as they no longer possess + any function following 1581cab. + + Fixes #38984 + + *Aaron Lipman* + * Deprecate `config.action_view.raise_on_missing_translations` in favor of `config.i18n.raise_on_missing_translations`. diff --git a/actionview/lib/action_view/base.rb b/actionview/lib/action_view/base.rb index e9002e9b8b..87141413d6 100644 --- a/actionview/lib/action_view/base.rb +++ b/actionview/lib/action_view/base.rb @@ -269,9 +269,9 @@ module ActionView #:nodoc: _prepare_context end - def _run(method, template, locals, buffer, &block) + def _run(method, template, locals, buffer, add_to_stack: true, &block) _old_output_buffer, _old_virtual_path, _old_template = @output_buffer, @virtual_path, @current_template - @current_template = template + @current_template = template if add_to_stack @output_buffer = buffer send(method, locals, buffer, &block) ensure diff --git a/actionview/lib/action_view/helpers/cache_helper.rb b/actionview/lib/action_view/helpers/cache_helper.rb index 3f396da288..3f87bcf9e1 100644 --- a/actionview/lib/action_view/helpers/cache_helper.rb +++ b/actionview/lib/action_view/helpers/cache_helper.rb @@ -165,7 +165,7 @@ module ActionView # expire the cache. def cache(name = {}, options = {}, &block) if controller.respond_to?(:perform_caching) && controller.perform_caching - name_options = options.slice(:skip_digest, :virtual_path) + name_options = options.slice(:skip_digest) safe_concat(fragment_for(cache_fragment_name(name, **name_options), options, &block)) else yield @@ -205,14 +205,11 @@ module ActionView # fragments can be manually bypassed. This is useful when cache fragments # cannot be manually expired unless you know the exact key which is the # case when using memcached. - # - # The digest will be generated using +virtual_path:+ if it is provided. - # - def cache_fragment_name(name = {}, skip_digest: nil, virtual_path: nil, digest_path: nil) + def cache_fragment_name(name = {}, skip_digest: nil, digest_path: nil) if skip_digest name else - fragment_name_with_digest(name, virtual_path, digest_path) + fragment_name_with_digest(name, digest_path) end end @@ -227,14 +224,11 @@ module ActionView end private - def fragment_name_with_digest(name, virtual_path, digest_path) - virtual_path ||= @virtual_path - - if virtual_path || digest_path - name = controller.url_for(name).split("://").last if name.is_a?(Hash) + def fragment_name_with_digest(name, digest_path) + name = controller.url_for(name).split("://").last if name.is_a?(Hash) + if @current_template&.virtual_path || digest_path digest_path ||= digest_path_from_template(@current_template) - [ digest_path, name ] else name @@ -243,10 +237,10 @@ module ActionView def fragment_for(name = {}, options = nil, &block) if content = read_fragment_for(name, options) - @view_renderer.cache_hits[@virtual_path] = :hit if defined?(@view_renderer) + @view_renderer.cache_hits[@current_template&.virtual_path] = :hit if defined?(@view_renderer) content else - @view_renderer.cache_hits[@virtual_path] = :miss if defined?(@view_renderer) + @view_renderer.cache_hits[@current_template&.virtual_path] = :miss if defined?(@view_renderer) write_fragment_for(name, options, &block) end end diff --git a/actionview/lib/action_view/renderer/partial_renderer.rb b/actionview/lib/action_view/renderer/partial_renderer.rb index 1503ba734d..39d24bd4f8 100644 --- a/actionview/lib/action_view/renderer/partial_renderer.rb +++ b/actionview/lib/action_view/renderer/partial_renderer.rb @@ -282,7 +282,7 @@ module ActionView identifier: template.identifier, layout: layout && layout.virtual_path ) do |payload| - content = template.render(view, locals) do |*name| + content = template.render(view, locals, add_to_stack: !block) do |*name| view._layout_for(*name, &block) end 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 788d171a90..ea02ffcae9 100644 --- a/actionview/lib/action_view/renderer/partial_renderer/collection_caching.rb +++ b/actionview/lib/action_view/renderer/partial_renderer/collection_caching.rb @@ -68,7 +68,7 @@ module ActionView end def expanded_cache_key(key, view, template, digest_path) - key = view.combined_fragment_cache_key(view.cache_fragment_name(key, virtual_path: template.virtual_path, digest_path: digest_path)) + key = view.combined_fragment_cache_key(view.cache_fragment_name(key, digest_path: digest_path)) key.frozen? ? key.dup : key # #read_multi & #write may require mutability, Dalli 2.6.0. end diff --git a/actionview/lib/action_view/template.rb b/actionview/lib/action_view/template.rb index cf0ef74959..d2d24299fe 100644 --- a/actionview/lib/action_view/template.rb +++ b/actionview/lib/action_view/template.rb @@ -176,10 +176,10 @@ module ActionView # This method is instrumented as "!render_template.action_view". Notice that # we use a bang in this instrumentation because you don't want to # consume this in production. This is only slow if it's being listened to. - def render(view, locals, buffer = ActionView::OutputBuffer.new, &block) + def render(view, locals, buffer = ActionView::OutputBuffer.new, add_to_stack: true, &block) instrument_render_template do compile!(view) - view._run(method_name, self, locals, buffer, &block) + view._run(method_name, self, locals, buffer, add_to_stack: add_to_stack, &block) end rescue => e handle_render_error(view, e) diff --git a/actionview/test/fixtures/test/cache_fragment_inside_render_layout_block_1.html.erb b/actionview/test/fixtures/test/cache_fragment_inside_render_layout_block_1.html.erb new file mode 100644 index 0000000000..60ec279ff4 --- /dev/null +++ b/actionview/test/fixtures/test/cache_fragment_inside_render_layout_block_1.html.erb @@ -0,0 +1 @@ +<%= render layout: "layouts/yield_only" do %><%= cache "foo" do %>CAT<% end %><% end %> \ No newline at end of file diff --git a/actionview/test/fixtures/test/cache_fragment_inside_render_layout_block_2.html.erb b/actionview/test/fixtures/test/cache_fragment_inside_render_layout_block_2.html.erb new file mode 100644 index 0000000000..49df88f56c --- /dev/null +++ b/actionview/test/fixtures/test/cache_fragment_inside_render_layout_block_2.html.erb @@ -0,0 +1 @@ +<%= render layout: "layouts/yield_only" do %><%= cache "foo" do %>DOG<% end %><% end %> \ No newline at end of file diff --git a/actionview/test/template/render_test.rb b/actionview/test/template/render_test.rb index e315d6e4e6..56f2674eb0 100644 --- a/actionview/test/template/render_test.rb +++ b/actionview/test/template/render_test.rb @@ -22,6 +22,7 @@ module RenderTestCases controller = TestController.new controller.perform_caching = true + controller.cache_store = :memory_store @view.controller = controller @controller_view = controller.view_context_class.with_empty_template_cache.new( @@ -705,6 +706,13 @@ class CachedViewRenderTest < ActiveSupport::TestCase assert_equal ActionView::OptimizedFileSystemResolver, view_paths.first.class setup_view(view_paths) end + + def test_cache_fragments_inside_render_layout_call_with_block + cat = @view.render(template: "test/cache_fragment_inside_render_layout_block_1") + dog = @view.render(template: "test/cache_fragment_inside_render_layout_block_2") + + assert_not_equal cat, dog + end end class LazyViewRenderTest < ActiveSupport::TestCase