From 8e4092750da8468e3236eed31ba3d24895752bce Mon Sep 17 00:00:00 2001 From: Lars Strojny Date: Thu, 8 Jan 2015 15:59:17 +0100 Subject: [PATCH 1/4] Include IDs in the exception message to ease debugging --- lib/Doctrine/ORM/EntityNotFoundException.php | 10 ++++- lib/Doctrine/ORM/Proxy/ProxyFactory.php | 41 +++++++++++++------- lib/Doctrine/ORM/UnitOfWork.php | 2 +- 3 files changed, 36 insertions(+), 17 deletions(-) diff --git a/lib/Doctrine/ORM/EntityNotFoundException.php b/lib/Doctrine/ORM/EntityNotFoundException.php index 18278f715..3a7024dc3 100644 --- a/lib/Doctrine/ORM/EntityNotFoundException.php +++ b/lib/Doctrine/ORM/EntityNotFoundException.php @@ -30,8 +30,14 @@ class EntityNotFoundException extends ORMException /** * Constructor. */ - public function __construct($class) + public function __construct($class, array $id = array()) { - parent::__construct("Entity of type '{$class}' was not found."); + $ids = array(); + foreach ($id as $key => $value) { + $ids[] = "{$key}({$value})"; + } + $idsFormatted = implode(', ', $ids); + $message = "Entity of type '" . $class . "'" . ($idsFormatted ? ' for IDs ' . $idsFormatted : '') . ' was not found'; + parent::__construct($message); } } diff --git a/lib/Doctrine/ORM/Proxy/ProxyFactory.php b/lib/Doctrine/ORM/Proxy/ProxyFactory.php index 8d4cb5662..2c170466f 100644 --- a/lib/Doctrine/ORM/Proxy/ProxyFactory.php +++ b/lib/Doctrine/ORM/Proxy/ProxyFactory.php @@ -28,6 +28,7 @@ use Doctrine\Common\Proxy\ProxyGenerator; use Doctrine\ORM\Persisters\EntityPersister; use Doctrine\ORM\EntityManager; use Doctrine\ORM\EntityNotFoundException; +use Doctrine\ORM\Utility\IdentifierFlattener; /** * This factory is used to create proxy objects for entities at runtime. @@ -54,6 +55,13 @@ class ProxyFactory extends AbstractProxyFactory */ private $proxyNs; + /** + * The IdentifierFlattener used for manipulating identifiers + * + * @var \Doctrine\ORM\Utility\IdentifierFlattener + */ + private $identifierFlattener; + /** * Initializes a new instance of the ProxyFactory class that is * connected to the given EntityManager. @@ -71,10 +79,10 @@ class ProxyFactory extends AbstractProxyFactory $proxyGenerator->setPlaceholder('baseProxyInterface', 'Doctrine\ORM\Proxy\Proxy'); parent::__construct($proxyGenerator, $em->getMetadataFactory(), $autoGenerate); - $this->em = $em; - $this->uow = $em->getUnitOfWork(); - $this->proxyNs = $proxyNs; - + $this->em = $em; + $this->uow = $em->getUnitOfWork(); + $this->proxyNs = $proxyNs; + $this->identifierFlattener = new IdentifierFlattener($this->uow, $em->getMetadataFactory()); } /** @@ -115,8 +123,9 @@ class ProxyFactory extends AbstractProxyFactory */ private function createInitializer(ClassMetadata $classMetadata, EntityPersister $entityPersister) { + $identifierFlattener = $this->identifierFlattener; if ($classMetadata->getReflectionClass()->hasMethod('__wakeup')) { - return function (BaseProxy $proxy) use ($entityPersister, $classMetadata) { + return function (BaseProxy $proxy) use ($entityPersister, $classMetadata, $identifierFlattener) { $initializer = $proxy->__getInitializer(); $cloner = $proxy->__getCloner(); @@ -138,17 +147,18 @@ class ProxyFactory extends AbstractProxyFactory $proxy->__setInitialized(true); $proxy->__wakeup(); - if (null === $entityPersister->loadById($classMetadata->getIdentifierValues($proxy), $proxy)) { + $id = $classMetadata->getIdentifierValues($proxy); + if (null === $entityPersister->loadById($id, $proxy)) { $proxy->__setInitializer($initializer); $proxy->__setCloner($cloner); $proxy->__setInitialized(false); - throw new EntityNotFoundException($classMetadata->getName()); + throw new EntityNotFoundException($classMetadata->getName(), $identifierFlattener->flattenIdentifier($classMetadata, $id)); } }; } - return function (BaseProxy $proxy) use ($entityPersister, $classMetadata) { + return function (BaseProxy $proxy) use ($entityPersister, $classMetadata, $identifierFlattener) { $initializer = $proxy->__getInitializer(); $cloner = $proxy->__getCloner(); @@ -169,12 +179,13 @@ class ProxyFactory extends AbstractProxyFactory $proxy->__setInitialized(true); - if (null === $entityPersister->loadById($classMetadata->getIdentifierValues($proxy), $proxy)) { + $id = $classMetadata->getIdentifierValues($proxy); + if (null === $entityPersister->loadById($id, $proxy)) { $proxy->__setInitializer($initializer); $proxy->__setCloner($cloner); $proxy->__setInitialized(false); - throw new EntityNotFoundException($classMetadata->getName()); + throw new EntityNotFoundException($classMetadata->getName(), $identifierFlattener->flattenIdentifier($classMetadata, $id)); } }; } @@ -191,19 +202,21 @@ class ProxyFactory extends AbstractProxyFactory */ private function createCloner(ClassMetadata $classMetadata, EntityPersister $entityPersister) { - return function (BaseProxy $proxy) use ($entityPersister, $classMetadata) { + $identifierFlattener = $this->identifierFlattener; + return function (BaseProxy $proxy) use ($entityPersister, $classMetadata, $identifierFlattener) { if ($proxy->__isInitialized()) { return; } $proxy->__setInitialized(true); $proxy->__setInitializer(null); - + $class = $entityPersister->getClassMetadata(); - $original = $entityPersister->loadById($classMetadata->getIdentifierValues($proxy)); + $id = $classMetadata->getIdentifierValues($proxy); + $original = $entityPersister->loadById($id); if (null === $original) { - throw new EntityNotFoundException($classMetadata->getName()); + throw new EntityNotFoundException($classMetadata->getName(), $identifierFlattener->flattenIdentifier($classMetadata, $id)); } foreach ($class->getReflectionClass()->getProperties() as $property) { diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index 8789b5f81..c1e3dd1ac 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -1843,7 +1843,7 @@ class UnitOfWork implements PropertyChangedListener // If the identifier is ASSIGNED, it is NEW, otherwise an error // since the managed entity was not found. if ( ! $class->isIdentifierNatural()) { - throw new EntityNotFoundException($class->getName()); + throw new EntityNotFoundException($class->getName(), $this->identifierFlattener->flattenIdentifier($class, $id)); } $managedCopy = $this->newInstance($class); From 66c556fbfd53bc019aecfd9f2f5ec99d6df59483 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Tue, 13 Jan 2015 02:48:57 +0100 Subject: [PATCH 2/4] #1240 DDC-3479 - Fixing minor CS issues (naming, alignment) --- lib/Doctrine/ORM/EntityNotFoundException.php | 16 +++++++--- lib/Doctrine/ORM/Proxy/ProxyFactory.php | 32 ++++++++++++++------ 2 files changed, 33 insertions(+), 15 deletions(-) diff --git a/lib/Doctrine/ORM/EntityNotFoundException.php b/lib/Doctrine/ORM/EntityNotFoundException.php index 3a7024dc3..7062112fc 100644 --- a/lib/Doctrine/ORM/EntityNotFoundException.php +++ b/lib/Doctrine/ORM/EntityNotFoundException.php @@ -29,15 +29,21 @@ class EntityNotFoundException extends ORMException { /** * Constructor. + * + * @param string $className + * @param string[] $id */ - public function __construct($class, array $id = array()) + public function __construct($className, array $id = array()) { $ids = array(); + foreach ($id as $key => $value) { - $ids[] = "{$key}({$value})"; + $ids[] = $key . '(' . $value . ')'; } - $idsFormatted = implode(', ', $ids); - $message = "Entity of type '" . $class . "'" . ($idsFormatted ? ' for IDs ' . $idsFormatted : '') . ' was not found'; - parent::__construct($message); + + + parent::__construct( + 'Entity of type \'' . $className . '\'' . ($ids ? ' for IDs ' . implode(', ', $ids) : '') . ' was not found' + ); } } diff --git a/lib/Doctrine/ORM/Proxy/ProxyFactory.php b/lib/Doctrine/ORM/Proxy/ProxyFactory.php index 2c170466f..5ecd883cf 100644 --- a/lib/Doctrine/ORM/Proxy/ProxyFactory.php +++ b/lib/Doctrine/ORM/Proxy/ProxyFactory.php @@ -147,13 +147,17 @@ class ProxyFactory extends AbstractProxyFactory $proxy->__setInitialized(true); $proxy->__wakeup(); - $id = $classMetadata->getIdentifierValues($proxy); - if (null === $entityPersister->loadById($id, $proxy)) { + $identifier = $classMetadata->getIdentifierValues($proxy); + + if (null === $entityPersister->loadById($identifier, $proxy)) { $proxy->__setInitializer($initializer); $proxy->__setCloner($cloner); $proxy->__setInitialized(false); - throw new EntityNotFoundException($classMetadata->getName(), $identifierFlattener->flattenIdentifier($classMetadata, $id)); + throw new EntityNotFoundException( + $classMetadata->getName(), + $identifierFlattener->flattenIdentifier($classMetadata, $identifier) + ); } }; } @@ -179,13 +183,17 @@ class ProxyFactory extends AbstractProxyFactory $proxy->__setInitialized(true); - $id = $classMetadata->getIdentifierValues($proxy); - if (null === $entityPersister->loadById($id, $proxy)) { + $identifier = $classMetadata->getIdentifierValues($proxy); + + if (null === $entityPersister->loadById($identifier, $proxy)) { $proxy->__setInitializer($initializer); $proxy->__setCloner($cloner); $proxy->__setInitialized(false); - throw new EntityNotFoundException($classMetadata->getName(), $identifierFlattener->flattenIdentifier($classMetadata, $id)); + throw new EntityNotFoundException( + $classMetadata->getName(), + $identifierFlattener->flattenIdentifier($classMetadata, $identifier) + ); } }; } @@ -203,6 +211,7 @@ class ProxyFactory extends AbstractProxyFactory private function createCloner(ClassMetadata $classMetadata, EntityPersister $entityPersister) { $identifierFlattener = $this->identifierFlattener; + return function (BaseProxy $proxy) use ($entityPersister, $classMetadata, $identifierFlattener) { if ($proxy->__isInitialized()) { return; @@ -211,12 +220,15 @@ class ProxyFactory extends AbstractProxyFactory $proxy->__setInitialized(true); $proxy->__setInitializer(null); - $class = $entityPersister->getClassMetadata(); - $id = $classMetadata->getIdentifierValues($proxy); - $original = $entityPersister->loadById($id); + $class = $entityPersister->getClassMetadata(); + $identifier = $classMetadata->getIdentifierValues($proxy); + $original = $entityPersister->loadById($identifier); if (null === $original) { - throw new EntityNotFoundException($classMetadata->getName(), $identifierFlattener->flattenIdentifier($classMetadata, $id)); + throw new EntityNotFoundException( + $classMetadata->getName(), + $identifierFlattener->flattenIdentifier($classMetadata, $identifier) + ); } foreach ($class->getReflectionClass()->getProperties() as $property) { From fc72b4195363e4aee2a39b9b686832bb93f64f24 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Tue, 13 Jan 2015 02:51:47 +0100 Subject: [PATCH 3/4] #1240 DDC-3479 - Using a static proxy constructor rather than the default constructor --- lib/Doctrine/ORM/EntityNotFoundException.php | 9 ++++++--- lib/Doctrine/ORM/Proxy/ProxyFactory.php | 6 +++--- lib/Doctrine/ORM/UnitOfWork.php | 5 ++++- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/lib/Doctrine/ORM/EntityNotFoundException.php b/lib/Doctrine/ORM/EntityNotFoundException.php index 7062112fc..e4ace3dc0 100644 --- a/lib/Doctrine/ORM/EntityNotFoundException.php +++ b/lib/Doctrine/ORM/EntityNotFoundException.php @@ -28,13 +28,16 @@ namespace Doctrine\ORM; class EntityNotFoundException extends ORMException { /** - * Constructor. + * Static constructor. * * @param string $className * @param string[] $id + * + * @return self */ - public function __construct($className, array $id = array()) + public static function fromClassNameAndIdentifier($className, array $id) { + $ids = array(); foreach ($id as $key => $value) { @@ -42,7 +45,7 @@ class EntityNotFoundException extends ORMException } - parent::__construct( + return new self( 'Entity of type \'' . $className . '\'' . ($ids ? ' for IDs ' . implode(', ', $ids) : '') . ' was not found' ); } diff --git a/lib/Doctrine/ORM/Proxy/ProxyFactory.php b/lib/Doctrine/ORM/Proxy/ProxyFactory.php index 5ecd883cf..157ae643f 100644 --- a/lib/Doctrine/ORM/Proxy/ProxyFactory.php +++ b/lib/Doctrine/ORM/Proxy/ProxyFactory.php @@ -154,7 +154,7 @@ class ProxyFactory extends AbstractProxyFactory $proxy->__setCloner($cloner); $proxy->__setInitialized(false); - throw new EntityNotFoundException( + throw EntityNotFoundException::fromClassNameAndIdentifier( $classMetadata->getName(), $identifierFlattener->flattenIdentifier($classMetadata, $identifier) ); @@ -190,7 +190,7 @@ class ProxyFactory extends AbstractProxyFactory $proxy->__setCloner($cloner); $proxy->__setInitialized(false); - throw new EntityNotFoundException( + throw EntityNotFoundException::fromClassNameAndIdentifier( $classMetadata->getName(), $identifierFlattener->flattenIdentifier($classMetadata, $identifier) ); @@ -225,7 +225,7 @@ class ProxyFactory extends AbstractProxyFactory $original = $entityPersister->loadById($identifier); if (null === $original) { - throw new EntityNotFoundException( + throw EntityNotFoundException::fromClassNameAndIdentifier( $classMetadata->getName(), $identifierFlattener->flattenIdentifier($classMetadata, $identifier) ); diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index c1e3dd1ac..e91ff3d12 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -1843,7 +1843,10 @@ class UnitOfWork implements PropertyChangedListener // If the identifier is ASSIGNED, it is NEW, otherwise an error // since the managed entity was not found. if ( ! $class->isIdentifierNatural()) { - throw new EntityNotFoundException($class->getName(), $this->identifierFlattener->flattenIdentifier($class, $id)); + throw EntityNotFoundException::fromClassNameAndIdentifier( + $class->getName(), + $this->identifierFlattener->flattenIdentifier($class, $id) + ); } $managedCopy = $this->newInstance($class); From 1bfa7ea7548789e03f17d55b1cba89bba46d259c Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Tue, 13 Jan 2015 02:55:51 +0100 Subject: [PATCH 4/4] #1240 DDC-3479 - Basic coverage for `EntityNotFoundException` --- lib/Doctrine/ORM/EntityNotFoundException.php | 1 - .../Tests/ORM/EntityNotFoundExceptionTest.php | 33 +++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) create mode 100644 tests/Doctrine/Tests/ORM/EntityNotFoundExceptionTest.php diff --git a/lib/Doctrine/ORM/EntityNotFoundException.php b/lib/Doctrine/ORM/EntityNotFoundException.php index e4ace3dc0..afe2f2242 100644 --- a/lib/Doctrine/ORM/EntityNotFoundException.php +++ b/lib/Doctrine/ORM/EntityNotFoundException.php @@ -37,7 +37,6 @@ class EntityNotFoundException extends ORMException */ public static function fromClassNameAndIdentifier($className, array $id) { - $ids = array(); foreach ($id as $key => $value) { diff --git a/tests/Doctrine/Tests/ORM/EntityNotFoundExceptionTest.php b/tests/Doctrine/Tests/ORM/EntityNotFoundExceptionTest.php new file mode 100644 index 000000000..7dce1b171 --- /dev/null +++ b/tests/Doctrine/Tests/ORM/EntityNotFoundExceptionTest.php @@ -0,0 +1,33 @@ + 'bar') + ); + + $this->assertInstanceOf('Doctrine\ORM\EntityNotFoundException', $exception); + $this->assertSame('Entity of type \'foo\' for IDs foo(bar) was not found', $exception->getMessage()); + + $exception = EntityNotFoundException::fromClassNameAndIdentifier( + 'foo', + array() + ); + + $this->assertInstanceOf('Doctrine\ORM\EntityNotFoundException', $exception); + $this->assertSame('Entity of type \'foo\' was not found', $exception->getMessage()); + } +}