Added support to removeElement remove items without initializing the PersistentCollection.

lib/Doctrine/ORM/PersistentCollection.php

@@ -21,6 +21,7 @@ namespace Doctrine\ORM;
 
 use Doctrine\ORM\Mapping\ClassMetadata,
     Doctrine\Common\Collections\Collection,
+    Doctrine\Common\Collections\ArrayCollection,
     Closure;
 
 /**
@@ -160,16 +161,18 @@ final class PersistentCollection implements Collection
     public function hydrateAdd($element)
     {
         $this->coll->add($element);
+
         // If _backRefFieldName is set and its a one-to-many association,
         // we need to set the back reference.
         if ($this->backRefFieldName && $this->association['type'] == ClassMetadata::ONE_TO_MANY) {
             // Set back reference to owner
-            $this->typeClass->reflFields[$this->backRefFieldName]
-                    ->setValue($element, $this->owner);
+            $this->typeClass->reflFields[$this->backRefFieldName]->setValue(
+                $element, $this->owner
+            );
+
             $this->em->getUnitOfWork()->setOriginalEntityProperty(
-                    spl_object_hash($element),
-                    $this->backRefFieldName,
-                    $this->owner);
+                spl_object_hash($element), $this->backRefFieldName, $this->owner
+            );
         }
     }
 
@@ -183,12 +186,14 @@ final class PersistentCollection implements Collection
     public function hydrateSet($key, $element)
     {
         $this->coll->set($key, $element);
+
         // If _backRefFieldName is set, then the association is bidirectional
         // and we need to set the back reference.
         if ($this->backRefFieldName && $this->association['type'] == ClassMetadata::ONE_TO_MANY) {
             // Set back reference to owner
-            $this->typeClass->reflFields[$this->backRefFieldName]
-                    ->setValue($element, $this->owner);
+            $this->typeClass->reflFields[$this->backRefFieldName]->setValue(
+                $element, $this->owner
+            );
         }
     }
 
@@ -198,24 +203,32 @@ final class PersistentCollection implements Collection
      */
     public function initialize()
     {
-        if ( ! $this->initialized && $this->association) {
-            if ($this->isDirty) {
+        if ($this->initialized || ! $this->association) {
+            return;
+        }
+
         // Has NEW objects added through add(). Remember them.
+        $newObjects = array();
+
+        if ($this->isDirty) {
             $newObjects = $this->coll->toArray();
         }
+
         $this->coll->clear();
         $this->em->getUnitOfWork()->loadCollection($this);
         $this->takeSnapshot();
+
         // Reattach NEW objects added through add(), if any.
-            if (isset($newObjects)) {
+        if ($newObjects) {
             foreach ($newObjects as $obj) {
                 $this->coll->add($obj);
             }
+
             $this->isDirty = true;
         }
+
         $this->initialized = true;
     }
-    }
 
     /**
      * INTERNAL:
@@ -246,8 +259,11 @@ final class PersistentCollection implements Collection
      */
     public function getDeleteDiff()
     {
-        return array_udiff_assoc($this->snapshot, $this->coll->toArray(),
-                function($a, $b) {return $a === $b ? 0 : 1;});
+        return array_udiff_assoc(
+            $this->snapshot,
+            $this->coll->toArray(),
+            function($a, $b) { return $a === $b ? 0 : 1; }
+        );
     }
 
     /**
@@ -258,8 +274,11 @@ final class PersistentCollection implements Collection
      */
     public function getInsertDiff()
     {
-        return array_udiff_assoc($this->coll->toArray(), $this->snapshot,
-                function($a, $b) {return $a === $b ? 0 : 1;});
+        return array_udiff_assoc(
+            $this->coll->toArray(),
+            $this->snapshot,
+            function($a, $b) { return $a === $b ? 0 : 1; }
+        );
     }
 
     /**
@@ -277,14 +296,19 @@ final class PersistentCollection implements Collection
      */
     private function changed()
     {
-        if ( ! $this->isDirty) {
+        if ($this->isDirty) {
+            return;
+        }
+
         $this->isDirty = true;
-            if ($this->association !== null && $this->association['isOwningSide'] && $this->association['type'] == ClassMetadata::MANY_TO_MANY &&
+
+        if ($this->association !== null &&
+            $this->association['isOwningSide'] &&
+            $this->association['type'] == ClassMetadata::MANY_TO_MANY &&
             $this->em->getClassMetadata(get_class($this->owner))->isChangeTrackingNotify()) {
             $this->em->getUnitOfWork()->scheduleForDirtyCheck($this->owner);
         }
     }
-    }
 
     /**
      * Gets a boolean flag indicating whether this collection is dirty which means
@@ -331,6 +355,7 @@ final class PersistentCollection implements Collection
     public function first()
     {
         $this->initialize();
+
         return $this->coll->first();
     }
 
@@ -338,6 +363,7 @@ final class PersistentCollection implements Collection
     public function last()
     {
         $this->initialize();
+
         return $this->coll->last();
     }
 
@@ -351,14 +377,20 @@ final class PersistentCollection implements Collection
         //       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) {
+
+        if ( ! $removed) {
+            return $removed;
+        }
+
         $this->changed();
-            if ($this->association !== null && $this->association['type'] == ClassMetadata::ONE_TO_MANY &&
+
+        if ($this->association !== null &&
+            $this->association['type'] == ClassMetadata::ONE_TO_MANY &&
             $this->association['orphanRemoval']) {
             $this->em->getUnitOfWork()->scheduleOrphanRemoval($removed);
         }
-        }
 
         return $removed;
     }
@@ -368,25 +400,36 @@ final class PersistentCollection implements Collection
      */
     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);
-        }*/
+        if ( ! $this->initialized && $this->association['fetch'] === Mapping\ClassMetadataInfo::FETCH_EXTRA_LAZY) {
+            if ($this->coll->contains($element)) {
+                return $this->coll->removeElement($element);
+            }
+
+            $persister = $this->em->getUnitOfWork()->getCollectionPersister($this->association);
+
+            if ($persister->removeElement($this, $element)) {
+                return $element;
+            }
+
+            return null;
+        }
 
         $this->initialize();
+
         $removed = $this->coll->removeElement($element);
-        if ($removed) {
+
+        if ( ! $removed) {
+            return $removed;
+        }
+
         $this->changed();
-            if ($this->association !== null && $this->association['type'] == ClassMetadata::ONE_TO_MANY &&
+
+        if ($this->association !== null &&
+            $this->association['type'] == ClassMetadata::ONE_TO_MANY &&
             $this->association['orphanRemoval']) {
             $this->em->getUnitOfWork()->scheduleOrphanRemoval($element);
         }
-        }
+
         return $removed;
     }
 
@@ -396,6 +439,7 @@ final class PersistentCollection implements Collection
     public function containsKey($key)
     {
         $this->initialize();
+
         return $this->coll->containsKey($key);
     }
 
@@ -405,13 +449,13 @@ final class PersistentCollection implements Collection
     public function contains($element)
     {
         if ( ! $this->initialized && $this->association['fetch'] == Mapping\ClassMetadataInfo::FETCH_EXTRA_LAZY) {
-            return $this->coll->contains($element) ||
-                   $this->em->getUnitOfWork()
-                            ->getCollectionPersister($this->association)
-                            ->contains($this, $element);
+            $persister = $this->em->getUnitOfWork()->getCollectionPersister($this->association);
+
+            return $this->coll->contains($element) || $persister->contains($this, $element);
         }
 
         $this->initialize();
+
         return $this->coll->contains($element);
     }
 
@@ -421,6 +465,7 @@ final class PersistentCollection implements Collection
     public function exists(Closure $p)
     {
         $this->initialize();
+
         return $this->coll->exists($p);
     }
 
@@ -430,6 +475,7 @@ final class PersistentCollection implements Collection
     public function indexOf($element)
     {
         $this->initialize();
+
         return $this->coll->indexOf($element);
     }
 
@@ -439,6 +485,7 @@ final class PersistentCollection implements Collection
     public function get($key)
     {
         $this->initialize();
+
         return $this->coll->get($key);
     }
 
@@ -448,6 +495,7 @@ final class PersistentCollection implements Collection
     public function getKeys()
     {
         $this->initialize();
+
         return $this->coll->getKeys();
     }
 
@@ -457,6 +505,7 @@ final class PersistentCollection implements Collection
     public function getValues()
     {
         $this->initialize();
+
         return $this->coll->getValues();
     }
 
@@ -466,12 +515,13 @@ final class PersistentCollection implements Collection
     public function count()
     {
         if ( ! $this->initialized && $this->association['fetch'] == Mapping\ClassMetadataInfo::FETCH_EXTRA_LAZY) {
-            return $this->em->getUnitOfWork()
-                        ->getCollectionPersister($this->association)
-                        ->count($this) + ($this->isDirty ? $this->coll->count() : 0);
+            $persister = $this->em->getUnitOfWork()->getCollectionPersister($this->association);
+
+            return $persister->count($this) + ($this->isDirty ? $this->coll->count() : 0);
         }
 
         $this->initialize();
+
         return $this->coll->count();
     }
 
@@ -481,7 +531,9 @@ final class PersistentCollection implements Collection
     public function set($key, $value)
     {
         $this->initialize();
+
         $this->coll->set($key, $value);
+
         $this->changed();
     }
 
@@ -491,7 +543,9 @@ final class PersistentCollection implements Collection
     public function add($value)
     {
         $this->coll->add($value);
+
         $this->changed();
+
         return true;
     }
 
@@ -501,6 +555,7 @@ final class PersistentCollection implements Collection
     public function isEmpty()
     {
         $this->initialize();
+
         return $this->coll->isEmpty();
     }
 
@@ -510,6 +565,7 @@ final class PersistentCollection implements Collection
     public function getIterator()
     {
         $this->initialize();
+
         return $this->coll->getIterator();
     }
 
@@ -519,6 +575,7 @@ final class PersistentCollection implements Collection
     public function map(Closure $func)
     {
         $this->initialize();
+
         return $this->coll->map($func);
     }
 
@@ -528,6 +585,7 @@ final class PersistentCollection implements Collection
     public function filter(Closure $p)
     {
         $this->initialize();
+
         return $this->coll->filter($p);
     }
 
@@ -537,6 +595,7 @@ final class PersistentCollection implements Collection
     public function forAll(Closure $p)
     {
         $this->initialize();
+
         return $this->coll->forAll($p);
     }
 
@@ -546,6 +605,7 @@ final class PersistentCollection implements Collection
     public function partition(Closure $p)
     {
         $this->initialize();
+
         return $this->coll->partition($p);
     }
 
@@ -555,6 +615,7 @@ final class PersistentCollection implements Collection
     public function toArray()
     {
         $this->initialize();
+
         return $this->coll->toArray();
     }
 
@@ -566,19 +627,28 @@ final class PersistentCollection implements Collection
         if ($this->initialized && $this->isEmpty()) {
             return;
         }
+
+        $uow = $this->em->getUnitOfWork();
+
         if ($this->association['type'] == ClassMetadata::ONE_TO_MANY && $this->association['orphanRemoval']) {
             // we need to initialize here, as orphan removal acts like implicit cascadeRemove,
             // hence for event listeners we need the objects in memory.
             $this->initialize();
+
             foreach ($this->coll as $element) {
-                $this->em->getUnitOfWork()->scheduleOrphanRemoval($element);
+                $uow->scheduleOrphanRemoval($element);
             }
         }
+
         $this->coll->clear();
+
         $this->initialized = true; // direct call, {@link initialize()} is too expensive
+
         if ($this->association['isOwningSide']) {
             $this->changed();
-            $this->em->getUnitOfWork()->scheduleCollectionDeletion($this);
+
+            $uow->scheduleCollectionDeletion($this);
+
             $this->takeSnapshot();
         }
     }
@@ -622,6 +692,7 @@ final class PersistentCollection implements Collection
         if ( ! isset($offset)) {
             return $this->add($value);
         }
+
         return $this->set($offset, $value);
     }
 
@@ -656,6 +727,8 @@ final class PersistentCollection implements Collection
 
     /**
      * Retrieves the wrapped Collection instance.
+     *
+     * @return Doctrine\Common\Collections\Collection
      */
     public function unwrap()
     {
@@ -671,20 +744,19 @@ final class PersistentCollection implements Collection
      *
      * @param int $offset
      * @param int $length
+     *
      * @return array
      */
     public function slice($offset, $length = null)
     {
-        if ( ! $this->initialized &&
-             ! $this->isDirty &&
-               $this->association['fetch'] == Mapping\ClassMetadataInfo::FETCH_EXTRA_LAZY) {
+        if ( ! $this->initialized && ! $this->isDirty && $this->association['fetch'] === Mapping\ClassMetadataInfo::FETCH_EXTRA_LAZY) {
+            $persister = $this->em->getUnitOfWork()->getCollectionPersister($this->association);
 
-            return $this->em->getUnitOfWork()
-                            ->getCollectionPersister($this->association)
-                            ->slice($this, $offset, $length);
+            return $persister->slice($this, $offset, $length);
         }
 
         $this->initialize();
+
         return $this->coll->slice($offset, $length);
     }
 }

lib/Doctrine/ORM/Persisters/AbstractCollectionPersister.php

@@ -151,6 +151,16 @@ abstract class AbstractCollectionPersister
         throw new \BadMethodCallException("Checking for existance of a key is not supported by this CollectionPersister.");
     }
 
+    public function removeElement(PersistentCollection $coll, $element)
+    {
+        throw new \BadMethodCallException("Removing an element is not supported by this CollectionPersister.");
+    }
+
+    public function removeKey(PersistentCollection $coll, $key)
+    {
+        throw new \BadMethodCallException("Removing a key is not supported by this CollectionPersister.");
+    }
+
     public function get(PersistentCollection $coll, $index)
     {
         throw new \BadMethodCallException("Selecting a collection by index is not supported by this CollectionPersister.");

lib/Doctrine/ORM/Persisters/ManyToManyPersister.php

@@ -28,6 +28,7 @@ use Doctrine\ORM\PersistentCollection,
  * Persister for many-to-many collections.
  *
  * @author  Roman Borschel <roman@code-factory.org>
+ * @author  Guilherme Blanco <guilhermeblanco@hotmail.com>
  * @since   2.0
  */
 class ManyToManyPersister extends AbstractCollectionPersister
@@ -79,8 +80,10 @@ class ManyToManyPersister extends AbstractCollectionPersister
         $columns = $mapping['joinTableColumns'];
         $class   = $this->_em->getClassMetadata(get_class($coll->getOwner()));
         
-        return 'INSERT INTO ' . $class->getQuotedJoinTableName($mapping, $this->_conn->getDatabasePlatform()) 
-             . ' (' . implode(', ', $columns) . ') VALUES (' . implode(', ', array_fill(0, count($columns), '?')) . ')';
+        $joinTable = $class->getQuotedJoinTableName($mapping, $this->_conn->getDatabasePlatform());
+        
+        return 'INSERT INTO ' . $joinTable . ' (' . implode(', ', $columns) . ')'
+             . ' VALUES (' . implode(', ', array_fill(0, count($columns), '?')) . ')';
     }
 
     /**
@@ -118,27 +121,21 @@ class ManyToManyPersister extends AbstractCollectionPersister
         }
 
         foreach ($mapping['joinTableColumns'] as $joinTableColumn) {
-            if (isset($mapping['relationToSourceKeyColumns'][$joinTableColumn])) {
-                if ($isComposite) {
-                    if ($class1->containsForeignIdentifier) {
+            $isRelationToSource = isset($mapping['relationToSourceKeyColumns'][$joinTableColumn]);
+            
+            if ( ! $isComposite) {
+                $params[] = $isRelationToSource ? array_pop($identifier1) : array_pop($identifier2);
+                
+                continue;
+            }
+            
+            if ($isRelationToSource) {
                 $params[] = $identifier1[$class1->getFieldForColumn($mapping['relationToSourceKeyColumns'][$joinTableColumn])];
-                    } else {
-                        $params[] = $identifier1[$class1->fieldNames[$mapping['relationToSourceKeyColumns'][$joinTableColumn]]];
+                
+                continue;
             }
-                } else {
-                    $params[] = array_pop($identifier1);
-                }
-            } else {
-                if ($isComposite) {
-                    if ($class2->containsForeignIdentifier) {
+            
             $params[] = $identifier2[$class2->getFieldForColumn($mapping['relationToTargetKeyColumns'][$joinTableColumn])];
-                    } else {
-                        $params[] = $identifier2[$class2->fieldNames[$mapping['relationToTargetKeyColumns'][$joinTableColumn]]];
-                    }
-                } else {
-                    $params[] = array_pop($identifier2);
-                }
-            }
         }
 
         return $params;
@@ -151,19 +148,11 @@ class ManyToManyPersister extends AbstractCollectionPersister
      */
     protected function _getDeleteSQL(PersistentCollection $coll)
     {
-        $mapping     = $coll->getMapping();
         $class   = $this->_em->getClassMetadata(get_class($coll->getOwner()));
-        $joinTable = $mapping['joinTable'];
-        $whereClause = '';
-        
-        foreach ($mapping['relationToSourceKeyColumns'] as $relationColumn => $srcColumn) {
-            if ($whereClause !== '') $whereClause .= ' AND ';
-            
-            $whereClause .= $relationColumn . ' = ?';
-        }
+        $mapping = $coll->getMapping();
         
         return 'DELETE FROM ' . $class->getQuotedJoinTableName($mapping, $this->_conn->getDatabasePlatform()) 
-             . ' WHERE ' . $whereClause;
+             . ' WHERE ' . implode(' = ? AND ', array_keys($mapping['relationToSourceKeyColumns'])) . ' = ?';
     }
 
     /**
@@ -175,19 +164,23 @@ class ManyToManyPersister extends AbstractCollectionPersister
      */
     protected function _getDeleteSQLParameters(PersistentCollection $coll)
     {
-        $params = array();
-        $mapping = $coll->getMapping();
         $identifier = $this->_uow->getEntityIdentifier($coll->getOwner());
+        $mapping    = $coll->getMapping();
+        $params     = array();
         
-        if (count($mapping['relationToSourceKeyColumns']) > 1) {
+        // Optimization for single column identifier
+        if (count($mapping['relationToSourceKeyColumns']) === 1) {
+            $params[] = array_pop($identifier);
+            
+            return $params;
+        }
+        
+        // Composite identifier
         $sourceClass = $this->_em->getClassMetadata(get_class($mapping->getOwner()));
         
         foreach ($mapping['relationToSourceKeyColumns'] as $relColumn => $srcColumn) {
             $params[] = $identifier[$sourceClass->fieldNames[$srcColumn]];
         }
-        } else {
-           $params[] = array_pop($identifier);
-        }
         
         return $params;
     }
@@ -197,7 +190,6 @@ class ManyToManyPersister extends AbstractCollectionPersister
      */
     public function count(PersistentCollection $coll)
     {
-        $params  = array();
         $mapping = $coll->getMapping();
         $class   = $this->_em->getClassMetadata($mapping['sourceEntity']);
         $id      = $this->_em->getUnitOfWork()->getEntityIdentifier($coll->getOwner());
@@ -209,25 +201,24 @@ class ManyToManyPersister extends AbstractCollectionPersister
             $joinColumns = $mapping['relationToTargetKeyColumns'];
         }
 
-        $whereClause = '';
+        $whereClauses = array();
+        $params  = array();
         
         foreach ($mapping['joinTableColumns'] as $joinTableColumn) {
-            if (isset($joinColumns[$joinTableColumn])) {
-                if ($whereClause !== '') {
-                    $whereClause .= ' AND ';
+            if ( ! isset($joinColumns[$joinTableColumn])) {
+                continue;
             }
             
-                $whereClause .= "$joinTableColumn = ?";
+            $whereClauses[] = $joinTableColumn . ' = ?';
 
             $params[] = ($class->containsForeignIdentifier)
                 ? $id[$class->getFieldForColumn($joinColumns[$joinTableColumn])]
                 : $id[$class->fieldNames[$joinColumns[$joinTableColumn]]];
         }
-        }
         
         $sql = 'SELECT COUNT(*)'
              . ' FROM ' . $class->getQuotedJoinTableName($mapping, $this->_conn->getDatabasePlatform()) 
-             . ' WHERE ' . $whereClause;
+             . ' WHERE ' . implode(' AND ', $whereClauses);
 
         return $this->_conn->fetchColumn($sql, $params);
     }
@@ -248,6 +239,7 @@ class ManyToManyPersister extends AbstractCollectionPersister
     /**
      * @param PersistentCollection $coll
      * @param object $element
+     * @return boolean
      */
     public function contains(PersistentCollection $coll, $element)
     {
@@ -258,7 +250,42 @@ class ManyToManyPersister extends AbstractCollectionPersister
             return false;
         }
 
-        $params = array();
+        list($quotedJoinTable, $whereClauses, $params) = $this->getJoinTableRestrictions($coll, $element);
+        
+        $sql = 'SELECT 1 FROM ' . $quotedJoinTable . ' WHERE ' . implode(' AND ', $whereClauses);
+
+        return (bool) $this->_conn->fetchColumn($sql, $params);
+    }
+    
+    /**
+     * @param PersistentCollection $coll
+     * @param object $element
+     * @return boolean
+     */
+    public function removeElement(PersistentCollection $coll, $element)
+    {
+        $uow = $this->_em->getUnitOfWork();
+
+        // shortcut for new entities
+        if ($uow->getEntityState($element, UnitOfWork::STATE_NEW) == UnitOfWork::STATE_NEW) {
+            return false;
+        }
+
+        list($quotedJoinTable, $whereClauses, $params) = $this->getJoinTableRestrictions($coll, $element);
+
+        $sql = 'DELETE FROM ' . $quotedJoinTable . ' WHERE ' . implode(' AND ', $whereClauses);
+             
+        return (bool) $this->_conn->executeUpdate($sql, $params);
+    }
+    
+    /**
+     * @param Doctrine\ORM\PersistentCollection $coll
+     * @param object $element
+     * @return array
+     */
+    private function getJoinTableRestrictions(PersistentCollection $coll, $element)
+    {
+        $uow     = $this->_em->getUnitOfWork();
         $mapping = $coll->getMapping();
         
         if ( ! $mapping['isOwningSide']) {
@@ -275,36 +302,26 @@ class ManyToManyPersister extends AbstractCollectionPersister
             $targetId = $uow->getEntityIdentifier($element);
         }
         
-        $whereClause = '';
+        $quotedJoinTable = $sourceClass->getQuotedJoinTableName($mapping, $this->_conn->getDatabasePlatform());
+        $whereClauses    = array();
+        $params          = array();
         
         foreach ($mapping['joinTableColumns'] as $joinTableColumn) {
+            $whereClauses[] = $joinTableColumn . ' = ?';
+
             if (isset($mapping['relationToTargetKeyColumns'][$joinTableColumn])) {
-                if ($whereClause !== '') {
-                    $whereClause .= ' AND ';
-                }
-                
-                $whereClause .= $joinTableColumn . ' = ?';
-
                 $params[] = ($targetClass->containsForeignIdentifier)
                     ? $targetId[$targetClass->getFieldForColumn($mapping['relationToTargetKeyColumns'][$joinTableColumn])]
                     : $targetId[$targetClass->fieldNames[$mapping['relationToTargetKeyColumns'][$joinTableColumn]]];
-            } else if (isset($mapping['relationToSourceKeyColumns'][$joinTableColumn])) {
-                if ($whereClause !== '') {
-                    $whereClause .= ' AND ';
+                continue;
             }
             
-                $whereClause .= $joinTableColumn . ' = ?';
-
+            // relationToSourceKeyColumns
             $params[] = ($sourceClass->containsForeignIdentifier)
                 ? $sourceId[$sourceClass->getFieldForColumn($mapping['relationToSourceKeyColumns'][$joinTableColumn])]
                 : $sourceId[$sourceClass->fieldNames[$mapping['relationToSourceKeyColumns'][$joinTableColumn]]];
         }
-        }
         
-        $sql = 'SELECT 1'
-             . ' FROM ' . $sourceClass->getQuotedJoinTableName($mapping, $this->_conn->getDatabasePlatform()) 
-             . ' WHERE ' . $whereClause;
-
-        return (bool) $this->_conn->fetchColumn($sql, $params);
+        return array($quotedJoinTable, $whereClauses, $params);
     }
 }

lib/Doctrine/ORM/Persisters/OneToManyPersister.php

@@ -27,13 +27,9 @@ use Doctrine\ORM\PersistentCollection,
 /**
  * Persister for one-to-many collections.
  *
- * IMPORTANT:
- * This persister is only used for uni-directional one-to-many mappings on a foreign key
- * (which are not yet supported). So currently this persister is not used.
- *
- * @since 2.0
  * @author  Roman Borschel <roman@code-factory.org>
- * @todo Remove
+ * @author  Guilherme Blanco <guilhermeblanco@hotmail.com>
+ * @since   2.0
  */
 class OneToManyPersister extends AbstractCollectionPersister
 {
@@ -48,24 +44,19 @@ class OneToManyPersister extends AbstractCollectionPersister
     protected function _getDeleteRowSQL(PersistentCollection $coll)
     {
         $mapping = $coll->getMapping();
-        $targetClass = $this->_em->getClassMetadata($mapping->getTargetEntityName());
-        $table = $targetClass->getTableName();
+        $class   = $this->_em->getClassMetadata($mapping['targetEntity']);
         
-        $ownerMapping = $targetClass->getAssociationMapping($mapping['mappedBy']);
-
-        $setClause = '';
-        foreach ($ownerMapping->sourceToTargetKeyColumns as $sourceCol => $targetCol) {
-            if ($setClause != '') $setClause .= ', ';
-            $setClause .= "$sourceCol = NULL";
+        return 'DELETE FROM ' . $class->getQuotedTableName($this->_conn->getDatabasePlatform())
+             . ' WHERE ' . implode('= ? AND ', $class->getIdentifierColumnNames()) . ' = ?';
     }
 
-        $whereClause = '';
-        foreach ($targetClass->getIdentifierColumnNames() as $idColumn) {
-            if ($whereClause != '') $whereClause .= ' AND ';
-            $whereClause .= "$idColumn = ?";
-        }
-
-        return array("UPDATE $table SET $setClause WHERE $whereClause", $this->_uow->getEntityIdentifier($element));
+    /**
+     * {@inheritdoc}
+     * 
+     */
+    protected function _getDeleteRowSQLParameters(PersistentCollection $coll, $element)
+    {
+        return array_values($this->_uow->getEntityIdentifier($element));
     }
 
     protected function _getInsertRowSQL(PersistentCollection $coll)
@@ -73,6 +64,16 @@ class OneToManyPersister extends AbstractCollectionPersister
         return "UPDATE xxx SET foreign_key = yyy WHERE foreign_key = zzz";
     }
 
+    /**
+     * Gets the SQL parameters for the corresponding SQL statement to insert the given
+     * element of the given collection into the database.
+     *
+     * @param PersistentCollection $coll
+     * @param mixed $element
+     */
+    protected function _getInsertRowSQLParameters(PersistentCollection $coll, $element)
+    {}
+
     /* Not used for OneToManyPersister */
     protected function _getUpdateRowSQL(PersistentCollection $coll)
     {
@@ -98,26 +99,6 @@ class OneToManyPersister extends AbstractCollectionPersister
     protected function _getDeleteSQLParameters(PersistentCollection $coll)
     {}
 
-    /**
-     * Gets the SQL parameters for the corresponding SQL statement to insert the given
-     * element of the given collection into the database.
-     *
-     * @param PersistentCollection $coll
-     * @param mixed $element
-     */
-    protected function _getInsertRowSQLParameters(PersistentCollection $coll, $element)
-    {}
-
-    /**
-     * Gets the SQL parameters for the corresponding SQL statement to delete the given
-     * element from the given collection.
-     *
-     * @param PersistentCollection $coll
-     * @param mixed $element
-     */
-    protected function _getDeleteRowSQLParameters(PersistentCollection $coll, $element)
-    {}
-
     /**
      * {@inheritdoc}
      */
@@ -126,24 +107,23 @@ class OneToManyPersister extends AbstractCollectionPersister
         $mapping     = $coll->getMapping();
         $targetClass = $this->_em->getClassMetadata($mapping['targetEntity']);
         $sourceClass = $this->_em->getClassMetadata($mapping['sourceEntity']);
-
-        $params = array();
         $id          = $this->_em->getUnitOfWork()->getEntityIdentifier($coll->getOwner());
 
-        $where = '';
+        $whereClauses = array();
+        $params       = array();
+        
         foreach ($targetClass->associationMappings[$mapping['mappedBy']]['joinColumns'] AS $joinColumn) {
-            if ($where != '') {
-                $where .= ' AND ';
-            }
-            $where .= $joinColumn['name'] . " = ?";
-            if ($targetClass->containsForeignIdentifier) {
-                $params[] = $id[$sourceClass->getFieldForColumn($joinColumn['referencedColumnName'])];
-            } else {
-                $params[] = $id[$sourceClass->fieldNames[$joinColumn['referencedColumnName']]];
-            }
+            $whereClauses[] = $joinColumn['name'] . ' = ?';
+            
+            $params[] = ($targetClass->containsForeignIdentifier)
+                ? $id[$sourceClass->getFieldForColumn($joinColumn['referencedColumnName'])]
+                : $id[$sourceClass->fieldNames[$joinColumn['referencedColumnName']]];
         }
 
-        $sql = "SELECT count(*) FROM " . $targetClass->getQuotedTableName($this->_conn->getDatabasePlatform()) . " WHERE " . $where;
+        $sql = 'SELECT count(*)'
+             . ' FROM ' . $targetClass->getQuotedTableName($this->_conn->getDatabasePlatform()) 
+             . ' WHERE ' . implode(' AND ', $whereClauses);
+        
         return $this->_conn->fetchColumn($sql, $params);
     }
 
@@ -156,14 +136,16 @@ class OneToManyPersister extends AbstractCollectionPersister
     public function slice(PersistentCollection $coll, $offset, $length = null)
     {
         $mapping   = $coll->getMapping();
-        return $this->_em->getUnitOfWork()
-                  ->getEntityPersister($mapping['targetEntity'])
-                  ->getOneToManyCollection($mapping, $coll->getOwner(), $offset, $length);
+        $uow       = $this->_em->getUnitOfWork();
+        $persister = $uow->getEntityPersister($mapping['targetEntity']);
+        
+        return $persister->getOneToManyCollection($mapping, $coll->getOwner(), $offset, $length);
     }
 
     /**
      * @param PersistentCollection $coll
      * @param object $element
+     * @return boolean
      */
     public function contains(PersistentCollection $coll, $element)
     {
@@ -175,11 +157,35 @@ class OneToManyPersister extends AbstractCollectionPersister
             return false;
         }
 
-        // only works with single id identifier entities. Will throw an exception in Entity Persisters
-        // if that is not the case for the 'mappedBy' field.
+        $persister = $uow->getEntityPersister($mapping['targetEntity']);
+        
+        // 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()) );
 
-        return $uow->getEntityPersister($mapping['targetEntity'])
-                   ->exists($element, array($mapping['mappedBy'] => $id));
+        return $persister->exists($element, array($mapping['mappedBy'] => $id));
+    }
+    
+    /**
+     * @param PersistentCollection $coll
+     * @param object $element
+     * @return boolean
+     */
+    public function removeElement(PersistentCollection $coll, $element)
+    {
+        $uow = $this->_em->getUnitOfWork();
+
+        // shortcut for new entities
+        if ($uow->getEntityState($element, UnitOfWork::STATE_NEW) == UnitOfWork::STATE_NEW) {
+            return false;
+        }
+
+        $mapping = $coll->getMapping();
+        $class   = $this->_em->getClassMetadata($mapping['targetEntity']);
+        $sql     = 'DELETE FROM ' . $class->getQuotedTableName($this->_conn->getDatabasePlatform())
+                 . ' WHERE ' . implode('= ? AND ', $class->getIdentifierColumnNames()) . ' = ?';
+               
+        return (bool) $this->_conn->executeUpdate($sql, $this->_getDeleteRowSQLParameters($coll, $element));
     }
 }

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

@@ -29,7 +29,6 @@ class ExtraLazyCollectionTest extends \Doctrine\Tests\OrmFunctionalTestCase
         $class = $this->_em->getClassMetadata('Doctrine\Tests\Models\CMS\CmsGroup');
         $class->associationMappings['users']['fetch'] = ClassMetadataInfo::FETCH_EXTRA_LAZY;
 
-
         $this->loadFixture();
     }
 
@@ -257,8 +256,8 @@ class ExtraLazyCollectionTest extends \Doctrine\Tests\OrmFunctionalTestCase
         $this->assertFalse($user->groups->isInitialized(), "Pre-Condition: Collection is not initialized.");
 
         $group      = $this->_em->find('Doctrine\Tests\Models\CMS\CmsGroup', $this->groupId);
-
         $queryCount = $this->getCurrentQueryCount();
+
         $this->assertTrue($user->groups->contains($group));
         $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.");
@@ -267,6 +266,7 @@ class ExtraLazyCollectionTest extends \Doctrine\Tests\OrmFunctionalTestCase
         $group->name = "A New group!";
 
         $queryCount = $this->getCurrentQueryCount();
+
         $this->assertFalse($user->groups->contains($group));
         $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.");
@@ -275,6 +275,7 @@ class ExtraLazyCollectionTest extends \Doctrine\Tests\OrmFunctionalTestCase
         $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->assertFalse($user->groups->isInitialized(), "Post-Condition: Collection is not initialized.");
@@ -304,6 +305,107 @@ class ExtraLazyCollectionTest extends \Doctrine\Tests\OrmFunctionalTestCase
         $this->assertFalse($user->groups->isInitialized(), "Post-Condition: Collection is not initialized.");
     }
 
+    /**
+     *
+     */
+    public function testRemoveElementOneToMany()
+    {
+        $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);
+        $queryCount = $this->getCurrentQueryCount();
+
+        $user->articles->removeElement($article);
+
+        $this->assertFalse($user->articles->isInitialized(), "Post-Condition: Collection is not initialized.");
+        $this->assertEquals($queryCount + 1, $this->getCurrentQueryCount());
+
+        $article = new \Doctrine\Tests\Models\CMS\CmsArticle();
+        $article->topic = "Testnew";
+        $article->text = "blub";
+
+        $queryCount = $this->getCurrentQueryCount();
+
+        $user->articles->removeElement($article);
+
+        $this->assertEquals($queryCount, $this->getCurrentQueryCount(), "Removing a new entity should cause no query to be executed.");
+
+        $this->_em->persist($article);
+        $this->_em->flush();
+
+        $queryCount = $this->getCurrentQueryCount();
+
+        $user->articles->removeElement($article);
+
+        $this->assertEquals($queryCount + 1, $this->getCurrentQueryCount(), "Removing a managed entity should cause one query to be executed.");
+        $this->assertFalse($user->articles->isInitialized(), "Post-Condition: Collection is not initialized.");
+    }
+
+    /**
+     *
+     */
+    public function testRemoveElementManyToMany()
+    {
+        $user = $this->_em->find('Doctrine\Tests\Models\CMS\CmsUser', $this->userId);
+        $this->assertFalse($user->groups->isInitialized(), "Pre-Condition: Collection is not initialized.");
+
+        $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->assertFalse($user->groups->isInitialized(), "Post-Condition: Collection is not initialized.");
+
+        $group = new \Doctrine\Tests\Models\CMS\CmsGroup();
+        $group->name = "A New group!";
+
+        $queryCount = $this->getCurrentQueryCount();
+
+        $user->groups->removeElement($group);
+
+        $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.");
+
+        $this->_em->persist($group);
+        $this->_em->flush();
+
+        $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->assertFalse($user->groups->isInitialized(), "Post-Condition: Collection is not initialized.");
+    }
+
+    /**
+     *
+     */
+    public function testRemoveElementManyToManyInverse()
+    {
+        $group = $this->_em->find('Doctrine\Tests\Models\CMS\CmsGroup', $this->groupId);
+        $this->assertFalse($group->users->isInitialized(), "Pre-Condition: Collection is not initialized.");
+
+        $user       = $this->_em->find('Doctrine\Tests\Models\CMS\CmsUser', $this->userId);
+        $queryCount = $this->getCurrentQueryCount();
+
+        $group->users->removeElement($user);
+
+        $this->assertEquals($queryCount + 1, $this->getCurrentQueryCount(), "Removing a managed entity should cause one query to be executed.");
+        $this->assertFalse($user->groups->isInitialized(), "Post-Condition: Collection is not initialized.");
+
+        $newUser = new \Doctrine\Tests\Models\CMS\CmsUser();
+        $newUser->name = "A New group!";
+
+        $queryCount = $this->getCurrentQueryCount();
+
+        $group->users->removeElement($newUser);
+
+        $this->assertEquals($queryCount, $this->getCurrentQueryCount(), "Removing a new entity should cause no query to be executed.");
+        $this->assertFalse($user->groups->isInitialized(), "Post-Condition: Collection is not initialized.");
+    }
+
     /**
      * @group DDC-1399
      */