Merge 178354b into 4e84cd1

zendframework · Apr 22, 2018 · ab9386c · ab9386c
2 parents 4e84cd1 + 178354b
commit ab9386c
Show file tree

Hide file tree

Showing 3 changed files with 53 additions and 33 deletions.
diff --git a/src/Psr/SimpleCache/SimpleCacheDecorator.php b/src/Psr/SimpleCache/SimpleCacheDecorator.php
@@ -18,6 +18,7 @@
 use Zend\Cache\Psr\SerializationTrait;
 use Zend\Cache\Storage\FlushableInterface;
 use Zend\Cache\Storage\StorageInterface;
+use Zend\EventManager\EventsCapableInterface;
 
 /**
  * Decorate a zend-cache storage adapter for usage as a PSR-16 implementation.
@@ -56,12 +57,44 @@ class SimpleCacheDecorator implements SimpleCacheInterface
 
     public function __construct(StorageInterface $storage)
     {
+        $this->attachExceptionHandlers();
         $this->memoizeSerializationCapabilities($storage);
         $this->memoizeTtlCapabilities($storage);
         $this->storage = $storage;
         $this->utc = new DateTimeZone('UTC');
     }
 
+    /**
+     * Attach exception handlers.
+     *
+     * We want to return true deleting keys even if a key doesn't exist.
+     * Zend cache storage adapters return false trying to delete a key when it doesn't exist.
+     * We need to know if an exception occurred when possible, and return false instead of true.
+     *
+     * Listeners are attached with the lower priority so the ExceptionHandler plugin is able to throw every exception.
+     */
+    private function attachExceptionHandlers()
+    {
+        if (! $this->storage instanceof EventsCapableInterface) {
+            return;
+        }
+
+        $events = [
+            'removeItem.exception',
+            'removeItems.exception',
+        ];
+
+        foreach ($events as $event) {
+            $this->storage->getEventManager()->attach(
+                $event,
+                function ($e) {
+                    throw new StorageException('A storage exception occurred', 0, $e);
+                },
+                \PHP_INT_MAX
+            );
+        }
+    }
+
     /**
      * {@inheritDoc}
      */
@@ -132,12 +165,11 @@ public function delete($key)
     {
         $this->validateKey($key);
 
-        if (! $this->storage->hasItem($key)) {
-            return true;
-        }
-
         try {
-            return $this->storage->removeItem($key);
+            $this->storage->removeItem($key);
+            return true;
+        } catch (StorageException $e) {
+            return false;
         } catch (Throwable $e) {
             throw static::translateException($e);
         } catch (Exception $e) {
@@ -255,24 +287,15 @@ public function deleteMultiple($keys)
         array_walk($keys, [$this, 'validateKey']);
 
         try {
-            $result = $this->storage->removeItems($keys);
+            $this->storage->removeItems($keys);
+            return true;
+        } catch (StorageException $e) {
+            return false;
         } catch (Throwable $e) {
             throw static::translateException($e);
         } catch (Exception $e) {
             throw static::translateException($e);
         }
-
-        if (empty($result)) {
-            return true;
-        }
-
-        foreach ($result as $index => $key) {
-            if (! $this->storage->hasItem($key)) {
-                unset($result[$index]);
-            }
-        }
-
-        return empty($result);
     }
 
     /**

diff --git a/src/Psr/SimpleCache/StorageException.php b/src/Psr/SimpleCache/StorageException.php
@@ -0,0 +1,9 @@
+<?php
+
+namespace Zend\Cache\Psr\SimpleCache;
+
+use Zend\Cache\Exception\RuntimeException;
+
+class StorageException extends RuntimeException
+{
+}
diff --git a/test/Psr/SimpleCache/SimpleCacheDecoratorTest.php b/test/Psr/SimpleCache/SimpleCacheDecoratorTest.php
@@ -379,22 +379,19 @@ public function testSetShouldReRaiseExceptionThrownByStorage()
 
     public function testDeleteShouldProxyToStorage()
     {
-        $this->storage->hasItem('key')->willReturn(true);
-        $this->storage->removeItem('key')->willReturn(true);
+        $this->storage->removeItem('key')->shouldBeCalled();
         $this->assertTrue($this->cache->delete('key'));
     }
 
     public function testDeleteShouldReturnTrueWhenItemDoesNotExist()
     {
-        $this->storage->hasItem('key')->willReturn(false);
-        $this->storage->removeItem('key')->shouldNotBeCalled();
+        $this->storage->removeItem('key')->shouldBeCalled();
         $this->assertTrue($this->cache->delete('key'));
     }
 
     public function testDeleteShouldReRaiseExceptionThrownByStorage()
     {
         $exception = new Exception\ExtensionNotLoadedException('failure', 500);
-        $this->storage->hasItem('key')->willReturn(true);
         $this->storage->removeItem('key')->willThrow($exception);
 
         try {
@@ -761,19 +758,10 @@ public function testDeleteMultipleReturnsTrueWhenProvidedWithAnEmptyArrayOfKeys(
         $this->assertTrue($this->cache->deleteMultiple([]));
     }
 
-    public function testDeleteMultipleProxiesToStorageAndReturnsFalseIfStorageReturnsNonEmptyArray()
-    {
-        $keys = ['one', 'two', 'three'];
-        $this->storage->removeItems($keys)->willReturn(['two']);
-        $this->storage->hasItem('two')->willReturn(true);
-        $this->assertFalse($this->cache->deleteMultiple($keys));
-    }
-
     public function testDeleteMultipleReturnsTrueIfKeyReturnedByStorageDoesNotExist()
     {
         $keys = ['one', 'two', 'three'];
-        $this->storage->removeItems($keys)->willReturn(['two']);
-        $this->storage->hasItem('two')->willReturn(false);
+        $this->storage->removeItems($keys)->shouldBeCalled();
         $this->assertTrue($this->cache->deleteMultiple($keys));
     }