Merge branch 'feature/#1354-one-to-many-orphan-removal'

Close #1354

lib/Doctrine/ORM/Persisters/Collection/OneToManyPersister.php

@@ -21,6 +21,7 @@ namespace Doctrine\ORM\Persisters\Collection;
 
 use Doctrine\Common\Collections\Criteria;
 use Doctrine\Common\Proxy\Proxy;
+use Doctrine\DBAL\Types\Type;
 use Doctrine\ORM\PersistentCollection;
 
 /**
@@ -38,10 +39,23 @@ class OneToManyPersister extends AbstractCollectionPersister
      */
     public function delete(PersistentCollection $collection)
     {
-        // This can never happen. One to many can only be inverse side.
+        // The only valid case here is when you have weak entities. In this
-        // For owning side one to many, it is required to have a join table,
+        // scenario, you have @OneToMany with orphanRemoval=true, and replacing
-        // then classifying it as a ManyToManyPersister.
+        // the entire collection with a new would trigger this operation.
-        return;
+        $mapping = $collection->getMapping();
+
+        if ( ! $mapping['orphanRemoval']) {
+            // Handling non-orphan removal should never happen, as @OneToMany
+            // can only be inverse side. For owning side one to many, it is
+            // required to have a join table, which would classify as a ManyToManyPersister.
+            return;
+        }
+
+        $targetClass = $this->em->getClassMetadata($mapping['targetEntity']);
+
+        return $targetClass->isInheritanceTypeJoined()
+            ? $this->deleteJoinedEntityCollection($collection)
+            : $this->deleteEntityCollection($collection);
     }
 
     /**
@@ -181,4 +195,97 @@ class OneToManyPersister extends AbstractCollectionPersister
     {
         throw new \BadMethodCallException("Filtering a collection by Criteria is not supported by this CollectionPersister.");
     }
+
+    /**
+     * @param PersistentCollection $collection
+     *
+     * @return int
+     *
+     * @throws \Doctrine\DBAL\DBALException
+     */
+    private function deleteEntityCollection(PersistentCollection $collection)
+    {
+        $mapping     = $collection->getMapping();
+        $identifier  = $this->uow->getEntityIdentifier($collection->getOwner());
+        $sourceClass = $this->em->getClassMetadata($mapping['sourceEntity']);
+        $targetClass = $this->em->getClassMetadata($mapping['targetEntity']);
+        $columns     = [];
+        $parameters  = [];
+
+        foreach ($targetClass->associationMappings[$mapping['mappedBy']]['joinColumns'] as $joinColumn) {
+            $columns[]    = $this->quoteStrategy->getJoinColumnName($joinColumn, $targetClass, $this->platform);
+            $parameters[] = $identifier[$sourceClass->getFieldForColumn($joinColumn['referencedColumnName'])];
+        }
+
+        $statement = 'DELETE FROM ' . $this->quoteStrategy->getTableName($targetClass, $this->platform)
+            . ' WHERE ' . implode(' = ? AND ', $columns) . ' = ?';
+
+        return $this->conn->executeUpdate($statement, $parameters);
+    }
+
+    /**
+     * Delete Class Table Inheritance entities.
+     * A temporary table is needed to keep IDs to be deleted in both parent and child class' tables.
+     *
+     * Thanks Steve Ebersole (Hibernate) for idea on how to tackle reliably this scenario, we owe him a beer! =)
+     *
+     * @param PersistentCollection $collection
+     *
+     * @return int
+     *
+     * @throws \Doctrine\DBAL\DBALException
+     */
+    private function deleteJoinedEntityCollection(PersistentCollection $collection)
+    {
+        $mapping     = $collection->getMapping();
+        $sourceClass = $this->em->getClassMetadata($mapping['sourceEntity']);
+        $targetClass = $this->em->getClassMetadata($mapping['targetEntity']);
+        $rootClass   = $this->em->getClassMetadata($targetClass->rootEntityName);
+
+        // 1) Build temporary table DDL
+        $tempTable         = $this->platform->getTemporaryTableName($rootClass->getTemporaryIdTableName());
+        $idColumnNames     = $rootClass->getIdentifierColumnNames();
+        $idColumnList      = implode(', ', $idColumnNames);
+        $columnDefinitions = [];
+
+        foreach ($idColumnNames as $idColumnName) {
+            $columnDefinitions[$idColumnName] = array(
+                'notnull' => true,
+                'type'    => Type::getType($rootClass->getTypeOfColumn($idColumnName)),
+            );
+        }
+
+        $statement = $this->platform->getCreateTemporaryTableSnippetSQL() . ' ' . $tempTable
+            . ' (' . $this->platform->getColumnDeclarationListSQL($columnDefinitions) . ')';
+
+        $this->conn->executeUpdate($statement);
+
+        // 2) Build insert table records into temporary table
+        $query = $this->em->createQuery(
+            ' SELECT t0.' . implode(', t0.', $rootClass->getIdentifierFieldNames())
+            . ' FROM ' . $targetClass->name . ' t0 WHERE t0.' . $mapping['mappedBy'] . ' = :owner'
+        )->setParameter('owner', $collection->getOwner());
+
+        $statement  = 'INSERT INTO ' . $tempTable . ' (' . $idColumnList . ') ' . $query->getSQL();
+        $parameters = array_values($sourceClass->getIdentifierValues($collection->getOwner()));
+        $numDeleted = $this->conn->executeUpdate($statement, $parameters);
+
+        // 3) Delete records on each table in the hierarchy
+        $classNames = array_merge($targetClass->parentClasses, array($targetClass->name), $targetClass->subClasses);
+
+        foreach (array_reverse($classNames) as $className) {
+            $tableName = $this->quoteStrategy->getTableName($this->em->getClassMetadata($className), $this->platform);
+            $statement = 'DELETE FROM ' . $tableName . ' WHERE (' . $idColumnList . ')'
+                . ' IN (SELECT ' . $idColumnList . ' FROM ' . $tempTable . ')';
+
+            $this->conn->executeUpdate($statement);
+        }
+
+        // 4) Drop temporary table
+        $statement = $this->platform->getDropTemporaryTableSQL($tempTable);
+
+        $this->conn->executeUpdate($statement);
+
+        return $numDeleted;
+    }
 }

lib/Doctrine/ORM/UnitOfWork.php

@@ -373,6 +373,11 @@ class UnitOfWork implements PropertyChangedListener
         $conn->beginTransaction();
 
         try {
+            // Collection deletions (deletions of complete collections)
+            foreach ($this->collectionDeletions as $collectionToDelete) {
+                $this->getCollectionPersister($collectionToDelete->getMapping())->delete($collectionToDelete);
+            }
+
             if ($this->entityInsertions) {
                 foreach ($commitOrder as $class) {
                     $this->executeInserts($class);
@@ -390,11 +395,6 @@ class UnitOfWork implements PropertyChangedListener
                 $this->executeExtraUpdates();
             }
 
-            // Collection deletions (deletions of complete collections)
-            foreach ($this->collectionDeletions as $collectionToDelete) {
-                $this->getCollectionPersister($collectionToDelete->getMapping())->delete($collectionToDelete);
-            }
-
             // Collection updates (deleteRows, updateRows, insertRows)
             foreach ($this->collectionUpdates as $collectionToUpdate) {
                 $this->getCollectionPersister($collectionToUpdate->getMapping())->update($collectionToUpdate);

tests/Doctrine/Tests/ORM/Functional/Ticket/DDC3644Test.php

@@ -0,0 +1,245 @@
+<?php
+
+namespace Doctrine\Tests\ORM\Functional\Ticket;
+
+use Doctrine\Common\Collections\ArrayCollection;
+use Doctrine\Common\Collections\Collection;
+use Doctrine\ORM\Mapping\JoinColumn;
+use Doctrine\Tests\OrmFunctionalTestCase;
+
+/**
+ * Functional tests for orphan removal with one to many association.
+ */
+class DDC3644Test extends OrmFunctionalTestCase
+{
+    protected function setUp()
+    {
+        parent::setUp();
+
+        $this->setUpEntitySchema(array(
+            'Doctrine\Tests\ORM\Functional\Ticket\DDC3644User',
+            'Doctrine\Tests\ORM\Functional\Ticket\DDC3644Address',
+            'Doctrine\Tests\ORM\Functional\Ticket\DDC3644Animal',
+            'Doctrine\Tests\ORM\Functional\Ticket\DDC3644Pet',
+        ));
+    }
+
+    /**
+     * @group DDC-3644
+     */
+    public function testIssueWithRegularEntity()
+    {
+        // Define initial dataset
+        $current   = new DDC3644Address('Sao Paulo, SP, Brazil');
+        $previous  = new DDC3644Address('Rio de Janeiro, RJ, Brazil');
+        $initial   = new DDC3644Address('Sao Carlos, SP, Brazil');
+        $addresses = new ArrayCollection(array($current, $previous, $initial));
+        $user      = new DDC3644User();
+
+        $user->name = 'Guilherme Blanco';
+        $user->setAddresses($addresses);
+
+        $this->_em->persist($user);
+        $this->_em->persist($current);
+        $this->_em->persist($previous);
+        $this->_em->persist($initial);
+
+        $this->_em->flush();
+
+        $userId = $user->id;
+        unset($current, $previous, $initial, $addresses, $user);
+
+        $this->_em->clear();
+
+        // Replace entire collection (this should trigger OneToManyPersister::remove())
+        $current   = new DDC3644Address('Toronto, ON, Canada');
+        $addresses = new ArrayCollection(array($current));
+        $user      = $this->_em->find(__NAMESPACE__ . '\DDC3644User', $userId);
+
+        $user->setAddresses($addresses);
+
+        $this->_em->persist($user);
+        $this->_em->persist($current);
+
+        $this->_em->flush();
+        $this->_em->clear();
+
+        // We should only have 1 item in the collection list now
+        $user = $this->_em->find(__NAMESPACE__ . '\DDC3644User', $userId);
+
+        $this->assertCount(1, $user->addresses);
+
+        // We should only have 1 item in the addresses table too
+        $repository = $this->_em->getRepository(__NAMESPACE__ . '\DDC3644Address');
+        $addresses  = $repository->findAll();
+
+        $this->assertCount(1, $addresses);
+    }
+
+    /**
+     * @group DDC-3644
+     */
+    public function testIssueWithJoinedEntity()
+    {
+        // Define initial dataset
+        $actual = new DDC3644Pet('Catharina');
+        $past   = new DDC3644Pet('Nanny');
+        $pets   = new ArrayCollection(array($actual, $past));
+        $user   = new DDC3644User();
+
+        $user->name = 'Guilherme Blanco';
+        $user->setPets($pets);
+
+        $this->_em->persist($user);
+        $this->_em->persist($actual);
+        $this->_em->persist($past);
+
+        $this->_em->flush();
+
+        $userId = $user->id;
+        unset($actual, $past, $pets, $user);
+
+        $this->_em->clear();
+
+        // Replace entire collection (this should trigger OneToManyPersister::remove())
+        $actual = new DDC3644Pet('Valentina');
+        $pets   = new ArrayCollection(array($actual));
+        $user   = $this->_em->find(__NAMESPACE__ . '\DDC3644User', $userId);
+
+        $user->setPets($pets);
+
+        $this->_em->persist($user);
+        $this->_em->persist($actual);
+
+        $this->_em->flush();
+        $this->_em->clear();
+
+        // We should only have 1 item in the collection list now
+        $user = $this->_em->find(__NAMESPACE__ . '\DDC3644User', $userId);
+
+        $this->assertCount(1, $user->pets);
+
+        // We should only have 1 item in the pets table too
+        $repository = $this->_em->getRepository(__NAMESPACE__ . '\DDC3644Pet');
+        $pets       = $repository->findAll();
+
+        $this->assertCount(1, $pets);
+    }
+}
+
+/**
+ * @Entity
+ */
+class DDC3644User
+{
+    /**
+     * @Id
+     * @GeneratedValue
+     * @Column(type="integer", name="hash_id")
+     */
+    public $id;
+
+    /**
+     * @Column(type="string")
+     */
+    public $name;
+
+    /**
+     * @OneToMany(targetEntity="DDC3644Address", mappedBy="user", orphanRemoval=true)
+     */
+    public $addresses = [];
+
+    /**
+     * @OneToMany(targetEntity="DDC3644Pet", mappedBy="owner", orphanRemoval=true)
+     */
+    public $pets = [];
+
+    public function setAddresses(Collection $addresses)
+    {
+        $self = $this;
+
+        $this->addresses = $addresses;
+
+        $addresses->map(function ($address) use ($self) {
+            $address->user = $self;
+        });
+    }
+
+    public function setPets(Collection $pets)
+    {
+        $self = $this;
+
+        $this->pets = $pets;
+
+        $pets->map(function ($pet) use ($self) {
+            $pet->owner = $self;
+        });
+    }
+}
+
+/**
+ * @Entity
+ */
+class DDC3644Address
+{
+    /**
+     * @Id
+     * @GeneratedValue
+     * @Column(type="integer")
+     */
+    public $id;
+
+    /**
+     * @ManyToOne(targetEntity="DDC3644User", inversedBy="addresses")
+     * @JoinColumn(referencedColumnName="hash_id")
+     */
+    public $user;
+
+    /**
+     * @Column(type="string")
+     */
+    public $address;
+
+    public function __construct($address)
+    {
+        $this->address = $address;
+    }
+}
+
+/**
+ * @Entity
+ * @InheritanceType("JOINED")
+ * @DiscriminatorColumn(name="discriminator", type="string")
+ * @DiscriminatorMap({"pet" = "DDC3644Pet"})
+ */
+abstract class DDC3644Animal
+{
+    /**
+     * @Id
+     * @GeneratedValue
+     * @Column(type="integer")
+     */
+    public $id;
+
+    /**
+     * @Column(type="string")
+     */
+    public $name;
+
+    public function __construct($name)
+    {
+        $this->name = $name;
+    }
+}
+
+/**
+ * @Entity
+ */
+class DDC3644Pet extends DDC3644Animal
+{
+    /**
+     * @ManyToOne(targetEntity="DDC3644User", inversedBy="pets")
+     * @JoinColumn(referencedColumnName="hash_id")
+     */
+    public $owner;
+}