lib/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb @@ -1,3 +1,5 @@ +* has_and_belongs_to_many association support + * Fix memory leak issue in cached_associations * DRYed-up belongs_to definition CHANGELOG @@ -2,6 +2,7 @@ require File.dirname(__FILE__) + '/associations/association_proxy' require File.dirname(__FILE__) + '/associations/association_collection' require File.dirname(__FILE__) + '/associations/has_many_association' +require File.dirname(__FILE__) + '/associations/has_and_belongs_to_many_association' module ActiveRecord module Associations @@ -119,6 +120,29 @@ module ActiveRecord valid_keys_for_has_many_association << :cached valid_keys_for_belongs_to_association << :cached + # TODO uncomment when Rails 2.2.1 comes out + # valid_keys_for_has_and_belongs_to_many_association << :cached + + # TODO remove when Rails 2.2.1 comes out + def create_has_and_belongs_to_many_reflection(association_id, options, &extension) #:nodoc: + options.assert_valid_keys( + :class_name, :table_name, :join_table, :foreign_key, :association_foreign_key, + :select, :conditions, :include, :order, :group, :limit, :offset, + :uniq, + :finder_sql, :delete_sql, :insert_sql, + :before_add, :after_add, :before_remove, :after_remove, + :extend, :readonly, + :validate, :cached + ) + + options[:extend] = create_extension_modules(association_id, extension, options[:extend]) + + reflection = create_reflection(:has_and_belongs_to_many, association_id, options, self) + + reflection.options[:join_table] ||= join_table_name(undecorated_table_name(self.to_s), undecorated_table_name(reflection.class_name)) + + reflection + end def add_has_many_cache_callbacks method_name = :has_many_after_save_cache_expire @@ -128,7 +152,7 @@ module ActiveRecord return unless self[:updated_at] self.class.reflections.each do |name, reflection| - cache_delete(reflection) if reflection.options[:cached] + expire_cache_for(reflection.class_name) end end after_save method_name @@ -140,7 +164,9 @@ module ActiveRecord return if respond_to? after_save_method_name define_method(after_save_method_name) do - send(reflection_name).expire_cache_for(self.class.name) + returning owner = send(reflection_name) do + owner.expire_cache_for(self.class.name) unless owner.blank? + end end alias_method after_destroy_method_name, after_save_method_name lib/activerecord/lib/active_record/associations.rb @@ -109,6 +109,19 @@ module ActiveRecord self end + def destroy_all #:nodoc: + transaction do + each { |record| record.destroy } + end + + reset_target! + + # TODO it should be achievable via callbacks + if @reflection.options[:cached] && @reflection.macro == :has_and_belongs_to_many + @owner.send(:cache_write, @reflection, self) + end + end + # Returns the size of the collection by executing a SELECT COUNT(*) # query if the collection hasn't been loaded, and calling # <tt>collection.size</tt> if it has. lib/activerecord/lib/active_record/associations/association_collection.rb @@ -55,6 +55,7 @@ namespace :cached_models do t.string :title t.text :text t.datetime :published_at + t.integer :rating, :default => 0 t.timestamps end @@ -65,7 +66,7 @@ namespace :cached_models do t.timestamps end - create_table :categories_posts, :force => true do |t| + create_table :categories_posts, :force => true, :id => false do |t| t.integer :category_id t.integer :post_id end tasks/cached_models_tasks.rake @@ -3,10 +3,262 @@ require File.dirname(__FILE__) + '/../../test_helper' class HasAndBelongsToManyAssociationTest < Test::Unit::TestCase include ActiveRecord::Associations - def test_should_not_raise_exception - assert_nothing_raised ArgumentError do - posts(:welcome).categories - categories(:announcements).posts + def setup + cache.clear rescue nil + end + + uses_mocha 'HasAndBelongsToManyAssociationTest' do + def test_should_always_use_cache_for_all_instances_which_reference_the_same_record + cache.expects(:read).with("#{cache_key}/cached_posts").returns association_proxy + expected = category.cached_posts + actual = Category.last.cached_posts + assert_equal expected, actual + end + + def test_should_expire_cache_on_update + cache.expects(:read).with("#{cache_key}/cached_posts").times(2).returns association_proxy + + category.cached_posts # force cache loading + category.update_attributes :name => category.name.upcase + + assert_equal posts_by_category(:rails), category.cached_posts + end + + def test_should_use_cache_when_find_with_scope + cache.expects(:read).with("#{cache_key}/cached_posts").returns association_proxy + post = category.cached_posts.find(posts(:cached_models).id) + assert_equal posts(:cached_models), post + end + + def test_should_use_cache_when_find_with_scope_using_multiple_ids + cache.expects(:read).with("#{cache_key}/cached_posts").returns association_proxy + posts = posts_by_category(:rails) + assert_equal posts, category.cached_posts.find(posts.map(&:id)) + end + + def test_should_use_cache_when_fetch_first_from_collection + cache.expects(:read).with("#{cache_key}/cached_posts").returns association_proxy + assert_equal [ posts_by_category(:rails).first ], category.cached_posts.first(1) + end + + def test_should_use_cache_when_fetch_last_from_collection + cache.expects(:read).with("#{cache_key}/cached_posts").returns association_proxy + assert_equal [ posts_by_category(:rails).last ], category.cached_posts.last(1) + end + + def test_should_unload_cache_when_reset_collection + cache.expects(:read).with("#{cache_key}/cached_posts").times(2).returns association_proxy + assert_false category.cached_posts.reset + assert_equal posts_by_category(:rails), category.cached_posts + end + + def test_should_not_use_cache_on_collection_sum + # calculations aren't supported for now + # TODO verify + cache.expects(:read).with("#{cache_key}/cached_posts").returns association_proxy + assert_equal posts_by_category(:rails).map(&:rating).sum, category.cached_posts.sum(:rating) + end + + def test_should_not_use_cache_on_false_cached_option + cache.expects(:read).never + category.posts + category.posts(true) # force reload + end + + def test_should_cache_associated_objects + cache.expects(:read).with("#{cache_key}/cached_posts").times(2).returns(posts_by_category(:rails)) + posts = category.cached_posts + assert_equal posts, category.cached_posts + end + + def test_should_safely_use_pagination + # pagination for now bypass cache and using database. + # the expectation is due to #cached_posts invocation. + cache.expects(:read).with("#{cache_key}/cached_posts").returns association_proxy + posts = category.cached_posts.paginate(:all, :page => 1, :per_page => 1) + assert_equal [ posts_by_category(:announcements).first ], posts + end + + def test_should_reload_association_and_refresh_the_cache_on_force_reload + cache.expects(:read).with("#{cache_key}/cached_posts").times(2).returns(posts_by_category(:rails)) + cache.expects(:write).times(3).returns true + reloaded_posts = category.cached_posts(true) + assert_equal reloaded_posts, category.cached_posts + end + + def test_should_cache_associated_ids + posts = posts_by_category(:rails) + ids = posts.map(&:id) + cache.expects(:read).with("#{cache_key}/cached_posts").returns posts + cache.expects(:fetch).with("#{cache_key}/cached_post_ids").returns ids + assert_equal ids, category.cached_post_ids + end + + def test_should_not_cache_associated_ids_on_false_cached_option + cache.expects(:fetch).never + category.post_ids + end + + def test_should_cache_all_eager_loaded_objects + cache.expects(:read).with("#{cache_key}/cached_posts_with_comments").returns(posts_by_category(:rails, true)) + posts = category.cached_posts_with_comments + assert_equal posts, category.cached_posts_with_comments + end + + def test_should_not_cache_eager_loaded_objects_on_false_cached_option + cache.expects(:read).never + category.posts_with_comments + end + + def test_should_refresh_cache_when_associated_elements_change + cache.expects(:read).with("#{cache_key}/cached_posts").times(2).returns association_proxy + post = category.cached_posts.last # force cache loading and fetch a post + post.update_attributes :title => 'Cached Models!' + assert_equal posts_by_category(:rails), category.cached_posts + end + + def test_should_refresh_cache_when_pushing_element_to_association + cache.expects(:read).with("#{cache_key}/cached_posts").times(3).returns association_proxy + cache.expects(:write).with("#{cache_key}/cached_posts", association_proxy).returns true + category.cached_posts # force cache loading + category.cached_posts << create_post + assert_equal posts_by_category(:rails), category.cached_posts + end + + def test_should_not_use_cache_when_pushing_element_to_association_on_false_cached_option + cache.expects(:write).never + category.posts # force association loading + category.posts << create_post + end + + def test_should_not_use_cache_when_pushing_element_to_association_belonging_to_anotner_model_on_false_cached_option + cache.expects(:delete).with("#{blogs(:weblog).cache_key}/posts").never + # TODO verify + cache.expects(:delete).with("#{categories(:rails).cache_key}/cached_posts").returns true + post = categories(:announcements).posts.first + category.cached_posts << post + assert_equal posts_by_category(:rails), category.cached_posts.reverse + end + + def test_should_update_cache_when_directly_assigning_a_new_collection + posts = [ posts_by_category(:rails).first ] + cache.expects(:read).with("#{cache_key}/cached_posts").times(2).returns association_proxy + category.cached_posts = posts + assert_equal posts_by_category(:rails), category.cached_posts + end + + def test_should_use_cache_for_collection_size + cache.expects(:read).with("#{cache_key}/cached_posts").returns association_proxy + assert_equal posts_by_category(:rails).size, category.cached_posts.size + end + + def test_should_use_cache_and_return_uniq_records_for_collection_size_on_uniq_option + cache.expects(:read).with("#{cache_key}/uniq_cached_posts").never # wuh?! + assert_equal posts_by_category(:rails).size, category.uniq_cached_posts.size + end + + def test_should_use_cache_for_collection_length + cache.expects(:read).with("#{cache_key}/cached_posts").returns association_proxy + assert_equal posts_by_category(:rails).length, category.cached_posts.length + end + + def test_should_use_cache_for_collection_empty + cache.expects(:read).with("#{cache_key}/cached_posts").returns association_proxy + assert_equal posts_by_category(:rails).empty?, category.cached_posts.empty? + end + + def test_should_use_cache_for_collection_any + cache.expects(:read).with("#{cache_key}/cached_posts").returns association_proxy + assert_equal posts_by_category(:rails).any?, category.cached_posts.any? end + + def test_should_use_cache_for_collection_include + cache.expects(:read).with("#{cache_key}/cached_posts").returns association_proxy + post = posts_by_category(:rails).first + assert category.cached_posts.include?(post) + end + end + + def test_should_refresh_caches_when_pushing_element_to_association_belonging_to_another_model + # TODO in this case we should only refresh cache of the new owner + # + # example: + # post = category1.cached_posts.last + # category2.cached_posts << post + # + # In this case should expire category2 cache only. + post = categories(:announcements).cached_posts.first + category.cached_posts << post + assert_equal posts_by_category(:rails), category.cached_posts.reverse + assert_equal posts_by_category(:announcements), categories(:announcements).cached_posts + end + + def test_should_update_cache_when_pushing_element_with_build + post = category.cached_posts.build post_options + post.save + assert_equal posts_by_category(:rails), category.cached_posts + end + + def test_should_update_cache_when_pushing_element_with_create + category.cached_posts.create post_options(:title => "CM Overview") + assert_equal posts_by_category(:rails), category.cached_posts + end + + def test_should_update_cache_when_pushing_element_with_create_bang_method + category.cached_posts.create! post_options(:title => "CM Overview!!") + assert_equal posts_by_category(:rails), category.cached_posts end + + def test_should_expire_cache_when_delete_all_elements_from_collection + category.cached_posts.delete_all + assert_equal posts_by_category(:rails), category.cached_posts + end + + def test_should_expire_cache_when_destroy_all_elements_from_collection + category.cached_posts.destroy_all + assert_equal posts_by_category(:rails), category.cached_posts + end + + def test_should_update_cache_when_clearing_collection + category.cached_posts.clear + assert_equal posts_by_category(:rails), category.cached_posts + end + + def test_should_update_cache_when_deleting_element_from_collection + category.cached_posts.delete(posts_by_category(:rails).first) + assert_equal posts_by_category(:rails), category.cached_posts + end + + def test_should_update_cache_when_replace_collection + post = create_post; post.save + posts = [ posts_by_category(:rails).first, post ] + category.cached_posts.replace(posts) + assert_equal posts_by_category(:rails), category.cached_posts + end + + def test_should_not_expire_cache_on_update_on_missing_updated_at + category.cached_posts # force cache loading + category.update_attributes :name => category.name.upcase + + assert_equal posts_by_category(:rails), category.cached_posts + end + + private + def association_proxy(category = :rails) + HasAndBelongsToManyAssociation.new(categories(category), Category.reflect_on_association(:cached_posts)) + end + + def posts_by_category(category, load_comments = false) + conditions = load_comments ? { :include => :comments } : { } + Post.find( :all, { :joins => 'LEFT JOIN categories_posts ON posts.id = categories_posts.post_id', + :conditions => [ 'category_id = ?', categories(category).id ] }.merge(conditions) ) + end + + def category + @category ||= categories(:rails) + end + + def cache_key + @cache_key ||= category.cache_key + end end test/active_record/associations/has_and_belongs_to_many_association_test.rb @@ -18,17 +18,9 @@ class HasManyAssociationTest < Test::Unit::TestCase def test_should_expire_cache_on_update author = authors(:luca) - old_cache_key = author.cache_key - cache.expects(:read).with("#{cache_key}/cached_posts").returns association_proxy - cache.expects(:delete).with("#{cache_key}/cached_posts").returns true - cache.expects(:delete).with("#{cache_key}/cached_comments").returns true - cache.expects(:delete).with("#{cache_key}/cached_posts_with_comments").returns true - author.cached_posts # force cache loading author.update_attributes :first_name => author.first_name.upcase - - # assert_not_equal old_cache_key, author.cache_key assert_equal posts_by_author(:luca), authors(:luca).cached_posts end @@ -115,7 +107,7 @@ class HasManyAssociationTest < Test::Unit::TestCase cache.expects(:read).never authors(:luca).posts_with_comments end - + def test_should_cache_polymorphic_associations cache.expects(:read).with("#{posts(:cached_models).cache_key}/cached_tags").returns(tags_by_post(:cached_models)) tags = posts(:cached_models).cached_tags @@ -139,8 +131,6 @@ class HasManyAssociationTest < Test::Unit::TestCase end def test_should_refresh_cache_when_associated_elements_change - cache.expects(:read).with("#{cache_key}/cached_posts").never - cache.expects(:delete).with("#{cache_key}/cached_posts").returns true post = authors(:luca).cached_posts.last # force cache loading and fetch a post post.update_attributes :title => 'Cached Models!' assert_equal posts_by_author(:luca), authors(:luca).cached_posts @@ -162,9 +152,9 @@ class HasManyAssociationTest < Test::Unit::TestCase def test_should_not_use_cache_when_pushing_element_to_association_belonging_to_anotner_model_on_false_cached_option cache.expects(:delete).with("#{blogs(:weblog).cache_key}/posts").never - cache.expects(:delete).with("#{cache_key}/cached_posts_with_comments").never + # TODO verify cache.expects(:delete).with("#{cache_key}/cached_posts").returns true - cache.expects(:delete).with("#{posts(:cached_models).cache_key}/cached_tags").returns true + cache.expects(:delete).with("#{cache_key}/cached_posts_with_comments").returns true post = blogs(:weblog).posts.last blogs(:blog).posts << post assert_equal posts_by_blog(:blog), blogs(:blog).posts @@ -333,20 +323,6 @@ class HasManyAssociationTest < Test::Unit::TestCase HasManyThroughAssociation.new(authors(author), Author.reflect_on_association(:cached_comments)) end - def create_post(options = {}) - Post.new({ :author_id => 1, - :title => 'CachedModels', - :text => 'Introduction to CachedModels plugin', - :published_at => 1.week.ago }.merge(options)) - end - - def post_options(options = {}) - { :blog_id => blogs(:weblog).id, - :title => "Cached models review", - :text => "Cached models review..", - :published_at => 1.week.ago }.merge(options) - end - def cache_key @cache_key ||= authors(:luca).cache_key end test/active_record/associations/has_many_association_test.rb @@ -1,3 +1,7 @@ announcements: id: 1 name: Announcements + +rails: + id: 2 + name: rails test/fixtures/categories.yml @@ -1,3 +1,7 @@ welcome_announcements: category_id: 1 post_id: 1 + +cached_models_rails: + category_id: 2 + post_id: 2 test/fixtures/categories_posts.yml @@ -5,6 +5,7 @@ welcome: title: Welcome text: Welcome to my blog published_at: <%= 3.years.ago %> + rating: 3 cached_models: id: 2 @@ -13,6 +14,7 @@ cached_models: title: Cached Models text: Cached Models plugin overview published_at: <%= 2.weeks.ago %> + rating: 5 fight_club: id: 3 @@ -21,3 +23,4 @@ fight_club: title: Fight Club text: This is a Fight Club review published_at: <%= 1.week.ago %> + rating: 5 test/fixtures/posts.yml @@ -1,3 +1,7 @@ class Category < ActiveRecord::Base has_and_belongs_to_many :posts + has_and_belongs_to_many :cached_posts, :class_name => 'Post', :cached => true + has_and_belongs_to_many :posts_with_comments, :class_name => 'Post', :include => :comments + has_and_belongs_to_many :cached_posts_with_comments, :class_name => 'Post', :include => :comments, :cached => true + has_and_belongs_to_many :uniq_cached_posts, :cached => true, :class_name => 'Post', :uniq => true end test/models/category.rb @@ -5,4 +5,5 @@ class Post < ActiveRecord::Base has_many :tags, :as => :taggable has_many :cached_tags, :as => :taggable, :class_name => 'Tag', :cached => true has_and_belongs_to_many :categories + has_and_belongs_to_many :cached_categories, :class_name => 'Category', :cached => true end test/models/post.rb @@ -67,6 +67,20 @@ class Test::Unit::TestCase def cache ActiveRecord::Base.rails_cache end + + def create_post(options = {}) + Post.new({ :author_id => 1, + :title => 'CachedModels', + :text => 'Introduction to CachedModels plugin', + :published_at => 1.week.ago }.merge(options)) + end + + def post_options(options = {}) + { :blog_id => blogs(:weblog).id, + :title => "Cached models review", + :text => "Cached models review..", + :published_at => 1.week.ago }.merge(options) + end end def uses_mocha(description) test/test_helper.rb 43699419fd3c626ed259e0ef65f61774512fb8f9 Luca Guidi guidi.luca@gmail.com http://github.com/jodosha/cached-models/commit/d6281f451a7a20035b5134b2e53f97d98b5b0f41 d6281f451a7a20035b5134b2e53f97d98b5b0f41 2008-11-12T03:37:16-08:00 2008-11-12T03:37:16-08:00 has_and_belongs_to_many association support 4b11c34bc6a377d5265ff10042f83b5393e3ed24 Luca Guidi guidi.luca@gmail.com