From abe915f23777efe10f17d611bf5718ca855a0704 Mon Sep 17 00:00:00 2001 From: Olek Janiszewski Date: Fri, 16 Dec 2011 17:58:22 +0100 Subject: [PATCH] Fix expanding cache key for single element arrays In short: expand_cache_key(element) should not equal expand_cache_key([element]) This way a fragment cache key for an index page with only a single element in the collection is different than a fragment cache for a typical show page for that element. --- activesupport/lib/active_support/cache.rb | 2 +- activesupport/test/caching_test.rb | 22 +++++++++++++++------- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/activesupport/lib/active_support/cache.rb b/activesupport/lib/active_support/cache.rb index 07f5fcdeb3..6452bed506 100644 --- a/activesupport/lib/active_support/cache.rb +++ b/activesupport/lib/active_support/cache.rb @@ -90,7 +90,7 @@ def expand_cache_key(key, namespace = nil) def retrieve_cache_key(key) case when key.respond_to?(:cache_key) then key.cache_key - when key.is_a?(Array) then key.map { |element| retrieve_cache_key(element) }.to_param + when key.is_a?(Array) then ['Array', *key.map { |element| retrieve_cache_key(element) }].to_param else key.to_param end.to_s end diff --git a/activesupport/test/caching_test.rb b/activesupport/test/caching_test.rb index 5d7464c623..e8b822fd83 100644 --- a/activesupport/test/caching_test.rb +++ b/activesupport/test/caching_test.rb @@ -4,19 +4,19 @@ class CacheKeyTest < ActiveSupport::TestCase def test_expand_cache_key - assert_equal '1/2/true', ActiveSupport::Cache.expand_cache_key([1, '2', true]) - assert_equal 'name/1/2/true', ActiveSupport::Cache.expand_cache_key([1, '2', true], :name) + assert_equal 'Array/1/2/true', ActiveSupport::Cache.expand_cache_key([1, '2', true]) + assert_equal 'name/Array/1/2/true', ActiveSupport::Cache.expand_cache_key([1, '2', true], :name) end def test_expand_cache_key_with_rails_cache_id begin ENV['RAILS_CACHE_ID'] = 'c99' assert_equal 'c99/foo', ActiveSupport::Cache.expand_cache_key(:foo) - assert_equal 'c99/foo', ActiveSupport::Cache.expand_cache_key([:foo]) - assert_equal 'c99/foo/bar', ActiveSupport::Cache.expand_cache_key([:foo, :bar]) + assert_equal 'c99/Array/foo', ActiveSupport::Cache.expand_cache_key([:foo]) + assert_equal 'c99/Array/foo/bar', ActiveSupport::Cache.expand_cache_key([:foo, :bar]) assert_equal 'nm/c99/foo', ActiveSupport::Cache.expand_cache_key(:foo, :nm) - assert_equal 'nm/c99/foo', ActiveSupport::Cache.expand_cache_key([:foo], :nm) - assert_equal 'nm/c99/foo/bar', ActiveSupport::Cache.expand_cache_key([:foo, :bar], :nm) + assert_equal 'nm/c99/Array/foo', ActiveSupport::Cache.expand_cache_key([:foo], :nm) + assert_equal 'nm/c99/Array/foo/bar', ActiveSupport::Cache.expand_cache_key([:foo, :bar], :nm) ensure ENV['RAILS_CACHE_ID'] = nil end @@ -55,7 +55,7 @@ def test_expand_cache_key_array_with_something_that_responds_to_cache_key def key.cache_key :foo_key end - assert_equal 'foo_key', ActiveSupport::Cache.expand_cache_key([key]) + assert_equal 'Array/foo_key', ActiveSupport::Cache.expand_cache_key([key]) end def test_expand_cache_key_of_nil @@ -69,6 +69,14 @@ def test_expand_cache_key_of_false def test_expand_cache_key_of_true assert_equal 'true', ActiveSupport::Cache.expand_cache_key(true) end + + def test_expand_cache_key_of_one_element_array_different_than_key_of_element + element = 'foo' + array = [element] + element_cache_key = ActiveSupport::Cache.expand_cache_key(element) + array_cache_key = ActiveSupport::Cache.expand_cache_key(array) + assert_not_equal element_cache_key, array_cache_key + end end class CacheStoreSettingTest < ActiveSupport::TestCase -- GitLab