From 2858b8290fc63ca96a31ef173c9cf128ec18bfe7 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Mon, 4 Jul 2011 23:19:08 +0200 Subject: [PATCH] DDC-1238 - Fixed a bug introduced when refactoring persisters hydration. This occurs when you call $em->clear() and you start accessing a proxy. --- .../Internal/Hydration/AbstractHydrator.php | 21 ++++++++ .../ORM/Internal/Hydration/ObjectHydrator.php | 6 +++ .../Hydration/SimpleObjectHydrator.php | 16 ++---- .../ORM/Persisters/BasicEntityPersister.php | 8 +-- lib/Doctrine/ORM/Query.php | 9 ++++ lib/Doctrine/ORM/UnitOfWork.php | 6 +-- .../ORM/Functional/Ticket/DDC1238Test.php | 52 +++++++++++-------- 7 files changed, 76 insertions(+), 42 deletions(-) diff --git a/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php b/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php index 4bc53f738..181854e36 100644 --- a/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php +++ b/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php @@ -287,4 +287,25 @@ abstract class AbstractHydrator return $rowData; } + + protected function registerManaged($class, $entity, $data) + { + if ($class->isIdentifierComposite) { + $id = array(); + foreach ($class->identifier as $fieldName) { + if (isset($class->associationMappings[$fieldName])) { + $id[$fieldName] = $data[$class->associationMappings[$fieldName]['joinColumns'][0]['name']]; + } else { + $id[$fieldName] = $data[$fieldName]; + } + } + } else { + if (isset($class->associationMappings[$class->identifier[0]])) { + $id = array($class->identifier[0] => $data[$class->associationMappings[$class->identifier[0]]['joinColumns'][0]['name']]); + } else { + $id = array($class->identifier[0] => $data[$class->identifier[0]]); + } + } + $this->_em->getUnitOfWork()->registerManaged($entity, $id, $data); + } } diff --git a/lib/Doctrine/ORM/Internal/Hydration/ObjectHydrator.php b/lib/Doctrine/ORM/Internal/Hydration/ObjectHydrator.php index 381fd4ab1..2935f67a7 100644 --- a/lib/Doctrine/ORM/Internal/Hydration/ObjectHydrator.php +++ b/lib/Doctrine/ORM/Internal/Hydration/ObjectHydrator.php @@ -205,6 +205,12 @@ class ObjectHydrator extends AbstractHydrator $className = $this->_ce[$className]->discriminatorMap[$data[$discrColumn]]; unset($data[$discrColumn]); } + + if (isset($this->_hints[Query::HINT_REFRESH_ENTITY]) && isset($this->_rootAliases[$dqlAlias])) { + $class = $this->_ce[$className]; + $this->registerManaged($class, $this->_hints[Query::HINT_REFRESH_ENTITY], $data); + } + return $this->_uow->createEntity($className, $data, $this->_hints); } diff --git a/lib/Doctrine/ORM/Internal/Hydration/SimpleObjectHydrator.php b/lib/Doctrine/ORM/Internal/Hydration/SimpleObjectHydrator.php index 3524f89e9..34d8e31b1 100644 --- a/lib/Doctrine/ORM/Internal/Hydration/SimpleObjectHydrator.php +++ b/lib/Doctrine/ORM/Internal/Hydration/SimpleObjectHydrator.php @@ -23,11 +23,10 @@ namespace Doctrine\ORM\Internal\Hydration; use \PDO; use Doctrine\ORM\Mapping\ClassMetadata; use Doctrine\DBAL\Types\Type; +use Doctrine\ORM\Query; class SimpleObjectHydrator extends AbstractHydrator { - const REFRESH_ENTITY = 'doctrine_refresh_entity'; - /** * @var ClassMetadata */ @@ -123,17 +122,8 @@ class SimpleObjectHydrator extends AbstractHydrator } } - if (isset($this->_hints[self::REFRESH_ENTITY])) { - $this->_hints[Query::HINT_REFRESH] = true; - $id = array(); - if ($this->_class->isIdentifierComposite) { - foreach ($this->_class->identifier as $fieldName) { - $id[$fieldName] = $data[$fieldName]; - } - } else { - $id = array($this->_class->identifier[0] => $data[$this->_class->identifier[0]]); - } - $this->_em->getUnitOfWork()->registerManaged($this->_hints[self::REFRESH_ENTITY], $id, $data); + if (isset($this->_hints[Query::HINT_REFRESH_ENTITY])) { + $this->registerManaged($this->class, $this->_hints[Query::HINT_REFRESH_ENTITY], $data); } $result[] = $this->_em->getUnitOfWork()->createEntity($entityName, $data, $this->_hints); diff --git a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php index 2ee3cf4fe..2de0611cd 100644 --- a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php +++ b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php @@ -570,6 +570,7 @@ class BasicEntityPersister if ($entity !== null) { $hints[Query::HINT_REFRESH] = true; + $hints[Query::HINT_REFRESH_ENTITY] = $entity; } if ($this->_selectJoinSql) { @@ -587,13 +588,12 @@ class BasicEntityPersister * * @param array $assoc The association to load. * @param object $sourceEntity The entity that owns the association (not necessarily the "owning side"). - * @param object $targetEntity The existing ghost entity (proxy) to load, if any. * @param array $identifier The identifier of the entity to load. Must be provided if * the association to load represents the owning side, otherwise * the identifier is derived from the $sourceEntity. * @return object The loaded and managed entity instance or NULL if the entity can not be found. */ - public function loadOneToOneEntity(array $assoc, $sourceEntity, $targetEntity, array $identifier = array()) + public function loadOneToOneEntity(array $assoc, $sourceEntity, array $identifier = array()) { if ($foundEntity = $this->_em->getUnitOfWork()->tryGetById($identifier, $assoc['targetEntity'])) { return $foundEntity; @@ -621,7 +621,7 @@ class BasicEntityPersister } */ - $targetEntity = $this->load($identifier, $targetEntity, $assoc, $hints); + $targetEntity = $this->load($identifier, null, $assoc, $hints); // Complete bidirectional association, if necessary if ($targetEntity !== null && $isInverseSingleValued) { @@ -641,7 +641,7 @@ class BasicEntityPersister } } - $targetEntity = $this->load($identifier, $targetEntity, $assoc); + $targetEntity = $this->load($identifier, null, $assoc); if ($targetEntity !== null) { $targetClass->setFieldValue($targetEntity, $assoc['mappedBy'], $sourceEntity); diff --git a/lib/Doctrine/ORM/Query.php b/lib/Doctrine/ORM/Query.php index fa7fff2e4..820711d55 100644 --- a/lib/Doctrine/ORM/Query.php +++ b/lib/Doctrine/ORM/Query.php @@ -53,6 +53,15 @@ final class Query extends AbstractQuery * @var string */ const HINT_REFRESH = 'doctrine.refresh'; + + + /** + * Internal hint: is set to the proxy entity that is currently triggered for loading + * + * @var string + */ + const HINT_REFRESH_ENTITY = 'doctrine.refresh.entity'; + /** * The forcePartialLoad query hint forces a particular query to return * partial objects. diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index 2a2ec68c6..684210272 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -1976,7 +1976,7 @@ class UnitOfWork implements PropertyChangedListener // a way to solve this with deferred eager loading, which means putting // an entity with subclasses at a *-to-one location is really bad! (performance-wise) $newValue = $this->getEntityPersister($assoc['targetEntity']) - ->loadOneToOneEntity($assoc, $entity, null, $associatedId); + ->loadOneToOneEntity($assoc, $entity, $associatedId); } else { // Deferred eager load only works for single identifier classes @@ -2012,7 +2012,7 @@ class UnitOfWork implements PropertyChangedListener } else { // Inverse side of x-to-one can never be lazy $class->reflFields[$field]->setValue($entity, $this->getEntityPersister($assoc['targetEntity']) - ->loadOneToOneEntity($assoc, $entity, null)); + ->loadOneToOneEntity($assoc, $entity)); } } else { // Inject collection @@ -2143,7 +2143,7 @@ class UnitOfWork implements PropertyChangedListener * @return array The identifier values. */ public function getEntityIdentifier($entity) - { + { return $this->entityIdentifiers[spl_object_hash($entity)]; } diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1238Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1238Test.php index 63b986bd7..93a89818e 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1238Test.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1238Test.php @@ -18,8 +18,6 @@ class DDC1238Test extends \Doctrine\Tests\OrmFunctionalTestCase try { $this->_schemaTool->createSchema(array( $this->_em->getClassMetadata(__NAMESPACE__ . '\\DDC1238User'), - #$this->_em->getClassMetadata(__NAMESPACE__ . '\\DDC1238UserBase'), - $this->_em->getClassMetadata(__NAMESPACE__ . '\\DDC1238UserSuperClass'), )); } catch(\PDOException $e) { @@ -40,27 +38,51 @@ class DDC1238Test extends \Doctrine\Tests\OrmFunctionalTestCase $user = $this->_em->getReference(__NAMESPACE__ . '\\DDC1238User', $userId); $this->_em->clear(); - #$user2 = $this->_em->getReference(__NAMESPACE__ . '\\DDC1238User', $userId); - xdebug_start_trace("/tmp/doctrine"); + $userId2 = $user->getId(); + $this->assertEquals($userId, $userId2, "This proxy can still be initialized."); + } + + public function testIssueProxyClear() + { + $user = new DDC1238User; + $user->setName("test"); + + $this->_em->persist($user); + $this->_em->flush(); + $this->_em->clear(); + $userId = $user->getId(); + $this->_em->clear(); - $this->assertNotSame($user, $user2); - $this->assertNull($userId, "This proxy is unitialized and was cleared from the identity map, so no loading possible."); + $user = $this->_em->getReference(__NAMESPACE__ . '\\DDC1238User', $userId); + $this->_em->clear(); + + $user2 = $this->_em->getReference(__NAMESPACE__ . '\\DDC1238User', $userId); + + $this->assertNull($user->getId(), "Now this is null, we already have a user instance of that type"); } } /** - * @MappedSuperclass + * @Entity */ -abstract class DDC1238UserSuperClass +class DDC1238User { + /** @Id @GeneratedValue @Column(type="integer") */ + private $id; + /** * @Column * @var string */ private $name; + public function getId() + { + return $this->id; + } + public function getName() { return $this->name; @@ -72,17 +94,3 @@ abstract class DDC1238UserSuperClass } } -/** - * @Entity - */ -class DDC1238User extends DDC1238UserSuperClass -{ - /** @Id @GeneratedValue @Column(type="integer") */ - private $id; - - public function getId() - { - return $this->id; - } -} -