From 5dcdf2322e843ffa2e5ab8bb3c287fc03d92c345 Mon Sep 17 00:00:00 2001 From: Philipp Date: Sun, 1 Aug 2021 13:06:19 +0200 Subject: [PATCH] Split IStorage and ISelectableStorage and make their behaviour homogenous --- src/Model/Storage/Database.php | 66 +++++++----- src/Model/Storage/ExternalResource.php | 56 ++-------- src/Model/Storage/Filesystem.php | 61 +++++++---- src/Model/Storage/ISelectableStorage.php | 100 ++++++++++++++++++ src/Model/Storage/IStorage.php | 80 ++------------ ...rage.php => ReferenceStorageException.php} | 26 +---- src/Model/Storage/StorageException.php | 6 +- src/Model/Storage/SystemResource.php | 47 ++------ src/Util/HTTPSignature.php | 3 +- 9 files changed, 216 insertions(+), 229 deletions(-) create mode 100644 src/Model/Storage/ISelectableStorage.php rename src/Model/Storage/{AbstractStorage.php => ReferenceStorageException.php} (60%) diff --git a/src/Model/Storage/Database.php b/src/Model/Storage/Database.php index 4eaa813ca..5e83ca856 100644 --- a/src/Model/Storage/Database.php +++ b/src/Model/Storage/Database.php @@ -21,8 +21,7 @@ namespace Friendica\Model\Storage; -use Friendica\Core\L10n; -use Psr\Log\LoggerInterface; +use Exception; use Friendica\Database\Database as DBA; /** @@ -30,7 +29,7 @@ use Friendica\Database\Database as DBA; * * This class manage data stored in database table. */ -class Database extends AbstractStorage +class Database implements ISelectableStorage { const NAME = 'Database'; @@ -39,47 +38,53 @@ class Database extends AbstractStorage /** * @param DBA $dba - * @param LoggerInterface $logger - * @param L10n $l10n */ - public function __construct(DBA $dba, LoggerInterface $logger, L10n $l10n) + public function __construct(DBA $dba) { - parent::__construct($l10n, $logger); - $this->dba = $dba; } /** * @inheritDoc */ - public function get(string $reference) + public function get(string $reference): string { - $result = $this->dba->selectFirst('storage', ['data'], ['id' => $reference]); - if (!$this->dba->isResult($result)) { - return ''; - } + try { + $result = $this->dba->selectFirst('storage', ['data'], ['id' => $reference]); + if (!$this->dba->isResult($result)) { + throw new ReferenceStorageException(sprintf('Database storage cannot find data for reference %s', $reference)); + } - return $result['data']; + return $result['data']; + } catch (Exception $exception) { + throw new StorageException(sprintf('Database storage failed to get %s', $reference), $exception->getCode(), $exception); + } } /** * @inheritDoc */ - public function put(string $data, string $reference = '') + public function put(string $data, string $reference = ''): string { if ($reference !== '') { - $result = $this->dba->update('storage', ['data' => $data], ['id' => $reference]); + try { + $result = $this->dba->update('storage', ['data' => $data], ['id' => $reference]); + } catch (Exception $exception) { + throw new StorageException(sprintf('Database storage failed to update %s', $reference), $exception->getCode(), $exception); + } if ($result === false) { - $this->logger->warning('Failed to update data.', ['id' => $reference, 'errorCode' => $this->dba->errorNo(), 'errorMessage' => $this->dba->errorMessage()]); - throw new StorageException($this->l10n->t('Database storage failed to update %s', $reference)); + throw new StorageException(sprintf('Database storage failed to update %s', $reference), 500, new Exception($this->dba->errorMessage(), $this->dba->errorNo())); } return $reference; } else { - $result = $this->dba->insert('storage', ['data' => $data]); + try { + $result = $this->dba->insert('storage', ['data' => $data]); + } catch (Exception $exception) { + throw new StorageException(sprintf('Database storage failed to insert %s', $reference), $exception->getCode(), $exception); + } if ($result === false) { - $this->logger->warning('Failed to insert data.', ['errorCode' => $this->dba->errorNo(), 'errorMessage' => $this->dba->errorMessage()]); - throw new StorageException($this->l10n->t('Database storage failed to insert data')); + throw new StorageException(sprintf('Database storage failed to update %s', $reference), 500, new Exception($this->dba->errorMessage(), $this->dba->errorNo())); } return $this->dba->lastInsertId(); @@ -91,13 +96,19 @@ class Database extends AbstractStorage */ public function delete(string $reference) { - return $this->dba->delete('storage', ['id' => $reference]); + try { + if (!$this->dba->delete('storage', ['id' => $reference])) { + throw new ReferenceStorageException(sprintf('Database storage failed to delete %s', $reference)); + } + } catch (Exception $exception) { + throw new StorageException(sprintf('Database storage failed to delete %s', $reference), $exception->getCode(), $exception); + } } /** * @inheritDoc */ - public function getOptions() + public function getOptions(): array { return []; } @@ -105,7 +116,7 @@ class Database extends AbstractStorage /** * @inheritDoc */ - public function saveOptions(array $data) + public function saveOptions(array $data): array { return []; } @@ -113,8 +124,13 @@ class Database extends AbstractStorage /** * @inheritDoc */ - public static function getName() + public static function getName(): string { return self::NAME; } + + public function __toString() + { + return self::getName(); + } } diff --git a/src/Model/Storage/ExternalResource.php b/src/Model/Storage/ExternalResource.php index 0e758c706..402619e67 100644 --- a/src/Model/Storage/ExternalResource.php +++ b/src/Model/Storage/ExternalResource.php @@ -21,9 +21,8 @@ namespace Friendica\Model\Storage; -use BadMethodCallException; +use Exception; use Friendica\Util\HTTPSignature; -use Friendica\Network\IHTTPRequest; /** * External resource storage class @@ -35,66 +34,33 @@ class ExternalResource implements IStorage { const NAME = 'ExternalResource'; - /** @var IHTTPRequest */ - private $httpRequest; - - public function __construct(IHTTPRequest $httpRequest) - { - $this->httpRequest = $httpRequest; - } - /** * @inheritDoc */ - public function get(string $reference) + public function get(string $reference): string { $data = json_decode($reference); if (empty($data->url)) { - return ""; + throw new ReferenceStorageException(sprintf('Invalid reference %s, cannot retrieve URL', $reference)); } $parts = parse_url($data->url); if (empty($parts['scheme']) || empty($parts['host'])) { - return ""; + throw new ReferenceStorageException(sprintf('Invalid reference %s, cannot extract scheme and host', $reference)); } - $fetchResult = HTTPSignature::fetchRaw($data->url, $data->uid, ['accept_content' => '']); + try { + $fetchResult = HTTPSignature::fetchRaw($data->url, $data->uid, ['accept_content' => '']); + } catch (Exception $exception) { + throw new StorageException(sprintf('External resource failed to get %s', $reference), $exception->getCode(), $exception); + } if ($fetchResult->isSuccess()) { return $fetchResult->getBody(); } else { - return ""; + throw new StorageException(sprintf('External resource failed to get %s', $reference), $fetchResult->getReturnCode(), new Exception($fetchResult->getBody())); } } - /** - * @inheritDoc - */ - public function put(string $data, string $reference = '') - { - throw new BadMethodCallException(); - } - - public function delete(string $reference) - { - throw new BadMethodCallException(); - } - - /** - * @inheritDoc - */ - public function getOptions() - { - return []; - } - - /** - * @inheritDoc - */ - public function saveOptions(array $data) - { - return []; - } - /** * @inheritDoc */ @@ -106,7 +72,7 @@ class ExternalResource implements IStorage /** * @inheritDoc */ - public static function getName() + public static function getName(): string { return self::NAME; } diff --git a/src/Model/Storage/Filesystem.php b/src/Model/Storage/Filesystem.php index dfe0b812e..7760b4a53 100644 --- a/src/Model/Storage/Filesystem.php +++ b/src/Model/Storage/Filesystem.php @@ -21,10 +21,10 @@ namespace Friendica\Model\Storage; +use Exception; use Friendica\Core\Config\IConfig; use Friendica\Core\L10n; use Friendica\Util\Strings; -use Psr\Log\LoggerInterface; /** * Filesystem based storage backend @@ -36,7 +36,7 @@ use Psr\Log\LoggerInterface; * Each new resource gets a value as reference and is saved in a * folder tree stucture created from that value. */ -class Filesystem extends AbstractStorage +class Filesystem implements ISelectableStorage { const NAME = 'Filesystem'; @@ -49,18 +49,19 @@ class Filesystem extends AbstractStorage /** @var string */ private $basePath; + /** @var L10n */ + private $l10n; + /** * Filesystem constructor. * * @param IConfig $config - * @param LoggerInterface $logger * @param L10n $l10n */ - public function __construct(IConfig $config, LoggerInterface $logger, L10n $l10n) + public function __construct(IConfig $config, L10n $l10n) { - parent::__construct($l10n, $logger); - $this->config = $config; + $this->l10n = $l10n; $path = $this->config->get('storage', 'filesystem_path', self::DEFAULT_BASE_FOLDER); $this->basePath = rtrim($path, '/'); @@ -73,7 +74,7 @@ class Filesystem extends AbstractStorage * * @return string */ - private function pathForRef(string $reference) + private function pathForRef(string $reference): string { $fold1 = substr($reference, 0, 2); $fold2 = substr($reference, 2, 2); @@ -84,7 +85,7 @@ class Filesystem extends AbstractStorage /** - * Create dirctory tree to store file, with .htaccess and index.html files + * Create directory tree to store file, with .htaccess and index.html files * * @param string $file Path and filename * @@ -96,8 +97,7 @@ class Filesystem extends AbstractStorage if (!is_dir($path)) { if (!mkdir($path, 0770, true)) { - $this->logger->warning('Failed to create dir.', ['path' => $path]); - throw new StorageException($this->l10n->t('Filesystem storage failed to create "%s". Check you write permissions.', $path)); + throw new StorageException(sprintf('Filesystem storage failed to create "%s". Check you write permissions.', $path)); } } @@ -118,23 +118,32 @@ class Filesystem extends AbstractStorage /** * @inheritDoc */ - public function get(string $reference) + public function get(string $reference): string { $file = $this->pathForRef($reference); if (!is_file($file)) { - return ''; + throw new ReferenceStorageException(sprintf('Filesystem storage failed to get the file %s, The file is invalid', $reference)); } - return file_get_contents($file); + $result = file_get_contents($file); + + // just in case the result is REALLY false, not zero or empty or anything else, throw the exception + if ($result === false) { + throw new StorageException(sprintf('Filesystem storage failed to get data to "%s". Check your write permissions', $file)); + } } /** * @inheritDoc */ - public function put(string $data, string $reference = '') + public function put(string $data, string $reference = ''): string { if ($reference === '') { - $reference = Strings::getRandomHex(); + try { + $reference = Strings::getRandomHex(); + } catch (Exception $exception) { + throw new StorageException('Filesystem storage failed to generate a random hex', $exception->getCode(), $exception); + } } $file = $this->pathForRef($reference); @@ -144,8 +153,7 @@ class Filesystem extends AbstractStorage // just in case the result is REALLY false, not zero or empty or anything else, throw the exception if ($result === false) { - $this->logger->warning('Failed to write data.', ['file' => $file]); - throw new StorageException($this->l10n->t('Filesystem storage failed to save data to "%s". Check your write permissions', $file)); + throw new StorageException(sprintf('Filesystem storage failed to save data to "%s". Check your write permissions', $file)); } chmod($file, 0660); @@ -158,17 +166,19 @@ class Filesystem extends AbstractStorage public function delete(string $reference) { $file = $this->pathForRef($reference); - // return true if file doesn't exists. we want to delete it: success with zero work! if (!is_file($file)) { - return true; + throw new ReferenceStorageException(sprintf('File with reference "%s" doesn\'t exist', $reference)); + } + + if (!unlink($file)) { + throw new StorageException(sprintf('Cannot delete with file with reference "%s"', $reference)); } - return unlink($file); } /** * @inheritDoc */ - public function getOptions() + public function getOptions(): array { return [ 'storagepath' => [ @@ -183,7 +193,7 @@ class Filesystem extends AbstractStorage /** * @inheritDoc */ - public function saveOptions(array $data) + public function saveOptions(array $data): array { $storagePath = $data['storagepath'] ?? ''; if ($storagePath === '' || !is_dir($storagePath)) { @@ -199,8 +209,13 @@ class Filesystem extends AbstractStorage /** * @inheritDoc */ - public static function getName() + public static function getName(): string { return self::NAME; } + + public function __toString() + { + return self::getName(); + } } diff --git a/src/Model/Storage/ISelectableStorage.php b/src/Model/Storage/ISelectableStorage.php new file mode 100644 index 000000000..f1477a79c --- /dev/null +++ b/src/Model/Storage/ISelectableStorage.php @@ -0,0 +1,100 @@ +. + * + */ + +namespace Friendica\Model\Storage; + +/** + * Interface for selectable storage backends + */ +interface ISelectableStorage extends IStorage +{ + /** + * Put data in backend as $ref. If $ref is not defined a new reference is created. + * + * @param string $data Data to save + * @param string $reference Data reference. Optional. + * + * @return string Saved data reference + * + * @throws StorageException in case there's an unexpected error + */ + public function put(string $data, string $reference = ""): string; + + /** + * Remove data from backend + * + * @param string $reference Data reference + * + * @throws StorageException in case there's an unexpected error + * @throws ReferenceStorageException in case the reference doesn't exist + */ + public function delete(string $reference); + + /** + * Get info about storage options + * + * @return array + * + * This method return an array with informations about storage options + * from which the form presented to the user is build. + * + * The returned array is: + * + * [ + * 'option1name' => [ ..info.. ], + * 'option2name' => [ ..info.. ], + * ... + * ] + * + * An empty array can be returned if backend doesn't have any options + * + * The info array for each option MUST be as follows: + * + * [ + * 'type', // define the field used in form, and the type of data. + * // one of 'checkbox', 'combobox', 'custom', 'datetime', + * // 'input', 'intcheckbox', 'password', 'radio', 'richtext' + * // 'select', 'select_raw', 'textarea' + * + * 'label', // Translatable label of the field + * 'value', // Current value + * 'help text', // Translatable description for the field + * extra data // Optional. Depends on 'type': + * // select: array [ value => label ] of choices + * // intcheckbox: value of input element + * // select_raw: prebuild html string of < option > tags + * ] + * + * See https://github.com/friendica/friendica/wiki/Quick-Template-Guide + */ + public function getOptions(): array; + + /** + * Validate and save options + * + * @param array $data Array [optionname => value] to be saved + * + * @return array Validation errors: [optionname => error message] + * + * Return array must be empty if no error. + */ + public function saveOptions(array $data): array; +} diff --git a/src/Model/Storage/IStorage.php b/src/Model/Storage/IStorage.php index 0a0f58958..884148741 100644 --- a/src/Model/Storage/IStorage.php +++ b/src/Model/Storage/IStorage.php @@ -22,9 +22,7 @@ namespace Friendica\Model\Storage; /** - * Interface for storage backends - * - * @todo Split this interface into "IStorage" for get() operations (including Resource fetching) and "IUserStorage" for real user backends including put/delete/options + * Interface for basic storage backends */ interface IStorage { @@ -34,77 +32,11 @@ interface IStorage * @param string $reference Data reference * * @return string + * + * @throws StorageException in case there's an unexpected error + * @throws ReferenceStorageException in case the reference doesn't exist */ - public function get(string $reference); - - /** - * Put data in backend as $ref. If $ref is not defined a new reference is created. - * - * @param string $data Data to save - * @param string $reference Data reference. Optional. - * - * @return string Saved data reference - */ - public function put(string $data, string $reference = ""); - - /** - * Remove data from backend - * - * @param string $reference Data reference - * - * @return boolean True on success - */ - public function delete(string $reference); - - /** - * Get info about storage options - * - * @return array - * - * This method return an array with informations about storage options - * from which the form presented to the user is build. - * - * The returned array is: - * - * [ - * 'option1name' => [ ..info.. ], - * 'option2name' => [ ..info.. ], - * ... - * ] - * - * An empty array can be returned if backend doesn't have any options - * - * The info array for each option MUST be as follows: - * - * [ - * 'type', // define the field used in form, and the type of data. - * // one of 'checkbox', 'combobox', 'custom', 'datetime', - * // 'input', 'intcheckbox', 'password', 'radio', 'richtext' - * // 'select', 'select_raw', 'textarea' - * - * 'label', // Translatable label of the field - * 'value', // Current value - * 'help text', // Translatable description for the field - * extra data // Optional. Depends on 'type': - * // select: array [ value => label ] of choices - * // intcheckbox: value of input element - * // select_raw: prebuild html string of < option > tags - * ] - * - * See https://github.com/friendica/friendica/wiki/Quick-Template-Guide - */ - public function getOptions(); - - /** - * Validate and save options - * - * @param array $data Array [optionname => value] to be saved - * - * @return array Validation errors: [optionname => error message] - * - * Return array must be empty if no error. - */ - public function saveOptions(array $data); + public function get(string $reference): string; /** * The name of the backend @@ -118,5 +50,5 @@ interface IStorage * * @return string */ - public static function getName(); + public static function getName(): string; } diff --git a/src/Model/Storage/AbstractStorage.php b/src/Model/Storage/ReferenceStorageException.php similarity index 60% rename from src/Model/Storage/AbstractStorage.php rename to src/Model/Storage/ReferenceStorageException.php index 97152ddd0..c4de1f879 100644 --- a/src/Model/Storage/AbstractStorage.php +++ b/src/Model/Storage/ReferenceStorageException.php @@ -21,31 +21,11 @@ namespace Friendica\Model\Storage; -use Friendica\Core\L10n; -use Psr\Log\LoggerInterface; +use Exception; /** - * A general storage class which loads common dependencies and implements common methods + * Storage Exception in case of invalid references */ -abstract class AbstractStorage implements IStorage +class ReferenceStorageException extends Exception { - /** @var L10n */ - protected $l10n; - /** @var LoggerInterface */ - protected $logger; - - /** - * @param L10n $l10n - * @param LoggerInterface $logger - */ - public function __construct(L10n $l10n, LoggerInterface $logger) - { - $this->l10n = $l10n; - $this->logger = $logger; - } - - public function __toString() - { - return static::getName(); - } } diff --git a/src/Model/Storage/StorageException.php b/src/Model/Storage/StorageException.php index 4287f403a..34a09d57b 100644 --- a/src/Model/Storage/StorageException.php +++ b/src/Model/Storage/StorageException.php @@ -21,9 +21,11 @@ namespace Friendica\Model\Storage; +use Exception; + /** - * Storage Exception + * Storage Exception for unexpected failures */ -class StorageException extends \Exception +class StorageException extends Exception { } diff --git a/src/Model/Storage/SystemResource.php b/src/Model/Storage/SystemResource.php index c7699c2b7..39bc0a024 100644 --- a/src/Model/Storage/SystemResource.php +++ b/src/Model/Storage/SystemResource.php @@ -21,8 +21,6 @@ namespace Friendica\Model\Storage; -use \BadMethodCallException; - /** * System resource storage class * @@ -39,45 +37,22 @@ class SystemResource implements IStorage /** * @inheritDoc */ - public function get(string $filename) + public function get(string $reference): string { - $folder = dirname($filename); + $folder = dirname($reference); if (!in_array($folder, self::VALID_FOLDERS)) { - return ""; + throw new ReferenceStorageException(sprintf('System Resource is invalid for reference %s, no valid folder found', $reference)); } - if (!file_exists($filename)) { - return ""; + if (!file_exists($reference)) { + throw new StorageException(sprintf('System Resource is invalid for reference %s, the file doesn\'t exist', $reference)); } - return file_get_contents($filename); - } + $content = file_get_contents($reference); - /** - * @inheritDoc - */ - public function put(string $data, string $filename = '') - { - throw new BadMethodCallException(); - } + if ($content === false) { + throw new StorageException(sprintf('Cannot get content for reference %s', $reference)); + } - public function delete(string $filename) - { - throw new BadMethodCallException(); - } - - /** - * @inheritDoc - */ - public function getOptions() - { - return []; - } - - /** - * @inheritDoc - */ - public function saveOptions(array $data) - { - return []; + return $content; } /** @@ -91,7 +66,7 @@ class SystemResource implements IStorage /** * @inheritDoc */ - public static function getName() + public static function getName(): string { return self::NAME; } diff --git a/src/Util/HTTPSignature.php b/src/Util/HTTPSignature.php index 3980b7fba..c8ff99eba 100644 --- a/src/Util/HTTPSignature.php +++ b/src/Util/HTTPSignature.php @@ -28,6 +28,7 @@ use Friendica\DI; use Friendica\Model\APContact; use Friendica\Model\Contact; use Friendica\Model\User; +use Friendica\Network\CurlResult; /** * Implements HTTP Signatures per draft-cavage-http-signatures-07. @@ -408,7 +409,7 @@ class HTTPSignature * 'nobody' => only return the header * 'cookiejar' => path to cookie jar file * - * @return object CurlResult + * @return CurlResult|void CurlResult * @throws \Friendica\Network\HTTPException\InternalServerErrorException */ public static function fetchRaw($request, $uid = 0, $opts = ['accept_content' => 'application/activity+json, application/ld+json'])