Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

ENHANCEMENT: add an infinite-loop check as validation in Hierarchy (o…

…s4399)

Check only when the parent has changed - hierarchy traversal is
expensive operation, so we do it only when it is needed.
  • Loading branch information...
commit be97535b1ed45eaedcee1ccca5bf194054233008 1 parent 42e6ae2
@mateusz mateusz authored
View
2  model/DataExtension.php
@@ -70,7 +70,7 @@ public static function unload_extra_statics($class, $extension) {
* @param $validationResult Local validation result
* @throws ValidationException
*/
- function validate(ValidationResult &$validationResult) {
+ function validate(ValidationResult $validationResult) {
}
/**
View
32 model/Hierarchy.php
@@ -31,6 +31,38 @@ static function add_to_class($class, $extensionClass, $args = null) {
}
/**
+ * Validate the owner object - check for existence of infinite loops.
+ */
+ function validate(ValidationResult $validationResult) {
+ if (!$this->owner->ID) return; // The object is new, won't be looping.
+ if (!$this->owner->ParentID) return; // The object has no parent, won't be looping.
+ if (!$this->owner->isChanged('ParentID')) return; // The parent has not changed, skip the check for performance reasons.
+
+ // Walk the hierarchy upwards until we reach the top, or until we reach the originating node again.
+ $node = $this->owner;
+ while($node) {
+ if ($node->ParentID==$this->owner->ID) {
+ // Hierarchy is looping.
+ $validationResult->error(
+ sprintf(
+ _t(
+ 'Hierarchy.InfiniteLoopNotAllowed',
+ 'Infinite loop found within the "%s" hierarchy. Please change the parent to resolve this',
+ 'First argument is the class that makes up the hierarchy.'
+ ),
+ $this->owner->class
+ ),
+ 'INFINITE_LOOP'
+ );
+ break;
+ }
+ $node = $node->ParentID ? $node->Parent() : null;
+ }
+
+ // At this point the $validationResult contains the response.
+ }
+
+ /**
* Returns the children of this DataObject as an XHTML UL. This will be called recursively on each child,
* so if they have children they will be displayed as a UL inside a LI.
* @param string $attributes Attributes to add to the UL.
View
19 tests/model/HierarchyTest.php
@@ -13,6 +13,25 @@ class HierarchyTest extends SapphireTest {
);
/**
+ * Test the Hierarchy prevents infinite loops.
+ */
+ function testPreventLoop() {
+ $obj2 = $this->objFromFixture('HierarchyTest_Object', 'obj2');
+ $obj2aa = $this->objFromFixture('HierarchyTest_Object', 'obj2aa');
+
+ $obj2->ParentID = $obj2aa->ID;
+ try {
+ $obj2->write();
+ }
+ catch (ValidationException $e) {
+ $this->assertContains('Infinite loop found within the "HierarchyTest_Object" hierarchy', $e->getMessage());
+ return;
+ }
+
+ $this->fail('Failed to prevent infinite loop in hierarchy.');
+ }
+
+ /**
* Test Hierarchy::AllHistoricalChildren().
*/
function testAllHistoricalChildren() {
Please sign in to comment.
Something went wrong with that request. Please try again.