From 6d151f953884f291e67f498c6889c4c610e27877 Mon Sep 17 00:00:00 2001
From: Sergei Morozov <morozov@tut.by>
Date: Thu, 12 Nov 2020 18:08:58 -0800
Subject: [PATCH] Throw exception on invalid LockMode

---
 src/Exception/InvalidLockMode.php            | 24 ++++++++++++++++++++
 src/Platforms/AbstractPlatform.php           | 22 ++++++++++++------
 src/Platforms/SQLServer2012Platform.php      | 19 ++++++++--------
 tests/Platforms/AbstractPlatformTestCase.php |  7 ++++++
 tests/Platforms/SQLServerPlatformTest.php    |  3 +--
 5 files changed, 57 insertions(+), 18 deletions(-)
 create mode 100644 src/Exception/InvalidLockMode.php

diff --git a/src/Exception/InvalidLockMode.php b/src/Exception/InvalidLockMode.php
new file mode 100644
index 00000000000..9a4d6c77bc9
--- /dev/null
+++ 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
index b0dcef25521..abfabdcc666 100644
--- 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
index 0571134f36a..51e87965243 100644
--- 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
index 04ca57c0220..5a6dab86558 100644
--- 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
index 303a2804afd..7e6925ebf1b 100644
--- 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)'],