Fix calls to removed lock methods (doctrine#11061)

jwage · Jan 30, 2024 · 258290f · 258290f
1 parent 398ab05
commit 258290f
Show file tree

Hide file tree

Showing 6 changed files with 63 additions and 20 deletions.
diff --git a/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php b/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php
@@ -31,6 +31,7 @@
 use Doctrine\ORM\Repository\Exception\InvalidFindByCall;
 use Doctrine\ORM\UnitOfWork;
 use Doctrine\ORM\Utility\IdentifierFlattener;
+use Doctrine\ORM\Utility\LockSqlHelper;
 use Doctrine\ORM\Utility\PersisterHelper;
 use LengthException;
 
@@ -92,6 +93,8 @@
  */
 class BasicEntityPersister implements EntityPersister
 {
+    use LockSqlHelper;
+
     /** @var array<string,string> */
     private static $comparisonMap = [
         Comparison::EQ          => '= %s',
@@ -1116,11 +1119,11 @@ public function getSelectSQL($criteria, $assoc = null, $lockMode = null, $limit
 
         switch ($lockMode) {
             case LockMode::PESSIMISTIC_READ:
-                $lockSql = ' ' . $this->platform->getReadLockSQL();
+                $lockSql = ' ' . $this->getReadLockSQL($this->platform);
                 break;
 
             case LockMode::PESSIMISTIC_WRITE:
-                $lockSql = ' ' . $this->platform->getWriteLockSQL();
+                $lockSql = ' ' . $this->getWriteLockSQL($this->platform);
                 break;
         }
 
@@ -1578,11 +1581,11 @@ public function lock(array $criteria, $lockMode)
 
         switch ($lockMode) {
             case LockMode::PESSIMISTIC_READ:
-                $lockSql = $this->platform->getReadLockSQL();
+                $lockSql = $this->getReadLockSQL($this->platform);
 
                 break;
             case LockMode::PESSIMISTIC_WRITE:
-                $lockSql = $this->platform->getWriteLockSQL();
+                $lockSql = $this->getWriteLockSQL($this->platform);
                 break;
         }
 

diff --git a/lib/Doctrine/ORM/Persisters/Entity/JoinedSubclassPersister.php b/lib/Doctrine/ORM/Persisters/Entity/JoinedSubclassPersister.php
@@ -10,6 +10,7 @@
 use Doctrine\DBAL\Types\Types;
 use Doctrine\ORM\Internal\SQLResultCasing;
 use Doctrine\ORM\Mapping\ClassMetadata;
+use Doctrine\ORM\Utility\LockSqlHelper;
 use Doctrine\ORM\Utility\PersisterHelper;
 use LengthException;
 
@@ -26,6 +27,7 @@
  */
 class JoinedSubclassPersister extends AbstractEntityInheritancePersister
 {
+    use LockSqlHelper;
     use SQLResultCasing;
 
     /**
@@ -316,12 +318,12 @@ public function getSelectSQL($criteria, $assoc = null, $lockMode = null, $limit
 
         switch ($lockMode) {
             case LockMode::PESSIMISTIC_READ:
-                $lockSql = ' ' . $this->platform->getReadLockSQL();
+                $lockSql = ' ' . $this->getReadLockSQL($this->platform);
 
                 break;
 
             case LockMode::PESSIMISTIC_WRITE:
-                $lockSql = ' ' . $this->platform->getWriteLockSQL();
+                $lockSql = ' ' . $this->getWriteLockSQL($this->platform);
 
                 break;
         }

diff --git a/lib/Doctrine/ORM/Query/SqlWalker.php b/lib/Doctrine/ORM/Query/SqlWalker.php
@@ -16,6 +16,7 @@
 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;
@@ -48,6 +49,8 @@
  */
 class SqlWalker implements TreeWalker
 {
+    use LockSqlHelper;
+
     public const HINT_DISTINCT = 'doctrine.distinct';
 
     /**
@@ -577,11 +580,11 @@ public function walkSelectStatement(AST\SelectStatement $AST)
         }
 
         if ($lockMode === LockMode::PESSIMISTIC_READ) {
-            return $sql . ' ' . $this->platform->getReadLockSQL();
+            return $sql . ' ' . $this->getReadLockSQL($this->platform);
         }
 
         if ($lockMode === LockMode::PESSIMISTIC_WRITE) {
-            return $sql . ' ' . $this->platform->getWriteLockSQL();
+            return $sql . ' ' . $this->getWriteLockSQL($this->platform);
         }
 
         if ($lockMode !== LockMode::OPTIMISTIC) {

diff --git a/lib/Doctrine/ORM/Utility/LockSqlHelper.php b/lib/Doctrine/ORM/Utility/LockSqlHelper.php
@@ -0,0 +1,35 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\ORM\Utility;
+
+use Doctrine\DBAL\Platforms\AbstractMySQLPlatform;
+use Doctrine\DBAL\Platforms\AbstractPlatform;
+use Doctrine\DBAL\Platforms\DB2Platform;
+use Doctrine\DBAL\Platforms\PostgreSQLPlatform;
+use Doctrine\DBAL\Platforms\SQLitePlatform;
+use Doctrine\DBAL\Platforms\SQLServerPlatform;
+
+/** @internal */
+trait LockSqlHelper
+{
+    private function getReadLockSQL(AbstractPlatform $platform): string
+    {
+        return match (true) {
+            $platform instanceof AbstractMySQLPlatform => 'LOCK IN SHARE MODE',
+            $platform instanceof PostgreSQLPlatform => 'FOR SHARE',
+            default => $this->getWriteLockSQL($platform),
+        };
+    }
+
+    private function getWriteLockSQL(AbstractPlatform $platform): string
+    {
+        return match (true) {
+            $platform instanceof DB2Platform => 'WITH RR USE AND KEEP UPDATE LOCKS',
+            $platform instanceof SQLitePlatform,
+            $platform instanceof SQLServerPlatform => '',
+            default => 'FOR UPDATE',
+        };
+    }
+}
diff --git a/psalm.xml b/psalm.xml
@@ -256,6 +256,7 @@
             <errorLevel type="suppress">
                 <file name="lib/Doctrine/ORM/Internal/SQLResultCasing.php"/>
                 <file name="lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php"/>
+                <file name="lib/Doctrine/ORM/Utility/LockSqlHelper.php"/>
             </errorLevel>
         </TypeDoesNotContainType>
         <UndefinedClass>

diff --git a/tests/Doctrine/Tests/ORM/Functional/Locking/LockTest.php b/tests/Doctrine/Tests/ORM/Functional/Locking/LockTest.php
@@ -5,6 +5,7 @@
 namespace Doctrine\Tests\ORM\Functional\Locking;
 
 use Doctrine\DBAL\LockMode;
+use Doctrine\DBAL\Platforms\SQLitePlatform;
 use Doctrine\ORM\OptimisticLockException;
 use Doctrine\ORM\Query;
 use Doctrine\ORM\TransactionRequiredException;
@@ -168,9 +169,7 @@ public function testRefreshWithLockPessimisticWriteNoTransactionThrowsException(
      */
     public function testLockPessimisticWrite(): void
     {
-        $writeLockSql = $this->_em->getConnection()->getDatabasePlatform()->getWriteLockSQL();
-
-        if (! $writeLockSql) {
+        if ($this->_em->getConnection()->getDatabasePlatform() instanceof SQLitePlatform) {
             self::markTestSkipped('Database Driver has no Write Lock support.');
         }
 
@@ -195,17 +194,15 @@ public function testLockPessimisticWrite(): void
             $lastLoggedQuery = $this->getLastLoggedQuery(1)['sql'];
         }
 
-        self::assertStringContainsString($writeLockSql, $lastLoggedQuery);
+        self::assertStringContainsString('FOR UPDATE', $lastLoggedQuery);
     }
 
     /**
      * @group locking
      */
     public function testRefreshWithLockPessimisticWrite(): void
     {
-        $writeLockSql = $this->_em->getConnection()->getDatabasePlatform()->getWriteLockSQL();
-
-        if (! $writeLockSql) {
+        if ($this->_em->getConnection()->getDatabasePlatform() instanceof SQLitePlatform) {
             self::markTestSkipped('Database Driver has no Write Lock support.');
         }
 
@@ -230,15 +227,13 @@ public function testRefreshWithLockPessimisticWrite(): void
             $lastLoggedQuery = $this->getLastLoggedQuery(1)['sql'];
         }
 
-        self::assertStringContainsString($writeLockSql, $lastLoggedQuery);
+        self::assertStringContainsString('FOR UPDATE', $lastLoggedQuery);
     }
 
     /** @group DDC-178 */
     public function testLockPessimisticRead(): void
     {
-        $readLockSql = $this->_em->getConnection()->getDatabasePlatform()->getReadLockSQL();
-
-        if (! $readLockSql) {
+        if ($this->_em->getConnection()->getDatabasePlatform() instanceof SQLitePlatform) {
             self::markTestSkipped('Database Driver has no Write Lock support.');
         }
 
@@ -264,7 +259,11 @@ public function testLockPessimisticRead(): void
             $lastLoggedQuery = $this->getLastLoggedQuery(1)['sql'];
         }
 
-        self::assertStringContainsString($readLockSql, $lastLoggedQuery);
+        self::assertThat($lastLoggedQuery, self::logicalOr(
+            self::stringContains('FOR UPDATE'),
+            self::stringContains('FOR SHARE'),
+            self::stringContains('LOCK IN SHARE MODE'),
+        ));
     }
 
     /** @group DDC-1693 */