From cfae81e11a07bace6212fa09393741eea9cb75b5 Mon Sep 17 00:00:00 2001 From: romanb Date: Thu, 7 May 2009 16:36:27 +0000 Subject: [PATCH] [2.0] Fixed hydration for bi-directional many-many associations. --- .../ORM/Internal/Hydration/ObjectHydrator.php | 109 ++++++++---------- .../ORM/Mapping/AssociationMapping.php | 2 + .../ORM/Mapping/ManyToManyMapping.php | 2 +- lib/Doctrine/ORM/PersistentCollection.php | 13 ++- lib/Doctrine/ORM/UnitOfWork.php | 4 +- tests/Doctrine/Tests/Models/CMS/CmsGroup.php | 4 + .../ORM/Functional/BasicFunctionalTest.php | 15 +++ 7 files changed, 85 insertions(+), 64 deletions(-) diff --git a/lib/Doctrine/ORM/Internal/Hydration/ObjectHydrator.php b/lib/Doctrine/ORM/Internal/Hydration/ObjectHydrator.php index 2826dec92..affda4a14 100644 --- a/lib/Doctrine/ORM/Internal/Hydration/ObjectHydrator.php +++ b/lib/Doctrine/ORM/Internal/Hydration/ObjectHydrator.php @@ -144,8 +144,8 @@ class ObjectHydrator extends AbstractHydrator /** * - * @param $component - * @return + * @param string $component + * @return PersistentCollection * @todo Consider inlining this method. */ private function getCollection($component) @@ -157,24 +157,25 @@ class ObjectHydrator extends AbstractHydrator /** * - * @param $entity - * @param $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 $entity1 - * @param $property - * @param $entity2 - * @param $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 $entity1 - * @param $property - * @param $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 $entity1 - * @param $property - * @param $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]); } } diff --git a/lib/Doctrine/ORM/Mapping/AssociationMapping.php b/lib/Doctrine/ORM/Mapping/AssociationMapping.php index 6872b15fe..5961fcc2d 100644 --- a/lib/Doctrine/ORM/Mapping/AssociationMapping.php +++ b/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 */ diff --git a/lib/Doctrine/ORM/Mapping/ManyToManyMapping.php b/lib/Doctrine/ORM/Mapping/ManyToManyMapping.php index 89838c543..f1df8ae41 100644 --- a/lib/Doctrine/ORM/Mapping/ManyToManyMapping.php +++ b/lib/Doctrine/ORM/Mapping/ManyToManyMapping.php @@ -129,7 +129,7 @@ class ManyToManyMapping extends AssociationMapping public function lazyLoadFor($entity, $entityManager) { - + //TODO } /** diff --git a/lib/Doctrine/ORM/PersistentCollection.php b/lib/Doctrine/ORM/PersistentCollection.php index ac9d7fef6..a45b8bf08 100644 --- a/lib/Doctrine/ORM/PersistentCollection.php +++ b/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() diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index 2df642432..0206501eb 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/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); } diff --git a/tests/Doctrine/Tests/Models/CMS/CmsGroup.php b/tests/Doctrine/Tests/Models/CMS/CmsGroup.php index e616e0c76..2a1f7b472 100644 --- a/tests/Doctrine/Tests/Models/CMS/CmsGroup.php +++ b/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; + } } diff --git a/tests/Doctrine/Tests/ORM/Functional/BasicFunctionalTest.php b/tests/Doctrine/Tests/ORM/Functional/BasicFunctionalTest.php index 897b17297..25c2eeecf 100644 --- a/tests/Doctrine/Tests/ORM/Functional/BasicFunctionalTest.php +++ b/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())); } } \ No newline at end of file