From 69073c4b37ee28f988306db4965f512b70f45181 Mon Sep 17 00:00:00 2001 From: "Roman S. Borschel" Date: Thu, 29 Jul 2010 22:27:00 +0200 Subject: [PATCH] Fixes for merging bidirectional associations where both sides define cascade=merge as well as fixes for handling null values and proxies properly in single-valued associations. --- lib/Doctrine/ORM/Proxy/ProxyFactory.php | 10 +--- lib/Doctrine/ORM/UnitOfWork.php | 38 +++++++----- .../Tests/Models/CMS/CmsPhonenumber.php | 2 +- .../ORM/Functional/BasicFunctionalTest.php | 33 ++++++++++- .../ORM/Functional/DetachedEntityTest.php | 58 +++++++++---------- 5 files changed, 86 insertions(+), 55 deletions(-) diff --git a/lib/Doctrine/ORM/Proxy/ProxyFactory.php b/lib/Doctrine/ORM/Proxy/ProxyFactory.php index 8051bc39a..54d234d06 100644 --- a/lib/Doctrine/ORM/Proxy/ProxyFactory.php +++ b/lib/Doctrine/ORM/Proxy/ProxyFactory.php @@ -221,9 +221,9 @@ class ProxyFactory $sleepImpl = ''; if ($class->reflClass->hasMethod('__sleep')) { - $sleepImpl .= 'return parent::__sleep();'; + $sleepImpl .= "return array_merge(array('__isInitialized__'), parent::__sleep());"; } else { - $sleepImpl .= 'return array('; + $sleepImpl .= "return array('__isInitialized__', "; $first = true; foreach ($class->getReflectionProperties() as $name => $prop) { @@ -268,8 +268,7 @@ class extends \ implements \Doctrine\ORM\Proxy\Proxy if ($this->_entityPersister->load($this->_identifier, $this) === null) { throw new \Doctrine\ORM\EntityNotFoundException(); } - unset($this->_entityPersister); - unset($this->_identifier); + unset($this->_entityPersister, $this->_identifier); } } @@ -277,9 +276,6 @@ class extends \ implements \Doctrine\ORM\Proxy\Proxy public function __sleep() { - if (!$this->__isInitialized__) { - throw new \RuntimeException("Not fully loaded proxy can not be serialized."); - } } }'; diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index 3f1b3e332..65142e84c 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -1349,11 +1349,13 @@ class UnitOfWork implements PropertyChangedListener return; // Prevent infinite recursion } + $visited[$oid] = $entity; // mark visited + $class = $this->em->getClassMetadata(get_class($entity)); // First we assume DETACHED, although it can still be NEW but we can avoid - // an extra db-roundtrip this way. If it is DETACHED or NEW, we need to fetch - // it from the db anyway in order to merge. + // an extra db-roundtrip this way. If it is not MANAGED but has an identity, + // we need to fetch it from the db anyway in order to merge. // MANAGED entities are ignored by the merge operation. if ($this->getEntityState($entity, self::STATE_DETACHED) == self::STATE_MANAGED) { $managedCopy = $entity; @@ -1407,23 +1409,27 @@ class UnitOfWork implements PropertyChangedListener } else { $assoc2 = $class->associationMappings[$name]; if ($assoc2->isOneToOne()) { - if ( ! $assoc2->isCascadeMerge) { - $other = $prop->getValue($entity); - if ($other !== null) { - if ($this->getEntityState($other, self::STATE_DETACHED) == self::STATE_MANAGED) { - $prop->setValue($managedCopy, $other); - } else { - $targetClass = $this->em->getClassMetadata($assoc2->targetEntityName); - $id = $targetClass->getIdentifierValues($other); - $proxy = $this->em->getProxyFactory()->getProxy($assoc2->targetEntityName, $id); - $prop->setValue($managedCopy, $proxy); - $this->registerManaged($proxy, $id, array()); - } + $other = $prop->getValue($entity); + if ($other === null) { + $prop->setValue($managedCopy, null); + } else if ($other instanceof Proxy && !$other->__isInitialized__) { + // do not merge fields marked lazy that have not been fetched. + continue; + } else if ( ! $assoc2->isCascadeMerge) { + if ($this->getEntityState($other, self::STATE_DETACHED) == self::STATE_MANAGED) { + $prop->setValue($managedCopy, $other); + } else { + $targetClass = $this->em->getClassMetadata($assoc2->targetEntityName); + $id = $targetClass->getIdentifierValues($other); + $proxy = $this->em->getProxyFactory()->getProxy($assoc2->targetEntityName, $id); + $prop->setValue($managedCopy, $proxy); + $this->registerManaged($proxy, $id, array()); } } } else { $mergeCol = $prop->getValue($entity); if ($mergeCol instanceof PersistentCollection && !$mergeCol->isInitialized()) { + // do not merge fields marked lazy that have not been fetched. // keep the lazy persistent collection of the managed copy. continue; } @@ -1451,7 +1457,6 @@ class UnitOfWork implements PropertyChangedListener $prevClass = $this->em->getClassMetadata(get_class($prevManagedCopy)); if ($assoc->isOneToOne()) { $prevClass->reflFields[$assocField]->setValue($prevManagedCopy, $managedCopy); - //TODO: What about back-reference if bidirectional? } else { $prevClass->reflFields[$assocField]->getValue($prevManagedCopy)->unwrap()->add($managedCopy); if ($assoc->isOneToMany()) { @@ -1460,6 +1465,9 @@ class UnitOfWork implements PropertyChangedListener } } + // Mark the managed copy visited as well + $visited[spl_object_hash($managedCopy)] = true; + $this->cascadeMerge($entity, $managedCopy, $visited); return $managedCopy; diff --git a/tests/Doctrine/Tests/Models/CMS/CmsPhonenumber.php b/tests/Doctrine/Tests/Models/CMS/CmsPhonenumber.php index 431b8c8ae..b62464dff 100644 --- a/tests/Doctrine/Tests/Models/CMS/CmsPhonenumber.php +++ b/tests/Doctrine/Tests/Models/CMS/CmsPhonenumber.php @@ -13,7 +13,7 @@ class CmsPhonenumber */ public $phonenumber; /** - * @ManyToOne(targetEntity="CmsUser", inversedBy="phonenumbers") + * @ManyToOne(targetEntity="CmsUser", inversedBy="phonenumbers", cascade={"merge"}) * @JoinColumn(name="user_id", referencedColumnName="id") */ public $user; diff --git a/tests/Doctrine/Tests/ORM/Functional/BasicFunctionalTest.php b/tests/Doctrine/Tests/ORM/Functional/BasicFunctionalTest.php index d265dc336..d12a183d9 100644 --- a/tests/Doctrine/Tests/ORM/Functional/BasicFunctionalTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/BasicFunctionalTest.php @@ -788,7 +788,8 @@ class BasicFunctionalTest extends \Doctrine\Tests\OrmFunctionalTestCase $userId = $managedUser->id; $this->_em->clear(); - $this->assertTrue($this->_em->find(get_class($managedUser), $userId) instanceof CmsUser); + $user2 = $this->_em->find(get_class($managedUser), $userId); + $this->assertTrue($user2 instanceof CmsUser); } public function testMergeThrowsExceptionIfEntityWithGeneratedIdentifierDoesNotExist() @@ -803,4 +804,34 @@ class BasicFunctionalTest extends \Doctrine\Tests\OrmFunctionalTestCase $this->fail(); } catch (\Doctrine\ORM\EntityNotFoundException $enfe) {} } + + /** + * @group DDC-634 + */ + public function testOneToOneMergeSetNull() + { + //$this->_em->getConnection()->getConfiguration()->setSQLLogger(new \Doctrine\DBAL\Logging\EchoSQLLogger); + $user = new CmsUser(); + $user->username = "beberlei"; + $user->name = "Benjamin E."; + $user->status = 'active'; + + $ph = new CmsPhonenumber(); + $ph->phonenumber = "12345"; + $user->addPhonenumber($ph); + + $this->_em->persist($user); + $this->_em->persist($ph); + $this->_em->flush(); + + $this->_em->clear(); + + $ph->user = null; + $managedPh = $this->_em->merge($ph); + + $this->_em->flush(); + $this->_em->clear(); + + $this->assertNull($this->_em->find(get_class($ph), $ph->phonenumber)->getUser()); + } } diff --git a/tests/Doctrine/Tests/ORM/Functional/DetachedEntityTest.php b/tests/Doctrine/Tests/ORM/Functional/DetachedEntityTest.php index 95c97ee6a..eee01fadf 100644 --- a/tests/Doctrine/Tests/ORM/Functional/DetachedEntityTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/DetachedEntityTest.php @@ -44,9 +44,10 @@ class DetachedEntityTest extends \Doctrine\Tests\OrmFunctionalTestCase $this->assertTrue($this->_em->contains($user2)); $this->assertEquals('Roman B.', $user2->name); } - + public function testSerializeUnserializeModifyMerge() { + //$this->_em->getConnection()->getConfiguration()->setSQLLogger(new \Doctrine\DBAL\Logging\EchoSQLLogger); $user = new CmsUser; $user->name = 'Guilherme'; $user->username = 'gblanco'; @@ -59,6 +60,7 @@ class DetachedEntityTest extends \Doctrine\Tests\OrmFunctionalTestCase $this->_em->persist($user); $this->_em->flush(); $this->assertTrue($this->_em->contains($user)); + $this->assertTrue($user->phonenumbers->isInitialized()); $serialized = serialize($user); $this->_em->clear(); @@ -103,42 +105,36 @@ class DetachedEntityTest extends \Doctrine\Tests\OrmFunctionalTestCase } catch (\Exception $expected) {} } - /** - * @group DDC-518 - */ - /*public function testMergeDetachedEntityWithNewlyPersistentOneToOneAssoc() - { + public function testUninitializedLazyAssociationsAreIgnoredOnMerge() { //$this->_em->getConnection()->getConfiguration()->setSQLLogger(new \Doctrine\DBAL\Logging\EchoSQLLogger); - // Create a detached user $user = new CmsUser; - $user->name = 'Roman'; - $user->username = 'romanb'; - $user->status = 'dev'; + $user->name = 'Guilherme'; + $user->username = 'gblanco'; + $user->status = 'developer'; + + $address = new CmsAddress; + $address->city = 'Berlin'; + $address->country = 'Germany'; + $address->street = 'Sesamestreet'; + $address->zip = 12345; + $address->setUser($user); + $this->_em->persist($address); $this->_em->persist($user); + $this->_em->flush(); $this->_em->clear(); - //$address = new CmsAddress; - //$address->city = 'Berlin'; - //$address->country = 'Germany'; - //$address->street = 'Sesamestreet'; - //$address->zip = 12345; - //$address->setUser($user); + $address2 = $this->_em->find(get_class($address), $address->id); + $this->assertTrue($address2->user instanceof \Doctrine\ORM\Proxy\Proxy); + $this->assertFalse($address2->user->__isInitialized__); + $detachedAddress2 = unserialize(serialize($address2)); + $this->assertTrue($detachedAddress2->user instanceof \Doctrine\ORM\Proxy\Proxy); + $this->assertFalse($detachedAddress2->user->__isInitialized__); - $phone = new CmsPhonenumber(); - $phone->phonenumber = '12345'; - - $user2 = $this->_em->merge($user); - - $user2->addPhonenumber($phone); - $this->_em->persist($phone); - - //$address->setUser($user2); - //$this->_em->persist($address); - - $this->_em->flush(); - - $this->assertEquals(1,1); - }*/ + $managedAddress2 = $this->_em->merge($detachedAddress2); + $this->assertTrue($managedAddress2->user instanceof \Doctrine\ORM\Proxy\Proxy); + $this->assertFalse($managedAddress2->user === $detachedAddress2->user); + $this->assertFalse($managedAddress2->user->__isInitialized__); + } }