From d8a28fe8a78d1c63b2c9f502ff81466c953dac1b Mon Sep 17 00:00:00 2001 From: Henry Hsu Date: Sun, 27 Sep 2009 10:21:37 -0700 Subject: [PATCH 1/2] Fixing :counter_cache not updated when appending to polymorphic :belongs_to, test for appending, updated tests for :add and :remove callbacks --- .../test/cases/associations/join_model_test.rb | 10 +++++++++- activerecord/test/cases/callbacks_test.rb | 16 ++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletions(-) diff --git a/activerecord/test/cases/associations/join_model_test.rb b/activerecord/test/cases/associations/join_model_test.rb index 21412d1..cc03aac 100644 --- a/activerecord/test/cases/associations/join_model_test.rb +++ b/activerecord/test/cases/associations/join_model_test.rb @@ -316,7 +316,7 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase end end - def test_belongs_to_polymorphic_with_counter_cache + def test_belongs_to_polymorphic_with_counter_cache__create__ assert_equal 1, posts(:welcome)[:taggings_count] tagging = posts(:welcome).taggings.create(:tag => tags(:general)) assert_equal 2, posts(:welcome, :reload)[:taggings_count] @@ -324,6 +324,14 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase assert_equal 1, posts(:welcome, :reload)[:taggings_count] end + def test_belongs_to_polymorphic_with_counter_cache__append__ + assert_equal 1, posts(:welcome)[:taggings_count] + posts(:welcome).taggings << (tagging = Tagging.create(:tag => tags(:general))) + assert_equal 2, posts(:welcome, :reload)[:taggings_count] + tagging.destroy + assert_equal 1, posts(:welcome, :reload)[:taggings_count] + end + def test_unavailable_through_reflection assert_raise(ActiveRecord::HasManyThroughAssociationNotFoundError) { authors(:david).nothings } end diff --git a/activerecord/test/cases/callbacks_test.rb b/activerecord/test/cases/callbacks_test.rb index 95fddae..871d918 100644 --- a/activerecord/test/cases/callbacks_test.rb +++ b/activerecord/test/cases/callbacks_test.rb @@ -252,6 +252,14 @@ class CallbacksTest < ActiveRecord::TestCase [ :before_create, :proc ], [ :before_create, :object ], [ :before_create, :block ], + [ :before_add, :string ], + [ :before_add, :proc ], + [ :before_add, :object ], + [ :before_add, :block ], + [ :after_add, :string ], + [ :after_add, :proc ], + [ :after_add, :object ], + [ :after_add, :block ], [ :after_create, :string ], [ :after_create, :proc ], [ :after_create, :object ], @@ -326,6 +334,14 @@ class CallbacksTest < ActiveRecord::TestCase [ :before_destroy, :proc ], [ :before_destroy, :object ], [ :before_destroy, :block ], + [ :before_remove, :string ], + [ :before_remove, :proc ], + [ :before_remove, :object ], + [ :before_remove, :block ], + [ :after_remove, :string ], + [ :after_remove, :proc ], + [ :after_remove, :object ], + [ :after_remove, :block ], [ :after_destroy, :string ], [ :after_destroy, :proc ], [ :after_destroy, :object ], -- 1.6.4.2 From a5d0e6f465da88a954f872b146a53461a5e1526a Mon Sep 17 00:00:00 2001 From: Henry Hsu Date: Sun, 27 Sep 2009 10:21:48 -0700 Subject: [PATCH 2/2] Fixes :counter_cache not updated when appending to polymorphic :belongs_to, adding :add and :remove callbacks --- activerecord/lib/active_record/associations.rb | 15 +++++++++------ activerecord/lib/active_record/callbacks.rb | 12 ++++++++++++ 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index c739fdd..8b050de 100755 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -1353,19 +1353,22 @@ module ActiveRecord def add_counter_cache_callbacks(reflection) cache_column = reflection.counter_cache_column - method_name = "belongs_to_counter_cache_after_create_for_#{reflection.name}".to_sym + method_name = "belongs_to_counter_cache_after_add_for_#{reflection.name}".to_sym define_method(method_name) do - association = send(reflection.name) - association.class.increment_counter(cache_column, association.id) unless association.nil? + association = send(reflection.name, true) rescue nil + + if send("#{reflection.primary_key_name}_changed?") && send("#{reflection.primary_key_name}_change") != send("#{reflection.primary_key_name}_change").compact + association.class.increment_counter(cache_column, association.id) unless association.nil? + end end - after_create(method_name) + after_add(method_name) - method_name = "belongs_to_counter_cache_before_destroy_for_#{reflection.name}".to_sym + method_name = "belongs_to_counter_cache_before_remove_for_#{reflection.name}".to_sym define_method(method_name) do association = send(reflection.name) association.class.decrement_counter(cache_column, association.id) unless association.nil? end - before_destroy(method_name) + before_remove(method_name) module_eval( "#{reflection.class_name}.send(:attr_readonly,\"#{cache_column}\".intern) if defined?(#{reflection.class_name}) && #{reflection.class_name}.respond_to?(:attr_readonly)" diff --git a/activerecord/lib/active_record/callbacks.rb b/activerecord/lib/active_record/callbacks.rb index e375037..db6c2f9 100644 --- a/activerecord/lib/active_record/callbacks.rb +++ b/activerecord/lib/active_record/callbacks.rb @@ -215,6 +215,7 @@ module ActiveRecord after_find after_initialize before_save after_save before_create after_create before_update after_update before_validation after_validation before_validation_on_create after_validation_on_create before_validation_on_update after_validation_on_update before_destroy after_destroy + before_add after_add before_remove after_remove ) def self.included(base) #:nodoc: @@ -263,7 +264,9 @@ module ActiveRecord def after_create() end def create_with_callbacks #:nodoc: return false if callback(:before_create) == false + return false if callback(:before_add) == false if changed? result = create_without_callbacks + callback(:after_add) if changed? callback(:after_create) result end @@ -279,7 +282,9 @@ module ActiveRecord def update_with_callbacks(*args) #:nodoc: return false if callback(:before_update) == false + return false if callback(:before_add) == false if changed? result = update_without_callbacks(*args) + callback(:after_add) if changed? callback(:after_update) result end @@ -334,11 +339,18 @@ module ActiveRecord def after_destroy() end def destroy_with_callbacks #:nodoc: return false if callback(:before_destroy) == false + return false if callback(:before_remove) == false result = destroy_without_callbacks + callback(:after_remove) callback(:after_destroy) result end + def before_add() end #:nodoc: + def after_add() end #:nodoc: + def before_remove() end #:nodoc: + def after_remove() end #:nodoc: + private def callback(method) result = run_callbacks(method) { |result, object| false == result } -- 1.6.4.2