From 536aca23daf0a6cd3745839ca538bd65391a2206 Mon Sep 17 00:00:00 2001 From: romanb Date: Sat, 27 Feb 2010 17:48:18 +0000 Subject: [PATCH] [2.0][DDC-371] Fixed together with other hydration/initialization issues. --- lib/Doctrine/DBAL/Types/DateTimeType.php | 2 +- .../ORM/Internal/Hydration/ObjectHydrator.php | 47 +++---- lib/Doctrine/ORM/PersistentCollection.php | 38 ++--- ...ManyToManyBidirectionalAssociationTest.php | 132 +++++++++++------- .../OneToManyBidirectionalAssociationTest.php | 3 + .../ORM/Functional/Ticket/DDC371Test.php | 69 +++++++++ 6 files changed, 179 insertions(+), 112 deletions(-) create mode 100644 tests/Doctrine/Tests/ORM/Functional/Ticket/DDC371Test.php diff --git a/lib/Doctrine/DBAL/Types/DateTimeType.php b/lib/Doctrine/DBAL/Types/DateTimeType.php index 1aca8bab2..f802ae2a5 100644 --- a/lib/Doctrine/DBAL/Types/DateTimeType.php +++ b/lib/Doctrine/DBAL/Types/DateTimeType.php @@ -23,7 +23,7 @@ class DateTimeType extends Type public function convertToDatabaseValue($value, AbstractPlatform $platform) { - return ($value !== null) + return ($value !== null) ? $value->format($platform->getDateTimeFormatString()) : null; } diff --git a/lib/Doctrine/ORM/Internal/Hydration/ObjectHydrator.php b/lib/Doctrine/ORM/Internal/Hydration/ObjectHydrator.php index cabd2aca3..02f21c948 100644 --- a/lib/Doctrine/ORM/Internal/Hydration/ObjectHydrator.php +++ b/lib/Doctrine/ORM/Internal/Hydration/ObjectHydrator.php @@ -81,15 +81,20 @@ class ObjectHydrator extends AbstractHydrator $this->_hints['fetched'][$sourceSubclassName][$assoc->sourceFieldName] = true; } } - if ($assoc->mappedByFieldName) { - $this->_hints['fetched'][$className][$assoc->mappedByFieldName] = true; - } else { - if (isset($sourceClass->inverseMappings[$className][$assoc->sourceFieldName])) { - $inverseAssoc = $sourceClass->inverseMappings[$className][$assoc->sourceFieldName]; - $this->_hints['fetched'][$className][$inverseAssoc->sourceFieldName] = true; - if ($class->subClasses) { - foreach ($class->subClasses as $targetSubclassName) { - $this->_hints['fetched'][$targetSubclassName][$inverseAssoc->sourceFieldName] = true; + if ( ! $assoc->isManyToMany()) { + // Mark any non-collection opposite sides as fetched, too. + if ($assoc->mappedByFieldName) { + $this->_hints['fetched'][$className][$assoc->mappedByFieldName] = true; + } else { + if (isset($class->inverseMappings[$sourceClassName][$assoc->sourceFieldName])) { + $inverseAssoc = $class->inverseMappings[$sourceClassName][$assoc->sourceFieldName]; + if ($inverseAssoc->isOneToOne()) { + $this->_hints['fetched'][$className][$inverseAssoc->sourceFieldName] = true; + if ($class->subClasses) { + foreach ($class->subClasses as $targetSubclassName) { + $this->_hints['fetched'][$targetSubclassName][$inverseAssoc->sourceFieldName] = true; + } + } } } } @@ -308,22 +313,7 @@ class ObjectHydrator extends AbstractHydrator } } else { $element = $this->_getEntity($data, $dqlAlias); - - // If it's a bi-directional many-to-many, also initialize the reverse collection, - // but only once, of course. - if ($relation->isManyToMany()) { - if ($relation->isOwningSide && isset($this->_ce[$entityName]->inverseMappings[$relation->sourceEntityName][$relationField])) { - $inverseFieldName = $this->_ce[$entityName]->inverseMappings[$relation->sourceEntityName][$relationField]->sourceFieldName; - if ( ! isset($this->_initializedCollections[spl_object_hash($element) . $inverseFieldName])) { - $this->_initRelatedCollection($element, $this->_ce[$entityName], $inverseFieldName); - } - } else if ($relation->mappedByFieldName) { - if ( ! isset($this->_initializedCollections[spl_object_hash($element) . $relation->mappedByFieldName])) { - $this->_initRelatedCollection($element, $this->_ce[$entityName], $relation->mappedByFieldName); - } - } - } - + if (isset($this->_rsm->indexByMap[$dqlAlias])) { $field = $this->_rsm->indexByMap[$dqlAlias]; $indexValue = $this->_ce[$entityName]->reflFields[$field]->getValue($element); @@ -359,12 +349,7 @@ class ObjectHydrator extends AbstractHydrator // If there is an inverse mapping on the target class its bidirectional if (isset($targetClass->inverseMappings[$relation->sourceEntityName][$relationField])) { $inverseAssoc = $targetClass->inverseMappings[$relation->sourceEntityName][$relationField]; - if ($inverseAssoc->isOneToMany()) { - /*// Only initialize reverse collection if it is not yet initialized. - if ( ! isset($this->_initializedCollections[spl_object_hash($element) . $inverseAssoc->sourceFieldName])) { - $this->_initRelatedCollection($element, $targetClass, $inverseAssoc->sourceFieldName); - }*/ - } else { + if ($inverseAssoc->isOneToOne()) { $targetClass->reflFields[$inverseAssoc->sourceFieldName]->setValue($element, $parentObject); $this->_uow->setOriginalEntityProperty(spl_object_hash($element), $inverseAssoc->sourceFieldName, $parentObject); } diff --git a/lib/Doctrine/ORM/PersistentCollection.php b/lib/Doctrine/ORM/PersistentCollection.php index d412d2e11..035b53ed2 100644 --- a/lib/Doctrine/ORM/PersistentCollection.php +++ b/lib/Doctrine/ORM/PersistentCollection.php @@ -165,31 +165,23 @@ final class PersistentCollection implements \Doctrine\Common\Collections\Collect /** * INTERNAL: * Adds an element to a collection during hydration. This will automatically - * complete bidirectional associations. + * complete bidirectional associations in the case of a one-to-many association. * * @param mixed $element The element to add. */ public function hydrateAdd($element) { $this->_coll->add($element); - // If _backRefFieldName is set, then the association is bidirectional // and we need to set the back reference. - if ($this->_backRefFieldName) { + if ($this->_backRefFieldName && $this->_association->isOneToMany()) { // Set back reference to owner - if ($this->_association->isOneToMany()) { - // OneToMany - $this->_typeClass->reflFields[$this->_backRefFieldName] - ->setValue($element, $this->_owner); - $this->_em->getUnitOfWork()->setOriginalEntityProperty( - spl_object_hash($element), - $this->_backRefFieldName, - $this->_owner); - } else { - // ManyToMany - $this->_typeClass->reflFields[$this->_backRefFieldName] - ->getValue($element)->unwrap()->add($this->_owner); - } + $this->_typeClass->reflFields[$this->_backRefFieldName] + ->setValue($element, $this->_owner); + $this->_em->getUnitOfWork()->setOriginalEntityProperty( + spl_object_hash($element), + $this->_backRefFieldName, + $this->_owner); } } @@ -203,20 +195,12 @@ final class PersistentCollection implements \Doctrine\Common\Collections\Collect public function hydrateSet($key, $element) { $this->_coll->set($key, $element); - // If _backRefFieldName is set, then the association is bidirectional // and we need to set the back reference. - if ($this->_backRefFieldName) { + if ($this->_backRefFieldName && $this->_association->isOneToMany()) { // Set back reference to owner - if ($this->_association->isOneToMany()) { - // OneToMany - $this->_typeClass->reflFields[$this->_backRefFieldName] - ->setValue($element, $this->_owner); - } else { - // ManyToMany - $this->_typeClass->reflFields[$this->_backRefFieldName] - ->getValue($element)->set($key, $this->_owner); - } + $this->_typeClass->reflFields[$this->_backRefFieldName] + ->setValue($element, $this->_owner); } } diff --git a/tests/Doctrine/Tests/ORM/Functional/ManyToManyBidirectionalAssociationTest.php b/tests/Doctrine/Tests/ORM/Functional/ManyToManyBidirectionalAssociationTest.php index a6eaf1275..0bb5ae7d8 100644 --- a/tests/Doctrine/Tests/ORM/Functional/ManyToManyBidirectionalAssociationTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/ManyToManyBidirectionalAssociationTest.php @@ -44,10 +44,8 @@ class ManyToManyBidirectionalAssociationTest extends AbstractManyToManyAssociati $this->_em->persist($this->firstProduct); $this->_em->flush(); - $this->assertForeignKeysContain($this->firstProduct->getId(), - $this->firstCategory->getId()); - $this->assertForeignKeysContain($this->firstProduct->getId(), - $this->secondCategory->getId()); + $this->assertForeignKeysContain($this->firstProduct->getId(), $this->firstCategory->getId()); + $this->assertForeignKeysContain($this->firstProduct->getId(), $this->secondCategory->getId()); } public function testRemovesAManyToManyAssociation() @@ -68,49 +66,20 @@ class ManyToManyBidirectionalAssociationTest extends AbstractManyToManyAssociati $this->assertForeignKeysNotContain($this->firstProduct->getId(), $this->secondCategory->getId()); } - public function testEagerLoadsInverseSide() + public function testEagerLoadFromInverseSideAndLazyLoadFromOwningSide() { + //$this->_em->getConnection()->getConfiguration()->setSqlLogger(new \Doctrine\DBAL\Logging\EchoSqlLogger); $this->_createLoadingFixture(); - list ($firstProduct, $secondProduct) = $this->_findProducts(); - $categories = $firstProduct->getCategories(); - $this->assertLoadingOfInverseSide($categories); - $this->assertLoadingOfInverseSide($secondProduct->getCategories()); + $categories = $this->_findCategories(); + $this->assertLazyLoadFromOwningSide($categories); } - public function testEagerLoadsOwningSide() + public function testEagerLoadFromOwningSideAndLazyLoadFromInverseSide() { + //$this->_em->getConnection()->getConfiguration()->setSqlLogger(new \Doctrine\DBAL\Logging\EchoSqlLogger); $this->_createLoadingFixture(); $products = $this->_findProducts(); - $this->assertLoadingOfOwningSide($products); - } - - public function testLazyLoadsCollectionOnTheInverseSide() - { - $this->_createLoadingFixture(); - - $metadata = $this->_em->getClassMetadata('Doctrine\Tests\Models\ECommerce\ECommerceCategory'); - $metadata->getAssociationMapping('products')->fetchMode = AssociationMapping::FETCH_LAZY; - - $query = $this->_em->createQuery('SELECT c FROM Doctrine\Tests\Models\ECommerce\ECommerceCategory c order by c.id'); - $categories = $query->getResult(); - $this->assertLoadingOfInverseSide($categories); - } - - public function testLazyLoadsCollectionOnTheOwningSide() - { - $this->_createLoadingFixture(); - - $metadata = $this->_em->getClassMetadata('Doctrine\Tests\Models\ECommerce\ECommerceProduct'); - $metadata->getAssociationMapping('categories')->fetchMode = AssociationMapping::FETCH_LAZY; - - $query = $this->_em->createQuery('SELECT p FROM Doctrine\Tests\Models\ECommerce\ECommerceProduct p order by p.id'); - $products = $query->getResult(); - - $this->assertEquals(2, count($products)); - $this->assertEquals(0, count($products[0]->getCategories()->unwrap())); - $this->assertEquals(0, count($products[1]->getCategories()->unwrap())); - - $this->assertLoadingOfOwningSide($products); + $this->assertLazyLoadFromInverseSide($products); } private function _createLoadingFixture() @@ -130,10 +99,38 @@ class ManyToManyBidirectionalAssociationTest extends AbstractManyToManyAssociati { $query = $this->_em->createQuery('SELECT p, c FROM Doctrine\Tests\Models\ECommerce\ECommerceProduct p LEFT JOIN p.categories c ORDER BY p.id, c.id'); //$query->setHint(Query::HINT_FORCE_PARTIAL_LOAD, true); - return $query->getResult(); + $result = $query->getResult(); + $this->assertEquals(2, count($result)); + $cats1 = $result[0]->getCategories(); + $cats2 = $result[1]->getCategories(); + $this->assertTrue($cats1->isInitialized()); + $this->assertTrue($cats2->isInitialized()); + $this->assertFalse($cats1[0]->getProducts()->isInitialized()); + $this->assertFalse($cats2[0]->getProducts()->isInitialized()); + + return $result; } - public function assertLoadingOfOwningSide($products) + protected function _findCategories() + { + $query = $this->_em->createQuery('SELECT c, p FROM Doctrine\Tests\Models\ECommerce\ECommerceCategory c LEFT JOIN c.products p ORDER BY c.id, p.id'); + //$query->setHint(Query::HINT_FORCE_PARTIAL_LOAD, true); + $result = $query->getResult(); + $this->assertEquals(2, count($result)); + $this->assertTrue($result[0] instanceof ECommerceCategory); + $this->assertTrue($result[1] instanceof ECommerceCategory); + $prods1 = $result[0]->getProducts(); + $prods2 = $result[1]->getProducts(); + $this->assertTrue($prods1->isInitialized()); + $this->assertTrue($prods2->isInitialized()); + + $this->assertFalse($prods1[0]->getCategories()->isInitialized()); + $this->assertFalse($prods2[0]->getCategories()->isInitialized()); + + return $result; + } + + public function assertLazyLoadFromInverseSide($products) { list ($firstProduct, $secondProduct) = $products; @@ -148,9 +145,17 @@ class ManyToManyBidirectionalAssociationTest extends AbstractManyToManyAssociati $firstCategoryProducts = $firstProductCategories[0]->getProducts(); $secondCategoryProducts = $firstProductCategories[1]->getProducts(); + + $this->assertFalse($firstCategoryProducts->isInitialized()); + $this->assertFalse($secondCategoryProducts->isInitialized()); + $this->assertEquals(0, $firstCategoryProducts->unwrap()->count()); + $this->assertEquals(0, $secondCategoryProducts->unwrap()->count()); - $this->assertEquals(2, count($firstCategoryProducts)); - $this->assertEquals(2, count($secondCategoryProducts)); + $this->assertEquals(2, count($firstCategoryProducts)); // lazy-load + $this->assertTrue($firstCategoryProducts->isInitialized()); + $this->assertFalse($secondCategoryProducts->isInitialized()); + $this->assertEquals(2, count($secondCategoryProducts)); // lazy-load + $this->assertTrue($secondCategoryProducts->isInitialized()); $this->assertTrue($firstCategoryProducts[0] instanceof ECommerceProduct); $this->assertTrue($firstCategoryProducts[1] instanceof ECommerceProduct); @@ -160,17 +165,38 @@ class ManyToManyBidirectionalAssociationTest extends AbstractManyToManyAssociati $this->assertCollectionEquals($firstCategoryProducts, $secondCategoryProducts); } - public function assertLoadingOfInverseSide($categories) + public function assertLazyLoadFromOwningSide($categories) { - $this->assertTrue($categories[0] instanceof ECommerceCategory); - $this->assertTrue($categories[1] instanceof ECommerceCategory); + list ($firstCategory, $secondCategory) = $categories; + + $firstCategoryProducts = $firstCategory->getProducts(); + $secondCategoryProducts = $secondCategory->getProducts(); - $this->assertEquals(2, count($categories[0]->getProducts())); - $this->assertTrue($categories[0]->getProducts()->get(0) instanceof ECommerceProduct); - $this->assertTrue($categories[0]->getProducts()->get(1) instanceof ECommerceProduct); + $this->assertEquals(2, count($firstCategoryProducts)); + $this->assertEquals(2, count($secondCategoryProducts)); - $this->assertEquals(2, count($categories[1]->getProducts())); - $this->assertTrue($categories[1]->getProducts()->get(0) instanceof ECommerceProduct); - $this->assertTrue($categories[1]->getProducts()->get(1) instanceof ECommerceProduct); + $this->assertTrue($firstCategoryProducts[0] === $secondCategoryProducts[0]); + $this->assertTrue($firstCategoryProducts[1] === $secondCategoryProducts[1]); + + $firstProductCategories = $firstCategoryProducts[0]->getCategories(); + $secondProductCategories = $firstCategoryProducts[1]->getCategories(); + + $this->assertFalse($firstProductCategories->isInitialized()); + $this->assertFalse($secondProductCategories->isInitialized()); + $this->assertEquals(0, $firstProductCategories->unwrap()->count()); + $this->assertEquals(0, $secondProductCategories->unwrap()->count()); + + $this->assertEquals(2, count($firstProductCategories)); // lazy-load + $this->assertTrue($firstProductCategories->isInitialized()); + $this->assertFalse($secondProductCategories->isInitialized()); + $this->assertEquals(2, count($secondProductCategories)); // lazy-load + $this->assertTrue($secondProductCategories->isInitialized()); + + $this->assertTrue($firstProductCategories[0] instanceof ECommerceCategory); + $this->assertTrue($firstProductCategories[1] instanceof ECommerceCategory); + $this->assertTrue($secondProductCategories[0] instanceof ECommerceCategory); + $this->assertTrue($secondProductCategories[1] instanceof ECommerceCategory); + + $this->assertCollectionEquals($firstProductCategories, $secondProductCategories); } } diff --git a/tests/Doctrine/Tests/ORM/Functional/OneToManyBidirectionalAssociationTest.php b/tests/Doctrine/Tests/ORM/Functional/OneToManyBidirectionalAssociationTest.php index c4bf28bd5..2cdd3011a 100644 --- a/tests/Doctrine/Tests/ORM/Functional/OneToManyBidirectionalAssociationTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/OneToManyBidirectionalAssociationTest.php @@ -74,13 +74,16 @@ class OneToManyBidirectionalAssociationTest extends \Doctrine\Tests\OrmFunctiona $query = $this->_em->createQuery('select p, f from Doctrine\Tests\Models\ECommerce\ECommerceProduct p join p.features f'); $result = $query->getResult(); $product = $result[0]; + $features = $product->getFeatures(); $this->assertTrue($features[0] instanceof ECommerceFeature); + $this->assertFalse($features[0]->getProduct() instanceof \Doctrine\ORM\Proxy\Proxy); $this->assertSame($product, $features[0]->getProduct()); $this->assertEquals('Model writing tutorial', $features[0]->getDescription()); $this->assertTrue($features[1] instanceof ECommerceFeature); $this->assertSame($product, $features[1]->getProduct()); + $this->assertFalse($features[1]->getProduct() instanceof \Doctrine\ORM\Proxy\Proxy); $this->assertEquals('Annotations examples', $features[1]->getDescription()); } diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC371Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC371Test.php new file mode 100644 index 000000000..9bffd96c5 --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC371Test.php @@ -0,0 +1,69 @@ +_em->getConnection()->getConfiguration()->setSqlLogger(new \Doctrine\DBAL\Logging\EchoSqlLogger); + $this->_schemaTool->createSchema(array( + $this->_em->getClassMetadata(__NAMESPACE__ . '\DDC371Parent'), + $this->_em->getClassMetadata(__NAMESPACE__ . '\DDC371Child') + )); + } + + public function testIssue() + { + $parent = new DDC371Parent; + $parent->data = 'parent'; + $parent->children = new \Doctrine\Common\Collections\ArrayCollection; + + $child = new DDC371Child; + $child->data = 'child'; + + $child->parent = $parent; + $parent->children->add($child); + + $this->_em->persist($parent); + $this->_em->persist($child); + + $this->_em->flush(); + $this->_em->clear(); + + $children = $this->_em->createQuery('select c,p from '.__NAMESPACE__.'\DDC371Child c ' + . 'left join c.parent p where c.id = 1 and p.id = 1') + ->setHint(Query::HINT_REFRESH, true) + ->getResult(); + + $this->assertEquals(1, count($children)); + $this->assertFalse($children[0]->parent instanceof \Doctrine\ORM\Proxy\Proxy); + $this->assertFalse($children[0]->parent->children->isInitialized()); + $this->assertEquals(0, $children[0]->parent->children->unwrap()->count()); + } +} + +/** @Entity */ +class DDC371Child { + /** @Id @Column(type="integer") @GeneratedValue */ + private $id; + /** @Column(type="string") */ + public $data; + /** @ManyToOne(targetEntity="DDC371Parent") @JoinColumn(name="parentId") */ + public $parent; +} + +/** @Entity */ +class DDC371Parent { + /** @Id @Column(type="integer") @GeneratedValue */ + private $id; + /** @Column(type="string") */ + public $data; + /** @OneToMany(targetEntity="DDC371Child", mappedBy="parent") */ + public $children; +} +