Merge pull request #4429 from morozov/issues/4428

Throw exception on invalid LockMode
doctrine · Nov 13, 2020 · e456ab1 · e456ab1
2 parents a501e55 + 6d151f9
commit e456ab1
Show file tree

Hide file tree

Showing 5 changed files with 57 additions and 18 deletions.
diff --git a/src/Exception/InvalidLockMode.php b/src/Exception/InvalidLockMode.php
@@ -0,0 +1,24 @@
+<?php
+
+namespace Doctrine\DBAL\Exception;
+
+use Doctrine\DBAL\Exception;
+
+use function sprintf;
+
+/**
+ * @psalm-immutable
+ */
+class InvalidLockMode extends Exception
+{
+    public static function fromLockMode(int $lockMode): self
+    {
+        return new self(
+            sprintf(
+                'Lock mode %d is invalid. The valid values are LockMode::NONE, LockMode::OPTIMISTIC'
+                    . ', LockMode::PESSIMISTIC_READ and LockMode::PESSIMISTIC_WRITE',
+                $lockMode
+            )
+        );
+    }
+}
diff --git a/src/Platforms/AbstractPlatform.php b/src/Platforms/AbstractPlatform.php
@@ -13,6 +13,8 @@
 use Doctrine\DBAL\Event\SchemaDropTableEventArgs;
 use Doctrine\DBAL\Events;
 use Doctrine\DBAL\Exception;
+use Doctrine\DBAL\Exception\InvalidLockMode;
+use Doctrine\DBAL\LockMode;
 use Doctrine\DBAL\Platforms\Keywords\KeywordList;
 use Doctrine\DBAL\Schema\Column;
 use Doctrine\DBAL\Schema\ColumnDiff;
@@ -1304,15 +1306,21 @@ public function getForUpdateSQL()
      * Honors that some SQL vendors such as MsSql use table hints for locking instead of the
      * ANSI SQL FOR UPDATE specification.
      *
-     * @param string   $fromClause The FROM clause to append the hint for the given lock mode to.
-     * @param int|null $lockMode   One of the Doctrine\DBAL\LockMode::* constants. If null is given, nothing will
-     *                             be appended to the FROM clause.
-     *
-     * @return string
+     * @param string $fromClause The FROM clause to append the hint for the given lock mode to
+     * @param int    $lockMode   One of the Doctrine\DBAL\LockMode::* constants
      */
-    public function appendLockHint($fromClause, $lockMode)
+    public function appendLockHint(string $fromClause, int $lockMode): string
     {
-        return $fromClause;
+        switch ($lockMode) {
+            case LockMode::NONE:
+            case LockMode::OPTIMISTIC:
+            case LockMode::PESSIMISTIC_READ:
+            case LockMode::PESSIMISTIC_WRITE:
+                return $fromClause;
+
+            default:
+                throw InvalidLockMode::fromLockMode($lockMode);
+        }
     }
 
     /**

diff --git a/src/Platforms/SQLServer2012Platform.php b/src/Platforms/SQLServer2012Platform.php
@@ -2,6 +2,7 @@
 
 namespace Doctrine\DBAL\Platforms;
 
+use Doctrine\DBAL\Exception\InvalidLockMode;
 use Doctrine\DBAL\LockMode;
 use Doctrine\DBAL\Schema\Column;
 use Doctrine\DBAL\Schema\ColumnDiff;
@@ -1535,23 +1536,23 @@ public function getForeignKeyReferentialActionSQL($action)
         return parent::getForeignKeyReferentialActionSQL($action);
     }
 
-    /**
-     * {@inheritDoc}
-     */
-    public function appendLockHint($fromClause, $lockMode)
+    public function appendLockHint(string $fromClause, int $lockMode): string
     {
-        switch (true) {
-            case $lockMode === LockMode::NONE:
+        switch ($lockMode) {
+            case LockMode::NONE:
                 return $fromClause . ' WITH (NOLOCK)';
 
-            case $lockMode === LockMode::PESSIMISTIC_READ:
+            case LockMode::OPTIMISTIC:
+                return $fromClause;
+
+            case LockMode::PESSIMISTIC_READ:
                 return $fromClause . ' WITH (HOLDLOCK, ROWLOCK)';
 
-            case $lockMode === LockMode::PESSIMISTIC_WRITE:
+            case LockMode::PESSIMISTIC_WRITE:
                 return $fromClause . ' WITH (UPDLOCK, ROWLOCK)';
 
             default:
-                return $fromClause;
+                throw InvalidLockMode::fromLockMode($lockMode);
         }
     }
 

diff --git a/tests/Platforms/AbstractPlatformTestCase.php b/tests/Platforms/AbstractPlatformTestCase.php
@@ -5,6 +5,7 @@
 use Doctrine\Common\EventManager;
 use Doctrine\DBAL\Events;
 use Doctrine\DBAL\Exception;
+use Doctrine\DBAL\Exception\InvalidLockMode;
 use Doctrine\DBAL\Platforms\AbstractPlatform;
 use Doctrine\DBAL\Platforms\Keywords\KeywordList;
 use Doctrine\DBAL\Schema\Column;
@@ -1477,6 +1478,12 @@ public function asciiStringSqlDeclarationDataProvider(): array
             ['CHAR(12)', ['length' => 12, 'fixed' => true]],
         ];
     }
+
+    public function testInvalidLockMode(): void
+    {
+        $this->expectException(InvalidLockMode::class);
+        $this->platform->appendLockHint('TABLE', 128);
+    }
 }
 
 interface GetCreateTableSqlDispatchEventListener

diff --git a/tests/Platforms/SQLServerPlatformTest.php b/tests/Platforms/SQLServerPlatformTest.php
@@ -16,7 +16,7 @@ public function createPlatform(): AbstractPlatform
     /**
      * @dataProvider getLockHints
      */
-    public function testAppendsLockHint(?int $lockMode, string $lockHint): void
+    public function testAppendsLockHint(int $lockMode, string $lockHint): void
     {
         $fromClause     = 'FROM users';
         $expectedResult = $fromClause . $lockHint;
@@ -30,7 +30,6 @@ public function testAppendsLockHint(?int $lockMode, string $lockHint): void
     public static function getLockHints(): iterable
     {
         return [
-            [null, ''],
             [LockMode::NONE, ' WITH (NOLOCK)'],
             [LockMode::OPTIMISTIC, ''],
             [LockMode::PESSIMISTIC_READ, ' WITH (HOLDLOCK, ROWLOCK)'],