From 0c7a7f320fd170d0909b029be551347780120357 Mon Sep 17 00:00:00 2001
From: Dag <me@dvikan.no>
Date: Sun, 8 May 2022 03:58:42 +0200
Subject: [PATCH] refactor: BridgeFactory (#2691)
---
actions/ConnectivityAction.php | 1 -
actions/DetectAction.php | 1 -
actions/DisplayAction.php | 1 -
actions/ListAction.php | 1 -
lib/BridgeCard.php | 1 -
lib/BridgeFactory.php | 283 ++++++++-------------------------
lib/BridgeList.php | 1 -
tests/ListActionTest.php | 1 -
8 files changed, 65 insertions(+), 225 deletions(-)
diff --git a/actions/ConnectivityAction.php b/actions/ConnectivityAction.php
index ea8cb54f..a24b4901 100644
--- a/actions/ConnectivityAction.php
+++ b/actions/ConnectivityAction.php
@@ -55,7 +55,6 @@ class ConnectivityAction extends ActionAbstract {
private function reportBridgeConnectivity($bridgeName) {
$bridgeFac = new \BridgeFactory();
- $bridgeFac->setWorkingDir(PATH_LIB_BRIDGES);
if(!$bridgeFac->isWhitelisted($bridgeName)) {
header('Content-Type: text/html');
diff --git a/actions/DetectAction.php b/actions/DetectAction.php
index 86605de4..e58aee93 100644
--- a/actions/DetectAction.php
+++ b/actions/DetectAction.php
@@ -20,7 +20,6 @@ class DetectAction extends ActionAbstract {
or returnClientError('You must specify a format!');
$bridgeFac = new \BridgeFactory();
- $bridgeFac->setWorkingDir(PATH_LIB_BRIDGES);
foreach($bridgeFac->getBridgeNames() as $bridgeName) {
diff --git a/actions/DisplayAction.php b/actions/DisplayAction.php
index 6f3b997a..99dd21ce 100644
--- a/actions/DisplayAction.php
+++ b/actions/DisplayAction.php
@@ -28,7 +28,6 @@ class DisplayAction extends ActionAbstract {
or returnClientError('You must specify a format!');
$bridgeFac = new \BridgeFactory();
- $bridgeFac->setWorkingDir(PATH_LIB_BRIDGES);
// whitelist control
if(!$bridgeFac->isWhitelisted($bridge)) {
diff --git a/actions/ListAction.php b/actions/ListAction.php
index f3509564..f7b92063 100644
--- a/actions/ListAction.php
+++ b/actions/ListAction.php
@@ -18,7 +18,6 @@ class ListAction extends ActionAbstract {
$list->total = 0;
$bridgeFac = new \BridgeFactory();
- $bridgeFac->setWorkingDir(PATH_LIB_BRIDGES);
foreach($bridgeFac->getBridgeNames() as $bridgeName) {
diff --git a/lib/BridgeCard.php b/lib/BridgeCard.php
index ebd958fb..befde86a 100644
--- a/lib/BridgeCard.php
+++ b/lib/BridgeCard.php
@@ -289,7 +289,6 @@ This bridge is not fetching its content through a secure connection</div>';
static function displayBridgeCard($bridgeName, $formats, $isActive = true){
$bridgeFac = new \BridgeFactory();
- $bridgeFac->setWorkingDir(PATH_LIB_BRIDGES);
$bridge = $bridgeFac->create($bridgeName);
diff --git a/lib/BridgeFactory.php b/lib/BridgeFactory.php
index 5281fa62..f435261c 100644
--- a/lib/BridgeFactory.php
+++ b/lib/BridgeFactory.php
@@ -1,240 +1,87 @@
<?php
-/**
- * This file is part of RSS-Bridge, a PHP project capable of generating RSS and
- * Atom feeds for websites that don't have one.
- *
- * For the full license information, please view the UNLICENSE file distributed
- * with this source code.
- *
- * @package Core
- * @license http://unlicense.org/ UNLICENSE
- * @link https://github.com/rss-bridge/rss-bridge
- */
-/**
- * Factory class responsible for creating bridge objects from a given working
- * directory, limited by a whitelist.
- *
- * This class is capable of:
- * - Locating bridge classes in the specified working directory (see {@see Bridge::$workingDir})
- * - Filtering bridges based on a whitelist (see {@see Bridge::$whitelist})
- * - Creating new bridge instances based on the bridge's name (see {@see Bridge::create()})
- *
- * The following example illustrates the intended use for this class.
- *
- * ```PHP
- * require_once __DIR__ . '/rssbridge.php';
- *
- * // Step 1: Set the working directory
- * Bridge::setWorkingDir(__DIR__ . '/../bridges/');
- *
- * // Step 2: Add bridges to the whitelist
- * Bridge::setWhitelist(array('GitHubIssue', 'GoogleSearch', 'Facebook', 'Twitter'));
- *
- * // Step 3: Create a new instance of a bridge (based on the name)
- * $bridge = Bridge::create('GitHubIssue');
- * ```
- */
-class BridgeFactory extends FactoryAbstract {
+final class BridgeFactory {
- /**
- * Holds a list of whitelisted bridges.
- *
- * Do not access this property directly!
- * Use {@see Bridge::getWhitelist()} instead.
- *
- * @var array
- */
- protected $whitelist = array();
+ private $folder;
+ private $bridgeNames = [];
+ private $whitelist = [];
- /**
- * Creates a new bridge object from the working directory.
- *
- * @throws \InvalidArgumentException if the requested bridge name is invalid.
- * @throws \Exception if the requested bridge doesn't exist in the working
- * directory.
- * @param string $name Name of the bridge object.
- * @return object|bool The bridge object or false if the class is not instantiable.
- */
- public function create($name){
- if(!$this->isBridgeName($name)) {
- throw new \InvalidArgumentException('Bridge name invalid!');
- }
+ public function __construct(string $folder = PATH_LIB_BRIDGES)
+ {
+ $this->folder = $folder;
- $name = $this->sanitizeBridgeName($name) . 'Bridge';
- $filePath = $this->getWorkingDir() . $name . '.php';
-
- if(!file_exists($filePath)) {
- throw new \Exception('Bridge file ' . $filePath . ' does not exist!');
- }
-
- if((new \ReflectionClass($name))->isInstantiable()) {
- return new $name();
- }
-
- return false;
- }
-
- /**
- * Returns true if the provided name is a valid bridge name.
- *
- * A valid bridge name starts with a capital letter ([A-Z]), followed by
- * zero or more alphanumeric characters or hyphen ([A-Za-z0-9-]).
- *
- * @param string $name The bridge name.
- * @return bool true if the name is a valid bridge name, false otherwise.
- */
- public function isBridgeName($name){
- return is_string($name) && preg_match('/^[A-Z][a-zA-Z0-9-]*$/', $name) === 1;
- }
-
- /**
- * Returns the list of bridge names from the working directory.
- *
- * The list is cached internally to allow for successive calls.
- *
- * @return array List of bridge names
- */
- public function getBridgeNames(){
-
- static $bridgeNames = array(); // Initialized on first call
-
- if(empty($bridgeNames)) {
- $files = scandir($this->getWorkingDir());
-
- if($files !== false) {
- foreach($files as $file) {
- if(preg_match('/^([^.]+)Bridge\.php$/U', $file, $out)) {
- $bridgeNames[] = $out[1];
- }
- }
+ // create names
+ foreach(scandir($this->folder) as $file) {
+ if(preg_match('/^([^.]+)Bridge\.php$/U', $file, $m)) {
+ $this->bridgeNames[] = $m[1];
}
}
- return $bridgeNames;
+ // create whitelist
+ if (file_exists(WHITELIST)) {
+ $contents = trim(file_get_contents(WHITELIST));
+ } elseif (file_exists(WHITELIST_DEFAULT)) {
+ $contents = trim(file_get_contents(WHITELIST_DEFAULT));
+ } else {
+ $contents = '';
+ }
+ if ($contents === '*') { // Whitelist all bridges
+ $this->whitelist = $this->getBridgeNames();
+ } else {
+ foreach (explode("\n", $contents) as $bridgeName) {
+ $this->whitelist[] = $this->sanitizeBridgeName($bridgeName);
+ }
+ }
}
- /**
- * Checks if a bridge is whitelisted.
- *
- * @param string $name Name of the bridge.
- * @return bool True if the bridge is whitelisted.
- */
- public function isWhitelisted($name){
- return in_array($this->sanitizeBridgeName($name), $this->getWhitelist());
+ public function create(string $name): BridgeInterface
+ {
+ if(preg_match('/^[A-Z][a-zA-Z0-9-]*$/', $name)) {
+ $className = sprintf('%sBridge', $this->sanitizeBridgeName($name));
+ return new $className();
+ }
+ throw new \InvalidArgumentException('Bridge name invalid!');
}
- /**
- * Returns the whitelist.
- *
- * On first call this function reads the whitelist from {@see WHITELIST} if
- * the file exists, {@see WHITELIST_DEFAULT} otherwise.
- * * Each line in the file specifies one bridge on the whitelist.
- * * An empty file disables all bridges.
- * * If the file only only contains `*`, all bridges are whitelisted.
- *
- * Use {@see Bridge::setWhitelist()} to specify a default whitelist **before**
- * calling this function! The list is cached internally to allow for
- * successive calls. If {@see Bridge::setWhitelist()} gets called after this
- * function, the whitelist is **not** updated again!
- *
- * @return array Array of whitelisted bridges
- */
- public function getWhitelist() {
+ public function getBridgeNames(): array
+ {
+ return $this->bridgeNames;
+ }
- static $firstCall = true; // Initialized on first call
+ public function isWhitelisted($name): bool
+ {
+ return in_array($this->sanitizeBridgeName($name), $this->whitelist);
+ }
- if($firstCall) {
-
- if(file_exists(WHITELIST)) {
- $contents = trim(file_get_contents(WHITELIST));
- } elseif(file_exists(WHITELIST_DEFAULT)) {
- $contents = trim(file_get_contents(WHITELIST_DEFAULT));
- } else {
- $contents = '';
- }
-
- if($contents === '*') { // Whitelist all bridges
- $this->whitelist = $this->getBridgeNames();
- } else {
- //$this->$whitelist = array_map('$this->sanitizeBridgeName', explode("\n", $contents));
- foreach(explode("\n", $contents) as $bridgeName) {
- $this->whitelist[] = $this->sanitizeBridgeName($bridgeName);
- }
- }
+ private function sanitizeBridgeName($name) {
+ if(!is_string($name)) {
+ return null;
}
- return $this->whitelist;
-
- }
-
- /**
- * Sets the (default) whitelist.
- *
- * If this function is called **before** {@see Bridge::getWhitelist()}, the
- * provided whitelist will be replaced by a custom whitelist specified in
- * {@see WHITELIST} (if it exists).
- *
- * If this function is called **after** {@see Bridge::getWhitelist()}, the
- * provided whitelist is taken as is (not updated by the custom whitelist
- * again).
- *
- * @param array $default The whitelist as array of bridge names.
- * @return void
- */
- public function setWhitelist($default = array()) {
- $this->whitelist = array_map('$this->sanitizeBridgeName', $default);
- }
-
- /**
- * Returns the sanitized bridge name.
- *
- * The bridge name can be specified in various ways:
- * * The PHP file name (i.e. `GitHubIssueBridge.php`)
- * * The PHP file name without file extension (i.e. `GitHubIssueBridge`)
- * * The bridge name (i.e. `GitHubIssue`)
- *
- * Casing is ignored (i.e. `GITHUBISSUE` and `githubissue` are the same).
- *
- * A bridge file matching the given bridge name must exist in the working
- * directory!
- *
- * @param string $name The bridge name
- * @return string|null The sanitized bridge name if the provided name is
- * valid, null otherwise.
- */
- protected function sanitizeBridgeName($name) {
-
- if(is_string($name)) {
-
- // Trim trailing '.php' if exists
- if(preg_match('/(.+)(?:\.php)/', $name, $matches)) {
- $name = $matches[1];
- }
-
- // Trim trailing 'Bridge' if exists
- if(preg_match('/(.+)(?:Bridge)/i', $name, $matches)) {
- $name = $matches[1];
- }
-
- // Improve performance for correctly written bridge names
- 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', $this->getBridgeNames()))) {
- $index = array_search(strtolower($name), array_map('strtolower', $this->getBridgeNames()));
- return $this->getBridgeNames()[$index];
- }
-
- Debug::log('Invalid bridge name specified: "' . $name . '"!');
-
+ // Trim trailing '.php' if exists
+ if (preg_match('/(.+)(?:\.php)/', $name, $matches)) {
+ $name = $matches[1];
}
- return null; // Bad parameter
+ // Trim trailing 'Bridge' if exists
+ if (preg_match('/(.+)(?:Bridge)/i', $name, $matches)) {
+ $name = $matches[1];
+ }
+ // Improve performance for correctly written bridge names
+ 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', $this->getBridgeNames()))) {
+ $index = array_search(strtolower($name), array_map('strtolower', $this->getBridgeNames()));
+ return $this->getBridgeNames()[$index];
+ }
+
+ Debug::log('Invalid bridge name specified: "' . $name . '"!');
+ return null;
}
}
diff --git a/lib/BridgeList.php b/lib/BridgeList.php
index 7b2d5268..4aca037c 100644
--- a/lib/BridgeList.php
+++ b/lib/BridgeList.php
@@ -63,7 +63,6 @@ EOD;
$inactiveBridges = '';
$bridgeFac = new \BridgeFactory();
- $bridgeFac->setWorkingDir(PATH_LIB_BRIDGES);
$bridgeList = $bridgeFac->getBridgeNames();
$formatFac = new FormatFactory();
diff --git a/tests/ListActionTest.php b/tests/ListActionTest.php
index f15e1b73..e914b7d3 100644
--- a/tests/ListActionTest.php
+++ b/tests/ListActionTest.php
@@ -41,7 +41,6 @@ class ListActionTest extends TestCase {
);
$bridgeFac = new BridgeFactory();
- $bridgeFac->setWorkingDir(PATH_LIB_BRIDGES);
$this->assertEquals(
count($bridgeFac->getBridgeNames()),