From 1d5a0a0a1b6d05072c1a1e9b23c1c2a3a2d8f877 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Fri, 5 Dec 2014 11:17:43 +0100 Subject: [PATCH 1/9] Adding docblocks --- .../Mapping/ReflectionEmbeddedProperty.php | 32 +++++++++++++++++-- 1 file changed, 29 insertions(+), 3 deletions(-) diff --git a/lib/Doctrine/ORM/Mapping/ReflectionEmbeddedProperty.php b/lib/Doctrine/ORM/Mapping/ReflectionEmbeddedProperty.php index 662c00690..afbd5a0da 100644 --- a/lib/Doctrine/ORM/Mapping/ReflectionEmbeddedProperty.php +++ b/lib/Doctrine/ORM/Mapping/ReflectionEmbeddedProperty.php @@ -18,6 +18,7 @@ */ namespace Doctrine\ORM\Mapping; +use ReflectionProperty; /** * Acts as a proxy to a nested Property structure, making it look like @@ -30,17 +31,38 @@ namespace Doctrine\ORM\Mapping; */ class ReflectionEmbeddedProperty { + /** + * @var ReflectionProperty + */ private $parentProperty; + + /** + * @var ReflectionProperty + */ private $childProperty; + + /** + * @var string + */ private $class; - public function __construct($parentProperty, $childProperty, $class) + /** + * @param ReflectionProperty $parentProperty + * @param ReflectionProperty $childProperty + * @param string $class + */ + public function __construct(ReflectionProperty $parentProperty, ReflectionProperty $childProperty, $class) { $this->parentProperty = $parentProperty; - $this->childProperty = $childProperty; - $this->class = $class; + $this->childProperty = $childProperty; + $this->class = (string) $class; } + /** + * @param $object + * + * @return object|null + */ public function getValue($object) { $embeddedObject = $this->parentProperty->getValue($object); @@ -52,6 +74,10 @@ class ReflectionEmbeddedProperty return $this->childProperty->getValue($embeddedObject); } + /** + * @param object $object + * @param mixed $value + */ public function setValue($object, $value) { $embeddedObject = $this->parentProperty->getValue($object); From fc3f233923abb5f868e7c27023092833ffcc7f8e Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Fri, 5 Dec 2014 11:18:10 +0100 Subject: [PATCH 2/9] Yodaism good for you: is. --- lib/Doctrine/ORM/Mapping/ReflectionEmbeddedProperty.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/Doctrine/ORM/Mapping/ReflectionEmbeddedProperty.php b/lib/Doctrine/ORM/Mapping/ReflectionEmbeddedProperty.php index afbd5a0da..adde0fa23 100644 --- a/lib/Doctrine/ORM/Mapping/ReflectionEmbeddedProperty.php +++ b/lib/Doctrine/ORM/Mapping/ReflectionEmbeddedProperty.php @@ -67,7 +67,7 @@ class ReflectionEmbeddedProperty { $embeddedObject = $this->parentProperty->getValue($object); - if ($embeddedObject === null) { + if (null === $embeddedObject) { return null; } @@ -82,7 +82,7 @@ class ReflectionEmbeddedProperty { $embeddedObject = $this->parentProperty->getValue($object); - if ($embeddedObject === null) { + if (null === $embeddedObject) { $embeddedObject = unserialize(sprintf('O:%d:"%s":0:{}', strlen($this->class), $this->class)); $this->parentProperty->setValue($object, $embeddedObject); } From 56cb47c5857caafc9e7e02e1bd1fed36b40a2f9c Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Fri, 5 Dec 2014 12:41:22 +0100 Subject: [PATCH 3/9] Adding a test asset to play around with reflection and internal classes --- .../Reflection/ArrayObjectExtendingClass.php | 15 +++++++++++++++ 1 file changed, 15 insertions(+) create mode 100644 tests/Doctrine/Tests/Models/Reflection/ArrayObjectExtendingClass.php diff --git a/tests/Doctrine/Tests/Models/Reflection/ArrayObjectExtendingClass.php b/tests/Doctrine/Tests/Models/Reflection/ArrayObjectExtendingClass.php new file mode 100644 index 000000000..4c20586b7 --- /dev/null +++ b/tests/Doctrine/Tests/Models/Reflection/ArrayObjectExtendingClass.php @@ -0,0 +1,15 @@ + Date: Fri, 5 Dec 2014 12:43:12 +0100 Subject: [PATCH 4/9] Namespace correction --- .../Tests/Models/Reflection/ArrayObjectExtendingClass.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Doctrine/Tests/Models/Reflection/ArrayObjectExtendingClass.php b/tests/Doctrine/Tests/Models/Reflection/ArrayObjectExtendingClass.php index 4c20586b7..b894a81d3 100644 --- a/tests/Doctrine/Tests/Models/Reflection/ArrayObjectExtendingClass.php +++ b/tests/Doctrine/Tests/Models/Reflection/ArrayObjectExtendingClass.php @@ -1,6 +1,6 @@ Date: Fri, 5 Dec 2014 12:56:34 +0100 Subject: [PATCH 5/9] Test to verify that `Doctrine\ORM\Mapping\ReflectionEmbeddedProperty` is able to interact with internal PHP classes --- .../ReflectionEmbeddedPropertyTest.php | 100 ++++++++++++++++++ 1 file changed, 100 insertions(+) create mode 100644 tests/Doctrine/Tests/ORM/Mapping/ReflectionEmbeddedPropertyTest.php diff --git a/tests/Doctrine/Tests/ORM/Mapping/ReflectionEmbeddedPropertyTest.php b/tests/Doctrine/Tests/ORM/Mapping/ReflectionEmbeddedPropertyTest.php new file mode 100644 index 000000000..84a614ddb --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Mapping/ReflectionEmbeddedPropertyTest.php @@ -0,0 +1,100 @@ +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)); + } + + /** + * Data provider + * + * @return ReflectionProperty[][] + */ + public function getTestedReflectionProperties() + { + return array( + 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; + } +} From a8b0ac82b4170094f065d4a850f613a6a4b7371f Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Fri, 5 Dec 2014 12:57:48 +0100 Subject: [PATCH 6/9] Adding a data-provider case for a generic model (non-internal class) --- .../ORM/Mapping/ReflectionEmbeddedPropertyTest.php | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/Doctrine/Tests/ORM/Mapping/ReflectionEmbeddedPropertyTest.php b/tests/Doctrine/Tests/ORM/Mapping/ReflectionEmbeddedPropertyTest.php index 84a614ddb..237de15d8 100644 --- a/tests/Doctrine/Tests/ORM/Mapping/ReflectionEmbeddedPropertyTest.php +++ b/tests/Doctrine/Tests/ORM/Mapping/ReflectionEmbeddedPropertyTest.php @@ -50,6 +50,17 @@ class ReflectionEmbeddedPropertyTest extends \PHPUnit_Framework_TestCase 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', From 112fdf46d08441fc3fb597d9da371a538692e4bb Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Fri, 5 Dec 2014 13:00:54 +0100 Subject: [PATCH 7/9] Using instantiator to work with internal PHP classes as embeddables --- .../ORM/Mapping/ReflectionEmbeddedProperty.php | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/lib/Doctrine/ORM/Mapping/ReflectionEmbeddedProperty.php b/lib/Doctrine/ORM/Mapping/ReflectionEmbeddedProperty.php index adde0fa23..659b6cfb7 100644 --- a/lib/Doctrine/ORM/Mapping/ReflectionEmbeddedProperty.php +++ b/lib/Doctrine/ORM/Mapping/ReflectionEmbeddedProperty.php @@ -18,6 +18,8 @@ */ namespace Doctrine\ORM\Mapping; + +use Doctrine\Instantiator\Instantiator; use ReflectionProperty; /** @@ -46,6 +48,11 @@ class ReflectionEmbeddedProperty */ private $class; + /** + * @var Instantiator|null + */ + private $instantiator; + /** * @param ReflectionProperty $parentProperty * @param ReflectionProperty $childProperty @@ -83,7 +90,10 @@ class ReflectionEmbeddedProperty $embeddedObject = $this->parentProperty->getValue($object); 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); } From b4a23e97a90ff279597acf7dbc6cb3348965371a Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Fri, 5 Dec 2014 13:06:41 +0100 Subject: [PATCH 8/9] `ReflectionEmbeddedProperty` should be like any `ReflectionProperty`, and should therefore extend it for type compatibility --- .../Mapping/ReflectionEmbeddedProperty.php | 21 +++++++------------ 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/lib/Doctrine/ORM/Mapping/ReflectionEmbeddedProperty.php b/lib/Doctrine/ORM/Mapping/ReflectionEmbeddedProperty.php index 659b6cfb7..0163ebb89 100644 --- a/lib/Doctrine/ORM/Mapping/ReflectionEmbeddedProperty.php +++ b/lib/Doctrine/ORM/Mapping/ReflectionEmbeddedProperty.php @@ -31,7 +31,7 @@ use ReflectionProperty; * * TODO: Move this class into Common\Reflection */ -class ReflectionEmbeddedProperty +class ReflectionEmbeddedProperty extends ReflectionProperty { /** * @var ReflectionProperty @@ -43,11 +43,6 @@ class ReflectionEmbeddedProperty */ private $childProperty; - /** - * @var string - */ - private $class; - /** * @var Instantiator|null */ @@ -62,15 +57,14 @@ class ReflectionEmbeddedProperty { $this->parentProperty = $parentProperty; $this->childProperty = $childProperty; - $this->class = (string) $class; + + parent::__construct($childProperty->getDeclaringClass()->getName(), $childProperty->getName()); } /** - * @param $object - * - * @return object|null + * {@inheritDoc} */ - public function getValue($object) + public function getValue($object = null) { $embeddedObject = $this->parentProperty->getValue($object); @@ -82,10 +76,9 @@ class ReflectionEmbeddedProperty } /** - * @param object $object - * @param mixed $value + * {@inheritDoc} */ - public function setValue($object, $value) + public function setValue($object, $value = null) { $embeddedObject = $this->parentProperty->getValue($object); From dcf824688a8bab35a03cdf58dd27be0cb1620c74 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Fri, 5 Dec 2014 13:15:15 +0100 Subject: [PATCH 9/9] Verifying that reflection properties that don't contain an embeddable will not crash reading properties, but will return `null` instead --- .../ReflectionEmbeddedPropertyTest.php | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/Doctrine/Tests/ORM/Mapping/ReflectionEmbeddedPropertyTest.php b/tests/Doctrine/Tests/ORM/Mapping/ReflectionEmbeddedPropertyTest.php index 237de15d8..8a67efdf3 100644 --- a/tests/Doctrine/Tests/ORM/Mapping/ReflectionEmbeddedPropertyTest.php +++ b/tests/Doctrine/Tests/ORM/Mapping/ReflectionEmbeddedPropertyTest.php @@ -42,6 +42,30 @@ class ReflectionEmbeddedPropertyTest extends \PHPUnit_Framework_TestCase $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 *