Renamed coll to collection and some small updates to tests.

lib/Doctrine/ORM/Persisters/AbstractCollectionPersister.php

@@ -20,6 +20,7 @@
 namespace Doctrine\ORM\Persisters;
 
 use Doctrine\ORM\EntityManager;
+use Doctrine\ORM\UnitOfWork;
 
 /**
  * Base class for all collection persisters.
@@ -71,4 +72,28 @@ abstract class AbstractCollectionPersister implements CollectionPersister
         $this->platform         = $this->conn->getDatabasePlatform();
         $this->quoteStrategy    = $em->getConfiguration()->getQuoteStrategy();
     }
+
+    /**
+     * Check if entity is in a valid state for operations.
+     *
+     * @param object $entity
+     *
+     * @return bool
+     */
+    protected function isValidEntityState($entity)
+    {
+        $entityState = $this->uow->getEntityState($entity, 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 && $this->uow->isScheduledForInsert($entity)) {
+            return false;
+        }
+
+        return true;
+    }
 }

lib/Doctrine/ORM/Persisters/ManyToManyPersister.php

@@ -23,7 +23,6 @@ use Doctrine\Common\Collections\Criteria;
 use Doctrine\ORM\Mapping\ClassMetadata;
 use Doctrine\ORM\PersistentCollection;
 use Doctrine\ORM\Query;
-use Doctrine\ORM\UnitOfWork;
 
 /**
  * Persister for many-to-many collections.
@@ -38,95 +37,109 @@ class ManyToManyPersister extends AbstractCollectionPersister
     /**
      * {@inheritdoc}
      */
-    public function delete(PersistentCollection $coll)
+    public function delete(PersistentCollection $collection)
     {
-        $mapping = $coll->getMapping();
+        $mapping = $collection->getMapping();
 
         if ( ! $mapping['isOwningSide']) {
             return; // ignore inverse side
         }
 
-        $this->conn->executeUpdate($this->getDeleteSQL($coll), $this->getDeleteSQLParameters($coll));
+        $this->conn->executeUpdate($this->getDeleteSQL($collection), $this->getDeleteSQLParameters($collection));
     }
 
     /**
      * {@inheritdoc}
      */
-    public function update(PersistentCollection $coll)
+    public function update(PersistentCollection $collection)
     {
-        $mapping = $coll->getMapping();
+        $mapping = $collection->getMapping();
 
         if ( ! $mapping['isOwningSide']) {
             return; // ignore inverse side
         }
 
-        $insertSql = $this->getInsertRowSQL($coll);
+        $insertSql = $this->getInsertRowSQL($collection);
-        $deleteSql = $this->getDeleteRowSQL($coll);
+        $deleteSql = $this->getDeleteRowSQL($collection);
 
-        foreach ($coll->getDeleteDiff() as $element) {
+        foreach ($collection->getDeleteDiff() as $element) {
-            $this->conn->executeUpdate($deleteSql, $this->getDeleteRowSQLParameters($coll, $element));
+            $this->conn->executeUpdate($deleteSql, $this->getDeleteRowSQLParameters($collection, $element));
         }
 
-        foreach ($coll->getInsertDiff() as $element) {
+        foreach ($collection->getInsertDiff() as $element) {
-            $this->conn->executeUpdate($insertSql, $this->getInsertRowSQLParameters($coll, $element));
+            $this->conn->executeUpdate($insertSql, $this->getInsertRowSQLParameters($collection, $element));
         }
     }
 
     /**
      * {@inheritdoc}
      */
-    public function get(PersistentCollection $coll, $index)
+    public function get(PersistentCollection $collection, $index)
     {
-        $mapping = $coll->getMapping();
+        $mapping = $collection->getMapping();
 
         if ( ! isset($mapping['indexBy'])) {
             throw new \BadMethodCallException("Selecting a collection by index is only supported on indexed collections.");
         }
 
         $persister = $this->uow->getEntityPersister($mapping['targetEntity']);
+        $mappedKey = $mapping['isOwningSide']
+            ? $mapping['inversedBy']
+            : $mapping['mappedBy'];
 
-        if ( ! $mapping['isOwningSide']) {
+        return $persister->load(array($mappedKey => $collection->getOwner(), $mapping['indexBy'] => $index), null, $mapping, array(), 0, 1);
-            return $persister->load(array($mapping['mappedBy'] => $coll->getOwner(), $mapping['indexBy'] => $index), null, $mapping, array(), 0, 1);
-        }
-
-        return $persister->load(array($mapping['inversedBy'] => $coll->getOwner(), $mapping['indexBy'] => $index), null, $mapping, array(), 0, 1);
     }
 
     /**
      * {@inheritdoc}
      */
-    public function count(PersistentCollection $coll)
+    public function count(PersistentCollection $collection)
     {
         $conditions     = array();
         $params         = array();
-        $mapping        = $coll->getMapping();
+        $mapping        = $collection->getMapping();
-        $association    = $mapping;
+        $id             = $this->uow->getEntityIdentifier($collection->getOwner());
-        $class          = $this->em->getClassMetadata($mapping['sourceEntity']);
+        $sourceClass    = $this->em->getClassMetadata($mapping['sourceEntity']);
-        $id             = $this->uow->getEntityIdentifier($coll->getOwner());
+        $targetClass    = $this->em->getClassMetadata($mapping['targetEntity']);
+        $association    = ( ! $mapping['isOwningSide'])
+            ? $targetClass->associationMappings[$mapping['mappedBy']]
+            : $mapping;
 
-        if ( ! $mapping['isOwningSide']) {
+        $joinTableName  = $this->quoteStrategy->getJoinTableName($association, $sourceClass, $this->platform);
-            $targetEntity   = $this->em->getClassMetadata($mapping['targetEntity']);
+        $joinColumns    = ( ! $mapping['isOwningSide'])
-            $association    = $targetEntity->associationMappings[$mapping['mappedBy']];
-        }
-
-        $joinColumns = ( ! $mapping['isOwningSide'])
             ? $association['joinTable']['inverseJoinColumns']
             : $association['joinTable']['joinColumns'];
 
         foreach ($joinColumns as $joinColumn) {
-            $columnName     = $this->quoteStrategy->getJoinColumnName($joinColumn, $class, $this->platform);
+            $columnName     = $this->quoteStrategy->getJoinColumnName($joinColumn, $sourceClass, $this->platform);
             $referencedName = $joinColumn['referencedColumnName'];
             $conditions[]   = 't.' . $columnName . ' = ?';
-            $params[]       = $id[$class->getFieldForColumn($referencedName)];
+            $params[]       = $id[$sourceClass->getFieldForColumn($referencedName)];
         }
 
-        $joinTableName = $this->quoteStrategy->getJoinTableName($association, $class, $this->platform);
         list($joinTargetEntitySQL, $filterSql) = $this->getFilterSql($mapping);
 
         if ($filterSql) {
             $conditions[] = $filterSql;
         }
 
+        // If there is a provided criteria, make part of conditions
+        // @todo Fix this. Current SQL returns something like:
+        //
+        /*if ($criteria && ($expression = $criteria->getWhereExpression()) !== null) {
+            // A join is needed on the target entity
+            $targetTableName = $this->quoteStrategy->getTableName($targetClass, $this->platform);
+            $targetJoinSql   = ' JOIN ' . $targetTableName . ' te'
+                . ' ON' . implode(' AND ', $this->getOnConditionSQL($association));
+
+            // And criteria conditions needs to be added
+            $persister    = $this->uow->getEntityPersister($targetClass->name);
+            $visitor      = new SqlExpressionVisitor($persister, $targetClass);
+            $conditions[] = $visitor->dispatch($expression);
+
+            $joinTargetEntitySQL = $targetJoinSql . $joinTargetEntitySQL;
+        }*/
+
         $sql = 'SELECT COUNT(*)'
             . ' FROM ' . $joinTableName . ' t'
             . $joinTargetEntitySQL
@@ -138,25 +151,25 @@ class ManyToManyPersister extends AbstractCollectionPersister
     /**
      * {@inheritDoc}
      */
-    public function slice(PersistentCollection $coll, $offset, $length = null)
+    public function slice(PersistentCollection $collection, $offset, $length = null)
     {
-        $mapping   = $coll->getMapping();
+        $mapping   = $collection->getMapping();
         $persister = $this->uow->getEntityPersister($mapping['targetEntity']);
 
-        return $persister->getManyToManyCollection($mapping, $coll->getOwner(), $offset, $length);
+        return $persister->getManyToManyCollection($mapping, $collection->getOwner(), $offset, $length);
     }
     /**
      * {@inheritdoc}
      */
-    public function containsKey(PersistentCollection $coll, $key)
+    public function containsKey(PersistentCollection $collection, $key)
     {
-        $mapping = $coll->getMapping();
+        $mapping = $collection->getMapping();
 
         if ( ! isset($mapping['indexBy'])) {
             throw new \BadMethodCallException("Selecting a collection by index is only supported on indexed collections.");
         }
 
-        list($quotedJoinTable, $whereClauses, $params) = $this->getJoinTableRestrictionsWithKey($coll, $key, true);
+        list($quotedJoinTable, $whereClauses, $params) = $this->getJoinTableRestrictionsWithKey($collection, $key, true);
 
         $sql = 'SELECT 1 FROM ' . $quotedJoinTable . ' WHERE ' . implode(' AND ', $whereClauses);
 
@@ -166,20 +179,13 @@ class ManyToManyPersister extends AbstractCollectionPersister
     /**
      * {@inheritDoc}
      */
-    public function contains(PersistentCollection $coll, $element)
+    public function contains(PersistentCollection $collection, $element)
     {
-        $entityState = $this->uow->getEntityState($element, UnitOfWork::STATE_NEW);
+        if ( ! $this->isValidEntityState($element)) {
-
-        if ($entityState === UnitOfWork::STATE_NEW) {
             return false;
         }
 
-        // Entity is scheduled for inclusion
+        list($quotedJoinTable, $whereClauses, $params) = $this->getJoinTableRestrictions($collection, $element, true);
-        if ($entityState === UnitOfWork::STATE_MANAGED && $this->uow->isScheduledForInsert($element)) {
-            return false;
-        }
-
-        list($quotedJoinTable, $whereClauses, $params) = $this->getJoinTableRestrictions($coll, $element, true);
 
         $sql = 'SELECT 1 FROM ' . $quotedJoinTable . ' WHERE ' . implode(' AND ', $whereClauses);
 
@@ -189,21 +195,13 @@ class ManyToManyPersister extends AbstractCollectionPersister
     /**
      * {@inheritDoc}
      */
-    public function removeElement(PersistentCollection $coll, $element)
+    public function removeElement(PersistentCollection $collection, $element)
     {
-        $entityState = $this->uow->getEntityState($element, UnitOfWork::STATE_NEW);
+        if ( ! $this->isValidEntityState($element)) {
-
-        if ($entityState === UnitOfWork::STATE_NEW) {
             return false;
         }
 
-        // If Entity is scheduled for inclusion, it is not in this collection.
+        list($quotedJoinTable, $whereClauses, $params) = $this->getJoinTableRestrictions($collection, $element, false);
-        // We can assure that because it would have return true before on array check
-        if ($entityState === UnitOfWork::STATE_MANAGED && $this->uow->isScheduledForInsert($element)) {
-            return false;
-        }
-
-        list($quotedJoinTable, $whereClauses, $params) = $this->getJoinTableRestrictions($coll, $element, false);
 
         $sql = 'DELETE FROM ' . $quotedJoinTable . ' WHERE ' . implode(' AND ', $whereClauses);
 
@@ -213,10 +211,10 @@ class ManyToManyPersister extends AbstractCollectionPersister
     /**
      * {@inheritDoc}
      */
-    public function loadCriteria(PersistentCollection $coll, Criteria $criteria)
+    public function loadCriteria(PersistentCollection $collection, Criteria $criteria)
     {
-        $mapping       = $coll->getMapping();
+        $mapping       = $collection->getMapping();
-        $owner         = $coll->getOwner();
+        $owner         = $collection->getOwner();
         $ownerMetadata = $this->em->getClassMetadata(get_class($owner));
         $whereClauses  = $params = array();
 
@@ -233,7 +231,7 @@ class ManyToManyPersister extends AbstractCollectionPersister
             $params[]           = $value;
         }
 
-        $mapping      = $coll->getMapping();
+        $mapping      = $collection->getMapping();
         $targetClass  = $this->em->getClassMetadata($mapping['targetEntity']);
         $tableName    = $this->quoteStrategy->getTableName($targetClass, $this->platform);
         $joinTable    = $this->quoteStrategy->getJoinTableName($mapping, $ownerMetadata, $this->platform);
@@ -321,16 +319,10 @@ class ManyToManyPersister extends AbstractCollectionPersister
      */
     protected function getOnConditionSQL($mapping)
     {
-        $association = $mapping;
-
-        if ( ! $mapping['isOwningSide']) {
-            $association = $this
-                ->em
-                ->getClassMetadata($mapping['targetEntity'])
-                ->associationMappings[$mapping['mappedBy']];
-        }
-
         $targetClass = $this->em->getClassMetadata($mapping['targetEntity']);
+        $association = ( ! $mapping['isOwningSide'])
+            ? $targetClass->associationMappings[$mapping['mappedBy']]
+            : $mapping;
 
         $joinColumns = $mapping['isOwningSide']
             ? $association['joinTable']['inverseJoinColumns']
@@ -353,11 +345,11 @@ class ManyToManyPersister extends AbstractCollectionPersister
      *
      * @override
      */
-    protected function getDeleteSQL(PersistentCollection $coll)
+    protected function getDeleteSQL(PersistentCollection $collection)
     {
         $columns    = array();
-        $mapping    = $coll->getMapping();
+        $mapping    = $collection->getMapping();
-        $class      = $this->em->getClassMetadata(get_class($coll->getOwner()));
+        $class      = $this->em->getClassMetadata(get_class($collection->getOwner()));
         $joinTable  = $this->quoteStrategy->getJoinTableName($mapping, $class, $this->platform);
 
         foreach ($mapping['joinTable']['joinColumns'] as $joinColumn) {
@@ -375,10 +367,10 @@ class ManyToManyPersister extends AbstractCollectionPersister
      *
      * @internal Order of the parameters must be the same as the order of the columns in getDeleteSql.
      */
-    protected function getDeleteSQLParameters(PersistentCollection $coll)
+    protected function getDeleteSQLParameters(PersistentCollection $collection)
     {
-        $mapping    = $coll->getMapping();
+        $mapping    = $collection->getMapping();
-        $identifier = $this->uow->getEntityIdentifier($coll->getOwner());
+        $identifier = $this->uow->getEntityIdentifier($collection->getOwner());
 
         // Optimization for single column identifier
         if (count($mapping['relationToSourceKeyColumns']) === 1) {
@@ -401,13 +393,13 @@ class ManyToManyPersister extends AbstractCollectionPersister
     /**
      * Gets the SQL statement used for deleting a row from the collection.
      *
-     * @param \Doctrine\ORM\PersistentCollection $coll
+     * @param \Doctrine\ORM\PersistentCollection $collection
      *
      * @return string
      */
-    protected function getDeleteRowSQL(PersistentCollection $coll)
+    protected function getDeleteRowSQL(PersistentCollection $collection)
     {
-        $mapping = $coll->getMapping();
+        $mapping = $collection->getMapping();
         $class   = $this->em->getClassMetadata($mapping['sourceEntity']);
         $columns = array();
 
@@ -429,28 +421,28 @@ class ManyToManyPersister extends AbstractCollectionPersister
      *
      * @internal Order of the parameters must be the same as the order of the columns in getDeleteRowSql.
      *
-     * @param \Doctrine\ORM\PersistentCollection $coll
+     * @param \Doctrine\ORM\PersistentCollection $collection
      * @param mixed                              $element
      *
      * @return array
      */
-    protected function getDeleteRowSQLParameters(PersistentCollection $coll, $element)
+    protected function getDeleteRowSQLParameters(PersistentCollection $collection, $element)
     {
-        return $this->collectJoinTableColumnParameters($coll, $element);
+        return $this->collectJoinTableColumnParameters($collection, $element);
     }
 
     /**
      * Gets the SQL statement used for inserting a row in the collection.
      *
-     * @param \Doctrine\ORM\PersistentCollection $coll
+     * @param \Doctrine\ORM\PersistentCollection $collection
      *
      * @return string
      */
-    protected function getInsertRowSQL(PersistentCollection $coll)
+    protected function getInsertRowSQL(PersistentCollection $collection)
     {
         $columns = array();
-        $mapping = $coll->getMapping();
+        $mapping = $collection->getMapping();
-        $class   = $this->em->getClassMetadata(get_class($coll->getOwner()));
+        $class   = $this->em->getClassMetadata($mapping['sourceEntity']);
 
         foreach ($mapping['joinTable']['joinColumns'] as $joinColumn) {
             $columns[] = $this->quoteStrategy->getJoinColumnName($joinColumn, $class, $this->platform);
@@ -472,37 +464,37 @@ class ManyToManyPersister extends AbstractCollectionPersister
      *
      * @internal Order of the parameters must be the same as the order of the columns in getInsertRowSql.
      *
-     * @param \Doctrine\ORM\PersistentCollection $coll
+     * @param \Doctrine\ORM\PersistentCollection $collection
      * @param mixed                              $element
      *
      * @return array
      */
-    protected function getInsertRowSQLParameters(PersistentCollection $coll, $element)
+    protected function getInsertRowSQLParameters(PersistentCollection $collection, $element)
     {
-        return $this->collectJoinTableColumnParameters($coll, $element);
+        return $this->collectJoinTableColumnParameters($collection, $element);
     }
 
     /**
      * Collects the parameters for inserting/deleting on the join table in the order
      * of the join table columns as specified in ManyToManyMapping#joinTableColumns.
      *
-     * @param \Doctrine\ORM\PersistentCollection $coll
+     * @param \Doctrine\ORM\PersistentCollection $collection
      * @param object                             $element
      *
      * @return array
      */
-    private function collectJoinTableColumnParameters(PersistentCollection $coll, $element)
+    private function collectJoinTableColumnParameters(PersistentCollection $collection, $element)
     {
         $params      = array();
-        $mapping     = $coll->getMapping();
+        $mapping     = $collection->getMapping();
         $isComposite = count($mapping['joinTableColumns']) > 2;
 
-        $identifier1 = $this->uow->getEntityIdentifier($coll->getOwner());
+        $identifier1 = $this->uow->getEntityIdentifier($collection->getOwner());
         $identifier2 = $this->uow->getEntityIdentifier($element);
 
         if ($isComposite) {
-            $class1 = $this->em->getClassMetadata(get_class($coll->getOwner()));
+            $class1 = $this->em->getClassMetadata(get_class($collection->getOwner()));
-            $class2 = $coll->getTypeClass();
+            $class2 = $collection->getTypeClass();
         }
 
         foreach ($mapping['joinTableColumns'] as $joinTableColumn) {
@@ -527,18 +519,18 @@ class ManyToManyPersister extends AbstractCollectionPersister
     }
 
     /**
-     * @param \Doctrine\ORM\PersistentCollection $coll
+     * @param \Doctrine\ORM\PersistentCollection $collection
      * @param string                             $key
      * @param boolean                            $addFilters Whether the filter SQL should be included or not.
      *
      * @return array
      */
-    private function getJoinTableRestrictionsWithKey(PersistentCollection $coll, $key, $addFilters)
+    private function getJoinTableRestrictionsWithKey(PersistentCollection $collection, $key, $addFilters)
     {
-        $filterMapping  = $coll->getMapping();
+        $filterMapping  = $collection->getMapping();
         $mapping        = $filterMapping;
         $indexBy        = $mapping['indexBy'];
-        $id             = $this->uow->getEntityIdentifier($coll->getOwner());
+        $id             = $this->uow->getEntityIdentifier($collection->getOwner());
 
         $targetEntity   = $this->em->getClassMetadata($mapping['targetEntity']);
 
@@ -596,28 +588,28 @@ class ManyToManyPersister extends AbstractCollectionPersister
     }
 
     /**
-     * @param \Doctrine\ORM\PersistentCollection $coll
+     * @param \Doctrine\ORM\PersistentCollection $collection
      * @param object                             $element
      * @param boolean                            $addFilters Whether the filter SQL should be included or not.
      *
      * @return array
      */
-    private function getJoinTableRestrictions(PersistentCollection $coll, $element, $addFilters)
+    private function getJoinTableRestrictions(PersistentCollection $collection, $element, $addFilters)
     {
-        $filterMapping  = $coll->getMapping();
+        $filterMapping  = $collection->getMapping();
         $mapping        = $filterMapping;
 
         if ( ! $mapping['isOwningSide']) {
             $sourceClass = $this->em->getClassMetadata($mapping['targetEntity']);
             $targetClass = $this->em->getClassMetadata($mapping['sourceEntity']);
             $sourceId = $this->uow->getEntityIdentifier($element);
-            $targetId = $this->uow->getEntityIdentifier($coll->getOwner());
+            $targetId = $this->uow->getEntityIdentifier($collection->getOwner());
 
             $mapping = $sourceClass->associationMappings[$mapping['mappedBy']];
         } else {
             $sourceClass = $this->em->getClassMetadata($mapping['sourceEntity']);
             $targetClass = $this->em->getClassMetadata($mapping['targetEntity']);
-            $sourceId = $this->uow->getEntityIdentifier($coll->getOwner());
+            $sourceId = $this->uow->getEntityIdentifier($collection->getOwner());
             $targetId = $this->uow->getEntityIdentifier($element);
         }
 

lib/Doctrine/ORM/Persisters/OneToManyPersister.php

@@ -36,7 +36,7 @@ class OneToManyPersister extends AbstractCollectionPersister
     /**
      * {@inheritdoc}
      */
-    public function delete(PersistentCollection $coll)
+    public function delete(PersistentCollection $collection)
     {
         // This can never happen. One to many can only be inverse side.
         // For owning side one to many, it is required to have a join table,
@@ -47,7 +47,7 @@ class OneToManyPersister extends AbstractCollectionPersister
     /**
      * {@inheritdoc}
      */
-    public function update(PersistentCollection $coll)
+    public function update(PersistentCollection $collection)
     {
         // This can never happen. One to many can only be inverse side.
         // For owning side one to many, it is required to have a join table,
@@ -58,9 +58,9 @@ class OneToManyPersister extends AbstractCollectionPersister
     /**
      * {@inheritdoc}
      */
-    public function get(PersistentCollection $coll, $index)
+    public function get(PersistentCollection $collection, $index)
     {
-        $mapping = $coll->getMapping();
+        $mapping = $collection->getMapping();
 
         if ( ! isset($mapping['indexBy'])) {
             throw new \BadMethodCallException("Selecting a collection by index is only supported on indexed collections.");
@@ -68,21 +68,21 @@ class OneToManyPersister extends AbstractCollectionPersister
 
         $persister = $this->uow->getEntityPersister($mapping['targetEntity']);
 
-        return $persister->load(array($mapping['mappedBy'] => $coll->getOwner(), $mapping['indexBy'] => $index), null, $mapping, array(), null, 1);
+        return $persister->load(array($mapping['mappedBy'] => $collection->getOwner(), $mapping['indexBy'] => $index), null, $mapping, array(), null, 1);
     }
 
     /**
      * {@inheritdoc}
      */
-    public function count(PersistentCollection $coll)
+    public function count(PersistentCollection $collection)
     {
-        $mapping   = $coll->getMapping();
+        $mapping   = $collection->getMapping();
         $persister = $this->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.
-        $criteria = new Criteria(Criteria::expr()->eq($mapping['mappedBy'], $coll->getOwner()));
+        $criteria = new Criteria(Criteria::expr()->eq($mapping['mappedBy'], $collection->getOwner()));
 
         return $persister->count($criteria);
     }
@@ -90,20 +90,20 @@ class OneToManyPersister extends AbstractCollectionPersister
     /**
      * {@inheritdoc}
      */
-    public function slice(PersistentCollection $coll, $offset, $length = null)
+    public function slice(PersistentCollection $collection, $offset, $length = null)
     {
-        $mapping   = $coll->getMapping();
+        $mapping   = $collection->getMapping();
         $persister = $this->uow->getEntityPersister($mapping['targetEntity']);
 
-        return $persister->getOneToManyCollection($mapping, $coll->getOwner(), $offset, $length);
+        return $persister->getOneToManyCollection($mapping, $collection->getOwner(), $offset, $length);
     }
 
     /**
      * {@inheritdoc}
      */
-    public function containsKey(PersistentCollection $coll, $key)
+    public function containsKey(PersistentCollection $collection, $key)
     {
-        $mapping = $coll->getMapping();
+        $mapping = $collection->getMapping();
 
         if ( ! isset($mapping['indexBy'])) {
             throw new \BadMethodCallException("Selecting a collection by index is only supported on indexed collections.");
@@ -116,7 +116,7 @@ class OneToManyPersister extends AbstractCollectionPersister
         // 'mappedBy' field.
         $criteria = new Criteria();
 
-        $criteria->andWhere(Criteria::expr()->eq($mapping['mappedBy'], $coll->getOwner()));
+        $criteria->andWhere(Criteria::expr()->eq($mapping['mappedBy'], $collection->getOwner()));
         $criteria->andWhere(Criteria::expr()->eq($mapping['indexBy'], $key));
 
         return (bool) $persister->count($criteria);
@@ -125,19 +125,19 @@ class OneToManyPersister extends AbstractCollectionPersister
      /**
      * {@inheritdoc}
      */
-    public function contains(PersistentCollection $coll, $element)
+    public function contains(PersistentCollection $collection, $element)
     {
         if ( ! $this->isValidEntityState($element)) {
             return false;
         }
 
-        $mapping   = $coll->getMapping();
+        $mapping   = $collection->getMapping();
         $persister = $this->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.
-        $criteria = new Criteria(Criteria::expr()->eq($mapping['mappedBy'], $coll->getOwner()));
+        $criteria = new Criteria(Criteria::expr()->eq($mapping['mappedBy'], $collection->getOwner()));
 
         return $persister->exists($element, $criteria);
     }
@@ -145,13 +145,13 @@ class OneToManyPersister extends AbstractCollectionPersister
     /**
      * {@inheritdoc}
      */
-    public function removeElement(PersistentCollection $coll, $element)
+    public function removeElement(PersistentCollection $collection, $element)
     {
         if ( ! $this->isValidEntityState($element)) {
             return false;
         }
 
-        $mapping   = $coll->getMapping();
+        $mapping   = $collection->getMapping();
         $persister = $this->uow->getEntityPersister($mapping['targetEntity']);
 
         return $persister->delete($element);
@@ -164,28 +164,4 @@ class OneToManyPersister extends AbstractCollectionPersister
     {
         throw new \BadMethodCallException("Filtering a collection by Criteria is not supported by this CollectionPersister.");
     }
-
-    /**
-     * Check if entity is in a valid state for operations.
-     *
-     * @param object $entity
-     *
-     * @return bool
-     */
-    private function isValidEntityState($entity)
-    {
-        $entityState = $this->uow->getEntityState($entity, 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 && $this->uow->isScheduledForInsert($entity)) {
-            return false;
-        }
-
-        return true;
-    }
 }

tests/Doctrine/Tests/Models/Quote/Group.php

@@ -39,4 +39,4 @@ class Group
         $this->name     = $name;
         $this->parent   = $parent;
     }
-}
+}

tests/Doctrine/Tests/Models/Quote/SimpleEntity.php

@@ -1,32 +0,0 @@
-<?php
-
-namespace Doctrine\Tests\Models\Quote;
-
-/**
- * @Entity
- * @Table(name="`ddc-1719-simple-entity`")
- */
-class SimpleEntity
-{
-
-    /**
-     * @Id
-     * @Column(type="integer", name="`simple-entity-id`")
-     * @GeneratedValue(strategy="AUTO")
-     */
-    public $id;
-
-    /**
-     * @Column(type="string", name="`simple-entity-value`")
-     */
-    public $value;
-
-    /**
-     * @param string $value
-     */
-    public function __construct($value)
-    {
-        $this->value = $value;
-    }
-
-}

tests/Doctrine/Tests/Models/Quote/User.php

@@ -34,7 +34,7 @@ class User
     public $address;
 
     /**
-     * @ManyToMany(targetEntity="Group", inversedBy="users", cascade={"all"})
+     * @ManyToMany(targetEntity="Group", inversedBy="users", cascade={"all"}, fetch="EXTRA_LAZY")
      * @JoinTable(name="`quote-users-groups`",
      *      joinColumns={
      *          @JoinColumn(
@@ -52,25 +52,6 @@ class User
      */
     public $groups;
 
-    /**
-     * @ManyToMany(targetEntity="Group", inversedBy="users", cascade={"all"}, fetch="EXTRA_LAZY")
-     * @JoinTable(name="`quote-extra-lazy-users-groups`",
-     *      joinColumns={
-     *          @JoinColumn(
-     *              name="`user-id`",
-     *              referencedColumnName="`user-id`"
-     *          )
-     *      },
-     *      inverseJoinColumns={
-     *          @JoinColumn(
-     *              name="`group-id`",
-     *              referencedColumnName="`group-id`"
-     *          )
-     *      }
-     * )
-     */
-    public $extraLazyGroups;
-
     public function __construct()
     {
         $this->phones = new ArrayCollection;

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

@@ -19,8 +19,10 @@
 namespace Doctrine\Tests\ORM\Functional;
 
 use Doctrine\Common\Collections\Criteria;
+use Doctrine\Tests\Models\Quote\Group;
+use Doctrine\Tests\Models\Quote\User as QuoteUser;
 use Doctrine\Tests\Models\Tweet\Tweet;
-use Doctrine\Tests\Models\Tweet\User;
+use Doctrine\Tests\Models\Tweet\User as TweetUser;
 
 /**
  * @author Michaël Gallego <mic.gallego@gmail.com>
@@ -30,6 +32,7 @@ class PersistentCollectionCriteriaTest extends \Doctrine\Tests\OrmFunctionalTest
     protected function setUp()
     {
         $this->useModelSet('tweet');
+        $this->useModelSet('quote');
         parent::setUp();
     }
 
@@ -41,9 +44,9 @@ class PersistentCollectionCriteriaTest extends \Doctrine\Tests\OrmFunctionalTest
         parent::tearDown();
     }
 
-    public function loadFixture()
+    public function loadTweetFixture()
     {
-        $author = new User();
+        $author = new TweetUser();
         $author->name = 'ngal';
         $this->_em->persist($author);
 
@@ -64,9 +67,27 @@ class PersistentCollectionCriteriaTest extends \Doctrine\Tests\OrmFunctionalTest
         $this->_em->clear();
     }
 
+    public function loadQuoteFixture()
+    {
+        $user = new QuoteUser();
+        $user->name = 'mgal';
+        $this->_em->persist($user);
+
+        $quote1 = new Group('quote1');
+        $user->groups->add($quote1);
+
+        $quote2 = new Group('quote2');
+        $user->groups->add($quote2);
+
+        $this->_em->flush();
+
+        $this->_em->clear();
+    }
+
     public function testCanCountWithoutLoadingPersistentCollection()
     {
-        $this->loadFixture();
+        $this->loadTweetFixture();
+
         $repository = $this->_em->getRepository('Doctrine\Tests\Models\Tweet\User');
 
         $user   = $repository->findOneBy(array('name' => 'ngal'));
@@ -87,4 +108,28 @@ class PersistentCollectionCriteriaTest extends \Doctrine\Tests\OrmFunctionalTest
         $this->assertCount(1, $tweets);
         $this->assertFalse($tweets->isInitialized());
     }
+
+    /*public function testCanCountWithoutLoadingManyToManyPersistentCollection()
+    {
+        $this->loadQuoteFixture();
+
+        $repository = $this->_em->getRepository('Doctrine\Tests\Models\Quote\User');
+
+        $user   = $repository->findOneBy(array('name' => 'mgal'));
+        $groups = $user->groups->matching(new Criteria());
+
+        $this->assertInstanceOf('Doctrine\ORM\LazyManyToManyCriteriaCollection', $groups);
+        $this->assertFalse($groups->isInitialized());
+        $this->assertCount(2, $groups);
+        $this->assertFalse($groups->isInitialized());
+
+        // Make sure it works with constraints
+        $criteria = new Criteria(Criteria::expr()->eq('name', 'quote1'));
+        $groups   = $user->groups->matching($criteria);
+
+        $this->assertInstanceOf('Doctrine\ORM\LazyManyToManyCriteriaCollection', $groups);
+        $this->assertFalse($groups->isInitialized());
+        $this->assertCount(1, $groups);
+        $this->assertFalse($groups->isInitialized());
+    }*/
 }

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

@@ -9,7 +9,7 @@ use Doctrine\Tests\Models\Quote\SimpleEntity;
  */
 class DDC1719Test extends \Doctrine\Tests\OrmFunctionalTestCase
 {
-    const CLASS_NAME = 'Doctrine\Tests\Models\Quote\SimpleEntity';
+    const CLASS_NAME = 'Doctrine\Tests\ORM\Functional\Ticket\DDC1719SimpleEntity';
 
     protected function setUp()
     {
@@ -31,8 +31,8 @@ class DDC1719Test extends \Doctrine\Tests\OrmFunctionalTestCase
 
     public function testCreateRetrieveUpdateDelete()
     {
-        $e1 = new SimpleEntity('Bar 1');
+        $e1 = new DDC1719SimpleEntity('Bar 1');
-        $e2 = new SimpleEntity('Foo 1');
+        $e2 = new DDC1719SimpleEntity('Foo 1');
 
         // Create
         $this->_em->persist($e1);
@@ -90,3 +90,32 @@ class DDC1719Test extends \Doctrine\Tests\OrmFunctionalTestCase
     }
 
 }
+
+/**
+ * @Entity
+ * @Table(name="`ddc-1719-simple-entity`")
+ */
+class DDC1719SimpleEntity
+{
+
+    /**
+     * @Id
+     * @Column(type="integer", name="`simple-entity-id`")
+     * @GeneratedValue(strategy="AUTO")
+     */
+    public $id;
+
+    /**
+     * @Column(type="string", name="`simple-entity-value`")
+     */
+    public $value;
+
+    /**
+     * @param string $value
+     */
+    public function __construct($value)
+    {
+        $this->value = $value;
+    }
+
+}

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

@@ -164,7 +164,7 @@ class DDC1885Test extends \Doctrine\Tests\OrmFunctionalTestCase
         $this->assertEquals('FabioBatSilva', $user->name);
         $this->assertEquals($u1Id, $user->id);
 
-        $this->assertCount(0, $user->extraLazyGroups);
+        $this->assertCount(2, $user->groups);
         $this->assertInstanceOf('Doctrine\Tests\Models\Quote\Group', $user->getGroups()->get(0));
         $this->assertInstanceOf('Doctrine\Tests\Models\Quote\Group', $user->getGroups()->get(1));
     }

tests/Doctrine/Tests/ORM/Persisters/BasicEntityPersisterTypeValueSqlTest.php

@@ -93,7 +93,7 @@ class BasicEntityPersisterTypeValueSqlTest extends \Doctrine\Tests\OrmTestCase
      */
     public function testStripNonAlphanumericCharactersFromSelectColumnListSQL()
     {
-        $persister  = new BasicEntityPersister($this->_em, $this->_em->getClassMetadata('Doctrine\Tests\Models\Quote\SimpleEntity'));
+        $persister  = new BasicEntityPersister($this->_em, $this->_em->getClassMetadata('Doctrine\Tests\ORM\Functional\Ticket\DDC1719SimpleEntity'));
         $method     = new \ReflectionMethod($persister, 'getSelectColumnsSQL');
         $method->setAccessible(true);
 
@@ -144,7 +144,7 @@ class BasicEntityPersisterTypeValueSqlTest extends \Doctrine\Tests\OrmTestCase
 
     public function testCountCondition()
     {
-        $persister = new BasicEntityPersister($this->_em, $this->_em->getClassMetadata('Doctrine\Tests\Models\Quote\SimpleEntity'));
+        $persister = new BasicEntityPersister($this->_em, $this->_em->getClassMetadata('Doctrine\Tests\ORM\Functional\Ticket\DDC1719SimpleEntity'));
 
         // Using a criteria as array
         $statement = $persister->getCountSQL(array('value' => 'bar'));

tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php

@@ -1889,17 +1889,17 @@ class SelectSqlGenerationTest extends \Doctrine\Tests\OrmTestCase
     public function testStripNonAlphanumericCharactersFromAlias()
     {
         $this->assertSqlGeneration(
-            'SELECT e FROM Doctrine\Tests\Models\Quote\SimpleEntity e',
+            'SELECT e FROM Doctrine\Tests\ORM\Functional\Ticket\DDC1719SimpleEntity e',
             'SELECT d0_."simple-entity-id" AS simpleentityid_0, d0_."simple-entity-value" AS simpleentityvalue_1 FROM "ddc-1719-simple-entity" d0_'
         );
 
         $this->assertSqlGeneration(
-            'SELECT e.value FROM Doctrine\Tests\Models\Quote\SimpleEntity e ORDER BY e.value',
+            'SELECT e.value FROM Doctrine\Tests\ORM\Functional\Ticket\DDC1719SimpleEntity e ORDER BY e.value',
             'SELECT d0_."simple-entity-value" AS simpleentityvalue_0 FROM "ddc-1719-simple-entity" d0_ ORDER BY d0_."simple-entity-value" ASC'
         );
 
         $this->assertSqlGeneration(
-            'SELECT TRIM(e.value) FROM Doctrine\Tests\Models\Quote\SimpleEntity e ORDER BY e.value',
+            'SELECT TRIM(e.value) FROM Doctrine\Tests\ORM\Functional\Ticket\DDC1719SimpleEntity e ORDER BY e.value',
             'SELECT TRIM(d0_."simple-entity-value") AS sclr_0 FROM "ddc-1719-simple-entity" d0_ ORDER BY d0_."simple-entity-value" ASC'
         );
     }

tests/Doctrine/Tests/OrmFunctionalTestCase.php

@@ -192,6 +192,13 @@ abstract class OrmFunctionalTestCase extends OrmTestCase
             'Doctrine\Tests\Models\DDC2504\DDC2504ChildClass',
             'Doctrine\Tests\Models\DDC2504\DDC2504OtherClass',
         ),
+        'quote' => array(
+            'Doctrine\Tests\Models\Quote\Address',
+            'Doctrine\Tests\Models\Quote\Group',
+            'Doctrine\Tests\Models\Quote\NumericEntity',
+            'Doctrine\Tests\Models\Quote\Phone',
+            'Doctrine\Tests\Models\Quote\User'
+        ),
     );
 
     /**
@@ -343,6 +350,13 @@ abstract class OrmFunctionalTestCase extends OrmTestCase
             $conn->executeUpdate('DELETE FROM cache_country');
         }
 
+        if (isset($this->_usedModelSets['quote'])) {
+            $conn->executeUpdate('DELETE FROM "quote-address"');
+            $conn->executeUpdate('DELETE FROM "quote-group"');
+            $conn->executeUpdate('DELETE FROM "quote-phone"');
+            $conn->executeUpdate('DELETE FROM "quote-user"');
+        }
+
         $this->_em->clear();
     }