Add lockMode to EntityManager#refresh() (#10040)

doctrine · Nov 1, 2022 · 25ce9b9 · 25ce9b9
1 parent 75340b6
commit 25ce9b9
Show file tree

Hide file tree

Showing 7 changed files with 110 additions and 13 deletions.
diff --git 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
@@ -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
@@ -693,22 +693,24 @@ 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);
         }
 
         $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
@@ -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
@@ -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,29 +2194,46 @@ 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);
     }
 
     /**
      * Executes a refresh operation on an entity.
      *
      * @param object $entity The entity to refresh.
      * @psalm-param array<int, object>  $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,19 +2250,21 @@ 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);
     }
 
     /**
      * Cascades a refresh operation to associated entities.
      *
      * @param object $entity
      * @psalm-param array<int, object> $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
@@ -269,9 +269,10 @@
     <MoreSpecificImplementedParamType occurrences="1">
       <code>$className</code>
     </MoreSpecificImplementedParamType>
-    <TooManyArguments occurrences="2">
+    <TooManyArguments occurrences="3">
       <code>find</code>
       <code>flush</code>
+      <code>refresh</code>
     </TooManyArguments>
   </file>
   <file src="lib/Doctrine/ORM/EntityManager.php">

diff --git 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
     {