Skip to content

Instantly share code, notes, and snippets.

@doctrinebot
Created December 13, 2015 18:49
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save doctrinebot/d1552a3ec4491ad1c3e6 to your computer and use it in GitHub Desktop.
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
<?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();
}
}
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
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