From 73afcec22aa6d681095b6d44deed61bffc1605d2 Mon Sep 17 00:00:00 2001 From: Guilherme Blanco Date: Mon, 12 Jan 2015 18:15:13 +0000 Subject: [PATCH] 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', + ), ); /**