From 127cee41ec8a13597b28f6925a007cb2d5bbf5cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9goire=20Paris?= Date: Sat, 25 May 2024 15:07:03 +0200 Subject: [PATCH] Port d05e8e8285a052ad715cd6fb4d6d80890cf3c4d8 The original commit has been made backward-compatible, and some properties of the entity manager have been marked as readonly. Closes #5933 --- UPGRADE.md | 20 +++++++++ psalm-baseline.xml | 3 ++ src/Cache/DefaultQueryCache.php | 36 +++++++-------- .../Entity/AbstractEntityPersister.php | 18 +++++--- ...onStrictReadWriteCachedEntityPersister.php | 4 +- .../Entity/ReadWriteCachedEntityPersister.php | 4 +- src/Decorator/EntityManagerDecorator.php | 4 +- src/EntityManager.php | 44 +++++++------------ src/EntityManagerInterface.php | 6 +++ src/Exception/EntityManagerClosed.php | 3 +- src/UnitOfWork.php | 2 +- tests/Tests/ORM/EntityManagerTest.php | 33 +------------- 12 files changed, 83 insertions(+), 94 deletions(-) diff --git a/UPGRADE.md b/UPGRADE.md index 1869e9f28ff..eaac72f5020 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -1,3 +1,23 @@ +# Upgrade to 3.3 + +## Deprecated closed status for EntityManager + +It is always open now. As a consequence, the following methods, classes and +properties have been deprecated and will be removed in 4.0: + +- `EntityManagerInterface::isOpen()`. All implementations should return `true`. +- `EntityManagerInterface::close()`. All implementations and callers should call `clear()` instead. +- `AbstractEntityPersister::$uow`: get the unit of work from `AbstractEntityPersister::$em` instead + +Mutating the following properties is deprecated and will not be possible in 4.0: + +- `EntityManager::$config` +- `EntityManager::$conn` +- `EntityManager::$eventManager` +- `EntityManager::$metadataFactory` +- `EntityManager::$proxyFactory` +- `EntityManager::$repositoryFactory` + # Upgrade to 3.2 ## Deprecate the `NotSupported` exception diff --git a/psalm-baseline.xml b/psalm-baseline.xml index e69c3fdb6c1..cb5f97f70c1 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -106,6 +106,9 @@ + + uow]]> + class]]> diff --git a/src/Cache/DefaultQueryCache.php b/src/Cache/DefaultQueryCache.php index f3bb8ac95c8..fb72885065a 100644 --- a/src/Cache/DefaultQueryCache.php +++ b/src/Cache/DefaultQueryCache.php @@ -16,7 +16,6 @@ use Doctrine\ORM\PersistentCollection; use Doctrine\ORM\Query; use Doctrine\ORM\Query\ResultSetMapping; -use Doctrine\ORM\UnitOfWork; use function array_map; use function array_shift; @@ -32,7 +31,6 @@ */ class DefaultQueryCache implements QueryCache { - private readonly UnitOfWork $uow; private readonly QueryCacheValidator $validator; protected CacheLogger|null $cacheLogger = null; @@ -45,7 +43,6 @@ public function __construct( ) { $cacheConfig = $em->getConfiguration()->getSecondLevelCacheConfiguration(); - $this->uow = $em->getUnitOfWork(); $this->cacheLogger = $cacheConfig->getCacheLogger(); $this->validator = $cacheConfig->getQueryValidator(); } @@ -74,7 +71,8 @@ public function get(QueryCacheKey $key, ResultSetMapping $rsm, array $hints = [] $result = []; $entityName = reset($rsm->aliasMap); $hasRelation = ! empty($rsm->relationMap); - $persister = $this->uow->getEntityPersister($entityName); + $uow = $this->em->getUnitOfWork(); + $persister = $uow->getEntityPersister($entityName); assert($persister instanceof CachedEntityPersister); $region = $persister->getCacheRegion(); @@ -100,7 +98,7 @@ public function get(QueryCacheKey $key, ResultSetMapping $rsm, array $hints = [] $this->cacheLogger?->entityCacheHit($regionName, $cacheKeys->identifiers[$index]); if (! $hasRelation) { - $result[$index] = $this->uow->createEntity($entityEntry->class, $entityEntry->resolveAssociationEntries($this->em), self::$hints); + $result[$index] = $uow->createEntity($entityEntry->class, $entityEntry->resolveAssociationEntries($this->em), self::$hints); continue; } @@ -108,7 +106,7 @@ public function get(QueryCacheKey $key, ResultSetMapping $rsm, array $hints = [] $data = $entityEntry->data; foreach ($entry['associations'] as $name => $assoc) { - $assocPersister = $this->uow->getEntityPersister($assoc['targetEntity']); + $assocPersister = $uow->getEntityPersister($assoc['targetEntity']); assert($assocPersister instanceof CachedEntityPersister); $assocRegion = $assocPersister->getCacheRegion(); @@ -121,12 +119,12 @@ public function get(QueryCacheKey $key, ResultSetMapping $rsm, array $hints = [] if ($assocEntry === null) { $this->cacheLogger?->entityCacheMiss($assocRegion->getName(), $assocKey); - $this->uow->hydrationComplete(); + $uow->hydrationComplete(); return null; } - $data[$name] = $this->uow->createEntity($assocEntry->class, $assocEntry->resolveAssociationEntries($this->em), self::$hints); + $data[$name] = $uow->createEntity($assocEntry->class, $assocEntry->resolveAssociationEntries($this->em), self::$hints); $this->cacheLogger?->entityCacheHit($assocRegion->getName(), $assocKey); @@ -149,12 +147,12 @@ public function get(QueryCacheKey $key, ResultSetMapping $rsm, array $hints = [] if ($assocEntry === null) { $this->cacheLogger?->entityCacheMiss($assocRegion->getName(), $assocKeys->identifiers[$assocIndex]); - $this->uow->hydrationComplete(); + $uow->hydrationComplete(); return null; } - $element = $this->uow->createEntity($assocEntry->class, $assocEntry->resolveAssociationEntries($this->em), self::$hints); + $element = $uow->createEntity($assocEntry->class, $assocEntry->resolveAssociationEntries($this->em), self::$hints); $collection->hydrateSet($assocIndex, $element); @@ -185,10 +183,10 @@ public function get(QueryCacheKey $key, ResultSetMapping $rsm, array $hints = [] } } - $result[$index] = $this->uow->createEntity($entityEntry->class, $data, self::$hints); + $result[$index] = $uow->createEntity($entityEntry->class, $data, self::$hints); } - $this->uow->hydrationComplete(); + $uow->hydrationComplete(); return $result; } @@ -217,7 +215,8 @@ public function put(QueryCacheKey $key, ResultSetMapping $rsm, mixed $result, ar $data = []; $entityName = reset($rsm->aliasMap); $rootAlias = key($rsm->aliasMap); - $persister = $this->uow->getEntityPersister($entityName); + $uow = $this->em->getUnitOfWork(); + $persister = $uow->getEntityPersister($entityName); if (! $persister instanceof CachedEntityPersister) { throw NonCacheableEntity::fromEntity($entityName); @@ -229,7 +228,7 @@ public function put(QueryCacheKey $key, ResultSetMapping $rsm, mixed $result, ar assert($cm instanceof ClassMetadata); foreach ($result as $index => $entity) { - $identifier = $this->uow->getEntityIdentifier($entity); + $identifier = $uow->getEntityIdentifier($entity); $entityKey = new EntityCacheKey($cm->rootEntityName, $identifier); if (($key->cacheMode & Cache::MODE_REFRESH) || ! $region->contains($entityKey)) { @@ -296,16 +295,17 @@ public function put(QueryCacheKey $key, ResultSetMapping $rsm, mixed $result, ar */ private function storeAssociationCache(QueryCacheKey $key, AssociationMapping $assoc, mixed $assocValue): array|null { - $assocPersister = $this->uow->getEntityPersister($assoc->targetEntity); + $uow = $this->em->getUnitOfWork(); + $assocPersister = $uow->getEntityPersister($assoc->targetEntity); $assocMetadata = $assocPersister->getClassMetadata(); $assocRegion = $assocPersister->getCacheRegion(); // Handle *-to-one associations if ($assoc->isToOne()) { - $assocIdentifier = $this->uow->getEntityIdentifier($assocValue); + $assocIdentifier = $uow->getEntityIdentifier($assocValue); $entityKey = new EntityCacheKey($assocMetadata->rootEntityName, $assocIdentifier); - if (! $this->uow->isUninitializedObject($assocValue) && ($key->cacheMode & Cache::MODE_REFRESH) || ! $assocRegion->contains($entityKey)) { + if (! $uow->isUninitializedObject($assocValue) && ($key->cacheMode & Cache::MODE_REFRESH) || ! $assocRegion->contains($entityKey)) { // Entity put fail if (! $assocPersister->storeEntityCache($assocValue, $entityKey)) { return null; @@ -323,7 +323,7 @@ private function storeAssociationCache(QueryCacheKey $key, AssociationMapping $a $list = []; foreach ($assocValue as $assocItemIndex => $assocItem) { - $assocIdentifier = $this->uow->getEntityIdentifier($assocItem); + $assocIdentifier = $uow->getEntityIdentifier($assocItem); $entityKey = new EntityCacheKey($assocMetadata->rootEntityName, $assocIdentifier); if (($key->cacheMode & Cache::MODE_REFRESH) || ! $assocRegion->contains($entityKey)) { diff --git a/src/Cache/Persister/Entity/AbstractEntityPersister.php b/src/Cache/Persister/Entity/AbstractEntityPersister.php index 9f371d8b205..9b5f48f9e48 100644 --- a/src/Cache/Persister/Entity/AbstractEntityPersister.php +++ b/src/Cache/Persister/Entity/AbstractEntityPersister.php @@ -34,6 +34,7 @@ abstract class AbstractEntityPersister implements CachedEntityPersister { + /** @deprecated get the unit of work with $this->em->getUnitOfWork() instead */ protected UnitOfWork $uow; protected ClassMetadataFactory $metadataFactory; @@ -57,7 +58,7 @@ abstract class AbstractEntityPersister implements CachedEntityPersister public function __construct( protected EntityPersister $persister, protected Region $region, - EntityManagerInterface $em, + protected EntityManagerInterface $em, protected ClassMetadata $class, ) { $configuration = $em->getConfiguration(); @@ -190,10 +191,11 @@ private function storeJoinedAssociations(object $entity): void continue; } - $assocId = $this->uow->getEntityIdentifier($assocEntity); + $uow = $this->em->getUnitOfWork(); + $assocId = $uow->getEntityIdentifier($assocEntity); $assocMetadata = $this->metadataFactory->getMetadataFor($assoc->targetEntity); $assocKey = new EntityCacheKey($assocMetadata->rootEntityName, $assocId); - $assocPersister = $this->uow->getEntityPersister($assoc->targetEntity); + $assocPersister = $uow->getEntityPersister($assoc->targetEntity); $assocPersister->storeEntityCache($assocEntity, $assocKey); } @@ -465,14 +467,15 @@ public function loadManyToManyCollection( object $sourceEntity, PersistentCollection $collection, ): array { - $persister = $this->uow->getCollectionPersister($assoc); + $uow = $this->em->getUnitOfWork(); + $persister = $uow->getCollectionPersister($assoc); $hasCache = ($persister instanceof CachedPersister); if (! $hasCache) { return $this->persister->loadManyToManyCollection($assoc, $sourceEntity, $collection); } - $ownerId = $this->uow->getEntityIdentifier($collection->getOwner()); + $ownerId = $uow->getEntityIdentifier($collection->getOwner()); $key = $this->buildCollectionCacheKey($assoc, $ownerId); $list = $persister->loadCollectionCache($collection, $key); @@ -496,14 +499,15 @@ public function loadOneToManyCollection( object $sourceEntity, PersistentCollection $collection, ): mixed { - $persister = $this->uow->getCollectionPersister($assoc); + $uow = $this->em->getUnitOfWork(); + $persister = $uow->getCollectionPersister($assoc); $hasCache = ($persister instanceof CachedPersister); if (! $hasCache) { return $this->persister->loadOneToManyCollection($assoc, $sourceEntity, $collection); } - $ownerId = $this->uow->getEntityIdentifier($collection->getOwner()); + $ownerId = $uow->getEntityIdentifier($collection->getOwner()); $key = $this->buildCollectionCacheKey($assoc, $ownerId); $list = $persister->loadCollectionCache($collection, $key); diff --git a/src/Cache/Persister/Entity/NonStrictReadWriteCachedEntityPersister.php b/src/Cache/Persister/Entity/NonStrictReadWriteCachedEntityPersister.php index 43c76ab2cac..a9a8bf47c6a 100644 --- a/src/Cache/Persister/Entity/NonStrictReadWriteCachedEntityPersister.php +++ b/src/Cache/Persister/Entity/NonStrictReadWriteCachedEntityPersister.php @@ -49,7 +49,7 @@ public function afterTransactionRolledBack(): void public function delete(object $entity): bool { - $key = new EntityCacheKey($this->class->rootEntityName, $this->uow->getEntityIdentifier($entity)); + $key = new EntityCacheKey($this->class->rootEntityName, $this->em->getUnitOfWork()->getEntityIdentifier($entity)); $deleted = $this->persister->delete($entity); if ($deleted) { @@ -71,7 +71,7 @@ public function update(object $entity): void private function updateCache(object $entity, bool $isChanged): bool { $class = $this->metadataFactory->getMetadataFor($entity::class); - $key = new EntityCacheKey($class->rootEntityName, $this->uow->getEntityIdentifier($entity)); + $key = new EntityCacheKey($class->rootEntityName, $this->em->getUnitOfWork()->getEntityIdentifier($entity)); $entry = $this->hydrator->buildCacheEntry($class, $key, $entity); $cached = $this->region->put($key, $entry); $isChanged = $isChanged || $cached; diff --git a/src/Cache/Persister/Entity/ReadWriteCachedEntityPersister.php b/src/Cache/Persister/Entity/ReadWriteCachedEntityPersister.php index a1ea0dcf07e..0683ce606ef 100644 --- a/src/Cache/Persister/Entity/ReadWriteCachedEntityPersister.php +++ b/src/Cache/Persister/Entity/ReadWriteCachedEntityPersister.php @@ -66,7 +66,7 @@ public function afterTransactionRolledBack(): void public function delete(object $entity): bool { - $key = new EntityCacheKey($this->class->rootEntityName, $this->uow->getEntityIdentifier($entity)); + $key = new EntityCacheKey($this->class->rootEntityName, $this->em->getUnitOfWork()->getEntityIdentifier($entity)); $lock = $this->region->lock($key); $deleted = $this->persister->delete($entity); @@ -88,7 +88,7 @@ public function delete(object $entity): bool public function update(object $entity): void { - $key = new EntityCacheKey($this->class->rootEntityName, $this->uow->getEntityIdentifier($entity)); + $key = new EntityCacheKey($this->class->rootEntityName, $this->em->getUnitOfWork()->getEntityIdentifier($entity)); $lock = $this->region->lock($key); $this->persister->update($entity); diff --git a/src/Decorator/EntityManagerDecorator.php b/src/Decorator/EntityManagerDecorator.php index 6f1b0419686..f9cd7733392 100644 --- a/src/Decorator/EntityManagerDecorator.php +++ b/src/Decorator/EntityManagerDecorator.php @@ -104,7 +104,7 @@ public function getReference(string $entityName, mixed $id): object|null public function close(): void { - $this->wrapped->close(); + $this->wrapped->clear(); } public function lock(object $entity, LockMode|int $lockMode, DateTimeInterface|int|null $lockVersion = null): void @@ -134,7 +134,7 @@ public function getConfiguration(): Configuration public function isOpen(): bool { - return $this->wrapped->isOpen(); + return true; } public function getUnitOfWork(): UnitOfWork diff --git a/src/EntityManager.php b/src/EntityManager.php index 8045ac2f5e3..411d2c93ec6 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\ORM\Exception\EntityManagerClosed; use Doctrine\ORM\Exception\InvalidHydrationMode; use Doctrine\ORM\Exception\MissingIdentifierField; use Doctrine\ORM\Exception\MissingMappingDriverImplementation; @@ -62,6 +61,8 @@ class EntityManager implements EntityManagerInterface { /** * The metadata factory, used to retrieve the ORM metadata of entity classes. + * + * @readonly */ private ClassMetadataFactory $metadataFactory; @@ -72,16 +73,22 @@ class EntityManager implements EntityManagerInterface /** * The event manager that is the central point of the event system. + * + * @readonly */ private EventManager $eventManager; /** * The proxy factory used to create dynamic proxies. + * + * @readonly */ private ProxyFactory $proxyFactory; /** * The repository factory used to create dynamic repositories. + * + * @readonly */ private RepositoryFactory $repositoryFactory; @@ -90,11 +97,6 @@ class EntityManager implements EntityManagerInterface */ private Expr|null $expressionBuilder = null; - /** - * Whether the EntityManager is closed or not. - */ - private bool $closed = false; - /** * Collection of query filters. */ @@ -112,7 +114,9 @@ class EntityManager implements EntityManagerInterface * @param Connection $conn The database connection used by the EntityManager. */ public function __construct( + /** @readonly */ private Connection $conn, + /** @readonly */ private Configuration $config, EventManager|null $eventManager = null, ) { @@ -186,7 +190,7 @@ public function wrapInTransaction(callable $func): mixed return $return; } catch (Throwable $e) { - $this->close(); + $this->clear(); $this->conn->rollBack(); throw $e; @@ -255,7 +259,6 @@ public function createQueryBuilder(): QueryBuilder */ public function flush(): void { - $this->errorIfClosed(); $this->unitOfWork->commit(); } @@ -406,13 +409,13 @@ public function getReference(string $entityName, mixed $id): object|null public function clear(): void { $this->unitOfWork->clear(); + + $this->unitOfWork = new UnitOfWork($this); } public function close(): void { $this->clear(); - - $this->closed = true; } /** @@ -429,8 +432,6 @@ public function close(): void */ public function persist(object $object): void { - $this->errorIfClosed(); - $this->unitOfWork->persist($object); } @@ -445,15 +446,11 @@ public function persist(object $object): void */ public function remove(object $object): void { - $this->errorIfClosed(); - $this->unitOfWork->remove($object); } public function refresh(object $object, LockMode|int|null $lockMode = null): void { - $this->errorIfClosed(); - $this->unitOfWork->refresh($object, $lockMode); } @@ -512,21 +509,10 @@ public function getConfiguration(): Configuration return $this->config; } - /** - * Throws an exception if the EntityManager is closed or currently not active. - * - * @throws EntityManagerClosed If the EntityManager is closed. - */ - private function errorIfClosed(): void - { - if ($this->closed) { - throw EntityManagerClosed::create(); - } - } - + /** @return true */ public function isOpen(): bool { - return ! $this->closed; + return true; } public function getUnitOfWork(): UnitOfWork diff --git a/src/EntityManagerInterface.php b/src/EntityManagerInterface.php index cf3102bb41b..97dcee223c1 100644 --- a/src/EntityManagerInterface.php +++ b/src/EntityManagerInterface.php @@ -167,6 +167,8 @@ public function getReference(string $entityName, mixed $id): object|null; * Closes the EntityManager. All entities that are currently managed * by this EntityManager become detached. The EntityManager may no longer * be used after it is closed. + * + * @deprecated call {@see EntityManagerInterface::clear()} instead */ public function close(): void; @@ -192,6 +194,10 @@ public function getConfiguration(): Configuration; /** * Check if the Entity manager is open or closed. + * + * @deprecated + * + * @return true */ public function isOpen(): bool; diff --git a/src/Exception/EntityManagerClosed.php b/src/Exception/EntityManagerClosed.php index a76920217f1..6a19f1487c5 100644 --- a/src/Exception/EntityManagerClosed.php +++ b/src/Exception/EntityManagerClosed.php @@ -6,10 +6,11 @@ use RuntimeException; +/** @deprecated */ final class EntityManagerClosed extends RuntimeException implements ManagerException { public static function create(): self { - return new self('The EntityManager is closed.'); + return new self('The EntityManager is closed. Call clear() to reopen it.'); } } diff --git a/src/UnitOfWork.php b/src/UnitOfWork.php index 4ec0906843b..322fe22a2f8 100644 --- a/src/UnitOfWork.php +++ b/src/UnitOfWork.php @@ -437,7 +437,7 @@ public function commit(): void throw new OptimisticLockException('Commit failed', null, $e ?? null); } } catch (Throwable $e) { - $this->em->close(); + $this->em->clear(); if ($conn->isTransactionActive()) { $conn->rollBack(); diff --git a/tests/Tests/ORM/EntityManagerTest.php b/tests/Tests/ORM/EntityManagerTest.php index 501f86550ce..f730e54a1a0 100644 --- a/tests/Tests/ORM/EntityManagerTest.php +++ b/tests/Tests/ORM/EntityManagerTest.php @@ -9,7 +9,6 @@ use Doctrine\ORM\Configuration; use Doctrine\ORM\EntityManager; use Doctrine\ORM\EntityManagerInterface; -use Doctrine\ORM\Exception\EntityManagerClosed; use Doctrine\ORM\Mapping\ClassMetadataFactory; use Doctrine\ORM\Proxy\ProxyFactory; use Doctrine\ORM\Query; @@ -23,7 +22,6 @@ use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Group; use ReflectionProperty; -use stdClass; use Symfony\Component\VarExporter\LazyGhostTrait; use TypeError; @@ -38,14 +36,6 @@ protected function setUp(): void $this->entityManager = $this->getTestEntityManager(); } - #[Group('DDC-899')] - public function testIsOpen(): void - { - self::assertTrue($this->entityManager->isOpen()); - $this->entityManager->close(); - self::assertFalse($this->entityManager->isOpen()); - } - public function testGetConnection(): void { self::assertInstanceOf(Connection::class, $this->entityManager->getConnection()); @@ -115,27 +105,6 @@ public function testCreateQuery(): void self::assertEquals('SELECT 1', $q->getDql()); } - /** @psalm-return list */ - public static function dataAffectedByErrorIfClosedException(): array - { - return [ - ['flush'], - ['persist'], - ['remove'], - ['refresh'], - ]; - } - - #[DataProvider('dataAffectedByErrorIfClosedException')] - public function testAffectedByErrorIfClosedException(string $methodName): void - { - $this->expectException(EntityManagerClosed::class); - $this->expectExceptionMessage('closed'); - - $this->entityManager->close(); - $this->entityManager->$methodName(new stdClass()); - } - /** @return Generator */ public static function dataToBeReturnedByWrapInTransaction(): Generator { @@ -172,7 +141,7 @@ public function testWrapInTransactionReThrowsThrowables(): void self::fail('TypeError expected to be thrown'); } catch (TypeError) { - self::assertFalse($this->entityManager->isOpen()); + self::assertTrue($this->entityManager->isOpen()); } }