Use PropertyAccess to populate model

sonata-project · Jul 25, 2020 · ff85de0 · ff85de0
1 parent 0c3c71b
commit ff85de0
Show file tree

Hide file tree

Showing 4 changed files with 80 additions and 25 deletions.
diff --git a/composer.json b/composer.json
@@ -36,6 +36,7 @@
         "symfony/doctrine-bridge": "^4.4",
         "symfony/form": "^4.4",
         "symfony/http-kernel": "^4.4",
+        "symfony/property-access": "^4.4",
         "twig/twig": "^2.6"
     },
     "provide": {

diff --git a/src/Model/ModelManager.php b/src/Model/ModelManager.php
@@ -23,15 +23,25 @@
 use Sonata\DoctrineMongoDBAdminBundle\Datagrid\ProxyQuery;
 use Sonata\Exporter\Source\DoctrineODMQuerySourceIterator;
 use Symfony\Bridge\Doctrine\ManagerRegistry;
+use Symfony\Component\PropertyAccess\PropertyAccess;
+use Symfony\Component\PropertyAccess\PropertyAccessorInterface;
 
 class ModelManager implements ModelManagerInterface
 {
     public const ID_SEPARATOR = '-';
+
+    /** @var ManagerRegistry */
     protected $registry;
 
-    public function __construct(ManagerRegistry $registry)
+    /**
+     * @var PropertyAccessorInterface|null
+     */
+    private $propertyAccessor;
+
+    public function __construct(ManagerRegistry $registry, ?PropertyAccessorInterface $propertyAccessor = null)
     {
         $this->registry = $registry;
+        $this->propertyAccessor = $propertyAccessor ?? PropertyAccess::createPropertyAccessor();
     }
 
     /**
@@ -452,39 +462,17 @@ public function modelReverseTransform($class, array $array = [])
         $instance = $this->getModelInstance($class);
         $metadata = $this->getMetadata($class);
 
-        $reflClass = $metadata->reflClass;
         foreach ($array as $name => $value) {
-            $reflection_property = false;
             // property or association ?
             if (\array_key_exists($name, $metadata->fieldMappings)) {
                 $property = $metadata->fieldMappings[$name]['fieldName'];
-                $reflection_property = $metadata->reflFields[$name];
             } elseif (\array_key_exists($name, $metadata->associationMappings)) {
                 $property = $metadata->associationMappings[$name]['fieldName'];
             } else {
                 $property = $name;
             }
 
-            $setter = 'set'.$this->camelize($name);
-
-            if ($reflClass->hasMethod($setter)) {
-                if (!$reflClass->getMethod($setter)->isPublic()) {
-                    throw new \BadMethodCallException(sprintf('Method "%s()" is not public in class "%s"', $setter, $reflClass->getName()));
-                }
-
-                $instance->$setter($value);
-            } elseif ($reflClass->hasMethod('__set')) {
-                // needed to support magic method __set
-                $instance->$property = $value;
-            } elseif ($reflClass->hasProperty($property)) {
-                if (!$reflClass->getProperty($property)->isPublic()) {
-                    throw new \BadMethodCallException(sprintf('Property "%s" is not public in class "%s". Maybe you should create the method "set%s()"?', $property, $reflClass->getName(), ucfirst($property)));
-                }
-
-                $instance->$property = $value;
-            } elseif ($reflection_property) {
-                $reflection_property->setValue($instance, $value);
-            }
+            $this->propertyAccessor->setValue($instance, $property, $value);
         }
 
         return $instance;

diff --git a/tests/Fixtures/Document/SimpleDocumentWithPrivateSetter.php b/tests/Fixtures/Document/SimpleDocumentWithPrivateSetter.php
@@ -0,0 +1,34 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of the Sonata Project package.
+ *
+ * (c) Thomas Rabaix <thomas.rabaix@sonata-project.org>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Sonata\DoctrineMongoDBAdminBundle\Tests\Fixtures\Document;
+
+final class SimpleDocumentWithPrivateSetter
+{
+    private $schmeckles;
+
+    public function __construct(int $schmeckles)
+    {
+        $this->setSchmeckles($schmeckles);
+    }
+
+    public function getSchmeckles(): int
+    {
+        return $this->schmeckles;
+    }
+
+    private function setSchmeckles($value): void
+    {
+        $this->schmeckles = $value;
+    }
+}
diff --git a/tests/Model/ModelManagerTest.php b/tests/Model/ModelManagerTest.php
@@ -30,7 +30,9 @@
 use Sonata\DoctrineMongoDBAdminBundle\Tests\Fixtures\Document\EmbeddedDocument;
 use Sonata\DoctrineMongoDBAdminBundle\Tests\Fixtures\Document\ProtectedDocument;
 use Sonata\DoctrineMongoDBAdminBundle\Tests\Fixtures\Document\SimpleDocument;
+use Sonata\DoctrineMongoDBAdminBundle\Tests\Fixtures\Document\SimpleDocumentWithPrivateSetter;
 use Symfony\Bridge\Doctrine\ManagerRegistry;
+use Symfony\Component\PropertyAccess\Exception\NoSuchPropertyException;
 
 class ModelManagerTest extends TestCase
 {
@@ -175,7 +177,7 @@ public function testGetParentMetadataForProperty(): void
         $this->assertSame($metadata->fieldMappings[$lastPropertyName]['type'], 'boolean');
     }
 
-    public function testModelReverseTransform(): void
+    public function testModelReverseTransformWithSetter(): void
     {
         $class = SimpleDocument::class;
 
@@ -194,6 +196,36 @@ public function testModelReverseTransform(): void
         $this->assertTrue($object->schwifty);
     }
 
+    public function testModelReverseTransformFailsWithPrivateSetter(): void
+    {
+        $class = SimpleDocumentWithPrivateSetter::class;
+        $manager = $this->createModelManager($class);
+
+        $this->expectException(NoSuchPropertyException::class);
+
+        $manager->modelReverseTransform($class, ['schmeckles' => 42]);
+    }
+
+    public function testModelReverseTransformFailsWithPrivateProperties(): void
+    {
+        $class = SimpleDocument::class;
+        $manager = $this->createModelManager($class);
+
+        $this->expectException(NoSuchPropertyException::class);
+
+        $manager->modelReverseTransform($class, ['plumbus' => 42]);
+    }
+
+    public function testModelReverseTransformFailsWithPrivateProperties2(): void
+    {
+        $class = SimpleDocument::class;
+        $manager = $this->createModelManager($class);
+
+        $this->expectException(NoSuchPropertyException::class);
+
+        $manager->modelReverseTransform($class, ['plumbus' => 42]);
+    }
+
     public function testCollections(): void
     {
         $registry = $this->createStub(ManagerRegistry::class);