Merge branch 'fix/#6001-second-level-cache-query-cache-timestamp-from-region'

Close #6001

UPGRADE.md

@@ -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

lib/Doctrine/ORM/AbstractQuery.php

@@ -28,6 +28,7 @@ use Doctrine\ORM\Cache\QueryCacheKey;
 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.
@@ -991,32 +992,54 @@ abstract class AbstractQuery
     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

lib/Doctrine/ORM/Cache/CacheConfiguration.php

@@ -110,7 +110,9 @@ class CacheConfiguration
     public function getQueryValidator()
     {
         if ($this->queryValidator === null) {
-            $this->queryValidator = new TimestampQueryCacheValidator();
+            $this->queryValidator = new TimestampQueryCacheValidator(
+                $this->cacheFactory->getTimestampRegion()
+            );
         }
 
          return $this->queryValidator;

lib/Doctrine/ORM/Cache/Persister/Entity/AbstractEntityPersister.php

@@ -287,17 +287,16 @@ abstract class AbstractEntityPersister implements CachedEntityPersister
      * @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);
     }
 
     /**
@@ -368,17 +367,16 @@ abstract class AbstractEntityPersister implements CachedEntityPersister
         }
 
         //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];
@@ -388,15 +386,15 @@ abstract class AbstractEntityPersister implements CachedEntityPersister
             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);
             }
         }
 
@@ -408,32 +406,31 @@ abstract class AbstractEntityPersister implements CachedEntityPersister
      */
     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);
             }
         }
 
@@ -511,31 +508,30 @@ abstract class AbstractEntityPersister implements CachedEntityPersister
         $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);
-        $hash        = $this->getHash($query, $criteria, $orderBy, $limit, $offset, $timestamp ? $timestamp->time : 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);
-        $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);
             }
         }
 

lib/Doctrine/ORM/Cache/QueryCacheEntry.php

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

lib/Doctrine/ORM/Cache/QueryCacheKey.php

@@ -45,14 +45,27 @@ class QueryCacheKey extends CacheKey
     public $cacheMode;
 
     /**
-     * @param string  $hash      Result cache id
+     * READ-ONLY: Public only for performance reasons, it should be considered immutable.
-     * @param integer $lifetime  Query lifetime
+     *
-     * @param integer $cacheMode Query cache mode
+     * @var TimestampCacheKey|null
      */
-    public function __construct($hash, $lifetime = 0, $cacheMode = Cache::MODE_NORMAL)
+    public $timestampKey;
-    {
+
-        $this->hash      = $hash;
+    /**
-        $this->lifetime  = $lifetime;
+     * @param string $hash Result cache id
-        $this->cacheMode = $cacheMode;
+     * @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,
+        TimestampCacheKey $timestampKey = null
+    ) {
+        $this->hash         = $hash;
+        $this->lifetime     = $lifetime;
+        $this->cacheMode    = $cacheMode;
+        $this->timestampKey = $timestampKey;
     }
 }

lib/Doctrine/ORM/Cache/TimestampQueryCacheValidator.php

@@ -26,15 +26,49 @@ namespace Doctrine\ORM\Cache;
  */
 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;
     }
 }

tests/Doctrine/Tests/ORM/Cache/CacheConfigTest.php

@@ -6,6 +6,7 @@ use Doctrine\ORM\Cache\CacheConfiguration;
 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;
 
 /**
@@ -67,6 +68,11 @@ class CacheConfigTest extends DoctrineTestCase
 
     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());
@@ -75,4 +81,4 @@ class CacheConfigTest extends DoctrineTestCase
 
         $this->assertEquals($validator, $this->config->getQueryValidator());
     }
-}
+}

tests/Doctrine/Tests/ORM/Cache/DefaultQueryCacheTest.php

@@ -436,7 +436,7 @@ class DefaultQueryCacheTest extends OrmTestCase
             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]));

tests/Doctrine/Tests/ORM/Functional/SecondLevelCacheQueryCacheTest.php

@@ -1120,4 +1120,37 @@ class SecondLevelCacheQueryCacheTest extends SecondLevelCacheAbstractTest
             ->setCacheable(true)
             ->getResult();
     }
-}
+
+    public function testQueryCacheShouldBeEvictedOnTimestampUpdate()
+    {
+        $this->loadFixturesCountries();
+        $this->_em->clear();
+
+        $queryCount = $this->getCurrentQueryCount();
+        $dql        = 'SELECT country FROM Doctrine\Tests\Models\Cache\Country country';
+
+        $result1    = $this->_em->createQuery($dql)
+            ->setCacheable(true)
+            ->getResult();
+
+        $this->assertCount(2, $result1);
+        $this->assertEquals($queryCount + 1, $this->getCurrentQueryCount());
+
+        $this->_em->persist(new Country('France'));
+        $this->_em->flush();
+        $this->_em->clear();
+
+        $queryCount = $this->getCurrentQueryCount();
+
+        $result2 = $this->_em->createQuery($dql)
+            ->setCacheable(true)
+            ->getResult();
+
+        $this->assertCount(3, $result2);
+        $this->assertEquals($queryCount + 1, $this->getCurrentQueryCount());
+
+        foreach ($result2 as $entity) {
+            $this->assertInstanceOf(Country::CLASSNAME, $entity);
+        }
+    }
+}