[Validator] Visitors may now abort the traversal by returning false from beforeTraversal()

src/Symfony/Component/Validator/NodeTraverser/NodeTraverserInterface.php

@@ -25,10 +25,13 @@ use Symfony\Component\Validator\NodeVisitor\NodeVisitorInterface;
  * {@link \Symfony\Component\Validator\NodeVisitor\NodeVisitorInterface::visit()}
  * of each visitor is called. At the end of the traversal, the traverser invokes
  * {@link \Symfony\Component\Validator\NodeVisitor\NodeVisitorInterface::afterTraversal()}
- * on each visitor.
+ * on each visitor. The visitors are called in the same order in which they are
+ * added to the traverser.
  *
- * The visitors should be called in the same order in which they are added to
+ * If the {@link traverse()} method is called recursively, the
- * the traverser.
+ * {@link \Symfony\Component\Validator\NodeVisitor\NodeVisitorInterface::beforeTraversal()}
+ * and {@link \Symfony\Component\Validator\NodeVisitor\NodeVisitorInterface::afterTraversal()}
+ * methods of the visitors will be invoked for each call.
  *
  * The validation graph typically contains nodes of the following types:
  *
@@ -92,5 +95,5 @@ interface NodeTraverserInterface
      * @param Node[]                    $nodes   The nodes to traverse
      * @param ExecutionContextInterface $context The validation context
      */
-    public function traverse(array $nodes, ExecutionContextInterface $context);
+    public function traverse($nodes, ExecutionContextInterface $context);
 }

src/Symfony/Component/Validator/NodeTraverser/NonRecursiveNodeTraverser.php

@@ -68,11 +68,6 @@ class NonRecursiveNodeTraverser implements NodeTraverserInterface
      */
     private $metadataFactory;
 
-    /**
-     * @var Boolean
-     */
-    private $traversalStarted = false;
-
     /**
      * Creates a new traverser.
      *
@@ -104,20 +99,20 @@ class NonRecursiveNodeTraverser implements NodeTraverserInterface
     /**
      * {@inheritdoc}
      */
-    public function traverse(array $nodes, ExecutionContextInterface $context)
+    public function traverse($nodes, ExecutionContextInterface $context)
     {
-        // beforeTraversal() and afterTraversal() are only executed for the
+        if (!is_array($nodes)) {
-        // top-level call of traverse()
+            $nodes = array($nodes);
-        $isTopLevelCall = !$this->traversalStarted;
+        }
 
-        if ($isTopLevelCall) {
+        $numberOfInitializedVisitors = $this->beforeTraversal($nodes, $context);
-            // Remember that the traversal was already started for the case of
-            // recursive calls to traverse()
-            $this->traversalStarted = true;
 
-            foreach ($this->visitors as $visitor) {
+        // If any of the visitors requested to abort the traversal, do so, but
-                $visitor->beforeTraversal($nodes, $context);
+        // clean up before
-            }
+        if ($numberOfInitializedVisitors < count($this->visitors)) {
+            $this->afterTraversal($nodes, $context, $numberOfInitializedVisitors);
+
+            return;
         }
 
         // This stack contains all the nodes that should be traversed
@@ -148,13 +143,60 @@ class NonRecursiveNodeTraverser implements NodeTraverserInterface
             }
         }
 
-        if ($isTopLevelCall) {
+        $this->afterTraversal($nodes, $context);
-            foreach ($this->visitors as $visitor) {
+    }
-                $visitor->afterTraversal($nodes, $context);
+
+    /**
+     * Executes the {@link NodeVisitorInterface::beforeTraversal()} method of
+     * each visitor.
+     *
+     * @param Node[]                    $nodes   The traversed nodes
+     * @param ExecutionContextInterface $context The current execution context
+     *
+     * @return integer The number of successful calls. This is lower than
+     *                 the number of visitors if any of the visitors'
+     *                 beforeTraversal() methods returned false
+     */
+    private function beforeTraversal($nodes, ExecutionContextInterface $context)
+    {
+        $numberOfCalls = 1;
+
+        foreach ($this->visitors as $visitor) {
+            if (false === $visitor->beforeTraversal($nodes, $context)) {
+                break;
             }
 
-            // Put the traverser back into its initial state
+            ++$numberOfCalls;
-            $this->traversalStarted = false;
+        }
+
+        return $numberOfCalls;
+    }
+
+    /**
+     * Executes the {@link NodeVisitorInterface::beforeTraversal()} method of
+     * each visitor.
+     *
+     * @param Node[]                    $nodes   The traversed nodes
+     * @param ExecutionContextInterface $context The current execution context
+     * @param integer|null              $limit   Limits the number of visitors
+     *                                           on which beforeTraversal()
+     *                                           should be called. All visitors
+     *                                           will be called by default
+     */
+    private function afterTraversal($nodes, ExecutionContextInterface $context, $limit = null)
+    {
+        if (null === $limit) {
+            $limit = count($this->visitors);
+        }
+
+        $numberOfCalls = 0;
+
+        foreach ($this->visitors as $visitor) {
+            $visitor->afterTraversal($nodes, $context);
+
+            if (++$numberOfCalls === $limit) {
+                return;
+            }
         }
     }
 

src/Symfony/Component/Validator/NodeVisitor/AbstractVisitor.php

@@ -27,14 +27,14 @@ abstract class AbstractVisitor implements NodeVisitorInterface
     /**
      * {@inheritdoc}
      */
-    public function beforeTraversal(array $nodes, ExecutionContextInterface $context)
+    public function beforeTraversal($nodes, ExecutionContextInterface $context)
     {
     }
 
     /**
      * {@inheritdoc}
      */
-    public function afterTraversal(array $nodes, ExecutionContextInterface $context)
+    public function afterTraversal($nodes, ExecutionContextInterface $context)
     {
     }
 

src/Symfony/Component/Validator/NodeVisitor/NodeValidationVisitor.php

@@ -29,16 +29,6 @@ use Symfony\Component\Validator\NodeTraverser\NodeTraverserInterface;
  */
 class NodeValidationVisitor extends AbstractVisitor implements GroupManagerInterface
 {
-    /**
-     * Stores the hashes of each validated object together with the groups
-     * in which that object was already validated.
-     *
-     * @var array
-     */
-    private $validatedObjects = array();
-
-    private $validatedConstraints = array();
-
     /**
      * @var ConstraintValidatorFactoryInterface
      */
@@ -49,20 +39,37 @@ class NodeValidationVisitor extends AbstractVisitor implements GroupManagerInter
      */
     private $nodeTraverser;
 
+    /**
+     * The currently validated group.
+     *
+     * @var string
+     */
     private $currentGroup;
 
+    /**
+     * Creates a new visitor.
+     *
+     * @param NodeTraverserInterface              $nodeTraverser    The node traverser
+     * @param ConstraintValidatorFactoryInterface $validatorFactory The validator factory
+     */
     public function __construct(NodeTraverserInterface $nodeTraverser, ConstraintValidatorFactoryInterface $validatorFactory)
     {
         $this->validatorFactory = $validatorFactory;
         $this->nodeTraverser = $nodeTraverser;
     }
 
-    public function afterTraversal(array $nodes, ExecutionContextInterface $context)
+    /**
-    {
+     * Validates a node's value against the constraints defined in the node's
-        $this->validatedObjects = array();
+     * metadata.
-        $this->validatedConstraints = array();
+     *
-    }
+     * Objects and constraints that were validated before in the same context
-
+     * will be skipped.
+     *
+     * @param Node                      $node    The current node
+     * @param ExecutionContextInterface $context The execution context
+     *
+     * @return Boolean Whether to traverse the successor nodes
+     */
     public function visit(Node $node, ExecutionContextInterface $context)
     {
         if ($node instanceof CollectionNode) {
@@ -84,21 +91,24 @@ class NodeValidationVisitor extends AbstractVisitor implements GroupManagerInter
         // simply continue traversal (if possible)
 
         foreach ($node->groups as $key => $group) {
-            // Remember which object was validated for which group
+            // Even if we remove the following clause, the constraints on an
-            // Skip validation if the object was already validated for this
+            // object won't be validated again due to the measures taken in
-            // group
+            // validateNodeForGroup().
+            // The following shortcut, however, prevents validatedNodeForGroup()
+            // from being called at all and enhances performance a bit.
             if ($node instanceof ClassNode) {
                 // Use the object hash for group sequences
                 $groupHash = is_object($group) ? spl_object_hash($group) : $group;
 
                 if ($context->isObjectValidatedForGroup($objectHash, $groupHash)) {
-                    // Skip this group when validating properties
+                    // Skip this group when validating the successor nodes
+                    // (property and/or collection nodes)
                     unset($node->groups[$key]);
 
                     continue;
                 }
 
-                //$context->markObjectAsValidatedForGroup($objectHash, $groupHash);
+                $context->markObjectAsValidatedForGroup($objectHash, $groupHash);
             }
 
             // Validate normal group
@@ -108,27 +118,34 @@ class NodeValidationVisitor extends AbstractVisitor implements GroupManagerInter
                 continue;
             }
 
-            // Skip the group sequence when validating properties
-            unset($node->groups[$key]);
-
             // Traverse group sequence until a violation is generated
             $this->traverseGroupSequence($node, $group, $context);
 
-            // Optimization: If the groups only contain the group sequence,
+            // Skip the group sequence when validating successor nodes
-            // we can skip the traversal for the properties of the object
+            unset($node->groups[$key]);
-            if (1 === count($node->groups)) {
-                return false;
-            }
         }
 
         return true;
     }
 
+    /**
+     * {@inheritdoc}
+     */
     public function getCurrentGroup()
     {
         return $this->currentGroup;
     }
 
+    /**
+     * Validates a node's value in each group of a group sequence.
+     *
+     * If any of the groups' constraints generates a violation, subsequent
+     * groups are not validated anymore.
+     *
+     * @param Node                      $node          The validated node
+     * @param GroupSequence             $groupSequence The group sequence
+     * @param ExecutionContextInterface $context       The execution context
+     */
     private function traverseGroupSequence(Node $node, GroupSequence $groupSequence, ExecutionContextInterface $context)
     {
         $violationCount = count($context->getViolations());
@@ -150,6 +167,17 @@ class NodeValidationVisitor extends AbstractVisitor implements GroupManagerInter
         }
     }
 
+    /**
+     * Validates a node's value against all constraints in the given group.
+     *
+     * @param Node                      $node       The validated node
+     * @param string                    $group      The group to validate
+     * @param ExecutionContextInterface $context    The execution context
+     * @param string                    $objectHash The hash of the node's
+     *                                              object (if any)
+     *
+     * @throws \Exception
+     */
     private function validateNodeForGroup(Node $node, $group, ExecutionContextInterface $context, $objectHash)
     {
         try {
@@ -176,8 +204,6 @@ class NodeValidationVisitor extends AbstractVisitor implements GroupManagerInter
 
                         $context->markPropertyConstraintAsValidated($objectHash, $propertyName, $constraintHash);
                     }
-
-                    $this->validatedConstraints[$objectHash][$constraintHash] = true;
                 }
 
                 $validator = $this->validatorFactory->getInstance($constraint);
@@ -187,6 +213,7 @@ class NodeValidationVisitor extends AbstractVisitor implements GroupManagerInter
 
             $this->currentGroup = null;
         } catch (\Exception $e) {
+            // Should be put into a finally block once we switch to PHP 5.5
             $this->currentGroup = null;
 
             throw $e;

src/Symfony/Component/Validator/NodeVisitor/NodeVisitorInterface.php

@@ -29,9 +29,31 @@ use Symfony\Component\Validator\Node\Node;
  */
 interface NodeVisitorInterface
 {
-    public function beforeTraversal(array $nodes, ExecutionContextInterface $context);
+    /**
+     * Called at the beginning of a traversal.
+     *
+     * @param Node[]                    $nodes   A list of Node instances
+     * @param ExecutionContextInterface $context The execution context
+     *
+     * @return Boolean Whether to continue the traversal
+     */
+    public function beforeTraversal($nodes, ExecutionContextInterface $context);
 
-    public function afterTraversal(array $nodes, ExecutionContextInterface $context);
+    /**
+     * Called at the end of a traversal.
+     *
+     * @param Node[]                    $nodes   A list of Node instances
+     * @param ExecutionContextInterface $context The execution context
+     */
+    public function afterTraversal($nodes, ExecutionContextInterface $context);
 
+    /**
+     * Called for each node during a traversal.
+     *
+     * @param Node                      $node    The current node
+     * @param ExecutionContextInterface $context The execution context
+     *
+     * @return Boolean Whether to traverse the node's successor nodes
+     */
     public function visit(Node $node, ExecutionContextInterface $context);
 }

src/Symfony/Component/Validator/NodeVisitor/ObjectInitializationVisitor.php

@@ -12,12 +12,18 @@
 namespace Symfony\Component\Validator\NodeVisitor;
 
 use Symfony\Component\Validator\Context\ExecutionContextInterface;
+use Symfony\Component\Validator\Exception\InvalidArgumentException;
 use Symfony\Component\Validator\Node\ClassNode;
 use Symfony\Component\Validator\Node\Node;
 use Symfony\Component\Validator\ObjectInitializerInterface;
 
 /**
- * @since  %%NextVersion%%
+ * Initializes the objects of all class nodes.
+ *
+ * You have to pass at least one instance of {@link ObjectInitializerInterface}
+ * to the constructor of this visitor.
+ *
+ * @since  2.5
  * @author Bernhard Schussek <bschussek@gmail.com>
  */
 class ObjectInitializationVisitor extends AbstractVisitor
@@ -27,30 +33,51 @@ class ObjectInitializationVisitor extends AbstractVisitor
      */
     private $initializers;
 
+    /**
+     * Creates a new visitor.
+     *
+     * @param ObjectInitializerInterface[] $initializers The object initializers
+     *
+     * @throws InvalidArgumentException
+     */
     public function __construct(array $initializers)
     {
         foreach ($initializers as $initializer) {
             if (!$initializer instanceof ObjectInitializerInterface) {
-                throw new \InvalidArgumentException('Validator initializers must implement ObjectInitializerInterface.');
+                throw new InvalidArgumentException(sprintf(
+                    'Validator initializers must implement '.
+                    '"Symfony\Component\Validator\ObjectInitializerInterface". '.
+                    'Got: "%s"',
+                    is_object($initializer) ? get_class($initializer) : gettype($initializer)
+                ));
             }
         }
 
         // If no initializer is present, this visitor should not even be created
         if (0 === count($initializers)) {
-            throw new \InvalidArgumentException('Please pass at least one initializer.');
+            throw new InvalidArgumentException('Please pass at least one initializer.');
         }
 
         $this->initializers = $initializers;
     }
 
+    /**
+     * Calls the {@link ObjectInitializerInterface::initialize()} method for
+     * the object of each class node.
+     *
+     * @param Node                      $node    The current node
+     * @param ExecutionContextInterface $context The execution context
+     *
+     * @return Boolean Always returns true
+     */
     public function visit(Node $node, ExecutionContextInterface $context)
     {
-        if (!$node instanceof ClassNode) {
+        if ($node instanceof ClassNode) {
-            return;
+            foreach ($this->initializers as $initializer) {
+                $initializer->initialize($node->value);
+            }
         }
 
-        foreach ($this->initializers as $initializer) {
+        return true;
-            $initializer->initialize($node->value);
-        }
     }
 }

src/Symfony/Component/Validator/Tests/NodeTraverser/NonRecursiveNodeTraverserTest.php

@@ -0,0 +1,91 @@
+<?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\Tests\NodeTraverser;
+
+use Symfony\Component\Validator\Mapping\GenericMetadata;
+use Symfony\Component\Validator\Node\GenericNode;
+use Symfony\Component\Validator\NodeTraverser\NonRecursiveNodeTraverser;
+use Symfony\Component\Validator\Tests\Fixtures\FakeMetadataFactory;
+
+/**
+ * @since  2.5
+ * @author Bernhard Schussek <bschussek@gmail.com>
+ */
+class NonRecursiveNodeTraverserTest extends \PHPUnit_Framework_TestCase
+{
+    /**
+     * @var FakeMetadataFactory
+     */
+    private $metadataFactory;
+
+    /**
+     * @var NonRecursiveNodeTraverser
+     */
+    private $traverser;
+
+    protected function setUp()
+    {
+        $this->metadataFactory = new FakeMetadataFactory();
+        $this->traverser = new NonRecursiveNodeTraverser($this->metadataFactory);
+    }
+
+    public function testVisitorsMayPreventTraversal()
+    {
+        $nodes = array(new GenericNode('value', new GenericMetadata(), '', array('Default')));
+        $context = $this->getMock('Symfony\Component\Validator\Context\ExecutionContextInterface');
+
+        $visitor1 = $this->getMock('Symfony\Component\Validator\NodeVisitor\NodeVisitorInterface');
+        $visitor2 = $this->getMock('Symfony\Component\Validator\NodeVisitor\NodeVisitorInterface');
+        $visitor3 = $this->getMock('Symfony\Component\Validator\NodeVisitor\NodeVisitorInterface');
+
+        $visitor1->expects($this->once())
+            ->method('beforeTraversal')
+            ->with($nodes, $context);
+
+        // abort traversal
+        $visitor2->expects($this->once())
+            ->method('beforeTraversal')
+            ->with($nodes, $context)
+            ->will($this->returnValue(false));
+
+        // never called
+        $visitor3->expects($this->never())
+            ->method('beforeTraversal');
+
+        $visitor1->expects($this->never())
+            ->method('visit');
+        $visitor2->expects($this->never())
+            ->method('visit');
+        $visitor2->expects($this->never())
+            ->method('visit');
+
+        // called in order to clean up
+        $visitor1->expects($this->once())
+            ->method('afterTraversal')
+            ->with($nodes, $context);
+
+        // abort traversal
+        $visitor2->expects($this->once())
+            ->method('afterTraversal')
+            ->with($nodes, $context);
+
+        // never called, because beforeTraversal() wasn't called either
+        $visitor3->expects($this->never())
+            ->method('afterTraversal');
+
+        $this->traverser->addVisitor($visitor1);
+        $this->traverser->addVisitor($visitor2);
+        $this->traverser->addVisitor($visitor3);
+
+        $this->traverser->traverse($nodes, $context);
+    }
+}