From dd883f2136417e22087f3b06c19df7bdf1fa039b Mon Sep 17 00:00:00 2001 From: Guilherme Blanco Date: Mon, 12 Jan 2015 22:04:04 +0000 Subject: [PATCH 01/15] Moved delete() and update() to proper locations. --- .../Persister/AbstractCollectionPersister.php | 16 - .../AbstractCollectionPersister.php | 117 +--- .../ORM/Persisters/CollectionPersister.php | 18 - .../ORM/Persisters/ManyToManyPersister.php | 642 ++++++++++-------- .../ORM/Persisters/OneToManyPersister.php | 52 +- .../AbstractCollectionPersisterTest.php | 32 - 6 files changed, 364 insertions(+), 513 deletions(-) diff --git a/lib/Doctrine/ORM/Cache/Persister/AbstractCollectionPersister.php b/lib/Doctrine/ORM/Cache/Persister/AbstractCollectionPersister.php index 12c234398..c6b377c07 100644 --- a/lib/Doctrine/ORM/Cache/Persister/AbstractCollectionPersister.php +++ b/lib/Doctrine/ORM/Cache/Persister/AbstractCollectionPersister.php @@ -226,22 +226,6 @@ abstract class AbstractCollectionPersister implements CachedCollectionPersister return $this->persister->count($collection); } - /** - * {@inheritdoc} - */ - public function deleteRows(PersistentCollection $collection) - { - $this->persister->deleteRows($collection); - } - - /** - * {@inheritdoc} - */ - public function insertRows(PersistentCollection $collection) - { - $this->persister->insertRows($collection); - } - /** * {@inheritdoc} */ diff --git a/lib/Doctrine/ORM/Persisters/AbstractCollectionPersister.php b/lib/Doctrine/ORM/Persisters/AbstractCollectionPersister.php index 0b1dcfbc3..a541e3b6b 100644 --- a/lib/Doctrine/ORM/Persisters/AbstractCollectionPersister.php +++ b/lib/Doctrine/ORM/Persisters/AbstractCollectionPersister.php @@ -79,73 +79,7 @@ abstract class AbstractCollectionPersister implements CollectionPersister */ public function delete(PersistentCollection $coll) { - $mapping = $coll->getMapping(); - - if ( ! $mapping['isOwningSide']) { - return; // ignore inverse side - } - - $this->conn->executeUpdate($this->getDeleteSQL($coll), $this->getDeleteSQLParameters($coll)); - } - - /** - * Gets the SQL statement for deleting the given collection. - * - * @param \Doctrine\ORM\PersistentCollection $coll - * - * @return string - */ - abstract protected function getDeleteSQL(PersistentCollection $coll); - - /** - * Gets the SQL parameters for the corresponding SQL statement to delete - * the given collection. - * - * @param \Doctrine\ORM\PersistentCollection $coll - * - * @return array - */ - abstract protected function getDeleteSQLParameters(PersistentCollection $coll); - - /** - * {@inheritdoc} - */ - public function update(PersistentCollection $coll) - { - $mapping = $coll->getMapping(); - - if ( ! $mapping['isOwningSide']) { - return; // ignore inverse side - } - - $this->deleteRows($coll); - $this->insertRows($coll); - } - - /** - * {@inheritdoc} - */ - public function deleteRows(PersistentCollection $coll) - { - $diff = $coll->getDeleteDiff(); - $sql = $this->getDeleteRowSQL($coll); - - foreach ($diff as $element) { - $this->conn->executeUpdate($sql, $this->getDeleteRowSQLParameters($coll, $element)); - } - } - - /** - * {@inheritdoc} - */ - public function insertRows(PersistentCollection $coll) - { - $diff = $coll->getInsertDiff(); - $sql = $this->getInsertRowSQL($coll); - - foreach ($diff as $element) { - $this->conn->executeUpdate($sql, $this->getInsertRowSQLParameters($coll, $element)); - } + throw new \BadMethodCallException("Deleting elements is not supported by this CollectionPersister."); } /** @@ -211,53 +145,4 @@ abstract class AbstractCollectionPersister implements CollectionPersister { throw new \BadMethodCallException("Filtering a collection by Criteria is not supported by this CollectionPersister."); } - - /** - * Gets the SQL statement used for deleting a row from the collection. - * - * @param \Doctrine\ORM\PersistentCollection $coll - * - * @return string - */ - abstract protected function getDeleteRowSQL(PersistentCollection $coll); - - /** - * Gets the SQL parameters for the corresponding SQL statement to delete the given - * element from the given collection. - * - * @param \Doctrine\ORM\PersistentCollection $coll - * @param mixed $element - * - * @return array - */ - abstract protected function getDeleteRowSQLParameters(PersistentCollection $coll, $element); - - /** - * Gets the SQL statement used for updating a row in the collection. - * - * @param \Doctrine\ORM\PersistentCollection $coll - * - * @return string - */ - abstract protected function getUpdateRowSQL(PersistentCollection $coll); - - /** - * Gets the SQL statement used for inserting a row in the collection. - * - * @param \Doctrine\ORM\PersistentCollection $coll - * - * @return string - */ - abstract protected function getInsertRowSQL(PersistentCollection $coll); - - /** - * Gets the SQL parameters for the corresponding SQL statement to insert the given - * element of the given collection into the database. - * - * @param \Doctrine\ORM\PersistentCollection $coll - * @param mixed $element - * - * @return array - */ - abstract protected function getInsertRowSQLParameters(PersistentCollection $coll, $element); } diff --git a/lib/Doctrine/ORM/Persisters/CollectionPersister.php b/lib/Doctrine/ORM/Persisters/CollectionPersister.php index eea429652..d2ad2e4da 100644 --- a/lib/Doctrine/ORM/Persisters/CollectionPersister.php +++ b/lib/Doctrine/ORM/Persisters/CollectionPersister.php @@ -50,24 +50,6 @@ interface CollectionPersister */ public function update(PersistentCollection $collection); - /** - * Deletes rows. - * - * @param \Doctrine\ORM\PersistentCollection $collection - * - * @return void - */ - public function deleteRows(PersistentCollection $collection); - - /** - * Inserts rows. - * - * @param \Doctrine\ORM\PersistentCollection $collection - * - * @return void - */ - public function insertRows(PersistentCollection $collection); - /** * Counts the size of this persistent collection. * diff --git a/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php b/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php index c8ac9d4d5..6ab3fc3d9 100644 --- a/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php +++ b/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php @@ -37,180 +37,42 @@ class ManyToManyPersister extends AbstractCollectionPersister { /** * {@inheritdoc} - * - * @override */ - protected function getDeleteRowSQL(PersistentCollection $coll) + public function delete(PersistentCollection $coll) { - $columns = array(); - $mapping = $coll->getMapping(); - $class = $this->em->getClassMetadata(get_class($coll->getOwner())); - $tableName = $this->quoteStrategy->getJoinTableName($mapping, $class, $this->platform); + $mapping = $coll->getMapping(); - foreach ($mapping['joinTable']['joinColumns'] as $joinColumn) { - $columns[] = $this->quoteStrategy->getJoinColumnName($joinColumn, $class, $this->platform); + if ( ! $mapping['isOwningSide']) { + return; // ignore inverse side } - foreach ($mapping['joinTable']['inverseJoinColumns'] as $joinColumn) { - $columns[] = $this->quoteStrategy->getJoinColumnName($joinColumn, $class, $this->platform); - } - - return 'DELETE FROM ' . $tableName - . ' WHERE ' . implode(' = ? AND ', $columns) . ' = ?'; + $this->conn->executeUpdate($this->getDeleteSQL($coll), $this->getDeleteSQLParameters($coll)); } /** * {@inheritdoc} - * - * @override - * - * @internal Order of the parameters must be the same as the order of the columns in getDeleteRowSql. */ - protected function getDeleteRowSQLParameters(PersistentCollection $coll, $element) + public function update(PersistentCollection $coll) { - return $this->collectJoinTableColumnParameters($coll, $element); - } + $mapping = $coll->getMapping(); - /** - * {@inheritdoc} - * - * @throws \BadMethodCallException Not used for OneToManyPersister - */ - protected function getUpdateRowSQL(PersistentCollection $coll) - { - throw new \BadMethodCallException("Insert Row SQL is not used for ManyToManyPersister"); - } - - /** - * {@inheritdoc} - * - * @override - * - * @internal Order of the parameters must be the same as the order of the columns in getInsertRowSql. - */ - protected function getInsertRowSQL(PersistentCollection $coll) - { - $columns = array(); - $mapping = $coll->getMapping(); - $class = $this->em->getClassMetadata(get_class($coll->getOwner())); - $joinTable = $this->quoteStrategy->getJoinTableName($mapping, $class, $this->platform); - - foreach ($mapping['joinTable']['joinColumns'] as $joinColumn) { - $columns[] = $this->quoteStrategy->getJoinColumnName($joinColumn, $class, $this->platform); + if ( ! $mapping['isOwningSide']) { + return; // ignore inverse side } - foreach ($mapping['joinTable']['inverseJoinColumns'] as $joinColumn) { - $columns[] = $this->quoteStrategy->getJoinColumnName($joinColumn, $class, $this->platform); + $diff = $coll->getDeleteDiff(); + $sql = $this->getDeleteRowSQL($coll); + + foreach ($diff as $element) { + $this->conn->executeUpdate($sql, $this->getDeleteRowSQLParameters($coll, $element)); } - return 'INSERT INTO ' . $joinTable . ' (' . implode(', ', $columns) . ')' - . ' VALUES (' . implode(', ', array_fill(0, count($columns), '?')) . ')'; - } + $diff = $coll->getInsertDiff(); + $sql = $this->getInsertRowSQL($coll); - /** - * {@inheritdoc} - * - * @override - * - * @internal Order of the parameters must be the same as the order of the columns in getInsertRowSql. - */ - protected function getInsertRowSQLParameters(PersistentCollection $coll, $element) - { - return $this->collectJoinTableColumnParameters($coll, $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 object $element - * - * @return array - */ - private function collectJoinTableColumnParameters(PersistentCollection $coll, $element) - { - $params = array(); - $mapping = $coll->getMapping(); - $isComposite = count($mapping['joinTableColumns']) > 2; - - $identifier1 = $this->uow->getEntityIdentifier($coll->getOwner()); - $identifier2 = $this->uow->getEntityIdentifier($element); - - if ($isComposite) { - $class1 = $this->em->getClassMetadata(get_class($coll->getOwner())); - $class2 = $coll->getTypeClass(); + foreach ($diff as $element) { + $this->conn->executeUpdate($sql, $this->getInsertRowSQLParameters($coll, $element)); } - - foreach ($mapping['joinTableColumns'] as $joinTableColumn) { - $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])]; - - continue; - } - - $params[] = $identifier2[$class2->getFieldForColumn($mapping['relationToTargetKeyColumns'][$joinTableColumn])]; - } - - return $params; - } - - /** - * {@inheritdoc} - * - * @override - */ - protected function getDeleteSQL(PersistentCollection $coll) - { - $columns = array(); - $mapping = $coll->getMapping(); - $class = $this->em->getClassMetadata(get_class($coll->getOwner())); - $joinTable = $this->quoteStrategy->getJoinTableName($mapping, $class, $this->platform); - - foreach ($mapping['joinTable']['joinColumns'] as $joinColumn) { - $columns[] = $this->quoteStrategy->getJoinColumnName($joinColumn, $class, $this->platform); - } - - return 'DELETE FROM ' . $joinTable - . ' WHERE ' . implode(' = ? AND ', $columns) . ' = ?'; - } - - /** - * {@inheritdoc} - * - * @override - * - * @internal Order of the parameters must be the same as the order of the columns in getDeleteSql. - */ - protected function getDeleteSQLParameters(PersistentCollection $coll) - { - $mapping = $coll->getMapping(); - $identifier = $this->uow->getEntityIdentifier($coll->getOwner()); - - // Optimization for single column identifier - if (count($mapping['relationToSourceKeyColumns']) === 1) { - return array(reset($identifier)); - } - - // Composite identifier - $sourceClass = $this->em->getClassMetadata($mapping['sourceEntity']); - $params = array(); - - foreach ($mapping['relationToSourceKeyColumns'] as $columnName => $refColumnName) { - $params[] = isset($sourceClass->fieldNames[$refColumnName]) - ? $identifier[$sourceClass->fieldNames[$refColumnName]] - : $identifier[$sourceClass->getFieldForColumn($columnName)]; - } - - return $params; } /** @@ -328,135 +190,46 @@ class ManyToManyPersister extends AbstractCollectionPersister } /** - * @param \Doctrine\ORM\PersistentCollection $coll - * @param string $key - * @param boolean $addFilters Whether the filter SQL should be included or not. - * - * @return array + * {@inheritDoc} */ - private function getJoinTableRestrictionsWithKey(PersistentCollection $coll, $key, $addFilters) + public function loadCriteria(PersistentCollection $coll, Criteria $criteria) { - $filterMapping = $coll->getMapping(); - $mapping = $filterMapping; - $indexBy = $mapping['indexBy']; - $id = $this->uow->getEntityIdentifier($coll->getOwner()); + $mapping = $coll->getMapping(); + $owner = $coll->getOwner(); + $ownerMetadata = $this->em->getClassMetadata(get_class($owner)); + $whereClauses = $params = array(); - $targetEntity = $this->em->getClassMetadata($mapping['targetEntity']); - - if (! $mapping['isOwningSide']) { - $associationSourceClass = $this->em->getClassMetadata($mapping['targetEntity']); - $mapping = $associationSourceClass->associationMappings[$mapping['mappedBy']]; - $joinColumns = $mapping['joinTable']['joinColumns']; - $relationMode = 'relationToTargetKeyColumns'; - } else { - $joinColumns = $mapping['joinTable']['inverseJoinColumns']; - $associationSourceClass = $this->em->getClassMetadata($mapping['sourceEntity']); - $relationMode = 'relationToSourceKeyColumns'; + foreach ($mapping['relationToSourceKeyColumns'] as $key => $value) { + $whereClauses[] = sprintf('t.%s = ?', $key); + $params[] = $ownerMetadata->getFieldValue($owner, $value); } - $quotedJoinTable = $this->quoteStrategy->getJoinTableName($mapping, $associationSourceClass, $this->platform). ' t'; - $whereClauses = array(); - $params = array(); - - $joinNeeded = !in_array($indexBy, $targetEntity->identifier); - - if ($joinNeeded) { // extra join needed if indexBy is not a @id - $joinConditions = array(); - - foreach ($joinColumns as $joinTableColumn) { - $joinConditions[] = 't.' . $joinTableColumn['name'] . ' = tr.' . $joinTableColumn['referencedColumnName']; - } - $tableName = $this->quoteStrategy->getTableName($targetEntity, $this->platform); - $quotedJoinTable .= ' JOIN ' . $tableName . ' tr ON ' . implode(' AND ', $joinConditions); - - $whereClauses[] = 'tr.' . $targetEntity->getColumnName($indexBy) . ' = ?'; - $params[] = $key; + $parameters = $this->expandCriteriaParameters($criteria); + foreach ($parameters as $parameter) { + list($name, $value) = $parameter; + $whereClauses[] = sprintf('te.%s = ?', $name); + $params[] = $value; } - foreach ($mapping['joinTableColumns'] as $joinTableColumn) { - if (isset($mapping[$relationMode][$joinTableColumn])) { - $whereClauses[] = 't.' . $joinTableColumn . ' = ?'; - $params[] = $targetEntity->containsForeignIdentifier - ? $id[$targetEntity->getFieldForColumn($mapping[$relationMode][$joinTableColumn])] - : $id[$targetEntity->fieldNames[$mapping[$relationMode][$joinTableColumn]]]; - } elseif (!$joinNeeded) { - $whereClauses[] = 't.' . $joinTableColumn . ' = ?'; - $params[] = $key; - } - } + $mapping = $coll->getMapping(); + $targetClass = $this->em->getClassMetadata($mapping['targetEntity']); + $tableName = $this->quoteStrategy->getTableName($targetClass, $this->platform); + $joinTable = $this->quoteStrategy->getJoinTableName($mapping, $ownerMetadata, $this->platform); + $onConditions = $this->getOnConditionSQL($mapping); - if ($addFilters) { - list($joinTargetEntitySQL, $filterSql) = $this->getFilterSql($filterMapping); + $rsm = new Query\ResultSetMappingBuilder($this->em); + $rsm->addRootEntityFromClassMetadata($mapping['targetEntity'], 'te'); - if ($filterSql) { - $quotedJoinTable .= ' ' . $joinTargetEntitySQL; - $whereClauses[] = $filterSql; - } - } + $sql = 'SELECT ' . $rsm->generateSelectClause() . ' FROM ' . $tableName . ' te' + . ' JOIN ' . $joinTable . ' t ON' + . implode(' AND ', $onConditions) + . ' WHERE ' . implode(' AND ', $whereClauses); - return array($quotedJoinTable, $whereClauses, $params); - } + $stmt = $this->conn->executeQuery($sql, $params); + $hydrator = $this->em->newHydrator(Query::HYDRATE_OBJECT); - /** - * @param \Doctrine\ORM\PersistentCollection $coll - * @param object $element - * @param boolean $addFilters Whether the filter SQL should be included or not. - * - * @return array - */ - private function getJoinTableRestrictions(PersistentCollection $coll, $element, $addFilters) - { - $filterMapping = $coll->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()); - - $mapping = $sourceClass->associationMappings[$mapping['mappedBy']]; - } else { - $sourceClass = $this->em->getClassMetadata($mapping['sourceEntity']); - $targetClass = $this->em->getClassMetadata($mapping['targetEntity']); - $sourceId = $this->uow->getEntityIdentifier($coll->getOwner()); - $targetId = $this->uow->getEntityIdentifier($element); - } - - $quotedJoinTable = $this->quoteStrategy->getJoinTableName($mapping, $sourceClass, $this->platform); - $whereClauses = array(); - $params = array(); - - foreach ($mapping['joinTableColumns'] as $joinTableColumn) { - $whereClauses[] = ($addFilters ? 't.' : '') . $joinTableColumn . ' = ?'; - - if (isset($mapping['relationToTargetKeyColumns'][$joinTableColumn])) { - $params[] = ($targetClass->containsForeignIdentifier) - ? $targetId[$targetClass->getFieldForColumn($mapping['relationToTargetKeyColumns'][$joinTableColumn])] - : $targetId[$targetClass->fieldNames[$mapping['relationToTargetKeyColumns'][$joinTableColumn]]]; - - continue; - } - - // relationToSourceKeyColumns - $params[] = ($sourceClass->containsForeignIdentifier) - ? $sourceId[$sourceClass->getFieldForColumn($mapping['relationToSourceKeyColumns'][$joinTableColumn])] - : $sourceId[$sourceClass->fieldNames[$mapping['relationToSourceKeyColumns'][$joinTableColumn]]]; - } - - if ($addFilters) { - $quotedJoinTable .= ' t'; - - list($joinTargetEntitySQL, $filterSql) = $this->getFilterSql($filterMapping); - - if ($filterSql) { - $quotedJoinTable .= ' ' . $joinTargetEntitySQL; - $whereClauses[] = $filterSql; - } - } - - return array($quotedJoinTable, $whereClauses, $params); + return $hydrator->hydrateAll($stmt, $rsm); } /** @@ -549,46 +322,313 @@ class ManyToManyPersister extends AbstractCollectionPersister } /** - * {@inheritDoc} + * {@inheritdoc} + * + * @override */ - public function loadCriteria(PersistentCollection $coll, Criteria $criteria) + protected function getDeleteSQL(PersistentCollection $coll) { - $mapping = $coll->getMapping(); - $owner = $coll->getOwner(); - $ownerMetadata = $this->em->getClassMetadata(get_class($owner)); - $whereClauses = $params = array(); + $columns = array(); + $mapping = $coll->getMapping(); + $class = $this->em->getClassMetadata(get_class($coll->getOwner())); + $joinTable = $this->quoteStrategy->getJoinTableName($mapping, $class, $this->platform); - foreach ($mapping['relationToSourceKeyColumns'] as $key => $value) { - $whereClauses[] = sprintf('t.%s = ?', $key); - $params[] = $ownerMetadata->getFieldValue($owner, $value); + foreach ($mapping['joinTable']['joinColumns'] as $joinColumn) { + $columns[] = $this->quoteStrategy->getJoinColumnName($joinColumn, $class, $this->platform); } - $parameters = $this->expandCriteriaParameters($criteria); + return 'DELETE FROM ' . $joinTable + . ' WHERE ' . implode(' = ? AND ', $columns) . ' = ?'; + } - foreach ($parameters as $parameter) { - list($name, $value) = $parameter; - $whereClauses[] = sprintf('te.%s = ?', $name); - $params[] = $value; + /** + * {@inheritdoc} + * + * @override + * + * @internal Order of the parameters must be the same as the order of the columns in getDeleteSql. + */ + protected function getDeleteSQLParameters(PersistentCollection $coll) + { + $mapping = $coll->getMapping(); + $identifier = $this->uow->getEntityIdentifier($coll->getOwner()); + + // Optimization for single column identifier + if (count($mapping['relationToSourceKeyColumns']) === 1) { + return array(reset($identifier)); } - $mapping = $coll->getMapping(); - $targetClass = $this->em->getClassMetadata($mapping['targetEntity']); - $tableName = $this->quoteStrategy->getTableName($targetClass, $this->platform); - $joinTable = $this->quoteStrategy->getJoinTableName($mapping, $ownerMetadata, $this->platform); - $onConditions = $this->getOnConditionSQL($mapping); + // Composite identifier + $sourceClass = $this->em->getClassMetadata($mapping['sourceEntity']); + $params = array(); - $rsm = new Query\ResultSetMappingBuilder($this->em); - $rsm->addRootEntityFromClassMetadata($mapping['targetEntity'], 'te'); + foreach ($mapping['relationToSourceKeyColumns'] as $columnName => $refColumnName) { + $params[] = isset($sourceClass->fieldNames[$refColumnName]) + ? $identifier[$sourceClass->fieldNames[$refColumnName]] + : $identifier[$sourceClass->getFieldForColumn($columnName)]; + } - $sql = 'SELECT ' . $rsm->generateSelectClause() . ' FROM ' . $tableName . ' te' - . ' JOIN ' . $joinTable . ' t ON' - . implode(' AND ', $onConditions) - . ' WHERE ' . implode(' AND ', $whereClauses); + return $params; + } - $stmt = $this->conn->executeQuery($sql, $params); - $hydrator = $this->em->newHydrator(Query::HYDRATE_OBJECT); + /** + * Gets the SQL statement used for deleting a row from the collection. + * + * @param \Doctrine\ORM\PersistentCollection $coll + * + * @return string + */ + protected function getDeleteRowSQL(PersistentCollection $coll) + { + $columns = array(); + $mapping = $coll->getMapping(); + $class = $this->em->getClassMetadata(get_class($coll->getOwner())); + $tableName = $this->quoteStrategy->getJoinTableName($mapping, $class, $this->platform); - return $hydrator->hydrateAll($stmt, $rsm); + foreach ($mapping['joinTable']['joinColumns'] as $joinColumn) { + $columns[] = $this->quoteStrategy->getJoinColumnName($joinColumn, $class, $this->platform); + } + + foreach ($mapping['joinTable']['inverseJoinColumns'] as $joinColumn) { + $columns[] = $this->quoteStrategy->getJoinColumnName($joinColumn, $class, $this->platform); + } + + return 'DELETE FROM ' . $tableName + . ' WHERE ' . implode(' = ? AND ', $columns) . ' = ?'; + } + + /** + * Gets the SQL parameters for the corresponding SQL statement to delete the given + * element from the given collection. + * + * @internal Order of the parameters must be the same as the order of the columns in getDeleteRowSql. + * + * @param \Doctrine\ORM\PersistentCollection $coll + * @param mixed $element + * + * @return array + */ + protected function getDeleteRowSQLParameters(PersistentCollection $coll, $element) + { + return $this->collectJoinTableColumnParameters($coll, $element); + } + + /** + * Gets the SQL statement used for inserting a row in the collection. + * + * @param \Doctrine\ORM\PersistentCollection $coll + * + * @return string + */ + protected function getInsertRowSQL(PersistentCollection $coll) + { + $columns = array(); + $mapping = $coll->getMapping(); + $class = $this->em->getClassMetadata(get_class($coll->getOwner())); + $joinTable = $this->quoteStrategy->getJoinTableName($mapping, $class, $this->platform); + + foreach ($mapping['joinTable']['joinColumns'] as $joinColumn) { + $columns[] = $this->quoteStrategy->getJoinColumnName($joinColumn, $class, $this->platform); + } + + foreach ($mapping['joinTable']['inverseJoinColumns'] as $joinColumn) { + $columns[] = $this->quoteStrategy->getJoinColumnName($joinColumn, $class, $this->platform); + } + + return 'INSERT INTO ' . $joinTable . ' (' . implode(', ', $columns) . ')' + . ' VALUES (' . implode(', ', array_fill(0, count($columns), '?')) . ')'; + } + + /** + * Gets the SQL parameters for the corresponding SQL statement to insert the given + * element of the given collection into the database. + * + * @internal Order of the parameters must be the same as the order of the columns in getInsertRowSql. + * + * @param \Doctrine\ORM\PersistentCollection $coll + * @param mixed $element + * + * @return array + */ + protected function getInsertRowSQLParameters(PersistentCollection $coll, $element) + { + return $this->collectJoinTableColumnParameters($coll, $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 object $element + * + * @return array + */ + private function collectJoinTableColumnParameters(PersistentCollection $coll, $element) + { + $params = array(); + $mapping = $coll->getMapping(); + $isComposite = count($mapping['joinTableColumns']) > 2; + + $identifier1 = $this->uow->getEntityIdentifier($coll->getOwner()); + $identifier2 = $this->uow->getEntityIdentifier($element); + + if ($isComposite) { + $class1 = $this->em->getClassMetadata(get_class($coll->getOwner())); + $class2 = $coll->getTypeClass(); + } + + foreach ($mapping['joinTableColumns'] as $joinTableColumn) { + $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])]; + + continue; + } + + $params[] = $identifier2[$class2->getFieldForColumn($mapping['relationToTargetKeyColumns'][$joinTableColumn])]; + } + + return $params; + } + + /** + * @param \Doctrine\ORM\PersistentCollection $coll + * @param string $key + * @param boolean $addFilters Whether the filter SQL should be included or not. + * + * @return array + */ + private function getJoinTableRestrictionsWithKey(PersistentCollection $coll, $key, $addFilters) + { + $filterMapping = $coll->getMapping(); + $mapping = $filterMapping; + $indexBy = $mapping['indexBy']; + $id = $this->uow->getEntityIdentifier($coll->getOwner()); + + $targetEntity = $this->em->getClassMetadata($mapping['targetEntity']); + + if (! $mapping['isOwningSide']) { + $associationSourceClass = $this->em->getClassMetadata($mapping['targetEntity']); + $mapping = $associationSourceClass->associationMappings[$mapping['mappedBy']]; + $joinColumns = $mapping['joinTable']['joinColumns']; + $relationMode = 'relationToTargetKeyColumns'; + } else { + $joinColumns = $mapping['joinTable']['inverseJoinColumns']; + $associationSourceClass = $this->em->getClassMetadata($mapping['sourceEntity']); + $relationMode = 'relationToSourceKeyColumns'; + } + + $quotedJoinTable = $this->quoteStrategy->getJoinTableName($mapping, $associationSourceClass, $this->platform). ' t'; + $whereClauses = array(); + $params = array(); + + $joinNeeded = !in_array($indexBy, $targetEntity->identifier); + + if ($joinNeeded) { // extra join needed if indexBy is not a @id + $joinConditions = array(); + + foreach ($joinColumns as $joinTableColumn) { + $joinConditions[] = 't.' . $joinTableColumn['name'] . ' = tr.' . $joinTableColumn['referencedColumnName']; + } + $tableName = $this->quoteStrategy->getTableName($targetEntity, $this->platform); + $quotedJoinTable .= ' JOIN ' . $tableName . ' tr ON ' . implode(' AND ', $joinConditions); + + $whereClauses[] = 'tr.' . $targetEntity->getColumnName($indexBy) . ' = ?'; + $params[] = $key; + + } + + foreach ($mapping['joinTableColumns'] as $joinTableColumn) { + if (isset($mapping[$relationMode][$joinTableColumn])) { + $whereClauses[] = 't.' . $joinTableColumn . ' = ?'; + $params[] = $targetEntity->containsForeignIdentifier + ? $id[$targetEntity->getFieldForColumn($mapping[$relationMode][$joinTableColumn])] + : $id[$targetEntity->fieldNames[$mapping[$relationMode][$joinTableColumn]]]; + } elseif (!$joinNeeded) { + $whereClauses[] = 't.' . $joinTableColumn . ' = ?'; + $params[] = $key; + } + } + + if ($addFilters) { + list($joinTargetEntitySQL, $filterSql) = $this->getFilterSql($filterMapping); + + if ($filterSql) { + $quotedJoinTable .= ' ' . $joinTargetEntitySQL; + $whereClauses[] = $filterSql; + } + } + + return array($quotedJoinTable, $whereClauses, $params); + } + + /** + * @param \Doctrine\ORM\PersistentCollection $coll + * @param object $element + * @param boolean $addFilters Whether the filter SQL should be included or not. + * + * @return array + */ + private function getJoinTableRestrictions(PersistentCollection $coll, $element, $addFilters) + { + $filterMapping = $coll->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()); + + $mapping = $sourceClass->associationMappings[$mapping['mappedBy']]; + } else { + $sourceClass = $this->em->getClassMetadata($mapping['sourceEntity']); + $targetClass = $this->em->getClassMetadata($mapping['targetEntity']); + $sourceId = $this->uow->getEntityIdentifier($coll->getOwner()); + $targetId = $this->uow->getEntityIdentifier($element); + } + + $quotedJoinTable = $this->quoteStrategy->getJoinTableName($mapping, $sourceClass, $this->platform); + $whereClauses = array(); + $params = array(); + + foreach ($mapping['joinTableColumns'] as $joinTableColumn) { + $whereClauses[] = ($addFilters ? 't.' : '') . $joinTableColumn . ' = ?'; + + if (isset($mapping['relationToTargetKeyColumns'][$joinTableColumn])) { + $params[] = ($targetClass->containsForeignIdentifier) + ? $targetId[$targetClass->getFieldForColumn($mapping['relationToTargetKeyColumns'][$joinTableColumn])] + : $targetId[$targetClass->fieldNames[$mapping['relationToTargetKeyColumns'][$joinTableColumn]]]; + + continue; + } + + // relationToSourceKeyColumns + $params[] = ($sourceClass->containsForeignIdentifier) + ? $sourceId[$sourceClass->getFieldForColumn($mapping['relationToSourceKeyColumns'][$joinTableColumn])] + : $sourceId[$sourceClass->fieldNames[$mapping['relationToSourceKeyColumns'][$joinTableColumn]]]; + } + + if ($addFilters) { + $quotedJoinTable .= ' t'; + + list($joinTargetEntitySQL, $filterSql) = $this->getFilterSql($filterMapping); + + if ($filterSql) { + $quotedJoinTable .= ' ' . $joinTargetEntitySQL; + $whereClauses[] = $filterSql; + } + } + + return array($quotedJoinTable, $whereClauses, $params); } /** diff --git a/lib/Doctrine/ORM/Persisters/OneToManyPersister.php b/lib/Doctrine/ORM/Persisters/OneToManyPersister.php index c1e8a6c38..5586bd99a 100644 --- a/lib/Doctrine/ORM/Persisters/OneToManyPersister.php +++ b/lib/Doctrine/ORM/Persisters/OneToManyPersister.php @@ -33,6 +33,28 @@ use Doctrine\ORM\UnitOfWork; */ class OneToManyPersister extends AbstractCollectionPersister { + /** + * {@inheritdoc} + */ + public function delete(PersistentCollection $coll) + { + // 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, + // then classifying it as a ManyToManyPersister. + return; + } + + /** + * {@inheritdoc} + */ + public function update(PersistentCollection $coll) + { + // 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, + // then classifying it as a ManyToManyPersister. + return; + } + /** * {@inheritdoc} */ @@ -97,36 +119,6 @@ class OneToManyPersister extends AbstractCollectionPersister throw new \BadMethodCallException("Insert Row SQL is not used for OneToManyPersister"); } - /** - * {@inheritdoc} - * - * @throws \BadMethodCallException Not used for OneToManyPersister. - */ - protected function getUpdateRowSQL(PersistentCollection $coll) - { - throw new \BadMethodCallException("Update Row SQL is not used for OneToManyPersister"); - } - - /** - * {@inheritdoc} - * - * @throws \BadMethodCallException Not used for OneToManyPersister. - */ - protected function getDeleteSQL(PersistentCollection $coll) - { - throw new \BadMethodCallException("Delete Row SQL is not used for OneToManyPersister"); - } - - /** - * {@inheritdoc} - * - * @throws \BadMethodCallException Not used for OneToManyPersister. - */ - protected function getDeleteSQLParameters(PersistentCollection $coll) - { - throw new \BadMethodCallException("Delete Row SQL is not used for OneToManyPersister"); - } - /** * {@inheritdoc} */ diff --git a/tests/Doctrine/Tests/ORM/Cache/Persister/AbstractCollectionPersisterTest.php b/tests/Doctrine/Tests/ORM/Cache/Persister/AbstractCollectionPersisterTest.php index d8c94a39c..e216cd79e 100644 --- a/tests/Doctrine/Tests/ORM/Cache/Persister/AbstractCollectionPersisterTest.php +++ b/tests/Doctrine/Tests/ORM/Cache/Persister/AbstractCollectionPersisterTest.php @@ -49,8 +49,6 @@ abstract class AbstractCollectionPersisterTest extends OrmTestCase protected $collectionPersisterMockMethods = array( 'delete', 'update', - 'deleteRows', - 'insertRows', 'count', 'slice', 'contains', @@ -154,36 +152,6 @@ abstract class AbstractCollectionPersisterTest extends OrmTestCase $this->assertNull($persister->update($collection)); } - public function testInvokeDeleteRows() - { - $entity = new State("Foo"); - $persister = $this->createPersisterDefault(); - $collection = $this->createCollection($entity); - - $this->em->getUnitOfWork()->registerManaged($entity, array('id'=>1), array('id'=>1, 'name'=>'Foo')); - - $this->collectionPersister->expects($this->once()) - ->method('deleteRows') - ->with($this->equalTo($collection)); - - $this->assertNull($persister->deleteRows($collection)); - } - - public function testInvokeInsertRows() - { - $entity = new State("Foo"); - $persister = $this->createPersisterDefault(); - $collection = $this->createCollection($entity); - - $this->em->getUnitOfWork()->registerManaged($entity, array('id'=>1), array('id'=>1, 'name'=>'Foo')); - - $this->collectionPersister->expects($this->once()) - ->method('insertRows') - ->with($this->equalTo($collection)); - - $this->assertNull($persister->insertRows($collection)); - } - public function testInvokeCount() { $entity = new State("Foo"); From 96955d6e79bf05d8f9abf3965270ae9f20a28c66 Mon Sep 17 00:00:00 2001 From: Guilherme Blanco Date: Tue, 13 Jan 2015 02:05:33 +0000 Subject: [PATCH 02/15] Some small improvements to persisters. --- .../AbstractCollectionPersister.php | 74 ------------------- .../ORM/Persisters/CollectionPersister.php | 10 --- .../ORM/Persisters/ManyToManyPersister.php | 23 +++--- .../ORM/Persisters/OneToManyPersister.php | 8 ++ 4 files changed, 22 insertions(+), 93 deletions(-) diff --git a/lib/Doctrine/ORM/Persisters/AbstractCollectionPersister.php b/lib/Doctrine/ORM/Persisters/AbstractCollectionPersister.php index a541e3b6b..a23721f31 100644 --- a/lib/Doctrine/ORM/Persisters/AbstractCollectionPersister.php +++ b/lib/Doctrine/ORM/Persisters/AbstractCollectionPersister.php @@ -19,9 +19,7 @@ namespace Doctrine\ORM\Persisters; -use Doctrine\Common\Collections\Criteria; use Doctrine\ORM\EntityManager; -use Doctrine\ORM\PersistentCollection; /** * Base class for all collection persisters. @@ -73,76 +71,4 @@ abstract class AbstractCollectionPersister implements CollectionPersister $this->platform = $this->conn->getDatabasePlatform(); $this->quoteStrategy = $em->getConfiguration()->getQuoteStrategy(); } - - /** - * {@inheritdoc} - */ - public function delete(PersistentCollection $coll) - { - throw new \BadMethodCallException("Deleting elements is not supported by this CollectionPersister."); - } - - /** - * {@inheritdoc} - */ - public function count(PersistentCollection $coll) - { - throw new \BadMethodCallException("Counting the size of this persistent collection is not supported by this CollectionPersister."); - } - - /** - * {@inheritdoc} - */ - public function slice(PersistentCollection $coll, $offset, $length = null) - { - throw new \BadMethodCallException("Slicing elements is not supported by this CollectionPersister."); - } - - /** - * {@inheritdoc} - */ - public function contains(PersistentCollection $coll, $element) - { - throw new \BadMethodCallException("Checking for existence of an element is not supported by this CollectionPersister."); - } - - /** - * {@inheritdoc} - */ - public function containsKey(PersistentCollection $coll, $key) - { - throw new \BadMethodCallException("Checking for existence of a key is not supported by this CollectionPersister."); - } - - /** - * {@inheritdoc} - */ - public function removeElement(PersistentCollection $coll, $element) - { - throw new \BadMethodCallException("Removing an element is not supported by this CollectionPersister."); - } - - /** - * {@inheritdoc} - */ - public function removeKey(PersistentCollection $coll, $key) - { - throw new \BadMethodCallException("Removing a key is not supported by this CollectionPersister."); - } - - /** - * {@inheritdoc} - */ - public function get(PersistentCollection $coll, $index) - { - throw new \BadMethodCallException("Selecting a collection by index is not supported by this CollectionPersister."); - } - - /** - * {@inheritdoc} - */ - public function loadCriteria(PersistentCollection $coll, Criteria $criteria) - { - throw new \BadMethodCallException("Filtering a collection by Criteria is not supported by this CollectionPersister."); - } } diff --git a/lib/Doctrine/ORM/Persisters/CollectionPersister.php b/lib/Doctrine/ORM/Persisters/CollectionPersister.php index d2ad2e4da..855b85a30 100644 --- a/lib/Doctrine/ORM/Persisters/CollectionPersister.php +++ b/lib/Doctrine/ORM/Persisters/CollectionPersister.php @@ -100,16 +100,6 @@ interface CollectionPersister */ public function removeElement(PersistentCollection $collection, $element); - /** - * Removes an element by key. - * - * @param \Doctrine\ORM\PersistentCollection $collection - * @param mixed $key - * - * @return void - */ - public function removeKey(PersistentCollection $collection, $key); - /** * Gets an element by key. * diff --git a/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php b/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php index 6ab3fc3d9..f671d8921 100644 --- a/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php +++ b/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php @@ -60,21 +60,26 @@ class ManyToManyPersister extends AbstractCollectionPersister return; // ignore inverse side } - $diff = $coll->getDeleteDiff(); - $sql = $this->getDeleteRowSQL($coll); + $insertSql = $this->getInsertRowSQL($coll); + $deleteSql = $this->getDeleteRowSQL($coll); - foreach ($diff as $element) { - $this->conn->executeUpdate($sql, $this->getDeleteRowSQLParameters($coll, $element)); + foreach ($coll->getDeleteDiff() as $element) { + $this->conn->executeUpdate($deleteSql, $this->getDeleteRowSQLParameters($coll, $element)); } - $diff = $coll->getInsertDiff(); - $sql = $this->getInsertRowSQL($coll); - - foreach ($diff as $element) { - $this->conn->executeUpdate($sql, $this->getInsertRowSQLParameters($coll, $element)); + foreach ($coll->getInsertDiff() as $element) { + $this->conn->executeUpdate($insertSql, $this->getInsertRowSQLParameters($coll, $element)); } } + /** + * {@inheritdoc} + */ + public function get(PersistentCollection $coll, $index) + { + throw new \BadMethodCallException("Selecting a collection by index is not supported by this CollectionPersister."); + } + /** * {@inheritdoc} */ diff --git a/lib/Doctrine/ORM/Persisters/OneToManyPersister.php b/lib/Doctrine/ORM/Persisters/OneToManyPersister.php index 5586bd99a..5727111a1 100644 --- a/lib/Doctrine/ORM/Persisters/OneToManyPersister.php +++ b/lib/Doctrine/ORM/Persisters/OneToManyPersister.php @@ -214,4 +214,12 @@ class OneToManyPersister extends AbstractCollectionPersister return $persister->delete($element); } + + /** + * {@inheritdoc} + */ + public function loadCriteria(PersistentCollection $collection, Criteria $criteria) + { + throw new \BadMethodCallException("Filtering a collection by Criteria is not supported by this CollectionPersister."); + } } From 965cdbdbbb2e7e699deeb86787d81d4e56a7fe89 Mon Sep 17 00:00:00 2001 From: Guilherme Blanco Date: Tue, 13 Jan 2015 02:18:49 +0000 Subject: [PATCH 03/15] Optimized column to field resolutions. --- .../ORM/Persisters/BasicEntityPersister.php | 16 +++------------- .../ORM/Persisters/ManyToManyPersister.php | 16 ++++------------ 2 files changed, 7 insertions(+), 25 deletions(-) diff --git a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php index 06c0923a9..1e6374319 100644 --- a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php +++ b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php @@ -670,19 +670,9 @@ class BasicEntityPersister implements EntityPersister $this->quotedColumns[$sourceColumn] = $quotedColumn; $this->columnTypes[$sourceColumn] = $targetClass->getTypeOfColumn($targetColumn); - switch (true) { - case $newVal === null: - $value = null; - break; - - case $targetClass->containsForeignIdentifier: - $value = $newValId[$targetClass->getFieldForColumn($targetColumn)]; - break; - - default: - $value = $newValId[$targetClass->fieldNames[$targetColumn]]; - break; - } + $value = ($newVal !== null) + ? $newValId[$targetClass->getFieldForColumn($targetColumn)] + : null; $result[$owningTable][$sourceColumn] = $value; } diff --git a/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php b/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php index f671d8921..7fd89245e 100644 --- a/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php +++ b/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php @@ -105,9 +105,7 @@ class ManyToManyPersister extends AbstractCollectionPersister $columnName = $this->quoteStrategy->getJoinColumnName($joinColumn, $class, $this->platform); $referencedName = $joinColumn['referencedColumnName']; $conditions[] = 't.' . $columnName . ' = ?'; - $params[] = ($class->containsForeignIdentifier) - ? $id[$class->getFieldForColumn($referencedName)] - : $id[$class->fieldNames[$referencedName]]; + $params[] = $id[$class->getFieldForColumn($referencedName)]; } $joinTableName = $this->quoteStrategy->getJoinTableName($association, $class, $this->platform); @@ -554,9 +552,7 @@ class ManyToManyPersister extends AbstractCollectionPersister foreach ($mapping['joinTableColumns'] as $joinTableColumn) { if (isset($mapping[$relationMode][$joinTableColumn])) { $whereClauses[] = 't.' . $joinTableColumn . ' = ?'; - $params[] = $targetEntity->containsForeignIdentifier - ? $id[$targetEntity->getFieldForColumn($mapping[$relationMode][$joinTableColumn])] - : $id[$targetEntity->fieldNames[$mapping[$relationMode][$joinTableColumn]]]; + $params[] = $id[$targetEntity->getFieldForColumn($mapping[$relationMode][$joinTableColumn])]; } elseif (!$joinNeeded) { $whereClauses[] = 't.' . $joinTableColumn . ' = ?'; $params[] = $key; @@ -609,17 +605,13 @@ class ManyToManyPersister extends AbstractCollectionPersister $whereClauses[] = ($addFilters ? 't.' : '') . $joinTableColumn . ' = ?'; if (isset($mapping['relationToTargetKeyColumns'][$joinTableColumn])) { - $params[] = ($targetClass->containsForeignIdentifier) - ? $targetId[$targetClass->getFieldForColumn($mapping['relationToTargetKeyColumns'][$joinTableColumn])] - : $targetId[$targetClass->fieldNames[$mapping['relationToTargetKeyColumns'][$joinTableColumn]]]; + $params[] = $targetId[$targetClass->getFieldForColumn($mapping['relationToTargetKeyColumns'][$joinTableColumn])]; continue; } // relationToSourceKeyColumns - $params[] = ($sourceClass->containsForeignIdentifier) - ? $sourceId[$sourceClass->getFieldForColumn($mapping['relationToSourceKeyColumns'][$joinTableColumn])] - : $sourceId[$sourceClass->fieldNames[$mapping['relationToSourceKeyColumns'][$joinTableColumn]]]; + $params[] = $sourceId[$sourceClass->getFieldForColumn($mapping['relationToSourceKeyColumns'][$joinTableColumn])]; } if ($addFilters) { From 678f47f494539019c8349de420c38dd489860c67 Mon Sep 17 00:00:00 2001 From: Guilherme Blanco Date: Tue, 13 Jan 2015 02:52:31 +0000 Subject: [PATCH 04/15] More deprecated code removal. --- .../ORM/Persisters/OneToManyPersister.php | 92 ++++++------------- 1 file changed, 26 insertions(+), 66 deletions(-) diff --git a/lib/Doctrine/ORM/Persisters/OneToManyPersister.php b/lib/Doctrine/ORM/Persisters/OneToManyPersister.php index 5727111a1..239477edb 100644 --- a/lib/Doctrine/ORM/Persisters/OneToManyPersister.php +++ b/lib/Doctrine/ORM/Persisters/OneToManyPersister.php @@ -70,55 +70,6 @@ class OneToManyPersister extends AbstractCollectionPersister return $persister->load(array($mapping['mappedBy'] => $coll->getOwner(), $mapping['indexBy'] => $index), null, null, array(), null, 1); } - /** - * Generates the SQL UPDATE that updates a particular row's foreign - * key to null. - * - * @param \Doctrine\ORM\PersistentCollection $coll - * - * @return string - * - * @override - */ - protected function getDeleteRowSQL(PersistentCollection $coll) - { - $mapping = $coll->getMapping(); - $class = $this->em->getClassMetadata($mapping['targetEntity']); - $tableName = $this->quoteStrategy->getTableName($class, $this->platform); - $idColumns = $class->getIdentifierColumnNames(); - - return 'DELETE FROM ' . $tableName - . ' WHERE ' . implode('= ? AND ', $idColumns) . ' = ?'; - } - - /** - * {@inheritdoc} - */ - protected function getDeleteRowSQLParameters(PersistentCollection $coll, $element) - { - return array_values($this->uow->getEntityIdentifier($element)); - } - - /** - * {@inheritdoc} - * - * @throws \BadMethodCallException Not used for OneToManyPersister. - */ - protected function getInsertRowSQL(PersistentCollection $coll) - { - throw new \BadMethodCallException("Insert Row SQL is not used for OneToManyPersister"); - } - - /** - * {@inheritdoc} - * - * @throws \BadMethodCallException Not used for OneToManyPersister. - */ - protected function getInsertRowSQLParameters(PersistentCollection $coll, $element) - { - throw new \BadMethodCallException("Insert Row SQL is not used for OneToManyPersister"); - } - /** * {@inheritdoc} */ @@ -170,14 +121,7 @@ class OneToManyPersister extends AbstractCollectionPersister */ public function contains(PersistentCollection $coll, $element) { - $entityState = $this->uow->getEntityState($element, UnitOfWork::STATE_NEW); - - if ($entityState === UnitOfWork::STATE_NEW) { - return false; - } - - // Entity is scheduled for inclusion - if ($entityState === UnitOfWork::STATE_MANAGED && $this->uow->isScheduledForInsert($element)) { + if ( ! $this->isValidEntityState($element)) { return false; } @@ -197,15 +141,7 @@ class OneToManyPersister extends AbstractCollectionPersister */ public function removeElement(PersistentCollection $coll, $element) { - $entityState = $this->uow->getEntityState($element, UnitOfWork::STATE_NEW); - - if ($entityState === UnitOfWork::STATE_NEW) { - return false; - } - - // If Entity is scheduled for inclusion, it is not in this collection. - // We can assure that because it would have return true before on array check - if ($entityState === UnitOfWork::STATE_MANAGED && $this->uow->isScheduledForInsert($element)) { + if ( ! $this->isValidEntityState($element)) { return false; } @@ -222,4 +158,28 @@ 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 $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; + } } From bc268da8c2002254be9cdd1c9d3f7b094c11ded7 Mon Sep 17 00:00:00 2001 From: Guilherme Blanco Date: Tue, 13 Jan 2015 03:30:07 +0000 Subject: [PATCH 05/15] Small optimization. --- lib/Doctrine/ORM/Persisters/ManyToManyPersister.php | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php b/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php index 7fd89245e..c4d495104 100644 --- a/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php +++ b/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php @@ -383,10 +383,9 @@ class ManyToManyPersister extends AbstractCollectionPersister */ protected function getDeleteRowSQL(PersistentCollection $coll) { - $columns = array(); - $mapping = $coll->getMapping(); - $class = $this->em->getClassMetadata(get_class($coll->getOwner())); - $tableName = $this->quoteStrategy->getJoinTableName($mapping, $class, $this->platform); + $mapping = $coll->getMapping(); + $class = $this->em->getClassMetadata($mapping['sourceEntity']); + $columns = array(); foreach ($mapping['joinTable']['joinColumns'] as $joinColumn) { $columns[] = $this->quoteStrategy->getJoinColumnName($joinColumn, $class, $this->platform); @@ -396,8 +395,8 @@ class ManyToManyPersister extends AbstractCollectionPersister $columns[] = $this->quoteStrategy->getJoinColumnName($joinColumn, $class, $this->platform); } - return 'DELETE FROM ' . $tableName - . ' WHERE ' . implode(' = ? AND ', $columns) . ' = ?'; + return 'DELETE FROM ' . $this->quoteStrategy->getJoinTableName($mapping, $class, $this->platform) + . ' WHERE ' . implode(' = ? AND ', $columns) . ' = ?'; } /** From a8dcc2acf3f10dd5a56170867267928ce2f4564b Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Tue, 13 Jan 2015 14:05:27 +0100 Subject: [PATCH 06/15] #1246 DDC-3487 - removing possible undefined value path for `$newValId` for clarity --- lib/Doctrine/ORM/Persisters/BasicEntityPersister.php | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php index 1e6374319..e289d540d 100644 --- a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php +++ b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php @@ -655,6 +655,8 @@ class BasicEntityPersister implements EntityPersister } } + $newValId = null; + if ($newVal !== null) { $newValId = $uow->getEntityIdentifier($newVal); } @@ -667,14 +669,11 @@ class BasicEntityPersister implements EntityPersister $targetColumn = $joinColumn['referencedColumnName']; $quotedColumn = $this->quoteStrategy->getJoinColumnName($joinColumn, $this->class, $this->platform); - $this->quotedColumns[$sourceColumn] = $quotedColumn; - $this->columnTypes[$sourceColumn] = $targetClass->getTypeOfColumn($targetColumn); - - $value = ($newVal !== null) + $this->quotedColumns[$sourceColumn] = $quotedColumn; + $this->columnTypes[$sourceColumn] = $targetClass->getTypeOfColumn($targetColumn); + $result[$owningTable][$sourceColumn] = $newValId ? $newValId[$targetClass->getFieldForColumn($targetColumn)] : null; - - $result[$owningTable][$sourceColumn] = $value; } } From 77234d6aec943ee7825331d6f184b22ecacc741d Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Tue, 13 Jan 2015 14:30:51 +0100 Subject: [PATCH 07/15] #1246 DDC-3487 - removing unused assignment --- lib/Doctrine/ORM/Persisters/ManyToManyPersister.php | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php b/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php index c4d495104..de995c529 100644 --- a/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php +++ b/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php @@ -229,10 +229,12 @@ class ManyToManyPersister extends AbstractCollectionPersister . implode(' AND ', $onConditions) . ' WHERE ' . implode(' AND ', $whereClauses); - $stmt = $this->conn->executeQuery($sql, $params); - $hydrator = $this->em->newHydrator(Query::HYDRATE_OBJECT); + $stmt = $this->conn->executeQuery($sql, $params); - return $hydrator->hydrateAll($stmt, $rsm); + return $this + ->em + ->newHydrator(Query::HYDRATE_OBJECT) + ->hydrateAll($stmt, $rsm); } /** From 5942b6c3028678047e571b9e1de78cc76aea7920 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Tue, 13 Jan 2015 14:31:22 +0100 Subject: [PATCH 08/15] #1246 DDC-3487 - re-aligning SQL string concatenation for readability --- lib/Doctrine/ORM/Persisters/ManyToManyPersister.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php b/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php index de995c529..b40cd488a 100644 --- a/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php +++ b/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php @@ -224,7 +224,8 @@ class ManyToManyPersister extends AbstractCollectionPersister $rsm = new Query\ResultSetMappingBuilder($this->em); $rsm->addRootEntityFromClassMetadata($mapping['targetEntity'], 'te'); - $sql = 'SELECT ' . $rsm->generateSelectClause() . ' FROM ' . $tableName . ' te' + $sql = 'SELECT ' . $rsm->generateSelectClause() + . ' FROM ' . $tableName . ' te' . ' JOIN ' . $joinTable . ' t ON' . implode(' AND ', $onConditions) . ' WHERE ' . implode(' AND ', $whereClauses); From b99f4461bef88b0a3a48bbe088af7a5aa444eb98 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Tue, 13 Jan 2015 14:37:32 +0100 Subject: [PATCH 09/15] #1246 DDC-3487 - re-aligning SQL string concatenation for readability, fixed docblock return value hint --- lib/Doctrine/ORM/Persisters/ManyToManyPersister.php | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php b/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php index b40cd488a..eff275359 100644 --- a/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php +++ b/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php @@ -249,7 +249,9 @@ class ManyToManyPersister extends AbstractCollectionPersister * * @param array $mapping Array containing mapping information. * - * @return string The SQL query part to add to a query. + * @return string[] ordered tuple: + * - JOIN condition to add to the SQL + * - WHERE condition to add to the SQL */ public function getFilterSql($mapping) { @@ -262,11 +264,9 @@ class ManyToManyPersister extends AbstractCollectionPersister } // A join is needed if there is filtering on the target entity - $tableName = $this->quoteStrategy->getTableName($rootClass, $this->platform); - $joinSql = ' JOIN ' . $tableName . ' te' . ' ON'; - $onConditions = $this->getOnConditionSQL($mapping); - - $joinSql .= implode(' AND ', $onConditions); + $tableName = $this->quoteStrategy->getTableName($rootClass, $this->platform); + $joinSql = ' JOIN ' . $tableName . ' te' + . ' ON' . implode(' AND ', $this->getOnConditionSQL($mapping)); return array($joinSql, $filterSql); } From 17a865ec7f71a6a6cf2373eb5a62f03014e7c175 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Tue, 13 Jan 2015 14:39:05 +0100 Subject: [PATCH 10/15] #1246 DDC-3487 - correcting docblock (static introspection fix) --- lib/Doctrine/ORM/Query/FilterCollection.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/Doctrine/ORM/Query/FilterCollection.php b/lib/Doctrine/ORM/Query/FilterCollection.php index d1d340c65..3eb6b1a48 100644 --- a/lib/Doctrine/ORM/Query/FilterCollection.php +++ b/lib/Doctrine/ORM/Query/FilterCollection.php @@ -57,7 +57,7 @@ class FilterCollection /** * Instances of enabled filters. * - * @var array + * @var \Doctrine\ORM\Query\Filter\SQLFilter[] */ private $enabledFilters = array(); @@ -85,7 +85,7 @@ class FilterCollection /** * Gets all the enabled filters. * - * @return array The enabled filters. + * @return \Doctrine\ORM\Query\Filter\SQLFilter[] The enabled filters. */ public function getEnabledFilters() { From c4366124c7e600800d48ce47b3a3b856b33c489a Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Tue, 13 Jan 2015 14:45:13 +0100 Subject: [PATCH 11/15] #1246 DDC-3487 - removed unused assignment, direct return instead --- lib/Doctrine/ORM/Persisters/ManyToManyPersister.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php b/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php index eff275359..17c98b90b 100644 --- a/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php +++ b/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php @@ -289,8 +289,9 @@ class ManyToManyPersister extends AbstractCollectionPersister } } - $sql = implode(' AND ', $filterClauses); - return $sql ? '(' . $sql . ')' : ''; + return $filterClauses + ? '(' . implode(' AND ', $filterClauses) . ')' + : ''; } /** From 97d1d5343e254ee946fd7a7ae6de858c96d3e92f Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Tue, 13 Jan 2015 14:46:34 +0100 Subject: [PATCH 12/15] #1246 DDC-3487 - removed unused assignment, making `$association` variable overwrite more obvious --- lib/Doctrine/ORM/Persisters/ManyToManyPersister.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php b/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php index 17c98b90b..746a350ac 100644 --- a/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php +++ b/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php @@ -306,8 +306,10 @@ class ManyToManyPersister extends AbstractCollectionPersister $association = $mapping; if ( ! $mapping['isOwningSide']) { - $class = $this->em->getClassMetadata($mapping['targetEntity']); - $association = $class->associationMappings[$mapping['mappedBy']]; + $association = $this + ->em + ->getClassMetadata($mapping['targetEntity']) + ->associationMappings[$mapping['mappedBy']]; } $targetClass = $this->em->getClassMetadata($mapping['targetEntity']); From 7f71cbc8c7971476db9c0d896086009f5299a53b Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Tue, 13 Jan 2015 14:54:00 +0100 Subject: [PATCH 13/15] #1246 DDC-3487 - removed unused assignment, minor alignment fixes --- .../ORM/Persisters/ManyToManyPersister.php | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php b/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php index 746a350ac..1cb9f8f74 100644 --- a/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php +++ b/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php @@ -347,7 +347,7 @@ class ManyToManyPersister extends AbstractCollectionPersister } return 'DELETE FROM ' . $joinTable - . ' WHERE ' . implode(' = ? AND ', $columns) . ' = ?'; + . ' WHERE ' . implode(' = ? AND ', $columns) . ' = ?'; } /** @@ -430,10 +430,9 @@ class ManyToManyPersister extends AbstractCollectionPersister */ protected function getInsertRowSQL(PersistentCollection $coll) { - $columns = array(); - $mapping = $coll->getMapping(); - $class = $this->em->getClassMetadata(get_class($coll->getOwner())); - $joinTable = $this->quoteStrategy->getJoinTableName($mapping, $class, $this->platform); + $columns = array(); + $mapping = $coll->getMapping(); + $class = $this->em->getClassMetadata(get_class($coll->getOwner())); foreach ($mapping['joinTable']['joinColumns'] as $joinColumn) { $columns[] = $this->quoteStrategy->getJoinColumnName($joinColumn, $class, $this->platform); @@ -443,8 +442,10 @@ class ManyToManyPersister extends AbstractCollectionPersister $columns[] = $this->quoteStrategy->getJoinColumnName($joinColumn, $class, $this->platform); } - return 'INSERT INTO ' . $joinTable . ' (' . implode(', ', $columns) . ')' - . ' VALUES (' . implode(', ', array_fill(0, count($columns), '?')) . ')'; + return 'INSERT INTO ' . $this->quoteStrategy->getJoinTableName($mapping, $class, $this->platform) + . ' (' . implode(', ', $columns) . ')' + . ' VALUES' + . ' (' . implode(', ', array_fill(0, count($columns), '?')) . ')'; } /** From 278b8bfa08e2671b558ee9cde829e4f1ba767770 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Tue, 13 Jan 2015 14:58:56 +0100 Subject: [PATCH 14/15] #1246 DDC-3487 - minor alignment fixes --- lib/Doctrine/ORM/Persisters/OneToManyPersister.php | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/lib/Doctrine/ORM/Persisters/OneToManyPersister.php b/lib/Doctrine/ORM/Persisters/OneToManyPersister.php index 239477edb..a890f19ac 100644 --- a/lib/Doctrine/ORM/Persisters/OneToManyPersister.php +++ b/lib/Doctrine/ORM/Persisters/OneToManyPersister.php @@ -67,7 +67,17 @@ class OneToManyPersister extends AbstractCollectionPersister throw new \BadMethodCallException("Selecting a collection by index is only supported on indexed collections."); } - return $persister->load(array($mapping['mappedBy'] => $coll->getOwner(), $mapping['indexBy'] => $index), null, null, array(), null, 1); + return $persister->load( + array( + $mapping['mappedBy'] => $coll->getOwner(), + $mapping['indexBy'] => $index + ), + null, + null, + array(), + null, + 1 + ); } /** From 35dd7f8e2b57daf4f0e7bb4dfc60f2c324ca5c69 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Tue, 13 Jan 2015 15:03:04 +0100 Subject: [PATCH 15/15] #1246 DDC-3487 - docblock args fixes --- lib/Doctrine/ORM/Persisters/OneToManyPersister.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Doctrine/ORM/Persisters/OneToManyPersister.php b/lib/Doctrine/ORM/Persisters/OneToManyPersister.php index a890f19ac..0e3f05642 100644 --- a/lib/Doctrine/ORM/Persisters/OneToManyPersister.php +++ b/lib/Doctrine/ORM/Persisters/OneToManyPersister.php @@ -172,7 +172,7 @@ class OneToManyPersister extends AbstractCollectionPersister /** * Check if entity is in a valid state for operations. * - * @param $entity + * @param object $entity * * @return bool */