8000 [Validator] Improved error messages displayed when the Valid constrai… · symfony/symfony@1fe3996 · GitHub
[go: up one dir, main page]
Skip to content

Commit 1fe3996

Browse files
webmozartwebmozart
committed
[Validator] Improved error messages displayed when the Valid constraint is misused
1 parent 83c058f commit 1fe3996

File tree

10 files changed

+235
-16
lines changed

10 files changed

+235
-16
lines changed

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

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
namespace Symfony\Component\Validator\Constraints;
1313

1414
use Symfony\Component\Validator\Constraint;
15+
use Symfony\Component\Validator\Exception\ConstraintDefinitionException;
1516

1617
/**
1718
* @Annotation
@@ -22,6 +23,28 @@ class All extends Constraint
2223
{
2324
public $constraints = array();
2425

26+
/**
27+
* {@inheritDoc}
28+
*/
29+
public function __construct($options = null)
30+
{
31+
parent::__construct($options);
32+
33+
if (!is_array($this->constraints)) {
34+
$this->constraints = array($this->constraints);
35+
}
36+
37+
foreach ($this->constraints as $constraint) {
38+
if (!$constraint instanceof Constraint) {
39+
throw new ConstraintDefinitionException('The value ' . $constraint . ' is not an instance of Constraint in constraint ' . __CLASS__);
40+
}
41+
42+
if ($constraint instanceof Valid) {
43+
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.');
44+
}
45+
}
46+ 6293
}
47+
2548
public function getDefaultOption()
2649
{
2750
return 'constraints';

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

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -44,12 +44,8 @@ public function validate($value, Constraint $constraint)
4444
$group = $this->context->getGroup();
4545
$propertyPath = $this->context->getPropertyPath();
4646

47-
// cannot simply cast to array, because then the object is converted to an
48-
// array instead of wrapped inside
49-
$constraints = is_array($constraint->constraints) ? $constraint->constraints : array($constraint->constraints);
50-
5147
foreach ($value as $key => $element) {
52-
foreach ($constraints as $constr) {
48+
foreach ($constraint->constraints as $constr) {
5349
$walker->walkConstraint($constr, $element, $group, $propertyPath.'['.$key.']');
5450
}
5551
}

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

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,9 @@
1212
namespace Symfony\Component\Validator\Constraints;
1313

1414
use Symfony\Component\Validator\Constraint;
15+
use Symfony\Component\Validator\Constraints\Collection\Required;
16+
use Symfony\Component\Validator\Constraints\Collection\Optional;
17+
use Symfony\Component\Validator\Exception\ConstraintDefinitionException;
1518

1619
/**
1720
* @Annotation
@@ -38,6 +41,30 @@ public function __construct($options = null)
3841
}
3942

4043
parent::__construct($options);
44+
45+
if (!is_array($this->fields)) {
46+
throw new ConstraintDefinitionException('The option "fields" is expected to be an array in constraint ' . __CLASS__);
47+
}
48+
49+
foreach ($this->fields as $fieldName => $field) {
50+
if (!$field instanceof Optional && !$field instanceof Required) {
51+
$this->fields[$fieldName] = $field = new Required($field);
52+
}
53+
54+
if (!is_array($field->constraints)) {
55+
$field->constraints = array($field->constraints);
56+
}
57+
58+
foreach ($field->constraints as $constraint) {
59+
if (!$constraint instanceof Constraint) {
60+
throw new ConstraintDefinitionException('The value ' . $constraint . ' of the field ' . $fieldName . ' is not an instance of Constraint in constraint ' . __CLASS__);
61+
}
62+
63+
if ($constraint instanceof Valid) {
64+
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.');
65+
}
66+
}
67+
}
4168
}
4269

4370
public function getRequiredOptions()

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

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -52,17 +52,7 @@ public function validate($value, Constraint $constraint)
5252
(is_array($value) && array_key_exists($field, $value)) ||
5353
($value instanceof \ArrayAccess && $value->offsetExists($field))
5454
) {
55-
if ($fieldConstraint instanceof Required || $fieldConstraint instanceof Optional) {
56-
$constraints = $fieldConstraint->constraints;
57-
} else {
58-
$constraints = $fieldConstraint;
59-
}
60-
61-
// cannot simply cast to array, because then the object is converted to an
62-
// array instead of wrapped inside
63-
$constraints = is_array($constraints) ? $constraints : array($constraints);
64-
65-
foreach ($constraints as $constr) {
55+
foreach ($fieldConstraint->constraints as $constr) {
6656
$walker->walkConstraint($constr, $value[$field], $group, $propertyPath.'['.$field.']');
6757
}
6858
} elseif (!$fieldConstraint instanceof Optional && !$constraint->allowMissingFields) {

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
namespace Symfony\Component\Validator\Constraints;
1313

1414
use Symfony\Component\Validator\Constraint;
15+
use Symfony\Component\Validator\Exception\ConstraintDefinitionException;
1516

1617
/**
1718
* @Annotation
@@ -23,4 +24,13 @@ class Valid extends Constraint
2324
public $traverse = true;
2425

2526
public $deep = false;
27+
28+
public function __construct($options = null)
29+
{
30+
if (is_array($options) && array_key_exists('groups', $options)) {
31+
throw new ConstraintDefinitionException('The option "groups" is not supported by the constraint ' . __CLASS__);
32+
}
33+
34+
parent::__construct($options);
35+
}
2636
}

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

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\Allator\Tests\Constraints;
13+
14+
use Symfony\Component\Validator\Constraints\All;
15+
use Symfony\Component\Validator\Constraints\Valid;
16+
17+
/**
18+
* @author Bernhard Schussek <bschussek@gmail.com>
19+
*/
20+
class AllTest extends \PHPUnit_Framework_TestCase
21+
{
22+
/**
23+
* @expectedException Symfony\Component\Validator\Exception\ConstraintDefinitionException
24+
*/
25+
public function testRejectNonConstraints()
26+
{
27+
new All(array(
28+
'foo',
29+
));
30+
}
31+
32+
/**
33+
* @expectedException Symfony\Component\Validator\Exception\ConstraintDefinitionException
34+
*/
35+
public function testRejectValidConstraint()
36+
{
37+
new All(array(
38+
new Valid(),
39+
));
40+
}
41+
}

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

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\Collectionator\Tests\Constraints;
13+
14+
use Symfony\Component\Validator\Constraints\Collection;
15+
use Symfony\Component\Validator\Constraints\Collection\Required;
16+
use Symfony\Component\Validator\Constraints\Collection\Optional;
17+
use Symfony\Component\Validator\Constraints\Valid;
18+
19+
/**
20+
* @author Bernhard Schussek <bschussek@gmail.com>
21+
*/
22+
class CollectionTest extends \PHPUnit_Framework_TestCase
23+
{
24+
/**
25+
* @expectedException Symfony\Component\Validator\Exception\ConstraintDefinitionException
26+
*/
27+
public function testRejectInvalidFieldsOption()
28+
{
29+
new Collection(array(
30+
'fields' => 'foo',
31+
));
32+
}
33+
34+
/**
35+
* @expectedException Symfony\Component\Validator\Exception\ConstraintDefinitionException
36+
*/
37+
public function testRejectNonConstraints()
38+
{
39+
new Collection(array(
40+
'foo' => 'bar',
41+
));
42+
}
43+
44+
/**
45+
* @expectedException Symfony\Component\Validator\Exception\ConstraintDefinitionException
46+
*/
47+
public function testRejectValidConstraint()
48+
{
49+
new Collection(array(
50+
'foo' => new Valid(),
51+
));
52+
}
53+
54+
/**
55+
* @expectedException Symfony\Component\Validator\Exception\ConstraintDefinitionException
56+
*/
57+
public function testRejectValidConstraintWithinOptional()
58+
{
59+
new Collection(array(
60+
'foo' => new Optional(new Valid()),
61+
));
62+
}
63+
64+
/**
65+
* @expectedException Symfony\Component\Validator\Exception\ConstraintDefinitionException
66+
*/
67+
public function testRejectValidConstraintWithinRequired()
68+
{
69+
new Collection(array(
70+
'foo' => new Required(new Valid()),
71+
));
72+
}
73+
}

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

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\Validator\Tests\Constraints;
13+
14+
use Symfony\Component\Validator\Constraints\Valid;
15+
16+
/**
17+
* @author Bernhard Schussek <bschussek@gmail.com>
18+
*/
19+
class ValidTest extends \PHPUnit_Framework_TestCase
20+
{
21+
/**
22+
* @expectedException Symfony\Component\Validator\Exception\ConstraintDefinitionException
23+
*/
24+
public function testRejectGroupsOption()
25+
{
26+
new Valid(array('groups' => 'foo'));
27+
}
28+
}

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
use Symfony\Component\Validator\ConstraintViolation;
2020
use Symfony\Component\Validator\ConstraintViolationList;
2121
use Symfony\Component\Validator\ConstraintValidatorFactory;
22+
use Symfony\Component\Validator\Constraints\Valid;
2223
use Symfony\Component\Validator\Mapping\ClassMetadata;
2324

2425
class ValidatorTest extends \PHPUnit_Framework_TestCase
@@ -200,6 +201,18 @@ public function testValidateValue()
200201
$this->assertEquals($violations, $this->validator->validateValue('Bernhard', new FailingConstraint()));
201202
}
202203

204+
/**
205+
* @expectedException Symfony\Component\Validator\Exception\ValidatorException
206+
*/
207+
public function testValidateValueRejectsValid()
208+
{
209+
$entity = new Entity();
210+
$metadata = new ClassMetadata(get_class($entity));
211+
$this->factory->addClassMetadata($metadata);
212+
213+
$this->validator->validateValue($entity, new Valid());
214+
}
215+
203216
public function testGetMetadataFactory()
204217
{
205218
$this->assertInstanceOf(

‎src/Symfony/Component/Validator/Validator.php

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,9 @@
1111

1212
namespace Symfony\Component\Validator;
1313

14+
use Symfony\Component\Validator\Constraints\Valid;
1415
use Symfony\Component\Validator\Mapping\ClassMetadataFactoryInterface;
16+
use Symfony\Component\Validator\Exception\ValidatorException;
1517

1618
/**
1719
* The default implementation of the ValidatorInterface.
@@ -104,6 +106,22 @@ public function validatePropertyValue($class, $property, $value, $groups = null)
104106
*/
105107
public function validateValue($value, Constraint $constraint, $groups = null)
106108
{
109+
if ($constraint instanceof Valid) {
110+
// Why can't the Valid constraint be executed directly?
111+
//
112+
// It cannot be executed like regular other constraints, because regular
113+
// constraints are only executed *if they belong to the validated group*.
114+
// The Valid constraint, on the other hand, is always executed and propagates
115+
// the group to the cascaded object. The propagated group depends on
116+
//
117+
// * Whether a group sequence is currently being executed. Then the default
118+
// group is propagated.
119+
//
120+
// * Otherwise the validated group is propagated.
121+
122+
throw new ValidatorException('The constraint ' . get_class($constraint) . ' cannot be validated. Use the method validate() instead.');
123+
}
124+
107125
$walk = function(GraphWalker $walker, $group) use ($constraint, $value) {
108126
return $walker->walkConstraint($constraint, $value, $group, '');
109127
};

0 commit comments

Comments
 (0)
0