#156 PSR-6 / PSR-16: use serialization with storage plugin

CHANGELOG.md

@@ -8,7 +8,7 @@ All notable changes to this project will be documented in this file, in reverse
 
 - [#148](https://github.com/zendframework/zend-cache/pull/148) adds support for PHP 7.1 and 7.2.
 
-- [#46](https://github.com/zendframework/zend-cache/issues/46) and [#155](https://github.com/zendframework/zend-cache/issues/155) add support for [PSR-6](https://www.php-fig.org/psr/psr-6/) (Caching Interface).
+- [#46](https://github.com/zendframework/zend-cache/issues/46), [#155](https://github.com/zendframework/zend-cache/issues/155), and [#161](https://github.com/zendframework/zend-cache/issues/161) add support for [PSR-6](https://www.php-fig.org/psr/psr-6/) (Caching Interface).
   They provides an implementation of `Psr\Cache\CacheItemPoolInterface` via
   `Zend\Cache\Psr\CacheItemPool\CacheItemPoolDecorator`, which accepts a
   `Zend\Cache\Storage\StorageInterface` instance to its constructor, and proxies
@@ -17,7 +17,7 @@ All notable changes to this project will be documented in this file, in reverse
   which provides a value object for both introspecting cache fetch results, as
   well as providing values to cache.
 
-- [#152](https://github.com/zendframework/zend-cache/pull/152) [#155](https://github.com/zendframework/zend-cache/pull/155), and [#159](https://github.com/zendframework/zend-cache/pull/159)
+- [#152](https://github.com/zendframework/zend-cache/pull/152), [#155](https://github.com/zendframework/zend-cache/pull/155), [#159](https://github.com/zendframework/zend-cache/pull/159), and [#161](https://github.com/zendframework/zend-cache/issues/161)
   add an adapter providing [PSR-16](https://www.php-fig.org/psr/psr-16/) (Caching Library Interface) support.
   The new class, `Zend\Cache\Psr\SimpleCache\SimpleCacheDecorator`, accepts a
   `Zend\Cache\Storage\StorageInterface` instance to its constructor, and proxies

src/Psr/CacheItemPool/CacheItemPoolDecorator.php

@@ -49,7 +49,6 @@ class CacheItemPoolDecorator implements CacheItemPoolInterface
     public function __construct(StorageInterface $storage)
     {
         $this->validateStorage($storage);
-        $this->memoizeSerializationCapabilities($storage);
         $this->storage = $storage;
     }
 
@@ -75,11 +74,6 @@ public function getItem($key)
             $isHit = false;
             try {
                 $value = $this->storage->getItem($key, $isHit);
-
-                if ($this->serializeValues && $isHit) {
-                    // will set $isHit = false if unserialization fails
-                    extract($this->unserialize($value));
-                }
             } catch (Exception\InvalidArgumentException $e) {
                 throw new InvalidArgumentException($e->getMessage(), $e->getCode(), $e);
             } catch (Exception\ExceptionInterface $e) {
@@ -121,11 +115,6 @@ public function getItems(array $keys = [])
 
             foreach ($cacheItems as $key => $value) {
                 $isHit = true;
-                if ($this->serializeValues) {
-                    // will set $isHit = false if unserialization fails
-                    extract($this->unserialize($value));
-                }
-
                 $items[$key] = new CacheItem($key, $value, $isHit);
             }
 
@@ -241,9 +230,6 @@ public function save(CacheItemInterface $item)
         try {
             // get item value and serialize, if required
             $value = $item->get();
-            if ($this->serializeValues) {
-                $value = serialize($value);
-            }
 
             // reset TTL on adapter, if required
             if ($itemTtl > 0) {
@@ -304,6 +290,15 @@ public function commit()
      */
     private function validateStorage(StorageInterface $storage)
     {
+        if ($this->isSerializationRequired($storage)) {
+            throw new CacheException(sprintf(
+                'The storage adapter "%s" requires a serializer plugin; please see'
+                . ' https://docs.zendframework.com/zend-cache/storage/plugin/#quick-start'
+                . ' for details on how to attach the plugin to your adapter.',
+                get_class($storage)
+            ));
+        }
+
         // all current adapters implement this
         if (! $storage instanceof FlushableInterface) {
             throw new CacheException(sprintf(
@@ -337,24 +332,6 @@ private function validateStorage(StorageInterface $storage)
         }
     }
 
-    /**
-     * Unserializes value, marking isHit false if it fails
-     * @param $value
-     * @return array
-     */
-    private function unserialize($value)
-    {
-        if ($value == static::$serializedFalse) {
-            return ['value' => false, 'isHit' => true];
-        }
-
-        if (false === ($value = unserialize($value))) {
-            return ['value' => null, 'isHit' => false];
-        }
-
-        return ['value' => $value, 'isHit' => true];
-    }
-
     /**
      * Returns true if deferred item exists for given key and has not expired
      * @param string $key

src/Psr/SerializationTrait.php

@@ -15,55 +15,27 @@
  */
 trait SerializationTrait
 {
-    /**
-     * @var bool
-     */
-    private $serializeValues = false;
-
-    /**
-     * @var string
-     */
-    private static $serializedFalse;
-
     /**
      * Determine if the given storage adapter requires serialization.
      *
-     * Determines if the given storage adapter requires serialization. If so,
-     * set $serializeValues to true, and serialize a boolean false for later
-     * comparisons.
-     *
      * @param StorageInterface $storage
-     * @return void
+     * @return bool
      */
-    private function memoizeSerializationCapabilities(StorageInterface $storage)
+    private function isSerializationRequired(StorageInterface $storage)
     {
         $capabilities = $storage->getCapabilities();
         $requiredTypes = ['string', 'integer', 'double', 'boolean', 'NULL', 'array', 'object'];
         $types = $capabilities->getSupportedDatatypes();
-        $shouldSerialize = false;
 
         foreach ($requiredTypes as $type) {
             // 'object' => 'object' is OK
             // 'integer' => 'string' is not (redis)
             // 'integer' => 'integer' is not (memcache)
             if (! (isset($types[$type]) && in_array($types[$type], [true, 'array', 'object'], true))) {
-                $shouldSerialize = true;
-                break;
+                return true;
             }
         }
 
-        if ($shouldSerialize) {
-            static::$serializedFalse = serialize(false);
-        }
-
-        $this->serializeValues = $shouldSerialize;
+        return false;
     }
-
-    /**
-     * Unserialize a value retrieved from the cache.
-     *
-     * @param string $value
-     * @return mixed
-     */
-    abstract public function unserialize($value);
 }

src/Psr/SimpleCache/SimpleCacheDecorator.php

@@ -57,7 +57,15 @@ class SimpleCacheDecorator implements SimpleCacheInterface
 
     public function __construct(StorageInterface $storage)
     {
-        $this->memoizeSerializationCapabilities($storage);
+        if ($this->isSerializationRequired($storage)) {
+            throw new SimpleCacheException(sprintf(
+                'The storage adapter "%s" requires a serializer plugin; please see'
+                . ' https://docs.zendframework.com/zend-cache/storage/plugin/#quick-start'
+                . ' for details on how to attach the plugin to your adapter.',
+                get_class($storage)
+            ));
+        }
+
         $this->memoizeTtlCapabilities($storage);
         $this->storage = $storage;
         $this->utc = new DateTimeZone('UTC');
@@ -79,11 +87,6 @@ public function get($key, $default = null)
             throw static::translateException($e);
         }
 
-        if ($this->serializeValues && $this->success) {
-            $result = $this->unserialize($result);
-            return $result === null ? $default : $result;
-        }
-
         $result = $result === null ? $default : $result;
         return $this->success ? $result : $default;
     }
@@ -111,7 +114,6 @@ public function set($key, $value, $ttl = null)
         $options = $this->storage->getOptions();
         $previousTtl = $options->getTtl();
         $options->setTtl($ttl);
-        $value = $this->serializeValues ? serialize($value) : $value;
 
         try {
             $result = $this->storage->setItem($key, $value);
@@ -185,12 +187,6 @@ public function getMultiple($keys, $default = null)
                 $results[$key] = $default;
                 continue;
             }
-
-            if (isset($results[$key]) && $this->serializeValues) {
-                $value = $this->unserialize($results[$key]);
-                $results[$key] = null === $value ? $default : $value;
-                continue;
-            }
         }
 
         return $results;
@@ -219,10 +215,6 @@ public function setMultiple($values, $ttl = null)
             return false;
         }
 
-        if ($this->serializeValues) {
-            return $this->setMultipleForStorageRequiringSerialization($values, $ttl);
-        }
-
         $options = $this->storage->getOptions();
         $previousTtl = $options->getTtl();
         $options->setTtl($ttl);
@@ -360,32 +352,6 @@ private function validateKey($key)
         }
     }
 
-    /**
-     * Unserializes a value.
-     *
-     * If the $value returned matches a serialized false value, returns
-     * false for the value.
-     *
-     * Otherwise, it unserializes the value. If it is a boolean false at
-     * that point, it returns a null; otherwise it returns the unserialized
-     * value.
-     *
-     * @param string $value
-     * @return mixed
-     */
-    private function unserialize($value)
-    {
-        if ($value == static::$serializedFalse) {
-            return false;
-        }
-
-        if (false === ($value = unserialize($value))) {
-            return null;
-        }
-
-        return $value;
-    }
-
     /**
      * Determine if the storage adapter provides per-item TTL capabilities
      *
@@ -478,30 +444,4 @@ private function convertIterableToArray($iterable, $useKeys, $forMethod)
         }
         return $array;
     }
-
-    /**
-     * Workaround for adapters requiring serialization of values.
-     *
-     * In performing integration tests, we found that every adapter that
-     * requires serialization would fail the tests for setMultiple(), in
-     * exactly the same way: inability to unserialize the data returned.
-     *
-     * The problem appears to be how each adapter's setItems() method handles
-     * the data provided.
-     *
-     * Storing each one by one works perfectly, however, so this is the
-     * approach taken.
-     *
-     * @param iterable $values
-     * @param null|int $ttl
-     * @return bool
-     */
-    private function setMultipleForStorageRequiringSerialization($values, $ttl)
-    {
-        $result = true;
-        foreach ($values as $key => $value) {
-            $result = $result && $this->set($key, $value, $ttl);
-        }
-        return $result;
-    }
 }

test/Psr/CacheItemPool/CacheItemPoolDecoratorTest.php

@@ -10,9 +10,11 @@
 use PHPUnit\Framework\TestCase;
 use Prophecy\Argument;
 use Psr\Cache\CacheItemInterface;
+use stdClass;
 use Zend\Cache\Exception;
 use Zend\Cache\Psr\CacheItemPool\CacheItemPoolDecorator;
 use Zend\Cache\Storage\Adapter\AbstractAdapter;
+use Zend\Cache\Storage\Capabilities;
 use Zend\Cache\Storage\StorageInterface;
 
 class CacheItemPoolDecoratorTest extends TestCase
@@ -25,6 +27,39 @@ class CacheItemPoolDecoratorTest extends TestCase
     public function testStorageNotFlushableThrowsException()
     {
         $storage = $this->prophesize(StorageInterface::class);
+
+        $capabilities = new Capabilities($storage->reveal(), new stdClass(), $this->defaultCapabilities);
+
+        $storage->getCapabilities()->willReturn($capabilities);
+
+        $this->getAdapter($storage);
+    }
+
+    /**
+     * @expectedException \Zend\Cache\Psr\CacheItemPool\CacheException
+     */
+    public function testStorageNeedsSerializerWillThrowException()
+    {
+        $storage = $this->prophesize(StorageInterface::class);
+
+        $dataTypes = [
+            'staticTtl' => true,
+            'minTtl' => 1,
+            'supportedDatatypes' => [
+                'NULL'     => true,
+                'boolean'  => true,
+                'integer'  => true,
+                'double'   => false,
+                'string'   => true,
+                'array'    => true,
+                'object'   => 'object',
+                'resource' => false,
+            ],
+        ];
+        $capabilities = new Capabilities($storage->reveal(), new stdClass(), $dataTypes);
+
+        $storage->getCapabilities()->willReturn($capabilities);
+
         $this->getAdapter($storage);
     }
 
@@ -46,50 +81,6 @@ public function testStorageZeroMinTtlThrowsException()
         $this->getAdapter($storage);
     }
 
-    public function testUnserialize()
-    {
-        // we can't test this without reflection: we can't prophesy args-by-ref (ie $storage->getItem('key', $isHit))
-        $unserialize = new \ReflectionMethod(CacheItemPoolDecorator::class, 'unserialize');
-        $unserialize->setAccessible(true);
-
-        $capabilities = $this->defaultCapabilities;
-        $capabilities['supportedDatatypes']['object'] = false;
-        $storage = $this->getStorageProphesy($capabilities);
-        $adapter = $this->getAdapter($storage);
-
-        $value = false;
-        $result = $unserialize->invoke($adapter, serialize($value));
-        $this->assertTrue($result['isHit'], "False value should be a hit");
-        $this->assertFalse($result['value'], "False value should be unserialized correctly");
-
-        $value = ['a' => 'b'];
-        $result = $unserialize->invoke($adapter, serialize($value));
-        $this->assertTrue($result['isHit'], "Array should be a hit");
-        $this->assertEquals($value, $result['value'], "Array should be unserialized correctly");
-
-        $result = $unserialize->invoke($adapter, null);
-        $this->assertFalse($result['isHit'], "Unserializable value should not be a hit");
-        $this->assertNull($result['value'], "Unserializable value should be null");
-    }
-
-    public function testUnsupportedDatatypeSerializesValues()
-    {
-        $test = ['a' => 'b'];
-        foreach ($this->defaultCapabilities['supportedDatatypes'] as $type => $value) {
-            if ($value) {
-                $capabilities = $this->defaultCapabilities;
-                $capabilities['supportedDatatypes'][$type] = false;
-                $storage = $this->getStorageProphesy($capabilities, false, AbstractAdapter::class);
-                $adapter = $this->getAdapter($storage);
-                $item = $adapter->getItem('foo');
-                $item->set($test);
-                $adapter->save($item);
-                $items = $adapter->getItems(['foo']);
-                $this->assertEquals($test, $items['foo']->get());
-            }
-        }
-    }
-
     public function testGetDeferredItem()
     {
         $adapter = $this->getAdapter();
@@ -542,7 +533,7 @@ private function getInvalidKeys()
             'key\\',
             'key@',
             'key:',
-            new \stdClass()
+            new stdClass()
         ];
     }