1
0
mirror of synced 2025-02-02 13:31:45 +03:00

#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:

434820973c/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:

434820973c/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.
This commit is contained in:
Marco Pivetta 2018-12-16 15:37:45 +01:00
parent 237bebe2ed
commit 960a437d46
2 changed files with 28 additions and 9 deletions

View File

@ -2,24 +2,26 @@
namespace Doctrine\Tests\ORM\Query; namespace Doctrine\Tests\ORM\Query;
use DateTime;
use Doctrine\Common\Cache\ArrayCache; use Doctrine\Common\Cache\ArrayCache;
use Doctrine\Common\Collections\ArrayCollection; use Doctrine\Common\Collections\ArrayCollection;
use Doctrine\DBAL\Types\Type;
use Doctrine\DBAL\Cache\QueryCacheProfile;
use Doctrine\ORM\EntityManager;
use Doctrine\ORM\Internal\Hydration\IterableResult; use Doctrine\ORM\Internal\Hydration\IterableResult;
use Doctrine\ORM\Query\Parameter; use Doctrine\ORM\Query\Parameter;
use Doctrine\ORM\Query\QueryException; use Doctrine\ORM\Query\QueryException;
use Doctrine\ORM\UnitOfWork;
use Doctrine\Tests\Mocks\DriverConnectionMock; use Doctrine\Tests\Mocks\DriverConnectionMock;
use Doctrine\Tests\Mocks\EntityManagerMock;
use Doctrine\Tests\Mocks\StatementArrayMock; use Doctrine\Tests\Mocks\StatementArrayMock;
use Doctrine\Tests\Models\CMS\CmsAddress; use Doctrine\Tests\Models\CMS\CmsAddress;
use Doctrine\Tests\Models\CMS\CmsUser; use Doctrine\Tests\Models\CMS\CmsUser;
use Doctrine\Tests\Models\Generic\DateTimeModel;
use Doctrine\Tests\OrmTestCase; use Doctrine\Tests\OrmTestCase;
class QueryTest extends OrmTestCase class QueryTest extends OrmTestCase
{ {
/** @var EntityManager */ /** @var EntityManagerMock */
protected $_em = null; protected $_em;
protected function setUp() protected function setUp()
{ {
@ -400,4 +402,22 @@ class QueryTest extends OrmTestCase
self::assertAttributeSame(null, '_queryCacheProfile', $query); 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());
}
} }

View File

@ -11,6 +11,7 @@ use Doctrine\ORM\Cache\DefaultCacheFactory;
use Doctrine\ORM\Configuration; use Doctrine\ORM\Configuration;
use Doctrine\ORM\Mapping\Driver\AnnotationDriver; use Doctrine\ORM\Mapping\Driver\AnnotationDriver;
use Doctrine\Tests\Mocks; use Doctrine\Tests\Mocks;
use Doctrine\Tests\Mocks\EntityManagerMock;
/** /**
* Base testcase class for all ORM testcases. * Base testcase class for all ORM testcases.
@ -113,10 +114,8 @@ abstract class OrmTestCase extends DoctrineTestCase
* @param mixed $conf * @param mixed $conf
* @param \Doctrine\Common\EventManager|null $eventManager * @param \Doctrine\Common\EventManager|null $eventManager
* @param bool $withSharedMetadata * @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 $metadataCache = $withSharedMetadata
? self::getSharedMetadataCacheImpl() ? self::getSharedMetadataCacheImpl()
@ -160,7 +159,7 @@ abstract class OrmTestCase extends DoctrineTestCase
$conn = DriverManager::getConnection($conn, $config, $eventManager); $conn = DriverManager::getConnection($conn, $config, $eventManager);
} }
return Mocks\EntityManagerMock::create($conn, $config, $eventManager); return EntityManagerMock::create($conn, $config, $eventManager);
} }
protected function enableSecondLevelCache($log = true) protected function enableSecondLevelCache($log = true)