Merge pull request #1550 from malarzm/gh1525-yet-again

Fix more re-used embedded docs issues
doctrine · Jan 10, 2017 · 758c29b · 758c29b
2 parents 26c3e2e + 408faeb
commit 758c29b
Show file tree

Hide file tree

Showing 2 changed files with 78 additions and 11 deletions.
diff --git a/lib/Doctrine/ODM/MongoDB/UnitOfWork.php b/lib/Doctrine/ODM/MongoDB/UnitOfWork.php
@@ -745,16 +745,6 @@ private function computeOrRecomputeChangeSet(ClassMetadata $class, $document, $r
                     if ($orgValue !== null) {
                         $this->scheduleOrphanRemoval($orgValue);
                     }
-
-                    if ($actualValue !== null) {
-                        list(, $knownParent, ) = $this->getParentAssociation($actualValue);
-                        if ($knownParent && $knownParent !== $document) {
-                            $actualValue = clone $actualValue;
-                            $class->setFieldValue($document, $class->fieldMappings[$propName]['fieldName'], $actualValue);
-                            $this->setOriginalDocumentProperty(spl_object_hash($document), $class->fieldMappings[$propName]['fieldName'], $actualValue);
-                        }
-                    }
-
                     $changeSet[$propName] = array($orgValue, $actualValue);
                     continue;
                 }
@@ -998,6 +988,10 @@ private function computeAssociationChanges($parentDocument, array $assoc, $value
                             if ($assoc['type'] === ClassMetadata::ONE) {
                                 $class->setFieldValue($parentDocument, $assoc['fieldName'], $entry);
                                 $this->setOriginalDocumentProperty(spl_object_hash($parentDocument), $assoc['fieldName'], $entry);
+                                $poid = spl_object_hash($parentDocument);
+                                if (isset($this->documentChangeSets[$poid][$assoc['fieldName']])) {
+                                    $this->documentChangeSets[$poid][$assoc['fieldName']][1] = $entry;
+                                }
                             } else {
                                 // must use unwrapped value to not trigger orphan removal
                                 $unwrappedValue[$key] = $entry;
@@ -2193,6 +2187,11 @@ function ($assoc) { return $assoc['isCascadePersist']; }
                 }
             } elseif ($relatedDocuments !== null) {
                 if ( ! empty($mapping['embedded'])) {
+                    list(, $knownParent, ) = $this->getParentAssociation($relatedDocuments);
+                    if ($knownParent && $knownParent !== $document) {
+                        $relatedDocuments = clone $relatedDocuments;
+                        $class->setFieldValue($document, $mapping['fieldName'], $relatedDocuments);
+                    }
                     $this->setParentAssociation($relatedDocuments, $mapping, $document, $mapping['fieldName']);
                 }
                 $this->doPersist($relatedDocuments, $visited);

diff --git a/tests/Doctrine/ODM/MongoDB/Tests/Functional/Ticket/GH1525Test.php b/tests/Doctrine/ODM/MongoDB/Tests/Functional/Ticket/GH1525Test.php
@@ -7,7 +7,7 @@
 
 class GH1525Test extends \Doctrine\ODM\MongoDB\Tests\BaseTest
 {
-    public function testEmbedClone()
+    public function testEmbedCloneTwoFlushesPerDocument()
     {
         $embedded = new GH1525Embedded('embedded');
 
@@ -38,6 +38,56 @@ public function testEmbedClone()
             $this->assertSame($test->embedMany[0]->name, $embedMany->name);
         }
     }
+
+    public function testEmbedCloneWithIdStrategyNoneOnParentAndEarlyPersist()
+    {
+        $uuidGen = new \Doctrine\ODM\MongoDB\Id\UuidGenerator();
+        $embedded = new GH1525Embedded('embedded');
+
+        $count = 2;
+        for ($i = 0; $i < $count; ++$i) {
+            $parent = new GH1525DocumentIdStrategyNone($uuidGen->generateV4(), 'test' . $i);
+            $this->dm->persist($parent);
+            $parent->embedded = $embedded;
+            $this->dm->flush();
+        }
+
+        $this->dm->clear();
+
+        for ($i = 0; $i < $count; ++$i) {
+            $test = $this->dm->getRepository(GH1525DocumentIdStrategyNone::class)->findOneBy(array('name' => 'test' . $i));
+
+            $this->assertInstanceOf(GH1525DocumentIdStrategyNone::class, $test);
+
+            $this->assertInstanceOf(GH1525Embedded::class, $test->embedded);
+            $this->assertSame($test->embedded->name, $embedded->name);
+        }
+    }
+
+    public function testEmbedCloneWithIdStrategyNoneOnParentAndLatePersist()
+    {
+        $uuidGen = new \Doctrine\ODM\MongoDB\Id\UuidGenerator();
+        $embedded = new GH1525Embedded('embedded');
+
+        $count = 2;
+        for ($i = 0; $i < $count; ++$i) {
+            $parent = new GH1525DocumentIdStrategyNone($uuidGen->generateV4(), 'test' . $i);
+            $parent->embedded = $embedded;
+            $this->dm->persist($parent);
+            $this->dm->flush();
+        }
+
+        $this->dm->clear();
+
+        for ($i = 0; $i < $count; ++$i) {
+            $test = $this->dm->getRepository(GH1525DocumentIdStrategyNone::class)->findOneBy(array('name' => 'test' . $i));
+
+            $this->assertInstanceOf(GH1525DocumentIdStrategyNone::class, $test);
+
+            $this->assertInstanceOf(GH1525Embedded::class, $test->embedded);
+            $this->assertSame($test->embedded->name, $embedded->name);
+        }
+    }
 }
 
 /** @ODM\Document(collection="document_test") */
@@ -62,6 +112,24 @@ public function __construct($name)
     }
 }
 
+/** @ODM\Document(collection="document_test_with_auto_ids") */
+class GH1525DocumentIdStrategyNone
+{
+    /** @ODM\Id(strategy="NONE") */
+    public $id;
+
+    /** @ODM\Field(type="string") */
+    public $name;
+
+    /** @ODM\EmbedOne(targetDocument="GH1525Embedded") */
+    public $embedded;
+
+    public function __construct($id, $name)
+    {
+        $this->id = $id;
+        $this->name = $name;
+    }
+}
 
 /** @ODM\EmbeddedDocument */
 class GH1525Embedded