diff --git a/lib/Doctrine/ORM/PersistentCollection.php b/lib/Doctrine/ORM/PersistentCollection.php index 66dca2bbd..2d8a77ef6 100644 --- a/lib/Doctrine/ORM/PersistentCollection.php +++ b/lib/Doctrine/ORM/PersistentCollection.php @@ -305,6 +305,7 @@ final class PersistentCollection implements Collection if ($this->association !== null && $this->association['isOwningSide'] && $this->association['type'] === ClassMetadata::MANY_TO_MANY && + $this->owner && $this->em->getClassMetadata(get_class($this->owner))->isChangeTrackingNotify()) { $this->em->getUnitOfWork()->scheduleForDirtyCheck($this->owner); } @@ -759,4 +760,27 @@ final class PersistentCollection implements Collection return $this->coll->slice($offset, $length); } + + /** + * Cleanup internal state of cloned persistent collection. + * + * The following problems have to be prevented: + * 1. Added entities are added to old PC + * 2. New collection is not dirty, if reused on other entity nothing + * changes. + * 3. Snapshot leads to invalid diffs being generated. + * 4. Lazy loading grabs entities from old owner object. + * 5. New collection is connected to old owner and leads to duplicate keys. + */ + public function __clone() + { + $this->initialize(); + $this->owner = null; + + if (is_object($this->coll)) { + $this->coll = clone $this->coll; + } + $this->snapshot = array(); + $this->changed(); + } } diff --git a/lib/Doctrine/ORM/Persisters/AbstractCollectionPersister.php b/lib/Doctrine/ORM/Persisters/AbstractCollectionPersister.php index 7f8c40b09..8c129481e 100644 --- a/lib/Doctrine/ORM/Persisters/AbstractCollectionPersister.php +++ b/lib/Doctrine/ORM/Persisters/AbstractCollectionPersister.php @@ -204,4 +204,4 @@ abstract class AbstractCollectionPersister * @param mixed $element */ abstract protected function _getInsertRowSQLParameters(PersistentCollection $coll, $element); -} \ No newline at end of file +} diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index 2f555e6e1..7ab4046e6 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -584,6 +584,23 @@ class UnitOfWork implements PropertyChangedListener $assoc = $class->associationMappings[$propName]; + // Persistent collection was exchanged with the "originally" + // created one. This can only mean it was cloned and replaced + // on another entity. + if ($actualValue instanceof PersistentCollection) { + $owner = $actualValue->getOwner(); + if ($owner === null) { // cloned + $actualValue->setOwner($entity, $assoc); + } else if ($owner !== $entity) { // no clone, we have to fix + if (!$actualValue->isInitialized()) { + $actualValue->initialize(); // we have to do this otherwise the cols share state + } + $newValue = clone $actualValue; + $newValue->setOwner($entity, $assoc); + $class->reflFields[$propName]->setValue($entity, $newValue); + } + } + if ($orgValue instanceof PersistentCollection) { // A PersistentCollection was de-referenced, so delete it. $coid = spl_object_hash($orgValue); diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1643Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1643Test.php new file mode 100644 index 000000000..53d6bb6e2 --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1643Test.php @@ -0,0 +1,121 @@ +useModelSet('cms'); + parent::setUp(); + + $user1 = new CmsUser(); + $user1->username = "beberlei"; + $user1->name = "Benjamin"; + $user1->status = "active"; + $group1 = new CmsGroup(); + $group1->name = "test"; + $group2 = new CmsGroup(); + $group2->name = "test"; + $user1->addGroup($group1); + $user1->addGroup($group2); + $user2 = new CmsUser(); + $user2->username = "romanb"; + $user2->name = "Roman"; + $user2->status = "active"; + + $this->_em->persist($user1); + $this->_em->persist($user2); + $this->_em->persist($group1); + $this->_em->persist($group2); + $this->_em->flush(); + $this->_em->clear(); + + $this->user1 = $this->_em->find(get_class($user1), $user1->id); + $this->user2 = $this->_em->find(get_class($user1), $user2->id); + } + + public function testClonePersistentCollectionAndReuse() + { + $user1 = $this->user1; + + $user1->groups = clone $user1->groups; + + $this->_em->flush(); + $this->_em->clear(); + + $user1 = $this->_em->find(get_class($user1), $user1->id); + + $this->assertEquals(2, count($user1->groups)); + } + + public function testClonePersistentCollectionAndShare() + { + $user1 = $this->user1; + $user2 = $this->user2; + + $user2->groups = clone $user1->groups; + + $this->_em->flush(); + $this->_em->clear(); + + $user1 = $this->_em->find(get_class($user1), $user1->id); + $user2 = $this->_em->find(get_class($user1), $user2->id); + + $this->assertEquals(2, count($user1->groups)); + $this->assertEquals(2, count($user2->groups)); + } + + public function testCloneThenDirtyPersistentCollection() + { + $user1 = $this->user1; + $user2 = $this->user2; + + $group3 = new CmsGroup(); + $group3->name = "test"; + $user2->groups = clone $user1->groups; + $user2->groups->add($group3); + + $this->_em->persist($group3); + $this->_em->flush(); + $this->_em->clear(); + + $user1 = $this->_em->find(get_class($user1), $user1->id); + $user2 = $this->_em->find(get_class($user1), $user2->id); + + $this->assertEquals(3, count($user2->groups)); + $this->assertEquals(2, count($user1->groups)); + } + + public function testNotCloneAndPassAroundFlush() + { + $user1 = $this->user1; + $user2 = $this->user2; + + $group3 = new CmsGroup(); + $group3->name = "test"; + $user2->groups = $user1->groups; + $user2->groups->add($group3); + + $this->assertEQuals(1, count($user1->groups->getInsertDiff())); + + $this->_em->persist($group3); + $this->_em->flush(); + $this->_em->clear(); + + $user1 = $this->_em->find(get_class($user1), $user1->id); + $user2 = $this->_em->find(get_class($user1), $user2->id); + + $this->assertEquals(3, count($user2->groups)); + $this->assertEquals(3, count($user1->groups)); + } +} +