Skip to content

Commit

Permalink
Fixes for merging bidirectional associations where both sides define …
Browse files Browse the repository at this point in the history 
…cascade=merge as well as fixes for handling null values and proxies properly in single-valued associations.
  • Loading branch information
@romanb
romanb committed Jul 30, 2010
1 parent 954a8c3 commit 69073c4
Show file tree
Hide file tree
Showing 5 changed files with 87 additions and 56 deletions.
10 changes: 3 additions & 7 deletions lib/Doctrine/ORM/Proxy/ProxyFactory.php
View file
Expand Up @@ -221,9 +221,9 @@ private function _generateSleep(ClassMetadata $class)
$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) {
Expand Down Expand Up @@ -268,18 +268,14 @@ private function _load()
if ($this->_entityPersister->load($this->_identifier, $this) === null) {
throw new \Doctrine\ORM\EntityNotFoundException();
}
unset($this->_entityPersister);
unset($this->_identifier);
unset($this->_entityPersister, $this->_identifier);
}
}
<methods>
public function __sleep()
{
if (!$this->__isInitialized__) {
throw new \RuntimeException("Not fully loaded proxy can not be serialized.");
}
<sleepImpl>
}
}';
Expand Down
38 changes: 23 additions & 15 deletions lib/Doctrine/ORM/UnitOfWork.php
View file
Expand Up @@ -1349,11 +1349,13 @@ private function doMerge($entity, array &$visited, $prevManagedCopy = null, $ass
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;
Expand Down Expand Up @@ -1407,23 +1409,27 @@ private function doMerge($entity, array &$visited, $prevManagedCopy = null, $ass
} 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;
}
Expand Down Expand Up @@ -1451,7 +1457,6 @@ private function doMerge($entity, array &$visited, $prevManagedCopy = null, $ass
$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()) {
Expand All @@ -1460,6 +1465,9 @@ private function doMerge($entity, array &$visited, $prevManagedCopy = null, $ass
}
}

// Mark the managed copy visited as well
$visited[spl_object_hash($managedCopy)] = true;

$this->cascadeMerge($entity, $managedCopy, $visited);

return $managedCopy;
Expand Down
2 changes: 1 addition & 1 deletion tests/Doctrine/Tests/Models/CMS/CmsPhonenumber.php
View file
Expand Up @@ -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;
Expand Down
33 changes: 32 additions & 1 deletion tests/Doctrine/Tests/ORM/Functional/BasicFunctionalTest.php
View file
Expand Up @@ -788,7 +788,8 @@ public function testMergePersistsNewEntities()
$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()
Expand All @@ -803,4 +804,34 @@ public function testMergeThrowsExceptionIfEntityWithGeneratedIdentifierDoesNotEx
$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());
}
}
60 changes: 28 additions & 32 deletions tests/Doctrine/Tests/ORM/Functional/DetachedEntityTest.php
View file
Expand Up @@ -44,9 +44,10 @@ public function testSimpleDetachMerge() {
$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';
Expand All @@ -59,6 +60,7 @@ public function testSerializeUnserializeModifyMerge()
$this->_em->persist($user);
$this->_em->flush();
$this->assertTrue($this->_em->contains($user));
$this->assertTrue($user->phonenumbers->isInitialized());

$serialized = serialize($user);
$this->_em->clear();
Expand Down Expand Up @@ -103,42 +105,36 @@ public function testDetachedEntityThrowsExceptionOnFlush()
} 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';
$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);
$phone = new CmsPhonenumber();
$phone->phonenumber = '12345';
$user->name = 'Guilherme';
$user->username = 'gblanco';
$user->status = 'developer';

$user2 = $this->_em->merge($user);
$user2->addPhonenumber($phone);
$this->_em->persist($phone);
$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);

//$address->setUser($user2);
//$this->_em->persist($address);
$this->_em->flush();
$this->_em->clear();

$this->assertEquals(1,1);
}*/
$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__);

$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__);
}
}

0 comments on commit 69073c4

Please sign in to comment.