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