From cccb1db2b2d9a6a96b7c005737ba266a3d9f2352 Mon Sep 17 00:00:00 2001 From: Bernhard Schussek Date: Thu, 26 Sep 2013 11:03:13 +0200 Subject: [PATCH] [Validator] Simplified usage of the Callback constraint --- UPGRADE-3.0.md | 58 +++++ .../Validator/Constraints/Callback.php | 38 ++- .../Constraints/CallbackValidator.php | 19 +- .../Mapping/Loader/AnnotationLoader.php | 8 +- .../Constraints/CallbackValidatorTest.php | 232 ++++++++++++++++-- .../Tests/Fixtures/CallbackClass.php | 24 ++ .../Validator/Tests/Fixtures/Entity.php | 16 ++ .../Mapping/Loader/AnnotationLoaderTest.php | 7 + .../Mapping/Loader/XmlFileLoaderTest.php | 4 + .../Mapping/Loader/YamlFileLoaderTest.php | 4 + .../Mapping/Loader/constraint-mapping.xml | 10 + .../Mapping/Loader/constraint-mapping.yml | 4 + 12 files changed, 393 insertions(+), 31 deletions(-) create mode 100644 src/Symfony/Component/Validator/Tests/Fixtures/CallbackClass.php diff --git a/UPGRADE-3.0.md b/UPGRADE-3.0.md index 880870a1cf..85cb84a8cc 100644 --- a/UPGRADE-3.0.md +++ b/UPGRADE-3.0.md @@ -425,6 +425,64 @@ UPGRADE FROM 2.x to 3.0 private $property; ``` + * The option "methods" of the `Callback` constraint was removed. You should + use the option "callback" instead. If you have multiple callbacks, add + multiple callback constraints instead. + + Before (YAML): + + ``` + constraints: + - Callback: [firstCallback, secondCallback] + ``` + + After (YAML): + + ``` + constraints: + - Callback: firstCallback + - Callback: secondCallback + ``` + + When using annotations, you can now put the Callback constraint directly on + the method that should be executed. + + Before (Annotations): + + ``` + use Symfony\Component\Validator\Constraints as Assert; + use Symfony\Component\Validator\ExecutionContextInterface; + + /** + * @Assert\Callback({"callback"}) + */ + class MyClass + { + public function callback(ExecutionContextInterface $context) + { + // ... + } + } + ``` + + After (Annotations): + + ``` + use Symfony\Component\Validator\Constraints as Assert; + use Symfony\Component\Validator\ExecutionContextInterface; + + class MyClass + { + /** + * @Assert\Callback + */ + public function callback(ExecutionContextInterface $context) + { + // ... + } + } + ``` + ### Yaml * The ability to pass file names to `Yaml::parse()` has been removed. diff --git a/src/Symfony/Component/Validator/Constraints/Callback.php b/src/Symfony/Component/Validator/Constraints/Callback.php index e93efa4b99..01aeb6ddb7 100644 --- a/src/Symfony/Component/Validator/Constraints/Callback.php +++ b/src/Symfony/Component/Validator/Constraints/Callback.php @@ -22,26 +22,52 @@ use Symfony\Component\Validator\Constraint; */ class Callback extends Constraint { + /** + * @var string|callable + * + * @since 2.4 + */ + public $callback; + + /** + * @var array + * + * @deprecated Deprecated since version 2.4, to be removed in Symfony 3.0. + */ public $methods; /** - * {@inheritDoc} + * {@inheritdoc} */ - public function getRequiredOptions() + public function __construct($options = null) { - return array('methods'); + // Invocation through annotations with an array parameter only + if (is_array($options) && 1 === count($options) && isset($options['value'])) { + $options = $options['value']; + } + + if (is_array($options) && !isset($options['callback']) && !isset($options['methods']) && !isset($options['groups'])) { + if (is_callable($options)) { + $options = array('callback' => $options); + } else { + // BC with Symfony < 2.4 + $options = array('methods' => $options); + } + } + + parent::__construct($options); } /** - * {@inheritDoc} + * {@inheritdoc} */ public function getDefaultOption() { - return 'methods'; + return 'callback'; } /** - * {@inheritDoc} + * {@inheritdoc} */ public function getTargets() { diff --git a/src/Symfony/Component/Validator/Constraints/CallbackValidator.php b/src/Symfony/Component/Validator/Constraints/CallbackValidator.php index ab3c56f98e..28b34250af 100644 --- a/src/Symfony/Component/Validator/Constraints/CallbackValidator.php +++ b/src/Symfony/Component/Validator/Constraints/CallbackValidator.php @@ -34,13 +34,20 @@ class CallbackValidator extends ConstraintValidator return; } + if (null !== $constraint->callback && null !== $constraint->methods) { + throw new ConstraintDefinitionException( + 'The Callback constraint supports either the option "callback" ' . + 'or "methods", but not both at the same time.' + ); + } + // has to be an array so that we can differentiate between callables // and method names - if (!is_array($constraint->methods)) { + if (null !== $constraint->methods && !is_array($constraint->methods)) { throw new UnexpectedTypeException($constraint->methods, 'array'); } - $methods = $constraint->methods; + $methods = $constraint->methods ?: array($constraint->callback); foreach ($methods as $method) { if (is_array($method) || $method instanceof \Closure) { @@ -54,7 +61,13 @@ class CallbackValidator extends ConstraintValidator throw new ConstraintDefinitionException(sprintf('Method "%s" targeted by Callback constraint does not exist', $method)); } - $object->$method($this->context); + $reflMethod = new \ReflectionMethod($object, $method); + + if ($reflMethod->isStatic()) { + $reflMethod->invoke(null, $object, $this->context); + } else { + $reflMethod->invoke($object, $this->context); + } } } } diff --git a/src/Symfony/Component/Validator/Mapping/Loader/AnnotationLoader.php b/src/Symfony/Component/Validator/Mapping/Loader/AnnotationLoader.php index 0e7e89b548..10745c72e7 100644 --- a/src/Symfony/Component/Validator/Mapping/Loader/AnnotationLoader.php +++ b/src/Symfony/Component/Validator/Mapping/Loader/AnnotationLoader.php @@ -12,6 +12,7 @@ namespace Symfony\Component\Validator\Mapping\Loader; use Doctrine\Common\Annotations\Reader; +use Symfony\Component\Validator\Constraints\Callback; use Symfony\Component\Validator\Exception\MappingException; use Symfony\Component\Validator\Mapping\ClassMetadata; use Symfony\Component\Validator\Constraints\GroupSequence; @@ -63,7 +64,12 @@ class AnnotationLoader implements LoaderInterface foreach ($reflClass->getMethods() as $method) { if ($method->getDeclaringClass()->name == $className) { foreach ($this->reader->getMethodAnnotations($method) as $constraint) { - if ($constraint instanceof Constraint) { + if ($constraint instanceof Callback) { + $constraint->callback = $method->getName(); + $constraint->methods = null; + + $metadata->addConstraint($constraint); + } elseif ($constraint instanceof Constraint) { if (preg_match('/^(get|is)(.+)$/i', $method->name, $matches)) { $metadata->addGetterConstraint(lcfirst($matches[2]), $constraint); } else { diff --git a/src/Symfony/Component/Validator/Tests/Constraints/CallbackValidatorTest.php b/src/Symfony/Component/Validator/Tests/Constraints/CallbackValidatorTest.php index 4d248c13c8..cdcd49bb58 100644 --- a/src/Symfony/Component/Validator/Tests/Constraints/CallbackValidatorTest.php +++ b/src/Symfony/Component/Validator/Tests/Constraints/CallbackValidatorTest.php @@ -17,9 +17,9 @@ use Symfony\Component\Validator\Constraints\CallbackValidator; class CallbackValidatorTest_Class { - public static function validateStatic($object, ExecutionContext $context) + public static function validateCallback($object, ExecutionContext $context) { - $context->addViolation('Static message', array('{{ value }}' => 'foobar'), 'invalidValue'); + $context->addViolation('Callback message', array('{{ value }}' => 'foobar'), 'invalidValue'); return false; } @@ -27,16 +27,16 @@ class CallbackValidatorTest_Class class CallbackValidatorTest_Object { - public function validateOne(ExecutionContext $context) + public function validate(ExecutionContext $context) { $context->addViolation('My message', array('{{ value }}' => 'foobar'), 'invalidValue'); return false; } - public function validateTwo(ExecutionContext $context) + public static function validateStatic($object, ExecutionContext $context) { - $context->addViolation('Other message', array('{{ value }}' => 'baz'), 'otherInvalidValue'); + $context->addViolation('Static message', array('{{ value }}' => 'baz'), 'otherInvalidValue'); return false; } @@ -68,10 +68,10 @@ class CallbackValidatorTest extends \PHPUnit_Framework_TestCase $this->validator->validate(null, new Callback(array('foo'))); } - public function testCallbackSingleMethod() + public function testSingleMethod() { $object = new CallbackValidatorTest_Object(); - $constraint = new Callback(array('validateOne')); + $constraint = new Callback('validate'); $this->context->expects($this->once()) ->method('addViolation') @@ -82,24 +82,137 @@ class CallbackValidatorTest extends \PHPUnit_Framework_TestCase $this->validator->validate($object, $constraint); } - public function testCallbackSingleStaticMethod() + public function testSingleMethodExplicitName() { $object = new CallbackValidatorTest_Object(); + $constraint = new Callback(array('callback' => 'validate')); + + $this->context->expects($this->once()) + ->method('addViolation') + ->with('My message', array( + '{{ value }}' => 'foobar', + )); + + $this->validator->validate($object, $constraint); + } + + public function testSingleStaticMethod() + { + $object = new CallbackValidatorTest_Object(); + $constraint = new Callback('validateStatic'); $this->context->expects($this->once()) ->method('addViolation') ->with('Static message', array( + '{{ value }}' => 'baz', + )); + + $this->validator->validate($object, $constraint); + } + + public function testClosure() + { + $object = new CallbackValidatorTest_Object(); + $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($object, new Callback(array( - array(__CLASS__.'_Class', 'validateStatic') - ))); + $this->validator->validate($object, $constraint); } - public function testCallbackMultipleMethods() + public function testClosureExplicitName() { $object = new CallbackValidatorTest_Object(); + $constraint = new Callback(array( + '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($object, $constraint); + } + + public function testArrayCallable() + { + $object = new CallbackValidatorTest_Object(); + $constraint = new Callback(array(__CLASS__.'_Class', 'validateCallback')); + + $this->context->expects($this->once()) + ->method('addViolation') + ->with('Callback message', array( + '{{ value }}' => 'foobar', + )); + + $this->validator->validate($object, $constraint); + } + + public function testArrayCallableExplicitName() + { + $object = new CallbackValidatorTest_Object(); + $constraint = new Callback(array( + 'callback' => array(__CLASS__.'_Class', 'validateCallback'), + )); + + $this->context->expects($this->once()) + ->method('addViolation') + ->with('Callback message', array( + '{{ value }}' => 'foobar', + )); + + $this->validator->validate($object, $constraint); + } + + // BC with Symfony < 2.4 + public function testSingleMethodBc() + { + $object = new CallbackValidatorTest_Object(); + $constraint = new Callback(array('validate')); + + $this->context->expects($this->once()) + ->method('addViolation') + ->with('My message', array( + '{{ value }}' => 'foobar', + )); + + $this->validator->validate($object, $constraint); + } + + // BC with Symfony < 2.4 + public function testSingleMethodBcExplicitName() + { + $object = new CallbackValidatorTest_Object(); + $constraint = new Callback(array('methods' => array('validate'))); + + $this->context->expects($this->once()) + ->method('addViolation') + ->with('My message', array( + '{{ value }}' => 'foobar', + )); + + $this->validator->validate($object, $constraint); + } + + // BC with Symfony < 2.4 + public function testMultipleMethodsBc() + { + $object = new CallbackValidatorTest_Object(); + $constraint = new Callback(array('validate', 'validateStatic')); $this->context->expects($this->at(0)) ->method('addViolation') @@ -108,23 +221,67 @@ class CallbackValidatorTest extends \PHPUnit_Framework_TestCase )); $this->context->expects($this->at(1)) ->method('addViolation') - ->with('Other message', array( + ->with('Static message', array( '{{ value }}' => 'baz', )); - $this->validator->validate($object, new Callback(array( - 'validateOne', 'validateTwo' - ))); + $this->validator->validate($object, $constraint); } - /** - * @expectedException \Symfony\Component\Validator\Exception\UnexpectedTypeException - */ - public function testExpectCallbackArray() + // BC with Symfony < 2.4 + public function testMultipleMethodsBcExplicitName() { $object = new CallbackValidatorTest_Object(); + $constraint = new Callback(array( + 'methods' => array('validate', 'validateStatic'), + )); - $this->validator->validate($object, new Callback('foobar')); + $this->context->expects($this->at(0)) + ->method('addViolation') + ->with('My message', array( + '{{ value }}' => 'foobar', + )); + $this->context->expects($this->at(1)) + ->method('addViolation') + ->with('Static message', array( + '{{ value }}' => 'baz', + )); + + $this->validator->validate($object, $constraint); + } + + // BC with Symfony < 2.4 + public function testSingleStaticMethodBc() + { + $object = new CallbackValidatorTest_Object(); + $constraint = new Callback(array( + array(__CLASS__.'_Class', 'validateCallback') + )); + + $this->context->expects($this->once()) + ->method('addViolation') + ->with('Callback message', array( + '{{ value }}' => 'foobar', + )); + + $this->validator->validate($object, $constraint); + } + + // BC with Symfony < 2.4 + public function testSingleStaticMethodBcExplicitName() + { + $object = new CallbackValidatorTest_Object(); + $constraint = new Callback(array( + 'methods' => array(array(__CLASS__.'_Class', 'validateCallback')), + )); + + $this->context->expects($this->once()) + ->method('addViolation') + ->with('Callback message', array( + '{{ value }}' => 'foobar', + )); + + $this->validator->validate($object, $constraint); } /** @@ -147,10 +304,43 @@ class CallbackValidatorTest extends \PHPUnit_Framework_TestCase $this->validator->validate($object, new Callback(array(array('foo', 'bar')))); } + /** + * @expectedException \Symfony\Component\Validator\Exception\ConstraintDefinitionException + */ + public function testExpectEitherCallbackOrMethods() + { + $object = new CallbackValidatorTest_Object(); + + $this->validator->validate($object, new Callback(array( + 'callback' => 'validate', + 'methods' => array('validateStatic'), + ))); + } + public function testConstraintGetTargets() { $constraint = new Callback(array('foo')); $this->assertEquals('class', $constraint->getTargets()); } + + // Should succeed. Needed when defining constraints as annotations. + public function testNoConstructorArguments() + { + new Callback(); + } + + public function testAnnotationInvocationSingleValued() + { + $constraint = new Callback(array('value' => 'validateStatic')); + + $this->assertEquals(new Callback('validateStatic'), $constraint); + } + + public function testAnnotationInvocationMultiValued() + { + $constraint = new Callback(array('value' => array(__CLASS__.'_Class', 'validateCallback'))); + + $this->assertEquals(new Callback(array(__CLASS__.'_Class', 'validateCallback')), $constraint); + } } diff --git a/src/Symfony/Component/Validator/Tests/Fixtures/CallbackClass.php b/src/Symfony/Component/Validator/Tests/Fixtures/CallbackClass.php new file mode 100644 index 0000000000..0f6a2f4ae3 --- /dev/null +++ b/src/Symfony/Component/Validator/Tests/Fixtures/CallbackClass.php @@ -0,0 +1,24 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Validator\Tests\Fixtures; + +use Symfony\Component\Validator\ExecutionContextInterface; + +/** + * @author Bernhard Schussek + */ +class CallbackClass +{ + public static function callback($object, ExecutionContextInterface $context) + { + } +} diff --git a/src/Symfony/Component/Validator/Tests/Fixtures/Entity.php b/src/Symfony/Component/Validator/Tests/Fixtures/Entity.php index e1cb3e0490..70bdc5aec6 100644 --- a/src/Symfony/Component/Validator/Tests/Fixtures/Entity.php +++ b/src/Symfony/Component/Validator/Tests/Fixtures/Entity.php @@ -12,10 +12,12 @@ namespace Symfony\Component\Validator\Tests\Fixtures; use Symfony\Component\Validator\Constraints as Assert; +use Symfony\Component\Validator\ExecutionContextInterface; /** * @Symfony\Component\Validator\Tests\Fixtures\ConstraintA * @Assert\GroupSequence({"Foo", "Entity"}) + * @Assert\Callback({"Symfony\Component\Validator\Tests\Fixtures\CallbackClass", "callback"}) */ class Entity extends EntityParent implements EntityInterface { @@ -58,4 +60,18 @@ class Entity extends EntityParent implements EntityInterface { return 'Overridden data'; } + + /** + * @Assert\Callback + */ + public function validateMe(ExecutionContextInterface $context) + { + } + + /** + * @Assert\Callback + */ + public static function validateMeStatic($object, ExecutionContextInterface $context) + { + } } diff --git a/src/Symfony/Component/Validator/Tests/Mapping/Loader/AnnotationLoaderTest.php b/src/Symfony/Component/Validator/Tests/Mapping/Loader/AnnotationLoaderTest.php index 731ab83561..0d255b8fca 100644 --- a/src/Symfony/Component/Validator/Tests/Mapping/Loader/AnnotationLoaderTest.php +++ b/src/Symfony/Component/Validator/Tests/Mapping/Loader/AnnotationLoaderTest.php @@ -13,6 +13,7 @@ namespace Symfony\Component\Validator\Tests\Mapping\Loader; use Doctrine\Common\Annotations\AnnotationReader; use Symfony\Component\Validator\Constraints\All; +use Symfony\Component\Validator\Constraints\Callback; use Symfony\Component\Validator\Constraints\Collection; use Symfony\Component\Validator\Constraints\NotNull; use Symfony\Component\Validator\Constraints\Range; @@ -50,6 +51,9 @@ class AnnotationLoaderTest extends \PHPUnit_Framework_TestCase $expected = new ClassMetadata('Symfony\Component\Validator\Tests\Fixtures\Entity'); $expected->setGroupSequence(array('Foo', 'Entity')); $expected->addConstraint(new ConstraintA()); + $expected->addConstraint(new Callback(array('Symfony\Component\Validator\Tests\Fixtures\CallbackClass', 'callback'))); + $expected->addConstraint(new Callback('validateMe')); + $expected->addConstraint(new Callback('validateMeStatic')); $expected->addPropertyConstraint('firstName', new NotNull()); $expected->addPropertyConstraint('firstName', new Range(array('min' => 3))); $expected->addPropertyConstraint('firstName', new All(array(new NotNull(), new Range(array('min' => 3))))); @@ -114,6 +118,9 @@ class AnnotationLoaderTest extends \PHPUnit_Framework_TestCase $expected->setGroupSequence(array('Foo', 'Entity')); $expected->addConstraint(new ConstraintA()); + $expected->addConstraint(new Callback(array('Symfony\Component\Validator\Tests\Fixtures\CallbackClass', 'callback'))); + $expected->addConstraint(new Callback('validateMe')); + $expected->addConstraint(new Callback('validateMeStatic')); $expected->addPropertyConstraint('firstName', new NotNull()); $expected->addPropertyConstraint('firstName', new Range(array('min' => 3))); $expected->addPropertyConstraint('firstName', new All(array(new NotNull(), new Range(array('min' => 3))))); diff --git a/src/Symfony/Component/Validator/Tests/Mapping/Loader/XmlFileLoaderTest.php b/src/Symfony/Component/Validator/Tests/Mapping/Loader/XmlFileLoaderTest.php index 7c6e355bd1..82195405e6 100644 --- a/src/Symfony/Component/Validator/Tests/Mapping/Loader/XmlFileLoaderTest.php +++ b/src/Symfony/Component/Validator/Tests/Mapping/Loader/XmlFileLoaderTest.php @@ -12,6 +12,7 @@ namespace Symfony\Component\Validator\Tests\Mapping\Loader; use Symfony\Component\Validator\Constraints\All; +use Symfony\Component\Validator\Constraints\Callback; use Symfony\Component\Validator\Constraints\Collection; use Symfony\Component\Validator\Constraints\NotNull; use Symfony\Component\Validator\Constraints\Range; @@ -51,6 +52,9 @@ class XmlFileLoaderTest extends \PHPUnit_Framework_TestCase $expected->setGroupSequence(array('Foo', 'Entity')); $expected->addConstraint(new ConstraintA()); $expected->addConstraint(new ConstraintB()); + $expected->addConstraint(new Callback('validateMe')); + $expected->addConstraint(new Callback('validateMeStatic')); + $expected->addConstraint(new Callback(array('Symfony\Component\Validator\Tests\Fixtures\CallbackClass', 'callback'))); $expected->addPropertyConstraint('firstName', new NotNull()); $expected->addPropertyConstraint('firstName', new Range(array('min' => 3))); $expected->addPropertyConstraint('firstName', new Choice(array('A', 'B'))); diff --git a/src/Symfony/Component/Validator/Tests/Mapping/Loader/YamlFileLoaderTest.php b/src/Symfony/Component/Validator/Tests/Mapping/Loader/YamlFileLoaderTest.php index dd394acaf0..0d9a0b62c3 100644 --- a/src/Symfony/Component/Validator/Tests/Mapping/Loader/YamlFileLoaderTest.php +++ b/src/Symfony/Component/Validator/Tests/Mapping/Loader/YamlFileLoaderTest.php @@ -12,6 +12,7 @@ namespace Symfony\Component\Validator\Tests\Mapping\Loader; use Symfony\Component\Validator\Constraints\All; +use Symfony\Component\Validator\Constraints\Callback; use Symfony\Component\Validator\Constraints\Collection; use Symfony\Component\Validator\Constraints\NotNull; use Symfony\Component\Validator\Constraints\Range; @@ -68,6 +69,9 @@ class YamlFileLoaderTest extends \PHPUnit_Framework_TestCase $expected->setGroupSequence(array('Foo', 'Entity')); $expected->addConstraint(new ConstraintA()); $expected->addConstraint(new ConstraintB()); + $expected->addConstraint(new Callback('validateMe')); + $expected->addConstraint(new Callback('validateMeStatic')); + $expected->addConstraint(new Callback(array('Symfony\Component\Validator\Tests\Fixtures\CallbackClass', 'callback'))); $expected->addPropertyConstraint('firstName', new NotNull()); $expected->addPropertyConstraint('firstName', new Range(array('min' => 3))); $expected->addPropertyConstraint('firstName', new Choice(array('A', 'B'))); diff --git a/src/Symfony/Component/Validator/Tests/Mapping/Loader/constraint-mapping.xml b/src/Symfony/Component/Validator/Tests/Mapping/Loader/constraint-mapping.xml index dfac70d9cf..1eee1cb180 100644 --- a/src/Symfony/Component/Validator/Tests/Mapping/Loader/constraint-mapping.xml +++ b/src/Symfony/Component/Validator/Tests/Mapping/Loader/constraint-mapping.xml @@ -21,6 +21,16 @@ + + validateMe + + validateMeStatic + + + Symfony\Component\Validator\Tests\Fixtures\CallbackClass + callback + + diff --git a/src/Symfony/Component/Validator/Tests/Mapping/Loader/constraint-mapping.yml b/src/Symfony/Component/Validator/Tests/Mapping/Loader/constraint-mapping.yml index 38188dc5d9..e52d3f04b2 100644 --- a/src/Symfony/Component/Validator/Tests/Mapping/Loader/constraint-mapping.yml +++ b/src/Symfony/Component/Validator/Tests/Mapping/Loader/constraint-mapping.yml @@ -11,6 +11,10 @@ Symfony\Component\Validator\Tests\Fixtures\Entity: - Symfony\Component\Validator\Tests\Fixtures\ConstraintA: ~ # Custom constraint with namespaces prefix - "custom:ConstraintB": ~ + # Callbacks + - Callback: validateMe + - Callback: validateMeStatic + - Callback: [Symfony\Component\Validator\Tests\Fixtures\CallbackClass, callback] properties: firstName: