From 2ead9e23aba10fa7cd67cdab18dafdfb9d83c863 Mon Sep 17 00:00:00 2001
From: Mathieu De Zutter <mathieu@dezutter.org>
Date: Wed, 5 Nov 2014 21:13:13 +0100
Subject: [PATCH] Fix merging of entities with associations to identical
 entities.

Without this patch, when an entity that refers multiple times to the same
associated entity gets merged, the second references becomes null.

The main issue is that even though doMerge returns a managed copy, that value
is not used while cascading the merge. These identicial entities are already
detected through the visitor map, but they are ignored.  There should be some
refactoring so cascadeMerge calls a function that checks if the parent must be
updated, based on the return value of its call to doMerge.  However, this patch
tries to impact the code as little as possible, and only introduces a new
function to avoid duplicate code.

The secondary issue arises when using inverted associations. In that case, it
is possible that an entity to be merged is already merged, so the the visitor
map is looked up by the hash of a managed copy instead of the original entity.
This means that in this case the visitor map entries should also be set to the
entity, instead of being set to 'true'.
---
 lib/Doctrine/ORM/UnitOfWork.php | 58 ++++++++++++++++++++++-----------
 1 file changed, 39 insertions(+), 19 deletions(-)

diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php
index 699926f43..45d11c7a4 100644
--- a/lib/Doctrine/ORM/UnitOfWork.php
+++ b/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, $prevManagedCopy, $assoc, $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']),
-                                    new ArrayCollection
-                                    );
+                                $this->em->getClassMetadata($assoc2['targetEntity']),
+                                new ArrayCollection
+                            );
                             $managedCol->setOwner($managedCopy, $assoc2);
                             $prop->setValue($managedCopy, $managedCol);
                             $this->originalEntityData[$oid][$name] = $managedCol;
@@ -1933,28 +1939,42 @@ class UnitOfWork implements PropertyChangedListener
         }
 
         if ($prevManagedCopy !== null) {
-            $assocField = $assoc['fieldName'];
-            $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);
-                }
-            }
+            $this->updateAssociationWithMergedEntity($entity, $prevManagedCopy, $assoc, $managedCopy);
         }
 
         // 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;
     }
 
+    /**
+     * @param object $entity
+     * @param object $prevManagedCopy
+     * @param array  $assoc
+     * @param object $managedCopy
+     */
+    private function updateAssociationWithMergedEntity($entity, $prevManagedCopy, array $assoc, $managedCopy)
+    {
+        $assocField = $assoc['fieldName'];
+        $prevClass = $this->em->getClassMetadata(get_class($prevManagedCopy));
+
+        if ($assoc['type'] & ClassMetadata::TO_ONE) {
+            $prevClass->reflFields[$assocField]->setValue($prevManagedCopy, $managedCopy);
+            return;
+        }
+
+        $value = $prevClass->reflFields[$assocField]->getValue($prevManagedCopy);
+        $value[] = $managedCopy;
+
+        if ($assoc['type'] == ClassMetadata::ONE_TO_MANY) {
+            $class = $this->em->getClassMetadata(get_class($entity));
+            $class->reflFields[$assoc['mappedBy']]->setValue($managedCopy, $prevManagedCopy);
+        }
+    }
+
     /**
      * Detaches an entity from the persistence management. It's persistence will
      * no longer be managed by Doctrine.