[DDC-1643] Fix bugs when cloning PersistentCollection and re-using it.

doctrine · Feb 17, 2012 · 647bd2b · 647bd2b
1 parent dadc85a
commit 647bd2b
Show file tree

Hide file tree

Showing 4 changed files with 163 additions and 1 deletion.
diff --git a/lib/Doctrine/ORM/PersistentCollection.php b/lib/Doctrine/ORM/PersistentCollection.php
@@ -305,6 +305,7 @@ private function changed()
         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 @@ public function slice($offset, $length = null)
 
         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
@@ -204,4 +204,4 @@ abstract protected function _getInsertRowSQL(PersistentCollection $coll);
      * @param mixed $element
      */
     abstract protected function _getInsertRowSQLParameters(PersistentCollection $coll, $element);
-}
+}
diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php
@@ -584,6 +584,23 @@ public function computeChangeSet(ClassMetadata $class, $entity)
 
                 $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
@@ -0,0 +1,121 @@
+<?php
+namespace Doctrine\Tests\ORM\Functional\Ticket;
+
+use Doctrine\Tests\Models\CMS\CmsUser;
+use Doctrine\Tests\Models\CMS\CmsGroup;
+
+/**
+ * @group DDC-1643
+ */
+class DDC1643Test extends \Doctrine\Tests\OrmFunctionalTestCase
+{
+    private $user1;
+    private $user2;
+
+    public function setUp()
+    {
+        $this->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));
+    }
+}
+