[Validator] Changed NodeTraverser to traverse nodes iteratively, not recursively

In this way, the deep method call chains are avoided. Also, it is possible to
avoid the many calls to leaveNode(), which are currently not really needed.
This commit is contained in:
Bernhard Schussek 2014-02-20 16:28:28 +01:00
parent cf1281feef
commit 94583a9289
13 changed files with 111 additions and 206 deletions

View File

@ -20,7 +20,7 @@ use Symfony\Component\Validator\ExecutionContextInterface as LegacyExecutionCont
use Symfony\Component\Validator\Group\GroupManagerInterface;
use Symfony\Component\Validator\Mapping\PropertyMetadataInterface;
use Symfony\Component\Validator\Node\Node;
use Symfony\Component\Validator\Util\NodeStackInterface;
use Symfony\Component\Validator\NodeVisitor\NodeObserverInterface;
use Symfony\Component\Validator\Util\PropertyPath;
use Symfony\Component\Validator\Validator\ValidatorInterface;
use Symfony\Component\Validator\Violation\ConstraintViolationBuilder;
@ -33,7 +33,7 @@ use Symfony\Component\Validator\Violation\ConstraintViolationBuilder;
*
* @see ExecutionContextInterface
*/
class ExecutionContext implements ExecutionContextInterface, LegacyExecutionContextInterface, NodeStackInterface
class ExecutionContext implements ExecutionContextInterface, LegacyExecutionContextInterface, NodeObserverInterface
{
/**
* @var ValidatorInterface
@ -76,13 +76,6 @@ class ExecutionContext implements ExecutionContextInterface, LegacyExecutionCont
*/
private $node;
/**
* The trace of nodes from the root node to the current node.
*
* @var \SplStack
*/
private $nodeStack;
/**
* Creates a new execution context.
*
@ -108,49 +101,18 @@ class ExecutionContext implements ExecutionContextInterface, LegacyExecutionCont
$this->translator = $translator;
$this->translationDomain = $translationDomain;
$this->violations = new ConstraintViolationList();
$this->nodeStack = new \SplStack();
}
/**
* Sets the values of the context to match the given node.
*
* Internally, all nodes are stored on a stack and can be removed from that
* stack using {@link popNode()}.
*
* @param Node $node The currently validated node
*/
public function pushNode(Node $node)
public function setCurrentNode(Node $node)
{
$this->nodeStack->push($node);
$this->node = $node;
}
/**
* Sets the values of the context to match the previous node.
*
* The current node is removed from the internal stack and returned.
*
* @return Node|null The currently validated node or null, if no node was
* on the stack
*/
public function popNode()
{
// Nothing to do if the stack is empty
if (0 === count($this->nodeStack)) {
return null;
}
// Remove the current node from the stack
$poppedNode = $this->nodeStack->pop();
// Adjust the current node to the previous node
$this->node = count($this->nodeStack) > 0
? $this->nodeStack->top()
: null;
return $poppedNode;
}
/**
* {@inheritdoc}
*/

View File

@ -11,6 +11,7 @@
namespace Symfony\Component\Validator\Node;
use Symfony\Component\Validator\Exception\UnexpectedTypeException;
use Symfony\Component\Validator\Mapping\PropertyMetadataInterface;
/**
@ -32,6 +33,11 @@ use Symfony\Component\Validator\Mapping\PropertyMetadataInterface;
*/
class PropertyNode extends Node
{
/**
* @var object
*/
public $object;
/**
* @var PropertyMetadataInterface
*/
@ -40,6 +46,8 @@ class PropertyNode extends Node
/**
* Creates a new property node.
*
* @param object $object The object the property
* belongs to
* @param mixed $value The property value
* @param PropertyMetadataInterface $metadata The property's metadata
* @param string $propertyPath The property path leading
@ -49,9 +57,15 @@ class PropertyNode extends Node
* @param string[]|null $cascadedGroups The groups in which
* cascaded objects should
* be validated
*
* @throws UnexpectedTypeException If $object is not an object
*/
public function __construct($value, PropertyMetadataInterface $metadata, $propertyPath, array $groups, $cascadedGroups = null)
public function __construct($object, $value, PropertyMetadataInterface $metadata, $propertyPath, array $groups, $cascadedGroups = null)
{
if (!is_object($object)) {
throw new UnexpectedTypeException($object, 'object');
}
parent::__construct(
$value,
$metadata,
@ -59,6 +73,8 @@ class PropertyNode extends Node
$groups,
$cascadedGroups
);
$this->object = $object;
}
}

View File

@ -44,6 +44,7 @@ class NodeTraverser implements NodeTraverserInterface
public function __construct(MetadataFactoryInterface $metadataFactory)
{
$this->visitors = new \SplObjectStorage();
$this->nodeQueue = new \SplQueue();
$this->metadataFactory = $metadataFactory;
}
@ -73,11 +74,19 @@ class NodeTraverser implements NodeTraverserInterface
}
}
$traversal = new Traversal($context);
foreach ($nodes as $node) {
if ($node instanceof ClassNode) {
$this->traverseClassNode($node, $context);
} else {
$this->traverseNode($node, $context);
$traversal->nodeQueue->enqueue($node);
while (!$traversal->nodeQueue->isEmpty()) {
$node = $traversal->nodeQueue->dequeue();
if ($node instanceof ClassNode) {
$this->traverseClassNode($node, $traversal);
} else {
$this->traverseNode($node, $traversal);
}
}
}
@ -91,49 +100,31 @@ class NodeTraverser implements NodeTraverserInterface
}
}
private function enterNode(Node $node, ExecutionContextInterface $context)
private function visit(Node $node, ExecutionContextInterface $context)
{
$continueTraversal = true;
foreach ($this->visitors as $visitor) {
if (false === $visitor->enterNode($node, $context)) {
$continueTraversal = false;
// Continue, so that the enterNode() method of all visitors
// is called
if (false === $visitor->visit($node, $context)) {
return false;
}
}
return $continueTraversal;
return true;
}
private function leaveNode(Node $node, ExecutionContextInterface $context)
private function traverseNode(Node $node, Traversal $traversal)
{
foreach ($this->visitors as $visitor) {
$visitor->leaveNode($node, $context);
}
}
private function traverseNode(Node $node, ExecutionContextInterface $context)
{
$continue = $this->enterNode($node, $context);
// Visitors have two possibilities to influence the traversal:
//
// 1. If a visitor's enterNode() method returns false, the traversal is
// skipped entirely.
// 2. If a visitor's enterNode() method removes a group from the node,
// that group will be skipped in the subtree of that node.
if (false === $continue) {
$this->leaveNode($node, $context);
if (false === $this->visit($node, $traversal->context)) {
return;
}
if (null === $node->value) {
$this->leaveNode($node, $context);
// Visitors have two possibilities to influence the traversal:
//
// 1. If a visitor's visit() method returns false, the traversal is
// skipped entirely.
// 2. If a visitor's visit() method removes a group from the node,
// that group will be skipped in the subtree of that node.
if (null === $node->value) {
return;
}
@ -144,8 +135,6 @@ class NodeTraverser implements NodeTraverserInterface
: $node->groups;
if (0 === count($cascadedGroups)) {
$this->leaveNode($node, $context);
return;
}
@ -161,11 +150,9 @@ class NodeTraverser implements NodeTraverserInterface
$node->propertyPath,
$cascadedGroups,
$traversalStrategy,
$context
$traversal
);
$this->leaveNode($node, $context);
return;
}
@ -178,25 +165,19 @@ class NodeTraverser implements NodeTraverserInterface
$node->propertyPath,
$cascadedGroups,
$traversalStrategy,
$context
$traversal
);
$this->leaveNode($node, $context);
return;
}
// Traverse only if the TRAVERSE bit is set
if (!($traversalStrategy & TraversalStrategy::TRAVERSE)) {
$this->leaveNode($node, $context);
return;
}
if (!$node->value instanceof \Traversable) {
if ($traversalStrategy & TraversalStrategy::IGNORE_NON_TRAVERSABLE) {
$this->leaveNode($node, $context);
return;
}
@ -212,15 +193,15 @@ class NodeTraverser implements NodeTraverserInterface
$node->propertyPath,
$cascadedGroups,
$traversalStrategy,
$context
$traversal
);
$this->leaveNode($node, $context);
}
private function traverseClassNode(ClassNode $node, ExecutionContextInterface $context, $traversalStrategy = TraversalStrategy::IMPLICIT)
private function traverseClassNode(ClassNode $node, Traversal $traversal, $traversalStrategy = TraversalStrategy::IMPLICIT)
{
$continue = $this->enterNode($node, $context);
if (false === $this->visit($node, $traversal->context)) {
return;
}
// Visitors have two possibilities to influence the traversal:
//
@ -229,21 +210,14 @@ class NodeTraverser implements NodeTraverserInterface
// 2. If a visitor's enterNode() method removes a group from the node,
// that group will be skipped in the subtree of that node.
if (false === $continue) {
$this->leaveNode($node, $context);
return;
}
if (0 === count($node->groups)) {
$this->leaveNode($node, $context);
return;
}
foreach ($node->metadata->getConstrainedProperties() as $propertyName) {
foreach ($node->metadata->getPropertyMetadata($propertyName) as $propertyMetadata) {
$propertyNode = new PropertyNode(
$traversal->nodeQueue->enqueue(new PropertyNode(
$node->value,
$propertyMetadata->getPropertyValue($node->value),
$propertyMetadata,
$node->propertyPath
@ -251,9 +225,7 @@ class NodeTraverser implements NodeTraverserInterface
: $propertyName,
$node->groups,
$node->cascadedGroups
);
$this->traverseNode($propertyNode, $context);
));
}
}
@ -265,15 +237,11 @@ class NodeTraverser implements NodeTraverserInterface
// Traverse only if the TRAVERSE bit is set
if (!($traversalStrategy & TraversalStrategy::TRAVERSE)) {
$this->leaveNode($node, $context);
return;
}
if (!$node->value instanceof \Traversable) {
if ($traversalStrategy & TraversalStrategy::IGNORE_NON_TRAVERSABLE) {
$this->leaveNode($node, $context);
return;
}
@ -289,13 +257,11 @@ class NodeTraverser implements NodeTraverserInterface
$node->propertyPath,
$node->groups,
$traversalStrategy,
$context
$traversal
);
$this->leaveNode($node, $context);
}
private function cascadeObject($object, $propertyPath, array $groups, $traversalStrategy, ExecutionContextInterface $context)
private function cascadeObject($object, $propertyPath, array $groups, $traversalStrategy, Traversal $traversal)
{
try {
$classMetadata = $this->metadataFactory->getMetadataFor($object);
@ -304,14 +270,12 @@ class NodeTraverser implements NodeTraverserInterface
// error
}
$classNode = new ClassNode(
$traversal->nodeQueue->enqueue(new ClassNode(
$object,
$classMetadata,
$propertyPath,
$groups
);
$this->traverseClassNode($classNode, $context, $traversalStrategy);
));
} catch (NoSuchMetadataException $e) {
// Rethrow if the TRAVERSE bit is not set
if (!($traversalStrategy & TraversalStrategy::TRAVERSE)) {
@ -329,12 +293,12 @@ class NodeTraverser implements NodeTraverserInterface
$propertyPath,
$groups,
$traversalStrategy,
$context
$traversal
);
}
}
private function cascadeEachObjectIn($collection, $propertyPath, array $groups, $traversalStrategy, ExecutionContextInterface $context)
private function cascadeEachObjectIn($collection, $propertyPath, array $groups, $traversalStrategy, Traversal $traversal)
{
if ($traversalStrategy & TraversalStrategy::RECURSIVE) {
// Try to traverse nested objects, but ignore if they do not
@ -357,7 +321,7 @@ class NodeTraverser implements NodeTraverserInterface
$propertyPath.'['.$key.']',
$groups,
$traversalStrategy,
$context
$traversal
);
continue;
@ -371,7 +335,7 @@ class NodeTraverser implements NodeTraverserInterface
$propertyPath.'['.$key.']',
$groups,
$traversalStrategy,
$context
$traversal
);
}
}

View File

@ -0,0 +1,31 @@
<?php
/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/
namespace Symfony\Component\Validator\NodeTraverser;
use Symfony\Component\Validator\Context\ExecutionContextInterface;
/**
* @since %%NextVersion%%
* @author Bernhard Schussek <bschussek@gmail.com>
*/
class Traversal
{
public $context;
public $nodeQueue;
public function __construct(ExecutionContextInterface $context)
{
$this->context = $context;
$this->nodeQueue = new \SplQueue();
}
}

View File

@ -28,11 +28,7 @@ abstract class AbstractVisitor implements NodeVisitorInterface
{
}
public function enterNode(Node $node, ExecutionContextInterface $context)
{
}
public function leaveNode(Node $node, ExecutionContextInterface $context)
public function visit(Node $node, ExecutionContextInterface $context)
{
}
}

View File

@ -14,7 +14,6 @@ namespace Symfony\Component\Validator\NodeVisitor;
use Symfony\Component\Validator\Context\ExecutionContextInterface;
use Symfony\Component\Validator\Exception\RuntimeException;
use Symfony\Component\Validator\Node\Node;
use Symfony\Component\Validator\Util\NodeStackInterface;
/**
* Updates the current context with the current node of the validation
@ -25,31 +24,17 @@ use Symfony\Component\Validator\Util\NodeStackInterface;
*/
class ContextRefresherVisitor extends AbstractVisitor
{
public function enterNode(Node $node, ExecutionContextInterface $context)
public function visit(Node $node, ExecutionContextInterface $context)
{
if (!$context instanceof NodeStackInterface) {
if (!$context instanceof NodeObserverInterface) {
throw new RuntimeException(sprintf(
'The ContextRefresherVisitor only supports instances of class '.
'"Symfony\Component\Validator\Context\NodeStackInterface". '.
'"Symfony\Component\Validator\NodeVisitor\NodeObserverInterface". '.
'An instance of class "%s" was given.',
get_class($context)
));
}
$context->pushNode($node);
}
public function leaveNode(Node $node, ExecutionContextInterface $context)
{
if (!$context instanceof NodeStackInterface) {
throw new RuntimeException(sprintf(
'The ContextRefresherVisitor only supports instances of class '.
'"Symfony\Component\Validator\Context\NodeStackInterface". '.
'An instance of class "%s" was given.',
get_class($context)
));
}
$context->popNode();
$context->setCurrentNode($node);
}
}

View File

@ -23,7 +23,7 @@ use Symfony\Component\Validator\Node\Node;
*/
class GroupSequenceResolverVisitor extends AbstractVisitor
{
public function enterNode(Node $node, ExecutionContextInterface $context)
public function visit(Node $node, ExecutionContextInterface $context)
{
if (!$node instanceof ClassNode) {
return;

View File

@ -9,7 +9,7 @@
* file that was distributed with this source code.
*/
namespace Symfony\Component\Validator\Util;
namespace Symfony\Component\Validator\NodeVisitor;
use Symfony\Component\Validator\Node\Node;
@ -17,9 +17,7 @@ use Symfony\Component\Validator\Node\Node;
* @since %%NextVersion%%
* @author Bernhard Schussek <bschussek@gmail.com>
*/
interface NodeStackInterface
interface NodeObserverInterface
{
public function pushNode(Node $node);
public function popNode();
public function setCurrentNode(Node $node);
}

View File

@ -42,29 +42,24 @@ class NodeValidatorVisitor extends AbstractVisitor implements GroupManagerInterf
private $currentGroup;
private $objectHashStack;
public function __construct(NodeTraverserInterface $nodeTraverser, ConstraintValidatorFactoryInterface $validatorFactory)
{
$this->validatorFactory = $validatorFactory;
$this->nodeTraverser = $nodeTraverser;
$this->objectHashStack = new \SplStack();
}
public function afterTraversal(array $nodes, ExecutionContextInterface $context)
{
$this->validatedObjects = array();
$this->validatedConstraints = array();
$this->objectHashStack = new \SplStack();
}
public function enterNode(Node $node, ExecutionContextInterface $context)
public function visit(Node $node, ExecutionContextInterface $context)
{
if ($node instanceof ClassNode) {
$objectHash = spl_object_hash($node->value);
$this->objectHashStack->push($objectHash);
} elseif ($node instanceof PropertyNode && count($this->objectHashStack) > 0) {
$objectHash = $this->objectHashStack->top();
} elseif ($node instanceof PropertyNode) {
$objectHash = spl_object_hash($node->object);
} else {
$objectHash = null;
}
@ -116,13 +111,6 @@ class NodeValidatorVisitor extends AbstractVisitor implements GroupManagerInterf
return true;
}
public function leaveNode(Node $node, ExecutionContextInterface $context)
{
if ($node instanceof ClassNode) {
$this->objectHashStack->pop();
}
}
public function getCurrentGroup()
{
return $this->currentGroup;

View File

@ -24,7 +24,5 @@ interface NodeVisitorInterface
public function afterTraversal(array $nodes, ExecutionContextInterface $context);
public function enterNode(Node $node, ExecutionContextInterface $context);
public function leaveNode(Node $node, ExecutionContextInterface $context);
public function visit(Node $node, ExecutionContextInterface $context);
}

View File

@ -43,7 +43,7 @@ class ObjectInitializerVisitor extends AbstractVisitor
$this->initializers = $initializers;
}
public function enterNode(Node $node, ExecutionContextInterface $context)
public function visit(Node $node, ExecutionContextInterface $context)
{
if (!$node instanceof ClassNode) {
return;

View File

@ -55,41 +55,6 @@ class ExecutionContextTest extends \PHPUnit_Framework_TestCase
);
}
public function testPushAndPop()
{
$metadata = $this->getMock('Symfony\Component\Validator\Mapping\MetadataInterface');
$node = new GenericNode('value', $metadata, '', array(), array());
$this->context->pushNode($node);
$this->assertSame('value', $this->context->getValue());
// the other methods are covered in AbstractValidatorTest
$this->assertSame($node, $this->context->popNode());
$this->assertNull($this->context->getValue());
}
public function testPushTwiceAndPop()
{
$metadata1 = $this->getMock('Symfony\Component\Validator\Mapping\MetadataInterface');
$node1 = new GenericNode('value', $metadata1, '', array(), array());
$metadata2 = $this->getMock('Symfony\Component\Validator\Mapping\MetadataInterface');
$node2 = new GenericNode('other value', $metadata2, '', array(), array());
$this->context->pushNode($node1);
$this->context->pushNode($node2);
$this->assertSame($node2, $this->context->popNode());
$this->assertSame('value', $this->context->getValue());
}
public function testPopWithoutPush()
{
$this->assertNull($this->context->popNode());
}
public function testGetGroup()
{
$this->groupManager->expects($this->once())

View File

@ -141,6 +141,7 @@ class ContextualValidator implements ContextualValidatorInterface
$propertyValue = $propertyMetadata->getPropertyValue($object);
$nodes[] = new PropertyNode(
$object,
$propertyValue,
$propertyMetadata,
PropertyPath::append($this->defaultPropertyPath, $propertyName),
@ -172,6 +173,7 @@ class ContextualValidator implements ContextualValidatorInterface
foreach ($propertyMetadatas as $propertyMetadata) {
$nodes[] = new PropertyNode(
$object,
$value,
$propertyMetadata,
PropertyPath::append($this->defaultPropertyPath, $propertyName),