Require explicit counter_cache option for has_many

activerecord/lib/active_record/associations/has_many_association.rb

@@ -80,8 +80,15 @@ def count_records
           [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())

activerecord/test/cases/associations/belongs_to_associations_test.rb

@@ -19,6 +19,9 @@
 require 'models/line_item'
 require 'models/column'
 require 'models/record'
+require 'models/ship'
+require 'models/treasure'
+require 'models/parrot'
 
 class BelongsToAssociationsTest < ActiveRecord::TestCase
   fixtures :accounts, :companies, :developers, :projects, :topics,
@@ -311,6 +314,22 @@ def test_with_select
     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"

activerecord/test/cases/associations/has_many_associations_test.rb

@@ -31,6 +31,8 @@
 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 @@ def test_deleting_before_save
     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

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
 

activerecord/test/models/post.rb

@@ -98,11 +98,11 @@ def add_joins_and_select
     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

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

activerecord/test/schema/schema.rb

@@ -671,6 +671,8 @@ def except(adapter_names_to_exclude)
     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