Skip to content

Commit

Permalink
bridge: Refactor bridge factory to non-static class
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@logmanoriginal
logmanoriginal committed Jun 18, 2019
1 parent 1ada9c2 commit 705b9da
Show file tree
Hide file tree
Showing 7 changed files with 52 additions and 101 deletions.
9 changes: 6 additions & 3 deletions actions/DetectAction.php
View file
Expand Up @@ -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;
Expand Down
7 changes: 5 additions & 2 deletions actions/DisplayAction.php
View file
Expand Up @@ -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);
Expand Down
9 changes: 6 additions & 3 deletions actions/ListAction.php
View file
Expand Up @@ -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

Expand All @@ -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,
Expand Down
5 changes: 4 additions & 1 deletion lib/BridgeCard.php
View file
Expand Up @@ -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 '';
Expand Down
113 changes: 25 additions & 88 deletions lib/Bridge.php → lib/BridgeFactory.php
View file
Expand Up @@ -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.
Expand All @@ -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.
Expand All @@ -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!');
Expand All @@ -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.
*
Expand All @@ -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;
}

Expand All @@ -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) {
Expand All @@ -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());
}

/**
Expand All @@ -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

Expand All @@ -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;

}

Expand All @@ -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);
}

/**
Expand All @@ -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)) {

Expand All @@ -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 . '"!');
Expand Down
7 changes: 5 additions & 2 deletions lib/BridgeList.php
View file
Expand Up @@ -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++;
Expand Down
3 changes: 1 addition & 2 deletions lib/rssbridge.php
View file
Expand Up @@ -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';
Expand All @@ -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) {
Expand Down

0 comments on commit 705b9da

Please sign in to comment.