Improve exception message

In setups where you have many parameters, or do not even realise you are using an entity, that additional piece of context can be helpful. The parameter name is not always available where the old exception was called though.
doctrine · Apr 10, 2022 · 46b0342 · 46b0342
1 parent d9e8e83
commit 46b0342
Show file tree

Hide file tree

Showing 6 changed files with 53 additions and 14 deletions.
diff --git a/UPGRADE.md b/UPGRADE.md
@@ -1,5 +1,11 @@
 # Upgrade to 2.12
 
+## Deprecate omitting `$class` argument to `ORMInvalidArgumentException::invalidIdentifierBindingEntity()`
+
+To make it easier to identify understand the cause for that exception, it is
+deprecated to omit the class name when calling
+`ORMInvalidArgumentException::invalidIdentifierBindingEntity()`.
+
 ## Deprecate custom repository classes that don't extend `EntityRepository`
 
 Although undocumented, it is currently possible to configure a custom repository

diff --git a/lib/Doctrine/ORM/AbstractQuery.php b/lib/Doctrine/ORM/AbstractQuery.php
@@ -10,6 +10,7 @@
 use Doctrine\Common\Cache\Psr6\DoctrineProvider;
 use Doctrine\Common\Collections\ArrayCollection;
 use Doctrine\Common\Collections\Collection;
+use Doctrine\Common\Util\ClassUtils;
 use Doctrine\DBAL\Cache\QueryCacheProfile;
 use Doctrine\DBAL\Result;
 use Doctrine\Deprecations\Deprecation;
@@ -434,10 +435,11 @@ public function processParameterValue($value)
         }
 
         try {
+            $class = ClassUtils::getClass($value);
             $value = $this->_em->getUnitOfWork()->getSingleIdentifierValue($value);
 
             if ($value === null) {
-                throw ORMInvalidArgumentException::invalidIdentifierBindingEntity();
+                throw ORMInvalidArgumentException::invalidIdentifierBindingEntity($class);
             }
         } catch (MappingException | ORMMappingException $e) {
             /* Silence any mapping exceptions. These can occur if the object in

diff --git a/lib/Doctrine/ORM/Cache/DefaultCache.php b/lib/Doctrine/ORM/Cache/DefaultCache.php
@@ -296,11 +296,14 @@ private function buildCollectionCacheKey(
      */
     private function toIdentifierArray(ClassMetadata $metadata, $identifier): array
     {
-        if (is_object($identifier) && $this->em->getMetadataFactory()->hasMetadataFor(ClassUtils::getClass($identifier))) {
-            $identifier = $this->uow->getSingleIdentifierValue($identifier);
+        if (is_object($identifier)) {
+            $class = ClassUtils::getClass($identifier);
+            if ($this->em->getMetadataFactory()->hasMetadataFor($class)) {
+                $identifier = $this->uow->getSingleIdentifierValue($identifier);
 
-            if ($identifier === null) {
-                throw ORMInvalidArgumentException::invalidIdentifierBindingEntity();
+                if ($identifier === null) {
+                    throw ORMInvalidArgumentException::invalidIdentifierBindingEntity($class);
+                }
             }
         }
 

diff --git a/lib/Doctrine/ORM/EntityManager.php b/lib/Doctrine/ORM/EntityManager.php
@@ -437,11 +437,14 @@ public function find($className, $id, $lockMode = null, $lockVersion = null)
         }
 
         foreach ($id as $i => $value) {
-            if (is_object($value) && $this->metadataFactory->hasMetadataFor(ClassUtils::getClass($value))) {
-                $id[$i] = $this->unitOfWork->getSingleIdentifierValue($value);
-
-                if ($id[$i] === null) {
-                    throw ORMInvalidArgumentException::invalidIdentifierBindingEntity();
+            if (is_object($value)) {
+                $className = ClassUtils::getClass($value);
+                if ($this->metadataFactory->hasMetadataFor($className)) {
+                    $id[$i] = $this->unitOfWork->getSingleIdentifierValue($value);
+
+                    if ($id[$i] === null) {
+                        throw ORMInvalidArgumentException::invalidIdentifierBindingEntity($className);
+                    }
                 }
             }
         }

diff --git a/lib/Doctrine/ORM/ORMInvalidArgumentException.php b/lib/Doctrine/ORM/ORMInvalidArgumentException.php
@@ -10,6 +10,8 @@
 
 use function array_map;
 use function count;
+use function func_get_arg;
+use function func_num_args;
 use function get_debug_type;
 use function gettype;
 use function implode;
@@ -198,9 +200,27 @@ public static function invalidCompositeIdentifier()
     /**
      * @return ORMInvalidArgumentException
      */
-    public static function invalidIdentifierBindingEntity()
+    public static function invalidIdentifierBindingEntity(/* string $class */)
     {
-        return new self('Binding entities to query parameters only allowed for entities that have an identifier.');
+        if (func_num_args() === 0) {
+            Deprecation::trigger(
+                'doctrine/orm',
+                'https://github.com/doctrine/orm/pull/9642',
+                'Omitting the class name in the exception method %s is deprecated.',
+                __METHOD__
+            );
+
+            return new self('Binding entities to query parameters only allowed for entities that have an identifier.');
+        }
+
+        return new self(sprintf(
+            <<<'EXCEPTION'
+Binding entities to query parameters only allowed for entities that have an identifier.
+Class "%s" does not have an identifier.
+EXCEPTION
+            ,
+            func_get_arg(0)
+        ));
     }
 
     /**

diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC2084Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC2084Test.php
@@ -54,10 +54,15 @@ public function testIssue(): void
         self::assertEquals('Foo', $e->getMyEntity2()->getValue());
     }
 
-    public function testinvalidIdentifierBindingEntityException(): void
+    public function testInvalidIdentifierBindingEntityException(): void
     {
         $this->expectException('Doctrine\ORM\ORMInvalidArgumentException');
-        $this->expectExceptionMessage('Binding entities to query parameters only allowed for entities that have an identifier.');
+        $this->expectExceptionMessage(
+            <<<'EXCEPTION'
+Binding entities to query parameters only allowed for entities that have an identifier.
+Class "Doctrine\Tests\ORM\Functional\Ticket\DDC2084\MyEntity2" does not have an identifier.
+EXCEPTION
+        );
         $this->_em->find(__NAMESPACE__ . '\DDC2084\MyEntity1', new DDC2084\MyEntity2('Foo'));
     }
 }