From f904a9ed535b0f5536d5a92cd5772d35eb5cc088 Mon Sep 17 00:00:00 2001 From: Bernhard Schussek Date: Mon, 30 Jan 2012 20:56:10 +0100 Subject: [PATCH] [Validator] Fixed: GraphWalker does not add constraint violation if error message is empty --- .../Validator/ConstraintValidator.php | 6 ++++- .../Validator/ConstraintViolationList.php | 21 +++++++++++++++- .../Component/Validator/GraphWalker.php | 24 +++++++++++-------- .../Validator/ExecutionContextTest.php | 4 ++-- .../Validator/Fixtures/FailingConstraint.php | 2 +- .../Component/Validator/GraphWalkerTest.php | 12 +++++----- .../Component/Validator/ValidatorTest.php | 10 ++++---- 7 files changed, 53 insertions(+), 26 deletions(-) diff --git a/src/Symfony/Component/Validator/ConstraintValidator.php b/src/Symfony/Component/Validator/ConstraintValidator.php index 54dafd3be74c..2abf92f9c146 100644 --- a/src/Symfony/Component/Validator/ConstraintValidator.php +++ b/src/Symfony/Component/Validator/ConstraintValidator.php @@ -11,7 +11,11 @@ namespace Symfony\Component\Validator; -/* +/** + * Base class for constraint validators + * + * @author Bernhard Schussek + * * @api */ abstract class ConstraintValidator implements ConstraintValidatorInterface diff --git a/src/Symfony/Component/Validator/ConstraintViolationList.php b/src/Symfony/Component/Validator/ConstraintViolationList.php index baeaef224bc6..808a69a182e2 100644 --- a/src/Symfony/Component/Validator/ConstraintViolationList.php +++ b/src/Symfony/Component/Validator/ConstraintViolationList.php @@ -12,14 +12,33 @@ namespace Symfony\Component\Validator; /** - * An array-acting object that holds many ConstrainViolation instances. + * An list of ConstrainViolation objects. + * + * @author Bernhard Schussek * * @api */ class ConstraintViolationList implements \IteratorAggregate, \Countable, \ArrayAccess { + /** + * The constraint violations + * + * @var array + */ protected $violations = array(); + /** + * Creates a new constraint violation list + * + * @param array $violations The constraint violations to add to the list + */ + public function __construct(array $violations = array()) + { + foreach ($violations as $violation) { + $this->add($violation); + } + } + /** * @return string */ diff --git a/src/Symfony/Component/Validator/GraphWalker.php b/src/Symfony/Component/Validator/GraphWalker.php index 0acd64d6cf2f..a88eb8b3a783 100644 --- a/src/Symfony/Component/Validator/GraphWalker.php +++ b/src/Symfony/Component/Validator/GraphWalker.php @@ -174,16 +174,20 @@ public function walkConstraint(Constraint $constraint, $value, $group, $property $validator->initialize($this->context); if (!$validator->isValid($value, $constraint)) { - // Resetting the property path. This is needed because some - // validators, like CollectionValidator, use the walker internally - // and so change the context. - $this->context->setPropertyPath($propertyPath); - - $this->context->addViolation( - $validator->getMessageTemplate(), - $validator->getMessageParameters(), - $value - ); + $messageTemplate = $validator->getMessageTemplate(); + $messageParams = $validator->getMessageParameters(); + + // Somewhat ugly hack: Don't add a violation if no message is set. + // This is required if the validator added its violations directly + // to the context already + if (!empty($messageTemplate)) { + // Resetting the property path. This is needed because some + // validators, like CollectionValidator, use the walker internally + // and so change the context. + $this->context->setPropertyPath($propertyPath); + + $this->context->addViolation($messageTemplate, $messageParams, $value); + } } } } diff --git a/tests/Symfony/Tests/Component/Validator/ExecutionContextTest.php b/tests/Symfony/Tests/Component/Validator/ExecutionContextTest.php index 1b47bdf2687b..a4410b0f0cc9 100644 --- a/tests/Symfony/Tests/Component/Validator/ExecutionContextTest.php +++ b/tests/Symfony/Tests/Component/Validator/ExecutionContextTest.php @@ -98,9 +98,9 @@ public function testViolationsAsString() $violations = $this->context->getViolations(); $expected = <<add(new ConstraintViolation( - '', + 'Failed', array(), 'Root', 'firstName', @@ -175,7 +175,7 @@ public function testWalkObjectInGroupSequencePropagatesDefaultGroup() // "Default" was launched $violations = new ConstraintViolationList(); $violations->add(new ConstraintViolation( - '', + 'Failed', array(), 'Root', 'reference', @@ -202,7 +202,7 @@ public function testWalkObjectInOtherGroupTraversesNoGroupSequence() // Only group "Second" was validated $violations = new ConstraintViolationList(); $violations->add(new ConstraintViolation( - '', + 'Failed', array(), 'Root', 'lastName', @@ -254,7 +254,7 @@ public function testWalkCascadedPropertyValidatesReferences() $violations = new ConstraintViolationList(); $violations->add(new ConstraintViolation( - '', + 'Failed', array(), 'Root', 'path', @@ -286,7 +286,7 @@ public function testWalkCascadedPropertyValidatesArraysByDefault() $violations = new ConstraintViolationList(); $violations->add(new ConstraintViolation( - '', + 'Failed', array(), 'Root', 'path[key]', @@ -319,7 +319,7 @@ public function testWalkCascadedPropertyValidatesTraversableByDefault() $violations = new ConstraintViolationList(); $violations->add(new ConstraintViolation( - '', + 'Failed', array(), 'Root', 'path[key]', diff --git a/tests/Symfony/Tests/Component/Validator/ValidatorTest.php b/tests/Symfony/Tests/Component/Validator/ValidatorTest.php index c161a499702e..b07e596a10c9 100644 --- a/tests/Symfony/Tests/Component/Validator/ValidatorTest.php +++ b/tests/Symfony/Tests/Component/Validator/ValidatorTest.php @@ -55,7 +55,7 @@ public function testValidate_defaultGroup() // Only the constraint of group "Default" failed $violations = new ConstraintViolationList(); $violations->add(new ConstraintViolation( - '', + 'Failed', array(), $entity, 'firstName', @@ -78,7 +78,7 @@ public function testValidate_oneGroup() // Only the constraint of group "Custom" failed $violations = new ConstraintViolationList(); $violations->add(new ConstraintViolation( - '', + 'Failed', array(), $entity, 'lastName', @@ -103,14 +103,14 @@ public function testValidate_multipleGroups() // The constraints of both groups failed $violations = new ConstraintViolationList(); $violations->add(new ConstraintViolation( - '', + 'Failed', array(), $entity, 'firstName', '' )); $violations->add(new ConstraintViolation( - '', + 'Failed', array(), $entity, 'lastName', @@ -150,7 +150,7 @@ public function testValidateValue() { $violations = new ConstraintViolationList(); $violations->add(new ConstraintViolation( - '', + 'Failed', array(), '', '',