Skip to content

Commit

Permalink
Update in-memory collections before dispatching the postRemove event
Browse files Browse the repository at this point in the history
Make sure in-memory collections have removed entities unset before the `postRemove` event is dispatched. This is related to #11098, although by itself probably not going to fix the regression.

 #### Background

When an entity is removed from the database, the UoW also takes care of removing that entity from all in-memory collections. #10763 moved this cleanup and taking snapshots of updated collections to a very late stage during the commit phase, in order to avoid other side effects.

Now, part of the issue in #11098 is that `postRemove` event listeners will be called at a point where the database-level `DELETE` has happened (although the transaction has not yet been committed), but the in-memory collections have not yet been updated.

 #### Suggested change

This PR splits taking the new collection snapshots and updating in-memory collections. The in-memory update happens along with the database-level execution, collection snapshots still happen after transaction commit.
  • Loading branch information
mpdude committed Dec 22, 2023
1 parent c288647 commit efd74f1
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 10 deletions.
20 changes: 10 additions & 10 deletions lib/Doctrine/ORM/UnitOfWork.php
Original file line number Diff line number Diff line change
Expand Up @@ -491,20 +491,12 @@ public function commit($entity = null)

$this->afterTransactionComplete();

// Unset removed entities from collections, and take new snapshots from
// all visited collections.
foreach ($this->visitedCollections as $coid => $coll) {
if (isset($this->pendingCollectionElementRemovals[$coid])) {
foreach ($this->pendingCollectionElementRemovals[$coid] as $key => $valueIgnored) {
unset($coll[$key]);
}
}

// Take new snapshots from all visited collections.
foreach ($this->visitedCollections as $coll) {
$coll->takeSnapshot();
}

$this->dispatchPostFlushEvent();

$this->postCommitCleanup($entity);
}

Expand Down Expand Up @@ -1317,6 +1309,14 @@ private function executeDeletions(): void
}
}

// Unset removed entities from collections
foreach ($this->pendingCollectionElementRemovals as $coid => $keys) {
$collection = $this->visitedCollections[$coid];
foreach ($keys as $key => $valueIgnored) {
unset($collection[$key]);
}
}

// Defer dispatching `postRemove` events to until all entities have been removed.
foreach ($eventsToDispatch as $event) {
$this->listenersInvoker->invoke(
Expand Down
41 changes: 41 additions & 0 deletions tests/Doctrine/Tests/ORM/Functional/EntityListenersTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

namespace Doctrine\Tests\ORM\Functional;

use Doctrine\Common\Collections\ArrayCollection;
use Doctrine\ORM\Event\PostPersistEventArgs;
use Doctrine\ORM\Event\PostRemoveEventArgs;
use Doctrine\ORM\Event\PreFlushEventArgs;
Expand All @@ -12,6 +13,7 @@
use Doctrine\ORM\UnitOfWork;
use Doctrine\Persistence\Event\LifecycleEventArgs;
use Doctrine\Tests\Models\Company\CompanyContractListener;
use Doctrine\Tests\Models\Company\CompanyEmployee;
use Doctrine\Tests\Models\Company\CompanyFixContract;
use Doctrine\Tests\Models\Company\CompanyPerson;
use Doctrine\Tests\OrmFunctionalTestCase;
Expand Down Expand Up @@ -266,4 +268,43 @@ public function postRemove(PostRemoveEventArgs $args): void

self::assertSame(2, $listener->invocationCount);
}

public function testPostRemoveCalledAfterAllInMemoryCollectionsHaveBeenUpdated(): void
{
$contract = new CompanyFixContract();
$contract->setFixPrice(2000);

$engineer = new CompanyEmployee();
$engineer->setName('J. Doe');
$engineer->setSalary(50);
$engineer->setDepartment('tech');

$contract->addEngineer($engineer);
$engineer->contracts = new ArrayCollection([$contract]);

$this->_em->persist($contract);
$this->_em->persist($engineer);
$this->_em->flush();

$this->_em->getEventManager()->addEventListener([Events::postRemove], new class ($contract) {
private $contract;

Check failure on line 290 in tests/Doctrine/Tests/ORM/Functional/EntityListenersTest.php

View workflow job for this annotation

GitHub Actions / coding-standards / Coding Standards (8.2)

Property class@anonymous::$contract does not have @var annotation for its value.

public function __construct(CompanyFixContract $contract)
{
$this->contract = $contract;
}

public function postRemove(): void
{
Assert::assertEmpty($this->contract->getEngineers()); // Assert collection has been updated before event was dispatched
Assert::assertTrue($this->contract->getEngineers()->isDirty()); // Collections are still dirty (is that relevant?)
}
});

$this->_em->remove($engineer);
$this->_em->flush();

self::assertEmpty($contract->getEngineers());
self::assertFalse($contract->getEngineers()->isDirty());
}
}

0 comments on commit efd74f1

Please sign in to comment.