Skip to content

Commit

Permalink
Compute particular commit order for deletions
Browse files Browse the repository at this point in the history
This fixes a bug which arises in some cases when trying to delete two entities which have a circular reference through other entities. UnifOfWork computed the order which was not correct for deletions and it failed with ForeignKeyConstraintViolationException. This also adds test for this issue.
  • Loading branch information
jusephe committed Feb 2, 2022
1 parent 4b88ce7 commit c58dea4
Show file tree
Hide file tree
Showing 3 changed files with 261 additions and 3 deletions.
58 changes: 56 additions & 2 deletions lib/Doctrine/ORM/UnitOfWork.php
Original file line number Diff line number Diff line change
Expand Up @@ -444,8 +444,14 @@ public function commit($entity = null)

// Entity deletions come last and need to be in reverse commit order
if ($this->entityDeletions) {
for ($count = count($commitOrder), $i = $count - 1; $i >= 0 && $this->entityDeletions; --$i) {
$this->executeDeletions($commitOrder[$i]);
// There are some cases in which the original $commitOrder is not correct for deletions,
// so we need to call a particular function to compute the right order.
// For example see tests/Doctrine/Tests/ORM/Functional/Ticket/GH9376Test.php.
// Without computing the special order for deletions, it tries to delete the entity set as a foreign key in another entity.
// getCommitOrderForDeletions() is similar to getCommitOrder(), but it operates only with entities set to be deleted.
$commitOrderForDeletions = $this->getCommitOrderForDeletions();
for ($count = count($commitOrderForDeletions), $i = $count - 1; $i >= 0 && $this->entityDeletions; --$i) {
$this->executeDeletions($commitOrderForDeletions[$i]);
}
}

Expand Down Expand Up @@ -1330,6 +1336,54 @@ private function getCommitOrder(): array
return $calc->sort();
}

/**
* Gets the commit order for deletions.
*
* @return list<object>
*/
private function getCommitOrderForDeletions(): array
{
$calc = $this->getCommitOrderCalculator();

// See if there are any new classes in the changeset, that are not in the
// commit order graph yet (don't have a node).
// We have to inspect changeSet to be able to correctly build dependencies.
// It is not possible to use IdentityMap here because post inserted ids
// are not yet available.
$newNodes = [];

foreach ($this->entityDeletions as $entity) {
$class = $this->em->getClassMetadata(get_class($entity));

if ($calc->hasNode($class->name)) {
continue;
}

$calc->addNode($class->name, $class);

$newNodes[] = $class;
}

// Calculate dependencies for new nodes
while ($class = array_pop($newNodes)) {
foreach ($class->associationMappings as $assoc) {
if (! ($assoc['isOwningSide'] && $assoc['type'] & ClassMetadata::TO_ONE)) {
continue;
}

$targetClass = $this->em->getClassMetadata($assoc['targetEntity']);

$joinColumns = reset($assoc['joinColumns']);

if ($calc->hasNode($targetClass->name)) {
$calc->addDependency($targetClass->name, $class->name, (int) empty($joinColumns['nullable']));
}
}
}

return $calc->sort();
}

/**
* Schedules an entity for insertion into the database.
* If the entity already has an identifier, it will be added to the identity map.
Expand Down
2 changes: 1 addition & 1 deletion psalm-baseline.xml
Original file line number Diff line number Diff line change
Expand Up @@ -3452,7 +3452,7 @@
<code>$class</code>
<code>$collectionToDelete</code>
<code>$collectionToUpdate</code>
<code>$commitOrder[$i]</code>
<code>$commitOrderForDeletions[$i]</code>
</ArgumentTypeCoercion>
<DocblockTypeContradiction occurrences="4">
<code>! is_object($object)</code>
Expand Down
204 changes: 204 additions & 0 deletions tests/Doctrine/Tests/ORM/Functional/Ticket/GH9376Test.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,204 @@
<?php

declare(strict_types=1);

namespace Doctrine\Tests\ORM\Functional\Ticket;

use Doctrine\ORM\Mapping as ORM;
use Doctrine\ORM\Mapping\Column;
use Doctrine\ORM\Mapping\Entity;
use Doctrine\ORM\Mapping\GeneratedValue;
use Doctrine\ORM\Mapping\Id;
use Doctrine\Tests\OrmFunctionalTestCase;

class GH9376Test extends OrmFunctionalTestCase
{
protected function setUp(): void
{
parent::setUp();

$this->_schemaTool->createSchema([
$this->_em->getClassMetadata(GH9376GiftVariant::class),
$this->_em->getClassMetadata(GH9376OrderGiftVariant::class),
$this->_em->getClassMetadata(GH9376Order::class),
$this->_em->getClassMetadata(GH9376ProductPartner::class),
$this->_em->getClassMetadata(GH9376Product::class),
$this->_em->getClassMetadata(GH9376Gift::class),
]);
}

protected function tearDown(): void
{
$this->_schemaTool->dropSchema([
$this->_em->getClassMetadata(GH9376GiftVariant::class),
$this->_em->getClassMetadata(GH9376OrderGiftVariant::class),
$this->_em->getClassMetadata(GH9376Order::class),
$this->_em->getClassMetadata(GH9376ProductPartner::class),
$this->_em->getClassMetadata(GH9376Product::class),
$this->_em->getClassMetadata(GH9376Gift::class),
]);

parent::tearDown();
}

public function testIssueRemove(): void
{
$product = new GH9376Product();
$gift = new GH9376Gift($product);
$giftVariant = new GH9376GiftVariant($gift);

$this->_em->persist($product);
$this->_em->persist($gift);
$this->_em->persist($giftVariant);
$this->_em->flush();
$this->_em->clear();

$persistedGiftVariant = $this->_em->find(GH9376GiftVariant::class, 1);
$this->_em->remove($persistedGiftVariant);

$persistedGift = $this->_em->find(GH9376Gift::class, 1);
$this->_em->remove($persistedGift);

$this->_em->flush();
$this->_em->clear();

self::assertEmpty($this->_em->getRepository(GH9376Gift::class)->findAll());
self::assertEmpty($this->_em->getRepository(GH9376GiftVariant::class)->findAll());
}
}

/**
* @Entity
*/
class GH9376GiftVariant
{
/**
* @var int
* @Id
* @Column(type="integer")
* @GeneratedValue
*/
public $id;

/**
* @ORM\ManyToOne(targetEntity=GH9376Gift::class)
* @ORM\JoinColumn(nullable=false)
*
* @var GH9376Gift
*/
public $gift;

public function __construct(GH9376Gift $gift)
{
$this->gift = $gift;
}
}

/**
* @Entity
*/
class GH9376OrderGiftVariant
{
/**
* @var int
* @Id @Column(type="integer")
* @GeneratedValue
*/
public $id;

/**
* @ORM\ManyToOne(targetEntity=GH9376GiftVariant::class)
* @ORM\JoinColumn(nullable=true)
*
* @var GH9376GiftVariant|null
*/
public $giftVariant;
}

/**
* @Entity
*/
class GH9376Order
{
/**
* @var int
* @Id @Column(type="integer")
* @GeneratedValue
*/
public $id;

/**
* @ORM\OneToOne(targetEntity=GH9376OrderGiftVariant::class, cascade={"persist"})
*
* @var GH9376OrderGiftVariant|null
*/
public $orderGiftVariant;
}

/**
* @Entity
*/
class GH9376ProductPartner
{
/**
* @var int
* @Id @Column(type="integer")
* @GeneratedValue
*/
public $id;

/**
* @ORM\OneToOne(targetEntity=GH9376Order::class)
* @ORM\JoinColumn(nullable=true)
*
* @var GH9376Order|null
*/
public $order = null;
}

/**
* @Entity
*/
class GH9376Product
{
/**
* @var int
* @Id @Column(type="integer")
* @GeneratedValue
*/
public $id;

/**
* @ORM\OneToOne(targetEntity=GH9376ProductPartner::class)
* @ORM\JoinColumn(nullable=true)
*
* @var GH9376ProductPartner|null
*/
public $productPartner = null;
}

/**
* @Entity
*/
class GH9376Gift
{
/**
* @var int
* @Id @Column(type="integer")
* @GeneratedValue
*/
public $id;

/**
* @ORM\ManyToOne(targetEntity=GH9376Product::class)
* @ORM\JoinColumn(nullable=false)
*
* @var GH9376Product
*/
public $product;

public function __construct(GH9376Product $product)
{
$this->product = $product;
}
}

0 comments on commit c58dea4

Please sign in to comment.