Skip to content

Commit 90b1735

Browse files
committed
Update in-memory collections before dispatching the postRemove event
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.
1 parent c288647 commit 90b1735

File tree

2 files changed

+52
-10
lines changed

2 files changed

+52
-10
lines changed

lib/Doctrine/ORM/UnitOfWork.php

+10-10
Original file line numberDiff line numberDiff line change
@@ -491,20 +491,12 @@ public function commit($entity = null)
491491

492492
$this->afterTransactionComplete();
493493

494-
// Unset removed entities from collections, and take new snapshots from
495-
// all visited collections.
496-
foreach ($this->visitedCollections as $coid => $coll) {
497-
if (isset($this->pendingCollectionElementRemovals[$coid])) {
498-
foreach ($this->pendingCollectionElementRemovals[$coid] as $key => $valueIgnored) {
499-
unset($coll[$key]);
500-
}
501-
}
502-
494+
// Take new snapshots from all visited collections.
495+
foreach ($this->visitedCollections as $coll) {
503496
$coll->takeSnapshot();
504497
}
505498

506499
$this->dispatchPostFlushEvent();
507-
508500
$this->postCommitCleanup($entity);
509501
}
510502

@@ -1317,6 +1309,14 @@ private function executeDeletions(): void
13171309
}
13181310
}
13191311

1312+
// Unset removed entities from collections
1313+
foreach ($this->pendingCollectionElementRemovals as $coid => $keys) {
1314+
$collection = $this->visitedCollections[$coid];
1315+
foreach ($keys as $key => $valueIgnored) {
1316+
unset($collection[$key]);
1317+
}
1318+
}
1319+
13201320
// Defer dispatching `postRemove` events to until all entities have been removed.
13211321
foreach ($eventsToDispatch as $event) {
13221322
$this->listenersInvoker->invoke(

tests/Doctrine/Tests/ORM/Functional/EntityListenersTest.php

+42
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
namespace Doctrine\Tests\ORM\Functional;
66

7+
use Doctrine\Common\Collections\ArrayCollection;
78
use Doctrine\ORM\Event\PostPersistEventArgs;
89
use Doctrine\ORM\Event\PostRemoveEventArgs;
910
use Doctrine\ORM\Event\PreFlushEventArgs;
@@ -12,6 +13,7 @@
1213
use Doctrine\ORM\UnitOfWork;
1314
use Doctrine\Persistence\Event\LifecycleEventArgs;
1415
use Doctrine\Tests\Models\Company\CompanyContractListener;
16+
use Doctrine\Tests\Models\Company\CompanyEmployee;
1517
use Doctrine\Tests\Models\Company\CompanyFixContract;
1618
use Doctrine\Tests\Models\Company\CompanyPerson;
1719
use Doctrine\Tests\OrmFunctionalTestCase;
@@ -266,4 +268,44 @@ public function postRemove(PostRemoveEventArgs $args): void
266268

267269
self::assertSame(2, $listener->invocationCount);
268270
}
271+
272+
public function testPostRemoveCalledAfterAllInMemoryCollectionsHaveBeenUpdated(): void
273+
{
274+
$contract = new CompanyFixContract();
275+
$contract->setFixPrice(2000);
276+
277+
$engineer = new CompanyEmployee();
278+
$engineer->setName('J. Doe');
279+
$engineer->setSalary(50);
280+
$engineer->setDepartment('tech');
281+
282+
$contract->addEngineer($engineer);
283+
$engineer->contracts = new ArrayCollection([$contract]);
284+
285+
$this->_em->persist($contract);
286+
$this->_em->persist($engineer);
287+
$this->_em->flush();
288+
289+
$this->_em->getEventManager()->addEventListener([Events::postRemove], new class ($contract) {
290+
/** @var CompanyFixContract */
291+
private $contract;
292+
293+
public function __construct(CompanyFixContract $contract)
294+
{
295+
$this->contract = $contract;
296+
}
297+
298+
public function postRemove(): void
299+
{
300+
Assert::assertEmpty($this->contract->getEngineers()); // Assert collection has been updated before event was dispatched
301+
Assert::assertTrue($this->contract->getEngineers()->isDirty()); // Collections are still dirty (is that relevant?)
302+
}
303+
});
304+
305+
$this->_em->remove($engineer);
306+
$this->_em->flush();
307+
308+
self::assertEmpty($contract->getEngineers());
309+
self::assertFalse($contract->getEngineers()->isDirty());
310+
}
269311
}

0 commit comments

Comments
 (0)