Merge pull request #1259 from malarzm/reuse-embeds-v2

Set parentAssociation on persist and fix embeds by then

Author: malarzm

lib/Doctrine/ODM/MongoDB/UnitOfWork.php

@@ -2332,7 +2332,6 @@ function ($assoc) { return $assoc['isCascadeMerge']; }
      *
      * @param object $document
      * @param array $visited
-     * @param array $insertNow
      */
     private function cascadePersist($document, array &$visited)
     {
@@ -2348,14 +2347,35 @@ function ($assoc) { return $assoc['isCascadePersist']; }
 
             if ($relatedDocuments instanceof Collection || is_array($relatedDocuments)) {
                 if ($relatedDocuments instanceof PersistentCollection) {
+                    if ($relatedDocuments->getOwner() !== $document) {
+                        $relatedDocuments = $this->fixPersistentCollectionOwnership($relatedDocuments, $document, $class, $mapping['name']);
+                    }
                     // Unwrap so that foreach() does not initialize
                     $relatedDocuments = $relatedDocuments->unwrap();
                 }
 
-                foreach ($relatedDocuments as $relatedDocument) {
+                $count = 0;
+                foreach ($relatedDocuments as $relatedKey => $relatedDocument) {
+                    if ( ! empty($mapping['embedded'])) {
+                        list(, $knownParent, ) = $this->getParentAssociation($relatedDocument);
+                        if ($knownParent && $knownParent !== $document) {
+                            $relatedDocument = clone $relatedDocument;
+                            $relatedDocuments[$relatedKey] = $relatedDocument;
+                        }
+                        $pathKey = ! isset($mapping['strategy']) || CollectionHelper::isList($mapping['strategy']) ? $count++ : $relatedKey;
+                        $this->setParentAssociation($relatedDocument, $mapping, $document, $mapping['name'] . '.' . $pathKey);
+                    }
                     $this->doPersist($relatedDocument, $visited);
                 }
             } elseif ($relatedDocuments !== null) {
+                if ( ! empty($mapping['embedded'])) {
+                    list(, $knownParent, ) = $this->getParentAssociation($relatedDocuments);
+                    if ($knownParent && $knownParent !== $document) {
+                        $relatedDocuments = clone $relatedDocuments;
+                        $class->setFieldValue($document, $mapping['name'], $relatedDocuments);
+                    }
+                    $this->setParentAssociation($relatedDocuments, $mapping, $document, $mapping['name']);
+                }
                 $this->doPersist($relatedDocuments, $visited);
             }
         }
@@ -2507,7 +2527,20 @@ public function unscheduleOrphanRemoval($document)
         }
     }
 
-    private function fixPersistentCollectionOwnership(PersistentCollection $coll, $document, $class, $propName)
+    /**
+     * Fixes PersistentCollection state if it wasn't used exactly as we had in mind:
+     *  1) sets owner if it was cloned
+     *  2) clones collection, sets owner, updates document's property and, if necessary, updates originalData
+     *  3) NOP if state is OK
+     * Returned collection should be used from now on (only important with 2nd point)
+     *
+     * @param PersistentCollection $coll
+     * @param object $document
+     * @param ClassMetadata $class
+     * @param string $propName
+     * @return PersistentCollection
+     */
+    private function fixPersistentCollectionOwnership(PersistentCollection $coll, $document, ClassMetadata $class, $propName)
     {
         $owner = $coll->getOwner();
         if ($owner === null) { // cloned
@@ -2519,8 +2552,10 @@ private function fixPersistentCollectionOwnership(PersistentCollection $coll, $d
             $newValue = clone $coll;
             $newValue->setOwner($document, $class->fieldMappings[$propName]);
             $class->reflFields[$propName]->setValue($document, $newValue);
-            // @todo following line should be superfluous once collections are stored in change sets
-            $this->setOriginalDocumentProperty(spl_object_hash($document), $propName, $newValue);
+            if ($this->isScheduledForUpdate($document)) {
+                // @todo following line should be superfluous once collections are stored in change sets
+                $this->setOriginalDocumentProperty(spl_object_hash($document), $propName, $newValue);
+            }
             return $newValue;
         }
         return $coll;

tests/Doctrine/ODM/MongoDB/Tests/Functional/EmbeddedTest.php

@@ -2,6 +2,7 @@
 
 namespace Doctrine\ODM\MongoDB\Tests\Functional;
 
+use Doctrine\Common\Collections\ArrayCollection;
 use Documents\Address;
 use Documents\Profile;
 use Documents\Phonenumber;
@@ -539,8 +540,12 @@ public function testWhenCopyingManyEmbedSubDocumentsFromOneDocumentToAnotherWill
         $test2 = new ChangeEmbeddedIdTest();
         $test2->embedMany = $test1->embedMany; //using clone will work
         $this->dm->persist($test2);
+
+        $this->assertNotSame($test1->embedMany->first(), $test2->embedMany->first());
+
         $this->dm->flush();
 
+
         //do some operations on test1
         $this->dm->persist($test1);
         $this->dm->flush();
@@ -564,6 +569,9 @@ public function testReusedEmbeddedDocumentsAreClonedInFact()
 
         $this->dm->persist($test1);
         $this->dm->persist($test2);
+
+        $this->assertNotSame($test1->embed, $test2->embed);
+
         $this->dm->flush();
 
         $this->assertNotSame($test1->embed, $test2->embed);
@@ -593,7 +601,12 @@ class ChangeEmbeddedIdTest
     /**
      * @ODM\EmbedMany(targetDocument="EmbeddedDocumentWithId")
      */
-    public $embedMany = array();
+    public $embedMany;
+
+    public function __construct()
+    {
+        $this->embedMany = new ArrayCollection();
+    }
 }
 
 /**