Fix for doctrine#7068: EntityManager::find() with pessimistic lock sh…

…ould check for transaction
Majkl578 · Jul 3, 2018 · bfd8571 · bfd8571
1 parent 4192c3a
commit bfd8571
Show file tree

Hide file tree

Showing 2 changed files with 70 additions and 9 deletions.
diff --git a/lib/Doctrine/ORM/EntityManager.php b/lib/Doctrine/ORM/EntityManager.php
@@ -23,6 +23,7 @@
 use Doctrine\DBAL\Connection;
 use Doctrine\DBAL\DriverManager;
 use Doctrine\DBAL\LockMode;
+use Doctrine\ORM\Mapping\ClassMetadata;
 use Doctrine\ORM\Query\ResultSetMapping;
 use Doctrine\ORM\Proxy\ProxyFactory;
 use Doctrine\ORM\Query\FilterCollection;
@@ -380,6 +381,10 @@ public function find($entityName, $id, $lockMode = null, $lockVersion = null)
     {
         $class = $this->metadataFactory->getMetadataFor(ltrim($entityName, '\\'));
 
+        if ($lockMode !== null) {
+            $this->checkLockRequirements($lockMode, $class);
+        }
+
         if ( ! is_array($id)) {
             if ($class->isIdentifierComposite) {
                 throw ORMInvalidArgumentException::invalidCompositeIdentifier();
@@ -441,10 +446,6 @@ public function find($entityName, $id, $lockMode = null, $lockVersion = null)
 
         switch (true) {
             case LockMode::OPTIMISTIC === $lockMode:
-                if ( ! $class->isVersioned) {
-                    throw OptimisticLockException::notVersioned($class->name);
-                }
-
                 $entity = $persister->load($sortedId);
 
                 $unitOfWork->lock($entity, $lockMode, $lockVersion);
@@ -453,12 +454,7 @@ public function find($entityName, $id, $lockMode = null, $lockVersion = null)
 
             case LockMode::PESSIMISTIC_READ === $lockMode:
             case LockMode::PESSIMISTIC_WRITE === $lockMode:
-                if ( ! $this->getConnection()->isTransactionActive()) {
-                    throw TransactionRequiredException::transactionRequired();
-                }
-
                 return $persister->load($sortedId, null, null, [], $lockMode);
-
             default:
                 return $persister->loadById($sortedId);
         }
@@ -915,4 +911,26 @@ public function hasFilters()
     {
         return null !== $this->filterCollection;
     }
+
+    /**
+     * @param int $lockMode
+     * @param ClassMetadata $class
+     * @throws OptimisticLockException
+     * @throws TransactionRequiredException
+     */
+    private function checkLockRequirements(int $lockMode, ClassMetadata $class): void
+    {
+        switch ($lockMode) {
+            case LockMode::OPTIMISTIC:
+                if (!$class->isVersioned) {
+                    throw OptimisticLockException::notVersioned($class->name);
+                }
+            // Intentional fallthrough
+            case LockMode::PESSIMISTIC_READ:
+            case LockMode::PESSIMISTIC_WRITE:
+                if (!$this->getConnection()->isTransactionActive()) {
+                    throw TransactionRequiredException::transactionRequired();
+                }
+        }
+    }
 }
diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/GH7068Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/GH7068Test.php
@@ -0,0 +1,43 @@
+<?php
+
+namespace Doctrine\Tests\ORM\Functional\Ticket;
+
+use Doctrine\DBAL\LockMode;
+use Doctrine\ORM\TransactionRequiredException;
+use Doctrine\Tests\OrmFunctionalTestCase;
+
+final class GH7068Test extends OrmFunctionalTestCase
+{
+    /**
+     * {@inheritDoc}
+     */
+    protected function setUp()
+    {
+        parent::setUp();
+
+        $this->setUpEntitySchema(
+            [
+                SomeEntity::class,
+            ]
+        );
+    }
+
+    public function testLockModeIsRespected()
+    {
+        $entity = new SomeEntity();
+        $this->_em->persist($entity);
+        $this->_em->flush();
+        $this->_em->clear();
+
+        $this->_em->find(SomeEntity::class, 1);
+
+        $this->expectException(TransactionRequiredException::class);
+        $this->_em->find(SomeEntity::class, 1, LockMode::PESSIMISTIC_WRITE);
+    }
+}
+
+/** @Entity */
+final class SomeEntity {
+    /** @Id @Column(type="integer") @GeneratedValue */
+    public $id;
+}