From dfbd9e6e2fa2e59878288946f71684e7f6c90539 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sun, 11 Apr 2010 16:43:33 +0200 Subject: [PATCH] DDC-178 - Add additional tests for Locking Support --- lib/Doctrine/ORM/EntityManager.php | 2 +- lib/Doctrine/ORM/OptimisticLockException.php | 5 + .../Persisters/StandardEntityPersister.php | 2 +- lib/Doctrine/ORM/UnitOfWork.php | 22 +-- .../Doctrine/Tests/Models/CMS/CmsArticle.php | 5 + .../ORM/Functional/EntityRepositoryTest.php | 3 +- .../Functional/EntityRepositoryTest.php.rej | 69 -------- .../Tests/ORM/Functional/Locking/AllTests.php | 1 + .../Tests/ORM/Functional/Locking/LockTest.php | 165 ++++++++++++++++++ .../ORM/Query/SelectSqlGenerationTest.php | 6 +- 10 files changed, 194 insertions(+), 86 deletions(-) delete mode 100644 tests/Doctrine/Tests/ORM/Functional/EntityRepositoryTest.php.rej create mode 100644 tests/Doctrine/Tests/ORM/Functional/Locking/LockTest.php diff --git a/lib/Doctrine/ORM/EntityManager.php b/lib/Doctrine/ORM/EntityManager.php index 7cbfb8843..7836ff782 100644 --- a/lib/Doctrine/ORM/EntityManager.php +++ b/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); } diff --git a/lib/Doctrine/ORM/OptimisticLockException.php b/lib/Doctrine/ORM/OptimisticLockException.php index 01f076467..c77299236 100644 --- a/lib/Doctrine/ORM/OptimisticLockException.php +++ b/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); diff --git a/lib/Doctrine/ORM/Persisters/StandardEntityPersister.php b/lib/Doctrine/ORM/Persisters/StandardEntityPersister.php index 0d4a26295..bc1846956 100644 --- a/lib/Doctrine/ORM/Persisters/StandardEntityPersister.php +++ b/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); } } diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index 431a2685c..eea12aeba 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/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 + ); } } diff --git a/tests/Doctrine/Tests/Models/CMS/CmsArticle.php b/tests/Doctrine/Tests/Models/CMS/CmsArticle.php index 1d7901557..222212907 100644 --- a/tests/Doctrine/Tests/Models/CMS/CmsArticle.php +++ b/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; diff --git a/tests/Doctrine/Tests/ORM/Functional/EntityRepositoryTest.php b/tests/Doctrine/Tests/ORM/Functional/EntityRepositoryTest.php index 7007c5e26..9bb2253ee 100644 --- a/tests/Doctrine/Tests/ORM/Functional/EntityRepositoryTest.php +++ b/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); } } diff --git a/tests/Doctrine/Tests/ORM/Functional/EntityRepositoryTest.php.rej b/tests/Doctrine/Tests/ORM/Functional/EntityRepositoryTest.php.rej deleted file mode 100644 index 17f059d65..000000000 --- a/tests/Doctrine/Tests/ORM/Functional/EntityRepositoryTest.php.rej +++ /dev/null @@ -1,69 +0,0 @@ -*************** -*** 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); -+ } - } - diff --git a/tests/Doctrine/Tests/ORM/Functional/Locking/AllTests.php b/tests/Doctrine/Tests/ORM/Functional/Locking/AllTests.php index 9b021ced8..be725f0b0 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Locking/AllTests.php +++ b/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; } diff --git a/tests/Doctrine/Tests/ORM/Functional/Locking/LockTest.php b/tests/Doctrine/Tests/ORM/Functional/Locking/LockTest.php new file mode 100644 index 000000000..974308742 --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Functional/Locking/LockTest.php @@ -0,0 +1,165 @@ +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 diff --git a/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php b/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php index 812513cd6..947825f72 100644 --- a/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php +++ b/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' ); }