Permalink
Browse files

Added validation to detect cycles in record ancestry

  • Loading branch information...
stefankroes committed Oct 18, 2009
1 parent ce731ef commit 488e75208606a709375fe4cf7531f20dd6fc241c
Showing with 26 additions and 15 deletions.
  1. +3 −3 README.rdoc
  2. +12 −9 lib/ancestry/acts_as_tree.rb
  3. +11 −3 test/acts_as_tree_test.rb
View
@@ -111,7 +111,7 @@ The arrange method also works on a scoped class, for example:
= Integrity Checking and Restoration
-I currently don't see any way Ancestry tree integrity could get compromised without explicitly setting cyclic parents but I included code for detecting integrity problems and restoring integrity just to be sure. To check integrity use: [Model].check_ancestry_integrity. An AncestryIntegrityException will be raised if there are any problems. To restore integrity use: [Model].restore_ancestry_integrity.
+I don't see any way Ancestry tree integrity could get compromised without explicitly setting cyclic parents or invalid ancestry and circumventing validation with update_attribute, if you do, please let me know. I did include methods for detecting integrity problems and restoring integrity just to be sure. To check integrity use: [Model].check_ancestry_integrity. An AncestryIntegrityException will be raised if there are any problems. To restore integrity use: [Model].restore_ancestry_integrity.
For example, from IRB:
@@ -130,7 +130,7 @@ For example, from IRB:
= Testing
-The Ancestry gem comes with a unit test suite consisting of about 1400 assertions in 18 tests. It takes about 4 seconds to run on sqlite. To run it yourself, install Ancestry as a plugin, go to the ancestry folder and type 'rake'. The test suite is located in 'test/acts_as_tree_test.rb'.
+The Ancestry gem comes with a unit test suite consisting of about 1500 assertions in 20 tests. It takes about 4 seconds to run on sqlite. To run it yourself, install Ancestry as a plugin, go to the ancestry folder and type 'rake'. The test suite is located in 'test/acts_as_tree_test.rb'.
= Internals
@@ -144,7 +144,7 @@ I will try to keep Ancestry up to date with changing versions of Rails and Ruby
= Feedback
-For questions, bug reports and feature requests, please contact me at s.a.kroes[at]gmail.com
+Question? Bug report? Faulty/incomplete documentation? Feature request? Please contact me at s.a.kroes[at]gmail.com
@@ -28,6 +28,9 @@ def acts_as_tree options = {}
# Validate format of ancestry column value
validates_format_of ancestry_column, :with => /^[0-9]+(\/[0-9]+)*$/, :allow_nil => true
+ # Validate that the ancestor ids don't include own id
+ validate :ancestry_exclude_self
+
# Named scopes
named_scope :roots, :conditions => {ancestry_column => nil}
named_scope :ancestors_of, lambda{ |object| {:conditions => to_node(object).ancestor_conditions} }
@@ -57,8 +60,8 @@ def orphan_strategy= orphan_strategy
else
raise AncestryException.new("Invalid orphan strategy, valid ones are :rootify, :restrict and :destroy.")
end
- end
-
+ end
+
# Arrangement
def arrange
# Get all nodes ordered by ancestry and start sorting them into an empty hash
@@ -105,9 +108,9 @@ def restore_ancestry_integrity
all.each do |node|
# ... set its ancestry to nil if invalid
if node.errors.invalid? node.class.ancestry_column
- node.update_attribute :ancestry, nil
+ node.update_attributes :ancestry => nil
end
- # ... save parent of this node in parents array if it actually exists
+ # ... 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
@@ -124,16 +127,16 @@ def restore_ancestry_integrity
until parent.nil?
ancestry, parent = ancestry.nil? ? parent : "#{parent}/#{ancestry}", parents[parent]
end
- node.update_attribute node.ancestry_column, ancestry
+ node.update_attributes node.ancestry_column => ancestry
end
end
end
module InstanceMethods
- # Fetch tree node if necessary
- def self.to_node object
- object.is_a?(self) ? object : find(object)
- end
+ # Validate that the ancestors don't include itself
+ def ancestry_exclude_self
+ errors.add_to_base "#{self.class.name.humanize} cannot be a descendant of itself." if ancestor_ids.include? self.id
+ end
# Update descendants with new ancestry
def update_descendants_with_new_ancestry
View
@@ -206,14 +206,14 @@ def test_named_scopes
end
def test_ancestroy_column_validation
- node = setup_test_nodes(TestNode, 1, 1).first.first
+ node = TestNode.create
['3', '10/2', '1/4/30', nil].each do |value|
node.write_attribute TestNode.ancestry_column, value
- assert node.save
+ node.valid?; assert !node.errors.invalid?(TestNode.ancestry_column)
end
['1/3/', '/2/3', 'a', 'a/b', '-34', '/54'].each do |value|
node.write_attribute TestNode.ancestry_column, value
- assert !node.save
+ node.valid?; assert node.errors.invalid?(TestNode.ancestry_column)
end
end
@@ -374,4 +374,12 @@ def test_node_creation_though_scope
other_grandchild.save!
assert_equal child, other_grandchild.parent
end
+
+ def test_validate_ancestry_exclude_self
+ parent = TestNode.create!
+ child = parent.children.create!
+ assert_raise ActiveRecord::RecordInvalid do
+ parent.update_attributes! :parent => child
+ end
+ end
end

0 comments on commit 488e752

Please sign in to comment.