From a103c28b08e6a60c8e1fdb693976ede4c86f015a Mon Sep 17 00:00:00 2001 From: Bernhard Schussek Date: Mon, 30 Jan 2012 20:57:20 +0100 Subject: [PATCH] [Validator] The Collection constraint adds "missing" and "extra" errors to the individual fields now --- CHANGELOG-2.1.md | 4 ++ .../Validator/Constraints/Collection.php | 4 +- .../Constraints/CollectionValidator.php | 43 ++++++++----------- .../Constraints/CollectionValidatorTest.php | 27 +++++++++++- .../Component/Validator/GraphWalkerTest.php | 4 +- 5 files changed, 52 insertions(+), 30 deletions(-) diff --git a/CHANGELOG-2.1.md b/CHANGELOG-2.1.md index f51c9827ea..a7082b195a 100644 --- a/CHANGELOG-2.1.md +++ b/CHANGELOG-2.1.md @@ -295,6 +295,10 @@ To get the diff between two versions, go to https://github.com/symfony/symfony/c * added a SizeLength validator * improved the ImageValidator with min width, max width, min height, and max height constraints * added support for MIME with wildcard in FileValidator + * changed Collection validator to add "missing" and "extra" errors to + individual fields + * changed default value for `extraFieldsMessage` and `missingFieldsMessage` + in Collection constraint ### Yaml diff --git a/src/Symfony/Component/Validator/Constraints/Collection.php b/src/Symfony/Component/Validator/Constraints/Collection.php index d4abc7a8fd..5740b98d34 100644 --- a/src/Symfony/Component/Validator/Constraints/Collection.php +++ b/src/Symfony/Component/Validator/Constraints/Collection.php @@ -23,8 +23,8 @@ class Collection extends Constraint public $fields; public $allowExtraFields = false; public $allowMissingFields = false; - public $extraFieldsMessage = 'The fields {{ fields }} were not expected'; - public $missingFieldsMessage = 'The fields {{ fields }} are missing'; + public $extraFieldsMessage = 'This field was not expected'; + public $missingFieldsMessage = 'This field is missing'; /** * {@inheritDoc} diff --git a/src/Symfony/Component/Validator/Constraints/CollectionValidator.php b/src/Symfony/Component/Validator/Constraints/CollectionValidator.php index f3dd7cbea3..8f6492952d 100644 --- a/src/Symfony/Component/Validator/Constraints/CollectionValidator.php +++ b/src/Symfony/Component/Validator/Constraints/CollectionValidator.php @@ -46,12 +46,7 @@ class CollectionValidator extends ConstraintValidator $group = $this->context->getGroup(); $propertyPath = $this->context->getPropertyPath(); - $missingFields = array(); - $extraFields = array(); - - foreach ($value as $field => $fieldValue) { - $extraFields[$field] = $fieldValue; - } + $valid = true; foreach ($constraint->fields as $field => $fieldConstraint) { if ( @@ -72,29 +67,27 @@ class CollectionValidator extends ConstraintValidator foreach ($constraints as $constr) { $walker->walkConstraint($constr, $value[$field], $group, $propertyPath.'['.$field.']'); } - - unset($extraFields[$field]); - } elseif (!$fieldConstraint instanceof Optional) { - $missingFields[] = $field; + } elseif (!$fieldConstraint instanceof Optional && !$constraint->allowMissingFields) { + $this->context->setPropertyPath($propertyPath.'['.$field.']'); + $this->context->addViolation($constraint->missingFieldsMessage, array( + '{{ field }}' => '"'.$field.'"' + ), $value); + $valid = false; } } - if (count($extraFields) > 0 && !$constraint->allowExtraFields) { - $this->setMessage($constraint->extraFieldsMessage, array( - '{{ fields }}' => '"'.implode('", "', array_keys($extraFields)).'"' - )); - - return false; + if (!$constraint->allowExtraFields) { + foreach ($value as $field => $fieldValue) { + if (!isset($constraint->fields[$field])) { + $this->context->setPropertyPath($propertyPath.'['.$field.']'); + $this->context->addViolation($constraint->extraFieldsMessage, array( + '{{ field }}' => '"'.$field.'"' + ), $value); + $valid = false; + } + } } - if (count($missingFields) > 0 && !$constraint->allowMissingFields) { - $this->setMessage($constraint->missingFieldsMessage, array( - '{{ fields }}' => '"'.implode('", "', $missingFields).'"' - )); - - return false; - } - - return true; + return $valid; } } diff --git a/tests/Symfony/Tests/Component/Validator/Constraints/CollectionValidatorTest.php b/tests/Symfony/Tests/Component/Validator/Constraints/CollectionValidatorTest.php index 19676fb07b..b9a50bf2da 100644 --- a/tests/Symfony/Tests/Component/Validator/Constraints/CollectionValidatorTest.php +++ b/tests/Symfony/Tests/Component/Validator/Constraints/CollectionValidatorTest.php @@ -11,6 +11,8 @@ namespace Symfony\Tests\Component\Validator\Constraints; +use Symfony\Component\Validator\ConstraintViolation; +use Symfony\Component\Validator\ConstraintViolationList; use Symfony\Component\Validator\ExecutionContext; use Symfony\Component\Validator\Constraints\Min; use Symfony\Component\Validator\Constraints\NotNull; @@ -125,9 +127,11 @@ abstract class CollectionValidatorTest extends \PHPUnit_Framework_TestCase public function testExtraFieldsDisallowed() { + $this->context->setPropertyPath('bar'); + $data = $this->prepareTestData(array( 'foo' => 5, - 'bar' => 6, + 'baz' => 6, )); $this->assertFalse($this->validator->isValid($data, new Collection(array( @@ -135,6 +139,16 @@ abstract class CollectionValidatorTest extends \PHPUnit_Framework_TestCase 'foo' => new Min(4), ), )))); + + $this->assertEquals(new ConstraintViolationList(array( + new ConstraintViolation( + 'This field was not expected', + array('{{ field }}' => '"baz"'), + 'Root', + 'bar[baz]', + $data + ), + )), $this->context->getViolations()); } // bug fix @@ -170,6 +184,7 @@ abstract class CollectionValidatorTest extends \PHPUnit_Framework_TestCase public function testMissingFieldsDisallowed() { + $this->context->setPropertyPath('bar'); $data = $this->prepareTestData(array()); $this->assertFalse($this->validator->isValid($data, new Collection(array( @@ -177,6 +192,16 @@ abstract class CollectionValidatorTest extends \PHPUnit_Framework_TestCase 'foo' => new Min(4), ), )))); + + $this->assertEquals(new ConstraintViolationList(array( + new ConstraintViolation( + 'This field is missing', + array('{{ field }}' => '"foo"'), + 'Root', + 'bar[foo]', + $data + ), + )), $this->context->getViolations()); } public function testMissingFieldsAllowed() diff --git a/tests/Symfony/Tests/Component/Validator/GraphWalkerTest.php b/tests/Symfony/Tests/Component/Validator/GraphWalkerTest.php index b2c590b020..61740ab6a4 100644 --- a/tests/Symfony/Tests/Component/Validator/GraphWalkerTest.php +++ b/tests/Symfony/Tests/Component/Validator/GraphWalkerTest.php @@ -441,7 +441,7 @@ class GraphWalkerTest extends \PHPUnit_Framework_TestCase $this->walker->walkConstraint($constraint, array('foo' => 'VALID'), 'Default', 'collection'); $violations = $this->walker->getViolations(); - $this->assertEquals('collection', $violations[0]->getPropertyPath()); + $this->assertEquals('collection[bar]', $violations[0]->getPropertyPath()); } public function testWalkObjectUsesCorrectPropertyPathInViolationsWhenUsingNestedCollections() @@ -455,7 +455,7 @@ class GraphWalkerTest extends \PHPUnit_Framework_TestCase $this->walker->walkConstraint($constraint, array('foo' => array('foo' => 'VALID')), 'Default', 'collection'); $violations = $this->walker->getViolations(); - $this->assertEquals('collection[foo]', $violations[0]->getPropertyPath()); + $this->assertEquals('collection[foo][bar]', $violations[0]->getPropertyPath()); } protected function getContext()