diff --git a/docs/en/cookbook/validation-of-entities.rst b/docs/en/cookbook/validation-of-entities.rst index a1e39a5e51b..a33cb983240 100644 --- a/docs/en/cookbook/validation-of-entities.rst +++ b/docs/en/cookbook/validation-of-entities.rst @@ -11,7 +11,7 @@ What we offer are hooks to execute any kind of validation. .. note:: You don't need to validate your entities in the lifecycle - events. Its only one of many options. Of course you can also + events. It is only one of many options. Of course you can also perform validations in value setters or any other method of your entities that are used in your code. diff --git a/docs/en/reference/association-mapping.rst b/docs/en/reference/association-mapping.rst index 6280eee0619..55dfe57671e 100644 --- a/docs/en/reference/association-mapping.rst +++ b/docs/en/reference/association-mapping.rst @@ -1300,8 +1300,8 @@ This is essentially the same as the following, more verbose, mapping: * @var Collection */ #[JoinTable(name: 'User_Group')] - #[JoinColumn(name: 'User_id', referencedColumnName: 'id')] - #[InverseJoinColumn(name: 'Group_id', referencedColumnName: 'id')] + #[JoinColumn(name: 'user_id', referencedColumnName: 'id')] + #[InverseJoinColumn(name: 'group_id', referencedColumnName: 'id')] #[ManyToMany(targetEntity: Group::class)] private Collection $groups; // ... @@ -1333,10 +1333,10 @@ This is essentially the same as the following, more verbose, mapping: - + - + diff --git a/src/Internal/Hydration/AbstractHydrator.php b/src/Internal/Hydration/AbstractHydrator.php index e9782e6d5fa..d7071cf7f68 100644 --- a/src/Internal/Hydration/AbstractHydrator.php +++ b/src/Internal/Hydration/AbstractHydrator.php @@ -182,29 +182,31 @@ public function toIterable($stmt, ResultSetMapping $resultSetMapping, array $hin $this->prepare(); - while (true) { - $row = $this->statement()->fetchAssociative(); - - if ($row === false) { - $this->cleanup(); + try { + while (true) { + $row = $this->statement()->fetchAssociative(); - break; - } + if ($row === false) { + break; + } - $result = []; + $result = []; - $this->hydrateRowData($row, $result); + $this->hydrateRowData($row, $result); - $this->cleanupAfterRowIteration(); - if (count($result) === 1) { - if (count($resultSetMapping->indexByMap) === 0) { - yield end($result); + $this->cleanupAfterRowIteration(); + if (count($result) === 1) { + if (count($resultSetMapping->indexByMap) === 0) { + yield end($result); + } else { + yield from $result; + } } else { - yield from $result; + yield $result; } - } else { - yield $result; } + } finally { + $this->cleanup(); } } diff --git a/src/ORMException.php b/src/ORMException.php index eb5aa6c7b40..898c20ecb0c 100644 --- a/src/ORMException.php +++ b/src/ORMException.php @@ -5,11 +5,31 @@ namespace Doctrine\ORM; use Doctrine\Common\Cache\Cache as CacheDriver; -use Doctrine\Persistence\ObjectRepository; +use Doctrine\ORM\Cache\Exception\InvalidResultCacheDriver; +use Doctrine\ORM\Cache\Exception\MetadataCacheNotConfigured; +use Doctrine\ORM\Cache\Exception\MetadataCacheUsesNonPersistentCache; +use Doctrine\ORM\Cache\Exception\QueryCacheNotConfigured; +use Doctrine\ORM\Cache\Exception\QueryCacheUsesNonPersistentCache; +use Doctrine\ORM\Exception\EntityManagerClosed; +use Doctrine\ORM\Exception\InvalidEntityRepository; +use Doctrine\ORM\Exception\InvalidHydrationMode; +use Doctrine\ORM\Exception\MismatchedEventManager; +use Doctrine\ORM\Exception\MissingIdentifierField; +use Doctrine\ORM\Exception\MissingMappingDriverImplementation; +use Doctrine\ORM\Exception\NamedNativeQueryNotFound; +use Doctrine\ORM\Exception\NamedQueryNotFound; +use Doctrine\ORM\Exception\ProxyClassesAlwaysRegenerating; +use Doctrine\ORM\Exception\UnexpectedAssociationValue; +use Doctrine\ORM\Exception\UnknownEntityNamespace; +use Doctrine\ORM\Exception\UnrecognizedIdentifierFields; +use Doctrine\ORM\Persisters\Exception\CantUseInOperatorOnCompositeKeys; +use Doctrine\ORM\Persisters\Exception\InvalidOrientation; +use Doctrine\ORM\Persisters\Exception\UnrecognizedField; +use Doctrine\ORM\Repository\Exception\InvalidFindByCall; +use Doctrine\ORM\Repository\Exception\InvalidMagicMethodCall; +use Doctrine\ORM\Tools\Exception\NotSupported; use Exception; -use function get_debug_type; -use function implode; use function sprintf; /** @@ -26,8 +46,7 @@ class ORMException extends Exception */ public static function missingMappingDriverImpl() { - return new self("It's a requirement to specify a Metadata Driver and pass it " . - 'to Doctrine\\ORM\\Configuration::setMetadataDriverImpl().'); + return MissingMappingDriverImplementation::create(); } /** @@ -39,11 +58,11 @@ public static function missingMappingDriverImpl() */ public static function namedQueryNotFound($queryName) { - return new self('Could not find a named query by the name "' . $queryName . '"'); + return NamedQueryNotFound::fromName($queryName); } /** - * @deprecated Use Doctrine\ORM\Exception\NamedQueryNotFound + * @deprecated Use Doctrine\ORM\Exception\NamedNativeQueryNotFound * * @param string $nativeQueryName * @@ -51,7 +70,7 @@ public static function namedQueryNotFound($queryName) */ public static function namedNativeQueryNotFound($nativeQueryName) { - return new self('Could not find a named native query by the name "' . $nativeQueryName . '"'); + return NamedNativeQueryNotFound::fromName($nativeQueryName); } /** @@ -63,7 +82,7 @@ public static function namedNativeQueryNotFound($nativeQueryName) */ public static function unrecognizedField($field) { - return new self(sprintf('Unrecognized field: %s', $field)); + return new UnrecognizedField(sprintf('Unrecognized field: %s', $field)); } /** @@ -78,7 +97,7 @@ public static function unrecognizedField($field) */ public static function unexpectedAssociationValue($class, $association, $given, $expected) { - return new self(sprintf('Found entity of type %s on association %s#%s, but expecting %s', $given, $class, $association, $expected)); + return UnexpectedAssociationValue::create($class, $association, $given, $expected); } /** @@ -91,7 +110,7 @@ public static function unexpectedAssociationValue($class, $association, $given, */ public static function invalidOrientation($className, $field) { - return new self('Invalid order by orientation specified for ' . $className . '#' . $field); + return InvalidOrientation::fromClassNameAndField($className, $field); } /** @@ -101,7 +120,7 @@ public static function invalidOrientation($className, $field) */ public static function entityManagerClosed() { - return new self('The EntityManager is closed.'); + return EntityManagerClosed::create(); } /** @@ -113,7 +132,7 @@ public static function entityManagerClosed() */ public static function invalidHydrationMode($mode) { - return new self(sprintf("'%s' is an invalid hydration mode.", $mode)); + return InvalidHydrationMode::fromMode($mode); } /** @@ -123,7 +142,7 @@ public static function invalidHydrationMode($mode) */ public static function mismatchedEventManager() { - return new self('Cannot use different EventManager instances for EntityManager and Connection.'); + return MismatchedEventManager::create(); } /** @@ -135,11 +154,11 @@ public static function mismatchedEventManager() */ public static function findByRequiresParameter($methodName) { - return new self("You need to pass a parameter to '" . $methodName . "'"); + return InvalidMagicMethodCall::onMissingParameter($methodName); } /** - * @deprecated Doctrine\ORM\Repository\Exception\InvalidFindByCall + * @deprecated Doctrine\ORM\Repository\Exception\InvalidMagicMethodCall::becauseFieldNotFoundIn() * * @param string $entityName * @param string $fieldName @@ -149,10 +168,7 @@ public static function findByRequiresParameter($methodName) */ public static function invalidMagicCall($entityName, $fieldName, $method) { - return new self( - "Entity '" . $entityName . "' has no field '" . $fieldName . "'. " . - "You can therefore not call '" . $method . "' on the entities' repository" - ); + return InvalidMagicMethodCall::becauseFieldNotFoundIn($entityName, $fieldName, $method); } /** @@ -165,10 +181,7 @@ public static function invalidMagicCall($entityName, $fieldName, $method) */ public static function invalidFindByInverseAssociation($entityName, $associationFieldName) { - return new self( - "You cannot search for the association field '" . $entityName . '#' . $associationFieldName . "', " . - 'because it is the inverse side of an association. Find methods only work on owning side associations.' - ); + return InvalidFindByCall::fromInverseSideUsage($entityName, $associationFieldName); } /** @@ -178,7 +191,7 @@ public static function invalidFindByInverseAssociation($entityName, $association */ public static function invalidResultCacheDriver() { - return new self('Invalid result cache driver; it must implement Doctrine\\Common\\Cache\\Cache.'); + return InvalidResultCacheDriver::create(); } /** @@ -188,7 +201,7 @@ public static function invalidResultCacheDriver() */ public static function notSupported() { - return new self('This behaviour is (currently) not supported by Doctrine 2'); + return NotSupported::create(); } /** @@ -198,7 +211,7 @@ public static function notSupported() */ public static function queryCacheNotConfigured() { - return new self('Query Cache is not configured.'); + return QueryCacheNotConfigured::create(); } /** @@ -208,7 +221,7 @@ public static function queryCacheNotConfigured() */ public static function metadataCacheNotConfigured() { - return new self('Class Metadata Cache is not configured.'); + return MetadataCacheNotConfigured::create(); } /** @@ -218,7 +231,7 @@ public static function metadataCacheNotConfigured() */ public static function queryCacheUsesNonPersistentCache(CacheDriver $cache) { - return new self('Query Cache uses a non-persistent cache driver, ' . get_debug_type($cache) . '.'); + return QueryCacheUsesNonPersistentCache::fromDriver($cache); } /** @@ -228,7 +241,7 @@ public static function queryCacheUsesNonPersistentCache(CacheDriver $cache) */ public static function metadataCacheUsesNonPersistentCache(CacheDriver $cache) { - return new self('Metadata Cache uses a non-persistent cache driver, ' . get_debug_type($cache) . '.'); + return MetadataCacheUsesNonPersistentCache::fromDriver($cache); } /** @@ -238,7 +251,7 @@ public static function metadataCacheUsesNonPersistentCache(CacheDriver $cache) */ public static function proxyClassesAlwaysRegenerating() { - return new self('Proxy Classes are always regenerating.'); + return ProxyClassesAlwaysRegenerating::create(); } /** @@ -250,9 +263,7 @@ public static function proxyClassesAlwaysRegenerating() */ public static function unknownEntityNamespace($entityNamespaceAlias) { - return new self( - sprintf("Unknown Entity namespace alias '%s'.", $entityNamespaceAlias) - ); + return UnknownEntityNamespace::fromNamespaceAlias($entityNamespaceAlias); } /** @@ -264,11 +275,7 @@ public static function unknownEntityNamespace($entityNamespaceAlias) */ public static function invalidEntityRepository($className) { - return new self(sprintf( - "Invalid repository class '%s'. It must be a %s.", - $className, - ObjectRepository::class - )); + return InvalidEntityRepository::fromClassName($className); } /** @@ -281,7 +288,7 @@ public static function invalidEntityRepository($className) */ public static function missingIdentifierField($className, $fieldName) { - return new self(sprintf('The identifier %s is missing for a query of %s', $fieldName, $className)); + return MissingIdentifierField::fromFieldAndClass($fieldName, $className); } /** @@ -294,10 +301,7 @@ public static function missingIdentifierField($className, $fieldName) */ public static function unrecognizedIdentifierFields($className, $fieldNames) { - return new self( - "Unrecognized identifier fields: '" . implode("', '", $fieldNames) . "' " . - "are not present on class '" . $className . "'." - ); + return UnrecognizedIdentifierFields::fromClassAndFieldNames($className, $fieldNames); } /** @@ -307,6 +311,6 @@ public static function unrecognizedIdentifierFields($className, $fieldNames) */ public static function cantUseInOperatorOnCompositeKeys() { - return new self("Can't use IN operator on entities that have composite keys."); + return CantUseInOperatorOnCompositeKeys::create(); } } diff --git a/src/UnitOfWork.php b/src/UnitOfWork.php index 25b88221583..f1affcf7ebc 100644 --- a/src/UnitOfWork.php +++ b/src/UnitOfWork.php @@ -1292,6 +1292,8 @@ private function executeDeletions(): void $eventsToDispatch = []; foreach ($entities as $entity) { + $this->removeFromIdentityMap($entity); + $oid = spl_object_id($entity); $class = $this->em->getClassMetadata(get_class($entity)); $persister = $this->getEntityPersister($class->name); @@ -1667,8 +1669,6 @@ public function scheduleForDelete($entity) return; } - $this->removeFromIdentityMap($entity); - unset($this->entityUpdates[$oid]); if (! isset($this->entityDeletions[$oid])) { @@ -3224,7 +3224,13 @@ public function triggerEagerLoads() * * @param PersistentCollection[] $collections * @param array $mapping - * @psalm-param array{targetEntity: class-string, sourceEntity: class-string, mappedBy: string, indexBy: string|null} $mapping + * @psalm-param array{ + * targetEntity: class-string, + * sourceEntity: class-string, + * mappedBy: string, + * indexBy: string|null, + * orderBy: array|null + * } $mapping */ private function eagerLoadCollections(array $collections, array $mapping): void { @@ -3241,7 +3247,7 @@ private function eagerLoadCollections(array $collections, array $mapping): void $entities[] = $collection->getOwner(); } - $found = $this->getEntityPersister($targetEntity)->loadAll([$mappedBy => $entities]); + $found = $this->getEntityPersister($targetEntity)->loadAll([$mappedBy => $entities], $mapping['orderBy'] ?? null); $targetClass = $this->em->getClassMetadata($targetEntity); $targetProperty = $targetClass->getReflectionProperty($mappedBy); diff --git a/tests/Tests/ORM/Functional/QueryDqlFunctionTest.php b/tests/Tests/ORM/Functional/QueryDqlFunctionTest.php index e1ad89ecc2d..f5dead66a31 100644 --- a/tests/Tests/ORM/Functional/QueryDqlFunctionTest.php +++ b/tests/Tests/ORM/Functional/QueryDqlFunctionTest.php @@ -249,7 +249,6 @@ public function testOperatorMultiply(): void self::assertEquals(1600000, $result[3]['op']); } - /** @group test */ public function testOperatorDiv(): void { $result = $this->_em->createQuery('SELECT m, (m.salary/0.5) AS op FROM Doctrine\Tests\Models\Company\CompanyManager m ORDER BY m.salary ASC') diff --git a/tests/Tests/ORM/Functional/Ticket/GH11163Test.php b/tests/Tests/ORM/Functional/Ticket/GH11163Test.php new file mode 100644 index 00000000000..927457840c5 --- /dev/null +++ b/tests/Tests/ORM/Functional/Ticket/GH11163Test.php @@ -0,0 +1,135 @@ +setUpEntitySchema([ + GH11163Bucket::class, + GH11163BucketItem::class, + ]); + } + + public function tearDown(): void + { + parent::tearDown(); + + $conn = static::$sharedConn; + $conn->executeStatement('DELETE FROM GH11163BucketItem'); + $conn->executeStatement('DELETE FROM GH11163Bucket'); + } + + public function testFetchEagerModeWithOrderBy(): void + { + // Load entities into database + $this->_em->persist($bucket = new GH11163Bucket(11163)); + $this->_em->persist(new GH11163BucketItem(1, $bucket, 2)); + $this->_em->persist(new GH11163BucketItem(2, $bucket, 3)); + $this->_em->persist(new GH11163BucketItem(3, $bucket, 1)); + $this->_em->flush(); + $this->_em->clear(); + + // Fetch entity from database + $dql = 'SELECT bucket FROM ' . GH11163Bucket::class . ' bucket WHERE bucket.id = :id'; + $bucket = $this->_em->createQuery($dql) + ->setParameter('id', 11163) + ->getSingleResult(); + + // Assert associated entity is loaded eagerly + static::assertInstanceOf(GH11163Bucket::class, $bucket); + static::assertInstanceOf(PersistentCollection::class, $bucket->items); + static::assertTrue($bucket->items->isInitialized()); + + static::assertCount(3, $bucket->items); + + // Assert order of entities + static::assertSame(1, $bucket->items[0]->position); + static::assertSame(3, $bucket->items[0]->id); + + static::assertSame(2, $bucket->items[1]->position); + static::assertSame(1, $bucket->items[1]->id); + + static::assertSame(3, $bucket->items[2]->position); + static::assertSame(2, $bucket->items[2]->id); + } +} + +/** + * @ORM\Entity + */ +class GH11163Bucket +{ + /** + * @ORM\Id + * @ORM\Column(type="integer") + * + * @var int + */ + private $id; + + /** + * @ORM\OneToMany( + * targetEntity=GH11163BucketItem::class, + * mappedBy="bucket", + * fetch="EAGER" + * ) + * @ORM\OrderBy({"position" = "ASC"}) + * + * @var Collection + */ + public $items; + + public function __construct(int $id) + { + $this->id = $id; + $this->items = new ArrayCollection(); + } +} + +/** + * @ORM\Entity + */ +class GH11163BucketItem +{ + /** + * @ORM\ManyToOne(targetEntity=GH11163Bucket::class, inversedBy="items") + * @ORM\JoinColumn(nullable=false) + * + * @var GH11163Bucket + */ + private $bucket; + + /** + * @ORM\Id + * @ORM\Column(type="integer") + * + * @var int + */ + public $id; + + /** + * @ORM\Column(type="integer", nullable=false) + * + * @var int + */ + public $position; + + public function __construct(int $id, GH11163Bucket $bucket, int $position) + { + $this->id = $id; + $this->bucket = $bucket; + $this->position = $position; + } +} diff --git a/tests/Tests/ORM/Functional/Ticket/GH6123Test.php b/tests/Tests/ORM/Functional/Ticket/GH6123Test.php new file mode 100644 index 00000000000..05a1a765f27 --- /dev/null +++ b/tests/Tests/ORM/Functional/Ticket/GH6123Test.php @@ -0,0 +1,88 @@ +createSchemaForModels( + GH6123Entity::class + ); + } + + public function testLoadingRemovedEntityFromDatabaseDoesNotCreateNewManagedEntityInstance(): void + { + $entity = new GH6123Entity(); + $this->_em->persist($entity); + $this->_em->flush(); + + self::assertSame(UnitOfWork::STATE_MANAGED, $this->_em->getUnitOfWork()->getEntityState($entity)); + self::assertFalse($this->_em->getUnitOfWork()->isScheduledForDelete($entity)); + + $this->_em->remove($entity); + + $freshEntity = $this->loadEntityFromDatabase($entity->id); + self::assertSame($entity, $freshEntity); + + self::assertSame(UnitOfWork::STATE_REMOVED, $this->_em->getUnitOfWork()->getEntityState($freshEntity)); + self::assertTrue($this->_em->getUnitOfWork()->isScheduledForDelete($freshEntity)); + } + + public function testRemovedEntityCanBePersistedAgain(): void + { + $entity = new GH6123Entity(); + $this->_em->persist($entity); + $this->_em->flush(); + + $this->_em->remove($entity); + self::assertSame(UnitOfWork::STATE_REMOVED, $this->_em->getUnitOfWork()->getEntityState($entity)); + self::assertTrue($this->_em->getUnitOfWork()->isScheduledForDelete($entity)); + + $this->loadEntityFromDatabase($entity->id); + + $this->_em->persist($entity); + self::assertSame(UnitOfWork::STATE_MANAGED, $this->_em->getUnitOfWork()->getEntityState($entity)); + self::assertFalse($this->_em->getUnitOfWork()->isScheduledForDelete($entity)); + + $this->_em->flush(); + } + + private function loadEntityFromDatabase(int $id): ?GH6123Entity + { + return $this->_em->createQueryBuilder() + ->select('e') + ->from(GH6123Entity::class, 'e') + ->where('e.id = :id') + ->setParameter('id', $id) + ->getQuery() + ->getOneOrNullResult(); + } +} + +/** + * @ORM\Entity + */ +#[ORM\Entity] +class GH6123Entity +{ + /** + * @ORM\Id + * @ORM\GeneratedValue + * @ORM\Column(type="integer") + * @var int + */ + #[ORM\Id] + #[ORM\GeneratedValue] + #[ORM\Column(type: Types::INTEGER)] + public $id; +} diff --git a/tests/Tests/ORM/Hydration/AbstractHydratorTest.php b/tests/Tests/ORM/Hydration/AbstractHydratorTest.php index 24ababac507..c08c24c0e5c 100644 --- a/tests/Tests/ORM/Hydration/AbstractHydratorTest.php +++ b/tests/Tests/ORM/Hydration/AbstractHydratorTest.php @@ -12,6 +12,7 @@ use Doctrine\ORM\Internal\Hydration\AbstractHydrator; use Doctrine\ORM\ORMException; use Doctrine\ORM\Query\ResultSetMapping; +use Doctrine\Tests\Models\Hydration\SimpleEntity; use Doctrine\Tests\OrmFunctionalTestCase; use PHPUnit\Framework\MockObject\MockObject; @@ -154,4 +155,33 @@ public function testHydrateAllClearsAllAttachedListenersEvenOnError(): void $this->expectException(ORMException::class); $this->hydrator->hydrateAll($this->mockResult, $this->mockResultMapping); } + + public function testToIterableIfYieldAndBreakBeforeFinishAlwaysCleansUp(): void + { + $this->setUpEntitySchema([SimpleEntity::class]); + + $entity1 = new SimpleEntity(); + $this->_em->persist($entity1); + $entity2 = new SimpleEntity(); + $this->_em->persist($entity2); + + $this->_em->flush(); + $this->_em->clear(); + + $evm = $this->_em->getEventManager(); + + $q = $this->_em->createQuery('SELECT e.id FROM ' . SimpleEntity::class . ' e'); + + // select two entities, but do no iterate + $q->toIterable(); + self::assertCount(0, $evm->getListeners(Events::onClear)); + + // select two entities, but abort after first record + foreach ($q->toIterable() as $result) { + self::assertCount(1, $evm->getListeners(Events::onClear)); + break; + } + + self::assertCount(0, $evm->getListeners(Events::onClear)); + } } diff --git a/tests/Tests/ORM/UnitOfWorkTest.php b/tests/Tests/ORM/UnitOfWorkTest.php index 0569bfd06dd..ee475e729d0 100644 --- a/tests/Tests/ORM/UnitOfWorkTest.php +++ b/tests/Tests/ORM/UnitOfWorkTest.php @@ -413,12 +413,18 @@ public function testRemovedAndRePersistedEntitiesAreInTheIdentityMapAndAreNotGar $entity->id = 123; $this->_unitOfWork->registerManaged($entity, ['id' => 123], []); + self::assertSame(UnitOfWork::STATE_MANAGED, $this->_unitOfWork->getEntityState($entity)); + self::assertFalse($this->_unitOfWork->isScheduledForDelete($entity)); self::assertTrue($this->_unitOfWork->isInIdentityMap($entity)); $this->_unitOfWork->remove($entity); - self::assertFalse($this->_unitOfWork->isInIdentityMap($entity)); + self::assertSame(UnitOfWork::STATE_REMOVED, $this->_unitOfWork->getEntityState($entity)); + self::assertTrue($this->_unitOfWork->isScheduledForDelete($entity)); + self::assertTrue($this->_unitOfWork->isInIdentityMap($entity)); $this->_unitOfWork->persist($entity); + self::assertSame(UnitOfWork::STATE_MANAGED, $this->_unitOfWork->getEntityState($entity)); + self::assertFalse($this->_unitOfWork->isScheduledForDelete($entity)); self::assertTrue($this->_unitOfWork->isInIdentityMap($entity)); }