diff --git a/lib/ancestry/instance_methods.rb b/lib/ancestry/instance_methods.rb index 08427c2b..ddadcdee 100644 --- a/lib/ancestry/instance_methods.rb +++ b/lib/ancestry/instance_methods.rb @@ -62,10 +62,12 @@ def touch_ancestors_callback # Counter Cache def increase_parent_counter_cache + # TODO: return if ancestry_callbacks_disabled? self.ancestry_base_class.increment_counter _counter_cache_column, parent_id end def decrease_parent_counter_cache + # TODO: ? # @_trigger_destroy_callback comes from activerecord, which makes sure only once decrement when concurrent deletion. # but @_trigger_destroy_callback began after rails@5.1.0.alpha. # https://github.com/rails/rails/blob/v5.2.0/activerecord/lib/active_record/persistence.rb#L340 @@ -78,14 +80,19 @@ def decrease_parent_counter_cache end def update_parent_counter_cache - changed = saved_change_to_attribute?(self.ancestry_base_class.ancestry_column) - - return unless changed + # TODO: + # # @_trigger_update_callback comes from activerecord, which makes sure only once decrement when concurrent deletion. + # # but @_trigger_update_callback began after rails@5.1.0.alpha. + # # https://github.com/rails/rails/blob/v5.2.0/activerecord/lib/active_record/persistence.rb#L340 + # # https://github.com/rails/rails/pull/27248 + # return if defined?(@_trigger_update_callback) && !@_trigger_update_callback + return unless saved_change_to_attribute?(self.ancestry_base_class.ancestry_column) if parent_id_was = parent_id_before_last_save self.ancestry_base_class.decrement_counter _counter_cache_column, parent_id_was end + # TODO: use increase_parent_counter_cache instead? parent_id && self.ancestry_base_class.increment_counter(_counter_cache_column, parent_id) end diff --git a/test/concerns/counter_cache_test.rb b/test/concerns/counter_cache_test.rb index 6ea4b20f..b9a85da0 100644 --- a/test/concerns/counter_cache_test.rb +++ b/test/concerns/counter_cache_test.rb @@ -19,6 +19,8 @@ def test_counter_cache_when_destroying end end + # is this a real use case? + # maybe test this with def test_counter_cache_when_reduplicate_destroying AncestryTestDatabase.with_model :depth => 2, :width => 2, :counter_cache => true do |_model, roots| parent = roots.first.first @@ -86,4 +88,30 @@ def test_counter_cache_when_updating_record end end end + + def test_counter_cache_on_sti + AncestryTestDatabase.with_model :counter_cache => true, :extra_columns => {:type => :string} do |model| + # NOTE rails does not properly remove classes from cache + # So these names need to be unique across the suite + # TODO: Object.const_set "#{model.name}Subclass1", Class.new(model) + + Object.const_set 'Subclass3', Class.new(model) + Object.const_set 'Subclass4', Class.new(model) + + node1 = Subclass3.create! + node2 = Subclass4.create! :parent => node1 + node3 = Subclass3.create! :parent => node1 + node4 = Subclass4.create! :parent => node3 + node5 = Subclass3.create! :parent => node4 + + assert_equal 2, node1.reload.children_count + assert_nil node2.reload.children_count + assert_equal 1, node3.reload.children_count + assert_equal 1, node4.reload.children_count + assert_nil node5.reload.children_count + + Object.send :remove_const, 'Subclass3' + Object.send :remove_const, 'Subclass4' + end + end end diff --git a/test/concerns/sti_support_test.rb b/test/concerns/sti_support_test.rb index 5ae1e015..ae55cddf 100644 --- a/test/concerns/sti_support_test.rb +++ b/test/concerns/sti_support_test.rb @@ -26,39 +26,18 @@ def test_sti_support end end + # Note: this is defining ancestry on the subclass + # But I don't think this does what people want + # The parent will still have ancestry, but pointing to the child class + # finders/counter caches may not find the parent class properly def test_sti_support_with_from_subclass AncestryTestDatabase.with_model :extra_columns => {:type => :string} do |model| subclass1 = Object.const_set 'SubclassWithAncestry', Class.new(model) subclass1.has_ancestry + # this was throwing an exception subclass1.create! Object.send :remove_const, 'SubclassWithAncestry' end end - - def test_sti_support_for_counter_cache - AncestryTestDatabase.with_model :counter_cache => true, :extra_columns => {:type => :string} do |model| - # NOTE had to use subclasses other than Subclass1/Subclass2 from above - # due to (I think) Rails caching those STI classes and that not getting - # reset between specs - - Object.const_set 'Subclass3', Class.new(model) - Object.const_set 'Subclass4', Class.new(model) - - node1 = Subclass3.create! - node2 = Subclass4.create! :parent => node1 - node3 = Subclass3.create! :parent => node1 - node4 = Subclass4.create! :parent => node3 - node5 = Subclass3.create! :parent => node4 - - assert_equal 2, node1.reload.children_count - assert_nil node2.reload.children_count - assert_equal 1, node3.reload.children_count - assert_equal 1, node4.reload.children_count - assert_nil node5.reload.children_count - - Object.send :remove_const, 'Subclass3' - Object.send :remove_const, 'Subclass4' - end - end end