Make caching layer not rely on closeCursor()

doctrine · Jun 6, 2020 · 70f8c01 · 70f8c01
1 parent acf84c3
commit 70f8c01
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 39 deletions.
diff --git a/docs/en/reference/caching.rst b/docs/en/reference/caching.rst
@@ -35,15 +35,13 @@ the default cache instance:
     new QueryCacheProfile(0, "some key", $cache);
 
 In order for the data to actually be cached its necessary to ensure that the entire
-result set is read (the easiest way to ensure this is to use ``fetchAll``) and the statement
-object is closed:
+result set is read (the easiest way to ensure this is to use one of the ``fetchAll*()`` methods):
 
 ::
 
     <?php
     $stmt = $conn->executeCacheQuery($query, $params, $types, new QueryCacheProfile(0, "some key"));
     $data = $stmt->fetchAllAssociative();
-    $stmt->closeCursor(); // at this point the result is cached
 
 .. warning::
 

diff --git a/lib/Doctrine/DBAL/Cache/ResultCacheStatement.php b/lib/Doctrine/DBAL/Cache/ResultCacheStatement.php
@@ -49,14 +49,7 @@ class ResultCacheStatement implements IteratorAggregate, ResultStatement, Forwar
     /** @var ResultStatement */
     private $statement;
 
-    /**
-     * Did we reach the end of the statement?
-     *
-     * @var bool
-     */
-    private $emptied = false;
-
-    /** @var array<int,array<string,mixed>> */
+    /** @var array<int,array<string,mixed>>|null */
     private $data;
 
     /** @var int */
@@ -82,19 +75,8 @@ public function __construct(ResultStatement $stmt, Cache $resultCache, $cacheKey
     public function closeCursor()
     {
         $this->statement->closeCursor();
-        if (! $this->emptied || $this->data === null) {
-            return true;
-        }
-
-        $data = $this->resultCache->fetch($this->cacheKey);
-        if (! $data) {
-            $data = [];
-        }
 
-        $data[$this->realKey] = $this->data;
-
-        $this->resultCache->save($this->cacheKey, $data, $this->lifetime);
-        unset($this->data);
+        $this->data = null;
 
         return true;
     }
@@ -168,7 +150,7 @@ public function fetch($fetchMode = null, $cursorOrientation = PDO::FETCH_ORI_NEX
             throw new InvalidArgumentException('Invalid fetch-style given for caching result.');
         }
 
-        $this->emptied = true;
+        $this->saveToCache();
 
         return false;
     }
@@ -182,8 +164,9 @@ public function fetchAll($fetchMode = null, $fetchArgument = null, $ctorArgs = n
     {
         $data = $this->statement->fetchAll(FetchMode::ASSOCIATIVE, $fetchArgument, $ctorArgs);
 
-        $this->data    = $data;
-        $this->emptied = true;
+        $this->data = $data;
+
+        $this->saveToCache();
 
         if ($fetchMode === FetchMode::NUMERIC) {
             foreach ($data as $i => $row) {
@@ -326,7 +309,7 @@ private function doFetch()
             return $row;
         }
 
-        $this->emptied = true;
+        $this->saveToCache();
 
         return false;
     }
@@ -336,7 +319,22 @@ private function doFetch()
      */
     private function store(array $data) : void
     {
-        $this->data    = $data;
-        $this->emptied = true;
+        $this->data = $data;
+    }
+
+    private function saveToCache() : void
+    {
+        if ($this->data === null) {
+            return;
+        }
+
+        $data = $this->resultCache->fetch($this->cacheKey);
+        if (! $data) {
+            $data = [];
+        }
+
+        $data[$this->realKey] = $this->data;
+
+        $this->resultCache->save($this->cacheKey, $data, $this->lifetime);
     }
 }
diff --git a/tests/Doctrine/Tests/DBAL/Functional/ResultCacheTest.php b/tests/Doctrine/Tests/DBAL/Functional/ResultCacheTest.php
@@ -133,7 +133,7 @@ private function assertStandardAndIteratorFetchAreEqual(int $fetchMode) : void
         self::assertEquals($data, $dataIterator);
     }
 
-    public function testDontCloseNoCache() : void
+    public function testFetchAndFinishSavesCache() : void
     {
         $stmt = $this->connection->executeQuery('SELECT * FROM caching ORDER BY test_int ASC', [], [], new QueryCacheProfile(0, 'testcachekey'));
 
@@ -151,15 +151,14 @@ public function testDontCloseNoCache() : void
             $data[] = $row;
         }
 
-        self::assertCount(2, $this->sqlLogger->queries);
+        self::assertCount(1, $this->sqlLogger->queries);
     }
 
     public function testDontFinishNoCache() : void
     {
         $stmt = $this->connection->executeQuery('SELECT * FROM caching ORDER BY test_int ASC', [], [], new QueryCacheProfile(0, 'testcachekey'));
 
         $stmt->fetch(FetchMode::ASSOCIATIVE);
-        $stmt->closeCursor();
 
         $stmt = $this->connection->executeQuery('SELECT * FROM caching ORDER BY test_int ASC', [], [], new QueryCacheProfile(0, 'testcachekey'));
 
@@ -168,12 +167,11 @@ public function testDontFinishNoCache() : void
         self::assertCount(2, $this->sqlLogger->queries);
     }
 
-    public function testFetchAllAndFinishSavesCache() : void
+    public function testFetchAllSavesCache() : void
     {
         $layerCache = new ArrayCache();
         $stmt       = $this->connection->executeQuery('SELECT * FROM caching WHERE test_int > 500', [], [], new QueryCacheProfile(0, 'testcachekey', $layerCache));
         $stmt->fetchAll();
-        $stmt->closeCursor();
 
         self::assertCount(1, $layerCache->fetch('testcachekey'));
     }
@@ -187,7 +185,6 @@ public function testFetchAllColumn() : void
 
         $stmt = $this->connection->executeCacheQuery($query, [], [], $qcp);
         $stmt->fetchAll(FetchMode::COLUMN);
-        $stmt->closeCursor();
 
         $stmt = $this->connection->executeCacheQuery($query, [], [], $qcp);
 
@@ -249,8 +246,6 @@ private function hydrateStmt(ResultStatement $stmt, int $fetchMode = FetchMode::
             $data[] = is_array($row) ? array_change_key_case($row, CASE_LOWER) : $row;
         }
 
-        $stmt->closeCursor();
-
         return $data;
     }
 
@@ -265,8 +260,6 @@ private function hydrateStmtIterator(ResultStatement $stmt, int $fetchMode = Fet
             $data[] = is_array($row) ? array_change_key_case($row, CASE_LOWER) : $row;
         }
 
-        $stmt->closeCursor();
-
         return $data;
     }
 }