diff --git a/README.md b/README.md index 7cd849a..45dd33f 100644 --- a/README.md +++ b/README.md @@ -314,6 +314,7 @@ When you include ```has_closure_tree``` in your model, you can provide a hash to * ```:hierarchy_table_name``` to override the hierarchy table name. This defaults to the singular name of the model + "_hierarchies", like ```tag_hierarchies```. * ```:dependent``` determines what happens when a node is destroyed. Defaults to ```nullify```. * ```:nullify``` will simply set the parent column to null. Each child node will be considered a "root" node. This is the default. + * ```:adopt``` will move children to their grandparent (parent's parent). If there is no grandparent, children become root nodes. This is useful for maintaining tree structure when removing intermediate nodes. * ```:delete_all``` will delete all descendant nodes (which circumvents the destroy hooks) * ```:destroy``` will destroy all descendant nodes (which runs the destroy hooks on each child node) * ```nil``` does nothing with descendant nodes diff --git a/lib/closure_tree/association_setup.rb b/lib/closure_tree/association_setup.rb index 3b5a379..04e8c04 100644 --- a/lib/closure_tree/association_setup.rb +++ b/lib/closure_tree/association_setup.rb @@ -20,7 +20,7 @@ module AssociationSetup has_many :children, *_ct.has_many_order_with_option, class_name: _ct.model_class.to_s, foreign_key: _ct.parent_column_name, - dependent: _ct.options[:dependent], + dependent: _ct.options[:dependent] == :adopt ? :nullify : _ct.options[:dependent], inverse_of: :parent do # We have to redefine hash_tree because the activerecord relation is already scoped to parent_id. def hash_tree(options = {}) @@ -47,4 +47,4 @@ def hash_tree(options = {}) source: :descendant end end -end \ No newline at end of file +end diff --git a/lib/closure_tree/hierarchy_maintenance.rb b/lib/closure_tree/hierarchy_maintenance.rb index 566e466..72974c7 100644 --- a/lib/closure_tree/hierarchy_maintenance.rb +++ b/lib/closure_tree/hierarchy_maintenance.rb @@ -52,12 +52,24 @@ def _ct_after_save def _ct_before_destroy _ct.with_advisory_lock do + adopt_children_to_grandparent if _ct.options[:dependent] == :adopt delete_hierarchy_references self.class.find(id).children.find_each(&:rebuild!) if _ct.options[:dependent] == :nullify end true # don't prevent destruction end + def adopt_children_to_grandparent + grandparent_id = read_attribute(_ct.parent_column_name) + children_ids = self.class.where(_ct.parent_column_name => id).pluck(:id) + + children_ids.each do |child_id| + child = self.class.find(child_id) + child.update_column(_ct.parent_column_name, grandparent_id) + child.rebuild! + end + end + def rebuild!(called_by_rebuild = false) _ct.with_advisory_lock do delete_hierarchy_references unless (defined? @was_new_record) && @was_new_record diff --git a/lib/closure_tree/support.rb b/lib/closure_tree/support.rb index 1245dab..05c2603 100644 --- a/lib/closure_tree/support.rb +++ b/lib/closure_tree/support.rb @@ -16,7 +16,7 @@ def initialize(model_class, options) @options = { parent_column_name: 'parent_id', - dependent: :nullify, # or :destroy or :delete_all -- see the README + dependent: :nullify, # or :destroy, :delete_all, or :adopt -- see the README name_column: 'name', with_advisory_lock: true, # This will be overridden by adapter support numeric_order: false diff --git a/test/closure_tree/adopt_test.rb b/test/closure_tree/adopt_test.rb new file mode 100644 index 0000000..8507af4 --- /dev/null +++ b/test/closure_tree/adopt_test.rb @@ -0,0 +1,270 @@ +# frozen_string_literal: true + +require 'test_helper' + +def run_adopt_tests_for(model_class) + describe "#{model_class} with dependent: :adopt" do + before do + model_class.delete_all + model_class.hierarchy_class.delete_all + end + + it 'moves children to grandparent when parent is destroyed and updates hierarchy table' do + p1 = model_class.create!(name: 'p1') + p2 = model_class.create!(name: 'p2', parent: p1) + p3 = model_class.create!(name: 'p3', parent: p2) + p4 = model_class.create!(name: 'p4', parent: p3) + + # Verify initial structure: p1 -> p2 -> p3 -> p4 + assert_equal p2, p3.parent + assert_equal p3, p4.parent + assert_equal p1, p2.parent + + # Verify initial hierarchy table entries + hierarchy = model_class.hierarchy_class + assert hierarchy.where(ancestor_id: p1.id, descendant_id: p4.id, generations: 3).exists? + assert hierarchy.where(ancestor_id: p2.id, descendant_id: p4.id, generations: 2).exists? + assert hierarchy.where(ancestor_id: p3.id, descendant_id: p4.id, generations: 1).exists? + assert hierarchy.where(ancestor_id: p3.id, descendant_id: p3.id, generations: 0).exists? + + # Destroy p3 + p3.destroy + + # After destroying p3, p4 should be adopted by p2 (p3's parent) + p4.reload + p2.reload + assert_equal p2, p4.parent, 'p4 should be moved to p2 (grandparent)' + assert_equal p1, p2.parent, 'p2 should still have p1 as parent' + assert_equal [p4], p2.children.to_a, 'p2 should have p4 as child' + + # Verify hierarchy table was updated correctly + # p3 should be removed from hierarchy + assert_empty hierarchy.where(ancestor_id: p3.id) + assert_empty hierarchy.where(descendant_id: p3.id) + + # p4 should now have p2 as direct parent (generations: 1) + assert hierarchy.where(ancestor_id: p2.id, descendant_id: p4.id, generations: 1).exists? + # p4 should have p1 as ancestor (generations: 2) + assert hierarchy.where(ancestor_id: p1.id, descendant_id: p4.id, generations: 2).exists? + # p4 should have itself (generations: 0) + assert hierarchy.where(ancestor_id: p4.id, descendant_id: p4.id, generations: 0).exists? + end + + it 'moves children to root when parent without grandparent is destroyed and updates hierarchy table' do + p1 = model_class.create!(name: 'p1') + p2 = model_class.create!(name: 'p2', parent: p1) + p3 = model_class.create!(name: 'p3', parent: p2) + + # Verify initial structure: p1 -> p2 -> p3 + assert_equal p1, p2.parent + assert_equal p2, p3.parent + + hierarchy = model_class.hierarchy_class + initial_p2_hierarchies = hierarchy.where(ancestor_id: p2.id).count + initial_p3_hierarchies = hierarchy.where(descendant_id: p3.id).count + + # Destroy p1 (root node) + p1.destroy + + # After destroying p1, p2 should become root, and p3 should still be child of p2 + p2.reload + p3.reload + assert_nil p2.parent, 'p2 should become root' + assert_equal p2, p3.parent, 'p3 should still have p2 as parent' + assert p2.root?, 'p2 should be a root node' + assert_equal [p3], p2.children.to_a, 'p2 should have p3 as child' + + # Verify hierarchy table: p1 should be removed + assert_empty hierarchy.where(ancestor_id: p1.id) + assert_empty hierarchy.where(descendant_id: p1.id) + + # p2 should now be a root (no ancestors) + assert hierarchy.where(ancestor_id: p2.id, descendant_id: p2.id, generations: 0).exists? + # p3 should still have p2 as parent + assert hierarchy.where(ancestor_id: p2.id, descendant_id: p3.id, generations: 1).exists? + end + + it 'handles multiple children being adopted and updates hierarchy table' do + p1 = model_class.create!(name: 'p1') + p2 = model_class.create!(name: 'p2', parent: p1) + c1 = model_class.create!(name: 'c1', parent: p2) + c2 = model_class.create!(name: 'c2', parent: p2) + c3 = model_class.create!(name: 'c3', parent: p2) + + # Verify initial structure: p1 -> p2 -> [c1, c2, c3] + assert_equal [c1, c2, c3].sort, p2.children.to_a.sort + + hierarchy = model_class.hierarchy_class + # Verify initial hierarchy: all children should have p1 and p2 as ancestors + [c1, c2, c3].each do |child| + assert hierarchy.where(ancestor_id: p1.id, descendant_id: child.id, generations: 2).exists? + assert hierarchy.where(ancestor_id: p2.id, descendant_id: child.id, generations: 1).exists? + end + + # Destroy p2 + p2.destroy + + # All children should be adopted by p1 + p1.reload + c1.reload + c2.reload + c3.reload + + assert_equal p1, c1.parent, 'c1 should be moved to p1' + assert_equal p1, c2.parent, 'c2 should be moved to p1' + assert_equal p1, c3.parent, 'c3 should be moved to p1' + assert_equal [c1, c2, c3].sort, p1.children.to_a.sort, 'p1 should have all three children' + + # Verify hierarchy table: p2 should be removed + assert_empty hierarchy.where(ancestor_id: p2.id) + assert_empty hierarchy.where(descendant_id: p2.id) + + # All children should now have p1 as direct parent (generations: 1) + [c1, c2, c3].each do |child| + assert hierarchy.where(ancestor_id: p1.id, descendant_id: child.id, generations: 1).exists? + # Should not have p2 in their ancestry anymore + assert_empty hierarchy.where(ancestor_id: p2.id, descendant_id: child.id) + end + end + + it 'maintains hierarchy relationships after adoption' do + p1 = model_class.create!(name: 'p1') + p2 = model_class.create!(name: 'p2', parent: p1) + p3 = model_class.create!(name: 'p3', parent: p2) + p4 = model_class.create!(name: 'p4', parent: p3) + p5 = model_class.create!(name: 'p5', parent: p4) + + # Verify initial structure: p1 -> p2 -> p3 -> p4 -> p5 + assert_equal %w[p1 p2 p3 p4 p5], p5.ancestry_path + + hierarchy = model_class.hierarchy_class + # Verify p5 has all ancestors in hierarchy + assert hierarchy.where(ancestor_id: p1.id, descendant_id: p5.id, generations: 4).exists? + assert hierarchy.where(ancestor_id: p2.id, descendant_id: p5.id, generations: 3).exists? + assert hierarchy.where(ancestor_id: p3.id, descendant_id: p5.id, generations: 2).exists? + assert hierarchy.where(ancestor_id: p4.id, descendant_id: p5.id, generations: 1).exists? + + # Destroy p3 + p3.destroy + + # After adoption, p4 and p5 should still maintain their relationship + p4.reload + p5.reload + assert_equal p2, p4.parent, 'p4 should be adopted by p2' + assert_equal p4, p5.parent, 'p5 should still have p4 as parent' + assert_equal %w[p1 p2 p4 p5], p5.ancestry_path, 'ancestry path should be updated correctly' + + # Verify hierarchy table: p3 should be removed + assert_empty hierarchy.where(ancestor_id: p3.id) + assert_empty hierarchy.where(descendant_id: p3.id) + + # p5 should now have p2 as ancestor (generations: 2) and p4 as parent (generations: 1) + assert hierarchy.where(ancestor_id: p2.id, descendant_id: p5.id, generations: 2).exists? + assert hierarchy.where(ancestor_id: p4.id, descendant_id: p5.id, generations: 1).exists? + assert hierarchy.where(ancestor_id: p1.id, descendant_id: p5.id, generations: 3).exists? + # p5 should not have p3 in its ancestry anymore + assert_empty hierarchy.where(ancestor_id: p3.id, descendant_id: p5.id) + end + + it 'handles deep nested structures correctly and updates hierarchy table' do + root = model_class.create!(name: 'root') + level1 = model_class.create!(name: 'level1', parent: root) + level2 = model_class.create!(name: 'level2', parent: level1) + level3 = model_class.create!(name: 'level3', parent: level2) + level4 = model_class.create!(name: 'level4', parent: level3) + + hierarchy = model_class.hierarchy_class + # Verify initial hierarchy for level4 + assert hierarchy.where(ancestor_id: root.id, descendant_id: level4.id, generations: 4).exists? + assert hierarchy.where(ancestor_id: level1.id, descendant_id: level4.id, generations: 3).exists? + assert hierarchy.where(ancestor_id: level2.id, descendant_id: level4.id, generations: 2).exists? + assert hierarchy.where(ancestor_id: level3.id, descendant_id: level4.id, generations: 1).exists? + + # Destroy level2 + level2.destroy + + # level3 should be adopted by level1, and level4 should still be child of level3 + level1.reload + level3.reload + level4.reload + + assert_equal level1, level3.parent, 'level3 should be adopted by level1' + assert_equal level3, level4.parent, 'level4 should still have level3 as parent' + assert_equal %w[root level1 level3 level4], level4.ancestry_path + + # Verify hierarchy table: level2 should be removed + assert_empty hierarchy.where(ancestor_id: level2.id) + assert_empty hierarchy.where(descendant_id: level2.id) + + # level4 should now have correct ancestry without level2 + assert hierarchy.where(ancestor_id: root.id, descendant_id: level4.id, generations: 3).exists? + assert hierarchy.where(ancestor_id: level1.id, descendant_id: level4.id, generations: 2).exists? + assert hierarchy.where(ancestor_id: level3.id, descendant_id: level4.id, generations: 1).exists? + # level4 should not have level2 in its ancestry anymore + assert_empty hierarchy.where(ancestor_id: level2.id, descendant_id: level4.id) + end + + it 'handles destroying a node with no children' do + p1 = model_class.create!(name: 'p1') + p2 = model_class.create!(name: 'p2', parent: p1) + leaf = model_class.create!(name: 'leaf', parent: p2) + + hierarchy = model_class.hierarchy_class + initial_count = hierarchy.count + + # Destroy leaf (has no children) + leaf.destroy + + # Should not raise any errors + p1.reload + p2.reload + assert_equal [p2], p1.children.to_a + assert_equal [], p2.children.to_a + + # Hierarchy should be cleaned up + assert_empty hierarchy.where(ancestor_id: leaf.id) + assert_empty hierarchy.where(descendant_id: leaf.id) + end + + it 'works with find_or_create_by_path' do + level3 = model_class.find_or_create_by_path(%w[root level1 level2 level3]) + root = level3.root + level1 = root.children.find_by(name: 'level1') + level2 = level1.children.find_by(name: 'level2') + + hierarchy = model_class.hierarchy_class + # Verify initial hierarchy + assert hierarchy.where(ancestor_id: root.id, descendant_id: level3.id).exists? + assert hierarchy.where(ancestor_id: level2.id, descendant_id: level3.id, generations: 1).exists? + + # Destroy level2 + level2.destroy + + # level3 should be adopted by level1 + level1.reload + level3.reload + assert_equal level1, level3.parent + assert_equal %w[root level1 level3], level3.ancestry_path + + # Verify hierarchy table + assert_empty hierarchy.where(ancestor_id: level2.id) + assert hierarchy.where(ancestor_id: level1.id, descendant_id: level3.id, generations: 1).exists? + assert hierarchy.where(ancestor_id: root.id, descendant_id: level3.id, generations: 2).exists? + end + end +end + +# Test with PostgreSQL +if postgresql?(ApplicationRecord.connection) + run_adopt_tests_for(AdoptableTag) +end + +# Test with MySQL +if mysql?(MysqlRecord.connection) + run_adopt_tests_for(MysqlAdoptableTag) +end + +# Test with SQLite +if sqlite?(SqliteRecord.connection) + run_adopt_tests_for(MemoryAdoptableTag) +end diff --git a/test/dummy/app/models/adoptable_tag.rb b/test/dummy/app/models/adoptable_tag.rb new file mode 100644 index 0000000..444c622 --- /dev/null +++ b/test/dummy/app/models/adoptable_tag.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class AdoptableTag < ApplicationRecord + has_closure_tree dependent: :adopt, name_column: 'name' +end + + diff --git a/test/dummy/app/models/memory_adoptable_tag.rb b/test/dummy/app/models/memory_adoptable_tag.rb new file mode 100644 index 0000000..8ba4e97 --- /dev/null +++ b/test/dummy/app/models/memory_adoptable_tag.rb @@ -0,0 +1,6 @@ +# frozen_string_literal: true + +class MemoryAdoptableTag < SqliteRecord + has_closure_tree dependent: :adopt, name_column: 'name' +end + diff --git a/test/dummy/app/models/mysql_adoptable_tag.rb b/test/dummy/app/models/mysql_adoptable_tag.rb new file mode 100644 index 0000000..4743efa --- /dev/null +++ b/test/dummy/app/models/mysql_adoptable_tag.rb @@ -0,0 +1,6 @@ +# frozen_string_literal: true + +class MysqlAdoptableTag < MysqlRecord + has_closure_tree dependent: :adopt, name_column: 'name' +end + diff --git a/test/dummy/db/schema.rb b/test/dummy/db/schema.rb index f5a27bd..3bcd58d 100644 --- a/test/dummy/db/schema.rb +++ b/test/dummy/db/schema.rb @@ -220,4 +220,52 @@ add_index 'scoped_item_hierarchies', %i[ancestor_id descendant_id generations], unique: true, name: 'scoped_item_anc_desc_idx' add_index 'scoped_item_hierarchies', [:descendant_id], name: 'scoped_item_desc_idx' + + create_table 'adoptable_tags' do |t| + t.string 'name' + t.references 'parent' + t.timestamps null: false + end + + create_table 'adoptable_tag_hierarchies', id: false do |t| + t.references 'ancestor', null: false + t.references 'descendant', null: false + t.integer 'generations', null: false + end + + add_index 'adoptable_tag_hierarchies', %i[ancestor_id descendant_id generations], unique: true, + name: 'adoptable_tag_anc_desc_idx' + add_index 'adoptable_tag_hierarchies', [:descendant_id], name: 'adoptable_tag_desc_idx' + + create_table 'mysql_adoptable_tags' do |t| + t.string 'name' + t.references 'parent' + t.timestamps null: false + end + + create_table 'mysql_adoptable_tag_hierarchies', id: false do |t| + t.references 'ancestor', null: false + t.references 'descendant', null: false + t.integer 'generations', null: false + end + + add_index 'mysql_adoptable_tag_hierarchies', %i[ancestor_id descendant_id generations], unique: true, + name: 'mysql_adoptable_tag_anc_desc_idx' + add_index 'mysql_adoptable_tag_hierarchies', [:descendant_id], name: 'mysql_adoptable_tag_desc_idx' + + create_table 'memory_adoptable_tags' do |t| + t.string 'name' + t.references 'parent' + t.timestamps null: false + end + + create_table 'memory_adoptable_tag_hierarchies', id: false do |t| + t.references 'ancestor', null: false + t.references 'descendant', null: false + t.integer 'generations', null: false + end + + add_index 'memory_adoptable_tag_hierarchies', %i[ancestor_id descendant_id generations], unique: true, + name: 'memory_adoptable_tag_anc_desc_idx' + add_index 'memory_adoptable_tag_hierarchies', [:descendant_id], name: 'memory_adoptable_tag_desc_idx' end diff --git a/test/dummy/db/secondary_schema.rb b/test/dummy/db/secondary_schema.rb index d7b141f..084bc48 100644 --- a/test/dummy/db/secondary_schema.rb +++ b/test/dummy/db/secondary_schema.rb @@ -29,4 +29,21 @@ add_index 'mysql_tag_hierarchies', %i[ancestor_id descendant_id generations], unique: true, name: 'mysql_tag_anc_des_idx' add_index 'mysql_tag_hierarchies', [:descendant_id], name: 'mysql_tag_desc_idx' + + create_table 'mysql_adoptable_tags', force: true do |t| + t.string 'name' + t.integer 'parent_id' + t.datetime 'created_at' + t.datetime 'updated_at' + end + + create_table 'mysql_adoptable_tag_hierarchies', id: false, force: true do |t| + t.integer 'ancestor_id', null: false + t.integer 'descendant_id', null: false + t.integer 'generations', null: false + end + + add_index 'mysql_adoptable_tag_hierarchies', %i[ancestor_id descendant_id generations], unique: true, + name: 'mysql_adoptable_tag_anc_desc_idx' + add_index 'mysql_adoptable_tag_hierarchies', [:descendant_id], name: 'mysql_adoptable_tag_desc_idx' end diff --git a/test/dummy/db/sqlite_schema.rb b/test/dummy/db/sqlite_schema.rb index ec31da9..1f0e85f 100644 --- a/test/dummy/db/sqlite_schema.rb +++ b/test/dummy/db/sqlite_schema.rb @@ -29,4 +29,21 @@ add_index 'sqlite_tag_hierarchies', %i[ancestor_id descendant_id generations], unique: true, name: 'sqlite_tag_anc_desc_idx' add_index 'sqlite_tag_hierarchies', [:descendant_id], name: 'sqlite_tag_desc_idx' + + create_table 'memory_adoptable_tags', force: true do |t| + t.string 'name' + t.integer 'parent_id' + t.datetime 'created_at' + t.datetime 'updated_at' + end + + create_table 'memory_adoptable_tag_hierarchies', id: false, force: true do |t| + t.integer 'ancestor_id', null: false + t.integer 'descendant_id', null: false + t.integer 'generations', null: false + end + + add_index 'memory_adoptable_tag_hierarchies', %i[ancestor_id descendant_id generations], unique: true, + name: 'memory_adoptable_tag_anc_desc_idx' + add_index 'memory_adoptable_tag_hierarchies', [:descendant_id], name: 'memory_adoptable_tag_desc_idx' end