From e6a44b145fbe0ee140fcd1eb9dcf169ab6c9da20 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Thu, 8 Apr 2010 22:50:06 +0200 Subject: [PATCH 1/5] [DDC-178] First approach to Locking support --- lib/Doctrine/DBAL/LockMode.php | 42 +++++++++++ .../DBAL/Platforms/AbstractPlatform.php | 39 +++++++++- lib/Doctrine/DBAL/Platforms/MsSqlPlatform.php | 28 ++++++++ lib/Doctrine/DBAL/Platforms/MySqlPlatform.php | 5 ++ .../DBAL/Platforms/PostgreSqlPlatform.php | 5 ++ .../DBAL/Platforms/SqlitePlatform.php | 5 ++ lib/Doctrine/ORM/EntityManager.php | 20 +++++- lib/Doctrine/ORM/EntityRepository.php | 30 ++++++-- lib/Doctrine/ORM/LockMode.php | 37 ++++++++++ lib/Doctrine/ORM/OptimisticLockException.php | 5 ++ .../Persisters/JoinedSubclassPersister.php | 2 +- .../Persisters/StandardEntityPersister.php | 58 +++++++++++++-- lib/Doctrine/ORM/PessimisticLockException.php | 40 +++++++++++ lib/Doctrine/ORM/Query.php | 38 ++++++++++ lib/Doctrine/ORM/Query/SqlWalker.php | 21 +++++- .../ORM/TransactionRequiredException.php | 40 +++++++++++ lib/Doctrine/ORM/UnitOfWork.php | 42 +++++++++++ .../ORM/Functional/EntityRepositoryTest.php | 56 +++++++++++++++ .../Functional/EntityRepositoryTest.php.rej | 69 ++++++++++++++++++ .../ORM/Query/SelectSqlGenerationTest.php | 71 ++++++++++++++++++- 20 files changed, 639 insertions(+), 14 deletions(-) create mode 100644 lib/Doctrine/DBAL/LockMode.php create mode 100644 lib/Doctrine/ORM/LockMode.php create mode 100644 lib/Doctrine/ORM/PessimisticLockException.php create mode 100644 lib/Doctrine/ORM/TransactionRequiredException.php create mode 100644 tests/Doctrine/Tests/ORM/Functional/EntityRepositoryTest.php.rej diff --git a/lib/Doctrine/DBAL/LockMode.php b/lib/Doctrine/DBAL/LockMode.php new file mode 100644 index 000000000..949072166 --- /dev/null +++ b/lib/Doctrine/DBAL/LockMode.php @@ -0,0 +1,42 @@ +. +*/ + +namespace Doctrine\DBAL; + +/** + * Contains all ORM LockModes + * + * @license http://www.opensource.org/licenses/lgpl-license.php LGPL + * @link www.doctrine-project.com + * @since 1.0 + * @version $Revision$ + * @author Benjamin Eberlei + * @author Roman Borschel + */ +class LockMode +{ + const NONE = 0; + const OPTIMISTIC = 1; + const PESSIMISTIC_READ = 2; + const PESSIMISTIC_WRITE = 4; + + final private function __construct() { } +} \ No newline at end of file diff --git a/lib/Doctrine/DBAL/Platforms/AbstractPlatform.php b/lib/Doctrine/DBAL/Platforms/AbstractPlatform.php index 961ca0b7d..1ca4c049a 100644 --- a/lib/Doctrine/DBAL/Platforms/AbstractPlatform.php +++ b/lib/Doctrine/DBAL/Platforms/AbstractPlatform.php @@ -488,11 +488,48 @@ abstract class AbstractPlatform return 'COS(' . $value . ')'; } - public function getForUpdateSql() + public function getForUpdateSQL() { return 'FOR UPDATE'; } + /** + * Honors that some SQL vendors such as MsSql use table hints for locking instead of the ANSI SQL FOR UPDATE specification. + * + * @param string $fromClause + * @param int $lockMode + * @return string + */ + public function appendLockHint($fromClause, $lockMode) + { + return $fromClause; + } + + /** + * Get the sql snippet to append to any SELECT statement which locks rows in shared read lock. + * + * This defaults to the ASNI SQL "FOR UPDATE", which is an exclusive lock (Write). Some database + * vendors allow to lighten this constraint up to be a real read lock. + * + * @return string + */ + public function getReadLockSQL() + { + return $this->getForUpdateSQL(); + } + + /** + * Get the SQL snippet to append to any SELECT statement which obtains an exclusive lock on the rows. + * + * The semantics of this lock mode should equal the SELECT .. FOR UPDATE of the ASNI SQL standard. + * + * @return string + */ + public function getWriteLockSQL() + { + return $this->getForUpdateSQL(); + } + public function getDropDatabaseSQL($database) { return 'DROP DATABASE ' . $database; diff --git a/lib/Doctrine/DBAL/Platforms/MsSqlPlatform.php b/lib/Doctrine/DBAL/Platforms/MsSqlPlatform.php index dd14bd8fd..4421c170c 100644 --- a/lib/Doctrine/DBAL/Platforms/MsSqlPlatform.php +++ b/lib/Doctrine/DBAL/Platforms/MsSqlPlatform.php @@ -483,4 +483,32 @@ class MsSqlPlatform extends AbstractPlatform { return 'TRUNCATE TABLE '.$tableName; } + + /** + * MsSql uses Table Hints for locking strategies instead of the ANSI SQL FOR UPDATE like hints. + * + * @return string + */ + public function getForUpdateSQL() + { + return ''; + } + + /** + * @license LGPL + * @author Hibernate + * @param string $fromClause + * @param int $lockMode + * @return string + */ + public function appendLockHint($fromClause, $lockMode) + { + if ($lockMode == \Doctrine\DBAL\LockMode::PESSIMISTIC_WRITE) { + return $fromClause . " WITH (UPDLOCK, ROWLOCK)"; + } else if ( $lockMode == \Doctrine\DBAL\LockMode::PESSIMISTIC_READ ) { + return $fromClause . " WITH (HOLDLOCK, ROWLOCK)"; + } else { + return $fromClause; + } + } } diff --git a/lib/Doctrine/DBAL/Platforms/MySqlPlatform.php b/lib/Doctrine/DBAL/Platforms/MySqlPlatform.php index 5bf9b84c2..b7ba86d88 100644 --- a/lib/Doctrine/DBAL/Platforms/MySqlPlatform.php +++ b/lib/Doctrine/DBAL/Platforms/MySqlPlatform.php @@ -666,4 +666,9 @@ class MySqlPlatform extends AbstractPlatform { return true; } + + public function getReadLockSQL() + { + return 'LOCK IN SHARE MODE'; + } } diff --git a/lib/Doctrine/DBAL/Platforms/PostgreSqlPlatform.php b/lib/Doctrine/DBAL/Platforms/PostgreSqlPlatform.php index 972ddc50a..f57c8e85a 100644 --- a/lib/Doctrine/DBAL/Platforms/PostgreSqlPlatform.php +++ b/lib/Doctrine/DBAL/Platforms/PostgreSqlPlatform.php @@ -637,4 +637,9 @@ class PostgreSqlPlatform extends AbstractPlatform { return 'TRUNCATE '.$tableName.' '.($cascade)?'CASCADE':''; } + + public function getReadLockSQL() + { + return 'FOR SHARE'; + } } diff --git a/lib/Doctrine/DBAL/Platforms/SqlitePlatform.php b/lib/Doctrine/DBAL/Platforms/SqlitePlatform.php index a1c2184b9..8b209fa60 100644 --- a/lib/Doctrine/DBAL/Platforms/SqlitePlatform.php +++ b/lib/Doctrine/DBAL/Platforms/SqlitePlatform.php @@ -428,4 +428,9 @@ class SqlitePlatform extends AbstractPlatform } return 0; } + + public function getForUpdateSql() + { + return ''; + } } diff --git a/lib/Doctrine/ORM/EntityManager.php b/lib/Doctrine/ORM/EntityManager.php index 897569b8c..7cbfb8843 100644 --- a/lib/Doctrine/ORM/EntityManager.php +++ b/lib/Doctrine/ORM/EntityManager.php @@ -288,11 +288,13 @@ class EntityManager * * @param string $entityName * @param mixed $identifier + * @param int $lockMode + * @param int $lockVersion * @return object */ - public function find($entityName, $identifier) + public function find($entityName, $identifier, $lockMode = LockMode::NONE, $lockVersion = null) { - return $this->getRepository($entityName)->find($identifier); + return $this->getRepository($entityName)->find($identifier, $lockMode, $lockVersion); } /** @@ -447,6 +449,20 @@ class EntityManager throw new \BadMethodCallException("Not implemented."); } + /** + * Acquire a lock on the given entity. + * + * @param object $entity + * @param int $lockMode + * @param int $lockVersion + * @throws OptimisticLockException + * @throws PessimisticLockException + */ + public function lock($entity, $lockMode, $lockVersion) + { + $this->_unitOfWork->lock($entity, $lockMode, $lockVersion); + } + /** * Gets the repository for an entity class. * diff --git a/lib/Doctrine/ORM/EntityRepository.php b/lib/Doctrine/ORM/EntityRepository.php index 1382cb5e6..35aa7481e 100644 --- a/lib/Doctrine/ORM/EntityRepository.php +++ b/lib/Doctrine/ORM/EntityRepository.php @@ -92,23 +92,45 @@ class EntityRepository * Finds an entity by its primary key / identifier. * * @param $id The identifier. - * @param int $hydrationMode The hydration mode to use. + * @param int $lockMode + * @param int $lockVersion * @return object The entity. */ - public function find($id) + public function find($id, $lockMode = LockMode::NONE, $lockVersion = null) { // Check identity map first if ($entity = $this->_em->getUnitOfWork()->tryGetById($id, $this->_class->rootEntityName)) { + if ($lockMode != LockMode::NONE) { + $this->_em->lock($entity, $lockMode, $lockVersion); + } + return $entity; // Hit! } if ( ! is_array($id) || count($id) <= 1) { - //FIXME: Not correct. Relies on specific order. + // @todo FIXME: Not correct. Relies on specific order. $value = is_array($id) ? array_values($id) : array($id); $id = array_combine($this->_class->identifier, $value); } - return $this->_em->getUnitOfWork()->getEntityPersister($this->_entityName)->load($id); + if ($lockMode == LockMode::NONE) { + return $this->_em->getUnitOfWork()->getEntityPersister($this->_entityName)->load($id); + } else if ($lockMode == LockMode::OPTIMISTIC) { + if (!$this->_class->isVersioned) { + throw OptimisticLockException::notVersioned($this->_entityName); + } + $entity = $this->_em->getUnitOfWork()->getEntityPersister($this->_entityName)->load($id); + + $this->_em->getUnitOfWork()->lock($entity, $lockMode, $lockVersion); + + return $entity; + } else { + if (!$this->_em->getConnection()->isTransactionActive()) { + throw TransactionRequiredException::transactionRequired(); + } + + return $this->_em->getUnitOfWork()->getEntityPersister($this->_entityName)->load($id, null, null, array(), $lockMode); + } } /** diff --git a/lib/Doctrine/ORM/LockMode.php b/lib/Doctrine/ORM/LockMode.php new file mode 100644 index 000000000..45f999a30 --- /dev/null +++ b/lib/Doctrine/ORM/LockMode.php @@ -0,0 +1,37 @@ +. +*/ + +namespace Doctrine\ORM; + +/** + * Contains all ORM LockModes + * + * @license http://www.opensource.org/licenses/lgpl-license.php LGPL + * @link www.doctrine-project.com + * @since 1.0 + * @version $Revision$ + * @author Benjamin Eberlei + * @author Roman Borschel + */ +class LockMode extends \Doctrine\DBAL\LockMode +{ + +} \ No newline at end of file diff --git a/lib/Doctrine/ORM/OptimisticLockException.php b/lib/Doctrine/ORM/OptimisticLockException.php index d937daed4..01f076467 100644 --- a/lib/Doctrine/ORM/OptimisticLockException.php +++ b/lib/Doctrine/ORM/OptimisticLockException.php @@ -36,4 +36,9 @@ class OptimisticLockException extends ORMException { return new self("The optimistic lock failed."); } + + public static function notVersioned($className) + { + return new self("Cannot obtain optimistic lock on unversioned entity ".$className); + } } \ No newline at end of file diff --git a/lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php b/lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php index c6e08abf4..ffe96b6ca 100644 --- a/lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php +++ b/lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php @@ -235,7 +235,7 @@ class JoinedSubclassPersister extends AbstractEntityInheritancePersister /** * {@inheritdoc} */ - protected function _getSelectEntitiesSQL(array &$criteria, $assoc = null, $orderBy = null) + protected function _getSelectEntitiesSQL(array &$criteria, $assoc = null, $orderBy = null, $lockMode = 0) { $idColumns = $this->_class->getIdentifierColumnNames(); $baseTableAlias = $this->_getSQLTableAlias($this->_class); diff --git a/lib/Doctrine/ORM/Persisters/StandardEntityPersister.php b/lib/Doctrine/ORM/Persisters/StandardEntityPersister.php index a02a9b191..1bde9a90e 100644 --- a/lib/Doctrine/ORM/Persisters/StandardEntityPersister.php +++ b/lib/Doctrine/ORM/Persisters/StandardEntityPersister.php @@ -423,11 +423,12 @@ class StandardEntityPersister * a new entity is created. * @param $assoc The association that connects the entity to load to another entity, if any. * @param array $hints Hints for entity creation. + * @param int $lockMode * @return The loaded entity instance or NULL if the entity/the data can not be found. */ - public function load(array $criteria, $entity = null, $assoc = null, array $hints = array()) + public function load(array $criteria, $entity = null, $assoc = null, array $hints = array(), $lockMode = 0) { - $sql = $this->_getSelectEntitiesSQL($criteria, $assoc); + $sql = $this->_getSelectEntitiesSQL($criteria, $assoc, null, $lockMode); $params = array_values($criteria); $stmt = $this->_conn->executeQuery($sql, $params); $result = $stmt->fetch(PDO::FETCH_ASSOC); @@ -641,9 +642,10 @@ class StandardEntityPersister * @param array $criteria * @param AssociationMapping $assoc * @param string $orderBy + * @param int $lockMode * @return string */ - protected function _getSelectEntitiesSQL(array &$criteria, $assoc = null, $orderBy = null) + protected function _getSelectEntitiesSQL(array &$criteria, $assoc = null, $orderBy = null, $lockMode = 0) { // Construct WHERE conditions $conditionSql = ''; @@ -671,10 +673,17 @@ class StandardEntityPersister ); } + $lockSql = ''; + if ($lockMode == \Doctrine\ORM\LockMode::PESSIMISTIC_READ) { + $lockSql = ' ' . $this->_platform->getReadLockSql(); + } else if ($lockMode == \Doctrine\ORM\LockMode::PESSIMISTIC_WRITE) { + $lockSql = ' ' . $this->_platform->getWriteLockSql(); + } + return 'SELECT ' . $this->_getSelectColumnListSQL() . ' FROM ' . $this->_class->getQuotedTableName($this->_platform) . ' ' . $this->_getSQLTableAlias($this->_class) - . ($conditionSql ? ' WHERE ' . $conditionSql : '') . $orderBySql; + . ($conditionSql ? ' WHERE ' . $conditionSql : '') . $orderBySql . $lockSql; } /** @@ -912,4 +921,45 @@ class StandardEntityPersister return $tableAlias; } + + /** + * Lock all rows of this entity matching the given criteria with the specified pessimistic lock mode + * + * @param array $criteria + * @param int $lockMode + * @return void + */ + public function lock(array $criteria, $lockMode) + { + // @todo Extract method to remove duplicate code from _getSelectEntitiesSQL()? + $conditionSql = ''; + foreach ($criteria as $field => $value) { + if ($conditionSql != '') { + $conditionSql .= ' AND '; + } + + if (isset($this->_class->columnNames[$field])) { + $conditionSql .= $this->_class->getQuotedColumnName($field, $this->_platform); + } else if (isset($this->_class->fieldNames[$field])) { + $conditionSql .= $this->_class->getQuotedColumnName($this->_class->fieldNames[$field], $this->_platform); + } else if ($assoc !== null) { + $conditionSql .= $field; + } else { + throw ORMException::unrecognizedField($field); + } + $conditionSql .= ' = ?'; + } + + if ($lockMode == \Doctrine\ORM\LockMode::PESSIMISTIC_READ) { + $lockSql = $this->_platform->getReadLockSql(); + } else if ($lockMode == \Doctrine\ORM\LockMode::PESSIMISTIC_WRITE) { + $lockSql = $this->_platform->getWriteLockSql(); + } + + $sql = 'SELECT 1 FROM ' . $this->_class->getQuotedTableName($this->_platform) . ' ' + . $this->_getSQLTableAlias($this->_class) + . ($conditionSql ? ' WHERE ' . $conditionSql : '') . ' ' . $lockSql; + $params = array_values($criteria); + $this->_conn->executeQuery($query, $params); + } } diff --git a/lib/Doctrine/ORM/PessimisticLockException.php b/lib/Doctrine/ORM/PessimisticLockException.php new file mode 100644 index 000000000..928ead765 --- /dev/null +++ b/lib/Doctrine/ORM/PessimisticLockException.php @@ -0,0 +1,40 @@ +. +*/ + +namespace Doctrine\ORM; + +/** + * Pessimistic Lock Exception + * + * @license http://www.opensource.org/licenses/lgpl-license.php LGPL + * @link www.doctrine-project.com + * @since 1.0 + * @version $Revision$ + * @author Benjamin Eberlei + * @author Roman Borschel + */ +class PessimisticLockException extends ORMException +{ + public static function lockFailed() + { + return new self("The pessimistic lock failed."); + } +} \ No newline at end of file diff --git a/lib/Doctrine/ORM/Query.php b/lib/Doctrine/ORM/Query.php index 7bbb663f4..bb3ab71f8 100644 --- a/lib/Doctrine/ORM/Query.php +++ b/lib/Doctrine/ORM/Query.php @@ -97,6 +97,11 @@ final class Query extends AbstractQuery */ const HINT_INTERNAL_ITERATION = 'doctrine.internal.iteration'; + /** + * @var string + */ + const HINT_LOCK_MODE = 'doctrine.lockMode'; + /** * @var integer $_state The current state of this query. */ @@ -491,6 +496,39 @@ final class Query extends AbstractQuery return parent::setHydrationMode($hydrationMode); } + /** + * Set the lock mode for this Query. + * + * @see Doctrine\ORM\LockMode + * @param int $lockMode + * @return Query + */ + public function setLockMode($lockMode) + { + if ($lockMode == LockMode::PESSIMISTIC_READ || $lockMode == LockMode::PESSIMISTIC_WRITE) { + if (!$this->_em->getConnection()->isTransactionActive()) { + throw TransactionRequiredException::transactionRequired(); + } + } + + $this->setHint(self::HINT_LOCK_MODE, $lockMode); + return $this; + } + + /** + * Get the current lock mode for this query. + * + * @return int + */ + public function getLockMode() + { + $lockMode = $this->getHint(self::HINT_LOCK_MODE); + if (!$lockMode) { + return LockMode::NONE; + } + return $lockMode; + } + /** * Generate a cache id for the query cache - reusing the Result-Cache-Id generator. * diff --git a/lib/Doctrine/ORM/Query/SqlWalker.php b/lib/Doctrine/ORM/Query/SqlWalker.php index d16d9ff60..5eff0110f 100644 --- a/lib/Doctrine/ORM/Query/SqlWalker.php +++ b/lib/Doctrine/ORM/Query/SqlWalker.php @@ -371,6 +371,25 @@ class SqlWalker implements TreeWalker $sql, $this->_query->getMaxResults(), $this->_query->getFirstResult() ); + if (($lockMode = $this->_query->getHint(Query::HINT_LOCK_MODE)) !== false) { + if ($lockMode == \Doctrine\ORM\LockMode::PESSIMISTIC_READ) { + $sql .= " " . $this->_platform->getReadLockSQL(); + } else if ($lockMode == \Doctrine\ORM\LockMode::PESSIMISTIC_WRITE) { + $sql .= " " . $this->_platform->getWriteLockSQL(); + } else if ($lockMode == \Doctrine\ORM\LockMode::OPTIMISTIC) { + $versionedClassFound = false; + foreach ($this->_selectedClasses AS $class) { + if ($class->isVersioned) { + $versionedClassFound = true; + } + } + + if (!$versionedClassFound) { + throw \Doctrine\ORM\OptimisticLockException::lockFailed(); + } + } + } + return $sql; } @@ -597,7 +616,7 @@ class SqlWalker implements TreeWalker $sql .= $this->walkJoinVariableDeclaration($joinVarDecl); } - return $sql; + return $this->_platform->appendLockHint($sql, $this->_query->getHint(Query::HINT_LOCK_MODE)); } /** diff --git a/lib/Doctrine/ORM/TransactionRequiredException.php b/lib/Doctrine/ORM/TransactionRequiredException.php new file mode 100644 index 000000000..170f63e50 --- /dev/null +++ b/lib/Doctrine/ORM/TransactionRequiredException.php @@ -0,0 +1,40 @@ +. +*/ + +namespace Doctrine\ORM; + +/** + * Is thrown when a transaction is required for the current operation, but there is none open. + * + * @license http://www.opensource.org/licenses/lgpl-license.php LGPL + * @link www.doctrine-project.com + * @since 1.0 + * @version $Revision$ + * @author Benjamin Eberlei + * @author Roman Borschel + */ +class TransactionRequiredException extends ORMException +{ + static public function transactionRequired() + { + return new self('An open transaction is required for this operation.'); + } +} \ No newline at end of file diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index d1a89bed4..f084640d8 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -1631,6 +1631,48 @@ class UnitOfWork implements PropertyChangedListener } } + /** + * Acquire a lock on the given entity. + * + * @param object $entity + * @param int $lockMode + * @param int $lockVersion + */ + public function lock($entity, $lockMode, $lockVersion = null) + { + $entityName = get_class($entity); + $class = $this->_em->getClassMetadata($entityName); + + if ($lockMode == LockMode::OPTIMISTIC) { + if (!$class->isVersioned) { + throw OptimisticLockException::notVersioned($entityName); + } + + if ($lockVersion != null) { + $entityVersion = $class->reflFields[$class->versionField]->getValue($entity); + if ($entityVersion != $lockVersion) { + throw OptimisticLockException::lockFailed(); + } + } + } else if ($lockMode == LockMode::PESSIMISTIC_READ || $lockMode == LockMode::PESSIMISTIC_WRITE) { + + if (!$this->_em->getConnection()->isTransactionActive()) { + throw TransactionRequiredException::transactionRequired(); + } + + if ($this->getEntityState($entity) == self::STATE_MANAGED) { + $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."); + } + } + } + /** * Gets the CommitOrderCalculator used by the UnitOfWork to order commits. * diff --git a/tests/Doctrine/Tests/ORM/Functional/EntityRepositoryTest.php b/tests/Doctrine/Tests/ORM/Functional/EntityRepositoryTest.php index 856c7b8d2..7007c5e26 100644 --- a/tests/Doctrine/Tests/ORM/Functional/EntityRepositoryTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/EntityRepositoryTest.php @@ -93,5 +93,61 @@ class EntityRepositoryTest extends \Doctrine\Tests\OrmFunctionalTestCase $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/EntityRepositoryTest.php.rej b/tests/Doctrine/Tests/ORM/Functional/EntityRepositoryTest.php.rej new file mode 100644 index 000000000..17f059d65 --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Functional/EntityRepositoryTest.php.rej @@ -0,0 +1,69 @@ +*************** +*** 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/Query/SelectSqlGenerationTest.php b/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php index 163bc594e..812513cd6 100644 --- a/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php +++ b/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php @@ -15,12 +15,15 @@ class SelectSqlGenerationTest extends \Doctrine\Tests\OrmTestCase $this->_em = $this->_getTestEntityManager(); } - public function assertSqlGeneration($dqlToBeTested, $sqlToBeConfirmed) + public function assertSqlGeneration($dqlToBeTested, $sqlToBeConfirmed, array $queryHints = array()) { try { $query = $this->_em->createQuery($dqlToBeTested); $query->setHint(Query::HINT_FORCE_PARTIAL_LOAD, true) ->useQueryCache(false); + foreach ($queryHints AS $name => $value) { + $query->setHint($name, $value); + } parent::assertEquals($sqlToBeConfirmed, $query->getSql()); $query->free(); } catch (\Exception $e) { @@ -584,4 +587,70 @@ class SelectSqlGenerationTest extends \Doctrine\Tests\OrmTestCase "SELECT c0_.name AS name0, (SELECT COUNT(c1_.phonenumber) AS dctrn__1 FROM cms_phonenumbers c1_ WHERE c1_.phonenumber = 1234) AS sclr1 FROM cms_users c0_ WHERE c0_.name = 'jon'" ); } + + /** + * @group locking + * @group DDC-178 + */ + public function testPessimisticWriteLockQueryHint() + { + if ($this->_em->getConnection()->getDatabasePlatform() instanceof \Doctrine\DBAL\Platforms\SqlitePlatform) { + $this->markTestSkipped('SqLite does not support Row locking at all.'); + } + + $this->assertSqlGeneration( + "SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u WHERE u.username = 'gblanco'", + "SELECT c0_.id AS id0, c0_.status AS status1, c0_.username AS username2, c0_.name AS name3 ". + "FROM cms_users c0_ WHERE c0_.username = 'gblanco' FOR UPDATE", + array(Query::HINT_LOCK_MODE => \Doctrine\ORM\LockMode::PESSIMISTIC_WRITE) + ); + } + + /** + * @group locking + * @group DDC-178 + */ + public function testPessimisticReadLockQueryHintPostgreSql() + { + $this->_em->getConnection()->setDatabasePlatform(new \Doctrine\DBAL\Platforms\PostgreSqlPlatform); + + $this->assertSqlGeneration( + "SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u WHERE u.username = 'gblanco'", + "SELECT c0_.id AS id0, c0_.status AS status1, c0_.username AS username2, c0_.name AS name3 ". + "FROM cms_users c0_ WHERE c0_.username = 'gblanco' FOR SHARE", + array(Query::HINT_LOCK_MODE => \Doctrine\ORM\LockMode::PESSIMISTIC_READ) + ); + } + + /** + * @group locking + * @group DDC-178 + */ + public function testPessimisticReadLockQueryHintMySql() + { + $this->_em->getConnection()->setDatabasePlatform(new \Doctrine\DBAL\Platforms\MySqlPlatform); + + $this->assertSqlGeneration( + "SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u WHERE u.username = 'gblanco'", + "SELECT c0_.id AS id0, c0_.status AS status1, c0_.username AS username2, c0_.name AS name3 ". + "FROM cms_users c0_ WHERE c0_.username = 'gblanco' LOCK IN SHARE MODE", + array(Query::HINT_LOCK_MODE => \Doctrine\ORM\LockMode::PESSIMISTIC_READ) + ); + } + + /** + * @group locking + * @group DDC-178 + */ + public function testPessimisticReadLockQueryHintOracle() + { + $this->_em->getConnection()->setDatabasePlatform(new \Doctrine\DBAL\Platforms\OraclePlatform); + + $this->assertSqlGeneration( + "SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u WHERE u.username = 'gblanco'", + "SELECT c0_.id AS id0, c0_.status AS status1, c0_.username AS username2, c0_.name AS name3 ". + "FROM cms_users c0_ WHERE c0_.username = 'gblanco' FOR UPDATE", + array(Query::HINT_LOCK_MODE => \Doctrine\ORM\LockMode::PESSIMISTIC_READ) + ); + } } From dfbd9e6e2fa2e59878288946f71684e7f6c90539 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sun, 11 Apr 2010 16:43:33 +0200 Subject: [PATCH 2/5] 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' ); } From b8402c9563682b1e547e501378ff8b50292cf3eb Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sun, 2 May 2010 13:04:25 +0200 Subject: [PATCH 3/5] Added Gearman Lock Test and Worker, verified lockings indeed works on MySQL, PostgreSQL and Oracle --- .../Functional/Locking/GearmanLockTest.php | 177 ++++++++++++++++++ .../Functional/Locking/LockAgentWorker.php | 111 +++++++++++ tests/README.markdown | 25 +++ 3 files changed, 313 insertions(+) create mode 100644 tests/Doctrine/Tests/ORM/Functional/Locking/GearmanLockTest.php create mode 100644 tests/Doctrine/Tests/ORM/Functional/Locking/LockAgentWorker.php create mode 100644 tests/README.markdown diff --git a/tests/Doctrine/Tests/ORM/Functional/Locking/GearmanLockTest.php b/tests/Doctrine/Tests/ORM/Functional/Locking/GearmanLockTest.php new file mode 100644 index 000000000..ced552422 --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Functional/Locking/GearmanLockTest.php @@ -0,0 +1,177 @@ +markTestSkipped('pecl/gearman is required for this test to run.'); + } + + $this->useModelSet('cms'); + parent::setUp(); + $this->tasks = array(); + + $this->gearman = new \GearmanClient(); + $this->gearman->addServer(); + $this->gearman->setCompleteCallback(array($this, "gearmanTaskCompleted")); + + $article = new CmsArticle(); + $article->text = "my article"; + $article->topic = "Hello"; + + $this->_em->persist($article); + $this->_em->flush(); + + $this->articleId = $article->id; + } + + public function gearmanTaskCompleted($task) + { + $this->maxRunTime = max($this->maxRunTime, $task->data()); + } + + public function testFindWithLock() + { + $this->asyncFindWithLock('Doctrine\Tests\Models\CMS\CmsArticle', $this->articleId, LockMode::PESSIMISTIC_WRITE); + $this->asyncFindWithLock('Doctrine\Tests\Models\CMS\CmsArticle', $this->articleId, LockMode::PESSIMISTIC_WRITE); + + $this->assertLockWorked(); + } + + public function testFindWithWriteThenReadLock() + { + $this->asyncFindWithLock('Doctrine\Tests\Models\CMS\CmsArticle', $this->articleId, LockMode::PESSIMISTIC_WRITE); + $this->asyncFindWithLock('Doctrine\Tests\Models\CMS\CmsArticle', $this->articleId, LockMode::PESSIMISTIC_READ); + + $this->assertLockWorked(); + } + + public function testFindWithReadThenWriteLock() + { + $this->asyncFindWithLock('Doctrine\Tests\Models\CMS\CmsArticle', $this->articleId, LockMode::PESSIMISTIC_READ); + $this->asyncFindWithLock('Doctrine\Tests\Models\CMS\CmsArticle', $this->articleId, LockMode::PESSIMISTIC_WRITE); + + $this->assertLockWorked(); + } + + public function testFindWithOneLock() + { + $this->asyncFindWithLock('Doctrine\Tests\Models\CMS\CmsArticle', $this->articleId, LockMode::PESSIMISTIC_WRITE); + $this->asyncFindWithLock('Doctrine\Tests\Models\CMS\CmsArticle', $this->articleId, LockMode::NONE); + + $this->assertLockDoesNotBlock(); + } + + public function testDqlWithLock() + { + $this->asyncDqlWithLock('SELECT a FROM Doctrine\Tests\Models\CMS\CmsArticle a', array(), LockMode::PESSIMISTIC_WRITE); + $this->asyncFindWithLock('Doctrine\Tests\Models\CMS\CmsArticle', $this->articleId, LockMode::PESSIMISTIC_WRITE); + + $this->assertLockWorked(); + } + + public function testLock() + { + $this->asyncFindWithLock('Doctrine\Tests\Models\CMS\CmsArticle', $this->articleId, LockMode::PESSIMISTIC_WRITE); + $this->asyncLock('Doctrine\Tests\Models\CMS\CmsArticle', $this->articleId, LockMode::PESSIMISTIC_WRITE); + + $this->assertLockWorked(); + } + + public function testLock2() + { + $this->asyncFindWithLock('Doctrine\Tests\Models\CMS\CmsArticle', $this->articleId, LockMode::PESSIMISTIC_WRITE); + $this->asyncLock('Doctrine\Tests\Models\CMS\CmsArticle', $this->articleId, LockMode::PESSIMISTIC_READ); + + $this->assertLockWorked(); + } + + public function testLock3() + { + $this->asyncFindWithLock('Doctrine\Tests\Models\CMS\CmsArticle', $this->articleId, LockMode::PESSIMISTIC_READ); + $this->asyncLock('Doctrine\Tests\Models\CMS\CmsArticle', $this->articleId, LockMode::PESSIMISTIC_WRITE); + + $this->assertLockWorked(); + } + + public function testLock4() + { + $this->asyncFindWithLock('Doctrine\Tests\Models\CMS\CmsArticle', $this->articleId, LockMode::NONE); + $this->asyncLock('Doctrine\Tests\Models\CMS\CmsArticle', $this->articleId, LockMode::PESSIMISTIC_WRITE); + + $this->assertLockDoesNotBlock(); + } + + protected function assertLockDoesNotBlock() + { + $this->assertLockWorked($onlyForSeconds = 1); + } + + protected function assertLockWorked($forTime = 2, $notLongerThan = null) + { + if ($notLongerThan === null) { + $notLongerThan = $forTime + 1; + } + + $this->gearman->runTasks(); + + $this->assertTrue($this->maxRunTime > $forTime, + "Because of locking this tests should have run at least " . $forTime . " seconds, ". + "but only did for " . $this->maxRunTime . " seconds."); + $this->assertTrue($this->maxRunTime < $notLongerThan, + "The longest task should not run longer than " . $notLongerThan . " seconds, ". + "but did for " . $this->maxRunTime . " seconds." + ); + } + + protected function asyncFindWithLock($entityName, $entityId, $lockMode) + { + $this->startGearmanJob('findWithLock', array( + 'entityName' => $entityName, + 'entityId' => $entityId, + 'lockMode' => $lockMode, + )); + } + + protected function asyncDqlWithLock($dql, $params, $lockMode) + { + $this->startGearmanJob('dqlWithLock', array( + 'dql' => $dql, + 'dqlParams' => $params, + 'lockMode' => $lockMode, + )); + } + + protected function asyncLock($entityName, $entityId, $lockMode) + { + $this->startGearmanJob('lock', array( + 'entityName' => $entityName, + 'entityId' => $entityId, + 'lockMode' => $lockMode, + )); + } + + protected function startGearmanJob($fn, $fixture) + { + $this->gearman->addTask($fn, serialize(array( + 'conn' => $this->_em->getConnection()->getParams(), + 'fixture' => $fixture + ))); + + $this->assertEquals(GEARMAN_SUCCESS, $this->gearman->returnCode()); + } +} \ No newline at end of file diff --git a/tests/Doctrine/Tests/ORM/Functional/Locking/LockAgentWorker.php b/tests/Doctrine/Tests/ORM/Functional/Locking/LockAgentWorker.php new file mode 100644 index 000000000..6b1e6d488 --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Functional/Locking/LockAgentWorker.php @@ -0,0 +1,111 @@ +addServer(); + $worker->addFunction("findWithLock", array($lockAgent, "findWithLock")); + $worker->addFunction("dqlWithLock", array($lockAgent, "dqlWithLock")); + $worker->addFunction('lock', array($lockAgent, 'lock')); + + while($worker->work()) { + if ($worker->returnCode() != GEARMAN_SUCCESS) { + echo "return_code: " . $worker->returnCode() . "\n"; + break; + } + } + } + + protected function process($job, \Closure $do) + { + $fixture = $this->processWorkload($job); + + $s = microtime(true); + $this->em->beginTransaction(); + $do($fixture, $this->em); + + sleep(1); + $this->em->rollback(); + $this->em->clear(); + $this->em->close(); + $this->em->getConnection()->close(); + + return (microtime(true) - $s); + } + + public function findWithLock($job) + { + return $this->process($job, function($fixture, $em) { + $entity = $em->find($fixture['entityName'], $fixture['entityId'], $fixture['lockMode']); + }); + } + + public function dqlWithLock($job) + { + return $this->process($job, function($fixture, $em) { + /* @var $query Doctrine\ORM\Query */ + $query = $em->createQuery($fixture['dql']); + $query->setLockMode($fixture['lockMode']); + $query->setParameters($fixture['dqlParams']); + $result = $query->getResult(); + }); + } + + public function lock($job) + { + return $this->process($job, function($fixture, $em) { + $entity = $em->find($fixture['entityName'], $fixture['entityId']); + $em->lock($entity, $fixture['lockMode']); + }); + } + + protected function processWorkload($job) + { + echo "Received job: " . $job->handle() . " for function " . $job->functionName() . "\n"; + + $workload = $job->workload(); + $workload = unserialize($workload); + + if (!isset($workload['conn']) || !is_array($workload['conn'])) { + throw new \InvalidArgumentException("Missing Database parameters"); + } + + $this->em = $this->createEntityManager($workload['conn']); + + if (!isset($workload['fixture'])) { + throw new \InvalidArgumentException("Missing Fixture parameters"); + } + return $workload['fixture']; + } + + protected function createEntityManager($conn) + { + $config = new \Doctrine\ORM\Configuration(); + $config->setProxyDir(__DIR__ . '/../../../Proxies'); + $config->setProxyNamespace('MyProject\Proxies'); + $config->setAutoGenerateProxyClasses(true); + + $annotDriver = $config->newDefaultAnnotationDriver(array(__DIR__ . '/../../../Models/')); + $config->setMetadataDriverImpl($annotDriver); + + $cache = new \Doctrine\Common\Cache\ArrayCache(); + $config->setMetadataCacheImpl($cache); + $config->setQueryCacheImpl($cache); + + $em = \Doctrine\ORM\EntityManager::create($conn, $config); + + return $em; + } +} + +LockAgentWorker::run(); \ No newline at end of file diff --git a/tests/README.markdown b/tests/README.markdown new file mode 100644 index 000000000..c1027aced --- /dev/null +++ b/tests/README.markdown @@ -0,0 +1,25 @@ +# Running the Doctrine 2 Testsuite + +## Setting up a PHPUnit Configuration XML + +.. + +## Testing Lock-Support + +The Lock support in Doctrine 2 is tested using Gearman, which allows to run concurrent tasks in parallel. +Install Gearman with PHP as follows: + +1. Go to http://www.gearman.org and download the latest Gearman Server +2. Compile it and then call ldconfig +3. Start it up "gearmand -vvvv" +4. Install pecl/gearman by calling "gearman-beta" + +You can then go into tests/ and start up two workers: + + php Doctrine/Tests/ORM/Functional/Locking/LockAgentWorker.php + +Then run the locking test-suite: + + phpunit --configuration Doctrine/Tests/ORM/Functional/Locking/GearmanLockTest.php + +This can run considerable time, because it is using sleep() to test for the timing ranges of locks. \ No newline at end of file From c3303881a957a45433a30a9efa457f0e3e92d918 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sun, 2 May 2010 13:18:47 +0200 Subject: [PATCH 4/5] Fixed IBM DB2 Platform to support locking --- lib/Doctrine/DBAL/Platforms/DB2Platform.php | 5 +++++ .../Tests/ORM/Functional/Locking/LockAgentWorker.php | 1 + 2 files changed, 6 insertions(+) diff --git a/lib/Doctrine/DBAL/Platforms/DB2Platform.php b/lib/Doctrine/DBAL/Platforms/DB2Platform.php index 9cc04840d..6b5072f2a 100644 --- a/lib/Doctrine/DBAL/Platforms/DB2Platform.php +++ b/lib/Doctrine/DBAL/Platforms/DB2Platform.php @@ -513,4 +513,9 @@ class DB2Platform extends AbstractPlatform { return strtoupper($column); } + + public function getForUpdateSQL() + { + return ' WITH RR USE AND KEEP UPDATE LOCKS'; + } } \ No newline at end of file diff --git a/tests/Doctrine/Tests/ORM/Functional/Locking/LockAgentWorker.php b/tests/Doctrine/Tests/ORM/Functional/Locking/LockAgentWorker.php index 6b1e6d488..146f2db4d 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Locking/LockAgentWorker.php +++ b/tests/Doctrine/Tests/ORM/Functional/Locking/LockAgentWorker.php @@ -101,6 +101,7 @@ class LockAgentWorker $cache = new \Doctrine\Common\Cache\ArrayCache(); $config->setMetadataCacheImpl($cache); $config->setQueryCacheImpl($cache); + $config->setSQLLogger(new \Doctrine\DBAL\Logging\EchoSQLLogger()); $em = \Doctrine\ORM\EntityManager::create($conn, $config); From 8a67621b6ae2d0388e506fc8406e15806ee19831 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sat, 15 May 2010 09:58:39 +0200 Subject: [PATCH 5/5] DDC-178 - Fixed problems occuring from merging upstream, re-ran API and tests, finalizing lock-support for merge with upstream --- lib/Doctrine/ORM/OptimisticLockException.php | 9 +++++---- lib/Doctrine/ORM/UnitOfWork.php | 2 +- .../Tests/ORM/Functional/Locking/GearmanLockTest.php | 8 ++++---- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/lib/Doctrine/ORM/OptimisticLockException.php b/lib/Doctrine/ORM/OptimisticLockException.php index eebb0150a..028698cd8 100644 --- a/lib/Doctrine/ORM/OptimisticLockException.php +++ b/lib/Doctrine/ORM/OptimisticLockException.php @@ -24,6 +24,7 @@ namespace Doctrine\ORM; * that uses optimistic locking through a version field fails. * * @author Roman Borschel + * @author Benjamin Eberlei * @since 2.0 */ class OptimisticLockException extends ORMException @@ -50,13 +51,13 @@ class OptimisticLockException extends ORMException return new self("The optimistic lock on an entity failed.", $entity); } - public static function lockFailedVersionMissmatch($expectedLockVersion, $actualLockVersion) + public static function lockFailedVersionMissmatch($entity, $expectedLockVersion, $actualLockVersion) { - return new self("The optimistic lock failed, version " . $expectedLockVersion . " was expected, but is actually ".$actualLockVersion); + return new self("The optimistic lock failed, version " . $expectedLockVersion . " was expected, but is actually ".$actualLockVersion, $entity); } - public static function notVersioned($className) + public static function notVersioned($entityName) { - return new self("Cannot obtain optimistic lock on unversioned entity ".$className); + return new self("Cannot obtain optimistic lock on unversioned entity " . $entityName, null); } } \ No newline at end of file diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index ef71d278c..2428098ef 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -1654,7 +1654,7 @@ class UnitOfWork implements PropertyChangedListener if ($lockVersion != null) { $entityVersion = $class->reflFields[$class->versionField]->getValue($entity); if ($entityVersion != $lockVersion) { - throw OptimisticLockException::lockFailedVersionMissmatch($lockVersion, $entityVersion); + throw OptimisticLockException::lockFailedVersionMissmatch($entity, $lockVersion, $entityVersion); } } } else if ($lockMode == LockMode::PESSIMISTIC_READ || $lockMode == LockMode::PESSIMISTIC_WRITE) { diff --git a/tests/Doctrine/Tests/ORM/Functional/Locking/GearmanLockTest.php b/tests/Doctrine/Tests/ORM/Functional/Locking/GearmanLockTest.php index ced552422..0efca5c12 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Locking/GearmanLockTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/Locking/GearmanLockTest.php @@ -140,7 +140,7 @@ class GearmanLockTest extends \Doctrine\Tests\OrmFunctionalTestCase protected function asyncFindWithLock($entityName, $entityId, $lockMode) { - $this->startGearmanJob('findWithLock', array( + $this->startJob('findWithLock', array( 'entityName' => $entityName, 'entityId' => $entityId, 'lockMode' => $lockMode, @@ -149,7 +149,7 @@ class GearmanLockTest extends \Doctrine\Tests\OrmFunctionalTestCase protected function asyncDqlWithLock($dql, $params, $lockMode) { - $this->startGearmanJob('dqlWithLock', array( + $this->startJob('dqlWithLock', array( 'dql' => $dql, 'dqlParams' => $params, 'lockMode' => $lockMode, @@ -158,14 +158,14 @@ class GearmanLockTest extends \Doctrine\Tests\OrmFunctionalTestCase protected function asyncLock($entityName, $entityId, $lockMode) { - $this->startGearmanJob('lock', array( + $this->startJob('lock', array( 'entityName' => $entityName, 'entityId' => $entityId, 'lockMode' => $lockMode, )); } - protected function startGearmanJob($fn, $fixture) + protected function startJob($fn, $fixture) { $this->gearman->addTask($fn, serialize(array( 'conn' => $this->_em->getConnection()->getParams(),