Change ActiveSupport::Cache behavior to always return duplicate objects instead of frozen objects.

2 files changed, 18 insertions, 22 deletions

diff --git a/activesupport/lib/active_support/cache.rb b/activesupport/lib/active_support/cache.rb
index ac88c82709..2d2264e58a 100644
--- a/activesupport/lib/active_support/cache.rb
+++ b/activesupport/lib/active_support/cache.rb
@@ -557,15 +557,14 @@ module ActiveSupport
         @expires_in = options[:expires_in]
         @expires_in = @expires_in.to_f if @expires_in
         @created_at = Time.now.to_f
-        if defined?(value)
+        if value.nil?
+          @value = nil
+        else
+          @value = Marshal.dump(value)
           if should_compress?(value, options)
-            @value = Zlib::Deflate.deflate(Marshal.dump(value))
+            @value = Zlib::Deflate.deflate(@value)
             @compressed = true
-          else
-            @value = value
           end
-        else
-          @value = nil
         end
       end
 
@@ -576,12 +575,8 @@ module ActiveSupport
 
       # Get the value stored in the cache.
       def value
-        if defined?(@value)
-          val = compressed? ? Marshal.load(Zlib::Inflate.inflate(@value)) : @value
-          unless val.frozen?
-            val.freeze rescue nil
-          end
-          val
+        if @value
+          Marshal.load(compressed? ? Zlib::Inflate.inflate(@value) : @value)
         end
       end
 
@@ -614,10 +609,8 @@ module ActiveSupport
       def size
         if @value.nil?
           0
-        elsif @value.respond_to?(:bytesize)
-          @value.bytesize
         else
-          Marshal.dump(@value).bytesize
+          @value.bytesize
         end
       end
 
diff --git a/activesupport/test/caching_test.rb b/activesupport/test/caching_test.rb
index 402c6695aa..dff3d6ef0d 100644
--- a/activesupport/test/caching_test.rb
+++ b/activesupport/test/caching_test.rb
@@ -204,7 +204,7 @@ module CacheStoreBehavior
     @cache.write('foo', 'bar', :compress => true)
     raw_value = @cache.send(:read_entry, 'foo', {}).raw_value
     assert_equal 'bar', @cache.read('foo')
-    assert_equal 'bar', raw_value
+    assert_equal 'bar', Marshal.load(raw_value)
   end
 
   def test_read_and_write_compressed_large_data
@@ -270,10 +270,12 @@ module CacheStoreBehavior
     assert !@cache.exist?('foo')
   end
 
-  def test_store_objects_should_be_immutable
+  def test_read_should_return_a_different_object_id_each_time_it_is_called
     @cache.write('foo', 'bar')
-    assert_raise(ActiveSupport::FrozenObjectError) { @cache.read('foo').gsub!(/.*/, 'baz') }
-    assert_equal 'bar', @cache.read('foo')
+    assert_not_equal @cache.read('foo').object_id, @cache.read('foo').object_id
+    value = @cache.read('foo')
+    value << 'bingo'
+    assert_not_equal value, @cache.read('foo')
   end
 
   def test_original_store_objects_should_not_be_immutable
@@ -551,7 +553,8 @@ end
 
 class MemoryStoreTest < ActiveSupport::TestCase
   def setup
-    @cache = ActiveSupport::Cache.lookup_store(:memory_store, :expires_in => 60, :size => 100)
+    @record_size = Marshal.dump("aaaaaaaaaa").bytesize
+    @cache = ActiveSupport::Cache.lookup_store(:memory_store, :expires_in => 60, :size => @record_size * 10)
   end
 
   include CacheStoreBehavior
@@ -566,7 +569,7 @@ class MemoryStoreTest < ActiveSupport::TestCase
     @cache.write(5, "eeeeeeeeee") && sleep(0.001)
     @cache.read(2) && sleep(0.001)
     @cache.read(4)
-    @cache.prune(30)
+    @cache.prune(@record_size * 3)
     assert_equal true, @cache.exist?(5)
     assert_equal true, @cache.exist?(4)
     assert_equal false, @cache.exist?(3)
@@ -719,7 +722,7 @@ class CacheEntryTest < ActiveSupport::TestCase
   def test_non_compress_values
     entry = ActiveSupport::Cache::Entry.new("value")
     assert_equal "value", entry.value
-    assert_equal "value", entry.raw_value
+    assert_equal "value", Marshal.load(entry.raw_value)
     assert_equal false, entry.compressed?
   end
 end