Merge pull request #316 from mateusz/infinite-loops

Re: SiteTree ParentID information can occassionally generate infinite loops.

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

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;
 	}
 
 	/**

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.

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().
 	 */