From 109d1b2b10ac8b602b6a1a9995c3fd9c63aefa22 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Mon, 14 Apr 2014 14:28:26 -0400 Subject: Use inheritance chain instead of callbacks to increment counter caches after create --- .../associations/belongs_to_association.rb | 18 ++++++++++++++---- .../active_record/associations/builder/belongs_to.rb | 13 +------------ activerecord/lib/active_record/counter_cache.rb | 16 ++++++++++++++++ 3 files changed, 31 insertions(+), 16 deletions(-) diff --git a/activerecord/lib/active_record/associations/belongs_to_association.rb b/activerecord/lib/active_record/associations/belongs_to_association.rb index 8272a5584c..b0820f662a 100644 --- a/activerecord/lib/active_record/associations/belongs_to_association.rb +++ b/activerecord/lib/active_record/associations/belongs_to_association.rb @@ -31,6 +31,14 @@ module ActiveRecord @updated end + def decrement_counters + with_cache_name { |name| decrement_counter name } + end + + def increment_counters + with_cache_name { |name| increment_counter name } + end + private def find_target? @@ -51,13 +59,15 @@ module ActiveRecord end end - def decrement_counters - with_cache_name { |name| decrement_counter name } + def decrement_counter(counter_cache_name) + if foreign_key_present? + klass.decrement_counter(counter_cache_name, target_id) + end end - def decrement_counter counter_cache_name + def increment_counter(counter_cache_name) if foreign_key_present? - klass.decrement_counter(counter_cache_name, target_id) + klass.increment_counter(counter_cache_name, target_id) end end diff --git a/activerecord/lib/active_record/associations/builder/belongs_to.rb b/activerecord/lib/active_record/associations/builder/belongs_to.rb index 11be92ae01..bd2ee1a929 100644 --- a/activerecord/lib/active_record/associations/builder/belongs_to.rb +++ b/activerecord/lib/active_record/associations/builder/belongs_to.rb @@ -26,16 +26,9 @@ module ActiveRecord::Associations::Builder private def self.add_counter_cache_methods(mixin) - return if mixin.method_defined? :belongs_to_counter_cache_after_create + return if mixin.method_defined? :belongs_to_counter_cache_after_update mixin.class_eval do - def belongs_to_counter_cache_after_create(reflection) - if record = send(reflection.name) - cache_column = reflection.counter_cache_column - record.class.increment_counter(cache_column, record.id) - @_after_create_counter_called = true - end - end def belongs_to_counter_cache_after_destroy(reflection) foreign_key = reflection.foreign_key.to_sym @@ -74,10 +67,6 @@ module ActiveRecord::Associations::Builder def self.add_counter_cache_callbacks(model, reflection) cache_column = reflection.counter_cache_column - model.after_create lambda { |record| - record.belongs_to_counter_cache_after_create(reflection) - } - model.after_destroy lambda { |record| record.belongs_to_counter_cache_after_destroy(reflection) } diff --git a/activerecord/lib/active_record/counter_cache.rb b/activerecord/lib/active_record/counter_cache.rb index a5897edf03..163da8e870 100644 --- a/activerecord/lib/active_record/counter_cache.rb +++ b/activerecord/lib/active_record/counter_cache.rb @@ -131,6 +131,16 @@ module ActiveRecord private + def _create_record(*) + id = super + + each_counter_cached_associations do |association| + association.increment_counters + end + + id + end + def destroy_row affected_rows = super @@ -139,5 +149,11 @@ module ActiveRecord affected_rows end + def each_counter_cached_associations + reflections.each do |name, reflection| + yield association(name) if reflection.belongs_to? && reflection.counter_cache_column + end + end + end end -- cgit v1.2.3 From 7e28b4ed9a1dbd131877cf784aad7ace49073a52 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Mon, 14 Apr 2014 14:55:34 -0400 Subject: Use inheritance chain instead of callbacks to increment counter caches after destroy --- .../lib/active_record/associations/builder/belongs_to.rb | 16 ---------------- activerecord/lib/active_record/counter_cache.rb | 8 ++++---- 2 files changed, 4 insertions(+), 20 deletions(-) diff --git a/activerecord/lib/active_record/associations/builder/belongs_to.rb b/activerecord/lib/active_record/associations/builder/belongs_to.rb index bd2ee1a929..7614fdd97f 100644 --- a/activerecord/lib/active_record/associations/builder/belongs_to.rb +++ b/activerecord/lib/active_record/associations/builder/belongs_to.rb @@ -30,18 +30,6 @@ module ActiveRecord::Associations::Builder mixin.class_eval do - def belongs_to_counter_cache_after_destroy(reflection) - foreign_key = reflection.foreign_key.to_sym - unless destroyed_by_association && destroyed_by_association.foreign_key.to_sym == foreign_key - record = send reflection.name - if record && self.actually_destroyed? - cache_column = reflection.counter_cache_column - record.class.decrement_counter(cache_column, record.id) - self.clear_destroy_state - end - end - end - def belongs_to_counter_cache_after_update(reflection) foreign_key = reflection.foreign_key cache_column = reflection.counter_cache_column @@ -67,10 +55,6 @@ module ActiveRecord::Associations::Builder def self.add_counter_cache_callbacks(model, reflection) cache_column = reflection.counter_cache_column - model.after_destroy lambda { |record| - record.belongs_to_counter_cache_after_destroy(reflection) - } - model.after_update lambda { |record| record.belongs_to_counter_cache_after_update(reflection) } diff --git a/activerecord/lib/active_record/counter_cache.rb b/activerecord/lib/active_record/counter_cache.rb index 163da8e870..1872d0c141 100644 --- a/activerecord/lib/active_record/counter_cache.rb +++ b/activerecord/lib/active_record/counter_cache.rb @@ -134,9 +134,7 @@ module ActiveRecord def _create_record(*) id = super - each_counter_cached_associations do |association| - association.increment_counters - end + each_counter_cached_associations(&:increment_counters) id end @@ -144,7 +142,9 @@ module ActiveRecord def destroy_row affected_rows = super - @_actually_destroyed = affected_rows > 0 + if affected_rows > 0 + each_counter_cached_associations(&:decrement_counters) + end affected_rows end -- cgit v1.2.3 From 5e32f976928e30da6d2017b415657950adf0c2a8 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Tue, 15 Apr 2014 13:35:02 -0400 Subject: Set _after_create_counter_called flag to make update counter cache work --- activerecord/lib/active_record/counter_cache.rb | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/activerecord/lib/active_record/counter_cache.rb b/activerecord/lib/active_record/counter_cache.rb index 1872d0c141..4108b46439 100644 --- a/activerecord/lib/active_record/counter_cache.rb +++ b/activerecord/lib/active_record/counter_cache.rb @@ -134,7 +134,12 @@ module ActiveRecord def _create_record(*) id = super - each_counter_cached_associations(&:increment_counters) + each_counter_cached_associations do |association| + if record = send(association.reflection.name) + association.increment_counters + @_after_create_counter_called = true + end + end id end -- cgit v1.2.3 From d2543412ff42d64c5fc5d763336b3f3ec9ab8eda Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Tue, 15 Apr 2014 14:11:47 -0400 Subject: Restore the destroy_by_association check in post destroy counter cache --- activerecord/lib/active_record/counter_cache.rb | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/activerecord/lib/active_record/counter_cache.rb b/activerecord/lib/active_record/counter_cache.rb index 4108b46439..b7b790322a 100644 --- a/activerecord/lib/active_record/counter_cache.rb +++ b/activerecord/lib/active_record/counter_cache.rb @@ -135,7 +135,7 @@ module ActiveRecord id = super each_counter_cached_associations do |association| - if record = send(association.reflection.name) + if send(association.reflection.name) association.increment_counters @_after_create_counter_called = true end @@ -148,7 +148,14 @@ module ActiveRecord affected_rows = super if affected_rows > 0 - each_counter_cached_associations(&:decrement_counters) + each_counter_cached_associations do |association| + foreign_key = association.reflection.foreign_key.to_sym + unless destroyed_by_association && destroyed_by_association.foreign_key.to_sym == foreign_key + if send(association.reflection.name) + association.decrement_counters + end + end + end end affected_rows -- cgit v1.2.3 From 7af987cf297efcb0a03a168ff9486c43e0b2ff97 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Tue, 15 Apr 2014 14:21:35 -0400 Subject: Hide BelongsToAssociation#increment_counters and #decrement_counters --- activerecord/lib/active_record/associations/belongs_to_association.rb | 4 ++-- activerecord/lib/active_record/associations/builder/belongs_to.rb | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/activerecord/lib/active_record/associations/belongs_to_association.rb b/activerecord/lib/active_record/associations/belongs_to_association.rb index b0820f662a..1edd4fa3aa 100644 --- a/activerecord/lib/active_record/associations/belongs_to_association.rb +++ b/activerecord/lib/active_record/associations/belongs_to_association.rb @@ -31,11 +31,11 @@ module ActiveRecord @updated end - def decrement_counters + def decrement_counters # :nodoc: with_cache_name { |name| decrement_counter name } end - def increment_counters + def increment_counters # :nodoc: with_cache_name { |name| increment_counter name } end diff --git a/activerecord/lib/active_record/associations/builder/belongs_to.rb b/activerecord/lib/active_record/associations/builder/belongs_to.rb index 7614fdd97f..47cc1f4b34 100644 --- a/activerecord/lib/active_record/associations/builder/belongs_to.rb +++ b/activerecord/lib/active_record/associations/builder/belongs_to.rb @@ -29,7 +29,6 @@ module ActiveRecord::Associations::Builder return if mixin.method_defined? :belongs_to_counter_cache_after_update mixin.class_eval do - def belongs_to_counter_cache_after_update(reflection) foreign_key = reflection.foreign_key cache_column = reflection.counter_cache_column -- cgit v1.2.3