diff --git a/model/DataExtension.php b/model/DataExtension.php index 4216c148f90..6f84415da4a 100644 --- a/model/DataExtension.php +++ b/model/DataExtension.php @@ -63,6 +63,15 @@ static function add_to_class($class, $extensionClass, $args = null) { public static function unload_extra_statics($class, $extension) { throw new Exception('unload_extra_statics gone'); } + + /** + * Hook for extension-specific validation. + * + * @param $validationResult Local validation result + * @throws ValidationException + */ + function validate(ValidationResult $validationResult) { + } /** * Edit the given query object to support queries for this extension diff --git a/model/DataObject.php b/model/DataObject.php index 852ca21fd93..fa6d0fe649c 100644 --- a/model/DataObject.php +++ b/model/DataObject.php @@ -883,7 +883,7 @@ public function forceChange() { * Validate the current object. * * By default, there is no validation - objects are always valid! However, you can overload this method in your - * DataObject sub-classes to specify custom validation. + * DataObject sub-classes to specify custom validation, or use the hook through DataExtension. * * Invalid objects won't be able to be written - a warning will be thrown and no write will occur. onBeforeWrite() * and onAfterWrite() won't get called either. @@ -894,7 +894,9 @@ public function forceChange() { * @return A {@link ValidationResult} object */ protected function validate() { - return new ValidationResult(); + $result = new ValidationResult(); + $this->extend('validate', $result); + return $result; } /** diff --git a/model/Hierarchy.php b/model/Hierarchy.php index 7a2366583e3..2d29de38797 100644 --- a/model/Hierarchy.php +++ b/model/Hierarchy.php @@ -30,6 +30,38 @@ static function add_to_class($class, $extensionClass, $args = null) { parent::add_to_class($class, $extensionClass, $args); } + /** + * 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. diff --git a/tests/model/HierarchyTest.php b/tests/model/HierarchyTest.php index ccffbc54806..4dfa5c96b49 100644 --- a/tests/model/HierarchyTest.php +++ b/tests/model/HierarchyTest.php @@ -12,6 +12,25 @@ class HierarchyTest extends SapphireTest { 'HierarchyTest_Object' ); + /** + * 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(). */