From c6b7bf70a09e87f167c6cbb0f4a1f00e741bf51a Mon Sep 17 00:00:00 2001 From: fulmeek <36341513+fulmeek@users.noreply.github.com> Date: Sat, 9 Mar 2019 08:57:19 +0100 Subject: [PATCH 1/8] Caches: clean up existing CacheInterface For consistency, functions should always return null on non-existing data. --- caches/FileCache.php | 15 +++++++++------ caches/SQLiteCache.php | 8 ++++---- lib/CacheInterface.php | 16 ++++++++-------- 3 files changed, 21 insertions(+), 18 deletions(-) diff --git a/caches/FileCache.php b/caches/FileCache.php index 04d08a253dc..69c20cf52ce 100644 --- a/caches/FileCache.php +++ b/caches/FileCache.php @@ -11,12 +11,14 @@ public function loadData(){ if(file_exists($this->getCacheFile())) { return unserialize(file_get_contents($this->getCacheFile())); } + + return null; } - public function saveData($datas){ + public function saveData($data){ // Notice: We use plain serialize() here to reduce memory footprint on // large input data. - $writeStream = file_put_contents($this->getCacheFile(), serialize($datas)); + $writeStream = file_put_contents($this->getCacheFile(), serialize($data)); if($writeStream === false) { throw new \Exception('Cannot write the cache... Do you have the right permissions ?'); @@ -29,13 +31,14 @@ public function getTime(){ $cacheFile = $this->getCacheFile(); clearstatcache(false, $cacheFile); if(file_exists($cacheFile)) { - return filemtime($cacheFile); + $time = filemtime($cacheFile); + return ($time !== false) ? $time : null; } - return false; + return null; } - public function purgeCache($duration){ + public function purgeCache($seconds){ $cachePath = $this->getPath(); if(file_exists($cachePath)) { $cacheIterator = new RecursiveIteratorIterator( @@ -47,7 +50,7 @@ public function purgeCache($duration){ if(in_array($cacheFile->getBasename(), array('.', '..', '.gitkeep'))) continue; elseif($cacheFile->isFile()) { - if(filemtime($cacheFile->getPathname()) < time() - $duration) + if(filemtime($cacheFile->getPathname()) < time() - $seconds) unlink($cacheFile->getPathname()); } } diff --git a/caches/SQLiteCache.php b/caches/SQLiteCache.php index 5cbb3772670..23bd54f6747 100644 --- a/caches/SQLiteCache.php +++ b/caches/SQLiteCache.php @@ -39,10 +39,10 @@ public function loadData(){ return null; } - public function saveData($datas){ + public function saveData($data){ $Qupdate = $this->db->prepare('INSERT OR REPLACE INTO storage (key, value, updated) VALUES (:key, :value, :updated)'); $Qupdate->bindValue(':key', $this->getCacheKey()); - $Qupdate->bindValue(':value', serialize($datas)); + $Qupdate->bindValue(':value', serialize($data)); $Qupdate->bindValue(':updated', time()); $Qupdate->execute(); @@ -63,9 +63,9 @@ public function getTime(){ return false; } - public function purgeCache($duration){ + public function purgeCache($seconds){ $Qdelete = $this->db->prepare('DELETE FROM storage WHERE updated < :expired'); - $Qdelete->bindValue(':expired', time() - $duration); + $Qdelete->bindValue(':expired', time() - $seconds); $Qdelete->execute(); } diff --git a/lib/CacheInterface.php b/lib/CacheInterface.php index a74fc0dd731..dfa6fd5e516 100644 --- a/lib/CacheInterface.php +++ b/lib/CacheInterface.php @@ -22,29 +22,29 @@ interface CacheInterface { /** * Loads data from cache * - * @return mixed The cache data + * @return mixed The cached data or null */ public function loadData(); /** * Stores data to the cache * - * @param mixed $datas The data to store + * @param mixed $data The data to store * @return self The cache object */ - public function saveData($datas); + public function saveData($data); /** - * Returns the timestamp for the curent cache file + * Returns the timestamp for the curent cache data * - * @return int Timestamp + * @return int Timestamp or null */ public function getTime(); /** - * Removes any data that is older than the specified duration from cache + * Removes any data that is older than the specified age from cache * - * @param int $duration The cache duration in seconds + * @param int $seconds The cache age in seconds */ - public function purgeCache($duration); + public function purgeCache($seconds); } From f3f9f93ec14d882fc83059ba50cb4d3d37db0a7d Mon Sep 17 00:00:00 2001 From: fulmeek <36341513+fulmeek@users.noreply.github.com> Date: Sat, 9 Mar 2019 10:03:54 +0100 Subject: [PATCH 2/8] Caches: set scope WordPressPluginUpdateBridge appears to have used its own cache instance in the past. Obviously not used anymore. --- bridges/ElloBridge.php | 2 +- bridges/WordPressPluginUpdateBridge.php | 12 ----------- caches/FileCache.php | 27 +++++++++++-------------- caches/SQLiteCache.php | 15 +++++++++----- lib/CacheInterface.php | 10 ++++++++- lib/contents.php | 4 ++-- 6 files changed, 34 insertions(+), 36 deletions(-) diff --git a/bridges/ElloBridge.php b/bridges/ElloBridge.php index 45d33a534c9..b109eb9f971 100644 --- a/bridges/ElloBridge.php +++ b/bridges/ElloBridge.php @@ -121,7 +121,7 @@ private function getUsername($post, $postData) { private function getAPIKey() { $cache = Cache::create(Configuration::getConfig('cache', 'type')); - $cache->setPath(PATH_CACHE); + $cache->setScope(get_called_class()); $cache->setParameters(['key']); $key = $cache->loadData(); diff --git a/bridges/WordPressPluginUpdateBridge.php b/bridges/WordPressPluginUpdateBridge.php index 51ddd5b7ea6..9101c4ee8f9 100644 --- a/bridges/WordPressPluginUpdateBridge.php +++ b/bridges/WordPressPluginUpdateBridge.php @@ -71,16 +71,4 @@ public function getName(){ return parent::getName(); } - - private function getCachedDate($url){ - Debug::log('getting pubdate from url ' . $url . ''); - // Initialize cache - $cache = Cache::create(Configuration::getConfig('cache', 'type')); - $cache->setPath(PATH_CACHE . 'pages/'); - $params = [$url]; - $cache->setParameters($params); - // Get cachefile timestamp - $time = $cache->getTime(); - return ($time !== false ? $time : time()); - } } diff --git a/caches/FileCache.php b/caches/FileCache.php index 69c20cf52ce..3d711bf9ad1 100644 --- a/caches/FileCache.php +++ b/caches/FileCache.php @@ -3,7 +3,6 @@ * Cache with file system */ class FileCache implements CacheInterface { - protected $path; protected $param; @@ -58,23 +57,15 @@ public function purgeCache($seconds){ } /** - * Set cache path + * Set cache scope * @return self */ - public function setPath($path){ - if(is_null($path) || !is_string($path)) { - throw new \Exception('The given path is invalid!'); + public function setScope($scope){ + if(is_null($scope) || !is_string($scope)) { + throw new \Exception('The given scope is invalid!'); } - $this->path = $path; - - // Make sure path ends with '/' or '\' - $lastchar = substr($this->path, -1, 1); - if($lastchar !== '/' && $lastchar !== '\\') - $this->path .= '/'; - - if(!is_dir($this->path)) - mkdir($this->path, 0755, true); + $this->path = PATH_CACHE . trim($scope, " \t\n\r\0\x0B\\\/") . '/'; return $this; } @@ -95,7 +86,13 @@ public function setParameters(array $param){ */ protected function getPath(){ if(is_null($this->path)) { - throw new \Exception('Call "setPath" first!'); + throw new \Exception('Call "setScope" first!'); + } + + if(!is_dir($this->path)) { + if (mkdir($this->path, 0755, true) !== true) { + throw new \Exception('Unable to create ' . $this->path); + } } return $this->path; diff --git a/caches/SQLiteCache.php b/caches/SQLiteCache.php index 23bd54f6747..da3afee6b42 100644 --- a/caches/SQLiteCache.php +++ b/caches/SQLiteCache.php @@ -3,14 +3,15 @@ * Cache based on SQLite 3 */ class SQLiteCache implements CacheInterface { - protected $path; + protected $scope; protected $param; private $db = null; public function __construct() { - if (!extension_loaded('sqlite3')) + if (!extension_loaded('sqlite3')) { die('"sqlite3" extension not loaded. Please check "php.ini"'); + } $file = PATH_CACHE . 'cache.sqlite'; @@ -73,8 +74,12 @@ public function purgeCache($seconds){ * Set cache path * @return self */ - public function setPath($path){ - $this->path = $path; + public function setScope($scope){ + if(is_null($scope) || !is_string($scope)) { + throw new \Exception('The given scope is invalid!'); + } + + $this->scope = $scope; return $this; } @@ -94,6 +99,6 @@ protected function getCacheKey(){ throw new \Exception('Call "setParameters" first!'); } - return hash('sha1', $this->path . http_build_query($this->param), true); + return hash('sha1', $this->scope . http_build_query($this->param), true); } } diff --git a/lib/CacheInterface.php b/lib/CacheInterface.php index dfa6fd5e516..59b66987505 100644 --- a/lib/CacheInterface.php +++ b/lib/CacheInterface.php @@ -15,10 +15,18 @@ * The cache interface * * @todo Add missing function to the interface - * @todo Explain parameters and return values in more detail * @todo Return self more often (to allow call chaining) */ interface CacheInterface { + /** + * Set scope of the current cache + * + * If $scope is an empty string, the cache is set to a global context. + * + * @param string $scope The scope the data is related to + */ + public function setScope($scope); + /** * Loads data from cache * diff --git a/lib/contents.php b/lib/contents.php index 976ecbee06d..2f5d5bcee3f 100644 --- a/lib/contents.php +++ b/lib/contents.php @@ -46,7 +46,7 @@ function getContents($url, $header = array(), $opts = array()){ // Initialize cache $cache = Cache::create(Configuration::getConfig('cache', 'type')); - $cache->setPath(PATH_CACHE . 'server/'); + $cache->setScope('server'); $cache->purgeCache(86400); // 24 hours (forced) $params = [$url]; @@ -269,7 +269,7 @@ function getSimpleHTMLDOMCached($url, // Initialize cache $cache = Cache::create(Configuration::getConfig('cache', 'type')); - $cache->setPath(PATH_CACHE . 'pages/'); + $cache->setScope('pages'); $cache->purgeCache(86400); // 24 hours (forced) $params = [$url]; From 40d9ca52d406ae2c693ce0746d2cc9a634c008c1 Mon Sep 17 00:00:00 2001 From: fulmeek <36341513+fulmeek@users.noreply.github.com> Date: Sat, 9 Mar 2019 11:00:28 +0100 Subject: [PATCH 3/8] Caches: set key to assign the current data Since $key can be anything, the cache implementation must ensure to assign the related data reliably; most commonly by serializing and hashing the key in an appropriate way. --- bridges/ElloBridge.php | 2 +- caches/FileCache.php | 26 ++++++++++++++++---------- caches/SQLiteCache.php | 25 +++++++++++++++++-------- lib/CacheInterface.php | 11 +++++++++++ lib/contents.php | 4 ++-- 5 files changed, 47 insertions(+), 21 deletions(-) diff --git a/bridges/ElloBridge.php b/bridges/ElloBridge.php index b109eb9f971..1f66edc375b 100644 --- a/bridges/ElloBridge.php +++ b/bridges/ElloBridge.php @@ -122,7 +122,7 @@ private function getUsername($post, $postData) { private function getAPIKey() { $cache = Cache::create(Configuration::getConfig('cache', 'type')); $cache->setScope(get_called_class()); - $cache->setParameters(['key']); + $cache->setKey(['key']); $key = $cache->loadData(); if($key == null) { diff --git a/caches/FileCache.php b/caches/FileCache.php index 3d711bf9ad1..b9cf0175bb1 100644 --- a/caches/FileCache.php +++ b/caches/FileCache.php @@ -4,7 +4,7 @@ */ class FileCache implements CacheInterface { protected $path; - protected $param; + protected $key; public function loadData(){ if(file_exists($this->getCacheFile())) { @@ -57,7 +57,7 @@ public function purgeCache($seconds){ } /** - * Set cache scope + * Set scope * @return self */ public function setScope($scope){ @@ -71,12 +71,20 @@ public function setScope($scope){ } /** - * Set HTTP GET parameters + * Set key * @return self */ - public function setParameters(array $param){ - $this->param = array_map('strtolower', $param); + public function setKey($key){ + if (!empty($key) && is_array($key)) { + $key = array_map('strtolower', $key); + } + $key = json_encode($key); + + if (!is_string($key)) { + throw new \Exception('The given key is invalid!'); + } + $this->key = $key; return $this; } @@ -111,12 +119,10 @@ protected function getCacheFile(){ * return string */ protected function getCacheName(){ - if(is_null($this->param)) { - throw new \Exception('Call "setParameters" first!'); + if(is_null($this->key)) { + throw new \Exception('Call "setKey" first!'); } - // Change character when making incompatible changes to prevent loading - // errors due to incompatible file contents \|/ - return hash('md5', http_build_query($this->param) . 'A') . '.cache'; + return hash('md5', $this->key) . '.cache'; } } diff --git a/caches/SQLiteCache.php b/caches/SQLiteCache.php index da3afee6b42..5aa072111a1 100644 --- a/caches/SQLiteCache.php +++ b/caches/SQLiteCache.php @@ -4,7 +4,7 @@ */ class SQLiteCache implements CacheInterface { protected $scope; - protected $param; + protected $key; private $db = null; @@ -71,7 +71,7 @@ public function purgeCache($seconds){ } /** - * Set cache path + * Set scope * @return self */ public function setScope($scope){ @@ -84,21 +84,30 @@ public function setScope($scope){ } /** - * Set HTTP GET parameters + * Set key * @return self */ - public function setParameters(array $param){ - $this->param = array_map('strtolower', $param); + public function setKey($key){ + if (!empty($key) && is_array($key)) { + $key = array_map('strtolower', $key); + } + $key = json_encode($key); + + if (!is_string($key)) { + throw new \Exception('The given key is invalid!'); + } + + $this->key = $key; return $this; } //////////////////////////////////////////////////////////////////////////// protected function getCacheKey(){ - if(is_null($this->param)) { - throw new \Exception('Call "setParameters" first!'); + if(is_null($this->key)) { + throw new \Exception('Call "setKey" first!'); } - return hash('sha1', $this->scope . http_build_query($this->param), true); + return hash('sha1', $this->scope . $this->key, true); } } diff --git a/lib/CacheInterface.php b/lib/CacheInterface.php index 59b66987505..3dc55b1da4d 100644 --- a/lib/CacheInterface.php +++ b/lib/CacheInterface.php @@ -27,6 +27,17 @@ interface CacheInterface { */ public function setScope($scope); + /** + * Set key to assign the current data + * + * Since $key can be anything, the cache implementation must ensure to + * assign the related data reliably; most commonly by serializing and + * hashing the key in an appropriate way. + * + * @param array $key The key the data is related to + */ + public function setKey($key); + /** * Loads data from cache * diff --git a/lib/contents.php b/lib/contents.php index 2f5d5bcee3f..e84086a9e35 100644 --- a/lib/contents.php +++ b/lib/contents.php @@ -50,7 +50,7 @@ function getContents($url, $header = array(), $opts = array()){ $cache->purgeCache(86400); // 24 hours (forced) $params = [$url]; - $cache->setParameters($params); + $cache->setKey($params); // Use file_get_contents if in CLI mode with no root certificates defined if(php_sapi_name() === 'cli' && empty(ini_get('curl.cainfo'))) { @@ -273,7 +273,7 @@ function getSimpleHTMLDOMCached($url, $cache->purgeCache(86400); // 24 hours (forced) $params = [$url]; - $cache->setParameters($params); + $cache->setKey($params); // Determine if cached file is within duration $time = $cache->getTime(); From 305e09156e35b0de63bb97d9a001232c29dabc82 Mon Sep 17 00:00:00 2001 From: fulmeek <36341513+fulmeek@users.noreply.github.com> Date: Sat, 9 Mar 2019 11:41:54 +0100 Subject: [PATCH 4/8] [SQLiteCache] add configuration Even though the default path for storage is perfectly fine, some people may want to use a different location. This is an example how a cache implementation is responsible for its requirements. --- caches/SQLiteCache.php | 10 +++++++++- config.default.ini.php | 5 +++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/caches/SQLiteCache.php b/caches/SQLiteCache.php index 5aa072111a1..94988826452 100644 --- a/caches/SQLiteCache.php +++ b/caches/SQLiteCache.php @@ -13,7 +13,15 @@ public function __construct() { die('"sqlite3" extension not loaded. Please check "php.ini"'); } - $file = PATH_CACHE . 'cache.sqlite'; + $file = Configuration::getConfig(get_called_class(), 'file'); + if (empty($file)) { + die('Configuration for ' . get_called_class() . ' missing. Please check your config.ini.php'); + } + if (dirname($file) == '.') { + $file = PATH_CACHE . $file; + } elseif (!is_dir(dirname($file))) { + die('Invalid configuration for ' . get_called_class() . '. Please check your config.ini.php'); + } if (!is_file($file)) { $this->db = new SQLite3($file); diff --git a/config.default.ini.php b/config.default.ini.php index 394658d607a..90498230101 100644 --- a/config.default.ini.php +++ b/config.default.ini.php @@ -52,3 +52,8 @@ ; The password for authentication. Insert this password when prompted for login. ; Use a strong password to prevent others from guessing your login! password = "" + +; --- Cache specific configuration --------------------------------------------- + +[SQLiteCache] +file = "cache.sqlite" From 6667a571ac148cfe4a74d657c4a3f1c9323f8bb3 Mon Sep 17 00:00:00 2001 From: fulmeek <36341513+fulmeek@users.noreply.github.com> Date: Sat, 9 Mar 2019 12:35:52 +0100 Subject: [PATCH 5/8] Caches: minor clean-up --- caches/SQLiteCache.php | 2 +- lib/CacheInterface.php | 3 --- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/caches/SQLiteCache.php b/caches/SQLiteCache.php index 94988826452..9f49eeea86f 100644 --- a/caches/SQLiteCache.php +++ b/caches/SQLiteCache.php @@ -69,7 +69,7 @@ public function getTime(){ } } - return false; + return null; } public function purgeCache($seconds){ diff --git a/lib/CacheInterface.php b/lib/CacheInterface.php index 3dc55b1da4d..091c5f029ac 100644 --- a/lib/CacheInterface.php +++ b/lib/CacheInterface.php @@ -13,9 +13,6 @@ /** * The cache interface - * - * @todo Add missing function to the interface - * @todo Return self more often (to allow call chaining) */ interface CacheInterface { /** From 095ed29e0bab1f1412eb3b3678edf8153245fa8c Mon Sep 17 00:00:00 2001 From: LogMANOriginal Date: Mon, 25 Mar 2019 21:35:12 +0100 Subject: [PATCH 6/8] Apply suggestions from code review Co-Authored-By: fulmeek <36341513+fulmeek@users.noreply.github.com> --- caches/FileCache.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/caches/FileCache.php b/caches/FileCache.php index b9cf0175bb1..79443ea151a 100644 --- a/caches/FileCache.php +++ b/caches/FileCache.php @@ -98,7 +98,7 @@ protected function getPath(){ } if(!is_dir($this->path)) { - if (mkdir($this->path, 0755, true) !== true) { + if (mkdir($this->path, 0644, true) !== true) { throw new \Exception('Unable to create ' . $this->path); } } From 8afc578dd37168bd9bc6111579d742fcb1df68bd Mon Sep 17 00:00:00 2001 From: fulmeek <36341513+fulmeek@users.noreply.github.com> Date: Mon, 25 Mar 2019 22:27:18 +0100 Subject: [PATCH 7/8] [DisplayAction] use new cache functions Done earlier, but forgotten to actually add these changes to the previous commits. --- actions/DisplayAction.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/actions/DisplayAction.php b/actions/DisplayAction.php index b223b757553..a1b106f5917 100644 --- a/actions/DisplayAction.php +++ b/actions/DisplayAction.php @@ -86,9 +86,9 @@ public function execute() { // Initialize cache $cache = Cache::create(Configuration::getConfig('cache', 'type')); - $cache->setPath(PATH_CACHE); + $cache->setScope(''); $cache->purgeCache(86400); // 24 hours - $cache->setParameters($cache_params); + $cache->setKey($cache_params); $items = array(); $infos = array(); From ade4b2694687a6ab617ce34c732dda5af171719d Mon Sep 17 00:00:00 2001 From: fulmeek <36341513+fulmeek@users.noreply.github.com> Date: Mon, 25 Mar 2019 22:31:17 +0100 Subject: [PATCH 8/8] Caches: make protected methods private + revert directory permissions The previous code suggestions to set directory permissions to 644 had to reverted to 755 as the executable bit is required to change into that folder. --- caches/FileCache.php | 8 ++++---- caches/SQLiteCache.php | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/caches/FileCache.php b/caches/FileCache.php index 79443ea151a..166ecdb5ba1 100644 --- a/caches/FileCache.php +++ b/caches/FileCache.php @@ -92,13 +92,13 @@ public function setKey($key){ * Return cache path (and create if not exist) * @return string Cache path */ - protected function getPath(){ + private function getPath(){ if(is_null($this->path)) { throw new \Exception('Call "setScope" first!'); } if(!is_dir($this->path)) { - if (mkdir($this->path, 0644, true) !== true) { + if (mkdir($this->path, 0755, true) !== true) { throw new \Exception('Unable to create ' . $this->path); } } @@ -110,7 +110,7 @@ protected function getPath(){ * Get the file name use for cache store * @return string Path to the file cache */ - protected function getCacheFile(){ + private function getCacheFile(){ return $this->getPath() . $this->getCacheName(); } @@ -118,7 +118,7 @@ protected function getCacheFile(){ * Determines file name for store the cache * return string */ - protected function getCacheName(){ + private function getCacheName(){ if(is_null($this->key)) { throw new \Exception('Call "setKey" first!'); } diff --git a/caches/SQLiteCache.php b/caches/SQLiteCache.php index 9f49eeea86f..7d0f584f375 100644 --- a/caches/SQLiteCache.php +++ b/caches/SQLiteCache.php @@ -111,7 +111,7 @@ public function setKey($key){ //////////////////////////////////////////////////////////////////////////// - protected function getCacheKey(){ + private function getCacheKey(){ if(is_null($this->key)) { throw new \Exception('Call "setKey" first!'); }