From ab0c7b11c8663cd119cd4b8877c00a115c9cbfe5 Mon Sep 17 00:00:00 2001
From: romanb <romanb@625475ce-881a-0410-a577-b389adb331d8>
Date: Wed, 11 Nov 2009 16:20:29 +0000
Subject: [PATCH] [2.0][DDC-136] Some fixes to internal UnitOfWork logic.

---
 lib/Doctrine/ORM/UnitOfWork.php               | 27 +++++++++++-----
 .../ORM/Functional/BasicFunctionalTest.php    |  3 ++
 .../Tests/ORM/Functional/IdentityMapTest.php  | 32 +++++++++++++++++++
 3 files changed, 54 insertions(+), 8 deletions(-)

diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php
index 57297e9d8..76583c7e5 100644
--- a/lib/Doctrine/ORM/UnitOfWork.php
+++ b/lib/Doctrine/ORM/UnitOfWork.php
@@ -771,7 +771,14 @@ class UnitOfWork implements PropertyChangedListener
         foreach ($this->_entityDeletions as $oid => $entity) {
             if (get_class($entity) == $className) {
                 $persister->delete($entity);
-                unset($this->_entityDeletions[$oid]);
+                unset(
+                    $this->_entityDeletions[$oid],
+                    $this->_entityIdentifiers[$oid],
+                    $this->_originalEntityData[$oid]
+                    );
+                // Entity with this $oid after deletion treated as NEW, even if the $oid
+                // is obtained by a new entity because the old one went out of scope.
+                $this->_entityStates[$oid] = self::STATE_NEW;
                 
                 if ($hasLifecycleCallbacks) {
                     $class->invokeLifecycleCallbacks(Events::postRemove, $entity);
@@ -919,16 +926,20 @@ class UnitOfWork implements PropertyChangedListener
     public function scheduleForDelete($entity)
     {
         $oid = spl_object_hash($entity);
-        if ( ! $this->isInIdentityMap($entity)) {
-            return;
-        }
-
-        $this->removeFromIdentityMap($entity);
-
+        
         if (isset($this->_entityInsertions[$oid])) {
+            if ($this->isInIdentityMap($entity)) {
+                $this->removeFromIdentityMap($entity);
+            }
             unset($this->_entityInsertions[$oid]);
             return; // entity has not been persisted yet, so nothing more to do.
         }
+        
+        if ( ! $this->isInIdentityMap($entity)) {
+            return; // ignore
+        }
+        
+        $this->removeFromIdentityMap($entity);
 
         if (isset($this->_entityUpdates[$oid])) {
             unset($this->_entityUpdates[$oid]);
@@ -1102,7 +1113,7 @@ class UnitOfWork implements PropertyChangedListener
         if ($idHash === '') {
             return false;
         }
-
+        
         return isset($this->_identityMap[$classMetadata->rootEntityName][$idHash]);
     }
 
diff --git a/tests/Doctrine/Tests/ORM/Functional/BasicFunctionalTest.php b/tests/Doctrine/Tests/ORM/Functional/BasicFunctionalTest.php
index 88c60f903..362875486 100644
--- a/tests/Doctrine/Tests/ORM/Functional/BasicFunctionalTest.php
+++ b/tests/Doctrine/Tests/ORM/Functional/BasicFunctionalTest.php
@@ -66,6 +66,9 @@ class BasicFunctionalTest extends \Doctrine\Tests\OrmFunctionalTestCase
         $this->assertFalse($this->_em->getUnitOfWork()->isScheduledForDelete($user));
         $this->assertFalse($this->_em->getUnitOfWork()->isScheduledForDelete($ph));
         $this->assertFalse($this->_em->getUnitOfWork()->isScheduledForDelete($ph2));
+        $this->assertEquals(\Doctrine\ORM\UnitOfWork::STATE_NEW, $this->_em->getUnitOfWork()->getEntityState($user));
+        $this->assertEquals(\Doctrine\ORM\UnitOfWork::STATE_NEW, $this->_em->getUnitOfWork()->getEntityState($ph));
+        $this->assertEquals(\Doctrine\ORM\UnitOfWork::STATE_NEW, $this->_em->getUnitOfWork()->getEntityState($ph2));
     }
 
     public function testOneToManyAssociationModification()
diff --git a/tests/Doctrine/Tests/ORM/Functional/IdentityMapTest.php b/tests/Doctrine/Tests/ORM/Functional/IdentityMapTest.php
index ba0c55242..8d39c235e 100644
--- a/tests/Doctrine/Tests/ORM/Functional/IdentityMapTest.php
+++ b/tests/Doctrine/Tests/ORM/Functional/IdentityMapTest.php
@@ -252,5 +252,37 @@ class IdentityMapTest extends \Doctrine\Tests\OrmFunctionalTestCase
         // Now the collection should be refreshed with correct count
         $this->assertEquals(4, count($user2->getPhonenumbers()));
     }
+    
+    public function testReusedSplObjectHashDoesNotConfuseUnitOfWork()
+    {
+        $hash1 = $this->subRoutine($this->_em);
+        // Make sure cycles are collected NOW, because a PersistentCollection references
+        // its owner, hence without forcing gc on cycles now the object will not (yet)
+        // be garbage collected and thus the object hash is not reused.
+        // This is not a memory leak!
+        gc_collect_cycles();
+        
+        $user1 = new CmsUser;
+        $user1->status = 'dev';
+        $user1->username = 'jwage';
+        $user1->name = 'Jonathan W.';
+        $hash2 = spl_object_hash($user1);
+        $this->assertEquals($hash1, $hash2); // Hash reused!
+        $this->_em->persist($user1);
+        $this->_em->flush();
+    }
+    
+    private function subRoutine($em) {
+        $user = new CmsUser;
+        $user->status = 'dev';
+        $user->username = 'romanb';
+        $user->name = 'Roman B.';
+        $em->persist($user);
+        $em->flush();
+        $em->remove($user);
+        $em->flush();
+        
+        return spl_object_hash($user);
+    }
 }