[Validator] Improved performance of cache key generation

symfony · fabpot · Mar 31, 2014 · Feb 12, 2014 · Feb 17, 2014 · Feb 17, 2014
commit 3183aed7cdd669ee6ad6fc4715e820dad1ae46c6
diff --git a/src/Symfony/Component/Validator/Context/ExecutionContext.php b/src/Symfony/Component/Validator/Context/ExecutionContext.php
@@ -104,7 +104,7 @@ class ExecutionContext implements ExecutionContextInterface
    <
8000
span class='blob-code-inner blob-code-marker ' data-code-marker=" ">     *
      * @var array
      */
-    private $validatedClassConstraints = array();
+    private $validatedConstraints = array();
 
     /**
      * Stores which property constraint has been validated for which property.
@@ -319,64 +319,36 @@ public function getMetadataFactory()
     /**
      * {@inheritdoc}
      */
-    public function markObjectAsValidatedForGroup($objectHash, $groupHash)
+    public function markGroupAsValidated($cacheKey, $groupHash)
     {
-        if (!isset($this->validatedObjects[$objectHash])) {
-            $this->validatedObjects[$objectHash] = array();
+        if (!isset($this->validatedObjects[$cacheKey])) {
+            $this->validatedObjects[$cacheKey] = array();
         }
 
-        $this->validatedObjects[$objectHash][$groupHash] = true;
+        $this->validatedObjects[$cacheKey][$groupHash] = true;
     }
 
     /**
      * {@inheritdoc}
      */
-    public function isObjectValidatedForGroup($objectHash, $groupHash)
+    public function isGroupValidated($cacheKey, $groupHash)
     {
-        return isset($this->validatedObjects[$objectHash][$groupHash]);
+        return isset($this->validatedObjects[$cacheKey][$groupHash]);
     }
 
     /**
      * {@inheritdoc}
      */
-    public function markClassConstraintAsValidated($objectHash, $constraintHash)
+    public function markConstraintAsValidated($cacheKey, $constraintHash)
     {
-        if (!isset($this->validatedClassConstraints[$objectHash])) {
-            $this->validatedClassConstraints[$objectHash] = array();
-        }
-
-        $this->validatedClassConstraints[$objectHash][$constraintHash] = true;
-    }
-
-    /**
-     * {@inheritdoc}
-     */
-    public function isClassConstraintValidated($objectHash, $constraintHash)
-    {
-        return isset($this->validatedClassConstraints[$objectHash][$constraintHash]);
-    }
-
-    /**
-     * {@inheritdoc}
-     */
-    public function markPropertyConstraintAsValidated($objectHash, $propertyName, $constraintHash)
-    {
-        if (!isset($this->validatedPropertyConstraints[$objectHash])) {
-            $this->validatedPropertyConstraints[$objectHash] = array();
-        }
-
-        if (!isset($this->validatedPropertyConstraints[$objectHash][$propertyName])) {
-            $this->validatedPropertyConstraints[$objectHash][$propertyName] = array();
-        }
-
-        $this->validatedPropertyConstraints[$objectHash][$propertyName][$constraintHash] = true;
+        $this->validatedConstraints[$cacheKey.':'.$constraintHash] = true;
     }
 
     /**
      * {@inheritdoc}
      */
-    public function isPropertyConstraintValidated($objectHash, $propertyName, $constraintHash)
+    public function isConstraintValidated($cacheKey, $constraintHash)
     {
-        return isset($this->validatedPropertyConstraints[$objectHash][$propertyName][$constraintHash]);
+        return isset($this->validatedConstraints[$cacheKey.':'.$constraintHash]);
     }
 }
diff --git a/src/Symfony/Component/Validator/Context/ExecutionContextInterface.php b/src/Symfony/Component/Validator/Context/ExecutionContextInterface.php
@@ -124,19 +124,19 @@ public function setGroup($group);
     /**
      * Marks an object as validated in a specific validation group.
      *
-     * @param string $objectHash The hash of the object
+     * @param string $cacheKey The hash of the object
      * @param string $groupHash  The group's name or hash, if it is group
      *                           sequence
      *
      * @internal Used by the validator engine. Should not be called by user
      *           code.
      */
-    public function markObjectAsValidatedForGroup($objectHash, $groupHash);
+    public function markGroupAsValidated($cacheKey, $groupHash);
 
     /**
      * Returns whether an object was validated in a specific validation group.
      *
-     * @param string $objectHash The hash of the object
+     * @param string $cacheKey The hash of the object
      * @param string $groupHash  The group's name or hash, if it is group
      *                           sequence
      *
@@ -146,56 +146,29 @@ public function markObjectAsValidatedForGroup($objectHash, $groupHash);
      * @internal Used by the validator engine. Should not be called by user
      *           code.
      */
-    public function isObjectValidatedForGroup($objectHash, $groupHash);
+    public function isGroupValidated($cacheKey, $groupHash);
 
     /**
      * Marks a constraint as validated for an object.
      *
+     * @param string $cacheKey     The hash of the object
      * @param string $constraintHash The hash of the constraint
      *
      * @internal Used by the validator engine. Should not be called by user
      *           code.
      */
-    public function markClassConstraintAsValidated($objectHash, $constraintHash);
+    public function markConstraintAsValidated($cacheKey, $constraintHash);
 
     /**
      * Returns whether a constraint was validated for an object.
      *
-     * @param string $objectHash     The hash of the object
+     * @param string $cacheKey     The hash of the object
      * @param string $constraintHash The hash of the constraint
      *
      * @return Boolean Whether the constraint was already validated
      *
      * @internal Used by the validator engine. Should not be called by user
      *           code.
      */
-    public function isClassConstraintValidated($objectHash, $constraintHash);
-
-    /**
-     * Marks a constraint as validated for an object and a property name.
-     *
-     * @param string $objectHash     The hash of the object
-     * @param string $propertyName   The property name
-     * @param string $constraintHash The hash of the constraint
-     *
-     * @internal Used by the validator engine. Should not be called by user
-     *           code.
-     */
-    public function markPropertyConstraintAsValidated($objectHash, $propertyName, $constraintHash);
-
-    /**
-     * Returns whether a constraint was validated for an object and a property
-     * name.
-     *
-     * @param string $objectHash     The hash of the object
-     * @param string $propertyName   The property name
-     * @param string $constraintHash The hash of the constraint
-     *
-     * @return Boolean Whether the constraint was already validated
-     *
-     * @internal Used by the validator engine. Should not be called by user
-     *           code.
-     */
-    public function isPropertyConstraintValidated($objectHash, $propertyName, $constraintHash);
+    public function isConstraintValidated($cacheKey, $constraintHash);
 }
diff --git a/src/Symfony/Component/Validator/Node/ClassNode.php b/src/Symfony/Component/Validator/Node/ClassNode.php
@@ -55,20 +55,20 @@ class ClassNode extends Node
      *
      * @see \Symfony\Component\Validator\Mapping\TraversalStrategy
      */
-    public function __construct($object, ClassMetadataInterface $metadata, $propertyPath, array $groups, $cascadedGroups = null, $traversalStrategy = TraversalStrategy::IMPLICIT)
+    public function __construct($object, $cacheKey, ClassMetadataInterface $metadata, $propertyPath, array $groups, $cascadedGroups = null, $traversalStrategy = TraversalStrategy::IMPLICIT)
     {
         if (!is_object($object)) {
             throw new UnexpectedTypeException($object, 'object');
         }
 
         parent::__construct(
             $object,
+            $cacheKey,
             $metadata,
             $propertyPath,
             $groups,
-            $cascadedGroups
+            $cascadedGroups,
+            $traversalStrategy
         );
-
-        $this->traversalStrategy = $traversalStrategy;
     }
 }
diff --git a/src/Symfony/Component/Validator/Node/CollectionNode.php b/src/Symfony/Component/Validator/Node/CollectionNode.php
@@ -56,6 +56,7 @@ public function __construct($collection, $propertyPath, array $groups, $cascaded
         parent::__construct(
             $collection,
             null,
+            null,
             $propertyPath,
             $groups,
             $cascadedGroups,

diff --git a/src/Symfony/Component/Validator/Node/Node.php b/src/Symfony/Component/Validator/Node/Node.php
@@ -30,6 +30,8 @@ abstract class Node
      */
     public $value;
 
+    public $cacheKey;
+
     /**
      * The metadata specifying how the value should be validated.
      *
@@ -82,13 +84,14 @@ abstract class Node
      *
      * @throws UnexpectedTypeException If $cascadedGroups is invalid
      */
-    public function __construct($value, MetadataInterface $metadata = null, $propertyPath, array $groups, $cascadedGroups = null, $traversalStrategy = TraversalStrategy::IMPLICIT)
+    public function __construct($value, $cacheKey, MetadataInterface $metadata = null, $propertyPath, array $groups, $cascadedGroups = null, $traversalStrategy = TraversalStrategy::IMPLICIT)
     {
         if (null !== $cascadedGroups && !is_array($cascadedGroups)) {
             throw new UnexpectedTypeException($cascadedGroups, 'null or array');
         }
 
         $this->value = $value;
+        $this->cacheKey = $cacheKey;
         $this->metadata = $metadata;
         $this->propertyPath = $propertyPath;
         $this->groups = $groups;

diff --git a/src/Symfony/Component/Validator/Node/PropertyNode.php b/src/Symfony/Component/Validator/Node/PropertyNode.php
@@ -41,11 +41,6 @@
  */
 class PropertyNode extends Node
 {
-    /**
-     * @var object
-     */
-    public $object;
-
     /**
      * @var PropertyMetadataInterface
      */
@@ -71,22 +66,17 @@ class PropertyNode extends Node
      *
      * @see \Symfony\Component\Validator\Mapping\TraversalStrategy
      */
-    public function __construct($object, $value, PropertyMetadataInterface $metadata, $propertyPath, array $groups, $cascadedGroups = null, $traversalStrategy = TraversalStrategy::IMPLICIT)
+    public function __construct($value, $cacheKey, PropertyMetadataInterface $metadata, $propertyPath, array $groups, $cascadedGroups = null, $traversalStrategy = TraversalStrategy::IMPLICIT)
     {
-        if (!is_object($object)) {
-            throw new UnexpectedTypeException($object, 'object');
-        }
-
         parent::__construct(
             $value,
+            $cacheKey,
             $metadata,
             $propertyPath,
             $groups,
             $cascadedGroups,
             $traversalStrategy
         );
-
-        $this->object = $object;
     }
 
 }
diff --git a/src/Symfony/Component/Validator/NodeTraverser/NonRecursiveNodeTraverser.php b/src/Symfony/Component/Validator/NodeTraverser/NonRecursiveNodeTraverser.php
@@ -274,8 +274,8 @@ private function traverseClassNode(ClassNode $node, ExecutionContextInterface $c
                 }
 
                 $nodeStack->push(new PropertyNode(
-                    $node->value,
                     $propertyMetadata->getPropertyValue($node->value),
+                    $node->cacheKey.':'.$propertyName,
                     $propertyMetadata,
                     $node->propertyPath
                         ? $node->propertyPath.'.'.$propertyName
@@ -530,6 +530,7 @@ private function cascadeObject($object, $propertyPath, array $groups, $traversal
 
             $nodeStack->push(new ClassNode(
                 $object,
+                spl_object_hash($object),
                 $classMetadata,
                 $propertyPath,
                 $groups,

diff --git a/src/Symfony/Component/Validator/NodeVisitor/NodeValidationVisitor.php b/src/Symfony/Component/Validator/NodeVisitor/NodeValidationVisitor.php
@@ -71,14 +71,6 @@ public function visit(Node $node, ExecutionContextInterface $context)
 
         $context->setNode($node->value, $node->metadata, $node->propertyPath);
 
-        if ($node instanceof ClassNode) {
-            $objectHash = spl_object_hash($node->value);
-        } elseif ($node instanceof PropertyNode) {
-            $objectHash = spl_object_hash($node->object);
-        } else {
-            $objectHash = null;
-        }
-
         // if group (=[<G1,G2>,G3,G4]) contains group sequence (=<G1,G2>)
         // then call traverse() with each entry of the group sequence and abort
         // if necessary (G1, G2)
@@ -97,15 +89,15 @@ public function visit(Node $node, ExecutionContextInterface $context)
                 // 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 the successor nodes
                     // (property and/or collection nodes)
                     unset($node->groups[$key]);
 
                     continue;
                 }
 
-                $context->markObjectAsValidatedForGroup($objectHash, $groupHash);
                $context->markGroupAsValidated($node->cacheKey, $groupHash);
 
                 // Replace the "Default" group by the group sequence defined
                 // for the class, if applicable
@@ -144,7 +136,7 @@ public function visit(Node $node, ExecutionContextInterface $context)
             }
 
             // Validate normal group
-            $this->validateNodeForGroup($node, $group, $context, $objectHash);
+            $this->validateInGroup($node, $group, $context);
         }
 
         return true;
@@ -190,31 +182,21 @@ private function traverseGroupSequence(Node $node, GroupSequence $groupSequence,
      *
      * @throws \Exception
      */
-    private function validateNodeForGroup(Node $node, $group, ExecutionContextInterface $context, $objectHash)
+    private function validateInGroup(Node $node, $group, ExecutionContextInterface $context)
     {
         $context->setGroup($group);
 
         foreach ($node->metadata->findConstraints($group) as $constraint) {
             // Prevent duplicate validation of constraints, in the case
             // that constraints belong to multiple validated groups
-            if (null !== $objectHash) {
+            if (null !== $node->cacheKey) {
                 $constraintHash = spl_object_hash($constraint);
 
-                if ($node instanceof ClassNode) {
-                    if ($context->isClassConstraintValidated($objectHash, $constraintHash)) {
-                        continue;
-                    }
-
-                    $context->markClassConstraintAsValidated($objectHash, $constraintHash);
-                } elseif ($node instanceof PropertyNode) {
-                    $propertyName = $node->metadata->getPropertyName();
-
-                    if ($context->isPropertyConstraintValidated($objectHash, $propertyName, $constraintHash)) {
-                        continue;
-                    }
-
-                    $context->markPropertyConstraintAsValidated($objectHash, $propertyName, $constraintHash);
+                if ($context->isConstraintValidated($node->cacheKey, $constraintHash)) {
+                    continue;
                 }
+
+                $context->markConstraintAsValidated($node->cacheKey, $constraintHash);
             }
 
             $validator = $this->validatorFactory->getInstance($constraint);

diff --git a/src/Symfony/Component/Validator/Tests/Node/ClassNodeTest.php b/src/Symfony/Component/Validator/Tests/Node/ClassNodeTest.php
@@ -26,6 +26,6 @@ public function testConstructorExpectsObject()
     {
         $metadata = $this->getMock('Symfony\Component\Validator\Mapping\ClassMetadataInterface');
 
-        new ClassNode('foobar', $metadata, '', array(), array());
+        new ClassNode('foobar', null, $metadata, '', array(), array());
     }
 }
diff --git a/src/Symfony/Component/Validator/Tests/NodeTraverser/NonRecursiveNodeTraverserTest.php b/src/Symfony/Component/Validator/Tests/NodeTraverser/NonRecursiveNodeTraverserTest.php
@@ -40,7 +40,7 @@ protected function setUp()
 
     public function testVisitorsMayPreventTraversal()
     {
-        $nodes = array(new GenericNode('value', new GenericMetadata(), '', array('Default')));
+        $nodes = array(new GenericNode('value', null, new GenericMetadata(), '', array('Default')));
         $context = $this->getMock('Symfony\Component\Validator\Context\ExecutionContextInterface');
 
         $visitor1 = $this->getMock('Symfony\Component\Validator\NodeVisitor\NodeVisitorInterface');