Remove MultiGetRegion and add native types to Region implementations (#…

…9403)
doctrine · Jan 19, 2022 · 6f150a3 · 6f150a3
1 parent 30ea3a2
commit 6f150a3
Show file tree

Hide file tree

Showing 14 changed files with 76 additions and 180 deletions.
diff --git a/UPGRADE.md b/UPGRADE.md
@@ -1,5 +1,9 @@
 # Upgrade to 3.0
 
+## BC BREAK: Remove `Doctrine\ORM\Cache\MultiGetRegion`
+
+The interface has been merged into `Doctrine\ORM\Cache\Region`.
+
 ## BC BREAK: Rename `AbstractIdGenerator::generate()` to `generateId()`
 
 * Implementations of `AbstractIdGenerator` have to implement the method

diff --git a/lib/Doctrine/ORM/Cache/ConcurrentRegion.php b/lib/Doctrine/ORM/Cache/ConcurrentRegion.php
@@ -22,17 +22,15 @@ interface ConcurrentRegion extends Region
      *
      * @throws LockException Indicates a problem accessing the region.
      */
-    public function lock(CacheKey $key);
+    public function lock(CacheKey $key): ?Lock;
 
     /**
      * Attempts to read unlock the mapping for the given key.
      *
      * @param CacheKey $key  The key of the item to unlock.
      * @param Lock     $lock The lock previously obtained from {@link readLock}
      *
-     * @return bool
-     *
      * @throws LockException Indicates a problem accessing the region.
      */
-    public function unlock(CacheKey $key, Lock $lock);
+    public function unlock(CacheKey $key, Lock $lock): bool;
 }
diff --git a/lib/Doctrine/ORM/Cache/MultiGetRegion.php b/lib/Doctrine/ORM/Cache/MultiGetRegion.php
diff --git a/lib/Doctrine/ORM/Cache/Region.php b/lib/Doctrine/ORM/Cache/Region.php
@@ -9,23 +9,19 @@
 /**
  * Defines a contract for accessing a particular named region.
  */
-interface Region extends MultiGetRegion
+interface Region
 {
     /**
      * Retrieve the name of this region.
-     *
-     * @return string The region name
      */
-    public function getName();
+    public function getName(): string;
 
     /**
      * Determine whether this region contains data for the given key.
      *
      * @param CacheKey $key The cache key
-     *
-     * @return bool TRUE if the underlying cache contains corresponding data; FALSE otherwise.
      */
-    public function contains(CacheKey $key);
+    public function contains(CacheKey $key): bool;
 
     /**
      * Get an item from the cache.
@@ -36,7 +32,17 @@ public function contains(CacheKey $key);
      *
      * @throws CacheException Indicates a problem accessing the item or region.
      */
-    public function get(CacheKey $key);
+    public function get(CacheKey $key): ?CacheEntry;
+
+    /**
+     * Get all items from the cache identified by $keys.
+     * It returns NULL if some elements can not be found.
+     *
+     * @param CollectionCacheEntry $collection The collection of the items to be retrieved.
+     *
+     * @return CacheEntry[]|null The cached entries or NULL if one or more entries can not be found
+     */
+    public function getMultiple(CollectionCacheEntry $collection): ?array;
 
     /**
      * Put an item into the cache.
@@ -45,29 +51,23 @@ 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);
+    public function put(CacheKey $key, CacheEntry $entry, ?Lock $lock = null): bool;
 
     /**
      * Remove an item from the cache.
      *
      * @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);
+    public function evict(CacheKey $key): bool;
 
     /**
      * Remove all contents of this particular cache region.
      *
-     * @return bool
-     *
      * @throws CacheException Indicates problem accessing the region.
      */
-    public function evictAll();
+    public function evictAll(): bool;
 }
diff --git a/lib/Doctrine/ORM/Cache/Region/DefaultRegion.php b/lib/Doctrine/ORM/Cache/Region/DefaultRegion.php
@@ -37,26 +37,17 @@ public function __construct(string $name, CacheItemPoolInterface $cacheItemPool,
         $this->lifetime      = $lifetime;
     }
 
-    /**
-     * {@inheritdoc}
-     */
-    public function getName()
+    public function getName(): string
     {
         return $this->name;
     }
 
-    /**
-     * {@inheritdoc}
-     */
-    public function contains(CacheKey $key)
+    public function contains(CacheKey $key): bool
     {
         return $this->cacheItemPool->hasItem($this->getCacheEntryKey($key));
     }
 
-    /**
-     * {@inheritdoc}
-     */
-    public function get(CacheKey $key)
+    public function get(CacheKey $key): ?CacheEntry
     {
         $item  = $this->cacheItemPool->getItem($this->getCacheEntryKey($key));
         $entry = $item->isHit() ? $item->get() : null;
@@ -71,7 +62,7 @@ public function get(CacheKey $key)
     /**
      * {@inheritdoc}
      */
-    public function getMultiple(CollectionCacheEntry $collection)
+    public function getMultiple(CollectionCacheEntry $collection): ?array
     {
         $keys = array_map(
             Closure::fromCallable([$this, 'getCacheEntryKey']),
@@ -100,12 +91,7 @@ public function getMultiple(CollectionCacheEntry $collection)
         return $result;
     }
 
-    /**
-     * {@inheritdoc}
-     *
-     * @return bool
-     */
-    public function put(CacheKey $key, CacheEntry $entry, ?Lock $lock = null)
+    public function put(CacheKey $key, CacheEntry $entry, ?Lock $lock = null): bool
     {
         $item = $this->cacheItemPool
             ->getItem($this->getCacheEntryKey($key))
@@ -118,22 +104,12 @@ public function put(CacheKey $key, CacheEntry $entry, ?Lock $lock = null)
         return $this->cacheItemPool->save($item);
     }
 
-    /**
-     * {@inheritdoc}
-     *
-     * @return bool
-     */
-    public function evict(CacheKey $key)
+    public function evict(CacheKey $key): bool
     {
         return $this->cacheItemPool->deleteItem($this->getCacheEntryKey($key));
     }
 
-    /**
-     * {@inheritdoc}
-     *
-     * @return bool
-     */
-    public function evictAll()
+    public function evictAll(): bool
     {
         return $this->cacheItemPool->clear(self::REGION_PREFIX . $this->name);
     }

diff --git a/lib/Doctrine/ORM/Cache/Region/FileLockRegion.php b/lib/Doctrine/ORM/Cache/Region/FileLockRegion.php
@@ -37,34 +37,23 @@ class FileLockRegion implements ConcurrentRegion
 {
     public const LOCK_EXTENSION = 'lock';
 
-    /** @var Region */
-    private $region;
-
-    /** @var string */
-    private $directory;
-
-    /** @psalm-var numeric-string */
-    private $lockLifetime;
-
     /**
-     * @param string         $directory
-     * @param numeric-string $lockLifetime
+     * @param numeric-string|int $lockLifetime
      *
      * @throws InvalidArgumentException
      */
-    public function __construct(Region $region, $directory, $lockLifetime)
-    {
+    public function __construct(
+        private Region $region,
+        private string $directory,
+        private string|int $lockLifetime
+    ) {
         if (! is_dir($directory) && ! @mkdir($directory, 0775, true)) {
             throw new InvalidArgumentException(sprintf('The directory "%s" does not exist and could not be created.', $directory));
         }
 
         if (! is_writable($directory)) {
             throw new InvalidArgumentException(sprintf('The directory "%s" is not writable.', $directory));
         }
-
-        $this->region       = $region;
-        $this->directory    = $directory;
-        $this->lockLifetime = $lockLifetime;
     }
 
     private function isLocked(CacheKey $key, ?Lock $lock = null): bool
@@ -119,18 +108,12 @@ private function getLockTime(string $filename)
         return @fileatime($filename);
     }
 
-    /**
-     * {@inheritdoc}
-     */
-    public function getName()
+    public function getName(): string
     {
         return $this->region->getName();
     }
 
-    /**
-     * {@inheritdoc}
-     */
-    public function contains(CacheKey $key)
+    public function contains(CacheKey $key): bool
     {
         if ($this->isLocked($key)) {
             return false;
@@ -139,10 +122,7 @@ public function contains(CacheKey $key)
         return $this->region->contains($key);
     }
 
-    /**
-     * {@inheritdoc}
-     */
-    public function get(CacheKey $key)
+    public function get(CacheKey $key): ?CacheEntry
     {
         if ($this->isLocked($key)) {
             return null;
@@ -154,7 +134,7 @@ public function get(CacheKey $key)
     /**
      * {@inheritdoc}
      */
-    public function getMultiple(CollectionCacheEntry $collection)
+    public function getMultiple(CollectionCacheEntry $collection): ?array
     {
         if (array_filter(array_map([$this, 'isLocked'], $collection->identifiers))) {
             return null;
@@ -163,10 +143,7 @@ public function getMultiple(CollectionCacheEntry $collection)
         return $this->region->getMultiple($collection);
     }
 
-    /**
-     * {@inheritdoc}
-     */
-    public function put(CacheKey $key, CacheEntry $entry, ?Lock $lock = null)
+    public function put(CacheKey $key, CacheEntry $entry, ?Lock $lock = null): bool
     {
         if ($this->isLocked($key, $lock)) {
             return false;
@@ -175,10 +152,7 @@ public function put(CacheKey $key, CacheEntry $entry, ?Lock $lock = null)
         return $this->region->put($key, $entry);
     }
 
-    /**
-     * {@inheritdoc}
-     */
-    public function evict(CacheKey $key)
+    public function evict(CacheKey $key): bool
     {
         if ($this->isLocked($key)) {
             @unlink($this->getLockFileName($key));
@@ -187,10 +161,7 @@ public function evict(CacheKey $key)
         return $this->region->evict($key);
     }
 
-    /**
-     * {@inheritdoc}
-     */
-    public function evictAll()
+    public function evictAll(): bool
     {
         // The check below is necessary because on some platforms glob returns false
         // when nothing matched (even though no errors occurred)
@@ -205,10 +176,7 @@ public function evictAll()
         return $this->region->evictAll();
     }
 
-    /**
-     * {@inheritdoc}
-     */
-    public function lock(CacheKey $key)
+    public function lock(CacheKey $key): ?Lock
     {
         if ($this->isLocked($key)) {
             return null;
@@ -226,10 +194,7 @@ public function lock(CacheKey $key)
         return $lock;
     }
 
-    /**
-     * {@inheritdoc}
-     */
-    public function unlock(CacheKey $key, Lock $lock)
+    public function unlock(CacheKey $key, Lock $lock): bool
     {
         if ($this->isLocked($key, $lock)) {
             return false;

diff --git a/lib/Doctrine/ORM/Cache/Region/UpdateTimestampCache.php b/lib/Doctrine/ORM/Cache/Region/UpdateTimestampCache.php
@@ -13,10 +13,7 @@
  */
 class UpdateTimestampCache extends DefaultRegion implements TimestampRegion
 {
-    /**
-     * {@inheritdoc}
-     */
-    public function update(CacheKey $key)
+    public function update(CacheKey $key): void
     {
         $this->put($key, new TimestampCacheEntry());
     }

diff --git a/lib/Doctrine/ORM/Cache/TimestampRegion.php b/lib/Doctrine/ORM/Cache/TimestampRegion.php
@@ -16,5 +16,5 @@ interface TimestampRegion extends Region
      *
      * @throws LockException Indicates a problem accessing the region.
      */
-    public function update(CacheKey $key);
+    public function update(CacheKey $key): void;
 }
diff --git a/psalm-baseline.xml b/psalm-baseline.xml
@@ -275,11 +275,6 @@
       <code>lock</code>
     </UndefinedInterfaceMethod>
   </file>
-  <file src="lib/Doctrine/ORM/Cache/Region/UpdateTimestampCache.php">
-    <MissingReturnType occurrences="1">
-      <code>update</code>
-    </MissingReturnType>
-  </file>
   <file src="lib/Doctrine/ORM/Cache/RegionsConfiguration.php">
     <RedundantCastGivenDocblockType occurrences="6">
       <code>(int) $defaultLifetime</code>
@@ -305,11 +300,6 @@
       <code>$timestamp-&gt;time</code>
     </NoInterfaceProperties>
   </file>
-  <file src="lib/Doctrine/ORM/Cache/TimestampRegion.php">
-    <MissingReturnType occurrences="1">
-      <code>update</code>
-    </MissingReturnType>
-  </file>
   <file src="lib/Doctrine/ORM/Configuration.php">
     <ArgumentTypeCoercion occurrences="2">
       <code>$className</code>