From 2cfc61db8445be0696003cda2f9c55d70bec6480 Mon Sep 17 00:00:00 2001 From: Guilherme Blanco Date: Tue, 6 Sep 2011 01:58:16 -0300 Subject: [PATCH] Fixed bug with orphanRemoval not removing associated Entity on OneToMany and OneToOne relationships. As defined in ClassMatedataInfo, in these situations, when orphanRemoval=true, cascade=remove is implicit. This fixes DDC-1321. --- .../ORM/Mapping/ClassMetadataInfo.php | 10 ++-- tests/Doctrine/Tests/Models/CMS/CmsUser.php | 2 +- .../Functional/OneToManyOrphanRemovalTest.php | 58 ++++++++++++++++++ .../Functional/OneToOneOrphanRemovalTest.php | 60 +++++++++++++++++++ .../ORM/Mapping/AbstractMappingDriverTest.php | 2 +- .../AbstractClassMetadataExporterTest.php | 14 +++-- .../Doctrine.Tests.ORM.Tools.Export.User.php | 4 +- .../Doctrine.Tests.ORM.Tools.Export.User.php | 7 ++- ...ctrine.Tests.ORM.Tools.Export.User.dcm.xml | 7 ++- ...ctrine.Tests.ORM.Tools.Export.User.dcm.yml | 6 +- 10 files changed, 147 insertions(+), 23 deletions(-) create mode 100644 tests/Doctrine/Tests/ORM/Functional/OneToManyOrphanRemovalTest.php create mode 100644 tests/Doctrine/Tests/ORM/Functional/OneToOneOrphanRemovalTest.php diff --git a/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php b/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php index d791ce0dd..8bdf42305 100644 --- a/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php +++ b/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php @@ -908,9 +908,8 @@ class ClassMetadataInfo implements ClassMetadata $mapping['targetToSourceKeyColumns'] = array_flip($mapping['sourceToTargetKeyColumns']); } - //TODO: if orphanRemoval, cascade=remove is implicit! - $mapping['orphanRemoval'] = isset($mapping['orphanRemoval']) ? - (bool) $mapping['orphanRemoval'] : false; + $mapping['orphanRemoval'] = isset($mapping['orphanRemoval']) ? (bool) $mapping['orphanRemoval'] : false; + $mapping['isCascadeRemove'] = $mapping['orphanRemoval'] ? true : $mapping['isCascadeRemove']; if (isset($mapping['id']) && $mapping['id'] === true && !$mapping['isOwningSide']) { throw MappingException::illegalInverseIdentifierAssocation($this->name, $mapping['fieldName']); @@ -935,9 +934,8 @@ class ClassMetadataInfo implements ClassMetadata throw MappingException::oneToManyRequiresMappedBy($mapping['fieldName']); } - //TODO: if orphanRemoval, cascade=remove is implicit! - $mapping['orphanRemoval'] = isset($mapping['orphanRemoval']) ? - (bool) $mapping['orphanRemoval'] : false; + $mapping['orphanRemoval'] = isset($mapping['orphanRemoval']) ? (bool) $mapping['orphanRemoval'] : false; + $mapping['isCascadeRemove'] = $mapping['orphanRemoval'] ? true : $mapping['isCascadeRemove']; if (isset($mapping['orderBy'])) { if ( ! is_array($mapping['orderBy'])) { diff --git a/tests/Doctrine/Tests/Models/CMS/CmsUser.php b/tests/Doctrine/Tests/Models/CMS/CmsUser.php index d9ac982ff..4047a3635 100644 --- a/tests/Doctrine/Tests/Models/CMS/CmsUser.php +++ b/tests/Doctrine/Tests/Models/CMS/CmsUser.php @@ -31,7 +31,7 @@ class CmsUser */ public $name; /** - * @OneToMany(targetEntity="CmsPhonenumber", mappedBy="user", cascade={"persist", "remove", "merge"}, orphanRemoval=true) + * @OneToMany(targetEntity="CmsPhonenumber", mappedBy="user", cascade={"persist", "merge"}, orphanRemoval=true) */ public $phonenumbers; /** diff --git a/tests/Doctrine/Tests/ORM/Functional/OneToManyOrphanRemovalTest.php b/tests/Doctrine/Tests/ORM/Functional/OneToManyOrphanRemovalTest.php new file mode 100644 index 000000000..78b2fe517 --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Functional/OneToManyOrphanRemovalTest.php @@ -0,0 +1,58 @@ +useModelSet('cms'); + + parent::setUp(); + } + + public function testOrphanRemoval() + { + $user = new CmsUser; + $user->status = 'dev'; + $user->username = 'romanb'; + $user->name = 'Roman B.'; + + $phone = new CmsPhonenumber; + $phone->phonenumber = '123456'; + + $user->addPhonenumber($phone); + + $this->_em->persist($user); + $this->_em->flush(); + + $userId = $user->getId(); + + $this->_em->clear(); + + $userProxy = $this->_em->getReference('Doctrine\Tests\Models\CMS\CmsUser', $userId); + + $this->_em->remove($userProxy); + $this->_em->flush(); + $this->_em->clear(); + + $query = $this->_em->createQuery('SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u'); + $result = $query->getResult(); + + $this->assertEquals(0, count($result), 'CmsUser should be removed by EntityManager'); + + $query = $this->_em->createQuery('SELECT p FROM Doctrine\Tests\Models\CMS\CmsPhonenumber p'); + $result = $query->getResult(); + + $this->assertEquals(0, count($result), 'CmsPhonenumber should be removed by orphanRemoval'); + } +} \ No newline at end of file diff --git a/tests/Doctrine/Tests/ORM/Functional/OneToOneOrphanRemovalTest.php b/tests/Doctrine/Tests/ORM/Functional/OneToOneOrphanRemovalTest.php new file mode 100644 index 000000000..e82961aff --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Functional/OneToOneOrphanRemovalTest.php @@ -0,0 +1,60 @@ +useModelSet('cms'); + + parent::setUp(); + } + + public function testOrphanRemoval() + { + $user = new CmsUser; + $user->status = 'dev'; + $user->username = 'romanb'; + $user->name = 'Roman B.'; + + $address = new CmsAddress; + $address->country = 'de'; + $address->zip = 1234; + $address->city = 'Berlin'; + + $user->setAddress($address); + + $this->_em->persist($user); + $this->_em->flush(); + + $userId = $user->getId(); + + $this->_em->clear(); + + $userProxy = $this->_em->getReference('Doctrine\Tests\Models\CMS\CmsUser', $userId); + + $this->_em->remove($userProxy); + $this->_em->flush(); + $this->_em->clear(); + + $query = $this->_em->createQuery('SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u'); + $result = $query->getResult(); + + $this->assertEquals(0, count($result), 'CmsUser should be removed by EntityManager'); + + $query = $this->_em->createQuery('SELECT a FROM Doctrine\Tests\Models\CMS\CmsAddress a'); + $result = $query->getResult(); + + $this->assertEquals(0, count($result), 'CmsAddress should be removed by orphanRemoval'); + } +} \ No newline at end of file diff --git a/tests/Doctrine/Tests/ORM/Mapping/AbstractMappingDriverTest.php b/tests/Doctrine/Tests/ORM/Mapping/AbstractMappingDriverTest.php index 8037a7fa7..74e7764a9 100644 --- a/tests/Doctrine/Tests/ORM/Mapping/AbstractMappingDriverTest.php +++ b/tests/Doctrine/Tests/ORM/Mapping/AbstractMappingDriverTest.php @@ -182,7 +182,7 @@ abstract class AbstractMappingDriverTest extends \Doctrine\Tests\OrmTestCase $this->assertTrue(isset($class->associationMappings['phonenumbers'])); $this->assertFalse($class->associationMappings['phonenumbers']['isOwningSide']); $this->assertTrue($class->associationMappings['phonenumbers']['isCascadePersist']); - $this->assertFalse($class->associationMappings['phonenumbers']['isCascadeRemove']); + $this->assertTrue($class->associationMappings['phonenumbers']['isCascadeRemove']); $this->assertFalse($class->associationMappings['phonenumbers']['isCascadeRefresh']); $this->assertFalse($class->associationMappings['phonenumbers']['isCascadeDetach']); $this->assertFalse($class->associationMappings['phonenumbers']['isCascadeMerge']); diff --git a/tests/Doctrine/Tests/ORM/Tools/Export/AbstractClassMetadataExporterTest.php b/tests/Doctrine/Tests/ORM/Tools/Export/AbstractClassMetadataExporterTest.php index 0dfbcc2f3..c9bfdccf0 100644 --- a/tests/Doctrine/Tests/ORM/Tools/Export/AbstractClassMetadataExporterTest.php +++ b/tests/Doctrine/Tests/ORM/Tools/Export/AbstractClassMetadataExporterTest.php @@ -219,10 +219,11 @@ abstract class AbstractClassMetadataExporterTest extends \Doctrine\Tests\OrmTest $this->assertEquals('CASCADE', $class->associationMappings['address']['joinColumns'][0]['onDelete']); $this->assertTrue($class->associationMappings['address']['isCascadeRemove']); - $this->assertFalse($class->associationMappings['address']['isCascadePersist']); + $this->assertTrue($class->associationMappings['address']['isCascadePersist']); $this->assertFalse($class->associationMappings['address']['isCascadeRefresh']); $this->assertFalse($class->associationMappings['address']['isCascadeMerge']); $this->assertFalse($class->associationMappings['address']['isCascadeDetach']); + $this->assertTrue($class->associationMappings['address']['orphanRemoval']); return $class; } @@ -239,11 +240,12 @@ abstract class AbstractClassMetadataExporterTest extends \Doctrine\Tests\OrmTest $this->assertEquals('user', $class->associationMappings['phonenumbers']['mappedBy']); $this->assertEquals(array('number' => 'ASC'), $class->associationMappings['phonenumbers']['orderBy']); - $this->assertFalse($class->associationMappings['phonenumbers']['isCascadeRemove']); + $this->assertTrue($class->associationMappings['phonenumbers']['isCascadeRemove']); $this->assertTrue($class->associationMappings['phonenumbers']['isCascadePersist']); $this->assertFalse($class->associationMappings['phonenumbers']['isCascadeRefresh']); - $this->assertFalse($class->associationMappings['phonenumbers']['isCascadeMerge']); + $this->assertTrue($class->associationMappings['phonenumbers']['isCascadeMerge']); $this->assertFalse($class->associationMappings['phonenumbers']['isCascadeDetach']); + $this->assertTrue($class->associationMappings['phonenumbers']['orphanRemoval']); return $class; } @@ -300,9 +302,11 @@ abstract class AbstractClassMetadataExporterTest extends \Doctrine\Tests\OrmTest public function testCascadeIsExported($class) { $this->assertTrue($class->associationMappings['phonenumbers']['isCascadePersist']); - $this->assertFalse($class->associationMappings['phonenumbers']['isCascadeMerge']); - $this->assertFalse($class->associationMappings['phonenumbers']['isCascadeRemove']); + $this->assertTrue($class->associationMappings['phonenumbers']['isCascadeMerge']); + $this->assertTrue($class->associationMappings['phonenumbers']['isCascadeRemove']); $this->assertFalse($class->associationMappings['phonenumbers']['isCascadeRefresh']); + $this->assertFalse($class->associationMappings['phonenumbers']['isCascadeDetach']); + $this->assertTrue($class->associationMappings['phonenumbers']['orphanRemoval']); return $class; } diff --git a/tests/Doctrine/Tests/ORM/Tools/Export/annotation/Doctrine.Tests.ORM.Tools.Export.User.php b/tests/Doctrine/Tests/ORM/Tools/Export/annotation/Doctrine.Tests.ORM.Tools.Export.User.php index 4fb951087..ddeece824 100644 --- a/tests/Doctrine/Tests/ORM/Tools/Export/annotation/Doctrine.Tests.ORM.Tools.Export.User.php +++ b/tests/Doctrine/Tests/ORM/Tools/Export/annotation/Doctrine.Tests.ORM.Tools.Export.User.php @@ -23,14 +23,14 @@ class User public $email; /** - * @OneToOne(targetEntity="Doctrine\Tests\ORM\Tools\Export\Address", cascade={"remove"}, inversedBy="user") + * @OneToOne(targetEntity="Doctrine\Tests\ORM\Tools\Export\Address", inversedBy="user", cascade={"persist"}, orphanRemoval=true) * @JoinColumn(name="address_id", onDelete="CASCADE") */ public $address; /** * - * @OneToMany(targetEntity="Doctrine\Tests\ORM\Tools\Export\Phonenumber", mappedBy="user", cascade={"persist"}) + * @OneToMany(targetEntity="Doctrine\Tests\ORM\Tools\Export\Phonenumber", mappedBy="user", cascade={"persist", "merge"}, orphanRemoval=true) * @OrderBy({"number"="ASC"}) */ public $phonenumbers; diff --git a/tests/Doctrine/Tests/ORM/Tools/Export/php/Doctrine.Tests.ORM.Tools.Export.User.php b/tests/Doctrine/Tests/ORM/Tools/Export/php/Doctrine.Tests.ORM.Tools.Export.User.php index 0dc182168..c50d2a75b 100644 --- a/tests/Doctrine/Tests/ORM/Tools/Export/php/Doctrine.Tests.ORM.Tools.Export.User.php +++ b/tests/Doctrine/Tests/ORM/Tools/Export/php/Doctrine.Tests.ORM.Tools.Export.User.php @@ -37,7 +37,7 @@ $metadata->mapOneToOne(array( 'inversedBy' => 'user', 'cascade' => array( - 0 => 'remove', + 0 => 'persist', ), 'mappedBy' => NULL, 'joinColumns' => @@ -49,7 +49,7 @@ $metadata->mapOneToOne(array( 'onDelete' => 'CASCADE', ), ), - 'orphanRemoval' => false, + 'orphanRemoval' => true, )); $metadata->mapOneToMany(array( 'fieldName' => 'phonenumbers', @@ -57,9 +57,10 @@ $metadata->mapOneToMany(array( 'cascade' => array( 1 => 'persist', + 2 => 'merge', ), 'mappedBy' => 'user', - 'orphanRemoval' => false, + 'orphanRemoval' => true, 'orderBy' => array( 'number' => 'ASC', diff --git a/tests/Doctrine/Tests/ORM/Tools/Export/xml/Doctrine.Tests.ORM.Tools.Export.User.dcm.xml b/tests/Doctrine/Tests/ORM/Tools/Export/xml/Doctrine.Tests.ORM.Tools.Export.User.dcm.xml index de638b236..c562003c6 100644 --- a/tests/Doctrine/Tests/ORM/Tools/Export/xml/Doctrine.Tests.ORM.Tools.Export.User.dcm.xml +++ b/tests/Doctrine/Tests/ORM/Tools/Export/xml/Doctrine.Tests.ORM.Tools.Export.User.dcm.xml @@ -20,14 +20,15 @@ - - + + - + + diff --git a/tests/Doctrine/Tests/ORM/Tools/Export/yaml/Doctrine.Tests.ORM.Tools.Export.User.dcm.yml b/tests/Doctrine/Tests/ORM/Tools/Export/yaml/Doctrine.Tests.ORM.Tools.Export.User.dcm.yml index 4c169f8d4..9231bb189 100644 --- a/tests/Doctrine/Tests/ORM/Tools/Export/yaml/Doctrine.Tests.ORM.Tools.Export.User.dcm.yml +++ b/tests/Doctrine/Tests/ORM/Tools/Export/yaml/Doctrine.Tests.ORM.Tools.Export.User.dcm.yml @@ -23,15 +23,17 @@ Doctrine\Tests\ORM\Tools\Export\User: name: address_id referencedColumnName: id onDelete: CASCADE - cascade: [ remove ] + cascade: [ persist ] inversedBy: user + orphanRemoval: true oneToMany: phonenumbers: targetEntity: Doctrine\Tests\ORM\Tools\Export\Phonenumber mappedBy: user orderBy: number: ASC - cascade: [ persist ] + cascade: [ persist, merge ] + orphanRemoval: true manyToMany: groups: targetEntity: Doctrine\Tests\ORM\Tools\Export\Group