From fe85bbdb06fa1a9c7cd5c01ba5b28893b4ad0caa Mon Sep 17 00:00:00 2001 From: Bernhard Schussek Date: Tue, 31 Jan 2012 18:42:14 +0100 Subject: [PATCH] [Validator] Simplified ExecutionContext::addViolation(), added ExecutionContext::addViolationAt() --- .../Constraints/CollectionValidator.php | 10 +- .../Component/Validator/ExecutionContext.php | 32 ++++- .../Component/Validator/GraphWalker.php | 4 +- .../Constraints/CollectionValidatorTest.php | 12 +- .../Validator/ExecutionContextTest.php | 118 +++++++++++++++++- .../Component/Validator/GraphWalkerTest.php | 5 +- 6 files changed, 162 insertions(+), 19 deletions(-) diff --git a/src/Symfony/Component/Validator/Constraints/CollectionValidator.php b/src/Symfony/Component/Validator/Constraints/CollectionValidator.php index 8f6492952d..2395fd23a7 100644 --- a/src/Symfony/Component/Validator/Constraints/CollectionValidator.php +++ b/src/Symfony/Component/Validator/Constraints/CollectionValidator.php @@ -68,10 +68,9 @@ class CollectionValidator extends ConstraintValidator $walker->walkConstraint($constr, $value[$field], $group, $propertyPath.'['.$field.']'); } } elseif (!$fieldConstraint instanceof Optional && !$constraint->allowMissingFields) { - $this->context->setPropertyPath($propertyPath.'['.$field.']'); - $this->context->addViolation($constraint->missingFieldsMessage, array( + $this->context->addViolationAt($propertyPath.'['.$field.']', $constraint->missingFieldsMessage, array( '{{ field }}' => '"'.$field.'"' - ), $value); + ), null); $valid = false; } } @@ -79,10 +78,9 @@ class CollectionValidator extends ConstraintValidator if (!$constraint->allowExtraFields) { foreach ($value as $field => $fieldValue) { if (!isset($constraint->fields[$field])) { - $this->context->setPropertyPath($propertyPath.'['.$field.']'); - $this->context->addViolation($constraint->extraFieldsMessage, array( + $this->context->addViolationAt($propertyPath.'['.$field.']', $constraint->extraFieldsMessage, array( '{{ field }}' => '"'.$field.'"' - ), $value); + ), $fieldValue); $valid = false; } } diff --git a/src/Symfony/Component/Validator/ExecutionContext.php b/src/Symfony/Component/Validator/ExecutionContext.php index 5dd58790aa..87fb1c7f99 100644 --- a/src/Symfony/Component/Validator/ExecutionContext.php +++ b/src/Symfony/Component/Validator/ExecutionContext.php @@ -14,13 +14,13 @@ namespace Symfony\Component\Validator; use Symfony\Component\Validator\Mapping\ClassMetadataFactoryInterface; /** - * The central object representing a single validation process. + * Stores the state of the current node in the validation graph. * * This object is used by the GraphWalker to initialize validation of different * items and keep track of the violations. * * @author Fabien Potencier - * @author Bernhard Schussek + * @author Bernhard Schussek * * @api */ @@ -30,6 +30,7 @@ class ExecutionContext protected $propertyPath; protected $class; protected $property; + protected $value; protected $group; protected $violations; protected $graphWalker; @@ -55,14 +56,27 @@ class ExecutionContext /** * @api */ - public function addViolation($message, array $params, $invalidValue) + public function addViolation($message, array $params = array(), $invalidValue = null) { $this->violations->add(new ConstraintViolation( $message, $params, $this->root, $this->propertyPath, - $invalidValue + // check using func_num_args() to allow passing null values + func_num_args() === 3 ? $invalidValue : $this->value + )); + } + + public function addViolationAt($propertyPath, $message, array $params = array(), $invalidValue = null) + { + $this->violations->add(new ConstraintViolation( + $message, + $params, + $this->root, + $propertyPath, + // check using func_num_args() to allow passing null values + func_num_args() === 4 ? $invalidValue : $this->value )); } @@ -111,6 +125,16 @@ class ExecutionContext return $this->property; } + public function setCurrentValue($value) + { + $this->value = $value; + } + + public function getCurrentValue() + { + return $this->value; + } + public function setGroup($group) { $this->group = $group; diff --git a/src/Symfony/Component/Validator/GraphWalker.php b/src/Symfony/Component/Validator/GraphWalker.php index a88eb8b3a7..7e9047e195 100644 --- a/src/Symfony/Component/Validator/GraphWalker.php +++ b/src/Symfony/Component/Validator/GraphWalker.php @@ -168,6 +168,7 @@ class GraphWalker { $validator = $this->validatorFactory->getInstance($constraint); + $this->context->setCurrentValue($value); $this->context->setPropertyPath($propertyPath); $this->context->setGroup($group); @@ -184,9 +185,10 @@ class GraphWalker // Resetting the property path. This is needed because some // validators, like CollectionValidator, use the walker internally // and so change the context. + $this->context->setCurrentValue($value); $this->context->setPropertyPath($propertyPath); - $this->context->addViolation($messageTemplate, $messageParams, $value); + $this->context->addViolation($messageTemplate, $messageParams); } } } diff --git a/tests/Symfony/Tests/Component/Validator/Constraints/CollectionValidatorTest.php b/tests/Symfony/Tests/Component/Validator/Constraints/CollectionValidatorTest.php index b9a50bf2da..9898425b2f 100644 --- a/tests/Symfony/Tests/Component/Validator/Constraints/CollectionValidatorTest.php +++ b/tests/Symfony/Tests/Component/Validator/Constraints/CollectionValidatorTest.php @@ -127,13 +127,14 @@ abstract class CollectionValidatorTest extends \PHPUnit_Framework_TestCase public function testExtraFieldsDisallowed() { - $this->context->setPropertyPath('bar'); - $data = $this->prepareTestData(array( 'foo' => 5, 'baz' => 6, )); + $this->context->setCurrentValue($data); + $this->context->setPropertyPath('bar'); + $this->assertFalse($this->validator->isValid($data, new Collection(array( 'fields' => array( 'foo' => new Min(4), @@ -146,7 +147,7 @@ abstract class CollectionValidatorTest extends \PHPUnit_Framework_TestCase array('{{ field }}' => '"baz"'), 'Root', 'bar[baz]', - $data + 6 ), )), $this->context->getViolations()); } @@ -184,8 +185,9 @@ abstract class CollectionValidatorTest extends \PHPUnit_Framework_TestCase public function testMissingFieldsDisallowed() { - $this->context->setPropertyPath('bar'); $data = $this->prepareTestData(array()); + $this->context->setCurrentValue($data); + $this->context->setPropertyPath('bar'); $this->assertFalse($this->validator->isValid($data, new Collection(array( 'fields' => array( @@ -199,7 +201,7 @@ abstract class CollectionValidatorTest extends \PHPUnit_Framework_TestCase array('{{ field }}' => '"foo"'), 'Root', 'bar[foo]', - $data + null ), )), $this->context->getViolations()); } diff --git a/tests/Symfony/Tests/Component/Validator/ExecutionContextTest.php b/tests/Symfony/Tests/Component/Validator/ExecutionContextTest.php index a4410b0f0c..fa30e76fe6 100644 --- a/tests/Symfony/Tests/Component/Validator/ExecutionContextTest.php +++ b/tests/Symfony/Tests/Component/Validator/ExecutionContextTest.php @@ -11,6 +11,10 @@ namespace Symfony\Tests\Component\Validator; +use Symfony\Component\Validator\ConstraintViolation; + +use Symfony\Component\Validator\ConstraintViolationList; + use Symfony\Component\Validator\ExecutionContext; class ExecutionContextTest extends \PHPUnit_Framework_TestCase @@ -43,9 +47,119 @@ class ExecutionContextTest extends \PHPUnit_Framework_TestCase public function testAddViolation() { $this->assertCount(0, $this->context->getViolations()); - $this->context->addViolation('', array(), ''); - $this->assertCount(1, $this->context->getViolations()); + $this->context->setPropertyPath('foo.bar'); + $this->context->addViolation('Error', array('foo' => 'bar'), 'invalid'); + + $this->assertEquals(new ConstraintViolationList(array( + new ConstraintViolation( + 'Error', + array('foo' => 'bar'), + 'Root', + 'foo.bar', + 'invalid' + ), + )), $this->context->getViolations()); + } + + public function testAddViolationUsesPreconfiguredValueIfNotPassed() + { + $this->assertCount(0, $this->context->getViolations()); + + $this->context->setPropertyPath('foo.bar'); + $this->context->setCurrentValue('invalid'); + $this->context->addViolation('Error'); + + $this->assertEquals(new ConstraintViolationList(array( + new ConstraintViolation( + 'Error', + array(), + 'Root', + 'foo.bar', + 'invalid' + ), + )), $this->context->getViolations()); + } + + public function testAddViolationUsesPassedNulValue() + { + $this->assertCount(0, $this->context->getViolations()); + + $this->context->setPropertyPath('foo.bar'); + $this->context->setCurrentValue('invalid'); + + // passed null value should override preconfigured value "invalid" + $this->context->addViolation('Error', array('foo' => 'bar'), null); + + $this->assertEquals(new ConstraintViolationList(array( + new ConstraintViolation( + 'Error', + array('foo' => 'bar'), + 'Root', + 'foo.bar', + null + ), + )), $this->context->getViolations()); + } + + public function testAddViolationAt() + { + $this->assertCount(0, $this->context->getViolations()); + + $this->context->setPropertyPath('foo.bar'); + + // override preconfigured property path + $this->context->addViolationAt('bar.baz', 'Error', array('foo' => 'bar'), 'invalid'); + + $this->assertEquals(new ConstraintViolationList(array( + new ConstraintViolation( + 'Error', + array('foo' => 'bar'), + 'Root', + 'bar.baz', + 'invalid' + ), + )), $this->context->getViolations()); + } + + public function testAddViolationAtUsesPreconfiguredValueIfNotPassed() + { + $this->assertCount(0, $this->context->getViolations()); + + $this->context->setPropertyPath('foo.bar'); + $this->context->setCurrentValue('invalid'); + $this->context->addViolationAt('bar.baz', 'Error'); + + $this->assertEquals(new ConstraintViolationList(array( + new ConstraintViolation( + 'Error', + array(), + 'Root', + 'bar.baz', + 'invalid' + ), + )), $this->context->getViolations()); + } + + public function testAddViolationAtUsesPassedNulValue() + { + $this->assertCount(0, $this->context->getViolations()); + + $this->context->setPropertyPath('foo.bar'); + $this->context->setCurrentValue('invalid'); + + // passed null value should override preconfigured value "invalid" + $this->context->addViolationAt('bar.baz', 'Error', array('foo' => 'bar'), null); + + $this->assertEquals(new ConstraintViolationList(array( + new ConstraintViolation( + 'Error', + array('foo' => 'bar'), + 'Root', + 'bar.baz', + null + ), + )), $this->context->getViolations()); } public function testGetViolations() diff --git a/tests/Symfony/Tests/Component/Validator/GraphWalkerTest.php b/tests/Symfony/Tests/Component/Validator/GraphWalkerTest.php index 61740ab6a4..30d2f3dda9 100644 --- a/tests/Symfony/Tests/Component/Validator/GraphWalkerTest.php +++ b/tests/Symfony/Tests/Component/Validator/GraphWalkerTest.php @@ -58,9 +58,11 @@ class GraphWalkerTest extends \PHPUnit_Framework_TestCase { $this->metadata->addConstraint(new ConstraintA()); - $this->walker->walkObject($this->metadata, new Entity(), 'Default', ''); + $entity = new Entity(); + $this->walker->walkObject($this->metadata, $entity, 'Default', ''); $this->assertEquals('Symfony\Tests\Component\Validator\Fixtures\Entity', $this->getContext()->getCurrentClass()); + $this->assertEquals($entity, $this->getContext()->getCurrentValue()); } public function testWalkObjectValidatesConstraints() @@ -220,6 +222,7 @@ class GraphWalkerTest extends \PHPUnit_Framework_TestCase $this->assertEquals('Symfony\Tests\Component\Validator\Fixtures\Entity', $this->getContext()->getCurrentClass()); $this->assertEquals('firstName', $this->getContext()->getCurrentProperty()); + $this->assertEquals('value', $this->getContext()->getCurrentValue()); } public function testWalkPropertyValueValidatesConstraints()