RetailCRM-Mirror/doctrine2
1
0
Fork 0
mirror of synced 2024-12-05 03:06:05 +03:00
Code Issues Projects Releases Wiki Activity

make lock mode usage consistent

Browse Source
This commit is contained in:
Steve Müller 2014-02-01 18:05:47 +01:00
parent 310afdf5d7
commit a6c8ab8a5f
13 changed files with 107 additions and 61 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

35
UPGRADE.md
View File

@ -10,6 +10,41 @@ the scheduled deletion for updated entities (by calling ``persist()`` if the ent
``entityUpdates`` and ``entityDeletions``). This does not work any longer, because the entire changeset
calculation logic is optimized away.
## Minor BC BREAK: Default lock mode changed from LockMode::NONE to null in method signatures
A misconception concerning default lock mode values in method signatures lead to unexpected behaviour
in SQL statements on SQL Server. With a default lock mode of ``LockMode::NONE`` throughout the
method signatures in ORM, the table lock hint ``WITH (NOLOCK)`` was appended to all locking related
queries by default. This could result in unpredictable results because an explicit ``WITH (NOLOCK)``
table hint tells SQL Server to run a specific query in transaction isolation level READ UNCOMMITTED
instead of the default READ COMMITTED transaction isolation level.
Therefore there now is a distinction between ``LockMode::NONE`` and ``null`` to be able to tell
Doctrine whether to add table lock hints to queries by intention or not. To achieve this, the following
method signatures have been changed to declare ``$lockMode = null`` instead of ``$lockMode = LockMode::NONE``:
- ``Doctrine\ORM\Cache\Persister\AbstractEntityPersister#getSelectSQL()``
- ``Doctrine\ORM\Cache\Persister\AbstractEntityPersister#load()``
- ``Doctrine\ORM\Cache\Persister\AbstractEntityPersister#refresh()``
- ``Doctrine\ORM\Decorator\EntityManagerDecorator#find()``
- ``Doctrine\ORM\EntityManager#find()``
- ``Doctrine\ORM\EntityRepository#find()``
- ``Doctrine\ORM\Persisters\BasicEntityPersister#getSelectSQL()``
- ``Doctrine\ORM\Persisters\BasicEntityPersister#load()``
- ``Doctrine\ORM\Persisters\BasicEntityPersister#refresh()``
- ``Doctrine\ORM\Persisters\EntityPersister#getSelectSQL()``
- ``Doctrine\ORM\Persisters\EntityPersister#load()``
- ``Doctrine\ORM\Persisters\EntityPersister#refresh()``
- ``Doctrine\ORM\Persisters\JoinedSubclassPersister#getSelectSQL()``
You should update signatures for these methods if you have subclassed one of the above classes.
Please also check the calling code of these methods in your application and update if necessary.
**Note:**
This in fact is really a minor BC BREAK and should not have any affect on database vendors
other than SQL Server because it is the only one that supports and therefore cares about
``LockMode::NONE``. It's really just a FIX for SQL Server environments using ORM.
# Upgrade to 2.4
## BC BREAK: Compatibility Bugfix in PersistentCollection#matching()

12
lib/Doctrine/ORM/Cache/Persister/AbstractEntityPersister.php
View File

@ -151,7 +151,7 @@ abstract class AbstractEntityPersister implements CachedEntityPersister
/**
* {@inheritdoc}
*/
public function getSelectSQL($criteria, $assoc = null, $lockMode = 0, $limit = null, $offset = null, array $orderBy = null)
public function getSelectSQL($criteria, $assoc = null, $lockMode = null, $limit = null, $offset = null, array $orderBy = null)
{
return $this->persister->getSelectSQL($criteria, $assoc, $lockMode, $limit, $offset, $orderBy);
}
@ -342,15 +342,15 @@ abstract class AbstractEntityPersister implements CachedEntityPersister
/**
* {@inheritdoc}
*/
public function load(array $criteria, $entity = null, $assoc = null, array $hints = array(), $lockMode = 0, $limit = null, array $orderBy = null)
public function load(array $criteria, $entity = null, $assoc = null, array $hints = array(), $lockMode = null, $limit = null, array $orderBy = null)
{
if ($entity !== null || $assoc !== null || ! empty($hints) || $lockMode !== 0) {
if ($entity !== null || $assoc !== null || ! empty($hints) || $lockMode !== null) {
return $this->persister->load($criteria, $entity, $assoc, $hints, $lockMode, $limit, $orderBy);
}
//handle only EntityRepository#findOneBy
$timestamp = $this->timestampRegion->get($this->timestampKey);
$query = $this->persister->getSelectSQL($criteria, null, 0, $limit, 0, $orderBy);
$query = $this->persister->getSelectSQL($criteria, null, null, $limit, null, $orderBy);
$hash = $this->getHash($query, $criteria, null, null, null, $timestamp ? $timestamp->time : null);
$rsm = $this->getResultSetMapping();
$querykey = new QueryCacheKey($hash, 0, Cache::MODE_NORMAL);
@ -390,7 +390,7 @@ abstract class AbstractEntityPersister implements CachedEntityPersister
public function loadAll(array $criteria = array(), array $orderBy = null, $limit = null, $offset = null)
{
$timestamp = $this->timestampRegion->get($this->timestampKey);
$query = $this->persister->getSelectSQL($criteria, null, 0, $limit, $offset, $orderBy);
$query = $this->persister->getSelectSQL($criteria, null, null, $limit, $offset, $orderBy);
$hash = $this->getHash($query, $criteria, null, null, null, $timestamp ? $timestamp->time : null);
$rsm = $this->getResultSetMapping();
$querykey = new QueryCacheKey($hash, 0, Cache::MODE_NORMAL);
@ -573,7 +573,7 @@ abstract class AbstractEntityPersister implements CachedEntityPersister
/**
* {@inheritdoc}
*/
public function refresh(array $id, $entity, $lockMode = 0)
public function refresh(array $id, $entity, $lockMode = null)
{
$this->persister->refresh($id, $entity, $lockMode);
}

3
lib/Doctrine/ORM/Decorator/EntityManagerDecorator.php
View File

@ -19,7 +19,6 @@
namespace Doctrine\ORM\Decorator;
use Doctrine\DBAL\LockMode;
use Doctrine\ORM\Query\ResultSetMapping;
use Doctrine\ORM\EntityManagerInterface;
use Doctrine\Common\Persistence\ObjectManagerDecorator;
@ -176,7 +175,7 @@ abstract class EntityManagerDecorator extends ObjectManagerDecorator implements
/**
* {@inheritdoc}
*/
public function find($entityName, $id, $lockMode = LockMode::NONE, $lockVersion = null)
public function find($entityName, $id, $lockMode = null, $lockVersion = null)
{
return $this->wrapped->find($entityName, $id, $lockMode, $lockVersion);
}

36
lib/Doctrine/ORM/EntityManager.php
View File

@ -358,10 +358,13 @@ use Doctrine\Common\Util\ClassUtils;
/**
* Finds an Entity by its identifier.
*
* @param string $entityName
* @param mixed $id
* @param integer $lockMode
* @param integer|null $lockVersion
* @param string $entityName The class name of the entity to find.
* @param mixed $id The identity of the entity to find.
* @param integer|null $lockMode One of the \Doctrine\DBAL\LockMode::* constants
* or NULL if no specific lock mode should be used
* during the search.
* @param integer|null $lockVersion The version of the entity to find when using
* optimistic locking.
*
* @return object|null The entity instance or NULL if the entity can not be found.
*
@ -370,7 +373,7 @@ use Doctrine\Common\Util\ClassUtils;
* @throws TransactionRequiredException
* @throws ORMException
*/
public function find($entityName, $id, $lockMode = LockMode::NONE, $lockVersion = null)
public function find($entityName, $id, $lockMode = null, $lockVersion = null)
{
$class = $this->metadataFactory->getMetadataFor(ltrim($entityName, '\\'));
@ -404,13 +407,14 @@ use Doctrine\Common\Util\ClassUtils;
return null;
}
switch ($lockMode) {
case LockMode::OPTIMISTIC:
switch (true) {
case LockMode::OPTIMISTIC === $lockMode:
$this->lock($entity, $lockMode, $lockVersion);
break;
case LockMode::PESSIMISTIC_READ:
case LockMode::PESSIMISTIC_WRITE:
case LockMode::NONE === $lockMode:
case LockMode::PESSIMISTIC_READ === $lockMode;
case LockMode::PESSIMISTIC_WRITE === $lockMode:
$persister = $unitOfWork->getEntityPersister($class->name);
$persister->refresh($sortedId, $entity, $lockMode);
break;
@ -421,11 +425,8 @@ use Doctrine\Common\Util\ClassUtils;
$persister = $unitOfWork->getEntityPersister($class->name);
switch ($lockMode) {
case LockMode::NONE:
return $persister->loadById($sortedId);
case LockMode::OPTIMISTIC:
switch (true) {
case LockMode::OPTIMISTIC === $lockMode:
if ( ! $class->isVersioned) {
throw OptimisticLockException::notVersioned($class->name);
}
@ -436,12 +437,17 @@ use Doctrine\Common\Util\ClassUtils;
return $entity;
default:
case LockMode::NONE === $lockMode:
case LockMode::PESSIMISTIC_READ === $lockMode:
case LockMode::PESSIMISTIC_WRITE === $lockMode:
if ( ! $this->getConnection()->isTransactionActive()) {
throw TransactionRequiredException::transactionRequired();
}
return $persister->load($sortedId, null, null, array(), $lockMode);
default:
return $persister->loadById($sortedId);
}
}

11
lib/Doctrine/ORM/EntityRepository.php
View File

@ -20,10 +20,7 @@
namespace Doctrine\ORM;
use Doctrine\ORM\Query\ResultSetMappingBuilder;
use Doctrine\DBAL\LockMode;
use Doctrine\Common\Persistence\ObjectRepository;
use Doctrine\Common\Collections\Selectable;
use Doctrine\Common\Collections\Criteria;
use Doctrine\Common\Collections\ArrayCollection;
@ -145,12 +142,14 @@ class EntityRepository implements ObjectRepository, Selectable
* Finds an entity by its primary key / identifier.
*
* @param mixed $id The identifier.
* @param int $lockMode The lock mode.
* @param int|null $lockMode One of the \Doctrine\DBAL\LockMode::* constants
* or NULL if no specific lock mode should be used
* during the search.
* @param int|null $lockVersion The lock version.
*
* @return object|null The entity instance or NULL if the entity can not be found.
*/
public function find($id, $lockMode = LockMode::NONE, $lockVersion = null)
public function find($id, $lockMode = null, $lockVersion = null)
{
return $this->_em->find($this->_entityName, $id, $lockMode, $lockVersion);
}
@ -194,7 +193,7 @@ class EntityRepository implements ObjectRepository, Selectable
{
$persister = $this->_em->getUnitOfWork()->getEntityPersister($this->_entityName);
return $persister->load($criteria, null, null, array(), 0, 1, $orderBy);
return $persister->load($criteria, null, null, array(), null, 1, $orderBy);
}
/**

14
lib/Doctrine/ORM/Persisters/BasicEntityPersister.php
View File

@ -706,7 +706,7 @@ class BasicEntityPersister implements EntityPersister
/**
* {@inheritdoc}
*/
public function load(array $criteria, $entity = null, $assoc = null, array $hints = array(), $lockMode = 0, $limit = null, array $orderBy = null)
public function load(array $criteria, $entity = null, $assoc = null, array $hints = array(), $lockMode = null, $limit = null, array $orderBy = null)
{
$sql = $this->getSelectSQL($criteria, $assoc, $lockMode, $limit, null, $orderBy);
list($params, $types) = $this->expandParameters($criteria);
@ -800,7 +800,7 @@ class BasicEntityPersister implements EntityPersister
/**
* {@inheritdoc}
*/
public function refresh(array $id, $entity, $lockMode = 0)
public function refresh(array $id, $entity, $lockMode = null)
{
$sql = $this->getSelectSQL($id, null, $lockMode);
list($params, $types) = $this->expandParameters($id);
@ -818,7 +818,7 @@ class BasicEntityPersister implements EntityPersister
$orderBy = $criteria->getOrderings();
$limit = $criteria->getMaxResults();
$offset = $criteria->getFirstResult();
$query = $this->getSelectSQL($criteria, null, 0, $limit, $offset, $orderBy);
$query = $this->getSelectSQL($criteria, null, null, $limit, $offset, $orderBy);
list($params, $types) = $this->expandCriteriaParameters($criteria);
@ -869,7 +869,7 @@ class BasicEntityPersister implements EntityPersister
*/
public function loadAll(array $criteria = array(), array $orderBy = null, $limit = null, $offset = null)
{
$sql = $this->getSelectSQL($criteria, null, 0, $limit, $offset, $orderBy);
$sql = $this->getSelectSQL($criteria, null, null, $limit, $offset, $orderBy);
list($params, $types) = $this->expandParameters($criteria);
$stmt = $this->conn->executeQuery($sql, $params, $types);
@ -1005,7 +1005,7 @@ class BasicEntityPersister implements EntityPersister
$criteria[$quotedJoinTable . '.' . $quotedKeyColumn] = $value;
}
$sql = $this->getSelectSQL($criteria, $assoc, 0, $limit, $offset);
$sql = $this->getSelectSQL($criteria, $assoc, null, $limit, $offset);
list($params, $types) = $this->expandParameters($criteria);
return $this->conn->executeQuery($sql, $params, $types);
@ -1014,7 +1014,7 @@ class BasicEntityPersister implements EntityPersister
/**
* {@inheritdoc}
*/
public function getSelectSQL($criteria, $assoc = null, $lockMode = 0, $limit = null, $offset = null, array $orderBy = null)
public function getSelectSQL($criteria, $assoc = null, $lockMode = null, $limit = null, $offset = null, array $orderBy = null)
{
$lockSql = '';
$joinSql = '';
@ -1673,7 +1673,7 @@ class BasicEntityPersister implements EntityPersister
$criteria[$tableAlias . "." . $targetKeyColumn] = $sourceClass->reflFields[$sourceClass->fieldNames[$sourceKeyColumn]]->getValue($sourceEntity);
}
$sql = $this->getSelectSQL($criteria, $assoc, 0, $limit, $offset);
$sql = $this->getSelectSQL($criteria, $assoc, null, $limit, $offset);
list($params, $types) = $this->expandParameters($criteria);
return $this->conn->executeQuery($sql, $params, $types);

22
lib/Doctrine/ORM/Persisters/EntityPersister.php
View File

@ -65,14 +65,14 @@ interface EntityPersister
*
* @param array|\Doctrine\Common\Collections\Criteria $criteria
* @param array|null $assoc
* @param int $lockMode
* @param int|null $lockMode
* @param int|null $limit
* @param int|null $offset
* @param array|null $orderBy
*
* @return string
*/
public function getSelectSQL($criteria, $assoc = null, $lockMode = 0, $limit = null, $offset = null, array $orderBy = null);
public function getSelectSQL($criteria, $assoc = null, $lockMode = null, $limit = null, $offset = null, array $orderBy = null);
/**
* Expands the parameters from the given criteria and use the correct binding types if found.
@ -160,7 +160,9 @@ interface EntityPersister
* @param object|null $entity The entity to load the data into. If not specified, a new entity is created.
* @param array|null $assoc The association that connects the entity to load to another entity, if any.
* @param array $hints Hints for entity creation.
* @param int $lockMode
* @param int|null $lockMode One of the \Doctrine\DBAL\LockMode::* constants
* or NULL if no specific lock mode should be used
* for loading the entity.
* @param int|null $limit Limit number of results.
* @param array|null $orderBy Criteria to order by.
*
@ -168,7 +170,7 @@ interface EntityPersister
*
* @todo Check identity map? loadById method? Try to guess whether $criteria is the id?
*/
public function load(array $criteria, $entity = null, $assoc = null, array $hints = array(), $lockMode = 0, $limit = null, array $orderBy = null);
public function load(array $criteria, $entity = null, $assoc = null, array $hints = array(), $lockMode = null, $limit = null, array $orderBy = null);
/**
* Loads an entity by identifier.
@ -201,14 +203,16 @@ interface EntityPersister
/**
* Refreshes a managed entity.
*
* @param array $id The identifier of the entity as an associative array from
* column or field names to values.
* @param object $entity The entity to refresh.
* @param int $lockMode
* @param array $id The identifier of the entity as an associative array from
* column or field names to values.
* @param object $entity The entity to refresh.
* @param int|null $lockMode One of the \Doctrine\DBAL\LockMode::* constants
* or NULL if no specific lock mode should be used
* for refreshing the managed entity.
*
* @return void
*/
public function refresh(array $id, $entity, $lockMode = 0);
public function refresh(array $id, $entity, $lockMode = null);
/**
* Loads Entities matching the given Criteria object.

7
lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php
View File

@ -296,7 +296,7 @@ class JoinedSubclassPersister extends AbstractEntityInheritancePersister
/**
* {@inheritdoc}
*/
public function getSelectSQL($criteria, $assoc = null, $lockMode = 0, $limit = null, $offset = null, array $orderBy = null)
public function getSelectSQL($criteria, $assoc = null, $lockMode = null, $limit = null, $offset = null, array $orderBy = null)
{
$joinSql = '';
$identifierColumn = $this->class->getIdentifierColumnNames();
@ -374,11 +374,12 @@ class JoinedSubclassPersister extends AbstractEntityInheritancePersister
}
$tableName = $this->quoteStrategy->getTableName($this->class, $this->platform);
$from = ' FROM ' . $tableName . ' ' . $baseTableAlias;
$where = $conditionSql != '' ? ' WHERE ' . $conditionSql : '';
$lock = $this->platform->appendLockHint($from, $lockMode);
$columnList = $this->getSelectColumnsSQL();
$query = 'SELECT ' . $columnList
. ' FROM '
. $tableName . ' ' . $baseTableAlias
. $lock
. $joinSql
. $where
. $orderBySql;

2
lib/Doctrine/ORM/Persisters/OneToManyPersister.php
View File

@ -45,7 +45,7 @@ class OneToManyPersister extends AbstractCollectionPersister
throw new \BadMethodCallException("Selecting a collection by index is only supported on indexed collections.");
}
return $persister->load(array($mapping['mappedBy'] => $coll->getOwner(), $mapping['indexBy'] => $index), null, null, array(), 0, 1);
return $persister->load(array($mapping['mappedBy'] => $coll->getOwner(), $mapping['indexBy'] => $index), null, null, array(), null, 1);
}
/**

8
lib/Doctrine/ORM/Query.php
View File

@ -644,7 +644,7 @@ final class Query extends AbstractQuery
*/
public function setLockMode($lockMode)
{
if (in_array($lockMode, array(LockMode::PESSIMISTIC_READ, LockMode::PESSIMISTIC_WRITE))) {
if (in_array($lockMode, array(LockMode::NONE, LockMode::PESSIMISTIC_READ, LockMode::PESSIMISTIC_WRITE), true)) {
if ( ! $this->_em->getConnection()->isTransactionActive()) {
throw TransactionRequiredException::transactionRequired();
}
@ -658,14 +658,14 @@ final class Query extends AbstractQuery
/**
* Get the current lock mode for this query.
*
* @return int
* @return int|null The current lock mode of this query or NULL if no specific lock mode is set.
*/
public function getLockMode()
{
$lockMode = $this->getHint(self::HINT_LOCK_MODE);
if ( ! $lockMode) {
return LockMode::NONE;
if (false === $lockMode) {
return null;
}
return $lockMode;

2
lib/Doctrine/ORM/Query/SqlWalker.php
View File

@ -542,7 +542,7 @@ class SqlWalker implements TreeWalker
$sql = $this->platform->modifyLimitQuery($sql, $limit, $offset);
}
if ($lockMode === false || $lockMode === LockMode::NONE) {
if ($lockMode === null || $lockMode === false || $lockMode === LockMode::NONE) {
return $sql;
}

10
lib/Doctrine/ORM/UnitOfWork.php
View File

@ -19,6 +19,7 @@
namespace Doctrine\ORM;
use Doctrine\DBAL\LockMode;
use Exception;
use InvalidArgumentException;
use UnexpectedValueException;
@ -2306,8 +2307,8 @@ class UnitOfWork implements PropertyChangedListener
$class = $this->em->getClassMetadata(get_class($entity));
switch ($lockMode) {
case \Doctrine\DBAL\LockMode::OPTIMISTIC;
switch (true) {
case LockMode::OPTIMISTIC === $lockMode;
if ( ! $class->isVersioned) {
throw OptimisticLockException::notVersioned($class->name);
}
@ -2324,8 +2325,9 @@ class UnitOfWork implements PropertyChangedListener
break;
case \Doctrine\DBAL\LockMode::PESSIMISTIC_READ:
case \Doctrine\DBAL\LockMode::PESSIMISTIC_WRITE:
case LockMode::NONE === $lockMode:
case LockMode::PESSIMISTIC_READ === $lockMode:
case LockMode::PESSIMISTIC_WRITE === $lockMode:
if (!$this->em->getConnection()->isTransactionActive()) {
throw TransactionRequiredException::transactionRequired();
}

2
tests/Doctrine/Tests/ORM/Functional/Ticket/DDC719Test.php
View File

@ -19,7 +19,7 @@ class DDC719Test extends \Doctrine\Tests\OrmFunctionalTestCase
{
$q = $this->_em->createQuery('SELECT g, c FROM Doctrine\Tests\ORM\Functional\Ticket\DDC719Group g LEFT JOIN g.children c WHERE g.parents IS EMPTY');
$referenceSQL = ($this->_em->getConnection()->getDatabasePlatform() instanceof SQLServerPlatform) ? 'SELECT g0_.name AS name0, g0_.description AS description1, g0_.id AS id2, g1_.name AS name3, g1_.description AS description4, g1_.id AS id5 FROM groups g0_ with (nolock) LEFT JOIN groups_groups g2_ ON g0_.id = g2_.parent_id LEFT JOIN groups g1_ ON g1_.id = g2_.child_id WHERE (SELECT COUNT(*) FROM groups_groups g3_ WHERE g3_.child_id = g0_.id) = 0' : 'SELECT g0_.name AS name0, g0_.description AS description1, g0_.id AS id2, g1_.name AS name3, g1_.description AS description4, g1_.id AS id5 FROM groups g0_ LEFT JOIN groups_groups g2_ ON g0_.id = g2_.parent_id LEFT JOIN groups g1_ ON g1_.id = g2_.child_id WHERE (SELECT COUNT(*) FROM groups_groups g3_ WHERE g3_.child_id = g0_.id) = 0';
$referenceSQL = 'SELECT g0_.name AS name0, g0_.description AS description1, g0_.id AS id2, g1_.name AS name3, g1_.description AS description4, g1_.id AS id5 FROM groups g0_ LEFT JOIN groups_groups g2_ ON g0_.id = g2_.parent_id LEFT JOIN groups g1_ ON g1_.id = g2_.child_id WHERE (SELECT COUNT(*) FROM groups_groups g3_ WHERE g3_.child_id = g0_.id) = 0';
$this->assertEquals(
strtolower($referenceSQL),