Skip to content

Commit

Permalink
format: Refactor format factory to non-static class
Browse files Browse the repository at this point in the history
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
  • Loading branch information
logmanoriginal committed Jun 18, 2019
1 parent 2460b67 commit fc8421e
Show file tree
Hide file tree
Showing 7 changed files with 32 additions and 95 deletions.
4 changes: 3 additions & 1 deletion actions/DisplayAction.php
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,9 @@ public function execute() {

// 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());
Expand Down
6 changes: 4 additions & 2 deletions lib/BridgeList.php
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,11 @@ private static function getBridges($showInactive, &$totalBridges, &$totalActiveB

$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);

Expand Down
94 changes: 15 additions & 79 deletions lib/Format.php → lib/FormatFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand All @@ -63,13 +41,13 @@ public function __construct(){
* @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!');
Expand All @@ -84,48 +62,6 @@ public static function create($name){
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.
*
Expand All @@ -135,7 +71,7 @@ public static function getWorkingDir(){
* @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;
}

Expand All @@ -146,11 +82,11 @@ public static function isFormatName($name){
*
* @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) {
Expand Down Expand Up @@ -181,7 +117,7 @@ public static function getFormatNames(){
* @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)) {

Expand All @@ -196,15 +132,15 @@ protected static function sanitizeFormatName($name) {
}

// 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 . '"!');
Expand Down
11 changes: 1 addition & 10 deletions lib/rssbridge.php
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@
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';
Expand All @@ -84,12 +84,3 @@
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());
}
4 changes: 3 additions & 1 deletion tests/AtomFormatTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,9 @@ private function setServerVars($list) {
}

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'));
Expand Down
4 changes: 3 additions & 1 deletion tests/JsonFormatTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,9 @@ private function setServerVars($list) {
}

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'));
Expand Down
4 changes: 3 additions & 1 deletion tests/MrssFormatTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,9 @@ private function setServerVars($list) {
}

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'));
Expand Down

0 comments on commit fc8421e

Please sign in to comment.