From 25ce9b910109363393e38549e759517076ee43d0 Mon Sep 17 00:00:00 2001 From: michnovka Date: Tue, 1 Nov 2022 23:59:11 +0100 Subject: [PATCH 1/2] Add lockMode to EntityManager#refresh() (#10040) --- .../transactions-and-concurrency.rst | 6 ++- .../ORM/Decorator/EntityManagerDecorator.php | 16 ++++++ lib/Doctrine/ORM/EntityManager.php | 8 +-- lib/Doctrine/ORM/EntityManagerInterface.php | 1 + lib/Doctrine/ORM/UnitOfWork.php | 37 ++++++++++--- psalm-baseline.xml | 3 +- .../Tests/ORM/Functional/Locking/LockTest.php | 52 +++++++++++++++++++ 7 files changed, 110 insertions(+), 13 deletions(-) diff --git a/docs/en/reference/transactions-and-concurrency.rst b/docs/en/reference/transactions-and-concurrency.rst index ccd91db6c3f..b0b0e943534 100644 --- a/docs/en/reference/transactions-and-concurrency.rst +++ b/docs/en/reference/transactions-and-concurrency.rst @@ -412,7 +412,7 @@ Doctrine ORM currently supports two pessimistic lock modes: locks other concurrent requests that attempt to update or lock rows in write mode. -You can use pessimistic locks in three different scenarios: +You can use pessimistic locks in four different scenarios: 1. Using @@ -424,6 +424,10 @@ You can use pessimistic locks in three different scenarios: or ``EntityManager#lock($entity, \Doctrine\DBAL\LockMode::PESSIMISTIC_READ)`` 3. Using + ``EntityManager#refresh($entity, \Doctrine\DBAL\LockMode::PESSIMISTIC_WRITE)`` + or + ``EntityManager#refresh($entity, \Doctrine\DBAL\LockMode::PESSIMISTIC_READ)`` +4. Using ``Query#setLockMode(\Doctrine\DBAL\LockMode::PESSIMISTIC_WRITE)`` or ``Query#setLockMode(\Doctrine\DBAL\LockMode::PESSIMISTIC_READ)`` diff --git a/lib/Doctrine/ORM/Decorator/EntityManagerDecorator.php b/lib/Doctrine/ORM/Decorator/EntityManagerDecorator.php index 1bd4c5c58cb..4c883067afe 100644 --- a/lib/Doctrine/ORM/Decorator/EntityManagerDecorator.php +++ b/lib/Doctrine/ORM/Decorator/EntityManagerDecorator.php @@ -9,6 +9,8 @@ use Doctrine\ORM\Query\ResultSetMapping; use Doctrine\Persistence\ObjectManagerDecorator; +use function func_get_arg; +use function func_num_args; use function get_debug_type; use function method_exists; use function sprintf; @@ -211,6 +213,20 @@ public function flush($entity = null) $this->wrapped->flush($entity); } + /** + * {@inheritdoc} + */ + public function refresh($object) + { + $lockMode = null; + + if (func_num_args() > 1) { + $lockMode = func_get_arg(1); + } + + $this->wrapped->refresh($object, $lockMode); + } + /** * {@inheritdoc} */ diff --git a/lib/Doctrine/ORM/EntityManager.php b/lib/Doctrine/ORM/EntityManager.php index 1956ad75166..e32993b828b 100644 --- a/lib/Doctrine/ORM/EntityManager.php +++ b/lib/Doctrine/ORM/EntityManager.php @@ -693,14 +693,16 @@ public function remove($entity) * Refreshes the persistent state of an entity from the database, * overriding any local changes that have not yet been persisted. * - * @param object $entity The entity to refresh. + * @param object $entity The entity to refresh + * @psalm-param LockMode::*|null $lockMode * * @return void * * @throws ORMInvalidArgumentException * @throws ORMException + * @throws TransactionRequiredException */ - public function refresh($entity) + public function refresh($entity, ?int $lockMode = null) { if (! is_object($entity)) { throw ORMInvalidArgumentException::invalidObject('EntityManager#refresh()', $entity); @@ -708,7 +710,7 @@ public function refresh($entity) $this->errorIfClosed(); - $this->unitOfWork->refresh($entity); + $this->unitOfWork->refresh($entity, $lockMode); } /** diff --git a/lib/Doctrine/ORM/EntityManagerInterface.php b/lib/Doctrine/ORM/EntityManagerInterface.php index 13e407d41ef..f806f1eef97 100644 --- a/lib/Doctrine/ORM/EntityManagerInterface.php +++ b/lib/Doctrine/ORM/EntityManagerInterface.php @@ -22,6 +22,7 @@ * * @method Mapping\ClassMetadataFactory getMetadataFactory() * @method mixed wrapInTransaction(callable $func) + * @method void refresh(object $object, ?int $lockMode = null) */ interface EntityManagerInterface extends ObjectManager { diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index 1d8e992a3ec..9b85bbd5558 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -64,6 +64,8 @@ use function assert; use function count; use function current; +use function func_get_arg; +use function func_num_args; use function get_class; use function get_debug_type; use function implode; @@ -2192,17 +2194,24 @@ private function doDetach( * Refreshes the state of the given entity from the database, overwriting * any local, unpersisted changes. * - * @param object $entity The entity to refresh. + * @param object $entity The entity to refresh * * @return void * * @throws InvalidArgumentException If the entity is not MANAGED. + * @throws TransactionRequiredException */ public function refresh($entity) { $visited = []; - $this->doRefresh($entity, $visited); + $lockMode = null; + + if (func_num_args() > 1) { + $lockMode = func_get_arg(1); + } + + $this->doRefresh($entity, $visited, $lockMode); } /** @@ -2210,11 +2219,21 @@ public function refresh($entity) * * @param object $entity The entity to refresh. * @psalm-param array $visited The already visited entities during cascades. + * @psalm-param LockMode::*|null $lockMode * * @throws ORMInvalidArgumentException If the entity is not MANAGED. + * @throws TransactionRequiredException */ - private function doRefresh($entity, array &$visited): void + private function doRefresh($entity, array &$visited, ?int $lockMode = null): void { + switch (true) { + case $lockMode === LockMode::PESSIMISTIC_READ: + case $lockMode === LockMode::PESSIMISTIC_WRITE: + if (! $this->em->getConnection()->isTransactionActive()) { + throw TransactionRequiredException::transactionRequired(); + } + } + $oid = spl_object_id($entity); if (isset($visited[$oid])) { @@ -2231,10 +2250,11 @@ private function doRefresh($entity, array &$visited): void $this->getEntityPersister($class->name)->refresh( array_combine($class->getIdentifierFieldNames(), $this->entityIdentifiers[$oid]), - $entity + $entity, + $lockMode ); - $this->cascadeRefresh($entity, $visited); + $this->cascadeRefresh($entity, $visited, $lockMode); } /** @@ -2242,8 +2262,9 @@ private function doRefresh($entity, array &$visited): void * * @param object $entity * @psalm-param array $visited + * @psalm-param LockMode::*|null $lockMode */ - private function cascadeRefresh($entity, array &$visited): void + private function cascadeRefresh($entity, array &$visited, ?int $lockMode = null): void { $class = $this->em->getClassMetadata(get_class($entity)); @@ -2266,13 +2287,13 @@ static function ($assoc) { case $relatedEntities instanceof Collection: case is_array($relatedEntities): foreach ($relatedEntities as $relatedEntity) { - $this->doRefresh($relatedEntity, $visited); + $this->doRefresh($relatedEntity, $visited, $lockMode); } break; case $relatedEntities !== null: - $this->doRefresh($relatedEntities, $visited); + $this->doRefresh($relatedEntities, $visited, $lockMode); break; default: diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 2d223fadb96..b324dc0bd44 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -269,9 +269,10 @@ $className - + find flush + refresh diff --git a/tests/Doctrine/Tests/ORM/Functional/Locking/LockTest.php b/tests/Doctrine/Tests/ORM/Functional/Locking/LockTest.php index 8b113c65f94..ebd3c3f7505 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Locking/LockTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/Locking/LockTest.php @@ -144,6 +144,23 @@ public function testLockPessimisticWriteNoTransactionThrowsException(): void $this->_em->lock($article, LockMode::PESSIMISTIC_WRITE); } + /** + * @group locking + */ + public function testRefreshWithLockPessimisticWriteNoTransactionThrowsException(): void + { + $article = new CmsArticle(); + $article->text = 'my article'; + $article->topic = 'Hello'; + + $this->_em->persist($article); + $this->_em->flush(); + + $this->expectException(TransactionRequiredException::class); + + $this->_em->refresh($article, LockMode::PESSIMISTIC_WRITE); + } + /** * @group DDC-178 * @group locking @@ -180,6 +197,41 @@ public function testLockPessimisticWrite(): void self::assertStringContainsString($writeLockSql, $lastLoggedQuery); } + /** + * @group locking + */ + public function testRefreshWithLockPessimisticWrite(): void + { + $writeLockSql = $this->_em->getConnection()->getDatabasePlatform()->getWriteLockSQL(); + + if (! $writeLockSql) { + self::markTestSkipped('Database Driver has no Write Lock support.'); + } + + $article = new CmsArticle(); + $article->text = 'my article'; + $article->topic = 'Hello'; + + $this->_em->persist($article); + $this->_em->flush(); + + $this->_em->beginTransaction(); + try { + $this->_em->refresh($article, LockMode::PESSIMISTIC_WRITE); + $this->_em->commit(); + } catch (Exception $e) { + $this->_em->rollback(); + } + + $lastLoggedQuery = $this->getLastLoggedQuery()['sql']; + // DBAL 2 logs a commit as last query. + if ($lastLoggedQuery === '"COMMIT"') { + $lastLoggedQuery = $this->getLastLoggedQuery(1)['sql']; + } + + self::assertStringContainsString($writeLockSql, $lastLoggedQuery); + } + /** @group DDC-178 */ public function testLockPessimisticRead(): void { From 474f76fc8b3e4f7587c514ef100a023d5136548d Mon Sep 17 00:00:00 2001 From: "Alexander M. Turek" Date: Wed, 2 Nov 2022 00:15:00 +0100 Subject: [PATCH 2/2] Remove Doctrine\Persistence\ObjectManager::refresh from Psalm baseline --- psalm-baseline.xml | 3 +-- psalm.xml | 3 +++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/psalm-baseline.xml b/psalm-baseline.xml index b324dc0bd44..2d223fadb96 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -269,10 +269,9 @@ $className - + find flush - refresh diff --git a/psalm.xml b/psalm.xml index e98dab7e832..609c236492c 100644 --- a/psalm.xml +++ b/psalm.xml @@ -160,6 +160,9 @@ + + +