diff --git a/UPGRADE.md b/UPGRADE.md index ed9a538b3..b71e10adb 100644 --- a/UPGRADE.md +++ b/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 diff --git a/lib/Doctrine/ORM/AbstractQuery.php b/lib/Doctrine/ORM/AbstractQuery.php index e1667add2..216fca542 100644 --- a/lib/Doctrine/ORM/AbstractQuery.php +++ b/lib/Doctrine/ORM/AbstractQuery.php @@ -28,7 +28,7 @@ use Doctrine\ORM\Cache\QueryCacheKey; use Doctrine\DBAL\Cache\QueryCacheProfile; use Doctrine\ORM\Cache; -use Doctrine\ORM\Query\QueryException; +use Doctrine\ORM\Query\ResultSetMapping; /** * Base contract for ORM queries. Base class for Query and NativeQuery. @@ -993,32 +993,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 diff --git a/lib/Doctrine/ORM/Cache/CacheConfiguration.php b/lib/Doctrine/ORM/Cache/CacheConfiguration.php index 4891cac49..eb3ec428d 100644 --- a/lib/Doctrine/ORM/Cache/CacheConfiguration.php +++ b/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; diff --git a/lib/Doctrine/ORM/Cache/Persister/Entity/AbstractEntityPersister.php b/lib/Doctrine/ORM/Cache/Persister/Entity/AbstractEntityPersister.php index eaead7cf7..a2071f42b 100644 --- a/lib/Doctrine/ORM/Cache/Persister/Entity/AbstractEntityPersister.php +++ b/lib/Doctrine/ORM/Cache/Persister/Entity/AbstractEntityPersister.php @@ -296,17 +296,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); } /** @@ -377,17 +376,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]; @@ -397,15 +395,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); } } @@ -417,32 +415,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); } } @@ -516,32 +513,34 @@ abstract class AbstractEntityPersister implements CachedEntityPersister */ public function loadCriteria(Criteria $criteria) { + $orderBy = $criteria->getOrderings(); + $limit = $criteria->getMaxResults(); + $offset = $criteria->getFirstResult(); $query = $this->persister->getSelectSQL($criteria); - $timestamp = $this->timestampRegion->get($this->timestampKey); - $hash = $this->getHash($query, $criteria, null, null, null, $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); } } diff --git a/lib/Doctrine/ORM/Cache/QueryCacheEntry.php b/lib/Doctrine/ORM/Cache/QueryCacheEntry.php index 1ba61bcf0..a8a26f6d2 100644 --- a/lib/Doctrine/ORM/Cache/QueryCacheEntry.php +++ b/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 integer $time + * @param float $time */ public function __construct($result, $time = null) { $this->result = $result; - $this->time = $time ?: time(); + $this->time = $time ?: microtime(true); } /** diff --git a/lib/Doctrine/ORM/Cache/QueryCacheKey.php b/lib/Doctrine/ORM/Cache/QueryCacheKey.php index 9a7d2b7bc..0e072a36f 100644 --- a/lib/Doctrine/ORM/Cache/QueryCacheKey.php +++ b/lib/Doctrine/ORM/Cache/QueryCacheKey.php @@ -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 function __construct($hash, $lifetime = 0, $cacheMode = Cache::MODE_NORMAL) - { - $this->hash = $hash; - $this->lifetime = $lifetime; - $this->cacheMode = $cacheMode; + 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, + TimestampCacheKey $timestampKey = null + ) { + $this->hash = $hash; + $this->lifetime = $lifetime; + $this->cacheMode = $cacheMode; + $this->timestampKey = $timestampKey; } } diff --git a/lib/Doctrine/ORM/Cache/TimestampQueryCacheValidator.php b/lib/Doctrine/ORM/Cache/TimestampQueryCacheValidator.php index 4e504e4f8..c6404d3b2 100644 --- a/lib/Doctrine/ORM/Cache/TimestampQueryCacheValidator.php +++ b/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; } } diff --git a/tests/Doctrine/Tests/ORM/Cache/CacheConfigTest.php b/tests/Doctrine/Tests/ORM/Cache/CacheConfigTest.php index 412d30207..d3d97bb04 100644 --- a/tests/Doctrine/Tests/ORM/Cache/CacheConfigTest.php +++ b/tests/Doctrine/Tests/ORM/Cache/CacheConfigTest.php @@ -2,8 +2,8 @@ namespace Doctrine\Tests\ORM\Cache; -use Doctrine\Tests\DoctrineTestCase; use Doctrine\ORM\Cache\CacheConfiguration; +use Doctrine\Tests\DoctrineTestCase; /** * @group DDC-2183 @@ -64,6 +64,11 @@ class CacheConfigTest extends DoctrineTestCase public function testSetGetQueryValidator() { + $factory = $this->getMock('Doctrine\ORM\Cache\CacheFactory'); + $factory->method('getTimestampRegion')->willReturn($this->getMock('Doctrine\ORM\Cache\TimestampRegion')); + + $this->config->setCacheFactory($factory); + $validator = $this->getMock('Doctrine\ORM\Cache\QueryCacheValidator'); $this->assertInstanceOf('Doctrine\ORM\Cache\TimestampQueryCacheValidator', $this->config->getQueryValidator()); @@ -72,4 +77,4 @@ class CacheConfigTest extends DoctrineTestCase $this->assertEquals($validator, $this->config->getQueryValidator()); } -} \ No newline at end of file +} diff --git a/tests/Doctrine/Tests/ORM/Cache/DefaultQueryCacheTest.php b/tests/Doctrine/Tests/ORM/Cache/DefaultQueryCacheTest.php index 3af1a3bc4..ad1da8d1b 100644 --- a/tests/Doctrine/Tests/ORM/Cache/DefaultQueryCacheTest.php +++ b/tests/Doctrine/Tests/ORM/Cache/DefaultQueryCacheTest.php @@ -386,7 +386,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])); diff --git a/tests/Doctrine/Tests/ORM/Functional/SecondLevelCacheQueryCacheTest.php b/tests/Doctrine/Tests/ORM/Functional/SecondLevelCacheQueryCacheTest.php index 995c1d07f..18618336a 100644 --- a/tests/Doctrine/Tests/ORM/Functional/SecondLevelCacheQueryCacheTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/SecondLevelCacheQueryCacheTest.php @@ -1035,4 +1035,37 @@ class SecondLevelCacheQueryCacheTest extends SecondLevelCacheAbstractTest ->setCacheable(true) ->getResult(); } -} \ No newline at end of file + + 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); + } + } +}