-
-
Save doctrinebot/d1552a3ec4491ad1c3e6 to your computer and use it in GitHub Desktop.
Attachments to Doctrine Jira Issue DDC-763 - https://github.com/doctrine/doctrine2/issues/5277
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
<?php | |
namespace Doctrine\Tests\ORM\Functional\Ticket; | |
use Doctrine\Common\Collections\ArrayCollection; | |
use Doctrine\Tests\Models\CMS\CmsUser; | |
use Doctrine\Tests\Models\CMS\CmsPhonenumber; | |
use Doctrine\Tests\Models\CMS\CmsGroup; | |
require_once __DIR__ . '/../../../TestInit.php'; | |
class DDC763Test extends \Doctrine\Tests\OrmFunctionalTestCase { | |
public function setUp() { | |
$this->useModelSet("cms"); | |
parent::setUp(); | |
} | |
/** | |
* Helper method to set cascade to merge only | |
*/ | |
private function setCascadeMergeFor($class) { | |
$metadata = $this->_em->getMetadataFactory()->getMetaDataFor($class); | |
foreach ($metadata->associationMappings as $key => $associationMapping) { | |
$metadata->associationMappings[$key]["isCascadePersist"] = false; | |
$metadata->associationMappings[$key]["isCascadeMerge"] = true; | |
$metadata->associationMappings[$key]["isCascadeRemove"] = false; | |
$metadata->associationMappings[$key]["isCascadeDetach"] = false; | |
} | |
} | |
/** | |
* Test that merging the same new entity twice doesn't add two copies (DDC-763) | |
*/ | |
public function testMultiMerge() { | |
$cmsUser = new CmsUser(); | |
$cmsUser->username = "dave"; | |
$cmsUser->name = "Dave Keen"; | |
$cmsUser->status = "testing"; | |
// This should only persist a single user | |
$this->_em->merge($cmsUser); | |
$this->_em->merge($cmsUser); | |
$this->_em->flush(); | |
// After this there should only be a single user | |
$cmsUsers = $this->_em->getRepository('Doctrine\Tests\Models\CMS\CmsUser')->findAll(); | |
$this->assertEquals(1, sizeof($cmsUsers)); | |
} | |
/** | |
* Test the more complex use case | |
*/ | |
public function testMultiCascadeMerge() { | |
$this->setCascadeMergeFor('Doctrine\Tests\Models\CMS\CmsUser'); | |
$this->setCascadeMergeFor('Doctrine\Tests\Models\CMS\CmsPhonenumber'); | |
$cmsUser = new CmsUser(); | |
$cmsUser->username = "dave"; | |
$cmsUser->name = "Dave Keen"; | |
$cmsUser->status = "testing"; | |
$mobilePhonenumber = new CmsPhonenumber(); | |
$mobilePhonenumber->phonenumber = "0123456789"; | |
$homePhonenumber = new CmsPhonenumber(); | |
$homePhonenumber->phonenumber = "9876543210"; | |
$cmsUser->phonenumbers->add($mobilePhonenumber); $mobilePhonenumber->user = $cmsUser; | |
$cmsUser->phonenumbers->add($homePhonenumber); $homePhonenumber->user = $cmsUser; | |
$this->_em->merge($cmsUser); | |
$this->_em->merge($mobilePhonenumber); | |
$this->_em->merge($homePhonenumber); | |
$this->_em->flush(); | |
// After this there should be one CmsUser and two CmsPhonenumbers | |
$cmsUsers = $this->_em->getRepository('Doctrine\Tests\Models\CMS\CmsUser')->findAll(); | |
$this->assertEquals(1, sizeof($cmsUsers)); | |
$phonenumbers = $this->_em->getRepository('Doctrine\Tests\Models\CMS\CmsPhonenumber')->findAll(); | |
$this->assertEquals(2, sizeof($phonenumbers)); | |
// Check the ManyToOne side of the association | |
$this->assertSame($phonenumbers[0]->user, $cmsUsers[0]); | |
$this->assertSame($phonenumbers[1]->user, $cmsUsers[0]); | |
// Check the OneToMany side of the association | |
$this->assertSame($cmsUsers[0]->phonenumbers[0], $phonenumbers[0]); | |
$this->assertSame($cmsUsers[0]->phonenumbers[1], $phonenumbers[1]); | |
} | |
/** | |
* Test that creating new entities with many to many associations write the correct number of entities and associations to the database. | |
*/ | |
public function testManyToManyPersistByReachability() { | |
$this->setCascadeMergeFor('Doctrine\Tests\Models\CMS\CmsUser'); | |
$this->setCascadeMergeFor('Doctrine\Tests\Models\CMS\CmsGroup'); | |
// Put entities in the database | |
$cmsUser1 = new CmsUser(); | |
$cmsUser1->username = "user1"; | |
$cmsUser1->name = "User 1"; | |
$cmsUser1->status = "testing"; | |
$cmsUser2 = new CmsUser(); | |
$cmsUser2->username = "user2"; | |
$cmsUser2->name = "User 1"; | |
$cmsUser2->status = "testing"; | |
$group1 = new CmsGroup(); | |
$group1->name = "Group 1"; | |
$group1->users = new ArrayCollection(); | |
$group2 = new CmsGroup(); | |
$group2->name = "Group 2"; | |
$group2->users = new ArrayCollection(); | |
$cmsUser1->addGroup($group1); | |
$cmsUser1->addGroup($group2); | |
$cmsUser2->addGroup($group1); | |
$cmsUser2->addGroup($group2); | |
$this->_em->merge($cmsUser1); | |
$this->_em->merge($cmsUser2); | |
$this->_em->flush(); | |
$this->_em->clear(); | |
$cmsUsers = $this->_em->getRepository('Doctrine\Tests\Models\CMS\CmsUser')->findAll(); | |
$cmsGroups = $this->_em->getRepository('Doctrine\Tests\Models\CMS\CmsGroup')->findAll(); | |
// Check the entities are in the database | |
$this->assertEquals(2, sizeof($cmsUsers)); | |
$this->assertEquals(2, sizeof($cmsGroups)); | |
// Check the associations between the entities are now in the database | |
$this->assertEquals(2, sizeof($cmsUsers[0]->groups)); | |
$this->assertEquals(2, sizeof($cmsUsers[1]->groups)); | |
$this->assertEquals(2, sizeof($cmsGroups[0]->users)); | |
$this->assertEquals(2, sizeof($cmsGroups[1]->users)); | |
$this->assertSame($cmsUsers[0]->groups[0], $cmsGroups[0]); | |
$this->assertSame($cmsUsers[0]->groups[1], $cmsGroups[1]); | |
$this->assertSame($cmsUsers[1]->groups[0], $cmsGroups[0]); | |
$this->assertSame($cmsUsers[1]->groups[1], $cmsGroups[1]); | |
$this->assertSame($cmsGroups[0]->users[0], $cmsUsers[0]); | |
$this->assertSame($cmsGroups[0]->users[1], $cmsUsers[1]); | |
$this->assertSame($cmsGroups[1]->users[0], $cmsUsers[0]); | |
$this->assertSame($cmsGroups[1]->users[1], $cmsUsers[1]); | |
} | |
/** | |
* Test that creating new entities with many to many associations and merging the same entities multiple times from different ends of the | |
* many to many association doesn't cause duplicate rows to be added to the database causing an integrity constraint violation. | |
*/ | |
public function testManyToManyDuplicatePersistByReachability() { | |
$this->setCascadeMergeFor('Doctrine\Tests\Models\CMS\CmsUser'); | |
$this->setCascadeMergeFor('Doctrine\Tests\Models\CMS\CmsGroup'); | |
// Put entities in the database | |
$cmsUser1 = new CmsUser(); | |
$cmsUser1->username = "user1"; | |
$cmsUser1->name = "User 1"; | |
$cmsUser1->status = "testing"; | |
$cmsUser2 = new CmsUser(); | |
$cmsUser2->username = "user2"; | |
$cmsUser2->name = "User 1"; | |
$cmsUser2->status = "testing"; | |
$group1 = new CmsGroup(); | |
$group1->name = "Group 1"; | |
$group1->users = new ArrayCollection(); | |
$group2 = new CmsGroup(); | |
$group2->name = "Group 2"; | |
$group2->users = new ArrayCollection(); | |
$cmsUser1->addGroup($group1); | |
$cmsUser1->addGroup($group2); | |
$cmsUser2->addGroup($group1); | |
$cmsUser2->addGroup($group2); | |
$this->_em->merge($cmsUser1); | |
$this->_em->merge($cmsUser2); | |
$this->_em->merge($group1); | |
$this->_em->merge($group2); | |
$this->_em->flush(); | |
} | |
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
From 9d471de22d2806f758aa11c76e6fe4b81ef16c05 Mon Sep 17 00:00:00 2001 | |
From: Dave Keen <dev@ruffness.com> | |
Date: Wed, 22 Sep 2010 02:12:47 +0200 | |
Subject: [PATCH 149/149] DDC-763 | |
--- | |
lib/Doctrine/ORM/UnitOfWork.php | 207 +++++++++++++++++++++----------------- | |
1 files changed, 114 insertions(+), 93 deletions(-) | |
diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php | |
index a051785..697e68e 100644 | |
--- a/lib/Doctrine/ORM/UnitOfWork.php | |
+++ b/lib/Doctrine/ORM/UnitOfWork.php | |
@@ -215,7 +215,14 @@ class UnitOfWork implements PropertyChangedListener | |
* @var array | |
*/ | |
private $orphanRemovals = array(); | |
- | |
+ | |
+ /** | |
+ * Entities that have been already merged | |
+ * | |
+ * @var array | |
+ */ | |
+ private $mergedEntities = array(); | |
+ | |
//private $_readOnlyObjects = array(); | |
/** | |
@@ -1336,118 +1343,124 @@ class UnitOfWork implements PropertyChangedListener | |
private function doMerge($entity, array &$visited, $prevManagedCopy = null, $assoc = null) | |
{ | |
$oid = spl_object_hash($entity); | |
+ | |
if (isset($visited[$oid])) { | |
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 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; | |
+ // If we have already merged this entity then get the managed copy from the merged entities | |
+ if (isset($this->mergedEntities[$oid])) { | |
+ $managedCopy = $this->mergedEntities[$oid]; | |
} else { | |
- // Try to look the entity up in the identity map. | |
- $id = $class->getIdentifierValues($entity); | |
- | |
- // If there is no ID, it is actually NEW. | |
- if ( ! $id) { | |
- $managedCopy = $class->newInstance(); | |
- $this->persistNew($class, $managedCopy); | |
+ // First we assume DETACHED, although it can still be NEW but we can avoid | |
+ // 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; | |
} else { | |
- $managedCopy = $this->tryGetById($id, $class->rootEntityName); | |
- if ($managedCopy) { | |
- // We have the entity in-memory already, just make sure its not removed. | |
- if ($this->getEntityState($managedCopy) == self::STATE_REMOVED) { | |
- throw new InvalidArgumentException('Removed entity detected during merge.' | |
- . ' Can not merge with a removed entity.'); | |
- } | |
- } else { | |
- // We need to fetch the managed copy in order to merge. | |
- $managedCopy = $this->em->find($class->name, $id); | |
- } | |
+ // Try to look the entity up in the identity map. | |
+ $id = $class->getIdentifierValues($entity); | |
- if ($managedCopy === null) { | |
- // If the identifier is ASSIGNED, it is NEW, otherwise an error | |
- // since the managed entity was not found. | |
- if ($class->isIdentifierNatural()) { | |
- $managedCopy = $class->newInstance(); | |
- $class->setIdentifierValues($managedCopy, $id); | |
- $this->persistNew($class, $managedCopy); | |
+ // If there is no ID, it is actually NEW. | |
+ if ( ! $id ) { | |
+ $managedCopy = $class->newInstance(); | |
+ $this->persistNew($class, $managedCopy); | |
+ } else { | |
+ $managedCopy = $this->tryGetById($id, $class->rootEntityName); | |
+ if ($managedCopy) { | |
+ // We have the entity in-memory already, just make sure its not removed. | |
+ if ($this->getEntityState($managedCopy) == self::STATE_REMOVED) { | |
+ throw new InvalidArgumentException('Removed entity detected during merge.' | |
+ . ' Can not merge with a removed entity.'); | |
+ } | |
} else { | |
- throw new EntityNotFoundException; | |
+ // We need to fetch the managed copy in order to merge. | |
+ $managedCopy = $this->em->find($class->name, $id); | |
} | |
- } | |
- } | |
- if ($class->isVersioned) { | |
- $managedCopyVersion = $class->reflFields[$class->versionField]->getValue($managedCopy); | |
- $entityVersion = $class->reflFields[$class->versionField]->getValue($entity); | |
- // Throw exception if versions dont match. | |
- if ($managedCopyVersion != $entityVersion) { | |
- throw OptimisticLockException::lockFailedVersionMissmatch($entityVersion, $managedCopyVersion); | |
+ if ($managedCopy === null) { | |
+ // If the identifier is ASSIGNED, it is NEW, otherwise an error | |
+ // since the managed entity was not found. | |
+ if ($class->isIdentifierNatural()) { | |
+ $managedCopy = $class->newInstance(); | |
+ $class->setIdentifierValues($managedCopy, $id); | |
+ $this->persistNew($class, $managedCopy); | |
+ } else { | |
+ throw new EntityNotFoundException; | |
+ } | |
+ } | |
} | |
- } | |
- // Merge state of $entity into existing (managed) entity | |
- foreach ($class->reflFields as $name => $prop) { | |
- if ( ! isset($class->associationMappings[$name])) { | |
- if ( ! $class->isIdentifier($name)) { | |
- $prop->setValue($managedCopy, $prop->getValue($entity)); | |
+ if ($class->isVersioned) { | |
+ $managedCopyVersion = $class->reflFields[$class->versionField]->getValue($managedCopy); | |
+ $entityVersion = $class->reflFields[$class->versionField]->getValue($entity); | |
+ // Throw exception if versions dont match. | |
+ if ($managedCopyVersion != $entityVersion) { | |
+ throw OptimisticLockException::lockFailedVersionMissmatch($entityVersion, $managedCopyVersion); | |
} | |
- } else { | |
- $assoc2 = $class->associationMappings[$name]; | |
- if ($assoc2['type'] & ClassMetadata::TO_ONE) { | |
- $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['targetEntity']); | |
- $id = $targetClass->getIdentifierValues($other); | |
- $proxy = $this->em->getProxyFactory()->getProxy($assoc2['targetEntity'], $id); | |
- $prop->setValue($managedCopy, $proxy); | |
- $this->registerManaged($proxy, $id, array()); | |
- } | |
+ } | |
+ | |
+ // Merge state of $entity into existing (managed) entity | |
+ foreach ($class->reflFields as $name => $prop) { | |
+ if ( ! isset($class->associationMappings[$name])) { | |
+ if ( ! $class->isIdentifier($name)) { | |
+ $prop->setValue($managedCopy, $prop->getValue($entity)); | |
} | |
} 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; | |
- } | |
+ $assoc2 = $class->associationMappings[$name]; | |
+ if ($assoc2['type'] & ClassMetadata::TO_ONE) { | |
+ $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['targetEntity']); | |
+ $id = $targetClass->getIdentifierValues($other); | |
+ $proxy = $this->em->getProxyFactory()->getProxy($assoc2['targetEntity'], $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; | |
+ } | |
- $managedCol = $prop->getValue($managedCopy); | |
- if (!$managedCol) { | |
- $managedCol = new PersistentCollection($this->em, | |
- $this->em->getClassMetadata($assoc2['targetEntity']), | |
- new ArrayCollection | |
- ); | |
- $managedCol->setOwner($managedCopy, $assoc2); | |
- $prop->setValue($managedCopy, $managedCol); | |
- $this->originalEntityData[$oid][$name] = $managedCol; | |
+ $managedCol = $prop->getValue($managedCopy); | |
+ if (!$managedCol) { | |
+ $managedCol = new PersistentCollection($this->em, | |
+ $this->em->getClassMetadata($assoc2['targetEntity']), | |
+ new ArrayCollection | |
+ ); | |
+ $managedCol->setOwner($managedCopy, $assoc2); | |
+ $prop->setValue($managedCopy, $managedCol); | |
+ $this->originalEntityData[$oid][$name] = $managedCol; | |
+ } | |
+ $managedCol->setInitialized($assoc2['isCascadeMerge']); | |
} | |
- $managedCol->setInitialized($assoc2['isCascadeMerge']); | |
+ } | |
+ if ($class->isChangeTrackingNotify()) { | |
+ // Just treat all properties as changed, there is no other choice. | |
+ $this->propertyChanged($managedCopy, $name, null, $prop->getValue($managedCopy)); | |
} | |
} | |
- if ($class->isChangeTrackingNotify()) { | |
- // Just treat all properties as changed, there is no other choice. | |
- $this->propertyChanged($managedCopy, $name, null, $prop->getValue($managedCopy)); | |
+ if ($class->isChangeTrackingDeferredExplicit()) { | |
+ $this->scheduleForDirtyCheck($entity); | |
} | |
} | |
- if ($class->isChangeTrackingDeferredExplicit()) { | |
- $this->scheduleForDirtyCheck($entity); | |
- } | |
} | |
if ($prevManagedCopy !== null) { | |
@@ -1456,7 +1469,9 @@ class UnitOfWork implements PropertyChangedListener | |
if ($assoc['type'] & ClassMetadata::TO_ONE) { | |
$prevClass->reflFields[$assocField]->setValue($prevManagedCopy, $managedCopy); | |
} else { | |
- $prevClass->reflFields[$assocField]->getValue($prevManagedCopy)->unwrap()->add($managedCopy); | |
+ // Only add the managed copy to the array collection if it isn't already there | |
+ $arrayCollection = $prevClass->reflFields[$assocField]->getValue($prevManagedCopy)->unwrap(); | |
+ if (!$arrayCollection->contains($managedCopy)) $arrayCollection->add($managedCopy); | |
if ($assoc['type'] == ClassMetadata::ONE_TO_MANY) { | |
$class->reflFields[$assoc['mappedBy']]->setValue($managedCopy, $prevManagedCopy); | |
} | |
@@ -1468,6 +1483,9 @@ class UnitOfWork implements PropertyChangedListener | |
$this->cascadeMerge($entity, $managedCopy, $visited); | |
+ // Mark the managed copy merged | |
+ $this->mergedEntities[$oid] = $managedCopy; | |
+ | |
return $managedCopy; | |
} | |
@@ -1635,8 +1653,10 @@ class UnitOfWork implements PropertyChangedListener | |
$this->doMerge($relatedEntity, $visited, $managedCopy, $assoc); | |
} | |
// Now we want to initialize $snapshot to whatever was originally in the database | |
- $managedCollection = $class->reflFields[$assoc['fieldName']]->getValue($managedCopy); | |
- $managedCollection->takeSnapshotFromDatabase(); | |
+ if ($assoc['isOwningSide']) { | |
+ $managedCollection = $class->reflFields[$assoc['fieldName']]->getValue($managedCopy); | |
+ $managedCollection->takeSnapshotFromDatabase(); | |
+ } | |
} else if ($relatedEntities !== null) { | |
$this->doMerge($relatedEntities, $visited, $managedCopy, $assoc); | |
} | |
@@ -1770,7 +1790,8 @@ class UnitOfWork implements PropertyChangedListener | |
$this->collectionDeletions = | |
$this->collectionUpdates = | |
$this->extraUpdates = | |
- $this->orphanRemovals = array(); | |
+ $this->orphanRemovals = | |
+ $this->mergedEntities = array(); | |
if ($this->commitOrderCalculator !== null) { | |
$this->commitOrderCalculator->clear(); | |
} | |
-- | |
1.6.5.1.1367.gcd48 | |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
diff --git "a/C:\\DOCUME~1\\ADMINI~1\\LOCALS~1\\Temp\\UnitOfWork_HEAD.php" "b/D:\\Projects\\ORM\\doctrine2\\lib\\Doctrine\\ORM\\UnitOfWork.php" | |
index 5088bb2..f39e511 100644 | |
--- "a/C:\\DOCUME~1\\ADMINI~1\\LOCALS~1\\Temp\\UnitOfWork_HEAD.php" | |
+++ "b/D:\\Projects\\ORM\\doctrine2\\lib\\Doctrine\\ORM\\UnitOfWork.php" | |
@@ -1465,7 +1465,11 @@ class UnitOfWork implements PropertyChangedListener | |
if ($assoc['type'] & ClassMetadata::TO_ONE) { | |
$prevClass->reflFields[$assocField]->setValue($prevManagedCopy, $managedCopy); | |
} else { | |
- $prevClass->reflFields[$assocField]->getValue($prevManagedCopy)->add($managedCopy); | |
+ //$prevClass->reflFields[$assocField]->getValue($prevManagedCopy)->add($managedCopy); | |
+ $arrayCollection = $prevClass->reflFields[$assocField]->getValue($prevManagedCopy)->unwrap(); | |
+ if (!$arrayCollection->contains($managedCopy)) | |
+ $arrayCollection->add($managedCopy); | |
+ | |
if ($assoc['type'] == ClassMetadata::ONE_TO_MANY) { | |
$class->reflFields[$assoc['mappedBy']]->setValue($managedCopy, $prevManagedCopy); | |
} |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment