mirror of
https://github.com/silverstripe/silverstripe-framework
synced 2024-10-01 05:39:10 +02:00
ENHANCEMENT: add an infinite-loop check as validation in Hierarchy (os4399)
Check only when the parent has changed - hierarchy traversal is expensive operation, so we do it only when it is needed.
This commit is contained in:
parent
42e6ae2cdf
commit
be97535b1e
@ -70,7 +70,7 @@ abstract class DataExtension extends Extension {
|
|||||||
* @param $validationResult Local validation result
|
* @param $validationResult Local validation result
|
||||||
* @throws ValidationException
|
* @throws ValidationException
|
||||||
*/
|
*/
|
||||||
function validate(ValidationResult &$validationResult) {
|
function validate(ValidationResult $validationResult) {
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
@ -30,6 +30,38 @@ class Hierarchy extends DataExtension {
|
|||||||
parent::add_to_class($class, $extensionClass, $args);
|
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,
|
* 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.
|
* so if they have children they will be displayed as a UL inside a LI.
|
||||||
|
@ -12,6 +12,25 @@ class HierarchyTest extends SapphireTest {
|
|||||||
'HierarchyTest_Object'
|
'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().
|
* Test Hierarchy::AllHistoricalChildren().
|
||||||
*/
|
*/
|
||||||
|
Loading…
Reference in New Issue
Block a user