[2.0] Fixed hydration for bi-directional many-many associations.

lib/Doctrine/ORM/Internal/Hydration/ObjectHydrator.php

@@ -144,8 +144,8 @@ class ObjectHydrator extends AbstractHydrator
 
     /**
      *
-     * @param <type> $component
-     * @return <type>
+     * @param string $component
+     * @return PersistentCollection
      * @todo Consider inlining this method.
      */
     private function getCollection($component)
@@ -157,24 +157,25 @@ class ObjectHydrator extends AbstractHydrator
 
     /**
      *
-     * @param <type> $entity
-     * @param <type> $name
+     * @param object $entity
+     * @param string $name
      * @todo Consider inlining this method.
      */
     private function initRelatedCollection($entity, $name)
     {
         $oid = spl_object_hash($entity);
         $classMetadata = $this->_classMetadatas[get_class($entity)];
-        if ( ! isset($this->_initializedRelations[$oid][$name])) {
-            $relation = $classMetadata->getAssociationMapping($name);
-            $relatedClass = $this->_em->getClassMetadata($relation->getTargetEntityName());
-            $coll = $this->getCollection($relatedClass->getClassName());
-            $coll->setOwner($entity, $relation);
-            $coll->setHydrationFlag(true);
-            $classMetadata->getReflectionProperty($name)->setValue($entity, $coll);
-            $this->_initializedRelations[$oid][$name] = true;
-            $this->_uow->setOriginalEntityProperty($oid, $name, $coll);
-        }
+
+        $relation = $classMetadata->getAssociationMapping($name);
+        $relatedClass = $this->_em->getClassMetadata($relation->getTargetEntityName());
+        $coll = $this->getCollection($relatedClass->getClassName());
+        $coll->setOwner($entity, $relation);
+
+        $classMetadata->getReflectionProperty($name)->setValue($entity, $coll);
+        $this->_uow->setOriginalEntityProperty($oid, $name, $coll);
+        $this->_initializedRelations[$oid][$name] = true;
+
+        return $coll;
     }
 
     private function isIndexKeyInUse($entity, $assocField, $indexField)
@@ -208,39 +209,6 @@ class ObjectHydrator extends AbstractHydrator
         return $entity;
     }
 
-    /**
-     * Adds an element to an indexed collection-valued property.
-     *
-     * @param <type> $entity1
-     * @param <type> $property
-     * @param <type> $entity2
-     * @param <type> $indexField
-     * @todo Consider inlining this method. It's called only once and inlining can
-     *       remove the need for the 2 get_class calls.
-     */
-    private function addRelatedIndexedEntity($entity1, $property, $entity2, $indexField)
-    {
-        $classMetadata1 = $this->_classMetadatas[get_class($entity1)];
-        $classMetadata2 = $this->_classMetadatas[get_class($entity2)];
-        $indexValue = $classMetadata2->getReflectionProperty($indexField)->getValue($entity2);
-        $classMetadata1->getReflectionProperty($property)->getValue($entity1)->set($indexValue, $entity2);
-    }
-
-    /**
-     * Adds an element to a collection-valued property.
-     *
-     * @param <type> $entity1
-     * @param <type> $property
-     * @param <type> $entity2
-     */
-    private function addRelatedEntity($entity1, $property, $entity2)
-    {
-        $this->_classMetadatas[get_class($entity1)]
-                ->getReflectionProperty($property)
-                ->getValue($entity1)
-                ->add($entity2);
-    }
-
     /**
      * Checks whether a field on an entity has a non-null value.
      *
@@ -258,9 +226,9 @@ class ObjectHydrator extends AbstractHydrator
     /**
      * Sets a related element.
      *
-     * @param <type> $entity1
-     * @param <type> $property
-     * @param <type> $entity2
+     * @param object $entity1
+     * @param string $property
+     * @param object $entity2
      */
     private function setRelatedElement($entity1, $property, $entity2)
     {
@@ -270,7 +238,6 @@ class ObjectHydrator extends AbstractHydrator
         $this->_uow->setOriginalEntityProperty($oid, $property, $entity2);
         $relation = $classMetadata1->getAssociationMapping($property);
         if ($relation->isOneToOne()) {
-            //$targetClass = $this->_em->getClassMetadata($relation->getTargetEntityName());
             $targetClass = $this->_classMetadatas[$relation->getTargetEntityName()];
             if ($relation->isOwningSide()) {
                 // If there is an inverse mapping on the target class its bidirectional
@@ -305,7 +272,7 @@ class ObjectHydrator extends AbstractHydrator
             unset($rowData['scalars']);
         }
 
-        // Now hydrate the entity data found in the current row.
+        // Hydrate the entity data found in the current row.
         foreach ($rowData as $dqlAlias => $data) {
             $index = false;
             $entityName = $this->_resultSetMapping->getClass($dqlAlias)->getClassName();
@@ -335,18 +302,43 @@ class ObjectHydrator extends AbstractHydrator
 
                 // Check the type of the relation (many or single-valued)
                 if ( ! $relation->isOneToOne()) {
-                    //$oneToOne = false;
                     if (isset($nonemptyComponents[$dqlAlias])) {
-                        $this->initRelatedCollection($baseElement, $relationAlias);
+                        if ( ! isset($this->_initializedRelations[spl_object_hash($baseElement)][$relationAlias])) {
+                            $this->initRelatedCollection($baseElement, $relationAlias)
+                                    ->setHydrationFlag(true);
+                        }
+                        
                         $indexExists = isset($this->_identifierMap[$path][$id[$parent]][$id[$dqlAlias]]);
                         $index = $indexExists ? $this->_identifierMap[$path][$id[$parent]][$id[$dqlAlias]] : false;
                         $indexIsValid = $index !== false ? $this->isIndexKeyInUse($baseElement, $relationAlias, $index) : false;
                         if ( ! $indexExists || ! $indexIsValid) {
                             $element = $this->getEntity($data, $entityName);
+
+                            // If it's a bi-directional many-to-many, also initialize the reverse collection.
+                            if ($relation->isManyToMany()) {
+                                if ($relation->isOwningSide()) {
+                                    $reverseFieldName = $this->_classMetadatas[get_class($element)]
+                                        ->getInverseAssociationMapping($relationAlias)
+                                        ->getSourceFieldName();
+                                    $this->initRelatedCollection($element, $reverseFieldName);
+                                } else if ($mappedByField = $relation->getMappedByFieldName()) {
+                                    $this->initRelatedCollection($element, $mappedByField);
+                                }
+                            }
+
                             if ($field = $this->_getCustomIndexField($dqlAlias)) {
-                                $this->addRelatedIndexedEntity($baseElement, $relationAlias, $element, $field);
+                                $indexValue = $this->_classMetadatas[get_class($element)]
+                                    ->getReflectionProperty($field)
+                                    ->getValue($element);
+                                $this->_classMetadatas[$parentClass]
+                                    ->getReflectionProperty($relationAlias)
+                                    ->getValue($baseElement)
+                                    ->set($indexValue, $element);
                             } else {
-                                $this->addRelatedEntity($baseElement, $relationAlias, $element);
+                                $this->_classMetadatas[$parentClass]
+                                    ->getReflectionProperty($relationAlias)
+                                    ->getValue($baseElement)
+                                    ->add($element);
                             }
                             $this->_identifierMap[$path][$id[$parent]][$id[$dqlAlias]] = $this->getLastKey(
                                     $this->_classMetadatas[$parentClass]
@@ -360,7 +352,6 @@ class ObjectHydrator extends AbstractHydrator
                         $this->setRelatedElement($baseElement, $relationAlias, $coll);
                     }
                 } else {
-                    //$oneToOne = true;
                     if ( ! isset($nonemptyComponents[$dqlAlias]) &&
                             ! $this->isFieldSet($baseElement, $relationAlias)) {
                         $this->setRelatedElement($baseElement, $relationAlias, null);
@@ -375,7 +366,7 @@ class ObjectHydrator extends AbstractHydrator
                         ->getValue($baseElement);
 
                 if ($coll !== null) {
-                    $this->updateResultPointer($coll, $index, $dqlAlias/*, $oneToOne*/);
+                    $this->updateResultPointer($coll, $index, $dqlAlias);
                 }
             } else {
                 // Its a root result element
@@ -409,7 +400,7 @@ class ObjectHydrator extends AbstractHydrator
                 } else {
                     $index = $this->_identifierMap[$dqlAlias][$id[$dqlAlias]];
                 }
-                $this->updateResultPointer($result, $index, $dqlAlias/*, false*/);
+                $this->updateResultPointer($result, $index, $dqlAlias);
                 //unset($rowData[$dqlAlias]);
             }
         }

lib/Doctrine/ORM/Mapping/AssociationMapping.php

@@ -310,6 +310,8 @@ abstract class AssociationMapping
     
     /**
      * Gets the field name of the owning side in a bi-directional association.
+     * This is only set on the inverse side. When invoked on the owning side,
+     * NULL is returned.
      *
      * @return string
      */

lib/Doctrine/ORM/Mapping/ManyToManyMapping.php

@@ -129,7 +129,7 @@ class ManyToManyMapping extends AssociationMapping
 
     public function lazyLoadFor($entity, $entityManager)
     {
-        
+        //TODO
     }
 
     /**

lib/Doctrine/ORM/PersistentCollection.php

@@ -251,9 +251,15 @@ final class PersistentCollection extends \Doctrine\Common\Collections\Collection
 
         if ($this->_hydrationFlag) {
             if ($this->_backRefFieldName) {
-                // set back reference to owner
-                $this->_ownerClass->getReflectionProperty($this->_backRefFieldName)
-                        ->setValue($value, $this->_owner);
+                // Set back reference to owner
+                if ($this->_association->isOneToMany()) {
+                    $this->_ownerClass->getReflectionProperty($this->_backRefFieldName)
+                            ->setValue($value, $this->_owner);
+                } else {
+                    // ManyToMany
+                    $this->_ownerClass->getReflectionProperty($this->_backRefFieldName)
+                            ->getValue($value)->add($this->_owner);
+                }
             }
         } else {
             $this->_changed();
@@ -388,6 +394,7 @@ final class PersistentCollection extends \Doctrine\Common\Collections\Collection
             }
         }*/
         parent::clear();
+        $this->_changed();
     }
     
     private function _changed()

lib/Doctrine/ORM/UnitOfWork.php

@@ -520,7 +520,8 @@ class UnitOfWork implements PropertyChangedListener
                 $this->_entityChangeSets[$oid] = $changeSet;
                 $this->_originalEntityData[$oid] = $data;
             } else if ($state == self::STATE_DELETED) {
-                throw DoctrineException::updateMe("Deleted entity in collection detected during flush.");
+                throw DoctrineException::updateMe("Deleted entity in collection detected during flush."
+                        . " Make sure you properly remove deleted entities from collections.");
             }
             // MANAGED associated entities are already taken into account
             // during changeset calculation anyway, since they are in the identity map.
@@ -1234,6 +1235,7 @@ class UnitOfWork implements PropertyChangedListener
             }*/
             $this->_mergeData($entity, $data, $class, true);
             $this->_entityIdentifiers[$oid] = $id;
+            $this->_entityStates[$oid] = self::STATE_MANAGED;
             $this->addToIdentityMap($entity);
         }
 

tests/Doctrine/Tests/Models/CMS/CmsGroup.php

@@ -41,5 +41,9 @@ class CmsGroup
     public function addUser(CmsUser $user) {
         $this->users[] = $user;
     }
+
+    public function getUsers() {
+        return $this->users;
+    }
 }
 

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

@@ -291,5 +291,20 @@ class BasicFunctionalTest extends \Doctrine\Tests\OrmFunctionalTestCase
         $this->assertEquals(1, $result[0]->getGroups()->count());
         $groups = $result[0]->getGroups();
         $this->assertEquals('Doctrine Developers', $groups[0]->getName());
+
+        $this->assertEquals(\Doctrine\ORM\UnitOfWork::STATE_MANAGED, $this->_em->getUnitOfWork()->getEntityState($result[0]));
+        $this->assertEquals(\Doctrine\ORM\UnitOfWork::STATE_MANAGED, $this->_em->getUnitOfWork()->getEntityState($groups[0]));
+
+        $this->assertTrue($groups instanceof \Doctrine\ORM\PersistentCollection);
+        $this->assertTrue($groups[0]->getUsers() instanceof \Doctrine\ORM\PersistentCollection);
+
+        $groups[0]->getUsers()->clear();
+        $groups->clear();
+
+        $this->_em->flush();
+        $this->_em->clear();
+
+        $query = $this->_em->createQuery("select u, g from Doctrine\Tests\Models\CMS\CmsUser u inner join u.groups g");
+        $this->assertEquals(0, count($query->getResultList()));
     }
 }