From 7bcbad076dafd127cd4eb535234add0318c593f6 Mon Sep 17 00:00:00 2001 From: Illia Somov Date: Mon, 7 May 2018 01:12:24 +0300 Subject: [PATCH 1/2] Split and deprecate AbstractQuery#useResultCache() --- UPGRADE.md | 10 +++ lib/Doctrine/ORM/AbstractQuery.php | 42 +++++++-- .../Tests/ORM/Functional/ResultCacheTest.php | 85 +++++++++++++++++-- tests/Doctrine/Tests/ORM/Query/QueryTest.php | 10 +-- 4 files changed, 128 insertions(+), 19 deletions(-) diff --git a/UPGRADE.md b/UPGRADE.md index 85f55d22504..d6970a42aff 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -1,10 +1,20 @@ # Upgrade to 2.7 +## Added `Doctrine\ORM\AbstractQuery#enableResultCache()` and `Doctrine\ORM\AbstractQuery#disableResultCache()` methods + +Method `Doctrine\ORM\AbstractQuery#useResultCache()` which could be used for both enabling and disabling the cache +(depending on passed flag) was split into two. + ## Minor BC BREAK: paginator output walkers aren't be called anymore on sub-queries for queries without max results To optimize DB interaction, `Doctrine\ORM\Tools\Pagination\Paginator` no longer fetches identifiers to be able to perform the pagination with join collections when max results isn't set in the query. +## Deprecated: `Doctrine\ORM\AbstractQuery#useResultCache()` + +Method `Doctrine\ORM\AbstractQuery#useResultCache()` is deprecated because it is split into `enableResultCache()` +and `disableResultCache()`. It will be removed in 3.0. + ## Deprecated code generators and related console commands These console commands have been deprecated: diff --git a/lib/Doctrine/ORM/AbstractQuery.php b/lib/Doctrine/ORM/AbstractQuery.php index 3ba49b08866..22403668675 100644 --- a/lib/Doctrine/ORM/AbstractQuery.php +++ b/lib/Doctrine/ORM/AbstractQuery.php @@ -583,21 +583,45 @@ public function getResultCacheDriver() * Set whether or not to cache the results of this query and if so, for * how long and which ID to use for the cache entry. * - * @param boolean $bool - * @param integer $lifetime - * @param string $resultCacheId + * @deprecated 2.7 Use {@see enableResultCache} and {@see disableResultCache} instead. + * + * @param bool $useCache + * @param int $lifetime + * @param string $resultCacheId * * @return static This query instance. */ - public function useResultCache($bool, $lifetime = null, $resultCacheId = null) + public function useResultCache($useCache, $lifetime = null, $resultCacheId = null) { - if ($bool) { - $this->setResultCacheLifetime($lifetime); - $this->setResultCacheId($resultCacheId); + return $useCache + ? $this->enableResultCache($lifetime, $resultCacheId) + : $this->disableResultCache(); + } - return $this; - } + /** + * Enables caching of the results of this query, for given or default amount of seconds + * and optionally specifies which ID to use for the cache entry. + * + * @param int|null $lifetime How long the cache entry is valid, in seconds. + * @param string|null $resultCacheId ID to use for the cache entry. + * + * @return static This query instance. + */ + public function enableResultCache(?int $lifetime = null, ?string $resultCacheId = null) : self + { + $this->setResultCacheLifetime($lifetime); + $this->setResultCacheId($resultCacheId); + return $this; + } + + /** + * Disables caching of the results of this query. + * + * @return static This query instance. + */ + public function disableResultCache() : self + { $this->_queryCacheProfile = null; return $this; diff --git a/tests/Doctrine/Tests/ORM/Functional/ResultCacheTest.php b/tests/Doctrine/Tests/ORM/Functional/ResultCacheTest.php index bfadf2c2543..05c89b60251 100644 --- a/tests/Doctrine/Tests/ORM/Functional/ResultCacheTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/ResultCacheTest.php @@ -89,7 +89,7 @@ public function testSetResultCacheId() $this->assertTrue($cache->contains('testing_result_cache_id')); } - public function testUseResultCache() + public function testUseResultCacheTrue() { $cache = new ArrayCache(); $query = $this->_em->createQuery('select ux from Doctrine\Tests\Models\CMS\CmsUser ux'); @@ -104,6 +104,22 @@ public function testUseResultCache() $this->_em->getConfiguration()->setResultCacheImpl(new ArrayCache()); } + public function testUseResultCacheFalse() + { + $cache = new ArrayCache(); + $query = $this->_em->createQuery('select ux from Doctrine\Tests\Models\CMS\CmsUser ux'); + + $query->setResultCacheDriver($cache); + $query->setResultCacheId('testing_result_cache_id'); + $query->useResultCache(false); + $users = $query->getResult(); + + $this->assertFalse($cache->contains('testing_result_cache_id')); + + $this->_em->getConfiguration()->setResultCacheImpl(new ArrayCache()); + } + + /** * @group DDC-1026 */ @@ -133,6 +149,65 @@ public function testUseResultCacheParams() $this->assertEquals($sqlCount + 2, count($this->_sqlLoggerStack->queries), "The next two sql should have been cached, but were not."); } + public function testEnableResultCache() + { + $cache = new ArrayCache(); + $query = $this->_em->createQuery('select ux from Doctrine\Tests\Models\CMS\CmsUser ux'); + + $query->enableResultCache(); + $query->setResultCacheDriver($cache); + $query->setResultCacheId('testing_result_cache_id'); + $users = $query->getResult(); + + $this->assertTrue($cache->contains('testing_result_cache_id')); + + $this->_em->getConfiguration()->setResultCacheImpl(new ArrayCache()); + } + + /** + * @group DDC-1026 + */ + public function testEnableResultCacheParams() + { + $cache = new ArrayCache(); + $sqlCount = count($this->_sqlLoggerStack->queries); + $query = $this->_em->createQuery('select ux from Doctrine\Tests\Models\CMS\CmsUser ux WHERE ux.id = ?1'); + + $query->setParameter(1, 1); + $query->setResultCacheDriver($cache); + $query->enableResultCache(); + $query->getResult(); + + $query->setParameter(1, 2); + $query->getResult(); + + $this->assertCount($sqlCount + 2, $this->_sqlLoggerStack->queries, "Two non-cached queries."); + + $query->setParameter(1, 1); + $query->enableResultCache(); + $query->getResult(); + + $query->setParameter(1, 2); + $query->getResult(); + + $this->assertCount($sqlCount + 2, $this->_sqlLoggerStack->queries, "The next two sql should have been cached, but were not."); + } + + public function testDisableResultCache() + { + $cache = new ArrayCache(); + $query = $this->_em->createQuery('select ux from Doctrine\Tests\Models\CMS\CmsUser ux'); + + $query->setResultCacheDriver($cache); + $query->setResultCacheId('testing_result_cache_id'); + $query->disableResultCache(); + $users = $query->getResult(); + + $this->assertFalse($cache->contains('testing_result_cache_id')); + + $this->_em->getConfiguration()->setResultCacheImpl(new ArrayCache()); + } + public function testNativeQueryResultCaching() { $cache = new ArrayCache(); @@ -143,7 +218,7 @@ public function testNativeQueryResultCaching() $query = $this->_em->createNativeQuery('select u.id FROM cms_users u WHERE u.id = ?', $rsm); $query->setParameter(1, 10); - $query->setResultCacheDriver($cache)->useResultCache(true); + $query->setResultCacheDriver($cache)->enableResultCache(); $this->assertEquals(0, $this->getCacheSize($cache)); @@ -229,7 +304,7 @@ public function testResultCacheWithObjectParameter() $cache = new ArrayCache(); - $query->setResultCacheDriver($cache)->useResultCache(true); + $query->setResultCacheDriver($cache)->enableResultCache(); $articles = $query->getResult(); @@ -241,7 +316,7 @@ public function testResultCacheWithObjectParameter() $query2 = $this->_em->createQuery('select a from Doctrine\Tests\Models\CMS\CmsArticle a WHERE a.user = ?1'); $query2->setParameter(1, $user1); - $query2->setResultCacheDriver($cache)->useResultCache(true); + $query2->setResultCacheDriver($cache)->enableResultCache(); $articles = $query2->getResult(); @@ -251,7 +326,7 @@ public function testResultCacheWithObjectParameter() $query3 = $this->_em->createQuery('select a from Doctrine\Tests\Models\CMS\CmsArticle a WHERE a.user = ?1'); $query3->setParameter(1, $user2); - $query3->setResultCacheDriver($cache)->useResultCache(true); + $query3->setResultCacheDriver($cache)->enableResultCache(); $articles = $query3->getResult(); diff --git a/tests/Doctrine/Tests/ORM/Query/QueryTest.php b/tests/Doctrine/Tests/ORM/Query/QueryTest.php index 0045647a09a..71422e641ad 100644 --- a/tests/Doctrine/Tests/ORM/Query/QueryTest.php +++ b/tests/Doctrine/Tests/ORM/Query/QueryTest.php @@ -128,7 +128,7 @@ public function testQueryDefaultResultCache() { $this->_em->getConfiguration()->setResultCacheImpl(new ArrayCache()); $q = $this->_em->createQuery("select a from Doctrine\Tests\Models\CMS\CmsArticle a"); - $q->useResultCache(true); + $q->enableResultCache(); $this->assertSame($this->_em->getConfiguration()->getResultCacheImpl(), $q->getQueryCacheProfile()->getResultCacheDriver()); } @@ -245,7 +245,7 @@ public function testResultCacheCaching() $driverConnectionMock->setStatementMock($stmt); $res = $this->_em->createQuery("select u from Doctrine\Tests\Models\CMS\CmsUser u") ->useQueryCache(true) - ->useResultCache(true, 60) + ->enableResultCache(60) //let it cache ->getResult(); @@ -255,7 +255,7 @@ public function testResultCacheCaching() $res = $this->_em->createQuery("select u from Doctrine\Tests\Models\CMS\CmsUser u") ->useQueryCache(true) - ->useResultCache(false) + ->disableResultCache() ->getResult(); $this->assertCount(0, $res); } @@ -278,7 +278,7 @@ public function testResultCacheEviction() $this->_em->getConfiguration()->setResultCacheImpl(new ArrayCache()); $query = $this->_em->createQuery("SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u") - ->useResultCache(true); + ->enableResultCache(); /** @var DriverConnectionMock $driverConnectionMock */ $driverConnectionMock = $this->_em->getConnection() @@ -397,7 +397,7 @@ public function testResultCacheProfileCanBeRemovedViaSetter() : void $this->_em->getConfiguration()->setResultCacheImpl(new ArrayCache()); $query = $this->_em->createQuery('SELECT u FROM ' . CmsUser::class . ' u'); - $query->useResultCache(true); + $query->enableResultCache(); $query->setResultCacheProfile(); self::assertAttributeSame(null, '_queryCacheProfile', $query); From e8f265d4806f2f05237a944f2f44bb7a9f6d0cc4 Mon Sep 17 00:00:00 2001 From: someniatko Date: Sat, 25 May 2019 20:15:48 +0300 Subject: [PATCH 2/2] Make ResultCacheTest tests slightly more logical --- .../Tests/ORM/Functional/ResultCacheTest.php | 53 ++++++++++++------- 1 file changed, 35 insertions(+), 18 deletions(-) diff --git a/tests/Doctrine/Tests/ORM/Functional/ResultCacheTest.php b/tests/Doctrine/Tests/ORM/Functional/ResultCacheTest.php index 05c89b60251..cffe07fe7d2 100644 --- a/tests/Doctrine/Tests/ORM/Functional/ResultCacheTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/ResultCacheTest.php @@ -8,6 +8,7 @@ use Doctrine\Tests\Models\CMS\CmsArticle; use Doctrine\Common\Cache\ArrayCache; use Doctrine\Tests\OrmFunctionalTestCase; +use function count; /** * ResultCacheTest @@ -112,7 +113,7 @@ public function testUseResultCacheFalse() $query->setResultCacheDriver($cache); $query->setResultCacheId('testing_result_cache_id'); $query->useResultCache(false); - $users = $query->getResult(); + $query->getResult(); $this->assertFalse($cache->contains('testing_result_cache_id')); @@ -129,27 +130,35 @@ public function testUseResultCacheParams() $sqlCount = count($this->_sqlLoggerStack->queries); $query = $this->_em->createQuery('select ux from Doctrine\Tests\Models\CMS\CmsUser ux WHERE ux.id = ?1'); - $query->setParameter(1, 1); $query->setResultCacheDriver($cache); $query->useResultCache(true); - $query->getResult(); + // these queries should result in cache miss: + $query->setParameter(1, 1); + $query->getResult(); $query->setParameter(1, 2); $query->getResult(); - $this->assertEquals($sqlCount + 2, count($this->_sqlLoggerStack->queries), "Two non-cached queries."); + $this->assertCount( + $sqlCount + 2, + $this->_sqlLoggerStack->queries, + 'Two non-cached queries.' + ); + // these two queries should actually be cached, as they repeat previous ones: $query->setParameter(1, 1); - $query->useResultCache(true); $query->getResult(); - $query->setParameter(1, 2); $query->getResult(); - $this->assertEquals($sqlCount + 2, count($this->_sqlLoggerStack->queries), "The next two sql should have been cached, but were not."); + $this->assertCount( + $sqlCount + 2, + $this->_sqlLoggerStack->queries, + 'The next two sql queries should have been cached, but were not.' + ); } - public function testEnableResultCache() + public function testEnableResultCache() : void { $cache = new ArrayCache(); $query = $this->_em->createQuery('select ux from Doctrine\Tests\Models\CMS\CmsUser ux'); @@ -157,7 +166,7 @@ public function testEnableResultCache() $query->enableResultCache(); $query->setResultCacheDriver($cache); $query->setResultCacheId('testing_result_cache_id'); - $users = $query->getResult(); + $query->getResult(); $this->assertTrue($cache->contains('testing_result_cache_id')); @@ -167,33 +176,41 @@ public function testEnableResultCache() /** * @group DDC-1026 */ - public function testEnableResultCacheParams() + public function testEnableResultCacheParams() : void { $cache = new ArrayCache(); $sqlCount = count($this->_sqlLoggerStack->queries); $query = $this->_em->createQuery('select ux from Doctrine\Tests\Models\CMS\CmsUser ux WHERE ux.id = ?1'); - $query->setParameter(1, 1); $query->setResultCacheDriver($cache); $query->enableResultCache(); - $query->getResult(); + // these queries should result in cache miss: + $query->setParameter(1, 1); + $query->getResult(); $query->setParameter(1, 2); $query->getResult(); - $this->assertCount($sqlCount + 2, $this->_sqlLoggerStack->queries, "Two non-cached queries."); + $this->assertCount( + $sqlCount + 2, + $this->_sqlLoggerStack->queries, + 'Two non-cached queries.' + ); + // these two queries should actually be cached, as they repeat previous ones: $query->setParameter(1, 1); - $query->enableResultCache(); $query->getResult(); - $query->setParameter(1, 2); $query->getResult(); - $this->assertCount($sqlCount + 2, $this->_sqlLoggerStack->queries, "The next two sql should have been cached, but were not."); + $this->assertCount( + $sqlCount + 2, + $this->_sqlLoggerStack->queries, + 'The next two sql queries should have been cached, but were not.' + ); } - public function testDisableResultCache() + public function testDisableResultCache() : void { $cache = new ArrayCache(); $query = $this->_em->createQuery('select ux from Doctrine\Tests\Models\CMS\CmsUser ux'); @@ -201,7 +218,7 @@ public function testDisableResultCache() $query->setResultCacheDriver($cache); $query->setResultCacheId('testing_result_cache_id'); $query->disableResultCache(); - $users = $query->getResult(); + $query->getResult(); $this->assertFalse($cache->contains('testing_result_cache_id'));