From aac523d15578b05084c13ec7f9dde9d8fe35186e Mon Sep 17 00:00:00 2001 From: Full Date: Sat, 4 Mar 2017 15:09:29 +0100 Subject: [PATCH 01/25] tests with custom inheritance tree --- .../Tests/Models/DDC6303/DDC6303Contract.php | 24 ++++++++ .../Tests/Models/DDC6303/DDC6303ContractA.php | 17 ++++++ .../Tests/Models/DDC6303/DDC6303ContractB.php | 17 ++++++ .../ORM/Functional/Ticket/DDC6303Test.php | 56 +++++++++++++++++++ .../Doctrine/Tests/OrmFunctionalTestCase.php | 6 ++ 5 files changed, 120 insertions(+) create mode 100644 tests/Doctrine/Tests/Models/DDC6303/DDC6303Contract.php create mode 100644 tests/Doctrine/Tests/Models/DDC6303/DDC6303ContractA.php create mode 100644 tests/Doctrine/Tests/Models/DDC6303/DDC6303ContractB.php create mode 100644 tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6303Test.php diff --git a/tests/Doctrine/Tests/Models/DDC6303/DDC6303Contract.php b/tests/Doctrine/Tests/Models/DDC6303/DDC6303Contract.php new file mode 100644 index 000000000..ccb2dee34 --- /dev/null +++ b/tests/Doctrine/Tests/Models/DDC6303/DDC6303Contract.php @@ -0,0 +1,24 @@ +useModelSet('ddc6303'); + parent::setUp(); + } + + public function testMixedTypeHydratedCorrectlyInJoinedInheritance() + { + $contractA = new DDC6303ContractA(); + $contractAData = 'authorized'; + $contractA->originalData = $contractAData; + + $contractB = new DDC6303ContractB(); + $contractBData = ['accepted', 'authorized']; + $contractB->originalData = $contractBData; + + $this->_em->persist($contractA); + $this->_em->persist($contractB); + + $this->_em->flush(); + + // clear entity manager so that $repository->find actually fetches them and uses the hydrator + // instead of just returning the existing managed entities + $this->_em->clear(); + + $repository = $this->_em->getRepository(DDC6303Contract::class); + + $contracts = $repository->createQueryBuilder('p') + ->getQuery()->getResult(); + + foreach( $contracts as $contract ){ + switch( $contract->id ){ + case $contractA->id: + static::assertEquals($contract->originalData, $contractAData); + break; + + case $contractB->id: + static::assertEquals($contract->originalData, $contractBData); + break; + } + } + } +} diff --git a/tests/Doctrine/Tests/OrmFunctionalTestCase.php b/tests/Doctrine/Tests/OrmFunctionalTestCase.php index e9299b1c6..5618fa120 100644 --- a/tests/Doctrine/Tests/OrmFunctionalTestCase.php +++ b/tests/Doctrine/Tests/OrmFunctionalTestCase.php @@ -312,6 +312,12 @@ abstract class OrmFunctionalTestCase extends OrmTestCase Models\Issue5989\Issue5989Employee::class, Models\Issue5989\Issue5989Manager::class, ], + 'ddc6303' => [ + Models\DDC6303\DDC6303Contract::class, + Models\DDC6303\DDC6303ContractA::class, + Models\DDC6303\DDC6303ContractB::class, + ] + ]; /** From 6d40859228d64ef6f2c5477042182bfd8bfd110d Mon Sep 17 00:00:00 2001 From: Full Date: Sat, 4 Mar 2017 15:27:02 +0100 Subject: [PATCH 02/25] added tests on empty values --- .../Internal/Hydration/AbstractHydrator.php | 6 ++- .../ORM/Functional/Ticket/DDC6303Test.php | 45 +++++++++++++++++++ 2 files changed, 50 insertions(+), 1 deletion(-) diff --git a/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php b/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php index 04e98c72e..7490e3e4f 100644 --- a/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php +++ b/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php @@ -294,10 +294,14 @@ abstract class AbstractHydrator $dqlAlias = $cacheKeyInfo['dqlAlias']; $type = $cacheKeyInfo['type']; + var_dump($dqlAlias); + var_dump($cacheKeyInfo); + echo "\n\n\n"; + // in an inheritance hierarchy the same field could be defined several times. // We overwrite this value so long we don't have a non-null value, that value we keep. // Per definition it cannot be that a field is defined several times and has several values. - if (isset($rowData['data'][$dqlAlias][$fieldName])) { + if (!empty($rowData['data'][$dqlAlias][$fieldName])) { break; } diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6303Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6303Test.php index 96dd32f9b..d967b3b7f 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6303Test.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6303Test.php @@ -53,4 +53,49 @@ class DDC6303Test extends \Doctrine\Tests\OrmFunctionalTestCase } } } + + public function testEmptyValuesInJoinedInheritance() + { + $contractStringEmptyData = ''; + $contractStringZeroData = 0; + + $contractArrayEmptyData = []; + + $contractStringEmpty = new DDC6303ContractA(); + $contractStringEmpty->originalData = $contractStringEmptyData; + + $contractStringZero = new DDC6303ContractA(); + $contractStringZero->originalData = $contractStringZeroData; + + $contractArrayEmpty = new DDC6303ContractB(); + $contractArrayEmpty->originalData = $contractArrayEmptyData; + + $this->_em->persist($contractStringZero); + $this->_em->persist($contractStringEmpty); + $this->_em->persist($contractArrayEmpty); + + $this->_em->flush(); + + // clear entity manager so that $repository->find actually fetches them and uses the hydrator + // instead of just returning the existing managed entities + $this->_em->clear(); + + $repository = $this->_em->getRepository(DDC6303Contract::class); + $dataMap = [ + $contractStringZero->id => $contractStringZeroData, + $contractStringEmpty->id => $contractStringEmptyData, + $contractArrayEmpty->id => $contractArrayEmptyData, + ]; + + $contracts = $repository->createQueryBuilder('p') + ->where('p.id IN(:ids)') + ->setParameter('ids', array_keys($dataMap)) + ->getQuery()->getResult(); + + + + foreach( $contracts as $contract ){ + static::assertEquals($contract->originalData, $dataMap[$contract->id], 'contract ' . get_class($contract) . ' not equals to original'); + } + } } From ad3b9de4b8a9a272fdb8c8183087148dd5b09114 Mon Sep 17 00:00:00 2001 From: Full Date: Sat, 4 Mar 2017 17:17:31 +0100 Subject: [PATCH 03/25] use dataMap also in first test function --- .../Internal/Hydration/AbstractHydrator.php | 35 +++++++++++++------ .../ORM/Functional/Ticket/DDC6303Test.php | 21 +++++------ 2 files changed, 33 insertions(+), 23 deletions(-) diff --git a/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php b/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php index 7490e3e4f..dda16b0b6 100644 --- a/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php +++ b/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php @@ -294,17 +294,20 @@ abstract class AbstractHydrator $dqlAlias = $cacheKeyInfo['dqlAlias']; $type = $cacheKeyInfo['type']; - var_dump($dqlAlias); - var_dump($cacheKeyInfo); - echo "\n\n\n"; - + if( + isset($cacheKeyInfo['discriminatorColumn']) && + isset($data[$cacheKeyInfo['discriminatorColumn']]) && + $data[$cacheKeyInfo['discriminatorColumn']] != $cacheKeyInfo['discriminatorValue'] + ){ + break; + } // in an inheritance hierarchy the same field could be defined several times. // We overwrite this value so long we don't have a non-null value, that value we keep. // Per definition it cannot be that a field is defined several times and has several values. - if (!empty($rowData['data'][$dqlAlias][$fieldName])) { + if (isset($rowData['data'][$dqlAlias][$fieldName])) { break; } - + $rowData['data'][$dqlAlias][$fieldName] = $type ? $type->convertToPHPValue($value, $this->_platform) : $value; @@ -379,13 +382,23 @@ abstract class AbstractHydrator $classMetadata = $this->getClassMetadata($this->_rsm->declaringClasses[$key]); $fieldName = $this->_rsm->fieldMappings[$key]; $fieldMapping = $classMetadata->fieldMappings[$fieldName]; + $ownerMap = $this->_rsm->columnOwnerMap[$key]; - return $this->_cache[$key] = [ - 'isIdentifier' => in_array($fieldName, $classMetadata->identifier), - 'fieldName' => $fieldName, - 'type' => Type::getType($fieldMapping['type']), - 'dqlAlias' => $this->_rsm->columnOwnerMap[$key], + $returnArray = [ + 'isIdentifier' => in_array($fieldName, $classMetadata->identifier), + 'fieldName' => $fieldName, + 'type' => Type::getType($fieldMapping['type']), + 'dqlAlias' => $ownerMap, + ]; + if( !empty($classMetadata->parentClasses)){ + $returnArray += [ + 'discriminatorColumn' => $this->_rsm->discriminatorColumns[$ownerMap], + 'discriminatorValue' => $classMetadata->discriminatorValue + ]; + } + + return $this->_cache[$key] = $returnArray; case (isset($this->_rsm->newObjectMappings[$key])): // WARNING: A NEW object is also a scalar, so it must be declared before! diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6303Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6303Test.php index d967b3b7f..a065e1d96 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6303Test.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6303Test.php @@ -38,19 +38,18 @@ class DDC6303Test extends \Doctrine\Tests\OrmFunctionalTestCase $repository = $this->_em->getRepository(DDC6303Contract::class); + $dataMap = [ + $contractA->id => $contractAData, + $contractB->id => $contractBData + ]; + $contracts = $repository->createQueryBuilder('p') - ->getQuery()->getResult(); + ->where('p.id IN(:ids)') + ->setParameter('ids', array_keys($dataMap)) + ->getQuery()->getResult(); foreach( $contracts as $contract ){ - switch( $contract->id ){ - case $contractA->id: - static::assertEquals($contract->originalData, $contractAData); - break; - - case $contractB->id: - static::assertEquals($contract->originalData, $contractBData); - break; - } + static::assertEquals($contract->originalData, $dataMap[$contract->id], 'contract ' . get_class($contract) . ' not equals to original'); } } @@ -92,8 +91,6 @@ class DDC6303Test extends \Doctrine\Tests\OrmFunctionalTestCase ->setParameter('ids', array_keys($dataMap)) ->getQuery()->getResult(); - - foreach( $contracts as $contract ){ static::assertEquals($contract->originalData, $dataMap[$contract->id], 'contract ' . get_class($contract) . ' not equals to original'); } From bf06b7dbbcb20a68c2e51744591c1a5bed2d2f00 Mon Sep 17 00:00:00 2001 From: Full Date: Sat, 4 Mar 2017 17:39:27 +0100 Subject: [PATCH 04/25] check if array is set before assigning --- lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php b/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php index dda16b0b6..7d2e64246 100644 --- a/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php +++ b/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php @@ -391,7 +391,7 @@ abstract class AbstractHydrator 'dqlAlias' => $ownerMap, ]; - if( !empty($classMetadata->parentClasses)){ + if( !empty($classMetadata->parentClasses && isset($this->_rsm->discriminatorColumns[$ownerMap]))){ $returnArray += [ 'discriminatorColumn' => $this->_rsm->discriminatorColumns[$ownerMap], 'discriminatorValue' => $classMetadata->discriminatorValue From 0b8e468f06114e3422779caabb91713d1a64d3da Mon Sep 17 00:00:00 2001 From: Full Date: Sat, 11 Mar 2017 20:24:46 +0100 Subject: [PATCH 05/25] fix wrong pharenthesis --- lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php b/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php index 7d2e64246..ae12a8947 100644 --- a/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php +++ b/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php @@ -391,7 +391,7 @@ abstract class AbstractHydrator 'dqlAlias' => $ownerMap, ]; - if( !empty($classMetadata->parentClasses && isset($this->_rsm->discriminatorColumns[$ownerMap]))){ + if( !empty($classMetadata->parentClasses) && isset($this->_rsm->discriminatorColumns[$ownerMap])){ $returnArray += [ 'discriminatorColumn' => $this->_rsm->discriminatorColumns[$ownerMap], 'discriminatorValue' => $classMetadata->discriminatorValue From 0072054020415fed75ee8e53f8670f25add651cc Mon Sep 17 00:00:00 2001 From: Full Date: Sat, 11 Mar 2017 20:37:32 +0100 Subject: [PATCH 06/25] moved all test models into test class --- .../Tests/Models/DDC6303/DDC6303Contract.php | 24 ------- .../Tests/Models/DDC6303/DDC6303ContractA.php | 17 ----- .../Tests/Models/DDC6303/DDC6303ContractB.php | 17 ----- .../ORM/Functional/Ticket/DDC6303Test.php | 66 +++++++++++++++++-- .../Doctrine/Tests/OrmFunctionalTestCase.php | 5 -- 5 files changed, 61 insertions(+), 68 deletions(-) delete mode 100644 tests/Doctrine/Tests/Models/DDC6303/DDC6303Contract.php delete mode 100644 tests/Doctrine/Tests/Models/DDC6303/DDC6303ContractA.php delete mode 100644 tests/Doctrine/Tests/Models/DDC6303/DDC6303ContractB.php diff --git a/tests/Doctrine/Tests/Models/DDC6303/DDC6303Contract.php b/tests/Doctrine/Tests/Models/DDC6303/DDC6303Contract.php deleted file mode 100644 index ccb2dee34..000000000 --- a/tests/Doctrine/Tests/Models/DDC6303/DDC6303Contract.php +++ /dev/null @@ -1,24 +0,0 @@ -useModelSet('ddc6303'); parent::setUp(); + try { + $this->_schemaTool->createSchema( + [ + $this->_em->getClassMetadata(DDC6303Contract::class), + $this->_em->getClassMetadata(DDC6303ContractA::class), + $this->_em->getClassMetadata(DDC6303ContractB::class) + ] + ); + } catch (\Exception $ignored) {} } public function testMixedTypeHydratedCorrectlyInJoinedInheritance() @@ -96,3 +100,55 @@ class DDC6303Test extends \Doctrine\Tests\OrmFunctionalTestCase } } } + + +/** + * @Entity + * @Table(name="ddc6303_contract") + * @InheritanceType("JOINED") + * @DiscriminatorColumn(name="discr", type="string") + * @DiscriminatorMap({ + * "contract" = "DDC6303Contract", + * "contract_b" = "DDC6303ContractB", + * "contract_a" = "DDC6303ContractA" + * }) + */ +class DDC6303Contract +{ + /** + * @Id + * @Column(type="integer") + * @GeneratedValue + */ + public $id; +} + + +/** + * @Entity + * @Table(name="ddc6303_contracts_a") + */ +class DDC6303ContractA extends DDC6303Contract +{ + /** + * @Column(type="string", nullable=true) + * + * @var string + */ + public $originalData; +} + + +/** + * @Entity + * @Table(name="ddc6303_contracts_b") + */ +class DDC6303ContractB extends DDC6303Contract +{ + /** + * @Column(type="simple_array", nullable=true) + * + * @var array + */ + public $originalData; +} \ No newline at end of file diff --git a/tests/Doctrine/Tests/OrmFunctionalTestCase.php b/tests/Doctrine/Tests/OrmFunctionalTestCase.php index 5618fa120..f4ba88275 100644 --- a/tests/Doctrine/Tests/OrmFunctionalTestCase.php +++ b/tests/Doctrine/Tests/OrmFunctionalTestCase.php @@ -311,11 +311,6 @@ abstract class OrmFunctionalTestCase extends OrmTestCase Models\Issue5989\Issue5989Person::class, Models\Issue5989\Issue5989Employee::class, Models\Issue5989\Issue5989Manager::class, - ], - 'ddc6303' => [ - Models\DDC6303\DDC6303Contract::class, - Models\DDC6303\DDC6303ContractA::class, - Models\DDC6303\DDC6303ContractB::class, ] ]; From 4c7286f57b652ad248d83c3bcbd1661c63701799 Mon Sep 17 00:00:00 2001 From: Full Date: Sat, 11 Mar 2017 20:39:10 +0100 Subject: [PATCH 07/25] removed trailing spaces --- lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php b/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php index ae12a8947..062f54b31 100644 --- a/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php +++ b/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php @@ -307,7 +307,7 @@ abstract class AbstractHydrator if (isset($rowData['data'][$dqlAlias][$fieldName])) { break; } - + $rowData['data'][$dqlAlias][$fieldName] = $type ? $type->convertToPHPValue($value, $this->_platform) : $value; From 82db643b4f9c1496259de7c17750b8f2c59efc0d Mon Sep 17 00:00:00 2001 From: fullbl Date: Sun, 19 Mar 2017 15:05:29 +0100 Subject: [PATCH 08/25] clarified what's the problem in a comment --- tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6303Test.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6303Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6303Test.php index c11bc49da..163dce137 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6303Test.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6303Test.php @@ -28,6 +28,7 @@ class DDC6303Test extends \Doctrine\Tests\OrmFunctionalTestCase $contractA->originalData = $contractAData; $contractB = new DDC6303ContractB(); + //contractA and contractB have an inheritance from Contract, but one has a string originalData and the second has an array $contractBData = ['accepted', 'authorized']; $contractB->originalData = $contractBData; @@ -61,7 +62,6 @@ class DDC6303Test extends \Doctrine\Tests\OrmFunctionalTestCase { $contractStringEmptyData = ''; $contractStringZeroData = 0; - $contractArrayEmptyData = []; $contractStringEmpty = new DDC6303ContractA(); @@ -151,4 +151,4 @@ class DDC6303ContractB extends DDC6303Contract * @var array */ public $originalData; -} \ No newline at end of file +} From db9c12f1aff1866aec0594a7035305b53fd86d07 Mon Sep 17 00:00:00 2001 From: Full Date: Sun, 2 Apr 2017 14:37:29 +0200 Subject: [PATCH 09/25] comparison on discriminator value with !== --- lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php b/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php index 062f54b31..95cffa125 100644 --- a/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php +++ b/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php @@ -297,10 +297,11 @@ abstract class AbstractHydrator if( isset($cacheKeyInfo['discriminatorColumn']) && isset($data[$cacheKeyInfo['discriminatorColumn']]) && - $data[$cacheKeyInfo['discriminatorColumn']] != $cacheKeyInfo['discriminatorValue'] + $data[$cacheKeyInfo['discriminatorColumn']] !== $cacheKeyInfo['discriminatorValue'] ){ break; } + // in an inheritance hierarchy the same field could be defined several times. // We overwrite this value so long we don't have a non-null value, that value we keep. // Per definition it cannot be that a field is defined several times and has several values. From 8cc29e84a0d68edbb8a5f09bf4bdc90b18190c6f Mon Sep 17 00:00:00 2001 From: fullbl Date: Mon, 26 Jun 2017 10:42:09 +0200 Subject: [PATCH 10/25] Update AbstractHydrator.php revert strict comparison in hydration (fails on some dates) --- lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php b/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php index 95cffa125..f430cb912 100644 --- a/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php +++ b/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php @@ -297,7 +297,7 @@ abstract class AbstractHydrator if( isset($cacheKeyInfo['discriminatorColumn']) && isset($data[$cacheKeyInfo['discriminatorColumn']]) && - $data[$cacheKeyInfo['discriminatorColumn']] !== $cacheKeyInfo['discriminatorValue'] + $data[$cacheKeyInfo['discriminatorColumn']] != $cacheKeyInfo['discriminatorValue'] ){ break; } From 7e7921e32f3c89992c522af7bc4927c74b7b62c8 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sat, 19 Aug 2017 15:58:50 +0200 Subject: [PATCH 11/25] #6303 #6304 documenting why the loose comparison is required --- lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php b/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php index f430cb912..7c7e2ef81 100644 --- a/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php +++ b/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php @@ -297,8 +297,9 @@ abstract class AbstractHydrator if( isset($cacheKeyInfo['discriminatorColumn']) && isset($data[$cacheKeyInfo['discriminatorColumn']]) && + // Note: loose comparison required. See https://github.com/doctrine/doctrine2/pull/6304#issuecomment-323294442 $data[$cacheKeyInfo['discriminatorColumn']] != $cacheKeyInfo['discriminatorValue'] - ){ + ) { break; } From c503b81421f35f092cecfd172fa7ed9e2d62e2e3 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sat, 19 Aug 2017 16:02:30 +0200 Subject: [PATCH 12/25] #6303 #6304 removing `+=` operator usage on arrays (scary\!), CS (alignment) --- .../Internal/Hydration/AbstractHydrator.php | 28 ++++++++++--------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php b/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php index 7c7e2ef81..b90d77790 100644 --- a/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php +++ b/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php @@ -385,22 +385,24 @@ abstract class AbstractHydrator $fieldName = $this->_rsm->fieldMappings[$key]; $fieldMapping = $classMetadata->fieldMappings[$fieldName]; $ownerMap = $this->_rsm->columnOwnerMap[$key]; - - $returnArray = [ - 'isIdentifier' => in_array($fieldName, $classMetadata->identifier), - 'fieldName' => $fieldName, - 'type' => Type::getType($fieldMapping['type']), - 'dqlAlias' => $ownerMap, - + $columnInfo = [ + 'isIdentifier' => \in_array($fieldName, $classMetadata->identifier, true), + 'fieldName' => $fieldName, + 'type' => Type::getType($fieldMapping['type']), + 'dqlAlias' => $ownerMap, ]; - if( !empty($classMetadata->parentClasses) && isset($this->_rsm->discriminatorColumns[$ownerMap])){ - $returnArray += [ - 'discriminatorColumn' => $this->_rsm->discriminatorColumns[$ownerMap], - 'discriminatorValue' => $classMetadata->discriminatorValue - ]; + + if($classMetadata->parentClasses && isset($this->_rsm->discriminatorColumns[$ownerMap])) { + return $this->_cache[$key] = \array_merge( + $columnInfo, + [ + 'discriminatorColumn' => $this->_rsm->discriminatorColumns[$ownerMap], + 'discriminatorValue' => $classMetadata->discriminatorValue + ] + ); } - return $this->_cache[$key] = $returnArray; + return $this->_cache[$key] = $columnInfo; case (isset($this->_rsm->newObjectMappings[$key])): // WARNING: A NEW object is also a scalar, so it must be declared before! From aaad25a06174e4dc71123806404a28cd9deaa0d1 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sat, 19 Aug 2017 16:25:20 +0200 Subject: [PATCH 13/25] #6303 #6304 reverting changes to the `OrmFunctionalTestCase` --- tests/Doctrine/Tests/OrmFunctionalTestCase.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/Doctrine/Tests/OrmFunctionalTestCase.php b/tests/Doctrine/Tests/OrmFunctionalTestCase.php index f4ba88275..e9299b1c6 100644 --- a/tests/Doctrine/Tests/OrmFunctionalTestCase.php +++ b/tests/Doctrine/Tests/OrmFunctionalTestCase.php @@ -311,8 +311,7 @@ abstract class OrmFunctionalTestCase extends OrmTestCase Models\Issue5989\Issue5989Person::class, Models\Issue5989\Issue5989Employee::class, Models\Issue5989\Issue5989Manager::class, - ] - + ], ]; /** From f6ce69fe296ebfa3b2dae790cf39b3d4c91e7ab4 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sat, 19 Aug 2017 16:37:24 +0200 Subject: [PATCH 14/25] #6303 #6304 minor CS fixes in the test code - alignment/formatting --- .../ORM/Functional/Ticket/DDC6303Test.php | 127 ++++++++---------- 1 file changed, 59 insertions(+), 68 deletions(-) diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6303Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6303Test.php index 163dce137..f11d9974f 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6303Test.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6303Test.php @@ -2,109 +2,120 @@ namespace Doctrine\Tests\ORM\Functional\Ticket; +use Doctrine\DBAL\Schema\SchemaException; +use Doctrine\Tests\OrmFunctionalTestCase; + /** - * @group DDC6303 + * @group #6303 */ -class DDC6303Test extends \Doctrine\Tests\OrmFunctionalTestCase +class DDC6303Test extends OrmFunctionalTestCase { public function setUp() { parent::setUp(); try { - $this->_schemaTool->createSchema( - [ + $this->_schemaTool->createSchema([ $this->_em->getClassMetadata(DDC6303Contract::class), $this->_em->getClassMetadata(DDC6303ContractA::class), - $this->_em->getClassMetadata(DDC6303ContractB::class) - ] - ); - } catch (\Exception $ignored) {} + $this->_em->getClassMetadata(DDC6303ContractB::class), + ]); + } catch (SchemaException $ignored) { + } } - + public function testMixedTypeHydratedCorrectlyInJoinedInheritance() { - $contractA = new DDC6303ContractA(); - $contractAData = 'authorized'; + $contractA = new DDC6303ContractA(); + $contractAData = 'authorized'; $contractA->originalData = $contractAData; $contractB = new DDC6303ContractB(); //contractA and contractB have an inheritance from Contract, but one has a string originalData and the second has an array - $contractBData = ['accepted', 'authorized']; + $contractBData = ['accepted', 'authorized']; $contractB->originalData = $contractBData; $this->_em->persist($contractA); $this->_em->persist($contractB); $this->_em->flush(); - + // clear entity manager so that $repository->find actually fetches them and uses the hydrator // instead of just returning the existing managed entities $this->_em->clear(); - $repository = $this->_em->getRepository(DDC6303Contract::class); + $repository = $this->_em->getRepository(DDC6303Contract::class); $dataMap = [ $contractA->id => $contractAData, - $contractB->id => $contractBData - ]; + $contractB->id => $contractBData, + ]; - $contracts = $repository->createQueryBuilder('p') + $contracts = $repository + ->createQueryBuilder('p') ->where('p.id IN(:ids)') ->setParameter('ids', array_keys($dataMap)) - ->getQuery()->getResult(); + ->getQuery()->getResult(); - foreach( $contracts as $contract ){ - static::assertEquals($contract->originalData, $dataMap[$contract->id], 'contract ' . get_class($contract) . ' not equals to original'); + foreach ($contracts as $contract) { + static::assertEquals( + $contract->originalData, + $dataMap[$contract->id], + 'contract ' . get_class($contract) . ' not equals to original' + ); } - } + } - public function testEmptyValuesInJoinedInheritance() + public function testEmptyValuesInJoinedInheritance() { $contractStringEmptyData = ''; - $contractStringZeroData = 0; - $contractArrayEmptyData = []; + $contractStringZeroData = 0; + $contractArrayEmptyData = []; $contractStringEmpty = new DDC6303ContractA(); + $contractStringZero = new DDC6303ContractA(); + $contractArrayEmpty = new DDC6303ContractB(); + $contractStringEmpty->originalData = $contractStringEmptyData; - - $contractStringZero = new DDC6303ContractA(); - $contractStringZero->originalData = $contractStringZeroData; - - $contractArrayEmpty = new DDC6303ContractB(); - $contractArrayEmpty->originalData = $contractArrayEmptyData; + $contractStringZero->originalData = $contractStringZeroData; + $contractArrayEmpty->originalData = $contractArrayEmptyData; $this->_em->persist($contractStringZero); $this->_em->persist($contractStringEmpty); $this->_em->persist($contractArrayEmpty); $this->_em->flush(); - + // clear entity manager so that $repository->find actually fetches them and uses the hydrator // instead of just returning the existing managed entities $this->_em->clear(); - $repository = $this->_em->getRepository(DDC6303Contract::class); - $dataMap = [ - $contractStringZero->id => $contractStringZeroData, + $repository = $this->_em->getRepository(DDC6303Contract::class); + $dataMap = [ + $contractStringZero->id => $contractStringZeroData, $contractStringEmpty->id => $contractStringEmptyData, - $contractArrayEmpty->id => $contractArrayEmptyData, - ]; + $contractArrayEmpty->id => $contractArrayEmptyData, + ]; - $contracts = $repository->createQueryBuilder('p') + $contracts = $repository + ->createQueryBuilder('p') ->where('p.id IN(:ids)') ->setParameter('ids', array_keys($dataMap)) - ->getQuery()->getResult(); + ->getQuery() + ->getResult(); - foreach( $contracts as $contract ){ - static::assertEquals($contract->originalData, $dataMap[$contract->id], 'contract ' . get_class($contract) . ' not equals to original'); + foreach ($contracts as $contract) { + static::assertEquals( + $contract->originalData, + $dataMap[$contract->id], + 'contract ' . get_class($contract) . ' not equals to original' + ); } - } + } } - /** * @Entity - * @Table(name="ddc6303_contract") + * @Table * @InheritanceType("JOINED") * @DiscriminatorColumn(name="discr", type="string") * @DiscriminatorMap({ @@ -115,40 +126,20 @@ class DDC6303Test extends \Doctrine\Tests\OrmFunctionalTestCase */ class DDC6303Contract { - /** - * @Id - * @Column(type="integer") - * @GeneratedValue - */ + /** @Id @Column(type="integer") @GeneratedValue */ public $id; } - -/** - * @Entity - * @Table(name="ddc6303_contracts_a") - */ +/** @Entity @Table */ class DDC6303ContractA extends DDC6303Contract { - /** - * @Column(type="string", nullable=true) - * - * @var string - */ + /** @Column(type="string", nullable=true) */ public $originalData; } - -/** - * @Entity - * @Table(name="ddc6303_contracts_b") - */ +/** @Entity @Table */ class DDC6303ContractB extends DDC6303Contract { - /** - * @Column(type="simple_array", nullable=true) - * - * @var array - */ + /** @Column(type="simple_array", nullable=true) */ public $originalData; } From 328467c226d408d25ff39aa096f927d3e0d7f199 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sat, 19 Aug 2017 16:48:50 +0200 Subject: [PATCH 15/25] #6303 #6304 simplified tests, removing references to `contract` naming --- .../ORM/Functional/Ticket/DDC6303Test.php | 57 ++++++++++--------- 1 file changed, 31 insertions(+), 26 deletions(-) diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6303Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6303Test.php index f11d9974f..5852be817 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6303Test.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6303Test.php @@ -3,6 +3,7 @@ namespace Doctrine\Tests\ORM\Functional\Ticket; use Doctrine\DBAL\Schema\SchemaException; +use Doctrine\ORM\Tools\ToolsException; use Doctrine\Tests\OrmFunctionalTestCase; /** @@ -13,29 +14,33 @@ class DDC6303Test extends OrmFunctionalTestCase public function setUp() { parent::setUp(); + try { $this->_schemaTool->createSchema([ - $this->_em->getClassMetadata(DDC6303Contract::class), - $this->_em->getClassMetadata(DDC6303ContractA::class), - $this->_em->getClassMetadata(DDC6303ContractB::class), + $this->_em->getClassMetadata(DDC6303BaseClass::class), + $this->_em->getClassMetadata(DDC6303ChildA::class), + $this->_em->getClassMetadata(DDC6303ChildB::class), ]); - } catch (SchemaException $ignored) { + } catch (ToolsException $ignored) { } } public function testMixedTypeHydratedCorrectlyInJoinedInheritance() { - $contractA = new DDC6303ContractA(); - $contractAData = 'authorized'; - $contractA->originalData = $contractAData; + $a = new DDC6303ChildA(); + $b = new DDC6303ChildB(); - $contractB = new DDC6303ContractB(); - //contractA and contractB have an inheritance from Contract, but one has a string originalData and the second has an array - $contractBData = ['accepted', 'authorized']; - $contractB->originalData = $contractBData; + $aData = 'authorized'; + $bData = ['accepted', 'authorized']; - $this->_em->persist($contractA); - $this->_em->persist($contractB); + // DDC6303ChildA and DDC6303ChildB have an inheritance from DDC6303BaseClass, + // but one has a string originalData and the second has an array, since the fields + // are mapped differently + $a->originalData = $aData; + $b->originalData = $bData; + + $this->_em->persist($a); + $this->_em->persist($b); $this->_em->flush(); @@ -43,11 +48,11 @@ class DDC6303Test extends OrmFunctionalTestCase // instead of just returning the existing managed entities $this->_em->clear(); - $repository = $this->_em->getRepository(DDC6303Contract::class); + $repository = $this->_em->getRepository(DDC6303BaseClass::class); $dataMap = [ - $contractA->id => $contractAData, - $contractB->id => $contractBData, + $a->id => $aData, + $b->id => $bData, ]; $contracts = $repository @@ -71,9 +76,9 @@ class DDC6303Test extends OrmFunctionalTestCase $contractStringZeroData = 0; $contractArrayEmptyData = []; - $contractStringEmpty = new DDC6303ContractA(); - $contractStringZero = new DDC6303ContractA(); - $contractArrayEmpty = new DDC6303ContractB(); + $contractStringEmpty = new DDC6303ChildA(); + $contractStringZero = new DDC6303ChildA(); + $contractArrayEmpty = new DDC6303ChildB(); $contractStringEmpty->originalData = $contractStringEmptyData; $contractStringZero->originalData = $contractStringZeroData; @@ -89,13 +94,14 @@ class DDC6303Test extends OrmFunctionalTestCase // instead of just returning the existing managed entities $this->_em->clear(); - $repository = $this->_em->getRepository(DDC6303Contract::class); + $repository = $this->_em->getRepository(DDC6303BaseClass::class); $dataMap = [ $contractStringZero->id => $contractStringZeroData, $contractStringEmpty->id => $contractStringEmptyData, $contractArrayEmpty->id => $contractArrayEmptyData, ]; + /* @var $contracts DDC6303ChildA[]|DDC6303ChildB[] */ $contracts = $repository ->createQueryBuilder('p') ->where('p.id IN(:ids)') @@ -119,26 +125,25 @@ class DDC6303Test extends OrmFunctionalTestCase * @InheritanceType("JOINED") * @DiscriminatorColumn(name="discr", type="string") * @DiscriminatorMap({ - * "contract" = "DDC6303Contract", - * "contract_b" = "DDC6303ContractB", - * "contract_a" = "DDC6303ContractA" + * DDC6303ChildA::class = DDC6303ChildA::class, + * DDC6303ChildB::class = DDC6303ChildB::class, * }) */ -class DDC6303Contract +abstract class DDC6303BaseClass { /** @Id @Column(type="integer") @GeneratedValue */ public $id; } /** @Entity @Table */ -class DDC6303ContractA extends DDC6303Contract +class DDC6303ChildA extends DDC6303BaseClass { /** @Column(type="string", nullable=true) */ public $originalData; } /** @Entity @Table */ -class DDC6303ContractB extends DDC6303Contract +class DDC6303ChildB extends DDC6303BaseClass { /** @Column(type="simple_array", nullable=true) */ public $originalData; From 0882b1021361be132e3e06027c3f022b58330731 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sat, 19 Aug 2017 16:51:31 +0200 Subject: [PATCH 16/25] #6303 #6304 simplified tests, removing references to `contract` naming --- .../ORM/Functional/Ticket/DDC6303Test.php | 57 ++++++++++--------- 1 file changed, 29 insertions(+), 28 deletions(-) diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6303Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6303Test.php index 5852be817..042e5a648 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6303Test.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6303Test.php @@ -55,38 +55,39 @@ class DDC6303Test extends OrmFunctionalTestCase $b->id => $bData, ]; - $contracts = $repository + /* @var $entities DDC6303ChildA[]|DDC6303ChildB[] */ + $entities = $repository ->createQueryBuilder('p') ->where('p.id IN(:ids)') ->setParameter('ids', array_keys($dataMap)) ->getQuery()->getResult(); - foreach ($contracts as $contract) { - static::assertEquals( - $contract->originalData, - $dataMap[$contract->id], - 'contract ' . get_class($contract) . ' not equals to original' + foreach ($entities as $entity) { + self::assertEquals( + $entity->originalData, + $dataMap[$entity->id], + get_class($entity) . ' not equals to original' ); } } public function testEmptyValuesInJoinedInheritance() { - $contractStringEmptyData = ''; - $contractStringZeroData = 0; - $contractArrayEmptyData = []; + $stringEmptyData = ''; + $stringZeroData = 0; + $arrayEmptyData = []; - $contractStringEmpty = new DDC6303ChildA(); - $contractStringZero = new DDC6303ChildA(); - $contractArrayEmpty = new DDC6303ChildB(); + $stringEmpty = new DDC6303ChildA(); + $stringZero = new DDC6303ChildA(); + $arrayEmpty = new DDC6303ChildB(); - $contractStringEmpty->originalData = $contractStringEmptyData; - $contractStringZero->originalData = $contractStringZeroData; - $contractArrayEmpty->originalData = $contractArrayEmptyData; + $stringEmpty->originalData = $stringEmptyData; + $stringZero->originalData = $stringZeroData; + $arrayEmpty->originalData = $arrayEmptyData; - $this->_em->persist($contractStringZero); - $this->_em->persist($contractStringEmpty); - $this->_em->persist($contractArrayEmpty); + $this->_em->persist($stringZero); + $this->_em->persist($stringEmpty); + $this->_em->persist($arrayEmpty); $this->_em->flush(); @@ -96,24 +97,24 @@ class DDC6303Test extends OrmFunctionalTestCase $repository = $this->_em->getRepository(DDC6303BaseClass::class); $dataMap = [ - $contractStringZero->id => $contractStringZeroData, - $contractStringEmpty->id => $contractStringEmptyData, - $contractArrayEmpty->id => $contractArrayEmptyData, + $stringZero->id => $stringZeroData, + $stringEmpty->id => $stringEmptyData, + $arrayEmpty->id => $arrayEmptyData, ]; - /* @var $contracts DDC6303ChildA[]|DDC6303ChildB[] */ - $contracts = $repository + /* @var $entities DDC6303ChildA[]|DDC6303ChildB[] */ + $entities = $repository ->createQueryBuilder('p') ->where('p.id IN(:ids)') ->setParameter('ids', array_keys($dataMap)) ->getQuery() ->getResult(); - foreach ($contracts as $contract) { - static::assertEquals( - $contract->originalData, - $dataMap[$contract->id], - 'contract ' . get_class($contract) . ' not equals to original' + foreach ($entities as $entity) { + self::assertEquals( + $entity->originalData, + $dataMap[$entity->id], + get_class($entity) . ' not equals to original' ); } } From 2ab363ab8228d11a2da87e76eaee28c8c1e8403a Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sat, 19 Aug 2017 16:53:41 +0200 Subject: [PATCH 17/25] #6303 #6304 adding `void` hints where applicable --- tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6303Test.php | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6303Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6303Test.php index 042e5a648..a6cd5a7d6 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6303Test.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6303Test.php @@ -2,7 +2,6 @@ namespace Doctrine\Tests\ORM\Functional\Ticket; -use Doctrine\DBAL\Schema\SchemaException; use Doctrine\ORM\Tools\ToolsException; use Doctrine\Tests\OrmFunctionalTestCase; @@ -11,7 +10,7 @@ use Doctrine\Tests\OrmFunctionalTestCase; */ class DDC6303Test extends OrmFunctionalTestCase { - public function setUp() + public function setUp() : void { parent::setUp(); @@ -25,7 +24,7 @@ class DDC6303Test extends OrmFunctionalTestCase } } - public function testMixedTypeHydratedCorrectlyInJoinedInheritance() + public function testMixedTypeHydratedCorrectlyInJoinedInheritance() : void { $a = new DDC6303ChildA(); $b = new DDC6303ChildB(); @@ -71,7 +70,7 @@ class DDC6303Test extends OrmFunctionalTestCase } } - public function testEmptyValuesInJoinedInheritance() + public function testEmptyValuesInJoinedInheritance() : void { $stringEmptyData = ''; $stringZeroData = 0; From e18fb6607d202ffcb6f05a23a2b79f962fc86b62 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sat, 19 Aug 2017 17:04:26 +0200 Subject: [PATCH 18/25] #6303 #6304 removing duplicate test details --- .../ORM/Functional/Ticket/DDC6303Test.php | 114 +++++++----------- 1 file changed, 41 insertions(+), 73 deletions(-) diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6303Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6303Test.php index a6cd5a7d6..51fcb1292 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6303Test.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6303Test.php @@ -26,95 +26,51 @@ class DDC6303Test extends OrmFunctionalTestCase public function testMixedTypeHydratedCorrectlyInJoinedInheritance() : void { - $a = new DDC6303ChildA(); - $b = new DDC6303ChildB(); - - $aData = 'authorized'; - $bData = ['accepted', 'authorized']; - // DDC6303ChildA and DDC6303ChildB have an inheritance from DDC6303BaseClass, // but one has a string originalData and the second has an array, since the fields // are mapped differently - $a->originalData = $aData; - $b->originalData = $bData; + $this->assertHydratedEntitiesSameToPersistedOnes([ + 'a' => new DDC6303ChildA('a', 'authorized'), + 'b' => new DDC6303ChildB('b', ['accepted', 'authorized']), + ]); - $this->_em->persist($a); - $this->_em->persist($b); - - $this->_em->flush(); - - // clear entity manager so that $repository->find actually fetches them and uses the hydrator - // instead of just returning the existing managed entities - $this->_em->clear(); - - $repository = $this->_em->getRepository(DDC6303BaseClass::class); - - $dataMap = [ - $a->id => $aData, - $b->id => $bData, - ]; - - /* @var $entities DDC6303ChildA[]|DDC6303ChildB[] */ - $entities = $repository - ->createQueryBuilder('p') - ->where('p.id IN(:ids)') - ->setParameter('ids', array_keys($dataMap)) - ->getQuery()->getResult(); - - foreach ($entities as $entity) { - self::assertEquals( - $entity->originalData, - $dataMap[$entity->id], - get_class($entity) . ' not equals to original' - ); - } } public function testEmptyValuesInJoinedInheritance() : void { - $stringEmptyData = ''; - $stringZeroData = 0; - $arrayEmptyData = []; - - $stringEmpty = new DDC6303ChildA(); - $stringZero = new DDC6303ChildA(); - $arrayEmpty = new DDC6303ChildB(); - - $stringEmpty->originalData = $stringEmptyData; - $stringZero->originalData = $stringZeroData; - $arrayEmpty->originalData = $arrayEmptyData; - - $this->_em->persist($stringZero); - $this->_em->persist($stringEmpty); - $this->_em->persist($arrayEmpty); + $this->assertHydratedEntitiesSameToPersistedOnes([ + 'stringEmpty' => new DDC6303ChildA('stringEmpty', ''), + 'stringZero' => new DDC6303ChildA('stringZero', 0), + 'arrayEmpty' => new DDC6303ChildB('arrayEmpty', []), + ]); + } + /** + * @param DDC6303ChildA[]|DDC6303ChildB[] $persistedEntities indexed by identifier + * + * @throws \Doctrine\Common\Persistence\Mapping\MappingException + * @throws \Doctrine\ORM\ORMException + * @throws \Doctrine\ORM\OptimisticLockException + */ + private function assertHydratedEntitiesSameToPersistedOnes(array $persistedEntities) : void + { + array_walk($persistedEntities, [$this->_em, 'persist']); $this->_em->flush(); - - // clear entity manager so that $repository->find actually fetches them and uses the hydrator - // instead of just returning the existing managed entities $this->_em->clear(); - $repository = $this->_em->getRepository(DDC6303BaseClass::class); - $dataMap = [ - $stringZero->id => $stringZeroData, - $stringEmpty->id => $stringEmptyData, - $arrayEmpty->id => $arrayEmptyData, - ]; - /* @var $entities DDC6303ChildA[]|DDC6303ChildB[] */ - $entities = $repository + $entities = $this + ->_em + ->getRepository(DDC6303BaseClass::class) ->createQueryBuilder('p') ->where('p.id IN(:ids)') - ->setParameter('ids', array_keys($dataMap)) - ->getQuery() - ->getResult(); + ->setParameter('ids', array_keys($persistedEntities)) + ->getQuery()->getResult(); + + self::assertCount(count($persistedEntities), $entities); foreach ($entities as $entity) { - self::assertEquals( - $entity->originalData, - $dataMap[$entity->id], - get_class($entity) . ' not equals to original' - ); + self::assertEquals($entity, $persistedEntities[$entity->id]); } } } @@ -131,7 +87,7 @@ class DDC6303Test extends OrmFunctionalTestCase */ abstract class DDC6303BaseClass { - /** @Id @Column(type="integer") @GeneratedValue */ + /** @Id @Column(type="string") @GeneratedValue(strategy="NONE") */ public $id; } @@ -140,6 +96,12 @@ class DDC6303ChildA extends DDC6303BaseClass { /** @Column(type="string", nullable=true) */ public $originalData; + + public function __construct(string $id, string $originalData) + { + $this->id = $id; + $this->originalData = $originalData; + } } /** @Entity @Table */ @@ -147,4 +109,10 @@ class DDC6303ChildB extends DDC6303BaseClass { /** @Column(type="simple_array", nullable=true) */ public $originalData; + + public function __construct(string $id, array $originalData) + { + $this->id = $id; + $this->originalData = $originalData; + } } From 468496be1a9800c56ca4dbb6bcced18d09077a4d Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sat, 19 Aug 2017 17:05:44 +0200 Subject: [PATCH 19/25] #6303 #6304 using strict types in the tests --- tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6303Test.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6303Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6303Test.php index 51fcb1292..17e956671 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6303Test.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6303Test.php @@ -1,4 +1,5 @@ id = $id; $this->originalData = $originalData; From 764ab5988288e80dfa68ad51d49a5ae550c9eb2c Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sat, 19 Aug 2017 17:06:27 +0200 Subject: [PATCH 20/25] #6303 #6304 making unnecessary visible fields `private` --- tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6303Test.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6303Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6303Test.php index 17e956671..1847dcde2 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6303Test.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6303Test.php @@ -96,7 +96,7 @@ abstract class DDC6303BaseClass class DDC6303ChildA extends DDC6303BaseClass { /** @Column(type="string", nullable=true) */ - public $originalData; + private $originalData; public function __construct(string $id, $originalData) { @@ -109,7 +109,7 @@ class DDC6303ChildA extends DDC6303BaseClass class DDC6303ChildB extends DDC6303BaseClass { /** @Column(type="simple_array", nullable=true) */ - public $originalData; + private $originalData; public function __construct(string $id, array $originalData) { From 8af68614fcdc69b2d8cc1f136cb28b4dbf2e8bf3 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sat, 19 Aug 2017 17:45:26 +0200 Subject: [PATCH 21/25] #6303 #6304 correcting type mapping. For `simple_array`, `[] == null` (which is bullshit), so we use `array` here --- .../ORM/Functional/Ticket/DDC6303Test.php | 34 +++++++++---------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6303Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6303Test.php index 1847dcde2..79324d0f2 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6303Test.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6303Test.php @@ -24,24 +24,24 @@ class DDC6303Test extends OrmFunctionalTestCase } catch (ToolsException $ignored) { } } - - public function testMixedTypeHydratedCorrectlyInJoinedInheritance() : void - { - // DDC6303ChildA and DDC6303ChildB have an inheritance from DDC6303BaseClass, - // but one has a string originalData and the second has an array, since the fields - // are mapped differently - $this->assertHydratedEntitiesSameToPersistedOnes([ - 'a' => new DDC6303ChildA('a', 'authorized'), - 'b' => new DDC6303ChildB('b', ['accepted', 'authorized']), - ]); - - } - +// +// public function testMixedTypeHydratedCorrectlyInJoinedInheritance() : void +// { +// // DDC6303ChildA and DDC6303ChildB have an inheritance from DDC6303BaseClass, +// // but one has a string originalData and the second has an array, since the fields +// // are mapped differently +// $this->assertHydratedEntitiesSameToPersistedOnes([ +// 'a' => new DDC6303ChildA('a', 'authorized'), +// 'b' => new DDC6303ChildB('b', ['accepted', 'authorized']), +// ]); +// +// } +// public function testEmptyValuesInJoinedInheritance() : void { $this->assertHydratedEntitiesSameToPersistedOnes([ - 'stringEmpty' => new DDC6303ChildA('stringEmpty', ''), - 'stringZero' => new DDC6303ChildA('stringZero', 0), +// 'stringEmpty' => new DDC6303ChildA('stringEmpty', ''), +// 'stringZero' => new DDC6303ChildA('stringZero', 0), 'arrayEmpty' => new DDC6303ChildB('arrayEmpty', []), ]); } @@ -95,7 +95,7 @@ abstract class DDC6303BaseClass /** @Entity @Table */ class DDC6303ChildA extends DDC6303BaseClass { - /** @Column(type="string", nullable=true) */ + /** @Column(type="string") */ private $originalData; public function __construct(string $id, $originalData) @@ -108,7 +108,7 @@ class DDC6303ChildA extends DDC6303BaseClass /** @Entity @Table */ class DDC6303ChildB extends DDC6303BaseClass { - /** @Column(type="simple_array", nullable=true) */ + /** @Column(type="array") */ private $originalData; public function __construct(string $id, array $originalData) From 42d9162bd507bb656af8468d2ba3a4413802189f Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sat, 19 Aug 2017 17:47:12 +0200 Subject: [PATCH 22/25] #6303 #6304 re-enabling disabled test code data --- .../ORM/Functional/Ticket/DDC6303Test.php | 30 +++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6303Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6303Test.php index 79324d0f2..7c4510c06 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6303Test.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6303Test.php @@ -24,24 +24,24 @@ class DDC6303Test extends OrmFunctionalTestCase } catch (ToolsException $ignored) { } } -// -// public function testMixedTypeHydratedCorrectlyInJoinedInheritance() : void -// { -// // DDC6303ChildA and DDC6303ChildB have an inheritance from DDC6303BaseClass, -// // but one has a string originalData and the second has an array, since the fields -// // are mapped differently -// $this->assertHydratedEntitiesSameToPersistedOnes([ -// 'a' => new DDC6303ChildA('a', 'authorized'), -// 'b' => new DDC6303ChildB('b', ['accepted', 'authorized']), -// ]); -// -// } -// + + public function testMixedTypeHydratedCorrectlyInJoinedInheritance() : void + { + // DDC6303ChildA and DDC6303ChildB have an inheritance from DDC6303BaseClass, + // but one has a string originalData and the second has an array, since the fields + // are mapped differently + $this->assertHydratedEntitiesSameToPersistedOnes([ + 'a' => new DDC6303ChildA('a', 'authorized'), + 'b' => new DDC6303ChildB('b', ['accepted', 'authorized']), + ]); + + } + public function testEmptyValuesInJoinedInheritance() : void { $this->assertHydratedEntitiesSameToPersistedOnes([ -// 'stringEmpty' => new DDC6303ChildA('stringEmpty', ''), -// 'stringZero' => new DDC6303ChildA('stringZero', 0), + 'stringEmpty' => new DDC6303ChildA('stringEmpty', ''), + 'stringZero' => new DDC6303ChildA('stringZero', 0), 'arrayEmpty' => new DDC6303ChildB('arrayEmpty', []), ]); } From b66643d52ef0e61ee14575d00ecfabcb76e13a96 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sat, 19 Aug 2017 17:49:14 +0200 Subject: [PATCH 23/25] #6303 #6304 removing useless union type usage --- tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6303Test.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6303Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6303Test.php index 7c4510c06..32cb5ce97 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6303Test.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6303Test.php @@ -47,7 +47,7 @@ class DDC6303Test extends OrmFunctionalTestCase } /** - * @param DDC6303ChildA[]|DDC6303ChildB[] $persistedEntities indexed by identifier + * @param DDC6303BaseClass[] $persistedEntities indexed by identifier * * @throws \Doctrine\Common\Persistence\Mapping\MappingException * @throws \Doctrine\ORM\ORMException @@ -59,7 +59,7 @@ class DDC6303Test extends OrmFunctionalTestCase $this->_em->flush(); $this->_em->clear(); - /* @var $entities DDC6303ChildA[]|DDC6303ChildB[] */ + /* @var $entities DDC6303BaseClass[] */ $entities = $this ->_em ->getRepository(DDC6303BaseClass::class) From 4cbcdb761a947c9026c0dfe850337e3cc045b1a6 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sat, 19 Aug 2017 18:25:44 +0200 Subject: [PATCH 24/25] #6303 #6304 documenting that the discriminator map order is extremely important for this test --- tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6303Test.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6303Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6303Test.php index 32cb5ce97..732808b5f 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6303Test.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6303Test.php @@ -82,9 +82,11 @@ class DDC6303Test extends OrmFunctionalTestCase * @InheritanceType("JOINED") * @DiscriminatorColumn(name="discr", type="string") * @DiscriminatorMap({ - * DDC6303ChildA::class = DDC6303ChildA::class, * DDC6303ChildB::class = DDC6303ChildB::class, + * DDC6303ChildA::class = DDC6303ChildA::class, * }) + * + * Note: discriminator map order *IS IMPORTANT* for this test */ abstract class DDC6303BaseClass { @@ -108,7 +110,7 @@ class DDC6303ChildA extends DDC6303BaseClass /** @Entity @Table */ class DDC6303ChildB extends DDC6303BaseClass { - /** @Column(type="array") */ + /** @Column(type="simple_array", nullable=true) */ private $originalData; public function __construct(string $id, array $originalData) From 44f2e22f1433fe704deb9b9e7a051b205070aa27 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sat, 19 Aug 2017 18:30:33 +0200 Subject: [PATCH 25/25] #6303 #6304 documenting why collisions in field name hydration in STI/JTI require additional information and checks in the hydration process --- lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php b/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php index b90d77790..b38a212c9 100644 --- a/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php +++ b/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php @@ -294,6 +294,8 @@ abstract class AbstractHydrator $dqlAlias = $cacheKeyInfo['dqlAlias']; $type = $cacheKeyInfo['type']; + // 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']]) && @@ -392,7 +394,9 @@ abstract class AbstractHydrator 'dqlAlias' => $ownerMap, ]; - if($classMetadata->parentClasses && isset($this->_rsm->discriminatorColumns[$ownerMap])) { + // 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])) { return $this->_cache[$key] = \array_merge( $columnInfo, [