diff options
7 files changed, 56 insertions, 6 deletions
diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb index 9f6c832c1b..19cdd7f470 100644 --- a/activerecord/lib/active_record/associations/has_many_association.rb +++ b/activerecord/lib/active_record/associations/has_many_association.rb @@ -87,8 +87,15 @@ module ActiveRecord [association_scope.limit_value, count].compact.min end + + # Returns whether a counter cache should be used for this association. + # + # The counter_cache option must be given on either the owner or inverse + # association, and the column must be present on the owner. def has_cached_counter?(reflection = reflection()) - owner.attribute_present?(cached_counter_attribute_name(reflection)) + if reflection.options[:counter_cache] || (inverse = inverse_which_updates_counter_cache(reflection)) && inverse.options[:counter_cache] + owner.attribute_present?(cached_counter_attribute_name(reflection)) + end end def cached_counter_attribute_name(reflection = reflection()) diff --git a/activerecord/test/cases/associations/belongs_to_associations_test.rb b/activerecord/test/cases/associations/belongs_to_associations_test.rb index e7a269c695..0d2e2264e3 100644 --- a/activerecord/test/cases/associations/belongs_to_associations_test.rb +++ b/activerecord/test/cases/associations/belongs_to_associations_test.rb @@ -21,6 +21,9 @@ require 'models/column' require 'models/record' require 'models/admin' require 'models/admin/user' +require 'models/ship' +require 'models/treasure' +require 'models/parrot' class BelongsToAssociationsTest < ActiveRecord::TestCase fixtures :accounts, :companies, :developers, :projects, :topics, @@ -347,6 +350,22 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase assert_equal 1, Company.all.merge!(:includes => :firm_with_select ).find(2).firm_with_select.attributes.size end + def test_belongs_to_without_counter_cache_option + # Ship has a conventionally named `treasures_count` column, but the counter_cache + # option is not given on the association. + ship = Ship.create(name: 'Countless') + + assert_no_difference lambda { ship.reload.treasures_count }, "treasures_count should not be changed unless counter_cache is given on the relation" do + treasure = Treasure.new(name: 'Gold', ship: ship) + treasure.save + end + + assert_no_difference lambda { ship.reload.treasures_count }, "treasures_count should not be changed unless counter_cache is given on the relation" do + treasure = ship.treasures.first + treasure.destroy + end + end + def test_belongs_to_counter debate = Topic.create("title" => "debate") assert_equal 0, debate.read_attribute("replies_count"), "No replies yet" diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index 2cbee3cd21..f487065d9d 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -31,6 +31,8 @@ require 'models/student' require 'models/pirate' require 'models/ship' require 'models/ship_part' +require 'models/treasure' +require 'models/parrot' require 'models/tyre' require 'models/subscriber' require 'models/subscription' @@ -932,6 +934,25 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_equal 0, new_firm.clients_of_firm.size end + def test_has_many_without_counter_cache_option + # Ship has a conventionally named `treasures_count` column, but the counter_cache + # option is not given on the association. + ship = Ship.create(name: 'Countless', treasures_count: 10) + + assert_not ship.treasures.instance_variable_get('@association').send(:has_cached_counter?) + + # Count should come from sql count() of treasures rather than treasures_count attribute + assert_equal ship.treasures.size, 0 + + assert_no_difference lambda { ship.reload.treasures_count }, "treasures_count should not be changed" do + ship.treasures.create(name: 'Gold') + end + + assert_no_difference lambda { ship.reload.treasures_count }, "treasures_count should not be changed" do + ship.treasures.destroy_all + end + end + def test_deleting_updates_counter_cache topic = Topic.order("id ASC").first assert_equal topic.replies.to_a.size, topic.replies_count diff --git a/activerecord/test/models/categorization.rb b/activerecord/test/models/categorization.rb index 6588531de6..4cd67c970a 100644 --- a/activerecord/test/models/categorization.rb +++ b/activerecord/test/models/categorization.rb @@ -1,6 +1,6 @@ class Categorization < ActiveRecord::Base belongs_to :post - belongs_to :category + belongs_to :category, counter_cache: true belongs_to :named_category, :class_name => 'Category', :foreign_key => :named_category_name, :primary_key => :name belongs_to :author diff --git a/activerecord/test/models/post.rb b/activerecord/test/models/post.rb index 10f13b67da..81a18188d4 100644 --- a/activerecord/test/models/post.rb +++ b/activerecord/test/models/post.rb @@ -98,11 +98,11 @@ class Post < ActiveRecord::Base end end - has_many :taggings_with_delete_all, :class_name => 'Tagging', :as => :taggable, :dependent => :delete_all - has_many :taggings_with_destroy, :class_name => 'Tagging', :as => :taggable, :dependent => :destroy + has_many :taggings_with_delete_all, :class_name => 'Tagging', :as => :taggable, :dependent => :delete_all, counter_cache: :taggings_with_delete_all_count + has_many :taggings_with_destroy, :class_name => 'Tagging', :as => :taggable, :dependent => :destroy, counter_cache: :taggings_with_destroy_count - has_many :tags_with_destroy, :through => :taggings, :source => :tag, :dependent => :destroy - has_many :tags_with_nullify, :through => :taggings, :source => :tag, :dependent => :nullify + has_many :tags_with_destroy, :through => :taggings, :source => :tag, :dependent => :destroy, counter_cache: :tags_with_destroy_count + has_many :tags_with_nullify, :through => :taggings, :source => :tag, :dependent => :nullify, counter_cache: :tags_with_nullify_count has_many :misc_tags, -> { where :tags => { :name => 'Misc' } }, :through => :taggings, :source => :tag has_many :funky_tags, :through => :taggings, :source => :tag diff --git a/activerecord/test/models/treasure.rb b/activerecord/test/models/treasure.rb index ffc65466d5..63ff0c23ec 100644 --- a/activerecord/test/models/treasure.rb +++ b/activerecord/test/models/treasure.rb @@ -1,6 +1,7 @@ class Treasure < ActiveRecord::Base has_and_belongs_to_many :parrots belongs_to :looter, :polymorphic => true + # No counter_cache option given belongs_to :ship has_many :price_estimates, :as => :estimate_of diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index 08ac7749f9..6f34115534 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -674,6 +674,8 @@ ActiveRecord::Schema.define do t.string :name t.integer :pirate_id t.integer :update_only_pirate_id + # Conventionally named column for counter_cache + t.integer :treasures_count, default: 0 t.datetime :created_at t.datetime :created_on t.datetime :updated_at |