[Validator] Autovalidation: skip readonly props

dunglas · dunglas · commit e7dc5e1045e0 · 2019-05-12T11:42:24.000+02:00
diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/validator.xml b/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>
diff --git a/src/Symfony/Component/Validator/Mapping/Loader/PropertyInfoLoader.php b/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 @@ public function loadClassMetadata(ClassMetadata $metadata)
         }
 
         foreach ($properties as $property) {
+            if (false === $this->accessExtractor->isWritable($className, $property)) {
+                continue;
+            }
+
             $types = $this->typeExtractor->getTypes($className, $property);
             if (null === $types) {
                 continue;
diff --git a/src/Symfony/Component/Validator/Tests/Fixtures/PropertyInfoLoaderEntity.php b/src/Symfony/Component/Validator/Tests/Fixtures/PropertyInfoLoaderEntity.php
@@ -46,4 +46,6 @@ class PropertyInfoLoaderEntity
      * })
      */
     public $alreadyPartiallyMappedCollection;
+
+    public $readOnly;
 }
diff --git a/src/Symfony/Component/Validator/Tests/Mapping/Loader/PropertyInfoLoaderTest.php b/src/Symfony/Component/Validator/Tests/Mapping/Loader/PropertyInfoLoaderTest.php
@@ -45,6 +45,7 @@ public function testLoadClassMetadata()
                 'alreadyMappedNotNull',
                 'alreadyMappedNotBlank',
                 'alreadyPartiallyMappedCollection',
+                'readOnly',
             ])
         ;
         $propertyInfoStub
@@ -58,11 +59,27 @@ public function testLoadClassMetadata()
                 [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 @@ public function testLoadClassMetadata()
         $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 @@ public function testClassValidator(bool $expected, string $classValidatorRegexp
             ->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));

Original file line number	Diff line number	Diff line change
`@@ -46,4 +46,6 @@ class PropertyInfoLoaderEntity`
`46`	`46`	`* })`
`47`	`47`	`*/`
`48`	`48`	`public $alreadyPartiallyMappedCollection;`
	`49`	`+`
	`50`	`+ public $readOnly;`
`49`	`51`	`}`