diff --git a/lib/Doctrine/ORM/Persisters/Collection/OneToManyPersister.php b/lib/Doctrine/ORM/Persisters/Collection/OneToManyPersister.php index c18fdbbb9..86b364df8 100644 --- a/lib/Doctrine/ORM/Persisters/Collection/OneToManyPersister.php +++ b/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. - // For owning side one to many, it is required to have a join table, - // then classifying it as a ManyToManyPersister. - return; + // The only valid case here is when you have weak entities. In this + // scenario, you have @OneToMany with orphanRemoval=true, and replacing + // the entire collection with a new would trigger this operation. + $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; + } } diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index aa4f00021..a58e687ef 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/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); diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC3644Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC3644Test.php new file mode 100644 index 000000000..46eeeab74 --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC3644Test.php @@ -0,0 +1,245 @@ +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; +} \ No newline at end of file