Merge pull request #1207 from Ocramius/hotfix/embedded-classes-reflection-new-instance-creation

Embedded classes reflection new instance creation with internal PHP classes

lib/Doctrine/ORM/Mapping/ReflectionEmbeddedProperty.php

@@ -19,6 +19,9 @@
 
 namespace Doctrine\ORM\Mapping;
 
+use Doctrine\Instantiator\Instantiator;
+use ReflectionProperty;
+
 /**
  * Acts as a proxy to a nested Property structure, making it look like
  * just a single scalar property.
@@ -28,36 +31,62 @@ namespace Doctrine\ORM\Mapping;
  *
  * TODO: Move this class into Common\Reflection
  */
-class ReflectionEmbeddedProperty
+class ReflectionEmbeddedProperty extends ReflectionProperty
 {
+    /**
+     * @var ReflectionProperty
+     */
     private $parentProperty;
-    private $childProperty;
-    private $class;
 
-    public function __construct($parentProperty, $childProperty, $class)
+    /**
+     * @var ReflectionProperty
+     */
+    private $childProperty;
+
+    /**
+     * @var Instantiator|null
+     */
+    private $instantiator;
+
+    /**
+     * @param ReflectionProperty $parentProperty
+     * @param ReflectionProperty $childProperty
+     * @param string             $class
+     */
+    public function __construct(ReflectionProperty $parentProperty, ReflectionProperty $childProperty, $class)
     {
         $this->parentProperty = $parentProperty;
-        $this->childProperty = $childProperty;
+        $this->childProperty  = $childProperty;
-        $this->class = $class;
+
+        parent::__construct($childProperty->getDeclaringClass()->getName(), $childProperty->getName());
     }
 
-    public function getValue($object)
+    /**
+     * {@inheritDoc}
+     */
+    public function getValue($object = null)
     {
         $embeddedObject = $this->parentProperty->getValue($object);
 
-        if ($embeddedObject === null) {
+        if (null === $embeddedObject) {
             return null;
         }
 
         return $this->childProperty->getValue($embeddedObject);
     }
 
-    public function setValue($object, $value)
+    /**
+     * {@inheritDoc}
+     */
+    public function setValue($object, $value = null)
     {
         $embeddedObject = $this->parentProperty->getValue($object);
 
-        if ($embeddedObject === null) {
+        if (null === $embeddedObject) {
-            $embeddedObject = unserialize(sprintf('O:%d:"%s":0:{}', strlen($this->class), $this->class));
+            $this->instantiator = $this->instantiator ?: new Instantiator();
+
+            $embeddedObject = $this->instantiator->instantiate($this->class);
+
             $this->parentProperty->setValue($object, $embeddedObject);
         }
 

tests/Doctrine/Tests/Models/Reflection/ArrayObjectExtendingClass.php

@@ -0,0 +1,15 @@
+<?php
+
+namespace Doctrine\Tests\Models\Reflection;
+
+use ArrayObject;
+
+/**
+ * A test asset extending {@see \ArrayObject}, useful for verifying internal classes issues with reflection
+ */
+class ArrayObjectExtendingClass extends ArrayObject
+{
+    private $privateProperty;
+    protected $protectedProperty;
+    public $publicProperty;
+}

tests/Doctrine/Tests/ORM/Mapping/ReflectionEmbeddedPropertyTest.php

@@ -0,0 +1,135 @@
+<?php
+
+namespace Doctrine\Tests\ORM\Mapping;
+
+use Doctrine\Instantiator\Instantiator;
+use Doctrine\ORM\Mapping\ReflectionEmbeddedProperty;
+use ReflectionProperty;
+
+/**
+ * Tests for {@see \Doctrine\ORM\Mapping\ReflectionEmbeddedProperty}
+ *
+ * @covers \Doctrine\ORM\Mapping\ReflectionEmbeddedProperty
+ */
+class ReflectionEmbeddedPropertyTest extends \PHPUnit_Framework_TestCase
+{
+    /**
+     * @param ReflectionProperty $parentProperty
+     * @param ReflectionProperty $childProperty
+     *
+     * @dataProvider getTestedReflectionProperties
+     */
+    public function testCanSetAndGetEmbeddedProperty(
+        ReflectionProperty $parentProperty,
+        ReflectionProperty $childProperty
+    ) {
+        $embeddedPropertyReflection = new ReflectionEmbeddedProperty(
+            $parentProperty,
+            $childProperty,
+            $childProperty->getDeclaringClass()->getName()
+        );
+
+        $instantiator = new Instantiator();
+
+        $object = $instantiator->instantiate($parentProperty->getDeclaringClass()->getName());
+
+        $embeddedPropertyReflection->setValue($object, 'newValue');
+
+        $this->assertSame('newValue', $embeddedPropertyReflection->getValue($object));
+
+        $embeddedPropertyReflection->setValue($object, 'changedValue');
+
+        $this->assertSame('changedValue', $embeddedPropertyReflection->getValue($object));
+    }
+
+    /**
+     * @param ReflectionProperty $parentProperty
+     * @param ReflectionProperty $childProperty
+     *
+     * @dataProvider getTestedReflectionProperties
+     */
+    public function testWillSkipReadingPropertiesFromNullEmbeddable(
+        ReflectionProperty $parentProperty,
+        ReflectionProperty $childProperty
+    )
+    {
+        $embeddedPropertyReflection = new ReflectionEmbeddedProperty(
+            $parentProperty,
+            $childProperty,
+            $childProperty->getDeclaringClass()->getName()
+        );
+
+        $instantiator = new Instantiator();
+
+        $this->assertNull($embeddedPropertyReflection->getValue(
+            $instantiator->instantiate($parentProperty->getDeclaringClass()->getName())
+        ));
+    }
+
+    /**
+     * Data provider
+     *
+     * @return ReflectionProperty[][]
+     */
+    public function getTestedReflectionProperties()
+    {
+        return array(
+            array(
+                $this->getReflectionProperty(
+                    'Doctrine\\Tests\\Models\\Generic\\BooleanModel',
+                    'id'
+                ),
+                $this->getReflectionProperty(
+                    'Doctrine\\Tests\\Models\\Generic\\BooleanModel',
+                    'id'
+                ),
+            ),
+            // reflection on classes extending internal PHP classes:
+            array(
+                $this->getReflectionProperty(
+                    'Doctrine\\Tests\\Models\\Reflection\\ArrayObjectExtendingClass',
+                    'publicProperty'
+                ),
+                $this->getReflectionProperty(
+                    'Doctrine\\Tests\\Models\\Reflection\\ArrayObjectExtendingClass',
+                    'privateProperty'
+                ),
+            ),
+            array(
+                $this->getReflectionProperty(
+                    'Doctrine\\Tests\\Models\\Reflection\\ArrayObjectExtendingClass',
+                    'publicProperty'
+                ),
+                $this->getReflectionProperty(
+                    'Doctrine\\Tests\\Models\\Reflection\\ArrayObjectExtendingClass',
+                    'protectedProperty'
+                ),
+            ),
+            array(
+                $this->getReflectionProperty(
+                    'Doctrine\\Tests\\Models\\Reflection\\ArrayObjectExtendingClass',
+                    'publicProperty'
+                ),
+                $this->getReflectionProperty(
+                    'Doctrine\\Tests\\Models\\Reflection\\ArrayObjectExtendingClass',
+                    'publicProperty'
+                ),
+            ),
+        );
+    }
+
+    /**
+     * @param string $className
+     * @param string $propertyName
+     *
+     * @return ReflectionProperty
+     */
+    private function getReflectionProperty($className, $propertyName)
+    {
+        $reflectionProperty = new ReflectionProperty($className, $propertyName);
+
+        $reflectionProperty->setAccessible(true);
+
+        return $reflectionProperty;
+    }
+}