From 233b3cd0b927501e19666e7102073c6222a37b2b Mon Sep 17 00:00:00 2001 From: beberlei Date: Fri, 9 Jul 2010 22:55:30 +0200 Subject: [PATCH] DDC-130 - Add initial version of deleteJoinTableRecords code on the persisters, flanked by 4 tests. --- .../ORM/Persisters/BasicEntityPersister.php | 57 +++++++++++++++++-- .../Persisters/JoinedSubclassPersister.php | 8 +-- .../Functional/ClassTableInheritanceTest.php | 29 ++++++++++ .../ManyToManyBasicAssociationTest.php | 31 ++++++++++ .../Functional/SingleTableInheritanceTest.php | 14 +++++ 5 files changed, 131 insertions(+), 8 deletions(-) diff --git a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php index cad7a2ab0..e62e8bbbf 100644 --- a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php +++ b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php @@ -335,6 +335,55 @@ class BasicEntityPersister } } + private $requiredJoinTableDeletions = null; + + /** + * @todo Add check for platform if it supports foreign keys/cascading. + * @param array $identifier + * @return void + */ + protected function deleteJoinTableRecords($identifier) + { + if ($this->requiredJoinTableDeletions === null) { + $this->requiredJoinTableDeletions = array(); + foreach ($this->_class->associationMappings AS $mapping) { + /* @var $mapping \Doctrine\ORM\Mapping\AssociationMapping */ + if ($mapping->isManyToMany()) { + // TODO: Write test for composite keys + if (!$mapping->isOwningSide) { + $relatedClass = $this->_em->getClassMetadata($mapping->targetEntityName); + $mapping = $relatedClass->associationMappings[$mapping->mappedBy]; + $keys = array_keys($mapping->relationToTargetKeyColumns); + + // this is not semantically correct, onDelete should be an option of the joinColumns + // @todo optimize this (potentially in the validate association/metadata already) + foreach ($mapping->joinTable['inverseJoinColumns'] AS $joinColumn) { + if (strtoupper($joinColumn['onDelete']) == 'CASCADE') { + continue; + } + } + } else { + // this is not semantically correct, onDelete should be an option of the joinColumns + // @todo optimize this (potentially in the validate association/metadata already) + foreach ($mapping->joinTable['joinColumns'] AS $joinColumn) { + if (strtoupper($joinColumn['onDelete']) == 'CASCADE') { + continue; + } + } + + $keys = array_keys($mapping->relationToSourceKeyColumns); + } + $this->requiredJoinTableDeletions[$mapping->joinTable['name']] = $keys; + } + } + } + + foreach ($this->requiredJoinTableDeletions AS $table => $keys) { + $id = array_combine($keys, $identifier); + $this->_conn->delete($table, $id); + } + } + /** * Deletes a managed entity. * @@ -347,10 +396,10 @@ class BasicEntityPersister */ public function delete($entity) { - $id = array_combine( - $this->_class->getIdentifierColumnNames(), - $this->_em->getUnitOfWork()->getEntityIdentifier($entity) - ); + $identifier = $this->_em->getUnitOfWork()->getEntityIdentifier($entity); + $this->deleteJoinTableRecords($identifier); + + $id = array_combine($this->_class->getIdentifierColumnNames(), $identifier); $this->_conn->delete($this->_class->table['name'], $id); } diff --git a/lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php b/lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php index ab413b604..dc7d802a2 100644 --- a/lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php +++ b/lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php @@ -206,10 +206,10 @@ class JoinedSubclassPersister extends AbstractEntityInheritancePersister */ public function delete($entity) { - $id = array_combine( - $this->_class->getIdentifierColumnNames(), - $this->_em->getUnitOfWork()->getEntityIdentifier($entity) - ); + $identifier = $this->_em->getUnitOfWork()->getEntityIdentifier($entity); + $this->deleteJoinTableRecords($identifier); + + $id = array_combine($this->_class->getIdentifierColumnNames(), $identifier); // If the database platform supports FKs, just // delete the row from the root table. Cascades do the rest. diff --git a/tests/Doctrine/Tests/ORM/Functional/ClassTableInheritanceTest.php b/tests/Doctrine/Tests/ORM/Functional/ClassTableInheritanceTest.php index ea1ad15d8..78d5d7987 100644 --- a/tests/Doctrine/Tests/ORM/Functional/ClassTableInheritanceTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/ClassTableInheritanceTest.php @@ -294,4 +294,33 @@ class ClassTableInheritanceTest extends \Doctrine\Tests\OrmFunctionalTestCase ->getResult()) > 0); } + + /** + * @group DDC-130 + */ + public function testDeleteJoinTableRecords() + { + $this->markTestSkipped('Nightmare! friends adds both ID 6-7 and 7-6 into two rows of the join table. How to detect this?'); + + $employee1 = new CompanyEmployee(); + $employee1->setName('gblanco'); + $employee1->setSalary(0); + $employee1->setDepartment('IT'); + + $employee2 = new CompanyEmployee(); + $employee2->setName('jwage'); + $employee2->setSalary(0); + $employee2->setDepartment('IT'); + + $employee1->addFriend($employee2); + + $this->_em->persist($employee1); + $this->_em->persist($employee2); + $this->_em->flush(); + + $this->_em->remove($employee1); + $this->_em->flush(); + + $this->assertNull($this->_em->find(get_class($employee1), $employee1->getId())); + } } diff --git a/tests/Doctrine/Tests/ORM/Functional/ManyToManyBasicAssociationTest.php b/tests/Doctrine/Tests/ORM/Functional/ManyToManyBasicAssociationTest.php index 752d11fb8..83a522eb1 100644 --- a/tests/Doctrine/Tests/ORM/Functional/ManyToManyBasicAssociationTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/ManyToManyBasicAssociationTest.php @@ -203,6 +203,37 @@ class ManyToManyBasicAssociationTest extends \Doctrine\Tests\OrmFunctionalTestCa $this->assertEquals(3, count($freshUser->getGroups())); } + /** + * @group DDC-130 + */ + public function testRemoveUserWithManyGroups() + { + $user = $this->addCmsUserGblancoWithGroups(2); + + $this->_em->remove($user); + $this->_em->flush(); + + $newUser = $this->_em->find(get_class($user), $user->getId()); + $this->assertNull($newUser); + } + + /** + * @group DDC-130 + */ + public function testRemoveGroupWithUser() + { + $user = $this->addCmsUserGblancoWithGroups(2); + + foreach ($user->getGroups() AS $group) { + $this->_em->remove($group); + } + $this->_em->flush(); + $this->_em->clear(); + + $newUser = $this->_em->find(get_class($user), $user->getId()); + $this->assertEquals(0, count($newUser->getGroups())); + } + /** * @param int $groupCount * @return CmsUser diff --git a/tests/Doctrine/Tests/ORM/Functional/SingleTableInheritanceTest.php b/tests/Doctrine/Tests/ORM/Functional/SingleTableInheritanceTest.php index b2bd382c6..fb9c93abf 100644 --- a/tests/Doctrine/Tests/ORM/Functional/SingleTableInheritanceTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/SingleTableInheritanceTest.php @@ -294,4 +294,18 @@ class SingleTableInheritanceTest extends \Doctrine\Tests\OrmFunctionalTestCase $this->assertFalse($contracts[0]->isCompleted(), "Only non completed contracts should be left."); } + + /** + * @group DDC-130 + */ + public function testDeleteJoinTableRecords() + { + $this->loadFullFixture(); + + // remove managed copy of the fix contract + $this->_em->remove($this->_em->find(get_class($this->fix), $this->fix->getId())); + $this->_em->flush(); + + $this->assertNull($this->_em->find(get_class($this->fix), $this->fix->getId()), "Contract should not be present in the database anymore."); + } }