Merge pull request #845 from flack/avoid-update-on-delete

Don't compute changeset for entities that are going to be deleted

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()

lib/Doctrine/ORM/UnitOfWork.php

@@ -463,10 +463,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);
         }
     }
@@ -756,10 +756,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);
                 }
             }

tests/Doctrine/Tests/ORM/Functional/Ticket/DDC2790Test.php

@@ -0,0 +1,77 @@
+<?php
+
+namespace Doctrine\Tests\ORM\Functional\Ticket;
+
+use Doctrine\Tests\Models\CMS\CmsUser;
+use Doctrine\ORM\Event\OnFlushEventArgs;
+use Doctrine\ORM\Events;
+
+/**
+ * @group DDC-2790
+ */
+class DDC2790Test extends \Doctrine\Tests\OrmFunctionalTestCase
+{
+    /**
+     * {@inheritDoc}
+     */
+    protected function setUp()
+    {
+        $this->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);
+        }
+    }
+}