From 89969dd7116cc433382f3f796f33fad3f6d461be Mon Sep 17 00:00:00 2001 From: Brad Murray Date: Tue, 15 Oct 2013 11:06:50 +1300 Subject: Add 2 tests, 1 of which fails, to isolate the digest_caching behaviour causing #12521 If config.action_view.cache_template_loading = false, most likely in a development configuration if config.cache_classes = false & config.action_controller.perform_caching = true. config.action_view.cache_template_loading defaults to the value of config.cache_classes --- actionview/test/template/digestor_test.rb | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) (limited to 'actionview') diff --git a/actionview/test/template/digestor_test.rb b/actionview/test/template/digestor_test.rb index 0f6b14a57d..00bdfad3b7 100644 --- a/actionview/test/template/digestor_test.rb +++ b/actionview/test/template/digestor_test.rb @@ -217,6 +217,32 @@ class TemplateDigestorTest < ActionView::TestCase ActionView::Resolver.caching = resolver_before end + def test_digest_cache_cleanup_with_recursion + first_digest = digest("level/_recursion") + second_digest = digest("level/_recursion") + + assert first_digest + + # If the cache is cleaned up correctly, subsequent digests should return the same + assert_equal first_digest, second_digest + end + + def test_digest_cache_cleanup_with_recursion_and_template_caching_off + resolver_before = ActionView::Resolver.caching + ActionView::Resolver.caching = false + + first_digest = digest("level/_recursion") + second_digest = digest("level/_recursion") + + assert first_digest + + # If the cache is cleaned up correctly, subsequent digests should return the same + assert_equal first_digest, second_digest + + ActionView::Resolver.caching = resolver_before + end + + private def assert_logged(message) old_logger = ActionView::Base.logger -- cgit v1.2.3 From 2b3a349123b3f09627bd3f4091e5b885ded0fbac Mon Sep 17 00:00:00 2001 From: Brad Murray Date: Tue, 15 Oct 2013 11:17:11 +1300 Subject: Ensure ActionView::Digest.cache is correctly cleaned up when ActionView::Resolver.caching = false. --- actionview/lib/action_view/digestor.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'actionview') diff --git a/actionview/lib/action_view/digestor.rb b/actionview/lib/action_view/digestor.rb index af158a630b..dc353b04ea 100644 --- a/actionview/lib/action_view/digestor.rb +++ b/actionview/lib/action_view/digestor.rb @@ -36,12 +36,12 @@ module ActionView end # Store the actual digest if config.cache_template_loading is true - klass.new(name, format, finder, options).digest.tap do |digest| - @@cache[cache_key] = digest if ActionView::Resolver.caching? - end - rescue Exception - @@cache.delete_pair(cache_key, false) if pre_stored # something went wrong, make sure not to corrupt the @@cache - raise + digest = klass.new(name, format, finder, options).digest + @@cache[cache_key] = digest if ActionView::Resolver.caching? + digest + ensure + # something went wrong or ActionView::Resolver.caching? is false, make sure not to corrupt the @@cache + @@cache.delete_pair(cache_key, false) if pre_stored || !digest end end -- cgit v1.2.3 From 6c04cb2261ee105e3566b8f84ccb355a649f6ce9 Mon Sep 17 00:00:00 2001 From: Brad Murray Date: Tue, 15 Oct 2013 11:23:37 +1300 Subject: update changelog --- actionview/CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'actionview') diff --git a/actionview/CHANGELOG.md b/actionview/CHANGELOG.md index a09650c559..59b803d088 100644 --- a/actionview/CHANGELOG.md +++ b/actionview/CHANGELOG.md @@ -1,3 +1,8 @@ +* Ensure ActionView::Digestor.cache is correctly cleaned up when + combining recursive templates with ActionView::Resolver.caching = false + + *wyaeld* + * Fix `collection_check_boxes` generated hidden input to use the name attribute provided in the options hash. -- cgit v1.2.3 From af1dc7f08a4e63cde7bd33d1c2afc782de65383d Mon Sep 17 00:00:00 2001 From: Brad Murray Date: Tue, 15 Oct 2013 21:08:47 +1300 Subject: update digestor code based on review --- actionview/lib/action_view/digestor.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'actionview') diff --git a/actionview/lib/action_view/digestor.rb b/actionview/lib/action_view/digestor.rb index dc353b04ea..6757d0cdd1 100644 --- a/actionview/lib/action_view/digestor.rb +++ b/actionview/lib/action_view/digestor.rb @@ -35,13 +35,13 @@ module ActionView Digestor end - # Store the actual digest if config.cache_template_loading is true digest = klass.new(name, format, finder, options).digest + # Store the actual digest if config.cache_template_loading is true @@cache[cache_key] = digest if ActionView::Resolver.caching? digest ensure # something went wrong or ActionView::Resolver.caching? is false, make sure not to corrupt the @@cache - @@cache.delete_pair(cache_key, false) if pre_stored || !digest + @@cache.delete_pair(cache_key, false) if pre_stored && !digest end end -- cgit v1.2.3 From 0cdce7f910708005acd99c80463e9efb3df942b0 Mon Sep 17 00:00:00 2001 From: Brad Murray Date: Thu, 17 Oct 2013 09:00:37 +1300 Subject: add a new local variable to track if digests are being stored, to ensure the cleanup works correctly --- actionview/lib/action_view/digestor.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'actionview') diff --git a/actionview/lib/action_view/digestor.rb b/actionview/lib/action_view/digestor.rb index 6757d0cdd1..5570e2a8dc 100644 --- a/actionview/lib/action_view/digestor.rb +++ b/actionview/lib/action_view/digestor.rb @@ -37,11 +37,11 @@ module ActionView digest = klass.new(name, format, finder, options).digest # Store the actual digest if config.cache_template_loading is true - @@cache[cache_key] = digest if ActionView::Resolver.caching? + @@cache[cache_key] = stored_digest = digest if ActionView::Resolver.caching? digest ensure # something went wrong or ActionView::Resolver.caching? is false, make sure not to corrupt the @@cache - @@cache.delete_pair(cache_key, false) if pre_stored && !digest + @@cache.delete_pair(cache_key, false) if pre_stored && !stored_digest end end -- cgit v1.2.3