DDC-178 - Add additional tests for Locking Support

lib/Doctrine/ORM/EntityManager.php

@@ -458,7 +458,7 @@ class EntityManager
      * @throws OptimisticLockException
      * @throws PessimisticLockException
      */
-    public function lock($entity, $lockMode, $lockVersion)
+    public function lock($entity, $lockMode, $lockVersion = null)
     {
         $this->_unitOfWork->lock($entity, $lockMode, $lockVersion);
     }

lib/Doctrine/ORM/OptimisticLockException.php

@@ -37,6 +37,11 @@ class OptimisticLockException extends ORMException
         return new self("The optimistic lock failed.");
     }
 
+    public static function lockFailedVersionMissmatch($expectedLockVersion, $actualLockVersion)
+    {
+        return new self("The optimistic lock failed, version " . $expectedLockVersion . " was expected, but is actually ".$actualLockVersion);
+    }
+
     public static function notVersioned($className)
     {
         return new self("Cannot obtain optimistic lock on unversioned entity ".$className);

lib/Doctrine/ORM/Persisters/StandardEntityPersister.php

@@ -960,6 +960,6 @@ class StandardEntityPersister
              . $this->_getSQLTableAlias($this->_class)
              . ($conditionSql ? ' WHERE ' . $conditionSql : '') . ' ' . $lockSql;
         $params = array_values($criteria);
-        $this->_conn->executeQuery($query, $params);
+        $this->_conn->executeQuery($sql, $params);
     }
 }

lib/Doctrine/ORM/UnitOfWork.php

@@ -1347,7 +1347,7 @@ class UnitOfWork implements PropertyChangedListener
                 $entityVersion = $class->reflFields[$class->versionField]->getValue($entity);
                 // Throw exception if versions dont match.
                 if ($managedCopyVersion != $entityVersion) {
-                    throw OptimisticLockException::lockFailed();
+                    throw OptimisticLockException::lockFailedVersionMissmatch($entityVersion, $managedCopyVersion);
                 }
             }
 
@@ -1640,6 +1640,10 @@ class UnitOfWork implements PropertyChangedListener
      */
     public function lock($entity, $lockMode, $lockVersion = null)
     {
+        if ($this->getEntityState($entity) != self::STATE_MANAGED) {
+            throw new \InvalidArgumentException("Entity is not MANAGED.");
+        }
+        
         $entityName = get_class($entity);
         $class = $this->_em->getClassMetadata($entityName);
 
@@ -1651,7 +1655,7 @@ class UnitOfWork implements PropertyChangedListener
             if ($lockVersion != null) {
                 $entityVersion = $class->reflFields[$class->versionField]->getValue($entity);
                 if ($entityVersion != $lockVersion) {
-                    throw OptimisticLockException::lockFailed();
+                    throw OptimisticLockException::lockFailedVersionMissmatch($lockVersion, $entityVersion);
                 }
             }
         } else if ($lockMode == LockMode::PESSIMISTIC_READ || $lockMode == LockMode::PESSIMISTIC_WRITE) {
@@ -1660,16 +1664,12 @@ class UnitOfWork implements PropertyChangedListener
                 throw TransactionRequiredException::transactionRequired();
             }
             
-            if ($this->getEntityState($entity) == self::STATE_MANAGED) {
-                $oid = spl_object_hash($entity);
+            $oid = spl_object_hash($entity);
 
-                $this->getEntityPersister($class->name)->lock(
-                    array_combine($class->getIdentifierColumnNames(), $this->_entityIdentifiers[$oid]),
-                    $entity
-                );
-            } else {
-                throw new \InvalidArgumentException("Entity is not MANAGED.");
-            }
+            $this->getEntityPersister($class->name)->lock(
+                array_combine($class->getIdentifierColumnNames(), $this->_entityIdentifiers[$oid]),
+                $lockMode
+            );
         }
     }
 

tests/Doctrine/Tests/Models/CMS/CmsArticle.php

@@ -31,6 +31,11 @@ class CmsArticle
      * @OneToMany(targetEntity="CmsComment", mappedBy="article")
      */
     public $comments;
+
+    /**
+     * @Version @column(type="integer")
+     */
+    public $version;
     
     public function setAuthor(CmsUser $author) {
         $this->user = $author;

tests/Doctrine/Tests/ORM/Functional/EntityRepositoryTest.php

@@ -145,8 +145,9 @@ class EntityRepositoryTest extends \Doctrine\Tests\OrmFunctionalTestCase
 
         $userId = $user->id;
 
-        $this->setExpectedException('Doctrine\ORM\OptimisticLockException');
         $this->_em->find('Doctrine\Tests\Models\Cms\CmsUser', $userId);
+        
+        $this->setExpectedException('Doctrine\ORM\OptimisticLockException');
         $this->_em->find('Doctrine\Tests\Models\Cms\CmsUser', $userId, \Doctrine\ORM\LockMode::OPTIMISTIC);
     }
 }

tests/Doctrine/Tests/ORM/Functional/EntityRepositoryTest.php.rej

-***************
-*** 93,97 ****
-          $this->_em->getRepository('Doctrine\Tests\Models\CMS\CmsUser')
-                    ->findByThisFieldDoesNotExist('testvalue');
-      }
-  }
-  
---- 93,153 ----
-          $this->_em->getRepository('Doctrine\Tests\Models\CMS\CmsUser')
-                    ->findByThisFieldDoesNotExist('testvalue');
-      }
-+ 
-+     /**
-+      * @group locking
-+      * @group DDC-178
-+      */
-+     public function testPessimisticReadLockWithoutTransaction_ThrowsException()
-+     {
-+         $this->setExpectedException('Doctrine\ORM\TransactionRequiredException');
-+ 
-+         $this->_em->getRepository('Doctrine\Tests\Models\CMS\CmsUser')
-+                   ->find(1, \Doctrine\ORM\LockMode::PESSIMISTIC_READ);
-+     }
-+ 
-+     /**
-+      * @group locking
-+      * @group DDC-178
-+      */
-+     public function testPessimisticWriteLockWithoutTransaction_ThrowsException()
-+     {
-+         $this->setExpectedException('Doctrine\ORM\TransactionRequiredException');
-+ 
-+         $this->_em->getRepository('Doctrine\Tests\Models\CMS\CmsUser')
-+                   ->find(1, \Doctrine\ORM\LockMode::PESSIMISTIC_WRITE);
-+     }
-+ 
-+     /**
-+      * @group locking
-+      * @group DDC-178
-+      */
-+     public function testOptimisticLockUnversionedEntity_ThrowsException()
-+     {
-+         $this->setExpectedException('Doctrine\ORM\OptimisticLockException');
-+ 
-+         $this->_em->getRepository('Doctrine\Tests\Models\CMS\CmsUser')
-+                   ->find(1, \Doctrine\ORM\LockMode::OPTIMISTIC);
-+     }
-+ 
-+     /**
-+      * @group locking
-+      * @group DDC-178
-+      */
-+     public function testIdentityMappedOptimisticLockUnversionedEntity_ThrowsException()
-+     {
-+         $user = new CmsUser;
-+         $user->name = 'Roman';
-+         $user->username = 'romanb';
-+         $user->status = 'freak';
-+         $this->_em->persist($user);
-+         $this->_em->flush();
-+ 
-+         $userId = $user->id;
-+ 
-+         $this->setExpectedException('Doctrine\ORM\OptimisticLockException');
-+         $this->_em->find('Doctrine\Tests\Models\Cms\CmsUser', $userId);
-+         $this->_em->find('Doctrine\Tests\Models\Cms\CmsUser', $userId, \Doctrine\ORM\LockMode::OPTIMISTIC);
-+     }
-  }
-  

tests/Doctrine/Tests/ORM/Functional/Locking/AllTests.php

@@ -20,6 +20,7 @@ class AllTests
         $suite = new \Doctrine\Tests\DoctrineTestSuite('Doctrine Orm Functional Locking');
 
         $suite->addTestSuite('Doctrine\Tests\ORM\Functional\Locking\OptimisticTest');
+        $suite->addTestSuite('Doctrine\Tests\ORM\Functional\Locking\LockTest');
 
         return $suite;
     }

tests/Doctrine/Tests/ORM/Functional/Locking/LockTest.php

+<?php
+
+namespace Doctrine\Tests\ORM\Functional\Locking;
+
+use Doctrine\Tests\Models\CMS\CmsArticle,
+    Doctrine\Tests\Models\CMS\CmsUser,
+    Doctrine\ORM\LockMode;
+
+require_once __DIR__ . '/../../../TestInit.php';
+
+class LockTest extends \Doctrine\Tests\OrmFunctionalTestCase
+{
+    protected function setUp()
+    {
+        $this->useModelSet('cms');
+        parent::setUp();
+    }
+
+    /**
+     * @group DDC-178
+     * @group locking
+     */
+    public function testLockVersionedEntity()
+    {
+        $article = new CmsArticle();
+        $article->text = "my article";
+        $article->topic = "Hello";
+
+        $this->_em->persist($article);
+        $this->_em->flush();
+
+        $this->_em->lock($article, LockMode::OPTIMISTIC, $article->version);
+    }
+
+    /**
+     * @group DDC-178
+     * @group locking
+     */
+    public function testLockVersionedEntity_MissmatchThrowsException()
+    {
+        $article = new CmsArticle();
+        $article->text = "my article";
+        $article->topic = "Hello";
+
+        $this->_em->persist($article);
+        $this->_em->flush();
+
+        $this->setExpectedException('Doctrine\ORM\OptimisticLockException');
+        $this->_em->lock($article, LockMode::OPTIMISTIC, $article->version + 1);
+    }
+
+    /**
+     * @group DDC-178
+     * @group locking
+     */
+    public function testLockUnversionedEntity_ThrowsException()
+    {
+        $user = new CmsUser();
+        $user->name = "foo";
+        $user->status = "active";
+        $user->username = "foo";
+
+        $this->_em->persist($user);
+        $this->_em->flush();
+
+        $this->setExpectedException('Doctrine\ORM\OptimisticLockException');
+        $this->_em->lock($user, LockMode::OPTIMISTIC);
+    }
+
+    /**
+     * @group DDC-178
+     * @group locking
+     */
+    public function testLockUnmanagedEntity_ThrowsException()
+    {
+        $article = new CmsArticle();
+
+        $this->setExpectedException('InvalidArgumentException', 'Entity is not MANAGED.');
+        $this->_em->lock($article, LockMode::OPTIMISTIC, $article->version + 1);
+    }
+
+    /**
+     * @group DDC-178
+     * @group locking
+     */
+    public function testLockPessimisticRead_NoTransaction_ThrowsException()
+    {
+        $article = new CmsArticle();
+        $article->text = "my article";
+        $article->topic = "Hello";
+
+        $this->_em->persist($article);
+        $this->_em->flush();
+
+        $this->setExpectedException('Doctrine\ORM\TransactionRequiredException');
+        $this->_em->lock($article, LockMode::PESSIMISTIC_READ);
+    }
+
+    /**
+     * @group DDC-178
+     * @group locking
+     */
+    public function testLockPessimisticWrite_NoTransaction_ThrowsException()
+    {
+        $article = new CmsArticle();
+        $article->text = "my article";
+        $article->topic = "Hello";
+
+        $this->_em->persist($article);
+        $this->_em->flush();
+
+        $this->setExpectedException('Doctrine\ORM\TransactionRequiredException');
+        $this->_em->lock($article, LockMode::PESSIMISTIC_WRITE);
+    }
+
+    /**
+     * @group DDC-178
+     * @group locking
+     */
+    public function testLockPessimisticWrite()
+    {
+        $writeLockSql = $this->_em->getConnection()->getDatabasePlatform()->getWriteLockSql();
+        if (strlen($writeLockSql) == 0) {
+            $this->markTestSkipped('Database Driver has no Write Lock support.');
+        }
+
+        $article = new CmsArticle();
+        $article->text = "my article";
+        $article->topic = "Hello";
+
+        $this->_em->persist($article);
+        $this->_em->flush();
+
+        $this->_em->beginTransaction();
+        $this->_em->lock($article, LockMode::PESSIMISTIC_WRITE);
+
+        $query = array_pop( $this->_sqlLoggerStack->queries );
+        $this->assertContains($writeLockSql, $query['sql']);
+    }
+
+    /**
+     * @group DDC-178
+     * @group locking
+     */
+    public function testLockPessimisticRead()
+    {
+        $readLockSql = $this->_em->getConnection()->getDatabasePlatform()->getReadLockSql();
+        if (strlen($readLockSql) == 0) {
+            $this->markTestSkipped('Database Driver has no Write Lock support.');
+        }
+
+        $article = new CmsArticle();
+        $article->text = "my article";
+        $article->topic = "Hello";
+
+        $this->_em->persist($article);
+        $this->_em->flush();
+
+        $this->_em->beginTransaction();
+        $this->_em->lock($article, LockMode::PESSIMISTIC_READ);
+
+        $query = array_pop( $this->_sqlLoggerStack->queries );
+        $this->assertContains($readLockSql, $query['sql']);
+    }
+}
\ No newline at end of file

tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php

@@ -59,7 +59,7 @@ class SelectSqlGenerationTest extends \Doctrine\Tests\OrmTestCase
     {
         $this->assertSqlGeneration(
             'SELECT a FROM Doctrine\Tests\Models\CMS\CmsArticle a ORDER BY a.user.name ASC',
-            'SELECT c0_.id AS id0, c0_.topic AS topic1, c0_.text AS text2 FROM cms_articles c0_ INNER JOIN cms_users c1_ ON c0_.user_id = c1_.id ORDER BY c1_.name ASC'
+            'SELECT c0_.id AS id0, c0_.topic AS topic1, c0_.text AS text2, c0_.version AS version3 FROM cms_articles c0_ INNER JOIN cms_users c1_ ON c0_.user_id = c1_.id ORDER BY c1_.name ASC'
         );
     }
 
@@ -183,11 +183,11 @@ class SelectSqlGenerationTest extends \Doctrine\Tests\OrmTestCase
         );
     }
 
-    public function testSupportsMultipleEntitesInFromClause()
+    public function testSupportsMultipleEntitiesInFromClause()
     {
         $this->assertSqlGeneration(
             'SELECT u, a FROM Doctrine\Tests\Models\CMS\CmsUser u, Doctrine\Tests\Models\CMS\CmsArticle a WHERE u.id = a.user.id',
-            'SELECT c0_.id AS id0, c0_.status AS status1, c0_.username AS username2, c0_.name AS name3, c1_.id AS id4, c1_.topic AS topic5, c1_.text AS text6 FROM cms_users c0_ INNER JOIN cms_users c2_ ON c1_.user_id = c2_.id WHERE c0_.id = c2_.id'
+            'SELECT c0_.id AS id0, c0_.status AS status1, c0_.username AS username2, c0_.name AS name3, c1_.id AS id4, c1_.topic AS topic5, c1_.text AS text6, c1_.version AS version7 FROM cms_users c0_ INNER JOIN cms_users c2_ ON c1_.user_id = c2_.id WHERE c0_.id = c2_.id'
         );
     }