Skip to content

Commit

Permalink
Merge branch 'fix/#6001-second-level-cache-query-cache-timestamp-from…
Browse files Browse the repository at this point in the history 
…-region'

Close #6001
  • Loading branch information
@Ocramius
Ocramius committed Sep 8, 2016
2 parents 5eebdcf + d27cffa commit 009e947
Show file tree
Hide file tree
Showing 10 changed files with 163 additions and 50 deletions.
6 changes: 6 additions & 0 deletions UPGRADE.md
View file
@@ -1,5 +1,11 @@
# Upgrade to 2.5

## Minor BC BREAK: query cache key time is now a float

As of 2.5.5, the `QueryCacheEntry#time` property will contain a float value
instead of an integer in order to have more precision and also to be consistent
with the `TimestampCacheEntry#time`.

## Minor BC BREAK: discriminator map must now include all non-transient classes

It is now required that you declare the root of an inheritance in the
Expand Down
35 changes: 29 additions & 6 deletions lib/Doctrine/ORM/AbstractQuery.php
View file
Expand Up @@ -28,6 +28,7 @@
use Doctrine\DBAL\Cache\QueryCacheProfile;

use Doctrine\ORM\Cache;
use Doctrine\ORM\Query\ResultSetMapping;

/**
* Base contract for ORM queries. Base class for Query and NativeQuery.
Expand Down Expand Up @@ -991,32 +992,54 @@ private function executeIgnoreQueryCache($parameters = null, $hydrationMode = nu
private function executeUsingQueryCache($parameters = null, $hydrationMode = null)
{
$rsm = $this->getResultSetMapping();
$querykey = new QueryCacheKey($this->getHash(), $this->lifetime, $this->cacheMode ?: Cache::MODE_NORMAL);
$queryCache = $this->_em->getCache()->getQueryCache($this->cacheRegion);
$result = $queryCache->get($querykey, $rsm, $this->_hints);
$queryKey = new QueryCacheKey(
$this->getHash(),
$this->lifetime,
$this->cacheMode ?: Cache::MODE_NORMAL,
$this->getTimestampKey()
);

$result = $queryCache->get($queryKey, $rsm, $this->_hints);

if ($result !== null) {
if ($this->cacheLogger) {
$this->cacheLogger->queryCacheHit($queryCache->getRegion()->getName(), $querykey);
$this->cacheLogger->queryCacheHit($queryCache->getRegion()->getName(), $queryKey);
}

return $result;
}

$result = $this->executeIgnoreQueryCache($parameters, $hydrationMode);
$cached = $queryCache->put($querykey, $rsm, $result, $this->_hints);
$cached = $queryCache->put($queryKey, $rsm, $result, $this->_hints);

if ($this->cacheLogger) {
$this->cacheLogger->queryCacheMiss($queryCache->getRegion()->getName(), $querykey);
$this->cacheLogger->queryCacheMiss($queryCache->getRegion()->getName(), $queryKey);

if ($cached) {
$this->cacheLogger->queryCachePut($queryCache->getRegion()->getName(), $querykey);
$this->cacheLogger->queryCachePut($queryCache->getRegion()->getName(), $queryKey);
}
}

return $result;
}

/**
* @return \Doctrine\ORM\Cache\TimestampCacheKey|null
*/
private function getTimestampKey()
{
$entityName = reset($this->_resultSetMapping->aliasMap);

if (empty($entityName)) {
return null;
}

$metadata = $this->_em->getClassMetadata($entityName);

return new Cache\TimestampCacheKey($metadata->getTableName());
}

/**
* Get the result cache id to use to store the result set cache entry.
* Will return the configured id if it exists otherwise a hash will be
Expand Down
4 changes: 3 additions & 1 deletion lib/Doctrine/ORM/Cache/CacheConfiguration.php
View file
Expand Up @@ -110,7 +110,9 @@ public function setRegionsConfiguration(RegionsConfiguration $regionsConfig)
public function getQueryValidator()
{
if ($this->queryValidator === null) {
$this->queryValidator = new TimestampQueryCacheValidator();
$this->queryValidator = new TimestampQueryCacheValidator(
$this->cacheFactory->getTimestampRegion()
);
}

return $this->queryValidator;
Expand Down
50 changes: 23 additions & 27 deletions lib/Doctrine/ORM/Cache/Persister/Entity/AbstractEntityPersister.php
View file
Expand Up @@ -287,17 +287,16 @@ private function storeJoinedAssociations($entity)
* @param array $orderBy
* @param integer $limit
* @param integer $offset
* @param integer $timestamp
*
* @return string
*/
protected function getHash($query, $criteria, array $orderBy = null, $limit = null, $offset = null, $timestamp = null)
protected function getHash($query, $criteria, array $orderBy = null, $limit = null, $offset = null)
{
list($params) = ($criteria instanceof Criteria)
? $this->persister->expandCriteriaParameters($criteria)
: $this->persister->expandParameters($criteria);

return sha1($query . serialize($params) . serialize($orderBy) . $limit . $offset . $timestamp);
return sha1($query . serialize($params) . serialize($orderBy) . $limit . $offset);
}

/**
Expand Down Expand Up @@ -368,17 +367,16 @@ public function load(array $criteria, $entity = null, $assoc = null, array $hint
}

//handle only EntityRepository#findOneBy
$timestamp = $this->timestampRegion->get($this->timestampKey);
$query = $this->persister->getSelectSQL($criteria, null, null, $limit, null, $orderBy);
$hash = $this->getHash($query, $criteria, null, null, null, $timestamp ? $timestamp->time : null);
$hash = $this->getHash($query, $criteria, null, null, null);
$rsm = $this->getResultSetMapping();
$querykey = new QueryCacheKey($hash, 0, Cache::MODE_NORMAL);
$queryKey = new QueryCacheKey($hash, 0, Cache::MODE_NORMAL, $this->timestampKey);
$queryCache = $this->cache->getQueryCache($this->regionName);
$result = $queryCache->get($querykey, $rsm);
$result = $queryCache->get($queryKey, $rsm);

if ($result !== null) {
if ($this->cacheLogger) {
$this->cacheLogger->queryCacheHit($this->regionName, $querykey);
$this->cacheLogger->queryCacheHit($this->regionName, $queryKey);
}

return $result[0];
Expand All @@ -388,15 +386,15 @@ public function load(array $criteria, $entity = null, $assoc = null, array $hint
return null;
}

$cached = $queryCache->put($querykey, $rsm, array($result));
$cached = $queryCache->put($queryKey, $rsm, array($result));

if ($this->cacheLogger) {
if ($result) {
$this->cacheLogger->queryCacheMiss($this->regionName, $querykey);
$this->cacheLogger->queryCacheMiss($this->regionName, $queryKey);
}

if ($cached) {
$this->cacheLogger->queryCachePut($this->regionName, $querykey);
$this->cacheLogger->queryCachePut($this->regionName, $queryKey);
}
}

Expand All @@ -408,32 +406,31 @@ public function load(array $criteria, $entity = null, $assoc = null, array $hint
*/
public function loadAll(array $criteria = array(), array $orderBy = null, $limit = null, $offset = null)
{
$timestamp = $this->timestampRegion->get($this->timestampKey);
$query = $this->persister->getSelectSQL($criteria, null, null, $limit, $offset, $orderBy);
$hash = $this->getHash($query, $criteria, null, null, null, $timestamp ? $timestamp->time : null);
$hash = $this->getHash($query, $criteria, null, null, null);
$rsm = $this->getResultSetMapping();
$querykey = new QueryCacheKey($hash, 0, Cache::MODE_NORMAL);
$queryKey = new QueryCacheKey($hash, 0, Cache::MODE_NORMAL, $this->timestampKey);
$queryCache = $this->cache->getQueryCache($this->regionName);
$result = $queryCache->get($querykey, $rsm);
$result = $queryCache->get($queryKey, $rsm);

if ($result !== null) {
if ($this->cacheLogger) {
$this->cacheLogger->queryCacheHit($this->regionName, $querykey);
$this->cacheLogger->queryCacheHit($this->regionName, $queryKey);
}

return $result;
}

$result = $this->persister->loadAll($criteria, $orderBy, $limit, $offset);
$cached = $queryCache->put($querykey, $rsm, $result);
$cached = $queryCache->put($queryKey, $rsm, $result);

if ($this->cacheLogger) {
if ($result) {
$this->cacheLogger->queryCacheMiss($this->regionName, $querykey);
$this->cacheLogger->queryCacheMiss($this->regionName, $queryKey);
}

if ($cached) {
$this->cacheLogger->queryCachePut($this->regionName, $querykey);
$this->cacheLogger->queryCachePut($this->regionName, $queryKey);
}
}

Expand Down Expand Up @@ -511,31 +508,30 @@ public function loadCriteria(Criteria $criteria)
$limit = $criteria->getMaxResults();
$offset = $criteria->getFirstResult();
$query = $this->persister->getSelectSQL($criteria);
$timestamp = $this->timestampRegion->get($this->timestampKey);
$hash = $this->getHash($query, $criteria, $orderBy, $limit, $offset, $timestamp ? $timestamp->time : null);
$hash = $this->getHash($query, $criteria, $orderBy, $limit, $offset);
$rsm = $this->getResultSetMapping();
$querykey = new QueryCacheKey($hash, 0, Cache::MODE_NORMAL);
$queryKey = new QueryCacheKey($hash, 0, Cache::MODE_NORMAL, $this->timestampKey);
$queryCache = $this->cache->getQueryCache($this->regionName);
$cacheResult = $queryCache->get($querykey, $rsm);
$cacheResult = $queryCache->get($queryKey, $rsm);

if ($cacheResult !== null) {
if ($this->cacheLogger) {
$this->cacheLogger->queryCacheHit($this->regionName, $querykey);
$this->cacheLogger->queryCacheHit($this->regionName, $queryKey);
}

return $cacheResult;
}

$result = $this->persister->loadCriteria($criteria);
$cached = $queryCache->put($querykey, $rsm, $result);
$cached = $queryCache->put($queryKey, $rsm, $result);

if ($this->cacheLogger) {
if ($result) {
$this->cacheLogger->queryCacheMiss($this->regionName, $querykey);
$this->cacheLogger->queryCacheMiss($this->regionName, $queryKey);
}

if ($cached) {
$this->cacheLogger->queryCachePut($this->regionName, $querykey);
$this->cacheLogger->queryCachePut($this->regionName, $queryKey);
}
}

Expand Down
8 changes: 4 additions & 4 deletions lib/Doctrine/ORM/Cache/QueryCacheEntry.php
View file
Expand Up @@ -38,18 +38,18 @@ class QueryCacheEntry implements CacheEntry
/**
* READ-ONLY: Public only for performance reasons, it should be considered immutable.
*
* @var integer Time creation of this cache entry
* @var float Time creation of this cache entry
*/
public $time;

/**
* @param array $result
* @param integer $time
* @param array $result
* @param float $time
*/
public function __construct($result, $time = null)
{
$this->result = $result;
$this->time = $time ?: time();
$this->time = $time ?: microtime(true);
}

/**
Expand Down
29 changes: 21 additions & 8 deletions lib/Doctrine/ORM/Cache/QueryCacheKey.php
View file
Expand Up @@ -45,14 +45,27 @@ class QueryCacheKey extends CacheKey
public $cacheMode;

/**
* @param string $hash Result cache id
* @param integer $lifetime Query lifetime
* @param integer $cacheMode Query cache mode
* READ-ONLY: Public only for performance reasons, it should be considered immutable.
*
* @var TimestampCacheKey|null
*/
public $timestampKey;

/**
* @param string $hash Result cache id
* @param integer $lifetime Query lifetime
* @param int $cacheMode Query cache mode
* @param TimestampCacheKey|null $timestampKey
*/
public function __construct($hash, $lifetime = 0, $cacheMode = Cache::MODE_NORMAL)
{
$this->hash = $hash;
$this->lifetime = $lifetime;
$this->cacheMode = $cacheMode;
public function __construct(
$hash,
$lifetime = 0,
$cacheMode = Cache::MODE_NORMAL,
TimestampCacheKey $timestampKey = null
) {
$this->hash = $hash;
$this->lifetime = $lifetime;
$this->cacheMode = $cacheMode;
$this->timestampKey = $timestampKey;
}
}
36 changes: 35 additions & 1 deletion lib/Doctrine/ORM/Cache/TimestampQueryCacheValidator.php
View file
Expand Up @@ -26,15 +26,49 @@
*/
class TimestampQueryCacheValidator implements QueryCacheValidator
{
/**
* @var TimestampRegion
*/
private $timestampRegion;

/**
* @param TimestampRegion $timestampRegion
*/
public function __construct(TimestampRegion $timestampRegion)
{
$this->timestampRegion = $timestampRegion;
}

/**
* {@inheritdoc}
*/
public function isValid(QueryCacheKey $key, QueryCacheEntry $entry)
{
if ($this->regionUpdated($key, $entry)) {
return false;
}

if ($key->lifetime == 0) {
return true;
}

return ($entry->time + $key->lifetime) > time();
return ($entry->time + $key->lifetime) > microtime(true);
}

/**
* @param QueryCacheKey $key
* @param QueryCacheEntry $entry
*
* @return bool
*/
private function regionUpdated(QueryCacheKey $key, QueryCacheEntry $entry)
{
if ($key->timestampKey === null) {
return false;
}

$timestamp = $this->timestampRegion->get($key->timestampKey);

return $timestamp && $timestamp->time > $entry->time;
}
}
8 changes: 7 additions & 1 deletion tests/Doctrine/Tests/ORM/Cache/CacheConfigTest.php
View file
Expand Up @@ -6,6 +6,7 @@
use Doctrine\ORM\Cache\CacheFactory;
use Doctrine\ORM\Cache\QueryCacheValidator;
use Doctrine\ORM\Cache\Logging\CacheLogger;
use Doctrine\ORM\Cache\TimestampRegion;
use Doctrine\Tests\DoctrineTestCase;

/**
Expand Down Expand Up @@ -67,6 +68,11 @@ public function testSetGetCacheFactory()

public function testSetGetQueryValidator()
{
$factory = $this->createMock(CacheFactory::class);
$factory->method('getTimestampRegion')->willReturn($this->createMock(TimestampRegion::class));

$this->config->setCacheFactory($factory);

$validator = $this->createMock(QueryCacheValidator::class);

$this->assertInstanceOf('Doctrine\ORM\Cache\TimestampQueryCacheValidator', $this->config->getQueryValidator());
Expand All @@ -75,4 +81,4 @@ public function testSetGetQueryValidator()

$this->assertEquals($validator, $this->config->getQueryValidator());
}
}
}
2 changes: 1 addition & 1 deletion tests/Doctrine/Tests/ORM/Cache/DefaultQueryCacheTest.php
View file
Expand Up @@ -436,7 +436,7 @@ public function testGetShouldIgnoreOldQueryCacheEntryResult()
array('id'=>2, 'name' => 'Bar')
);

$entry->time = time() - 100;
$entry->time = microtime(true) - 100;

$this->region->addReturn('get', $entry);
$this->region->addReturn('get', new EntityCacheEntry(Country::CLASSNAME, $entities[0]));
Expand Down

0 comments on commit 009e947

Please sign in to comment.