[Validator] Fix hack for nested Collection/All losing context

src/Symfony/Component/Validator/ConstraintValidatorFactory.php

@@ -45,16 +45,7 @@ public function getInstance(Constraint $constraint)
     {
         $className = $constraint->validatedBy();
 
-        // The second condition is a hack that is needed when CollectionValidator
-        // calls itself recursively (Collection constraints can be nested).
-        // Since the context of the validator is overwritten when initialize()
-        // is called for the nested constraint, the outer validator is
-        // acting on the wrong context when the nested validation terminates.
-        //
-        // A better solution - which should be approached in Symfony 3.0 - is to
-        // remove the initialize() method and pass the context as last argument
-        // to validate() instead.
-        if (!isset($this->validators[$className]) || 'Symfony\Component\Validator\Constraints\CollectionValidator' === $className) {
+        if (!isset($this->validators[$className])) {
             $this->validators[$className] = 'validator.expression' === $className
                 ? new ExpressionValidator($this->propertyAccessor)
                 : new $className();

src/Symfony/Component/Validator/Constraints/AllValidator.php

@@ -35,11 +35,12 @@ public function validate($value, Constraint $constraint)
             throw new UnexpectedTypeException($value, 'array or Traversable');
         }
 
-        $group = $this->context->getGroup();
+        $context = $this->context;
+        $group = $context->getGroup();
 
         foreach ($value as $key => $element) {
             foreach ($constraint->constraints as $constr) {
-                $this->context->validateValue($element, $constr, '['.$key.']', $group);
+                $context->validateValue($element, $constr, '['.$key.']', $group);
             }
         }
     }

src/Symfony/Component/Validator/Constraints/CollectionValidator.php

@@ -35,7 +35,17 @@ public function validate($value, Constraint $constraint)
             throw new UnexpectedTypeException($value, 'array or Traversable and ArrayAccess');
         }
 
-        $group = $this->context->getGroup();
+        // We need to keep the initialized context when CollectionValidator
+        // calls itself recursively (Collection constraints can be nested).
+        // Since the context of the validator is overwritten when initialize()
+        // is called for the nested constraint, the outer validator is
+        // acting on the wrong context when the nested validation terminates.
+        //
+        // A better solution - which should be approached in Symfony 3.0 - is to
+        // remove the initialize() method and pass the context as last argument
+        // to validate() instead.
+        $context = $this->context;
+        $group = $context->getGroup();
 
         foreach ($constraint->fields as $field => $fieldConstraint) {
             if (
@@ -44,10 +54,10 @@ public function validate($value, Constraint $constraint)
                 ($value instanceof \ArrayAccess && $value->offsetExists($field))
             ) {
                 foreach ($fieldConstraint->constraints as $constr) {
-                    $this->context->validateValue($value[$field], $constr, '['.$field.']', $group);
+                    $context->validateValue($value[$field], $constr, '['.$field.']', $group);
                 }
             } elseif (!$fieldConstraint instanceof Optional && !$constraint->allowMissingFields) {
-                $this->context->addViolationAt('['.$field.']', $constraint->missingFieldsMessage, array(
+                $context->addViolationAt('['.$field.']', $constraint->missingFieldsMessage, array(
                     '{{ field }}' => $field
                 ), null);
             }
@@ -56,7 +66,7 @@ public function validate($value, Constraint $constraint)
         if (!$constraint->allowExtraFields) {
             foreach ($value as $field => $fieldValue) {
                 if (!isset($constraint->fields[$field])) {
-                    $this->context->addViolationAt('['.$field.']', $constraint->extraFieldsMessage, array(
+                    $context->addViolationAt('['.$field.']', $constraint->extraFieldsMessage, array(
                         '{{ field }}' => $field
                     ), $fieldValue);
                 }

src/Symfony/Component/Validator/Tests/ExecutionContextTest.php

@@ -11,13 +11,14 @@
 
 namespace Symfony\Component\Validator\Tests;
 
+use Symfony\Component\Validator\Constraints\Collection;
+use Symfony\Component\Validator\Constraints\All;
+use Symfony\Component\Validator\ConstraintValidatorFactory;
 use Symfony\Component\Validator\ConstraintViolation;
 use Symfony\Component\Validator\ConstraintViolationList;
 use Symfony\Component\Validator\ExecutionContext;
-use Symfony\Component\Validator\Constraints\Collection;
 use Symfony\Component\Validator\Tests\Fixtures\ConstraintA;
 use Symfony\Component\Validator\ValidationVisitor;
-use Symfony\Component\Validator\ConstraintValidatorFactory;
 
 class ExecutionContextTest extends \PHPUnit_Framework_TestCase
 {
@@ -277,22 +278,50 @@ public function testGetPropertyPathWithEmptyCurrentPropertyPath()
         $this->assertEquals('bam.baz', $this->context->getPropertyPath('bam.baz'));
     }
 
-    public function testGetPropertyPathWithNestedCollectionsMixed()
+    public function testGetPropertyPathWithNestedCollectionsAndAllMixed()
     {
         $constraints = new Collection(array(
-            'foo' => new Collection(array(
-                'foo' => new ConstraintA(),
-                'bar' => new ConstraintA(),
-             )),
+            'shelves' => new All(array('constraints' => array(
+                new Collection(array(
+                    'name'  => new ConstraintA(),
+                    'books' => new All(array('constraints' => array(
+                        new ConstraintA()
+                    )))
+                ))
+            ))),
             'name' => new ConstraintA()
         ));
+        $data = array(
+            'shelves' => array(
+                array(
+                    'name' => 'Research',
+                    'books' => array('foo', 'bar'),
+                ),
+                array(
+                    'name' => 'VALID',
+                    'books' => array('foozy', 'VALID', 'bazzy'),
+                ),
+            ),
+            'name' => 'Library',
+        );
+        $expectedViolationPaths = array(
+            '[shelves][0][name]',
+            '[shelves][0][books][0]',
+            '[shelves][0][books][1]',
+            '[shelves][1][books][0]',
+            '[shelves][1][books][2]',
+            '[name]'
+        );
 
         $visitor = new ValidationVisitor('Root', $this->metadataFactory, new ConstraintValidatorFactory(), $this->translator);
         $context = new ExecutionContext($visitor, $this->translator, self::TRANS_DOMAIN);
-        $context->validateValue(array('foo' => array('foo' => 'VALID')), $constraints);
-        $violations = $context->getViolations();
+        $context->validateValue($data, $constraints);
+
+        foreach ($context->getViolations() as $violation) {
+            $violationPaths[] = $violation->getPropertyPath();
+        }
 
-        $this->assertEquals('[name]', $violations[1]->getPropertyPath());
+        $this->assertEquals($expectedViolationPaths, $violationPaths);
     }
 }