From fc8421ed504d613cfcb6f042665c0439ff1fcf35 Mon Sep 17 00:00:00 2001 From: logmanoriginal Date: Tue, 18 Jun 2019 19:15:20 +0200 Subject: [PATCH] format: Refactor format factory to non-static class The format 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 --- actions/DisplayAction.php | 4 +- lib/BridgeList.php | 6 +- lib/{Format.php => FormatFactory.php} | 94 +++++---------------------- lib/rssbridge.php | 11 +--- tests/AtomFormatTest.php | 4 +- tests/JsonFormatTest.php | 4 +- tests/MrssFormatTest.php | 4 +- 7 files changed, 32 insertions(+), 95 deletions(-) rename lib/{Format.php => FormatFactory.php} (60%) diff --git a/actions/DisplayAction.php b/actions/DisplayAction.php index 17bc28b7..9b4d363f 100644 --- a/actions/DisplayAction.php +++ b/actions/DisplayAction.php @@ -215,7 +215,9 @@ class DisplayAction extends ActionAbstract { // Data transformation try { - $format = Format::create($format); + $formatFac = new FormatFactory(); + $formatFac->setWorkingDir(PATH_LIB_FORMATS); + $format = $formatFac->create($format); $format->setItems($items); $format->setExtraInfos($infos); $format->setLastModified($cache->getTime()); diff --git a/lib/BridgeList.php b/lib/BridgeList.php index 0c3c4ffa..9f77f7af 100644 --- a/lib/BridgeList.php +++ b/lib/BridgeList.php @@ -63,9 +63,11 @@ EOD; $bridgeFac = new \BridgeFactory(); $bridgeFac->setWorkingDir(PATH_LIB_BRIDGES); - $bridgeList = $bridgeFac->getBridgeNames(); - $formats = Format::getFormatNames(); + + $formatFac = new FormatFactory(); + $formatFac->setWorkingDir(PATH_LIB_FORMATS); + $formats = $formatFac->getFormatNames(); $totalBridges = count($bridgeList); diff --git a/lib/Format.php b/lib/FormatFactory.php similarity index 60% rename from lib/Format.php rename to lib/FormatFactory.php index edbe5f57..28db7596 100644 --- a/lib/Format.php +++ b/lib/FormatFactory.php @@ -31,29 +31,7 @@ * $format = Format::create('Atom'); * ``` */ -class Format { - - /** - * Holds a path to the working directory. - * - * Do not access this property directly! - * Use {@see Format::setWorkingDir()} and {@see Format::getWorkingDir()} instead. - * - * @var string|null - */ - protected static $workingDir = null; - - /** - * Throws an exception when trying to create a new instance of this class. - * Use {@see Format::create()} to create a new format object from the working - * directory. - * - * @throws \LogicException if called. - */ - public function __construct(){ - throw new \LogicException('Use ' . __CLASS__ . '::create($name) to create cache objects!'); - } - +class FormatFactory extends FactoryAbstract { /** * Creates a new format object from the working directory. * @@ -63,13 +41,13 @@ class Format { * @param string $name Name of the format object. * @return object|bool The format object or false if the class is not instantiable. */ - public static function create($name){ - if(!self::isFormatName($name)) { + public function create($name){ + if(!$this->isFormatName($name)) { throw new \InvalidArgumentException('Format name invalid!'); } - $name = self::sanitizeFormatName($name) . 'Format'; - $pathFormat = self::getWorkingDir() . $name . '.php'; + $name = $this->sanitizeFormatName($name) . 'Format'; + $pathFormat = $this->getWorkingDir() . $name . '.php'; if(!file_exists($pathFormat)) { throw new \Exception('Format file ' . $filePath . ' does not exist!'); @@ -84,48 +62,6 @@ class Format { 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('Dir format must be a 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 Format::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 format name. * @@ -135,7 +71,7 @@ class Format { * @param string $name The format name. * @return bool true if the name is a valid format name, false otherwise. */ - public static function isFormatName($name){ + public function isFormatName($name){ return is_string($name) && preg_match('/^[A-Z][a-zA-Z0-9-]*$/', $name) === 1; } @@ -146,11 +82,11 @@ class Format { * * @return array List of format names */ - public static function getFormatNames(){ + public function getFormatNames(){ static $formatNames = array(); // Initialized on first call if(empty($formatNames)) { - $files = scandir(self::getWorkingDir()); + $files = scandir($this->getWorkingDir()); if($files !== false) { foreach($files as $file) { @@ -181,7 +117,7 @@ class Format { * @return string|null The sanitized format name if the provided name is * valid, null otherwise. */ - protected static function sanitizeFormatName($name) { + protected function sanitizeFormatName($name) { if(is_string($name)) { @@ -196,15 +132,15 @@ class Format { } // Improve performance for correctly written format names - if(in_array($name, self::getFormatNames())) { - $index = array_search($name, self::getFormatNames()); - return self::getFormatNames()[$index]; + if(in_array($name, $this->getFormatNames())) { + $index = array_search($name, $this->getFormatNames()); + return $this->getFormatNames()[$index]; } // The name is valid if a corresponding format file is found on disk - if(in_array(strtolower($name), array_map('strtolower', self::getFormatNames()))) { - $index = array_search(strtolower($name), array_map('strtolower', self::getFormatNames())); - return self::getFormatNames()[$index]; + if(in_array(strtolower($name), array_map('strtolower', $this->getFormatNames()))) { + $index = array_search(strtolower($name), array_map('strtolower', $this->getFormatNames())); + return $this->getFormatNames()[$index]; } Debug::log('Invalid format name: "' . $name . '"!'); diff --git a/lib/rssbridge.php b/lib/rssbridge.php index eda4e583..a025f229 100644 --- a/lib/rssbridge.php +++ b/lib/rssbridge.php @@ -61,7 +61,7 @@ require_once PATH_LIB . 'FactoryAbstract.php'; require_once PATH_LIB . 'FeedItem.php'; require_once PATH_LIB . 'Debug.php'; require_once PATH_LIB . 'Exceptions.php'; -require_once PATH_LIB . 'Format.php'; +require_once PATH_LIB . 'FormatFactory.php'; require_once PATH_LIB . 'FormatAbstract.php'; require_once PATH_LIB . 'BridgeFactory.php'; require_once PATH_LIB . 'BridgeAbstract.php'; @@ -84,12 +84,3 @@ require_once PATH_LIB . 'contents.php'; define('MAX_FILE_SIZE', 10000000); /* Allow larger files for simple_html_dom */ require_once PATH_LIB_VENDOR . 'simplehtmldom/simple_html_dom.php'; require_once PATH_LIB_VENDOR . 'php-urljoin/src/urljoin.php'; - -// Initialize static members -try { - Format::setWorkingDir(PATH_LIB_FORMATS); -} catch(Exception $e) { - error_log($e); - header('Content-type: text/plain', true, 500); - die($e->getMessage()); -} diff --git a/tests/AtomFormatTest.php b/tests/AtomFormatTest.php index 1a8905fb..818b82eb 100644 --- a/tests/AtomFormatTest.php +++ b/tests/AtomFormatTest.php @@ -77,7 +77,9 @@ class AtomFormatTest extends TestCase { } private function initFormat() { - $this->format = \Format::create('Atom'); + $formatFac = new FormatFactory(); + $formatFac->setWorkingDir(PATH_LIB_FORMATS); + $this->format = $formatFac->create('Atom'); $this->format->setItems($this->sample->items); $this->format->setExtraInfos($this->sample->meta); $this->format->setLastModified(strtotime('2000-01-01 12:00:00 UTC')); diff --git a/tests/JsonFormatTest.php b/tests/JsonFormatTest.php index 24d09067..a9417e25 100644 --- a/tests/JsonFormatTest.php +++ b/tests/JsonFormatTest.php @@ -77,7 +77,9 @@ class JsonFormatTest extends TestCase { } private function initFormat() { - $this->format = \Format::create('Json'); + $formatFac = new FormatFactory(); + $formatFac->setWorkingDir(PATH_LIB_FORMATS); + $this->format = $formatFac->create('Json'); $this->format->setItems($this->sample->items); $this->format->setExtraInfos($this->sample->meta); $this->format->setLastModified(strtotime('2000-01-01 12:00:00 UTC')); diff --git a/tests/MrssFormatTest.php b/tests/MrssFormatTest.php index b4dd32a9..0ddc33c4 100644 --- a/tests/MrssFormatTest.php +++ b/tests/MrssFormatTest.php @@ -78,7 +78,9 @@ class MrssFormatTest extends TestCase { } private function initFormat() { - $this->format = \Format::create('Mrss'); + $formatFac = new FormatFactory(); + $formatFac->setWorkingDir(PATH_LIB_FORMATS); + $this->format = $formatFac->create('Mrss'); $this->format->setItems($this->sample->items); $this->format->setExtraInfos($this->sample->meta); $this->format->setLastModified(strtotime('2000-01-01 12:00:00 UTC'));