Skip to content

Commit

Permalink
Fixed concurrency issues, using Thread#current to store cached_associ…
Browse files Browse the repository at this point in the history
…ations instead of ivar
  • Loading branch information
Luca Guidi committed Oct 20, 2008
1 parent 864007d commit 57e5c00
Show file tree
Hide file tree
Showing 5 changed files with 71 additions and 56 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG
@@ -1,3 +1,5 @@
* Fixed concurrency issues, using Thread#current to store cached_associations instead of ivar

* Make sure tests suite runs in 'test' environment. Introduced SKIP_MOCHA env variable, in order to run tests directly on cache

$ rake cached_models SKIP_MOCHA=true
Expand Down
5 changes: 3 additions & 2 deletions lib/activerecord/lib/active_record/base.rb
Expand Up @@ -63,7 +63,8 @@ def reflection_cache_key(reflection)
end

def cached_associations
@cached_associations ||= {}
cached_associations = (Thread.current[:cached_associations] ||= {})
cached_associations[cache_key] ||= {}
end
end
end
end
2 changes: 1 addition & 1 deletion tasks/cached_models_tasks.rake
@@ -1,4 +1,4 @@
RAILS_ENV = "test"
# RAILS_ENV = "test"

require 'rubygems'
require 'active_record'
Expand Down
108 changes: 55 additions & 53 deletions test/active_record/associations/has_many_association_test.rb
Expand Up @@ -199,31 +199,6 @@ def test_should_refresh_cache_when_pushing_element_to_association
assert_equal posts_by_author(:luca), authors(:luca).cached_posts
end

def test_should_refresh_caches_when_pushing_element_to_association_belonging_to_another_model
cache.expects(:fetch).with("#{authors(:chuck).cache_key}/cached_posts").times(2).returns association_proxy(:chuck)
cache.expects(:fetch).with("#{cache_key}/cached_posts").times(2).returns association_proxy

post = authors(:chuck).cached_posts.last
authors(:luca).cached_posts << post

assert_equal posts_by_author(:luca), authors(:luca).cached_posts
assert_equal posts_by_author(:chuck), authors(:chuck).cached_posts
end

def test_should_refresh_caches_when_pushing_element_to_polymorphic_association_belonging_to_another_model
cache.expects(:fetch).with("#{posts(:welcome).cache_key}/cached_tags").times(2).returns tags_association_proxy
cache.expects(:fetch).with("#{posts(:cached_models).cache_key}/cached_tags").times(2).returns tags_association_proxy(:cached_models)
tag = posts(:welcome).cached_tags.last

posts(:cached_models).cached_tags << tag

# NOTE for some weird reason the assertion fails, even if the collections are equals.
# I forced the comparision between the ids.
assert_equal tags_by_post(:cached_models).map(&:id).sort,
posts(:cached_models).cached_tags.map(&:id).sort
assert_equal tags_by_post(:welcome), posts(:welcome).cached_tags
end

def test_should_not_use_cache_when_pushing_element_to_association_on_false_cached_option
cache.expects(:write).never

Expand All @@ -246,34 +221,6 @@ def test_should_not_use_cache_when_pushing_element_to_polymorphic_association_be

assert_equal tags_by_post(:cached_models), posts(:cached_models).tags
end

def test_should_update_cache_when_pushing_element_with_build
cache.expects(:fetch).with("#{cache_key}/cached_posts").times(2).returns association_proxy

author = authors(:luca)
post = author.cached_posts.build post_options
post.save

assert_equal posts_by_author(:luca), author.cached_posts
end

def test_should_update_cache_when_pushing_element_with_create
cache.expects(:fetch).with("#{cache_key}/cached_posts").times(2).returns association_proxy

author = authors(:luca)
author.cached_posts.create post_options(:title => "CM Overview")

assert_equal posts_by_author(:luca), author.cached_posts
end

def test_should_update_cache_when_pushing_element_with_create_bang_method
cache.expects(:fetch).with("#{cache_key}/cached_posts").times(2).returns association_proxy

author = authors(:luca)
author.cached_posts.create! post_options(:title => "CM Overview!!")

assert_equal posts_by_author(:luca), author.cached_posts
end

def test_should_update_cache_when_deleting_element_from_collection
cache.expects(:fetch).with("#{cache_key}/cached_posts").times(2).returns association_proxy
Expand Down Expand Up @@ -349,6 +296,61 @@ def test_should_use_cache_for_collection_include
end
end

uses_memcached 'HasManyAssociationTest' do
def test_should_refresh_caches_when_pushing_element_to_association_belonging_to_another_model
# cache.expects(:fetch).with("#{authors(:chuck).cache_key}/cached_posts").times(2).returns association_proxy(:chuck)
# cache.expects(:fetch).with("#{cache_key}/cached_posts").times(2).returns association_proxy

post = authors(:chuck).cached_posts.last
authors(:luca).cached_posts << post

assert_equal posts_by_author(:luca), authors(:luca).cached_posts
assert_equal posts_by_author(:chuck), authors(:chuck).cached_posts
end

def test_should_refresh_caches_when_pushing_element_to_polymorphic_association_belonging_to_another_model
# cache.expects(:fetch).with("#{posts(:welcome).cache_key}/cached_tags").times(2).returns tags_association_proxy
# cache.expects(:fetch).with("#{posts(:cached_models).cache_key}/cached_tags").times(2).returns tags_association_proxy(:cached_models)
tag = posts(:welcome).cached_tags.last

posts(:cached_models).cached_tags << tag

# NOTE for some weird reason the assertion fails, even if the collections are equals.
# I forced the comparision between the ids.
assert_equal tags_by_post(:cached_models).map(&:id).sort,
posts(:cached_models).cached_tags.map(&:id).sort
assert_equal tags_by_post(:welcome), posts(:welcome).cached_tags
end

def test_should_update_cache_when_pushing_element_with_build
# cache.expects(:fetch).with("#{cache_key}/cached_posts").times(2).returns association_proxy

author = authors(:luca)
post = author.cached_posts.build post_options
post.save

assert_equal posts_by_author(:luca), author.cached_posts
end

def test_should_update_cache_when_pushing_element_with_create
# cache.expects(:fetch).with("#{cache_key}/cached_posts").times(2).returns association_proxy

author = authors(:luca)
author.cached_posts.create post_options(:title => "CM Overview")

assert_equal posts_by_author(:luca), author.cached_posts
end

def test_should_update_cache_when_pushing_element_with_create_bang_method
# cache.expects(:fetch).with("#{cache_key}/cached_posts").times(2).returns association_proxy

author = authors(:luca)
author.cached_posts.create! post_options(:title => "CM Overview!!")

assert_equal posts_by_author(:luca), author.cached_posts
end
end

private
def posts_by_author(author, include_comments = false)
conditions = include_comments ? { :include => :comments } : { }
Expand Down
10 changes: 10 additions & 0 deletions test/test_helper.rb
@@ -1,3 +1,5 @@
RAILS_ENV = "test" unless defined? RAILS_ENV

require 'test/unit'
require 'rubygems'
require 'active_support'
Expand Down Expand Up @@ -61,6 +63,14 @@ def uses_mocha(description)
$stderr.puts "Skipping #{description} tests. `gem install mocha` and try again."
end

def uses_memcached(description)
require 'memcache'
MemCache.new('localhost').stats
yield
rescue MemCache::MemCacheError
$stderr.puts "Skipping #{test_name} tests. Start memcached and try again."
end

if ENV['SKIP_MOCHA'] == 'true'
class Object
def expects(*args)
Expand Down

0 comments on commit 57e5c00

Please sign in to comment.