Browse Source

Add ensureValidKeys() to ensure the validity of cache key sets

This satisfies the requirements of getMultiple() & deleteMultiple() in the PSR-16 specs:

> "MUST be thrown if $keys is neither an array nor a Traversable,or if any of the $keys are not a legal value."

But still does not fully satisfy the requirements of setMultiple(), as currently only the keys are validated:

> "MUST be thrown if $values is neither an array nor a Traversable,or if any of the $values are not a legal value."
Marc Würth 7 years ago
parent
commit
b1cce65c7c
2 changed files with 92 additions and 1 deletions
  1. 25 1
      src/Cache/SimpleCacheEngine.php
  2. 67 0
      tests/TestCase/Cache/SimpleCacheEngineTest.php

+ 25 - 1
src/Cache/SimpleCacheEngine.php

@@ -43,7 +43,7 @@ class SimpleCacheEngine implements CacheInterface
     }
 
     /**
-     * Ensure the validity of the cache key.
+     * Ensure the validity of the given cache key.
      *
      * @param string $key Key to check.
      * @return void
@@ -57,6 +57,24 @@ class SimpleCacheEngine implements CacheInterface
     }
 
     /**
+     * Ensure the validity of the given cache keys.
+     *
+     * @param mixed $keys The keys to check.
+     * @return void
+     * @throws \Psr\SimpleCache\InvalidArgumentException When the keys are not valid.
+     */
+    protected function ensureValidKeys($keys)
+    {
+        if (!is_array($keys) && !($keys instanceOf \Traversable)) {
+            throw new InvalidArgumentException('A cache key set must be either an array or a Traversable.');
+        }
+
+        foreach ($keys as $key) {
+            $this->ensureValidKey($key);
+        }
+    }
+
+    /**
      * Fetches the value for a given key from the cache.
      *
      * @param string $key The unique key of this item in the cache.
@@ -140,6 +158,8 @@ class SimpleCacheEngine implements CacheInterface
      */
     public function getMultiple($keys, $default = null)
     {
+        $this->ensureValidKeys($keys);
+
         $results = $this->innerEngine->readMany($keys);
         foreach ($results as $key => $value) {
             if ($value === false) {
@@ -163,6 +183,8 @@ class SimpleCacheEngine implements CacheInterface
      */
     public function setMultiple($values, $ttl = null)
     {
+        $this->ensureValidKeys(array_keys($values));
+
         if ($ttl !== null) {
             $restore = $this->innerEngine->getConfig('duration');
             $this->innerEngine->setConfig('duration', $ttl);
@@ -186,6 +208,8 @@ class SimpleCacheEngine implements CacheInterface
      */
     public function deleteMultiple($keys)
     {
+        $this->ensureValidKeys($keys);
+
         $result = $this->innerEngine->deleteMany($keys);
         foreach ($result as $key => $success) {
             if ($success === false) {

+ 67 - 0
tests/TestCase/Cache/SimpleCacheEngineTest.php

@@ -185,6 +185,32 @@ class SimpleCacheEngineTest extends TestCase
     }
 
     /**
+     * test getting multiple keys with an invalid key
+     *
+     * @return void
+     */
+    public function testGetMultipleInvalidKey()
+    {
+        $this->expectException(InvalidArgumentException::class);
+        $this->expectExceptionMessage('A cache key must be a non-empty string.');
+        $withInvalidKey = [''];
+        $this->cache->getMultiple($withInvalidKey);
+    }
+
+    /**
+     * test getting multiple keys with an invalid keys parameter
+     *
+     * @return void
+     */
+    public function testGetMultipleInvalidKeys()
+    {
+        $this->expectException(InvalidArgumentException::class);
+        $this->expectExceptionMessage('A cache key set must be either an array or a Traversable.');
+        $notAnArray = 'neither an array nor a Traversable';
+        $this->cache->getMultiple($notAnArray);
+    }
+
+    /**
      * test getMultiple adding defaults in.
      *
      * @return void
@@ -221,6 +247,21 @@ class SimpleCacheEngineTest extends TestCase
     }
 
     /**
+     * test setMultiple with an invalid key
+     *
+     * @return void
+     */
+    public function testSetMultipleInvalidKey()
+    {
+        $this->expectException(InvalidArgumentException::class);
+        $this->expectExceptionMessage('A cache key must be a non-empty string.');
+        $data = [
+            '' => 'a value wuth an invalid key',
+        ];
+        $this->cache->setMultiple($data);
+    }
+
+    /**
      * test setMultiple with ttl parameter
      *
      * @return void
@@ -260,6 +301,32 @@ class SimpleCacheEngineTest extends TestCase
     }
 
     /**
+     * test deleting multiple keys with an invalid key
+     *
+     * @return void
+     */
+    public function testDeleteMultipleInvalidKey()
+    {
+        $this->expectException(InvalidArgumentException::class);
+        $this->expectExceptionMessage('A cache key must be a non-empty string.');
+        $withInvalidKey = [''];
+        $this->cache->deleteMultiple($withInvalidKey);
+    }
+
+    /**
+     * test deleting multiple keys with an invalid keys parameter
+     *
+     * @return void
+     */
+    public function testDeleteMultipleInvalidKeys()
+    {
+        $this->expectException(InvalidArgumentException::class);
+        $this->expectExceptionMessage('A cache key set must be either an array or a Traversable.');
+        $notAnArray = 'neither an array nor a Traversable';
+        $this->cache->deleteMultiple($notAnArray);
+    }
+
+    /**
      * test partial success with deleteMultiple
      *
      * @return void