bugfix: CacheItemPoolDecorator uses `StorageItemInterface::hasItems…

…` to verify deletion The decorator has to verify if the keys still exist when the underlying storage states the deletion was not successful. This fixes a possible regression in 2.10.2. Closes #99 Signed-off-by: Maximilian Bösing <2189546+boesing@users.noreply.github.com>
laminas · May 3, 2021 · 17c5669 · 17c5669
1 parent e0cd8fc
commit 17c5669
Show file tree

Hide file tree

Showing 3 changed files with 70 additions and 3 deletions.
diff --git a/src/Psr/CacheItemPool/CacheItemPoolDecorator.php b/src/Psr/CacheItemPool/CacheItemPoolDecorator.php
@@ -15,6 +15,9 @@
 use Laminas\Cache\Storage\StorageInterface;
 use Psr\Cache\CacheItemInterface;
 use Psr\Cache\CacheItemPoolInterface;
+use function array_unique;
+use function in_array;
+use function is_array;
 
 /**
  * Decorate laminas-cache adapters as PSR-6 cache item pools.
@@ -194,13 +197,25 @@ public function deleteItems(array $keys)
         $this->deferred = array_diff_key($this->deferred, array_flip($keys));
 
         try {
-            return $this->storage->removeItems($keys) === [];
+            $result = $this->storage->removeItems($keys);
         } catch (Exception\InvalidArgumentException $e) {
             throw new InvalidArgumentException($e->getMessage(), $e->getCode(), $e);
         } catch (Exception\ExceptionInterface $e) {
+            return false;
         }
 
-        return false;
+        // BC compatibility can be removed in 3.0
+        if (! is_array($result)) {
+            return $result !== null;
+        }
+
+        if ($result === []) {
+            return true;
+        }
+
+        $existing = $this->storage->hasItems($result);
+        $unified = array_unique($existing);
+        return !in_array(true, $unified, true);
     }
 
     /**

diff --git a/test/Psr/CacheItemPool/CacheItemPoolDecoratorTest.php b/test/Psr/CacheItemPool/CacheItemPoolDecoratorTest.php
@@ -555,4 +555,46 @@ private function getAdapter($storage = null)
         }
         return new CacheItemPoolDecorator($storage->reveal());
     }
+
+    public function testCanHandleRemoveItemsReturningNonArray()
+    {
+        $adapter = $this->getStorageProphesy();
+        $adapter
+            ->removeItems(Argument::type('array'))
+            ->willReturn(null);
+
+        $cache = new CacheItemPoolDecorator($adapter->reveal());
+
+        self::assertFalse($cache->deleteItems(['foo']));
+    }
+
+    /**
+     * @param bool $exists
+     * @param bool $sucsessful
+     *
+     * @dataProvider deletionVerificationProvider
+     */
+    public function testWillVerifyKeyExistenceByUsingHasItemsWhenDeletionWasNotSuccessful($exists, $sucsessful)
+    {
+        $adapter = $this->getStorageProphesy();
+        $adapter
+            ->removeItems(Argument::type('array'))
+            ->willReturn(['foo']);
+
+        $adapter
+            ->hasItems(Argument::exact(['foo']))
+            ->willReturn(['foo' => $exists]);
+
+        $cache = new CacheItemPoolDecorator($adapter->reveal());
+
+        self::assertEquals($sucsessful, $cache->deleteItems(['foo']));
+    }
+
+    public function deletionVerificationProvider()
+    {
+        return [
+            'deletion failed due to hasItems states the key still exists' => [true, false],
+            'deletion successful due to hasItems states the key does not exist' => [false, true],
+        ];
+    }
 }
diff --git a/test/Psr/CacheItemPool/MockStorageTrait.php b/test/Psr/CacheItemPool/MockStorageTrait.php
@@ -73,6 +73,16 @@ protected function getStorageProphesy($capabilities = false, $options = false, $
                 return $adapterOptions;
             });
 
+        $storage->hasItems(Argument::type('array'))
+            ->will(function ($args) use (&$items) {
+                $keys = $args[0];
+                $status = [];
+                foreach ($keys as $key) {
+                    $status[$key] = array_key_exists($key, $items);
+                }
+
+                return $status;
+            });
         $storage->hasItem(Argument::type('string'))
             ->will(function ($args) use (&$items) {
                 $key = $args[0];
@@ -118,7 +128,7 @@ protected function getStorageProphesy($capabilities = false, $options = false, $
         $storage->removeItems(Argument::type('array'))
             ->will(function ($args) use (&$items) {
                 $items = array_diff_key($items, array_flip($args[0]));
-                return true;
+                return [];
             });
 
         return $storage;