Skip to content

Commit

Permalink
Fix calls to removed lock methods
Browse files Browse the repository at this point in the history
  • Loading branch information
derrabus committed Nov 15, 2023
1 parent 21466a0 commit 532f566
Show file tree
Hide file tree
Showing 7 changed files with 66 additions and 22 deletions.
11 changes: 7 additions & 4 deletions lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,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;

Expand Down Expand Up @@ -96,6 +97,8 @@
*/
class BasicEntityPersister implements EntityPersister
{
use LockSqlHelper;

/** @var array<string,string> */
private static array $comparisonMap = [
Comparison::EQ => '= %s',
Expand Down Expand Up @@ -1074,8 +1077,8 @@ public function getSelectSQL(
: $this->getSelectConditionSQL($criteria, $assoc);

$lockSql = match ($lockMode) {
LockMode::PESSIMISTIC_READ => ' ' . $this->platform->getReadLockSQL(),
LockMode::PESSIMISTIC_WRITE => ' ' . $this->platform->getWriteLockSQL(),
LockMode::PESSIMISTIC_READ => ' ' . $this->getReadLockSQL($this->platform),
LockMode::PESSIMISTIC_WRITE => ' ' . $this->getWriteLockSQL($this->platform),
default => '',
};

Expand Down Expand Up @@ -1505,8 +1508,8 @@ public function lock(array $criteria, LockMode|int $lockMode): void
$conditionSql = $this->getSelectConditionSQL($criteria);

$lockSql = match ($lockMode) {
LockMode::PESSIMISTIC_READ => $this->platform->getReadLockSQL(),
LockMode::PESSIMISTIC_WRITE => $this->platform->getWriteLockSQL(),
LockMode::PESSIMISTIC_READ => $this->getReadLockSQL($this->platform),
LockMode::PESSIMISTIC_WRITE => $this->getWriteLockSQL($this->platform),
default => '',
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
use Doctrine\ORM\Internal\SQLResultCasing;
use Doctrine\ORM\Mapping\AssociationMapping;
use Doctrine\ORM\Mapping\ClassMetadata;
use Doctrine\ORM\Utility\LockSqlHelper;
use Doctrine\ORM\Utility\PersisterHelper;
use LengthException;

Expand All @@ -27,6 +28,7 @@
*/
class JoinedSubclassPersister extends AbstractEntityInheritancePersister
{
use LockSqlHelper;
use SQLResultCasing;

/**
Expand Down Expand Up @@ -279,12 +281,12 @@ public function getSelectSQL(

switch ($lockMode) {
case LockMode::PESSIMISTIC_READ:
$lockSql = ' ' . $this->platform->getReadLockSQL();
$lockSql = ' ' . $this->getReadLockSQL($this->platform);

Check warning on line 284 in lib/Doctrine/ORM/Persisters/Entity/JoinedSubclassPersister.php

View check run for this annotation

Codecov / codecov/patch

lib/Doctrine/ORM/Persisters/Entity/JoinedSubclassPersister.php#L284

Added line #L284 was not covered by tests

break;

case LockMode::PESSIMISTIC_WRITE:
$lockSql = ' ' . $this->platform->getWriteLockSQL();
$lockSql = ' ' . $this->getWriteLockSQL($this->platform);

Check warning on line 289 in lib/Doctrine/ORM/Persisters/Entity/JoinedSubclassPersister.php

View check run for this annotation

Codecov / codecov/patch

lib/Doctrine/ORM/Persisters/Entity/JoinedSubclassPersister.php#L289

Added line #L289 was not covered by tests

break;
}
Expand Down
7 changes: 5 additions & 2 deletions lib/Doctrine/ORM/Query/SqlWalker.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,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;
Expand Down Expand Up @@ -46,6 +47,8 @@
*/
class SqlWalker
{
use LockSqlHelper;

public const HINT_DISTINCT = 'doctrine.distinct';

private readonly ResultSetMapping $rsm;
Expand Down Expand Up @@ -475,11 +478,11 @@ public function walkSelectStatement(AST\SelectStatement $selectStatement): strin
}

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) {
Expand Down
35 changes: 35 additions & 0 deletions lib/Doctrine/ORM/Utility/LockSqlHelper.php
Original file line number Diff line number Diff line change
@@ -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,

Check failure on line 30 in lib/Doctrine/ORM/Utility/LockSqlHelper.php

View workflow job for this annotation

GitHub Actions / Static Analysis with Psalm (3.7)

TypeDoesNotContainType

lib/Doctrine/ORM/Utility/LockSqlHelper.php:30:13: TypeDoesNotContainType: Cannot resolve types for $platform - Doctrine\DBAL\Platforms\AbstractPlatform does not contain Doctrine\DBAL\Platforms\SQLitePlatform (see https://psalm.dev/056)
$platform instanceof SQLServerPlatform => '',
default => 'FOR UPDATE',
};
}
}
2 changes: 2 additions & 0 deletions phpstan-dbal3.neon
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ parameters:
message: '~^Unreachable statement \- code above always terminates\.$~'
path: lib/Doctrine/ORM/Mapping/AssociationMapping.php

- '~^Class Doctrine\\DBAL\\Platforms\\SQLitePlatform not found\.$~'

# To be removed in 4.0
-
message: '#Negated boolean expression is always false\.#'
Expand Down
4 changes: 2 additions & 2 deletions psalm.xml
Original file line number Diff line number Diff line change
Expand Up @@ -197,8 +197,8 @@
</TypeDoesNotContainType>
<UndefinedClass>
<errorLevel type="suppress">
<!-- Persistence 2 compatibility -->
<referencedClass name="Doctrine\Persistence\ObjectManagerAware"/>
<!-- Compatibility with DBAL 3 -->
<referencedClass name="Doctrine\DBAL\Platforms\SQLitePlatform"/>
</errorLevel>
</UndefinedClass>
<UndefinedMethod>
Expand Down
23 changes: 11 additions & 12 deletions tests/Doctrine/Tests/ORM/Functional/Locking/LockTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -136,9 +137,7 @@ public function testRefreshWithLockPessimisticWriteNoTransactionThrowsException(
#[Group('locking')]
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.');
}

Expand All @@ -163,15 +162,13 @@ 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.');
}

Expand All @@ -196,15 +193,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.');
}

Expand All @@ -230,7 +225,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')]
Expand Down

0 comments on commit 532f566

Please sign in to comment.