From 9016a5a854b089d7fdbd06224afc862ab1f5a485 Mon Sep 17 00:00:00 2001 From: flack Date: Tue, 12 Nov 2013 00:11:50 +0100 Subject: [PATCH 1/4] don't compute changeset for entities that are going to be deleted --- lib/Doctrine/ORM/UnitOfWork.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index 5de769f63..fdd15f7db 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -445,10 +445,10 @@ class UnitOfWork implements PropertyChangedListener return; } - // Only MANAGED entities that are NOT SCHEDULED FOR INSERTION are processed here. + // Only MANAGED entities that are NOT SCHEDULED FOR INSERTION OR DELETION are processed here. $oid = spl_object_hash($entity); - if ( ! isset($this->entityInsertions[$oid]) && isset($this->entityStates[$oid])) { + if ( ! isset($this->entityInsertions[$oid]) && ! isset($this->entityDeletions[$oid]) && isset($this->entityStates[$oid])) { $this->computeChangeSet($class, $entity); } } From 980771810033d72346b02d08d1f115380bde70b5 Mon Sep 17 00:00:00 2001 From: flack Date: Sun, 17 Nov 2013 11:31:21 +0100 Subject: [PATCH 2/4] Also skip entities scheduled for deletion when committing multiple entities --- lib/Doctrine/ORM/UnitOfWork.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index fdd15f7db..9818ca6a8 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -733,10 +733,10 @@ class UnitOfWork implements PropertyChangedListener continue; } - // Only MANAGED entities that are NOT SCHEDULED FOR INSERTION are processed here. + // Only MANAGED entities that are NOT SCHEDULED FOR INSERTION OR DELETION are processed here. $oid = spl_object_hash($entity); - if ( ! isset($this->entityInsertions[$oid]) && isset($this->entityStates[$oid])) { + if ( ! isset($this->entityInsertions[$oid]) && ! isset($this->entityDeletions[$oid]) && isset($this->entityStates[$oid])) { $this->computeChangeSet($class, $entity); } } From 88ae5883f367b8db5d87aa471c32a1d395b696ad Mon Sep 17 00:00:00 2001 From: flack Date: Sun, 17 Nov 2013 11:51:09 +0100 Subject: [PATCH 3/4] Add testcase --- .../ORM/Functional/Ticket/DDC2790Test.php | 77 +++++++++++++++++++ 1 file changed, 77 insertions(+) create mode 100644 tests/Doctrine/Tests/ORM/Functional/Ticket/DDC2790Test.php diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC2790Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC2790Test.php new file mode 100644 index 000000000..ee2668931 --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC2790Test.php @@ -0,0 +1,77 @@ +useModelSet('cms'); + parent::setUp(); + } + + /** + * Verifies that entities scheduled for deletion are not treated as updated by UoW, + * even if their properties are changed after the remove() call + */ + public function testIssue() + { + $this->_em->getEventManager()->addEventListener(Events::onFlush, new OnFlushListener); + + $entity = new CmsUser; + $entity->username = 'romanb'; + $entity->name = 'Roman'; + + $qb = $this->_em->createQueryBuilder(); + $qb->from(get_class($entity), 'c'); + $qb->select("count(c)"); + $initial = intval($qb->getQuery()->getSingleScalarResult()); + + $this->_em->persist($entity); + $this->_em->flush(); + + $this->_em->remove($entity); + // in Doctrine <2.5, this causes an UPDATE statement to be added before the DELETE statement + // (and consequently also triggers preUpdate/postUpdate for the entity in question) + $entity->name = 'Robin'; + + $this->_em->flush($entity); + + $qb = $this->_em->createQueryBuilder(); + $qb->from(get_class($entity), 'c'); + $qb->select("count(c)"); + $count = intval($qb->getQuery()->getSingleScalarResult()); + $this->assertEquals($initial, $count); + } +} + +class OnFlushListener +{ + /** + * onFLush listener that tries to cancel deletions by calling persist if the entity is listed + * as updated in UoW + */ + public function onFlush(OnFlushEventArgs $args) + { + $em = $args->getEntityManager(); + $uow = $em->getUnitOfWork(); + $deletions = $uow->getScheduledEntityDeletions(); + $updates = $uow->getScheduledEntityUpdates(); + + $undelete = array_intersect_key($deletions, $updates); + foreach ($undelete as $d) + { + $em->persist($d); + } + } +} From 3d12920cd46b82b7cecf0f1f47270569d37d5e12 Mon Sep 17 00:00:00 2001 From: flack Date: Sun, 17 Nov 2013 12:06:47 +0100 Subject: [PATCH 4/4] Add note about changed behaviour --- UPGRADE.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/UPGRADE.md b/UPGRADE.md index b864d8614..ae8e12ede 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -1,3 +1,15 @@ +# Upgrade to 2.5 + +## Updates on entities scheduled for deletion are no longer processed + +In Doctrine 2.4, if you modified properties of an entity scheduled for deletion, UnitOfWork would +produce an UPDATE statement to be executed right before the DELETE statement. The entity in question +was therefore present in ``UnitOfWork#entityUpdates``, which means that ``preUpdate`` and ``postUpdate`` +listeners were (quite pointlessly) called. In ``preFlush`` listeners, it used to be possible to undo +the scheduled deletion for updated entities (by calling ``persist()`` if the entity was found in both +``entityUpdates`` and ``entityDeletions``). This does not work any longer, because the entire changeset +calculation logic is optimized away. + # Upgrade to 2.4 ## BC BREAK: Compatibility Bugfix in PersistentCollection#matching()