[2.0][DDC-158] Fixed. Collections did not take snapshots after lazy...

[2.0][DDC-158] Fixed. Collections did not take snapshots after lazy initialization leading to wrong change sets.

lib/Doctrine/ORM/ORMException.php

@@ -13,4 +13,11 @@ class ORMException extends \Exception
     {
         return new self("Unrecognized field: $field");
     }
+    
+    public static function removedEntityInCollectionDetected($entity, $assoc)
+    {
+        return new self("Removed entity of type " . get_class($entity)
+                . " detected in collection '" . $assoc->sourceFieldName . "' during flush."
+                . " Remove deleted entities from collections.");
+    }
 }

lib/Doctrine/ORM/PersistentCollection.php

@@ -241,6 +241,7 @@ final class PersistentCollection implements \Doctrine\Common\Collections\Collect
                     $this->_coll->add($obj);
                 }
             }
+            $this->takeSnapshot();
             $this->_initialized = true;
         }
     }
@@ -376,6 +377,10 @@ final class PersistentCollection implements \Doctrine\Common\Collections\Collect
      */
     public function remove($key)
     {
+        // TODO: If the keys are persistent as well (not yet implemented)
+        //       and the collection is not initialized and orphanRemoval is
+        //       not used we can issue a straight SQL delete/update on the
+        //       association (table). Without initializing the collection.
         $this->_initialize();
         $removed = $this->_coll->remove($key);
         if ($removed) {
@@ -394,6 +399,16 @@ final class PersistentCollection implements \Doctrine\Common\Collections\Collect
      */
     public function removeElement($element)
     {
+        // TODO: Assuming the identity of entities in a collection is always based
+        //       on their primary key (there is no equals/hashCode in PHP),
+        //       if the collection is not initialized, we could issue a straight
+        //       SQL DELETE/UPDATE on the association (table) without initializing
+        //       the collection.
+        /*if ( ! $this->_initialized) {
+            $this->_em->getUnitOfWork()->getCollectionPersister($this->_association)
+                ->deleteRows($this, $element);
+        }*/
+        
         $this->_initialize();
         $result = $this->_coll->removeElement($element);
         $this->_changed();
@@ -414,6 +429,12 @@ final class PersistentCollection implements \Doctrine\Common\Collections\Collect
      */
     public function contains($element)
     {
+        // TODO: Assuming the identity of entities in a collection is always based
+        //       on their primary key (there is no equals/hashCode in PHP),
+        //       if the collection is not initialized, we could issue a straight
+        //       SQL "SELECT 1" on the association (table) without initializing
+        //       the collection.
+        
         $this->_initialize();
         return $this->_coll->contains($element);
     }

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

@@ -32,6 +32,7 @@ class AllTests
         $suite->addTestSuite('Doctrine\Tests\ORM\Functional\OneToOneUnidirectionalAssociationTest');
         $suite->addTestSuite('Doctrine\Tests\ORM\Functional\OneToOneBidirectionalAssociationTest');
         $suite->addTestSuite('Doctrine\Tests\ORM\Functional\OneToManyBidirectionalAssociationTest');
+        $suite->addTestSuite('Doctrine\Tests\ORM\Functional\ManyToManyBasicAssociationTest');
         $suite->addTestSuite('Doctrine\Tests\ORM\Functional\ManyToManyUnidirectionalAssociationTest');
         $suite->addTestSuite('Doctrine\Tests\ORM\Functional\ManyToManyBidirectionalAssociationTest');
         $suite->addTestSuite('Doctrine\Tests\ORM\Functional\OneToOneSelfReferentialAssociationTest');

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

+<?php
+
+namespace Doctrine\Tests\ORM\Functional;
+
+use Doctrine\Tests\Models\CMS\CmsUser,
+    Doctrine\Tests\Models\CMS\CmsGroup;
+
+require_once __DIR__ . '/../../TestInit.php';
+
+/**
+ * Basic many-to-many association tests.
+ * ("Working with associations")
+ * 
+ * @author robo
+ */
+class ManyToManyBasicAssociationTest extends \Doctrine\Tests\OrmFunctionalTestCase
+{
+    protected function setUp()
+    {
+        $this->useModelSet('cms');
+        parent::setUp();
+    }
+    
+    public function testManyToManyAddRemove()
+    {
+        // Set up user with 2 groups
+        $user = new CmsUser;
+        $user->username = 'romanb';
+        $user->status = 'dev';
+        $user->name = 'Roman B.';
+        
+        $group1 = new CmsGroup;
+        $group1->name = 'Developers';
+        
+        $group2 = new CmsGroup;
+        $group2->name = 'Humans';
+        
+        $user->addGroup($group1);
+        $user->addGroup($group2);
+        
+        $this->_em->persist($user); // cascades to groups
+        $this->_em->flush();
+        
+        $this->_em->clear();
+        
+        $uRep = $this->_em->getRepository(get_class($user));
+    
+        // Get user
+        $user = $uRep->findOneById($user->getId());
+    
+        $this->assertFalse($user->getGroups()->isInitialized());
+        
+        // Check groups
+        $this->assertEquals(2, $user->getGroups()->count());
+    
+        $this->assertTrue($user->getGroups()->isInitialized());
+        
+        // Remove first group
+        unset($user->groups[0]);
+        //$user->getGroups()->remove(0);
+    
+        $this->_em->flush();
+        $this->_em->clear();
+    
+        // Reload same user
+        $user2 = $uRep->findOneById($user->getId());
+    
+        // Check groups
+        $this->assertEquals(1, $user2->getGroups()->count());        
+    }
+    
+    public function testManyToManyInverseSideIgnored()
+    {
+        $user = new CmsUser;
+        $user->username = 'romanb';
+        $user->status = 'dev';
+        $user->name = 'Roman B.';
+        
+        $group = new CmsGroup;
+        $group->name = 'Humans';
+        
+        // modify directly, addUser() would also (properly) set the owning side
+        $group->users[] = $user;
+        
+        $this->_em->persist($user);
+        $this->_em->persist($group);
+        $this->_em->flush();
+        $this->_em->clear();
+        
+        // Association should not exist
+        $user2 = $this->_em->find(get_class($user), $user->getId());
+        $this->assertEquals(0, $user2->getGroups()->count());
+    }
+    
+}

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

@@ -59,10 +59,13 @@ class ManyToManyBidirectionalAssociationTest extends AbstractManyToManyAssociati
 
         $this->_em->flush();
 
-        $this->assertForeignKeysNotContain($this->firstProduct->getId(),
-                                   $this->firstCategory->getId());
-        $this->assertForeignKeysContain($this->firstProduct->getId(),
-                                   $this->secondCategory->getId());
+        $this->assertForeignKeysNotContain($this->firstProduct->getId(), $this->firstCategory->getId());
+        $this->assertForeignKeysContain($this->firstProduct->getId(), $this->secondCategory->getId());
+        
+        $this->firstProduct->getCategories()->remove(1);
+        $this->_em->flush();
+        
+        $this->assertForeignKeysNotContain($this->firstProduct->getId(), $this->secondCategory->getId());
     }
 
     public function testEagerLoadsInverseSide()