Merge branch 'hotfix/#1173-merge-association-to-identical-entities'

Close #1173

lib/Doctrine/ORM/UnitOfWork.php

@@ -1782,10 +1782,14 @@ class UnitOfWork implements PropertyChangedListener
         $oid = spl_object_hash($entity);
 
         if (isset($visited[$oid])) {
-            return $visited[$oid]; // Prevent infinite recursion
+            $managedCopy = $visited[$oid];
-        }
 
-        $visited[$oid] = $entity; // mark visited
+            if ($prevManagedCopy !== null) {
+                $this->updateAssociationWithMergedEntity($entity, $assoc, $prevManagedCopy, $managedCopy);
+            }
+
+            return $managedCopy;
+        }
 
         $class = $this->em->getClassMetadata(get_class($entity));
 
@@ -1855,6 +1859,8 @@ class UnitOfWork implements PropertyChangedListener
                 }
             }
 
+            $visited[$oid] = $managedCopy; // mark visited
+
             // Merge state of $entity into existing (managed) entity
             foreach ($class->reflClass->getProperties() as $prop) {
                 $name = $prop->name;
@@ -1898,9 +1904,9 @@ class UnitOfWork implements PropertyChangedListener
                         $managedCol = $prop->getValue($managedCopy);
                         if (!$managedCol) {
                             $managedCol = new PersistentCollection($this->em,
-                                    $this->em->getClassMetadata($assoc2['targetEntity']),
+                                $this->em->getClassMetadata($assoc2['targetEntity']),
-                                    new ArrayCollection
+                                new ArrayCollection
-                                    );
+                            );
                             $managedCol->setOwner($managedCopy, $assoc2);
                             $prop->setValue($managedCopy, $managedCol);
                             $this->originalEntityData[$oid][$name] = $managedCol;
@@ -1933,28 +1939,48 @@ class UnitOfWork implements PropertyChangedListener
         }
 
         if ($prevManagedCopy !== null) {
-            $assocField = $assoc['fieldName'];
+            $this->updateAssociationWithMergedEntity($entity, $assoc, $prevManagedCopy, $managedCopy);
-            $prevClass = $this->em->getClassMetadata(get_class($prevManagedCopy));
-
-            if ($assoc['type'] & ClassMetadata::TO_ONE) {
-                $prevClass->reflFields[$assocField]->setValue($prevManagedCopy, $managedCopy);
-            } else {
-                $prevClass->reflFields[$assocField]->getValue($prevManagedCopy)->add($managedCopy);
-
-                if ($assoc['type'] == ClassMetadata::ONE_TO_MANY) {
-                    $class->reflFields[$assoc['mappedBy']]->setValue($managedCopy, $prevManagedCopy);
-                }
-            }
         }
 
         // Mark the managed copy visited as well
-        $visited[spl_object_hash($managedCopy)] = true;
+        $visited[spl_object_hash($managedCopy)] = $managedCopy;
 
         $this->cascadeMerge($entity, $managedCopy, $visited);
 
         return $managedCopy;
     }
 
+    /**
+     * Sets/adds associated managed copies into the previous entity's association field
+     *
+     * @param object $entity
+     * @param array  $association
+     * @param object $previousManagedCopy
+     * @param object $managedCopy
+     *
+     * @return void
+     */
+    private function updateAssociationWithMergedEntity($entity, array $association, $previousManagedCopy, $managedCopy)
+    {
+        $assocField = $association['fieldName'];
+        $prevClass  = $this->em->getClassMetadata(get_class($previousManagedCopy));
+
+        if ($association['type'] & ClassMetadata::TO_ONE) {
+            $prevClass->reflFields[$assocField]->setValue($previousManagedCopy, $managedCopy);
+
+            return;
+        }
+
+        $value   = $prevClass->reflFields[$assocField]->getValue($previousManagedCopy);
+        $value[] = $managedCopy;
+
+        if ($association['type'] == ClassMetadata::ONE_TO_MANY) {
+            $class = $this->em->getClassMetadata(get_class($entity));
+
+            $class->reflFields[$association['mappedBy']]->setValue($managedCopy, $previousManagedCopy);
+        }
+    }
+
     /**
      * Detaches an entity from the persistence management. It's persistence will
      * no longer be managed by Doctrine.

tests/Doctrine/Tests/ORM/Functional/MergeSharedEntitiesTest.php

@@ -0,0 +1,113 @@
+<?php
+/*
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ * This software consists of voluntary contributions made by many individuals
+ * and is licensed under the MIT license. For more information, see
+ * <http://www.doctrine-project.org>.
+ */
+
+namespace Doctrine\Tests\ORM\Functional;
+
+use Doctrine\ORM\Tools\ToolsException;
+use Doctrine\Tests\OrmFunctionalTestCase;
+
+class MergeSharedEntitiesTest extends OrmFunctionalTestCase
+{
+    /**
+     * {@inheritDoc}
+     */
+    protected function setUp()
+    {
+        parent::setUp();
+
+        try {
+            $this->_schemaTool->createSchema(array(
+                $this->_em->getClassMetadata(__NAMESPACE__ . '\MSEFile'),
+                $this->_em->getClassMetadata(__NAMESPACE__ . '\MSEPicture'),
+            ));
+        } catch (ToolsException $ignored) {
+        }
+    }
+
+    public function testMergeSharedNewEntities()
+    {
+        $file    = new MSEFile;
+        $picture = new MSEPicture;
+
+        $picture->file      = $file;
+        $picture->otherFile = $file;
+
+        $picture = $this->_em->merge($picture);
+
+        $this->assertEquals($picture->file, $picture->otherFile, 'Identical entities must remain identical');
+    }
+
+    public function testMergeSharedManagedEntities()
+    {
+        $file    = new MSEFile;
+        $picture = new MSEPicture;
+
+        $picture->file      = $file;
+        $picture->otherFile = $file;
+
+        $this->_em->persist($file);
+        $this->_em->persist($picture);
+        $this->_em->flush();
+        $this->_em->clear();
+
+        $picture = $this->_em->merge($picture);
+
+        $this->assertEquals($picture->file, $picture->otherFile, 'Identical entities must remain identical');
+    }
+
+    public function testMergeSharedDetachedSerializedEntities()
+    {
+        $file    = new MSEFile;
+        $picture = new MSEPicture;
+
+        $picture->file      = $file;
+        $picture->otherFile = $file;
+
+        $serializedPicture = serialize($picture);
+
+        $this->_em->persist($file);
+        $this->_em->persist($picture);
+        $this->_em->flush();
+        $this->_em->clear();
+
+        $picture = $this->_em->merge(unserialize($serializedPicture));
+
+        $this->assertEquals($picture->file, $picture->otherFile, 'Identical entities must remain identical');
+    }
+}
+
+/** @Entity */
+class MSEPicture
+{
+    /** @Column(type="integer") @Id @GeneratedValue */
+    public $id;
+
+    /** @ManyToOne(targetEntity="MSEFile", cascade={"merge"}) */
+    public $file;
+
+    /** @ManyToOne(targetEntity="MSEFile", cascade={"merge"}) */
+    public $otherFile;
+}
+
+/** @Entity */
+class MSEFile
+{
+    /** @Column(type="integer") @Id @GeneratedValue(strategy="AUTO") */
+    public $id;
+}