From 258290f7cbc521263f0de4e434ae686e510be54c Mon Sep 17 00:00:00 2001 From: "Alexander M. Turek" Date: Wed, 15 Nov 2023 23:48:59 +0100 Subject: [PATCH] Fix calls to removed lock methods (#11061) --- .../Entity/BasicEntityPersister.php | 11 +++--- .../Entity/JoinedSubclassPersister.php | 6 ++-- lib/Doctrine/ORM/Query/SqlWalker.php | 7 ++-- lib/Doctrine/ORM/Utility/LockSqlHelper.php | 35 +++++++++++++++++++ psalm.xml | 1 + .../Tests/ORM/Functional/Locking/LockTest.php | 23 ++++++------ 6 files changed, 63 insertions(+), 20 deletions(-) create mode 100644 lib/Doctrine/ORM/Utility/LockSqlHelper.php diff --git a/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php b/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php index 0d11dc9238d..e83589c4180 100644 --- 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 */ 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 index f286ac1d8e4..1d6a47855f9 100644 --- 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 index 34e5b22580d..f3d0ab794c0 100644 --- 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 new file mode 100644 index 00000000000..7d135eb8cb5 --- /dev/null +++ b/lib/Doctrine/ORM/Utility/LockSqlHelper.php @@ -0,0 +1,35 @@ + '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 index a8a096c3ae4..3b9a3b96ba8 100644 --- a/psalm.xml +++ b/psalm.xml @@ -256,6 +256,7 @@ + diff --git a/tests/Doctrine/Tests/ORM/Functional/Locking/LockTest.php b/tests/Doctrine/Tests/ORM/Functional/Locking/LockTest.php index c3992212748..4258b549e97 100644 --- 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,7 +194,7 @@ public function testLockPessimisticWrite(): void $lastLoggedQuery = $this->getLastLoggedQuery(1)['sql']; } - self::assertStringContainsString($writeLockSql, $lastLoggedQuery); + self::assertStringContainsString('FOR UPDATE', $lastLoggedQuery); } /** @@ -203,9 +202,7 @@ public function testLockPessimisticWrite(): void */ 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 */