bug #31481 [Validator] Autovalidation: skip readonly props (dunglas)

This PR was merged into the 4.3 branch.

Discussion
----------

[Validator] Autovalidation: skip readonly props

| Q             | A
| ------------- | ---
| Branch?       | 4.3
| Bug fix?      | yes
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| BC breaks?    | no
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tests pass?   | yes    <!-- please add some, will be required by reviewers -->
| Fixed tickets | n/a
| License       | MIT
| Doc PR        | n/a

Read-only properties (such as an autogenerated ID) must be skipped during the validation (it makes no sense to validate a RO property anyway).
This is an allowed BC break because this feature will be introduced in 4.3.

Commits
-------

e7dc5e1045 [Validator] Autovalidation: skip readonly props

src/Symfony/Bundle/FrameworkBundle/Resources/config/validator.xml

@@ -71,6 +71,7 @@
         <service id="validator.property_info_loader" class="Symfony\Component\Validator\Mapping\Loader\PropertyInfoLoader">
             <argument type="service" id="property_info" />
             <argument type="service" id="property_info" />
+            <argument type="service" id="property_info" />
 
             <tag name="validator.auto_mapper" />
         </service>

src/Symfony/Component/Validator/Mapping/Loader/PropertyInfoLoader.php

@@ -11,6 +11,7 @@
 
 namespace Symfony\Component\Validator\Mapping\Loader;
 
+use Symfony\Component\PropertyInfo\PropertyAccessExtractorInterface;
 use Symfony\Component\PropertyInfo\PropertyListExtractorInterface;
 use Symfony\Component\PropertyInfo\PropertyTypeExtractorInterface;
 use Symfony\Component\PropertyInfo\Type as PropertyInfoType;
@@ -29,12 +30,14 @@ final class PropertyInfoLoader implements LoaderInterface
 {
     private $listExtractor;
     private $typeExtractor;
+    private $accessExtractor;
     private $classValidatorRegexp;
 
-    public function __construct(PropertyListExtractorInterface $listExtractor, PropertyTypeExtractorInterface $typeExtractor, string $classValidatorRegexp = null)
+    public function __construct(PropertyListExtractorInterface $listExtractor, PropertyTypeExtractorInterface $typeExtractor, PropertyAccessExtractorInterface $accessExtractor, string $classValidatorRegexp = null)
     {
         $this->listExtractor = $listExtractor;
         $this->typeExtractor = $typeExtractor;
+        $this->accessExtractor = $accessExtractor;
         $this->classValidatorRegexp = $classValidatorRegexp;
     }
 
@@ -53,6 +56,10 @@ final class PropertyInfoLoader implements LoaderInterface
         }
 
         foreach ($properties as $property) {
+            if (false === $this->accessExtractor->isWritable($className, $property)) {
+                continue;
+            }
+
             $types = $this->typeExtractor->getTypes($className, $property);
             if (null === $types) {
                 continue;

src/Symfony/Component/Validator/Tests/Fixtures/PropertyInfoLoaderEntity.php

@@ -46,4 +46,6 @@ class PropertyInfoLoaderEntity
      * })
      */
     public $alreadyPartiallyMappedCollection;
+
+    public $readOnly;
 }

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

@@ -45,6 +45,7 @@ class PropertyInfoLoaderTest extends TestCase
                 'alreadyMappedNotNull',
                 'alreadyMappedNotBlank',
                 'alreadyPartiallyMappedCollection',
+                'readOnly',
             ])
         ;
         $propertyInfoStub
@@ -58,11 +59,27 @@ class PropertyInfoLoaderTest extends TestCase
                 [new Type(Type::BUILTIN_TYPE_FLOAT, true)], // The existing constraint is float
                 [new Type(Type::BUILTIN_TYPE_STRING, true)],
                 [new Type(Type::BUILTIN_TYPE_STRING, true)],
-                [new Type(Type::BUILTIN_TYPE_ARRAY, true, null, true, null, new Type(Type::BUILTIN_TYPE_FLOAT))]
+                [new Type(Type::BUILTIN_TYPE_ARRAY, true, null, true, null, new Type(Type::BUILTIN_TYPE_FLOAT))],
+                [new Type(Type::BUILTIN_TYPE_STRING)]
+            ))
+        ;
+        $propertyInfoStub
+            ->method('isWritable')
+            ->will($this->onConsecutiveCalls(
+                true,
+                true,
+                true,
+                true,
+                true,
+                true,
+                true,
+                true,
+                true,
+                false
             ))
         ;
 
-        $propertyInfoLoader = new PropertyInfoLoader($propertyInfoStub, $propertyInfoStub);
+        $propertyInfoLoader = new PropertyInfoLoader($propertyInfoStub, $propertyInfoStub, $propertyInfoStub);
 
         $validator = Validation::createValidatorBuilder()
             ->enableAnnotationMapping()
@@ -137,6 +154,9 @@ class PropertyInfoLoaderTest extends TestCase
         $this->assertSame('string', $alreadyPartiallyMappedCollectionConstraints[0]->constraints[0]->type);
         $this->assertInstanceOf(Iban::class, $alreadyPartiallyMappedCollectionConstraints[0]->constraints[1]);
         $this->assertInstanceOf(NotNull::class, $alreadyPartiallyMappedCollectionConstraints[0]->constraints[2]);
+
+        $readOnlyMetadata = $classMetadata->getPropertyMetadata('readOnly');
+        $this->assertEmpty($readOnlyMetadata);
     }
 
     /**
@@ -154,7 +174,7 @@ class PropertyInfoLoaderTest extends TestCase
             ->willReturn([new Type(Type::BUILTIN_TYPE_STRING)])
         ;
 
-        $propertyInfoLoader = new PropertyInfoLoader($propertyInfoStub, $propertyInfoStub, $classValidatorRegexp);
+        $propertyInfoLoader = new PropertyInfoLoader($propertyInfoStub, $propertyInfoStub, $propertyInfoStub, $classValidatorRegexp);
 
         $classMetadata = new ClassMetadata(PropertyInfoLoaderEntity::class);
         $this->assertSame($expected, $propertyInfoLoader->loadClassMetadata($classMetadata));