Skip to content

Commit

Permalink
cache: Refactor cache factory to non-static class
Browse files Browse the repository at this point in the history
The cache factory can be based on the abstract factory class if it
wasn't static. This allows for higher abstraction and makes future
extensions possible. Also, not all parts of RSS-Bridge need to work
on the same instance of the factory.

References #1001
  • Loading branch information
logmanoriginal committed Jun 18, 2019
1 parent 705b9da commit 2460b67
Show file tree
Hide file tree
Showing 5 changed files with 25 additions and 82 deletions.
4 changes: 3 additions & 1 deletion actions/DisplayAction.php
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,9 @@ public function execute() {
);

// Initialize cache
$cache = Cache::create(Configuration::getConfig('cache', 'type'));
$cacheFac = new CacheFactory();
$cacheFac->setWorkingDir(PATH_LIB_CACHES);
$cache = $cacheFac->create(Configuration::getConfig('cache', 'type'));
$cache->setScope('');
$cache->purgeCache(86400); // 24 hours
$cache->setKey($cache_params);
Expand Down
4 changes: 3 additions & 1 deletion bridges/ElloBridge.php
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,9 @@ private function getUsername($post, $postData) {
}

private function getAPIKey() {
$cache = Cache::create(Configuration::getConfig('cache', 'type'));
$cacheFac = new CacheFactory();
$cacheFac->setWorkingDir(PATH_LIB_CACHES);
$cache = $cacheFac->create(Configuration::getConfig('cache', 'type'));
$cache->setScope(get_called_class());
$cache->setKey(['key']);
$key = $cache->loadData();
Expand Down
88 changes: 12 additions & 76 deletions lib/Cache.php → lib/CacheFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,29 +31,7 @@
* $cache = Cache::create('FileCache');
* ```
*/
class Cache {

/**
* Holds a path to the working directory.
*
* Do not access this property directly!
* Use {@see Cache::setWorkingDir()} and {@see Cache::getWorkingDir()} instead.
*
* @var string|null
*/
protected static $workingDir = null;

/**
* Throws an exception when trying to create a new instance of this class.
* Use {@see Cache::create()} to create a new cache object from the working
* directory.
*
* @throws \LogicException if called.
*/
public function __construct(){
throw new \LogicException('Use ' . __CLASS__ . '::create($name) to create cache objects!');
}

class CacheFactory extends FactoryAbstract {
/**
* Creates a new cache object from the working directory.
*
Expand All @@ -63,14 +41,14 @@ public function __construct(){
* @param string $name Name of the cache object.
* @return object|bool The cache object or false if the class is not instantiable.
*/
public static function create($name){
$name = self::sanitizeCacheName($name) . 'Cache';
public function create($name){
$name = $this->sanitizeCacheName($name) . 'Cache';

if(!self::isCacheName($name)) {
if(!$this->isCacheName($name)) {
throw new \InvalidArgumentException('Cache name invalid!');
}

$filePath = self::getWorkingDir() . $name . '.php';
$filePath = $this->getWorkingDir() . $name . '.php';

if(!file_exists($filePath)) {
throw new \Exception('Cache file ' . $filePath . ' does not exist!');
Expand All @@ -85,48 +63,6 @@ public static function create($name){
return false;
}

/**
* Sets the working directory.
*
* @param string $dir Path to a directory containing cache classes
* @throws \InvalidArgumentException if $dir is not a string.
* @throws \Exception if the working directory doesn't exist.
* @throws \InvalidArgumentException if $dir is not a directory.
* @return void
*/
public static function setWorkingDir($dir){
self::$workingDir = null;

if(!is_string($dir)) {
throw new \InvalidArgumentException('Working directory is not a valid string!');
}

if(!file_exists($dir)) {
throw new \Exception('Working directory does not exist!');
}

if(!is_dir($dir)) {
throw new \InvalidArgumentException('Working directory is not a directory!');
}

self::$workingDir = realpath($dir) . '/';
}

/**
* Returns the working directory.
* The working directory must be set with {@see Cache::setWorkingDir()}!
*
* @throws \LogicException if the working directory is not set.
* @return string The current working directory.
*/
public static function getWorkingDir(){
if(is_null(self::$workingDir)) {
throw new \LogicException('Working directory is not set!');
}

return self::$workingDir;
}

/**
* Returns true if the provided name is a valid cache name.
*
Expand All @@ -136,7 +72,7 @@ public static function getWorkingDir(){
* @param string $name The cache name.
* @return bool true if the name is a valid cache name, false otherwise.
*/
public static function isCacheName($name){
public function isCacheName($name){
return is_string($name) && preg_match('/^[A-Z][a-zA-Z0-9-]*$/', $name) === 1;
}

Expand All @@ -147,12 +83,12 @@ public static function isCacheName($name){
*
* @return array List of cache names
*/
public static function getCacheNames(){
public function getCacheNames(){

static $cacheNames = array(); // Initialized on first call

if(empty($cacheNames)) {
$files = scandir(self::getWorkingDir());
$files = scandir($this->getWorkingDir());

if($files !== false) {
foreach($files as $file) {
Expand Down Expand Up @@ -183,7 +119,7 @@ public static function getCacheNames(){
* @return string|null The sanitized cache name if the provided name is
* valid, null otherwise.
*/
protected static function sanitizeCacheName($name) {
protected function sanitizeCacheName($name) {

if(is_string($name)) {

Expand All @@ -198,9 +134,9 @@ protected static function sanitizeCacheName($name) {
}

// The name is valid if a corresponding file is found on disk
if(in_array(strtolower($name), array_map('strtolower', self::getCacheNames()))) {
$index = array_search(strtolower($name), array_map('strtolower', self::getCacheNames()));
return self::getCacheNames()[$index];
if(in_array(strtolower($name), array_map('strtolower', $this->getCacheNames()))) {
$index = array_search(strtolower($name), array_map('strtolower', $this->getCacheNames()));
return $this->getCacheNames()[$index];
}

Debug::log('Invalid cache name specified: "' . $name . '"!');
Expand Down
8 changes: 6 additions & 2 deletions lib/contents.php
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,9 @@ function getContents($url, $header = array(), $opts = array()){
Debug::log('Reading contents from "' . $url . '"');

// Initialize cache
$cache = Cache::create(Configuration::getConfig('cache', 'type'));
$cacheFac = new CacheFactory();
$cacheFac->setWorkingDir(PATH_LIB_CACHES);
$cache = $cacheFac->create(Configuration::getConfig('cache', 'type'));
$cache->setScope('server');
$cache->purgeCache(86400); // 24 hours (forced)

Expand Down Expand Up @@ -270,7 +272,9 @@ function getSimpleHTMLDOMCached($url,
Debug::log('Caching url ' . $url . ', duration ' . $duration);

// Initialize cache
$cache = Cache::create(Configuration::getConfig('cache', 'type'));
$cacheFac = new CacheFactory();
$cacheFac->setWorkingDir(PATH_LIB_CACHES);
$cache = $cacheFac->create(Configuration::getConfig('cache', 'type'));
$cache->setScope('pages');
$cache->purgeCache(86400); // 24 hours (forced)

Expand Down
3 changes: 1 addition & 2 deletions lib/rssbridge.php
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@
require_once PATH_LIB . 'BridgeFactory.php';
require_once PATH_LIB . 'BridgeAbstract.php';
require_once PATH_LIB . 'FeedExpander.php';
require_once PATH_LIB . 'Cache.php';
require_once PATH_LIB . 'CacheFactory.php';
require_once PATH_LIB . 'Authentication.php';
require_once PATH_LIB . 'Configuration.php';
require_once PATH_LIB . 'BridgeCard.php';
Expand All @@ -88,7 +88,6 @@
// Initialize static members
try {
Format::setWorkingDir(PATH_LIB_FORMATS);
Cache::setWorkingDir(PATH_LIB_CACHES);
} catch(Exception $e) {
error_log($e);
header('Content-type: text/plain', true, 500);
Expand Down

0 comments on commit 2460b67

Please sign in to comment.