Skip to content

Commit 77c82cf

Browse files
fabpotfabpot
committed
feature #39032 [Validator] Allow load mappings from attributes without doctrine/annotations (derrabus)
This PR was merged into the 5.2-dev branch. Discussion ---------- [Validator] Allow load mappings from attributes without doctrine/annotations | Q | A | ------------- | --- | Branch? | 5.2 | Bug fix? | no | New feature? | yes | Deprecations? | yes | Tickets | #38096 | License | MIT | Doc PR | TODO Follows #38309. Currently, we cannot enable constraint mapping from attributes without having `doctrine/annotations` installed. Lifting that limitation is a bit tricky because `ValidatorBuilder::enableAnnotationMapping()` creates an annotation reader if you don't pass one. This PR aims at deprecating this behavior. I know it's a bit late for such a change in 5.2 and I should have seen earlier that this part was missing. 😓 Since I don't expect people to go all-in on attributes on day one, it's probably okay to postpone this change to 5.3. Commits ------- 441c806 [Validator] Allow load mappings from attributes without doctrine/annotations.
2 parents 7b4d22f + 441c806 commit 77c82cf

File tree

11 files changed

+206
-36
lines changed

11 files changed

+206
-36
lines changed

‎UPGRADE-5.2.md‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,36 @@ Validator
126126

127127
* Deprecated the `NumberConstraintTrait` trait.
128128

129+
* Deprecated setting a Doctrine annotation reader via `ValidatorBuilder::enableAnnotationMapping()`
130+
131+
Before:
132+
133+
```php
134+
$builder->enableAnnotationMapping($reader);
135+
```
136+
137+
After:
138+
139+
```php
140+
$builder->enableAnnotationMapping(true)
141+
->setDoctrineAnnotationReader($reader);
142+
```
143+
144+
* Deprecated creating a Doctrine annotation reader via `ValidatorBuilder::enableAnnotationMapping()`
145+
146+
Before:
147+
148+
```php
149+
$builder->enableAnnotationMapping();
150+
```
151+
152+
After:
153+
154+
```php
155+
$builder->enableAnnotationMapping(true)
156+
->addDefaultDoctrineAnnotationReader();
157+
```
158+
129159
Security
130160
--------
131161

‎UPGRADE-6.0.md‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,36 @@ Validator
189189

190190
* Removed the `NumberConstraintTrait` trait.
191191

192+
* `ValidatorBuilder::enableAnnotationMapping()` does not accept a Doctrine annotation reader anymore.
193+
194+
Before:
195+
196+
```php
197+
$builder->enableAnnotationMapping($reader);
198+
```
199+
200+
After:
201+
202+
```php
203+
$builder->enableAnnotationMapping(true)
204+
->setDoctrineAnnotationReader($reader);
205+
```
206+
207+
* `ValidatorBuilder::enableAnnotationMapping()` won't automatically setup a Doctrine annotation reader anymore.
208+
209+
Before:
210+
211+
```php
212+
$builder->enableAnnotationMapping();
213+
```
214+
215+
After:
216+
217+
```php
218+
$builder->enableAnnotationMapping(true)
219+
->addDefaultDoctrineAnnotationReader();
220+
```
221+
192222
Yaml
193223
----
194224

‎src/Symfony/Bridge/Doctrine/Tests/Validator/DoctrineLoaderTest.php‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,8 @@ protected function setUp(): void
4646
public function testLoadClassMetadata()
4747
{
4848
$validator = Validation::createValidatorBuilder()
49-
->enableAnnotationMapping()
49+
->enableAnnotationMapping(true)
50+
->addDefaultDoctrineAnnotationReader()
5051
->addLoader(new DoctrineLoader(DoctrineTestHelper::createTestEntityManager(), '{^Symfony\\\\Bridge\\\\Doctrine\\\\Tests\\\\Fixtures\\\\DoctrineLoader}'))
5152
->getValidator()
5253
;
@@ -151,7 +152,8 @@ public function testLoadClassMetadata()
151152
public function testFieldMappingsConfiguration()
152153
{
153154
$validator = Validation::createValidatorBuilder()
154-
->enableAnnotationMapping()
155+
->enableAnnotationMapping(true)
156+
->addDefaultDoctrineAnnotationReader()
155157
->addXmlMappings([__DIR__.'/../Resources/validator/BaseUser.xml'])
156158
->addLoader(
157159
new DoctrineLoader(
@@ -192,7 +194,8 @@ public function regexpProvider()
192194
public function testClassNoAutoMapping()
193195
{
194196
$validator = Validation::createValidatorBuilder()
195-
->enableAnnotationMapping()
197+
->enableAnnotationMapping(true)
198+
->addDefaultDoctrineAnnotationReader()
196199
->addLoader(new DoctrineLoader(DoctrineTestHelper::createTestEntityManager(), '{.*}'))
197200
->getValidator();
198201

‎src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1303,11 +1303,14 @@ private function registerValidationConfiguration(array $config, ContainerBuilder
13031303
$definition->replaceArgument(0, $config['email_validation_mode']);
13041304

13051305
if (\array_key_exists('enable_annotations', $config) && $config['enable_annotations']) {
1306-
if (!$this->annotationsConfigEnabled) {
1307-
throw new \LogicException('"enable_annotations" on the validator cannot be set as Annotations support is disabled.');
1306+
if (!$this->annotationsConfigEnabled && \PHP_VERSION_ID < 80000) {
1307+
throw new \LogicException('"enable_annotations" on the validator cannot be set as Doctrine Annotations support is disabled.');
13081308
}
13091309

1310-
$validatorBuilder->addMethodCall('enableAnnotationMapping', [new Reference('annotation_reader')]);
1310+
$validatorBuilder->addMethodCall('enableAnnotationMapping', [true]);
1311+
if ($this->annotationsConfigEnabled) {
1312+
$validatorBuilder->addMethodCall('setDoctrineAnnotationReader', [new Reference('annotation_reader')]);
1313+
}
13111314
}
13121315

13131316
if (\array_key_exists('static_method', $config) && $config['static_method']) {

‎src/Symfony/Bundle/FrameworkBundle/Tests/CacheWarmer/ValidatorCacheWarmerTest.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ public function testWarmUp()
2626
$validatorBuilder->addXmlMapping(__DIR__.'/../Fixtures/Validation/Resources/person.xml');
2727
$validatorBuilder->addYamlMapping(__DIR__.'/../Fixtures/Validation/Resources/author.yml');
2828
$validatorBuilder->addMethodMapping('loadValidatorMetadata');
29-
$validatorBuilder->enableAnnotationMapping();
29+
$validatorBuilder->enableAnnotationMapping(true)->addDefaultDoctrineAnnotationReader();
3030

3131
$file = sys_get_temp_dir().'/cache-validator.php';
3232
@unlink($file);
@@ -46,7 +46,7 @@ public function testWarmUpWithAnnotations()
4646
{
4747
$validatorBuilder = new ValidatorBuilder();
4848
$validatorBuilder->addYamlMapping(__DIR__.'/../Fixtures/Validation/Resources/categories.yml');
49-
$validatorBuilder->enableAnnotationMapping();
49+
$validatorBuilder->enableAnnotationMapping(true)->addDefaultDoctrineAnnotationReader();
5050

5151
$file = sys_get_temp_dir().'/cache-validator-with-annotations.php';
5252
@unlink($file);

‎src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php‎

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -870,7 +870,7 @@ public function testValidation()
870870

871871
$annotations = !class_exists(FullStack::class) && class_exists(Annotation::class);
872872

873-
$this->assertCount($annotations ? 7 : 6, $calls);
873+
$this->assertCount($annotations ? 8 : 6, $calls);
874874
$this->assertSame('setConstraintValidatorFactory', $calls[0][0]);
875875
$this->assertEquals([new Reference('validator.validator_factory')], $calls[0][1]);
876876
$this->assertSame('setTranslator', $calls[1][0]);
@@ -882,6 +882,7 @@ public function testValidation()
882882
$i = 3;
883883
if ($annotations) {
884884
$this->assertSame('enableAnnotationMapping', $calls[++$i][0]);
885+
$this->assertSame('setDoctrineAnnotationReader', $calls[++$i][0]);
885886
}
886887
$this->assertSame('addMethodMapping', $calls[++$i][0]);
887888
$this->assertSame(['loadValidatorMetadata'], $calls[$i][1]);
@@ -923,13 +924,14 @@ public function testValidationAnnotations()
923924

924925
$calls = $container->getDefinition('validator.builder')->getMethodCalls();
925926

926-
$this->assertCount(7, $calls);
927+
$this->assertCount(8, $calls);
927928
$this->assertSame('enableAnnotationMapping', $calls[4][0]);
928-
$this->assertEquals([new Reference('annotation_reader')], $calls[4][1]);
929-
$this->assertSame('addMethodMapping', $calls[5][0]);
930-
$this->assertSame(['loadValidatorMetadata'], $calls[5][1]);
931-
$this->assertSame('setMappingCache', $calls[6][0]);
932-
$this->assertEquals([new Reference('validator.mapping.cache.adapter')], $calls[6][1]);
929+
$this->assertSame('setDoctrineAnnotationReader', $calls[5][0]);
930+
$this->assertEquals([new Reference('annotation_reader')], $calls[5][1]);
931+
$this->assertSame('addMethodMapping', $calls[6][0]);
932+
$this->assertSame(['loadValidatorMetadata'], $calls[6][1]);
933+
$this->assertSame('setMappingCache', $calls[7][0]);
934+
$this->assertEquals([new Reference('validator.mapping.cache.adapter')], $calls[7][1]);
933935
// no cache this time
934936
}
935937

@@ -944,14 +946,15 @@ public function testValidationPaths()
944946

945947
$calls = $container->getDefinition('validator.builder')->getMethodCalls();
946948

947-
$this->assertCount(8, $calls);
949+
$this->assertCount(9, $calls);
948950
$this->assertSame('addXmlMappings', $calls[3][0]);
949951
$this->assertSame('addYamlMappings', $calls[4][0]);
950952
$this->assertSame('enableAnnotationMapping', $calls[5][0]);
951-
$this->assertSame('addMethodMapping', $calls[6][0]);
952-
$this->assertSame(['loadValidatorMetadata'], $calls[6][1]);
953-
$this->assertSame('setMappingCache', $calls[7][0]);
954-
$this->assertEquals([new Reference('validator.mapping.cache.adapter')], $calls[7][1]);
953+
$this->assertSame('setDoctrineAnnotationReader', $calls[6][0]);
954+
$this->assertSame('addMethodMapping', $calls[7][0]);
955+
$this->assertSame(['loadValidatorMetadata'], $calls[7][1]);
956+
$this->assertSame('setMappingCache', $calls[8][0]);
957+
$this->assertEquals([new Reference('validator.mapping.cache.adapter')], $calls[8][1]);
955958

956959
$xmlMappings = $calls[3][1][0];
957960
$this->assertCount(3, $xmlMappings);
@@ -1004,11 +1007,12 @@ public function testValidationNoStaticMethod()
10041007

10051008
$annotations = !class_exists(FullStack::class) && class_exists(Annotation::class);
10061009

1007-
$this->assertCount($annotations ? 6 : 5, $calls);
1010+
$this->assertCount($annotations ? 7 : 5, $calls);
10081011
$this->assertSame('addXmlMappings', $calls[3][0]);
10091012
$i = 3;
10101013
if ($annotations) {
10111014
$this->assertSame('enableAnnotationMapping', $calls[++$i][0]);
1015+
$this->assertSame('setDoctrineAnnotationReader', $calls[++$i][0]);
10121016
}
10131017
$this->assertSame('setMappingCache', $calls[++$i][0]);
10141018
$this->assertEquals([new Reference('validator.mapping.cache.adapter')], $calls[$i][1]);

‎src/Symfony/Component/Validator/CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ CHANGELOG
3434
* added support for UUIDv6 in `Uuid` constraint
3535
* enabled the validator to load constraints from PHP attributes
3636
* deprecated the `NumberConstraintTrait` trait
37+
* 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
3738

3839
5.1.0
3940
-----

‎src/Symfony/Component/Validator/Tests/Constraints/ValidValidatorTest.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ class ValidValidatorTest extends TestCase
1212
public function testPropertyPathsArePassedToNestedContexts()
1313
{
1414
$validatorBuilder = new ValidatorBuilder();
15-
$validator = $validatorBuilder->enableAnnotationMapping()->getValidator();
15+
$validator = $validatorBuilder->enableAnnotationMapping(true)->addDefaultDoctrineAnnotationReader()->getValidator();
1616

1717
$violations = $validator->validate(new Foo(), null, ['nested']);
1818

@@ -23,7 +23,7 @@ public function testPropertyPathsArePassedToNestedContexts()
2323
public function testNullValues()
2424
{
2525
$validatorBuilder = new ValidatorBuilder();
26-
$validator = $validatorBuilder->enableAnnotationMapping()->getValidator();
26+
$validator = $validatorBuilder->enableAnnotationMapping(true)->addDefaultDoctrineAnnotationReader()->getValidator();
2727

2828
$foo = new Foo();
2929
$foo->fooBar = null;

‎src/Symfony/Component/Validator/Tests/Mapping/Loader/PropertyInfoLoaderTest.php‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,8 @@ public function testLoadClassMetadata()
8989
$propertyInfoLoader = new PropertyInfoLoader($propertyInfoStub, $propertyInfoStub, $propertyInfoStub, '{.*}');
9090

9191
$validator = Validation::createValidatorBuilder()
92-
->enableAnnotationMapping()
92+
->enableAnnotationMapping(true)
93+
->addDefaultDoctrineAnnotationReader()
9394
->addLoader($propertyInfoLoader)
9495
->getValidator()
9596
;
@@ -220,7 +221,8 @@ public function testClassNoAutoMapping()
220221

221222
$propertyInfoLoader = new PropertyInfoLoader($propertyInfoStub, $propertyInfoStub, $propertyInfoStub, '{.*}');
222223
$validator = Validation::createValidatorBuilder()
223-
->enableAnnotationMapping()
224+
->enableAnnotationMapping(true)
225+
->addDefaultDoctrineAnnotationReader()
224226
->addLoader($propertyInfoLoader)
225227
->getValidator()
226228
;

‎src/Symfony/Component/Validator/Tests/ValidatorBuilderTest.php‎

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,18 @@
1111

1212
namespace Symfony\Component\Validator\Tests;
1313

14+
use Doctrine\Common\Annotations\CachedReader;
15+
use Doctrine\Common\Annotations\Reader;
1416
use PHPUnit\Framework\TestCase;
1517
use Psr\Cache\CacheItemPoolInterface;
18+
use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait;
19+
use Symfony\Component\Validator\Mapping\Loader\AnnotationLoader;
1620
use Symfony\Component\Validator\ValidatorBuilder;
1721

1822
class ValidatorBuilderTest extends TestCase
1923
{
24+
use ExpectDeprecationTrait;
25+
2026
/**
2127
* @var ValidatorBuilder
2228
*/
@@ -74,9 +80,74 @@ public function testAddMethodMappings()
7480
$this->assertSame($this->builder, $this->builder->addMethodMappings([]));
7581
}
7682

83+
/**
84+
* @group legacy
85+
*/
7786
public function testEnableAnnotationMapping()
7887
{
88+
$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.');
7989
$this->assertSame($this->builder, $this->builder->enableAnnotationMapping());
90+
91+
$loaders = $this->builder->getLoaders();
92+
$this->assertCount(1, $loaders);
93+
$this->assertInstanceOf(AnnotationLoader::class, $loaders[0]);
94+
95+
$r = new \ReflectionProperty(AnnotationLoader::class, 'reader');
96+
$r->setAccessible(true);
97+
98+
$this->assertInstanceOf(CachedReader::class, $r->getValue($loaders[0]));
99+
}
100+
101+
public function testEnableAnnotationMappingWithDefaultDoctrineAnnotationReader()
102+
{
103+
$this->assertSame($this->builder, $this->builder->enableAnnotationMapping(true));
104+
$this->assertSame($this->builder, $this->builder->addDefaultDoctrineAnnotationReader());
105+
106+
$loaders = $this->builder->getLoaders();
107+
$this->assertCount(1, $loaders);
108+
$this->assertInstanceOf(AnnotationLoader::class, $loaders[0]);
109+
110+
$r = new \ReflectionProperty(AnnotationLoader::class, 'reader');
111+
$r->setAccessible(true);
112+
113+
$this->assertInstanceOf(CachedReader::class, $r->getValue($loaders[0]));
114+
}
115+
116+
/**
117+
* @group legacy
118+
*/
119+
public function testEnableAnnotationMappingWithCustomDoctrineAnnotationReaderLegacy()
120+
{
121+
$reader = $this->createMock(Reader::class);
122+
123+
$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.');
124+
$this->assertSame($this->builder, $this->builder->enableAnnotationMapping($reader));
125+
126+
$loaders = $this->builder->getLoaders();
127+
$this->assertCount(1, $loaders);
128+
$this->assertInstanceOf(AnnotationLoader::class, $loaders[0]);
129+
130+
$r = new \ReflectionProperty(AnnotationLoader::class, 'reader');
131+
$r->setAccessible(true);
132+
133+
$this->assertSame($reader, $r->getValue($loaders[0]));
134+
}
135+
136+
public function testEnableAnnotationMappingWithCustomDoctrineAnnotationReader()
137+
{
138+
$reader = $this->createMock(Reader::class);
139+
140+
$this->assertSame($this->builder, $this->builder->enableAnnotationMapping(true));
141+
$this->assertSame($this->builder, $this->builder->setDoctrineAnnotationReader($reader));
142+
143+
$loaders = $this->builder->getLoaders();
144+
$this->assertCount(1, $loaders);
145+
$this->assertInstanceOf(AnnotationLoader::class, $loaders[0]);
146+
147+
$r = new \ReflectionProperty(AnnotationLoader::class, 'reader');
148+
$r->setAccessible(true);
149+
150+
$this->assertSame($reader, $r->getValue($loaders[0]));
80151
}
81152

82153
public function testDisableAnnotationMapping()

0 commit comments

Comments
 (0)