From 705b9daa0bcfab2e6c342547eb7bf6324bd6c10f Mon Sep 17 00:00:00 2001 From: logmanoriginal Date: Tue, 18 Jun 2019 18:55:29 +0200 Subject: [PATCH] bridge: Refactor bridge factory to non-static class The bridge 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 bridge factory. References #1001 --- actions/DetectAction.php | 9 +- actions/DisplayAction.php | 7 +- actions/ListAction.php | 9 +- lib/BridgeCard.php | 5 +- lib/{Bridge.php => BridgeFactory.php} | 113 ++++++-------------------- lib/BridgeList.php | 7 +- lib/rssbridge.php | 3 +- 7 files changed, 52 insertions(+), 101 deletions(-) rename lib/{Bridge.php => BridgeFactory.php} (66%) diff --git a/actions/DetectAction.php b/actions/DetectAction.php index 2ad79a27378..86605de41d8 100644 --- a/actions/DetectAction.php +++ b/actions/DetectAction.php @@ -19,13 +19,16 @@ public function execute() { $format = $this->userData['format'] or returnClientError('You must specify a format!'); - foreach(Bridge::getBridgeNames() as $bridgeName) { + $bridgeFac = new \BridgeFactory(); + $bridgeFac->setWorkingDir(PATH_LIB_BRIDGES); - if(!Bridge::isWhitelisted($bridgeName)) { + foreach($bridgeFac->getBridgeNames() as $bridgeName) { + + if(!$bridgeFac->isWhitelisted($bridgeName)) { continue; } - $bridge = Bridge::create($bridgeName); + $bridge = $bridgeFac->create($bridgeName); if($bridge === false) { continue; diff --git a/actions/DisplayAction.php b/actions/DisplayAction.php index 7a59ad7f1d0..1dec5cbffa0 100644 --- a/actions/DisplayAction.php +++ b/actions/DisplayAction.php @@ -18,14 +18,17 @@ public function execute() { $format = $this->userData['format'] or returnClientError('You must specify a format!'); + $bridgeFac = new \BridgeFactory(); + $bridgeFac->setWorkingDir(PATH_LIB_BRIDGES); + // whitelist control - if(!Bridge::isWhitelisted($bridge)) { + if(!$bridgeFac->isWhitelisted($bridge)) { throw new \Exception('This bridge is not whitelisted', 401); die; } // Data retrieval - $bridge = Bridge::create($bridge); + $bridge = $bridgeFac->create($bridge); $noproxy = array_key_exists('_noproxy', $this->userData) && filter_var($this->userData['_noproxy'], FILTER_VALIDATE_BOOLEAN); diff --git a/actions/ListAction.php b/actions/ListAction.php index 03e06119622..92aef0e0f5e 100644 --- a/actions/ListAction.php +++ b/actions/ListAction.php @@ -17,9 +17,12 @@ public function execute() { $list->bridges = array(); $list->total = 0; - foreach(Bridge::getBridgeNames() as $bridgeName) { + $bridgeFac = new \BridgeFactory(); + $bridgeFac->setWorkingDir(PATH_LIB_BRIDGES); - $bridge = Bridge::create($bridgeName); + foreach($bridgeFac->getBridgeNames() as $bridgeName) { + + $bridge = $bridgeFac->create($bridgeName); if($bridge === false) { // Broken bridge, show as inactive @@ -31,7 +34,7 @@ public function execute() { } - $status = Bridge::isWhitelisted($bridgeName) ? 'active' : 'inactive'; + $status = $bridgeFac->isWhitelisted($bridgeName) ? 'active' : 'inactive'; $list->bridges[$bridgeName] = array( 'status' => $status, diff --git a/lib/BridgeCard.php b/lib/BridgeCard.php index 697433ffb65..8c36919c5e4 100644 --- a/lib/BridgeCard.php +++ b/lib/BridgeCard.php @@ -299,7 +299,10 @@ private static function getCheckboxInput($entry, $id, $name) { */ static function displayBridgeCard($bridgeName, $formats, $isActive = true){ - $bridge = Bridge::create($bridgeName); + $bridgeFac = new \BridgeFactory(); + $bridgeFac->setWorkingDir(PATH_LIB_BRIDGES); + + $bridge = $bridgeFac->create($bridgeName); if($bridge == false) return ''; diff --git a/lib/Bridge.php b/lib/BridgeFactory.php similarity index 66% rename from lib/Bridge.php rename to lib/BridgeFactory.php index 31607922341..fea254f1cb3 100644 --- a/lib/Bridge.php +++ b/lib/BridgeFactory.php @@ -35,17 +35,7 @@ * $bridge = Bridge::create('GitHubIssue'); * ``` */ -class Bridge { - - /** - * Holds a path to the working directory. - * - * Do not access this property directly! - * Use {@see Bridge::setWorkingDir()} and {@see Bridge::getWorkingDir()} instead. - * - * @var string|null - */ - protected static $workingDir = null; +class BridgeFactory extends FactoryAbstract { /** * Holds a list of whitelisted bridges. @@ -55,18 +45,7 @@ class Bridge { * * @var array */ - protected static $whitelist = array(); - - /** - * Throws an exception when trying to create a new instance of this class. - * Use {@see Bridge::create()} to instanciate a new bridge from the working - * directory. - * - * @throws \LogicException if called. - */ - public function __construct(){ - throw new \LogicException('Use ' . __CLASS__ . '::create($name) to create bridge objects!'); - } + protected $whitelist = array(); /** * Creates a new bridge object from the working directory. @@ -77,13 +56,13 @@ public function __construct(){ * @param string $name Name of the bridge object. * @return object|bool The bridge object or false if the class is not instantiable. */ - public static function create($name){ - if(!self::isBridgeName($name)) { + public function create($name){ + if(!$this->isBridgeName($name)) { throw new \InvalidArgumentException('Bridge name invalid!'); } - $name = self::sanitizeBridgeName($name) . 'Bridge'; - $filePath = self::getWorkingDir() . $name . '.php'; + $name = $this->sanitizeBridgeName($name) . 'Bridge'; + $filePath = $this->getWorkingDir() . $name . '.php'; if(!file_exists($filePath)) { throw new \Exception('Bridge file ' . $filePath . ' does not exist!'); @@ -98,48 +77,6 @@ public static function create($name){ return false; } - /** - * Sets the working directory. - * - * @param string $dir Path to the directory containing bridges. - * @throws \LogicException if the provided path is not a valid string. - * @throws \Exception if the provided path does not 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 specified with {@see Bridge::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 bridge name. * @@ -149,7 +86,7 @@ public static function getWorkingDir(){ * @param string $name The bridge name. * @return bool true if the name is a valid bridge name, false otherwise. */ - public static function isBridgeName($name){ + public function isBridgeName($name){ return is_string($name) && preg_match('/^[A-Z][a-zA-Z0-9-]*$/', $name) === 1; } @@ -160,12 +97,12 @@ public static function isBridgeName($name){ * * @return array List of bridge names */ - public static function getBridgeNames(){ + public function getBridgeNames(){ static $bridgeNames = array(); // Initialized on first call if(empty($bridgeNames)) { - $files = scandir(self::getWorkingDir()); + $files = scandir($this->getWorkingDir()); if($files !== false) { foreach($files as $file) { @@ -185,8 +122,8 @@ public static function getBridgeNames(){ * @param string $name Name of the bridge. * @return bool True if the bridge is whitelisted. */ - public static function isWhitelisted($name){ - return in_array(self::sanitizeBridgeName($name), self::getWhitelist()); + public function isWhitelisted($name){ + return in_array($this->sanitizeBridgeName($name), $this->getWhitelist()); } /** @@ -205,7 +142,7 @@ public static function isWhitelisted($name){ * * @return array Array of whitelisted bridges */ - public static function getWhitelist() { + public function getWhitelist() { static $firstCall = true; // Initialized on first call @@ -220,17 +157,17 @@ public static function getWhitelist() { } if($contents === '*') { // Whitelist all bridges - self::$whitelist = self::getBridgeNames(); + $this->whitelist = $this->getBridgeNames(); } else { - //self::$whitelist = array_map('self::sanitizeBridgeName', explode("\n", $contents)); + //$this->$whitelist = array_map('$this->sanitizeBridgeName', explode("\n", $contents)); foreach(explode("\n", $contents) as $bridgeName) { - self::$whitelist[] = self::sanitizeBridgeName($bridgeName); + $this->whitelist[] = $this->sanitizeBridgeName($bridgeName); } } } - return self::$whitelist; + return $this->whitelist; } @@ -248,8 +185,8 @@ public static function getWhitelist() { * @param array $default The whitelist as array of bridge names. * @return void */ - public static function setWhitelist($default = array()) { - self::$whitelist = array_map('self::sanitizeBridgeName', $default); + public function setWhitelist($default = array()) { + $this->whitelist = array_map('$this->sanitizeBridgeName', $default); } /** @@ -269,7 +206,7 @@ public static function setWhitelist($default = array()) { * @return string|null The sanitized bridge name if the provided name is * valid, null otherwise. */ - protected static function sanitizeBridgeName($name) { + protected function sanitizeBridgeName($name) { if(is_string($name)) { @@ -284,15 +221,15 @@ protected static function sanitizeBridgeName($name) { } // Improve performance for correctly written bridge names - if(in_array($name, self::getBridgeNames())) { - $index = array_search($name, self::getBridgeNames()); - return self::getBridgeNames()[$index]; + if(in_array($name, $this->getBridgeNames())) { + $index = array_search($name, $this->getBridgeNames()); + return $this->getBridgeNames()[$index]; } // The name is valid if a corresponding bridge file is found on disk - if(in_array(strtolower($name), array_map('strtolower', self::getBridgeNames()))) { - $index = array_search(strtolower($name), array_map('strtolower', self::getBridgeNames())); - return self::getBridgeNames()[$index]; + if(in_array(strtolower($name), array_map('strtolower', $this->getBridgeNames()))) { + $index = array_search(strtolower($name), array_map('strtolower', $this->getBridgeNames())); + return $this->getBridgeNames()[$index]; } Debug::log('Invalid bridge name specified: "' . $name . '"!'); diff --git a/lib/BridgeList.php b/lib/BridgeList.php index 8a334a3f62d..0c3c4ffa3f5 100644 --- a/lib/BridgeList.php +++ b/lib/BridgeList.php @@ -61,14 +61,17 @@ private static function getBridges($showInactive, &$totalBridges, &$totalActiveB $totalActiveBridges = 0; $inactiveBridges = ''; - $bridgeList = Bridge::getBridgeNames(); + $bridgeFac = new \BridgeFactory(); + $bridgeFac->setWorkingDir(PATH_LIB_BRIDGES); + + $bridgeList = $bridgeFac->getBridgeNames(); $formats = Format::getFormatNames(); $totalBridges = count($bridgeList); foreach($bridgeList as $bridgeName) { - if(Bridge::isWhitelisted($bridgeName)) { + if($bridgeFac->isWhitelisted($bridgeName)) { $body .= BridgeCard::displayBridgeCard($bridgeName, $formats); $totalActiveBridges++; diff --git a/lib/rssbridge.php b/lib/rssbridge.php index 3b0e65d4658..a10c117da83 100644 --- a/lib/rssbridge.php +++ b/lib/rssbridge.php @@ -63,7 +63,7 @@ require_once PATH_LIB . 'Exceptions.php'; require_once PATH_LIB . 'Format.php'; require_once PATH_LIB . 'FormatAbstract.php'; -require_once PATH_LIB . 'Bridge.php'; +require_once PATH_LIB . 'BridgeFactory.php'; require_once PATH_LIB . 'BridgeAbstract.php'; require_once PATH_LIB . 'FeedExpander.php'; require_once PATH_LIB . 'Cache.php'; @@ -87,7 +87,6 @@ // Initialize static members try { - Bridge::setWorkingDir(PATH_LIB_BRIDGES); Format::setWorkingDir(PATH_LIB_FORMATS); Cache::setWorkingDir(PATH_LIB_CACHES); } catch(Exception $e) {