Permalink
Browse files

Perform a deep #dup on query cache results so that modifying activere…

…cord attributes does not modify the cached attributes. [Rick]

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@7238 5ecf4fe2-1ee6-0310-87b1-e25e094e27de
  • Loading branch information...
1 parent d51e7f8 commit f5ea6f880f365c5383d029bddc917baa82e56d55 @technoweenie technoweenie committed Jul 25, 2007
Showing with 29 additions and 20 deletions.
  1. +2 −0 activerecord/CHANGELOG
  2. +8 −2 activerecord/lib/active_record/query_cache.rb
  3. +19 −18 activerecord/test/query_cache_test.rb
View
@@ -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:
@@ -58,8 +58,14 @@ def cache(sql)
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)
@@ -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 @@ def test_cache_does_not_blow_up_other_connections
"Connections should be different, Course connects to a different database"
end
end
-
-
end
-
uses_mocha('QueryCacheExpiryTest') do
class QueryCacheExpiryTest < Test::Unit::TestCase

0 comments on commit f5ea6f8

Please sign in to comment.