[Validator] Allow load mappings from attributes without doctrine/anno…

…tations.
symfony · Nov 7, 2020 · d66cfc3 · d66cfc3
1 parent e8c253a
commit d66cfc3
Show file tree

Hide file tree

Showing 5 changed files with 115 additions and 15 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -34,6 +34,7 @@ CHANGELOG
  * added support for UUIDv6 in `Uuid` constraint
  * enabled the validator to load constraints from PHP attributes
  * deprecated the `NumberConstraintTrait` trait
+ * deprecated setting or creating a Doctrine annotation reader via `ValidatorBuilder::enableAnnotationMapping()`, pass `true` as first parameter and additionally call `setDoctrineAnnotationReader()` or `addDefaultDoctrineAnnotationReader()` to set up the annotation reader
 
 5.1.0
 -----

diff --git a/Tests/Constraints/ValidValidatorTest.php b/Tests/Constraints/ValidValidatorTest.php
@@ -12,7 +12,7 @@ class ValidValidatorTest extends TestCase
     public function testPropertyPathsArePassedToNestedContexts()
     {
         $validatorBuilder = new ValidatorBuilder();
-        $validator = $validatorBuilder->enableAnnotationMapping()->getValidator();
+        $validator = $validatorBuilder->enableAnnotationMapping(true)->addDefaultDoctrineAnnotationReader()->getValidator();
 
         $violations = $validator->validate(new Foo(), null, ['nested']);
 
@@ -23,7 +23,7 @@ public function testPropertyPathsArePassedToNestedContexts()
     public function testNullValues()
     {
         $validatorBuilder = new ValidatorBuilder();
-        $validator = $validatorBuilder->enableAnnotationMapping()->getValidator();
+        $validator = $validatorBuilder->enableAnnotationMapping(true)->addDefaultDoctrineAnnotationReader()->getValidator();
 
         $foo = new Foo();
         $foo->fooBar = null;

diff --git a/Tests/Mapping/Loader/PropertyInfoLoaderTest.php b/Tests/Mapping/Loader/PropertyInfoLoaderTest.php
@@ -89,7 +89,8 @@ public function testLoadClassMetadata()
         $propertyInfoLoader = new PropertyInfoLoader($propertyInfoStub, $propertyInfoStub, $propertyInfoStub, '{.*}');
 
         $validator = Validation::createValidatorBuilder()
-            ->enableAnnotationMapping()
+            ->enableAnnotationMapping(true)
+            ->addDefaultDoctrineAnnotationReader()
             ->addLoader($propertyInfoLoader)
             ->getValidator()
         ;
@@ -220,7 +221,8 @@ public function testClassNoAutoMapping()
 
         $propertyInfoLoader = new PropertyInfoLoader($propertyInfoStub, $propertyInfoStub, $propertyInfoStub, '{.*}');
         $validator = Validation::createValidatorBuilder()
-            ->enableAnnotationMapping()
+            ->enableAnnotationMapping(true)
+            ->addDefaultDoctrineAnnotationReader()
             ->addLoader($propertyInfoLoader)
             ->getValidator()
         ;

diff --git a/Tests/ValidatorBuilderTest.php b/Tests/ValidatorBuilderTest.php
@@ -11,12 +11,18 @@
 
 namespace Symfony\Component\Validator\Tests;
 
+use Doctrine\Common\Annotations\CachedReader;
+use Doctrine\Common\Annotations\Reader;
 use PHPUnit\Framework\TestCase;
 use Psr\Cache\CacheItemPoolInterface;
+use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait;
+use Symfony\Component\Validator\Mapping\Loader\AnnotationLoader;
 use Symfony\Component\Validator\ValidatorBuilder;
 
 class ValidatorBuilderTest extends TestCase
 {
+    use ExpectDeprecationTrait;
+
     /**
      * @var ValidatorBuilder
      */
@@ -74,9 +80,74 @@ public function testAddMethodMappings()
         $this->assertSame($this->builder, $this->builder->addMethodMappings([]));
     }
 
+    /**
+     * @group legacy
+     */
     public function testEnableAnnotationMapping()
     {
+        $this->expectDeprecation('Since symfony/validator 5.2: Not passing true as first argument to "Symfony\Component\Validator\ValidatorBuilder::enableAnnotationMapping" is deprecated. Pass true and call "addDefaultDoctrineAnnotationReader()" if you want to enable annotation mapping with Doctrine Annotations.');
         $this->assertSame($this->builder, $this->builder->enableAnnotationMapping());
+
+        $loaders = $this->builder->getLoaders();
+        $this->assertCount(1, $loaders);
+        $this->assertInstanceOf(AnnotationLoader::class, $loaders[0]);
+
+        $r = new \ReflectionProperty(AnnotationLoader::class, 'reader');
+        $r->setAccessible(true);
+
+        $this->assertInstanceOf(CachedReader::class, $r->getValue($loaders[0]));
+    }
+
+    public function testEnableAnnotationMappingWithDefaultDoctrineAnnotationReader()
+    {
+        $this->assertSame($this->builder, $this->builder->enableAnnotationMapping(true));
+        $this->assertSame($this->builder, $this->builder->addDefaultDoctrineAnnotationReader());
+
+        $loaders = $this->builder->getLoaders();
+        $this->assertCount(1, $loaders);
+        $this->assertInstanceOf(AnnotationLoader::class, $loaders[0]);
+
+        $r = new \ReflectionProperty(AnnotationLoader::class, 'reader');
+        $r->setAccessible(true);
+
+        $this->assertInstanceOf(CachedReader::class, $r->getValue($loaders[0]));
+    }
+
+    /**
+     * @group legacy
+     */
+    public function testEnableAnnotationMappingWithCustomDoctrineAnnotationReaderLegacy()
+    {
+        $reader = $this->createMock(Reader::class);
+
+        $this->expectDeprecation('Since symfony/validator 5.2: Passing an instance of "'.\get_class($reader).'" as first argument to "Symfony\Component\Validator\ValidatorBuilder::enableAnnotationMapping" is deprecated. Pass true instead and call setDoctrineAnnotationReader() if you want to enable annotation mapping with Doctrine Annotations.');
+        $this->assertSame($this->builder, $this->builder->enableAnnotationMapping($reader));
+
+        $loaders = $this->builder->getLoaders();
+        $this->assertCount(1, $loaders);
+        $this->assertInstanceOf(AnnotationLoader::class, $loaders[0]);
+
+        $r = new \ReflectionProperty(AnnotationLoader::class, 'reader');
+        $r->setAccessible(true);
+
+        $this->assertSame($reader, $r->getValue($loaders[0]));
+    }
+
+    public function testEnableAnnotationMappingWithCustomDoctrineAnnotationReader()
+    {
+        $reader = $this->createMock(Reader::class);
+
+        $this->assertSame($this->builder, $this->builder->enableAnnotationMapping(true));
+        $this->assertSame($this->builder, $this->builder->setDoctrineAnnotationReader($reader));
+
+        $loaders = $this->builder->getLoaders();
+        $this->assertCount(1, $loaders);
+        $this->assertInstanceOf(AnnotationLoader::class, $loaders[0]);
+
+        $r = new \ReflectionProperty(AnnotationLoader::class, 'reader');
+        $r->setAccessible(true);
+
+        $this->assertSame($reader, $r->getValue($loaders[0]));
     }
 
     public function testDisableAnnotationMapping()

diff --git a/ValidatorBuilder.php b/ValidatorBuilder.php
@@ -17,7 +17,6 @@
 use Doctrine\Common\Cache\ArrayCache;
 use Psr\Cache\CacheItemPoolInterface;
 use Symfony\Component\Validator\Context\ExecutionContextFactory;
-use Symfony\Component\Validator\Exception\LogicException;
 use Symfony\Component\Validator\Exception\ValidatorException;
 use Symfony\Component\Validator\Mapping\Factory\LazyLoadingMetadataFactory;
 use Symfony\Component\Validator\Mapping\Factory\MetadataFactoryInterface;
@@ -53,6 +52,7 @@ class ValidatorBuilder
      * @var Reader|null
      */
     private $annotationReader;
+    private $enableAnnotationMapping = false;
 
     /**
      * @var MetadataFactoryInterface|null
@@ -212,23 +212,28 @@ public function addMethodMappings(array $methodNames)
     /**
      * Enables annotation based constraint mapping.
      *
+     * @param bool $skipDoctrineAnnotations
+     *
      * @return $this
      */
-    public function enableAnnotationMapping(Reader $annotationReader = null)
+    public function enableAnnotationMapping(/* bool $skipDoctrineAnnotations = true */)
     {
         if (null !== $this->metadataFactory) {
             throw new ValidatorException('You cannot enable annotation mapping after setting a custom metadata factory. Configure your metadata factory instead.');
         }
 
-        if (null === $annotationReader) {
-            if (!class_exists(AnnotationReader::class) || !class_exists(ArrayCache::class)) {
-                throw new LogicException('Enabling annotation based constraint mapping requires the packages doctrine/annotations and doctrine/cache to be installed.');
-            }
-
-            $annotationReader = new CachedReader(new AnnotationReader(), new ArrayCache());
+        $skipDoctrineAnnotations = 1 > \func_num_args() ? false : func_get_arg(0);
+        if (false === $skipDoctrineAnnotations || null === $skipDoctrineAnnotations) {
+            trigger_deprecation('symfony/validator', '5.2', 'Not passing true as first argument to "%s" is deprecated. Pass true and call "addDefaultDoctrineAnnotationReader()" if you want to enable annotation mapping with Doctrine Annotations.', __METHOD__);
+            $this->addDefaultDoctrineAnnotationReader();
+        } elseif ($skipDoctrineAnnotations instanceof Reader) {
+            trigger_deprecation('symfony/validator', '5.2', 'Passing an instance of "%s" as first argument to "%s" is deprecated. Pass true instead and call setDoctrineAnnotationReader() if you want to enable annotation mapping with Doctrine Annotations.', get_debug_type($skipDoctrineAnnotations), __METHOD__);
+            $this->setDoctrineAnnotationReader($skipDoctrineAnnotations);
+        } elseif (true !== $skipDoctrineAnnotations) {
+            throw new \TypeError(sprintf('"%s": Argument 1 is expected to be a boolean, "%s" given.', __METHOD__, get_debug_type($skipDoctrineAnnotations)));
         }
 
-        $this->annotationReader = $annotationReader;
+        $this->enableAnnotationMapping = true;
 
         return $this;
     }
@@ -240,19 +245,40 @@ public function enableAnnotationMapping(Reader $annotationReader = null)
      */
     public function disableAnnotationMapping()
     {
+        $this->enableAnnotationMapping = false;
         $this->annotationReader = null;
 
         return $this;
     }
 
+    /**
+     * @return $this
+     */
+    public function setDoctrineAnnotationReader(?Reader $reader): self
+    {
+        $this->annotationReader = $reader;
+
+        return $this;
+    }
+
+    /**
+     * @return $this
+     */
+    public function addDefaultDoctrineAnnotationReader(): self
+    {
+        $this->annotationReader = new CachedReader(new AnnotationReader(), new ArrayCache());
+
+        return $this;
+    }
+
     /**
      * Sets the class metadata factory used by the validator.
      *
      * @return $this
      */
     public function setMetadataFactory(MetadataFactoryInterface $metadataFactory)
     {
-        if (\count($this->xmlMappings) > 0 || \count($this->yamlMappings) > 0 || \count($this->methodMappings) > 0 || null !== $this->annotationReader) {
+        if (\count($this->xmlMappings) > 0 || \count($this->yamlMappings) > 0 || \count($this->methodMappings) > 0 || $this->enableAnnotationMapping) {
             throw new ValidatorException('You cannot set a custom metadata factory after adding custom mappings. You should do either of both.');
         }
 
@@ -346,7 +372,7 @@ public function getLoaders()
             $loaders[] = new StaticMethodLoader($methodName);
         }
 
-        if ($this->annotationReader) {
+        if ($this->enableAnnotationMapping) {
             $loaders[] = new AnnotationLoader($this->annotationReader);
         }