From d2405ded5db366de5634356c92cd7cb154f64c66 Mon Sep 17 00:00:00 2001 From: romanb Date: Tue, 26 May 2009 15:42:54 +0000 Subject: [PATCH] [2.0] Enhanced one-to-one self-referential association handling. --- .../ORM/Internal/Hydration/ObjectHydrator.php | 7 +++-- .../Persisters/StandardEntityPersister.php | 29 ++++++++++++------- lib/Doctrine/ORM/UnitOfWork.php | 24 +++++++++++---- .../Functional/ClassTableInheritanceTest.php | 5 ++-- 4 files changed, 43 insertions(+), 22 deletions(-) diff --git a/lib/Doctrine/ORM/Internal/Hydration/ObjectHydrator.php b/lib/Doctrine/ORM/Internal/Hydration/ObjectHydrator.php index d6fba4d2e..c2224558f 100644 --- a/lib/Doctrine/ORM/Internal/Hydration/ObjectHydrator.php +++ b/lib/Doctrine/ORM/Internal/Hydration/ObjectHydrator.php @@ -288,11 +288,14 @@ class ObjectHydrator extends AbstractHydrator if ($relation->isOwningSide) { // If there is an inverse mapping on the target class its bidirectional if (isset($targetClass->inverseMappings[$property])) { - $sourceProp = $targetClass->inverseMappings[$fieldName]->sourceFieldName; + $sourceProp = $targetClass->inverseMappings[$property]->sourceFieldName; $targetClass->reflFields[$sourceProp]->setValue($entity2, $entity1); + } else if ($classMetadata1 === $targetClass) { + // Special case: self-referencing one-one on the same class + $targetClass->reflFields[$property]->setValue($entity2, $entity1); } } else { - // For sure bidirectional, as there is no inverse side in unidirectional + // For sure bidirectional, as there is no inverse side in unidirectional mappings $targetClass->reflFields[$relation->mappedByFieldName]->setValue($entity2, $entity1); } } diff --git a/lib/Doctrine/ORM/Persisters/StandardEntityPersister.php b/lib/Doctrine/ORM/Persisters/StandardEntityPersister.php index a77ec6cf6..99a66bccd 100644 --- a/lib/Doctrine/ORM/Persisters/StandardEntityPersister.php +++ b/lib/Doctrine/ORM/Persisters/StandardEntityPersister.php @@ -23,6 +23,7 @@ namespace Doctrine\ORM\Persisters; use Doctrine\DBAL\Types\Type; use Doctrine\ORM\EntityManager; +use Doctrine\ORM\UnitOfWork; use Doctrine\ORM\PersistentCollection; use Doctrine\ORM\Mapping\ClassMetadata; @@ -92,7 +93,7 @@ class StandardEntityPersister */ public function addInsert($entity) { - $this->_queuedInserts[] = $entity; + $this->_queuedInserts[spl_object_hash($entity)] = $entity; } /** @@ -237,16 +238,22 @@ class StandardEntityPersister continue; } - //TODO: If the one-one is self-referencing, check whether the referenced entity ($newVal) - // is still scheduled for insertion. If so: - // 1) set $newVal = null, so that we insert a null value - // 2) schedule $entity for an update, so that the FK gets set through an update - // later, after the referenced entity has been inserted. - //$needsPostponedUpdate = ... - /*if ($assocMapping->sourceEntityName == $assocMapping->targetEntityName && - isset($this->_queuedInserts[spl_object_hash($entity)])) { - echo "SELF-REFERENCING!"; - }*/ + // Special case: One-one self-referencing of the same class. + if ($newVal !== null && $assocMapping->sourceEntityName == $assocMapping->targetEntityName) { + $oid = spl_object_hash($newVal); + $isScheduledForInsert = $uow->isRegisteredNew($newVal); + if (isset($this->_queuedInserts[$oid]) || $isScheduledForInsert) { + // The associated entity $newVal is not yet persisted, so we must + // set $newVal = null, in order to insert a null value and update later. + $newVal = null; + } else if ($isInsert && ! $isScheduledForInsert && $uow->getEntityState($newVal) == UnitOfWork::STATE_MANAGED) { + // $newVal is already fully persisted + // Clear changeset of $newVal, so that only the identifier is updated. + // Not sure this is really rock-solid here but it seems to work. + $uow->clearEntityChangeSet($oid); + $uow->propertyChanged($newVal, $field, $entity, $entity); + } + } foreach ($assocMapping->sourceToTargetKeyColumns as $sourceColumn => $targetColumn) { $otherClass = $this->_em->getClassMetadata($assocMapping->targetEntityName); diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index d81626533..d0e280461 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -269,8 +269,6 @@ class UnitOfWork implements PropertyChangedListener throw $e; } - //TODO: commit transaction here? - // Take new snapshots from visited collections foreach ($this->_visitedCollections as $coll) { $coll->takeSnapshot(); @@ -548,9 +546,10 @@ class UnitOfWork implements PropertyChangedListener { $className = $class->name; $persister = $this->getEntityPersister($className); - foreach ($this->_entityInsertions as $entity) { + foreach ($this->_entityInsertions as $oid => $entity) { if (get_class($entity) == $className) { $persister->addInsert($entity); + unset($this->_entityInsertions[$oid]); } } $postInsertIds = $persister->executeInserts(); @@ -577,9 +576,10 @@ class UnitOfWork implements PropertyChangedListener { $className = $class->name; $persister = $this->getEntityPersister($className); - foreach ($this->_entityUpdates as $entity) { + foreach ($this->_entityUpdates as $oid => $entity) { if (get_class($entity) == $className) { $persister->update($entity); + unset($this->_entityUpdates[$oid]); } } } @@ -593,9 +593,10 @@ class UnitOfWork implements PropertyChangedListener { $className = $class->name; $persister = $this->getEntityPersister($className); - foreach ($this->_entityDeletions as $entity) { + foreach ($this->_entityDeletions as $oid => $entity) { if (get_class($entity) == $className) { $persister->delete($entity); + unset($this->_entityDeletions[$oid]); } } } @@ -1532,6 +1533,17 @@ class UnitOfWork implements PropertyChangedListener $this->_originalEntityData[$oid] = $data; $this->addToIdentityMap($entity); } + + /** + * INTERNAL: + * Clears the property changeset of the entity with the given OID. + * + * @param string $oid The entity's OID. + */ + public function clearEntityChangeSet($oid) + { + unset($this->_entityChangeSets[$oid]); + } /* PropertyChangedListener implementation */ @@ -1552,7 +1564,7 @@ class UnitOfWork implements PropertyChangedListener $this->_entityChangeSets[$oid][$propertyName] = array($oldValue, $newValue); if (isset($class->associationMappings[$propertyName])) { - $assoc = $class->associationMappings[$name]; + $assoc = $class->associationMappings[$propertyName]; if ($assoc->isOneToOne() && $assoc->isOwningSide) { $this->_entityUpdates[$oid] = $entity; } else if ($oldValue instanceof PersistentCollection) { diff --git a/tests/Doctrine/Tests/ORM/Functional/ClassTableInheritanceTest.php b/tests/Doctrine/Tests/ORM/Functional/ClassTableInheritanceTest.php index afb37cdea..2bd573b98 100644 --- a/tests/Doctrine/Tests/ORM/Functional/ClassTableInheritanceTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/ClassTableInheritanceTest.php @@ -146,8 +146,7 @@ class ClassTableInheritanceTest extends \Doctrine\Tests\OrmFunctionalTestCase $this->assertTrue($result[0] instanceof CompanyPerson); $this->assertEquals('Mary Smith', $result[0]->getName()); $this->assertTrue($result[0]->getSpouse() instanceof CompanyEmployee); - - //var_dump($result); - + $this->assertEquals('John Smith', $result[0]->getSpouse()->getName()); + $this->assertSame($result[0], $result[0]->getSpouse()->getSpouse()); } }