Merge pull request #7701 from someniatko/deprecate-use-result-cache

Split and deprecate AbstractQuery#useResultCache()

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:

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;

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
@@ -89,7 +90,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 +105,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);
+        $query->getResult();
+
+        $this->assertFalse($cache->contains('testing_result_cache_id'));
+
+        $this->_em->getConfiguration()->setResultCacheImpl(new ArrayCache());
+    }
+
+
     /**
      * @group DDC-1026
      */
@@ -113,24 +130,99 @@ 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);
+
+        // 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.'
+        );
+
+        // these two queries should actually be cached, as they repeat previous ones:
+        $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,
+            'The next two sql queries should have been cached, but were not.'
+        );
+    }
+
+    public function testEnableResultCache() : void
+    {
+        $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');
+        $query->getResult();
+
+        $this->assertTrue($cache->contains('testing_result_cache_id'));
 
+        $this->_em->getConfiguration()->setResultCacheImpl(new ArrayCache());
+    }
+
+    /**
+     * @group DDC-1026
+     */
+    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->setResultCacheDriver($cache);
+        $query->enableResultCache();
+
+        // these queries should result in cache miss:
         $query->setParameter(1, 1);
-        $query->useResultCache(true);
+        $query->getResult();
+        $query->setParameter(1, 2);
         $query->getResult();
 
+        $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->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 testDisableResultCache() : void
+    {
+        $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();
+        $query->getResult();
+
+        $this->assertFalse($cache->contains('testing_result_cache_id'));
+
+        $this->_em->getConfiguration()->setResultCacheImpl(new ArrayCache());
     }
 
     public function testNativeQueryResultCaching()
@@ -143,7 +235,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 +321,7 @@ public function testResultCacheWithObjectParameter()
 
         $cache = new ArrayCache();
 
-        $query->setResultCacheDriver($cache)->useResultCache(true);
+        $query->setResultCacheDriver($cache)->enableResultCache();
 
         $articles = $query->getResult();
 
@@ -241,7 +333,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 +343,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();
 

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);