From e235044c472f7ad0f9257e9e8c725786ad0b4cba Mon Sep 17 00:00:00 2001 From: romanb Date: Thu, 25 Feb 2010 15:47:20 +0000 Subject: [PATCH] [2.0] Fixed query and result cache to work nice together and avoid unnecessary cache lookups. --- lib/Doctrine/ORM/AbstractQuery.php | 2 +- .../ORM/Internal/Hydration/ObjectHydrator.php | 6 +- lib/Doctrine/ORM/Query.php | 79 ++++++++++++++----- .../Tests/ORM/Functional/QueryCacheTest.php | 1 + .../ORM/Query/SelectSqlGenerationTest.php | 3 +- 5 files changed, 65 insertions(+), 26 deletions(-) diff --git a/lib/Doctrine/ORM/AbstractQuery.php b/lib/Doctrine/ORM/AbstractQuery.php index 9d0a2da27..c36b793bb 100644 --- a/lib/Doctrine/ORM/AbstractQuery.php +++ b/lib/Doctrine/ORM/AbstractQuery.php @@ -477,7 +477,7 @@ abstract class AbstractQuery } if ($hydrationMode !== null) { - $this->_hydrationMode = $hydrationMode; + $this->setHydrationMode($hydrationMode); } $params = $this->getParameters($params); diff --git a/lib/Doctrine/ORM/Internal/Hydration/ObjectHydrator.php b/lib/Doctrine/ORM/Internal/Hydration/ObjectHydrator.php index 7a364bb25..cabd2aca3 100644 --- a/lib/Doctrine/ORM/Internal/Hydration/ObjectHydrator.php +++ b/lib/Doctrine/ORM/Internal/Hydration/ObjectHydrator.php @@ -157,9 +157,9 @@ class ObjectHydrator extends AbstractHydrator $this->_initializedCollections[$oid . $fieldName] = $value; } else if (isset($this->_hints[Query::HINT_REFRESH])) { // Is already PersistentCollection, but REFRESH - $value->clear(); $value->setDirty(false); $value->setInitialized(true); + $value->unwrap()->clear(); $this->_initializedCollections[$oid . $fieldName] = $value; } else { // Is already PersistentCollection, and DONT REFRESH @@ -360,10 +360,10 @@ class ObjectHydrator extends AbstractHydrator if (isset($targetClass->inverseMappings[$relation->sourceEntityName][$relationField])) { $inverseAssoc = $targetClass->inverseMappings[$relation->sourceEntityName][$relationField]; if ($inverseAssoc->isOneToMany()) { - // Only initialize reverse collection if it is not yet initialized. + /*// Only initialize reverse collection if it is not yet initialized. if ( ! isset($this->_initializedCollections[spl_object_hash($element) . $inverseAssoc->sourceFieldName])) { $this->_initRelatedCollection($element, $targetClass, $inverseAssoc->sourceFieldName); - } + }*/ } else { $targetClass->reflFields[$inverseAssoc->sourceFieldName]->setValue($element, $parentObject); $this->_uow->setOriginalEntityProperty(spl_object_hash($element), $inverseAssoc->sourceFieldName, $parentObject); diff --git a/lib/Doctrine/ORM/Query.php b/lib/Doctrine/ORM/Query.php index b71842e69..fca742663 100644 --- a/lib/Doctrine/ORM/Query.php +++ b/lib/Doctrine/ORM/Query.php @@ -121,6 +121,11 @@ final class Query extends AbstractQuery * @var int Query Cache lifetime. */ private $_queryCacheTTL; + + /** + * @var boolean Whether to use a query cache, if available. Defaults to TRUE. + */ + private $_useQueryCache = true; // End of Caching Stuff @@ -154,11 +159,29 @@ final class Query extends AbstractQuery */ private function _parse() { - if ($this->_state === self::STATE_DIRTY) { + if ($this->_state === self::STATE_CLEAN) { + return $this->_parserResult; + } + + // Check query cache. + if ($this->_useQueryCache && ($queryCache = $this->getQueryCacheDriver())) { + $hash = $this->_getQueryCacheId(); + $cached = $this->_expireQueryCache ? false : $queryCache->fetch($hash); + if ($cached === false) { + // Cache miss. + $parser = new Parser($this); + $this->_parserResult = $parser->parse(); + $queryCache->save($hash, $this->_parserResult, null); + } else { + // Cache hit. + $this->_parserResult = $cached; + } + } else { $parser = new Parser($this); $this->_parserResult = $parser->parse(); - $this->_state = self::STATE_CLEAN; } + $this->_state = self::STATE_CLEAN; + return $this->_parserResult; } @@ -171,26 +194,8 @@ final class Query extends AbstractQuery */ protected function _doExecute(array $params) { - // Check query cache - if ($queryCache = $this->getQueryCacheDriver()) { - $hash = $this->_getQueryCacheId(); - $cached = ($this->_expireQueryCache) ? false : $queryCache->fetch($hash); - - if ($cached === false) { - // Cache miss. - $executor = $this->_parse()->getSqlExecutor(); - $queryCache->save($hash, $this->_parserResult, null); - } else { - // Cache hit. - $this->_parserResult = $cached; - $executor = $this->_parserResult->getSqlExecutor(); - } - } else { - $executor = $this->_parse()->getSqlExecutor(); - } - + $executor = $this->_parse()->getSqlExecutor(); $params = $this->_prepareParams($params); - if ( ! $this->_resultSetMapping) { $this->_resultSetMapping = $this->_parserResult->getResultSetMapping(); } @@ -251,6 +256,18 @@ final class Query extends AbstractQuery $this->_queryCache = $queryCache; return $this; } + + /** + * Defines whether the query should make use of a query cache, if available. + * + * @param boolean $bool + * @return @return Query This query instance. + */ + public function useQueryCache($bool) + { + $this->_useQueryCache = $bool; + return $this; + } /** * Returns the cache driver used for query caching. @@ -386,6 +403,7 @@ final class Query extends AbstractQuery public function setFirstResult($firstResult) { $this->_firstResult = $firstResult; + $this->_state = self::STATE_DIRTY; return $this; } @@ -409,6 +427,7 @@ final class Query extends AbstractQuery public function setMaxResults($maxResults) { $this->_maxResults = $maxResults; + $this->_state = self::STATE_DIRTY; return $this; } @@ -436,6 +455,24 @@ final class Query extends AbstractQuery $this->setHint(self::HINT_INTERNAL_ITERATION, true); return parent::iterate($params, $hydrationMode); } + + /** + * {@inheritdoc} + */ + public function setHint($name, $value) + { + $this->_state = self::STATE_DIRTY; + return parent::setHint($name, $value); + } + + /** + * {@inheritdoc} + */ + public function setHydrationMode($hydrationMode) + { + $this->_state = self::STATE_DIRTY; + return parent::setHydrationMode($hydrationMode); + } /** * Generate a cache id for the query cache - reusing the Result-Cache-Id generator. diff --git a/tests/Doctrine/Tests/ORM/Functional/QueryCacheTest.php b/tests/Doctrine/Tests/ORM/Functional/QueryCacheTest.php index 1d95d0ca1..46681ac05 100644 --- a/tests/Doctrine/Tests/ORM/Functional/QueryCacheTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/QueryCacheTest.php @@ -47,6 +47,7 @@ class QueryCacheTest extends \Doctrine\Tests\OrmFunctionalTestCase $cacheCount = count($cache->getIds()); $query->setFirstResult(10); + $query->setMaxResults(9999); $query->getResult(); $this->assertEquals($cacheCount + 1, count($cache->getIds())); diff --git a/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php b/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php index b74463a3e..4ae0745d6 100644 --- a/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php +++ b/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php @@ -20,7 +20,8 @@ class SelectSqlGenerationTest extends \Doctrine\Tests\OrmTestCase { try { $query = $this->_em->createQuery($dqlToBeTested); - $query->setHint(Query::HINT_FORCE_PARTIAL_LOAD, true); + $query->setHint(Query::HINT_FORCE_PARTIAL_LOAD, true) + ->useQueryCache(false); parent::assertEquals($sqlToBeConfirmed, $query->getSql()); $query->free(); } catch (\Exception $e) {