From 16a8f10fd2fca952b08a656e909dd0fa0a86baed Mon Sep 17 00:00:00 2001 From: Matthias Pigulla Date: Wed, 9 Oct 2024 15:37:04 +0200 Subject: [PATCH 01/21] Remove a misleading comment (#11644) --- src/Query/Exec/SingleTableDeleteUpdateExecutor.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/Query/Exec/SingleTableDeleteUpdateExecutor.php b/src/Query/Exec/SingleTableDeleteUpdateExecutor.php index 7f7496e397a..4a4a2986467 100644 --- a/src/Query/Exec/SingleTableDeleteUpdateExecutor.php +++ b/src/Query/Exec/SingleTableDeleteUpdateExecutor.php @@ -14,8 +14,6 @@ * that are mapped to a single table. * * @link www.doctrine-project.org - * - * @todo This is exactly the same as SingleSelectExecutor. Unify in SingleStatementExecutor. */ class SingleTableDeleteUpdateExecutor extends AbstractSqlExecutor { From 4a3c7f05bf4319e17f355dcea2fdfa19d232ed5a Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Wed, 9 Oct 2024 17:13:58 +0200 Subject: [PATCH 02/21] Revert "Remove unused exception" This reverts commit 689da1f251957f49f1126b92bcaf8fb12df6f56e. --- src/Query/QueryException.php | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/Query/QueryException.php b/src/Query/QueryException.php index ae945b167fe..5c82b20a7a4 100644 --- a/src/Query/QueryException.php +++ b/src/Query/QueryException.php @@ -88,6 +88,15 @@ public static function iterateWithFetchJoinCollectionNotAllowed(AssociationMappi ); } + public static function partialObjectsAreDangerous(): self + { + return new self( + 'Loading partial objects is dangerous. Fetch full objects or consider ' . + 'using a different fetch mode. If you really want partial objects, ' . + 'set the doctrine.forcePartialLoad query hint to TRUE.', + ); + } + /** * @param string[] $assoc * @psalm-param array $assoc From f71725575c605cb77ecc29c7131d5b39ab9e03ef Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Wed, 9 Oct 2024 17:34:59 +0200 Subject: [PATCH 03/21] Revert undprecate PARTIAL for objects in DQL. --- docs/en/index.rst | 1 + .../reference/dql-doctrine-query-language.rst | 24 ++++- docs/en/reference/partial-objects.rst | 98 +++++++++++++++++++ docs/en/sidebar.rst | 1 + src/Cache/DefaultQueryCache.php | 5 + src/Cache/Exception/FeatureNotImplemented.php | 5 + src/Decorator/EntityManagerDecorator.php | 1 + src/EntityManager.php | 1 + src/Query.php | 8 ++ src/Query/Parser.php | 5 +- src/Query/SqlWalker.php | 40 +++++--- src/UnitOfWork.php | 17 +++- ...PartialObjectHydrationPerformanceBench.php | 84 ++++++++++++++++ ...PartialObjectHydrationPerformanceBench.php | 72 ++++++++++++++ .../OneToOneUnidirectionalAssociationTest.php | 14 +++ .../ORM/Functional/PostLoadEventTest.php | 21 ++++ .../SecondLevelCacheQueryCacheTest.php | 25 +++++ .../ORM/Functional/Ticket/DDC163Test.php | 2 +- .../ORM/Functional/Ticket/DDC2519Test.php | 76 ++++++++++++++ .../ORM/Functional/Ticket/GH8443Test.php | 32 ++++++ .../Tests/ORM/Functional/ValueObjectsTest.php | 54 +++++++++- .../ORM/Query/LanguageRecognitionTest.php | 2 - tests/Tests/ORM/Query/ParserTest.php | 9 -- .../ORM/Query/SelectSqlGenerationTest.php | 82 +++++++++++++--- .../LimitSubqueryOutputWalkerTest.php | 2 +- tests/Tests/ORM/UnitOfWorkTest.php | 9 -- 26 files changed, 629 insertions(+), 61 deletions(-) create mode 100644 docs/en/reference/partial-objects.rst create mode 100644 tests/Performance/Hydration/MixedQueryFetchJoinPartialObjectHydrationPerformanceBench.php create mode 100644 tests/Performance/Hydration/SimpleQueryPartialObjectHydrationPerformanceBench.php create mode 100644 tests/Tests/ORM/Functional/Ticket/DDC2519Test.php diff --git a/docs/en/index.rst b/docs/en/index.rst index 4d23062cd0d..32f8c070175 100644 --- a/docs/en/index.rst +++ b/docs/en/index.rst @@ -74,6 +74,7 @@ Advanced Topics * :doc:`Improving Performance ` * :doc:`Caching ` * :doc:`Partial Hydration ` +* :doc:`Partial Objects ` * :doc:`Change Tracking Policies ` * :doc:`Best Practices ` * :doc:`Metadata Drivers ` diff --git a/docs/en/reference/dql-doctrine-query-language.rst b/docs/en/reference/dql-doctrine-query-language.rst index ab3cb138889..29d1a163d78 100644 --- a/docs/en/reference/dql-doctrine-query-language.rst +++ b/docs/en/reference/dql-doctrine-query-language.rst @@ -533,14 +533,23 @@ back. Instead, you receive only arrays as a flat rectangular result set, similar to how you would if you were just using SQL directly and joining some data. -If you want to select a partial number of fields for hydration entity in -the context of array hydration and joins you can use the ``partial`` DQL keyword: +If you want to select partial objects or fields in array hydration you can use the ``partial`` +DQL keyword: + +.. code-block:: php + + createQuery('SELECT partial u.{id, username} FROM CmsUser u'); + $users = $query->getResult(); // array of partially loaded CmsUser objects + +You use the partial syntax when joining as well: .. code-block:: php createQuery('SELECT partial u.{id, username}, partial a.{id, name} FROM CmsUser u JOIN u.articles a'); - $users = $query->getArrayResult(); // array of partially loaded CmsUser and CmsArticle fields + $usersArray = $query->getArrayResult(); // array of partially loaded CmsUser and CmsArticle fields + $users = $query->getResult(); // array of partially loaded CmsUser objects "NEW" Operator Syntax ^^^^^^^^^^^^^^^^^^^^^ @@ -1370,6 +1379,15 @@ exist mostly internal query hints that are not be consumed in userland. However the following few hints are to be used in userland: + +- ``Query::HINT_FORCE_PARTIAL_LOAD`` - Allows to hydrate objects + although not all their columns are fetched. This query hint can be + used to handle memory consumption problems with large result-sets + that contain char or binary data. Doctrine has no way of implicitly + reloading this data. Partially loaded objects have to be passed to + ``EntityManager::refresh()`` if they are to be reloaded fully from + the database. This query hint is deprecated and will be removed + in the future (\ `Details `_) - ``Query::HINT_REFRESH`` - This query is used internally by ``EntityManager::refresh()`` and can be used in userland as well. If you specify this hint and a query returns the data for an entity diff --git a/docs/en/reference/partial-objects.rst b/docs/en/reference/partial-objects.rst new file mode 100644 index 00000000000..51f173adf6c --- /dev/null +++ b/docs/en/reference/partial-objects.rst @@ -0,0 +1,98 @@ +Partial Objects +=============== + + +.. note:: + + Creating Partial Objects through DQL is deprecated and + will be removed in the future, use data transfer object + support in DQL instead. (\ `Details + `_) + +A partial object is an object whose state is not fully initialized +after being reconstituted from the database and that is +disconnected from the rest of its data. The following section will +describe why partial objects are problematic and what the approach +of Doctrine2 to this problem is. + +.. note:: + + The partial object problem in general does not apply to + methods or queries where you do not retrieve the query result as + objects. Examples are: ``Query#getArrayResult()``, + ``Query#getScalarResult()``, ``Query#getSingleScalarResult()``, + etc. + +.. warning:: + + Use of partial objects is tricky. Fields that are not retrieved + from the database will not be updated by the UnitOfWork even if they + get changed in your objects. You can only promote a partial object + to a fully-loaded object by calling ``EntityManager#refresh()`` + or a DQL query with the refresh flag. + + +What is the problem? +-------------------- + +In short, partial objects are problematic because they are usually +objects with broken invariants. As such, code that uses these +partial objects tends to be very fragile and either needs to "know" +which fields or methods can be safely accessed or add checks around +every field access or method invocation. The same holds true for +the internals, i.e. the method implementations, of such objects. +You usually simply assume the state you need in the method is +available, after all you properly constructed this object before +you pushed it into the database, right? These blind assumptions can +quickly lead to null reference errors when working with such +partial objects. + +It gets worse with the scenario of an optional association (0..1 to +1). When the associated field is NULL, you don't know whether this +object does not have an associated object or whether it was simply +not loaded when the owning object was loaded from the database. + +These are reasons why many ORMs do not allow partial objects at all +and instead you always have to load an object with all its fields +(associations being proxied). One secure way to allow partial +objects is if the programming language/platform allows the ORM tool +to hook deeply into the object and instrument it in such a way that +individual fields (not only associations) can be loaded lazily on +first access. This is possible in Java, for example, through +bytecode instrumentation. In PHP though this is not possible, so +there is no way to have "secure" partial objects in an ORM with +transparent persistence. + +Doctrine, by default, does not allow partial objects. That means, +any query that only selects partial object data and wants to +retrieve the result as objects (i.e. ``Query#getResult()``) will +raise an exception telling you that partial objects are dangerous. +If you want to force a query to return you partial objects, +possibly as a performance tweak, you can use the ``partial`` +keyword as follows: + +.. code-block:: php + + createQuery("select partial u.{id,name} from MyApp\Domain\User u"); + +You can also get a partial reference instead of a proxy reference by +calling: + +.. code-block:: php + + getPartialReference('MyApp\Domain\User', 1); + +Partial references are objects with only the identifiers set as they +are passed to the second argument of the ``getPartialReference()`` method. +All other fields are null. + +When should I force partial objects? +------------------------------------ + +Mainly for optimization purposes, but be careful of premature +optimization as partial objects lead to potentially more fragile +code. + + diff --git a/docs/en/sidebar.rst b/docs/en/sidebar.rst index 7ccdfb3a054..f52801c6b37 100644 --- a/docs/en/sidebar.rst +++ b/docs/en/sidebar.rst @@ -38,6 +38,7 @@ reference/native-sql reference/change-tracking-policies reference/partial-hydration + reference/partial-objects reference/attributes-reference reference/xml-mapping reference/php-mapping diff --git a/src/Cache/DefaultQueryCache.php b/src/Cache/DefaultQueryCache.php index f3bb8ac95c8..08e703cd4b0 100644 --- a/src/Cache/DefaultQueryCache.php +++ b/src/Cache/DefaultQueryCache.php @@ -16,6 +16,7 @@ use Doctrine\ORM\PersistentCollection; use Doctrine\ORM\Query; use Doctrine\ORM\Query\ResultSetMapping; +use Doctrine\ORM\Query\SqlWalker; use Doctrine\ORM\UnitOfWork; use function array_map; @@ -210,6 +211,10 @@ public function put(QueryCacheKey $key, ResultSetMapping $rsm, mixed $result, ar throw FeatureNotImplemented::nonSelectStatements(); } + if (($hints[SqlWalker::HINT_PARTIAL] ?? false) === true || ($hints[Query::HINT_FORCE_PARTIAL_LOAD] ?? false) === true) { + throw FeatureNotImplemented::partialEntities(); + } + if (! ($key->cacheMode & Cache::MODE_PUT)) { return false; } diff --git a/src/Cache/Exception/FeatureNotImplemented.php b/src/Cache/Exception/FeatureNotImplemented.php index 8767d574190..7bae90b775d 100644 --- a/src/Cache/Exception/FeatureNotImplemented.php +++ b/src/Cache/Exception/FeatureNotImplemented.php @@ -20,4 +20,9 @@ public static function nonSelectStatements(): self { return new self('Second-level cache query supports only select statements.'); } + + public static function partialEntities(): self + { + return new self('Second level cache does not support partial entities.'); + } } diff --git a/src/Decorator/EntityManagerDecorator.php b/src/Decorator/EntityManagerDecorator.php index 6f1b0419686..212e01e511b 100644 --- a/src/Decorator/EntityManagerDecorator.php +++ b/src/Decorator/EntityManagerDecorator.php @@ -8,6 +8,7 @@ use Doctrine\Common\EventManager; use Doctrine\DBAL\Connection; use Doctrine\DBAL\LockMode; +use Doctrine\Deprecations\Deprecation; use Doctrine\ORM\Cache; use Doctrine\ORM\Configuration; use Doctrine\ORM\EntityManagerInterface; diff --git a/src/EntityManager.php b/src/EntityManager.php index 8045ac2f5e3..9de636e5b75 100644 --- a/src/EntityManager.php +++ b/src/EntityManager.php @@ -9,6 +9,7 @@ use Doctrine\Common\EventManager; use Doctrine\DBAL\Connection; use Doctrine\DBAL\LockMode; +use Doctrine\Deprecations\Deprecation; use Doctrine\ORM\Exception\EntityManagerClosed; use Doctrine\ORM\Exception\InvalidHydrationMode; use Doctrine\ORM\Exception\MissingIdentifierField; diff --git a/src/Query.php b/src/Query.php index a869316d3e7..d8a88e8fe83 100644 --- a/src/Query.php +++ b/src/Query.php @@ -70,6 +70,14 @@ class Query extends AbstractQuery */ public const HINT_REFRESH_ENTITY = 'doctrine.refresh.entity'; + /** + * The forcePartialLoad query hint forces a particular query to return + * partial objects. + * + * @todo Rename: HINT_OPTIMIZE + */ + public const HINT_FORCE_PARTIAL_LOAD = 'doctrine.forcePartialLoad'; + /** * The includeMetaColumns query hint causes meta columns like foreign keys and * discriminator columns to be selected and returned as part of the query result. diff --git a/src/Query/Parser.php b/src/Query/Parser.php index 875783c87d4..12fe0c97691 100644 --- a/src/Query/Parser.php +++ b/src/Query/Parser.php @@ -5,6 +5,7 @@ namespace Doctrine\ORM\Query; use Doctrine\Common\Lexer\Token; +use Doctrine\Deprecations\Deprecation; use Doctrine\ORM\EntityManagerInterface; use Doctrine\ORM\Internal\Hydration\HydrationException; use Doctrine\ORM\Mapping\AssociationMapping; @@ -1674,10 +1675,6 @@ public function JoinAssociationDeclaration(): AST\JoinAssociationDeclaration */ public function PartialObjectExpression(): AST\PartialObjectExpression { - if ($this->query->getHydrationMode() === Query::HYDRATE_OBJECT) { - throw HydrationException::partialObjectHydrationDisallowed(); - } - $this->match(TokenType::T_PARTIAL); $partialFieldSet = []; diff --git a/src/Query/SqlWalker.php b/src/Query/SqlWalker.php index 46296e719e7..9beeff337be 100644 --- a/src/Query/SqlWalker.php +++ b/src/Query/SqlWalker.php @@ -321,6 +321,11 @@ private function generateClassTableInheritanceJoins( $sql .= implode(' AND ', array_filter($sqlParts)); } + // Ignore subclassing inclusion if partial objects is disallowed + if ($this->query->getHint(Query::HINT_FORCE_PARTIAL_LOAD)) { + return $sql; + } + // LEFT JOIN child class tables foreach ($class->subClasses as $subClassName) { $subClass = $this->em->getClassMetadata($subClassName); @@ -659,7 +664,8 @@ public function walkSelectClause(AST\SelectClause $selectClause): string $this->query->setHint(self::HINT_DISTINCT, true); } - $addMetaColumns = $this->query->getHydrationMode() === Query::HYDRATE_OBJECT + $addMetaColumns = ! $this->query->getHint(Query::HINT_FORCE_PARTIAL_LOAD) && + $this->query->getHydrationMode() === Query::HYDRATE_OBJECT || $this->query->getHint(Query::HINT_INCLUDE_META_COLUMNS); foreach ($this->selectedClasses as $selectedClass) { @@ -1398,28 +1404,30 @@ public function walkSelectExpression(AST\SelectExpression $selectExpression): st // 1) on Single Table Inheritance: always, since its marginal overhead // 2) on Class Table Inheritance only if partial objects are disallowed, // since it requires outer joining subtables. - foreach ($class->subClasses as $subClassName) { - $subClass = $this->em->getClassMetadata($subClassName); - $sqlTableAlias = $this->getSQLTableAlias($subClass->getTableName(), $dqlAlias); + if ($class->isInheritanceTypeSingleTable() || ! $this->query->getHint(Query::HINT_FORCE_PARTIAL_LOAD)) { + foreach ($class->subClasses as $subClassName) { + $subClass = $this->em->getClassMetadata($subClassName); + $sqlTableAlias = $this->getSQLTableAlias($subClass->getTableName(), $dqlAlias); - foreach ($subClass->fieldMappings as $fieldName => $mapping) { - if (isset($mapping->inherited) || ($partialFieldSet && ! in_array($fieldName, $partialFieldSet, true))) { - continue; - } + foreach ($subClass->fieldMappings as $fieldName => $mapping) { + if (isset($mapping->inherited) || ($partialFieldSet && !in_array($fieldName, $partialFieldSet, true))) { + continue; + } - $columnAlias = $this->getSQLColumnAlias($mapping->columnName); - $quotedColumnName = $this->quoteStrategy->getColumnName($fieldName, $subClass, $this->platform); + $columnAlias = $this->getSQLColumnAlias($mapping->columnName); + $quotedColumnName = $this->quoteStrategy->getColumnName($fieldName, $subClass, $this->platform); - $col = $sqlTableAlias . '.' . $quotedColumnName; + $col = $sqlTableAlias . '.' . $quotedColumnName; - $type = Type::getType($mapping->type); - $col = $type->convertToPHPValueSQL($col, $this->platform); + $type = Type::getType($mapping->type); + $col = $type->convertToPHPValueSQL($col, $this->platform); - $sqlParts[] = $col . ' AS ' . $columnAlias; + $sqlParts[] = $col . ' AS ' . $columnAlias; - $this->scalarResultAliasMap[$resultAlias][] = $columnAlias; + $this->scalarResultAliasMap[$resultAlias][] = $columnAlias; - $this->rsm->addFieldResult($dqlAlias, $columnAlias, $fieldName, $subClassName); + $this->rsm->addFieldResult($dqlAlias, $columnAlias, $fieldName, $subClassName); + } } } diff --git a/src/UnitOfWork.php b/src/UnitOfWork.php index 85a33620aa8..811aa895b42 100644 --- a/src/UnitOfWork.php +++ b/src/UnitOfWork.php @@ -12,6 +12,7 @@ use Doctrine\DBAL; use Doctrine\DBAL\Connections\PrimaryReadReplicaConnection; use Doctrine\DBAL\LockMode; +use Doctrine\Deprecations\Deprecation; use Doctrine\ORM\Cache\Persister\CachedPersister; use Doctrine\ORM\Event\ListenersInvoker; use Doctrine\ORM\Event\OnClearEventArgs; @@ -2355,10 +2356,6 @@ public function isCollectionScheduledForDeletion(PersistentCollection $coll): bo */ public function createEntity(string $className, array $data, array &$hints = []): object { - if (isset($hints[SqlWalker::HINT_PARTIAL])) { - throw HydrationException::partialObjectHydrationDisallowed(); - } - $class = $this->em->getClassMetadata($className); $id = $this->identifierFlattener->flattenIdentifier($class, $data); @@ -2417,6 +2414,18 @@ public function createEntity(string $className, array $data, array &$hints = []) unset($this->eagerLoadingEntities[$class->rootEntityName]); } + // Properly initialize any unfetched associations, if partial objects are not allowed. + if (isset($hints[Query::HINT_FORCE_PARTIAL_LOAD])) { + Deprecation::trigger( + 'doctrine/orm', + 'https://github.com/doctrine/orm/issues/8471', + 'Partial Objects are deprecated (here entity %s)', + $className, + ); + + return $entity; + } + foreach ($class->associationMappings as $field => $assoc) { // Check if the association is not among the fetch-joined associations already. if (isset($hints['fetchAlias'], $hints['fetched'][$hints['fetchAlias']][$field])) { diff --git a/tests/Performance/Hydration/MixedQueryFetchJoinPartialObjectHydrationPerformanceBench.php b/tests/Performance/Hydration/MixedQueryFetchJoinPartialObjectHydrationPerformanceBench.php new file mode 100644 index 00000000000..6a1638afcf5 --- /dev/null +++ b/tests/Performance/Hydration/MixedQueryFetchJoinPartialObjectHydrationPerformanceBench.php @@ -0,0 +1,84 @@ + '1', + 'u__status' => 'developer', + 'u__username' => 'romanb', + 'u__name' => 'Roman', + 'sclr0' => 'ROMANB', + 'p__phonenumber' => '42', + ], + [ + 'u__id' => '1', + 'u__status' => 'developer', + 'u__username' => 'romanb', + 'u__name' => 'Roman', + 'sclr0' => 'ROMANB', + 'p__phonenumber' => '43', + ], + [ + 'u__id' => '2', + 'u__status' => 'developer', + 'u__username' => 'romanb', + 'u__name' => 'Roman', + 'sclr0' => 'JWAGE', + 'p__phonenumber' => '91', + ], + ]; + + for ($i = 4; $i < 2000; ++$i) { + $resultSet[] = [ + 'u__id' => $i, + 'u__status' => 'developer', + 'u__username' => 'jwage', + 'u__name' => 'Jonathan', + 'sclr0' => 'JWAGE' . $i, + 'p__phonenumber' => '91', + ]; + } + + $this->result = ArrayResultFactory::createFromArray($resultSet); + $this->hydrator = new ObjectHydrator(EntityManagerFactory::getEntityManager([])); + $this->rsm = new ResultSetMapping(); + + $this->rsm->addEntityResult(CmsUser::class, 'u'); + $this->rsm->addJoinedEntityResult(CmsPhonenumber::class, 'p', 'u', 'phonenumbers'); + $this->rsm->addFieldResult('u', 'u__id', 'id'); + $this->rsm->addFieldResult('u', 'u__status', 'status'); + $this->rsm->addFieldResult('u', 'u__username', 'username'); + $this->rsm->addFieldResult('u', 'u__name', 'name'); + $this->rsm->addScalarResult('sclr0', 'nameUpper'); + $this->rsm->addFieldResult('p', 'p__phonenumber', 'phonenumber'); + } + + public function benchHydration(): void + { + $this->hydrator->hydrateAll($this->result, $this->rsm, [Query::HINT_FORCE_PARTIAL_LOAD => true]); + } +} diff --git a/tests/Performance/Hydration/SimpleQueryPartialObjectHydrationPerformanceBench.php b/tests/Performance/Hydration/SimpleQueryPartialObjectHydrationPerformanceBench.php new file mode 100644 index 00000000000..b894068eb2c --- /dev/null +++ b/tests/Performance/Hydration/SimpleQueryPartialObjectHydrationPerformanceBench.php @@ -0,0 +1,72 @@ + '1', + 'u__status' => 'developer', + 'u__username' => 'romanb', + 'u__name' => 'Roman', + ], + [ + 'u__id' => '1', + 'u__status' => 'developer', + 'u__username' => 'romanb', + 'u__name' => 'Roman', + ], + [ + 'u__id' => '2', + 'u__status' => 'developer', + 'u__username' => 'romanb', + 'u__name' => 'Roman', + ], + ]; + + for ($i = 4; $i < 10000; ++$i) { + $resultSet[] = [ + 'u__id' => $i, + 'u__status' => 'developer', + 'u__username' => 'jwage', + 'u__name' => 'Jonathan', + ]; + } + + $this->result = ArrayResultFactory::createFromArray($resultSet); + $this->hydrator = new ObjectHydrator(EntityManagerFactory::getEntityManager([])); + $this->rsm = new ResultSetMapping(); + + $this->rsm->addEntityResult(CmsUser::class, 'u'); + $this->rsm->addFieldResult('u', 'u__id', 'id'); + $this->rsm->addFieldResult('u', 'u__status', 'status'); + $this->rsm->addFieldResult('u', 'u__username', 'username'); + $this->rsm->addFieldResult('u', 'u__name', 'name'); + } + + public function benchHydration(): void + { + $this->hydrator->hydrateAll($this->result, $this->rsm, [Query::HINT_FORCE_PARTIAL_LOAD => true]); + } +} diff --git a/tests/Tests/ORM/Functional/OneToOneUnidirectionalAssociationTest.php b/tests/Tests/ORM/Functional/OneToOneUnidirectionalAssociationTest.php index cdb34bfcdbf..b2baaaa254e 100644 --- a/tests/Tests/ORM/Functional/OneToOneUnidirectionalAssociationTest.php +++ b/tests/Tests/ORM/Functional/OneToOneUnidirectionalAssociationTest.php @@ -5,6 +5,7 @@ namespace Doctrine\Tests\ORM\Functional; use Doctrine\ORM\Mapping\ClassMetadata; +use Doctrine\ORM\Query; use Doctrine\Tests\Models\ECommerce\ECommerceProduct; use Doctrine\Tests\Models\ECommerce\ECommerceShipping; use Doctrine\Tests\OrmFunctionalTestCase; @@ -78,6 +79,19 @@ public function testLazyLoadsObjects(): void self::assertEquals(1, $product->getShipping()->getDays()); } + public function testDoesNotLazyLoadObjectsIfConfigurationDoesNotAllowIt(): void + { + $this->createFixture(); + + $query = $this->_em->createQuery('select p from Doctrine\Tests\Models\ECommerce\ECommerceProduct p'); + $query->setHint(Query::HINT_FORCE_PARTIAL_LOAD, true); + + $result = $query->getResult(); + $product = $result[0]; + + self::assertNull($product->getShipping()); + } + protected function createFixture(): void { $product = new ECommerceProduct(); diff --git a/tests/Tests/ORM/Functional/PostLoadEventTest.php b/tests/Tests/ORM/Functional/PostLoadEventTest.php index 839a8313983..d76a044fd05 100644 --- a/tests/Tests/ORM/Functional/PostLoadEventTest.php +++ b/tests/Tests/ORM/Functional/PostLoadEventTest.php @@ -136,6 +136,27 @@ public function testLoadedProxyEntityShouldTriggerEvent(): void $userProxy->getName(); } + public function testLoadedProxyPartialShouldTriggerEvent(): void + { + $eventManager = $this->_em->getEventManager(); + + // Should not be invoked during getReference call + $mockListener = $this->createMock(PostLoadListener::class); + + // CmsUser (partially loaded), CmsAddress (inverse ToOne), 2 CmsPhonenumber + $mockListener + ->expects(self::exactly(4)) + ->method('postLoad') + ->will(self::returnValue(true)); + + $eventManager->addEventListener([Events::postLoad], $mockListener); + + $query = $this->_em->createQuery('SELECT PARTIAL u.{id, name}, p FROM Doctrine\Tests\Models\CMS\CmsUser u JOIN u.phonenumbers p WHERE u.id = :id'); + + $query->setParameter('id', $this->userId); + $query->getResult(); + } + public function testLoadedProxyAssociationToOneShouldTriggerEvent(): void { $user = $this->_em->find(CmsUser::class, $this->userId); diff --git a/tests/Tests/ORM/Functional/SecondLevelCacheQueryCacheTest.php b/tests/Tests/ORM/Functional/SecondLevelCacheQueryCacheTest.php index a3350aa5e6c..e2dec649342 100644 --- a/tests/Tests/ORM/Functional/SecondLevelCacheQueryCacheTest.php +++ b/tests/Tests/ORM/Functional/SecondLevelCacheQueryCacheTest.php @@ -1086,6 +1086,31 @@ public function testHintClearEntityRegionDeleteStatement(): void self::assertFalse($this->cache->containsEntity(Country::class, $this->countries[1]->getId())); } + public function testCacheablePartialQueryException(): void + { + $this->expectException(CacheException::class); + $this->expectExceptionMessage('Second level cache does not support partial entities.'); + $this->evictRegions(); + $this->loadFixturesCountries(); + + $this->_em->createQuery('SELECT PARTIAL c.{id} FROM Doctrine\Tests\Models\Cache\Country c') + ->setCacheable(true) + ->getResult(); + } + + public function testCacheableForcePartialLoadHintQueryException(): void + { + $this->expectException(CacheException::class); + $this->expectExceptionMessage('Second level cache does not support partial entities.'); + $this->evictRegions(); + $this->loadFixturesCountries(); + + $this->_em->createQuery('SELECT c FROM Doctrine\Tests\Models\Cache\Country c') + ->setCacheable(true) + ->setHint(Query::HINT_FORCE_PARTIAL_LOAD, true) + ->getResult(); + } + public function testNonCacheableQueryDeleteStatementException(): void { $this->expectException(CacheException::class); diff --git a/tests/Tests/ORM/Functional/Ticket/DDC163Test.php b/tests/Tests/ORM/Functional/Ticket/DDC163Test.php index 83aebb4dcae..6c0c7c94fe3 100644 --- a/tests/Tests/ORM/Functional/Ticket/DDC163Test.php +++ b/tests/Tests/ORM/Functional/Ticket/DDC163Test.php @@ -46,7 +46,7 @@ public function testQueryWithOrConditionUsingTwoRelationOnSameEntity(): void $this->_em->flush(); $this->_em->clear(); - $dql = 'SELECT person.name as person_name, spouse.name as spouse_name,friend.name as friend_name + $dql = 'SELECT PARTIAL person.{id,name}, PARTIAL spouse.{id,name}, PARTIAL friend.{id,name} FROM Doctrine\Tests\Models\Company\CompanyPerson person LEFT JOIN person.spouse spouse LEFT JOIN person.friends friend diff --git a/tests/Tests/ORM/Functional/Ticket/DDC2519Test.php b/tests/Tests/ORM/Functional/Ticket/DDC2519Test.php new file mode 100644 index 00000000000..b8dfba13b9c --- /dev/null +++ b/tests/Tests/ORM/Functional/Ticket/DDC2519Test.php @@ -0,0 +1,76 @@ +useModelSet('legacy'); + + parent::setUp(); + + $this->loadFixture(); + } + + #[Group('DDC-2519')] + public function testIssue(): void + { + $dql = 'SELECT PARTIAL l.{_source, _target} FROM Doctrine\Tests\Models\Legacy\LegacyUserReference l'; + $result = $this->_em->createQuery($dql)->getResult(); + + self::assertCount(2, $result); + self::assertInstanceOf(LegacyUserReference::class, $result[0]); + self::assertInstanceOf(LegacyUserReference::class, $result[1]); + + self::assertInstanceOf(LegacyUser::class, $result[0]->source()); + self::assertInstanceOf(LegacyUser::class, $result[0]->target()); + self::assertInstanceOf(LegacyUser::class, $result[1]->source()); + self::assertInstanceOf(LegacyUser::class, $result[1]->target()); + + self::assertTrue($this->isUninitializedObject($result[0]->target())); + self::assertTrue($this->isUninitializedObject($result[0]->source())); + self::assertTrue($this->isUninitializedObject($result[1]->target())); + self::assertTrue($this->isUninitializedObject($result[1]->source())); + + self::assertNotNull($result[0]->source()->getId()); + self::assertNotNull($result[0]->target()->getId()); + self::assertNotNull($result[1]->source()->getId()); + self::assertNotNull($result[1]->target()->getId()); + } + + public function loadFixture(): void + { + $user1 = new LegacyUser(); + $user1->username = 'FabioBatSilva'; + $user1->name = 'Fabio B. Silva'; + + $user2 = new LegacyUser(); + $user2->username = 'doctrinebot'; + $user2->name = 'Doctrine Bot'; + + $user3 = new LegacyUser(); + $user3->username = 'test'; + $user3->name = 'Tester'; + + $this->_em->persist($user1); + $this->_em->persist($user2); + $this->_em->persist($user3); + + $this->_em->flush(); + + $this->_em->persist(new LegacyUserReference($user1, $user2, 'foo')); + $this->_em->persist(new LegacyUserReference($user1, $user3, 'bar')); + + $this->_em->flush(); + $this->_em->clear(); + } +} diff --git a/tests/Tests/ORM/Functional/Ticket/GH8443Test.php b/tests/Tests/ORM/Functional/Ticket/GH8443Test.php index 0b5352a6ea8..44695f1856e 100644 --- a/tests/Tests/ORM/Functional/Ticket/GH8443Test.php +++ b/tests/Tests/ORM/Functional/Ticket/GH8443Test.php @@ -14,6 +14,9 @@ use Doctrine\ORM\Mapping\JoinColumn; use Doctrine\ORM\Mapping\OneToOne; use Doctrine\ORM\Mapping\Table; +use Doctrine\ORM\Query; +use Doctrine\Tests\Models\Company\CompanyManager; +use Doctrine\Tests\Models\Company\CompanyPerson; use Doctrine\Tests\OrmFunctionalTestCase; use PHPUnit\Framework\Attributes\Group; @@ -30,6 +33,35 @@ protected function setUp(): void $this->createSchemaForModels(GH8443Foo::class); } + #[Group('GH-8443')] + public function testJoinRootEntityWithForcePartialLoad(): void + { + $person = new CompanyPerson(); + $person->setName('John'); + + $manager = new CompanyManager(); + $manager->setName('Adam'); + $manager->setSalary(1000); + $manager->setDepartment('IT'); + $manager->setTitle('manager'); + + $manager->setSpouse($person); + + $this->_em->persist($person); + $this->_em->persist($manager); + $this->_em->flush(); + $this->_em->clear(); + + $manager = $this->_em->createQuery( + "SELECT m from Doctrine\Tests\Models\Company\CompanyManager m + JOIN m.spouse s + WITH s.name = 'John'", + )->setHint(Query::HINT_FORCE_PARTIAL_LOAD, true)->getSingleResult(); + $this->_em->refresh($manager); + + $this->assertEquals('John', $manager->getSpouse()->getName()); + } + #[Group('GH-8443')] public function testJoinRootEntityWithOnlyOneEntityInHierarchy(): void { diff --git a/tests/Tests/ORM/Functional/ValueObjectsTest.php b/tests/Tests/ORM/Functional/ValueObjectsTest.php index 6656d916ee0..3186e71d357 100644 --- a/tests/Tests/ORM/Functional/ValueObjectsTest.php +++ b/tests/Tests/ORM/Functional/ValueObjectsTest.php @@ -209,6 +209,58 @@ public function testDqlOnEmbeddedObjectsField(): void self::assertNull($this->_em->find(DDC93Person::class, $person->id)); } + public function testPartialDqlOnEmbeddedObjectsField(): void + { + $person = new DDC93Person('Karl', new DDC93Address('Foo', '12345', 'Gosport', new DDC93Country('England'))); + $this->_em->persist($person); + $this->_em->flush(); + $this->_em->clear(); + + // Prove that the entity was persisted correctly. + $dql = 'SELECT p FROM ' . __NAMESPACE__ . '\\DDC93Person p WHERE p.name = :name'; + + $person = $this->_em->createQuery($dql) + ->setParameter('name', 'Karl') + ->getSingleResult(); + + self::assertEquals('Gosport', $person->address->city); + self::assertEquals('Foo', $person->address->street); + self::assertEquals('12345', $person->address->zip); + self::assertEquals('England', $person->address->country->name); + + // Clear the EM and prove that the embeddable can be the subject of a partial query. + $this->_em->clear(); + + $dql = 'SELECT PARTIAL p.{id,address.city} FROM ' . __NAMESPACE__ . '\\DDC93Person p WHERE p.name = :name'; + + $person = $this->_em->createQuery($dql) + ->setParameter('name', 'Karl') + ->getSingleResult(); + + // Selected field must be equal, all other fields must be null. + self::assertEquals('Gosport', $person->address->city); + self::assertNull($person->address->street); + self::assertNull($person->address->zip); + self::assertNull($person->address->country); + self::assertNull($person->name); + + // Clear the EM and prove that the embeddable can be the subject of a partial query regardless of attributes positions. + $this->_em->clear(); + + $dql = 'SELECT PARTIAL p.{address.city, id} FROM ' . __NAMESPACE__ . '\\DDC93Person p WHERE p.name = :name'; + + $person = $this->_em->createQuery($dql) + ->setParameter('name', 'Karl') + ->getSingleResult(); + + // Selected field must be equal, all other fields must be null. + self::assertEquals('Gosport', $person->address->city); + self::assertNull($person->address->street); + self::assertNull($person->address->zip); + self::assertNull($person->address->country); + self::assertNull($person->name); + } + public function testDqlWithNonExistentEmbeddableField(): void { $this->expectException(QueryException::class); @@ -224,7 +276,7 @@ public function testPartialDqlWithNonExistentEmbeddableField(): void $this->expectExceptionMessage("no mapped field named 'address.asdfasdf'"); $this->_em->createQuery('SELECT PARTIAL p.{id,address.asdfasdf} FROM ' . __NAMESPACE__ . '\\DDC93Person p') - ->getArrayResult(); + ->execute(); } public function testEmbeddableWithInheritance(): void diff --git a/tests/Tests/ORM/Query/LanguageRecognitionTest.php b/tests/Tests/ORM/Query/LanguageRecognitionTest.php index ec57b1f1382..de212ebdda5 100644 --- a/tests/Tests/ORM/Query/LanguageRecognitionTest.php +++ b/tests/Tests/ORM/Query/LanguageRecognitionTest.php @@ -532,13 +532,11 @@ public function testUnknownAbstractSchemaName(): void public function testCorrectPartialObjectLoad(): void { - $this->hydrationMode = AbstractQuery::HYDRATE_ARRAY; $this->assertValidDQL('SELECT PARTIAL u.{id,name} FROM Doctrine\Tests\Models\CMS\CmsUser u'); } public function testIncorrectPartialObjectLoadBecauseOfMissingIdentifier(): void { - $this->hydrationMode = AbstractQuery::HYDRATE_ARRAY; $this->assertInvalidDQL('SELECT PARTIAL u.{name} FROM Doctrine\Tests\Models\CMS\CmsUser u'); } diff --git a/tests/Tests/ORM/Query/ParserTest.php b/tests/Tests/ORM/Query/ParserTest.php index 6290bbc4dab..ed75be517b0 100644 --- a/tests/Tests/ORM/Query/ParserTest.php +++ b/tests/Tests/ORM/Query/ParserTest.php @@ -116,15 +116,6 @@ public function testNullLookahead(): void $parser->match(TokenType::T_SELECT); } - public function testPartialExpressionWithObjectHydratorThrows(): void - { - $this->expectException(HydrationException::class); - $this->expectExceptionMessage('Hydration of entity objects is not allowed when DQL PARTIAL keyword is used.'); - - $parser = $this->createParser(CmsUser::class); - $parser->PartialObjectExpression(); - } - private function createParser(string $dql): Parser { $query = new Query($this->getTestEntityManager()); diff --git a/tests/Tests/ORM/Query/SelectSqlGenerationTest.php b/tests/Tests/ORM/Query/SelectSqlGenerationTest.php index 3969fe51636..6b7d9585462 100644 --- a/tests/Tests/ORM/Query/SelectSqlGenerationTest.php +++ b/tests/Tests/ORM/Query/SelectSqlGenerationTest.php @@ -1335,8 +1335,6 @@ public function testIdentityFunctionWithCompositePrimaryKey(): void #[Group('DDC-2519')] public function testPartialWithAssociationIdentifier(): void { - $this->hydrationMode = ORMQuery::HYDRATE_ARRAY; - $this->assertSqlGeneration( 'SELECT PARTIAL l.{_source, _target} FROM Doctrine\Tests\Models\Legacy\LegacyUserReference l', 'SELECT l0_.iUserIdSource AS iUserIdSource_0, l0_.iUserIdTarget AS iUserIdTarget_1 FROM legacy_users_reference l0_', @@ -1382,58 +1380,122 @@ public function testIdentityFunctionDoesNotAcceptStateField(): void } #[Group('DDC-1389')] - public function testInheritanceTypeJoinInRootClass(): void + public function testInheritanceTypeJoinInRootClassWithDisabledForcePartialLoad(): void { $this->assertSqlGeneration( 'SELECT p FROM Doctrine\Tests\Models\Company\CompanyPerson p', 'SELECT c0_.id AS id_0, c0_.name AS name_1, c1_.title AS title_2, c2_.salary AS salary_3, c2_.department AS department_4, c2_.startDate AS startDate_5, c0_.discr AS discr_6, c0_.spouse_id AS spouse_id_7, c1_.car_id AS car_id_8 FROM company_persons c0_ LEFT JOIN company_managers c1_ ON c0_.id = c1_.id LEFT JOIN company_employees c2_ ON c0_.id = c2_.id', + [ORMQuery::HINT_FORCE_PARTIAL_LOAD => false], ); } #[Group('DDC-1389')] - public function testInheritanceTypeJoinInChildClass(): void + public function testInheritanceTypeJoinInRootClassWithEnabledForcePartialLoad(): void + { + $this->assertSqlGeneration( + 'SELECT p FROM Doctrine\Tests\Models\Company\CompanyPerson p', + 'SELECT c0_.id AS id_0, c0_.name AS name_1, c0_.discr AS discr_2 FROM company_persons c0_', + [ORMQuery::HINT_FORCE_PARTIAL_LOAD => true], + ); + } + + #[Group('DDC-1389')] + public function testInheritanceTypeJoinInChildClassWithDisabledForcePartialLoad(): void { $this->assertSqlGeneration( 'SELECT e FROM Doctrine\Tests\Models\Company\CompanyEmployee e', 'SELECT c0_.id AS id_0, c0_.name AS name_1, c1_.salary AS salary_2, c1_.department AS department_3, c1_.startDate AS startDate_4, c2_.title AS title_5, c0_.discr AS discr_6, c0_.spouse_id AS spouse_id_7, c2_.car_id AS car_id_8 FROM company_employees c1_ INNER JOIN company_persons c0_ ON c1_.id = c0_.id LEFT JOIN company_managers c2_ ON c1_.id = c2_.id', + [ORMQuery::HINT_FORCE_PARTIAL_LOAD => false], + ); + } + + #[Group('DDC-1389')] + public function testInheritanceTypeJoinInChildClassWithEnabledForcePartialLoad(): void + { + $this->assertSqlGeneration( + 'SELECT e FROM Doctrine\Tests\Models\Company\CompanyEmployee e', + 'SELECT c0_.id AS id_0, c0_.name AS name_1, c1_.salary AS salary_2, c1_.department AS department_3, c1_.startDate AS startDate_4, c0_.discr AS discr_5 FROM company_employees c1_ INNER JOIN company_persons c0_ ON c1_.id = c0_.id', + [ORMQuery::HINT_FORCE_PARTIAL_LOAD => true], ); } #[Group('DDC-1389')] - public function testInheritanceTypeJoinInLeafClass(): void + public function testInheritanceTypeJoinInLeafClassWithDisabledForcePartialLoad(): void { $this->assertSqlGeneration( 'SELECT m FROM Doctrine\Tests\Models\Company\CompanyManager m', 'SELECT c0_.id AS id_0, c0_.name AS name_1, c1_.salary AS salary_2, c1_.department AS department_3, c1_.startDate AS startDate_4, c2_.title AS title_5, c0_.discr AS discr_6, c0_.spouse_id AS spouse_id_7, c2_.car_id AS car_id_8 FROM company_managers c2_ INNER JOIN company_employees c1_ ON c2_.id = c1_.id INNER JOIN company_persons c0_ ON c2_.id = c0_.id', + [ORMQuery::HINT_FORCE_PARTIAL_LOAD => false], + ); + } + + #[Group('DDC-1389')] + public function testInheritanceTypeJoinInLeafClassWithEnabledForcePartialLoad(): void + { + $this->assertSqlGeneration( + 'SELECT m FROM Doctrine\Tests\Models\Company\CompanyManager m', + 'SELECT c0_.id AS id_0, c0_.name AS name_1, c1_.salary AS salary_2, c1_.department AS department_3, c1_.startDate AS startDate_4, c2_.title AS title_5, c0_.discr AS discr_6 FROM company_managers c2_ INNER JOIN company_employees c1_ ON c2_.id = c1_.id INNER JOIN company_persons c0_ ON c2_.id = c0_.id', + [ORMQuery::HINT_FORCE_PARTIAL_LOAD => true], ); } #[Group('DDC-1389')] - public function testInheritanceTypeSingleTableInRootClass(): void + public function testInheritanceTypeSingleTableInRootClassWithDisabledForcePartialLoad(): void { $this->assertSqlGeneration( 'SELECT c FROM Doctrine\Tests\Models\Company\CompanyContract c', "SELECT c0_.id AS id_0, c0_.completed AS completed_1, c0_.fixPrice AS fixPrice_2, c0_.hoursWorked AS hoursWorked_3, c0_.pricePerHour AS pricePerHour_4, c0_.maxPrice AS maxPrice_5, c0_.discr AS discr_6, c0_.salesPerson_id AS salesPerson_id_7 FROM company_contracts c0_ WHERE c0_.discr IN ('fix', 'flexible', 'flexultra')", + [ORMQuery::HINT_FORCE_PARTIAL_LOAD => false], + ); + } + + #[Group('DDC-1389')] + public function testInheritanceTypeSingleTableInRootClassWithEnabledForcePartialLoad(): void + { + $this->assertSqlGeneration( + 'SELECT c FROM Doctrine\Tests\Models\Company\CompanyContract c', + "SELECT c0_.id AS id_0, c0_.completed AS completed_1, c0_.fixPrice AS fixPrice_2, c0_.hoursWorked AS hoursWorked_3, c0_.pricePerHour AS pricePerHour_4, c0_.maxPrice AS maxPrice_5, c0_.discr AS discr_6 FROM company_contracts c0_ WHERE c0_.discr IN ('fix', 'flexible', 'flexultra')", + [ORMQuery::HINT_FORCE_PARTIAL_LOAD => true], ); } #[Group('DDC-1389')] public function testInheritanceTypeSingleTableInChildClassWithDisabledForcePartialLoad(): void { - $this->hydrationMode = ORMQuery::HYDRATE_ARRAY; + $this->assertSqlGeneration( + 'SELECT fc FROM Doctrine\Tests\Models\Company\CompanyFlexContract fc', + "SELECT c0_.id AS id_0, c0_.completed AS completed_1, c0_.hoursWorked AS hoursWorked_2, c0_.pricePerHour AS pricePerHour_3, c0_.maxPrice AS maxPrice_4, c0_.discr AS discr_5, c0_.salesPerson_id AS salesPerson_id_6 FROM company_contracts c0_ WHERE c0_.discr IN ('flexible', 'flexultra')", + [ORMQuery::HINT_FORCE_PARTIAL_LOAD => false], + ); + } + #[Group('DDC-1389')] + public function testInheritanceTypeSingleTableInChildClassWithEnabledForcePartialLoad(): void + { $this->assertSqlGeneration( 'SELECT fc FROM Doctrine\Tests\Models\Company\CompanyFlexContract fc', "SELECT c0_.id AS id_0, c0_.completed AS completed_1, c0_.hoursWorked AS hoursWorked_2, c0_.pricePerHour AS pricePerHour_3, c0_.maxPrice AS maxPrice_4, c0_.discr AS discr_5 FROM company_contracts c0_ WHERE c0_.discr IN ('flexible', 'flexultra')", + [ORMQuery::HINT_FORCE_PARTIAL_LOAD => true], ); } #[Group('DDC-1389')] - public function testInheritanceTypeSingleTableInLeafClass(): void + public function testInheritanceTypeSingleTableInLeafClassWithDisabledForcePartialLoad(): void { $this->assertSqlGeneration( 'SELECT fuc FROM Doctrine\Tests\Models\Company\CompanyFlexUltraContract fuc', "SELECT c0_.id AS id_0, c0_.completed AS completed_1, c0_.hoursWorked AS hoursWorked_2, c0_.pricePerHour AS pricePerHour_3, c0_.maxPrice AS maxPrice_4, c0_.discr AS discr_5, c0_.salesPerson_id AS salesPerson_id_6 FROM company_contracts c0_ WHERE c0_.discr IN ('flexultra')", + [ORMQuery::HINT_FORCE_PARTIAL_LOAD => false], + ); + } + + #[Group('DDC-1389')] + public function testInheritanceTypeSingleTableInLeafClassWithEnabledForcePartialLoad(): void + { + $this->assertSqlGeneration( + 'SELECT fuc FROM Doctrine\Tests\Models\Company\CompanyFlexUltraContract fuc', + "SELECT c0_.id AS id_0, c0_.completed AS completed_1, c0_.hoursWorked AS hoursWorked_2, c0_.pricePerHour AS pricePerHour_3, c0_.maxPrice AS maxPrice_4, c0_.discr AS discr_5 FROM company_contracts c0_ WHERE c0_.discr IN ('flexultra')", + [ORMQuery::HINT_FORCE_PARTIAL_LOAD => true], ); } @@ -1712,8 +1774,6 @@ public function testCustomTypeValueSqlForAllFields(): void public function testCustomTypeValueSqlForPartialObject(): void { - $this->hydrationMode = ORMQuery::HYDRATE_ARRAY; - if (DBALType::hasType('negative_to_positive')) { DBALType::overrideType('negative_to_positive', NegativeToPositiveType::class); } else { @@ -1722,7 +1782,7 @@ public function testCustomTypeValueSqlForPartialObject(): void $this->assertSqlGeneration( 'SELECT partial p.{id, customInteger} FROM Doctrine\Tests\Models\CustomType\CustomTypeParent p', - 'SELECT c0_.id AS id_0, -(c0_.customInteger) AS customInteger_1 FROM customtype_parents c0_', + 'SELECT c0_.id AS id_0, -(c0_.customInteger) AS customInteger_1, c0_.child_id AS child_id_2 FROM customtype_parents c0_', ); } diff --git a/tests/Tests/ORM/Tools/Pagination/LimitSubqueryOutputWalkerTest.php b/tests/Tests/ORM/Tools/Pagination/LimitSubqueryOutputWalkerTest.php index 0f5bac25b72..0e14402b5a0 100644 --- a/tests/Tests/ORM/Tools/Pagination/LimitSubqueryOutputWalkerTest.php +++ b/tests/Tests/ORM/Tools/Pagination/LimitSubqueryOutputWalkerTest.php @@ -137,7 +137,7 @@ public function testCountQueryWithComplexScalarOrderByItemWithoutJoin(): void ); } - public function testCountQueryWithComplexScalarOrderByItemJoined(): void + public function testCountQueryWithComplexScalarOrderByItemJoinedWithoutPartial(): void { $this->entityManager = $this->createTestEntityManagerWithPlatform(new MySQLPlatform()); diff --git a/tests/Tests/ORM/UnitOfWorkTest.php b/tests/Tests/ORM/UnitOfWorkTest.php index 8de0eb03e25..dba967fe87f 100644 --- a/tests/Tests/ORM/UnitOfWorkTest.php +++ b/tests/Tests/ORM/UnitOfWorkTest.php @@ -651,15 +651,6 @@ public function testItThrowsWhenApplicationProvidedIdsCollide(): void $this->_unitOfWork->persist($phone2); } - - public function testItThrowsWhenCreateEntityWithSqlWalkerPartialQueryHint(): void - { - $this->expectException(HydrationException::class); - $this->expectExceptionMessage('Hydration of entity objects is not allowed when DQL PARTIAL keyword is used.'); - - $hints = [SqlWalker::HINT_PARTIAL => true]; - $this->_unitOfWork->createEntity(VersionedAssignedIdentifierEntity::class, ['id' => 1], $hints); - } } From 7ef1f0a379dc122a252fe90aba5d556199025831 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Wed, 9 Oct 2024 22:14:01 +0200 Subject: [PATCH 04/21] Phpcs fixes --- src/Decorator/EntityManagerDecorator.php | 1 - src/EntityManager.php | 1 - src/Query/Parser.php | 2 -- src/Query/SqlWalker.php | 8 ++++---- src/UnitOfWork.php | 2 -- tests/Tests/ORM/Query/ParserTest.php | 1 - tests/Tests/ORM/UnitOfWorkTest.php | 2 -- 7 files changed, 4 insertions(+), 13 deletions(-) diff --git a/src/Decorator/EntityManagerDecorator.php b/src/Decorator/EntityManagerDecorator.php index 212e01e511b..6f1b0419686 100644 --- a/src/Decorator/EntityManagerDecorator.php +++ b/src/Decorator/EntityManagerDecorator.php @@ -8,7 +8,6 @@ use Doctrine\Common\EventManager; use Doctrine\DBAL\Connection; use Doctrine\DBAL\LockMode; -use Doctrine\Deprecations\Deprecation; use Doctrine\ORM\Cache; use Doctrine\ORM\Configuration; use Doctrine\ORM\EntityManagerInterface; diff --git a/src/EntityManager.php b/src/EntityManager.php index 9de636e5b75..8045ac2f5e3 100644 --- a/src/EntityManager.php +++ b/src/EntityManager.php @@ -9,7 +9,6 @@ use Doctrine\Common\EventManager; use Doctrine\DBAL\Connection; use Doctrine\DBAL\LockMode; -use Doctrine\Deprecations\Deprecation; use Doctrine\ORM\Exception\EntityManagerClosed; use Doctrine\ORM\Exception\InvalidHydrationMode; use Doctrine\ORM\Exception\MissingIdentifierField; diff --git a/src/Query/Parser.php b/src/Query/Parser.php index 12fe0c97691..76e6757052d 100644 --- a/src/Query/Parser.php +++ b/src/Query/Parser.php @@ -5,9 +5,7 @@ namespace Doctrine\ORM\Query; use Doctrine\Common\Lexer\Token; -use Doctrine\Deprecations\Deprecation; use Doctrine\ORM\EntityManagerInterface; -use Doctrine\ORM\Internal\Hydration\HydrationException; use Doctrine\ORM\Mapping\AssociationMapping; use Doctrine\ORM\Mapping\ClassMetadata; use Doctrine\ORM\Query; diff --git a/src/Query/SqlWalker.php b/src/Query/SqlWalker.php index 9beeff337be..155461ab2dd 100644 --- a/src/Query/SqlWalker.php +++ b/src/Query/SqlWalker.php @@ -1406,21 +1406,21 @@ public function walkSelectExpression(AST\SelectExpression $selectExpression): st // since it requires outer joining subtables. if ($class->isInheritanceTypeSingleTable() || ! $this->query->getHint(Query::HINT_FORCE_PARTIAL_LOAD)) { foreach ($class->subClasses as $subClassName) { - $subClass = $this->em->getClassMetadata($subClassName); + $subClass = $this->em->getClassMetadata($subClassName); $sqlTableAlias = $this->getSQLTableAlias($subClass->getTableName(), $dqlAlias); foreach ($subClass->fieldMappings as $fieldName => $mapping) { - if (isset($mapping->inherited) || ($partialFieldSet && !in_array($fieldName, $partialFieldSet, true))) { + if (isset($mapping->inherited) || ($partialFieldSet && ! in_array($fieldName, $partialFieldSet, true))) { continue; } - $columnAlias = $this->getSQLColumnAlias($mapping->columnName); + $columnAlias = $this->getSQLColumnAlias($mapping->columnName); $quotedColumnName = $this->quoteStrategy->getColumnName($fieldName, $subClass, $this->platform); $col = $sqlTableAlias . '.' . $quotedColumnName; $type = Type::getType($mapping->type); - $col = $type->convertToPHPValueSQL($col, $this->platform); + $col = $type->convertToPHPValueSQL($col, $this->platform); $sqlParts[] = $col . ' AS ' . $columnAlias; diff --git a/src/UnitOfWork.php b/src/UnitOfWork.php index 811aa895b42..ef31a6bbcd6 100644 --- a/src/UnitOfWork.php +++ b/src/UnitOfWork.php @@ -29,7 +29,6 @@ use Doctrine\ORM\Exception\ORMException; use Doctrine\ORM\Exception\UnexpectedAssociationValue; use Doctrine\ORM\Id\AssignedGenerator; -use Doctrine\ORM\Internal\Hydration\HydrationException; use Doctrine\ORM\Internal\HydrationCompleteHandler; use Doctrine\ORM\Internal\StronglyConnectedComponents; use Doctrine\ORM\Internal\TopologicalSort; @@ -45,7 +44,6 @@ use Doctrine\ORM\Persisters\Entity\JoinedSubclassPersister; use Doctrine\ORM\Persisters\Entity\SingleTablePersister; use Doctrine\ORM\Proxy\InternalProxy; -use Doctrine\ORM\Query\SqlWalker; use Doctrine\ORM\Utility\IdentifierFlattener; use Doctrine\Persistence\PropertyChangedListener; use Exception; diff --git a/tests/Tests/ORM/Query/ParserTest.php b/tests/Tests/ORM/Query/ParserTest.php index ed75be517b0..430177b1fcc 100644 --- a/tests/Tests/ORM/Query/ParserTest.php +++ b/tests/Tests/ORM/Query/ParserTest.php @@ -4,7 +4,6 @@ namespace Doctrine\Tests\ORM\Query; -use Doctrine\ORM\Internal\Hydration\HydrationException; use Doctrine\ORM\Query; use Doctrine\ORM\Query\Parser; use Doctrine\ORM\Query\QueryException; diff --git a/tests/Tests/ORM/UnitOfWorkTest.php b/tests/Tests/ORM/UnitOfWorkTest.php index dba967fe87f..a9112fa5d51 100644 --- a/tests/Tests/ORM/UnitOfWorkTest.php +++ b/tests/Tests/ORM/UnitOfWorkTest.php @@ -13,7 +13,6 @@ use Doctrine\DBAL\Platforms\AbstractPlatform; use Doctrine\ORM\EntityNotFoundException; use Doctrine\ORM\Exception\EntityIdentityCollisionException; -use Doctrine\ORM\Internal\Hydration\HydrationException; use Doctrine\ORM\Mapping\ClassMetadata; use Doctrine\ORM\Mapping\Column; use Doctrine\ORM\Mapping\Entity; @@ -23,7 +22,6 @@ use Doctrine\ORM\Mapping\Version; use Doctrine\ORM\OptimisticLockException; use Doctrine\ORM\ORMInvalidArgumentException; -use Doctrine\ORM\Query\SqlWalker; use Doctrine\ORM\UnitOfWork; use Doctrine\Tests\Mocks\EntityManagerMock; use Doctrine\Tests\Mocks\EntityPersisterMock; From 51be1b1d526342543f783f18352862cbdd2a8551 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9goire=20Paris?= Date: Wed, 9 Oct 2024 15:11:06 +0200 Subject: [PATCH 05/21] Run risky code in finally block catch blocks are not supposed to fail. If you want to do something despite an exception happening, you should do it in a finally block. Closes #7545 --- src/EntityManager.php | 29 ++++++++++++-------- src/UnitOfWork.php | 19 ++++++++------ tests/Tests/ORM/EntityManagerTest.php | 34 ++++++++++++++++++++++++ tests/Tests/ORM/UnitOfWorkTest.php | 38 +++++++++++++++++++++++++++ 4 files changed, 101 insertions(+), 19 deletions(-) diff --git a/src/EntityManager.php b/src/EntityManager.php index f7d47d7b12e..da09ed1b77a 100644 --- a/src/EntityManager.php +++ b/src/EntityManager.php @@ -32,7 +32,6 @@ use Doctrine\Persistence\Mapping\MappingException; use Doctrine\Persistence\ObjectRepository; use InvalidArgumentException; -use Throwable; use function array_keys; use function class_exists; @@ -246,18 +245,22 @@ public function transactional($func) $this->conn->beginTransaction(); + $successful = false; + try { $return = $func($this); $this->flush(); $this->conn->commit(); - return $return ?: true; - } catch (Throwable $e) { - $this->close(); - $this->conn->rollBack(); + $successful = true; - throw $e; + return $return ?: true; + } finally { + if (! $successful) { + $this->close(); + $this->conn->rollBack(); + } } } @@ -268,18 +271,22 @@ public function wrapInTransaction(callable $func) { $this->conn->beginTransaction(); + $successful = false; + try { $return = $func($this); $this->flush(); $this->conn->commit(); - return $return; - } catch (Throwable $e) { - $this->close(); - $this->conn->rollBack(); + $successful = true; - throw $e; + return $return; + } finally { + if (! $successful) { + $this->close(); + $this->conn->rollBack(); + } } } diff --git a/src/UnitOfWork.php b/src/UnitOfWork.php index 39ba6b68b7f..97d80862e39 100644 --- a/src/UnitOfWork.php +++ b/src/UnitOfWork.php @@ -49,7 +49,6 @@ use Exception; use InvalidArgumentException; use RuntimeException; -use Throwable; use UnexpectedValueException; use function array_chunk; @@ -427,6 +426,8 @@ public function commit($entity = null) $conn = $this->em->getConnection(); $conn->beginTransaction(); + $successful = false; + try { // Collection deletions (deletions of complete collections) foreach ($this->collectionDeletions as $collectionToDelete) { @@ -478,16 +479,18 @@ public function commit($entity = null) throw new OptimisticLockException('Commit failed', $object); } - } catch (Throwable $e) { - $this->em->close(); - if ($conn->isTransactionActive()) { - $conn->rollBack(); - } + $successful = true; + } finally { + if (! $successful) { + $this->em->close(); - $this->afterTransactionRolledBack(); + if ($conn->isTransactionActive()) { + $conn->rollBack(); + } - throw $e; + $this->afterTransactionRolledBack(); + } } $this->afterTransactionComplete(); diff --git a/tests/Tests/ORM/EntityManagerTest.php b/tests/Tests/ORM/EntityManagerTest.php index c3ad9f559e7..c9b85f6b4f1 100644 --- a/tests/Tests/ORM/EntityManagerTest.php +++ b/tests/Tests/ORM/EntityManagerTest.php @@ -21,9 +21,12 @@ use Doctrine\ORM\UnitOfWork; use Doctrine\Persistence\Mapping\Driver\MappingDriver; use Doctrine\Persistence\Mapping\MappingException; +use Doctrine\Tests\Mocks\ConnectionMock; +use Doctrine\Tests\Mocks\EntityManagerMock; use Doctrine\Tests\Models\CMS\CmsUser; use Doctrine\Tests\Models\GeoNames\Country; use Doctrine\Tests\OrmTestCase; +use Exception; use Generator; use InvalidArgumentException; use stdClass; @@ -31,6 +34,7 @@ use function get_class; use function random_int; +use function sprintf; use function sys_get_temp_dir; use function uniqid; @@ -382,4 +386,34 @@ public function testDeprecatedFlushWithArguments(): void $this->entityManager->flush($entity); } + + /** @dataProvider entityManagerMethodNames */ + public function testItPreservesTheOriginalExceptionOnRollbackFailure(string $methodName): void + { + $entityManager = new EntityManagerMock(new class extends ConnectionMock { + public function rollBack(): bool + { + throw new Exception('Rollback exception'); + } + }); + + try { + $entityManager->transactional(static function (): void { + throw new Exception('Original exception'); + }); + self::fail('Exception expected'); + } catch (Exception $e) { + self::assertSame('Rollback exception', $e->getMessage()); + self::assertNotNull($e->getPrevious()); + self::assertSame('Original exception', $e->getPrevious()->getMessage()); + } + } + + /** @return Generator */ + public function entityManagerMethodNames(): Generator + { + foreach (['transactional', 'wrapInTransaction'] as $methodName) { + yield sprintf('%s()', $methodName) => [$methodName]; + } + } } diff --git a/tests/Tests/ORM/UnitOfWorkTest.php b/tests/Tests/ORM/UnitOfWorkTest.php index ee475e729d0..ae6b1d282f3 100644 --- a/tests/Tests/ORM/UnitOfWorkTest.php +++ b/tests/Tests/ORM/UnitOfWorkTest.php @@ -41,6 +41,7 @@ use Doctrine\Tests\Models\GeoNames\Country; use Doctrine\Tests\OrmTestCase; use Doctrine\Tests\PHPUnitCompatibility\MockBuilderCompatibilityTools; +use Exception; use PHPUnit\Framework\MockObject\MockObject; use stdClass; @@ -971,6 +972,43 @@ public function testItThrowsWhenApplicationProvidedIdsCollide(): void $this->_unitOfWork->persist($phone2); } + + public function testItPreservesTheOriginalExceptionOnRollbackFailure(): void + { + $this->_connectionMock = new class extends ConnectionMock { + public function commit(): bool + { + return false; // this should cause an exception + } + + public function rollBack(): bool + { + throw new Exception('Rollback exception'); + } + }; + $this->_emMock = new EntityManagerMock($this->_connectionMock); + $this->_unitOfWork = new UnitOfWorkMock($this->_emMock); + $this->_emMock->setUnitOfWork($this->_unitOfWork); + + // Setup fake persister and id generator + $userPersister = new EntityPersisterMock($this->_emMock, $this->_emMock->getClassMetadata(ForumUser::class)); + $userPersister->setMockIdGeneratorType(ClassMetadata::GENERATOR_TYPE_IDENTITY); + $this->_unitOfWork->setEntityPersister(ForumUser::class, $userPersister); + + // Create a test user + $user = new ForumUser(); + $user->username = 'Jasper'; + $this->_unitOfWork->persist($user); + + try { + $this->_unitOfWork->commit(); + self::fail('Exception expected'); + } catch (Exception $e) { + self::assertSame('Rollback exception', $e->getMessage()); + self::assertNotNull($e->getPrevious()); + self::assertSame('Commit failed', $e->getPrevious()->getMessage()); + } + } } /** @Entity */ From 5f4ecfd1d8a03522b3742d2c12f4403e55320a22 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Thu, 10 Oct 2024 10:38:33 +0200 Subject: [PATCH 06/21] Fix imports --- ...ixedQueryFetchJoinPartialObjectHydrationPerformanceBench.php | 2 +- .../SimpleQueryPartialObjectHydrationPerformanceBench.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Performance/Hydration/MixedQueryFetchJoinPartialObjectHydrationPerformanceBench.php b/tests/Performance/Hydration/MixedQueryFetchJoinPartialObjectHydrationPerformanceBench.php index 6a1638afcf5..822a019c7d7 100644 --- a/tests/Performance/Hydration/MixedQueryFetchJoinPartialObjectHydrationPerformanceBench.php +++ b/tests/Performance/Hydration/MixedQueryFetchJoinPartialObjectHydrationPerformanceBench.php @@ -8,8 +8,8 @@ use Doctrine\ORM\Internal\Hydration\ObjectHydrator; use Doctrine\ORM\Query; use Doctrine\ORM\Query\ResultSetMapping; -use Doctrine\Performance\ArrayResultFactory; use Doctrine\Performance\EntityManagerFactory; +use Doctrine\Tests\Mocks\ArrayResultFactory; use Doctrine\Tests\Models\CMS\CmsPhonenumber; use Doctrine\Tests\Models\CMS\CmsUser; use PhpBench\Benchmark\Metadata\Annotations\BeforeMethods; diff --git a/tests/Performance/Hydration/SimpleQueryPartialObjectHydrationPerformanceBench.php b/tests/Performance/Hydration/SimpleQueryPartialObjectHydrationPerformanceBench.php index b894068eb2c..72c2ec73d98 100644 --- a/tests/Performance/Hydration/SimpleQueryPartialObjectHydrationPerformanceBench.php +++ b/tests/Performance/Hydration/SimpleQueryPartialObjectHydrationPerformanceBench.php @@ -8,8 +8,8 @@ use Doctrine\ORM\Internal\Hydration\ObjectHydrator; use Doctrine\ORM\Query; use Doctrine\ORM\Query\ResultSetMapping; -use Doctrine\Performance\ArrayResultFactory; use Doctrine\Performance\EntityManagerFactory; +use Doctrine\Tests\Mocks\ArrayResultFactory; use Doctrine\Tests\Models\CMS\CmsUser; use PhpBench\Benchmark\Metadata\Annotations\BeforeMethods; From da7854f586507872e6db421d7273e7666ecd71bf Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Thu, 10 Oct 2024 10:41:04 +0200 Subject: [PATCH 07/21] Fix imports --- ...ixedQueryFetchJoinPartialObjectHydrationPerformanceBench.php | 2 +- .../SimpleQueryPartialObjectHydrationPerformanceBench.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Performance/Hydration/MixedQueryFetchJoinPartialObjectHydrationPerformanceBench.php b/tests/Performance/Hydration/MixedQueryFetchJoinPartialObjectHydrationPerformanceBench.php index 822a019c7d7..ef1d1b4bdca 100644 --- a/tests/Performance/Hydration/MixedQueryFetchJoinPartialObjectHydrationPerformanceBench.php +++ b/tests/Performance/Hydration/MixedQueryFetchJoinPartialObjectHydrationPerformanceBench.php @@ -63,7 +63,7 @@ public function init(): void ]; } - $this->result = ArrayResultFactory::createFromArray($resultSet); + $this->result = ArrayResultFactory::createWrapperResultFromArray($resultSet); $this->hydrator = new ObjectHydrator(EntityManagerFactory::getEntityManager([])); $this->rsm = new ResultSetMapping(); diff --git a/tests/Performance/Hydration/SimpleQueryPartialObjectHydrationPerformanceBench.php b/tests/Performance/Hydration/SimpleQueryPartialObjectHydrationPerformanceBench.php index 72c2ec73d98..a5c78670727 100644 --- a/tests/Performance/Hydration/SimpleQueryPartialObjectHydrationPerformanceBench.php +++ b/tests/Performance/Hydration/SimpleQueryPartialObjectHydrationPerformanceBench.php @@ -54,7 +54,7 @@ public function init(): void ]; } - $this->result = ArrayResultFactory::createFromArray($resultSet); + $this->result = ArrayResultFactory::createWrapperResultFromArray($resultSet); $this->hydrator = new ObjectHydrator(EntityManagerFactory::getEntityManager([])); $this->rsm = new ResultSetMapping(); From b6137c89110952664cdc0b2e4df7f98716ff3f93 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9goire=20Paris?= Date: Thu, 10 Oct 2024 10:40:01 +0200 Subject: [PATCH 08/21] Add guard clause It maybe happen that the SQL COMMIT statement is successful, but then something goes wrong. In that kind of case, you do not want to attempt a rollback. This was implemented in UnitOfWork::commit(), but for some reason not in the similar EntityManager methods. --- src/EntityManager.php | 8 ++++++-- tests/Tests/ORM/EntityManagerTest.php | 28 +++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/src/EntityManager.php b/src/EntityManager.php index da09ed1b77a..b677c79dc13 100644 --- a/src/EntityManager.php +++ b/src/EntityManager.php @@ -259,7 +259,9 @@ public function transactional($func) } finally { if (! $successful) { $this->close(); - $this->conn->rollBack(); + if ($this->conn->isTransactionActive()) { + $this->conn->rollBack(); + } } } } @@ -285,7 +287,9 @@ public function wrapInTransaction(callable $func) } finally { if (! $successful) { $this->close(); - $this->conn->rollBack(); + if ($this->conn->isTransactionActive()) { + $this->conn->rollBack(); + } } } } diff --git a/tests/Tests/ORM/EntityManagerTest.php b/tests/Tests/ORM/EntityManagerTest.php index c9b85f6b4f1..88ff5ed924f 100644 --- a/tests/Tests/ORM/EntityManagerTest.php +++ b/tests/Tests/ORM/EntityManagerTest.php @@ -29,6 +29,7 @@ use Exception; use Generator; use InvalidArgumentException; +use PHPUnit\Framework\Assert; use stdClass; use TypeError; @@ -409,6 +410,33 @@ public function rollBack(): bool } } + /** @dataProvider entityManagerMethodNames */ + public function testItDoesNotAttemptToRollbackIfNoTransactionIsActive(string $methodName): void + { + $entityManager = new EntityManagerMock( + new class extends ConnectionMock { + public function commit(): bool + { + throw new Exception('Commit exception that happens after doing the actual commit'); + } + + public function rollBack(): bool + { + Assert::fail('Should not attempt to rollback if no transaction is active'); + } + + public function isTransactionActive(): bool + { + return false; + } + } + ); + + $this->expectExceptionMessage('Commit exception'); + $entityManager->$methodName(static function (): void { + }); + } + /** @return Generator */ public function entityManagerMethodNames(): Generator { From bac1c17eabef3aad449197a442380f024f01be7e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9goire=20Paris?= Date: Thu, 10 Oct 2024 11:05:30 +0200 Subject: [PATCH 09/21] Remove submodule remnant This should make a warning we have in the CI go away. > fatal: No url found for submodule path 'docs/en/_theme' in .gitmodules --- docs/en/_theme | 1 - 1 file changed, 1 deletion(-) delete mode 160000 docs/en/_theme diff --git a/docs/en/_theme b/docs/en/_theme deleted file mode 160000 index 6f1bc8bead1..00000000000 --- a/docs/en/_theme +++ /dev/null @@ -1 +0,0 @@ -Subproject commit 6f1bc8bead17b8032389659c0b071d00f2c58328 From bea454eefc4bd341e1550b0befd6052db2d175fe Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Thu, 10 Oct 2024 13:54:34 +0200 Subject: [PATCH 10/21] [GH-8471] undeprecate partials completly (#11647) * [GH-8471] Undeprecate all PARTIAL object usage. --- UPGRADE.md | 12 +++++++++--- src/Query/Parser.php | 8 -------- src/UnitOfWork.php | 10 ---------- 3 files changed, 9 insertions(+), 21 deletions(-) diff --git a/UPGRADE.md b/UPGRADE.md index a42be6a4378..a44bbd5a35b 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -8,10 +8,16 @@ change in behavior. Progress on this is tracked at https://github.com/doctrine/orm/issues/11624 . -## PARTIAL DQL syntax is undeprecated for non-object hydration +## PARTIAL DQL syntax is undeprecated -Use of the PARTIAL keyword is not deprecated anymore in DQL when used with a hydrator -that is not creating entities, such as the ArrayHydrator. +Use of the PARTIAL keyword is not deprecated anymore in DQL, because we will be +able to support PARTIAL objects with PHP 8.4 Lazy Objects and +Symfony/VarExporter in a better way. When we decided to remove this feature +these two abstractions did not exist yet. + +WARNING: If you want to upgrade to 3.x and still use PARTIAL keyword in DQL +with array or object hydrators, then you have to directly migrate to ORM 3.3.x or higher. +PARTIAL keyword in DQL is not available in 3.0, 3.1 and 3.2 of ORM. ## Deprecate `\Doctrine\ORM\Query\Parser::setCustomOutputTreeWalker()` diff --git a/src/Query/Parser.php b/src/Query/Parser.php index 2a95297efd9..388cfcc8e47 100644 --- a/src/Query/Parser.php +++ b/src/Query/Parser.php @@ -1850,14 +1850,6 @@ public function JoinAssociationDeclaration() */ public function PartialObjectExpression() { - if ($this->query->getHydrationMode() === Query::HYDRATE_OBJECT) { - Deprecation::trigger( - 'doctrine/orm', - 'https://github.com/doctrine/orm/issues/8471', - 'PARTIAL syntax in DQL is deprecated for object hydration.' - ); - } - $this->match(TokenType::T_PARTIAL); $partialFieldSet = []; diff --git a/src/UnitOfWork.php b/src/UnitOfWork.php index 46c2893dcc3..2969a3e0e0f 100644 --- a/src/UnitOfWork.php +++ b/src/UnitOfWork.php @@ -41,7 +41,6 @@ use Doctrine\ORM\Persisters\Entity\JoinedSubclassPersister; use Doctrine\ORM\Persisters\Entity\SingleTablePersister; use Doctrine\ORM\Proxy\InternalProxy; -use Doctrine\ORM\Query\SqlWalker; use Doctrine\ORM\Utility\IdentifierFlattener; use Doctrine\Persistence\Mapping\RuntimeReflectionService; use Doctrine\Persistence\NotifyPropertyChanged; @@ -2920,15 +2919,6 @@ private function newInstance(ClassMetadata $class) */ public function createEntity($className, array $data, &$hints = []) { - if (isset($hints[SqlWalker::HINT_PARTIAL])) { - Deprecation::trigger( - 'doctrine/orm', - 'https://github.com/doctrine/orm/issues/8471', - 'Partial Objects are deprecated for object hydration (here entity %s)', - $className - ); - } - $class = $this->em->getClassMetadata($className); $id = $this->identifierFlattener->flattenIdentifier($class, $data); From c223b8f635c6a65c4349a8651e7683ce0e93bccb Mon Sep 17 00:00:00 2001 From: eltharin Date: Thu, 10 Oct 2024 14:33:12 +0200 Subject: [PATCH 11/21] Allow named Arguments to be passed to Dto Allow to change argument order or use variadic argument in dto constructor using new named keyword --- .../reference/dql-doctrine-query-language.rst | 65 +++- src/Exception/DuplicateFieldException.php | 17 + src/Exception/NoMatchingPropertyException.php | 17 + src/Internal/Hydration/AbstractHydrator.php | 26 +- src/Query/Parser.php | 74 +++- src/Query/ResultSetMapping.php | 22 -- src/Query/SqlWalker.php | 5 +- src/Query/TokenType.php | 1 + .../Models/CMS/CmsAddressDTONamedArgs.php | 16 + .../Tests/Models/CMS/CmsUserDTONamedArgs.php | 18 + .../Models/CMS/CmsUserDTOVariadicArg.php | 21 ++ .../Tests/ORM/Functional/NewOperatorTest.php | 327 ++++++++++++++++++ .../ORM/Hydration/ResultSetMappingTest.php | 17 - 13 files changed, 550 insertions(+), 76 deletions(-) create mode 100644 src/Exception/DuplicateFieldException.php create mode 100644 src/Exception/NoMatchingPropertyException.php create mode 100644 tests/Tests/Models/CMS/CmsAddressDTONamedArgs.php create mode 100644 tests/Tests/Models/CMS/CmsUserDTONamedArgs.php create mode 100644 tests/Tests/Models/CMS/CmsUserDTOVariadicArg.php diff --git a/docs/en/reference/dql-doctrine-query-language.rst b/docs/en/reference/dql-doctrine-query-language.rst index ab3cb138889..8608049d1fc 100644 --- a/docs/en/reference/dql-doctrine-query-language.rst +++ b/docs/en/reference/dql-doctrine-query-language.rst @@ -591,7 +591,7 @@ You can also nest several DTO : // Bind values to the object properties. } } - + class AddressDTO { public function __construct(string $street, string $city, string $zip) @@ -599,15 +599,72 @@ You can also nest several DTO : // Bind values to the object properties. } } - + .. code-block:: php createQuery('SELECT NEW CustomerDTO(c.name, e.email, NEW AddressDTO(a.street, a.city, a.zip)) FROM Customer c JOIN c.email e JOIN c.address a'); $users = $query->getResult(); // array of CustomerDTO - + Note that you can only pass scalar expressions or other Data Transfer Objects to the constructor. +If you use your data transfer objects for multiple queries, and you would rather not have to +specify arguments that precede the ones you are really interested in, you can use named arguments. + +Consider the following DTO, which uses optional arguments: + +.. code-block:: php + + createQuery('SELECT NEW NAMED CustomerDTO(a.city, c.name) FROM Customer c JOIN c.address a'); + $users = $query->getResult(); // array of CustomerDTO + + // CustomerDTO => {name : 'SMITH', email: null, city: 'London', value: null} + +ORM will also give precedence to column aliases over column names : + +.. code-block:: php + + createQuery('SELECT NEW NAMED CustomerDTO(c.name, CONCAT(a.city, ' ' , a.zip) AS value) FROM Customer c JOIN c.address a'); + $users = $query->getResult(); // array of CustomerDTO + + // CustomerDTO => {name : 'DOE', email: null, city: null, value: 'New York 10011'} + +To define a custom name for a DTO constructor argument, you can either alias the column with the ``AS`` keyword. + +The ``NAMED`` keyword must precede all DTO you want to instantiate : + +.. code-block:: php + + createQuery('SELECT NEW NAMED CustomerDTO(c.name, NEW NAMED AddressDTO(a.street, a.city, a.zip) AS address) FROM Customer c JOIN c.address a'); + $users = $query->getResult(); // array of CustomerDTO + + // CustomerDTO => {name : 'DOE', email: null, city: null, value: 'New York 10011'} + +If two arguments have the same name, a ``DuplicateFieldException`` is thrown. +If a field cannot be matched with a property name, a ``NoMatchingPropertyException`` is thrown. This typically happens when using functions without aliasing them. + Using INDEX BY ~~~~~~~~~~~~~~ @@ -1627,7 +1684,7 @@ Select Expressions PartialObjectExpression ::= "PARTIAL" IdentificationVariable "." PartialFieldSet PartialFieldSet ::= "{" SimpleStateField {"," SimpleStateField}* "}" NewObjectExpression ::= "NEW" AbstractSchemaName "(" NewObjectArg {"," NewObjectArg}* ")" - NewObjectArg ::= ScalarExpression | "(" Subselect ")" | NewObjectExpression + NewObjectArg ::= (ScalarExpression | "(" Subselect ")" | NewObjectExpression) ["AS" AliasResultVariable] Conditional Expressions ~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/src/Exception/DuplicateFieldException.php b/src/Exception/DuplicateFieldException.php new file mode 100644 index 00000000000..ec7cb00593e --- /dev/null +++ b/src/Exception/DuplicateFieldException.php @@ -0,0 +1,17 @@ + []]; + $rowData = ['data' => [], 'newObjects' => []]; foreach ($data as $key => $value) { $cacheKeyInfo = $this->hydrateColumnInfo($key); @@ -282,10 +282,6 @@ protected function gatherRowData(array $data, array &$id, array &$nonemptyCompon $value = $this->buildEnum($value, $cacheKeyInfo['enumType']); } - if (! isset($rowData['newObjects'])) { - $rowData['newObjects'] = []; - } - $rowData['newObjects'][$objIndex]['class'] = $cacheKeyInfo['class']; $rowData['newObjects'][$objIndex]['args'][$argIndex] = $value; break; @@ -341,28 +337,22 @@ protected function gatherRowData(array $data, array &$id, array &$nonemptyCompon } foreach ($this->resultSetMapping()->nestedNewObjectArguments as $objIndex => ['ownerIndex' => $ownerIndex, 'argIndex' => $argIndex]) { - if (! isset($rowData['newObjects'][$objIndex])) { + if (! isset($rowData['newObjects'][$ownerIndex . ':' . $argIndex])) { continue; } - $newObject = $rowData['newObjects'][$objIndex]; - unset($rowData['newObjects'][$objIndex]); + $newObject = $rowData['newObjects'][$ownerIndex . ':' . $argIndex]; + unset($rowData['newObjects'][$ownerIndex . ':' . $argIndex]); - $class = $newObject['class']; - $args = $newObject['args']; - $obj = $class->newInstanceArgs($args); + $obj = $newObject['class']->newInstanceArgs($newObject['args']); $rowData['newObjects'][$ownerIndex]['args'][$argIndex] = $obj; } - if (isset($rowData['newObjects'])) { - foreach ($rowData['newObjects'] as $objIndex => $newObject) { - $class = $newObject['class']; - $args = $newObject['args']; - $obj = $class->newInstanceArgs($args); + foreach ($rowData['newObjects'] as $objIndex => $newObject) { + $obj = $newObject['class']->newInstanceArgs($newObject['args']); - $rowData['newObjects'][$objIndex]['obj'] = $obj; - } + $rowData['newObjects'][$objIndex]['obj'] = $obj; } return $rowData; diff --git a/src/Query/Parser.php b/src/Query/Parser.php index 875783c87d4..febbca2f5e8 100644 --- a/src/Query/Parser.php +++ b/src/Query/Parser.php @@ -6,6 +6,8 @@ use Doctrine\Common\Lexer\Token; use Doctrine\ORM\EntityManagerInterface; +use Doctrine\ORM\Exception\DuplicateFieldException; +use Doctrine\ORM\Exception\NoMatchingPropertyException; use Doctrine\ORM\Internal\Hydration\HydrationException; use Doctrine\ORM\Mapping\AssociationMapping; use Doctrine\ORM\Mapping\ClassMetadata; @@ -15,6 +17,7 @@ use ReflectionClass; use function array_intersect; +use function array_key_exists; use function array_search; use function assert; use function class_exists; @@ -30,6 +33,7 @@ use function strrpos; use function strtolower; use function substr; +use function trim; /** * An LL(*) recursive-descent parser for the context-free grammar of the Doctrine Query Language. @@ -1734,20 +1738,26 @@ public function PartialObjectExpression(): AST\PartialObjectExpression */ public function NewObjectExpression(): AST\NewObjectExpression { - $args = []; + $useNamedArguments = false; + $args = []; + $argFieldAlias = []; $this->match(TokenType::T_NEW); + if ($this->lexer->isNextToken(TokenType::T_NAMED)) { + $this->match(TokenType::T_NAMED); + $useNamedArguments = true; + } + $className = $this->AbstractSchemaName(); // note that this is not yet validated $token = $this->lexer->token; $this->match(TokenType::T_OPEN_PARENTHESIS); - $args[] = $this->NewObjectArg(); + $this->addArgument($args, $useNamedArguments); while ($this->lexer->isNextToken(TokenType::T_COMMA)) { $this->match(TokenType::T_COMMA); - - $args[] = $this->NewObjectArg(); + $this->addArgument($args, $useNamedArguments); } $this->match(TokenType::T_CLOSE_PARENTHESIS); @@ -1764,29 +1774,71 @@ public function NewObjectExpression(): AST\NewObjectExpression return $expression; } + /** @param array $args */ + public function addArgument(array &$args, bool $useNamedArguments): void + { + $fieldAlias = null; + + if ($useNamedArguments) { + $startToken = $this->lexer->lookahead?->position ?? 0; + + $newArg = $this->NewObjectArg($fieldAlias); + + $key = $fieldAlias ?? $newArg->field ?? null; + + if ($key === null) { + throw NoMatchingPropertyException::create(trim(substr( + ($this->query->getDQL() ?? ''), + $startToken, + ($this->lexer->lookahead->position ?? 0) - $startToken, + ))); + } + + if (array_key_exists($key, $args)) { + throw DuplicateFieldException::create($key, trim(substr( + ($this->query->getDQL() ?? ''), + $startToken, + ($this->lexer->lookahead->position ?? 0) - $startToken, + ))); + } + + $args[$key] = $newArg; + } else { + $args[] = $this->NewObjectArg($fieldAlias); + } + } + /** - * NewObjectArg ::= ScalarExpression | "(" Subselect ")" | NewObjectExpression + * NewObjectArg ::= (ScalarExpression | "(" Subselect ")" | NewObjectExpression) ["AS" AliasResultVariable] */ - public function NewObjectArg(): mixed + public function NewObjectArg(string|null &$fieldAlias = null): mixed { + $fieldAlias = null; + assert($this->lexer->lookahead !== null); $token = $this->lexer->lookahead; $peek = $this->lexer->glimpse(); assert($peek !== null); + + $expression = null; + if ($token->type === TokenType::T_OPEN_PARENTHESIS && $peek->type === TokenType::T_SELECT) { $this->match(TokenType::T_OPEN_PARENTHESIS); $expression = $this->Subselect(); $this->match(TokenType::T_CLOSE_PARENTHESIS); - - return $expression; + } elseif ($token->type === TokenType::T_NEW) { + $expression = $this->NewObjectExpression(); + } else { + $expression = $this->ScalarExpression(); } - if ($token->type === TokenType::T_NEW) { - return $this->NewObjectExpression(); + if ($this->lexer->isNextToken(TokenType::T_AS)) { + $this->match(TokenType::T_AS); + $fieldAlias = $this->AliasIdentificationVariable(); } - return $this->ScalarExpression(); + return $expression; } /** diff --git a/src/Query/ResultSetMapping.php b/src/Query/ResultSetMapping.php index c95b089a73b..38ed1bf6416 100644 --- a/src/Query/ResultSetMapping.php +++ b/src/Query/ResultSetMapping.php @@ -4,7 +4,6 @@ namespace Doctrine\ORM\Query; -use function array_merge; use function count; /** @@ -552,25 +551,4 @@ public function addMetaResult( return $this; } - - public function addNewObjectAsArgument(string|int $alias, string|int $objOwner, int $objOwnerIdx): static - { - $owner = [ - 'ownerIndex' => $objOwner, - 'argIndex' => $objOwnerIdx, - ]; - - if (! isset($this->nestedNewObjectArguments[$owner['ownerIndex']])) { - $this->nestedNewObjectArguments[$alias] = $owner; - - return $this; - } - - $this->nestedNewObjectArguments = array_merge( - [$alias => $owner], - $this->nestedNewObjectArguments, - ); - - return $this; - } } diff --git a/src/Query/SqlWalker.php b/src/Query/SqlWalker.php index 46296e719e7..04d7d953ab4 100644 --- a/src/Query/SqlWalker.php +++ b/src/Query/SqlWalker.php @@ -1510,6 +1510,7 @@ public function walkNewObject(AST\NewObjectExpression $newObjectExpression, stri $this->newObjectStack[] = [$objIndex, $argIndex]; $sqlSelectExpressions[] = $e->dispatch($this); array_pop($this->newObjectStack); + $this->rsm->nestedNewObjectArguments[$columnAlias] = ['ownerIndex' => $objIndex, 'argIndex' => $argIndex]; break; case $e instanceof AST\Subselect: @@ -1563,10 +1564,6 @@ public function walkNewObject(AST\NewObjectExpression $newObjectExpression, stri 'objIndex' => $objIndex, 'argIndex' => $argIndex, ]; - - if ($objOwner !== null && $objOwnerIdx !== null) { - $this->rsm->addNewObjectAsArgument($objIndex, $objOwner, $objOwnerIdx); - } } return implode(', ', $sqlSelectExpressions); diff --git a/src/Query/TokenType.php b/src/Query/TokenType.php index bf1c351c2a6..47cc7912711 100644 --- a/src/Query/TokenType.php +++ b/src/Query/TokenType.php @@ -89,4 +89,5 @@ enum TokenType: int case T_WHEN = 254; case T_WHERE = 255; case T_WITH = 256; + case T_NAMED = 257; } diff --git a/tests/Tests/Models/CMS/CmsAddressDTONamedArgs.php b/tests/Tests/Models/CMS/CmsAddressDTONamedArgs.php new file mode 100644 index 00000000000..547c7fb0c31 --- /dev/null +++ b/tests/Tests/Models/CMS/CmsAddressDTONamedArgs.php @@ -0,0 +1,16 @@ +name = $args['name'] ?? null; + $this->email = $args['email'] ?? null; + $this->phonenumbers = $args['phonenumbers'] ?? null; + $this->address = $args['address'] ?? null; + } +} diff --git a/tests/Tests/ORM/Functional/NewOperatorTest.php b/tests/Tests/ORM/Functional/NewOperatorTest.php index 4497af517bf..5a742c1b3a9 100644 --- a/tests/Tests/ORM/Functional/NewOperatorTest.php +++ b/tests/Tests/ORM/Functional/NewOperatorTest.php @@ -4,19 +4,25 @@ namespace Doctrine\Tests\ORM\Functional; +use Doctrine\ORM\Exception\DuplicateFieldException; +use Doctrine\ORM\Exception\NoMatchingPropertyException; use Doctrine\ORM\Query; use Doctrine\ORM\Query\QueryException; use Doctrine\Tests\Models\CMS\CmsAddress; use Doctrine\Tests\Models\CMS\CmsAddressDTO; +use Doctrine\Tests\Models\CMS\CmsAddressDTONamedArgs; use Doctrine\Tests\Models\CMS\CmsEmail; use Doctrine\Tests\Models\CMS\CmsPhonenumber; use Doctrine\Tests\Models\CMS\CmsUser; use Doctrine\Tests\Models\CMS\CmsUserDTO; +use Doctrine\Tests\Models\CMS\CmsUserDTONamedArgs; +use Doctrine\Tests\Models\CMS\CmsUserDTOVariadicArg; use Doctrine\Tests\OrmFunctionalTestCase; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Group; use function count; +use function sprintf; #[Group('DDC-1574')] class NewOperatorTest extends OrmFunctionalTestCase @@ -1080,6 +1086,327 @@ public function testShouldSupportNestedNewOperators(): void self::assertSame($this->fixtures[1]->username, $result[1]['cmsUserUsername']); self::assertSame($this->fixtures[2]->username, $result[2]['cmsUserUsername']); } + + public function testNamedArguments(): void + { + $dql = <<<'SQL' + SELECT + new named CmsUserDTONamedArgs( + e.email, + u.name, + CONCAT(a.country, ' ', a.city, ' ', a.zip) AS address + ) 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 + SQL; + + $query = $this->getEntityManager()->createQuery($dql); + $result = $query->getResult(); + + self::assertCount(3, $result); + + self::assertInstanceOf(CmsUserDTONamedArgs::class, $result[0]['user']); + self::assertInstanceOf(CmsUserDTONamedArgs::class, $result[1]['user']); + self::assertInstanceOf(CmsUserDTONamedArgs::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::assertSame(sprintf( + '%s %s %s', + $this->fixtures[0]->address->country, + $this->fixtures[0]->address->city, + $this->fixtures[0]->address->zip, + ), $result[0]['user']->address); + self::assertSame( + sprintf( + '%s %s %s', + $this->fixtures[1]->address->country, + $this->fixtures[1]->address->city, + $this->fixtures[1]->address->zip, + ), + $result[1]['user']->address, + ); + self::assertSame( + sprintf( + '%s %s %s', + $this->fixtures[2]->address->country, + $this->fixtures[2]->address->city, + $this->fixtures[2]->address->zip, + ), + $result[2]['user']->address, + ); + + 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' + SELECT + new named CmsUserDTOVariadicArg( + CONCAT(a.country, ' ', a.city, ' ', a.zip) AS address, + e.email, + u.name + ) 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 + SQL; + + $query = $this->getEntityManager()->createQuery($dql); + $result = $query->getResult(); + + self::assertCount(3, $result); + + self::assertInstanceOf(CmsUserDTOVariadicArg::class, $result[0]['user']); + self::assertInstanceOf(CmsUserDTOVariadicArg::class, $result[1]['user']); + self::assertInstanceOf(CmsUserDTOVariadicArg::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::assertSame( + sprintf( + '%s %s %s', + $this->fixtures[0]->address->country, + $this->fixtures[0]->address->city, + $this->fixtures[0]->address->zip, + ), + $result[0]['user']->address, + ); + self::assertSame( + sprintf( + '%s %s %s', + $this->fixtures[1]->address->country, + $this->fixtures[1]->address->city, + $this->fixtures[1]->address->zip, + ), + $result[1]['user']->address, + ); + self::assertSame( + sprintf( + '%s %s %s', + $this->fixtures[2]->address->country, + $this->fixtures[2]->address->city, + $this->fixtures[2]->address->zip, + ), + $result[2]['user']->address, + ); + + 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 testShouldSupportNestedNewOperatorsAndNamedArguments(): void + { + $dql = ' + SELECT + new named CmsUserDTONamedArgs( + e.email, + u.name as name, + new CmsAddressDTO( + a.country, + a.city, + a.zip + ) as addressDto + ) 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(CmsUserDTONamedArgs::class, $result[0]['user']); + self::assertInstanceOf(CmsUserDTONamedArgs::class, $result[1]['user']); + self::assertInstanceOf(CmsUserDTONamedArgs::class, $result[2]['user']); + + self::assertNull($result[0]['user']->address); + self::assertNull($result[1]['user']->address); + self::assertNull($result[2]['user']->address); + + self::assertInstanceOf(CmsAddressDTO::class, $result[0]['user']->addressDto); + self::assertInstanceOf(CmsAddressDTO::class, $result[1]['user']->addressDto); + self::assertInstanceOf(CmsAddressDTO::class, $result[2]['user']->addressDto); + + 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']->addressDto->city); + self::assertSame($this->fixtures[1]->address->city, $result[1]['user']->addressDto->city); + self::assertSame($this->fixtures[2]->address->city, $result[2]['user']->addressDto->city); + + self::assertSame($this->fixtures[0]->address->country, $result[0]['user']->addressDto->country); + self::assertSame($this->fixtures[1]->address->country, $result[1]['user']->addressDto->country); + self::assertSame($this->fixtures[2]->address->country, $result[2]['user']->addressDto->country); + + 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 testShouldSupportNestedNamedArguments(): void + { + $dql = ' + SELECT + new named CmsUserDTONamedArgs( + e.email, + u.name as name, + new named CmsAddressDTONamedArgs( + a.zip, + a.city, + a.country + ) as addressDtoNamedArgs + ) 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(CmsUserDTONamedArgs::class, $result[0]['user']); + self::assertInstanceOf(CmsUserDTONamedArgs::class, $result[1]['user']); + self::assertInstanceOf(CmsUserDTONamedArgs::class, $result[2]['user']); + + self::assertNull($result[0]['user']->address); + self::assertNull($result[1]['user']->address); + self::assertNull($result[2]['user']->address); + + self::assertNull($result[0]['user']->addressDto); + self::assertNull($result[1]['user']->addressDto); + self::assertNull($result[2]['user']->addressDto); + + self::assertInstanceOf(CmsAddressDTONamedArgs::class, $result[0]['user']->addressDtoNamedArgs); + self::assertInstanceOf(CmsAddressDTONamedArgs::class, $result[1]['user']->addressDtoNamedArgs); + self::assertInstanceOf(CmsAddressDTONamedArgs::class, $result[2]['user']->addressDtoNamedArgs); + + 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']->addressDtoNamedArgs->city); + self::assertSame($this->fixtures[1]->address->city, $result[1]['user']->addressDtoNamedArgs->city); + self::assertSame($this->fixtures[2]->address->city, $result[2]['user']->addressDtoNamedArgs->city); + + self::assertSame($this->fixtures[0]->address->country, $result[0]['user']->addressDtoNamedArgs->country); + self::assertSame($this->fixtures[1]->address->country, $result[1]['user']->addressDtoNamedArgs->country); + self::assertSame($this->fixtures[2]->address->country, $result[2]['user']->addressDtoNamedArgs->country); + + 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 testExceptionIfTwoAliases(): void + { + $dql = ' + SELECT + new named Doctrine\Tests\Models\CMS\CmsUserDTO( + u.name, + u.username AS name + ) + FROM + Doctrine\Tests\Models\CMS\CmsUser u'; + + $this->expectException(DuplicateFieldException::class); + $this->expectExceptionMessage('Name "name" for "u.username AS name" already in use.'); + + $query = $this->_em->createQuery($dql); + $result = $query->getResult(); + } + + public function testExceptionIfFunctionHasNoAlias(): void + { + $dql = " + SELECT + new named Doctrine\Tests\Models\CMS\CmsUserDTO( + u.name, + CASE WHEN (e.email = 'email@test1.com') THEN 'TEST1' ELSE 'OTHER_TEST' END + ) + FROM + Doctrine\Tests\Models\CMS\CmsUser u + JOIN + u.email e"; + + $this->expectException(NoMatchingPropertyException::class); + $this->expectExceptionMessage('Column name "CASE WHEN (e.email = \'email@test1.com\') THEN \'TEST1\' ELSE \'OTHER_TEST\' END" does not match any property name. Consider aliasing it to the name of an existing property.'); + + $query = $this->_em->createQuery($dql); + $result = $query->getResult(); + } } class ClassWithTooMuchArgs diff --git a/tests/Tests/ORM/Hydration/ResultSetMappingTest.php b/tests/Tests/ORM/Hydration/ResultSetMappingTest.php index 14b9205abfe..0c20eab0866 100644 --- a/tests/Tests/ORM/Hydration/ResultSetMappingTest.php +++ b/tests/Tests/ORM/Hydration/ResultSetMappingTest.php @@ -102,21 +102,4 @@ public function testIndexByMetadataColumn(): void self::assertTrue($this->_rsm->hasIndexBy('lu')); } - - public function testNewObjectNestedArgumentsDeepestLeavesShouldComeFirst(): void - { - $this->_rsm->addNewObjectAsArgument('objALevel2', 'objALevel1', 0); - $this->_rsm->addNewObjectAsArgument('objALevel3', 'objALevel2', 1); - $this->_rsm->addNewObjectAsArgument('objBLevel3', 'objBLevel2', 0); - $this->_rsm->addNewObjectAsArgument('objBLevel2', 'objBLevel1', 1); - - $expectedArgumentMapping = [ - 'objALevel3' => ['ownerIndex' => 'objALevel2', 'argIndex' => 1], - 'objALevel2' => ['ownerIndex' => 'objALevel1', 'argIndex' => 0], - 'objBLevel3' => ['ownerIndex' => 'objBLevel2', 'argIndex' => 0], - 'objBLevel2' => ['ownerIndex' => 'objBLevel1', 'argIndex' => 1], - ]; - - self::assertSame($expectedArgumentMapping, $this->_rsm->nestedNewObjectArguments); - } } From 39d2136f466187a8d1169c1fc307ebd8244a0e1a Mon Sep 17 00:00:00 2001 From: Matthias Pigulla Date: Thu, 10 Oct 2024 15:15:08 +0200 Subject: [PATCH 12/21] Fix different first/max result values taking up query cache space (#11188) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Add a test covering the #11112 issue * Add new OutputWalker and SqlFinalizer interfaces * Add a SingleSelectSqlFinalizer that can take care of adding offset/limit as well as locking mode statements to a given SQL query. Add a FinalizedSelectExecutor that executes given, finalized SQL statements. * In SqlWalker, split SQL query generation into the two parts that shall happen before and after the finalization phase. Move the part that generates "pre-finalization" SQL into a dedicated method. Use a side channel in SingleSelectSqlFinalizer to access the "finalization" logic and avoid duplication. * Fix CS violations * Skip the GH11112 test while applying refactorings * Avoid a Psalm complaint due to invalid (?) docblock syntax * Establish alternate code path - queries can obtain the sql executor through the finalizer, parser knows about output walkers yielding finalizers * Remove a possibly premature comment * Re-enable the #11112 test * Fix CS * Make RootTypeWalker inherit from SqlOutputWalker so it becomes finalizer-aware * Update QueryCacheTest, since first/max results no longer need extra cache entries * Fix ParserResultSerializationTest by forcing the parser to produce a ParserResult of the old kind (with the executor already constructed) * Fix WhereInWalkerTest * Update lib/Doctrine/ORM/Query/Exec/PreparedExecutorFinalizer.php Co-authored-by: Grégoire Paris * Fix tests * Fix a Psalm complaint * Fix a test * Fix CS * Make the NullSqlWalker an instance of SqlOutputWalker * Avoid multiple cache entries caused by LimitSubqueryOutputWalker * Fix Psalm complaints * Fix static analysis complaints * Remove experimental code that I committed accidentally * Remove unnecessary baseline entry * Make AddUnknownQueryComponentWalker subclass SqlOutputWalker That way, we have no remaining classes in the codebase subclassing SqlWalker but not SqlOutputWalker * Use more expressive exception classes * Add a deprecation message * Move SqlExecutor creation to ParserResult, to minimize public methods available on it * Avoid keeping the SqlExecutor in the Query, since it must be generated just in time (e. g. in case Query parameters change) * Address PHPStan complaints * Fix tests * Small refactorings * Add an upgrade notice * Small refactorings * Update the Psalm baseline * Add a missing namespace import * Update Psalm baseline * Fix CS * Fix Psalm baseline --------- Co-authored-by: Grégoire Paris --- UPGRADE.md | 9 ++ psalm-baseline.xml | 14 +-- src/Query.php | 34 +++++++- src/Query/Exec/FinalizedSelectExecutor.php | 33 +++++++ src/Query/Exec/PreparedExecutorFinalizer.php | 28 ++++++ src/Query/Exec/SingleSelectSqlFinalizer.php | 64 ++++++++++++++ src/Query/Exec/SqlFinalizer.php | 26 ++++++ src/Query/OutputWalker.php | 28 ++++++ src/Query/Parser.php | 22 ++++- src/Query/ParserResult.php | 37 +++++++- src/Query/SqlOutputWalker.php | 29 +++++++ src/Query/SqlWalker.php | 85 ++++++++++--------- src/Tools/Pagination/CountOutputWalker.php | 11 +-- .../Pagination/LimitSubqueryOutputWalker.php | 71 ++++++++++++---- src/Tools/Pagination/RootTypeWalker.php | 16 +++- tests/Tests/Mocks/NullSqlWalker.php | 20 +++-- .../ParserResultSerializationTest.php | 38 +++++++-- tests/Tests/ORM/Functional/QueryCacheTest.php | 8 +- .../ORM/Functional/Ticket/GH11112Test.php | 79 +++++++++++++++++ .../Tests/ORM/Query/CustomTreeWalkersTest.php | 9 +- .../LimitSubqueryOutputWalkerTest.php | 23 +++++ .../ORM/Tools/Pagination/PaginatorTest.php | 6 +- .../Tools/Pagination/WhereInWalkerTest.php | 5 +- 23 files changed, 591 insertions(+), 104 deletions(-) create mode 100644 src/Query/Exec/FinalizedSelectExecutor.php create mode 100644 src/Query/Exec/PreparedExecutorFinalizer.php create mode 100644 src/Query/Exec/SingleSelectSqlFinalizer.php create mode 100644 src/Query/Exec/SqlFinalizer.php create mode 100644 src/Query/OutputWalker.php create mode 100644 src/Query/SqlOutputWalker.php create mode 100644 tests/Tests/ORM/Functional/Ticket/GH11112Test.php diff --git a/UPGRADE.md b/UPGRADE.md index a44bbd5a35b..9de8af04d38 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -1,5 +1,14 @@ # Upgrade to 2.20 +## Add `Doctrine\ORM\Query\OutputWalker` interface, deprecate `Doctrine\ORM\Query\SqlWalker::getExecutor()` + +Output walkers should implement the new `\Doctrine\ORM\Query\OutputWalker` interface and create +`Doctrine\ORM\Query\Exec\SqlFinalizer` instances instead of `Doctrine\ORM\Query\Exec\AbstractSqlExecutor`s. +The output walker must not base its workings on the query `firstResult`/`maxResult` values, so that the +`SqlFinalizer` can be kept in the query cache and used regardless of the actual `firstResult`/`maxResult` values. +Any operation dependent on `firstResult`/`maxResult` should take place within the `SqlFinalizer::createExecutor()` +method. Details can be found at https://github.com/doctrine/orm/pull/11188. + ## Explictly forbid property hooks Property hooks are not supported yet by Doctrine ORM. Until support is added, diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 2d96702abe2..28ed8aab049 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -1868,6 +1868,12 @@ _sqlStatements = &$this->sqlStatements]]> + + + + + + @@ -1996,6 +2002,9 @@ + + + @@ -2057,11 +2066,6 @@ - - - - - parameters)]]> diff --git a/src/Query.php b/src/Query.php index 2654e04b0cc..48464e52631 100644 --- a/src/Query.php +++ b/src/Query.php @@ -18,6 +18,8 @@ use Doctrine\ORM\Query\AST\SelectStatement; use Doctrine\ORM\Query\AST\UpdateStatement; use Doctrine\ORM\Query\Exec\AbstractSqlExecutor; +use Doctrine\ORM\Query\Exec\SqlFinalizer; +use Doctrine\ORM\Query\OutputWalker; use Doctrine\ORM\Query\Parameter; use Doctrine\ORM\Query\ParameterTypeInferer; use Doctrine\ORM\Query\Parser; @@ -33,6 +35,7 @@ use function count; use function get_debug_type; use function in_array; +use function is_a; use function is_int; use function ksort; use function md5; @@ -196,7 +199,7 @@ class Query extends AbstractQuery */ public function getSQL() { - return $this->parse()->getSqlExecutor()->getSqlStatements(); + return $this->getSqlExecutor()->getSqlStatements(); } /** @@ -285,7 +288,7 @@ private function parse(): ParserResult */ protected function _doExecute() { - $executor = $this->parse()->getSqlExecutor(); + $executor = $this->getSqlExecutor(); if ($this->_queryCacheProfile) { $executor->setQueryCacheProfile($this->_queryCacheProfile); @@ -813,11 +816,31 @@ protected function getQueryCacheId(): string { ksort($this->_hints); + if (! $this->hasHint(self::HINT_CUSTOM_OUTPUT_WALKER)) { + // Assume Parser will create the SqlOutputWalker; save is_a call, which might trigger a class load + $firstAndMaxResult = ''; + } else { + $outputWalkerClass = $this->getHint(self::HINT_CUSTOM_OUTPUT_WALKER); + if (is_a($outputWalkerClass, OutputWalker::class, true)) { + $firstAndMaxResult = ''; + } else { + Deprecation::trigger( + 'doctrine/orm', + 'https://github.com/doctrine/orm/pull/11188/', + 'Your output walker class %s should implement %s in order to provide a %s. This also means the output walker should not use the query firstResult/maxResult values, which should be read from the query by the SqlFinalizer only.', + $outputWalkerClass, + OutputWalker::class, + SqlFinalizer::class + ); + $firstAndMaxResult = '&firstResult=' . $this->firstResult . '&maxResult=' . $this->maxResults; + } + } + return md5( $this->getDQL() . serialize($this->_hints) . '&platform=' . get_debug_type($this->getEntityManager()->getConnection()->getDatabasePlatform()) . ($this->_em->hasFilters() ? $this->_em->getFilters()->getHash() : '') . - '&firstResult=' . $this->firstResult . '&maxResult=' . $this->maxResults . + $firstAndMaxResult . '&hydrationMode=' . $this->_hydrationMode . '&types=' . serialize($this->parsedTypes) . 'DOCTRINE_QUERY_CACHE_SALT' ); } @@ -836,4 +859,9 @@ public function __clone() $this->state = self::STATE_DIRTY; } + + private function getSqlExecutor(): AbstractSqlExecutor + { + return $this->parse()->prepareSqlExecutor($this); + } } diff --git a/src/Query/Exec/FinalizedSelectExecutor.php b/src/Query/Exec/FinalizedSelectExecutor.php new file mode 100644 index 00000000000..a97e6f428c5 --- /dev/null +++ b/src/Query/Exec/FinalizedSelectExecutor.php @@ -0,0 +1,33 @@ +sqlStatements = $sql; + } + + /** + * @param list|array $params + * @param array|array $types + */ + public function execute(Connection $conn, array $params, array $types): Result + { + return $conn->executeQuery($this->getSqlStatements(), $params, $types, $this->queryCacheProfile); + } +} diff --git a/src/Query/Exec/PreparedExecutorFinalizer.php b/src/Query/Exec/PreparedExecutorFinalizer.php new file mode 100644 index 00000000000..b89b124e352 --- /dev/null +++ b/src/Query/Exec/PreparedExecutorFinalizer.php @@ -0,0 +1,28 @@ +executor = $exeutor; + } + + public function createExecutor(Query $query): AbstractSqlExecutor + { + return $this->executor; + } +} diff --git a/src/Query/Exec/SingleSelectSqlFinalizer.php b/src/Query/Exec/SingleSelectSqlFinalizer.php new file mode 100644 index 00000000000..5180be9e7b2 --- /dev/null +++ b/src/Query/Exec/SingleSelectSqlFinalizer.php @@ -0,0 +1,64 @@ +sql = $sql; + } + + /** + * This method exists temporarily to support old SqlWalker interfaces. + * + * @internal + * + * @psalm-internal Doctrine\ORM + */ + public function finalizeSql(Query $query): string + { + $platform = $query->getEntityManager()->getConnection()->getDatabasePlatform(); + + $sql = $platform->modifyLimitQuery($this->sql, $query->getMaxResults(), $query->getFirstResult()); + + $lockMode = $query->getHint(Query::HINT_LOCK_MODE) ?: LockMode::NONE; + + if ($lockMode !== LockMode::NONE && $lockMode !== LockMode::OPTIMISTIC && $lockMode !== LockMode::PESSIMISTIC_READ && $lockMode !== LockMode::PESSIMISTIC_WRITE) { + throw QueryException::invalidLockMode(); + } + + if ($lockMode === LockMode::PESSIMISTIC_READ) { + $sql .= ' ' . $this->getReadLockSQL($platform); + } elseif ($lockMode === LockMode::PESSIMISTIC_WRITE) { + $sql .= ' ' . $this->getWriteLockSQL($platform); + } + + return $sql; + } + + /** @return FinalizedSelectExecutor */ + public function createExecutor(Query $query): AbstractSqlExecutor + { + return new FinalizedSelectExecutor($this->finalizeSql($query)); + } +} diff --git a/src/Query/Exec/SqlFinalizer.php b/src/Query/Exec/SqlFinalizer.php new file mode 100644 index 00000000000..cddad84e8a3 --- /dev/null +++ b/src/Query/Exec/SqlFinalizer.php @@ -0,0 +1,26 @@ +queryComponents = $treeWalkerChain->getQueryComponents(); } - $outputWalkerClass = $this->customOutputWalker ?: SqlWalker::class; + $outputWalkerClass = $this->customOutputWalker ?: SqlOutputWalker::class; $outputWalker = new $outputWalkerClass($this->query, $this->parserResult, $this->queryComponents); - // Assign an SQL executor to the parser result - $this->parserResult->setSqlExecutor($outputWalker->getExecutor($AST)); + if ($outputWalker instanceof OutputWalker) { + $finalizer = $outputWalker->getFinalizer($AST); + $this->parserResult->setSqlFinalizer($finalizer); + } else { + Deprecation::trigger( + 'doctrine/orm', + 'https://github.com/doctrine/orm/pull/11188/', + 'Your output walker class %s should implement %s in order to provide a %s. This also means the output walker should not use the query firstResult/maxResult values, which should be read from the query by the SqlFinalizer only.', + $outputWalkerClass, + OutputWalker::class, + SqlFinalizer::class + ); + // @phpstan-ignore method.deprecated + $executor = $outputWalker->getExecutor($AST); + // @phpstan-ignore method.deprecated + $this->parserResult->setSqlExecutor($executor); + } return $this->parserResult; } diff --git a/src/Query/ParserResult.php b/src/Query/ParserResult.php index e5d36c9bbe3..86a4e4d3b64 100644 --- a/src/Query/ParserResult.php +++ b/src/Query/ParserResult.php @@ -4,7 +4,10 @@ namespace Doctrine\ORM\Query; +use Doctrine\ORM\Query; use Doctrine\ORM\Query\Exec\AbstractSqlExecutor; +use Doctrine\ORM\Query\Exec\SqlFinalizer; +use LogicException; use function sprintf; @@ -20,15 +23,23 @@ class ParserResult 'sqlExecutor' => '_sqlExecutor', 'resultSetMapping' => '_resultSetMapping', 'parameterMappings' => '_parameterMappings', + 'sqlFinalizer' => 'sqlFinalizer', ]; /** * The SQL executor used for executing the SQL. * - * @var AbstractSqlExecutor + * @var ?AbstractSqlExecutor */ private $sqlExecutor; + /** + * The SQL executor used for executing the SQL. + * + * @var ?SqlFinalizer + */ + private $sqlFinalizer; + /** * The ResultSetMapping that describes how to map the SQL result set. * @@ -75,6 +86,8 @@ public function setResultSetMapping(ResultSetMapping $rsm) /** * Sets the SQL executor that should be used for this ParserResult. * + * @deprecated + * * @param AbstractSqlExecutor $executor * * @return void @@ -87,13 +100,33 @@ public function setSqlExecutor($executor) /** * Gets the SQL executor used by this ParserResult. * - * @return AbstractSqlExecutor + * @deprecated + * + * @return ?AbstractSqlExecutor */ public function getSqlExecutor() { return $this->sqlExecutor; } + public function setSqlFinalizer(SqlFinalizer $finalizer): void + { + $this->sqlFinalizer = $finalizer; + } + + public function prepareSqlExecutor(Query $query): AbstractSqlExecutor + { + if ($this->sqlFinalizer !== null) { + return $this->sqlFinalizer->createExecutor($query); + } + + if ($this->sqlExecutor !== null) { + return $this->sqlExecutor; + } + + throw new LogicException('This ParserResult lacks both the SqlFinalizer as well as the (legacy) SqlExecutor'); + } + /** * Adds a DQL to SQL parameter mapping. One DQL parameter name/position can map to * several SQL parameter positions. diff --git a/src/Query/SqlOutputWalker.php b/src/Query/SqlOutputWalker.php new file mode 100644 index 00000000000..e737e1c9c53 --- /dev/null +++ b/src/Query/SqlOutputWalker.php @@ -0,0 +1,29 @@ +createSqlForFinalizer($AST)); + + case $AST instanceof AST\UpdateStatement: + return new PreparedExecutorFinalizer($this->createUpdateStatementExecutor($AST)); + + case $AST instanceof AST\DeleteStatement: + return new PreparedExecutorFinalizer($this->createDeleteStatementExecutor($AST)); + } + + throw new LogicException('Unexpected AST node type'); + } +} diff --git a/src/Query/SqlWalker.php b/src/Query/SqlWalker.php index 4c25fb63a68..e4e2de5c5c1 100644 --- a/src/Query/SqlWalker.php +++ b/src/Query/SqlWalker.php @@ -16,7 +16,6 @@ use Doctrine\ORM\OptimisticLockException; use Doctrine\ORM\Query; use Doctrine\ORM\Utility\HierarchyDiscriminatorResolver; -use Doctrine\ORM\Utility\LockSqlHelper; use Doctrine\ORM\Utility\PersisterHelper; use InvalidArgumentException; use LogicException; @@ -49,8 +48,6 @@ */ class SqlWalker implements TreeWalker { - use LockSqlHelper; - public const HINT_DISTINCT = 'doctrine.distinct'; /** @@ -278,34 +275,48 @@ public function setQueryComponent($dqlAlias, array $queryComponent) /** * Gets an executor that can be used to execute the result of this walker. * + * @deprecated Output walkers should no longer create the executor directly, but instead provide + * a SqlFinalizer by implementing the `OutputWalker` interface. Thus, this method is + * no longer needed and will be removed in 4.0. + * * @param AST\DeleteStatement|AST\UpdateStatement|AST\SelectStatement $AST * * @return Exec\AbstractSqlExecutor - * - * @not-deprecated */ public function getExecutor($AST) { switch (true) { case $AST instanceof AST\DeleteStatement: - $primaryClass = $this->em->getClassMetadata($AST->deleteClause->abstractSchemaName); - - return $primaryClass->isInheritanceTypeJoined() - ? new Exec\MultiTableDeleteExecutor($AST, $this) - : new Exec\SingleTableDeleteUpdateExecutor($AST, $this); + return $this->createDeleteStatementExecutor($AST); case $AST instanceof AST\UpdateStatement: - $primaryClass = $this->em->getClassMetadata($AST->updateClause->abstractSchemaName); - - return $primaryClass->isInheritanceTypeJoined() - ? new Exec\MultiTableUpdateExecutor($AST, $this) - : new Exec\SingleTableDeleteUpdateExecutor($AST, $this); + return $this->createUpdateStatementExecutor($AST); default: return new Exec\SingleSelectExecutor($AST, $this); } } + /** @psalm-internal Doctrine\ORM */ + protected function createUpdateStatementExecutor(AST\UpdateStatement $AST): Exec\AbstractSqlExecutor + { + $primaryClass = $this->em->getClassMetadata($AST->updateClause->abstractSchemaName); + + return $primaryClass->isInheritanceTypeJoined() + ? new Exec\MultiTableUpdateExecutor($AST, $this) + : new Exec\SingleTableDeleteUpdateExecutor($AST, $this); + } + + /** @psalm-internal Doctrine\ORM */ + protected function createDeleteStatementExecutor(AST\DeleteStatement $AST): Exec\AbstractSqlExecutor + { + $primaryClass = $this->em->getClassMetadata($AST->deleteClause->abstractSchemaName); + + return $primaryClass->isInheritanceTypeJoined() + ? new Exec\MultiTableDeleteExecutor($AST, $this) + : new Exec\SingleTableDeleteUpdateExecutor($AST, $this); + } + /** * Generates a unique, short SQL table alias. * @@ -561,10 +572,15 @@ private function generateFilterConditionSQL( */ public function walkSelectStatement(AST\SelectStatement $AST) { - $limit = $this->query->getMaxResults(); - $offset = $this->query->getFirstResult(); - $lockMode = $this->query->getHint(Query::HINT_LOCK_MODE) ?: LockMode::NONE; - $sql = $this->walkSelectClause($AST->selectClause) + $sql = $this->createSqlForFinalizer($AST); + $finalizer = new Exec\SingleSelectSqlFinalizer($sql); + + return $finalizer->finalizeSql($this->query); + } + + protected function createSqlForFinalizer(AST\SelectStatement $AST): string + { + $sql = $this->walkSelectClause($AST->selectClause) . $this->walkFromClause($AST->fromClause) . $this->walkWhereClause($AST->whereClause); @@ -585,31 +601,22 @@ public function walkSelectStatement(AST\SelectStatement $AST) $sql .= ' ORDER BY ' . $orderBySql; } - $sql = $this->platform->modifyLimitQuery($sql, $limit, $offset); - - if ($lockMode === LockMode::NONE) { - return $sql; - } + $this->assertOptimisticLockingHasAllClassesVersioned(); - if ($lockMode === LockMode::PESSIMISTIC_READ) { - return $sql . ' ' . $this->getReadLockSQL($this->platform); - } - - if ($lockMode === LockMode::PESSIMISTIC_WRITE) { - return $sql . ' ' . $this->getWriteLockSQL($this->platform); - } + return $sql; + } - if ($lockMode !== LockMode::OPTIMISTIC) { - throw QueryException::invalidLockMode(); - } + private function assertOptimisticLockingHasAllClassesVersioned(): void + { + $lockMode = $this->query->getHint(Query::HINT_LOCK_MODE) ?: LockMode::NONE; - foreach ($this->selectedClasses as $selectedClass) { - if (! $selectedClass['class']->isVersioned) { - throw OptimisticLockException::lockFailed($selectedClass['class']->name); + if ($lockMode === LockMode::OPTIMISTIC) { + foreach ($this->selectedClasses as $selectedClass) { + if (! $selectedClass['class']->isVersioned) { + throw OptimisticLockException::lockFailed($selectedClass['class']->name); + } } } - - return $sql; } /** diff --git a/src/Tools/Pagination/CountOutputWalker.php b/src/Tools/Pagination/CountOutputWalker.php index 0929e2ab065..92615f8aab5 100644 --- a/src/Tools/Pagination/CountOutputWalker.php +++ b/src/Tools/Pagination/CountOutputWalker.php @@ -11,7 +11,7 @@ use Doctrine\ORM\Query\Parser; use Doctrine\ORM\Query\ParserResult; use Doctrine\ORM\Query\ResultSetMapping; -use Doctrine\ORM\Query\SqlWalker; +use Doctrine\ORM\Query\SqlOutputWalker; use RuntimeException; use function array_diff; @@ -36,7 +36,7 @@ * * @psalm-import-type QueryComponent from Parser */ -class CountOutputWalker extends SqlWalker +class CountOutputWalker extends SqlOutputWalker { /** @var AbstractPlatform */ private $platform; @@ -62,16 +62,13 @@ public function __construct($query, $parserResult, array $queryComponents) parent::__construct($query, $parserResult, $queryComponents); } - /** - * {@inheritDoc} - */ - public function walkSelectStatement(SelectStatement $AST) + protected function createSqlForFinalizer(SelectStatement $AST): string { if ($this->platform instanceof SQLServerPlatform) { $AST->orderByClause = null; } - $sql = parent::walkSelectStatement($AST); + $sql = parent::createSqlForFinalizer($AST); if ($AST->groupByClause) { return sprintf( diff --git a/src/Tools/Pagination/LimitSubqueryOutputWalker.php b/src/Tools/Pagination/LimitSubqueryOutputWalker.php index f92c1145d4f..0e858ce0ecb 100644 --- a/src/Tools/Pagination/LimitSubqueryOutputWalker.php +++ b/src/Tools/Pagination/LimitSubqueryOutputWalker.php @@ -14,15 +14,20 @@ use Doctrine\ORM\Mapping\QuoteStrategy; use Doctrine\ORM\OptimisticLockException; use Doctrine\ORM\Query; +use Doctrine\ORM\Query\AST\DeleteStatement; use Doctrine\ORM\Query\AST\OrderByClause; use Doctrine\ORM\Query\AST\PathExpression; use Doctrine\ORM\Query\AST\SelectExpression; use Doctrine\ORM\Query\AST\SelectStatement; +use Doctrine\ORM\Query\AST\UpdateStatement; +use Doctrine\ORM\Query\Exec\SingleSelectSqlFinalizer; +use Doctrine\ORM\Query\Exec\SqlFinalizer; use Doctrine\ORM\Query\Parser; use Doctrine\ORM\Query\ParserResult; use Doctrine\ORM\Query\QueryException; use Doctrine\ORM\Query\ResultSetMapping; -use Doctrine\ORM\Query\SqlWalker; +use Doctrine\ORM\Query\SqlOutputWalker; +use LogicException; use RuntimeException; use function array_diff; @@ -50,7 +55,7 @@ * * @psalm-import-type QueryComponent from Parser */ -class LimitSubqueryOutputWalker extends SqlWalker +class LimitSubqueryOutputWalker extends SqlOutputWalker { private const ORDER_BY_PATH_EXPRESSION = '/(?platform = $query->getEntityManager()->getConnection()->getDatabasePlatform(); $this->rsm = $parserResult->getResultSetMapping(); + $query = clone $query; + // Reset limit and offset $this->firstResult = $query->getFirstResult(); $this->maxResults = $query->getMaxResults(); @@ -158,11 +165,33 @@ private function rebuildOrderByForRowNumber(SelectStatement $AST): void */ public function walkSelectStatement(SelectStatement $AST) { + $sqlFinalizer = $this->getFinalizer($AST); + + $query = $this->getQuery(); + + $abstractSqlExecutor = $sqlFinalizer->createExecutor($query); + + return $abstractSqlExecutor->getSqlStatements(); + } + + /** + * @param DeleteStatement|UpdateStatement|SelectStatement $AST + * + * @return SingleSelectSqlFinalizer + */ + public function getFinalizer($AST): SqlFinalizer + { + if (! $AST instanceof SelectStatement) { + throw new LogicException(self::class . ' is to be used on SelectStatements only'); + } + if ($this->platformSupportsRowNumber()) { - return $this->walkSelectStatementWithRowNumber($AST); + $sql = $this->createSqlWithRowNumber($AST); + } else { + $sql = $this->createSqlWithoutRowNumber($AST); } - return $this->walkSelectStatementWithoutRowNumber($AST); + return new SingleSelectSqlFinalizer($sql); } /** @@ -174,6 +203,16 @@ public function walkSelectStatement(SelectStatement $AST) * @throws RuntimeException */ public function walkSelectStatementWithRowNumber(SelectStatement $AST) + { + // Apply the limit and offset. + return $this->platform->modifyLimitQuery( + $this->createSqlWithRowNumber($AST), + $this->maxResults, + $this->firstResult + ); + } + + private function createSqlWithRowNumber(SelectStatement $AST): string { $hasOrderBy = false; $outerOrderBy = ' ORDER BY dctrn_minrownum ASC'; @@ -203,13 +242,6 @@ public function walkSelectStatementWithRowNumber(SelectStatement $AST) $sql .= $orderGroupBy . $outerOrderBy; } - // Apply the limit and offset. - $sql = $this->platform->modifyLimitQuery( - $sql, - $this->maxResults, - $this->firstResult - ); - // Add the columns to the ResultSetMapping. It's not really nice but // it works. Preferably I'd clear the RSM or simply create a new one // but that is not possible from inside the output walker, so we dirty @@ -232,6 +264,16 @@ public function walkSelectStatementWithRowNumber(SelectStatement $AST) * @throws RuntimeException */ public function walkSelectStatementWithoutRowNumber(SelectStatement $AST, $addMissingItemsFromOrderByToSelect = true) + { + // Apply the limit and offset. + return $this->platform->modifyLimitQuery( + $this->createSqlWithoutRowNumber($AST, $addMissingItemsFromOrderByToSelect), + $this->maxResults, + $this->firstResult + ); + } + + private function createSqlWithoutRowNumber(SelectStatement $AST, bool $addMissingItemsFromOrderByToSelect = true): string { // We don't want to call this recursively! if ($AST->orderByClause instanceof OrderByClause && $addMissingItemsFromOrderByToSelect) { @@ -260,13 +302,6 @@ public function walkSelectStatementWithoutRowNumber(SelectStatement $AST, $addMi // https://github.com/doctrine/orm/issues/2630 $sql = $this->preserveSqlOrdering($sqlIdentifier, $innerSql, $sql, $orderByClause); - // Apply the limit and offset. - $sql = $this->platform->modifyLimitQuery( - $sql, - $this->maxResults, - $this->firstResult - ); - // Add the columns to the ResultSetMapping. It's not really nice but // it works. Preferably I'd clear the RSM or simply create a new one // but that is not possible from inside the output walker, so we dirty diff --git a/src/Tools/Pagination/RootTypeWalker.php b/src/Tools/Pagination/RootTypeWalker.php index dc1d77a51c7..ec45dbd6fc1 100644 --- a/src/Tools/Pagination/RootTypeWalker.php +++ b/src/Tools/Pagination/RootTypeWalker.php @@ -5,7 +5,10 @@ namespace Doctrine\ORM\Tools\Pagination; use Doctrine\ORM\Query\AST; -use Doctrine\ORM\Query\SqlWalker; +use Doctrine\ORM\Query\Exec\FinalizedSelectExecutor; +use Doctrine\ORM\Query\Exec\PreparedExecutorFinalizer; +use Doctrine\ORM\Query\Exec\SqlFinalizer; +use Doctrine\ORM\Query\SqlOutputWalker; use Doctrine\ORM\Utility\PersisterHelper; use RuntimeException; @@ -22,7 +25,7 @@ * Returning the type instead of a "real" SQL statement is a slight hack. However, it has the * benefit that the DQL -> root entity id type resolution can be cached in the query cache. */ -final class RootTypeWalker extends SqlWalker +final class RootTypeWalker extends SqlOutputWalker { public function walkSelectStatement(AST\SelectStatement $AST): string { @@ -45,4 +48,13 @@ public function walkSelectStatement(AST\SelectStatement $AST): string ->getEntityManager() )[0]; } + + public function getFinalizer($AST): SqlFinalizer + { + if (! $AST instanceof AST\SelectStatement) { + throw new RuntimeException(self::class . ' is to be used on SelectStatements only'); + } + + return new PreparedExecutorFinalizer(new FinalizedSelectExecutor($this->walkSelectStatement($AST))); + } } diff --git a/tests/Tests/Mocks/NullSqlWalker.php b/tests/Tests/Mocks/NullSqlWalker.php index 9d54e703547..90cf0c6e926 100644 --- a/tests/Tests/Mocks/NullSqlWalker.php +++ b/tests/Tests/Mocks/NullSqlWalker.php @@ -7,12 +7,14 @@ use Doctrine\DBAL\Connection; use Doctrine\ORM\Query\AST; use Doctrine\ORM\Query\Exec\AbstractSqlExecutor; -use Doctrine\ORM\Query\SqlWalker; +use Doctrine\ORM\Query\Exec\PreparedExecutorFinalizer; +use Doctrine\ORM\Query\Exec\SqlFinalizer; +use Doctrine\ORM\Query\SqlOutputWalker; /** * SqlWalker implementation that does not produce SQL. */ -class NullSqlWalker extends SqlWalker +class NullSqlWalker extends SqlOutputWalker { public function walkSelectStatement(AST\SelectStatement $AST): string { @@ -29,13 +31,15 @@ public function walkDeleteStatement(AST\DeleteStatement $AST): string return ''; } - public function getExecutor($AST): AbstractSqlExecutor + public function getFinalizer($AST): SqlFinalizer { - return new class extends AbstractSqlExecutor { - public function execute(Connection $conn, array $params, array $types): int - { - return 0; + return new PreparedExecutorFinalizer( + new class extends AbstractSqlExecutor { + public function execute(Connection $conn, array $params, array $types): int + { + return 0; + } } - }; + ); } } diff --git a/tests/Tests/ORM/Functional/ParserResultSerializationTest.php b/tests/Tests/ORM/Functional/ParserResultSerializationTest.php index 0841c5ff3a0..7b2ae59119a 100644 --- a/tests/Tests/ORM/Functional/ParserResultSerializationTest.php +++ b/tests/Tests/ORM/Functional/ParserResultSerializationTest.php @@ -6,6 +6,8 @@ use Closure; use Doctrine\ORM\Query; +use Doctrine\ORM\Query\Exec\FinalizedSelectExecutor; +use Doctrine\ORM\Query\Exec\PreparedExecutorFinalizer; use Doctrine\ORM\Query\Exec\SingleSelectExecutor; use Doctrine\ORM\Query\ParserResult; use Doctrine\ORM\Query\ResultSetMapping; @@ -35,18 +37,40 @@ protected function setUp(): void * * @dataProvider provideToSerializedAndBack */ - public function testSerializeParserResult(Closure $toSerializedAndBack): void + public function testSerializeParserResultForQueryWithSqlWalker(Closure $toSerializedAndBack): void { $query = $this->_em ->createQuery('SELECT u FROM Doctrine\Tests\Models\Company\CompanyEmployee u WHERE u.name = :name'); + // Use the (legacy) SqlWalker which directly puts an SqlExecutor instance into the parser result + $query->setHint(Query::HINT_CUSTOM_OUTPUT_WALKER, Query\SqlWalker::class); + $parserResult = self::parseQuery($query); $unserialized = $toSerializedAndBack($parserResult); $this->assertInstanceOf(ParserResult::class, $unserialized); $this->assertInstanceOf(ResultSetMapping::class, $unserialized->getResultSetMapping()); $this->assertEquals(['name' => [0]], $unserialized->getParameterMappings()); - $this->assertInstanceOf(SingleSelectExecutor::class, $unserialized->getSqlExecutor()); + $this->assertNotNull($unserialized->prepareSqlExecutor($query)); + } + + /** + * @param Closure(ParserResult): ParserResult $toSerializedAndBack + * + * @dataProvider provideToSerializedAndBack + */ + public function testSerializeParserResultForQueryWithSqlOutputWalker(Closure $toSerializedAndBack): void + { + $query = $this->_em + ->createQuery('SELECT u FROM Doctrine\Tests\Models\Company\CompanyEmployee u WHERE u.name = :name'); + + $parserResult = self::parseQuery($query); + $unserialized = $toSerializedAndBack($parserResult); + + $this->assertInstanceOf(ParserResult::class, $unserialized); + $this->assertInstanceOf(ResultSetMapping::class, $unserialized->getResultSetMapping()); + $this->assertEquals(['name' => [0]], $unserialized->getParameterMappings()); + $this->assertNotNull($unserialized->prepareSqlExecutor($query)); } /** @return Generator */ @@ -75,6 +99,9 @@ public function testItSerializesParserResultWithAForwardCompatibleFormat(): void $query = $this->_em ->createQuery('SELECT u FROM Doctrine\Tests\Models\Company\CompanyEmployee u WHERE u.name = :name'); + // Use the (legacy) SqlWalker which directly puts an SqlExecutor instance into the parser result + $query->setHint(Query::HINT_CUSTOM_OUTPUT_WALKER, Query\SqlWalker::class); + $parserResult = self::parseQuery($query); $serialized = serialize($parserResult); $this->assertStringNotContainsString( @@ -118,11 +145,12 @@ public static function provideSerializedSingleSelectResults(): Generator public function testSymfony44ProvidedData(): void { - $sqlExecutor = $this->createMock(SingleSelectExecutor::class); + $sqlExecutor = new FinalizedSelectExecutor('test'); + $sqlFinalizer = new PreparedExecutorFinalizer($sqlExecutor); $resultSetMapping = $this->createMock(ResultSetMapping::class); $parserResult = new ParserResult(); - $parserResult->setSqlExecutor($sqlExecutor); + $parserResult->setSqlFinalizer($sqlFinalizer); $parserResult->setResultSetMapping($resultSetMapping); $parserResult->addParameterMapping('name', 0); @@ -132,7 +160,7 @@ public function testSymfony44ProvidedData(): void $this->assertInstanceOf(ParserResult::class, $unserialized); $this->assertInstanceOf(ResultSetMapping::class, $unserialized->getResultSetMapping()); $this->assertEquals(['name' => [0]], $unserialized->getParameterMappings()); - $this->assertInstanceOf(SingleSelectExecutor::class, $unserialized->getSqlExecutor()); + $this->assertEquals($sqlExecutor, $unserialized->prepareSqlExecutor($this->createMock(Query::class))); } private static function parseQuery(Query $query): ParserResult diff --git a/tests/Tests/ORM/Functional/QueryCacheTest.php b/tests/Tests/ORM/Functional/QueryCacheTest.php index 4935c3d78ee..3894ecef697 100644 --- a/tests/Tests/ORM/Functional/QueryCacheTest.php +++ b/tests/Tests/ORM/Functional/QueryCacheTest.php @@ -43,7 +43,7 @@ public function testQueryCacheDependsOnHints(): array } /** @depends testQueryCacheDependsOnHints */ - public function testQueryCacheDependsOnFirstResult(array $previous): void + public function testQueryCacheDoesNotDependOnFirstResultForDefaultOutputWalker(array $previous): void { [$query, $cache] = $previous; assert($query instanceof Query); @@ -55,11 +55,11 @@ public function testQueryCacheDependsOnFirstResult(array $previous): void $query->setMaxResults(9999); $query->getResult(); - self::assertCount($cacheCount + 1, $cache->getValues()); + self::assertCount($cacheCount, $cache->getValues()); } /** @depends testQueryCacheDependsOnHints */ - public function testQueryCacheDependsOnMaxResults(array $previous): void + public function testQueryCacheDoesNotDependOnMaxResultsForDefaultOutputWalker(array $previous): void { [$query, $cache] = $previous; assert($query instanceof Query); @@ -70,7 +70,7 @@ public function testQueryCacheDependsOnMaxResults(array $previous): void $query->setMaxResults(10); $query->getResult(); - self::assertCount($cacheCount + 1, $cache->getValues()); + self::assertCount($cacheCount, $cache->getValues()); } /** @depends testQueryCacheDependsOnHints */ diff --git a/tests/Tests/ORM/Functional/Ticket/GH11112Test.php b/tests/Tests/ORM/Functional/Ticket/GH11112Test.php new file mode 100644 index 00000000000..3dbdb533b33 --- /dev/null +++ b/tests/Tests/ORM/Functional/Ticket/GH11112Test.php @@ -0,0 +1,79 @@ +useModelSet('cms'); + self::$queryCache = new ArrayAdapter(); + + parent::setUp(); + } + + public function testSimpleQueryHasLimitAndOffsetApplied(): void + { + $platform = $this->_em->getConnection()->getDatabasePlatform(); + $query = $this->_em->createQuery('SELECT u FROM ' . CmsUser::class . ' u'); + $originalSql = $query->getSQL(); + + $query->setMaxResults(10); + $query->setFirstResult(20); + $sqlMax10First20 = $query->getSQL(); + + $query->setMaxResults(30); + $query->setFirstResult(40); + $sqlMax30First40 = $query->getSQL(); + + // The SQL is platform specific and may even be something with outer SELECTS being added. So, + // derive the expected value at runtime through the platform. + self::assertSame($platform->modifyLimitQuery($originalSql, 10, 20), $sqlMax10First20); + self::assertSame($platform->modifyLimitQuery($originalSql, 30, 40), $sqlMax30First40); + + $cacheEntries = self::$queryCache->getValues(); + self::assertCount(1, $cacheEntries); + } + + public function testSubqueryLimitAndOffsetAreIgnored(): void + { + // Not sure what to do about this test. Basically, I want to make sure that + // firstResult/maxResult for subqueries are not relevant, they do not make it + // into the final query at all. That would give us the guarantee that the + // "sql finalizer" step is sufficient for the final, "outer" query and we + // do not need to run finalizers for the subqueries. + + // This DQL/query makes no sense, it's just about creating a subquery in the first place + $queryBuilder = $this->_em->createQueryBuilder(); + $queryBuilder + ->select('o') + ->from(CmsUser::class, 'o') + ->where($queryBuilder->expr()->exists( + $this->_em->createQueryBuilder() + ->select('u') + ->from(CmsUser::class, 'u') + ->setFirstResult(10) + ->setMaxResults(20) + )); + + $query = $queryBuilder->getQuery(); + $originalSql = $query->getSQL(); + + $clone = clone $query; + $clone->setFirstResult(24); + $clone->setMaxResults(42); + $limitedSql = $clone->getSQL(); + + $platform = $this->_em->getConnection()->getDatabasePlatform(); + + // The SQL is platform specific and may even be something with outer SELECTS being added. So, + // derive the expected value at runtime through the platform. + self::assertSame($platform->modifyLimitQuery($originalSql, 42, 24), $limitedSql); + } +} diff --git a/tests/Tests/ORM/Query/CustomTreeWalkersTest.php b/tests/Tests/ORM/Query/CustomTreeWalkersTest.php index b2267078ff5..eb27b6113a3 100644 --- a/tests/Tests/ORM/Query/CustomTreeWalkersTest.php +++ b/tests/Tests/ORM/Query/CustomTreeWalkersTest.php @@ -6,6 +6,7 @@ use Doctrine\ORM\EntityManagerInterface; use Doctrine\ORM\Query; +use Doctrine\ORM\Query\AST; use Doctrine\ORM\Query\QueryException; use Doctrine\ORM\Query\SqlWalker; use Doctrine\ORM\Query\TreeWalker; @@ -110,13 +111,13 @@ public function testSupportsSeveralHintsQueries(): void } } -class AddUnknownQueryComponentWalker extends Query\SqlWalker +class AddUnknownQueryComponentWalker extends Query\SqlOutputWalker { - public function walkSelectStatement(Query\AST\SelectStatement $selectStatement): void + protected function createSqlForFinalizer(AST\SelectStatement $AST): string { - parent::walkSelectStatement($selectStatement); - $this->setQueryComponent('x', []); + + return parent::createSqlForFinalizer($AST); } } diff --git a/tests/Tests/ORM/Tools/Pagination/LimitSubqueryOutputWalkerTest.php b/tests/Tests/ORM/Tools/Pagination/LimitSubqueryOutputWalkerTest.php index cb69a3a5674..95d2047d593 100644 --- a/tests/Tests/ORM/Tools/Pagination/LimitSubqueryOutputWalkerTest.php +++ b/tests/Tests/ORM/Tools/Pagination/LimitSubqueryOutputWalkerTest.php @@ -10,6 +10,7 @@ use Doctrine\DBAL\Platforms\PostgreSQLPlatform; use Doctrine\ORM\Query; use Doctrine\ORM\Tools\Pagination\LimitSubqueryOutputWalker; +use Symfony\Component\Cache\Adapter\ArrayAdapter; use function class_exists; @@ -385,6 +386,28 @@ public function testLimitSubqueryOrderBySubSelectOrderByExpressionOracle(): void ); } + public function testParsingQueryWithDifferentLimitOffsetValuesTakesOnlyOneCacheEntry(): void + { + $queryCache = new ArrayAdapter(); + $this->entityManager->getConfiguration()->setQueryCache($queryCache); + + $query = $this->createQuery('SELECT p, c, a FROM Doctrine\Tests\ORM\Tools\Pagination\MyBlogPost p JOIN p.category c JOIN p.author a'); + + self::assertSame( + 'SELECT DISTINCT id_0 FROM (SELECT m0_.id AS id_0, m0_.title AS title_1, c1_.id AS id_2, a2_.id AS id_3, a2_.name AS name_4, m0_.author_id AS author_id_5, m0_.category_id AS category_id_6 FROM MyBlogPost m0_ INNER JOIN Category c1_ ON m0_.category_id = c1_.id INNER JOIN Author a2_ ON m0_.author_id = a2_.id) dctrn_result LIMIT 20 OFFSET 10', + $query->getSQL() + ); + + $query->setFirstResult(30)->setMaxResults(40); + + self::assertSame( + 'SELECT DISTINCT id_0 FROM (SELECT m0_.id AS id_0, m0_.title AS title_1, c1_.id AS id_2, a2_.id AS id_3, a2_.name AS name_4, m0_.author_id AS author_id_5, m0_.category_id AS category_id_6 FROM MyBlogPost m0_ INNER JOIN Category c1_ ON m0_.category_id = c1_.id INNER JOIN Author a2_ ON m0_.author_id = a2_.id) dctrn_result LIMIT 40 OFFSET 30', + $query->getSQL() + ); + + self::assertCount(1, $queryCache->getValues()); + } + private function createQuery(string $dql): Query { $query = $this->entityManager->createQuery($dql); diff --git a/tests/Tests/ORM/Tools/Pagination/PaginatorTest.php b/tests/Tests/ORM/Tools/Pagination/PaginatorTest.php index 1c8b47ac0cf..23ff3b90be4 100644 --- a/tests/Tests/ORM/Tools/Pagination/PaginatorTest.php +++ b/tests/Tests/ORM/Tools/Pagination/PaginatorTest.php @@ -87,7 +87,8 @@ public function testExtraParametersAreStrippedWhenWalkerRemovingOriginalSelectEl public function testPaginatorNotCaringAboutExtraParametersWithoutOutputWalkers(): void { - $this->connection->expects(self::exactly(3))->method('executeQuery'); + $result = $this->getMockBuilder(Result::class)->disableOriginalConstructor()->getMock(); + $this->connection->expects(self::exactly(3))->method('executeQuery')->willReturn($result); $this->createPaginatorWithExtraParametersWithoutOutputWalkers([])->count(); $this->createPaginatorWithExtraParametersWithoutOutputWalkers([[10]])->count(); @@ -96,7 +97,8 @@ public function testPaginatorNotCaringAboutExtraParametersWithoutOutputWalkers() public function testgetIteratorDoesCareAboutExtraParametersWithoutOutputWalkersWhenResultIsNotEmpty(): void { - $this->connection->expects(self::exactly(1))->method('executeQuery'); + $result = $this->getMockBuilder(Result::class)->disableOriginalConstructor()->getMock(); + $this->connection->expects(self::exactly(1))->method('executeQuery')->willReturn($result); $this->expectException(Query\QueryException::class); $this->expectExceptionMessage('Too many parameters: the query defines 1 parameters and you bound 2'); diff --git a/tests/Tests/ORM/Tools/Pagination/WhereInWalkerTest.php b/tests/Tests/ORM/Tools/Pagination/WhereInWalkerTest.php index 1f1710d2cf1..6f5c9c6dd22 100644 --- a/tests/Tests/ORM/Tools/Pagination/WhereInWalkerTest.php +++ b/tests/Tests/ORM/Tools/Pagination/WhereInWalkerTest.php @@ -21,9 +21,10 @@ public function testDqlQueryTransformation(string $dql, string $expectedSql): vo $query->setHint(Query::HINT_CUSTOM_TREE_WALKERS, [WhereInWalker::class]); $query->setHint(WhereInWalker::HINT_PAGINATOR_HAS_IDS, true); - $result = (new Parser($query))->parse(); + $result = (new Parser($query))->parse(); + $executor = $result->prepareSqlExecutor($query); - self::assertEquals($expectedSql, $result->getSqlExecutor()->getSqlStatements()); + self::assertEquals($expectedSql, $executor->getSqlStatements()); self::assertEquals([0], $result->getSqlParameterPositions(WhereInWalker::PAGINATOR_ID_ALIAS)); } From ee0d7197dd2be959bb07bf25dee442ccf9d7aa8c Mon Sep 17 00:00:00 2001 From: Simon Podlipsky Date: Fri, 11 Oct 2024 13:00:52 +0200 Subject: [PATCH 13/21] test: cover all transactional methods in `EntityManagerTest::testItPreservesTheOriginalExceptionOnRollbackFailure()` --- tests/Tests/ORM/EntityManagerTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Tests/ORM/EntityManagerTest.php b/tests/Tests/ORM/EntityManagerTest.php index 88ff5ed924f..202560602cd 100644 --- a/tests/Tests/ORM/EntityManagerTest.php +++ b/tests/Tests/ORM/EntityManagerTest.php @@ -399,7 +399,7 @@ public function rollBack(): bool }); try { - $entityManager->transactional(static function (): void { + $entityManager->$methodName(static function (): void { throw new Exception('Original exception'); }); self::fail('Exception expected'); From 5bfb7449671a392242477c5ba63331076e936597 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Fri, 11 Oct 2024 15:29:19 +0200 Subject: [PATCH 14/21] The MySQL/Maria EnumType added in DBAL 4.2 has a new known option "values". (#11657) --- src/Tools/SchemaTool.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Tools/SchemaTool.php b/src/Tools/SchemaTool.php index cff59aecdd8..e0a24d9459d 100644 --- a/src/Tools/SchemaTool.php +++ b/src/Tools/SchemaTool.php @@ -47,7 +47,7 @@ */ class SchemaTool { - private const KNOWN_COLUMN_OPTIONS = ['comment', 'unsigned', 'fixed', 'default']; + private const KNOWN_COLUMN_OPTIONS = ['comment', 'unsigned', 'fixed', 'default', 'values']; private readonly AbstractPlatform $platform; private readonly QuoteStrategy $quoteStrategy; From 522863116a245c7bfcf6f5f2458755601b1d1909 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9goire=20Paris?= Date: Fri, 11 Oct 2024 20:23:33 +0200 Subject: [PATCH 15/21] Update branch metadata (#11663) 2.20.0 just got released --- .doctrine-project.json | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/.doctrine-project.json b/.doctrine-project.json index f3a38fb4bdd..7865e2dd1a6 100644 --- a/.doctrine-project.json +++ b/.doctrine-project.json @@ -35,17 +35,23 @@ "slug": "3.0", "maintained": false }, + { + "name": "2.21", + "branchName": "2.21.x", + "slug": "2.21", + "upcoming": true + }, { "name": "2.20", "branchName": "2.20.x", "slug": "2.20", - "upcoming": true + "maintained": true }, { "name": "2.19", "branchName": "2.19.x", "slug": "2.19", - "maintained": true + "maintained": false }, { "name": "2.18", From f5fb400d0f13fc58431ac06185e53edac4bea704 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sat, 12 Oct 2024 02:32:15 +0200 Subject: [PATCH 16/21] Address review comments. --- docs/en/reference/partial-hydration.rst | 5 ----- docs/en/reference/partial-objects.rst | 10 ---------- src/UnitOfWork.php | 12 ------------ 3 files changed, 27 deletions(-) diff --git a/docs/en/reference/partial-hydration.rst b/docs/en/reference/partial-hydration.rst index 16879c45c52..fda14b98efa 100644 --- a/docs/en/reference/partial-hydration.rst +++ b/docs/en/reference/partial-hydration.rst @@ -1,11 +1,6 @@ Partial Hydration ================= -.. note:: - - Creating Partial Objects through DQL was possible in ORM 2, - but is only supported for array hydration as of ORM 3. - Partial hydration of entities is allowed in the array hydrator, when only a subset of the fields of an entity are loaded from the database and the nested results are still created based on the entity relationship structure. diff --git a/docs/en/reference/partial-objects.rst b/docs/en/reference/partial-objects.rst index 51f173adf6c..3835123257a 100644 --- a/docs/en/reference/partial-objects.rst +++ b/docs/en/reference/partial-objects.rst @@ -1,14 +1,6 @@ Partial Objects =============== - -.. note:: - - Creating Partial Objects through DQL is deprecated and - will be removed in the future, use data transfer object - support in DQL instead. (\ `Details - `_) - A partial object is an object whose state is not fully initialized after being reconstituted from the database and that is disconnected from the rest of its data. The following section will @@ -94,5 +86,3 @@ When should I force partial objects? Mainly for optimization purposes, but be careful of premature optimization as partial objects lead to potentially more fragile code. - - diff --git a/src/UnitOfWork.php b/src/UnitOfWork.php index ef31a6bbcd6..751d36ee21a 100644 --- a/src/UnitOfWork.php +++ b/src/UnitOfWork.php @@ -2412,18 +2412,6 @@ public function createEntity(string $className, array $data, array &$hints = []) unset($this->eagerLoadingEntities[$class->rootEntityName]); } - // Properly initialize any unfetched associations, if partial objects are not allowed. - if (isset($hints[Query::HINT_FORCE_PARTIAL_LOAD])) { - Deprecation::trigger( - 'doctrine/orm', - 'https://github.com/doctrine/orm/issues/8471', - 'Partial Objects are deprecated (here entity %s)', - $className, - ); - - return $entity; - } - foreach ($class->associationMappings as $field => $assoc) { // Check if the association is not among the fetch-joined associations already. if (isset($hints['fetchAlias'], $hints['fetched'][$hints['fetchAlias']][$field])) { From 516b5931934a07d3071bb2cd4b833f1d0729a22c Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sat, 12 Oct 2024 15:41:31 +0200 Subject: [PATCH 17/21] Fix phpcs --- src/UnitOfWork.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/UnitOfWork.php b/src/UnitOfWork.php index df2996ce3bd..4cf3e9e2d9f 100644 --- a/src/UnitOfWork.php +++ b/src/UnitOfWork.php @@ -12,7 +12,6 @@ use Doctrine\DBAL; use Doctrine\DBAL\Connections\PrimaryReadReplicaConnection; use Doctrine\DBAL\LockMode; -use Doctrine\Deprecations\Deprecation; use Doctrine\ORM\Cache\Persister\CachedPersister; use Doctrine\ORM\Event\ListenersInvoker; use Doctrine\ORM\Event\OnClearEventArgs; From c7e5605d116cca2229db3ee012eb78a420cd174c Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sat, 12 Oct 2024 15:43:46 +0200 Subject: [PATCH 18/21] Fix test --- src/Query/Parser.php | 1 - tests/Tests/ORM/UnitOfWorkTest.php | 9 --------- 2 files changed, 10 deletions(-) diff --git a/src/Query/Parser.php b/src/Query/Parser.php index 3b1779e4307..bcfad85c422 100644 --- a/src/Query/Parser.php +++ b/src/Query/Parser.php @@ -9,7 +9,6 @@ use Doctrine\ORM\EntityManagerInterface; use Doctrine\ORM\Exception\DuplicateFieldException; use Doctrine\ORM\Exception\NoMatchingPropertyException; -use Doctrine\ORM\Internal\Hydration\HydrationException; use Doctrine\ORM\Mapping\AssociationMapping; use Doctrine\ORM\Mapping\ClassMetadata; use Doctrine\ORM\Query; diff --git a/tests/Tests/ORM/UnitOfWorkTest.php b/tests/Tests/ORM/UnitOfWorkTest.php index 900ee218300..2a421c6687d 100644 --- a/tests/Tests/ORM/UnitOfWorkTest.php +++ b/tests/Tests/ORM/UnitOfWorkTest.php @@ -691,15 +691,6 @@ public function rollBack(): void self::assertSame('Commit failed', $e->getPrevious()->getMessage()); } } - - public function testItThrowsWhenCreateEntityWithSqlWalkerPartialQueryHint(): void - { - $this->expectException(HydrationException::class); - $this->expectExceptionMessage('Hydration of entity objects is not allowed when DQL PARTIAL keyword is used.'); - - $hints = [SqlWalker::HINT_PARTIAL => true]; - $this->_unitOfWork->createEntity(VersionedAssignedIdentifierEntity::class, ['id' => 1], $hints); - } } From 7c9b74221fd2c0e3245fb43a607a0ec113dd037e Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sat, 12 Oct 2024 16:31:02 +0200 Subject: [PATCH 19/21] fix phpbench tests. --- ...ueryFetchJoinPartialObjectHydrationPerformanceBench.php | 7 +++++++ .../SimpleQueryPartialObjectHydrationPerformanceBench.php | 7 +++++++ 2 files changed, 14 insertions(+) diff --git a/tests/Performance/Hydration/MixedQueryFetchJoinPartialObjectHydrationPerformanceBench.php b/tests/Performance/Hydration/MixedQueryFetchJoinPartialObjectHydrationPerformanceBench.php index ef1d1b4bdca..0694e8fff6a 100644 --- a/tests/Performance/Hydration/MixedQueryFetchJoinPartialObjectHydrationPerformanceBench.php +++ b/tests/Performance/Hydration/MixedQueryFetchJoinPartialObjectHydrationPerformanceBench.php @@ -10,6 +10,7 @@ use Doctrine\ORM\Query\ResultSetMapping; use Doctrine\Performance\EntityManagerFactory; use Doctrine\Tests\Mocks\ArrayResultFactory; +use Doctrine\Tests\Models\CMS\CmsAddress; use Doctrine\Tests\Models\CMS\CmsPhonenumber; use Doctrine\Tests\Models\CMS\CmsUser; use PhpBench\Benchmark\Metadata\Annotations\BeforeMethods; @@ -33,6 +34,7 @@ public function init(): void 'u__name' => 'Roman', 'sclr0' => 'ROMANB', 'p__phonenumber' => '42', + 'a__id' => '1', ], [ 'u__id' => '1', @@ -41,6 +43,7 @@ public function init(): void 'u__name' => 'Roman', 'sclr0' => 'ROMANB', 'p__phonenumber' => '43', + 'a__id' => '1', ], [ 'u__id' => '2', @@ -49,6 +52,7 @@ public function init(): void 'u__name' => 'Roman', 'sclr0' => 'JWAGE', 'p__phonenumber' => '91', + 'a__id' => '1', ], ]; @@ -60,6 +64,7 @@ public function init(): void 'u__name' => 'Jonathan', 'sclr0' => 'JWAGE' . $i, 'p__phonenumber' => '91', + 'a__id' => '1', ]; } @@ -75,6 +80,8 @@ public function init(): void $this->rsm->addFieldResult('u', 'u__name', 'name'); $this->rsm->addScalarResult('sclr0', 'nameUpper'); $this->rsm->addFieldResult('p', 'p__phonenumber', 'phonenumber'); + $this->rsm->addJoinedEntityResult(CmsAddress::class, 'a', 'u', 'address'); + $this->rsm->addFieldResult('a', 'a__id', 'id'); } public function benchHydration(): void diff --git a/tests/Performance/Hydration/SimpleQueryPartialObjectHydrationPerformanceBench.php b/tests/Performance/Hydration/SimpleQueryPartialObjectHydrationPerformanceBench.php index a5c78670727..14ed606508c 100644 --- a/tests/Performance/Hydration/SimpleQueryPartialObjectHydrationPerformanceBench.php +++ b/tests/Performance/Hydration/SimpleQueryPartialObjectHydrationPerformanceBench.php @@ -10,6 +10,7 @@ use Doctrine\ORM\Query\ResultSetMapping; use Doctrine\Performance\EntityManagerFactory; use Doctrine\Tests\Mocks\ArrayResultFactory; +use Doctrine\Tests\Models\CMS\CmsAddress; use Doctrine\Tests\Models\CMS\CmsUser; use PhpBench\Benchmark\Metadata\Annotations\BeforeMethods; @@ -30,18 +31,21 @@ public function init(): void 'u__status' => 'developer', 'u__username' => 'romanb', 'u__name' => 'Roman', + 'a__id' => '1', ], [ 'u__id' => '1', 'u__status' => 'developer', 'u__username' => 'romanb', 'u__name' => 'Roman', + 'a__id' => '1', ], [ 'u__id' => '2', 'u__status' => 'developer', 'u__username' => 'romanb', 'u__name' => 'Roman', + 'a__id' => '1', ], ]; @@ -51,6 +55,7 @@ public function init(): void 'u__status' => 'developer', 'u__username' => 'jwage', 'u__name' => 'Jonathan', + 'a__id' => '1', ]; } @@ -63,6 +68,8 @@ public function init(): void $this->rsm->addFieldResult('u', 'u__status', 'status'); $this->rsm->addFieldResult('u', 'u__username', 'username'); $this->rsm->addFieldResult('u', 'u__name', 'name'); + $this->rsm->addJoinedEntityResult(CmsAddress::class, 'a', 'u', 'address'); + $this->rsm->addFieldResult('a', 'a__id', 'id'); } public function benchHydration(): void From 60c245413dbff6710786f2dbd091c69407e972ee Mon Sep 17 00:00:00 2001 From: "Alexander M. Turek" Date: Sat, 12 Oct 2024 17:35:44 +0200 Subject: [PATCH 20/21] Auto-detect values for EnumType columns (#11666) --- psalm-baseline.xml | 1 + src/Mapping/ClassMetadata.php | 15 ++++-- src/Mapping/DefaultTypedFieldMapper.php | 49 ++++++++++++------- tests/Tests/Models/Enums/CardNativeEnum.php | 25 ++++++++++ .../Models/Enums/TypedCardNativeEnum.php | 23 +++++++++ tests/Tests/ORM/Functional/EnumTest.php | 29 +++++++---- 6 files changed, 111 insertions(+), 31 deletions(-) create mode 100644 tests/Tests/Models/Enums/CardNativeEnum.php create mode 100644 tests/Tests/Models/Enums/TypedCardNativeEnum.php diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 886493223c9..807db27145e 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -402,6 +402,7 @@ + diff --git a/src/Mapping/ClassMetadata.php b/src/Mapping/ClassMetadata.php index 70d3ea7042f..7c9020805a5 100644 --- a/src/Mapping/ClassMetadata.php +++ b/src/Mapping/ClassMetadata.php @@ -7,6 +7,7 @@ use BackedEnum; use BadMethodCallException; use Doctrine\DBAL\Platforms\AbstractPlatform; +use Doctrine\DBAL\Types\Types; use Doctrine\Deprecations\Deprecation; use Doctrine\Instantiator\Instantiator; use Doctrine\Instantiator\InstantiatorInterface; @@ -23,6 +24,7 @@ use ReflectionProperty; use Stringable; +use function array_column; use function array_diff; use function array_intersect; use function array_key_exists; @@ -34,6 +36,7 @@ use function assert; use function class_exists; use function count; +use function defined; use function enum_exists; use function explode; use function in_array; @@ -1119,9 +1122,7 @@ private function validateAndCompleteTypedFieldMapping(array $mapping): array { $field = $this->reflClass->getProperty($mapping['fieldName']); - $mapping = $this->typedFieldMapper->validateAndComplete($mapping, $field); - - return $mapping; + return $this->typedFieldMapper->validateAndComplete($mapping, $field); } /** @@ -1232,6 +1233,14 @@ protected function validateAndCompleteFieldMapping(array $mapping): FieldMapping if (! empty($mapping->id)) { $this->containsEnumIdentifier = true; } + + if ( + defined('Doctrine\DBAL\Types\Types::ENUM') + && $mapping->type === Types::ENUM + && ! isset($mapping->options['values']) + ) { + $mapping->options['values'] = array_column($mapping->enumType::cases(), 'value'); + } } return $mapping; diff --git a/src/Mapping/DefaultTypedFieldMapper.php b/src/Mapping/DefaultTypedFieldMapper.php index 49144b8b7c8..40b37b8c426 100644 --- a/src/Mapping/DefaultTypedFieldMapper.php +++ b/src/Mapping/DefaultTypedFieldMapper.php @@ -16,6 +16,7 @@ use function array_merge; use function assert; +use function defined; use function enum_exists; use function is_a; @@ -49,30 +50,40 @@ public function validateAndComplete(array $mapping, ReflectionProperty $field): { $type = $field->getType(); + if (! $type instanceof ReflectionNamedType) { + return $mapping; + } + if ( - ! isset($mapping['type']) - && ($type instanceof ReflectionNamedType) + ! $type->isBuiltin() + && enum_exists($type->getName()) + && (! isset($mapping['type']) || ( + defined('Doctrine\DBAL\Types\Types::ENUM') + && $mapping['type'] === Types::ENUM + )) ) { - if (! $type->isBuiltin() && enum_exists($type->getName())) { - $reflection = new ReflectionEnum($type->getName()); - if (! $reflection->isBacked()) { - throw MappingException::backedEnumTypeRequired( - $field->class, - $mapping['fieldName'], - $type->getName(), - ); - } + $reflection = new ReflectionEnum($type->getName()); + if (! $reflection->isBacked()) { + throw MappingException::backedEnumTypeRequired( + $field->class, + $mapping['fieldName'], + $type->getName(), + ); + } - assert(is_a($type->getName(), BackedEnum::class, true)); - $mapping['enumType'] = $type->getName(); - $type = $reflection->getBackingType(); + assert(is_a($type->getName(), BackedEnum::class, true)); + $mapping['enumType'] = $type->getName(); + $type = $reflection->getBackingType(); - assert($type instanceof ReflectionNamedType); - } + assert($type instanceof ReflectionNamedType); + } - if (isset($this->typedFieldMappings[$type->getName()])) { - $mapping['type'] = $this->typedFieldMappings[$type->getName()]; - } + if (isset($mapping['type'])) { + return $mapping; + } + + if (isset($this->typedFieldMappings[$type->getName()])) { + $mapping['type'] = $this->typedFieldMappings[$type->getName()]; } return $mapping; diff --git a/tests/Tests/Models/Enums/CardNativeEnum.php b/tests/Tests/Models/Enums/CardNativeEnum.php new file mode 100644 index 00000000000..83d4d59a778 --- /dev/null +++ b/tests/Tests/Models/Enums/CardNativeEnum.php @@ -0,0 +1,25 @@ + ['H', 'D', 'C', 'S', 'Z']])] + public $suit; +} diff --git a/tests/Tests/Models/Enums/TypedCardNativeEnum.php b/tests/Tests/Models/Enums/TypedCardNativeEnum.php new file mode 100644 index 00000000000..59e4eb00e55 --- /dev/null +++ b/tests/Tests/Models/Enums/TypedCardNativeEnum.php @@ -0,0 +1,23 @@ +_em->flush(); $this->_em->clear(); - $fetchedCard = $this->_em->find(Card::class, $card->id); + $fetchedCard = $this->_em->find($cardClass, $card->id); $this->assertInstanceOf(Suit::class, $fetchedCard->suit); $this->assertEquals(Suit::Clubs, $fetchedCard->suit); @@ -417,6 +421,10 @@ public function testFindByEnum(): void #[DataProvider('provideCardClasses')] public function testEnumWithNonMatchingDatabaseValueThrowsException(string $cardClass): void { + if ($cardClass === TypedCardNativeEnum::class) { + self::markTestSkipped('MySQL won\'t allow us to insert invalid values in this case.'); + } + $this->setUpEntitySchema([$cardClass]); $card = new $cardClass(); @@ -429,7 +437,7 @@ public function testEnumWithNonMatchingDatabaseValueThrowsException(string $card $metadata = $this->_em->getClassMetadata($cardClass); $this->_em->getConnection()->update( $metadata->table['name'], - [$metadata->fieldMappings['suit']->columnName => 'invalid'], + [$metadata->fieldMappings['suit']->columnName => 'Z'], [$metadata->fieldMappings['id']->columnName => $card->id], ); @@ -437,7 +445,7 @@ public function testEnumWithNonMatchingDatabaseValueThrowsException(string $card $this->expectExceptionMessage(sprintf( <<<'EXCEPTION' Context: Trying to hydrate enum property "%s::$suit" -Problem: Case "invalid" is not listed in enum "Doctrine\Tests\Models\Enums\Suit" +Problem: Case "Z" is not listed in enum "Doctrine\Tests\Models\Enums\Suit" Solution: Either add the case to the enum type or migrate the database column to use another case of the enum EXCEPTION , @@ -447,13 +455,16 @@ public function testEnumWithNonMatchingDatabaseValueThrowsException(string $card $this->_em->find($cardClass, $card->id); } - /** @return array */ - public static function provideCardClasses(): array + /** @return iterable */ + public static function provideCardClasses(): iterable { - return [ - Card::class => [Card::class], - TypedCard::class => [TypedCard::class], - ]; + yield Card::class => [Card::class]; + yield TypedCard::class => [TypedCard::class]; + + if (class_exists(EnumType::class)) { + yield CardNativeEnum::class => [CardNativeEnum::class]; + yield TypedCardNativeEnum::class => [TypedCardNativeEnum::class]; + } } public function testItAllowsReadingAttributes(): void From cf8f5f9f935455976f637051dacdcc7f5b5d42e1 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sat, 12 Oct 2024 21:01:04 +0200 Subject: [PATCH 21/21] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Grégoire Paris --- docs/en/reference/dql-doctrine-query-language.rst | 2 +- docs/en/reference/partial-objects.rst | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/en/reference/dql-doctrine-query-language.rst b/docs/en/reference/dql-doctrine-query-language.rst index 889013a3ba5..e668c08fd82 100644 --- a/docs/en/reference/dql-doctrine-query-language.rst +++ b/docs/en/reference/dql-doctrine-query-language.rst @@ -542,7 +542,7 @@ DQL keyword: $query = $em->createQuery('SELECT partial u.{id, username} FROM CmsUser u'); $users = $query->getResult(); // array of partially loaded CmsUser objects -You use the partial syntax when joining as well: +You can use the partial syntax when joining as well: .. code-block:: php diff --git a/docs/en/reference/partial-objects.rst b/docs/en/reference/partial-objects.rst index 3835123257a..3123c083f3f 100644 --- a/docs/en/reference/partial-objects.rst +++ b/docs/en/reference/partial-objects.rst @@ -5,7 +5,7 @@ A partial object is an object whose state is not fully initialized after being reconstituted from the database and that is disconnected from the rest of its data. The following section will describe why partial objects are problematic and what the approach -of Doctrine2 to this problem is. +of Doctrine to this problem is. .. note::