From 01ceeda376f6e6ff43e78918474a8d2e6a8c83e6 Mon Sep 17 00:00:00 2001 From: Bernhard Schussek Date: Fri, 21 Feb 2014 17:10:39 +0100 Subject: [PATCH] [Validator] Improved test coverage of the Traverse constraint --- .../Validator/Constraints/Traverse.php | 2 - .../Component/Validator/Constraints/Valid.php | 10 +- .../Context/LegacyExecutionContext.php | 13 +- .../Validator/Mapping/ClassMetadata.php | 12 +- .../Validator/Mapping/GenericMetadata.php | 13 ++ .../Validator/Mapping/MemberMetadata.php | 15 -- .../Tests/Validator/Abstract2Dot5ApiTest.php | 194 +++++++++++++++++- 7 files changed, 214 insertions(+), 45 deletions(-) diff --git a/src/Symfony/Component/Validator/Constraints/Traverse.php b/src/Symfony/Component/Validator/Constraints/Traverse.php index 6c22056191..4abae6c67a 100644 --- a/src/Symfony/Component/Validator/Constraints/Traverse.php +++ b/src/Symfony/Component/Validator/Constraints/Traverse.php @@ -24,8 +24,6 @@ class Traverse extends Constraint { public $traverse = true; - public $deep = false; - public function __construct($options = null) { if (is_array($options) && array_key_exists('groups', $options)) { diff --git a/src/Symfony/Component/Validator/Constraints/Valid.php b/src/Symfony/Component/Validator/Constraints/Valid.php index 6ad09624b5..fe50d2d241 100644 --- a/src/Symfony/Component/Validator/Constraints/Valid.php +++ b/src/Symfony/Component/Validator/Constraints/Valid.php @@ -25,6 +25,9 @@ class Valid extends Constraint { public $traverse = true; + /** + * @deprecated Deprecated as of version 2.5, to be removed in Symfony 3.0. + */ public $deep = true; public function __construct($options = null) @@ -38,11 +41,4 @@ class Valid extends Constraint parent::__construct($options); } - - public function getDefaultOption() - { - // Traverse is extended for backwards compatibility reasons - // The parent class should be removed in 3.0 - return null; - } } diff --git a/src/Symfony/Component/Validator/Context/LegacyExecutionContext.php b/src/Symfony/Component/Validator/Context/LegacyExecutionContext.php index b84d1c379c..1a147e441c 100644 --- a/src/Symfony/Component/Validator/Context/LegacyExecutionContext.php +++ b/src/Symfony/Component/Validator/Context/LegacyExecutionContext.php @@ -111,10 +111,8 @@ class LegacyExecutionContext extends ExecutionContext public function validate($value, $subPath = '', $groups = null, $traverse = false, $deep = false) { if (is_array($value)) { - $constraint = new Traverse(array( - 'traverse' => true, - 'deep' => $deep, - )); + // The $traverse flag is ignored for arrays + $constraint = new Valid(array('traverse' => true, 'deep' => $deep)); return $this ->getValidator() @@ -125,16 +123,13 @@ class LegacyExecutionContext extends ExecutionContext } if ($traverse && $value instanceof \Traversable) { - $constraints = array( - new Valid(), - new Traverse(array('traverse' => true, 'deep' => $deep)), - ); + $constraint = new Valid(array('traverse' => true, 'deep' => $deep)); return $this ->getValidator() ->inContext($this) ->atPath($subPath) - ->validate($value, $constraints, $groups) + ->validate($value, $constraint, $groups) ; } diff --git a/src/Symfony/Component/Validator/Mapping/ClassMetadata.php b/src/Symfony/Component/Validator/Mapping/ClassMetadata.php index 30c7b1ad3b..85e872026d 100644 --- a/src/Symfony/Component/Validator/Mapping/ClassMetadata.php +++ b/src/Symfony/Component/Validator/Mapping/ClassMetadata.php @@ -201,20 +201,12 @@ class ClassMetadata extends ElementMetadata implements LegacyMetadataInterface, } if ($constraint instanceof Traverse) { - if (true === $constraint->traverse) { + if ($constraint->traverse) { // If traverse is true, traversal should be explicitly enabled $this->traversalStrategy = TraversalStrategy::TRAVERSE; - - if (!$constraint->deep) { - $this->traversalStrategy |= TraversalStrategy::STOP_RECURSION; - } - } elseif (false === $constraint->traverse) { + } else { // If traverse is false, traversal should be explicitly disabled $this->traversalStrategy = TraversalStrategy::NONE; - } else { - // Else, traverse depending on the contextual information that - // is available during validation - $this->traversalStrategy = TraversalStrategy::IMPLICIT; } // The constraint is not added diff --git a/src/Symfony/Component/Validator/Mapping/GenericMetadata.php b/src/Symfony/Component/Validator/Mapping/GenericMetadata.php index f77e11736a..6c4f186991 100644 --- a/src/Symfony/Component/Validator/Mapping/GenericMetadata.php +++ b/src/Symfony/Component/Validator/Mapping/GenericMetadata.php @@ -12,8 +12,10 @@ namespace Symfony\Component\Validator\Mapping; use Symfony\Component\Validator\Constraint; +use Symfony\Component\Validator\Constraints\Traverse; use Symfony\Component\Validator\Constraints\Valid; use Symfony\Component\Validator\Exception\BadMethodCallException; +use Symfony\Component\Validator\Exception\ConstraintDefinitionException; use Symfony\Component\Validator\ValidationVisitorInterface; /** @@ -103,9 +105,20 @@ class GenericMetadata implements MetadataInterface * @param Constraint $constraint The constraint to add * * @return GenericMetadata This object + * + * @throws ConstraintDefinitionException When trying to add the + * {@link Traverse} constraint */ public function addConstraint(Constraint $constraint) { + if ($constraint instanceof Traverse) { + throw new ConstraintDefinitionException(sprintf( + 'The constraint "%s" can only be put on classes. Please use '. + '"Symfony\Component\Validator\Constraints\Valid" instead.', + get_class($constraint) + )); + } + if ($constraint instanceof Valid) { $this->cascadingStrategy = CascadingStrategy::CASCADE; diff --git a/src/Symfony/Component/Validator/Mapping/MemberMetadata.php b/src/Symfony/Component/Validator/Mapping/MemberMetadata.php index 6aa0ead2ce..60dc417024 100644 --- a/src/Symfony/Component/Validator/Mapping/MemberMetadata.php +++ b/src/Symfony/Component/Validator/Mapping/MemberMetadata.php @@ -58,21 +58,6 @@ abstract class MemberMetadata extends ElementMetadata implements PropertyMetadat )); } - // BC with Symfony < 2.5 - if ($constraint instanceof Valid) { - if (true === $constraint->traverse) { - // Try to traverse cascaded objects, but ignore if they do not - // implement Traversable - $this->traversalStrategy = TraversalStrategy::IMPLICIT; - - if (!$constraint->deep) { - $this->traversalStrategy |= TraversalStrategy::STOP_RECURSION; - } - } elseif (false === $constraint->traverse) { - $this->traversalStrategy = TraversalStrategy::NONE; - } - } - parent::addConstraint($constraint); return $this; diff --git a/src/Symfony/Component/Validator/Tests/Validator/Abstract2Dot5ApiTest.php b/src/Symfony/Component/Validator/Tests/Validator/Abstract2Dot5ApiTest.php index 2d34116e97..4559a3ad32 100644 --- a/src/Symfony/Component/Validator/Tests/Validator/Abstract2Dot5ApiTest.php +++ b/src/Symfony/Component/Validator/Tests/Validator/Abstract2Dot5ApiTest.php @@ -328,18 +328,208 @@ abstract class Abstract2Dot5ApiTest extends AbstractValidatorTest $this->assertNull($violations[0]->getCode()); } + public function testTraversalEnabledOnClass() + { + $entity = new Entity(); + $traversable = new \ArrayIterator(array('key' => $entity)); + + $callback = function ($value, ExecutionContextInterface $context) { + $context->addViolation('Message'); + }; + + $traversableMetadata = new ClassMetadata('ArrayIterator'); + $traversableMetadata->addConstraint(new Traverse(true)); + + $this->metadataFactory->addMetadata($traversableMetadata); + $this->metadata->addConstraint(new Callback(array( + 'callback' => $callback, + 'groups' => 'Group', + ))); + + $violations = $this->validate($traversable, new Valid(), 'Group'); + + /** @var ConstraintViolationInterface[] $violations */ + $this->assertCount(1, $violations); + } + + public function testTraversalDisabledOnClass() + { + $test = $this; + $entity = new Entity(); + $traversable = new \ArrayIterator(array('key' => $entity)); + + $callback = function ($value, ExecutionContextInterface $context) use ($test) { + $test->fail('Should not be called'); + }; + + $traversableMetadata = new ClassMetadata('ArrayIterator'); + $traversableMetadata->addConstraint(new Traverse(false)); + + $this->metadataFactory->addMetadata($traversableMetadata); + $this->metadata->addConstraint(new Callback(array( + 'callback' => $callback, + 'groups' => 'Group', + ))); + + $violations = $this->validate($traversable, new Valid(), 'Group'); + + /** @var ConstraintViolationInterface[] $violations */ + $this->assertCount(0, $violations); + } + /** * @expectedException \Symfony\Component\Validator\Exception\ConstraintDefinitionException */ - public function testExpectTraversableIfTraverseOnClass() + public function testExpectTraversableIfTraversalEnabledOnClass() { $entity = new Entity(); - $this->metadata->addConstraint(new Traverse()); + $this->metadata->addConstraint(new Traverse(true)); $this->validator->validate($entity); } + public function testReferenceTraversalDisabledOnClass() + { + $test = $this; + $entity = new Entity(); + $entity->reference = new \ArrayIterator(array('key' => new Reference())); + + $callback = function ($value, ExecutionContextInterface $context) use ($test) { + $test->fail('Should not be called'); + }; + + $traversableMetadata = new ClassMetadata('ArrayIterator'); + $traversableMetadata->addConstraint(new Traverse(false)); + + $this->metadataFactory->addMetadata($traversableMetadata); + $this->referenceMetadata->addConstraint(new Callback(array( + 'callback' => $callback, + 'groups' => 'Group', + ))); + $this->metadata->addPropertyConstraint('reference', new Valid()); + + $violations = $this->validate($entity, new Valid(), 'Group'); + + /** @var ConstraintViolationInterface[] $violations */ + $this->assertCount(0, $violations); + } + + public function testReferenceTraversalEnabledOnReferenceDisabledOnClass() + { + $test = $this; + $entity = new Entity(); + $entity->reference = new \ArrayIterator(array('key' => new Reference())); + + $callback = function ($value, ExecutionContextInterface $context) use ($test) { + $test->fail('Should not be called'); + }; + + $traversableMetadata = new ClassMetadata('ArrayIterator'); + $traversableMetadata->addConstraint(new Traverse(false)); + + $this->metadataFactory->addMetadata($traversableMetadata); + $this->referenceMetadata->addConstraint(new Callback(array( + 'callback' => $callback, + 'groups' => 'Group', + ))); + $this->metadata->addPropertyConstraint('reference', new Valid(array( + 'traverse' => true, + ))); + + $violations = $this->validate($entity, new Valid(), 'Group'); + + /** @var ConstraintViolationInterface[] $violations */ + $this->assertCount(0, $violations); + } + + public function testReferenceTraversalDisabledOnReferenceEnabledOnClass() + { + $test = $this; + $entity = new Entity(); + $entity->reference = new \ArrayIterator(array('key' => new Reference())); + + $callback = function ($value, ExecutionContextInterface $context) use ($test) { + $test->fail('Should not be called'); + }; + + $traversableMetadata = new ClassMetadata('ArrayIterator'); + $traversableMetadata->addConstraint(new Traverse(true)); + + $this->metadataFactory->addMetadata($traversableMetadata); + $this->referenceMetadata->addConstraint(new Callback(array( + 'callback' => $callback, + 'groups' => 'Group', + ))); + $this->metadata->addPropertyConstraint('reference', new Valid(array( + 'traverse' => false, + ))); + + $violations = $this->validate($entity, new Valid(), 'Group'); + + /** @var ConstraintViolationInterface[] $violations */ + $this->assertCount(0, $violations); + } + + public function testReferenceTraversalRecursionEnabledOnReferenceTraversalEnabledOnClass() + { + $entity = new Entity(); + $entity->reference = new \ArrayIterator(array( + 2 => new \ArrayIterator(array('key' => new Reference())), + )); + + $callback = function ($value, ExecutionContextInterface $context) { + $context->addViolation('Message'); + }; + + $traversableMetadata = new ClassMetadata('ArrayIterator'); + $traversableMetadata->addConstraint(new Traverse(true)); + + $this->metadataFactory->addMetadata($traversableMetadata); + $this->referenceMetadata->addConstraint(new Callback(array( + 'callback' => $callback, + 'groups' => 'Group', + ))); + $this->metadata->addPropertyConstraint('reference', new Valid(array( + 'deep' => true, + ))); + + $violations = $this->validate($entity, new Valid(), 'Group'); + + /** @var ConstraintViolationInterface[] $violations */ + $this->assertCount(1, $violations); + } + + public function testReferenceTraversalRecursionDisabledOnReferenceTraversalEnabledOnClass() + { + $test = $this; + $entity = new Entity(); + $entity->reference = new \ArrayIterator(array( + 2 => new \ArrayIterator(array('key' => new Reference())), + )); + + $callback = function ($value, ExecutionContextInterface $context) use ($test) { + $test->fail('Should not be called'); + }; + + $traversableMetadata = new ClassMetadata('ArrayIterator'); + $traversableMetadata->addConstraint(new Traverse(true)); + + $this->metadataFactory->addMetadata($traversableMetadata); + $this->referenceMetadata->addConstraint(new Callback(array( + 'callback' => $callback, + 'groups' => 'Group', + ))); + $this->metadata->addPropertyConstraint('reference', new Valid(array( + 'deep' => false, + ))); + + $violations = $this->validate($entity, new Valid(), 'Group'); + + /** @var ConstraintViolationInterface[] $violations */ + $this->assertCount(0, $violations); + } + public function testAddCustomizedViolation() { $entity = new Entity();