From 2eb7dedf4fd36da91f09bdbf10c9d5acaf2a881a Mon Sep 17 00:00:00 2001 From: Asmir Mustafic Date: Sun, 18 Jan 2015 23:18:41 +0100 Subject: [PATCH 01/14] Refactored IdentifierFlattener --- lib/Doctrine/ORM/UnitOfWork.php | 26 +++++++------------ .../ORM/Utility/IdentifierFlattener.php | 24 +++++++++-------- .../ORM/Utility/IdentifierFlattenerTest.php | 11 ++++---- 3 files changed, 29 insertions(+), 32 deletions(-) diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index 8f82aa96f..0294b9f82 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -49,6 +49,7 @@ use Doctrine\ORM\Persisters\Entity\JoinedSubclassPersister; use Doctrine\ORM\Persisters\Collection\OneToManyPersister; use Doctrine\ORM\Persisters\Collection\ManyToManyPersister; use Doctrine\ORM\Utility\IdentifierFlattener; +use Doctrine\ORM\Cache\AssociationCacheEntry; /** * The UnitOfWork is responsible for tracking changes to objects during an @@ -2490,22 +2491,7 @@ class UnitOfWork implements PropertyChangedListener $class = $this->em->getClassMetadata($className); //$isReadOnly = isset($hints[Query::HINT_READ_ONLY]); - if ($class->isIdentifierComposite) { - $id = array(); - - foreach ($class->identifier as $fieldName) { - $id[$fieldName] = isset($class->associationMappings[$fieldName]) - ? $data[$class->associationMappings[$fieldName]['joinColumns'][0]['name']] - : $data[$fieldName]; - } - } else { - $id = isset($class->associationMappings[$class->identifier[0]]) - ? $data[$class->associationMappings[$class->identifier[0]]['joinColumns'][0]['name']] - : $data[$class->identifier[0]]; - - $id = array($class->identifier[0] => $id); - } - + $id = $this->identifierFlattener->flattenIdentifier($class, $data); $idHash = implode(' ', $id); if (isset($this->identityMap[$class->rootEntityName][$idHash])) { @@ -2639,10 +2625,18 @@ class UnitOfWork implements PropertyChangedListener if ($joinColumnValue !== null) { if ($targetClass->containsForeignIdentifier) { + if ($joinColumnValue instanceof AssociationCacheEntry) { + $joinColumnValue = implode(' ', $joinColumnValue->identifier); + } + $associatedId[$targetClass->getFieldForColumn($targetColumn)] = $joinColumnValue; } else { $associatedId[$targetClass->fieldNames[$targetColumn]] = $joinColumnValue; } + } elseif ($targetClass->containsForeignIdentifier && in_array($targetClass->getFieldForColumn($targetColumn), $targetClass->identifier, true)) { + // the missing key is part of target's entity primary key + $associatedId = array(); + break; } } diff --git a/lib/Doctrine/ORM/Utility/IdentifierFlattener.php b/lib/Doctrine/ORM/Utility/IdentifierFlattener.php index 166e0acb6..fbad711e3 100644 --- a/lib/Doctrine/ORM/Utility/IdentifierFlattener.php +++ b/lib/Doctrine/ORM/Utility/IdentifierFlattener.php @@ -71,22 +71,24 @@ final class IdentifierFlattener { $flatId = array(); - foreach ($id as $idField => $idValue) { - if (isset($class->associationMappings[$idField]) && is_object($idValue)) { - /* @var $targetClassMetadata ClassMetadata */ + foreach ($class->identifier as $field) { + if (isset($class->associationMappings[$field]) && isset($id[$field]) && is_object($id[$field])) { $targetClassMetadata = $this->metadataFactory->getMetadataFor( - $class->associationMappings[$idField]['targetEntity'] + $class->associationMappings[$field]['targetEntity'] ); - - $associatedId = $this->unitOfWork->isInIdentityMap($idValue) - ? $this->unitOfWork->getEntityIdentifier($idValue) - : $targetClassMetadata->getIdentifierValues($idValue); - - $flatId[$idField] = $associatedId[$targetClassMetadata->identifier[0]]; + $associatedId = $this->flattenIdentifier($targetClassMetadata, $this->unitOfWork->getEntityIdentifier($id[$field])); + $flatId[$field] = implode(' ', $associatedId); + } elseif (isset($class->associationMappings[$field])) { + $associatedId = array(); + foreach ($class->associationMappings[$field]['joinColumns'] as $joinColumn) { + $associatedId[] = $id[$joinColumn['name']]; + } + $flatId[$field] = implode(' ', $associatedId); } else { - $flatId[$idField] = $idValue; + $flatId[$field] = $id[$field]; } } + return $flatId; } diff --git a/tests/Doctrine/Tests/ORM/Utility/IdentifierFlattenerTest.php b/tests/Doctrine/Tests/ORM/Utility/IdentifierFlattenerTest.php index 63536c886..dad6ce4b3 100644 --- a/tests/Doctrine/Tests/ORM/Utility/IdentifierFlattenerTest.php +++ b/tests/Doctrine/Tests/ORM/Utility/IdentifierFlattenerTest.php @@ -9,6 +9,7 @@ use Doctrine\Tests\Models\Cache\City; use Doctrine\Tests\Models\Cache\Flight; use Doctrine\ORM\ORMException; use Doctrine\ORM\Utility\IdentifierFlattener; + /** * Test the IdentifierFlattener utility class * @@ -87,7 +88,7 @@ class IdentifierFlattenerTest extends OrmFunctionalTestCase $this->assertArrayHasKey('secondEntity', $flatIds, 'It should be called secondEntity'); - $this->assertSame($id['secondEntity']->id, $flatIds['secondEntity']); + $this->assertEquals($id['secondEntity']->id, $flatIds['secondEntity']); } /** @@ -115,8 +116,8 @@ class IdentifierFlattenerTest extends OrmFunctionalTestCase $this->assertArrayHasKey('leavingFrom', $id); $this->assertArrayHasKey('goingTo', $id); - $this->assertSame($leeds, $id['leavingFrom']); - $this->assertSame($london, $id['goingTo']); + $this->assertEquals($leeds, $id['leavingFrom']); + $this->assertEquals($london, $id['goingTo']); $flatIds = $this->identifierFlattener->flattenIdentifier($class, $id); @@ -125,7 +126,7 @@ class IdentifierFlattenerTest extends OrmFunctionalTestCase $this->assertArrayHasKey('leavingFrom', $flatIds); $this->assertArrayHasKey('goingTo', $flatIds); - $this->assertSame($id['leavingFrom']->getId(), $flatIds['leavingFrom']); - $this->assertSame($id['goingTo']->getId(), $flatIds['goingTo']); + $this->assertEquals($id['leavingFrom']->getId(), $flatIds['leavingFrom']); + $this->assertEquals($id['goingTo']->getId(), $flatIds['goingTo']); } } From 5e29bbd41fe1c5c265a8c87c536892217cb2de33 Mon Sep 17 00:00:00 2001 From: Asmir Mustafic Date: Sun, 18 Jan 2015 23:20:47 +0100 Subject: [PATCH 02/14] Improved composite primary key support --- lib/Doctrine/ORM/ORMException.php | 8 + .../Entity/BasicEntityPersister.php | 233 ++++++++++++------ 2 files changed, 170 insertions(+), 71 deletions(-) diff --git a/lib/Doctrine/ORM/ORMException.php b/lib/Doctrine/ORM/ORMException.php index 96e342eb2..d4a729d7e 100644 --- a/lib/Doctrine/ORM/ORMException.php +++ b/lib/Doctrine/ORM/ORMException.php @@ -317,4 +317,12 @@ class ORMException extends Exception { return new self("It is not allowed to overwrite internal function '$functionName' in the DQL parser through user-defined functions."); } + + /** + * @return ORMException + */ + public static function cantUseInOperatorOnCompositeKeys() + { + return new self("Can't use IN operator on entities that have composite keys."); + } } diff --git a/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php b/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php index 4bc4a2fd0..715a0696d 100644 --- a/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php +++ b/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php @@ -335,7 +335,11 @@ class BasicEntityPersister implements EntityPersister . ' FROM ' . $tableName . ' WHERE ' . implode(' = ? AND ', $identifier) . ' = ?'; - $flatId = $this->identifierFlattener->flattenIdentifier($versionedClass, (array) $id); + if (!is_array($id)) { + $id = array($this->class->identifier[0] => $id); + } + + $flatId = $this->identifierFlattener->flattenIdentifier($versionedClass, $id); $value = $this->conn->fetchColumn($sql, array_values($flatId)); @@ -852,12 +856,12 @@ class BasicEntityPersister implements EntityPersister list($params, $types) = $valueVisitor->getParamsAndTypes(); foreach ($params as $param) { - $sqlParams[] = PersisterHelper::getIdentifierValues($param, $this->em); + $sqlParams = array_merge($sqlParams, $this->getValues($param)); } foreach ($types as $type) { - list($field, $value) = $type; - $sqlTypes[] = $this->getType($field, $value, $this->class); + list ($field, $value) = $type; + $sqlTypes = array_merge($sqlTypes, $this->getTypes($field, $value, $this->class)); } return array($sqlParams, $sqlTypes); @@ -1565,40 +1569,61 @@ class BasicEntityPersister implements EntityPersister */ public function getSelectConditionStatementSQL($field, $value, $assoc = null, $comparison = null) { - $placeholder = '?'; - $condition = $this->getSelectConditionStatementColumnSQL($field, $assoc); + $selectedColumns = array(); + $columns = $this->getSelectConditionStatementColumnSQL($field, $assoc); - if (isset($this->class->fieldMappings[$field]['requireSQLConversion'])) { - $placeholder = Type::getType($this->class->getTypeOfField($field))->convertToDatabaseValueSQL($placeholder, $this->platform); + if (count($columns) > 1 && $comparison === Comparison::IN) { + /* + * @todo try to support multi-column IN expressions. + * Example: (col1, col2) IN (('val1A', 'val2A'), ('val1B', 'val2B')) + */ + throw ORMException::cantUseInOperatorOnCompositeKeys(); } - if ($comparison !== null) { + foreach ($columns as $column) { + $placeholder = '?'; - // special case null value handling - if (($comparison === Comparison::EQ || $comparison === Comparison::IS) && $value === null) { - return $condition . ' IS NULL'; - } else if ($comparison === Comparison::NEQ && $value === null) { - return $condition . ' IS NOT NULL'; + if (isset($this->class->fieldMappings[$field]['requireSQLConversion'])) { + $placeholder = Type::getType($this->class->getTypeOfField($field))->convertToDatabaseValueSQL($placeholder, $this->platform); } - return $condition . ' ' . sprintf(self::$comparisonMap[$comparison], $placeholder); - } + if (null !== $comparison) { + // special case null value handling + if (($comparison === Comparison::EQ || $comparison === Comparison::IS) && null ===$value) { + $selectedColumns[] = $column . ' IS NULL'; + continue; + } - if (is_array($value)) { - $in = sprintf('%s IN (%s)' , $condition, $placeholder); + if ($comparison === Comparison::NEQ && null === $value) { + $selectedColumns[] = $column . ' IS NOT NULL'; + continue; + } - if (false !== array_search(null, $value, true)) { - return sprintf('(%s OR %s IS NULL)' , $in, $condition); + $selectedColumns[] = $column . ' ' . sprintf(self::$comparisonMap[$comparison], $placeholder); + continue; } - return $in; + if (is_array($value)) { + $in = sprintf('%s IN (%s)', $column, $placeholder); + + if (false !== array_search(null, $value, true)) { + $selectedColumns[] = sprintf('(%s OR %s IS NULL)', $in, $column); + continue; + } + + $selectedColumns[] = $in; + continue; + } + + if (null === $value) { + $selectedColumns[] = sprintf('%s IS NULL', $column); + continue; + } + + $selectedColumns[] = sprintf('%s = %s', $column, $placeholder); } - if ($value === null) { - return sprintf('%s IS NULL' , $condition); - } - - return sprintf('%s = %s' , $condition, $placeholder); + return implode(' AND ', $selectedColumns); } /** @@ -1607,7 +1632,7 @@ class BasicEntityPersister implements EntityPersister * @param string $field * @param array|null $assoc * - * @return string + * @return array * * @throws \Doctrine\ORM\ORMException */ @@ -1618,36 +1643,38 @@ class BasicEntityPersister implements EntityPersister ? $this->class->fieldMappings[$field]['inherited'] : $this->class->name; - return $this->getSQLTableAlias($className) . '.' . $this->quoteStrategy->getColumnName($field, $this->class, $this->platform); + return array($this->getSQLTableAlias($className) . '.' . $this->quoteStrategy->getColumnName($field, $this->class, $this->platform)); } if (isset($this->class->associationMappings[$field])) { $association = $this->class->associationMappings[$field]; - // Many-To-Many requires join table check for joinColumn + $columns = array(); if ($association['type'] === ClassMetadata::MANY_TO_MANY) { if ( ! $association['isOwningSide']) { $association = $assoc; } $joinTableName = $this->quoteStrategy->getJoinTableName($association, $this->class, $this->platform); - $joinColumn = $assoc['isOwningSide'] - ? $association['joinTable']['joinColumns'][0] - : $association['joinTable']['inverseJoinColumns'][0]; + foreach ($association['joinTable']['joinColumns'] as $joinColumn) { + $columns[] = $joinTableName . '.' . $this->quoteStrategy->getJoinColumnName($joinColumn, $this->class, $this->platform); + } - return $joinTableName . '.' . $this->quoteStrategy->getJoinColumnName($joinColumn, $this->class, $this->platform); + } else { + + if ( ! $association['isOwningSide']) { + throw ORMException::invalidFindByInverseAssociation($this->class->name, $field); + } + + $className = (isset($association['inherited'])) + ? $association['inherited'] + : $this->class->name; + + foreach ($association['joinColumns'] as $joinColumn) { + $columns[] = $this->getSQLTableAlias($className) . '.' . $this->quoteStrategy->getJoinColumnName($joinColumn, $this->class, $this->platform); + } } - - if ( ! $association['isOwningSide']) { - throw ORMException::invalidFindByInverseAssociation($this->class->name, $field); - } - - $joinColumn = $association['joinColumns'][0]; - $className = (isset($association['inherited'])) - ? $association['inherited'] - : $this->class->name; - - return $this->getSQLTableAlias($className) . '.' . $this->quoteStrategy->getJoinColumnName($joinColumn, $this->class, $this->platform); + return $columns; } if ($assoc !== null && strpos($field, " ") === false && strpos($field, "(") === false) { @@ -1655,12 +1682,13 @@ class BasicEntityPersister implements EntityPersister // therefore checking for spaces and function calls which are not allowed. // found a join column condition, not really a "field" - return $field; + return array($field); } throw ORMException::unrecognizedField($field); } + /** * Gets the conditional SQL fragment used in the WHERE clause when selecting * entities in this persister. @@ -1778,8 +1806,8 @@ class BasicEntityPersister implements EntityPersister continue; // skip null values. } - $types[] = $this->getType($field, $value, $this->class); - $params[] = $this->getValue($value); + $types = array_merge($types, $this->getTypes($field, $value, $this->class)); + $params = array_merge($params, $this->getValues($value)); } return array($params, $types); @@ -1807,13 +1835,98 @@ class BasicEntityPersister implements EntityPersister continue; // skip null values. } - $types[] = $this->getType($criterion['field'], $criterion['value'], $criterion['class']); - $params[] = PersisterHelper::getIdentifierValues($criterion['value'], $this->em); + $types = array_merge($types, $this->getTypes($criterion['field'], $criterion['value'], $criterion['class'])); + $params = array_merge($params, $this->getValues($criterion['value'])); } return array($params, $types); } + /** + * Infers field types to be used by parameter type casting. + * + * @param string $field + * @param mixed $value + * + * @return array + * + * @throws \Doctrine\ORM\Query\QueryException + */ + private function getTypes($field, $value, ClassMetadata $class) + { + $types = array(); + switch (true) { + case (isset($this->class->fieldMappings[$field])): + $types[] = $this->class->fieldMappings[$field]['type']; + break; + + case (isset($this->class->associationMappings[$field])): + $assoc = $this->class->associationMappings[$field]; + + $targetPersister = $this->em->getUnitOfWork()->getEntityPersister($assoc['targetEntity']); + if ($assoc['type'] === ClassMetadata::MANY_TO_MANY) { + + if(!$assoc['isOwningSide']){ + $class = $this->em->getClassMetadata($assoc['targetEntity']); + $assoc = $class->associationMappings[$assoc['mappedBy']]; + } + + $parameters = $targetPersister->expandParameters($assoc['relationToSourceKeyColumns']); + } else { + $parameters = $targetPersister->expandParameters($assoc['isOwningSide']?$assoc['targetToSourceKeyColumns']:$assoc['sourceToTargetKeyColumns']); + } + $types = array_merge($types, $parameters[1]); + break; + + default: + $types[] = null; + break; + } + + if (is_array($value)) { + $types = array_map(function ($type) { + $type = Type::getType($type)->getBindingType(); + return $type + Connection::ARRAY_PARAM_OFFSET; + }, $types); + } + + return $types; + } + + /** + * Retrieves the parameters that identifies a value. + * + * @param mixed $value + * + * @return array + */ + private function getValues($value) + { + if (is_array($value)) { + $newValue = array(); + + foreach ($value as $itemValue) { + $newValue = array_merge($newValue, $this->getValues($itemValue)); + } + + return array($newValue); + } + + if (is_object($value) && $this->em->getMetadataFactory()->hasMetadataFor(ClassUtils::getClass($value))) { + $class = $this->em->getClassMetadata(get_class($value)); + if ($class->isIdentifierComposite) { + $newValue = array(); + foreach ($class->getIdentifierValues($value) as $innerValue) { + $newValue = array_merge($newValue, $this->getValues($innerValue)); + } + + return $newValue; + } + } + + return array($this->getIndividualValue($value)); + } + /** * Infers field type to be used by parameter type casting. * @@ -1837,28 +1950,6 @@ class BasicEntityPersister implements EntityPersister return $type; } - /** - * Retrieves parameter value. - * - * @param mixed $value - * - * @return mixed - */ - private function getValue($value) - { - if ( ! is_array($value)) { - return $this->getIndividualValue($value); - } - - $newValue = array(); - - foreach ($value as $itemValue) { - $newValue[] = $this->getIndividualValue($itemValue); - } - - return $newValue; - } - /** * Retrieves an individual parameter value. * From 7222991b13562c76336a56e9bcbfcee288fdc6df Mon Sep 17 00:00:00 2001 From: Asmir Mustafic Date: Sun, 18 Jan 2015 23:21:57 +0100 Subject: [PATCH 03/14] Tested composite primary key support --- .../Doctrine/Tests/Models/GeoNames/Admin1.php | 44 ++++++++++++ .../Models/GeoNames/Admin1AlternateName.php | 41 ++++++++++++ tests/Doctrine/Tests/Models/GeoNames/City.php | 47 +++++++++++++ .../Tests/Models/GeoNames/Country.php | 29 ++++++++ ...ompositePrimaryKeyWithAssociationsTest.php | 60 +++++++++++++++++ ...tyPersisterCompositeTypeParametersTest.php | 67 +++++++++++++++++++ ...sicEntityPersisterCompositeTypeSqlTest.php | 62 +++++++++++++++++ .../Doctrine/Tests/OrmFunctionalTestCase.php | 15 ++++- 8 files changed, 364 insertions(+), 1 deletion(-) create mode 100644 tests/Doctrine/Tests/Models/GeoNames/Admin1.php create mode 100644 tests/Doctrine/Tests/Models/GeoNames/Admin1AlternateName.php create mode 100644 tests/Doctrine/Tests/Models/GeoNames/City.php create mode 100644 tests/Doctrine/Tests/Models/GeoNames/Country.php create mode 100644 tests/Doctrine/Tests/ORM/Functional/CompositePrimaryKeyWithAssociationsTest.php create mode 100644 tests/Doctrine/Tests/ORM/Persisters/BasicEntityPersisterCompositeTypeParametersTest.php create mode 100644 tests/Doctrine/Tests/ORM/Persisters/BasicEntityPersisterCompositeTypeSqlTest.php diff --git a/tests/Doctrine/Tests/Models/GeoNames/Admin1.php b/tests/Doctrine/Tests/Models/GeoNames/Admin1.php new file mode 100644 index 000000000..15a0147ed --- /dev/null +++ b/tests/Doctrine/Tests/Models/GeoNames/Admin1.php @@ -0,0 +1,44 @@ +id = $id; + $this->name = $name; + $this->country = $country; + } +} diff --git a/tests/Doctrine/Tests/Models/GeoNames/Admin1AlternateName.php b/tests/Doctrine/Tests/Models/GeoNames/Admin1AlternateName.php new file mode 100644 index 000000000..49d9939bd --- /dev/null +++ b/tests/Doctrine/Tests/Models/GeoNames/Admin1AlternateName.php @@ -0,0 +1,41 @@ +id = $id; + $this->name = $name; + $this->admin1 = $admin1; + } +} diff --git a/tests/Doctrine/Tests/Models/GeoNames/City.php b/tests/Doctrine/Tests/Models/GeoNames/City.php new file mode 100644 index 000000000..411f4db91 --- /dev/null +++ b/tests/Doctrine/Tests/Models/GeoNames/City.php @@ -0,0 +1,47 @@ +id = $id; + $this->name = $name; + } +} diff --git a/tests/Doctrine/Tests/Models/GeoNames/Country.php b/tests/Doctrine/Tests/Models/GeoNames/Country.php new file mode 100644 index 000000000..ebde305f8 --- /dev/null +++ b/tests/Doctrine/Tests/Models/GeoNames/Country.php @@ -0,0 +1,29 @@ +id = $id; + $this->name = $name; + } +} diff --git a/tests/Doctrine/Tests/ORM/Functional/CompositePrimaryKeyWithAssociationsTest.php b/tests/Doctrine/Tests/ORM/Functional/CompositePrimaryKeyWithAssociationsTest.php new file mode 100644 index 000000000..6ded5bb49 --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Functional/CompositePrimaryKeyWithAssociationsTest.php @@ -0,0 +1,60 @@ +useModelSet('geonames'); + parent::setUp(); + + $it = new Country("IT", "Italy"); + + $this->_em->persist($it); + $this->_em->flush(); + + $admin1 = new Admin1(1, "Rome", $it); + + $this->_em->persist($admin1); + $this->_em->flush(); + + $name1 = new Admin1AlternateName(1, "Roma", $admin1); + $name2 = new Admin1AlternateName(2, "Rome", $admin1); + + $admin1->names[] = $name1; + $admin1->names[] = $name2; + + $this->_em->persist($admin1); + $this->_em->persist($name1); + $this->_em->persist($name2); + + $this->_em->flush(); + + $this->_em->clear(); + } + + public function testFindByAbleToGetCompositeEntitiesWithMixedTypeIdentifiers() + { + $admin1Repo = $this->_em->getRepository('Doctrine\Tests\Models\GeoNames\Admin1'); + $admin1NamesRepo = $this->_em->getRepository('Doctrine\Tests\Models\GeoNames\Admin1AlternateName'); + + $admin1Rome = $admin1Repo->findOneBy(array('country' => 'IT', 'id' => 1)); + + $names = $admin1NamesRepo->findBy(array('admin1' => $admin1Rome)); + $this->assertCount(2, $names); + + $name1 = $admin1NamesRepo->findOneBy(array('admin1' => $admin1Rome, 'id' => 1)); + $name2 = $admin1NamesRepo->findOneBy(array('admin1' => $admin1Rome, 'id' => 2)); + + $this->assertEquals(1, $name1->id); + $this->assertEquals("Roma", $name1->name); + + $this->assertEquals(2, $name2->id); + $this->assertEquals("Rome", $name2->name); + } +} diff --git a/tests/Doctrine/Tests/ORM/Persisters/BasicEntityPersisterCompositeTypeParametersTest.php b/tests/Doctrine/Tests/ORM/Persisters/BasicEntityPersisterCompositeTypeParametersTest.php new file mode 100644 index 000000000..76daadc3f --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Persisters/BasicEntityPersisterCompositeTypeParametersTest.php @@ -0,0 +1,67 @@ +_em = $this->_getTestEntityManager(); + + $this->_em->getClassMetadata('Doctrine\Tests\Models\GeoNames\Country'); + $this->_em->getClassMetadata('Doctrine\Tests\Models\GeoNames\Admin1'); + $this->_em->getClassMetadata('Doctrine\Tests\Models\GeoNames\Admin1AlternateName'); + + $this->_persister = new BasicEntityPersister($this->_em, $this->_em->getClassMetadata('Doctrine\Tests\Models\GeoNames\Admin1AlternateName')); + + } + + public function testExpandParametersWillExpandCompositeEntityKeys() + { + $country = new Country("IT", "Italy"); + $admin1 = new Admin1(10, "Rome", $country); + + + list ($values, $types) = $this->_persister->expandParameters(array( + 'admin1' => $admin1 + )); + + $this->assertEquals(array('integer', 'string'), $types); + $this->assertEquals(array(10, 'IT'), $values); + } + + public function testExpandCriteriaParametersWillExpandCompositeEntityKeys() + { + $country = new Country("IT", "Italy"); + $admin1 = new Admin1(10, "Rome", $country); + + $criteria = Criteria::create(); + + $criteria->andWhere(Criteria::expr()->eq("admin1", $admin1)); + + list ($values, $types) = $this->_persister->expandCriteriaParameters($criteria); + + $this->assertEquals(array('integer', 'string'), $types); + $this->assertEquals(array(10, 'IT'), $values); + } +} diff --git a/tests/Doctrine/Tests/ORM/Persisters/BasicEntityPersisterCompositeTypeSqlTest.php b/tests/Doctrine/Tests/ORM/Persisters/BasicEntityPersisterCompositeTypeSqlTest.php new file mode 100644 index 000000000..54b1a2351 --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Persisters/BasicEntityPersisterCompositeTypeSqlTest.php @@ -0,0 +1,62 @@ +_em = $this->_getTestEntityManager(); + + $this->_persister = new BasicEntityPersister($this->_em, $this->_em->getClassMetadata('Doctrine\Tests\Models\GeoNames\Admin1AlternateName')); + } + + public function testSelectConditionStatementEq() + { + $statement = $this->_persister->getSelectConditionStatementSQL('admin1', 1, array(), Comparison::EQ); + $this->assertEquals('t0.admin1 = ? AND t0.country = ?', $statement); + } + + public function testSelectConditionStatementEqNull() + { + $statement = $this->_persister->getSelectConditionStatementSQL('admin1', null, array(), Comparison::IS); + $this->assertEquals('t0.admin1 IS NULL AND t0.country IS NULL', $statement); + } + + public function testSelectConditionStatementNeqNull() + { + $statement = $this->_persister->getSelectConditionStatementSQL('admin1', null, array(), Comparison::NEQ); + $this->assertEquals('t0.admin1 IS NOT NULL AND t0.country IS NOT NULL', $statement); + } + + /** + * @expectedException Doctrine\ORM\ORMException + */ + public function testSelectConditionStatementIn() + { + $this->_persister->getSelectConditionStatementSQL('admin1', array(), array(), Comparison::IN); + } +} diff --git a/tests/Doctrine/Tests/OrmFunctionalTestCase.php b/tests/Doctrine/Tests/OrmFunctionalTestCase.php index fedeed734..7aaeae81a 100644 --- a/tests/Doctrine/Tests/OrmFunctionalTestCase.php +++ b/tests/Doctrine/Tests/OrmFunctionalTestCase.php @@ -252,6 +252,12 @@ abstract class OrmFunctionalTestCase extends OrmTestCase 'Doctrine\Tests\Models\ValueConversionType\InversedManyToManyExtraLazyEntity', 'Doctrine\Tests\Models\ValueConversionType\OwningManyToManyExtraLazyEntity' ), + 'geonames' => array( + 'Doctrine\Tests\Models\GeoNames\Country', + 'Doctrine\Tests\Models\GeoNames\Admin1', + 'Doctrine\Tests\Models\GeoNames\Admin1AlternateName', + 'Doctrine\Tests\Models\GeoNames\City' + ) ); /** @@ -483,6 +489,12 @@ abstract class OrmFunctionalTestCase extends OrmTestCase $conn->executeUpdate('DELETE FROM vct_owning_manytomany_extralazy'); $conn->executeUpdate('DELETE FROM vct_inversed_manytomany_extralazy'); } + if (isset($this->_usedModelSets['geonames'])) { + $conn->executeUpdate('DELETE FROM geonames_admin1_alternate_name'); + $conn->executeUpdate('DELETE FROM geonames_admin1'); + $conn->executeUpdate('DELETE FROM geonames_city'); + $conn->executeUpdate('DELETE FROM geonames_country'); + } $this->_em->clear(); } @@ -623,7 +635,8 @@ abstract class OrmFunctionalTestCase extends OrmTestCase } $config->setMetadataDriverImpl($config->newDefaultAnnotationDriver(array( - realpath(__DIR__ . '/Models/Cache') + realpath(__DIR__ . '/Models/Cache'), + realpath(__DIR__ . '/Models/GeoNames') ), true)); $conn = static::$_sharedConn; From 51b34919ba4437593009829898fa19e4613d0160 Mon Sep 17 00:00:00 2001 From: Asmir Mustafic Date: Sun, 18 Jan 2015 23:23:56 +0100 Subject: [PATCH 04/14] Second level cache check with composite primary keys --- .../Cache/Persister/Entity/AbstractEntityPersister.php | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/lib/Doctrine/ORM/Cache/Persister/Entity/AbstractEntityPersister.php b/lib/Doctrine/ORM/Cache/Persister/Entity/AbstractEntityPersister.php index eae4c8c33..eaead7cf7 100644 --- a/lib/Doctrine/ORM/Cache/Persister/Entity/AbstractEntityPersister.php +++ b/lib/Doctrine/ORM/Cache/Persister/Entity/AbstractEntityPersister.php @@ -27,6 +27,7 @@ use Doctrine\ORM\Cache\CollectionCacheKey; use Doctrine\ORM\Cache\TimestampCacheKey; use Doctrine\ORM\Cache\QueryCacheKey; use Doctrine\ORM\Cache\Persister\CachedPersister; +use Doctrine\ORM\Cache\CacheException; use Doctrine\ORM\Mapping\ClassMetadata; use Doctrine\ORM\PersistentCollection; use Doctrine\ORM\EntityManagerInterface; @@ -233,6 +234,14 @@ abstract class AbstractEntityPersister implements CachedEntityPersister $class = $this->metadataFactory->getMetadataFor($className); } + if ($class->containsForeignIdentifier) { + foreach ($class->associationMappings as $name => $assoc) { + if (!empty($assoc['id']) && !isset($assoc['cache'])) { + throw CacheException::nonCacheableEntityAssociation($class->name, $name); + } + } + } + $entry = $this->hydrator->buildCacheEntry($class, $key, $entity); $cached = $this->region->put($key, $entry); From 2e890362c5496d6bdbc51ed1ac9686787d0ab28b Mon Sep 17 00:00:00 2001 From: Asmir Mustafic Date: Sun, 18 Jan 2015 23:24:15 +0100 Subject: [PATCH 05/14] Tested second level cache with composite primary keys --- ...ompositePrimaryKeyWithAssociationsTest.php | 81 +++++++++++++++++++ 1 file changed, 81 insertions(+) create mode 100644 tests/Doctrine/Tests/ORM/Functional/SecondLevelCacheCompositePrimaryKeyWithAssociationsTest.php diff --git a/tests/Doctrine/Tests/ORM/Functional/SecondLevelCacheCompositePrimaryKeyWithAssociationsTest.php b/tests/Doctrine/Tests/ORM/Functional/SecondLevelCacheCompositePrimaryKeyWithAssociationsTest.php new file mode 100644 index 000000000..dc4f40db0 --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Functional/SecondLevelCacheCompositePrimaryKeyWithAssociationsTest.php @@ -0,0 +1,81 @@ +enableSecondLevelCache(); + $this->useModelSet('geonames'); + parent::setUp(); + + $this->cache = $this->_em->getCache(); + + $it = new Country("IT", "Italy"); + + $this->_em->persist($it); + $this->_em->flush(); + + $admin1 = new Admin1(1, "Rome", $it); + + $this->_em->persist($admin1); + $this->_em->flush(); + + $name1 = new Admin1AlternateName(1, "Roma", $admin1); + $name2 = new Admin1AlternateName(2, "Rome", $admin1); + + $admin1->names[] = $name1; + $admin1->names[] = $name2; + + $this->_em->persist($admin1); + $this->_em->persist($name1); + $this->_em->persist($name2); + + $this->_em->flush(); + $this->_em->clear(); + $this->evictRegions(); + + } + + public function testFindByReturnsCachedEntity() + { + $admin1Repo = $this->_em->getRepository('Doctrine\Tests\Models\GeoNames\Admin1'); + + $queries = $this->getCurrentQueryCount(); + + $admin1Rome = $admin1Repo->findOneBy(array('country' => 'IT', 'id' => 1)); + + $this->assertEquals("Italy", $admin1Rome->country->name); + $this->assertEquals(2, count($admin1Rome->names)); + $this->assertEquals($queries + 3, $this->getCurrentQueryCount()); + + $this->_em->clear(); + + $queries = $this->getCurrentQueryCount(); + + $admin1Rome = $admin1Repo->findOneBy(array('country' => 'IT', 'id' => 1)); + + $this->assertEquals("Italy", $admin1Rome->country->name); + $this->assertEquals(2, count($admin1Rome->names)); + $this->assertEquals($queries, $this->getCurrentQueryCount()); + } + + private function evictRegions() + { + $this->cache->evictQueryRegions(); + $this->cache->evictEntityRegions(); + $this->cache->evictCollectionRegions(); + } +} From 583811558239bd7f8f375e7c2c363628537b5625 Mon Sep 17 00:00:00 2001 From: Asmir Mustafic Date: Thu, 29 Jan 2015 00:43:32 +0100 Subject: [PATCH 06/14] Fixed type checking on to-many relations --- .../Entity/BasicEntityPersister.php | 53 ++++++------------- .../ORM/Utility/IdentifierFlattener.php | 6 ++- lib/Doctrine/ORM/Utility/PersisterHelper.php | 20 ++++--- 3 files changed, 29 insertions(+), 50 deletions(-) diff --git a/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php b/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php index 715a0696d..a7c5a8d27 100644 --- a/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php +++ b/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php @@ -1809,7 +1809,6 @@ class BasicEntityPersister implements EntityPersister $types = array_merge($types, $this->getTypes($field, $value, $this->class)); $params = array_merge($params, $this->getValues($value)); } - return array($params, $types); } @@ -1838,7 +1837,6 @@ class BasicEntityPersister implements EntityPersister $types = array_merge($types, $this->getTypes($criterion['field'], $criterion['value'], $criterion['class'])); $params = array_merge($params, $this->getValues($criterion['value'])); } - return array($params, $types); } @@ -1855,27 +1853,32 @@ class BasicEntityPersister implements EntityPersister private function getTypes($field, $value, ClassMetadata $class) { $types = array(); + switch (true) { - case (isset($this->class->fieldMappings[$field])): - $types[] = $this->class->fieldMappings[$field]['type']; + case (isset($class->fieldMappings[$field])): + $types = array_merge($types, PersisterHelper::getTypeOfField($field, $class, $this->em)); break; + case (isset($class->associationMappings[$field])): + $assoc = $class->associationMappings[$field]; - case (isset($this->class->associationMappings[$field])): - $assoc = $this->class->associationMappings[$field]; + $class = $this->em->getClassMetadata($assoc['targetEntity']); + if (!$assoc['isOwningSide']) { + $assoc = $class->associationMappings[$assoc['mappedBy']]; + $class = $this->em->getClassMetadata($assoc['targetEntity']); + } - $targetPersister = $this->em->getUnitOfWork()->getEntityPersister($assoc['targetEntity']); if ($assoc['type'] === ClassMetadata::MANY_TO_MANY) { - if(!$assoc['isOwningSide']){ - $class = $this->em->getClassMetadata($assoc['targetEntity']); - $assoc = $class->associationMappings[$assoc['mappedBy']]; + foreach ($assoc['relationToSourceKeyColumns'] as $field => $val){ + $types = array_merge($types, PersisterHelper::getTypeOfField($val, $class, $this->em)); } - $parameters = $targetPersister->expandParameters($assoc['relationToSourceKeyColumns']); } else { - $parameters = $targetPersister->expandParameters($assoc['isOwningSide']?$assoc['targetToSourceKeyColumns']:$assoc['sourceToTargetKeyColumns']); + + foreach ($assoc['targetToSourceKeyColumns'] as $field => $val){ + $types = array_merge($types, PersisterHelper::getTypeOfField($field, $class, $this->em)); + } } - $types = array_merge($types, $parameters[1]); break; default: @@ -1889,7 +1892,6 @@ class BasicEntityPersister implements EntityPersister return $type + Connection::ARRAY_PARAM_OFFSET; }, $types); } - return $types; } @@ -1927,29 +1929,6 @@ class BasicEntityPersister implements EntityPersister return array($this->getIndividualValue($value)); } - /** - * Infers field type to be used by parameter type casting. - * - * @param string $fieldName - * @param mixed $value - * @param ClassMetadata $class - * - * @return integer - * - * @throws \Doctrine\ORM\Query\QueryException - */ - private function getType($fieldName, $value, ClassMetadata $class) - { - $type = PersisterHelper::getTypeOfField($fieldName, $class, $this->em); - - if (is_array($value)) { - $type = Type::getType($type)->getBindingType(); - $type += Connection::ARRAY_PARAM_OFFSET; - } - - return $type; - } - /** * Retrieves an individual parameter value. * diff --git a/lib/Doctrine/ORM/Utility/IdentifierFlattener.php b/lib/Doctrine/ORM/Utility/IdentifierFlattener.php index fbad711e3..bb6e33464 100644 --- a/lib/Doctrine/ORM/Utility/IdentifierFlattener.php +++ b/lib/Doctrine/ORM/Utility/IdentifierFlattener.php @@ -71,8 +71,10 @@ final class IdentifierFlattener { $flatId = array(); - foreach ($class->identifier as $field) { - if (isset($class->associationMappings[$field]) && isset($id[$field]) && is_object($id[$field])) { + foreach ($class->identifier as $field) { + if (isset($class->associationMappings[$field]) && isset($id[$field]) && $id[$field] instanceof \Doctrine\ORM\Cache\AssociationCacheEntry) { + $flatId[$field] = implode(' ', $id[$field]->identifier); + } elseif (isset($class->associationMappings[$field]) && isset($id[$field]) && is_object($id[$field])) { $targetClassMetadata = $this->metadataFactory->getMetadataFor( $class->associationMappings[$field]['targetEntity'] ); diff --git a/lib/Doctrine/ORM/Utility/PersisterHelper.php b/lib/Doctrine/ORM/Utility/PersisterHelper.php index 6c3af11da..662e6c74a 100644 --- a/lib/Doctrine/ORM/Utility/PersisterHelper.php +++ b/lib/Doctrine/ORM/Utility/PersisterHelper.php @@ -40,18 +40,18 @@ class PersisterHelper * @param ClassMetadata $class * @param EntityManagerInterface $em * - * @return string|null + * @return array * * @throws QueryException */ public static function getTypeOfField($fieldName, ClassMetadata $class, EntityManagerInterface $em) { if (isset($class->fieldMappings[$fieldName])) { - return $class->fieldMappings[$fieldName]['type']; + return array($class->fieldMappings[$fieldName]['type']); } if ( ! isset($class->associationMappings[$fieldName])) { - return null; + return array(); } $assoc = $class->associationMappings[$fieldName]; @@ -60,20 +60,18 @@ class PersisterHelper return self::getTypeOfField($assoc['mappedBy'], $em->getClassMetadata($assoc['targetEntity']), $em); } - if (($assoc['type'] & ClassMetadata::MANY_TO_MANY) > 0) { + if ($assoc['type'] & ClassMetadata::MANY_TO_MANY) { $joinData = $assoc['joinTable']; } else { $joinData = $assoc; } - if (count($joinData['joinColumns']) > 1) { - throw QueryException::associationPathCompositeKeyNotSupported(); + $types = array(); + $targetClass = $em->getClassMetadata($assoc['targetEntity']); + foreach ($joinData['joinColumns'] as $joinColumn) { + $types[] = self::getTypeOfColumn($joinColumn['referencedColumnName'], $targetClass, $em); } - - $targetColumnName = $joinData['joinColumns'][0]['referencedColumnName']; - $targetClass = $em->getClassMetadata($assoc['targetEntity']); - - return self::getTypeOfColumn($targetColumnName, $targetClass, $em); + return $types; } /** From 7948b0c160adc78540eb86494b0ae6b5f8f48f14 Mon Sep 17 00:00:00 2001 From: Asmir Mustafic Date: Thu, 29 Jan 2015 22:16:25 +0100 Subject: [PATCH 07/14] Identity map check --- lib/Doctrine/ORM/Utility/IdentifierFlattener.php | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/lib/Doctrine/ORM/Utility/IdentifierFlattener.php b/lib/Doctrine/ORM/Utility/IdentifierFlattener.php index bb6e33464..9108c35c8 100644 --- a/lib/Doctrine/ORM/Utility/IdentifierFlattener.php +++ b/lib/Doctrine/ORM/Utility/IdentifierFlattener.php @@ -78,7 +78,13 @@ final class IdentifierFlattener $targetClassMetadata = $this->metadataFactory->getMetadataFor( $class->associationMappings[$field]['targetEntity'] ); - $associatedId = $this->flattenIdentifier($targetClassMetadata, $this->unitOfWork->getEntityIdentifier($id[$field])); + + if ($this->unitOfWork->isInIdentityMap($id[$field])) { + $associatedId = $this->flattenIdentifier($targetClassMetadata, $this->unitOfWork->getEntityIdentifier($id[$field])); + } else { + $associatedId = $this->flattenIdentifier($targetClassMetadata, $targetClassMetadata->getIdentifierValues($id[$field])); + } + $flatId[$field] = implode(' ', $associatedId); } elseif (isset($class->associationMappings[$field])) { $associatedId = array(); @@ -90,7 +96,6 @@ final class IdentifierFlattener $flatId[$field] = $id[$field]; } } - return $flatId; } From 4323d9ce4ca0c14dcf6691b5235c06c7cb169c66 Mon Sep 17 00:00:00 2001 From: Asmir Mustafic Date: Thu, 29 Jan 2015 22:36:07 +0100 Subject: [PATCH 08/14] Removed unused methods --- lib/Doctrine/ORM/Utility/PersisterHelper.php | 36 -------------------- 1 file changed, 36 deletions(-) diff --git a/lib/Doctrine/ORM/Utility/PersisterHelper.php b/lib/Doctrine/ORM/Utility/PersisterHelper.php index 662e6c74a..888d17d13 100644 --- a/lib/Doctrine/ORM/Utility/PersisterHelper.php +++ b/lib/Doctrine/ORM/Utility/PersisterHelper.php @@ -131,40 +131,4 @@ class PersisterHelper $class->getName() )); } - - /** - * @param mixed $value - * @param EntityManagerInterface $em - * - * @return mixed - */ - public static function getIdentifierValues($value, EntityManagerInterface $em) - { - if ( ! is_array($value)) { - return self::getIndividualValue($value, $em); - } - - $newValue = array(); - - foreach ($value as $fieldName => $fieldValue) { - $newValue[$fieldName] = self::getIndividualValue($fieldValue, $em); - } - - return $newValue; - } - - /** - * @param mixed $value - * @param EntityManagerInterface $em - * - * @return mixed - */ - private static function getIndividualValue($value, EntityManagerInterface $em) - { - if ( ! is_object($value) || ! $em->getMetadataFactory()->hasMetadataFor(ClassUtils::getClass($value))) { - return $value; - } - - return $em->getUnitOfWork()->getSingleIdentifierValue($value); - } } From 4d531d8855a7fe599ba1d1d2b8611260c02e264a Mon Sep 17 00:00:00 2001 From: Asmir Mustafic Date: Thu, 29 Jan 2015 22:48:34 +0100 Subject: [PATCH 09/14] Right type detection on to-many relations --- .../Entity/BasicEntityPersister.php | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php b/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php index a7c5a8d27..c5f56fc9f 100644 --- a/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php +++ b/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php @@ -1806,7 +1806,7 @@ class BasicEntityPersister implements EntityPersister continue; // skip null values. } - $types = array_merge($types, $this->getTypes($field, $value, $this->class)); + $types = array_merge($types, $this->getTypes($field, $value, $this->class)); $params = array_merge($params, $this->getValues($value)); } return array($params, $types); @@ -1860,24 +1860,17 @@ class BasicEntityPersister implements EntityPersister break; case (isset($class->associationMappings[$field])): $assoc = $class->associationMappings[$field]; - $class = $this->em->getClassMetadata($assoc['targetEntity']); if (!$assoc['isOwningSide']) { $assoc = $class->associationMappings[$assoc['mappedBy']]; $class = $this->em->getClassMetadata($assoc['targetEntity']); } - if ($assoc['type'] === ClassMetadata::MANY_TO_MANY) { - - foreach ($assoc['relationToSourceKeyColumns'] as $field => $val){ - $types = array_merge($types, PersisterHelper::getTypeOfField($val, $class, $this->em)); - } - - } else { - - foreach ($assoc['targetToSourceKeyColumns'] as $field => $val){ - $types = array_merge($types, PersisterHelper::getTypeOfField($field, $class, $this->em)); - } + $columns = $assoc['type'] === ClassMetadata::MANY_TO_MANY + ? $assoc['relationToTargetKeyColumns'] + : $assoc['sourceToTargetKeyColumns']; + foreach ($columns as $column){ + $types[] = PersisterHelper::getTypeOfColumn($column, $class, $this->em); } break; From 1630ec1ebdc2e5502bb351de6438cd70d7ceeef7 Mon Sep 17 00:00:00 2001 From: Asmir Mustafic Date: Fri, 30 Jan 2015 00:24:09 +0100 Subject: [PATCH 10/14] Fixed owing-inverse side search by criteria --- .../ORM/Persisters/Entity/BasicEntityPersister.php | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php b/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php index c5f56fc9f..9158f7b6b 100644 --- a/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php +++ b/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php @@ -1650,14 +1650,18 @@ class BasicEntityPersister implements EntityPersister $association = $this->class->associationMappings[$field]; // Many-To-Many requires join table check for joinColumn $columns = array(); + $class = $this->class; if ($association['type'] === ClassMetadata::MANY_TO_MANY) { if ( ! $association['isOwningSide']) { $association = $assoc; } + $joinColumns = $assoc['isOwningSide'] + ? $association['joinTable']['joinColumns'] + : $association['joinTable']['inverseJoinColumns']; - $joinTableName = $this->quoteStrategy->getJoinTableName($association, $this->class, $this->platform); - foreach ($association['joinTable']['joinColumns'] as $joinColumn) { - $columns[] = $joinTableName . '.' . $this->quoteStrategy->getJoinColumnName($joinColumn, $this->class, $this->platform); + $joinTableName = $this->quoteStrategy->getJoinTableName($association, $class, $this->platform); + foreach ($joinColumns as $joinColumn) { + $columns[] = $joinTableName . '.' . $this->quoteStrategy->getJoinColumnName($joinColumn, $class, $this->platform); } } else { From c9e66e464da6548723841b30b8a00d32e44e28f4 Mon Sep 17 00:00:00 2001 From: Asmir Mustafic Date: Mon, 2 Feb 2015 23:59:32 +0100 Subject: [PATCH 11/14] Changed getSelectConditionStatementColumnSQL return docblock --- lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php b/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php index 9158f7b6b..f85aedfaf 100644 --- a/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php +++ b/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php @@ -1632,7 +1632,7 @@ class BasicEntityPersister implements EntityPersister * @param string $field * @param array|null $assoc * - * @return array + * @return string[] * * @throws \Doctrine\ORM\ORMException */ From cb52782e5e987f35ee16edd148328fed0b17b3da Mon Sep 17 00:00:00 2001 From: Asmir Mustafic Date: Tue, 3 Feb 2015 00:11:42 +0100 Subject: [PATCH 12/14] Default Version Value identifier is always an array --- .../Entity/BasicEntityPersister.php | 19 +++++++++---------- .../Entity/JoinedSubclassPersister.php | 9 ++++++--- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php b/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php index f85aedfaf..a1d1969a8 100644 --- a/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php +++ b/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php @@ -281,8 +281,11 @@ class BasicEntityPersister implements EntityPersister $stmt->execute(); if ($isPostInsertId) { - $id = $idGenerator->generate($this->em, $entity); - $postInsertIds[$id] = $entity; + $generatedId = $idGenerator->generate($this->em, $entity); + $id = array( + $this->class->identifier[0] => $generatedId + ); + $postInsertIds[$generatedId] = $entity; } else { $id = $this->class->getIdentifierValues($entity); } @@ -304,11 +307,11 @@ class BasicEntityPersister implements EntityPersister * entities version field. * * @param object $entity - * @param mixed $id + * @param array $id * * @return void */ - protected function assignDefaultVersionValue($entity, $id) + protected function assignDefaultVersionValue($entity, array $id) { $value = $this->fetchVersionValue($this->class, $id); @@ -319,11 +322,11 @@ class BasicEntityPersister implements EntityPersister * Fetches the current version value of a versioned entity. * * @param \Doctrine\ORM\Mapping\ClassMetadata $versionedClass - * @param mixed $id + * @param array $id * * @return mixed */ - protected function fetchVersionValue($versionedClass, $id) + protected function fetchVersionValue($versionedClass, array $id) { $versionField = $versionedClass->versionField; $tableName = $this->quoteStrategy->getTableName($versionedClass, $this->platform); @@ -335,10 +338,6 @@ class BasicEntityPersister implements EntityPersister . ' FROM ' . $tableName . ' WHERE ' . implode(' = ? AND ', $identifier) . ' = ?'; - if (!is_array($id)) { - $id = array($this->class->identifier[0] => $id); - } - $flatId = $this->identifierFlattener->flattenIdentifier($versionedClass, $id); $value = $this->conn->fetchColumn($sql, array_values($flatId)); diff --git a/lib/Doctrine/ORM/Persisters/Entity/JoinedSubclassPersister.php b/lib/Doctrine/ORM/Persisters/Entity/JoinedSubclassPersister.php index a5bd84348..d4159ce47 100644 --- a/lib/Doctrine/ORM/Persisters/Entity/JoinedSubclassPersister.php +++ b/lib/Doctrine/ORM/Persisters/Entity/JoinedSubclassPersister.php @@ -176,8 +176,11 @@ class JoinedSubclassPersister extends AbstractEntityInheritancePersister $rootTableStmt->execute(); if ($isPostInsertId) { - $id = $idGenerator->generate($this->em, $entity); - $postInsertIds[$id] = $entity; + $generatedId = $idGenerator->generate($this->em, $entity); + $id = array( + $this->class->identifier[0] => $generatedId + ); + $postInsertIds[$generatedId] = $entity; } else { $id = $this->em->getUnitOfWork()->getEntityIdentifier($entity); } @@ -572,7 +575,7 @@ class JoinedSubclassPersister extends AbstractEntityInheritancePersister /** * {@inheritdoc} */ - protected function assignDefaultVersionValue($entity, $id) + protected function assignDefaultVersionValue($entity, array $id) { $value = $this->fetchVersionValue($this->getVersionedClassMetadata(), $id); $this->class->setFieldValue($entity, $this->class->versionField, $value); From 8eea7c86f77ad4a60c120f9f0a2691c8a2ffc65b Mon Sep 17 00:00:00 2001 From: Asmir Mustafic Date: Tue, 3 Feb 2015 00:27:02 +0100 Subject: [PATCH 13/14] Resolve association entries on multi get cache --- lib/Doctrine/ORM/Cache/DefaultCollectionHydrator.php | 2 +- lib/Doctrine/ORM/UnitOfWork.php | 4 ---- lib/Doctrine/ORM/Utility/IdentifierFlattener.php | 4 +--- 3 files changed, 2 insertions(+), 8 deletions(-) diff --git a/lib/Doctrine/ORM/Cache/DefaultCollectionHydrator.php b/lib/Doctrine/ORM/Cache/DefaultCollectionHydrator.php index 5ca1e59df..f6583cbd2 100644 --- a/lib/Doctrine/ORM/Cache/DefaultCollectionHydrator.php +++ b/lib/Doctrine/ORM/Cache/DefaultCollectionHydrator.php @@ -89,7 +89,7 @@ class DefaultCollectionHydrator implements CollectionHydrator /* @var $entityEntries \Doctrine\ORM\Cache\EntityCacheEntry[] */ foreach ($entityEntries as $index => $entityEntry) { - $list[$index] = $this->uow->createEntity($entityEntry->class, $entityEntry->data, self::$hints); + $list[$index] = $this->uow->createEntity($entityEntry->class, $entityEntry->resolveAssociationEntries($this->em), self::$hints); } array_walk($list, function($entity, $index) use ($collection) { diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index 0294b9f82..ff4752bdd 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -2625,10 +2625,6 @@ class UnitOfWork implements PropertyChangedListener if ($joinColumnValue !== null) { if ($targetClass->containsForeignIdentifier) { - if ($joinColumnValue instanceof AssociationCacheEntry) { - $joinColumnValue = implode(' ', $joinColumnValue->identifier); - } - $associatedId[$targetClass->getFieldForColumn($targetColumn)] = $joinColumnValue; } else { $associatedId[$targetClass->fieldNames[$targetColumn]] = $joinColumnValue; diff --git a/lib/Doctrine/ORM/Utility/IdentifierFlattener.php b/lib/Doctrine/ORM/Utility/IdentifierFlattener.php index 9108c35c8..7c9e488e5 100644 --- a/lib/Doctrine/ORM/Utility/IdentifierFlattener.php +++ b/lib/Doctrine/ORM/Utility/IdentifierFlattener.php @@ -72,9 +72,7 @@ final class IdentifierFlattener $flatId = array(); foreach ($class->identifier as $field) { - if (isset($class->associationMappings[$field]) && isset($id[$field]) && $id[$field] instanceof \Doctrine\ORM\Cache\AssociationCacheEntry) { - $flatId[$field] = implode(' ', $id[$field]->identifier); - } elseif (isset($class->associationMappings[$field]) && isset($id[$field]) && is_object($id[$field])) { + if (isset($class->associationMappings[$field]) && isset($id[$field]) && is_object($id[$field])) { $targetClassMetadata = $this->metadataFactory->getMetadataFor( $class->associationMappings[$field]['targetEntity'] ); From 2a99d5a19bda121eac12c0832ac9f87b6d750b75 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Mon, 16 Feb 2015 01:02:56 +0000 Subject: [PATCH 14/14] #1113 - minor CS fixes (spacing/alignment) --- .../Entity/BasicEntityPersister.php | 29 +++++++++++++------ lib/Doctrine/ORM/UnitOfWork.php | 4 ++- .../ORM/Utility/IdentifierFlattener.php | 3 ++ lib/Doctrine/ORM/Utility/PersisterHelper.php | 4 ++- 4 files changed, 29 insertions(+), 11 deletions(-) diff --git a/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php b/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php index a1d1969a8..5d096a9dc 100644 --- a/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php +++ b/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php @@ -1649,16 +1649,19 @@ class BasicEntityPersister implements EntityPersister $association = $this->class->associationMappings[$field]; // Many-To-Many requires join table check for joinColumn $columns = array(); - $class = $this->class; + $class = $this->class; + if ($association['type'] === ClassMetadata::MANY_TO_MANY) { if ( ! $association['isOwningSide']) { $association = $assoc; } - $joinColumns = $assoc['isOwningSide'] + + $joinTableName = $this->quoteStrategy->getJoinTableName($association, $class, $this->platform); + $joinColumns = $assoc['isOwningSide'] ? $association['joinTable']['joinColumns'] : $association['joinTable']['inverseJoinColumns']; - $joinTableName = $this->quoteStrategy->getJoinTableName($association, $class, $this->platform); + foreach ($joinColumns as $joinColumn) { $columns[] = $joinTableName . '.' . $this->quoteStrategy->getJoinColumnName($joinColumn, $class, $this->platform); } @@ -1691,7 +1694,6 @@ class BasicEntityPersister implements EntityPersister throw ORMException::unrecognizedField($field); } - /** * Gets the conditional SQL fragment used in the WHERE clause when selecting * entities in this persister. @@ -1812,6 +1814,7 @@ class BasicEntityPersister implements EntityPersister $types = array_merge($types, $this->getTypes($field, $value, $this->class)); $params = array_merge($params, $this->getValues($value)); } + return array($params, $types); } @@ -1840,6 +1843,7 @@ class BasicEntityPersister implements EntityPersister $types = array_merge($types, $this->getTypes($criterion['field'], $criterion['value'], $criterion['class'])); $params = array_merge($params, $this->getValues($criterion['value'])); } + return array($params, $types); } @@ -1861,10 +1865,12 @@ class BasicEntityPersister implements EntityPersister case (isset($class->fieldMappings[$field])): $types = array_merge($types, PersisterHelper::getTypeOfField($field, $class, $this->em)); break; + case (isset($class->associationMappings[$field])): $assoc = $class->associationMappings[$field]; $class = $this->em->getClassMetadata($assoc['targetEntity']); - if (!$assoc['isOwningSide']) { + + if (! $assoc['isOwningSide']) { $assoc = $class->associationMappings[$assoc['mappedBy']]; $class = $this->em->getClassMetadata($assoc['targetEntity']); } @@ -1872,6 +1878,7 @@ class BasicEntityPersister implements EntityPersister $columns = $assoc['type'] === ClassMetadata::MANY_TO_MANY ? $assoc['relationToTargetKeyColumns'] : $assoc['sourceToTargetKeyColumns']; + foreach ($columns as $column){ $types[] = PersisterHelper::getTypeOfColumn($column, $class, $this->em); } @@ -1883,11 +1890,14 @@ class BasicEntityPersister implements EntityPersister } if (is_array($value)) { - $types = array_map(function ($type) { - $type = Type::getType($type)->getBindingType(); - return $type + Connection::ARRAY_PARAM_OFFSET; - }, $types); + return array_map( + function ($type) { + return Type::getType($type)->getBindingType() + Connection::ARRAY_PARAM_OFFSET; + }, + $types + ); } + return $types; } @@ -1914,6 +1924,7 @@ class BasicEntityPersister implements EntityPersister $class = $this->em->getClassMetadata(get_class($value)); if ($class->isIdentifierComposite) { $newValue = array(); + foreach ($class->getIdentifierValues($value) as $innerValue) { $newValue = array_merge($newValue, $this->getValues($innerValue)); } diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index ff4752bdd..c976443a8 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -2629,7 +2629,9 @@ class UnitOfWork implements PropertyChangedListener } else { $associatedId[$targetClass->fieldNames[$targetColumn]] = $joinColumnValue; } - } elseif ($targetClass->containsForeignIdentifier && in_array($targetClass->getFieldForColumn($targetColumn), $targetClass->identifier, true)) { + } elseif ($targetClass->containsForeignIdentifier + && in_array($targetClass->getFieldForColumn($targetColumn), $targetClass->identifier, true) + ) { // the missing key is part of target's entity primary key $associatedId = array(); break; diff --git a/lib/Doctrine/ORM/Utility/IdentifierFlattener.php b/lib/Doctrine/ORM/Utility/IdentifierFlattener.php index 7c9e488e5..4a58dfeda 100644 --- a/lib/Doctrine/ORM/Utility/IdentifierFlattener.php +++ b/lib/Doctrine/ORM/Utility/IdentifierFlattener.php @@ -73,6 +73,7 @@ final class IdentifierFlattener foreach ($class->identifier as $field) { if (isset($class->associationMappings[$field]) && isset($id[$field]) && is_object($id[$field])) { + /* @var $targetClassMetadata ClassMetadata */ $targetClassMetadata = $this->metadataFactory->getMetadataFor( $class->associationMappings[$field]['targetEntity'] ); @@ -86,9 +87,11 @@ final class IdentifierFlattener $flatId[$field] = implode(' ', $associatedId); } elseif (isset($class->associationMappings[$field])) { $associatedId = array(); + foreach ($class->associationMappings[$field]['joinColumns'] as $joinColumn) { $associatedId[] = $id[$joinColumn['name']]; } + $flatId[$field] = implode(' ', $associatedId); } else { $flatId[$field] = $id[$field]; diff --git a/lib/Doctrine/ORM/Utility/PersisterHelper.php b/lib/Doctrine/ORM/Utility/PersisterHelper.php index 888d17d13..6fe7545ea 100644 --- a/lib/Doctrine/ORM/Utility/PersisterHelper.php +++ b/lib/Doctrine/ORM/Utility/PersisterHelper.php @@ -66,11 +66,13 @@ class PersisterHelper $joinData = $assoc; } - $types = array(); + $types = array(); $targetClass = $em->getClassMetadata($assoc['targetEntity']); + foreach ($joinData['joinColumns'] as $joinColumn) { $types[] = self::getTypeOfColumn($joinColumn['referencedColumnName'], $targetClass, $em); } + return $types; }