Skip to content

Commit

Permalink
Use PSR-6 for accessing the query cache (#9004)
Browse files Browse the repository at this point in the history
  • Loading branch information
derrabus authored Sep 13, 2021
1 parent 31d8bd7 commit 6371081
Show file tree
Hide file tree
Showing 5 changed files with 106 additions and 58 deletions.
68 changes: 49 additions & 19 deletions lib/Doctrine/ORM/Query.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,13 @@
namespace Doctrine\ORM;

use Doctrine\Common\Cache\Cache;
use Doctrine\Common\Cache\Psr6\CacheAdapter;
use Doctrine\Common\Cache\Psr6\DoctrineProvider;
use Doctrine\Common\Collections\ArrayCollection;
use Doctrine\DBAL\Cache\QueryCacheProfile;
use Doctrine\DBAL\LockMode;
use Doctrine\DBAL\Types\Type;
use Doctrine\Deprecations\Deprecation;
use Doctrine\ORM\Internal\Hydration\IterableResult;
use Doctrine\ORM\Mapping\ClassMetadata;
use Doctrine\ORM\Query\AST\DeleteStatement;
Expand Down Expand Up @@ -157,7 +159,7 @@ final class Query extends AbstractQuery
/**
* The cache driver used for caching queries.
*
* @var Cache|null
* @var CacheItemPoolInterface|null
*/
private $queryCache;

Expand Down Expand Up @@ -241,7 +243,7 @@ private function parse(): ParserResult
$this->_state = self::STATE_CLEAN;
$this->parsedTypes = $types;

$queryCache = $this->getQueryCacheDriver();
$queryCache = $this->queryCache ?? $this->_em->getConfiguration()->getQueryCache();
// Check query cache.
if (! ($this->useQueryCache && $queryCache)) {
$parser = new Parser($this);
Expand All @@ -251,22 +253,24 @@ private function parse(): ParserResult
return $this->parserResult;
}

$hash = $this->getQueryCacheId();
$cached = $this->expireQueryCache ? false : $queryCache->fetch($hash);
$cacheItem = $queryCache->getItem($this->getQueryCacheId());

if ($cached instanceof ParserResult) {
// Cache hit.
$this->parserResult = $cached;
if (! $this->expireQueryCache && $cacheItem->isHit()) {
$cached = $cacheItem->get();
if ($cached instanceof ParserResult) {
// Cache hit.
$this->parserResult = $cached;

return $this->parserResult;
return $this->parserResult;
}
}

// Cache miss.
$parser = new Parser($this);

$this->parserResult = $parser->parse();

$queryCache->save($hash, $this->parserResult, $this->queryCacheTTL);
$queryCache->save($cacheItem->set($this->parserResult)->expiresAfter($this->queryCacheTTL));

return $this->parserResult;
}
Expand Down Expand Up @@ -461,11 +465,32 @@ private function resolveParameterValue(Parameter $parameter): array
/**
* Defines a cache driver to be used for caching queries.
*
* @deprecated Call {@see setQueryCache()} instead.
*
* @param Cache|null $queryCache Cache driver.
*
* @return self This query instance.
* @return $this
*/
public function setQueryCacheDriver($queryCache): self
{
Deprecation::trigger(
'doctrine/orm',
'https://github.com/doctrine/orm/pull/9004',
'%s is deprecated and will be removed in Doctrine 3.0. Use setQueryCache() instead.',
__METHOD__
);

$this->queryCache = $queryCache ? CacheAdapter::wrap($queryCache) : null;

return $this;
}

/**
* Defines a cache driver to be used for caching queries.
*
* @return $this
*/
public function setQueryCache(?CacheItemPoolInterface $queryCache): self
{
$this->queryCache = $queryCache;

Expand All @@ -477,7 +502,7 @@ public function setQueryCacheDriver($queryCache): self
*
* @param bool $bool
*
* @return self This query instance.
* @return $this
*/
public function useQueryCache($bool): self
{
Expand All @@ -489,16 +514,21 @@ public function useQueryCache($bool): self
/**
* Returns the cache driver used for query caching.
*
* @deprecated
*
* @return Cache|null The cache driver used for query caching or NULL, if
* this Query does not use query caching.
*/
public function getQueryCacheDriver(): ?Cache
{
if ($this->queryCache) {
return $this->queryCache;
}
Deprecation::trigger(
'doctrine/orm',
'https://github.com/doctrine/orm/pull/9004',
'%s is deprecated and will be removed in Doctrine 3.0 without replacement.',
__METHOD__
);

$queryCache = $this->_em->getConfiguration()->getQueryCache();
$queryCache = $this->queryCache ?? $this->_em->getConfiguration()->getQueryCache();

return $queryCache ? DoctrineProvider::wrap($queryCache) : null;
}
Expand All @@ -508,7 +538,7 @@ public function getQueryCacheDriver(): ?Cache
*
* @param int $timeToLive How long the cache entry is valid.
*
* @return self This query instance.
* @return $this
*/
public function setQueryCacheLifetime($timeToLive): self
{
Expand All @@ -534,7 +564,7 @@ public function getQueryCacheLifetime(): ?int
*
* @param bool $expire Whether or not to force query cache expiration.
*
* @return self This query instance.
* @return $this
*/
public function expireQueryCache($expire = true): self
{
Expand Down Expand Up @@ -615,7 +645,7 @@ public function contains($dql): bool
*
* @param int|null $firstResult The first result to return.
*
* @return self This query object.
* @return $this
*/
public function setFirstResult($firstResult): self
{
Expand All @@ -641,7 +671,7 @@ public function getFirstResult(): ?int
*
* @param int|null $maxResults
*
* @return self This query object.
* @return $this
*/
public function setMaxResults($maxResults): self
{
Expand Down
62 changes: 34 additions & 28 deletions tests/Doctrine/Tests/ORM/Functional/QueryCacheTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,11 @@

namespace Doctrine\Tests\ORM\Functional;

use Doctrine\Common\Cache\Cache;
use Doctrine\Common\Cache\CacheProvider;
use Doctrine\Common\Cache\Psr6\DoctrineProvider;
use Doctrine\ORM\Query;
use Doctrine\ORM\Query\Exec\AbstractSqlExecutor;
use Doctrine\ORM\Query\ParserResult;
use Doctrine\Tests\OrmFunctionalTestCase;
use Psr\Cache\CacheItemInterface;
use Psr\Cache\CacheItemPoolInterface;
use Symfony\Component\Cache\Adapter\ArrayAdapter;

Expand All @@ -31,15 +29,15 @@ public function testQueryCacheDependsOnHints(): array
$query = $this->_em->createQuery('select ux from Doctrine\Tests\Models\CMS\CmsUser ux');

$cache = new ArrayAdapter();
$query->setQueryCacheDriver(DoctrineProvider::wrap($cache));
$query->setQueryCache($cache);

$query->getResult();
self::assertCount(2, $cache->getValues());
self::assertCount(1, $cache->getValues());

$query->setHint('foo', 'bar');

$query->getResult();
self::assertCount(3, $cache->getValues());
self::assertCount(2, $cache->getValues());

return [$query, $cache];
}
Expand Down Expand Up @@ -100,14 +98,25 @@ public function testQueryCacheNoHitSaveParserResult(): void

$query = $this->_em->createQuery('select ux from Doctrine\Tests\Models\CMS\CmsUser ux');

$cache = $this->createMock(Cache::class);
$cache = $this->createMock(CacheItemPoolInterface::class);
$query->setQueryCache($cache);

$cacheItem = $this->createMock(CacheItemInterface::class);
$cacheItem->method('isHit')->willReturn(false);
$cacheItem->expects(self::never())->method('get');
$cacheItem->expects(self::once())->method('set')->with(self::isInstanceOf(ParserResult::class))->willReturnSelf();
$cacheItem->method('expiresAfter')->willReturnSelf();

$query->setQueryCacheDriver($cache);
$cache->expects(self::once())
->method('getItem')
->with(self::isType('string'))
->willReturn($cacheItem);

$cache
->expects(self::once())
->method('save')
->with(self::isType('string'), self::isInstanceOf(ParserResult::class));
->with(self::identicalTo($cacheItem))
->willReturn(true);

$query->getResult();
}
Expand All @@ -124,34 +133,31 @@ public function testQueryCacheHitDoesNotSaveParserResult(): void

$sqlExecMock->expects(self::once())
->method('execute')
->will(self::returnValue(10));
->willReturn(10);

$parserResultMock = $this->getMockBuilder(ParserResult::class)
->setMethods(['getSqlExecutor'])
->getMock();
$parserResultMock->expects(self::once())
->method('getSqlExecutor')
->will(self::returnValue($sqlExecMock));

$cache = $this->getMockBuilder(CacheProvider::class)
->setMethods(['doFetch', 'doContains', 'doSave', 'doDelete', 'doFlush', 'doGetStats'])
->getMock();

$cache->expects(self::exactly(2))
->method('doFetch')
->withConsecutive(
[self::isType('string')],
[self::isType('string')]
)
->willReturnOnConsecutiveCalls(
self::returnValue(1),
self::returnValue($parserResultMock)
);
->willReturn($sqlExecMock);

$cache = $this->createMock(CacheItemPoolInterface::class);

$cacheItem = $this->createMock(CacheItemInterface::class);
$cacheItem->method('isHit')->willReturn(true);
$cacheItem->method('get')->willReturn($parserResultMock);
$cacheItem->expects(self::never())->method('set');

$cache->expects(self::once())
->method('getItem')
->with(self::isType('string'))
->willReturn($cacheItem);

$cache->expects(self::never())
->method('doSave');
->method('save');

$query->setQueryCacheDriver($cache);
$query->setQueryCache($cache);

$query->getResult();
}
Expand Down
9 changes: 4 additions & 5 deletions tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

namespace Doctrine\Tests\ORM\Functional;

use Doctrine\Common\Cache\Psr6\DoctrineProvider;
use Doctrine\DBAL\Connection;
use Doctrine\DBAL\Types\Types;
use Doctrine\ORM\Configuration;
Expand Down Expand Up @@ -399,21 +398,21 @@ public function testQueryCacheDependsOnFilters(): void
$query = $this->_em->createQuery('select ux from Doctrine\Tests\Models\CMS\CmsUser ux');

$cache = new ArrayAdapter();
$query->setQueryCacheDriver(DoctrineProvider::wrap($cache));
$query->setQueryCache($cache);

$query->getResult();
self::assertCount(2, $cache->getValues());
self::assertCount(1, $cache->getValues());

$conf = $this->_em->getConfiguration();
$conf->addFilter('locale', MyLocaleFilter::class);
$this->_em->getFilters()->enable('locale');

$query->getResult();
self::assertCount(3, $cache->getValues());
self::assertCount(2, $cache->getValues());

// Another time doesn't add another cache entry
$query->getResult();
self::assertCount(3, $cache->getValues());
self::assertCount(2, $cache->getValues());
}

public function testQueryGenerationDependsOnFilters(): void
Expand Down
3 changes: 1 addition & 2 deletions tests/Doctrine/Tests/ORM/Functional/Ticket/DDC2224Test.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

namespace Doctrine\Tests\ORM\Functional\Ticket;

use Doctrine\Common\Cache\Psr6\DoctrineProvider;
use Doctrine\DBAL\Platforms\AbstractPlatform;
use Doctrine\DBAL\Types\Type;
use Doctrine\ORM\Mapping\Column;
Expand All @@ -31,7 +30,7 @@ public function testIssue(): Query
{
$dql = 'SELECT e FROM ' . __NAMESPACE__ . '\DDC2224Entity e WHERE e.field = :field';
$query = $this->_em->createQuery($dql);
$query->setQueryCacheDriver(DoctrineProvider::wrap(new ArrayAdapter()));
$query->setQueryCache(new ArrayAdapter());

$query->setParameter('field', 'test', 'DDC2224Type');
self::assertStringEndsWith('.field = FUNCTION(?)', $query->getSQL());
Expand Down
22 changes: 18 additions & 4 deletions tests/Doctrine/Tests/ORM/Query/QueryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
use Doctrine\Tests\Models\Generic\DateTimeModel;
use Doctrine\Tests\OrmTestCase;
use Generator;
use Psr\Cache\CacheItemPoolInterface;
use Symfony\Component\Cache\Adapter\ArrayAdapter;

use function assert;
Expand Down Expand Up @@ -106,12 +107,14 @@ public function testFluentQueryInterface(): void
$q2 = $q->expireQueryCache(true)
->setQueryCacheLifetime(3600)
->setQueryCacheDriver(null)
->setQueryCache(null)
->expireResultCache(true)
->setHint('foo', 'bar')
->setHint('bar', 'baz')
->setParameter(1, 'bar')
->setParameters(new ArrayCollection([new Parameter(2, 'baz')]))
->setResultCacheDriver(null)
->setResultCache(null)
->setResultCacheId('foo')
->setDQL('foo')
->setFirstResult(10)
Expand Down Expand Up @@ -554,15 +557,15 @@ public function testGetParameterColonNormalize(): void

public function testGetQueryCacheDriverWithDefaults(): void
{
$cache = $this->createMock(Cache::class);
$cache = $this->createMock(CacheItemPoolInterface::class);

$this->entityManager->getConfiguration()->setQueryCacheImpl($cache);
$this->entityManager->getConfiguration()->setQueryCache($cache);
$query = $this->entityManager->createQuery('select u from ' . CmsUser::class . ' u');

self::assertSame($cache, $query->getQueryCacheDriver());
self::assertSame($cache, CacheAdapter::wrap($query->getQueryCacheDriver()));
}

public function testGetQueryCacheDriverWithCacheExplicitlySet(): void
public function testGetQueryCacheDriverWithCacheExplicitlySetLegacy(): void
{
$cache = $this->createMock(Cache::class);

Expand All @@ -572,4 +575,15 @@ public function testGetQueryCacheDriverWithCacheExplicitlySet(): void

self::assertSame($cache, $query->getQueryCacheDriver());
}

public function testGetQueryCacheDriverWithCacheExplicitlySet(): void
{
$cache = $this->createMock(CacheItemPoolInterface::class);

$query = $this->entityManager
->createQuery('select u from ' . CmsUser::class . ' u')
->setQueryCache($cache);

self::assertSame($cache, CacheAdapter::wrap($query->getQueryCacheDriver()));
}
}

0 comments on commit 6371081

Please sign in to comment.