Perform a deep #dup on query cache results so that modifying activerecord attributes does not modify the cached attributes. [Rick]

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@7238 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

3 files changed, 29 insertions, 20 deletions

diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG
index 4adcfcc4df..19733ae2a6 100644
--- a/activerecord/CHANGELOG
+++ b/activerecord/CHANGELOG
@@ -1,5 +1,7 @@
 *SVN*
 
+* Perform a deep #dup on query cache results so that modifying activerecord attributes does not modify the cached attributes.  [Rick]
+
 # Ensure that has_many :through associations use a count query instead of loading the target when #size is called.  Closes #8800 [lifo]
 
 * Added :unless clause to validations #8003 [monki]. Example: 
diff --git a/activerecord/lib/active_record/query_cache.rb b/activerecord/lib/active_record/query_cache.rb
index 9296c5b81a..191e5a3d97 100644
--- a/activerecord/lib/active_record/query_cache.rb
+++ b/activerecord/lib/active_record/query_cache.rb
@@ -58,8 +58,14 @@ module ActiveRecord
         else
           @query_cache[sql] = yield
         end
-        
-        result ? result.dup : nil
+
+        if result
+          # perform a deep #dup in case result is an array
+          result = result.collect { |row| row.dup } if result.respond_to?(:collect)
+          result.dup
+        else
+          nil
+        end
       end
     
       def method_missing(method, *arguments, &proc)
diff --git a/activerecord/test/query_cache_test.rb b/activerecord/test/query_cache_test.rb
index 4cadc32316..b09ceea746 100644
--- a/activerecord/test/query_cache_test.rb
+++ b/activerecord/test/query_cache_test.rb
@@ -8,39 +8,43 @@ class QueryCacheTest < Test::Unit::TestCase
   fixtures :tasks
   
   def test_find_queries
-    assert_queries(2) {  Task.find(1); Task.find(1) }        
+    assert_queries(2) {  Task.find(1); Task.find(1) }
   end
 
   def test_find_queries_with_cache
     Task.cache do
-      assert_queries(1) {  Task.find(1); Task.find(1) }    
+      assert_queries(1) {  Task.find(1); Task.find(1) }
     end
   end
   
-  def test_find_queries_with_cache
-    Task.cache do
-      assert_queries(1) {  Task.find(1); Task.find(1) }    
-    end
-  end
-  
-  def test_query_cache_returned        
+  def test_query_cache_returned
     assert_not_equal ActiveRecord::QueryCache, Task.connection.class
-    Task.cache do 
-      assert_equal ActiveRecord::QueryCache, Task.connection.class      
+    Task.cache do
+      assert_equal ActiveRecord::QueryCache, Task.connection.class
     end    
   end
-  
+
+  def test_query_cache_dups_results_correctly
+    Task.cache do
+      now  = Time.now.utc
+      task = Task.find 1
+      assert_not_equal now, task.starting
+      task.starting = now
+      task.reload
+      assert_not_equal now, task.starting
+    end
+  end
 
   def test_cache_is_scoped_on_actual_class_only
     Task.cache do
-      assert_queries(2) {  Topic.find(1); Topic.find(1) }    
+      Topic.columns # don't count this query
+      assert_queries(2) {  Topic.find(1); Topic.find(1); }
     end
   end
   
-  
   def test_cache_is_scoped_on_all_descending_classes
     ActiveRecord::Base.cache do
-      assert_queries(1) {  Task.find(1); Task.find(1) }    
+      assert_queries(1) {  Task.find(1); Task.find(1) }
     end
   end
   
@@ -53,11 +57,8 @@ class QueryCacheTest < Test::Unit::TestCase
           "Connections should be different, Course connects to a different database"
     end
   end
-    
-  
 end
 
-
 uses_mocha('QueryCacheExpiryTest') do
 
 class QueryCacheExpiryTest < Test::Unit::TestCase