From 61f09e335ecd3b249725169f4be12a2faa4db29b Mon Sep 17 00:00:00 2001 From: romanb Date: Thu, 19 Nov 2009 13:12:00 +0000 Subject: [PATCH] [2.0][DDC-158] Fixed. Collections did not take snapshots after lazy initialization leading to wrong change sets. --- lib/Doctrine/ORM/ORMException.php | 7 ++ lib/Doctrine/ORM/PersistentCollection.php | 21 ++++ .../Tests/ORM/Functional/AllTests.php | 1 + .../ManyToManyBasicAssociationTest.php | 95 +++++++++++++++++++ ...ManyToManyBidirectionalAssociationTest.php | 11 ++- 5 files changed, 131 insertions(+), 4 deletions(-) create mode 100644 tests/Doctrine/Tests/ORM/Functional/ManyToManyBasicAssociationTest.php diff --git a/lib/Doctrine/ORM/ORMException.php b/lib/Doctrine/ORM/ORMException.php index abde5f253..7f541cf7a 100644 --- a/lib/Doctrine/ORM/ORMException.php +++ b/lib/Doctrine/ORM/ORMException.php @@ -13,4 +13,11 @@ class ORMException extends \Exception { return new self("Unrecognized field: $field"); } + + public static function removedEntityInCollectionDetected($entity, $assoc) + { + return new self("Removed entity of type " . get_class($entity) + . " detected in collection '" . $assoc->sourceFieldName . "' during flush." + . " Remove deleted entities from collections."); + } } diff --git a/lib/Doctrine/ORM/PersistentCollection.php b/lib/Doctrine/ORM/PersistentCollection.php index 85d6d13da..b7e7d7ccb 100644 --- a/lib/Doctrine/ORM/PersistentCollection.php +++ b/lib/Doctrine/ORM/PersistentCollection.php @@ -241,6 +241,7 @@ final class PersistentCollection implements \Doctrine\Common\Collections\Collect $this->_coll->add($obj); } } + $this->takeSnapshot(); $this->_initialized = true; } } @@ -376,6 +377,10 @@ final class PersistentCollection implements \Doctrine\Common\Collections\Collect */ public function remove($key) { + // TODO: If the keys are persistent as well (not yet implemented) + // and the collection is not initialized and orphanRemoval is + // not used we can issue a straight SQL delete/update on the + // association (table). Without initializing the collection. $this->_initialize(); $removed = $this->_coll->remove($key); if ($removed) { @@ -394,6 +399,16 @@ final class PersistentCollection implements \Doctrine\Common\Collections\Collect */ public function removeElement($element) { + // TODO: Assuming the identity of entities in a collection is always based + // on their primary key (there is no equals/hashCode in PHP), + // if the collection is not initialized, we could issue a straight + // SQL DELETE/UPDATE on the association (table) without initializing + // the collection. + /*if ( ! $this->_initialized) { + $this->_em->getUnitOfWork()->getCollectionPersister($this->_association) + ->deleteRows($this, $element); + }*/ + $this->_initialize(); $result = $this->_coll->removeElement($element); $this->_changed(); @@ -414,6 +429,12 @@ final class PersistentCollection implements \Doctrine\Common\Collections\Collect */ public function contains($element) { + // TODO: Assuming the identity of entities in a collection is always based + // on their primary key (there is no equals/hashCode in PHP), + // if the collection is not initialized, we could issue a straight + // SQL "SELECT 1" on the association (table) without initializing + // the collection. + $this->_initialize(); return $this->_coll->contains($element); } diff --git a/tests/Doctrine/Tests/ORM/Functional/AllTests.php b/tests/Doctrine/Tests/ORM/Functional/AllTests.php index 36c8f0872..d6e6bc2a7 100644 --- a/tests/Doctrine/Tests/ORM/Functional/AllTests.php +++ b/tests/Doctrine/Tests/ORM/Functional/AllTests.php @@ -32,6 +32,7 @@ class AllTests $suite->addTestSuite('Doctrine\Tests\ORM\Functional\OneToOneUnidirectionalAssociationTest'); $suite->addTestSuite('Doctrine\Tests\ORM\Functional\OneToOneBidirectionalAssociationTest'); $suite->addTestSuite('Doctrine\Tests\ORM\Functional\OneToManyBidirectionalAssociationTest'); + $suite->addTestSuite('Doctrine\Tests\ORM\Functional\ManyToManyBasicAssociationTest'); $suite->addTestSuite('Doctrine\Tests\ORM\Functional\ManyToManyUnidirectionalAssociationTest'); $suite->addTestSuite('Doctrine\Tests\ORM\Functional\ManyToManyBidirectionalAssociationTest'); $suite->addTestSuite('Doctrine\Tests\ORM\Functional\OneToOneSelfReferentialAssociationTest'); diff --git a/tests/Doctrine/Tests/ORM/Functional/ManyToManyBasicAssociationTest.php b/tests/Doctrine/Tests/ORM/Functional/ManyToManyBasicAssociationTest.php new file mode 100644 index 000000000..447428a04 --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Functional/ManyToManyBasicAssociationTest.php @@ -0,0 +1,95 @@ +useModelSet('cms'); + parent::setUp(); + } + + public function testManyToManyAddRemove() + { + // Set up user with 2 groups + $user = new CmsUser; + $user->username = 'romanb'; + $user->status = 'dev'; + $user->name = 'Roman B.'; + + $group1 = new CmsGroup; + $group1->name = 'Developers'; + + $group2 = new CmsGroup; + $group2->name = 'Humans'; + + $user->addGroup($group1); + $user->addGroup($group2); + + $this->_em->persist($user); // cascades to groups + $this->_em->flush(); + + $this->_em->clear(); + + $uRep = $this->_em->getRepository(get_class($user)); + + // Get user + $user = $uRep->findOneById($user->getId()); + + $this->assertFalse($user->getGroups()->isInitialized()); + + // Check groups + $this->assertEquals(2, $user->getGroups()->count()); + + $this->assertTrue($user->getGroups()->isInitialized()); + + // Remove first group + unset($user->groups[0]); + //$user->getGroups()->remove(0); + + $this->_em->flush(); + $this->_em->clear(); + + // Reload same user + $user2 = $uRep->findOneById($user->getId()); + + // Check groups + $this->assertEquals(1, $user2->getGroups()->count()); + } + + public function testManyToManyInverseSideIgnored() + { + $user = new CmsUser; + $user->username = 'romanb'; + $user->status = 'dev'; + $user->name = 'Roman B.'; + + $group = new CmsGroup; + $group->name = 'Humans'; + + // modify directly, addUser() would also (properly) set the owning side + $group->users[] = $user; + + $this->_em->persist($user); + $this->_em->persist($group); + $this->_em->flush(); + $this->_em->clear(); + + // Association should not exist + $user2 = $this->_em->find(get_class($user), $user->getId()); + $this->assertEquals(0, $user2->getGroups()->count()); + } + +} diff --git a/tests/Doctrine/Tests/ORM/Functional/ManyToManyBidirectionalAssociationTest.php b/tests/Doctrine/Tests/ORM/Functional/ManyToManyBidirectionalAssociationTest.php index 2b5054b9c..a6eaf1275 100644 --- a/tests/Doctrine/Tests/ORM/Functional/ManyToManyBidirectionalAssociationTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/ManyToManyBidirectionalAssociationTest.php @@ -59,10 +59,13 @@ class ManyToManyBidirectionalAssociationTest extends AbstractManyToManyAssociati $this->_em->flush(); - $this->assertForeignKeysNotContain($this->firstProduct->getId(), - $this->firstCategory->getId()); - $this->assertForeignKeysContain($this->firstProduct->getId(), - $this->secondCategory->getId()); + $this->assertForeignKeysNotContain($this->firstProduct->getId(), $this->firstCategory->getId()); + $this->assertForeignKeysContain($this->firstProduct->getId(), $this->secondCategory->getId()); + + $this->firstProduct->getCategories()->remove(1); + $this->_em->flush(); + + $this->assertForeignKeysNotContain($this->firstProduct->getId(), $this->secondCategory->getId()); } public function testEagerLoadsInverseSide()