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
This commit is contained in:
logmanoriginal 2019-06-18 18:55:29 +02:00
parent 1ada9c26f8
commit 705b9daa0b
7 changed files with 52 additions and 101 deletions

View file

@ -19,13 +19,16 @@ class DetectAction extends ActionAbstract {
$format = $this->userData['format'] $format = $this->userData['format']
or returnClientError('You must specify a 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; continue;
} }
$bridge = Bridge::create($bridgeName); $bridge = $bridgeFac->create($bridgeName);
if($bridge === false) { if($bridge === false) {
continue; continue;

View file

@ -18,14 +18,17 @@ class DisplayAction extends ActionAbstract {
$format = $this->userData['format'] $format = $this->userData['format']
or returnClientError('You must specify a format!'); or returnClientError('You must specify a format!');
$bridgeFac = new \BridgeFactory();
$bridgeFac->setWorkingDir(PATH_LIB_BRIDGES);
// whitelist control // whitelist control
if(!Bridge::isWhitelisted($bridge)) { if(!$bridgeFac->isWhitelisted($bridge)) {
throw new \Exception('This bridge is not whitelisted', 401); throw new \Exception('This bridge is not whitelisted', 401);
die; die;
} }
// Data retrieval // Data retrieval
$bridge = Bridge::create($bridge); $bridge = $bridgeFac->create($bridge);
$noproxy = array_key_exists('_noproxy', $this->userData) $noproxy = array_key_exists('_noproxy', $this->userData)
&& filter_var($this->userData['_noproxy'], FILTER_VALIDATE_BOOLEAN); && filter_var($this->userData['_noproxy'], FILTER_VALIDATE_BOOLEAN);

View file

@ -17,9 +17,12 @@ class ListAction extends ActionAbstract {
$list->bridges = array(); $list->bridges = array();
$list->total = 0; $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 if($bridge === false) { // Broken bridge, show as inactive
@ -31,7 +34,7 @@ class ListAction extends ActionAbstract {
} }
$status = Bridge::isWhitelisted($bridgeName) ? 'active' : 'inactive'; $status = $bridgeFac->isWhitelisted($bridgeName) ? 'active' : 'inactive';
$list->bridges[$bridgeName] = array( $list->bridges[$bridgeName] = array(
'status' => $status, 'status' => $status,

View file

@ -299,7 +299,10 @@ This bridge is not fetching its content through a secure connection</div>';
*/ */
static function displayBridgeCard($bridgeName, $formats, $isActive = true){ 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) if($bridge == false)
return ''; return '';

View file

@ -35,17 +35,7 @@
* $bridge = Bridge::create('GitHubIssue'); * $bridge = Bridge::create('GitHubIssue');
* ``` * ```
*/ */
class Bridge { class BridgeFactory extends FactoryAbstract {
/**
* 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;
/** /**
* Holds a list of whitelisted bridges. * Holds a list of whitelisted bridges.
@ -55,18 +45,7 @@ class Bridge {
* *
* @var array * @var array
*/ */
protected static $whitelist = array(); protected $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!');
}
/** /**
* Creates a new bridge object from the working directory. * Creates a new bridge object from the working directory.
@ -77,13 +56,13 @@ class Bridge {
* @param string $name Name of the bridge object. * @param string $name Name of the bridge object.
* @return object|bool The bridge object or false if the class is not instantiable. * @return object|bool The bridge object or false if the class is not instantiable.
*/ */
public static function create($name){ public function create($name){
if(!self::isBridgeName($name)) { if(!$this->isBridgeName($name)) {
throw new \InvalidArgumentException('Bridge name invalid!'); throw new \InvalidArgumentException('Bridge name invalid!');
} }
$name = self::sanitizeBridgeName($name) . 'Bridge'; $name = $this->sanitizeBridgeName($name) . 'Bridge';
$filePath = self::getWorkingDir() . $name . '.php'; $filePath = $this->getWorkingDir() . $name . '.php';
if(!file_exists($filePath)) { if(!file_exists($filePath)) {
throw new \Exception('Bridge file ' . $filePath . ' does not exist!'); throw new \Exception('Bridge file ' . $filePath . ' does not exist!');
@ -98,48 +77,6 @@ class Bridge {
return false; 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. * Returns true if the provided name is a valid bridge name.
* *
@ -149,7 +86,7 @@ class Bridge {
* @param string $name The bridge name. * @param string $name The bridge name.
* @return bool true if the name is a valid bridge name, false otherwise. * @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; return is_string($name) && preg_match('/^[A-Z][a-zA-Z0-9-]*$/', $name) === 1;
} }
@ -160,12 +97,12 @@ class Bridge {
* *
* @return array List of bridge names * @return array List of bridge names
*/ */
public static function getBridgeNames(){ public function getBridgeNames(){
static $bridgeNames = array(); // Initialized on first call static $bridgeNames = array(); // Initialized on first call
if(empty($bridgeNames)) { if(empty($bridgeNames)) {
$files = scandir(self::getWorkingDir()); $files = scandir($this->getWorkingDir());
if($files !== false) { if($files !== false) {
foreach($files as $file) { foreach($files as $file) {
@ -185,8 +122,8 @@ class Bridge {
* @param string $name Name of the bridge. * @param string $name Name of the bridge.
* @return bool True if the bridge is whitelisted. * @return bool True if the bridge is whitelisted.
*/ */
public static function isWhitelisted($name){ public function isWhitelisted($name){
return in_array(self::sanitizeBridgeName($name), self::getWhitelist()); return in_array($this->sanitizeBridgeName($name), $this->getWhitelist());
} }
/** /**
@ -205,7 +142,7 @@ class Bridge {
* *
* @return array Array of whitelisted bridges * @return array Array of whitelisted bridges
*/ */
public static function getWhitelist() { public function getWhitelist() {
static $firstCall = true; // Initialized on first call static $firstCall = true; // Initialized on first call
@ -220,17 +157,17 @@ class Bridge {
} }
if($contents === '*') { // Whitelist all bridges if($contents === '*') { // Whitelist all bridges
self::$whitelist = self::getBridgeNames(); $this->whitelist = $this->getBridgeNames();
} else { } 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) { 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 @@ class Bridge {
* @param array $default The whitelist as array of bridge names. * @param array $default The whitelist as array of bridge names.
* @return void * @return void
*/ */
public static function setWhitelist($default = array()) { public function setWhitelist($default = array()) {
self::$whitelist = array_map('self::sanitizeBridgeName', $default); $this->whitelist = array_map('$this->sanitizeBridgeName', $default);
} }
/** /**
@ -269,7 +206,7 @@ class Bridge {
* @return string|null The sanitized bridge name if the provided name is * @return string|null The sanitized bridge name if the provided name is
* valid, null otherwise. * valid, null otherwise.
*/ */
protected static function sanitizeBridgeName($name) { protected function sanitizeBridgeName($name) {
if(is_string($name)) { if(is_string($name)) {
@ -284,15 +221,15 @@ class Bridge {
} }
// Improve performance for correctly written bridge names // Improve performance for correctly written bridge names
if(in_array($name, self::getBridgeNames())) { if(in_array($name, $this->getBridgeNames())) {
$index = array_search($name, self::getBridgeNames()); $index = array_search($name, $this->getBridgeNames());
return self::getBridgeNames()[$index]; return $this->getBridgeNames()[$index];
} }
// The name is valid if a corresponding bridge file is found on disk // The name is valid if a corresponding bridge file is found on disk
if(in_array(strtolower($name), array_map('strtolower', self::getBridgeNames()))) { if(in_array(strtolower($name), array_map('strtolower', $this->getBridgeNames()))) {
$index = array_search(strtolower($name), array_map('strtolower', self::getBridgeNames())); $index = array_search(strtolower($name), array_map('strtolower', $this->getBridgeNames()));
return self::getBridgeNames()[$index]; return $this->getBridgeNames()[$index];
} }
Debug::log('Invalid bridge name specified: "' . $name . '"!'); Debug::log('Invalid bridge name specified: "' . $name . '"!');

View file

@ -61,14 +61,17 @@ EOD;
$totalActiveBridges = 0; $totalActiveBridges = 0;
$inactiveBridges = ''; $inactiveBridges = '';
$bridgeList = Bridge::getBridgeNames(); $bridgeFac = new \BridgeFactory();
$bridgeFac->setWorkingDir(PATH_LIB_BRIDGES);
$bridgeList = $bridgeFac->getBridgeNames();
$formats = Format::getFormatNames(); $formats = Format::getFormatNames();
$totalBridges = count($bridgeList); $totalBridges = count($bridgeList);
foreach($bridgeList as $bridgeName) { foreach($bridgeList as $bridgeName) {
if(Bridge::isWhitelisted($bridgeName)) { if($bridgeFac->isWhitelisted($bridgeName)) {
$body .= BridgeCard::displayBridgeCard($bridgeName, $formats); $body .= BridgeCard::displayBridgeCard($bridgeName, $formats);
$totalActiveBridges++; $totalActiveBridges++;

View file

@ -63,7 +63,7 @@ require_once PATH_LIB . 'Debug.php';
require_once PATH_LIB . 'Exceptions.php'; require_once PATH_LIB . 'Exceptions.php';
require_once PATH_LIB . 'Format.php'; require_once PATH_LIB . 'Format.php';
require_once PATH_LIB . 'FormatAbstract.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 . 'BridgeAbstract.php';
require_once PATH_LIB . 'FeedExpander.php'; require_once PATH_LIB . 'FeedExpander.php';
require_once PATH_LIB . 'Cache.php'; require_once PATH_LIB . 'Cache.php';
@ -87,7 +87,6 @@ require_once PATH_LIB_VENDOR . 'php-urljoin/src/urljoin.php';
// Initialize static members // Initialize static members
try { try {
Bridge::setWorkingDir(PATH_LIB_BRIDGES);
Format::setWorkingDir(PATH_LIB_FORMATS); Format::setWorkingDir(PATH_LIB_FORMATS);
Cache::setWorkingDir(PATH_LIB_CACHES); Cache::setWorkingDir(PATH_LIB_CACHES);
} catch(Exception $e) { } catch(Exception $e) {