From 68436fee757a0b069429d3b496dabb6d9480afe1 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Mon, 20 Feb 2012 10:33:16 +0100 Subject: [PATCH] [DDC-1654] Add support for orphanRemoval on ManyToMany associations. This only makes sense when ManyToMany is used as uni-directional OneToMany association with join table. The join column has a unique constraint on it to enforce this on the DB level, but we dont validate that this actually happens. Foreign Key constraints help prevent issues and notify developers early if they use it wrong. --- doctrine-mapping.xsd | 1 + .../ORM/Mapping/ClassMetadataInfo.php | 4 +- .../ORM/Mapping/Driver/AnnotationDriver.php | 1 + lib/Doctrine/ORM/Mapping/Driver/XmlDriver.php | 4 + .../ORM/Mapping/Driver/YamlDriver.php | 4 + lib/Doctrine/ORM/Mapping/ManyToMany.php | 2 + lib/Doctrine/ORM/PersistentCollection.php | 6 +- .../ORM/Functional/Ticket/DDC1654Test.php | 103 ++++++++++++++++++ .../ORM/Mapping/ClassMetadataBuilderTest.php | 1 + .../Doctrine/Tests/OrmFunctionalTestCase.php | 25 +++++ 10 files changed, 147 insertions(+), 4 deletions(-) create mode 100644 tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1654Test.php diff --git a/doctrine-mapping.xsd b/doctrine-mapping.xsd index 0d3704a7e..96f155a60 100644 --- a/doctrine-mapping.xsd +++ b/doctrine-mapping.xsd @@ -371,6 +371,7 @@ + diff --git a/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php b/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php index 0c28ae473..69164bd6d 100644 --- a/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php +++ b/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php @@ -1115,7 +1115,7 @@ class ClassMetadataInfo implements ClassMetadata $mapping['targetEntity'] = ltrim($mapping['targetEntity'], '\\'); } - if ( ($mapping['type'] & (self::MANY_TO_ONE|self::MANY_TO_MANY)) > 0 && + if ( ($mapping['type'] & self::MANY_TO_ONE) > 0 && isset($mapping['orphanRemoval']) && $mapping['orphanRemoval'] == true) { @@ -1335,6 +1335,8 @@ class ClassMetadataInfo implements ClassMetadata } } + $mapping['orphanRemoval'] = isset($mapping['orphanRemoval']) ? (bool) $mapping['orphanRemoval'] : false; + if (isset($mapping['orderBy'])) { if ( ! is_array($mapping['orderBy'])) { throw new \InvalidArgumentException("'orderBy' is expected to be an array, not ".gettype($mapping['orderBy'])); diff --git a/lib/Doctrine/ORM/Mapping/Driver/AnnotationDriver.php b/lib/Doctrine/ORM/Mapping/Driver/AnnotationDriver.php index 93e8473a4..0b1ec83cb 100644 --- a/lib/Doctrine/ORM/Mapping/Driver/AnnotationDriver.php +++ b/lib/Doctrine/ORM/Mapping/Driver/AnnotationDriver.php @@ -412,6 +412,7 @@ class AnnotationDriver implements Driver $mapping['inversedBy'] = $manyToManyAnnot->inversedBy; $mapping['cascade'] = $manyToManyAnnot->cascade; $mapping['indexBy'] = $manyToManyAnnot->indexBy; + $mapping['orphanRemoval'] = $manyToManyAnnot->orphanRemoval; $mapping['fetch'] = $this->getFetchMode($className, $manyToManyAnnot->fetch); if ($orderByAnnot = $this->_reader->getPropertyAnnotation($property, 'Doctrine\ORM\Mapping\OrderBy')) { diff --git a/lib/Doctrine/ORM/Mapping/Driver/XmlDriver.php b/lib/Doctrine/ORM/Mapping/Driver/XmlDriver.php index b62df3893..4a07974d9 100644 --- a/lib/Doctrine/ORM/Mapping/Driver/XmlDriver.php +++ b/lib/Doctrine/ORM/Mapping/Driver/XmlDriver.php @@ -402,6 +402,10 @@ class XmlDriver extends AbstractFileDriver $mapping['fetch'] = constant('Doctrine\ORM\Mapping\ClassMetadata::FETCH_' . (string)$manyToManyElement['fetch']); } + if (isset($manyToManyElement['orphan-removal'])) { + $mapping['orphanRemoval'] = (bool)$manyToManyElement['orphan-removal']; + } + if (isset($manyToManyElement['mapped-by'])) { $mapping['mappedBy'] = (string)$manyToManyElement['mapped-by']; } else if (isset($manyToManyElement->{'join-table'})) { diff --git a/lib/Doctrine/ORM/Mapping/Driver/YamlDriver.php b/lib/Doctrine/ORM/Mapping/Driver/YamlDriver.php index ef0eff6ec..b66f29407 100644 --- a/lib/Doctrine/ORM/Mapping/Driver/YamlDriver.php +++ b/lib/Doctrine/ORM/Mapping/Driver/YamlDriver.php @@ -451,6 +451,10 @@ class YamlDriver extends AbstractFileDriver $mapping['indexBy'] = $manyToManyElement['indexBy']; } + if (isset($manyToManyElement['orphanRemoval'])) { + $mapping['orphanRemoval'] = (bool)$manyToManyElement['orphanRemoval']; + } + $metadata->mapManyToMany($mapping); } } diff --git a/lib/Doctrine/ORM/Mapping/ManyToMany.php b/lib/Doctrine/ORM/Mapping/ManyToMany.php index 1e2ae06c0..28f41aaff 100644 --- a/lib/Doctrine/ORM/Mapping/ManyToMany.php +++ b/lib/Doctrine/ORM/Mapping/ManyToMany.php @@ -35,6 +35,8 @@ final class ManyToMany implements Annotation public $cascade; /** @var string */ public $fetch = 'LAZY'; + /** @var boolean */ + public $orphanRemoval = false; /** @var string */ public $indexBy; } diff --git a/lib/Doctrine/ORM/PersistentCollection.php b/lib/Doctrine/ORM/PersistentCollection.php index 2d8a77ef6..3bfb0d1eb 100644 --- a/lib/Doctrine/ORM/PersistentCollection.php +++ b/lib/Doctrine/ORM/PersistentCollection.php @@ -388,7 +388,7 @@ final class PersistentCollection implements Collection $this->changed(); if ($this->association !== null && - $this->association['type'] == ClassMetadata::ONE_TO_MANY && + $this->association['type'] & ClassMetadata::TO_MANY && $this->association['orphanRemoval']) { $this->em->getUnitOfWork()->scheduleOrphanRemoval($removed); } @@ -426,7 +426,7 @@ final class PersistentCollection implements Collection $this->changed(); if ($this->association !== null && - $this->association['type'] === ClassMetadata::ONE_TO_MANY && + $this->association['type'] & ClassMetadata::TO_MANY && $this->association['orphanRemoval']) { $this->em->getUnitOfWork()->scheduleOrphanRemoval($element); } @@ -631,7 +631,7 @@ final class PersistentCollection implements Collection $uow = $this->em->getUnitOfWork(); - if ($this->association['type'] === ClassMetadata::ONE_TO_MANY && $this->association['orphanRemoval']) { + if ($this->association['type'] & ClassMetadata::TO_MANY && $this->association['orphanRemoval']) { // we need to initialize here, as orphan removal acts like implicit cascadeRemove, // hence for event listeners we need the objects in memory. $this->initialize(); diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1654Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1654Test.php new file mode 100644 index 000000000..016961935 --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1654Test.php @@ -0,0 +1,103 @@ +setUpEntitySchema(array( + __NAMESPACE__ . '\\DDC1654Post', + __NAMESPACE__ . '\\DDC1654Comment', + )); + } + + public function testManyToManyRemoveFromCollectionOrphanRemoval() + { + $post = new DDC1654Post(); + $post->comments[] = new DDC1654Comment(); + $post->comments[] = new DDC1654Comment(); + + $this->_em->persist($post); + $this->_em->flush(); + + $post->comments->remove(0); + $post->comments->remove(1); + + $this->_em->flush(); + $this->_em->clear(); + + $comments = $this->_em->getRepository(__NAMESPACE__ . '\\DDC1654Comment')->findAll(); + $this->assertEquals(0, count($comments)); + } + + public function testManyToManyRemoveElementFromCollectionOrphanRemoval() + { + $post = new DDC1654Post(); + $post->comments[] = new DDC1654Comment(); + $post->comments[] = new DDC1654Comment(); + + $this->_em->persist($post); + $this->_em->flush(); + + $post->comments->removeElement($post->comments[0]); + $post->comments->removeElement($post->comments[1]); + + $this->_em->flush(); + $this->_em->clear(); + + $comments = $this->_em->getRepository(__NAMESPACE__ . '\\DDC1654Comment')->findAll(); + $this->assertEquals(0, count($comments)); + } + + public function testManyToManyClearCollectionOrphanRemoval() + { + $post = new DDC1654Post(); + $post->comments[] = new DDC1654Comment(); + $post->comments[] = new DDC1654Comment(); + + $this->_em->persist($post); + $this->_em->flush(); + + $post->comments->clear(); + + $this->_em->flush(); + $this->_em->clear(); + + $comments = $this->_em->getRepository(__NAMESPACE__ . '\\DDC1654Comment')->findAll(); + $this->assertEquals(0, count($comments)); + + } +} + +/** + * @Entity + */ +class DDC1654Post +{ + /** + * @Id @Column(type="integer") @GeneratedValue + */ + public $id; + + /** + * @ManyToMany(targetEntity="DDC1654Comment", orphanRemoval=true, + * cascade={"persist"}) + */ + public $comments = array(); +} + +/** + * @Entity + */ +class DDC1654Comment +{ + /** + * @Id @Column(type="integer") @GeneratedValue + */ + public $id; +} diff --git a/tests/Doctrine/Tests/ORM/Mapping/ClassMetadataBuilderTest.php b/tests/Doctrine/Tests/ORM/Mapping/ClassMetadataBuilderTest.php index 888db0d81..f14b60b3f 100644 --- a/tests/Doctrine/Tests/ORM/Mapping/ClassMetadataBuilderTest.php +++ b/tests/Doctrine/Tests/ORM/Mapping/ClassMetadataBuilderTest.php @@ -377,6 +377,7 @@ class ClassMetadataBuilderTest extends \Doctrine\Tests\OrmTestCase array( 'user_id' => 'id', ), + 'orphanRemoval' => false, ), ), $this->cm->associationMappings); } diff --git a/tests/Doctrine/Tests/OrmFunctionalTestCase.php b/tests/Doctrine/Tests/OrmFunctionalTestCase.php index 641826925..4b92ed5d3 100644 --- a/tests/Doctrine/Tests/OrmFunctionalTestCase.php +++ b/tests/Doctrine/Tests/OrmFunctionalTestCase.php @@ -38,6 +38,12 @@ abstract class OrmFunctionalTestCase extends OrmTestCase /** Whether the database schema has already been created. */ protected static $_tablesCreated = array(); + /** + * Array of entity class name to their tables that were created. + * @var array + */ + protected static $_entityTablesCreated = array(); + /** List of model sets and their classes. */ protected static $_modelSets = array( 'cms' => array( @@ -235,6 +241,25 @@ abstract class OrmFunctionalTestCase extends OrmTestCase $this->_em->clear(); } + protected function setUpEntitySchema(array $classNames) + { + if ($this->_em === null) { + throw new \RuntimeException("EntityManager not set, you have to call parent::setUp() before invoking this method."); + } + + $classes = array(); + foreach ($classNames as $className) { + if ( ! isset(static::$_entityTablesCreated[$className])) { + static::$_entityTablesCreated[$className] = true; + $classes[] = $this->_em->getClassMetadata($className); + } + } + + if ($classes) { + $this->_schemaTool->createSchema($classes); + } + } + /** * Creates a connection to the test database, if there is none yet, and * creates the necessary tables.