From a09a5b9b7b41c6c6b292f8998e2dd0470ebcb286 Mon Sep 17 00:00:00 2001 From: "Fabio B. Silva" Date: Fri, 19 Oct 2012 23:18:01 -0300 Subject: [PATCH 1/3] Fix DDC-2084 --- lib/Doctrine/ORM/AbstractQuery.php | 46 ++---- lib/Doctrine/ORM/EntityManager.php | 11 +- .../ORM/ORMInvalidArgumentException.php | 11 ++ lib/Doctrine/ORM/UnitOfWork.php | 24 ++++ .../ORM/Functional/Ticket/DDC2084Test.php | 131 ++++++++++++++++++ 5 files changed, 189 insertions(+), 34 deletions(-) create mode 100644 tests/Doctrine/Tests/ORM/Functional/Ticket/DDC2084Test.php diff --git a/lib/Doctrine/ORM/AbstractQuery.php b/lib/Doctrine/ORM/AbstractQuery.php index 0913b73f4..e8cc06141 100644 --- a/lib/Doctrine/ORM/AbstractQuery.php +++ b/lib/Doctrine/ORM/AbstractQuery.php @@ -26,6 +26,7 @@ use Doctrine\DBAL\Types\Type; use Doctrine\DBAL\Cache\QueryCacheProfile; use Doctrine\ORM\Query\QueryException; +use Doctrine\ORM\ORMInvalidArgumentException; /** * Base contract for ORM queries. Base class for Query and NativeQuery. @@ -247,44 +248,23 @@ abstract class AbstractQuery */ public function processParameterValue($value) { - switch (true) { - case is_array($value): - foreach ($value as $key => $paramValue) { - $paramValue = $this->processParameterValue($paramValue); - $value[$key] = is_array($paramValue) ? $paramValue[key($paramValue)] : $paramValue; - } + if (is_array($value)) { + foreach ($value as $key => $paramValue) { + $paramValue = $this->processParameterValue($paramValue); + $value[$key] = is_array($paramValue) ? reset($paramValue) : $paramValue; + } - return $value; - - case is_object($value) && $this->_em->getMetadataFactory()->hasMetadataFor(ClassUtils::getClass($value)): - return $this->convertObjectParameterToScalarValue($value); - - default: - return $value; - } - } - - private 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." - ); + return $value; } - $values = ($this->_em->getUnitOfWork()->getEntityState($value) === UnitOfWork::STATE_MANAGED) - ? $this->_em->getUnitOfWork()->getEntityIdentifier($value) - : $class->getIdentifierValues($value); + if (is_object($value) && $this->_em->getMetadataFactory()->hasMetadataFor(ClassUtils::getClass($value))) { + $value = $this->_em->getUnitOfWork()->getSingleIdentifierValue($value); - $value = $values[$class->getSingleIdentifierFieldName()]; + if ($value === null) { + throw ORMInvalidArgumentException::invalidIdentifierBindingEntity(); + } - if (null === $value) { - throw new \InvalidArgumentException( - "Binding entities to query parameters only allowed for entities that have an identifier." - ); + return $value; } return $value; diff --git a/lib/Doctrine/ORM/EntityManager.php b/lib/Doctrine/ORM/EntityManager.php index 36895f046..9a6974424 100644 --- a/lib/Doctrine/ORM/EntityManager.php +++ b/lib/Doctrine/ORM/EntityManager.php @@ -29,6 +29,7 @@ use Doctrine\ORM\Mapping\ClassMetadataFactory; use Doctrine\ORM\Query\ResultSetMapping; use Doctrine\ORM\Proxy\ProxyFactory; use Doctrine\ORM\Query\FilterCollection; +use Doctrine\Common\Util\ClassUtils; /** * The EntityManager is the central access point to ORM functionality. @@ -354,7 +355,7 @@ class EntityManager implements ObjectManager $this->unitOfWork->commit($entity); } - + /** * Finds an Entity by its identifier. * @@ -369,6 +370,14 @@ class EntityManager implements ObjectManager { $class = $this->metadataFactory->getMetadataFor(ltrim($entityName, '\\')); + if (is_object($id) && $this->metadataFactory->hasMetadataFor(ClassUtils::getClass($id))) { + $id = $this->unitOfWork->getSingleIdentifierValue($id); + + if ($id === null) { + throw ORMInvalidArgumentException::invalidIdentifierBindingEntity(); + } + } + if ( ! is_array($id)) { $id = array($class->identifier[0] => $id); } diff --git a/lib/Doctrine/ORM/ORMInvalidArgumentException.php b/lib/Doctrine/ORM/ORMInvalidArgumentException.php index d88917340..c5df4d2fa 100644 --- a/lib/Doctrine/ORM/ORMInvalidArgumentException.php +++ b/lib/Doctrine/ORM/ORMInvalidArgumentException.php @@ -102,6 +102,17 @@ class ORMInvalidArgumentException extends \InvalidArgumentException ' to be an entity object, '. gettype($given) . ' given.'); } + public static function invalidCompositeIdentifier() + { + return new self("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."); + } + + public static function invalidIdentifierBindingEntity() + { + return new self("Binding entities to query parameters only allowed for entities that have an identifier."); + } + /** * Helper method to show an object as string. * diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index c5f7837e8..75e7a3501 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -2737,6 +2737,30 @@ class UnitOfWork implements PropertyChangedListener return $this->entityIdentifiers[spl_object_hash($entity)]; } + /** + * Process an entity instance to extract their identifier values. + * + * @param object $entity The entity instance. + * + * @return scalar + * + * @throws \Doctrine\ORM\ORMInvalidArgumentException + */ + public function getSingleIdentifierValue($entity) + { + $class = $this->em->getClassMetadata(get_class($entity)); + + if ($class->isIdentifierComposite) { + throw ORMInvalidArgumentException::invalidCompositeIdentifier(); + } + + $values = ($this->getEntityState($entity) === UnitOfWork::STATE_MANAGED) + ? $this->getEntityIdentifier($entity) + : $class->getIdentifierValues($entity); + + return isset($values[$class->identifier[0]]) ? $values[$class->identifier[0]] : null; + } + /** * Tries to find an entity with the given identifier in the identity map of * this UnitOfWork. diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC2084Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC2084Test.php new file mode 100644 index 000000000..08669e6fc --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC2084Test.php @@ -0,0 +1,131 @@ +_schemaTool->createSchema(array( + $this->_em->getClassMetadata(__NAMESPACE__ . '\DDC2084\MyEntity1'), + $this->_em->getClassMetadata(__NAMESPACE__ . '\DDC2084\MyEntity2'), + )); + } catch (\Exception $exc) { + } + } + + public function loadFixture() + { + $e2 = new DDC2084\MyEntity2('Foo'); + $e1 = new DDC2084\MyEntity1($e2); + + $this->_em->persist($e2); + $this->_em->flush(); + + $this->_em->persist($e1); + $this->_em->flush(); + + $this->_em->clear(); + + return $e1; + } + + public function testIssue() + { + $e1 = $this->loadFixture(); + $e2 = $e1->getMyEntity2(); + $e = $this->_em->find(__NAMESPACE__ . '\DDC2084\MyEntity1', $e2); + + $this->assertInstanceOf(__NAMESPACE__ . '\DDC2084\MyEntity1', $e); + $this->assertInstanceOf(__NAMESPACE__ . '\DDC2084\MyEntity2', $e->getMyEntity2()); + $this->assertEquals('Foo', $e->getMyEntity2()->getValue()); + } + + /** + * @expectedException \Doctrine\ORM\ORMInvalidArgumentException + * @expectedExceptionMessage Binding entities to query parameters only allowed for entities that have an identifier. + */ + public function testinvalidIdentifierBindingEntityException() + { + $this->_em->find(__NAMESPACE__ . '\DDC2084\MyEntity1', new DDC2084\MyEntity2('Foo')); + } +} + +namespace Doctrine\Tests\ORM\Functional\Ticket\DDC2084; + +/** + * @Entity + * @Table(name="DDC2084_ENTITY1") + */ +class MyEntity1 +{ + /** + * @Id + * @OneToOne(targetEntity="MyEntity2") + * @JoinColumn(name="entity2_id", referencedColumnName="id", nullable=false) + */ + private $entity2; + + public function __construct(MyEntity2 $myEntity2) + { + $this->entity2 = $myEntity2; + } + + public function setMyEntity2(MyEntity2 $myEntity2) + { + $this->entity2 = $myEntity2; + } + + public function getMyEntity2() + { + return $this->entity2; + } +} + +/** + * @Entity + * @Table(name="DDC2084_ENTITY2") + */ +class MyEntity2 +{ + /** + * @Id + * @Column(type="integer") + * @GeneratedValue(strategy="AUTO") + */ + private $id; + + /** + * @Column + */ + private $value; + + public function __construct($value) + { + $this->value = $value; + } + + public function getId() + { + return $this->id; + } + + public function getValue() + { + return $this->value; + } + + public function setValue($value) + { + $this->value = $value; + } +} \ No newline at end of file From c4e6a04676f398ec9dba5779796c2521ba6ca8ba Mon Sep 17 00:00:00 2001 From: "Fabio B. Silva" Date: Sat, 27 Oct 2012 18:43:34 -0200 Subject: [PATCH 2/3] remove duplicate return statement --- lib/Doctrine/ORM/AbstractQuery.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/Doctrine/ORM/AbstractQuery.php b/lib/Doctrine/ORM/AbstractQuery.php index e8cc06141..5d6a91d13 100644 --- a/lib/Doctrine/ORM/AbstractQuery.php +++ b/lib/Doctrine/ORM/AbstractQuery.php @@ -263,8 +263,6 @@ abstract class AbstractQuery if ($value === null) { throw ORMInvalidArgumentException::invalidIdentifierBindingEntity(); } - - return $value; } return $value; From 62f43e6ea2367e15ca6c18768870fe8c62d3b185 Mon Sep 17 00:00:00 2001 From: "Fabio B. Silva" Date: Sun, 28 Oct 2012 15:18:21 -0200 Subject: [PATCH 3/3] remove require_once --- tests/Doctrine/Tests/ORM/Functional/Ticket/DDC2084Test.php | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC2084Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC2084Test.php index 08669e6fc..abd0fe2d8 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC2084Test.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC2084Test.php @@ -2,9 +2,6 @@ namespace Doctrine\Tests\ORM\Functional\Ticket; - -require_once __DIR__ . '/../../../TestInit.php'; - /** * @group DDC-2084 */