From 960a437d46e6bc0df5049da0ae5e9a9f5d067516 Mon Sep 17 00:00:00 2001
From: Marco Pivetta <ocramius@gmail.com>
Date: Sun, 16 Dec 2018 15:37:45 +0100
Subject: [PATCH] #7527 failing test case:
 `UnitOfWork#getSingleIdentifierValue()` should not be called for a well
 specified parameter type

As previously reported by @flaushi in https://github.com/doctrine/doctrine2/pull/7471#discussion_r241949045, we discovered
that binding a parameter causes a `ClassMetadataFactory#getClassMetadata()` call, which in turn leads to large performance
regression when using any `object` type as parameter.

Following two snippets lead to an internal `ClassMetadataFactory#getClassMetadata()` call, which in turn leads to an
exception being thrown and garbage collected, plus multiple associated performance implications:

```php
$query->setParameter('foo', new DateTime());
$query->getResult();
```

```php
$query->setParameter('foo', new DateTime(), DateTimeType::NAME);
$query->getResult();
```

This is due to following portion of code:

https://github.com/doctrine/doctrine2/blob/434820973cadf2da2d66e7184be370084cc32ca8/lib/Doctrine/ORM/Query.php#L406-L409

Notice how `$value = $this->processParameterValue($value);` happens before attempting to infer the type for the parameter value.

That call leads to this segment being reached, which leads to the regression:

https://github.com/doctrine/doctrine2/blob/434820973cadf2da2d66e7184be370084cc32ca8/lib/Doctrine/ORM/AbstractQuery.php#L423-L433

Assuming the bound parameter type is provided, we can completely skip attempting to introspect the given object:

```php
$query->setParameter('foo', new DateTime(), DateTimeType::NAME);
$query->getResult();
```

Processing the parameter value is not needed in this case, so we can safely skip that logic for all known parameters.
In order to not introduce a BC break or change the `AbstractQuery#processParameterValue()` implementation, we could filter
out all parameters for which the type is given upfront, and later on merge them back in instead.

The test expectation to be set is for `UnitOfWork#getSingleIdentifierValue()` to never be called.
---
 tests/Doctrine/Tests/ORM/Query/QueryTest.php | 30 ++++++++++++++++----
 tests/Doctrine/Tests/OrmTestCase.php         |  7 ++---
 2 files changed, 28 insertions(+), 9 deletions(-)

diff --git a/tests/Doctrine/Tests/ORM/Query/QueryTest.php b/tests/Doctrine/Tests/ORM/Query/QueryTest.php
index 8a3caf66f05..0045647a09a 100644
--- a/tests/Doctrine/Tests/ORM/Query/QueryTest.php
+++ b/tests/Doctrine/Tests/ORM/Query/QueryTest.php
@@ -2,24 +2,26 @@
 
 namespace Doctrine\Tests\ORM\Query;
 
+use DateTime;
 use Doctrine\Common\Cache\ArrayCache;
 use Doctrine\Common\Collections\ArrayCollection;
-
-use Doctrine\DBAL\Cache\QueryCacheProfile;
-use Doctrine\ORM\EntityManager;
+use Doctrine\DBAL\Types\Type;
 use Doctrine\ORM\Internal\Hydration\IterableResult;
 use Doctrine\ORM\Query\Parameter;
 use Doctrine\ORM\Query\QueryException;
+use Doctrine\ORM\UnitOfWork;
 use Doctrine\Tests\Mocks\DriverConnectionMock;
+use Doctrine\Tests\Mocks\EntityManagerMock;
 use Doctrine\Tests\Mocks\StatementArrayMock;
 use Doctrine\Tests\Models\CMS\CmsAddress;
 use Doctrine\Tests\Models\CMS\CmsUser;
+use Doctrine\Tests\Models\Generic\DateTimeModel;
 use Doctrine\Tests\OrmTestCase;
 
 class QueryTest extends OrmTestCase
 {
-    /** @var EntityManager */
-    protected $_em = null;
+    /** @var EntityManagerMock */
+    protected $_em;
 
     protected function setUp()
     {
@@ -400,4 +402,22 @@ public function testResultCacheProfileCanBeRemovedViaSetter() : void
 
         self::assertAttributeSame(null, '_queryCacheProfile', $query);
     }
+
+    /** @group 7527 */
+    public function testValuesAreNotBeingResolvedForSpecifiedParameterTypes() : void
+    {
+        $unitOfWork = $this->createMock(UnitOfWork::class);
+
+        $this->_em->setUnitOfWork($unitOfWork);
+
+        $unitOfWork
+            ->expects(self::never())
+            ->method('getSingleIdentifierValue');
+
+        $query = $this->_em->createQuery('SELECT d FROM ' . DateTimeModel::class . ' d WHERE d.datetime = :value');
+
+        $query->setParameter('value', new DateTime(), Type::DATETIME);
+
+        self::assertEmpty($query->getResult());
+    }
 }
diff --git a/tests/Doctrine/Tests/OrmTestCase.php b/tests/Doctrine/Tests/OrmTestCase.php
index 46e8ea143ee..6fdbe147110 100644
--- a/tests/Doctrine/Tests/OrmTestCase.php
+++ b/tests/Doctrine/Tests/OrmTestCase.php
@@ -11,6 +11,7 @@
 use Doctrine\ORM\Configuration;
 use Doctrine\ORM\Mapping\Driver\AnnotationDriver;
 use Doctrine\Tests\Mocks;
+use Doctrine\Tests\Mocks\EntityManagerMock;
 
 /**
  * Base testcase class for all ORM testcases.
@@ -113,10 +114,8 @@ protected function createAnnotationDriver($paths = [], $alias = null)
      * @param mixed                              $conf
      * @param \Doctrine\Common\EventManager|null $eventManager
      * @param bool                               $withSharedMetadata
-     *
-     * @return \Doctrine\ORM\EntityManager
      */
-    protected function _getTestEntityManager($conn = null, $conf = null, $eventManager = null, $withSharedMetadata = true)
+    protected function _getTestEntityManager($conn = null, $conf = null, $eventManager = null, $withSharedMetadata = true) : EntityManagerMock
     {
         $metadataCache = $withSharedMetadata
             ? self::getSharedMetadataCacheImpl()
@@ -160,7 +159,7 @@ protected function _getTestEntityManager($conn = null, $conf = null, $eventManag
             $conn = DriverManager::getConnection($conn, $config, $eventManager);
         }
 
-        return Mocks\EntityManagerMock::create($conn, $config, $eventManager);
+        return EntityManagerMock::create($conn, $config, $eventManager);
     }
 
     protected function enableSecondLevelCache($log = true)