From 3fd6a3f4afd69c4aceac38879fdf569e79e95178 Mon Sep 17 00:00:00 2001 From: eltharin Date: Thu, 20 Feb 2025 09:24:13 +0100 Subject: [PATCH] fix bugs nested dto are not in good argument order dto with only dto does'nt work --- src/Internal/Hydration/AbstractHydrator.php | 31 ++- src/Query/AST/NewObjectExpression.php | 14 +- src/Query/Parser.php | 7 +- src/Query/ResultSetMapping.php | 7 + src/Query/SqlWalker.php | 26 +- tests/Tests/Models/CMS/CmsAddressDTO.php | 2 +- tests/Tests/Models/CMS/CmsDumbDTO.php | 17 ++ .../Tests/ORM/Functional/NewOperatorTest.php | 250 +++++++++++++++++- 8 files changed, 314 insertions(+), 40 deletions(-) create mode 100644 tests/Tests/Models/CMS/CmsDumbDTO.php diff --git a/src/Internal/Hydration/AbstractHydrator.php b/src/Internal/Hydration/AbstractHydrator.php index 0a44d3a02b5..591ba347cd7 100644 --- a/src/Internal/Hydration/AbstractHydrator.php +++ b/src/Internal/Hydration/AbstractHydrator.php @@ -18,12 +18,14 @@ use LogicException; use ReflectionClass; +use function array_key_exists; use function array_map; use function array_merge; use function count; use function end; use function in_array; use function is_array; +use function ksort; /** * Base class for all hydrators. A hydrator is a class that provides some form @@ -263,6 +265,15 @@ protected function gatherRowData(array $data, array &$id, array &$nonemptyCompon { $rowData = ['data' => [], 'newObjects' => []]; + foreach ($this->rsm->newObjectMappings as $mapping) { + $this->rsm->newObject[$mapping['objIndex']] = $mapping['className']; + } + + foreach ($this->rsm->newObject as $objIndex => $newObject) { + $rowData['newObjects'][$objIndex]['class'] = new ReflectionClass($newObject); + $rowData['newObjects'][$objIndex]['args'] = []; + } + foreach ($data as $key => $value) { $cacheKeyInfo = $this->hydrateColumnInfo($key); if ($cacheKeyInfo === null) { @@ -282,7 +293,6 @@ protected function gatherRowData(array $data, array &$id, array &$nonemptyCompon $value = $this->buildEnum($value, $cacheKeyInfo['enumType']); } - $rowData['newObjects'][$objIndex]['class'] = $cacheKeyInfo['class']; $rowData['newObjects'][$objIndex]['args'][$argIndex] = $value; break; @@ -336,21 +346,17 @@ protected function gatherRowData(array $data, array &$id, array &$nonemptyCompon } } - foreach ($this->resultSetMapping()->nestedNewObjectArguments as $objIndex => ['ownerIndex' => $ownerIndex, 'argIndex' => $argIndex]) { - if (! isset($rowData['newObjects'][$ownerIndex . ':' . $argIndex])) { - continue; + foreach ($this->resultSetMapping()->nestedNewObjectArguments as ['ownerIndex' => $ownerIndex, 'argIndex' => $argIndex, 'argAlias' => $argAlias]) { + if (array_key_exists($argAlias, $rowData['newObjects'])) { + ksort($rowData['newObjects'][$argAlias]['args']); + $rowData['newObjects'][$ownerIndex]['args'][$argIndex] = $rowData['newObjects'][$argAlias]['class']->newInstanceArgs($rowData['newObjects'][$argAlias]['args']); + unset($rowData['newObjects'][$argAlias]); } - - $newObject = $rowData['newObjects'][$ownerIndex . ':' . $argIndex]; - unset($rowData['newObjects'][$ownerIndex . ':' . $argIndex]); - - $obj = $newObject['class']->newInstanceArgs($newObject['args']); - - $rowData['newObjects'][$ownerIndex]['args'][$argIndex] = $obj; } foreach ($rowData['newObjects'] as $objIndex => $newObject) { - $obj = $newObject['class']->newInstanceArgs($newObject['args']); + ksort($rowData['newObjects'][$objIndex]['args']); + $obj = $rowData['newObjects'][$objIndex]['class']->newInstanceArgs($rowData['newObjects'][$objIndex]['args']); $rowData['newObjects'][$objIndex]['obj'] = $obj; } @@ -454,7 +460,6 @@ protected function hydrateColumnInfo(string $key): array|null 'type' => Type::getType($this->rsm->typeMappings[$key]), 'argIndex' => $mapping['argIndex'], 'objIndex' => $mapping['objIndex'], - 'class' => new ReflectionClass($mapping['className']), 'enumType' => $this->rsm->enumMappings[$key] ?? null, ]; diff --git a/src/Query/AST/NewObjectExpression.php b/src/Query/AST/NewObjectExpression.php index 7383c487234..1d46722676e 100644 --- a/src/Query/AST/NewObjectExpression.php +++ b/src/Query/AST/NewObjectExpression.php @@ -6,6 +6,9 @@ use Doctrine\ORM\Query\SqlWalker; +use function func_get_arg; +use function func_num_args; + /** * NewObjectExpression ::= "NEW" IdentificationVariable "(" NewObjectArg {"," NewObjectArg}* ")" * @@ -13,13 +16,18 @@ */ class NewObjectExpression extends Node { - /** @param mixed[] $args */ + /** + * @param class-string $className + * @param mixed[] $args + */ public function __construct(public string $className, public array $args) { } - public function dispatch(SqlWalker $walker): string + public function dispatch(SqlWalker $walker /*, string|null $parentAlias = null */): string { - return $walker->walkNewObject($this); + $parentAlias = func_num_args() > 1 ? func_get_arg(1) : null; + + return $walker->walkNewObject($this, $parentAlias); } } diff --git a/src/Query/Parser.php b/src/Query/Parser.php index 86fb8243b49..1b7c9fc6490 100644 --- a/src/Query/Parser.php +++ b/src/Query/Parser.php @@ -1767,6 +1767,7 @@ public function NewObjectExpression(): AST\NewObjectExpression $useNamedArguments = true; } + /** @var class-string $className */ $className = $this->AbstractSchemaName(); // note that this is not yet validated $token = $this->lexer->token; @@ -1854,7 +1855,11 @@ public function NewObjectArg(string|null &$fieldAlias = null): mixed if ($this->lexer->isNextToken(TokenType::T_AS)) { $this->match(TokenType::T_AS); - $fieldAlias = $this->AliasIdentificationVariable(); + $this->match(TokenType::T_IDENTIFIER); + + assert($this->lexer->token !== null); + + $fieldAlias = $this->lexer->token->value; } return $expression; diff --git a/src/Query/ResultSetMapping.php b/src/Query/ResultSetMapping.php index c0ccc127fd5..d8f4bc76693 100644 --- a/src/Query/ResultSetMapping.php +++ b/src/Query/ResultSetMapping.php @@ -159,6 +159,13 @@ class ResultSetMapping */ public array $newObjectMappings = []; + /** + * Maps object Ids in the result set to classnames. + * + * @phpstan-var array + */ + public array $newObject = []; + /** * Maps last argument for new objects in order to initiate object construction * diff --git a/src/Query/SqlWalker.php b/src/Query/SqlWalker.php index 9089995e432..27063b65162 100644 --- a/src/Query/SqlWalker.php +++ b/src/Query/SqlWalker.php @@ -24,10 +24,8 @@ use function array_keys; use function array_map; use function array_merge; -use function array_pop; use function assert; use function count; -use function end; use function implode; use function in_array; use function is_array; @@ -84,13 +82,6 @@ class SqlWalker */ private int $newObjectCounter = 0; - /** - * Contains nesting levels of new objects arguments - * - * @phpstan-var array - */ - private array $newObjectStack = []; - private readonly EntityManagerInterface $em; private readonly Connection $conn; @@ -1507,14 +1498,7 @@ public function walkParenthesisExpression(AST\ParenthesisExpression $parenthesis public function walkNewObject(AST\NewObjectExpression $newObjectExpression, string|null $newObjectResultAlias = null): string { $sqlSelectExpressions = []; - $objOwner = $objOwnerIdx = null; - - if ($this->newObjectStack !== []) { - [$objOwner, $objOwnerIdx] = end($this->newObjectStack); - $objIndex = $objOwner . ':' . $objOwnerIdx; - } else { - $objIndex = $newObjectResultAlias ?: $this->newObjectCounter++; - } + $objIndex = $newObjectResultAlias ?: $this->newObjectCounter++; foreach ($newObjectExpression->args as $argIndex => $e) { $resultAlias = $this->scalarResultCounter++; @@ -1523,10 +1507,8 @@ public function walkNewObject(AST\NewObjectExpression $newObjectExpression, stri switch (true) { case $e instanceof AST\NewObjectExpression: - $this->newObjectStack[] = [$objIndex, $argIndex]; - $sqlSelectExpressions[] = $e->dispatch($this); - array_pop($this->newObjectStack); - $this->rsm->nestedNewObjectArguments[$columnAlias] = ['ownerIndex' => $objIndex, 'argIndex' => $argIndex]; + $sqlSelectExpressions[] = $e->dispatch($this, $columnAlias); + $this->rsm->nestedNewObjectArguments[$columnAlias] = ['ownerIndex' => $objIndex, 'argIndex' => $argIndex, 'argAlias' => $columnAlias]; break; case $e instanceof AST\Subselect: @@ -1582,6 +1564,8 @@ public function walkNewObject(AST\NewObjectExpression $newObjectExpression, stri ]; } + $this->rsm->newObject[$objIndex] = $newObjectExpression->className; + return implode(', ', $sqlSelectExpressions); } diff --git a/tests/Tests/Models/CMS/CmsAddressDTO.php b/tests/Tests/Models/CMS/CmsAddressDTO.php index 502644ed25e..0d615aadbb9 100644 --- a/tests/Tests/Models/CMS/CmsAddressDTO.php +++ b/tests/Tests/Models/CMS/CmsAddressDTO.php @@ -6,7 +6,7 @@ class CmsAddressDTO { - public function __construct(public string|null $country = null, public string|null $city = null, public string|null $zip = null, public CmsAddressDTO|string|null $address = null) + public function __construct(public string|null $country = null, public string|null $city = null, public string|null $zip = null, public string|null $address = null, public CmsDumbDTO|null $other = null) { } } diff --git a/tests/Tests/Models/CMS/CmsDumbDTO.php b/tests/Tests/Models/CMS/CmsDumbDTO.php new file mode 100644 index 00000000000..8891436fad6 --- /dev/null +++ b/tests/Tests/Models/CMS/CmsDumbDTO.php @@ -0,0 +1,17 @@ +fixtures[1]->address->country, $result[1]['user']->address->country); self::assertSame($this->fixtures[2]->address->country, $result[2]['user']->address->country); + self::assertInstanceOf(CmsDumbDTO::class, $result[0]['user']->address->other); + self::assertInstanceOf(CmsDumbDTO::class, $result[1]['user']->address->other); + self::assertInstanceOf(CmsDumbDTO::class, $result[2]['user']->address->other); + + self::assertSame($this->fixtures[0]->address->country, $result[0]['user']->address->other->val1); + self::assertSame($this->fixtures[1]->address->country, $result[1]['user']->address->other->val1); + self::assertSame($this->fixtures[2]->address->country, $result[2]['user']->address->other->val1); + + self::assertSame($this->fixtures[0]->address->city, $result[0]['user']->address->other->val2); + self::assertSame($this->fixtures[1]->address->city, $result[1]['user']->address->other->val2); + self::assertSame($this->fixtures[2]->address->city, $result[2]['user']->address->other->val2); + + self::assertSame($this->fixtures[0]->status, $result[0]['status']); + self::assertSame($this->fixtures[1]->status, $result[1]['status']); + self::assertSame($this->fixtures[2]->status, $result[2]['status']); + + self::assertSame($this->fixtures[0]->username, $result[0]['cmsUserUsername']); + self::assertSame($this->fixtures[1]->username, $result[1]['cmsUserUsername']); + self::assertSame($this->fixtures[2]->username, $result[2]['cmsUserUsername']); + } + + public function testShouldSupportNestedNewOperatorsWithDtoFirst(): void + { + $dql = ' + SELECT + new CmsUserDTO( + u.name, + e.email, + new CmsAddressDTO( + a.country, + a.city, + a.zip + ), + 555812452 + ) as user, + u.status, + u.username as cmsUserUsername + FROM + Doctrine\Tests\Models\CMS\CmsUser u + JOIN + u.email e + JOIN + u.address a + ORDER BY + u.name'; + + $query = $this->getEntityManager()->createQuery($dql); + $result = $query->getResult(); + + self::assertCount(3, $result); + + self::assertInstanceOf(CmsUserDTO::class, $result[0]['user']); + self::assertInstanceOf(CmsUserDTO::class, $result[1]['user']); + self::assertInstanceOf(CmsUserDTO::class, $result[2]['user']); + + self::assertInstanceOf(CmsAddressDTO::class, $result[0]['user']->address); + self::assertInstanceOf(CmsAddressDTO::class, $result[1]['user']->address); + self::assertInstanceOf(CmsAddressDTO::class, $result[2]['user']->address); + + self::assertSame($this->fixtures[0]->name, $result[0]['user']->name); + self::assertSame($this->fixtures[1]->name, $result[1]['user']->name); + self::assertSame($this->fixtures[2]->name, $result[2]['user']->name); + + self::assertSame($this->fixtures[0]->email->email, $result[0]['user']->email); + self::assertSame($this->fixtures[1]->email->email, $result[1]['user']->email); + self::assertSame($this->fixtures[2]->email->email, $result[2]['user']->email); + + self::assertSame($this->fixtures[0]->address->city, $result[0]['user']->address->city); + self::assertSame($this->fixtures[1]->address->city, $result[1]['user']->address->city); + self::assertSame($this->fixtures[2]->address->city, $result[2]['user']->address->city); + + self::assertSame($this->fixtures[0]->address->country, $result[0]['user']->address->country); + self::assertSame($this->fixtures[1]->address->country, $result[1]['user']->address->country); + self::assertSame($this->fixtures[2]->address->country, $result[2]['user']->address->country); + + self::assertSame(555812452, $result[0]['user']->phonenumbers); + self::assertSame(555812452, $result[1]['user']->phonenumbers); + self::assertSame(555812452, $result[2]['user']->phonenumbers); + self::assertSame($this->fixtures[0]->status, $result[0]['status']); self::assertSame($this->fixtures[1]->status, $result[1]['status']); self::assertSame($this->fixtures[2]->status, $result[2]['status']); @@ -1087,6 +1168,68 @@ public function testShouldSupportNestedNewOperators(): void self::assertSame($this->fixtures[2]->username, $result[2]['cmsUserUsername']); } + public function testOnlyObjectInObject(): void + { + $dql = ' + SELECT + new CmsDumbDTO( + new CmsDumbDTO( + u.name, + e.email + ), + new CmsAddressDTO( + a.country, + a.city, + a.zip + ) + ) as user + FROM + Doctrine\Tests\Models\CMS\CmsUser u + JOIN + u.email e + JOIN + u.address a + ORDER BY + u.name'; + + $query = $this->getEntityManager()->createQuery($dql); + $result = $query->getResult(); + + self::assertCount(3, $result); + + self::assertInstanceOf(CmsDumbDTO::class, $result[0]); + self::assertInstanceOf(CmsDumbDTO::class, $result[1]); + self::assertInstanceOf(CmsDumbDTO::class, $result[2]); + + self::assertInstanceOf(CmsDumbDTO::class, $result[0]->val1); + self::assertInstanceOf(CmsDumbDTO::class, $result[1]->val1); + self::assertInstanceOf(CmsDumbDTO::class, $result[2]->val1); + + self::assertSame($this->fixtures[0]->name, $result[0]->val1->val1); + self::assertSame($this->fixtures[1]->name, $result[1]->val1->val1); + self::assertSame($this->fixtures[2]->name, $result[2]->val1->val1); + + self::assertSame($this->fixtures[0]->email->email, $result[0]->val1->val2); + self::assertSame($this->fixtures[1]->email->email, $result[1]->val1->val2); + self::assertSame($this->fixtures[2]->email->email, $result[2]->val1->val2); + + self::assertInstanceOf(CmsAddressDTO::class, $result[0]->val2); + self::assertInstanceOf(CmsAddressDTO::class, $result[1]->val2); + self::assertInstanceOf(CmsAddressDTO::class, $result[2]->val2); + + self::assertSame($this->fixtures[0]->address->country, $result[0]->val2->country); + self::assertSame($this->fixtures[1]->address->country, $result[1]->val2->country); + self::assertSame($this->fixtures[2]->address->country, $result[2]->val2->country); + + self::assertSame($this->fixtures[0]->address->city, $result[0]->val2->city); + self::assertSame($this->fixtures[1]->address->city, $result[1]->val2->city); + self::assertSame($this->fixtures[2]->address->city, $result[2]->val2->city); + + self::assertSame($this->fixtures[0]->address->zip, $result[0]->val2->zip); + self::assertSame($this->fixtures[1]->address->zip, $result[1]->val2->zip); + self::assertSame($this->fixtures[2]->address->zip, $result[2]->val2->zip); + } + public function testNamedArguments(): void { $dql = <<<'SQL' @@ -1159,6 +1302,111 @@ public function testNamedArguments(): void self::assertSame($this->fixtures[2]->username, $result[2]['cmsUserUsername']); } + public function testShouldSupportNestedNewOperatorsWithNestedDtoNotLast(): void + { + $dql = ' + SELECT + new CmsUserDTO( + u.name, + e.email, + new CmsAddressDTO( + a.country, + a.city, + a.zip, + \'Abbey Road\', + new CmsDumbDTO( + a.country, + a.city, + new CmsDumbDTO( + a.zip, + 456 + ), + a.zip + ) + ), + 555812452 + ) as user, + u.status, + u.username as cmsUserUsername + FROM + Doctrine\Tests\Models\CMS\CmsUser u + JOIN + u.email e + JOIN + u.address a + ORDER BY + u.name'; + + $query = $this->getEntityManager()->createQuery($dql); + $result = $query->getResult(); + + self::assertCount(3, $result); + + self::assertInstanceOf(CmsUserDTO::class, $result[0]['user']); + self::assertInstanceOf(CmsUserDTO::class, $result[1]['user']); + self::assertInstanceOf(CmsUserDTO::class, $result[2]['user']); + + self::assertSame($this->fixtures[0]->name, $result[0]['user']->name); + self::assertSame($this->fixtures[1]->name, $result[1]['user']->name); + self::assertSame($this->fixtures[2]->name, $result[2]['user']->name); + + self::assertSame($this->fixtures[0]->email->email, $result[0]['user']->email); + self::assertSame($this->fixtures[1]->email->email, $result[1]['user']->email); + self::assertSame($this->fixtures[2]->email->email, $result[2]['user']->email); + + self::assertInstanceOf(CmsAddressDTO::class, $result[0]['user']->address); + self::assertInstanceOf(CmsAddressDTO::class, $result[1]['user']->address); + self::assertInstanceOf(CmsAddressDTO::class, $result[2]['user']->address); + + self::assertSame($this->fixtures[0]->address->country, $result[0]['user']->address->country); + self::assertSame($this->fixtures[1]->address->country, $result[1]['user']->address->country); + self::assertSame($this->fixtures[2]->address->country, $result[2]['user']->address->country); + + self::assertSame($this->fixtures[2]->address->city, $result[2]['user']->address->city); + self::assertSame($this->fixtures[0]->address->city, $result[0]['user']->address->city); + self::assertSame($this->fixtures[1]->address->city, $result[1]['user']->address->city); + + self::assertInstanceOf(CmsDumbDTO::class, $result[0]['user']->address->other); + self::assertInstanceOf(CmsDumbDTO::class, $result[1]['user']->address->other); + self::assertInstanceOf(CmsDumbDTO::class, $result[2]['user']->address->other); + + self::assertSame($this->fixtures[0]->address->country, $result[0]['user']->address->other->val1); + self::assertSame($this->fixtures[1]->address->country, $result[1]['user']->address->other->val1); + self::assertSame($this->fixtures[2]->address->country, $result[2]['user']->address->other->val1); + + self::assertSame($this->fixtures[0]->address->city, $result[0]['user']->address->other->val2); + self::assertSame($this->fixtures[1]->address->city, $result[1]['user']->address->other->val2); + self::assertSame($this->fixtures[2]->address->city, $result[2]['user']->address->other->val2); + + self::assertInstanceOf(CmsDumbDTO::class, $result[0]['user']->address->other->val3); + self::assertInstanceOf(CmsDumbDTO::class, $result[1]['user']->address->other->val3); + self::assertInstanceOf(CmsDumbDTO::class, $result[2]['user']->address->other->val3); + + self::assertSame($this->fixtures[0]->address->zip, $result[0]['user']->address->other->val3->val1); + self::assertSame($this->fixtures[1]->address->zip, $result[1]['user']->address->other->val3->val1); + self::assertSame($this->fixtures[2]->address->zip, $result[2]['user']->address->other->val3->val1); + + self::assertSame(456, $result[0]['user']->address->other->val3->val2); + self::assertSame(456, $result[1]['user']->address->other->val3->val2); + self::assertSame(456, $result[2]['user']->address->other->val3->val2); + + self::assertSame($this->fixtures[0]->address->zip, $result[0]['user']->address->other->val4); + self::assertSame($this->fixtures[1]->address->zip, $result[1]['user']->address->other->val4); + self::assertSame($this->fixtures[2]->address->zip, $result[2]['user']->address->other->val4); + + self::assertSame(555812452, $result[0]['user']->phonenumbers); + self::assertSame(555812452, $result[1]['user']->phonenumbers); + self::assertSame(555812452, $result[2]['user']->phonenumbers); + + self::assertSame($this->fixtures[0]->status, $result[0]['status']); + self::assertSame($this->fixtures[1]->status, $result[1]['status']); + self::assertSame($this->fixtures[2]->status, $result[2]['status']); + + self::assertSame($this->fixtures[0]->username, $result[0]['cmsUserUsername']); + self::assertSame($this->fixtures[1]->username, $result[1]['cmsUserUsername']); + self::assertSame($this->fixtures[2]->username, $result[2]['cmsUserUsername']); + } + public function testVariadicArgument(): void { $dql = <<<'SQL'