From eea121dea814991a825dc1816c3d6254c627ea0b Mon Sep 17 00:00:00 2001 From: "Alexander M. Turek" Date: Sun, 19 Dec 2021 16:18:10 +0100 Subject: [PATCH] Fix return types of cache interfaces --- lib/Doctrine/ORM/Cache/CollectionHydrator.php | 2 +- lib/Doctrine/ORM/Cache/ConcurrentRegion.php | 4 +- lib/Doctrine/ORM/Cache/DefaultQueryCache.php | 2 +- lib/Doctrine/ORM/Cache/EntityHydrator.php | 2 + lib/Doctrine/ORM/Cache/Region.php | 6 +++ .../ORM/Cache/Region/FileLockRegion.php | 2 - phpstan-baseline.neon | 10 ---- psalm-baseline.xml | 43 --------------- .../Doctrine/Tests/Mocks/CacheRegionMock.php | 34 +++--------- .../Tests/Mocks/ConcurrentRegionMock.php | 52 +++++-------------- .../Tests/ORM/Cache/DefaultQueryCacheTest.php | 12 ++--- 11 files changed, 39 insertions(+), 130 deletions(-) diff --git a/lib/Doctrine/ORM/Cache/CollectionHydrator.php b/lib/Doctrine/ORM/Cache/CollectionHydrator.php index 7adcb5ecf7b..d0842f3c5bc 100644 --- a/lib/Doctrine/ORM/Cache/CollectionHydrator.php +++ b/lib/Doctrine/ORM/Cache/CollectionHydrator.php @@ -28,7 +28,7 @@ public function buildCacheEntry(ClassMetadata $metadata, CollectionCacheKey $key * @param CollectionCacheEntry $entry The cached collection entry. * @param PersistentCollection $collection The collection to load the cache into. * - * @return mixed[] + * @return mixed[]|null */ public function loadCacheEntry(ClassMetadata $metadata, CollectionCacheKey $key, CollectionCacheEntry $entry, PersistentCollection $collection); } diff --git a/lib/Doctrine/ORM/Cache/ConcurrentRegion.php b/lib/Doctrine/ORM/Cache/ConcurrentRegion.php index 92640a7b282..10935795296 100644 --- a/lib/Doctrine/ORM/Cache/ConcurrentRegion.php +++ b/lib/Doctrine/ORM/Cache/ConcurrentRegion.php @@ -18,7 +18,7 @@ interface ConcurrentRegion extends Region * * @param CacheKey $key The key of the item to lock. * - * @return Lock A lock instance or NULL if the lock already exists. + * @return Lock|null A lock instance or NULL if the lock already exists. * * @throws LockException Indicates a problem accessing the region. */ @@ -30,7 +30,7 @@ public function lock(CacheKey $key); * @param CacheKey $key The key of the item to unlock. * @param Lock $lock The lock previously obtained from {@link readLock} * - * @return void + * @return bool * * @throws LockException Indicates a problem accessing the region. */ diff --git a/lib/Doctrine/ORM/Cache/DefaultQueryCache.php b/lib/Doctrine/ORM/Cache/DefaultQueryCache.php index fd4ff78f29e..db7b2ff5ae1 100644 --- a/lib/Doctrine/ORM/Cache/DefaultQueryCache.php +++ b/lib/Doctrine/ORM/Cache/DefaultQueryCache.php @@ -45,7 +45,7 @@ class DefaultQueryCache implements QueryCache /** @var QueryCacheValidator */ private $validator; - /** @var CacheLogger */ + /** @var CacheLogger|null */ protected $cacheLogger; /** @var array */ diff --git a/lib/Doctrine/ORM/Cache/EntityHydrator.php b/lib/Doctrine/ORM/Cache/EntityHydrator.php index 46531dc9c8c..75ac03f3537 100644 --- a/lib/Doctrine/ORM/Cache/EntityHydrator.php +++ b/lib/Doctrine/ORM/Cache/EntityHydrator.php @@ -25,6 +25,8 @@ public function buildCacheEntry(ClassMetadata $metadata, EntityCacheKey $key, $e * @param EntityCacheKey $key The entity cache key. * @param EntityCacheEntry $entry The entity cache entry. * @param object $entity The entity to load the cache into. If not specified, a new entity is created. + * + * @return object|null */ public function loadCacheEntry(ClassMetadata $metadata, EntityCacheKey $key, EntityCacheEntry $entry, $entity = null); } diff --git a/lib/Doctrine/ORM/Cache/Region.php b/lib/Doctrine/ORM/Cache/Region.php index 37d353ef880..46b0624f5ec 100644 --- a/lib/Doctrine/ORM/Cache/Region.php +++ b/lib/Doctrine/ORM/Cache/Region.php @@ -45,6 +45,8 @@ public function get(CacheKey $key); * @param CacheEntry $entry The entry to cache. * @param Lock|null $lock The lock previously obtained. * + * @return bool + * * @throws CacheException Indicates a problem accessing the region. */ public function put(CacheKey $key, CacheEntry $entry, ?Lock $lock = null); @@ -54,6 +56,8 @@ public function put(CacheKey $key, CacheEntry $entry, ?Lock $lock = null); * * @param CacheKey $key The key under which to cache the item. * + * @return bool + * * @throws CacheException Indicates a problem accessing the region. */ public function evict(CacheKey $key); @@ -61,6 +65,8 @@ public function evict(CacheKey $key); /** * Remove all contents of this particular cache region. * + * @return bool + * * @throws CacheException Indicates problem accessing the region. */ public function evictAll(); diff --git a/lib/Doctrine/ORM/Cache/Region/FileLockRegion.php b/lib/Doctrine/ORM/Cache/Region/FileLockRegion.php index 6d0b606d3d5..44e7eaaa2c9 100644 --- a/lib/Doctrine/ORM/Cache/Region/FileLockRegion.php +++ b/lib/Doctrine/ORM/Cache/Region/FileLockRegion.php @@ -228,8 +228,6 @@ public function lock(CacheKey $key) /** * {@inheritdoc} - * - * @return bool */ public function unlock(CacheKey $key, Lock $lock) { diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 01eff86d365..eaa04b2691e 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -1,10 +1,5 @@ parameters: ignoreErrors: - - - message: "#^Method Doctrine\\\\ORM\\\\Cache\\\\DefaultCollectionHydrator\\:\\:loadCacheEntry\\(\\) should return array but returns null\\.$#" - count: 1 - path: lib/Doctrine/ORM/Cache/DefaultCollectionHydrator.php - - message: "#^Call to an undefined method Doctrine\\\\ORM\\\\Persisters\\\\Entity\\\\EntityPersister\\:\\:getCacheRegion\\(\\)\\.$#" count: 1 @@ -110,11 +105,6 @@ parameters: count: 1 path: lib/Doctrine/ORM/Cache/Region/DefaultRegion.php - - - message: "#^Method Doctrine\\\\ORM\\\\Cache\\\\Region\\\\FileLockRegion\\:\\:lock\\(\\) should return Doctrine\\\\ORM\\\\Cache\\\\Lock but returns null\\.$#" - count: 2 - path: lib/Doctrine/ORM/Cache/Region/FileLockRegion.php - - message: "#^Access to an undefined property Doctrine\\\\ORM\\\\Cache\\\\CacheEntry\\:\\:\\$time\\.$#" count: 1 diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 24b0416c18c..9d3ec789da8 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -90,18 +90,7 @@ (string) $fileLockRegionDirectory - - - loadCacheEntry - - - null - - - - loadCacheEntry - $fieldMapping['columnName'] @@ -127,9 +116,6 @@ $assocEntry->class $assocEntry->class - - $cacheConfig->getCacheLogger() - getCacheLogger @@ -149,11 +135,6 @@ $value->class - - - loadCacheEntry - - collectionCacheHit @@ -304,13 +285,6 @@ lock - - - evict - evictAll - put - - MultiGetCache @@ -339,23 +313,6 @@ (string) $name - - - bool - - - lock - - - evict - evictAll - put - - - null - null - - update diff --git a/tests/Doctrine/Tests/Mocks/CacheRegionMock.php b/tests/Doctrine/Tests/Mocks/CacheRegionMock.php index c7e515a117c..781d4d9cf65 100644 --- a/tests/Doctrine/Tests/Mocks/CacheRegionMock.php +++ b/tests/Doctrine/Tests/Mocks/CacheRegionMock.php @@ -24,7 +24,7 @@ class CacheRegionMock implements Region public $returns = []; /** @var string */ - public $name; + public $name = 'mock'; /** * Queue a return value for a specific method invocation @@ -52,50 +52,35 @@ private function getReturn(string $method, $default) return $default; } - /** - * {@inheritdoc} - */ - public function getName() + public function getName(): string { $this->calls[__FUNCTION__][] = []; return $this->name; } - /** - * {@inheritdoc} - */ - public function contains(CacheKey $key) + public function contains(CacheKey $key): bool { $this->calls[__FUNCTION__][] = ['key' => $key]; return $this->getReturn(__FUNCTION__, false); } - /** - * {@inheritdoc} - */ - public function evict(CacheKey $key) + public function evict(CacheKey $key): bool { $this->calls[__FUNCTION__][] = ['key' => $key]; return $this->getReturn(__FUNCTION__, true); } - /** - * {@inheritdoc} - */ - public function evictAll() + public function evictAll(): bool { $this->calls[__FUNCTION__][] = []; return $this->getReturn(__FUNCTION__, true); } - /** - * {@inheritdoc} - */ - public function get(CacheKey $key) + public function get(CacheKey $key): ?CacheEntry { $this->calls[__FUNCTION__][] = ['key' => $key]; @@ -105,17 +90,14 @@ public function get(CacheKey $key) /** * {@inheritdoc} */ - public function getMultiple(CollectionCacheEntry $collection) + public function getMultiple(CollectionCacheEntry $collection): ?array { $this->calls[__FUNCTION__][] = ['collection' => $collection]; return $this->getReturn(__FUNCTION__, null); } - /** - * {@inheritdoc} - */ - public function put(CacheKey $key, CacheEntry $entry, ?Lock $lock = null) + public function put(CacheKey $key, CacheEntry $entry, ?Lock $lock = null): bool { $this->calls[__FUNCTION__][] = ['key' => $key, 'entry' => $entry]; diff --git a/tests/Doctrine/Tests/Mocks/ConcurrentRegionMock.php b/tests/Doctrine/Tests/Mocks/ConcurrentRegionMock.php index b8685327932..2b22dd21d71 100644 --- a/tests/Doctrine/Tests/Mocks/ConcurrentRegionMock.php +++ b/tests/Doctrine/Tests/Mocks/ConcurrentRegionMock.php @@ -41,10 +41,8 @@ public function __construct(Region $region) /** * Dequeue an exception for a specific method invocation - * - * @return mixed */ - private function throwException(string $method) + private function throwException(string $method): void { if (isset($this->exceptions[$method]) && ! empty($this->exceptions[$method])) { $exception = array_shift($this->exceptions[$method]); @@ -71,10 +69,7 @@ public function setLock(CacheKey $key, Lock $lock): void $this->locks[$key->hash] = $lock; } - /** - * {@inheritdoc} - */ - public function contains(CacheKey $key) + public function contains(CacheKey $key): bool { $this->calls[__FUNCTION__][] = ['key' => $key]; @@ -87,10 +82,7 @@ public function contains(CacheKey $key) return $this->region->contains($key); } - /** - * {@inheritdoc} - */ - public function evict(CacheKey $key) + public function evict(CacheKey $key): bool { $this->calls[__FUNCTION__][] = ['key' => $key]; @@ -99,10 +91,7 @@ public function evict(CacheKey $key) return $this->region->evict($key); } - /** - * {@inheritdoc} - */ - public function evictAll() + public function evictAll(): bool { $this->calls[__FUNCTION__][] = []; @@ -111,10 +100,7 @@ public function evictAll() return $this->region->evictAll(); } - /** - * {@inheritdoc} - */ - public function get(CacheKey $key) + public function get(CacheKey $key): ?CacheEntry { $this->calls[__FUNCTION__][] = ['key' => $key]; @@ -130,7 +116,7 @@ public function get(CacheKey $key) /** * {@inheritdoc} */ - public function getMultiple(CollectionCacheEntry $collection) + public function getMultiple(CollectionCacheEntry $collection): ?array { $this->calls[__FUNCTION__][] = ['collection' => $collection]; @@ -139,10 +125,7 @@ public function getMultiple(CollectionCacheEntry $collection) return $this->region->getMultiple($collection); } - /** - * {@inheritdoc} - */ - public function getName() + public function getName(): string { $this->calls[__FUNCTION__][] = []; @@ -151,10 +134,7 @@ public function getName() return $this->region->getName(); } - /** - * {@inheritdoc} - */ - public function put(CacheKey $key, CacheEntry $entry, ?Lock $lock = null) + public function put(CacheKey $key, CacheEntry $entry, ?Lock $lock = null): bool { $this->calls[__FUNCTION__][] = ['key' => $key, 'entry' => $entry]; @@ -171,10 +151,7 @@ public function put(CacheKey $key, CacheEntry $entry, ?Lock $lock = null) return $this->region->put($key, $entry); } - /** - * {@inheritdoc} - */ - public function lock(CacheKey $key) + public function lock(CacheKey $key): ?Lock { $this->calls[__FUNCTION__][] = ['key' => $key]; @@ -187,23 +164,22 @@ public function lock(CacheKey $key) return $this->locks[$key->hash] = Lock::createLockRead(); } - /** - * {@inheritdoc} - */ - public function unlock(CacheKey $key, Lock $lock) + public function unlock(CacheKey $key, Lock $lock): bool { $this->calls[__FUNCTION__][] = ['key' => $key, 'lock' => $lock]; $this->throwException(__FUNCTION__); if (! isset($this->locks[$key->hash])) { - return; + return false; } if ($this->locks[$key->hash]->value !== $lock->value) { - throw LockException::unexpectedLockValue($lock); + throw new LockException('unexpected lock value'); } unset($this->locks[$key->hash]); + + return true; } } diff --git a/tests/Doctrine/Tests/ORM/Cache/DefaultQueryCacheTest.php b/tests/Doctrine/Tests/ORM/Cache/DefaultQueryCacheTest.php index 1768a181e03..17be99bd187 100644 --- a/tests/Doctrine/Tests/ORM/Cache/DefaultQueryCacheTest.php +++ b/tests/Doctrine/Tests/ORM/Cache/DefaultQueryCacheTest.php @@ -4,7 +4,6 @@ namespace Doctrine\Tests\ORM\Cache; -use ArrayObject; use Doctrine\Common\Collections\Collection; use Doctrine\ORM\Cache; use Doctrine\ORM\Cache\DefaultQueryCache; @@ -17,6 +16,7 @@ use Doctrine\ORM\EntityManager; use Doctrine\ORM\EntityManagerInterface; use Doctrine\ORM\Query\ResultSetMappingBuilder; +use Doctrine\Tests\Mocks\CacheEntryMock; use Doctrine\Tests\Mocks\CacheRegionMock; use Doctrine\Tests\Mocks\TimestampRegionMock; use Doctrine\Tests\Models\Cache\City; @@ -540,12 +540,10 @@ public function testGetShouldIgnoreNonQueryCacheEntryResult(): void { $rsm = new ResultSetMappingBuilder($this->em); $key = new QueryCacheKey('query.key1', 0); - $entry = new ArrayObject( - [ - ['identifier' => ['id' => 1]], - ['identifier' => ['id' => 2]], - ] - ); + $entry = new CacheEntryMock([ + ['identifier' => ['id' => 1]], + ['identifier' => ['id' => 2]], + ]); $data = [ ['id' => 1, 'name' => 'Foo'],