From 827b4e5d756363cf666b42f4f6e8ff4bc13acba9 Mon Sep 17 00:00:00 2001 From: Matias Griese Date: Fri, 19 Jan 2018 11:28:47 +0200 Subject: [PATCH 1/4] Clean up Utils class with minor fixes --- CHANGELOG.md | 1 + .../Framework/Cache/Adapter/ChainCache.php | 25 ++++++++++- .../Framework/Cache/Adapter/DoctrineCache.php | 26 +++++++++++ .../Framework/Cache/Adapter/FileCache.php | 44 ++++++++++++++++++- .../Framework/Cache/Adapter/MemoryCache.php | 9 +--- .../Framework/Cache/Adapter/SessionCache.php | 12 +++-- .../src/Grav/Framework/Cache/CacheTrait.php | 25 +++++++++-- 7 files changed, 125 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3fb9a791b..b611a2741 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ 1. [](#improved) * Objects: Add protected function `getElement()` to get serialized value for a single property * `ObjectPropertyTrait`: Added protected functions `isPropertyLoaded()`, `offsetLoad()`, `offsetPrepare()` and `offsetSerialize()` + * `Grav\Framework\Cache`: Allow unlimited TTL # v1.4.0-beta.3 ## 12/29/2017 diff --git a/system/src/Grav/Framework/Cache/Adapter/ChainCache.php b/system/src/Grav/Framework/Cache/Adapter/ChainCache.php index 7301f3c0d..75b3b64d8 100644 --- a/system/src/Grav/Framework/Cache/Adapter/ChainCache.php +++ b/system/src/Grav/Framework/Cache/Adapter/ChainCache.php @@ -58,6 +58,9 @@ class ChainCache extends AbstractCache $this->count = count($caches); } + /** + * @inheritdoc + */ public function doGet($key, $miss) { foreach ($this->caches as $i => $cache) { @@ -75,6 +78,9 @@ class ChainCache extends AbstractCache return $miss; } + /** + * @inheritdoc + */ public function doSet($key, $value, $ttl) { $success = true; @@ -87,6 +93,9 @@ class ChainCache extends AbstractCache return $success; } + /** + * @inheritdoc + */ public function doDelete($key) { $success = true; @@ -99,6 +108,9 @@ class ChainCache extends AbstractCache return $success; } + /** + * @inheritdoc + */ public function doClear() { $success = true; @@ -110,7 +122,9 @@ class ChainCache extends AbstractCache return $success; } - + /** + * @inheritdoc + */ public function doGetMultiple($keys, $miss) { $list = []; @@ -136,6 +150,9 @@ class ChainCache extends AbstractCache return $values; } + /** + * @inheritdoc + */ public function doSetMultiple($values, $ttl) { $success = true; @@ -148,6 +165,9 @@ class ChainCache extends AbstractCache return $success; } + /** + * @inheritdoc + */ public function doDeleteMultiple($keys) { $success = true; @@ -160,6 +180,9 @@ class ChainCache extends AbstractCache return $success; } + /** + * @inheritdoc + */ public function doHas($key) { foreach ($this->caches as $cache) { diff --git a/system/src/Grav/Framework/Cache/Adapter/DoctrineCache.php b/system/src/Grav/Framework/Cache/Adapter/DoctrineCache.php index c78d5abd4..25e42ea98 100644 --- a/system/src/Grav/Framework/Cache/Adapter/DoctrineCache.php +++ b/system/src/Grav/Framework/Cache/Adapter/DoctrineCache.php @@ -10,6 +10,7 @@ namespace Grav\Framework\Cache\Adapter; use Doctrine\Common\Cache\CacheProvider; use Grav\Framework\Cache\AbstractCache; +use Grav\Framework\Cache\Exception\InvalidArgumentException; /** * Cache class for PSR-16 compatible "Simple Cache" implementation using Doctrine Cache backend. @@ -28,6 +29,7 @@ class DoctrineCache extends AbstractCache * @param CacheProvider $doctrineCache * @param string $namespace * @param null|int|\DateInterval $defaultLifetime + * @throws InvalidArgumentException */ public function __construct(CacheProvider $doctrineCache, $namespace = '', $defaultLifetime = null) { @@ -41,6 +43,9 @@ class DoctrineCache extends AbstractCache $this->driver = $doctrineCache; } + /** + * @inheritdoc + */ public function doGet($key, $miss) { $value = $this->driver->fetch($key); @@ -49,31 +54,49 @@ class DoctrineCache extends AbstractCache return $value !== false || $this->driver->contains($key) ? $value : $miss; } + /** + * @inheritdoc + */ public function doSet($key, $value, $ttl) { return $this->driver->save($key, $value, (int) $ttl); } + /** + * @inheritdoc + */ public function doDelete($key) { return $this->driver->delete($key); } + /** + * @inheritdoc + */ public function doClear() { return $this->driver->deleteAll(); } + /** + * @inheritdoc + */ public function doGetMultiple($keys, $miss) { return $this->driver->fetchMultiple($keys); } + /** + * @inheritdoc + */ public function doSetMultiple($values, $ttl) { return $this->driver->saveMultiple($values, (int) $ttl); } + /** + * @inheritdoc + */ public function doDeleteMultiple($keys) { // TODO: Remove when Doctrine Cache has been updated to support the feature. @@ -89,6 +112,9 @@ class DoctrineCache extends AbstractCache return $this->driver->deleteMultiple($keys); } + /** + * @inheritdoc + */ public function doHas($key) { return $this->driver->contains($key); diff --git a/system/src/Grav/Framework/Cache/Adapter/FileCache.php b/system/src/Grav/Framework/Cache/Adapter/FileCache.php index eb2e61ff9..50f9f6c66 100644 --- a/system/src/Grav/Framework/Cache/Adapter/FileCache.php +++ b/system/src/Grav/Framework/Cache/Adapter/FileCache.php @@ -15,6 +15,8 @@ use Grav\Framework\Cache\Exception\InvalidArgumentException; /** * Cache class for PSR-16 compatible "Simple Cache" implementation using file backend. * + * Defaults to 1 year TTL. Does not support unlimited TTL. + * * @package Grav\Framework\Cache */ class FileCache extends AbstractCache @@ -22,6 +24,17 @@ class FileCache extends AbstractCache private $directory; private $tmp; + /** + * @inheritdoc + */ + public function __construct($namespace = '', $defaultLifetime = null) + { + parent::__construct($namespace, $defaultLifetime ?: 31557600); // = 1 year + } + + /** + * @inheritdoc + */ public function doGet($key, $miss) { $now = time(); @@ -47,9 +60,13 @@ class FileCache extends AbstractCache return $miss; } + /** + * @inheritdoc + * @throws CacheException + */ public function doSet($key, $value, $ttl) { - $expiresAt = time() + ($ttl ?: 31557600); // = 1 year + $expiresAt = time() + (int)$ttl; $result = $this->write( $this->getFile($key, true), @@ -64,6 +81,9 @@ class FileCache extends AbstractCache return $result; } + /** + * @inheritdoc + */ public function doDelete($key) { $file = $this->getFile($key); @@ -71,6 +91,9 @@ class FileCache extends AbstractCache return (!file_exists($file) || @unlink($file) || !file_exists($file)); } + /** + * @inheritdoc + */ public function doClear() { $result = true; @@ -83,6 +106,9 @@ class FileCache extends AbstractCache return $result; } + /** + * @inheritdoc + */ public function doHas($key) { $file = $this->getFile($key); @@ -90,6 +116,11 @@ class FileCache extends AbstractCache return file_exists($file) && (@filemtime($file) > time() || $this->doGet($key, null)); } + /** + * @param string $key + * @param bool $mkdir + * @return string + */ protected function getFile($key, $mkdir = false) { $hash = str_replace('/', '-', base64_encode(hash('sha256', static::class . $key, true))); @@ -102,6 +133,11 @@ class FileCache extends AbstractCache return $dir . substr($hash, 2, 20); } + /** + * @param string $namespace + * @param string $directory + * @throws InvalidArgumentException + */ private function init($namespace, $directory) { if (!isset($directory[0])) { @@ -129,6 +165,12 @@ class FileCache extends AbstractCache $this->directory = $directory; } + /** + * @param string $file + * @param string $data + * @param int|null $expiresAt + * @return bool + */ private function write($file, $data, $expiresAt = null) { set_error_handler(__CLASS__.'::throwError'); diff --git a/system/src/Grav/Framework/Cache/Adapter/MemoryCache.php b/system/src/Grav/Framework/Cache/Adapter/MemoryCache.php index 43a32d273..84911fbe1 100644 --- a/system/src/Grav/Framework/Cache/Adapter/MemoryCache.php +++ b/system/src/Grav/Framework/Cache/Adapter/MemoryCache.php @@ -12,6 +12,7 @@ use Grav\Framework\Cache\AbstractCache; /** * Cache class for PSR-16 compatible "Simple Cache" implementation using in memory backend. + * * Memory backend does not use namespace or default ttl as the cache is unique to each cache object and request. * * @package Grav\Framework\Cache @@ -23,14 +24,6 @@ class MemoryCache extends AbstractCache */ protected $cache = []; - /** - * Memory Cache constructor. - */ - public function __construct() - { - parent::__construct('', 300); - } - public function doGet($key, $miss) { if (!array_key_exists($key, $this->cache)) { diff --git a/system/src/Grav/Framework/Cache/Adapter/SessionCache.php b/system/src/Grav/Framework/Cache/Adapter/SessionCache.php index eeb5d3052..b1e9e9e51 100644 --- a/system/src/Grav/Framework/Cache/Adapter/SessionCache.php +++ b/system/src/Grav/Framework/Cache/Adapter/SessionCache.php @@ -29,7 +29,13 @@ class SessionCache extends AbstractCache public function doSet($key, $value, $ttl) { - $_SESSION[$this->getNamespace()][$key] = [self::VALUE => $value, self::LIFETIME => time() + $ttl]; + $stored = [self::VALUE => $value]; + if (null !== $ttl) { + $stored[self::LIFETIME] = time() + $ttl; + + } + + $_SESSION[$this->getNamespace()][$key] = $stored; return true; } @@ -43,7 +49,7 @@ class SessionCache extends AbstractCache public function doClear() { - $_SESSION[$this->getNamespace()] = []; + unset($_SESSION[$this->getNamespace()]); return true; } @@ -62,7 +68,7 @@ class SessionCache extends AbstractCache { $stored = isset($_SESSION[$this->getNamespace()][$key]) ? $_SESSION[$this->getNamespace()][$key] : null; - if ($stored && $stored[self::LIFETIME] < time()) { + if (isset($stored[self::LIFETIME]) && $stored[self::LIFETIME] < time()) { unset($_SESSION[$this->getNamespace()][$key]); $stored = null; } diff --git a/system/src/Grav/Framework/Cache/CacheTrait.php b/system/src/Grav/Framework/Cache/CacheTrait.php index 326b8132c..c80bc3ac9 100644 --- a/system/src/Grav/Framework/Cache/CacheTrait.php +++ b/system/src/Grav/Framework/Cache/CacheTrait.php @@ -41,7 +41,7 @@ trait CacheTrait protected function init($namespace = '', $defaultLifetime = null) { $this->namespace = (string) $namespace; - $this->defaultLifetime = $this->convertTtl($defaultLifetime, true); + $this->defaultLifetime = $this->convertTtl($defaultLifetime); $this->miss = new \stdClass; } @@ -220,6 +220,11 @@ trait CacheTrait abstract public function doDelete($key); abstract public function doClear(); + /** + * @param array $keys + * @param mixed $miss + * @return array + */ public function doGetMultiple($keys, $miss) { $results = []; @@ -234,6 +239,11 @@ trait CacheTrait return $results; } + /** + * @param array $values + * @param int $ttl + * @return bool + */ public function doSetMultiple($values, $ttl) { $success = true; @@ -245,6 +255,10 @@ trait CacheTrait return $success; } + /** + * @param array $keys + * @return bool + */ public function doDeleteMultiple($keys) { $success = true; @@ -287,6 +301,10 @@ trait CacheTrait } } + /** + * @param array $keys + * @throws InvalidArgumentException + */ protected function validateKeys($keys) { foreach ($keys as $key) { @@ -296,13 +314,12 @@ trait CacheTrait /** * @param null|int|\DateInterval $ttl - * @param bool $ignoreDefault Used internally inside $this->init(). * @return int|null * @throws InvalidArgumentException */ - protected function convertTtl($ttl, $ignoreDefault = false) + protected function convertTtl($ttl) { - if (!$ignoreDefault && $ttl === null) { + if ($ttl === null) { return $this->getDefaultLifetime(); } From c473c0baafa69fbb95af1fbfa003d4a05bb9b1b2 Mon Sep 17 00:00:00 2001 From: Matias Griese Date: Fri, 19 Jan 2018 11:49:01 +0200 Subject: [PATCH 2/4] Fixes wrong exception in ChainCache --- system/src/Grav/Framework/Cache/Adapter/ChainCache.php | 7 ++++--- system/src/Grav/Framework/Cache/Adapter/FileCache.php | 1 + 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/system/src/Grav/Framework/Cache/Adapter/ChainCache.php b/system/src/Grav/Framework/Cache/Adapter/ChainCache.php index 75b3b64d8..eed108457 100644 --- a/system/src/Grav/Framework/Cache/Adapter/ChainCache.php +++ b/system/src/Grav/Framework/Cache/Adapter/ChainCache.php @@ -10,6 +10,7 @@ namespace Grav\Framework\Cache\Adapter; use Grav\Framework\Cache\AbstractCache; use Grav\Framework\Cache\CacheInterface; +use Grav\Framework\Cache\Exception\InvalidArgumentException; /** * Cache class for PSR-16 compatible "Simple Cache" implementation using chained cache adapters. @@ -32,19 +33,19 @@ class ChainCache extends AbstractCache * Chain Cache constructor. * @param array $caches * @param null|int|\DateInterval $defaultLifetime - * @throws \InvalidArgumentException + * @throws InvalidArgumentException */ public function __construct(array $caches, $defaultLifetime = null) { parent::__construct('', $defaultLifetime); if (!$caches) { - throw new \InvalidArgumentException('At least one cache must be specified'); + throw new InvalidArgumentException('At least one cache must be specified'); } foreach ($caches as $cache) { if (!$cache instanceof CacheInterface) { - throw new \InvalidArgumentException( + throw new InvalidArgumentException( sprintf( "The class '%s' does not implement the '%s' interface", get_class($cache), diff --git a/system/src/Grav/Framework/Cache/Adapter/FileCache.php b/system/src/Grav/Framework/Cache/Adapter/FileCache.php index 50f9f6c66..d703f6190 100644 --- a/system/src/Grav/Framework/Cache/Adapter/FileCache.php +++ b/system/src/Grav/Framework/Cache/Adapter/FileCache.php @@ -194,6 +194,7 @@ class FileCache extends AbstractCache /** * @internal + * @throws \ErrorException */ public static function throwError($type, $message, $file, $line) { From 73bd402551aff64fc528e2e0ea7166b320965107 Mon Sep 17 00:00:00 2001 From: Matias Griese Date: Fri, 19 Jan 2018 12:03:32 +0200 Subject: [PATCH 3/4] `Framework\Cache`: Update all exceptions to be compatible with `Psr\SimpleCache` --- .../Framework/Cache/Adapter/ChainCache.php | 2 +- .../Framework/Cache/Adapter/DoctrineCache.php | 3 ++- .../Framework/Cache/Adapter/FileCache.php | 4 ++-- .../src/Grav/Framework/Cache/CacheTrait.php | 22 +++++++++---------- 4 files changed, 16 insertions(+), 15 deletions(-) diff --git a/system/src/Grav/Framework/Cache/Adapter/ChainCache.php b/system/src/Grav/Framework/Cache/Adapter/ChainCache.php index eed108457..041b676f2 100644 --- a/system/src/Grav/Framework/Cache/Adapter/ChainCache.php +++ b/system/src/Grav/Framework/Cache/Adapter/ChainCache.php @@ -33,7 +33,7 @@ class ChainCache extends AbstractCache * Chain Cache constructor. * @param array $caches * @param null|int|\DateInterval $defaultLifetime - * @throws InvalidArgumentException + * @throws \Psr\SimpleCache\InvalidArgumentException */ public function __construct(array $caches, $defaultLifetime = null) { diff --git a/system/src/Grav/Framework/Cache/Adapter/DoctrineCache.php b/system/src/Grav/Framework/Cache/Adapter/DoctrineCache.php index 25e42ea98..3a69c1ff9 100644 --- a/system/src/Grav/Framework/Cache/Adapter/DoctrineCache.php +++ b/system/src/Grav/Framework/Cache/Adapter/DoctrineCache.php @@ -29,7 +29,7 @@ class DoctrineCache extends AbstractCache * @param CacheProvider $doctrineCache * @param string $namespace * @param null|int|\DateInterval $defaultLifetime - * @throws InvalidArgumentException + * @throws \Psr\SimpleCache\InvalidArgumentException */ public function __construct(CacheProvider $doctrineCache, $namespace = '', $defaultLifetime = null) { @@ -96,6 +96,7 @@ class DoctrineCache extends AbstractCache /** * @inheritdoc + * @throws \Psr\SimpleCache\InvalidArgumentException */ public function doDeleteMultiple($keys) { diff --git a/system/src/Grav/Framework/Cache/Adapter/FileCache.php b/system/src/Grav/Framework/Cache/Adapter/FileCache.php index d703f6190..389b626e1 100644 --- a/system/src/Grav/Framework/Cache/Adapter/FileCache.php +++ b/system/src/Grav/Framework/Cache/Adapter/FileCache.php @@ -62,7 +62,7 @@ class FileCache extends AbstractCache /** * @inheritdoc - * @throws CacheException + * @throws \Psr\SimpleCache\CacheException */ public function doSet($key, $value, $ttl) { @@ -136,7 +136,7 @@ class FileCache extends AbstractCache /** * @param string $namespace * @param string $directory - * @throws InvalidArgumentException + * @throws \Psr\SimpleCache\InvalidArgumentException */ private function init($namespace, $directory) { diff --git a/system/src/Grav/Framework/Cache/CacheTrait.php b/system/src/Grav/Framework/Cache/CacheTrait.php index c80bc3ac9..7fb37c1f9 100644 --- a/system/src/Grav/Framework/Cache/CacheTrait.php +++ b/system/src/Grav/Framework/Cache/CacheTrait.php @@ -36,7 +36,7 @@ trait CacheTrait * * @param string $namespace * @param null|int|\DateInterval $defaultLifetime - * @throws InvalidArgumentException + * @throws \Psr\SimpleCache\InvalidArgumentException */ protected function init($namespace = '', $defaultLifetime = null) { @@ -63,7 +63,7 @@ trait CacheTrait /** * @inheritdoc - * @throws InvalidArgumentException + * @throws \Psr\SimpleCache\InvalidArgumentException */ public function get($key, $default = null) { @@ -76,7 +76,7 @@ trait CacheTrait /** * @inheritdoc - * @throws InvalidArgumentException + * @throws \Psr\SimpleCache\InvalidArgumentException */ public function set($key, $value, $ttl = null) { @@ -90,7 +90,7 @@ trait CacheTrait /** * @inheritdoc - * @throws InvalidArgumentException + * @throws \Psr\SimpleCache\InvalidArgumentException */ public function delete($key) { @@ -109,7 +109,7 @@ trait CacheTrait /** * @inheritdoc - * @throws InvalidArgumentException + * @throws \Psr\SimpleCache\InvalidArgumentException */ public function getMultiple($keys, $default = null) { @@ -149,7 +149,7 @@ trait CacheTrait /** * @inheritdoc - * @throws InvalidArgumentException + * @throws \Psr\SimpleCache\InvalidArgumentException */ public function setMultiple($values, $ttl = null) { @@ -180,7 +180,7 @@ trait CacheTrait /** * @inheritdoc - * @throws InvalidArgumentException + * @throws \Psr\SimpleCache\InvalidArgumentException */ public function deleteMultiple($keys) { @@ -206,7 +206,7 @@ trait CacheTrait /** * @inheritdoc - * @throws InvalidArgumentException + * @throws \Psr\SimpleCache\InvalidArgumentException */ public function has($key) { @@ -274,7 +274,7 @@ trait CacheTrait /** * @param string $key - * @throws InvalidArgumentException + * @throws \Psr\SimpleCache\InvalidArgumentException */ protected function validateKey($key) { @@ -303,7 +303,7 @@ trait CacheTrait /** * @param array $keys - * @throws InvalidArgumentException + * @throws \Psr\SimpleCache\InvalidArgumentException */ protected function validateKeys($keys) { @@ -315,7 +315,7 @@ trait CacheTrait /** * @param null|int|\DateInterval $ttl * @return int|null - * @throws InvalidArgumentException + * @throws \Psr\SimpleCache\InvalidArgumentException */ protected function convertTtl($ttl) { From 8d39fdf23cde2a8154f739edf7b39567e9af0b4c Mon Sep 17 00:00:00 2001 From: Matias Griese Date: Fri, 19 Jan 2018 12:53:33 +0200 Subject: [PATCH 4/4] Bug fix on Cache TTL = null --- system/src/Grav/Common/Markdown/ParsedownExtra.php | 2 ++ system/src/Grav/Framework/Cache/CacheTrait.php | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/system/src/Grav/Common/Markdown/ParsedownExtra.php b/system/src/Grav/Common/Markdown/ParsedownExtra.php index 5d0799e02..481c52f1d 100644 --- a/system/src/Grav/Common/Markdown/ParsedownExtra.php +++ b/system/src/Grav/Common/Markdown/ParsedownExtra.php @@ -17,10 +17,12 @@ class ParsedownExtra extends \ParsedownExtra * * @param $page * @param $defaults + * @throws \Exception */ public function __construct($page, $defaults) { parent::__construct(); + $this->init($page, $defaults); } } diff --git a/system/src/Grav/Framework/Cache/CacheTrait.php b/system/src/Grav/Framework/Cache/CacheTrait.php index 7fb37c1f9..65f4d8d40 100644 --- a/system/src/Grav/Framework/Cache/CacheTrait.php +++ b/system/src/Grav/Framework/Cache/CacheTrait.php @@ -85,7 +85,7 @@ trait CacheTrait $ttl = $this->convertTtl($ttl); // If a negative or zero TTL is provided, the item MUST be deleted from the cache. - return $ttl <= 0 ? $this->doDelete($key) : $this->doSet($key, $value, $ttl); + return null !== $ttl && $ttl <= 0 ? $this->doDelete($key) : $this->doSet($key, $value, $ttl); } /** @@ -175,7 +175,7 @@ trait CacheTrait $ttl = $this->convertTtl($ttl); // If a negative or zero TTL is provided, the item MUST be deleted from the cache. - return $ttl <= 0 ? $this->doDeleteMultiple($keys) : $this->doSetMultiple($values, $ttl); + return null !== $ttl && $ttl <= 0 ? $this->doDeleteMultiple($keys) : $this->doSetMultiple($values, $ttl); } /**