From 932a56f26fd2e356d7a1584aed98cf80caa07693 Mon Sep 17 00:00:00 2001 From: Guilherme Blanco Date: Mon, 12 Jan 2015 19:52:27 +0000 Subject: [PATCH] Internalize UnitOfWork in CollectionPersisters. Updated related code. --- .../ORM/Persisters/EntityPersister.php | 2 +- .../ORM/Persisters/ManyToManyPersister.php | 35 ++++++-------- .../ORM/Persisters/OneToManyPersister.php | 32 +++++-------- .../Models/DDC2504/DDC2504ChildClass.php | 4 +- .../Models/DDC2504/DDC2504OtherClass.php | 12 +---- .../Tests/Models/DDC2504/DDC2504RootClass.php | 2 +- .../Functional/ExtraLazyCollectionTest.php | 46 +++++++++---------- 7 files changed, 55 insertions(+), 78 deletions(-) diff --git a/lib/Doctrine/ORM/Persisters/EntityPersister.php b/lib/Doctrine/ORM/Persisters/EntityPersister.php index 3cc824e39..7a0190a16 100644 --- a/lib/Doctrine/ORM/Persisters/EntityPersister.php +++ b/lib/Doctrine/ORM/Persisters/EntityPersister.php @@ -153,7 +153,7 @@ interface EntityPersister * * @param object $entity The entity to delete. * - * @return bool + * @return bool TRUE if the entity got deleted in the database, FALSE otherwise. */ public function delete($entity); diff --git a/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php b/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php index f9e8a7d14..c8ac9d4d5 100644 --- a/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php +++ b/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php @@ -223,7 +223,7 @@ class ManyToManyPersister extends AbstractCollectionPersister $mapping = $coll->getMapping(); $association = $mapping; $class = $this->em->getClassMetadata($mapping['sourceEntity']); - $id = $this->em->getUnitOfWork()->getEntityIdentifier($coll->getOwner()); + $id = $this->uow->getEntityIdentifier($coll->getOwner()); if ( ! $mapping['isOwningSide']) { $targetEntity = $this->em->getClassMetadata($mapping['targetEntity']); @@ -263,9 +263,10 @@ class ManyToManyPersister extends AbstractCollectionPersister */ public function slice(PersistentCollection $coll, $offset, $length = null) { - $mapping = $coll->getMapping(); + $mapping = $coll->getMapping(); + $persister = $this->uow->getEntityPersister($mapping['targetEntity']); - return $this->em->getUnitOfWork()->getEntityPersister($mapping['targetEntity'])->getManyToManyCollection($mapping, $coll->getOwner(), $offset, $length); + return $persister->getManyToManyCollection($mapping, $coll->getOwner(), $offset, $length); } /** * {@inheritdoc} @@ -273,7 +274,9 @@ class ManyToManyPersister extends AbstractCollectionPersister public function containsKey(PersistentCollection $coll, $key) { list($quotedJoinTable, $whereClauses, $params) = $this->getJoinTableRestrictionsWithKey($coll, $key, true); + $sql = 'SELECT 1 FROM ' . $quotedJoinTable . ' WHERE ' . implode(' AND ', $whereClauses); + return (bool) $this->conn->fetchColumn($sql, $params); } @@ -282,17 +285,14 @@ class ManyToManyPersister extends AbstractCollectionPersister */ public function contains(PersistentCollection $coll, $element) { - $uow = $this->em->getUnitOfWork(); - - // Shortcut for new entities - $entityState = $uow->getEntityState($element, UnitOfWork::STATE_NEW); + $entityState = $this->uow->getEntityState($element, UnitOfWork::STATE_NEW); if ($entityState === UnitOfWork::STATE_NEW) { return false; } // Entity is scheduled for inclusion - if ($entityState === UnitOfWork::STATE_MANAGED && $uow->isScheduledForInsert($element)) { + if ($entityState === UnitOfWork::STATE_MANAGED && $this->uow->isScheduledForInsert($element)) { return false; } @@ -308,10 +308,7 @@ class ManyToManyPersister extends AbstractCollectionPersister */ public function removeElement(PersistentCollection $coll, $element) { - $uow = $this->em->getUnitOfWork(); - - // shortcut for new entities - $entityState = $uow->getEntityState($element, UnitOfWork::STATE_NEW); + $entityState = $this->uow->getEntityState($element, UnitOfWork::STATE_NEW); if ($entityState === UnitOfWork::STATE_NEW) { return false; @@ -319,7 +316,7 @@ class ManyToManyPersister extends AbstractCollectionPersister // If Entity is scheduled for inclusion, it is not in this collection. // We can assure that because it would have return true before on array check - if ($entityState === UnitOfWork::STATE_MANAGED && $uow->isScheduledForInsert($element)) { + if ($entityState === UnitOfWork::STATE_MANAGED && $this->uow->isScheduledForInsert($element)) { return false; } @@ -339,11 +336,10 @@ class ManyToManyPersister extends AbstractCollectionPersister */ private function getJoinTableRestrictionsWithKey(PersistentCollection $coll, $key, $addFilters) { - $uow = $this->em->getUnitOfWork(); $filterMapping = $coll->getMapping(); $mapping = $filterMapping; $indexBy = $mapping['indexBy']; - $id = $uow->getEntityIdentifier($coll->getOwner()); + $id = $this->uow->getEntityIdentifier($coll->getOwner()); $targetEntity = $this->em->getClassMetadata($mapping['targetEntity']); @@ -411,22 +407,21 @@ class ManyToManyPersister extends AbstractCollectionPersister */ private function getJoinTableRestrictions(PersistentCollection $coll, $element, $addFilters) { - $uow = $this->em->getUnitOfWork(); $filterMapping = $coll->getMapping(); $mapping = $filterMapping; if ( ! $mapping['isOwningSide']) { $sourceClass = $this->em->getClassMetadata($mapping['targetEntity']); $targetClass = $this->em->getClassMetadata($mapping['sourceEntity']); - $sourceId = $uow->getEntityIdentifier($element); - $targetId = $uow->getEntityIdentifier($coll->getOwner()); + $sourceId = $this->uow->getEntityIdentifier($element); + $targetId = $this->uow->getEntityIdentifier($coll->getOwner()); $mapping = $sourceClass->associationMappings[$mapping['mappedBy']]; } else { $sourceClass = $this->em->getClassMetadata($mapping['sourceEntity']); $targetClass = $this->em->getClassMetadata($mapping['targetEntity']); - $sourceId = $uow->getEntityIdentifier($coll->getOwner()); - $targetId = $uow->getEntityIdentifier($element); + $sourceId = $this->uow->getEntityIdentifier($coll->getOwner()); + $targetId = $this->uow->getEntityIdentifier($element); } $quotedJoinTable = $this->quoteStrategy->getJoinTableName($mapping, $sourceClass, $this->platform); diff --git a/lib/Doctrine/ORM/Persisters/OneToManyPersister.php b/lib/Doctrine/ORM/Persisters/OneToManyPersister.php index a2f22e75b..c1e8a6c38 100644 --- a/lib/Doctrine/ORM/Persisters/OneToManyPersister.php +++ b/lib/Doctrine/ORM/Persisters/OneToManyPersister.php @@ -39,8 +39,7 @@ class OneToManyPersister extends AbstractCollectionPersister public function get(PersistentCollection $coll, $index) { $mapping = $coll->getMapping(); - $uow = $this->em->getUnitOfWork(); - $persister = $uow->getEntityPersister($mapping['targetEntity']); + $persister = $this->uow->getEntityPersister($mapping['targetEntity']); if (!isset($mapping['indexBy'])) { throw new \BadMethodCallException("Selecting a collection by index is only supported on indexed collections."); @@ -134,8 +133,7 @@ class OneToManyPersister extends AbstractCollectionPersister public function count(PersistentCollection $coll) { $mapping = $coll->getMapping(); - $uow = $this->em->getUnitOfWork(); - $persister = $uow->getEntityPersister($mapping['targetEntity']); + $persister = $this->uow->getEntityPersister($mapping['targetEntity']); // only works with single id identifier entities. Will throw an // exception in Entity Persisters if that is not the case for the @@ -151,8 +149,7 @@ class OneToManyPersister extends AbstractCollectionPersister public function slice(PersistentCollection $coll, $offset, $length = null) { $mapping = $coll->getMapping(); - $uow = $this->em->getUnitOfWork(); - $persister = $uow->getEntityPersister($mapping['targetEntity']); + $persister = $this->uow->getEntityPersister($mapping['targetEntity']); return $persister->getOneToManyCollection($mapping, $coll->getOwner(), $offset, $length); } @@ -163,8 +160,7 @@ class OneToManyPersister extends AbstractCollectionPersister public function containsKey(PersistentCollection $coll, $key) { $mapping = $coll->getMapping(); - $uow = $this->em->getUnitOfWork(); - $persister = $uow->getEntityPersister($mapping['targetEntity']); + $persister = $this->uow->getEntityPersister($mapping['targetEntity']); // only works with single id identifier entities. Will throw an // exception in Entity Persisters if that is not the case for the @@ -182,22 +178,19 @@ class OneToManyPersister extends AbstractCollectionPersister */ public function contains(PersistentCollection $coll, $element) { - $mapping = $coll->getMapping(); - $uow = $this->em->getUnitOfWork(); - - // shortcut for new entities - $entityState = $uow->getEntityState($element, UnitOfWork::STATE_NEW); + $entityState = $this->uow->getEntityState($element, UnitOfWork::STATE_NEW); if ($entityState === UnitOfWork::STATE_NEW) { return false; } // Entity is scheduled for inclusion - if ($entityState === UnitOfWork::STATE_MANAGED && $uow->isScheduledForInsert($element)) { + if ($entityState === UnitOfWork::STATE_MANAGED && $this->uow->isScheduledForInsert($element)) { return false; } - $persister = $uow->getEntityPersister($mapping['targetEntity']); + $mapping = $coll->getMapping(); + $persister = $this->uow->getEntityPersister($mapping['targetEntity']); // only works with single id identifier entities. Will throw an // exception in Entity Persisters if that is not the case for the @@ -212,10 +205,7 @@ class OneToManyPersister extends AbstractCollectionPersister */ public function removeElement(PersistentCollection $coll, $element) { - $uow = $this->em->getUnitOfWork(); - - // shortcut for new entities - $entityState = $uow->getEntityState($element, UnitOfWork::STATE_NEW); + $entityState = $this->uow->getEntityState($element, UnitOfWork::STATE_NEW); if ($entityState === UnitOfWork::STATE_NEW) { return false; @@ -223,12 +213,12 @@ class OneToManyPersister extends AbstractCollectionPersister // If Entity is scheduled for inclusion, it is not in this collection. // We can assure that because it would have return true before on array check - if ($entityState === UnitOfWork::STATE_MANAGED && $uow->isScheduledForInsert($element)) { + if ($entityState === UnitOfWork::STATE_MANAGED && $this->uow->isScheduledForInsert($element)) { return false; } $mapping = $coll->getMapping(); - $persister = $uow->getEntityPersister($mapping['targetEntity']); + $persister = $this->uow->getEntityPersister($mapping['targetEntity']); return $persister->delete($element); } diff --git a/tests/Doctrine/Tests/Models/DDC2504/DDC2504ChildClass.php b/tests/Doctrine/Tests/Models/DDC2504/DDC2504ChildClass.php index e8578cab3..a43de5bbf 100644 --- a/tests/Doctrine/Tests/Models/DDC2504/DDC2504ChildClass.php +++ b/tests/Doctrine/Tests/Models/DDC2504/DDC2504ChildClass.php @@ -1,8 +1,10 @@ childClasses = new \Doctrine\Common\Collections\ArrayCollection(); } - - public function addChildClass($childClass) - { - $this->childClasses[] = $childClass; - } - - public function getChildClasses() - { - return $this->childClasses; - } } diff --git a/tests/Doctrine/Tests/Models/DDC2504/DDC2504RootClass.php b/tests/Doctrine/Tests/Models/DDC2504/DDC2504RootClass.php index 55664b8fa..18edd75a3 100644 --- a/tests/Doctrine/Tests/Models/DDC2504/DDC2504RootClass.php +++ b/tests/Doctrine/Tests/Models/DDC2504/DDC2504RootClass.php @@ -25,4 +25,4 @@ class DDC2504RootClass * @ManyToOne(targetEntity="DDC2504OtherClass", inversedBy="childClasses") */ public $other; -} \ No newline at end of file +} diff --git a/tests/Doctrine/Tests/ORM/Functional/ExtraLazyCollectionTest.php b/tests/Doctrine/Tests/ORM/Functional/ExtraLazyCollectionTest.php index cac9176b7..634f34833 100644 --- a/tests/Doctrine/Tests/ORM/Functional/ExtraLazyCollectionTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/ExtraLazyCollectionTest.php @@ -141,8 +141,8 @@ class ExtraLazyCollectionTest extends \Doctrine\Tests\OrmFunctionalTestCase { $otherClass = $this->_em->find('Doctrine\Tests\Models\DDC2504\DDC2504OtherClass', $this->ddc2504OtherClassId); - $this->assertFalse($otherClass->getChildClasses()->isInitialized(), "Pre-Condition"); - $this->assertEquals(2, count($otherClass->getChildClasses())); + $this->assertFalse($otherClass->childClasses->isInitialized(), "Pre-Condition"); + $this->assertEquals(2, count($otherClass->childClasses)); } /** @@ -299,21 +299,21 @@ class ExtraLazyCollectionTest extends \Doctrine\Tests\OrmFunctionalTestCase public function testContainsOneToManyJoinedInheritance() { $otherClass = $this->_em->find('Doctrine\Tests\Models\DDC2504\DDC2504OtherClass', $this->ddc2504OtherClassId); - $this->assertFalse($otherClass->getChildClasses()->isInitialized(), "Pre-Condition: Collection is not initialized."); + $this->assertFalse($otherClass->childClasses->isInitialized(), "Pre-Condition: Collection is not initialized."); // Test One to Many existence retrieved from DB $childClass = $this->_em->find('Doctrine\Tests\Models\DDC2504\DDC2504ChildClass', $this->ddc2504ChildClassId); $queryCount = $this->getCurrentQueryCount(); - $this->assertTrue($otherClass->getChildClasses()->contains($childClass)); - $this->assertFalse($otherClass->getChildClasses()->isInitialized(), "Post-Condition: Collection is not initialized."); + $this->assertTrue($otherClass->childClasses->contains($childClass)); + $this->assertFalse($otherClass->childClasses->isInitialized(), "Post-Condition: Collection is not initialized."); $this->assertEquals($queryCount + 1, $this->getCurrentQueryCount()); // Test One to Many existence with state new $childClass = new \Doctrine\Tests\Models\DDC2504\DDC2504ChildClass(); $queryCount = $this->getCurrentQueryCount(); - $this->assertFalse($otherClass->getChildClasses()->contains($childClass)); + $this->assertFalse($otherClass->childClasses->contains($childClass)); $this->assertEquals($queryCount, $this->getCurrentQueryCount(), "Checking for contains of new entity should cause no query to be executed."); // Test One to Many existence with state clear @@ -321,9 +321,9 @@ class ExtraLazyCollectionTest extends \Doctrine\Tests\OrmFunctionalTestCase $this->_em->flush(); $queryCount = $this->getCurrentQueryCount(); - $this->assertFalse($otherClass->getChildClasses()->contains($childClass)); + $this->assertFalse($otherClass->childClasses->contains($childClass)); $this->assertEquals($queryCount+1, $this->getCurrentQueryCount(), "Checking for contains of persisted entity should cause one query to be executed."); - $this->assertFalse($otherClass->getChildClasses()->isInitialized(), "Post-Condition: Collection is not initialized."); + $this->assertFalse($otherClass->childClasses->isInitialized(), "Post-Condition: Collection is not initialized."); // Test One to Many existence with state managed $childClass = new \Doctrine\Tests\Models\DDC2504\DDC2504ChildClass(); @@ -332,12 +332,12 @@ class ExtraLazyCollectionTest extends \Doctrine\Tests\OrmFunctionalTestCase $queryCount = $this->getCurrentQueryCount(); - $this->assertFalse($otherClass->getChildClasses()->contains($childClass)); + $this->assertFalse($otherClass->childClasses->contains($childClass)); $this->assertEquals($queryCount, $this->getCurrentQueryCount(), "Checking for contains of managed entity (but not persisted) should cause no query to be executed."); - $this->assertFalse($otherClass->getChildClasses()->isInitialized(), "Post-Condition: Collection is not initialized."); + $this->assertFalse($otherClass->childClasses->isInitialized(), "Post-Condition: Collection is not initialized."); - $this->assertFalse($otherClass->getChildClasses()->isInitialized(), "Pre-Condition"); - $this->assertEquals(2, count($otherClass->getChildClasses())); + $this->assertFalse($otherClass->childClasses->isInitialized(), "Pre-Condition"); + $this->assertEquals(2, count($otherClass->childClasses)); } /** @@ -472,15 +472,15 @@ class ExtraLazyCollectionTest extends \Doctrine\Tests\OrmFunctionalTestCase public function testRemoveElementOneToManyJoinedInheritance() { $otherClass = $this->_em->find('Doctrine\Tests\Models\DDC2504\DDC2504OtherClass', $this->ddc2504OtherClassId); - $this->assertFalse($otherClass->getChildClasses()->isInitialized(), "Pre-Condition: Collection is not initialized."); + $this->assertFalse($otherClass->childClasses->isInitialized(), "Pre-Condition: Collection is not initialized."); // Test One to Many removal with Entity retrieved from DB $childClass = $this->_em->find('Doctrine\Tests\Models\DDC2504\DDC2504ChildClass', $this->ddc2504ChildClassId); $queryCount = $this->getCurrentQueryCount(); - $otherClass->getChildClasses()->removeElement($childClass); + $otherClass->childClasses->removeElement($childClass); - $this->assertFalse($otherClass->getChildClasses()->isInitialized(), "Post-Condition: Collection is not initialized."); + $this->assertFalse($otherClass->childClasses->isInitialized(), "Post-Condition: Collection is not initialized."); $this->assertEquals($queryCount + 2, $this->getCurrentQueryCount()); // Test One to Many removal with Entity state as new @@ -488,7 +488,7 @@ class ExtraLazyCollectionTest extends \Doctrine\Tests\OrmFunctionalTestCase $queryCount = $this->getCurrentQueryCount(); - $otherClass->getChildClasses()->removeElement($childClass); + $otherClass->childClasses->removeElement($childClass); $this->assertEquals($queryCount, $this->getCurrentQueryCount(), "Removing a new entity should cause no query to be executed."); @@ -498,10 +498,10 @@ class ExtraLazyCollectionTest extends \Doctrine\Tests\OrmFunctionalTestCase $queryCount = $this->getCurrentQueryCount(); - $otherClass->getChildClasses()->removeElement($childClass); + $otherClass->childClasses->removeElement($childClass); $this->assertEquals($queryCount + 2, $this->getCurrentQueryCount(), "Removing a persisted entity should cause two queries to be executed."); - $this->assertFalse($otherClass->getChildClasses()->isInitialized(), "Post-Condition: Collection is not initialized."); + $this->assertFalse($otherClass->childClasses->isInitialized(), "Post-Condition: Collection is not initialized."); // Test One to Many removal with Entity state as managed $childClass = new \Doctrine\Tests\Models\DDC2504\DDC2504ChildClass(); @@ -510,7 +510,7 @@ class ExtraLazyCollectionTest extends \Doctrine\Tests\OrmFunctionalTestCase $queryCount = $this->getCurrentQueryCount(); - $otherClass->getChildClasses()->removeElement($childClass); + $otherClass->childClasses->removeElement($childClass); $this->assertEquals($queryCount, $this->getCurrentQueryCount(), "Removing a managed entity should cause no query to be executed."); } @@ -708,10 +708,10 @@ class ExtraLazyCollectionTest extends \Doctrine\Tests\OrmFunctionalTestCase $queryCount = $this->getCurrentQueryCount(); - $contains = $otherClass->getChildClasses()->containsKey($this->ddc2504ChildClassId); + $contains = $otherClass->childClasses->containsKey($this->ddc2504ChildClassId); $this->assertTrue($contains); - $this->assertFalse($otherClass->getChildClasses()->isInitialized()); + $this->assertFalse($otherClass->childClasses->isInitialized()); $this->assertEquals($queryCount + 1, $this->getCurrentQueryCount()); } @@ -881,8 +881,8 @@ class ExtraLazyCollectionTest extends \Doctrine\Tests\OrmFunctionalTestCase $childClass1->other = $otherClass; $childClass2->other = $otherClass; - $otherClass->addChildClass($childClass1); - $otherClass->addChildClass($childClass2); + $otherClass->childClasses[] = $childClass1; + $otherClass->childClasses[] = $childClass2; $this->_em->persist($childClass1); $this->_em->persist($childClass2);