From 73afcec22aa6d681095b6d44deed61bffc1605d2 Mon Sep 17 00:00:00 2001 From: Guilherme Blanco Date: Mon, 12 Jan 2015 18:15:13 +0000 Subject: [PATCH 1/9] Implemented support for one to many extra lazy with joined inheritance. --- .../AbstractCollectionPersister.php | 2 +- .../ORM/Persisters/BasicEntityPersister.php | 3 +- .../ORM/Persisters/EntityPersister.php | 3 +- .../Persisters/JoinedSubclassPersister.php | 8 +- .../ORM/Persisters/OneToManyPersister.php | 74 +++------ .../Models/DDC2504/DDC2504ChildClass.php | 8 + .../Models/DDC2504/DDC2504OtherClass.php | 37 +++++ .../Tests/Models/DDC2504/DDC2504RootClass.php | 28 ++++ .../Functional/ExtraLazyCollectionTest.php | 143 ++++++++++++++++++ .../Doctrine/Tests/OrmFunctionalTestCase.php | 7 +- 10 files changed, 254 insertions(+), 59 deletions(-) create mode 100644 tests/Doctrine/Tests/Models/DDC2504/DDC2504ChildClass.php create mode 100644 tests/Doctrine/Tests/Models/DDC2504/DDC2504OtherClass.php create mode 100644 tests/Doctrine/Tests/Models/DDC2504/DDC2504RootClass.php diff --git a/lib/Doctrine/ORM/Persisters/AbstractCollectionPersister.php b/lib/Doctrine/ORM/Persisters/AbstractCollectionPersister.php index 0b1dcfbc3..c89974a52 100644 --- a/lib/Doctrine/ORM/Persisters/AbstractCollectionPersister.php +++ b/lib/Doctrine/ORM/Persisters/AbstractCollectionPersister.php @@ -85,7 +85,7 @@ abstract class AbstractCollectionPersister implements CollectionPersister return; // ignore inverse side } - $this->conn->executeUpdate($this->getDeleteSQL($coll), $this->getDeleteSQLParameters($coll)); + return $this->conn->executeUpdate($this->getDeleteSQL($coll), $this->getDeleteSQLParameters($coll)); } /** diff --git a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php index 7fd070f82..06c0923a9 100644 --- a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php +++ b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php @@ -585,7 +585,8 @@ class BasicEntityPersister implements EntityPersister }, $class->identifier); $this->deleteJoinTableRecords($identifier); - $this->conn->delete($tableName, $id, $types); + + return (bool) $this->conn->delete($tableName, $id, $types); } /** diff --git a/lib/Doctrine/ORM/Persisters/EntityPersister.php b/lib/Doctrine/ORM/Persisters/EntityPersister.php index bc2685852..3cc824e39 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 void + * @return bool */ public function delete($entity); @@ -161,6 +161,7 @@ interface EntityPersister * Count entities (optionally filtered by a criteria) * * @param array|\Doctrine\Common\Collections\Criteria $criteria + * * @return int */ public function count($criteria = array()); diff --git a/lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php b/lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php index fc90d6cad..d2321dc10 100644 --- a/lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php +++ b/lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php @@ -275,15 +275,13 @@ class JoinedSubclassPersister extends AbstractEntityInheritancePersister $rootClass = $this->em->getClassMetadata($this->class->rootEntityName); $rootTable = $this->quoteStrategy->getTableName($rootClass, $this->platform); - $this->conn->delete($rootTable, $id); - - return; + return (bool) $this->conn->delete($rootTable, $id); } // Delete from all tables individually, starting from this class' table up to the root table. $rootTable = $this->quoteStrategy->getTableName($this->class, $this->platform); - $this->conn->delete($rootTable, $id); + $affectedRows = $this->conn->delete($rootTable, $id); foreach ($this->class->parentClasses as $parentClass) { $parentMetadata = $this->em->getClassMetadata($parentClass); @@ -291,6 +289,8 @@ class JoinedSubclassPersister extends AbstractEntityInheritancePersister $this->conn->delete($parentTable, $id); } + + return (bool) $affectedRows; } /** diff --git a/lib/Doctrine/ORM/Persisters/OneToManyPersister.php b/lib/Doctrine/ORM/Persisters/OneToManyPersister.php index ef5703a6a..a2f22e75b 100644 --- a/lib/Doctrine/ORM/Persisters/OneToManyPersister.php +++ b/lib/Doctrine/ORM/Persisters/OneToManyPersister.php @@ -115,7 +115,7 @@ class OneToManyPersister extends AbstractCollectionPersister */ protected function getDeleteSQL(PersistentCollection $coll) { - throw new \BadMethodCallException("Update Row SQL is not used for OneToManyPersister"); + throw new \BadMethodCallException("Delete Row SQL is not used for OneToManyPersister"); } /** @@ -125,7 +125,7 @@ class OneToManyPersister extends AbstractCollectionPersister */ protected function getDeleteSQLParameters(PersistentCollection $coll) { - throw new \BadMethodCallException("Update Row SQL is not used for OneToManyPersister"); + throw new \BadMethodCallException("Delete Row SQL is not used for OneToManyPersister"); } /** @@ -133,11 +133,16 @@ class OneToManyPersister extends AbstractCollectionPersister */ public function count(PersistentCollection $coll) { - list($quotedJoinTable, $whereClauses, $params) = $this->getJoinTableRestrictions($coll, true); + $mapping = $coll->getMapping(); + $uow = $this->em->getUnitOfWork(); + $persister = $uow->getEntityPersister($mapping['targetEntity']); - $sql = 'SELECT count(*) FROM ' . $quotedJoinTable . ' WHERE ' . implode(' AND ', $whereClauses); + // only works with single id identifier entities. Will throw an + // exception in Entity Persisters if that is not the case for the + // 'mappedBy' field. + $criteria = new Criteria(Criteria::expr()->eq($mapping['mappedBy'], $coll->getOwner())); - return $this->conn->fetchColumn($sql, $params); + return $persister->count($criteria); } /** @@ -157,50 +162,19 @@ class OneToManyPersister extends AbstractCollectionPersister */ public function containsKey(PersistentCollection $coll, $key) { - list($quotedJoinTable, $whereClauses, $params) = $this->getJoinTableRestrictions($coll, true); + $mapping = $coll->getMapping(); + $uow = $this->em->getUnitOfWork(); + $persister = $uow->getEntityPersister($mapping['targetEntity']); - $mapping = $coll->getMapping(); - $sourceClass = $this->em->getClassMetadata($mapping['sourceEntity']); + // only works with single id identifier entities. Will throw an + // exception in Entity Persisters if that is not the case for the + // 'mappedBy' field. + $criteria = new Criteria(); - $whereClauses[] = $sourceClass->getColumnName($mapping['indexBy']) . ' = ?'; - $params[] = $key; + $criteria->andWhere(Criteria::expr()->eq($mapping['mappedBy'], $coll->getOwner())); + $criteria->andWhere(Criteria::expr()->eq($mapping['indexBy'], $key)); - $sql = 'SELECT 1 FROM ' . $quotedJoinTable . ' WHERE ' . implode(' AND ', $whereClauses); - - return (bool) $this->conn->fetchColumn($sql, $params); - } - - private function getJoinTableRestrictions(PersistentCollection $coll, $addFilters) - { - $mapping = $coll->getMapping(); - $targetClass = $this->em->getClassMetadata($mapping['targetEntity']); - $sourceClass = $this->em->getClassMetadata($mapping['sourceEntity']); - $id = $this->em->getUnitOfWork()->getEntityIdentifier($coll->getOwner()); - - $whereClauses = array(); - $params = array(); - - $joinColumns = $targetClass->associationMappings[$mapping['mappedBy']]['joinColumns']; - foreach ($joinColumns as $joinColumn) { - $whereClauses[] = $joinColumn['name'] . ' = ?'; - - $params[] = ($targetClass->containsForeignIdentifier) - ? $id[$sourceClass->getFieldForColumn($joinColumn['referencedColumnName'])] - : $id[$sourceClass->fieldNames[$joinColumn['referencedColumnName']]]; - } - - if ($addFilters) { - $filterTargetClass = $this->em->getClassMetadata($targetClass->rootEntityName); - foreach ($this->em->getFilters()->getEnabledFilters() as $filter) { - if ($filterExpr = $filter->addFilterConstraint($filterTargetClass, 't')) { - $whereClauses[] = '(' . $filterExpr . ')'; - } - } - } - - $quotedJoinTable = $this->quoteStrategy->getTableName($targetClass, $this->platform) . ' t'; - - return array($quotedJoinTable, $whereClauses, $params); + return (bool) $persister->count($criteria); } /** @@ -253,11 +227,9 @@ class OneToManyPersister extends AbstractCollectionPersister return false; } - $mapping = $coll->getMapping(); - $class = $this->em->getClassMetadata($mapping['targetEntity']); - $sql = 'DELETE FROM ' . $this->quoteStrategy->getTableName($class, $this->platform) - . ' WHERE ' . implode('= ? AND ', $class->getIdentifierColumnNames()) . ' = ?'; + $mapping = $coll->getMapping(); + $persister = $uow->getEntityPersister($mapping['targetEntity']); - return (bool) $this->conn->executeUpdate($sql, $this->getDeleteRowSQLParameters($coll, $element)); + return $persister->delete($element); } } diff --git a/tests/Doctrine/Tests/Models/DDC2504/DDC2504ChildClass.php b/tests/Doctrine/Tests/Models/DDC2504/DDC2504ChildClass.php new file mode 100644 index 000000000..e8578cab3 --- /dev/null +++ b/tests/Doctrine/Tests/Models/DDC2504/DDC2504ChildClass.php @@ -0,0 +1,8 @@ +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 new file mode 100644 index 000000000..55664b8fa --- /dev/null +++ b/tests/Doctrine/Tests/Models/DDC2504/DDC2504RootClass.php @@ -0,0 +1,28 @@ +useModelSet('cms'); + $this->useModelSet('ddc2504'); parent::setUp(); $class = $this->_em->getClassMetadata('Doctrine\Tests\Models\CMS\CmsUser'); @@ -131,6 +134,17 @@ class ExtraLazyCollectionTest extends \Doctrine\Tests\OrmFunctionalTestCase $this->assertEquals(2, count($user->articles)); } + /** + * @group DDC-2504 + */ + public function testCountOneToManyJoinedInheritance() + { + $otherClass = $this->_em->find('Doctrine\Tests\Models\DDC2504\DDC2504OtherClass', $this->ddc2504OtherClassId); + + $this->assertFalse($otherClass->getChildClasses()->isInitialized(), "Pre-Condition"); + $this->assertEquals(2, count($otherClass->getChildClasses())); + } + /** * @group DDC-546 */ @@ -279,6 +293,53 @@ class ExtraLazyCollectionTest extends \Doctrine\Tests\OrmFunctionalTestCase $this->assertFalse($user->articles->isInitialized(), "Post-Condition: Collection is not initialized."); } + /** + * @group DDC-2504 + */ + 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."); + + // 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->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->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 + $this->_em->persist($childClass); + $this->_em->flush(); + + $queryCount = $this->getCurrentQueryCount(); + $this->assertFalse($otherClass->getChildClasses()->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."); + + // Test One to Many existence with state managed + $childClass = new \Doctrine\Tests\Models\DDC2504\DDC2504ChildClass(); + + $this->_em->persist($childClass); + + $queryCount = $this->getCurrentQueryCount(); + + $this->assertFalse($otherClass->getChildClasses()->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->getChildClasses()->isInitialized(), "Pre-Condition"); + $this->assertEquals(2, count($otherClass->getChildClasses())); + } + /** * @group DDC-546 */ @@ -405,6 +466,55 @@ class ExtraLazyCollectionTest extends \Doctrine\Tests\OrmFunctionalTestCase $this->assertEquals($queryCount, $this->getCurrentQueryCount(), "Removing a managed entity should cause no query to be executed."); } + /** + * + */ + 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."); + + // 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); + + $this->assertFalse($otherClass->getChildClasses()->isInitialized(), "Post-Condition: Collection is not initialized."); + $this->assertEquals($queryCount + 2, $this->getCurrentQueryCount()); + + // Test One to Many removal with Entity state as new + $childClass = new \Doctrine\Tests\Models\DDC2504\DDC2504ChildClass(); + + $queryCount = $this->getCurrentQueryCount(); + + $otherClass->getChildClasses()->removeElement($childClass); + + $this->assertEquals($queryCount, $this->getCurrentQueryCount(), "Removing a new entity should cause no query to be executed."); + + // Test One to Many removal with Entity state as clean + $this->_em->persist($childClass); + $this->_em->flush(); + + $queryCount = $this->getCurrentQueryCount(); + + $otherClass->getChildClasses()->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."); + + // Test One to Many removal with Entity state as managed + $childClass = new \Doctrine\Tests\Models\DDC2504\DDC2504ChildClass(); + + $this->_em->persist($childClass); + + $queryCount = $this->getCurrentQueryCount(); + + $otherClass->getChildClasses()->removeElement($childClass); + + $this->assertEquals($queryCount, $this->getCurrentQueryCount(), "Removing a managed entity should cause no query to be executed."); + } + /** * */ @@ -589,6 +699,22 @@ class ExtraLazyCollectionTest extends \Doctrine\Tests\OrmFunctionalTestCase $this->assertEquals($queryCount + 1, $this->getCurrentQueryCount()); } + public function testContainsKeyIndexByOneToManyJoinedInheritance() + { + $class = $this->_em->getClassMetadata('Doctrine\Tests\Models\DDC2504\DDC2504OtherClass'); + $class->associationMappings['childClasses']['indexBy'] = 'id'; + + $otherClass = $this->_em->find('Doctrine\Tests\Models\DDC2504\DDC2504OtherClass', $this->ddc2504OtherClassId); + + $queryCount = $this->getCurrentQueryCount(); + + $contains = $otherClass->getChildClasses()->containsKey($this->ddc2504ChildClassId); + + $this->assertTrue($contains); + $this->assertFalse($otherClass->getChildClasses()->isInitialized()); + $this->assertEquals($queryCount + 1, $this->getCurrentQueryCount()); + } + public function testContainsKeyIndexByManyToMany() { $user = $this->_em->find('Doctrine\Tests\Models\CMS\CmsUser', $this->userId2); @@ -747,6 +873,21 @@ class ExtraLazyCollectionTest extends \Doctrine\Tests\OrmFunctionalTestCase $user1->addPhonenumber($phonenumber1); + // DDC-2504 + $otherClass = new \Doctrine\Tests\Models\DDC2504\DDC2504OtherClass(); + $childClass1 = new \Doctrine\Tests\Models\DDC2504\DDC2504ChildClass(); + $childClass2 = new \Doctrine\Tests\Models\DDC2504\DDC2504ChildClass(); + + $childClass1->other = $otherClass; + $childClass2->other = $otherClass; + + $otherClass->addChildClass($childClass1); + $otherClass->addChildClass($childClass2); + + $this->_em->persist($childClass1); + $this->_em->persist($childClass2); + $this->_em->persist($otherClass); + $this->_em->flush(); $this->_em->clear(); @@ -757,6 +898,8 @@ class ExtraLazyCollectionTest extends \Doctrine\Tests\OrmFunctionalTestCase $this->topic = $article1->topic; $this->phonenumber = $phonenumber1->phonenumber; + $this->ddc2504OtherClassId = $otherClass->id; + $this->ddc2504ChildClassId = $childClass1->id; } } diff --git a/tests/Doctrine/Tests/OrmFunctionalTestCase.php b/tests/Doctrine/Tests/OrmFunctionalTestCase.php index 16d0d2573..65096415a 100644 --- a/tests/Doctrine/Tests/OrmFunctionalTestCase.php +++ b/tests/Doctrine/Tests/OrmFunctionalTestCase.php @@ -186,7 +186,12 @@ abstract class OrmFunctionalTestCase extends OrmTestCase 'tweet' => array( 'Doctrine\Tests\Models\Tweet\User', 'Doctrine\Tests\Models\Tweet\Tweet' - ) + ), + 'ddc2504' => array( + 'Doctrine\Tests\Models\DDC2504\DDC2504RootClass', + 'Doctrine\Tests\Models\DDC2504\DDC2504ChildClass', + 'Doctrine\Tests\Models\DDC2504\DDC2504OtherClass', + ), ); /** From c5f1b99721217bf5791b08dc0139773f6f06d7ab Mon Sep 17 00:00:00 2001 From: Guilherme Blanco Date: Mon, 12 Jan 2015 18:18:15 +0000 Subject: [PATCH 2/9] Fixed wrong return. --- lib/Doctrine/ORM/Persisters/AbstractCollectionPersister.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Doctrine/ORM/Persisters/AbstractCollectionPersister.php b/lib/Doctrine/ORM/Persisters/AbstractCollectionPersister.php index c89974a52..0b1dcfbc3 100644 --- a/lib/Doctrine/ORM/Persisters/AbstractCollectionPersister.php +++ b/lib/Doctrine/ORM/Persisters/AbstractCollectionPersister.php @@ -85,7 +85,7 @@ abstract class AbstractCollectionPersister implements CollectionPersister return; // ignore inverse side } - return $this->conn->executeUpdate($this->getDeleteSQL($coll), $this->getDeleteSQLParameters($coll)); + $this->conn->executeUpdate($this->getDeleteSQL($coll), $this->getDeleteSQLParameters($coll)); } /** From 932a56f26fd2e356d7a1584aed98cf80caa07693 Mon Sep 17 00:00:00 2001 From: Guilherme Blanco Date: Mon, 12 Jan 2015 19:52:27 +0000 Subject: [PATCH 3/9] 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); From 8d287b17d7884cea53037174f8d86e8bc25ac255 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Mon, 12 Jan 2015 21:29:48 +0100 Subject: [PATCH 4/9] #1245 DDC-2504 - splitting test method into single feature checks --- .../Functional/ExtraLazyCollectionTest.php | 66 ++++++++++++++----- 1 file changed, 50 insertions(+), 16 deletions(-) diff --git a/tests/Doctrine/Tests/ORM/Functional/ExtraLazyCollectionTest.php b/tests/Doctrine/Tests/ORM/Functional/ExtraLazyCollectionTest.php index 634f34833..5b5f1bd51 100644 --- a/tests/Doctrine/Tests/ORM/Functional/ExtraLazyCollectionTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/ExtraLazyCollectionTest.php @@ -3,6 +3,7 @@ namespace Doctrine\Tests\ORM\Functional; use Doctrine\ORM\Mapping\ClassMetadataInfo; +use Doctrine\Tests\Models\DDC2504\DDC2504ChildClass; /** * Description of ExtraLazyCollectionTest @@ -296,25 +297,52 @@ class ExtraLazyCollectionTest extends \Doctrine\Tests\OrmFunctionalTestCase /** * @group DDC-2504 */ - public function testContainsOneToManyJoinedInheritance() + public function testLazyOneToManyJoinedInheritanceIsLazilyInitialized() + { + $otherClass = $this->_em->find('Doctrine\Tests\Models\DDC2504\DDC2504OtherClass', $this->ddc2504OtherClassId); + + $this->assertFalse($otherClass->childClasses->isInitialized(), 'Collection is not initialized.'); + } + + /** + * @group DDC-2504 + */ + public function testContainsOnOneToManyJoinedInheritanceWillNotInitializeCollectionWhenMatchingItemIsFound() { $otherClass = $this->_em->find('Doctrine\Tests\Models\DDC2504\DDC2504OtherClass', $this->ddc2504OtherClassId); - $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->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(); + $this->assertFalse($otherClass->childClasses->isInitialized(), 'Collection is not initialized.'); + $this->assertEquals($queryCount + 1, $this->getCurrentQueryCount(), 'Search operation was performed via SQL'); + } + /** + * @group DDC-2504 + */ + public function testContainsOnOneToManyJoinedInheritanceWillNotCauseQueriesWhenNonPersistentItemIsMatched() + { + $otherClass = $this->_em->find('Doctrine\Tests\Models\DDC2504\DDC2504OtherClass', $this->ddc2504OtherClassId); $queryCount = $this->getCurrentQueryCount(); - $this->assertFalse($otherClass->childClasses->contains($childClass)); - $this->assertEquals($queryCount, $this->getCurrentQueryCount(), "Checking for contains of new entity should cause no query to be executed."); + + $this->assertFalse($otherClass->childClasses->contains(new DDC2504ChildClass())); + $this->assertEquals( + $queryCount, + $this->getCurrentQueryCount(), + 'Checking for contains of new entity should cause no query to be executed.' + ); + } + + /** + * @group DDC-2504 + */ + public function testContainsOnOneToManyJoinedInheritanceWillNotInitializeCollectionWithClearStateMatchingItem() + { + $otherClass = $this->_em->find('Doctrine\Tests\Models\DDC2504\DDC2504OtherClass', $this->ddc2504OtherClassId); + $childClass = new DDC2504ChildClass(); // Test One to Many existence with state clear $this->_em->persist($childClass); @@ -322,11 +350,17 @@ class ExtraLazyCollectionTest extends \Doctrine\Tests\OrmFunctionalTestCase $queryCount = $this->getCurrentQueryCount(); $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->assertEquals($queryCount + 1, $this->getCurrentQueryCount(), "Checking for contains of persisted entity should cause one query to be executed."); $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(); + /** + * @group DDC-2504 + */ + public function testContainsOnOneToManyJoinedInheritanceWillNotInitializeCollectionWithNewStateNotMatchingItem() + { + $otherClass = $this->_em->find('Doctrine\Tests\Models\DDC2504\DDC2504OtherClass', $this->ddc2504OtherClassId); + $childClass = new DDC2504ChildClass(); $this->_em->persist($childClass); @@ -484,7 +518,7 @@ class ExtraLazyCollectionTest extends \Doctrine\Tests\OrmFunctionalTestCase $this->assertEquals($queryCount + 2, $this->getCurrentQueryCount()); // Test One to Many removal with Entity state as new - $childClass = new \Doctrine\Tests\Models\DDC2504\DDC2504ChildClass(); + $childClass = new DDC2504ChildClass(); $queryCount = $this->getCurrentQueryCount(); @@ -504,7 +538,7 @@ class ExtraLazyCollectionTest extends \Doctrine\Tests\OrmFunctionalTestCase $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(); + $childClass = new DDC2504ChildClass(); $this->_em->persist($childClass); @@ -875,8 +909,8 @@ class ExtraLazyCollectionTest extends \Doctrine\Tests\OrmFunctionalTestCase // DDC-2504 $otherClass = new \Doctrine\Tests\Models\DDC2504\DDC2504OtherClass(); - $childClass1 = new \Doctrine\Tests\Models\DDC2504\DDC2504ChildClass(); - $childClass2 = new \Doctrine\Tests\Models\DDC2504\DDC2504ChildClass(); + $childClass1 = new DDC2504ChildClass(); + $childClass2 = new DDC2504ChildClass(); $childClass1->other = $otherClass; $childClass2->other = $otherClass; From 7a0cb1c370e9546981376b3b7da5197921b6a196 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Mon, 12 Jan 2015 21:34:12 +0100 Subject: [PATCH 5/9] #1245 DDC-2504 - splitting test method into a test about `count()` and one about `contains()` on a lazy persistent collection --- .../Tests/ORM/Functional/ExtraLazyCollectionTest.php | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/tests/Doctrine/Tests/ORM/Functional/ExtraLazyCollectionTest.php b/tests/Doctrine/Tests/ORM/Functional/ExtraLazyCollectionTest.php index 5b5f1bd51..90479ca39 100644 --- a/tests/Doctrine/Tests/ORM/Functional/ExtraLazyCollectionTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/ExtraLazyCollectionTest.php @@ -369,9 +369,18 @@ class ExtraLazyCollectionTest extends \Doctrine\Tests\OrmFunctionalTestCase $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->childClasses->isInitialized(), "Post-Condition: Collection is not initialized."); + } + + /** + * @group DDC-2504 + */ + public function testCountingOnOneToManyJoinedInheritanceWillNotInitializeCollection() + { + $otherClass = $this->_em->find('Doctrine\Tests\Models\DDC2504\DDC2504OtherClass', $this->ddc2504OtherClassId); - $this->assertFalse($otherClass->childClasses->isInitialized(), "Pre-Condition"); $this->assertEquals(2, count($otherClass->childClasses)); + + $this->assertFalse($otherClass->childClasses->isInitialized()); } /** From ec08286173711325b0a5c59fe6fad1547a697da0 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Mon, 12 Jan 2015 21:37:53 +0100 Subject: [PATCH 6/9] #1245 DDC-2504 - constants over string references --- .../Models/DDC2504/DDC2504ChildClass.php | 1 + .../Models/DDC2504/DDC2504OtherClass.php | 10 +++++-- .../Functional/ExtraLazyCollectionTest.php | 27 ++++++++++--------- 3 files changed, 23 insertions(+), 15 deletions(-) diff --git a/tests/Doctrine/Tests/Models/DDC2504/DDC2504ChildClass.php b/tests/Doctrine/Tests/Models/DDC2504/DDC2504ChildClass.php index a43de5bbf..08a4605b6 100644 --- a/tests/Doctrine/Tests/Models/DDC2504/DDC2504ChildClass.php +++ b/tests/Doctrine/Tests/Models/DDC2504/DDC2504ChildClass.php @@ -7,4 +7,5 @@ namespace Doctrine\Tests\Models\DDC2504; */ class DDC2504ChildClass extends DDC2504RootClass { + const CLASSNAME = __CLASS__; } diff --git a/tests/Doctrine/Tests/Models/DDC2504/DDC2504OtherClass.php b/tests/Doctrine/Tests/Models/DDC2504/DDC2504OtherClass.php index cc9b46c04..a855cdf8a 100644 --- a/tests/Doctrine/Tests/Models/DDC2504/DDC2504OtherClass.php +++ b/tests/Doctrine/Tests/Models/DDC2504/DDC2504OtherClass.php @@ -2,11 +2,15 @@ namespace Doctrine\Tests\Models\DDC2504; +use Doctrine\Common\Collections\ArrayCollection; + /** * @Entity */ class DDC2504OtherClass { + const CLASSNAME = __CLASS__; + /** * @Column(type="integer") * @Id @GeneratedValue @@ -14,14 +18,16 @@ class DDC2504OtherClass public $id; /** - * @var Doctrine\Tests\Models\DDC2504\DDC2504ChildClass + * @var \Doctrine\Tests\Models\DDC2504\DDC2504ChildClass * * @OneToMany(targetEntity="DDC2504ChildClass", mappedBy="other", fetch="EXTRA_LAZY") + * + * @var ArrayCollection|\Doctrine\ORM\PersistentCollection */ public $childClasses; public function __construct() { - $this->childClasses = new \Doctrine\Common\Collections\ArrayCollection(); + $this->childClasses = new ArrayCollection(); } } diff --git a/tests/Doctrine/Tests/ORM/Functional/ExtraLazyCollectionTest.php b/tests/Doctrine/Tests/ORM/Functional/ExtraLazyCollectionTest.php index 90479ca39..94dbfbf97 100644 --- a/tests/Doctrine/Tests/ORM/Functional/ExtraLazyCollectionTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/ExtraLazyCollectionTest.php @@ -4,6 +4,7 @@ namespace Doctrine\Tests\ORM\Functional; use Doctrine\ORM\Mapping\ClassMetadataInfo; use Doctrine\Tests\Models\DDC2504\DDC2504ChildClass; +use Doctrine\Tests\Models\DDC2504\DDC2504OtherClass; /** * Description of ExtraLazyCollectionTest @@ -140,7 +141,7 @@ class ExtraLazyCollectionTest extends \Doctrine\Tests\OrmFunctionalTestCase */ public function testCountOneToManyJoinedInheritance() { - $otherClass = $this->_em->find('Doctrine\Tests\Models\DDC2504\DDC2504OtherClass', $this->ddc2504OtherClassId); + $otherClass = $this->_em->find(DDC2504OtherClass::CLASSNAME, $this->ddc2504OtherClassId); $this->assertFalse($otherClass->childClasses->isInitialized(), "Pre-Condition"); $this->assertEquals(2, count($otherClass->childClasses)); @@ -299,7 +300,7 @@ class ExtraLazyCollectionTest extends \Doctrine\Tests\OrmFunctionalTestCase */ public function testLazyOneToManyJoinedInheritanceIsLazilyInitialized() { - $otherClass = $this->_em->find('Doctrine\Tests\Models\DDC2504\DDC2504OtherClass', $this->ddc2504OtherClassId); + $otherClass = $this->_em->find(DDC2504OtherClass::CLASSNAME, $this->ddc2504OtherClassId); $this->assertFalse($otherClass->childClasses->isInitialized(), 'Collection is not initialized.'); } @@ -309,10 +310,10 @@ class ExtraLazyCollectionTest extends \Doctrine\Tests\OrmFunctionalTestCase */ public function testContainsOnOneToManyJoinedInheritanceWillNotInitializeCollectionWhenMatchingItemIsFound() { - $otherClass = $this->_em->find('Doctrine\Tests\Models\DDC2504\DDC2504OtherClass', $this->ddc2504OtherClassId); + $otherClass = $this->_em->find(DDC2504OtherClass::CLASSNAME, $this->ddc2504OtherClassId); // Test One to Many existence retrieved from DB - $childClass = $this->_em->find('Doctrine\Tests\Models\DDC2504\DDC2504ChildClass', $this->ddc2504ChildClassId); + $childClass = $this->_em->find(DDC2504ChildClass::CLASSNAME, $this->ddc2504ChildClassId); $queryCount = $this->getCurrentQueryCount(); $this->assertTrue($otherClass->childClasses->contains($childClass)); @@ -325,7 +326,7 @@ class ExtraLazyCollectionTest extends \Doctrine\Tests\OrmFunctionalTestCase */ public function testContainsOnOneToManyJoinedInheritanceWillNotCauseQueriesWhenNonPersistentItemIsMatched() { - $otherClass = $this->_em->find('Doctrine\Tests\Models\DDC2504\DDC2504OtherClass', $this->ddc2504OtherClassId); + $otherClass = $this->_em->find(DDC2504OtherClass::CLASSNAME, $this->ddc2504OtherClassId); $queryCount = $this->getCurrentQueryCount(); $this->assertFalse($otherClass->childClasses->contains(new DDC2504ChildClass())); @@ -341,7 +342,7 @@ class ExtraLazyCollectionTest extends \Doctrine\Tests\OrmFunctionalTestCase */ public function testContainsOnOneToManyJoinedInheritanceWillNotInitializeCollectionWithClearStateMatchingItem() { - $otherClass = $this->_em->find('Doctrine\Tests\Models\DDC2504\DDC2504OtherClass', $this->ddc2504OtherClassId); + $otherClass = $this->_em->find(DDC2504OtherClass::CLASSNAME, $this->ddc2504OtherClassId); $childClass = new DDC2504ChildClass(); // Test One to Many existence with state clear @@ -359,7 +360,7 @@ class ExtraLazyCollectionTest extends \Doctrine\Tests\OrmFunctionalTestCase */ public function testContainsOnOneToManyJoinedInheritanceWillNotInitializeCollectionWithNewStateNotMatchingItem() { - $otherClass = $this->_em->find('Doctrine\Tests\Models\DDC2504\DDC2504OtherClass', $this->ddc2504OtherClassId); + $otherClass = $this->_em->find(DDC2504OtherClass::CLASSNAME, $this->ddc2504OtherClassId); $childClass = new DDC2504ChildClass(); $this->_em->persist($childClass); @@ -376,7 +377,7 @@ class ExtraLazyCollectionTest extends \Doctrine\Tests\OrmFunctionalTestCase */ public function testCountingOnOneToManyJoinedInheritanceWillNotInitializeCollection() { - $otherClass = $this->_em->find('Doctrine\Tests\Models\DDC2504\DDC2504OtherClass', $this->ddc2504OtherClassId); + $otherClass = $this->_em->find(DDC2504OtherClass::CLASSNAME, $this->ddc2504OtherClassId); $this->assertEquals(2, count($otherClass->childClasses)); @@ -514,11 +515,11 @@ class ExtraLazyCollectionTest extends \Doctrine\Tests\OrmFunctionalTestCase */ public function testRemoveElementOneToManyJoinedInheritance() { - $otherClass = $this->_em->find('Doctrine\Tests\Models\DDC2504\DDC2504OtherClass', $this->ddc2504OtherClassId); + $otherClass = $this->_em->find(DDC2504OtherClass::CLASSNAME, $this->ddc2504OtherClassId); $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); + $childClass = $this->_em->find(DDC2504ChildClass::CLASSNAME, $this->ddc2504ChildClassId); $queryCount = $this->getCurrentQueryCount(); $otherClass->childClasses->removeElement($childClass); @@ -744,10 +745,10 @@ class ExtraLazyCollectionTest extends \Doctrine\Tests\OrmFunctionalTestCase public function testContainsKeyIndexByOneToManyJoinedInheritance() { - $class = $this->_em->getClassMetadata('Doctrine\Tests\Models\DDC2504\DDC2504OtherClass'); + $class = $this->_em->getClassMetadata(DDC2504OtherClass::CLASSNAME); $class->associationMappings['childClasses']['indexBy'] = 'id'; - $otherClass = $this->_em->find('Doctrine\Tests\Models\DDC2504\DDC2504OtherClass', $this->ddc2504OtherClassId); + $otherClass = $this->_em->find(DDC2504OtherClass::CLASSNAME, $this->ddc2504OtherClassId); $queryCount = $this->getCurrentQueryCount(); @@ -917,7 +918,7 @@ class ExtraLazyCollectionTest extends \Doctrine\Tests\OrmFunctionalTestCase $user1->addPhonenumber($phonenumber1); // DDC-2504 - $otherClass = new \Doctrine\Tests\Models\DDC2504\DDC2504OtherClass(); + $otherClass = new DDC2504OtherClass(); $childClass1 = new DDC2504ChildClass(); $childClass2 = new DDC2504ChildClass(); From 25d40caf1e6c23c7c46f6f693def859155672c0e Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Mon, 12 Jan 2015 21:43:37 +0100 Subject: [PATCH 7/9] #1245 DDC-2504 - extracting test: Removing a managed item from a one-to-many extra-lazy association to a JTI does not initialize the collection --- .../Functional/ExtraLazyCollectionTest.php | 24 ++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/tests/Doctrine/Tests/ORM/Functional/ExtraLazyCollectionTest.php b/tests/Doctrine/Tests/ORM/Functional/ExtraLazyCollectionTest.php index 94dbfbf97..308b3aad2 100644 --- a/tests/Doctrine/Tests/ORM/Functional/ExtraLazyCollectionTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/ExtraLazyCollectionTest.php @@ -511,7 +511,29 @@ class ExtraLazyCollectionTest extends \Doctrine\Tests\OrmFunctionalTestCase } /** - * + * @group DDC-2504 + */ + public function testRemovalOfManagedElementFromOneToManyJoinedInheritanceCollectionDoesNotInitializeIt() + { + $otherClass = $this->_em->find(DDC2504OtherClass::CLASSNAME, $this->ddc2504OtherClassId); + $childClass = $this->_em->find(DDC2504ChildClass::CLASSNAME, $this->ddc2504ChildClassId); + + $queryCount = $this->getCurrentQueryCount(); + + $otherClass->childClasses->removeElement($childClass); + + $this->assertFalse($otherClass->childClasses->isInitialized(), 'Collection is not initialized.'); + $this->assertEquals( + $queryCount + 2, + $this->getCurrentQueryCount(), + 'One removal per table in the JTI has been executed' + ); + + $this->assertFalse($otherClass->childClasses->contains($childClass)); + } + + /** + * @group DDC-2504 */ public function testRemoveElementOneToManyJoinedInheritance() { From ebf581176125e6c290e080d3cfe74a2ae13ce334 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Mon, 12 Jan 2015 21:55:57 +0100 Subject: [PATCH 8/9] #1245 DDC-2504 - extracting test: Removing an unmanaged/persisted/new item from a one-to-many extra-lazy association to a JTI does not initialize the collection --- .../Functional/ExtraLazyCollectionTest.php | 61 +++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/tests/Doctrine/Tests/ORM/Functional/ExtraLazyCollectionTest.php b/tests/Doctrine/Tests/ORM/Functional/ExtraLazyCollectionTest.php index 308b3aad2..a94542949 100644 --- a/tests/Doctrine/Tests/ORM/Functional/ExtraLazyCollectionTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/ExtraLazyCollectionTest.php @@ -532,6 +532,67 @@ class ExtraLazyCollectionTest extends \Doctrine\Tests\OrmFunctionalTestCase $this->assertFalse($otherClass->childClasses->contains($childClass)); } + /** + * @group DDC-2504 + */ + public function testRemovalOfNonManagedElementFromOneToManyJoinedInheritanceCollectionDoesNotInitializeIt() + { + $otherClass = $this->_em->find(DDC2504OtherClass::CLASSNAME, $this->ddc2504OtherClassId); + $queryCount = $this->getCurrentQueryCount(); + + $otherClass->childClasses->removeElement(new DDC2504ChildClass()); + + $this->assertEquals( + $queryCount, + $this->getCurrentQueryCount(), + 'Removing an unmanaged entity should cause no query to be executed.' + ); + } + + /** + * @group DDC-2504 + */ + public function testRemovalOfNewElementFromOneToManyJoinedInheritanceCollectionDoesNotInitializeIt() + { + $otherClass = $this->_em->find(DDC2504OtherClass::CLASSNAME, $this->ddc2504OtherClassId); + $childClass = new DDC2504ChildClass(); + + $this->_em->persist($childClass); + + $queryCount = $this->getCurrentQueryCount(); + + $otherClass->childClasses->removeElement($childClass); + + $this->assertEquals( + $queryCount, + $this->getCurrentQueryCount(), + 'Removing a new entity should cause no query to be executed.' + ); + } + + /** + * @group DDC-2504 + */ + public function testRemovalOfNewManagedElementFromOneToManyJoinedInheritanceCollectionDoesNotInitializeIt() + { + $otherClass = $this->_em->find(DDC2504OtherClass::CLASSNAME, $this->ddc2504OtherClassId); + $childClass = new DDC2504ChildClass(); + + $this->_em->persist($childClass); + $this->_em->flush(); + + $queryCount = $this->getCurrentQueryCount(); + + $otherClass->childClasses->removeElement($childClass); + + $this->assertEquals( + $queryCount + 2, + $this->getCurrentQueryCount(), + 'Removing a persisted entity should cause two queries to be executed.' + ); + $this->assertFalse($otherClass->childClasses->isInitialized(), 'Collection is not initialized.'); + } + /** * @group DDC-2504 */ From 0f362b06501d0d43fed252cae2f21030a5d00adb Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Mon, 12 Jan 2015 22:03:54 +0100 Subject: [PATCH 9/9] #1245 DDC-2504 - removing duplicate test --- .../Functional/ExtraLazyCollectionTest.php | 49 ------------------- 1 file changed, 49 deletions(-) diff --git a/tests/Doctrine/Tests/ORM/Functional/ExtraLazyCollectionTest.php b/tests/Doctrine/Tests/ORM/Functional/ExtraLazyCollectionTest.php index a94542949..e724517e8 100644 --- a/tests/Doctrine/Tests/ORM/Functional/ExtraLazyCollectionTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/ExtraLazyCollectionTest.php @@ -593,55 +593,6 @@ class ExtraLazyCollectionTest extends \Doctrine\Tests\OrmFunctionalTestCase $this->assertFalse($otherClass->childClasses->isInitialized(), 'Collection is not initialized.'); } - /** - * @group DDC-2504 - */ - public function testRemoveElementOneToManyJoinedInheritance() - { - $otherClass = $this->_em->find(DDC2504OtherClass::CLASSNAME, $this->ddc2504OtherClassId); - $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(DDC2504ChildClass::CLASSNAME, $this->ddc2504ChildClassId); - $queryCount = $this->getCurrentQueryCount(); - - $otherClass->childClasses->removeElement($childClass); - - $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 - $childClass = new DDC2504ChildClass(); - - $queryCount = $this->getCurrentQueryCount(); - - $otherClass->childClasses->removeElement($childClass); - - $this->assertEquals($queryCount, $this->getCurrentQueryCount(), "Removing a new entity should cause no query to be executed."); - - // Test One to Many removal with Entity state as clean - $this->_em->persist($childClass); - $this->_em->flush(); - - $queryCount = $this->getCurrentQueryCount(); - - $otherClass->childClasses->removeElement($childClass); - - $this->assertEquals($queryCount + 2, $this->getCurrentQueryCount(), "Removing a persisted entity should cause two queries to be executed."); - $this->assertFalse($otherClass->childClasses->isInitialized(), "Post-Condition: Collection is not initialized."); - - // Test One to Many removal with Entity state as managed - $childClass = new DDC2504ChildClass(); - - $this->_em->persist($childClass); - - $queryCount = $this->getCurrentQueryCount(); - - $otherClass->childClasses->removeElement($childClass); - - $this->assertEquals($queryCount, $this->getCurrentQueryCount(), "Removing a managed entity should cause no query to be executed."); - } - /** * */