From 77e79ecd92acd43a282566e5d297b8e74dc14f05 Mon Sep 17 00:00:00 2001 From: Daniel Schierbeck Date: Mon, 15 Jul 2013 15:59:18 +0200 Subject: [PATCH] Bust the template digest cache key when details are changed Since the lookup details will influence which template is resolved, they need to be included in the cache key -- otherwise two different templates may erroneously share the same digest value. --- actionview/CHANGELOG.md | 6 +++++ actionview/lib/action_view/digestor.rb | 5 +++- actionview/test/template/digestor_test.rb | 30 ++++++++++++++++++++++- 3 files changed, 39 insertions(+), 2 deletions(-) diff --git a/actionview/CHANGELOG.md b/actionview/CHANGELOG.md index 53142a835e..03fd6e7413 100644 --- a/actionview/CHANGELOG.md +++ b/actionview/CHANGELOG.md @@ -13,6 +13,12 @@ *Josh Lauer*, *Justin Ridgewell* +* Fixed a bug where the lookup details were not being taken into account + when caching the digest of a template - changes to the details now + cause a different cache key to be used. + + *Daniel Schierbeck* + * Added an `extname` hash option for `javascript_include_tag` method. Before: diff --git a/actionview/lib/action_view/digestor.rb b/actionview/lib/action_view/digestor.rb index 95c3200c81..af158a630b 100644 --- a/actionview/lib/action_view/digestor.rb +++ b/actionview/lib/action_view/digestor.rb @@ -10,7 +10,10 @@ module ActionView class << self def digest(name, format, finder, options = {}) - cache_key = ([name, format] + Array.wrap(options[:dependencies])).join('.') + details_key = finder.details_key.hash + dependencies = Array.wrap(options[:dependencies]) + cache_key = ([name, details_key, format] + dependencies).join('.') + # this is a correctly done double-checked locking idiom # (ThreadSafe::Cache's lookups have volatile semantics) @@cache[cache_key] || @@digest_monitor.synchronize do diff --git a/actionview/test/template/digestor_test.rb b/actionview/test/template/digestor_test.rb index c6608e214a..0f6b14a57d 100644 --- a/actionview/test/template/digestor_test.rb +++ b/actionview/test/template/digestor_test.rb @@ -15,6 +15,16 @@ end class FixtureFinder FIXTURES_DIR = "#{File.dirname(__FILE__)}/../fixtures/digestor" + attr_reader :details + + def initialize + @details = {} + end + + def details_key + details.hash + end + def find(logical_name, keys, partial, options) FixtureTemplate.new("digestor/#{partial ? logical_name.gsub(%r|/([^/]+)$|, '/_\1') : logical_name}.#{options[:formats].first}.erb") end @@ -140,6 +150,20 @@ class TemplateDigestorTest < ActionView::TestCase end end + def test_details_are_included_in_cache_key + # Cache the template digest. + 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.details[:foo] = "bar" + + # The cache is busted. + assert_not_equal old_digest, digest("events/_event") + end + def test_extra_whitespace_in_render_partial assert_digest_difference("messages/edit") do change_template("messages/_form") @@ -220,7 +244,11 @@ class TemplateDigestorTest < ActionView::TestCase end def digest(template_name, options={}) - ActionView::Digestor.digest(template_name, :html, FixtureFinder.new, options) + ActionView::Digestor.digest(template_name, :html, finder, options) + end + + def finder + @finder ||= FixtureFinder.new end def change_template(template_name)