[Validator] Improved test coverage and prevented duplicate validation of constraints

This commit is contained in:
Bernhard Schussek 2014-02-21 15:26:27 +01:00
parent 186c115894
commit 299c2dca10
14 changed files with 346 additions and 92 deletions

View File

@ -33,10 +33,6 @@ class CallbackValidator extends ConstraintValidator
if (!$constraint instanceof Callback) {
throw new UnexpectedTypeException($constraint, __NAMESPACE__.'\Callback');
}
if (null === $object) {
return;
}
if (null !== $constraint->callback && null !== $constraint->methods) {
throw new ConstraintDefinitionException(
@ -60,18 +56,24 @@ class CallbackValidator extends ConstraintValidator
}
call_user_func($method, $object, $this->context);
continue;
}
if (null === $object) {
continue;
}
if (!method_exists($object, $method)) {
throw new ConstraintDefinitionException(sprintf('Method "%s" targeted by Callback constraint does not exist', $method));
}
$reflMethod = new \ReflectionMethod($object, $method);
if ($reflMethod->isStatic()) {
$reflMethod->invoke(null, $object, $this->context);
} else {
if (!method_exists($object, $method)) {
throw new ConstraintDefinitionException(sprintf('Method "%s" targeted by Callback constraint does not exist', $method));
}
$reflMethod = new \ReflectionMethod($object, $method);
if ($reflMethod->isStatic()) {
$reflMethod->invoke(null, $object, $this->context);
} else {
$reflMethod->invoke($object, $this->context);
}
$reflMethod->invoke($object, $this->context);
}
}
}

View File

@ -19,7 +19,6 @@ use Symfony\Component\Validator\Exception\BadMethodCallException;
use Symfony\Component\Validator\Group\GroupManagerInterface;
use Symfony\Component\Validator\Mapping\PropertyMetadataInterface;
use Symfony\Component\Validator\Node\Node;
use Symfony\Component\Validator\NodeVisitor\NodeObserverInterface;
use Symfony\Component\Validator\Util\PropertyPath;
use Symfony\Component\Validator\Validator\ValidatorInterface;
use Symfony\Component\Validator\Violation\ConstraintViolationBuilder;
@ -32,7 +31,7 @@ use Symfony\Component\Validator\Violation\ConstraintViolationBuilder;
*
* @see ExecutionContextInterface
*/
class ExecutionContext implements ExecutionContextInterface, NodeObserverInterface
class ExecutionContext implements ExecutionContextInterface
{
/**
* @var ValidatorInterface
@ -75,6 +74,27 @@ class ExecutionContext implements ExecutionContextInterface, NodeObserverInterfa
*/
private $node;
/**
* Stores which objects have been validated in which group.
*
* @var array
*/
private $validatedObjects = array();
/**
* Stores which class constraint has been validated for which object.
*
* @var array
*/
private $validatedClassConstraints = array();
/**
* Stores which property constraint has been validated for which property.
*
* @var array
*/
private $validatedPropertyConstraints = array();
/**
* Creates a new execution context.
*
@ -279,4 +299,68 @@ class ExecutionContext implements ExecutionContextInterface, NodeObserverInterfa
'or hasMetadataFor() instead or enable the legacy mode.'
);
}
/**
* {@inheritdoc}
*/
public function markObjectAsValidatedForGroup($objectHash, $groupHash)
{
if (!isset($this->validatedObjects[$objectHash])) {
$this->validatedObjects[$objectHash] = array();
}
$this->validatedObjects[$objectHash][$groupHash] = true;
}
/**
* {@inheritdoc}
*/
public function isObjectValidatedForGroup($objectHash, $groupHash)
{
return isset($this->validatedObjects[$objectHash][$groupHash]);
}
/**
* {@inheritdoc}
*/
public function markClassConstraintAsValidated($objectHash, $constraintHash)
{
if (!isset($this->validatedClassConstraints[$objectHash])) {
$this->validatedClassConstraints[$objectHash] = array();
}
$this->validatedClassConstraints[$objectHash][$constraintHash] = true;
}
/**
* {@inheritdoc}
*/
public function isClassConstraintValidated($objectHash, $constraintHash)
{
return isset($this->validatedClassConstraints[$objectHash][$constraintHash]);
}
/**
* {@inheritdoc}
*/
public function markPropertyConstraintAsValidated($objectHash, $propertyName, $constraintHash)
{
if (!isset($this->validatedPropertyConstraints[$objectHash])) {
$this->validatedPropertyConstraints[$objectHash] = array();
}
if (!isset($this->validatedPropertyConstraints[$objectHash][$propertyName])) {
$this->validatedPropertyConstraints[$objectHash][$propertyName] = array();
}
$this->validatedPropertyConstraints[$objectHash][$propertyName][$constraintHash] = true;
}
/**
* {@inheritdoc}
*/
public function isPropertyConstraintValidated($objectHash, $propertyName, $constraintHash)
{
return isset($this->validatedPropertyConstraints[$objectHash][$propertyName][$constraintHash]);
}
}

View File

@ -12,6 +12,7 @@
namespace Symfony\Component\Validator\Context;
use Symfony\Component\Validator\ExecutionContextInterface as LegacyExecutionContextInterface;
use Symfony\Component\Validator\Node\Node;
use Symfony\Component\Validator\Validator\ValidatorInterface;
use Symfony\Component\Validator\Violation\ConstraintViolationBuilderInterface;
@ -97,4 +98,92 @@ interface ExecutionContextInterface extends LegacyExecutionContextInterface
* @return ValidatorInterface
*/
public function getValidator();
/**
* Sets the currently traversed node.
*
* @param Node $node The current node
*
* @internal Used by the validator engine. Should not be called by user
* code.
*/
public function setCurrentNode(Node $node);
/**
* Marks an object as validated in a specific validation group.
*
* @param string $objectHash The hash of the object
* @param string $groupHash The group's name or hash, if it is group
* sequence
*
* @internal Used by the validator engine. Should not be called by user
* code.
*/
public function markObjectAsValidatedForGroup($objectHash, $groupHash);
/**
* Returns whether an object was validated in a specific validation group.
*
* @param string $objectHash The hash of the object
* @param string $groupHash The group's name or hash, if it is group
* sequence
*
* @return Boolean Whether the object was already validated for that
* group
*
* @internal Used by the validator engine. Should not be called by user
* code.
*/
public function isObjectValidatedForGroup($objectHash, $groupHash);
/**
* Marks a constraint as validated for an object.
*
* @param string $objectHash The hash of the object
* @param string $constraintHash The hash of the constraint
*
* @internal Used by the validator engine. Should not be called by user
* code.
*/
public function markClassConstraintAsValidated($objectHash, $constraintHash);
/**
* Returns whether a constraint was validated for an object.
*
* @param string $objectHash The hash of the object
* @param string $constraintHash The hash of the constraint
*
* @return Boolean Whether the constraint was already validated
*
* @internal Used by the validator engine. Should not be called by user
* code.
*/
public function isClassConstraintValidated($objectHash, $constraintHash);
/**
* Marks a constraint as validated for an object and a property name.
*
* @param string $objectHash The hash of the object
* @param string $propertyName The property name
* @param string $constraintHash The hash of the constraint
*
* @internal Used by the validator engine. Should not be called by user
* code.
*/
public function markPropertyConstraintAsValidated($objectHash, $propertyName, $constraintHash);
/**
* Returns whether a constraint was validated for an object and a property
* name.
*
* @param string $objectHash The hash of the object
* @param string $propertyName The property name
* @param string $constraintHash The hash of the constraint
*
* @return Boolean Whether the constraint was already validated
*
* @internal Used by the validator engine. Should not be called by user
* code.
*/
public function isPropertyConstraintValidated($objectHash, $propertyName, $constraintHash);
}

View File

@ -15,19 +15,32 @@ use Symfony\Component\Validator\Context\ExecutionContextInterface;
use Symfony\Component\Validator\Node\Node;
/**
* @since %%NextVersion%%
* Base visitor with empty method stubs.
*
* @since 2.5
* @author Bernhard Schussek <bschussek@gmail.com>
*
* @see NodeVisitorInterface
*/
abstract class AbstractVisitor implements NodeVisitorInterface
{
/**
* {@inheritdoc}
*/
public function beforeTraversal(array $nodes, ExecutionContextInterface $context)
{
}
/**
* {@inheritdoc}
*/
public function afterTraversal(array $nodes, ExecutionContextInterface $context)
{
}
/**
* {@inheritdoc}
*/
public function visit(Node $node, ExecutionContextInterface $context)
{
}

View File

@ -12,29 +12,24 @@
namespace Symfony\Component\Validator\NodeVisitor;
use Symfony\Component\Validator\Context\ExecutionContextInterface;
use Symfony\Component\Validator\Exception\RuntimeException;
use Symfony\Component\Validator\Node\Node;
/**
* Updates the current context with the current node of the validation
* traversal.
* Informs the execution context about the currently validated node.
*
* @since 2.5
* @author Bernhard Schussek <bschussek@gmail.com>
*/
class ContextUpdateVisitor extends AbstractVisitor
{
/**
* Updates the execution context.
*
* @param Node $node The current node
* @param ExecutionContextInterface $context The execution context
*/
public function visit(Node $node, ExecutionContextInterface $context)
{
if (!$context instanceof NodeObserverInterface) {
throw new RuntimeException(sprintf(
'The ContextUpdateVisitor only supports instances of class '.
'"Symfony\Component\Validator\NodeVisitor\NodeObserverInterface". '.
'An instance of class "%s" was given.',
get_class($context)
));
}
$context->setCurrentNode($node);
}
}

View File

@ -18,11 +18,25 @@ use Symfony\Component\Validator\Node\ClassNode;
use Symfony\Component\Validator\Node\Node;
/**
* @since %%NextVersion%%
* Checks class nodes whether their "Default" group is replaced by a group
* sequence and adjusts the validation groups accordingly.
*
* If the "Default" group is replaced for a class node, and if the validated
* groups of the node contain the group "Default", that group is replaced by
* the group sequence specified in the class' metadata.
*
* @since 2.5
* @author Bernhard Schussek <bschussek@gmail.com>
*/
class GroupSequenceResolvingVisitor extends AbstractVisitor
class DefaultGroupReplacingVisitor extends AbstractVisitor
{
/**
* Replaces the "Default" group in the node's groups by the class' group
* sequence.
*
* @param Node $node The current node
* @param ExecutionContextInterface $context The execution context
*/
public function visit(Node $node, ExecutionContextInterface $context)
{
if (!$node instanceof ClassNode) {
@ -30,16 +44,19 @@ class GroupSequenceResolvingVisitor extends AbstractVisitor
}
if ($node->metadata->hasGroupSequence()) {
// The group sequence is statically defined for the class
$groupSequence = $node->metadata->getGroupSequence();
} elseif ($node->metadata->isGroupSequenceProvider()) {
// The group sequence is dynamically obtained from the validated
// object
/** @var \Symfony\Component\Validator\GroupSequenceProviderInterface $value */
$groupSequence = $node->value->getGroupSequence();
// TODO test
if (!$groupSequence instanceof GroupSequence) {
$groupSequence = new GroupSequence($groupSequence);
}
} else {
// The "Default" group is not overridden. Quit.
return;
}

View File

@ -1,23 +0,0 @@
<?php
/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/
namespace Symfony\Component\Validator\NodeVisitor;
use Symfony\Component\Validator\Node\Node;
/**
* @since %%NextVersion%%
* @author Bernhard Schussek <bschussek@gmail.com>
*/
interface NodeObserverInterface
{
public function setCurrentNode(Node $node);
}

View File

@ -22,11 +22,19 @@ use Symfony\Component\Validator\Node\PropertyNode;
use Symfony\Component\Validator\NodeTraverser\NodeTraverserInterface;
/**
* @since %%NextVersion%%
* Validates a node's value against the constraints defined in it's metadata.
*
* @since 2.5
* @author Bernhard Schussek <bschussek@gmail.com>
*/
class NodeValidationVisitor extends AbstractVisitor implements GroupManagerInterface
{
/**
* Stores the hashes of each validated object together with the groups
* in which that object was already validated.
*
* @var array
*/
private $validatedObjects = array();
private $validatedConstraints = array();
@ -83,19 +91,19 @@ class NodeValidationVisitor extends AbstractVisitor implements GroupManagerInter
// Use the object hash for group sequences
$groupHash = is_object($group) ? spl_object_hash($group) : $group;
if (isset($this->validatedObjects[$objectHash][$groupHash])) {
if ($context->isObjectValidatedForGroup($objectHash, $groupHash)) {
// Skip this group when validating properties
unset($node->groups[$key]);
continue;
}
$this->validatedObjects[$objectHash][$groupHash] = true;
//$context->markObjectAsValidatedForGroup($objectHash, $groupHash);
}
// Validate normal group
if (!$group instanceof GroupSequence) {
$this->validateNodeForGroup($objectHash, $node, $group, $context);
$this->validateNodeForGroup($node, $group, $context, $objectHash);
continue;
}
@ -142,20 +150,31 @@ class NodeValidationVisitor extends AbstractVisitor implements GroupManagerInter
}
}
private function validateNodeForGroup($objectHash, Node $node, $group, ExecutionContextInterface $context)
private function validateNodeForGroup(Node $node, $group, ExecutionContextInterface $context, $objectHash)
{
try {
$this->currentGroup = $group;
foreach ($node->metadata->findConstraints($group) as $constraint) {
// Remember the validated constraints of each object to prevent
// duplicate validation of constraints that belong to multiple
// validated groups
// Prevent duplicate validation of constraints, in the case
// that constraints belong to multiple validated groups
if (null !== $objectHash) {
$constraintHash = spl_object_hash($constraint);
if (isset($this->validatedConstraints[$objectHash][$constraintHash])) {
continue;
if ($node instanceof ClassNode) {
if ($context->isClassConstraintValidated($objectHash, $constraintHash)) {
continue;
}
$context->markClassConstraintAsValidated($objectHash, $constraintHash);
} elseif ($node instanceof PropertyNode) {
$propertyName = $node->metadata->getPropertyName();
if ($context->isPropertyConstraintValidated($objectHash, $propertyName, $constraintHash)) {
continue;
}
$context->markPropertyConstraintAsValidated($objectHash, $propertyName, $constraintHash);
}
$this->validatedConstraints[$objectHash][$constraintHash] = true;

View File

@ -15,8 +15,17 @@ use Symfony\Component\Validator\Context\ExecutionContextInterface;
use Symfony\Component\Validator\Node\Node;
/**
* @since %%NextVersion%%
* A node visitor invoked by the node traverser.
*
* At the beginning of the traversal, the method {@link beforeTraversal()} is
* called. For each traversed node, the method {@link visit()} is called. At
* last, the method {@link afterTraversal()} is called when the traversal is
* complete.
*
* @since 2.5
* @author Bernhard Schussek <bschussek@gmail.com>
*
* @see \Symfony\Component\Validator\NodeTraverser\NodeTraverserInterface
*/
interface NodeVisitorInterface
{

View File

@ -129,6 +129,23 @@ class CallbackValidatorTest extends \PHPUnit_Framework_TestCase
$this->validator->validate($object, $constraint);
}
public function testClosureNullObject()
{
$constraint = new Callback(function ($object, ExecutionContext $context) {
$context->addViolation('My message', array('{{ value }}' => 'foobar'), 'invalidValue');
return false;
});
$this->context->expects($this->once())
->method('addViolation')
->with('My message', array(
'{{ value }}' => 'foobar',
));
$this->validator->validate(null, $constraint);
}
public function testClosureExplicitName()
{
$object = new CallbackValidatorTest_Object();
@ -163,6 +180,19 @@ class CallbackValidatorTest extends \PHPUnit_Framework_TestCase
$this->validator->validate($object, $constraint);
}
public function testArrayCallableNullObject()
{
$constraint = new Callback(array(__CLASS__.'_Class', 'validateCallback'));
$this->context->expects($this->once())
->method('addViolation')
->with('Callback message', array(
'{{ value }}' => 'foobar',
));
$this->validator->validate(null, $constraint);
}
public function testArrayCallableExplicitName()
{
$object = new CallbackValidatorTest_Object();

View File

@ -66,25 +66,6 @@ abstract class Abstract2Dot5ApiTest extends AbstractValidatorTest
return $this->validator->validatePropertyValue($object, $propertyName, $value, $groups);
}
public function testNoDuplicateValidationIfConstraintInMultipleGroups()
{
$entity = new Entity();
$callback = function ($value, ExecutionContextInterface $context) {
$context->addViolation('Message');
};
$this->metadata->addConstraint(new Callback(array(
'callback' => $callback,
'groups' => array('Group 1', 'Group 2'),
)));
$violations = $this->validator->validate($entity, new Valid(), array('Group 1', 'Group 2'));
/** @var ConstraintViolationInterface[] $violations */
$this->assertCount(1, $violations);
}
public function testGroupSequenceAbortsAfterFailedGroup()
{
$entity = new Entity();
@ -441,4 +422,42 @@ abstract class Abstract2Dot5ApiTest extends AbstractValidatorTest
$this->validator->validate($entity);
}
public function testNoDuplicateValidationIfClassConstraintInMultipleGroups()
{
$entity = new Entity();
$callback = function ($value, ExecutionContextInterface $context) {
$context->addViolation('Message');
};
$this->metadata->addConstraint(new Callback(array(
'callback' => $callback,
'groups' => array('Group 1', 'Group 2'),
)));
$violations = $this->validator->validate($entity, new Valid(), array('Group 1', 'Group 2'));
/** @var ConstraintViolationInterface[] $violations */
$this->assertCount(1, $violations);
}
public function testNoDuplicateValidationIfPropertyConstraintInMultipleGroups()
{
$entity = new Entity();
$callback = function ($value, ExecutionContextInterface $context) {
$context->addViolation('Message');
};
$this->metadata->addPropertyConstraint('firstName', new Callback(array(
'callback' => $callback,
'groups' => array('Group 1', 'Group 2'),
)));
$violations = $this->validator->validate($entity, new Valid(), array('Group 1', 'Group 2'));
/** @var ConstraintViolationInterface[] $violations */
$this->assertCount(1, $violations);
}
}

View File

@ -16,7 +16,7 @@ use Symfony\Component\Validator\ConstraintValidatorFactory;
use Symfony\Component\Validator\Context\LegacyExecutionContextFactory;
use Symfony\Component\Validator\MetadataFactoryInterface;
use Symfony\Component\Validator\NodeVisitor\ContextUpdateVisitor;
use Symfony\Component\Validator\NodeVisitor\GroupSequenceResolvingVisitor;
use Symfony\Component\Validator\NodeVisitor\DefaultGroupReplacingVisitor;
use Symfony\Component\Validator\NodeVisitor\NodeValidationVisitor;
use Symfony\Component\Validator\NodeTraverser\NonRecursiveNodeTraverser;
use Symfony\Component\Validator\Validator\LegacyValidator;
@ -38,7 +38,7 @@ class LegacyValidator2Dot5ApiTest extends Abstract2Dot5ApiTest
$nodeValidator = new NodeValidationVisitor($nodeTraverser, new ConstraintValidatorFactory());
$contextFactory = new LegacyExecutionContextFactory($nodeValidator, new DefaultTranslator());
$validator = new LegacyValidator($contextFactory, $nodeTraverser, $metadataFactory);
$groupSequenceResolver = new GroupSequenceResolvingVisitor();
$groupSequenceResolver = new DefaultGroupReplacingVisitor();
$contextRefresher = new ContextUpdateVisitor();
$nodeTraverser->addVisitor($groupSequenceResolver);

View File

@ -16,7 +16,7 @@ use Symfony\Component\Validator\ConstraintValidatorFactory;
use Symfony\Component\Validator\Context\LegacyExecutionContextFactory;
use Symfony\Component\Validator\MetadataFactoryInterface;
use Symfony\Component\Validator\NodeVisitor\ContextUpdateVisitor;
use Symfony\Component\Validator\NodeVisitor\GroupSequenceResolvingVisitor;
use Symfony\Component\Validator\NodeVisitor\DefaultGroupReplacingVisitor;
use Symfony\Component\Validator\NodeVisitor\NodeValidationVisitor;
use Symfony\Component\Validator\NodeTraverser\NonRecursiveNodeTraverser;
use Symfony\Component\Validator\Validator\LegacyValidator;
@ -38,7 +38,7 @@ class LegacyValidatorLegacyApiTest extends AbstractLegacyApiTest
$nodeValidator = new NodeValidationVisitor($nodeTraverser, new ConstraintValidatorFactory());
$contextFactory = new LegacyExecutionContextFactory($nodeValidator, new DefaultTranslator());
$validator = new LegacyValidator($contextFactory, $nodeTraverser, $metadataFactory);
$groupSequenceResolver = new GroupSequenceResolvingVisitor();
$groupSequenceResolver = new DefaultGroupReplacingVisitor();
$contextRefresher = new ContextUpdateVisitor();
$nodeTraverser->addVisitor($groupSequenceResolver);

View File

@ -16,7 +16,7 @@ use Symfony\Component\Validator\ConstraintValidatorFactory;
use Symfony\Component\Validator\Context\ExecutionContextFactory;
use Symfony\Component\Validator\MetadataFactoryInterface;
use Symfony\Component\Validator\NodeVisitor\ContextUpdateVisitor;
use Symfony\Component\Validator\NodeVisitor\GroupSequenceResolvingVisitor;
use Symfony\Component\Validator\NodeVisitor\DefaultGroupReplacingVisitor;
use Symfony\Component\Validator\NodeVisitor\NodeValidationVisitor;
use Symfony\Component\Validator\NodeTraverser\NonRecursiveNodeTraverser;
use Symfony\Component\Validator\Validator\Validator;
@ -29,7 +29,7 @@ class Validator2Dot5ApiTest extends Abstract2Dot5ApiTest
$nodeValidator = new NodeValidationVisitor($nodeTraverser, new ConstraintValidatorFactory());
$contextFactory = new ExecutionContextFactory($nodeValidator, new DefaultTranslator());
$validator = new Validator($contextFactory, $nodeTraverser, $metadataFactory);
$groupSequenceResolver = new GroupSequenceResolvingVisitor();
$groupSequenceResolver = new DefaultGroupReplacingVisitor();
$contextRefresher = new ContextUpdateVisitor();
$nodeTraverser->addVisitor($groupSequenceResolver);