Permalink
Browse files

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

…nt is misused
  • Loading branch information...
1 parent d65e5cd commit cd0190ceddbad95a119b5c37d6ba9c61bd2c7a4b @webmozart webmozart committed Jul 11, 2012
View
@@ -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';
@@ -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.']');
}
}
View
@@ -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()
@@ -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) {
View
@@ -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);
+ }
}
@@ -0,0 +1,41 @@
+<?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\Allator\Tests\Constraints;
+
+use Symfony\Component\Validator\Constraints\All;
+use Symfony\Component\Validator\Constraints\Valid;
+
+/**
+ * @author Bernhard Schussek <bschussek@gmail.com>
+ */
+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(),
+ ));
+ }
+}
@@ -0,0 +1,73 @@
+<?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\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 <bschussek@gmail.com>
+ */
+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()),
+ ));
+ }
+}
@@ -0,0 +1,28 @@
+<?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\Constraints;
+
+use Symfony\Component\Validator\Constraints\Valid;
+
+/**
+ * @author Bernhard Schussek <bschussek@gmail.com>
+ */
+class ValidTest extends \PHPUnit_Framework_TestCase
+{
+ /**
+ * @expectedException Symfony\Component\Validator\Exception\ConstraintDefinitionException
+ */
+ public function testRejectGroupsOption()
+ {
+ new Valid(array('groups' => 'foo'));
+ }
+}
View
@@ -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(
View
@@ -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, '');
};

0 comments on commit cd0190c

Please sign in to comment.