From eaa7c81653ced5d3c3ee500bdb85b03e896ffd94 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9rick=20Dufresne?= Date: Thu, 29 Jun 2017 15:38:17 -0400 Subject: [PATCH 1/3] cleanup hierarchies dependant on deleted element instead of clearing the whole table --- lib/closure_tree/hierarchy_maintenance.rb | 20 +++++++++++- spec/db/schema.rb | 5 +-- spec/hierarchy_maintenance_spec.rb | 39 +++++++++++++++++++++++ 3 files changed, 59 insertions(+), 5 deletions(-) diff --git a/lib/closure_tree/hierarchy_maintenance.rb b/lib/closure_tree/hierarchy_maintenance.rb index e9fe761e..e93cadd9 100644 --- a/lib/closure_tree/hierarchy_maintenance.rb +++ b/lib/closure_tree/hierarchy_maintenance.rb @@ -112,11 +112,29 @@ module ClassMethods # Note that the hierarchy table will be truncated. def rebuild! _ct.with_advisory_lock do - hierarchy_class.delete_all # not destroy_all -- we just want a simple truncate. + cleanup! roots.find_each { |n| n.send(:rebuild!) } # roots just uses the parent_id column, so this is safe. end nil end + + def cleanup! + ids_to_delete = [] + + hierarchy_table = hierarchy_class.arel_table + + [:descendant_id, :ancestor_id].each do |foreign_key| + arel_join = hierarchy_table.join(arel_table, Arel::Nodes::OuterJoin) + .on(arel_table[primary_key].eq(hierarchy_table[foreign_key])) + .join_sources + + where_condition = {} + where_condition[table_name] = {} + where_condition[table_name][primary_key] = nil + + hierarchy_class.joins(arel_join).where(where_condition).destroy_all + end + end end end end diff --git a/spec/db/schema.rb b/spec/db/schema.rb index 18d3f4fd..e8f5bb50 100644 --- a/spec/db/schema.rb +++ b/spec/db/schema.rb @@ -137,15 +137,12 @@ add_foreign_key(:metal, :metal, :column => 'parent_id') - create_table "metal_hierarchies", :id => false do |t| + create_table "metal_hierarchies" do |t| t.integer "ancestor_id", :null => false t.integer "descendant_id", :null => false t.integer "generations", :null => false end - add_foreign_key(:metal_hierarchies, :metal, :column => 'ancestor_id') - add_foreign_key(:metal_hierarchies, :metal, :column => 'descendant_id') - create_table 'menu_items' do |t| t.string 'name' t.integer 'parent_id' diff --git a/spec/hierarchy_maintenance_spec.rb b/spec/hierarchy_maintenance_spec.rb index 6c9b8f60..38794e35 100644 --- a/spec/hierarchy_maintenance_spec.rb +++ b/spec/hierarchy_maintenance_spec.rb @@ -13,4 +13,43 @@ expect(MetalHierarchy.count).to eq(hierarchy_count) end end + + describe '.cleanup!' do + let!(:parent) { Metal.create(:value => "parent metal") } + let!(:child) { Metal.create(:value => "child metal", parent: parent) } + + before do + MetalHierarchy.delete_all + Metal.rebuild! + end + + context 'when an element is deleted' do + it 'should delete the child hierarchies' do + child.delete + + Metal.cleanup! + + expect(MetalHierarchy.where(descendant_id: child.id)).to be_empty + expect(MetalHierarchy.where(ancestor_id: child.id)).to be_empty + end + + it 'should not delete the parent hierarchies' do + child.delete + Metal.cleanup! + expect(MetalHierarchy.where(ancestor_id: parent.id).size).to eq 1 + end + + it 'should not delete other hierarchies' do + other_parent = Metal.create(:value => "other parent metal") + other_child = Metal.create(:value => "other child metal", parent: other_parent) + Metal.rebuild! + + child.delete + Metal.cleanup! + + expect(MetalHierarchy.where(ancestor_id: other_parent.id).size).to eq 2 + expect(MetalHierarchy.where(descendant_id: other_child.id).size).to eq 2 + end + end + end end From 2eb4888ad0ca0446dada4d016e931b4d765bdd76 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9rick=20Dufresne?= Date: Fri, 30 Jun 2017 08:43:33 -0400 Subject: [PATCH 2/3] execute only one delete query for cleanup --- lib/closure_tree/hierarchy_maintenance.rb | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/lib/closure_tree/hierarchy_maintenance.rb b/lib/closure_tree/hierarchy_maintenance.rb index e93cadd9..870e9336 100644 --- a/lib/closure_tree/hierarchy_maintenance.rb +++ b/lib/closure_tree/hierarchy_maintenance.rb @@ -119,21 +119,25 @@ def rebuild! end def cleanup! - ids_to_delete = [] - hierarchy_table = hierarchy_class.arel_table + query = hierarchy_class + alias_tables = [] [:descendant_id, :ancestor_id].each do |foreign_key| - arel_join = hierarchy_table.join(arel_table, Arel::Nodes::OuterJoin) - .on(arel_table[primary_key].eq(hierarchy_table[foreign_key])) + alias_name = foreign_key.to_s.split('_').first + "s" + alias_table = Arel::Table.new(table_name).alias(alias_name) + alias_tables << alias_table + arel_join = hierarchy_table.join(alias_table, Arel::Nodes::OuterJoin) + .on(alias_table[primary_key].eq(hierarchy_table[foreign_key])) .join_sources - where_condition = {} - where_condition[table_name] = {} - where_condition[table_name][primary_key] = nil - - hierarchy_class.joins(arel_join).where(where_condition).destroy_all + query = query.joins(arel_join) end + + query.where( + alias_tables.first[primary_key].eq(nil) + .or(alias_tables.second[primary_key].eq(nil)) + ).destroy_all end end end From 78547aca83fe8cdea6a137799e5520b0c9779bbd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9rick=20Dufresne?= Date: Thu, 20 Jul 2017 10:53:27 -0400 Subject: [PATCH 3/3] delete element from hierarchy without id --- lib/closure_tree/hierarchy_maintenance.rb | 13 ++++--------- spec/db/schema.rb | 2 +- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/lib/closure_tree/hierarchy_maintenance.rb b/lib/closure_tree/hierarchy_maintenance.rb index 870e9336..99cafd9f 100644 --- a/lib/closure_tree/hierarchy_maintenance.rb +++ b/lib/closure_tree/hierarchy_maintenance.rb @@ -121,23 +121,18 @@ def rebuild! def cleanup! hierarchy_table = hierarchy_class.arel_table - query = hierarchy_class - alias_tables = [] [:descendant_id, :ancestor_id].each do |foreign_key| alias_name = foreign_key.to_s.split('_').first + "s" alias_table = Arel::Table.new(table_name).alias(alias_name) - alias_tables << alias_table arel_join = hierarchy_table.join(alias_table, Arel::Nodes::OuterJoin) .on(alias_table[primary_key].eq(hierarchy_table[foreign_key])) .join_sources - query = query.joins(arel_join) - end + lonely_childs = hierarchy_class.joins(arel_join).where(alias_table[primary_key].eq(nil)) + ids = lonely_childs.pluck(foreign_key) - query.where( - alias_tables.first[primary_key].eq(nil) - .or(alias_tables.second[primary_key].eq(nil)) - ).destroy_all + hierarchy_class.where(hierarchy_table[foreign_key].in(ids)).delete_all + end end end end diff --git a/spec/db/schema.rb b/spec/db/schema.rb index e8f5bb50..ec8cff72 100644 --- a/spec/db/schema.rb +++ b/spec/db/schema.rb @@ -137,7 +137,7 @@ add_foreign_key(:metal, :metal, :column => 'parent_id') - create_table "metal_hierarchies" do |t| + create_table "metal_hierarchies", :id => false do |t| t.integer "ancestor_id", :null => false t.integer "descendant_id", :null => false t.integer "generations", :null => false