From 15a43029024f1f6269df8695ee85c6984fdc1c2f Mon Sep 17 00:00:00 2001 From: Kevin Bond Date: Mon, 15 Jan 2018 14:01:56 -0500 Subject: [PATCH 1/4] Inheritance middle-layer doesn't get hydrated with HYDRATE_OBJECT --- .../ORM/Functional/Ticket/GH6937Test.php | 119 ++++++++++++++++++ 1 file changed, 119 insertions(+) create mode 100644 tests/Doctrine/Tests/ORM/Functional/Ticket/GH6937Test.php diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/GH6937Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/GH6937Test.php new file mode 100644 index 000000000..756111b98 --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/GH6937Test.php @@ -0,0 +1,119 @@ +setUpEntitySchema([GH6937Person::class, GH6937Employee::class, GH6937Manager::class]); + } + + public function testPhoneNumberIsPopulatedWithFind() : void + { + $manager = new GH6937Manager(); + $manager->name = 'Kevin'; + $manager->phoneNumber = '555-5555'; + $manager->department = 'Accounting'; + + $this->_em->persist($manager); + $this->_em->flush(); + $this->_em->clear(); + + $persistedManager = $this->_em->find(GH6937Person::class, $manager->id); + + self::assertSame('Kevin', $persistedManager->name); + self::assertSame('555-5555', $persistedManager->phoneNumber); + self::assertSame('Accounting', $persistedManager->department); + } + + public function testPhoneNumberIsPopulatedWithQueryBuilderUsingSimpleObjectHydrator() : void + { + $manager = new GH6937Manager(); + $manager->name = 'Kevin'; + $manager->phoneNumber = '555-5555'; + $manager->department = 'Accounting'; + + $this->_em->persist($manager); + $this->_em->flush(); + $this->_em->clear(); + + $persistedManager = $this->_em->getRepository(GH6937Person::class) + ->createQueryBuilder('e') + ->where('e.id = :id') + ->setParameter('id', $manager->id) + ->getQuery() + ->getOneOrNullResult(AbstractQuery::HYDRATE_SIMPLEOBJECT); + + self::assertSame('Kevin', $persistedManager->name); + self::assertSame('555-5555', $persistedManager->phoneNumber); + self::assertSame('Accounting', $persistedManager->department); + } + + public function testPhoneNumberIsPopulatedWithQueryBuilder() : void + { + $manager = new GH6937Manager(); + $manager->name = 'Kevin'; + $manager->phoneNumber = '555-5555'; + $manager->department = 'Accounting'; + + $this->_em->persist($manager); + $this->_em->flush(); + $this->_em->clear(); + + $persistedManager = $this->_em->getRepository(GH6937Person::class) + ->createQueryBuilder('e') + ->where('e.id = :id') + ->setParameter('id', $manager->id) + ->getQuery() + ->getOneOrNullResult(); + + self::assertSame('Kevin', $persistedManager->name); + self::assertSame('555-5555', $persistedManager->phoneNumber); + self::assertSame('Accounting', $persistedManager->department); + } +} + +/** + * @Entity + * @InheritanceType("JOINED") + * @DiscriminatorColumn(name="discr", type="string") + * @DiscriminatorMap({"employee"=GH6937Employee::class, "manager"=GH6937Manager::class}) + */ +abstract class GH6937Person +{ + /** @Id @Column(type="integer") @GeneratedValue */ + public $id; + + /** @Column(type="string") */ + public $name; +} + +/** + * @Entity + */ +abstract class GH6937Employee extends GH6937Person +{ + /** @Column(type="string") */ + public $phoneNumber; +} + +/** + * @Entity + */ +class GH6937Manager extends GH6937Employee +{ + /** @Column(type="string") */ + public $department; +} From 48ca6dbcec125c7b273bdfe278f9448cd2f0db5b Mon Sep 17 00:00:00 2001 From: Toni Cornelissen Date: Sun, 18 Feb 2018 09:45:20 +0100 Subject: [PATCH 2/4] Use partial discriminator map on multi-inheritance Hydrator was ignoring data from subclasses when using multiple inheritance levels. With this patch it will now use the discriminator values from all subclasses of the class being hydrated. --- .../Internal/Hydration/AbstractHydrator.php | 20 ++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php b/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php index b38a212c9..1fd327be8 100644 --- a/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php +++ b/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php @@ -24,6 +24,8 @@ use Doctrine\ORM\EntityManagerInterface; use Doctrine\ORM\Events; use Doctrine\ORM\Mapping\ClassMetadata; use PDO; +use function array_map; +use function in_array; /** * Base class for all hydrators. A hydrator is a class that provides some form @@ -296,11 +298,9 @@ abstract class AbstractHydrator // If there are field name collisions in the child class, then we need // to only hydrate if we are looking at the correct discriminator value - if( - isset($cacheKeyInfo['discriminatorColumn']) && - isset($data[$cacheKeyInfo['discriminatorColumn']]) && + if (isset($cacheKeyInfo['discriminatorColumn'], $data[$cacheKeyInfo['discriminatorColumn']]) // Note: loose comparison required. See https://github.com/doctrine/doctrine2/pull/6304#issuecomment-323294442 - $data[$cacheKeyInfo['discriminatorColumn']] != $cacheKeyInfo['discriminatorValue'] + && ! in_array($data[$cacheKeyInfo['discriminatorColumn']], $cacheKeyInfo['discriminatorValues']) ) { break; } @@ -397,11 +397,21 @@ abstract class AbstractHydrator // the current discriminator value must be saved in order to disambiguate fields hydration, // should there be field name collisions if ($classMetadata->parentClasses && isset($this->_rsm->discriminatorColumns[$ownerMap])) { + $discriminatorValues = array_map( + function (string $subClass) { + return $this->getClassMetadata($subClass)->discriminatorValue; + }, + $classMetadata->subClasses + ); + + $discriminatorValues[] = $classMetadata->discriminatorValue; + return $this->_cache[$key] = \array_merge( $columnInfo, [ 'discriminatorColumn' => $this->_rsm->discriminatorColumns[$ownerMap], - 'discriminatorValue' => $classMetadata->discriminatorValue + 'discriminatorValue' => $classMetadata->discriminatorValue, + 'discriminatorValues' => $discriminatorValues, ] ); } From 2905b435db46b71a501991dccb256a7c037d8972 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lu=C3=ADs=20Cobucci?= Date: Sun, 18 Feb 2018 09:49:41 +0100 Subject: [PATCH 3/4] Remove loose comparison on discriminator values According to mapping drivers the discriminator values can always be converted to strings so it's safe to assume that we can actually do a strict comparison during hydration. --- lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php b/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php index 1fd327be8..144fe8257 100644 --- a/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php +++ b/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php @@ -299,8 +299,7 @@ abstract class AbstractHydrator // If there are field name collisions in the child class, then we need // to only hydrate if we are looking at the correct discriminator value if (isset($cacheKeyInfo['discriminatorColumn'], $data[$cacheKeyInfo['discriminatorColumn']]) - // Note: loose comparison required. See https://github.com/doctrine/doctrine2/pull/6304#issuecomment-323294442 - && ! in_array($data[$cacheKeyInfo['discriminatorColumn']], $cacheKeyInfo['discriminatorValues']) + && ! in_array((string) $data[$cacheKeyInfo['discriminatorColumn']], $cacheKeyInfo['discriminatorValues'], true) ) { break; } @@ -398,13 +397,13 @@ abstract class AbstractHydrator // should there be field name collisions if ($classMetadata->parentClasses && isset($this->_rsm->discriminatorColumns[$ownerMap])) { $discriminatorValues = array_map( - function (string $subClass) { - return $this->getClassMetadata($subClass)->discriminatorValue; + function (string $subClass) : string { + return (string) $this->getClassMetadata($subClass)->discriminatorValue; }, $classMetadata->subClasses ); - $discriminatorValues[] = $classMetadata->discriminatorValue; + $discriminatorValues[] = (string) $classMetadata->discriminatorValue; return $this->_cache[$key] = \array_merge( $columnInfo, From f2da5bc93efd95d546f5396e8489ddaafef006a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lu=C3=ADs=20Cobucci?= Date: Mon, 19 Feb 2018 11:05:30 +0100 Subject: [PATCH 4/4] Extract private method to retrieve discriminator values --- .../Internal/Hydration/AbstractHydrator.php | 28 ++++++++++++------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php b/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php index 144fe8257..5fe4fa1e5 100644 --- a/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php +++ b/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php @@ -396,21 +396,12 @@ abstract class AbstractHydrator // the current discriminator value must be saved in order to disambiguate fields hydration, // should there be field name collisions if ($classMetadata->parentClasses && isset($this->_rsm->discriminatorColumns[$ownerMap])) { - $discriminatorValues = array_map( - function (string $subClass) : string { - return (string) $this->getClassMetadata($subClass)->discriminatorValue; - }, - $classMetadata->subClasses - ); - - $discriminatorValues[] = (string) $classMetadata->discriminatorValue; - return $this->_cache[$key] = \array_merge( $columnInfo, [ 'discriminatorColumn' => $this->_rsm->discriminatorColumns[$ownerMap], 'discriminatorValue' => $classMetadata->discriminatorValue, - 'discriminatorValues' => $discriminatorValues, + 'discriminatorValues' => $this->getDiscriminatorValues($classMetadata), ] ); } @@ -463,6 +454,23 @@ abstract class AbstractHydrator return null; } + /** + * @return string[] + */ + private function getDiscriminatorValues(ClassMetadata $classMetadata) : array + { + $values = array_map( + function (string $subClass) : string { + return (string) $this->getClassMetadata($subClass)->discriminatorValue; + }, + $classMetadata->subClasses + ); + + $values[] = (string) $classMetadata->discriminatorValue; + + return $values; + } + /** * Retrieve ClassMetadata associated to entity class name. *