From e6a44b145fbe0ee140fcd1eb9dcf169ab6c9da20 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Thu, 8 Apr 2010 22:50:06 +0200 Subject: [PATCH 01/17] [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 02/17] 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 03/17] 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 04/17] 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 668456e4d80456e392f42e27c745a80172a9e382 Mon Sep 17 00:00:00 2001 From: David Abdemoulaie Date: Fri, 14 May 2010 14:21:46 -0500 Subject: [PATCH 05/17] [DDC-588] EntityManager::refresh uses fieldName instead of columnName --- lib/Doctrine/ORM/UnitOfWork.php | 2 +- .../ORM/Functional/Ticket/DDC588Test.php | 48 +++++++++++++++++++ 2 files changed, 49 insertions(+), 1 deletion(-) create mode 100644 tests/Doctrine/Tests/ORM/Functional/Ticket/DDC588Test.php diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index 6bf7d6691..948ee1be2 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -1480,7 +1480,7 @@ class UnitOfWork implements PropertyChangedListener $class = $this->_em->getClassMetadata(get_class($entity)); if ($this->getEntityState($entity) == self::STATE_MANAGED) { $this->getEntityPersister($class->name)->refresh( - array_combine($class->getIdentifierColumnNames(), $this->_entityIdentifiers[$oid]), + array_combine($class->getIdentifierFieldNames(), $this->_entityIdentifiers[$oid]), $entity ); } else { diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC588Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC588Test.php new file mode 100644 index 000000000..3f899d786 --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC588Test.php @@ -0,0 +1,48 @@ +_schemaTool->createSchema(array( + $this->_em->getClassMetadata(__NAMESPACE__ . '\DDC588Site'), + )); + } + + public function testIssue() + { + $site = new DDC588Site('Foo'); + + $this->_em->persist($site); + $this->_em->flush(); + // Following should not result in exception + $this->_em->refresh($site); + } +} + +/** + * @Entity + */ +class DDC588Site +{ + /** + * @Id + * @Column(type="integer", name="site_id") + * @GeneratedValue + */ + public $id; + + /** + * @Column(type="string", length=45) + */ + protected $name = null; + + public function __construct($name = '') + { + $this->name = $name; + } +} From 8a67621b6ae2d0388e506fc8406e15806ee19831 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sat, 15 May 2010 09:58:39 +0200 Subject: [PATCH 06/17] 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(), From 78328ec6eac2574ee1930bbf3b98ad759f6c1741 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sat, 15 May 2010 11:48:20 +0200 Subject: [PATCH 07/17] DDC-178 - Removed Doctrine\ORM\LockMode in favour of Doctrine\DBAL\LockMode --- lib/Doctrine/ORM/EntityManager.php | 1 + lib/Doctrine/ORM/EntityRepository.php | 2 ++ lib/Doctrine/ORM/Persisters/BasicEntityPersister.php | 8 ++++---- lib/Doctrine/ORM/Query.php | 5 +++-- lib/Doctrine/ORM/Query/SqlWalker.php | 6 +++--- lib/Doctrine/ORM/UnitOfWork.php | 4 ++-- .../Tests/ORM/Functional/EntityRepositoryTest.php | 8 ++++---- .../Tests/ORM/Functional/Locking/GearmanLockTest.php | 2 +- tests/Doctrine/Tests/ORM/Functional/Locking/LockTest.php | 2 +- .../Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php | 8 ++++---- 10 files changed, 25 insertions(+), 21 deletions(-) diff --git a/lib/Doctrine/ORM/EntityManager.php b/lib/Doctrine/ORM/EntityManager.php index a423bcdd1..0ff16f640 100644 --- a/lib/Doctrine/ORM/EntityManager.php +++ b/lib/Doctrine/ORM/EntityManager.php @@ -22,6 +22,7 @@ namespace Doctrine\ORM; use Closure, Exception, Doctrine\Common\EventManager, Doctrine\DBAL\Connection, + Doctrine\DBAL\LockMode, Doctrine\ORM\Mapping\ClassMetadata, Doctrine\ORM\Mapping\ClassMetadataFactory, Doctrine\ORM\Proxy\ProxyFactory; diff --git a/lib/Doctrine/ORM/EntityRepository.php b/lib/Doctrine/ORM/EntityRepository.php index d979792f4..2e4a2191c 100644 --- a/lib/Doctrine/ORM/EntityRepository.php +++ b/lib/Doctrine/ORM/EntityRepository.php @@ -19,6 +19,8 @@ namespace Doctrine\ORM; +use Doctrine\DBAL\LockMode; + /** * An EntityRepository serves as a repository for entities with generic as well as * business specific methods for retrieving entities. diff --git a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php index 0e21f6da5..22388f4fb 100644 --- a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php +++ b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php @@ -790,9 +790,9 @@ class BasicEntityPersister : ''; $lockSql = ''; - if ($lockMode == \Doctrine\ORM\LockMode::PESSIMISTIC_READ) { + if ($lockMode == \Doctrine\DBAL\LockMode::PESSIMISTIC_READ) { $lockSql = ' ' . $this->_platform->getReadLockSql(); - } else if ($lockMode == \Doctrine\ORM\LockMode::PESSIMISTIC_WRITE) { + } else if ($lockMode == \Doctrine\DBAL\LockMode::PESSIMISTIC_WRITE) { $lockSql = ' ' . $this->_platform->getWriteLockSql(); } @@ -1028,9 +1028,9 @@ class BasicEntityPersister { $conditionSql = $this->_getSelectConditionSQL($criteria); - if ($lockMode == \Doctrine\ORM\LockMode::PESSIMISTIC_READ) { + if ($lockMode == \Doctrine\DBAL\LockMode::PESSIMISTIC_READ) { $lockSql = $this->_platform->getReadLockSql(); - } else if ($lockMode == \Doctrine\ORM\LockMode::PESSIMISTIC_WRITE) { + } else if ($lockMode == \Doctrine\DBAL\LockMode::PESSIMISTIC_WRITE) { $lockSql = $this->_platform->getWriteLockSql(); } diff --git a/lib/Doctrine/ORM/Query.php b/lib/Doctrine/ORM/Query.php index 1619aebd2..ac0d5bdd6 100644 --- a/lib/Doctrine/ORM/Query.php +++ b/lib/Doctrine/ORM/Query.php @@ -19,7 +19,8 @@ namespace Doctrine\ORM; -use Doctrine\ORM\Query\Parser, +use Doctrine\DBAL\LockMode, + Doctrine\ORM\Query\Parser, Doctrine\ORM\Query\QueryException; /** @@ -495,7 +496,7 @@ final class Query extends AbstractQuery /** * Set the lock mode for this Query. * - * @see Doctrine\ORM\LockMode + * @see Doctrine\DBAL\LockMode * @param int $lockMode * @return Query */ diff --git a/lib/Doctrine/ORM/Query/SqlWalker.php b/lib/Doctrine/ORM/Query/SqlWalker.php index ca445d81f..c0d68829b 100644 --- a/lib/Doctrine/ORM/Query/SqlWalker.php +++ b/lib/Doctrine/ORM/Query/SqlWalker.php @@ -367,11 +367,11 @@ class SqlWalker implements TreeWalker ); if (($lockMode = $this->_query->getHint(Query::HINT_LOCK_MODE)) !== false) { - if ($lockMode == \Doctrine\ORM\LockMode::PESSIMISTIC_READ) { + if ($lockMode == \Doctrine\DBAL\LockMode::PESSIMISTIC_READ) { $sql .= " " . $this->_platform->getReadLockSQL(); - } else if ($lockMode == \Doctrine\ORM\LockMode::PESSIMISTIC_WRITE) { + } else if ($lockMode == \Doctrine\DBAL\LockMode::PESSIMISTIC_WRITE) { $sql .= " " . $this->_platform->getWriteLockSQL(); - } else if ($lockMode == \Doctrine\ORM\LockMode::OPTIMISTIC) { + } else if ($lockMode == \Doctrine\DBAL\LockMode::OPTIMISTIC) { $versionedClassFound = false; foreach ($this->_selectedClasses AS $class) { if ($class->isVersioned) { diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index 2428098ef..f02f1e71b 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -1646,7 +1646,7 @@ class UnitOfWork implements PropertyChangedListener $entityName = get_class($entity); $class = $this->_em->getClassMetadata($entityName); - if ($lockMode == LockMode::OPTIMISTIC) { + if ($lockMode == \Doctrine\DBAL\LockMode::OPTIMISTIC) { if (!$class->isVersioned) { throw OptimisticLockException::notVersioned($entityName); } @@ -1657,7 +1657,7 @@ class UnitOfWork implements PropertyChangedListener throw OptimisticLockException::lockFailedVersionMissmatch($entity, $lockVersion, $entityVersion); } } - } else if ($lockMode == LockMode::PESSIMISTIC_READ || $lockMode == LockMode::PESSIMISTIC_WRITE) { + } else if (in_array($lockMode, array(\Doctrine\DBAL\LockMode::PESSIMISTIC_READ, \Doctrine\DBAL\LockMode::PESSIMISTIC_WRITE))) { if (!$this->_em->getConnection()->isTransactionActive()) { throw TransactionRequiredException::transactionRequired(); diff --git a/tests/Doctrine/Tests/ORM/Functional/EntityRepositoryTest.php b/tests/Doctrine/Tests/ORM/Functional/EntityRepositoryTest.php index 9bb2253ee..15ad790f9 100644 --- a/tests/Doctrine/Tests/ORM/Functional/EntityRepositoryTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/EntityRepositoryTest.php @@ -103,7 +103,7 @@ class EntityRepositoryTest extends \Doctrine\Tests\OrmFunctionalTestCase $this->setExpectedException('Doctrine\ORM\TransactionRequiredException'); $this->_em->getRepository('Doctrine\Tests\Models\CMS\CmsUser') - ->find(1, \Doctrine\ORM\LockMode::PESSIMISTIC_READ); + ->find(1, \Doctrine\DBAL\LockMode::PESSIMISTIC_READ); } /** @@ -115,7 +115,7 @@ class EntityRepositoryTest extends \Doctrine\Tests\OrmFunctionalTestCase $this->setExpectedException('Doctrine\ORM\TransactionRequiredException'); $this->_em->getRepository('Doctrine\Tests\Models\CMS\CmsUser') - ->find(1, \Doctrine\ORM\LockMode::PESSIMISTIC_WRITE); + ->find(1, \Doctrine\DBAL\LockMode::PESSIMISTIC_WRITE); } /** @@ -127,7 +127,7 @@ class EntityRepositoryTest extends \Doctrine\Tests\OrmFunctionalTestCase $this->setExpectedException('Doctrine\ORM\OptimisticLockException'); $this->_em->getRepository('Doctrine\Tests\Models\CMS\CmsUser') - ->find(1, \Doctrine\ORM\LockMode::OPTIMISTIC); + ->find(1, \Doctrine\DBAL\LockMode::OPTIMISTIC); } /** @@ -148,7 +148,7 @@ class EntityRepositoryTest extends \Doctrine\Tests\OrmFunctionalTestCase $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); + $this->_em->find('Doctrine\Tests\Models\Cms\CmsUser', $userId, \Doctrine\DBAL\LockMode::OPTIMISTIC); } } diff --git a/tests/Doctrine/Tests/ORM/Functional/Locking/GearmanLockTest.php b/tests/Doctrine/Tests/ORM/Functional/Locking/GearmanLockTest.php index 0efca5c12..c1d4037d3 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Locking/GearmanLockTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/Locking/GearmanLockTest.php @@ -4,7 +4,7 @@ namespace Doctrine\Tests\ORM\Functional\Locking; use Doctrine\Tests\Models\CMS\CmsArticle, Doctrine\Tests\Models\CMS\CmsUser, - Doctrine\ORM\LockMode, + Doctrine\DBAL\LockMode, Doctrine\ORM\EntityManager; require_once __DIR__ . '/../../../TestInit.php'; diff --git a/tests/Doctrine/Tests/ORM/Functional/Locking/LockTest.php b/tests/Doctrine/Tests/ORM/Functional/Locking/LockTest.php index 5d0834557..c5316f769 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Locking/LockTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/Locking/LockTest.php @@ -4,7 +4,7 @@ namespace Doctrine\Tests\ORM\Functional\Locking; use Doctrine\Tests\Models\CMS\CmsArticle, Doctrine\Tests\Models\CMS\CmsUser, - Doctrine\ORM\LockMode, + Doctrine\DBAL\LockMode, Doctrine\ORM\EntityManager; require_once __DIR__ . '/../../../TestInit.php'; diff --git a/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php b/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php index 0b8fc8847..426b054e2 100644 --- a/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php +++ b/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php @@ -603,7 +603,7 @@ class SelectSqlGenerationTest extends \Doctrine\Tests\OrmTestCase "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) + array(Query::HINT_LOCK_MODE => \Doctrine\DBAL\LockMode::PESSIMISTIC_WRITE) ); } @@ -619,7 +619,7 @@ class SelectSqlGenerationTest extends \Doctrine\Tests\OrmTestCase "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) + array(Query::HINT_LOCK_MODE => \Doctrine\DBAL\LockMode::PESSIMISTIC_READ) ); } @@ -646,7 +646,7 @@ class SelectSqlGenerationTest extends \Doctrine\Tests\OrmTestCase "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) + array(Query::HINT_LOCK_MODE => \Doctrine\DBAL\LockMode::PESSIMISTIC_READ) ); } @@ -662,7 +662,7 @@ class SelectSqlGenerationTest extends \Doctrine\Tests\OrmTestCase "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) + array(Query::HINT_LOCK_MODE => \Doctrine\DBAL\LockMode::PESSIMISTIC_READ) ); } From ad402c6ded46f0f31766fc651bf5e02b429f7405 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sat, 15 May 2010 11:53:28 +0200 Subject: [PATCH 08/17] DDC-178 DQL Optimistic Lock now requires ALL classes to be versioned, otherwise throws exception --- lib/Doctrine/ORM/Query/SqlWalker.php | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/lib/Doctrine/ORM/Query/SqlWalker.php b/lib/Doctrine/ORM/Query/SqlWalker.php index c0d68829b..8d7dd6259 100644 --- a/lib/Doctrine/ORM/Query/SqlWalker.php +++ b/lib/Doctrine/ORM/Query/SqlWalker.php @@ -372,16 +372,11 @@ class SqlWalker implements TreeWalker } else if ($lockMode == \Doctrine\DBAL\LockMode::PESSIMISTIC_WRITE) { $sql .= " " . $this->_platform->getWriteLockSQL(); } else if ($lockMode == \Doctrine\DBAL\LockMode::OPTIMISTIC) { - $versionedClassFound = false; foreach ($this->_selectedClasses AS $class) { - if ($class->isVersioned) { - $versionedClassFound = true; + if (!$class->isVersioned) { + throw \Doctrine\ORM\OptimisticLockException::lockFailed(); } } - - if (!$versionedClassFound) { - throw \Doctrine\ORM\OptimisticLockException::lockFailed(); - } } } From dbb5795c53a51627b5e52be96a9d9d52509c241e Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sat, 15 May 2010 12:04:46 +0200 Subject: [PATCH 09/17] DDC-178 Removed Doctrine\ORM\LockMode --- lib/Doctrine/ORM/LockMode.php | 37 ----------------------------------- 1 file changed, 37 deletions(-) delete mode 100644 lib/Doctrine/ORM/LockMode.php diff --git a/lib/Doctrine/ORM/LockMode.php b/lib/Doctrine/ORM/LockMode.php deleted file mode 100644 index 45f999a30..000000000 --- a/lib/Doctrine/ORM/LockMode.php +++ /dev/null @@ -1,37 +0,0 @@ -. -*/ - -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 From 46684ea5c986bd79f89600127e3c5d89db99ea5e Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sat, 15 May 2010 19:51:48 +0200 Subject: [PATCH 10/17] Added a bunch of functional tests for the public read, write and transactional public DBAL API, passing all the current drivers Added a Write Test --- lib/Doctrine/DBAL/Connection.php | 4 +- tests/Doctrine/Tests/DBAL/ConnectionTest.php | 46 +++++++ .../Tests/DBAL/Functional/AllTests.php | 1 + .../Tests/DBAL/Functional/ConnectionTest.php | 39 +++++- .../Tests/DBAL/Functional/DataAccessTest.php | 107 +++++++++++++++ .../Tests/DBAL/Functional/WriteTest.php | 122 ++++++++++++++++++ .../Doctrine/Tests/DbalFunctionalTestCase.php | 6 + 7 files changed, 319 insertions(+), 6 deletions(-) create mode 100644 tests/Doctrine/Tests/DBAL/Functional/WriteTest.php diff --git a/lib/Doctrine/DBAL/Connection.php b/lib/Doctrine/DBAL/Connection.php index a2b277ebd..3a2c1b9f9 100644 --- a/lib/Doctrine/DBAL/Connection.php +++ b/lib/Doctrine/DBAL/Connection.php @@ -753,7 +753,7 @@ class Connection implements DriverConnection public function commit() { if ($this->_transactionNestingLevel == 0) { - throw ConnectionException::commitFailedNoActiveTransaction(); + throw ConnectionException::noActiveTransaction(); } if ($this->_isRollbackOnly) { throw ConnectionException::commitFailedRollbackOnly(); @@ -779,7 +779,7 @@ class Connection implements DriverConnection public function rollback() { if ($this->_transactionNestingLevel == 0) { - throw ConnectionException::rollbackFailedNoActiveTransaction(); + throw ConnectionException::noActiveTransaction(); } $this->connect(); diff --git a/tests/Doctrine/Tests/DBAL/ConnectionTest.php b/tests/Doctrine/Tests/DBAL/ConnectionTest.php index 6390bf374..88eb23286 100644 --- a/tests/Doctrine/Tests/DBAL/ConnectionTest.php +++ b/tests/Doctrine/Tests/DBAL/ConnectionTest.php @@ -11,6 +11,11 @@ use Doctrine\DBAL\Events; class ConnectionTest extends \Doctrine\Tests\DbalTestCase { + /** + * @var Doctrine\DBAL\Connection + */ + protected $_conn = null; + public function setUp() { $params = array( @@ -23,6 +28,47 @@ class ConnectionTest extends \Doctrine\Tests\DbalTestCase $this->_conn = \Doctrine\DBAL\DriverManager::getConnection($params); } + public function testIsConnected() + { + $this->assertFalse($this->_conn->isConnected()); + } + + public function testNoTransactionActiveByDefault() + { + $this->assertFalse($this->_conn->isTransactionActive()); + } + + public function testCommitWithNoActiveTransaction_ThrowsException() + { + $this->setExpectedException('Doctrine\DBAL\ConnectionException'); + $this->_conn->commit(); + } + + public function testRollbackWithNoActiveTransaction_ThrowsException() + { + $this->setExpectedException('Doctrine\DBAL\ConnectionException'); + $this->_conn->rollback(); + } + + public function testSetRollbackOnlyNoActiveTransaction_ThrowsException() + { + $this->setExpectedException('Doctrine\DBAL\ConnectionException'); + $this->_conn->setRollbackOnly(); + } + + public function testIsRollbackOnlyNoActiveTransaction_ThrowsException() + { + $this->setExpectedException('Doctrine\DBAL\ConnectionException'); + $this->_conn->isRollbackOnly(); + } + + public function testGetConfiguration() + { + $config = $this->_conn->getConfiguration(); + + $this->assertType('Doctrine\DBAL\Configuration', $config); + } + public function testGetHost() { $this->assertEquals('localhost', $this->_conn->getHost()); diff --git a/tests/Doctrine/Tests/DBAL/Functional/AllTests.php b/tests/Doctrine/Tests/DBAL/Functional/AllTests.php index 88574060c..573177070 100644 --- a/tests/Doctrine/Tests/DBAL/Functional/AllTests.php +++ b/tests/Doctrine/Tests/DBAL/Functional/AllTests.php @@ -28,6 +28,7 @@ class AllTests $suite->addTestSuite('Doctrine\Tests\DBAL\Functional\Schema\Db2SchemaManagerTest'); $suite->addTestSuite('Doctrine\Tests\DBAL\Functional\ConnectionTest'); $suite->addTestSuite('Doctrine\Tests\DBAL\Functional\DataAccessTest'); + $suite->addTestSuite('Doctrine\Tests\DBAL\Functional\WriteTest'); return $suite; } diff --git a/tests/Doctrine/Tests/DBAL/Functional/ConnectionTest.php b/tests/Doctrine/Tests/DBAL/Functional/ConnectionTest.php index 26663fcd0..5cb6d4167 100644 --- a/tests/Doctrine/Tests/DBAL/Functional/ConnectionTest.php +++ b/tests/Doctrine/Tests/DBAL/Functional/ConnectionTest.php @@ -8,7 +8,25 @@ require_once __DIR__ . '/../../TestInit.php'; class ConnectionTest extends \Doctrine\Tests\DbalFunctionalTestCase { - + public function setUp() + { + $this->resetSharedConn(); + parent::setUp(); + } + + public function testGetWrappedConnection() + { + $this->assertType('Doctrine\DBAL\Driver\Connection', $this->_conn->getWrappedConnection()); + } + + public function testCommitWithRollbackOnlyThrowsException() + { + $this->_conn->beginTransaction(); + $this->_conn->setRollbackOnly(); + $this->setExpectedException('Doctrine\DBAL\ConnectionException'); + $this->_conn->commit(); + } + public function testTransactionNestingBehavior() { try { @@ -36,7 +54,7 @@ class ConnectionTest extends \Doctrine\Tests\DbalFunctionalTestCase } } - public function testTransactionBehavior() + public function testTransactionBehaviorWithRollback() { try { $this->_conn->beginTransaction(); @@ -50,7 +68,10 @@ class ConnectionTest extends \Doctrine\Tests\DbalFunctionalTestCase $this->_conn->rollback(); $this->assertEquals(0, $this->_conn->getTransactionNestingLevel()); } - + } + + public function testTransactionBehaviour() + { try { $this->_conn->beginTransaction(); $this->assertEquals(1, $this->_conn->getTransactionNestingLevel()); @@ -61,6 +82,10 @@ class ConnectionTest extends \Doctrine\Tests\DbalFunctionalTestCase } $this->assertEquals(0, $this->_conn->getTransactionNestingLevel()); + } + + public function testTransactionalWithException() + { try { $this->_conn->transactional(function($conn) { $conn->executeQuery("select 1"); @@ -70,5 +95,11 @@ class ConnectionTest extends \Doctrine\Tests\DbalFunctionalTestCase $this->assertEquals(0, $this->_conn->getTransactionNestingLevel()); } } - + + public function testTransactional() + { + $this->_conn->transactional(function($conn) { + $conn->executeQuery("select 1"); + }); + } } \ No newline at end of file diff --git a/tests/Doctrine/Tests/DBAL/Functional/DataAccessTest.php b/tests/Doctrine/Tests/DBAL/Functional/DataAccessTest.php index 7d5123d66..0e9693a14 100644 --- a/tests/Doctrine/Tests/DBAL/Functional/DataAccessTest.php +++ b/tests/Doctrine/Tests/DBAL/Functional/DataAccessTest.php @@ -25,6 +25,101 @@ class DataAccessTest extends \Doctrine\Tests\DbalFunctionalTestCase } } + public function testPrepareWithBindValue() + { + $sql = "SELECT test_int, test_string FROM fetch_table WHERE test_int = ? AND test_string = ?"; + $stmt = $this->_conn->prepare($sql); + $this->assertType('Doctrine\DBAL\Statement', $stmt); + + $stmt->bindValue(1, 1); + $stmt->bindValue(2, 'foo'); + $stmt->execute(); + + $row = $stmt->fetch(\PDO::FETCH_ASSOC); + $row = array_change_key_case($row, \CASE_LOWER); + $this->assertEquals(array('test_int' => 1, 'test_string' => 'foo'), $row); + } + + public function testPrepareWithBindParam() + { + $paramInt = 1; + $paramStr = 'foo'; + + $sql = "SELECT test_int, test_string FROM fetch_table WHERE test_int = ? AND test_string = ?"; + $stmt = $this->_conn->prepare($sql); + $this->assertType('Doctrine\DBAL\Statement', $stmt); + + $stmt->bindParam(1, $paramInt); + $stmt->bindParam(2, $paramStr); + $stmt->execute(); + + $row = $stmt->fetch(\PDO::FETCH_ASSOC); + $row = array_change_key_case($row, \CASE_LOWER); + $this->assertEquals(array('test_int' => 1, 'test_string' => 'foo'), $row); + } + + public function testPrepareWithFetchAll() + { + $paramInt = 1; + $paramStr = 'foo'; + + $sql = "SELECT test_int, test_string FROM fetch_table WHERE test_int = ? AND test_string = ?"; + $stmt = $this->_conn->prepare($sql); + $this->assertType('Doctrine\DBAL\Statement', $stmt); + + $stmt->bindParam(1, $paramInt); + $stmt->bindParam(2, $paramStr); + $stmt->execute(); + + $rows = $stmt->fetchAll(\PDO::FETCH_ASSOC); + $rows[0] = array_change_key_case($rows[0], \CASE_LOWER); + $this->assertEquals(array('test_int' => 1, 'test_string' => 'foo'), $rows[0]); + } + + public function testPrepareWithFetchColumn() + { + $paramInt = 1; + $paramStr = 'foo'; + + $sql = "SELECT test_int FROM fetch_table WHERE test_int = ? AND test_string = ?"; + $stmt = $this->_conn->prepare($sql); + $this->assertType('Doctrine\DBAL\Statement', $stmt); + + $stmt->bindParam(1, $paramInt); + $stmt->bindParam(2, $paramStr); + $stmt->execute(); + + $column = $stmt->fetchColumn(); + $this->assertEquals(1, $column); + } + + public function testPrepareWithQuoted() + { + $table = 'fetch_table'; + $paramInt = 1; + $paramStr = 'foo'; + + $sql = "SELECT test_int, test_string FROM " . $this->_conn->quoteIdentifier($table) . " ". + "WHERE test_int = " . $this->_conn->quote($paramInt) . " AND test_string = " . $this->_conn->quote($paramStr); + $stmt = $this->_conn->prepare($sql); + $this->assertType('Doctrine\DBAL\Statement', $stmt); + } + + public function testPrepareWithExecuteParams() + { + $paramInt = 1; + $paramStr = 'foo'; + + $sql = "SELECT test_int, test_string FROM fetch_table WHERE test_int = ? AND test_string = ?"; + $stmt = $this->_conn->prepare($sql); + $this->assertType('Doctrine\DBAL\Statement', $stmt); + $stmt->execute(array($paramInt, $paramStr)); + + $row = $stmt->fetch(\PDO::FETCH_ASSOC); + $row = array_change_key_case($row, \CASE_LOWER); + $this->assertEquals(array('test_int' => 1, 'test_string' => 'foo'), $row); + } + public function testFetchAll() { $sql = "SELECT test_int, test_string FROM fetch_table WHERE test_int = ? AND test_string = ?"; @@ -60,4 +155,16 @@ class DataAccessTest extends \Doctrine\Tests\DbalFunctionalTestCase $this->assertEquals('foo', $row[1]); } + public function testFetchColumn() + { + $sql = "SELECT test_int, test_string FROM fetch_table WHERE test_int = ? AND test_string = ?"; + $testInt = $this->_conn->fetchColumn($sql, array(1, 'foo'), 0); + + $this->assertEquals(1, $testInt); + + $sql = "SELECT test_int, test_string FROM fetch_table WHERE test_int = ? AND test_string = ?"; + $testString = $this->_conn->fetchColumn($sql, array(1, 'foo'), 1); + + $this->assertEquals('foo', $testString); + } } \ No newline at end of file diff --git a/tests/Doctrine/Tests/DBAL/Functional/WriteTest.php b/tests/Doctrine/Tests/DBAL/Functional/WriteTest.php new file mode 100644 index 000000000..3b2538fee --- /dev/null +++ b/tests/Doctrine/Tests/DBAL/Functional/WriteTest.php @@ -0,0 +1,122 @@ +addColumn('test_int', 'integer'); + $table->addColumn('test_string', 'string', array('notnull' => false)); + + $sm = $this->_conn->getSchemaManager(); + $sm->createTable($table); + } catch(\Exception $e) { + + } + $this->_conn->executeUpdate('DELETE FROM write_table'); + } + + public function testExecuteUpdate() + { + $sql = "INSERT INTO " . $this->_conn->quoteIdentifier('write_table') . " ( " . + $this->_conn->quoteIdentifier('test_int') . " ) VALUES ( " . $this->_conn->quote(1) . ")"; + $affected = $this->_conn->executeUpdate($sql); + + $this->assertEquals(1, $affected, "executeUpdate() should return the number of affected rows!"); + } + + public function testExecuteUpdateWithTypes() + { + $sql = "INSERT INTO write_table (test_int, test_string) VALUES (?, ?)"; + $affected = $this->_conn->executeUpdate($sql, array(1, 'foo'), array(\PDO::PARAM_INT, \PDO::PARAM_STR)); + + $this->assertEquals(1, $affected, "executeUpdate() should return the number of affected rows!"); + } + + public function testPrepareRowCountReturnsAffectedRows() + { + $sql = "INSERT INTO write_table (test_int, test_string) VALUES (?, ?)"; + $stmt = $this->_conn->prepare($sql); + + $stmt->bindValue(1, 1); + $stmt->bindValue(2, "foo"); + $stmt->execute(); + + $this->assertEquals(1, $stmt->rowCount()); + } + + public function testPrepareWithPdoTypes() + { + $sql = "INSERT INTO write_table (test_int, test_string) VALUES (?, ?)"; + $stmt = $this->_conn->prepare($sql); + + $stmt->bindValue(1, 1, \PDO::PARAM_INT); + $stmt->bindValue(2, "foo", \PDO::PARAM_STR); + $stmt->execute(); + + $this->assertEquals(1, $stmt->rowCount()); + } + + public function testPrepareWithDbalTypes() + { + $sql = "INSERT INTO write_table (test_int, test_string) VALUES (?, ?)"; + $stmt = $this->_conn->prepare($sql); + + $stmt->bindValue(1, 1, Type::getType('integer')); + $stmt->bindValue(2, "foo", Type::getType('string')); + $stmt->execute(); + + $this->assertEquals(1, $stmt->rowCount()); + } + + public function testPrepareWithDbalTypeNames() + { + $sql = "INSERT INTO write_table (test_int, test_string) VALUES (?, ?)"; + $stmt = $this->_conn->prepare($sql); + + $stmt->bindValue(1, 1, 'integer'); + $stmt->bindValue(2, "foo", 'string'); + $stmt->execute(); + + $this->assertEquals(1, $stmt->rowCount()); + } + + public function insertRows() + { + $this->assertEquals(1, $this->_conn->insert('write_table', array('test_int' => 1))); + $this->assertEquals(1, $this->_conn->insert('write_table', array('test_int' => 2))); + } + + public function testInsert() + { + $this->insertRows(); + } + + public function testDelete() + { + $this->insertRows(); + + $this->assertEquals(1, $this->_conn->delete('write_table', array('test_int' => 2))); + $this->assertEquals(1, count($this->_conn->fetchAll('SELECT * FROM write_table'))); + + $this->assertEquals(1, $this->_conn->delete('write_table', array('test_int' => 1))); + $this->assertEquals(0, count($this->_conn->fetchAll('SELECT * FROM write_table'))); + } + + public function testUpdate() + { + $this->insertRows(); + + $this->assertEquals(1, $this->_conn->update('write_table', array('test_int' => 2), array('test_int' => 1))); + $this->assertEquals(2, $this->_conn->update('write_table', array('test_int' => 3), array('test_int' => 2))); + } +} \ No newline at end of file diff --git a/tests/Doctrine/Tests/DbalFunctionalTestCase.php b/tests/Doctrine/Tests/DbalFunctionalTestCase.php index e5400211c..c4705a25e 100644 --- a/tests/Doctrine/Tests/DbalFunctionalTestCase.php +++ b/tests/Doctrine/Tests/DbalFunctionalTestCase.php @@ -12,6 +12,12 @@ class DbalFunctionalTestCase extends DbalTestCase */ protected $_conn; + protected function resetSharedConn() + { + $this->sharedFixture['conn'] = null; + self::$_sharedConn = null; + } + protected function setUp() { if (isset($this->sharedFixture['conn'])) { From 74df4c39dca87031b6e8505ba670ceabe8ae2ea5 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sat, 15 May 2010 20:54:22 +0200 Subject: [PATCH 11/17] DDC-592 - Fix Validator notices when mappedBy or inversedBy properties dont exist --- lib/Doctrine/ORM/Tools/SchemaValidator.php | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/lib/Doctrine/ORM/Tools/SchemaValidator.php b/lib/Doctrine/ORM/Tools/SchemaValidator.php index 0a13aa898..3df2e3497 100644 --- a/lib/Doctrine/ORM/Tools/SchemaValidator.php +++ b/lib/Doctrine/ORM/Tools/SchemaValidator.php @@ -93,9 +93,7 @@ class SchemaValidator if (!$targetMetadata->hasAssociation($assoc->mappedBy)) { $ce[] = "The association " . $class->name . "#" . $fieldName . " refers to the owning side ". "field " . $assoc->targetEntityName . "#" . $assoc->mappedBy . " which does not exist."; - } - - if ($targetMetadata->associationMappings[$assoc->mappedBy]->inversedBy == null) { + } else if ($targetMetadata->associationMappings[$assoc->mappedBy]->inversedBy == null) { $ce[] = "The field " . $class->name . "#" . $fieldName . " is on the inverse side of a ". "bi-directional relationship, but the specified mappedBy association on the target-entity ". $assoc->targetEntityName . "#" . $assoc->mappedBy . " does not contain the required ". @@ -115,11 +113,8 @@ class SchemaValidator if (!$targetMetadata->hasAssociation($assoc->inversedBy)) { $ce[] = "The association " . $class->name . "#" . $fieldName . " refers to the inverse side ". "field " . $assoc->targetEntityName . "#" . $assoc->inversedBy . " which does not exist."; - } - - if (isset($targetMetadata->associationMappings[$assoc->mappedBy]) && - $targetMetadata->associationMappings[$assoc->mappedBy]->mappedBy == null) { - $ce[] = "The field " . $class->name . "#" . $fieldName . " is on the inverse side of a ". + } else if ($targetMetadata->associationMappings[$assoc->inversedBy]->mappedBy == null) { + $ce[] = "The field " . $class->name . "#" . $fieldName . " is on the owning side of a ". "bi-directional relationship, but the specified mappedBy association on the target-entity ". $assoc->targetEntityName . "#" . $assoc->mappedBy . " does not contain the required ". "'inversedBy' attribute."; @@ -175,8 +170,6 @@ class SchemaValidator } } } - } else { - } if ($ce) { From ba6cb6bd141e92eb32f1cab43dff0e8b4eaf36b1 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sat, 15 May 2010 21:32:34 +0200 Subject: [PATCH 12/17] Fixed nasty bug with Proxies not getting initialized correctly. --- lib/Doctrine/ORM/UnitOfWork.php | 5 + .../ORM/Functional/Ticket/DDC440Test.php | 207 ++++++++++++++++++ 2 files changed, 212 insertions(+) create mode 100644 tests/Doctrine/Tests/ORM/Functional/Ticket/DDC440Test.php diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index f02f1e71b..828bd19ed 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -1775,6 +1775,10 @@ class UnitOfWork implements PropertyChangedListener if ($entity instanceof Proxy && ! $entity->__isInitialized__) { $entity->__isInitialized__ = true; $overrideLocalValues = true; + $this->_originalEntityData[$oid] = $data; + if ($entity instanceof NotifyPropertyChanged) { + $entity->addPropertyChangedListener($this); + } } else { $overrideLocalValues = isset($hints[Query::HINT_REFRESH]); } @@ -1844,6 +1848,7 @@ class UnitOfWork implements PropertyChangedListener $this->_entityIdentifiers[$newValueOid] = $associatedId; $this->_identityMap[$targetClass->rootEntityName][$relatedIdHash] = $newValue; $this->_entityStates[$newValueOid] = self::STATE_MANAGED; + // make sure that when an proxy is then finally loaded, $this->_originalEntityData is set also! } } $this->_originalEntityData[$oid][$field] = $newValue; diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC440Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC440Test.php new file mode 100644 index 000000000..e8c0bf085 --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC440Test.php @@ -0,0 +1,207 @@ +_schemaTool->createSchema(array( + $this->_em->getClassMetadata('Doctrine\Tests\ORM\Functional\Ticket\DDC440Phone'), + $this->_em->getClassMetadata('Doctrine\Tests\ORM\Functional\Ticket\DDC440Client') + )); + } catch (\Exception $e) { + // Swallow all exceptions. We do not test the schema tool here. + } + } + + /** + * @group DDC-440 + */ + public function testOriginalEntityDataEmptyWhenProxyLoadedFromTwoAssociations() + { + + + /* The key of the problem is that the first phone is fetched via two association, main_phone and phones. + * + * You will notice that the original_entity_datas are not loaded for the first phone. (They are for the second) + * + * In the Client entity definition, if you define the main_phone relation after the phones relation, both assertions pass. + * (for the sake or this test, I defined the main_phone relation before the phones relation) + * + */ + + //Initialize some data + $client = new DDC440Client; + $client->setName('Client1'); + + $phone = new DDC440Phone; + $phone->setNumber('418 111-1111'); + $phone->setClient($client); + + $phone2 = new DDC440Phone; + $phone2->setNumber('418 222-2222'); + $phone2->setClient($client); + + $client->setMainPhone($phone); + + $this->_em->persist($client); + $this->_em->flush(); + $id = $client->getId(); + $this->_em->clear(); + + $uw = $this->_em->getUnitOfWork(); + $client = $this->_em->find('Doctrine\Tests\ORM\Functional\Ticket\DDC440Client', $id); + $clientPhones = $client->getPhones(); + $p1 = $clientPhones[0]; + $p2 = $clientPhones[1]; + + // Test the first phone. The assertion actually failed because original entity data is not set properly. + // This was because it is also set as MainPhone and that one is created as a proxy, not the + // original object when the find on Client is called. However loading proxies did not work correctly. + $this->assertType('Doctrine\Tests\ORM\Functional\Ticket\DDC440Phone', $p1); + $originalData = $uw->getOriginalEntityData($p1); + $this->assertEquals($phone->getNumber(), $originalData['number']); + + + //If you comment out previous test, this one should pass + $this->assertType('Doctrine\Tests\ORM\Functional\Ticket\DDC440Phone', $p2); + $originalData = $uw->getOriginalEntityData($p2); + $this->assertEquals($phone2->getNumber(), $originalData['number']); + + } +} + +/** +* @Entity +* @Table(name="phone") +*/ +class DDC440Phone { + + /** + * @Column(name="id", type="integer") + * @Id + * @GeneratedValue(strategy="AUTO") + */ + protected $id; + + /** + * @ManyToOne(targetEntity="DDC440Client",inversedBy="phones") + * @JoinColumns({ + * @JoinColumn(name="client_id", referencedColumnName="id") + * }) + */ + protected $client; + + /** + * @Column(name="number", type="string") + */ + protected $number; + + public function setNumber($value){ + $this->number = $value; + } + + public function getNumber(){ + return $this->number; + } + + public function setClient(DDC440Client $value, $update_inverse=true) + { + $this->client = $value; + if($update_inverse){ + $value->addPhone($this); + } + } + + public function getClient() + { + return $this->client; + } + + public function getId() + { + return $this->id; + } + + public function setId($value){ + $this->id = $value; + } +} + +/** + * @Entity + * @Table(name="client") + */ +class DDC440Client { + + /** + * @Column(name="id", type="integer") + * @Id + * @GeneratedValue(strategy="AUTO") + */ + protected $id; + + /** + * @OneToOne(targetEntity="DDC440Phone", fetch="EAGER") + * @JoinColumns({ + * @JoinColumn(name="main_phone_id", referencedColumnName="id",onDelete="SET NULL") + * }) + */ + protected $main_phone; + + /** + * @OneToMany(targetEntity="DDC440Phone", mappedBy="client", cascade={"persist", "remove"}, fetch="EAGER") + */ + protected $phones; + + + /** + * @Column(name="name", type="string") + */ + protected $name; + + public function __construct(){ + + } + + public function setName($value){ + $this->name = $value; + } + + public function getName(){ + return $this->name; + } + + public function addPhone(DDC440Phone $value) + { + $this->phones[] = $value; + $value->setClient($this, false); + } + + public function getPhones() + { + return $this->phones; + } + + public function setMainPhone(DDC440Phone $value){ + $this->main_phone = $value; + } + + public function getMainPhone(){ + return $this->main_phone; + } + + public function getId() + { + return $this->id; + } + + public function setId($value){ + $this->id = $value; + } +} From 1067118174357606992685f52973ec868ad78245 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sat, 15 May 2010 21:52:59 +0200 Subject: [PATCH 13/17] DDC-568 - Fix bug with hints not being passed to hydrator by AbstractQuery::iterate() --- lib/Doctrine/ORM/AbstractQuery.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Doctrine/ORM/AbstractQuery.php b/lib/Doctrine/ORM/AbstractQuery.php index 9ad032bec..7c3c42f9c 100644 --- a/lib/Doctrine/ORM/AbstractQuery.php +++ b/lib/Doctrine/ORM/AbstractQuery.php @@ -463,7 +463,7 @@ abstract class AbstractQuery public function iterate(array $params = array(), $hydrationMode = self::HYDRATE_OBJECT) { return $this->_em->newHydrator($this->_hydrationMode)->iterate( - $this->_doExecute($params, $hydrationMode), $this->_resultSetMapping + $this->_doExecute($params, $hydrationMode), $this->_resultSetMapping, $this->_hints ); } From d098d62e1e7b070ba8557b861ea1011c6ea119de Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sat, 15 May 2010 23:07:00 +0200 Subject: [PATCH 14/17] DDC-527 - Fixed bug in OCI8 Driver --- lib/Doctrine/DBAL/Driver/OCI8/OCI8Statement.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/Doctrine/DBAL/Driver/OCI8/OCI8Statement.php b/lib/Doctrine/DBAL/Driver/OCI8/OCI8Statement.php index 4ebdbec1c..1f1b0567c 100644 --- a/lib/Doctrine/DBAL/Driver/OCI8/OCI8Statement.php +++ b/lib/Doctrine/DBAL/Driver/OCI8/OCI8Statement.php @@ -175,7 +175,8 @@ class OCI8Statement implements \Doctrine\DBAL\Driver\Statement } $result = array(); - oci_fetch_all($this->_sth, $result, 0, -1, self::$fetchStyleMap[$fetchStyle] | OCI_RETURN_NULLS | OCI_FETCHSTATEMENT_BY_ROW); + oci_fetch_all($this->_sth, $result, 0, -1, + self::$fetchStyleMap[$fetchStyle] | OCI_RETURN_NULLS | OCI_FETCHSTATEMENT_BY_ROW | OCI_RETURN_LOBS); return $result; } From 3045507a9bd235726da94ba414bd6600d6864994 Mon Sep 17 00:00:00 2001 From: "Roman S. Borschel" Date: Sun, 16 May 2010 13:30:40 +0200 Subject: [PATCH 15/17] [DDC-593] Fixed. --- lib/Doctrine/ORM/Query/SqlWalker.php | 2 +- .../Tests/ORM/Query/SelectSqlGenerationTest.php | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/lib/Doctrine/ORM/Query/SqlWalker.php b/lib/Doctrine/ORM/Query/SqlWalker.php index 52e73585e..3de86103a 100644 --- a/lib/Doctrine/ORM/Query/SqlWalker.php +++ b/lib/Doctrine/ORM/Query/SqlWalker.php @@ -1620,7 +1620,7 @@ class SqlWalker implements TreeWalker { return ($arithmeticExpr->isSimpleArithmeticExpression()) ? $this->walkSimpleArithmeticExpression($arithmeticExpr->simpleArithmeticExpression) - : $this->walkSubselect($arithmeticExpr->subselect); + : '(' . $this->walkSubselect($arithmeticExpr->subselect) . ')'; } /** diff --git a/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php b/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php index 716182ac1..5de5d0349 100644 --- a/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php +++ b/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php @@ -325,6 +325,17 @@ class SelectSqlGenerationTest extends \Doctrine\Tests\OrmTestCase ); } + /** + * @group DDC-593 + */ + public function testSubqueriesInComparisonExpression() + { + $this->assertSqlGeneration( + 'SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u WHERE (u.id >= (SELECT u2.id FROM Doctrine\Tests\Models\CMS\CmsUser u2 WHERE u2.name = :name)) AND (u.id <= (SELECT u3.id FROM Doctrine\Tests\Models\CMS\CmsUser u3 WHERE u3.name = :name))', + 'SELECT c0_.id AS id0, c0_.status AS status1, c0_.username AS username2, c0_.name AS name3 FROM cms_users c0_ WHERE (c0_.id >= (SELECT c1_.id FROM cms_users c1_ WHERE c1_.name = ?)) AND (c0_.id <= (SELECT c2_.id FROM cms_users c2_ WHERE c2_.name = ?))' + ); + } + public function testSupportsMemberOfExpression() { // "Get all users who have $phone as a phonenumber." (*cough* doesnt really make sense...) From a45560dbd0e865607a8b897a9c06696257dcfc5f Mon Sep 17 00:00:00 2001 From: "Roman S. Borschel" Date: Tue, 18 May 2010 22:19:08 +0200 Subject: [PATCH 16/17] [DDC-505] Fixed and small lexer simplifcations that were marked as todo. --- lib/Doctrine/ORM/Query/Lexer.php | 194 +++++++----------- lib/Doctrine/ORM/Query/Parser.php | 9 +- .../ORM/Query/LanguageRecognitionTest.php | 15 +- 3 files changed, 99 insertions(+), 119 deletions(-) diff --git a/lib/Doctrine/ORM/Query/Lexer.php b/lib/Doctrine/ORM/Query/Lexer.php index 7fe7ecb50..4c6645b04 100644 --- a/lib/Doctrine/ORM/Query/Lexer.php +++ b/lib/Doctrine/ORM/Query/Lexer.php @@ -1,7 +1,5 @@ - * @author Janne Vanhala - * @author Roman Borschel - * @license http://www.opensource.org/licenses/lgpl-license.php LGPL - * @link www.doctrine-project.org - * @since 2.0 - * @version $Revision$ + * @author Guilherme Blanco + * @author Janne Vanhala + * @author Roman Borschel + * @since 2.0 */ class Lexer extends \Doctrine\Common\Lexer { + // All tokens that are not valid identifiers must be < 100 const T_NONE = 1; - const T_IDENTIFIER = 2; - const T_INTEGER = 3; - const T_STRING = 4; - const T_INPUT_PARAMETER = 5; - const T_FLOAT = 6; - + const T_INTEGER = 2; + const T_STRING = 3; + const T_INPUT_PARAMETER = 4; + const T_FLOAT = 5; + const T_CLOSE_PARENTHESIS = 6; + const T_OPEN_PARENTHESIS = 7; + const T_COMMA = 8; + const T_DIVIDE = 9; + const T_DOT = 10; + const T_EQUALS = 11; + const T_GREATER_THAN = 12; + const T_LOWER_THAN = 13; + const T_MINUS = 14; + const T_MULTIPLY = 15; + const T_NEGATE = 16; + const T_PLUS = 17; + const T_OPEN_CURLY_BRACE = 18; + const T_CLOSE_CURLY_BRACE = 19; + + // All tokens that are also identifiers should be >= 100 + const T_IDENTIFIER = 100; const T_ALL = 101; const T_AND = 102; const T_ANY = 103; @@ -50,62 +61,46 @@ class Lexer extends \Doctrine\Common\Lexer const T_BETWEEN = 107; const T_BOTH = 108; const T_BY = 109; - const T_CLOSE_PARENTHESIS = 110; - const T_COMMA = 111; - const T_COUNT = 112; - const T_DELETE = 113; - const T_DESC = 114; - const T_DISTINCT = 115; - const T_DIVIDE = 116; - const T_DOT = 117; - const T_EMPTY = 118; - const T_EQUALS = 119; - const T_ESCAPE = 120; - const T_EXISTS = 121; - const T_FALSE = 122; - const T_FROM = 123; - const T_GREATER_THAN = 124; - const T_GROUP = 125; - const T_HAVING = 126; - const T_IN = 127; - const T_INDEX = 128; - const T_INNER = 129; - const T_IS = 130; - const T_JOIN = 131; - const T_LEADING = 132; - const T_LEFT = 133; - const T_LIKE = 134; - const T_LIMIT = 135; - const T_LOWER_THAN = 136; - const T_MAX = 137; - const T_MEMBER = 138; - const T_MIN = 139; - const T_MINUS = 140; - const T_MOD = 141; - const T_MULTIPLY = 142; - const T_NEGATE = 143; - const T_NOT = 144; - const T_NULL = 145; - const T_OF = 146; - const T_OFFSET = 147; - const T_OPEN_PARENTHESIS = 149; - const T_OR = 150; - const T_ORDER = 151; - const T_OUTER = 152; - const T_PLUS = 153; - const T_SELECT = 154; - const T_SET = 155; - const T_SIZE = 156; - const T_SOME = 157; - const T_SUM = 158; - const T_TRAILING = 159; - const T_TRUE = 160; - const T_UPDATE = 161; - const T_WHERE = 162; - const T_WITH = 163; - const T_PARTIAL = 164; - const T_OPEN_CURLY_BRACE = 165; - const T_CLOSE_CURLY_BRACE = 166; + const T_COUNT = 110; + const T_DELETE = 111; + const T_DESC = 112; + const T_DISTINCT = 113; + const T_EMPTY = 114; + const T_ESCAPE = 115; + const T_EXISTS = 116; + const T_FALSE = 117; + const T_FROM = 118; + const T_GROUP = 119; + const T_HAVING = 120; + const T_IN = 121; + const T_INDEX = 122; + const T_INNER = 123; + const T_IS = 124; + const T_JOIN = 125; + const T_LEADING = 126; + const T_LEFT = 127; + const T_LIKE = 128; + const T_MAX = 129; + const T_MEMBER = 130; + const T_MIN = 131; + const T_NOT = 132; + const T_NULL = 133; + const T_OF = 134; + const T_OR = 135; + const T_ORDER = 136; + const T_OUTER = 137; + const T_SELECT = 138; + const T_SET = 139; + const T_SIZE = 140; + const T_SOME = 141; + const T_SUM = 142; + const T_TRAILING = 143; + const T_TRUE = 144; + const T_UPDATE = 145; + const T_WHERE = 146; + const T_WITH = 147; + const T_PARTIAL = 148; + const T_MOD = 149; /** * Creates a new query scanner object. @@ -144,22 +139,26 @@ class Lexer extends \Doctrine\Common\Lexer protected function _getType(&$value) { $type = self::T_NONE; - $newVal = $this->_getNumeric($value); - - // Recognizing numeric values - if ($newVal !== false){ - $value = $newVal; + // Recognizing numeric values + if (is_numeric($value)) { return (strpos($value, '.') !== false || stripos($value, 'e') !== false) ? self::T_FLOAT : self::T_INTEGER; } - + + // Differentiate between quoted names, identifiers, input parameters and symbols if ($value[0] === "'") { $value = str_replace("''", "'", substr($value, 1, strlen($value) - 2)); - return self::T_STRING; } else if (ctype_alpha($value[0]) || $value[0] === '_') { - return $this->_checkLiteral($value); + $name = 'Doctrine\ORM\Query\Lexer::T_' . strtoupper($value); + if (defined($name)) { + $type = constant($name); + if ($type > 100) { + return $type; + } + } + return self::T_IDENTIFIER; } else if ($value[0] === '?' || $value[0] === ':') { return self::T_INPUT_PARAMETER; } else { @@ -186,41 +185,4 @@ class Lexer extends \Doctrine\Common\Lexer return $type; } - - /** - * @todo Inline this method. - */ - private function _getNumeric($value) - { - if ( ! is_scalar($value)) { - return false; - } - // Checking for valid numeric numbers: 1.234, -1.234e-2 - if (is_numeric($value)) { - return $value; - } - - return false; - } - - /** - * Checks if an identifier is a keyword and returns its correct type. - * - * @param string $identifier identifier name - * @return int token type - * @todo Inline this method. - */ - private function _checkLiteral($identifier) - { - $name = 'Doctrine\ORM\Query\Lexer::T_' . strtoupper($identifier); - - if (defined($name)) { - $type = constant($name); - if ($type > 100) { - return $type; - } - } - - return self::T_IDENTIFIER; - } } \ No newline at end of file diff --git a/lib/Doctrine/ORM/Query/Parser.php b/lib/Doctrine/ORM/Query/Parser.php index 241a28751..70ec16484 100644 --- a/lib/Doctrine/ORM/Query/Parser.php +++ b/lib/Doctrine/ORM/Query/Parser.php @@ -231,7 +231,11 @@ class Parser */ public function match($token) { - if ( ! ($this->_lexer->lookahead['type'] === $token)) { + // short-circuit on first condition, usually types match + if ($this->_lexer->lookahead['type'] !== $token && + $token !== Lexer::T_IDENTIFIER && + $this->_lexer->lookahead['type'] <= Lexer::T_IDENTIFIER + ) { $this->syntaxError($this->_lexer->getLiteral($token)); } @@ -890,7 +894,8 @@ class Parser $identVariable = $this->IdentificationVariable(); $this->match(Lexer::T_DOT); - $this->match($this->_lexer->lookahead['type']); + $this->match(Lexer::T_IDENTIFIER); + //$this->match($this->_lexer->lookahead['type']); $field = $this->_lexer->token['value']; // Validate association field diff --git a/tests/Doctrine/Tests/ORM/Query/LanguageRecognitionTest.php b/tests/Doctrine/Tests/ORM/Query/LanguageRecognitionTest.php index 9fc1d6024..0518e2975 100644 --- a/tests/Doctrine/Tests/ORM/Query/LanguageRecognitionTest.php +++ b/tests/Doctrine/Tests/ORM/Query/LanguageRecognitionTest.php @@ -413,12 +413,23 @@ class LanguageRecognitionTest extends \Doctrine\Tests\OrmTestCase { $this->assertValidDql('SELECT u, u.id + ?1 AS someNumber FROM Doctrine\Tests\Models\CMS\CmsUser u'); } - + + /** + * @group DDC-505 + */ public function testDQLKeywordInJoinIsAllowed() { $this->assertValidDql('SELECT u FROM ' . __NAMESPACE__ . '\DQLKeywordsModelUser u JOIN u.group g'); } + /** + * @group DDC-505 + */ + public function testDQLKeywordInConditionIsAllowed() + { + $this->assertValidDql('SELECT g FROM ' . __NAMESPACE__ . '\DQLKeywordsModelGroup g WHERE g.from=0'); + } + /* The exception is currently thrown in the SQLWalker, not earlier. public function testInverseSideSingleValuedAssociationPathNotAllowed() { @@ -441,4 +452,6 @@ class DQLKeywordsModelGroup { /** @Id @Column(type="integer") @GeneratedValue */ private $id; + /** @Column */ + private $from; } From 5bbe6c7292c37ac0a8b84f04dd6c321ee7917e34 Mon Sep 17 00:00:00 2001 From: David Abdemoulaie Date: Tue, 18 May 2010 17:20:40 -0500 Subject: [PATCH 17/17] Revert "Adding missing OnFlush annotation." This reverts commit 79d3f655ef7d90ebc9e22557dee6183ef7696bf1. --- lib/Doctrine/ORM/Mapping/Driver/DoctrineAnnotations.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Doctrine/ORM/Mapping/Driver/DoctrineAnnotations.php b/lib/Doctrine/ORM/Mapping/Driver/DoctrineAnnotations.php index fb76cfe1b..537aaf3a3 100644 --- a/lib/Doctrine/ORM/Mapping/Driver/DoctrineAnnotations.php +++ b/lib/Doctrine/ORM/Mapping/Driver/DoctrineAnnotations.php @@ -134,4 +134,4 @@ final class PostUpdate extends Annotation {} final class PreRemove extends Annotation {} final class PostRemove extends Annotation {} final class PostLoad extends Annotation {} -final class OnFlush extends Annotation {} \ No newline at end of file +