From bd1bc072706d41cdd37082b47e5af480a5102be5 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sat, 18 Feb 2012 16:07:55 +0100 Subject: [PATCH] [DDC-1651] Convert entities as parameters early in setParameter() to avoid them being part of result cache strings, which causes non-uniqueness. --- lib/Doctrine/ORM/AbstractQuery.php | 48 +++++++++++++++++++ lib/Doctrine/ORM/Query.php | 36 +------------- .../Functional/CompositePrimaryKeyTest.php | 22 ++++++++- .../Tests/ORM/Functional/QueryTest.php | 22 ++++++++- 4 files changed, 92 insertions(+), 36 deletions(-) diff --git a/lib/Doctrine/ORM/AbstractQuery.php b/lib/Doctrine/ORM/AbstractQuery.php index 5ff47c15a..eb6917b82 100644 --- a/lib/Doctrine/ORM/AbstractQuery.php +++ b/lib/Doctrine/ORM/AbstractQuery.php @@ -208,6 +208,7 @@ abstract class AbstractQuery { $key = trim($key, ':'); + $value = $this->processParameterValue($value); if ($type === null) { $type = Query\ParameterTypeInferer::inferType($value); } @@ -218,6 +219,53 @@ abstract class AbstractQuery return $this; } + /** + * Process an individual parameter value + * + * @param mixed $value + * @return array + */ + private function processParameterValue($value) + { + switch (true) { + case is_array($value): + for ($i = 0, $l = count($value); $i < $l; $i++) { + $paramValue = $this->processParameterValue($value[$i]); + $value[$i] = is_array($paramValue) ? $paramValue[key($paramValue)] : $paramValue; + } + + return $value; + + case is_object($value) && $this->_em->getMetadataFactory()->hasMetadataFor(get_class($value)): + return $this->convertObjectParameterToScalarValue($value); + + default: + return $value; + } + } + + protected function convertObjectParameterToScalarValue($value) + { + $class = $this->_em->getClassMetadata(get_class($value)); + + if ($class->isIdentifierComposite) { + throw new \InvalidArgumentException("Binding an entity with a composite primary key to a query is not supported. You should split the parameter into the explicit fields and bind them seperately."); + } + + if ($this->_em->getUnitOfWork()->getEntityState($value) === UnitOfWork::STATE_MANAGED) { + $values = $this->_em->getUnitOfWork()->getEntityIdentifier($value); + } else { + $values = $class->getIdentifierValues($value); + } + + $value = $values[$class->getSingleIdentifierFieldName()]; + if (!$value) { + throw new \InvalidArgumentException("Binding entities to query parameters only allowed for entities that have an identifier."); + } + + return $value; + } + /** * Sets a collection of query parameters. * diff --git a/lib/Doctrine/ORM/Query.php b/lib/Doctrine/ORM/Query.php index ff3f8426e..16c6c3803 100644 --- a/lib/Doctrine/ORM/Query.php +++ b/lib/Doctrine/ORM/Query.php @@ -282,7 +282,8 @@ final class Query extends AbstractQuery } $sqlPositions = $paramMappings[$key]; - $value = array_values($this->processParameterValue($value)); + // optimized multi value sql positions away for now, they are not allowed in DQL anyways. + $value = array($value); $countValue = count($value); for ($i = 0, $l = count($sqlPositions); $i < $l; $i++) { @@ -305,39 +306,6 @@ final class Query extends AbstractQuery return array($sqlParams, $types); } - /** - * Process an individual parameter value - * - * @param mixed $value - * @return array - */ - private function processParameterValue($value) - { - switch (true) { - case is_array($value): - for ($i = 0, $l = count($value); $i < $l; $i++) { - $paramValue = $this->processParameterValue($value[$i]); - - // TODO: What about Entities that have composite primary key? - $value[$i] = is_array($paramValue) ? $paramValue[key($paramValue)] : $paramValue; - } - - return array($value); - - case is_object($value) && $this->_em->getMetadataFactory()->hasMetadataFor(get_class($value)): - if ($this->_em->getUnitOfWork()->getEntityState($value) === UnitOfWork::STATE_MANAGED) { - return array_values($this->_em->getUnitOfWork()->getEntityIdentifier($value)); - } - - $class = $this->_em->getClassMetadata(get_class($value)); - - return array_values($class->getIdentifierValues($value)); - - default: - return array($value); - } - } - /** * Defines a cache driver to be used for caching queries. * diff --git a/tests/Doctrine/Tests/ORM/Functional/CompositePrimaryKeyTest.php b/tests/Doctrine/Tests/ORM/Functional/CompositePrimaryKeyTest.php index 80e2fa0d6..4d321ad04 100644 --- a/tests/Doctrine/Tests/ORM/Functional/CompositePrimaryKeyTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/CompositePrimaryKeyTest.php @@ -4,6 +4,7 @@ namespace Doctrine\Tests\ORM\Functional; use Doctrine\Tests\Models\Navigation\NavCountry; use Doctrine\Tests\Models\Navigation\NavPointOfInterest; use Doctrine\Tests\Models\Navigation\NavTour; +use Doctrine\Tests\Models\Navigation\NavPhotos; require_once __DIR__ . '/../../TestInit.php'; @@ -51,6 +52,25 @@ class CompositePrimaryKeyTest extends \Doctrine\Tests\OrmFunctionalTestCase $this->assertEquals('Brandenburger Tor', $poi->getName()); } + /** + * @group DDC-1651 + */ + public function testSetParameterCompositeKeyObject() + { + $this->putGermanysBrandenburderTor(); + + $poi = $this->_em->find('Doctrine\Tests\Models\Navigation\NavPointOfInterest', array('lat' => 100, 'long' => 200)); + $photo = new NavPhotos($poi, "asdf"); + $this->_em->persist($photo); + $this->_em->flush(); + $this->_em->clear(); + + $dql = 'SELECT t FROM Doctrine\Tests\Models\Navigation\NavPhotos t WHERE t.poi = ?1'; + + $this->setExpectedException('Doctrine\ORM\Query\QueryException', 'A single-valued association path expression to an entity with a composite primary key is not supported.'); + $sql = $this->_em->createQuery($dql)->getSQL(); + } + public function testManyToManyCompositeRelation() { $this->putGermanysBrandenburderTor(); @@ -98,4 +118,4 @@ class CompositePrimaryKeyTest extends \Doctrine\Tests\OrmFunctionalTestCase $this->setExpectedException('Doctrine\ORM\ORMException', 'The identifier long is missing for a query of Doctrine\Tests\Models\Navigation\NavPointOfInterest'); $poi = $this->_em->find('Doctrine\Tests\Models\Navigation\NavPointOfInterest', array('key1' => 100)); } -} \ No newline at end of file +} diff --git a/tests/Doctrine/Tests/ORM/Functional/QueryTest.php b/tests/Doctrine/Tests/ORM/Functional/QueryTest.php index 557e4689e..b19a8a6d2 100644 --- a/tests/Doctrine/Tests/ORM/Functional/QueryTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/QueryTest.php @@ -599,4 +599,24 @@ class QueryTest extends \Doctrine\Tests\OrmFunctionalTestCase $this->assertEquals(3, count($users)); $this->assertInstanceOf('Doctrine\Tests\Models\CMS\CmsUser', $users[0]); } -} \ No newline at end of file + + /** + * @group DDC-1651 + */ + public function testSetParameterBindingSingleIdentifierObjectConverted() + { + $userC = new CmsUser; + $userC->name = 'Jonathan'; + $userC->username = 'jwage'; + $userC->status = 'developer'; + $this->_em->persist($userC); + + $this->_em->flush(); + $this->_em->clear(); + + $q = $this->_em->createQuery("SELECT DISTINCT u from Doctrine\Tests\Models\CMS\CmsUser u WHERE u.id = ?1"); + $q->setParameter(1, $userC); + + $this->assertEquals($userC->id, $q->getParameter(1)); + } +}