diff --git a/src/Cache/CachingResult.php b/src/Cache/CachingResult.php index b634afa4f2d..340bfa90429 100644 --- a/src/Cache/CachingResult.php +++ b/src/Cache/CachingResult.php @@ -43,18 +43,23 @@ class CachingResult implements Result /** @var array>|null */ private $data; + /** @var array */ + private $fetchedData; + /** - * @param string $cacheKey - * @param string $realKey - * @param int $lifetime + * @param string $cacheKey + * @param string $realKey + * @param int $lifetime + * @param array $fetchedData */ - public function __construct(Result $result, Cache $cache, $cacheKey, $realKey, $lifetime) + public function __construct(Result $result, Cache $cache, $cacheKey, $realKey, $lifetime, array $fetchedData) { - $this->result = $result; - $this->cache = $cache; - $this->cacheKey = $cacheKey; - $this->realKey = $realKey; - $this->lifetime = $lifetime; + $this->result = $result; + $this->cache = $cache; + $this->cacheKey = $cacheKey; + $this->realKey = $realKey; + $this->lifetime = $lifetime; + $this->fetchedData = $fetchedData; } /** @@ -170,14 +175,8 @@ private function saveToCache(): void return; } - $data = $this->cache->fetch($this->cacheKey); - - if ($data === false) { - $data = []; - } - - $data[$this->realKey] = $this->data; + $this->fetchedData[$this->realKey] = $this->data; - $this->cache->save($this->cacheKey, $data, $this->lifetime); + $this->cache->save($this->cacheKey, $this->fetchedData, $this->lifetime); } } diff --git a/src/Connection.php b/src/Connection.php index 39376bb0474..1be8cee2260 100644 --- a/src/Connection.php +++ b/src/Connection.php @@ -1013,6 +1013,8 @@ public function executeCacheQuery($sql, $params, $types, QueryCacheProfile $qcp) } elseif (array_key_exists($realKey, $data)) { $result = new ArrayResult([]); } + } else { + $data = []; } if (! isset($result)) { @@ -1021,7 +1023,8 @@ public function executeCacheQuery($sql, $params, $types, QueryCacheProfile $qcp) $resultCache, $cacheKey, $realKey, - $qcp->getLifetime() + $qcp->getLifetime(), + $data ); } diff --git a/tests/Cache/CachingResultTest.php b/tests/Cache/CachingResultTest.php new file mode 100644 index 00000000000..349457c4428 --- /dev/null +++ b/tests/Cache/CachingResultTest.php @@ -0,0 +1,77 @@ + */ + private $resultData = ['id' => 123, 'field' => 'value']; + + /** @var array */ + private $cachedData; + + /** @var Cache */ + private $cache; + + /** @var Result */ + private $result; + + protected function setUp(): void + { + $this->result = $this->createMock(Result::class); + $this->result->expects(self::exactly(2)) + ->method('fetchAssociative') + ->will($this->onConsecutiveCalls($this->resultData, false)); + + $this->cache = $this->createMock(Cache::class); + $this->cache->expects(self::exactly(1)) + ->method('save') + ->willReturnCallback(function (string $id, $data, int $ttl): void { + $this->assertEquals($this->cacheKey, $id, 'The cache key should match the given one'); + $this->assertEquals($this->lifetime, $ttl, 'The cache key ttl should match the given one'); + $this->cachedData = $data; + }); + } + + public function testShouldSaveResultToCache(): void + { + $cachingResult = new CachingResult( + $this->result, + $this->cache, + $this->cacheKey, + $this->realKey, + $this->lifetime, + ['otherRealKey' => 'resultValue'] + ); + + do { + $row = $cachingResult->fetchAssociative(); + } while ($row !== false); + + $this->assertContains( + $this->resultData, + $this->cachedData[$this->realKey], + 'CachingResult should cache data from the given result' + ); + + $this->assertEquals( + 'resultValue', + $this->cachedData['otherRealKey'], + 'CachingResult should not change other keys from cache' + ); + } +} diff --git a/tests/Functional/ResultCacheTest.php b/tests/Functional/ResultCacheTest.php index e0ddf392d3c..2ec6ebe1dc2 100644 --- a/tests/Functional/ResultCacheTest.php +++ b/tests/Functional/ResultCacheTest.php @@ -225,6 +225,64 @@ public function testFetchAllSavesCache(): void self::assertCount(1, $layerCache->fetch('testcachekey')); } + public function testCacheQueriedOnlyOnceForCacheMiss(): void + { + $layerCache = $this->createMock(ArrayCache::class); + $layerCache->expects(self::once()) + ->method('fetch') + ->willReturn(false); + + $layerCache->expects(self::once()) + ->method('save'); + + $result = $this->connection->executeQuery( + 'SELECT * FROM caching WHERE test_int > 500', + [], + [], + new QueryCacheProfile(0, 'testcachekey', $layerCache) + ); + $result->fetchAllAssociative(); + } + + public function testDifferentQueriesMayBeSavedToOneCacheKey(): void + { + $layerCache = new ArrayCache(); + + $this->executeTwoCachedQueries($layerCache); + self::assertCount(2, $this->sqlLogger->queries, 'Two queries executed'); + self::assertCount(2, $layerCache->fetch('testcachekey'), 'Both queries are saved to cache'); + + $this->executeTwoCachedQueries($layerCache); + self::assertCount(2, $this->sqlLogger->queries, 'Consecutive queries are fetched from cache'); + + $layerCache->delete('testcachekey'); + $this->executeTwoCachedQueries($layerCache); + self::assertCount( + 4, + $this->sqlLogger->queries, + 'Deleting one cache key leads to deleting cache for both queris' + ); + } + + private function executeTwoCachedQueries(ArrayCache $cache): void + { + $result = $this->connection->executeQuery( + 'SELECT * FROM caching WHERE test_int > 500', + [], + [], + new QueryCacheProfile(0, 'testcachekey', $cache) + ); + $result->fetchAllAssociative(); + + $result = $this->connection->executeQuery( + 'SELECT * FROM caching WHERE test_int > 400', + [], + [], + new QueryCacheProfile(0, 'testcachekey', $cache) + ); + $result->fetchAllAssociative(); + } + public function testFetchColumn(): void { $query = $this->connection->getDatabasePlatform()