From 13fa4ebac099cf4446ab8a5b777c88325ce2c829 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maximilian=20B=C3=B6sing?= <2189546+boesing@users.noreply.github.com> Date: Mon, 3 May 2021 21:12:28 +0200 Subject: [PATCH 1/2] bugfix: `CacheItemPoolDecorator` uses `StorageItemInterface::hasItems` to verify deletion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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> --- .../CacheItemPool/CacheItemPoolDecorator.php | 19 ++++++++- .../CacheItemPoolDecoratorTest.php | 42 +++++++++++++++++++ test/Psr/CacheItemPool/MockStorageTrait.php | 12 +++++- 3 files changed, 70 insertions(+), 3 deletions(-) diff --git a/src/Psr/CacheItemPool/CacheItemPoolDecorator.php b/src/Psr/CacheItemPool/CacheItemPoolDecorator.php index 96de64ad..4c19d00a 100644 --- 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 index a8033f61..94050427 100644 --- a/test/Psr/CacheItemPool/CacheItemPoolDecoratorTest.php +++ b/test/Psr/CacheItemPool/CacheItemPoolDecoratorTest.php @@ -531,4 +531,46 @@ private function getAdapter(?ObjectProphecy $storage = null): CacheItemPoolDecor assert($revealedStorage instanceof StorageInterface); return new CacheItemPoolDecorator($revealedStorage); } + + 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 index af96312e..5caa1c0c 100644 --- a/test/Psr/CacheItemPool/MockStorageTrait.php +++ b/test/Psr/CacheItemPool/MockStorageTrait.php @@ -73,6 +73,16 @@ protected function getStorageProphecy($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 getStorageProphecy($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; From 8ff716eab018634a9c19e43bb5aee8fdc0dddd32 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maximilian=20B=C3=B6sing?= <2189546+boesing@users.noreply.github.com> Date: Mon, 3 May 2021 22:21:40 +0200 Subject: [PATCH 2/2] qa: modify unit tests to fit in 2.11.x MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Maximilian Bösing <2189546+boesing@users.noreply.github.com> --- test/Psr/CacheItemPool/CacheItemPoolDecoratorTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/Psr/CacheItemPool/CacheItemPoolDecoratorTest.php b/test/Psr/CacheItemPool/CacheItemPoolDecoratorTest.php index 94050427..7bffd87e 100644 --- a/test/Psr/CacheItemPool/CacheItemPoolDecoratorTest.php +++ b/test/Psr/CacheItemPool/CacheItemPoolDecoratorTest.php @@ -534,7 +534,7 @@ private function getAdapter(?ObjectProphecy $storage = null): CacheItemPoolDecor public function testCanHandleRemoveItemsReturningNonArray() { - $adapter = $this->getStorageProphesy(); + $adapter = $this->getStorageProphecy(); $adapter ->removeItems(Argument::type('array')) ->willReturn(null); @@ -552,7 +552,7 @@ public function testCanHandleRemoveItemsReturningNonArray() */ public function testWillVerifyKeyExistenceByUsingHasItemsWhenDeletionWasNotSuccessful($exists, $sucsessful) { - $adapter = $this->getStorageProphesy(); + $adapter = $this->getStorageProphecy(); $adapter ->removeItems(Argument::type('array')) ->willReturn(['foo']);