Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix calls to removed lock methods #11061

Merged
merged 1 commit into from
Nov 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@

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,
$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
5 changes: 3 additions & 2 deletions psalm.xml
Original file line number Diff line number Diff line change
Expand Up @@ -193,12 +193,13 @@
<file name="lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php"/>
<!-- DBAL 3 compatibility -->
<file name="lib/Doctrine/ORM/UnitOfWork.php"/>
<file name="lib/Doctrine/ORM/Utility/LockSqlHelper.php"/>
</errorLevel>
</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
Loading