From a1d77bdc653a14c6767a0b980fc20c00f37b99c0 Mon Sep 17 00:00:00 2001 From: Guilherme Blanco Date: Thu, 15 Jan 2015 03:14:48 +0000 Subject: [PATCH] Renamed coll to collection and some small updates to tests. --- .../AbstractCollectionPersister.php | 25 +++ .../ORM/Persisters/ManyToManyPersister.php | 206 +++++++++--------- .../ORM/Persisters/OneToManyPersister.php | 62 ++---- tests/Doctrine/Tests/Models/Quote/Group.php | 2 +- .../Tests/Models/Quote/SimpleEntity.php | 32 --- tests/Doctrine/Tests/Models/Quote/User.php | 21 +- .../PersistentCollectionCriteriaTest.php | 53 ++++- .../ORM/Functional/Ticket/DDC1719Test.php | 35 ++- .../ORM/Functional/Ticket/DDC1885Test.php | 2 +- .../BasicEntityPersisterTypeValueSqlTest.php | 4 +- .../ORM/Query/SelectSqlGenerationTest.php | 6 +- .../Doctrine/Tests/OrmFunctionalTestCase.php | 14 ++ 12 files changed, 246 insertions(+), 216 deletions(-) delete mode 100644 tests/Doctrine/Tests/Models/Quote/SimpleEntity.php diff --git a/lib/Doctrine/ORM/Persisters/AbstractCollectionPersister.php b/lib/Doctrine/ORM/Persisters/AbstractCollectionPersister.php index a23721f31..669cc22f2 100644 --- a/lib/Doctrine/ORM/Persisters/AbstractCollectionPersister.php +++ b/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; + } } diff --git a/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php b/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php index e09d28b40..50cf89a56 100644 --- a/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php +++ b/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); - $deleteSql = $this->getDeleteRowSQL($coll); + $insertSql = $this->getInsertRowSQL($collection); + $deleteSql = $this->getDeleteRowSQL($collection); - foreach ($coll->getDeleteDiff() as $element) { - $this->conn->executeUpdate($deleteSql, $this->getDeleteRowSQLParameters($coll, $element)); + foreach ($collection->getDeleteDiff() as $element) { + $this->conn->executeUpdate($deleteSql, $this->getDeleteRowSQLParameters($collection, $element)); } - foreach ($coll->getInsertDiff() as $element) { - $this->conn->executeUpdate($insertSql, $this->getInsertRowSQLParameters($coll, $element)); + foreach ($collection->getInsertDiff() as $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($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); + return $persister->load(array($mappedKey => $collection->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(); - $association = $mapping; - $class = $this->em->getClassMetadata($mapping['sourceEntity']); - $id = $this->uow->getEntityIdentifier($coll->getOwner()); + $mapping = $collection->getMapping(); + $id = $this->uow->getEntityIdentifier($collection->getOwner()); + $sourceClass = $this->em->getClassMetadata($mapping['sourceEntity']); + $targetClass = $this->em->getClassMetadata($mapping['targetEntity']); + $association = ( ! $mapping['isOwningSide']) + ? $targetClass->associationMappings[$mapping['mappedBy']] + : $mapping; - if ( ! $mapping['isOwningSide']) { - $targetEntity = $this->em->getClassMetadata($mapping['targetEntity']); - $association = $targetEntity->associationMappings[$mapping['mappedBy']]; - } - - $joinColumns = ( ! $mapping['isOwningSide']) + $joinTableName = $this->quoteStrategy->getJoinTableName($association, $sourceClass, $this->platform); + $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 ($entityState === UnitOfWork::STATE_NEW) { + if ( ! $this->isValidEntityState($element)) { return false; } - // Entity is scheduled for inclusion - if ($entityState === UnitOfWork::STATE_MANAGED && $this->uow->isScheduledForInsert($element)) { - return false; - } - - list($quotedJoinTable, $whereClauses, $params) = $this->getJoinTableRestrictions($coll, $element, true); + list($quotedJoinTable, $whereClauses, $params) = $this->getJoinTableRestrictions($collection, $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 ($entityState === UnitOfWork::STATE_NEW) { + if ( ! $this->isValidEntityState($element)) { 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($element)) { - return false; - } - - list($quotedJoinTable, $whereClauses, $params) = $this->getJoinTableRestrictions($coll, $element, false); + list($quotedJoinTable, $whereClauses, $params) = $this->getJoinTableRestrictions($collection, $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(); - $owner = $coll->getOwner(); + $mapping = $collection->getMapping(); + $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(); - $class = $this->em->getClassMetadata(get_class($coll->getOwner())); + $mapping = $collection->getMapping(); + $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(); - $identifier = $this->uow->getEntityIdentifier($coll->getOwner()); + $mapping = $collection->getMapping(); + $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(); - $class = $this->em->getClassMetadata(get_class($coll->getOwner())); + $mapping = $collection->getMapping(); + $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())); - $class2 = $coll->getTypeClass(); + $class1 = $this->em->getClassMetadata(get_class($collection->getOwner())); + $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); } diff --git a/lib/Doctrine/ORM/Persisters/OneToManyPersister.php b/lib/Doctrine/ORM/Persisters/OneToManyPersister.php index 9d2e1a4a6..f610942cc 100644 --- a/lib/Doctrine/ORM/Persisters/OneToManyPersister.php +++ b/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; - } } diff --git a/tests/Doctrine/Tests/Models/Quote/Group.php b/tests/Doctrine/Tests/Models/Quote/Group.php index c653b6974..b416fbef5 100644 --- a/tests/Doctrine/Tests/Models/Quote/Group.php +++ b/tests/Doctrine/Tests/Models/Quote/Group.php @@ -39,4 +39,4 @@ class Group $this->name = $name; $this->parent = $parent; } -} \ No newline at end of file +} diff --git a/tests/Doctrine/Tests/Models/Quote/SimpleEntity.php b/tests/Doctrine/Tests/Models/Quote/SimpleEntity.php deleted file mode 100644 index 3eda35a4a..000000000 --- a/tests/Doctrine/Tests/Models/Quote/SimpleEntity.php +++ /dev/null @@ -1,32 +0,0 @@ -value = $value; - } - -} \ No newline at end of file diff --git a/tests/Doctrine/Tests/Models/Quote/User.php b/tests/Doctrine/Tests/Models/Quote/User.php index d034f598d..d6d1835a2 100644 --- a/tests/Doctrine/Tests/Models/Quote/User.php +++ b/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; diff --git a/tests/Doctrine/Tests/ORM/Functional/PersistentCollectionCriteriaTest.php b/tests/Doctrine/Tests/ORM/Functional/PersistentCollectionCriteriaTest.php index 2fa6d51f9..b6e58246f 100644 --- a/tests/Doctrine/Tests/ORM/Functional/PersistentCollectionCriteriaTest.php +++ b/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 @@ -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()); + }*/ } diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1719Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1719Test.php index ee23f9dea..a9cadb3cc 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1719Test.php +++ b/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'); - $e2 = new SimpleEntity('Foo 1'); + $e1 = new DDC1719SimpleEntity('Bar 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; + } + +} diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1885Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1885Test.php index 35161c303..91f49282e 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1885Test.php +++ b/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)); } diff --git a/tests/Doctrine/Tests/ORM/Persisters/BasicEntityPersisterTypeValueSqlTest.php b/tests/Doctrine/Tests/ORM/Persisters/BasicEntityPersisterTypeValueSqlTest.php index b9c2ed7bb..8e9ac08df 100644 --- a/tests/Doctrine/Tests/ORM/Persisters/BasicEntityPersisterTypeValueSqlTest.php +++ b/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')); diff --git a/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php b/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php index 0b5a60c96..39c207152 100644 --- a/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php +++ b/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' ); } diff --git a/tests/Doctrine/Tests/OrmFunctionalTestCase.php b/tests/Doctrine/Tests/OrmFunctionalTestCase.php index 65096415a..d556bfe65 100644 --- a/tests/Doctrine/Tests/OrmFunctionalTestCase.php +++ b/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(); }