Fixed DDC-1608. Non-initialized PersistentCollection methods removeElement and contains now deal correctly with managed entities.

lib/Doctrine/ORM/Persisters/ManyToManyPersister.php

@@ -253,10 +253,15 @@ class ManyToManyPersister extends AbstractCollectionPersister
     {
         $uow = $this->_em->getUnitOfWork();
 
-        // shortcut for new entities
+        // Shortcut for new entities
         $entityState = $uow->getEntityState($element, UnitOfWork::STATE_NEW);
-        if ($entityState === UnitOfWork::STATE_NEW || 
+
-            ($entityState === UnitOfWork::STATE_MANAGED && $uow->isScheduledForInsert($element))) {
+        if ($entityState === UnitOfWork::STATE_NEW) {
+            return false;
+        }
+
+        // Entity is scheduled for inclusion
+        if ($entityState === UnitOfWork::STATE_MANAGED && $uow->isScheduledForInsert($element)) {
             return false;
         }
 
@@ -277,7 +282,15 @@ class ManyToManyPersister extends AbstractCollectionPersister
         $uow = $this->_em->getUnitOfWork();
 
         // shortcut for new entities
-        if ($uow->getEntityState($element, UnitOfWork::STATE_NEW) === UnitOfWork::STATE_NEW) {
+        $entityState = $uow->getEntityState($element, UnitOfWork::STATE_NEW);
+
+        if ($entityState === UnitOfWork::STATE_NEW) {
+            return false;
+        }
+
+        // If Entity is scheduled for inclusion, it is not in this collection.
+        // We can assure that because it would have return true before on array check
+        if ($entityState === UnitOfWork::STATE_MANAGED && $uow->isScheduledForInsert($element)) {
             return false;
         }
 

lib/Doctrine/ORM/Persisters/OneToManyPersister.php

@@ -159,7 +159,14 @@ class OneToManyPersister extends AbstractCollectionPersister
         $uow     = $this->_em->getUnitOfWork();
 
         // shortcut for new entities
-        if ($uow->getEntityState($element, UnitOfWork::STATE_NEW) == UnitOfWork::STATE_NEW) {
+        $entityState = $uow->getEntityState($element, UnitOfWork::STATE_NEW);
+
+        if ($entityState === UnitOfWork::STATE_NEW) {
+            return false;
+        }
+
+        // Entity is scheduled for inclusion
+        if ($entityState === UnitOfWork::STATE_MANAGED && $uow->isScheduledForInsert($element)) {
             return false;
         }
 
@@ -168,7 +175,7 @@ class OneToManyPersister extends AbstractCollectionPersister
         // only works with single id identifier entities. Will throw an
         // exception in Entity Persisters if that is not the case for the
         // 'mappedBy' field.
-        $id = current( $uow->getEntityIdentifier($coll->getOwner()) );
+        $id = current( $uow->getEntityIdentifier($coll->getOwner()));
 
         return $persister->exists($element, array($mapping['mappedBy'] => $id));
     }
@@ -183,7 +190,15 @@ class OneToManyPersister extends AbstractCollectionPersister
         $uow = $this->_em->getUnitOfWork();
 
         // shortcut for new entities
-        if ($uow->getEntityState($element, UnitOfWork::STATE_NEW) == UnitOfWork::STATE_NEW) {
+        $entityState = $uow->getEntityState($element, UnitOfWork::STATE_NEW);
+
+        if ($entityState === UnitOfWork::STATE_NEW) {
+            return false;
+        }
+
+        // If Entity is scheduled for inclusion, it is not in this collection.
+        // We can assure that because it would have return true before on array check
+        if ($entityState === UnitOfWork::STATE_MANAGED && $uow->isScheduledForInsert($element)) {
             return false;
         }
 

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

@@ -64,7 +64,7 @@ class ExtraLazyCollectionTest extends \Doctrine\Tests\OrmFunctionalTestCase
     /**
      * @group DDC-546
      */
-    public function testCountWhenNewEntitysPresent()
+    public function testCountWhenNewEntityPresent()
     {
         $user = $this->_em->find('Doctrine\Tests\Models\CMS\CmsUser', $this->userId);
 
@@ -223,13 +223,15 @@ class ExtraLazyCollectionTest extends \Doctrine\Tests\OrmFunctionalTestCase
         $user = $this->_em->find('Doctrine\Tests\Models\CMS\CmsUser', $this->userId);
         $this->assertFalse($user->articles->isInitialized(), "Pre-Condition: Collection is not initialized.");
 
-        $article = $this->_em->find('Doctrine\Tests\Models\CMS\CmsArticle', $this->articleId);
+        // Test One to Many existance retrieved from DB
-
+        $article    = $this->_em->find('Doctrine\Tests\Models\CMS\CmsArticle', $this->articleId);
         $queryCount = $this->getCurrentQueryCount();
+
         $this->assertTrue($user->articles->contains($article));
         $this->assertFalse($user->articles->isInitialized(), "Post-Condition: Collection is not initialized.");
         $this->assertEquals($queryCount + 1, $this->getCurrentQueryCount());
 
+        // Test One to Many existance with state new
         $article = new \Doctrine\Tests\Models\CMS\CmsArticle();
         $article->topic = "Testnew";
         $article->text = "blub";
@@ -238,12 +240,26 @@ class ExtraLazyCollectionTest extends \Doctrine\Tests\OrmFunctionalTestCase
         $this->assertFalse($user->articles->contains($article));
         $this->assertEquals($queryCount, $this->getCurrentQueryCount(), "Checking for contains of new entity should cause no query to be executed.");
 
+        // Test One to Many existance with state clear
         $this->_em->persist($article);
         $this->_em->flush();
 
         $queryCount = $this->getCurrentQueryCount();
         $this->assertFalse($user->articles->contains($article));
-        $this->assertEquals($queryCount+1, $this->getCurrentQueryCount(), "Checking for contains of managed entity should cause one query to be executed.");
+        $this->assertEquals($queryCount+1, $this->getCurrentQueryCount(), "Checking for contains of persisted entity should cause one query to be executed.");
+        $this->assertFalse($user->articles->isInitialized(), "Post-Condition: Collection is not initialized.");
+
+        // Test One to Many existance with state managed
+        $article = new \Doctrine\Tests\Models\CMS\CmsArticle();
+        $article->topic = "How to not fail anymore on tests";
+        $article->text = "That is simple! Just write more tests!";
+
+        $this->_em->persist($article);
+
+        $queryCount = $this->getCurrentQueryCount();
+
+        $this->assertFalse($user->articles->contains($article));
+        $this->assertEquals($queryCount, $this->getCurrentQueryCount(), "Checking for contains of managed entity (but not persisted) should cause no query to be executed.");
         $this->assertFalse($user->articles->isInitialized(), "Post-Condition: Collection is not initialized.");
     }
 
@@ -255,6 +271,7 @@ class ExtraLazyCollectionTest extends \Doctrine\Tests\OrmFunctionalTestCase
         $user = $this->_em->find('Doctrine\Tests\Models\CMS\CmsUser', $this->userId);
         $this->assertFalse($user->groups->isInitialized(), "Pre-Condition: Collection is not initialized.");
 
+        // Test Many to Many existance retrieved from DB
         $group      = $this->_em->find('Doctrine\Tests\Models\CMS\CmsGroup', $this->groupId);
         $queryCount = $this->getCurrentQueryCount();
 
@@ -262,6 +279,7 @@ class ExtraLazyCollectionTest extends \Doctrine\Tests\OrmFunctionalTestCase
         $this->assertEquals($queryCount + 1, $this->getCurrentQueryCount(), "Checking for contains of managed entity should cause one query to be executed.");
         $this->assertFalse($user->groups->isInitialized(), "Post-Condition: Collection is not initialized.");
 
+        // Test Many to Many existance with state new
         $group = new \Doctrine\Tests\Models\CMS\CmsGroup();
         $group->name = "A New group!";
 
@@ -271,13 +289,26 @@ class ExtraLazyCollectionTest extends \Doctrine\Tests\OrmFunctionalTestCase
         $this->assertEquals($queryCount, $this->getCurrentQueryCount(), "Checking for contains of new entity should cause no query to be executed.");
         $this->assertFalse($user->groups->isInitialized(), "Post-Condition: Collection is not initialized.");
 
+        // Test Many to Many existance with state clear
         $this->_em->persist($group);
         $this->_em->flush();
 
         $queryCount = $this->getCurrentQueryCount();
 
         $this->assertFalse($user->groups->contains($group));
-        $this->assertEquals($queryCount + 1, $this->getCurrentQueryCount(), "Checking for contains of managed entity should cause one query to be executed.");
+        $this->assertEquals($queryCount + 1, $this->getCurrentQueryCount(), "Checking for contains of persisted entity should cause one query to be executed.");
+        $this->assertFalse($user->groups->isInitialized(), "Post-Condition: Collection is not initialized.");
+
+        // Test Many to Many existance with state managed
+        $group = new \Doctrine\Tests\Models\CMS\CmsGroup();
+        $group->name = "My managed group";
+
+        $this->_em->persist($group);
+
+        $queryCount = $this->getCurrentQueryCount();
+
+        $this->assertFalse($user->groups->contains($group));
+        $this->assertEquals($queryCount, $this->getCurrentQueryCount(), "Checking for contains of managed entity (but not persisted) should cause no query to be executed.");
         $this->assertFalse($user->groups->isInitialized(), "Post-Condition: Collection is not initialized.");
     }
 
@@ -313,6 +344,7 @@ class ExtraLazyCollectionTest extends \Doctrine\Tests\OrmFunctionalTestCase
         $user = $this->_em->find('Doctrine\Tests\Models\CMS\CmsUser', $this->userId);
         $this->assertFalse($user->articles->isInitialized(), "Pre-Condition: Collection is not initialized.");
 
+        // Test One to Many removal with Entity retrieved from DB
         $article    = $this->_em->find('Doctrine\Tests\Models\CMS\CmsArticle', $this->articleId);
         $queryCount = $this->getCurrentQueryCount();
 
@@ -321,6 +353,7 @@ class ExtraLazyCollectionTest extends \Doctrine\Tests\OrmFunctionalTestCase
         $this->assertFalse($user->articles->isInitialized(), "Post-Condition: Collection is not initialized.");
         $this->assertEquals($queryCount + 1, $this->getCurrentQueryCount());
 
+        // Test One to Many removal with Entity state as new
         $article = new \Doctrine\Tests\Models\CMS\CmsArticle();
         $article->topic = "Testnew";
         $article->text = "blub";
@@ -331,6 +364,7 @@ class ExtraLazyCollectionTest extends \Doctrine\Tests\OrmFunctionalTestCase
 
         $this->assertEquals($queryCount, $this->getCurrentQueryCount(), "Removing a new entity should cause no query to be executed.");
 
+        // Test One to Many removal with Entity state as clean
         $this->_em->persist($article);
         $this->_em->flush();
 
@@ -338,8 +372,21 @@ class ExtraLazyCollectionTest extends \Doctrine\Tests\OrmFunctionalTestCase
 
         $user->articles->removeElement($article);
 
-        $this->assertEquals($queryCount + 1, $this->getCurrentQueryCount(), "Removing a managed entity should cause one query to be executed.");
+        $this->assertEquals($queryCount + 1, $this->getCurrentQueryCount(), "Removing a persisted entity should cause one query to be executed.");
         $this->assertFalse($user->articles->isInitialized(), "Post-Condition: Collection is not initialized.");
+
+        // Test One to Many removal with Entity state as managed
+        $article = new \Doctrine\Tests\Models\CMS\CmsArticle();
+        $article->topic = "How to not fail anymore on tests";
+        $article->text = "That is simple! Just write more tests!";
+
+        $this->_em->persist($article);
+
+        $queryCount = $this->getCurrentQueryCount();
+
+        $user->articles->removeElement($article);
+
+        $this->assertEquals($queryCount, $this->getCurrentQueryCount(), "Removing a managed entity should cause no query to be executed.");
     }
 
     /**
@@ -350,14 +397,16 @@ class ExtraLazyCollectionTest extends \Doctrine\Tests\OrmFunctionalTestCase
         $user = $this->_em->find('Doctrine\Tests\Models\CMS\CmsUser', $this->userId);
         $this->assertFalse($user->groups->isInitialized(), "Pre-Condition: Collection is not initialized.");
 
+        // Test Many to Many removal with Entity retrieved from DB
         $group      = $this->_em->find('Doctrine\Tests\Models\CMS\CmsGroup', $this->groupId);
         $queryCount = $this->getCurrentQueryCount();
 
         $user->groups->removeElement($group);
 
-        $this->assertEquals($queryCount + 1, $this->getCurrentQueryCount(), "Removing a managed entity should cause one query to be executed.");
+        $this->assertEquals($queryCount + 1, $this->getCurrentQueryCount(), "Removing a persisted entity should cause one query to be executed.");
         $this->assertFalse($user->groups->isInitialized(), "Post-Condition: Collection is not initialized.");
 
+        // Test Many to Many removal with Entity state as new
         $group = new \Doctrine\Tests\Models\CMS\CmsGroup();
         $group->name = "A New group!";
 
@@ -368,6 +417,7 @@ class ExtraLazyCollectionTest extends \Doctrine\Tests\OrmFunctionalTestCase
         $this->assertEquals($queryCount, $this->getCurrentQueryCount(), "Removing new entity should cause no query to be executed.");
         $this->assertFalse($user->groups->isInitialized(), "Post-Condition: Collection is not initialized.");
 
+        // Test Many to Many removal with Entity state as clean
         $this->_em->persist($group);
         $this->_em->flush();
 
@@ -375,7 +425,20 @@ class ExtraLazyCollectionTest extends \Doctrine\Tests\OrmFunctionalTestCase
 
         $user->groups->removeElement($group);
 
-        $this->assertEquals($queryCount + 1, $this->getCurrentQueryCount(), "Removing a managed entity should cause one query to be executed.");
+        $this->assertEquals($queryCount + 1, $this->getCurrentQueryCount(), "Removing a persisted entity should cause one query to be executed.");
+        $this->assertFalse($user->groups->isInitialized(), "Post-Condition: Collection is not initialized.");
+
+        // Test Many to Many removal with Entity state as managed
+        $group = new \Doctrine\Tests\Models\CMS\CmsGroup();
+        $group->name = "A New group!";
+
+        $this->_em->persist($group);
+
+        $queryCount = $this->getCurrentQueryCount();
+
+        $user->groups->removeElement($group);
+
+        $this->assertEquals($queryCount, $this->getCurrentQueryCount(), "Removing a managed entity should cause no query to be executed.");
         $this->assertFalse($user->groups->isInitialized(), "Post-Condition: Collection is not initialized.");
     }
 

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

@@ -1,55 +0,0 @@
-<?php
-
-namespace Doctrine\Tests\ORM\Functional;
-
-use Doctrine\Common\Collections\ArrayCollection;
-require_once __DIR__ . '/../../TestInit.php';
-
-/**
- * 
- */
-class ManyToManyExtraLazyContainsTest extends \Doctrine\Tests\OrmFunctionalTestCase
-{
-    public function setUp()
-    {
-        $this->useModelSet('company');
-        parent::setUp();
-    }
-
-    public function testManyToManyExtraLazyContainsAddedPendingInsertEntityIsTrue()
-    {
-        $contract = new \Doctrine\Tests\Models\Company\CompanyFlexContract();
-
-        $this->_em->persist($contract);
-        $this->_em->flush();
-        
-        $this->_em->clear();
-        $contract = $this->_em->find('Doctrine\Tests\Models\Company\CompanyFlexContract', $contract->getId());
-        
-        $pendingInsertManager = new \Doctrine\Tests\Models\Company\CompanyManager();
-        $this->_em->persist($pendingInsertManager);
-        $contract->getManagers()->add($pendingInsertManager);
-        
-        $result = $contract->getManagers()->contains($pendingInsertManager);
-
-        $this->assertTrue($result);
-    }
-
-    public function testManyToManyExtraLazyContainsNonAddedPendingInsertEntityIsFalse()
-    {
-        $contract = new \Doctrine\Tests\Models\Company\CompanyFlexContract();
-
-        $this->_em->persist($contract);
-        $this->_em->flush();
-        
-        $this->_em->clear();
-        $contract = $this->_em->find('Doctrine\Tests\Models\Company\CompanyFlexContract', $contract->getId());
-        
-        $pendingInsertManager = new \Doctrine\Tests\Models\Company\CompanyManager();
-        $this->_em->persist($pendingInsertManager);
-        
-        $result = $contract->getManagers()->contains($pendingInsertManager);
-
-        $this->assertFalse($result);
-    }
-}