From a8657a0ceb34172427041d11e9fa1c4f2d5b0214 Mon Sep 17 00:00:00 2001 From: "Alexander M. Turek" Date: Tue, 27 Apr 2021 09:30:26 +0200 Subject: [PATCH] Deprecate doctrine/cache in favor of psr/cache --- composer.json | 4 +- phpstan.neon.dist | 6 +++ psalm.xml.dist | 12 +++++ src/Cache/CachingResult.php | 19 +++---- src/Cache/QueryCacheProfile.php | 77 +++++++++++++++++++++++----- src/Configuration.php | 49 ++++++++++++++++++ src/Connection.php | 7 +-- tests/ConnectionTest.php | 54 +++++++++++-------- tests/Functional/ResultCacheTest.php | 56 ++++++++++++++++++-- 9 files changed, 234 insertions(+), 50 deletions(-) diff --git a/composer.json b/composer.json index b78e5bc23d7..f4a4e9cf1db 100644 --- a/composer.json +++ b/composer.json @@ -33,7 +33,7 @@ "require": { "php": "^7.3 || ^8.0", "composer/package-versions-deprecated": "^1.11.99", - "doctrine/cache": "^1.0", + "doctrine/cache": "^1.11", "doctrine/deprecations": "^0.5.3", "doctrine/event-manager": "^1.0" }, @@ -44,7 +44,9 @@ "phpstan/phpstan-strict-rules": "^0.12.2", "phpunit/phpunit": "9.5.0", "psalm/plugin-phpunit": "0.13.0", + "psr/cache": "^1.0 || ^2.0 || ^3.0", "squizlabs/php_codesniffer": "3.6.0", + "symfony/cache": "^5.2", "symfony/console": "^2.0.5|^3.0|^4.0|^5.0", "vimeo/psalm": "4.6.4" }, diff --git a/phpstan.neon.dist b/phpstan.neon.dist index d7aa7887cf4..ac18aef3bac 100644 --- a/phpstan.neon.dist +++ b/phpstan.neon.dist @@ -106,5 +106,11 @@ parameters: message: '~^Instanceof between Doctrine\\DBAL\\Platforms\\Keywords\\KeywordList and Doctrine\\DBAL\\Platforms\\Keywords\\KeywordList will always evaluate to true\.~' paths: - %currentWorkingDirectory%/src/Platforms/AbstractPlatform.php + + # We're checking for invalid invalid input + - + message: "#^Strict comparison using \\!\\=\\= between null and null will always evaluate to false\\.$#" + count: 1 + path: src/Cache/QueryCacheProfile.php includes: - vendor/phpstan/phpstan-strict-rules/rules.neon diff --git a/psalm.xml.dist b/psalm.xml.dist index 771d687ad46..a3dbac3d024 100644 --- a/psalm.xml.dist +++ b/psalm.xml.dist @@ -45,6 +45,11 @@ is no longer supported. --> + + @@ -87,6 +92,12 @@ See https://github.com/doctrine/dbal/pull/4518 --> + + + @@ -100,6 +111,7 @@ 1. Union types not supported at the language level (require dropping PHP 7 support) 2. Associative arrays with typed elements used instead of classes (require breaking API changes) --> + diff --git a/src/Cache/CachingResult.php b/src/Cache/CachingResult.php index c060673c548..3f8698625f9 100644 --- a/src/Cache/CachingResult.php +++ b/src/Cache/CachingResult.php @@ -2,11 +2,11 @@ namespace Doctrine\DBAL\Cache; -use Doctrine\Common\Cache\Cache; use Doctrine\DBAL\Driver\Exception; use Doctrine\DBAL\Driver\FetchUtils; use Doctrine\DBAL\Driver\Result as DriverResult; use Doctrine\DBAL\Result; +use Psr\Cache\CacheItemPoolInterface; use function array_map; use function array_values; @@ -26,7 +26,7 @@ */ class CachingResult implements DriverResult { - /** @var Cache */ + /** @var CacheItemPoolInterface */ private $cache; /** @var string */ @@ -49,7 +49,7 @@ class CachingResult implements DriverResult * @param string $realKey * @param int $lifetime */ - public function __construct(Result $result, Cache $cache, $cacheKey, $realKey, $lifetime) + public function __construct(Result $result, CacheItemPoolInterface $cache, $cacheKey, $realKey, $lifetime) { $this->result = $result; $this->cache = $cache; @@ -171,14 +171,15 @@ private function saveToCache(): void return; } - $data = $this->cache->fetch($this->cacheKey); + $item = $this->cache->getItem($this->cacheKey); + $data = $item->isHit() ? $item->get() : []; + $data[$this->realKey] = $this->data; - if ($data === false) { - $data = []; + $item->set($data); + if ($this->lifetime > 0) { + $item->expiresAfter($this->lifetime); } - $data[$this->realKey] = $this->data; - - $this->cache->save($this->cacheKey, $data, $this->lifetime); + $this->cache->save($item); } } diff --git a/src/Cache/QueryCacheProfile.php b/src/Cache/QueryCacheProfile.php index 3d5b814d2b7..b269c0978cc 100644 --- a/src/Cache/QueryCacheProfile.php +++ b/src/Cache/QueryCacheProfile.php @@ -3,11 +3,18 @@ namespace Doctrine\DBAL\Cache; use Doctrine\Common\Cache\Cache; +use Doctrine\Common\Cache\Psr6\CacheAdapter; +use Doctrine\Common\Cache\Psr6\DoctrineProvider; use Doctrine\DBAL\Types\Type; +use Doctrine\Deprecations\Deprecation; +use Psr\Cache\CacheItemPoolInterface; +use TypeError; +use function get_class; use function hash; use function serialize; use function sha1; +use function sprintf; /** * Query Cache Profile handles the data relevant for query caching. @@ -16,8 +23,8 @@ */ class QueryCacheProfile { - /** @var Cache|null */ - private $resultCacheDriver; + /** @var CacheItemPoolInterface|null */ + private $resultCache; /** @var int */ private $lifetime = 0; @@ -26,22 +33,56 @@ class QueryCacheProfile private $cacheKey; /** - * @param int $lifetime - * @param string|null $cacheKey + * @param int $lifetime + * @param string|null $cacheKey + * @param CacheItemPoolInterface|Cache|null $resultCache */ - public function __construct($lifetime = 0, $cacheKey = null, ?Cache $resultCache = null) + public function __construct($lifetime = 0, $cacheKey = null, ?object $resultCache = null) + { + $this->lifetime = $lifetime; + $this->cacheKey = $cacheKey; + if ($resultCache instanceof CacheItemPoolInterface) { + $this->resultCache = $resultCache; + } elseif ($resultCache instanceof Cache) { + Deprecation::trigger( + 'doctrine/dbal', + 'TODO', + 'Passing an instance of %s as $resultCache is deprecated. Pass an instance of %s instead.', + Cache::class, + CacheItemPoolInterface::class + ); + + $this->resultCache = CacheAdapter::wrap($resultCache); + } elseif ($resultCache !== null) { + throw new TypeError(sprintf( + '$resultCache: Expected either null or an instance of %s or %s, got %s.', + CacheItemPoolInterface::class, + Cache::class, + get_class($resultCache) + )); + } + } + + public function getResultCache(): ?CacheItemPoolInterface { - $this->lifetime = $lifetime; - $this->cacheKey = $cacheKey; - $this->resultCacheDriver = $resultCache; + return $this->resultCache; } /** + * @deprecated Use {@see getResultCache()} instead. + * * @return Cache|null */ public function getResultCacheDriver() { - return $this->resultCacheDriver; + Deprecation::trigger( + 'doctrine/dbal', + 'TODO', + '%s is deprecated, call getResultCache() instead.', + __METHOD__ + ); + + return $this->resultCache !== null ? DoctrineProvider::wrap($this->resultCache) : null; } /** @@ -93,12 +134,24 @@ public function generateCacheKeys($sql, $params, $types, array $connectionParams return [$cacheKey, $realCacheKey]; } + public function setResultCache(CacheItemPoolInterface $cache): QueryCacheProfile + { + return new QueryCacheProfile($this->lifetime, $this->cacheKey, $cache); + } + /** * @return QueryCacheProfile */ public function setResultCacheDriver(Cache $cache) { - return new QueryCacheProfile($this->lifetime, $this->cacheKey, $cache); + Deprecation::trigger( + 'doctrine/dbal', + 'TODO', + '%s is deprecated, call setResultCache() instead.', + __METHOD__ + ); + + return new QueryCacheProfile($this->lifetime, $this->cacheKey, CacheAdapter::wrap($cache)); } /** @@ -108,7 +161,7 @@ public function setResultCacheDriver(Cache $cache) */ public function setCacheKey($cacheKey) { - return new QueryCacheProfile($this->lifetime, $cacheKey, $this->resultCacheDriver); + return new QueryCacheProfile($this->lifetime, $cacheKey, $this->resultCache); } /** @@ -118,6 +171,6 @@ public function setCacheKey($cacheKey) */ public function setLifetime($lifetime) { - return new QueryCacheProfile($lifetime, $this->cacheKey, $this->resultCacheDriver); + return new QueryCacheProfile($lifetime, $this->cacheKey, $this->resultCache); } } diff --git a/src/Configuration.php b/src/Configuration.php index 109289f5a30..88a30876230 100644 --- a/src/Configuration.php +++ b/src/Configuration.php @@ -3,8 +3,12 @@ namespace Doctrine\DBAL; use Doctrine\Common\Cache\Cache; +use Doctrine\Common\Cache\Psr6\CacheAdapter; +use Doctrine\Common\Cache\Psr6\DoctrineProvider; use Doctrine\DBAL\Driver\Middleware; use Doctrine\DBAL\Logging\SQLLogger; +use Doctrine\Deprecations\Deprecation; +use Psr\Cache\CacheItemPoolInterface; /** * Configuration container for the Doctrine DBAL. @@ -24,6 +28,15 @@ class Configuration /** * The cache driver implementation that is used for query result caching. * + * @var CacheItemPoolInterface|null + */ + protected $resultCache; + + /** + * The cache driver implementation that is used for query result caching. + * + * @deprecated Use {@see $resultCache} instead. + * * @var Cache|null */ protected $resultCacheImpl; @@ -61,17 +74,53 @@ public function getSQLLogger(): ?SQLLogger /** * Gets the cache driver implementation that is used for query result caching. */ + public function getResultCache(): ?CacheItemPoolInterface + { + return $this->resultCache; + } + + /** + * Gets the cache driver implementation that is used for query result caching. + * + * @deprecated Use {@see getResultCache()} instead. + */ public function getResultCacheImpl(): ?Cache { + Deprecation::trigger( + 'doctrine/dbal', + 'TODO', + '%s is deprecated, call getResultCache() instead.', + __METHOD__ + ); + return $this->resultCacheImpl; } /** * Sets the cache driver implementation that is used for query result caching. */ + public function setResultCache(CacheItemPoolInterface $cache): void + { + $this->resultCacheImpl = DoctrineProvider::wrap($cache); + $this->resultCache = $cache; + } + + /** + * Sets the cache driver implementation that is used for query result caching. + * + * @deprecated Use {@see setResultCache()} instead. + */ public function setResultCacheImpl(Cache $cacheImpl): void { + Deprecation::trigger( + 'doctrine/dbal', + 'TODO', + '%s is deprecated, call setResultCache() instead.', + __METHOD__ + ); + $this->resultCacheImpl = $cacheImpl; + $this->resultCache = CacheAdapter::wrap($cacheImpl); } /** diff --git a/src/Connection.php b/src/Connection.php index f16d40efd40..cef96c991f8 100644 --- a/src/Connection.php +++ b/src/Connection.php @@ -1087,7 +1087,7 @@ public function executeQuery( */ public function executeCacheQuery($sql, $params, $types, QueryCacheProfile $qcp): Result { - $resultCache = $qcp->getResultCacheDriver() ?? $this->_config->getResultCacheImpl(); + $resultCache = $qcp->getResultCache() ?? $this->_config->getResultCache(); if ($resultCache === null) { throw CacheException::noResultDriverConfigured(); @@ -1099,9 +1099,10 @@ public function executeCacheQuery($sql, $params, $types, QueryCacheProfile $qcp) [$cacheKey, $realKey] = $qcp->generateCacheKeys($sql, $params, $types, $connectionParams); // fetch the row pointers entry - $data = $resultCache->fetch($cacheKey); + $item = $resultCache->getItem($cacheKey); - if ($data !== false) { + if ($item->isHit()) { + $data = $item->get(); // is the real key part of this row pointers map or is the cache only pointing to other cache keys? if (isset($data[$realKey])) { $result = new ArrayResult($data[$realKey]); diff --git a/tests/ConnectionTest.php b/tests/ConnectionTest.php index 85e6a88794b..a670f14040c 100644 --- a/tests/ConnectionTest.php +++ b/tests/ConnectionTest.php @@ -2,7 +2,6 @@ namespace Doctrine\DBAL\Tests; -use Doctrine\Common\Cache\Cache; use Doctrine\Common\EventManager; use Doctrine\DBAL\Cache\QueryCacheProfile; use Doctrine\DBAL\Configuration; @@ -22,6 +21,8 @@ use Doctrine\DBAL\VersionAwarePlatformDriver; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; +use Psr\Cache\CacheItemInterface; +use Psr\Cache\CacheItemPoolInterface; use stdClass; /** @@ -628,13 +629,17 @@ public function testPlatformDetectionIsTriggerOnlyOnceOnRetrievingPlatform(): vo public function testConnectionParamsArePassedToTheQueryCacheProfileInExecuteCacheQuery(): void { - $resultCacheDriverMock = $this->createMock(Cache::class); + $cacheItemMock = $this->createMock(CacheItemInterface::class); + $cacheItemMock->method('isHit')->willReturn(true); + $cacheItemMock->method('get')->willReturn(['realKey' => []]); - $resultCacheDriverMock + $resultCacheMock = $this->createMock(CacheItemPoolInterface::class); + + $resultCacheMock ->expects(self::atLeastOnce()) - ->method('fetch') + ->method('getItem') ->with('cacheKey') - ->will(self::returnValue(['realKey' => []])); + ->willReturn($cacheItemMock); $query = 'SELECT * FROM foo WHERE bar = ?'; $params = [666]; @@ -643,9 +648,8 @@ public function testConnectionParamsArePassedToTheQueryCacheProfileInExecuteCach $queryCacheProfileMock = $this->createMock(QueryCacheProfile::class); $queryCacheProfileMock - ->expects(self::any()) - ->method('getResultCacheDriver') - ->will(self::returnValue($resultCacheDriverMock)); + ->method('getResultCache') + ->willReturn($resultCacheMock); // This is our main expectation $queryCacheProfileMock @@ -661,20 +665,23 @@ public function testConnectionParamsArePassedToTheQueryCacheProfileInExecuteCach public function testShouldNotPassPlatformInParamsToTheQueryCacheProfileInExecuteCacheQuery(): void { - $resultCacheDriverMock = $this->createMock(Cache::class); + $cacheItemMock = $this->createMock(CacheItemInterface::class); + $cacheItemMock->method('isHit')->willReturn(true); + $cacheItemMock->method('get')->willReturn(['realKey' => []]); + + $resultCacheMock = $this->createMock(CacheItemPoolInterface::class); - $resultCacheDriverMock + $resultCacheMock ->expects(self::atLeastOnce()) - ->method('fetch') + ->method('getItem') ->with('cacheKey') - ->will(self::returnValue(['realKey' => []])); + ->willReturn($cacheItemMock); $queryCacheProfileMock = $this->createMock(QueryCacheProfile::class); $queryCacheProfileMock - ->expects(self::any()) - ->method('getResultCacheDriver') - ->will(self::returnValue($resultCacheDriverMock)); + ->method('getResultCache') + ->willReturn($resultCacheMock); $query = 'SELECT 1'; @@ -735,18 +742,21 @@ public function testExecuteCacheQueryStripsPlatformFromConnectionParamsBeforeGen $queryCacheProfile = $this->createMock(QueryCacheProfile::class); - $resultCacheDriver = $this->createMock(Cache::class); + $resultCache = $this->createMock(CacheItemPoolInterface::class); $queryCacheProfile - ->expects(self::any()) - ->method('getResultCacheDriver') - ->will(self::returnValue($resultCacheDriver)); + ->method('getResultCache') + ->willReturn($resultCache); + + $cacheItemMock = $this->createMock(CacheItemInterface::class); + $cacheItemMock->method('isHit')->willReturn(true); + $cacheItemMock->method('get')->willReturn(['realKey' => []]); - $resultCacheDriver + $resultCache ->expects(self::atLeastOnce()) - ->method('fetch') + ->method('getItem') ->with('cacheKey') - ->will(self::returnValue(['realKey' => []])); + ->willReturn($cacheItemMock); $query = 'SELECT 1'; diff --git a/tests/Functional/ResultCacheTest.php b/tests/Functional/ResultCacheTest.php index 87ea8f25cc3..36d7ad668d1 100644 --- a/tests/Functional/ResultCacheTest.php +++ b/tests/Functional/ResultCacheTest.php @@ -8,6 +8,7 @@ use Doctrine\DBAL\Result; use Doctrine\DBAL\Schema\Table; use Doctrine\DBAL\Tests\FunctionalTestCase; +use Symfony\Component\Cache\Adapter\ArrayAdapter; use function array_change_key_case; use function array_shift; @@ -47,8 +48,8 @@ protected function setUp(): void $config = $this->connection->getConfiguration(); $config->setSQLLogger($this->sqlLogger = new DebugStack()); - $cache = new ArrayCache(); - $config->setResultCacheImpl($cache); + $cache = new ArrayAdapter(); + $config->setResultCache($cache); } protected function tearDown(): void @@ -212,6 +213,25 @@ public function testDontFinishNoCache(): void * @dataProvider fetchAllProvider */ public function testFetchingAllRowsSavesCache(callable $fetchAll): void + { + $layerCache = new ArrayAdapter(); + + $result = $this->connection->executeQuery( + 'SELECT * FROM caching WHERE test_int > 500', + [], + [], + new QueryCacheProfile(0, 'testcachekey', $layerCache) + ); + + $fetchAll($result); + + self::assertCount(1, $layerCache->getItem('testcachekey')->get()); + } + + /** + * @dataProvider fetchAllProvider + */ + public function testFetchingAllRowsSavesCacheLegacy(callable $fetchAll): void { $layerCache = new ArrayCache(); @@ -256,7 +276,7 @@ public function testFetchColumn(): void $query = $this->connection->getDatabasePlatform() ->getDummySelectSQL('1'); - $qcp = new QueryCacheProfile(0, null, new ArrayCache()); + $qcp = new QueryCacheProfile(0, null, new ArrayAdapter()); $result = $this->connection->executeCacheQuery($query, [], [], $qcp); $result->fetchFirstColumn(); @@ -335,6 +355,36 @@ public function testChangeCacheImpl(): void return $result->fetchAssociative(); }); + $secondCache = new ArrayAdapter(); + + $stmt = $this->connection->executeQuery( + 'SELECT * FROM caching WHERE test_int > 500', + [], + [], + new QueryCacheProfile(10, 'emptycachekey', $secondCache) + ); + + $this->hydrateViaIteration($stmt, static function (Result $result) { + return $result->fetchAssociative(); + }); + + self::assertCount(2, $this->sqlLogger->queries, 'two hits'); + self::assertCount(1, $secondCache->getItem('emptycachekey')->get()); + } + + public function testChangeCacheImplLegacy(): void + { + $stmt = $this->connection->executeQuery( + 'SELECT * FROM caching WHERE test_int > 500', + [], + [], + new QueryCacheProfile(10, 'emptycachekey') + ); + + $this->hydrateViaIteration($stmt, static function (Result $result) { + return $result->fetchAssociative(); + }); + $secondCache = new ArrayCache(); $stmt = $this->connection->executeQuery(