Skip to content
Browse files

Update other counter caches on destroy

  • Loading branch information...
1 parent 34c7e73 commit 66679c8ecd9e916cbd96745b853603bc2fed7639 @iangreenleaf iangreenleaf committed
View
5 activerecord/CHANGELOG.md
@@ -1,5 +1,10 @@
## Rails 4.0.0 (unreleased) ##
+* Models with multiple counter cache associations now update correctly on destroy.
+ See #7706.
+
+ *Ian Young*
+
* If inverse_of is true on an association, then when one calls +find()+ on
the association, ActiveRecord will first look through the in-memory objects
in the association for a particular id. Then, it will go to the DB if it
View
2 activerecord/lib/active_record/associations/builder/belongs_to.rb
@@ -31,7 +31,7 @@ def belongs_to_counter_cache_after_create_for_#{name}
end
def belongs_to_counter_cache_before_destroy_for_#{name}
- unless marked_for_destruction?
+ unless destroyed_by_association && destroyed_by_association.foreign_key.to_sym == #{foreign_key.to_sym.inspect}
record = #{name}
record.class.decrement_counter(:#{cache_column}, record.id) unless record.nil?
end
View
2 activerecord/lib/active_record/associations/has_many_association.rb
@@ -22,7 +22,7 @@ def handle_dependency
else
if options[:dependent] == :destroy
# No point in executing the counter update since we're going to destroy the parent anyway
- load_target.each(&:mark_for_destruction)
+ load_target.each { |t| t.destroyed_by_association = reflection }
destroy_all
else
delete_all
View
14 activerecord/lib/active_record/autosave_association.rb
@@ -212,6 +212,7 @@ def add_autosave_association_callbacks(reflection)
# Reloads the attributes of the object as usual and clears <tt>marked_for_destruction</tt> flag.
def reload(options = nil)
@marked_for_destruction = false
+ @destroyed_by_association = nil
super
end
@@ -231,6 +232,19 @@ def marked_for_destruction?
@marked_for_destruction
end
+ # Records the association that is being destroyed and destroying this
+ # record in the process.
+ def destroyed_by_association=(reflection)
+ @destroyed_by_association = reflection
+ end
+
+ # Returns the association for the parent being destroyed.
+ #
+ # Used to avoid updating the counter cache unnecessarily.
+ def destroyed_by_association
+ @destroyed_by_association
+ end
+
# Returns whether or not this record has been changed in any way (including whether
# any of its nested autosave associations are likewise changed)
def changed_for_autosave?
View
1 activerecord/lib/active_record/core.rb
@@ -427,6 +427,7 @@ def init_internals
@readonly = false
@destroyed = false
@marked_for_destruction = false
+ @destroyed_by_association = nil
@new_record = true
@txn = nil
@_start_transaction_state = {}
View
8 activerecord/test/cases/counter_cache_test.rb
@@ -115,6 +115,14 @@ class ::SpecialReply < ::Reply
end
end
+ test "update other counters on parent destroy" do
+ david, joanna = dog_lovers(:david, :joanna)
+
+ assert_difference 'joanna.reload.dogs_count', -1 do
+ david.destroy
+ end
+ end
+
test "reset the right counter if two have the same foreign key" do
michael = people(:michael)
assert_nothing_raised(ActiveRecord::StatementInvalid) do
View
3 activerecord/test/fixtures/dog_lovers.yml
@@ -2,3 +2,6 @@ david:
id: 1
bred_dogs_count: 0
trained_dogs_count: 1
+joanna:
+ id: 2
+ dogs_count: 1
View
1 activerecord/test/fixtures/dogs.yml
@@ -1,3 +1,4 @@
sophie:
id: 1
trainer_id: 1
+ dog_lover_id: 2
View
5 activerecord/test/models/dog.rb
@@ -1,4 +1,5 @@
class Dog < ActiveRecord::Base
- belongs_to :breeder, :class_name => "DogLover", :counter_cache => :bred_dogs_count
- belongs_to :trainer, :class_name => "DogLover", :counter_cache => :trained_dogs_count
+ belongs_to :breeder, class_name: "DogLover", counter_cache: :bred_dogs_count
+ belongs_to :trainer, class_name: "DogLover", counter_cache: :trained_dogs_count
+ belongs_to :doglover, foreign_key: :dog_lover_id, class_name: "DogLover", counter_cache: true
end
View
5 activerecord/test/models/dog_lover.rb
@@ -1,4 +1,5 @@
class DogLover < ActiveRecord::Base
- has_many :trained_dogs, :class_name => "Dog", :foreign_key => :trainer_id
- has_many :bred_dogs, :class_name => "Dog", :foreign_key => :breeder_id
+ has_many :trained_dogs, class_name: "Dog", foreign_key: :trainer_id, dependent: :destroy
+ has_many :bred_dogs, class_name: "Dog", foreign_key: :breeder_id
+ has_many :dogs
end
View
8 activerecord/test/schema/schema.rb
@@ -230,14 +230,16 @@ def create_table(*args, &block)
t.integer :access_level, :default => 1
end
- create_table :dog_lovers, :force => true do |t|
- t.integer :trained_dogs_count, :default => 0
- t.integer :bred_dogs_count, :default => 0
+ create_table :dog_lovers, force: true do |t|
+ t.integer :trained_dogs_count, default: 0
+ t.integer :bred_dogs_count, default: 0
+ t.integer :dogs_count, default: 0
end
create_table :dogs, :force => true do |t|
t.integer :trainer_id
t.integer :breeder_id
+ t.integer :dog_lover_id
end
create_table :edges, :force => true, :id => false do |t|

0 comments on commit 66679c8

Please sign in to comment.
Something went wrong with that request. Please try again.