[Validator] Improved error messages displayed when the Valid constrai…

…nt is misused

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

@@ -12,6 +12,7 @@
 namespace Symfony\Component\Validator\Constraints;
 
 use Symfony\Component\Validator\Constraint;
+use Symfony\Component\Validator\Exception\ConstraintDefinitionException;
 
 /**
  * @Annotation
@@ -22,6 +23,28 @@ class All extends Constraint
 {
  public $constraints = array();
 
+ /**
+ * {@inheritDoc}
+ */
+ public function __construct($options = null)
+ {
+ parent::__construct($options);
+
+ if (!is_array($this->constraints)) {
+ $this->constraints = array($this->constraints);
+ }
+
+ foreach ($this->constraints as $constraint) {
+ if (!$constraint instanceof Constraint) {
+ throw new ConstraintDefinitionException('The value ' . $constraint . ' is not an instance of Constraint in constraint ' . __CLASS__);
+ }
+
+ if ($constraint instanceof Valid) {
+ throw new ConstraintDefinitionException('The constraint Valid cannot be nested inside constraint ' . __CLASS__ . '. You can only declare the Valid constraint directly on a field or method.');
+ }
+ }
+ }
+
  public function getDefaultOption()
  {
  return 'constraints';

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

@@ -44,12 +44,8 @@ public function validate($value, Constraint $constraint)
  $group = $this->context->getGroup();
  $propertyPath = $this->context->getPropertyPath();
 
- // cannot simply cast to array, because then the object is converted to an
- // array instead of wrapped inside
- $constraints = is_array($constraint->constraints) ? $constraint->constraints : array($constraint->constraints);
-
  foreach ($value as $key => $element) {
- foreach ($constraints as $constr) {
+ foreach ($constraint->constraints as $constr) {
  $walker->walkConstraint($constr, $element, $group, $propertyPath.'['.$key.']');
  }
  }

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

@@ -12,6 +12,9 @@
 namespace Symfony\Component\Validator\Constraints;
 
 use Symfony\Component\Validator\Constraint;
+use Symfony\Component\Validator\Constraints\Collection\Required;
+use Symfony\Component\Validator\Constraints\Collection\Optional;
+use Symfony\Component\Validator\Exception\ConstraintDefinitionException;
 
 /**
  * @Annotation
@@ -38,6 +41,30 @@ public function __construct($options = null)
  }
 
  parent::__construct($options);
+
+ if (!is_array($this->fields)) {
+ throw new ConstraintDefinitionException('The option "fields" is expected to be an array in constraint ' . __CLASS__);
+ }
+
+ foreach ($this->fields as $fieldName => $field) {
+ if (!$field instanceof Optional && !$field instanceof Required) {
+ $this->fields[$fieldName] = $field = new Required($field);
+ }
+
+ if (!is_array($field->constraints)) {
+ $field->constraints = array($field->constraints);
+ }
+
+ foreach ($field->constraints as $constraint) {
+ if (!$constraint instanceof Constraint) {
+ throw new ConstraintDefinitionException('The value ' . $constraint . ' of the field ' . $fieldName . ' is not an instance of Constraint in constraint ' . __CLASS__);
+ }
+
+ if ($constraint instanceof Valid) {
+ throw new ConstraintDefinitionException('The constraint Valid cannot be nested inside constraint ' . __CLASS__ . '. You can only declare the Valid constraint directly on a field or method.');
+ }
+ }
+ }
  }
 
  public function getRequiredOptions()

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

@@ -52,17 +52,7 @@ public function validate($value, Constraint $constraint)
  (is_array($value) && array_key_exists($field, $value)) ||
  ($value instanceof \ArrayAccess && $value->offsetExists($field))
  ) {
- if ($fieldConstraint instanceof Required || $fieldConstraint instanceof Optional) {
- $constraints = $fieldConstraint->constraints;
- } else {
- $constraints = $fieldConstraint;
- }
-
- // cannot simply cast to array, because then the object is converted to an
- // array instead of wrapped inside
- $constraints = is_array($constraints) ? $constraints : array($constraints);
-
- foreach ($constraints as $constr) {
+ foreach ($fieldConstraint->constraints as $constr) {
  $walker->walkConstraint($constr, $value[$field], $group, $propertyPath.'['.$field.']');
  }
  } elseif (!$fieldConstraint instanceof Optional && !$constraint->allowMissingFields) {

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

@@ -12,6 +12,7 @@
 namespace Symfony\Component\Validator\Constraints;
 
 use Symfony\Component\Validator\Constraint;
+use Symfony\Component\Validator\Exception\ConstraintDefinitionException;
 
 /**
  * @Annotation
@@ -23,4 +24,13 @@ class Valid extends Constraint
  public $traverse = true;
 
  public $deep = false;
+
+ public function __construct($options = null)
+ {
+ if (is_array($options) && array_key_exists('groups', $options)) {
+ throw new ConstraintDefinitionException('The option "groups" is not supported by the constraint ' . __CLASS__);
+ }
+
+ parent::__construct($options);
+ }
 }

src/Symfony/Component/Validator/Tests/Constraints/AllTest.php

@@ -0,0 +1,41 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <[email protected]>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\Allator\Tests\Constraints;
+
+use Symfony\Component\Validator\Constraints\All;
+use Symfony\Component\Validator\Constraints\Valid;
+
+/**
+ * @author Bernhard Schussek <[email protected]>
+ */
+class AllTest extends \PHPUnit_Framework_TestCase
+{
+ /**
+ * @expectedException Symfony\Component\Validator\Exception\ConstraintDefinitionException
+ */
+ public function testRejectNonConstraints()
+ {
+ new All(array(
+ 'foo',
+ ));
+ }
+
+ /**
+ * @expectedException Symfony\Component\Validator\Exception\ConstraintDefinitionException
+ */
+ public function testRejectValidConstraint()
+ {
+ new All(array(
+ new Valid(),
+ ));
+ }
+}

src/Symfony/Component/Validator/Tests/Constraints/CollectionTest.php

@@ -0,0 +1,73 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <[email protected]>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\Collectionator\Tests\Constraints;
+
+use Symfony\Component\Validator\Constraints\Collection;
+use Symfony\Component\Validator\Constraints\Collection\Required;
+use Symfony\Component\Validator\Constraints\Collection\Optional;
+use Symfony\Component\Validator\Constraints\Valid;
+
+/**
+ * @author Bernhard Schussek <[email protected]>
+ */
+class CollectionTest extends \PHPUnit_Framework_TestCase
+{
+ /**
+ * @expectedException Symfony\Component\Validator\Exception\ConstraintDefinitionException
+ */
+ public function testRejectInvalidFieldsOption()
+ {
+ new Collection(array(
+ 'fields' => 'foo',
+ ));
+ }
+
+ /**
+ * @expectedException Symfony\Component\Validator\Exception\ConstraintDefinitionException
+ */
+ public function testRejectNonConstraints()
+ {
+ new Collection(array(
+ 'foo' => 'bar',
+ ));
+ }
+
+ /**
+ * @expectedException Symfony\Component\Validator\Exception\ConstraintDefinitionException
+ */
+ public function testRejectValidConstraint()
+ {
+ new Collection(array(
+ 'foo' => new Valid(),
+ ));
+ }
+
+ /**
+ * @expectedException Symfony\Component\Validator\Exception\ConstraintDefinitionException
+ */
+ public function testRejectValidConstraintWithinOptional()
+ {
+ new Collection(array(
+ 'foo' => new Optional(new Valid()),
+ ));
+ }
+
+ /**
+ * @expectedException Symfony\Component\Validator\Exception\ConstraintDefinitionException
+ */
+ public function testRejectValidConstraintWithinRequired()
+ {
+ new Collection(array(
+ 'foo' => new Required(new Valid()),
+ ));
+ }
+}

src/Symfony/Component/Validator/Tests/Constraints/ValidTest.php

@@ -0,0 +1,28 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <[email protected]>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\Validator\Tests\Constraints;
+
+use Symfony\Component\Validator\Constraints\Valid;
+
+/**
+ * @author Bernhard Schussek <[email protected]>
+ */
+class ValidTest extends \PHPUnit_Framework_TestCase
+{
+ /**
+ * @expectedException Symfony\Component\Validator\Exception\ConstraintDefinitionException
+ */
+ public function testRejectGroupsOption()
+ {
+ new Valid(array('groups' => 'foo'));
+ }
+}

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

@@ -19,6 +19,7 @@
 use Symfony\Component\Validator\ConstraintViolation;
 use Symfony\Component\Validator\ConstraintViolationList;
 use Symfony\Component\Validator\ConstraintValidatorFactory;
+use Symfony\Component\Validator\Constraints\Valid;
 use Symfony\Component\Validator\Mapping\ClassMetadata;
 
 class ValidatorTest extends \PHPUnit_Framework_TestCase
@@ -200,6 +201,18 @@ public function testValidateValue()
  $this->assertEquals($violations, $this->validator->validateValue('Bernhard', new FailingConstraint()));
  }
 
+ /**
+ * @expectedException Symfony\Component\Validator\Exception\ValidatorException
+ */
+ public function testValidateValueRejectsValid()
+ {
+ $entity = new Entity();
+ $metadata = new ClassMetadata(get_class($entity));
+ $this->factory->addClassMetadata($metadata);
+
+ $this->validator->validateValue($entity, new Valid());
+ }
+
  public function testGetMetadataFactory()
  {
  $this->assertInstanceOf(

src/Symfony/Component/Validator/Validator.php

@@ -11,7 +11,9 @@
 
 namespace Symfony\Component\Validator;
 
+use Symfony\Component\Validator\Constraints\Valid;
 use Symfony\Component\Validator\Mapping\ClassMetadataFactoryInterface;
+use Symfony\Component\Validator\Exception\ValidatorException;
 
 /**
  * The default implementation of the ValidatorInterface.
@@ -104,6 +106,22 @@ public function validatePropertyValue($class, $property, $value, $groups = null)
  */
  public function validateValue($value, Constraint $constraint, $groups = null)
  {
+ if ($constraint instanceof Valid) {
+ // Why can't the Valid constraint be executed directly?
+ //
+ // It cannot be executed like regular other constraints, because regular
+ // constraints are only executed *if they belong to the validated group*.
+ // The Valid constraint, on the other hand, is always executed and propagates
+ // the group to the cascaded object. The propagated group depends on
+ //
+ // * Whether a group sequence is currently being executed. Then the default
+ // group is propagated.
+ //
+ // * Otherwise the validated group is propagated.
+
+ throw new ValidatorException('The constraint ' . get_class($constraint) . ' cannot be validated. Use the method validate() instead.');
+ }
+
  $walk = function(GraphWalker $walker, $group) use ($constraint, $value) {
  return $walker->walkConstraint($constraint, $value, $group, '');
  };