From 59fe2da7cfa1555d144acc4320f83f8acb6eb51c Mon Sep 17 00:00:00 2001 From: Tomas Date: Tue, 12 Apr 2022 22:07:50 +0200 Subject: [PATCH 1/7] First attempt on fix of issue #9622 --- .../Internal/Hydration/AbstractHydrator.php | 9 ++++++- lib/Doctrine/ORM/Query/ResultSetMapping.php | 21 +++++++++++++++ lib/Doctrine/ORM/Query/SqlWalker.php | 4 +++ .../Tests/ORM/Functional/EnumTest.php | 26 +++++++++++++++++++ 4 files changed, 59 insertions(+), 1 deletion(-) diff --git a/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php b/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php index 139f7fa6fab..845681dabb5 100644 --- a/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php +++ b/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php @@ -429,7 +429,12 @@ protected function gatherRowData(array $data, array &$id, array &$nonemptyCompon $type = $cacheKeyInfo['type']; $value = $type->convertToPHPValue($value, $this->_platform); - $rowData['scalars'][$fieldName] = $value; + if (isset($cacheKeyInfo['enumType'])) { + $rowData['scalars'][$fieldName] = ($cacheKeyInfo['enumType'])::from($value); + } else { + $rowData['scalars'][$fieldName] = $value; + } + break; //case (isset($cacheKeyInfo['isMetaColumn'])): @@ -572,6 +577,7 @@ protected function hydrateColumnInfo($key) 'fieldName' => $this->_rsm->scalarMappings[$key], 'type' => Type::getType($this->_rsm->typeMappings[$key]), 'dqlAlias' => '', + 'enumType' => $this->_rsm->enumMappings[$key] ?? null, ]; case isset($this->_rsm->scalarMappings[$key]): @@ -579,6 +585,7 @@ protected function hydrateColumnInfo($key) 'isScalar' => true, 'fieldName' => $this->_rsm->scalarMappings[$key], 'type' => Type::getType($this->_rsm->typeMappings[$key]), + 'enumType' => $this->_rsm->enumMappings[$key] ?? null, ]; case isset($this->_rsm->metaMappings[$key]): diff --git a/lib/Doctrine/ORM/Query/ResultSetMapping.php b/lib/Doctrine/ORM/Query/ResultSetMapping.php index a258d840212..305ed7492c1 100644 --- a/lib/Doctrine/ORM/Query/ResultSetMapping.php +++ b/lib/Doctrine/ORM/Query/ResultSetMapping.php @@ -77,6 +77,14 @@ class ResultSetMapping */ public $scalarMappings = []; + /** + * Maps scalar columns to enums + * + * @ignore + * @psalm-var array + */ + public $enumMappings = []; + /** * Maps column names in the result set to the alias/field type to use in the mapped result. * @@ -384,6 +392,19 @@ public function addScalarResult($columnName, $alias, $type = 'string') return $this; } + /** + * Adds a scalar result mapping. + * + * @param string $columnName The name of the column in the SQL result set. + * @param string $enumType The enum type + * + * @return $this + */ + public function addEnumResult($columnName, $enumType) + { + $this->enumMappings[$columnName] = $enumType; + } + /** * Adds a metadata parameter mappings. * diff --git a/lib/Doctrine/ORM/Query/SqlWalker.php b/lib/Doctrine/ORM/Query/SqlWalker.php index bbaa47130c5..7539b53ec91 100644 --- a/lib/Doctrine/ORM/Query/SqlWalker.php +++ b/lib/Doctrine/ORM/Query/SqlWalker.php @@ -1359,6 +1359,10 @@ public function walkSelectExpression($selectExpression) if (! $hidden) { $this->rsm->addScalarResult($columnAlias, $resultAlias, $fieldMapping['type']); $this->scalarFields[$dqlAlias][$fieldName] = $columnAlias; + + if (! empty($fieldMapping['enumType'])) { + $this->rsm->addEnumResult($columnAlias, $fieldMapping['enumType']); + } } break; diff --git a/tests/Doctrine/Tests/ORM/Functional/EnumTest.php b/tests/Doctrine/Tests/ORM/Functional/EnumTest.php index 39a595f1083..79073999245 100644 --- a/tests/Doctrine/Tests/ORM/Functional/EnumTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/EnumTest.php @@ -61,6 +61,32 @@ public function testEnumMapping(string $cardClass): void $this->assertEquals(Suit::Clubs, $fetchedCard->suit); } + /** + * @param class-string $cardClass + * + * @dataProvider provideCardClasses + */ + public function testEnumHydration(string $cardClass): void + { + $this->setUpEntitySchema([$cardClass]); + + $card = new $cardClass(); + $card->suit = Suit::Clubs; + + $this->_em->persist($card); + $this->_em->flush(); + $this->_em->clear(); + + $result = $this->_em->createQueryBuilder() + ->from($cardClass, 'c') + ->select('c.id, c.suit') + ->getQuery() + ->getResult(); + + $this->assertInstanceOf(Suit::class, $result[0]['suit']); + $this->assertEquals(Suit::Clubs, $result[0]['suit']); + } + public function testFindByEnum(): void { $this->setUpEntitySchema([Card::class]); From e0831a66c54860c82bddf4170f8a2d157c1660b1 Mon Sep 17 00:00:00 2001 From: Tomas Date: Tue, 12 Apr 2022 22:34:33 +0200 Subject: [PATCH 2/7] Fixed return type for addEnumResult --- lib/Doctrine/ORM/Query/ResultSetMapping.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/Doctrine/ORM/Query/ResultSetMapping.php b/lib/Doctrine/ORM/Query/ResultSetMapping.php index 305ed7492c1..17f95b8af10 100644 --- a/lib/Doctrine/ORM/Query/ResultSetMapping.php +++ b/lib/Doctrine/ORM/Query/ResultSetMapping.php @@ -403,6 +403,8 @@ public function addScalarResult($columnName, $alias, $type = 'string') public function addEnumResult($columnName, $enumType) { $this->enumMappings[$columnName] = $enumType; + + return $this; } /** From e295c0aa655f723d8d6c622a43d9fedca1e10144 Mon Sep 17 00:00:00 2001 From: Tomas Date: Tue, 12 Apr 2022 22:39:10 +0200 Subject: [PATCH 3/7] Fix PHPCS useless parenthesis --- 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 845681dabb5..7491f172af4 100644 --- a/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php +++ b/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php @@ -430,7 +430,8 @@ protected function gatherRowData(array $data, array &$id, array &$nonemptyCompon $value = $type->convertToPHPValue($value, $this->_platform); if (isset($cacheKeyInfo['enumType'])) { - $rowData['scalars'][$fieldName] = ($cacheKeyInfo['enumType'])::from($value); + $enumType = $cacheKeyInfo['enumType']; + $rowData['scalars'][$fieldName] = $enumType::from($value); } else { $rowData['scalars'][$fieldName] = $value; } From 2e42b7c245df3299c5cce0ae5ae70288cb8644a7 Mon Sep 17 00:00:00 2001 From: Tomas Date: Tue, 12 Apr 2022 22:53:47 +0200 Subject: [PATCH 4/7] testEnumArrayHydration showing why this is unfixable for general types --- tests/Doctrine/Tests/Models/Enums/Scale.php | 2 +- .../Tests/ORM/Functional/EnumTest.php | 37 ++++++++++++++----- 2 files changed, 29 insertions(+), 10 deletions(-) diff --git a/tests/Doctrine/Tests/Models/Enums/Scale.php b/tests/Doctrine/Tests/Models/Enums/Scale.php index 96402697d74..45e56a00019 100644 --- a/tests/Doctrine/Tests/Models/Enums/Scale.php +++ b/tests/Doctrine/Tests/Models/Enums/Scale.php @@ -23,7 +23,7 @@ class Scale /** * @Column(type="simple_array", enumType=Unit::class) - * @var Unit + * @var Unit[] */ #[Column(type: 'simple_array', enumType: Unit::class)] public $supportedUnits; diff --git a/tests/Doctrine/Tests/ORM/Functional/EnumTest.php b/tests/Doctrine/Tests/ORM/Functional/EnumTest.php index 79073999245..b8ca7e9acc0 100644 --- a/tests/Doctrine/Tests/ORM/Functional/EnumTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/EnumTest.php @@ -61,16 +61,11 @@ public function testEnumMapping(string $cardClass): void $this->assertEquals(Suit::Clubs, $fetchedCard->suit); } - /** - * @param class-string $cardClass - * - * @dataProvider provideCardClasses - */ - public function testEnumHydration(string $cardClass): void + public function testEnumHydration(): void { - $this->setUpEntitySchema([$cardClass]); + $this->setUpEntitySchema([Card::class]); - $card = new $cardClass(); + $card = new Card(); $card->suit = Suit::Clubs; $this->_em->persist($card); @@ -78,7 +73,7 @@ public function testEnumHydration(string $cardClass): void $this->_em->clear(); $result = $this->_em->createQueryBuilder() - ->from($cardClass, 'c') + ->from(Card::class, 'c') ->select('c.id, c.suit') ->getQuery() ->getResult(); @@ -87,6 +82,30 @@ public function testEnumHydration(string $cardClass): void $this->assertEquals(Suit::Clubs, $result[0]['suit']); } + public function testEnumArrayHydration(): void + { + self::markTestSkipped('This test is broken, see https://github.com/doctrine/orm/pull/9657'); + + $this->setUpEntitySchema([Scale::class]); + + $scale = new Scale(); + $scale->supportedUnits = [Unit::Gram, Unit::Meter]; + + $this->_em->persist($scale); + $this->_em->flush(); + $this->_em->clear(); + + $result = $this->_em->createQueryBuilder() + ->from(Scale::class, 's') + ->select('s.id, s.supportedUnits') + ->getQuery() + ->getResult(); + + $this->assertIsArray($result->supportedUnits); + $this->assertContains(Unit::Gram, $result->supportedUnits); + $this->assertContains(Unit::Meter, $result->supportedUnits); + } + public function testFindByEnum(): void { $this->setUpEntitySchema([Card::class]); From 8e076ed8d4e64ed540bb74cb390366ffcac0d969 Mon Sep 17 00:00:00 2001 From: Tomas Date: Tue, 12 Apr 2022 23:40:05 +0200 Subject: [PATCH 5/7] Fixed testEnumArrayHydration --- tests/Doctrine/Tests/ORM/Functional/EnumTest.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/Doctrine/Tests/ORM/Functional/EnumTest.php b/tests/Doctrine/Tests/ORM/Functional/EnumTest.php index b8ca7e9acc0..2028896933d 100644 --- a/tests/Doctrine/Tests/ORM/Functional/EnumTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/EnumTest.php @@ -101,9 +101,9 @@ public function testEnumArrayHydration(): void ->getQuery() ->getResult(); - $this->assertIsArray($result->supportedUnits); - $this->assertContains(Unit::Gram, $result->supportedUnits); - $this->assertContains(Unit::Meter, $result->supportedUnits); + $this->assertIsArray($result[0]['supportedUnits']); + $this->assertContains(Unit::Gram, $result[0]['supportedUnits']); + $this->assertContains(Unit::Meter, $result[0]['supportedUnits']); } public function testFindByEnum(): void From 27115ae40417c515cd69dab70a1dfce4e7a4646a Mon Sep 17 00:00:00 2001 From: Tomas Date: Wed, 13 Apr 2022 10:02:55 +0200 Subject: [PATCH 6/7] Fix array of enums --- .../Internal/Hydration/AbstractHydrator.php | 19 +++++-- .../Tests/Models/Enums/CardWithNullable.php | 49 +++++++++++++++++++ .../Tests/ORM/Functional/EnumTest.php | 17 +++++-- 3 files changed, 77 insertions(+), 8 deletions(-) create mode 100644 tests/Doctrine/Tests/Models/Enums/CardWithNullable.php diff --git a/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php b/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php index 7491f172af4..2115a9e819a 100644 --- a/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php +++ b/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php @@ -4,6 +4,7 @@ namespace Doctrine\ORM\Internal\Hydration; +use BackedEnum; use Doctrine\DBAL\Driver\ResultStatement; use Doctrine\DBAL\ForwardCompatibility\Result as ForwardCompatibilityResult; use Doctrine\DBAL\Platforms\AbstractPlatform; @@ -27,6 +28,7 @@ use function end; use function get_debug_type; use function in_array; +use function is_array; use function sprintf; /** @@ -429,13 +431,20 @@ protected function gatherRowData(array $data, array &$id, array &$nonemptyCompon $type = $cacheKeyInfo['type']; $value = $type->convertToPHPValue($value, $this->_platform); - if (isset($cacheKeyInfo['enumType'])) { - $enumType = $cacheKeyInfo['enumType']; - $rowData['scalars'][$fieldName] = $enumType::from($value); - } else { - $rowData['scalars'][$fieldName] = $value; + // Reimplement ReflectionEnumProperty code + if ($value !== null && isset($cacheKeyInfo['enumType'])) { + $enumType = $cacheKeyInfo['enumType']; + if (is_array($value)) { + $value = array_map(static function ($value) use ($enumType): BackedEnum { + return $enumType::from($value); + }, $value); + } else { + $value = $enumType::from($value); + } } + $rowData['scalars'][$fieldName] = $value; + break; //case (isset($cacheKeyInfo['isMetaColumn'])): diff --git a/tests/Doctrine/Tests/Models/Enums/CardWithNullable.php b/tests/Doctrine/Tests/Models/Enums/CardWithNullable.php new file mode 100644 index 00000000000..ea35461a81e --- /dev/null +++ b/tests/Doctrine/Tests/Models/Enums/CardWithNullable.php @@ -0,0 +1,49 @@ +mapField( + [ + 'id' => true, + 'fieldName' => 'id', + 'type' => 'integer', + ] + ); + $metadata->mapField( + [ + 'fieldName' => 'suit', + 'type' => 'string', + 'enumType' => Suit::class, + 'nullable' => true, + ] + ); + } +} diff --git a/tests/Doctrine/Tests/ORM/Functional/EnumTest.php b/tests/Doctrine/Tests/ORM/Functional/EnumTest.php index 2028896933d..262ee9c58e5 100644 --- a/tests/Doctrine/Tests/ORM/Functional/EnumTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/EnumTest.php @@ -10,6 +10,7 @@ use Doctrine\ORM\Tools\SchemaTool; use Doctrine\Tests\Models\Enums\Card; use Doctrine\Tests\Models\Enums\CardWithDefault; +use Doctrine\Tests\Models\Enums\CardWithNullable; use Doctrine\Tests\Models\Enums\Product; use Doctrine\Tests\Models\Enums\Quantity; use Doctrine\Tests\Models\Enums\Scale; @@ -63,12 +64,16 @@ public function testEnumMapping(string $cardClass): void public function testEnumHydration(): void { - $this->setUpEntitySchema([Card::class]); + $this->setUpEntitySchema([Card::class, CardWithNullable::class]); $card = new Card(); $card->suit = Suit::Clubs; + $cardWithNullable = new CardWithNullable(); + $cardWithNullable->suit = null; + $this->_em->persist($card); + $this->_em->persist($cardWithNullable); $this->_em->flush(); $this->_em->clear(); @@ -80,12 +85,18 @@ public function testEnumHydration(): void $this->assertInstanceOf(Suit::class, $result[0]['suit']); $this->assertEquals(Suit::Clubs, $result[0]['suit']); + + $result = $this->_em->createQueryBuilder() + ->from(CardWithNullable::class, 'c') + ->select('c.id, c.suit') + ->getQuery() + ->getResult(); + + $this->assertNull($result[0]['suit']); } public function testEnumArrayHydration(): void { - self::markTestSkipped('This test is broken, see https://github.com/doctrine/orm/pull/9657'); - $this->setUpEntitySchema([Scale::class]); $scale = new Scale(); From 06d88f75eab2df3350542aa1250b4ac66e29f13b Mon Sep 17 00:00:00 2001 From: michnovka Date: Fri, 15 Apr 2022 20:33:00 +0200 Subject: [PATCH 7/7] Update tests/Doctrine/Tests/ORM/Functional/EnumTest.php Co-authored-by: Alexander M. Turek --- tests/Doctrine/Tests/ORM/Functional/EnumTest.php | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/Doctrine/Tests/ORM/Functional/EnumTest.php b/tests/Doctrine/Tests/ORM/Functional/EnumTest.php index 262ee9c58e5..9d1b1c92888 100644 --- a/tests/Doctrine/Tests/ORM/Functional/EnumTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/EnumTest.php @@ -112,9 +112,7 @@ public function testEnumArrayHydration(): void ->getQuery() ->getResult(); - $this->assertIsArray($result[0]['supportedUnits']); - $this->assertContains(Unit::Gram, $result[0]['supportedUnits']); - $this->assertContains(Unit::Meter, $result[0]['supportedUnits']); + self::assertEqualsCanonicalizing([Unit::Gram, Unit::Meter], $result[0]['supportedUnits']); } public function testFindByEnum(): void