From 770f2e36eb8b9865ee0ea1f81f5ca21068bebe3e Mon Sep 17 00:00:00 2001 From: Arthur Holstvoogd Date: Wed, 8 Jun 2011 11:34:11 +0200 Subject: [PATCH 1/3] Fix bug in restore_ancestry_integrity!, added test to verify we didn't remove the ancestry from all nodes. --- lib/ancestry/class_methods.rb | 51 ++++++++++++++++++----------------- test/has_ancestry_test.rb | 9 ++++--- 2 files changed, 33 insertions(+), 27 deletions(-) diff --git a/lib/ancestry/class_methods.rb b/lib/ancestry/class_methods.rb index c7184ef1..feaf0a18 100644 --- a/lib/ancestry/class_methods.rb +++ b/lib/ancestry/class_methods.rb @@ -107,33 +107,36 @@ def check_ancestry_integrity! options = {} # Integrity restoration def restore_ancestry_integrity! parents = {} - # For each node ... - self.base_class.find_each do |node| - # ... set its ancestry to nil if invalid - if node.errors[node.class.ancestry_column].blank? - node.without_ancestry_callbacks do - node.update_attribute node.ancestry_column, nil + # Wrap the whole thing in a transaction ... + self.base_class.transaction do + # For each node ... + self.base_class.find_each do |node| + # ... set its ancestry to nil if invalid + if !node.valid? and !node.errors[node.class.ancestry_column].blank? + node.without_ancestry_callbacks do + node.update_attribute node.ancestry_column, nil + end end - end - # ... save parent of this node in parents array if it exists - parents[node.id] = node.parent_id if exists? node.parent_id + # ... save parent of this node in parents array if it exists + parents[node.id] = node.parent_id if exists? node.parent_id - # Reset parent id in array to nil if it introduces a cycle - parent = parents[node.id] - until parent.nil? || parent == node.id - parent = parents[parent] - end - parents[node.id] = nil if parent == node.id - end - # For each node ... - self.base_class.find_each do |node| - # ... rebuild ancestry from parents array - ancestry, parent = nil, parents[node.id] - until parent.nil? - ancestry, parent = if ancestry.nil? then parent else "#{parent}/#{ancestry}" end, parents[parent] + # Reset parent id in array to nil if it introduces a cycle + parent = parents[node.id] + until parent.nil? || parent == node.id + parent = parents[parent] + end + parents[node.id] = nil if parent == node.id end - node.without_ancestry_callbacks do - node.update_attribute node.ancestry_column, ancestry + # For each node ... + self.base_class.find_each do |node| + # ... rebuild ancestry from parents array + ancestry, parent = nil, parents[node.id] + until parent.nil? + ancestry, parent = if ancestry.nil? then parent else "#{parent}/#{ancestry}" end, parents[parent] + end + node.without_ancestry_callbacks do + node.update_attribute node.ancestry_column, ancestry + end end end end diff --git a/test/has_ancestry_test.rb b/test/has_ancestry_test.rb index dca1d90c..87d9110f 100644 --- a/test/has_ancestry_test.rb +++ b/test/has_ancestry_test.rb @@ -379,23 +379,26 @@ def assert_integrity_restoration model assert_nothing_raised do model.check_ancestry_integrity! end + assert model.all.any? {|node| node.ancestry.present? }, "Expected some nodes not to be roots" + assert_equal model.count, model.roots.collect {|node| node.descendants.count + 1 }.sum end def test_integrity_restoration + width, depth = 3, 3 # Check that integrity is restored for invalid format for ancestry column - AncestryTestDatabase.with_model :width => 3, :depth => 3 do |model, roots| + AncestryTestDatabase.with_model :width => width, :depth => depth do |model, roots| roots.first.first.update_attribute model.ancestry_column, 'invalid_ancestry' assert_integrity_restoration model end # Check that integrity is restored for non-existent ancestor - AncestryTestDatabase.with_model :width => 3, :depth => 3 do |model, roots| + AncestryTestDatabase.with_model :width => width, :depth => depth do |model, roots| roots.first.first.update_attribute model.ancestry_column, 35 assert_integrity_restoration model end # Check that integrity is restored for cyclic ancestry - AncestryTestDatabase.with_model :width => 3, :depth => 3 do |model, roots| + AncestryTestDatabase.with_model :width => width, :depth => depth do |model, roots| node = roots.first.first node.update_attribute model.ancestry_column, node.id assert_integrity_restoration model From 8e3247bd4d0a8a432164da7e40c2988760bac1b9 Mon Sep 17 00:00:00 2001 From: Roel van der Hoorn Date: Mon, 12 Mar 2012 12:30:28 +0100 Subject: [PATCH 2/3] Fix three warnings: "parenthesize argument(s) for future version" --- lib/ancestry/class_methods.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/ancestry/class_methods.rb b/lib/ancestry/class_methods.rb index feaf0a18..a65d37d7 100644 --- a/lib/ancestry/class_methods.rb +++ b/lib/ancestry/class_methods.rb @@ -78,19 +78,19 @@ def check_ancestry_integrity! options = {} begin # ... check validity of ancestry column if !node.valid? and !node.errors[node.class.ancestry_column].blank? - raise Ancestry::AncestryIntegrityException.new "Invalid format for ancestry column of node #{node.id}: #{node.read_attribute node.ancestry_column}." + raise Ancestry::AncestryIntegrityException.new("Invalid format for ancestry column of node #{node.id}: #{node.read_attribute node.ancestry_column}.") end # ... check that all ancestors exist node.ancestor_ids.each do |ancestor_id| unless exists? ancestor_id - raise Ancestry::AncestryIntegrityException.new "Reference to non-existent node in node #{node.id}: #{ancestor_id}." + raise Ancestry::AncestryIntegrityException.new("Reference to non-existent node in node #{node.id}: #{ancestor_id}.") end end # ... check that all node parents are consistent with values observed earlier node.path_ids.zip([nil] + node.path_ids).each do |node_id, parent_id| parents[node_id] = parent_id unless parents.has_key? node_id unless parents[node_id] == parent_id - raise Ancestry::AncestryIntegrityException.new "Conflicting parent id found in node #{node.id}: #{parent_id || 'nil'} for node #{node_id} while expecting #{parents[node_id] || 'nil'}" + raise Ancestry::AncestryIntegrityException.new("Conflicting parent id found in node #{node.id}: #{parent_id || 'nil'} for node #{node_id} while expecting #{parents[node_id] || 'nil'}") end end rescue Ancestry::AncestryIntegrityException => integrity_exception From 7b214ee81bc3fe13096b1d5be70099c40d6be88a Mon Sep 17 00:00:00 2001 From: Roel van der Hoorn Date: Thu, 15 Mar 2012 13:10:37 +0100 Subject: [PATCH 3/3] Up the version number. --- README.rdoc | 9 ++++++--- ancestry.gemspec | 2 +- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/README.rdoc b/README.rdoc index ac3cf8f3..12428d05 100644 --- a/README.rdoc +++ b/README.rdoc @@ -1,6 +1,6 @@ = Ancestry -Ancestry is a gem/plugin that allows the records of a Ruby on Rails ActiveRecord model to be organised as a tree structure (or hierarchy). It uses a single, intuitively formatted database column, using a variation on the materialised path pattern. It exposes all the standard tree structure relations (ancestors, parent, root, children, siblings, descendants) and all of them can be fetched in a single sql query. Additional features are STI support, scopes, depth caching, depth constraints, easy migration from older plugins/gems, integrity checking, integrity restoration, arrangement of (sub)tree into hashes and different strategies for dealing with orphaned records. +Ancestry is a gem/plugin that allows the records of a Ruby on Rails ActiveRecord model to be organised as a tree structure (or hierarchy). It uses a single, intuitively formatted database column, using a variation on the materialised path pattern. It exposes all the standard tree structure relations (ancestors, parent, root, children, siblings, descendants) and all of them can be fetched in a single SQL query. Additional features are STI support, scopes, depth caching, depth constraints, easy migration from older plugins/gems, integrity checking, integrity restoration, arrangement of (sub)tree into hashes and different strategies for dealing with orphaned records. = Installation @@ -228,7 +228,7 @@ The Ancestry gem comes with a unit test suite consisting of about 1800 assertion = Internals -As can be seen in the previous section, Ancestry stores a path from the root to the parent for every node. This is a variation on the materialised path database pattern. It allows Ancestry to fetch any relation (siblings, descendants, etc.) in a single sql query without the complicated algorithms and incomprehensibility associated with left and right values. Additionally, any inserts, deletes and updates only affect nodes within the affected node's own subtree. +As can be seen in the previous section, Ancestry stores a path from the root to the parent for every node. This is a variation on the materialised path database pattern. It allows Ancestry to fetch any relation (siblings, descendants, etc.) in a single SQL query without the complicated algorithms and incomprehensibility associated with left and right values. Additionally, any inserts, deletes and updates only affect nodes within the affected node's own subtree. In the example above, the ancestry column is created as a string. This puts a limitation on the depth of the tree of about 40 or 50 levels, which I think may be enough for most users. To increase the maximum depth of the tree, increase the size of the string that is being used or change it to a text to remove the limitation entirely. Changing it to a text will however decrease performance because an index cannot be put on the column in that case. @@ -238,7 +238,10 @@ The materialised path pattern requires Ancestry to use a 'like' condition in ord The latest version of ancestry is recommended. The three numbers of each version numbers are respectively the major, minor and patch versions. We started with major version 1 because it looks so much better and ancestry was already quite mature and complete when it was published. The major version is only bumped when backwards compatibility is broken. The minor version is bumped when new features are added. The patch version is bumped when bugs are fixed. -- Version 1.2.4 (2011-4-22) +- Version 1.2.5 (2012-03-15) + - Fixed warnings: "parenthesize argument(s) for future version" + - Fixed a bug in the restore_ancestry_integrity! method (thx Arthur Holstvoogd) +- Version 1.2.4 (2011-04-22) - Prepended table names to column names in queries (thx raelik) - Better check to see if acts_as_tree can be overloaded (thx jims) - Performance inprovements (thx kueda) diff --git a/ancestry.gemspec b/ancestry.gemspec index c257d824..129b2509 100644 --- a/ancestry.gemspec +++ b/ancestry.gemspec @@ -3,7 +3,7 @@ Gem::Specification.new do |s| s.description = 'Organise ActiveRecord model into a tree structure' s.summary = 'Ancestry allows the records of a ActiveRecord model to be organised in a tree structure, using a single, intuitively formatted database column. It exposes all the standard tree structure relations (ancestors, parent, root, children, siblings, descendants) and all of them can be fetched in a single sql query. Additional features are named_scopes, integrity checking, integrity restoration, arrangement of (sub)tree into hashes and different strategies for dealing with orphaned records.' - s.version = '1.2.3' + s.version = '1.2.5' s.author = 'Stefan Kroes' s.email = 's.a.kroes@gmail.com'