From 5dcdf2322e843ffa2e5ab8bb3c287fc03d92c345 Mon Sep 17 00:00:00 2001 From: Philipp Date: Sun, 1 Aug 2021 13:06:19 +0200 Subject: [PATCH 01/19] 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 4eaa813ca6..5e83ca8568 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 0e758c7062..402619e675 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 dfe0b812ea..7760b4a530 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 0000000000..f1477a79cf --- /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 0a0f589587..8841487412 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 97152ddd06..c4de1f879e 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 4287f403a1..34a09d57bc 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 c7699c2b79..39bc0a0245 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 3980b7fba3..c8ff99eba0 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']) From 29c7552df57c31d81e8ce2458e2873f4d2ece35e Mon Sep 17 00:00:00 2001 From: Philipp Date: Sun, 1 Aug 2021 14:00:48 +0200 Subject: [PATCH 02/19] Refactor IStorage --- src/Console/Storage.php | 6 +- src/Core/StorageManager.php | 90 +++++++++++++++++++------- src/DI.php | 4 +- src/Model/Attach.php | 18 ++++-- src/Model/Photo.php | 25 +++++-- src/Model/Storage/ExternalResource.php | 4 +- src/Model/Storage/Filesystem.php | 2 + src/Module/Admin/Storage.php | 17 +++-- src/Worker/MoveStorage.php | 2 +- static/dependencies.config.php | 4 +- tests/src/Core/StorageManagerTest.php | 4 +- 11 files changed, 121 insertions(+), 55 deletions(-) diff --git a/src/Console/Storage.php b/src/Console/Storage.php index 93bbb26579..70e8e26302 100644 --- a/src/Console/Storage.php +++ b/src/Console/Storage.php @@ -131,10 +131,10 @@ HELP; throw new CommandArgsException('Invalid arguments'); } - $name = $this->args[1]; - $class = $this->storageManager->getByName($name); + $name = $this->args[1]; + $class = $this->storageManager->getSelectableStorageByName($name); - if ($class === '') { + if (is_null($class)) { $this->out($name . ' is not a registered backend.'); return -1; } diff --git a/src/Core/StorageManager.php b/src/Core/StorageManager.php index 64e53c10b9..03f89f312a 100644 --- a/src/Core/StorageManager.php +++ b/src/Core/StorageManager.php @@ -61,8 +61,6 @@ class StorageManager private $logger; /** @var L10n */ private $l10n; - /** @var IHTTPRequest */ - private $httpRequest; /** @var Storage\IStorage */ private $currentBackend; @@ -73,25 +71,24 @@ class StorageManager * @param LoggerInterface $logger * @param L10n $l10n */ - public function __construct(Database $dba, IConfig $config, LoggerInterface $logger, L10n $l10n, IHTTPRequest $httpRequest) + public function __construct(Database $dba, IConfig $config, LoggerInterface $logger, L10n $l10n) { $this->dba = $dba; $this->config = $config; $this->logger = $logger; $this->l10n = $l10n; - $this->httpRequest = $httpRequest; $this->backends = $config->get('storage', 'backends', self::DEFAULT_BACKENDS); $currentName = $this->config->get('storage', 'name', ''); // you can only use user backends as a "default" backend, so the second parameter is true - $this->currentBackend = $this->getByName($currentName, true); + $this->currentBackend = $this->getSelectableStorageByName($currentName); } /** * Return current storage backend class * - * @return Storage\IStorage|null + * @return Storage\ISelectableStorage|null */ public function getBackend() { @@ -99,16 +96,15 @@ class StorageManager } /** - * Return storage backend class by registered name + * Returns a selectable storage backend class by registered name * * @param string|null $name Backend name - * @param boolean $onlyUserBackend True, if just user specific instances should be returrned (e.g. not SystemResource) * - * @return Storage\IStorage|null null if no backend registered at $name + * @return Storage\ISelectableStorage|null null if no backend registered at $name * * @throws \Friendica\Network\HTTPException\InternalServerErrorException */ - public function getByName(string $name = null, $onlyUserBackend = false) + public function getSelectableStorageByName(string $name = null) { // @todo 2020.09 Remove this call after 2 releases $name = $this->checkLegacyBackend($name); @@ -116,22 +112,70 @@ class StorageManager // If there's no cached instance create a new instance if (!isset($this->backendInstances[$name])) { // If the current name isn't a valid backend (or the SystemResource instance) create it - if ($this->isValidBackend($name, $onlyUserBackend)) { + if ($this->isValidBackend($name, true)) { switch ($name) { // Try the filesystem backend case Storage\Filesystem::getName(): - $this->backendInstances[$name] = new Storage\Filesystem($this->config, $this->logger, $this->l10n); + $this->backendInstances[$name] = new Storage\Filesystem($this->config, $this->l10n); break; // try the database backend case Storage\Database::getName(): - $this->backendInstances[$name] = new Storage\Database($this->dba, $this->logger, $this->l10n); + $this->backendInstances[$name] = new Storage\Database($this->dba); + break; + default: + $data = [ + 'name' => $name, + 'storage' => null, + ]; + Hook::callAll('storage_instance', $data); + if (($data['storage'] ?? null) instanceof Storage\ISelectableStorage) { + $this->backendInstances[$data['name'] ?? $name] = $data['storage']; + } else { + return null; + } + break; + } + } else { + return null; + } + } + + return $this->backendInstances[$name]; + } + + /** + * Return storage backend class by registered name + * + * @param string|null $name Backend name + * + * @return Storage\IStorage|null null if no backend registered at $name + * + * @throws \Friendica\Network\HTTPException\InternalServerErrorException + */ + public function getByName(string $name = null) + { + // @todo 2020.09 Remove this call after 2 releases + $name = $this->checkLegacyBackend($name); + + // If there's no cached instance create a new instance + if (!isset($this->backendInstances[$name])) { + // If the current name isn't a valid backend (or the SystemResource instance) create it + if ($this->isValidBackend($name, false)) { + switch ($name) { + // Try the filesystem backend + case Storage\Filesystem::getName(): + $this->backendInstances[$name] = new Storage\Filesystem($this->config, $this->l10n); + break; + // try the database backend + case Storage\Database::getName(): + $this->backendInstances[$name] = new Storage\Database($this->dba); break; // at least, try if there's an addon for the backend case Storage\SystemResource::getName(): $this->backendInstances[$name] = new Storage\SystemResource(); break; case Storage\ExternalResource::getName(): - $this->backendInstances[$name] = new Storage\ExternalResource($this->httpRequest); + $this->backendInstances[$name] = new Storage\ExternalResource(); break; default: $data = [ @@ -190,18 +234,14 @@ class StorageManager /** * Set current storage backend class * - * @param string $name Backend class name + * @param Storage\ISelectableStorage $storage The storage class * * @return boolean True, if the set was successful */ - public function setBackend(string $name = null) + public function setBackend(Storage\ISelectableStorage $storage) { - if (!$this->isValidBackend($name, false)) { - return false; - } - - if ($this->config->set('storage', 'name', $name)) { - $this->currentBackend = $this->getByName($name, false); + if ($this->config->set('storage', 'name', $storage::getName())) { + $this->currentBackend = $storage; return true; } else { return false; @@ -277,7 +317,7 @@ class StorageManager * Copy existing data to destination storage and delete from source. * This method cannot move to legacy in-table `data` field. * - * @param Storage\IStorage $destination Destination storage class name + * @param Storage\ISelectableStorage $destination Destination storage class name * @param array $tables Tables to look in for resources. Optional, defaults to ['photo', 'attach'] * @param int $limit Limit of the process batch size, defaults to 5000 * @@ -285,7 +325,7 @@ class StorageManager * @throws Storage\StorageException * @throws Exception */ - public function move(Storage\IStorage $destination, array $tables = self::TABLES, int $limit = 5000) + public function move(Storage\ISelectableStorage $destination, array $tables = self::TABLES, int $limit = 5000) { if (!$this->isValidBackend($destination, true)) { throw new Storage\StorageException(sprintf("Can't move to storage backend '%s'", $destination::getName())); @@ -304,7 +344,7 @@ class StorageManager while ($resource = $this->dba->fetch($resources)) { $id = $resource['id']; $data = $resource['data']; - $source = $this->getByName($resource['backend-class']); + $source = $this->getSelectableStorageByName($resource['backend-class']); $sourceRef = $resource['backend-ref']; if (!empty($source)) { diff --git a/src/DI.php b/src/DI.php index 02620ea11e..9e96ddc015 100644 --- a/src/DI.php +++ b/src/DI.php @@ -387,11 +387,11 @@ abstract class DI } /** - * @return Model\Storage\IStorage + * @return Model\Storage\ISelectableStorage */ public static function storage() { - return self::$dice->create(Model\Storage\IStorage::class); + return self::$dice->create(Model\Storage\ISelectableStorage::class); } // diff --git a/src/Model/Attach.php b/src/Model/Attach.php index 6182727c97..d3ff5462ca 100644 --- a/src/Model/Attach.php +++ b/src/Model/Attach.php @@ -25,6 +25,7 @@ use Friendica\Core\System; use Friendica\Database\DBA; use Friendica\Database\DBStructure; use Friendica\DI; +use Friendica\Model\Storage\ReferenceStorageException; use Friendica\Object\Image; use Friendica\Util\DateTimeFormat; use Friendica\Util\Mimetype; @@ -173,7 +174,12 @@ class Attach return $i['data']; } else { $backendRef = $item['backend-ref']; - return $backendClass->get($backendRef); + try { + return $backendClass->get($backendRef); + } catch (ReferenceStorageException $referenceStorageException) { + DI::logger()->debug('No data found for item', ['item' => $item, 'exception' => $referenceStorageException]); + return ''; + } } } @@ -278,7 +284,7 @@ class Attach $items = self::selectToArray(['backend-class','backend-ref'], $conditions); foreach($items as $item) { - $backend_class = DI::storageManager()->getByName($item['backend-class'] ?? ''); + $backend_class = DI::storageManager()->getSelectableStorageByName($item['backend-class'] ?? ''); if (!empty($backend_class)) { $fields['backend-ref'] = $backend_class->put($img->asString(), $item['backend-ref'] ?? ''); } else { @@ -310,9 +316,13 @@ class Attach $items = self::selectToArray(['backend-class','backend-ref'], $conditions); foreach($items as $item) { - $backend_class = DI::storageManager()->getByName($item['backend-class'] ?? ''); + $backend_class = DI::storageManager()->getSelectableStorageByName($item['backend-class'] ?? ''); if (!empty($backend_class)) { - $backend_class->delete($item['backend-ref'] ?? ''); + try { + $backend_class->delete($item['backend-ref'] ?? ''); + } catch (ReferenceStorageException $referenceStorageException) { + DI::logger()->debug('Item doesn\'t exist.', ['conditions' => $conditions, 'exception' => $referenceStorageException]); + } } } diff --git a/src/Model/Photo.php b/src/Model/Photo.php index 26369a3540..f74c68da58 100644 --- a/src/Model/Photo.php +++ b/src/Model/Photo.php @@ -28,6 +28,8 @@ use Friendica\Database\DBA; use Friendica\Database\DBStructure; use Friendica\DI; use Friendica\Model\Storage\ExternalResource; +use Friendica\Model\Storage\ReferenceStorageException; +use Friendica\Model\Storage\StorageException; use Friendica\Model\Storage\SystemResource; use Friendica\Object\Image; use Friendica\Util\DateTimeFormat; @@ -183,9 +185,10 @@ class Photo * * @param array $photo Photo data. Needs at least 'id', 'type', 'backend-class', 'backend-ref' * - * @return \Friendica\Object\Image + * @return \Friendica\Object\Image|string * @throws \Friendica\Network\HTTPException\InternalServerErrorException * @throws \ImagickException + * @throws StorageException */ public static function getImageDataForPhoto(array $photo) { @@ -198,12 +201,17 @@ class Photo // legacy data storage in "data" column $i = self::selectFirst(['data'], ['id' => $photo['id']]); if ($i === false) { - return null; + return ''; } $data = $i['data']; } else { $backendRef = $photo['backend-ref'] ?? ''; - $data = $backendClass->get($backendRef); + try { + $data = $backendClass->get($backendRef); + } catch (ReferenceStorageException $referenceStorageException) { + DI::logger()->debug('No data found for photo', ['photo' => $photo, 'exception' => $referenceStorageException]); + return ''; + } } return $data; } @@ -339,7 +347,7 @@ class Photo if (DBA::isResult($existing_photo)) { $backend_ref = (string)$existing_photo["backend-ref"]; - $storage = DI::storageManager()->getByName($existing_photo["backend-class"] ?? ''); + $storage = DI::storageManager()->getSelectableStorageByName($existing_photo["backend-class"] ?? ''); } else { $storage = DI::storage(); } @@ -403,11 +411,14 @@ class Photo $photos = DBA::select('photo', ['id', 'backend-class', 'backend-ref'], $conditions); while ($photo = DBA::fetch($photos)) { - $backend_class = DI::storageManager()->getByName($photo['backend-class'] ?? ''); + $backend_class = DI::storageManager()->getSelectableStorageByName($photo['backend-class'] ?? ''); if (!empty($backend_class)) { - if ($backend_class->delete($photo["backend-ref"] ?? '')) { + try { + $backend_class->delete($item['backend-ref'] ?? ''); // Delete the photos after they had been deleted successfully DBA::delete("photo", ['id' => $photo['id']]); + } catch (ReferenceStorageException $referenceStorageException) { + DI::logger()->debug('phot doesn\'t exist.', ['conditions' => $conditions, 'exception' => $referenceStorageException]); } } } @@ -437,7 +448,7 @@ class Photo $photos = self::selectToArray(['backend-class', 'backend-ref'], $conditions); foreach($photos as $photo) { - $backend_class = DI::storageManager()->getByName($photo['backend-class'] ?? ''); + $backend_class = DI::storageManager()->getSelectableStorageByName($photo['backend-class'] ?? ''); if (!empty($backend_class)) { $fields["backend-ref"] = $backend_class->put($img->asString(), $photo['backend-ref']); } else { diff --git a/src/Model/Storage/ExternalResource.php b/src/Model/Storage/ExternalResource.php index 402619e675..918bcf8ac3 100644 --- a/src/Model/Storage/ExternalResource.php +++ b/src/Model/Storage/ExternalResource.php @@ -52,12 +52,12 @@ class ExternalResource implements IStorage 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); + throw new ReferenceStorageException(sprintf('External resource failed to get %s', $reference), $exception->getCode(), $exception); } if ($fetchResult->isSuccess()) { return $fetchResult->getBody(); } else { - throw new StorageException(sprintf('External resource failed to get %s', $reference), $fetchResult->getReturnCode(), new Exception($fetchResult->getBody())); + throw new ReferenceStorageException(sprintf('External resource failed to get %s', $reference), $fetchResult->getReturnCode(), new Exception($fetchResult->getBody())); } } diff --git a/src/Model/Storage/Filesystem.php b/src/Model/Storage/Filesystem.php index 7760b4a530..fe5230049f 100644 --- a/src/Model/Storage/Filesystem.php +++ b/src/Model/Storage/Filesystem.php @@ -131,6 +131,8 @@ class Filesystem implements ISelectableStorage if ($result === false) { throw new StorageException(sprintf('Filesystem storage failed to get data to "%s". Check your write permissions', $file)); } + + return $result; } /** diff --git a/src/Module/Admin/Storage.php b/src/Module/Admin/Storage.php index 1010707124..374d692bf7 100644 --- a/src/Module/Admin/Storage.php +++ b/src/Module/Admin/Storage.php @@ -23,7 +23,7 @@ namespace Friendica\Module\Admin; use Friendica\Core\Renderer; use Friendica\DI; -use Friendica\Model\Storage\IStorage; +use Friendica\Model\Storage\ISelectableStorage; use Friendica\Module\BaseAdmin; use Friendica\Util\Strings; @@ -37,8 +37,8 @@ class Storage extends BaseAdmin $storagebackend = Strings::escapeTags(trim($parameters['name'] ?? '')); - /** @var IStorage $newstorage */ - $newstorage = DI::storageManager()->getByName($storagebackend); + /** @var ISelectableStorage $newstorage */ + $newstorage = DI::storageManager()->getSelectableStorageByName($storagebackend); // save storage backend form $storage_opts = $newstorage->getOptions(); @@ -68,7 +68,10 @@ class Storage extends BaseAdmin } if (!empty($_POST['submit_save_set'])) { - if (empty($storagebackend) || !DI::storageManager()->setBackend($storagebackend)) { + /** @var ISelectableStorage $newstorage */ + $newstorage = DI::storageManager()->getSelectableStorageByName($storagebackend); + + if (!DI::storageManager()->setBackend($newstorage)) { notice(DI::l10n()->t('Invalid storage backend setting value.')); } } @@ -89,7 +92,7 @@ class Storage extends BaseAdmin $storage_form_prefix = preg_replace('|[^a-zA-Z0-9]|', '', $name); $storage_form = []; - foreach (DI::storageManager()->getByName($name)->getOptions() as $option => $info) { + foreach (DI::storageManager()->getSelectableStorageByName($name)->getOptions() as $option => $info) { $type = $info[0]; // Backward compatibilty with yesno field description if ($type == 'yesno') { @@ -108,7 +111,7 @@ class Storage extends BaseAdmin 'name' => $name, 'prefix' => $storage_form_prefix, 'form' => $storage_form, - 'active' => $current_storage_backend instanceof IStorage && $name === $current_storage_backend::getName(), + 'active' => $current_storage_backend instanceof ISelectableStorage && $name === $current_storage_backend::getName(), ]; } @@ -124,7 +127,7 @@ class Storage extends BaseAdmin '$noconfig' => DI::l10n()->t('This backend doesn\'t have custom settings'), '$baseurl' => DI::baseUrl()->get(true), '$form_security_token' => self::getFormSecurityToken("admin_storage"), - '$storagebackend' => $current_storage_backend instanceof IStorage ? $current_storage_backend::getName() : DI::l10n()->t('Database (legacy)'), + '$storagebackend' => $current_storage_backend instanceof ISelectableStorage ? $current_storage_backend::getName() : DI::l10n()->t('Database (legacy)'), '$availablestorageforms' => $available_storage_forms, ]); } diff --git a/src/Worker/MoveStorage.php b/src/Worker/MoveStorage.php index ac70c63545..34ecb5c147 100644 --- a/src/Worker/MoveStorage.php +++ b/src/Worker/MoveStorage.php @@ -34,7 +34,7 @@ class MoveStorage public static function execute() { $current = DI::storage(); - $moved = DI::storageManager()->move($current); + $moved = DI::storageManager()->move($current); if ($moved) { Worker::add(PRIORITY_LOW, 'MoveStorage'); diff --git a/static/dependencies.config.php b/static/dependencies.config.php index 90fa13684c..ea57efca5a 100644 --- a/static/dependencies.config.php +++ b/static/dependencies.config.php @@ -44,7 +44,7 @@ use Friendica\Core\Session\ISession; use Friendica\Core\StorageManager; use Friendica\Database\Database; use Friendica\Factory; -use Friendica\Model\Storage\IStorage; +use Friendica\Model\Storage\ISelectableStorage; use Friendica\Model\User\Cookie; use Friendica\Network; use Friendica\Util; @@ -213,7 +213,7 @@ return [ $_SERVER, $_COOKIE ], ], - IStorage::class => [ + ISelectableStorage::class => [ 'instanceOf' => StorageManager::class, 'call' => [ ['getBackend', [], Dice::CHAIN_CALL], diff --git a/tests/src/Core/StorageManagerTest.php b/tests/src/Core/StorageManagerTest.php index deb9c4b11f..537fd841e4 100644 --- a/tests/src/Core/StorageManagerTest.php +++ b/tests/src/Core/StorageManagerTest.php @@ -309,7 +309,7 @@ class StorageManagerTest extends DatabaseTest $this->loadFixture(__DIR__ . '/../../datasets/storage/database.fixture.php', $this->dba); $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->l10n, $this->httpRequest); - $storage = $storageManager->getByName($name); + $storage = $storageManager->getSelectableStorageByName($name); $storageManager->move($storage); $photos = $this->dba->select('photo', ['backend-ref', 'backend-class', 'id', 'data']); @@ -334,7 +334,7 @@ class StorageManagerTest extends DatabaseTest $this->expectException(StorageException::class); $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->l10n, $this->httpRequest); - $storage = $storageManager->getByName(Storage\SystemResource::getName()); + $storage = $storageManager->getSelectableStorageByName(Storage\SystemResource::getName()); $storageManager->move($storage); } } From 90c99520bb2dd4817799da81e39f0043eefb171c Mon Sep 17 00:00:00 2001 From: Philipp Date: Sun, 1 Aug 2021 14:31:57 +0200 Subject: [PATCH 03/19] Fix Storage Exceptions --- src/Model/Storage/Database.php | 12 ++++++-- tests/Util/SampleStorageBackend.php | 17 +++++------ tests/src/Core/StorageManagerTest.php | 30 +++++++++++-------- .../src/Model/Storage/DatabaseStorageTest.php | 5 +--- .../Model/Storage/FilesystemStorageTest.php | 3 +- tests/src/Model/Storage/StorageTest.php | 14 +++++---- 6 files changed, 46 insertions(+), 35 deletions(-) diff --git a/src/Model/Storage/Database.php b/src/Model/Storage/Database.php index 5e83ca8568..9edfe21708 100644 --- a/src/Model/Storage/Database.php +++ b/src/Model/Storage/Database.php @@ -57,7 +57,11 @@ class Database implements ISelectableStorage return $result['data']; } catch (Exception $exception) { - throw new StorageException(sprintf('Database storage failed to get %s', $reference), $exception->getCode(), $exception); + if ($exception instanceof ReferenceStorageException) { + throw $exception; + } else { + throw new StorageException(sprintf('Database storage failed to get %s', $reference), $exception->getCode(), $exception); + } } } @@ -101,7 +105,11 @@ class Database implements ISelectableStorage 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); + if ($exception instanceof ReferenceStorageException) { + throw $exception; + } else { + throw new StorageException(sprintf('Database storage failed to delete %s', $reference), $exception->getCode(), $exception); + } } } diff --git a/tests/Util/SampleStorageBackend.php b/tests/Util/SampleStorageBackend.php index 3d5789b0de..1ebd64f6ee 100644 --- a/tests/Util/SampleStorageBackend.php +++ b/tests/Util/SampleStorageBackend.php @@ -22,14 +22,14 @@ namespace Friendica\Test\Util; use Friendica\Core\Hook; -use Friendica\Model\Storage\IStorage; +use Friendica\Model\Storage\ISelectableStorage; use Friendica\Core\L10n; /** * A backend storage example class */ -class SampleStorageBackend implements IStorage +class SampleStorageBackend implements ISelectableStorage { const NAME = 'Sample Storage'; @@ -62,14 +62,14 @@ class SampleStorageBackend implements IStorage $this->l10n = $l10n; } - public function get(string $reference) + public function get(string $reference): string { // we return always the same image data. Which file we load is defined by // a config key - return $this->data[$reference] ?? null; + return $this->data[$reference] ?? ''; } - public function put(string $data, string $reference = '') + public function put(string $data, string $reference = ''): string { if ($reference === '') { $reference = 'sample'; @@ -89,12 +89,12 @@ class SampleStorageBackend implements IStorage return true; } - public function getOptions() + public function getOptions(): array { return $this->options; } - public function saveOptions(array $data) + public function saveOptions(array $data): array { $this->options = $data; @@ -107,7 +107,7 @@ class SampleStorageBackend implements IStorage return self::NAME; } - public static function getName() + public static function getName(): string { return self::NAME; } @@ -120,4 +120,3 @@ class SampleStorageBackend implements IStorage Hook::register('storage_instance', __DIR__ . '/SampleStorageBackendInstance.php', 'create_instance'); } } - diff --git a/tests/src/Core/StorageManagerTest.php b/tests/src/Core/StorageManagerTest.php index 537fd841e4..f01640dce4 100644 --- a/tests/src/Core/StorageManagerTest.php +++ b/tests/src/Core/StorageManagerTest.php @@ -177,7 +177,11 @@ class StorageManagerTest extends DatabaseTest { $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->l10n, $this->httpRequest); - $storage = $storageManager->getByName($name, $userBackend); + if ($userBackend) { + $storage = $storageManager->getSelectableStorageByName($name); + } else { + $storage = $storageManager->getByName($name); + } if (!empty($assert)) { self::assertInstanceOf(Storage\IStorage::class, $storage); @@ -195,7 +199,7 @@ class StorageManagerTest extends DatabaseTest */ public function testIsValidBackend($name, $assert, $assertName, $userBackend) { - $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->l10n, $this->httpRequest); + $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->l10n); // true in every of the backends self::assertEquals(!empty($assertName), $storageManager->isValidBackend($name)); @@ -209,7 +213,7 @@ class StorageManagerTest extends DatabaseTest */ public function testListBackends() { - $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->l10n, $this->httpRequest); + $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->l10n); self::assertEquals(StorageManager::DEFAULT_BACKENDS, $storageManager->listBackends()); } @@ -221,12 +225,13 @@ class StorageManagerTest extends DatabaseTest */ public function testGetBackend($name, $assert, $assertName, $userBackend) { - $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->l10n, $this->httpRequest); + $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->l10n); self::assertNull($storageManager->getBackend()); if ($userBackend) { - $storageManager->setBackend($name); + $selBackend = $storageManager->getSelectableStorageByName($name); + $storageManager->setBackend($selBackend); self::assertInstanceOf($assert, $storageManager->getBackend()); } @@ -242,7 +247,7 @@ class StorageManagerTest extends DatabaseTest { $this->config->set('storage', 'name', $name); - $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->l10n, $this->httpRequest); + $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->l10n); if ($userBackend) { self::assertInstanceOf($assert, $storageManager->getBackend()); @@ -267,7 +272,7 @@ class StorageManagerTest extends DatabaseTest ->addRule(ISession::class, ['instanceOf' => Session\Memory::class, 'shared' => true, 'call' => null]); DI::init($dice); - $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->l10n, $this->httpRequest); + $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->l10n); self::assertTrue($storageManager->register(SampleStorageBackend::class)); @@ -282,7 +287,7 @@ class StorageManagerTest extends DatabaseTest SampleStorageBackend::registerHook(); Hook::loadHooks(); - self::assertTrue($storageManager->setBackend(SampleStorageBackend::NAME)); + self::assertTrue($storageManager->setBackend( $storageManager->getSelectableStorageByName(SampleStorageBackend::NAME))); self::assertEquals(SampleStorageBackend::NAME, $this->config->get('storage', 'name')); self::assertInstanceOf(SampleStorageBackend::class, $storageManager->getBackend()); @@ -308,7 +313,7 @@ class StorageManagerTest extends DatabaseTest $this->loadFixture(__DIR__ . '/../../datasets/storage/database.fixture.php', $this->dba); - $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->l10n, $this->httpRequest); + $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->l10n); $storage = $storageManager->getSelectableStorageByName($name); $storageManager->move($storage); @@ -328,12 +333,11 @@ class StorageManagerTest extends DatabaseTest /** * Test moving data to a WRONG storage */ - public function testMoveStorageWrong() + public function testWrongSelectableStorage() { - $this->expectExceptionMessage("Can't move to storage backend 'SystemResource'"); - $this->expectException(StorageException::class); + $this->expectException(\TypeError::class); - $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->l10n, $this->httpRequest); + $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->l10n); $storage = $storageManager->getSelectableStorageByName(Storage\SystemResource::getName()); $storageManager->move($storage); } diff --git a/tests/src/Model/Storage/DatabaseStorageTest.php b/tests/src/Model/Storage/DatabaseStorageTest.php index 21d6e18652..5e61ea13a4 100644 --- a/tests/src/Model/Storage/DatabaseStorageTest.php +++ b/tests/src/Model/Storage/DatabaseStorageTest.php @@ -62,10 +62,7 @@ class DatabaseStorageTest extends StorageTest $dba = new StaticDatabase($configCache, $profiler, $logger); - /** @var MockInterface|L10n $l10n */ - $l10n = \Mockery::mock(L10n::class)->makePartial(); - - return new Database($dba, $logger, $l10n); + return new Database($dba); } protected function assertOption(IStorage $storage) diff --git a/tests/src/Model/Storage/FilesystemStorageTest.php b/tests/src/Model/Storage/FilesystemStorageTest.php index ebe29f0c4c..3319ea3fa4 100644 --- a/tests/src/Model/Storage/FilesystemStorageTest.php +++ b/tests/src/Model/Storage/FilesystemStorageTest.php @@ -50,7 +50,6 @@ class FilesystemStorageTest extends StorageTest protected function getInstance() { - $logger = new NullLogger(); $profiler = \Mockery::mock(Profiler::class); $profiler->shouldReceive('startRecording'); $profiler->shouldReceive('stopRecording'); @@ -63,7 +62,7 @@ class FilesystemStorageTest extends StorageTest ->with('storage', 'filesystem_path', Filesystem::DEFAULT_BASE_FOLDER) ->andReturn($this->root->getChild('storage')->url()); - return new Filesystem($this->config, $logger, $l10n); + return new Filesystem($this->config, $l10n); } protected function assertOption(IStorage $storage) diff --git a/tests/src/Model/Storage/StorageTest.php b/tests/src/Model/Storage/StorageTest.php index d978f013d9..eadddcf02d 100644 --- a/tests/src/Model/Storage/StorageTest.php +++ b/tests/src/Model/Storage/StorageTest.php @@ -22,6 +22,8 @@ namespace Friendica\Test\src\Model\Storage; use Friendica\Model\Storage\IStorage; +use Friendica\Model\Storage\ReferenceStorageException; +use Friendica\Model\Storage\StorageException; use Friendica\Test\MockedTest; abstract class StorageTest extends MockedTest @@ -62,7 +64,7 @@ abstract class StorageTest extends MockedTest self::assertEquals('data12345', $instance->get($ref)); - self::assertTrue($instance->delete($ref)); + $instance->delete($ref); } /** @@ -70,10 +72,11 @@ abstract class StorageTest extends MockedTest */ public function testInvalidDelete() { + self::expectException(ReferenceStorageException::class); + $instance = $this->getInstance(); - // Even deleting not existing references should return "true" - self::assertTrue($instance->delete(-1234456)); + $instance->delete(-1234456); } /** @@ -81,10 +84,11 @@ abstract class StorageTest extends MockedTest */ public function testInvalidGet() { + self::expectException(ReferenceStorageException::class); + $instance = $this->getInstance(); - // Invalid references return an empty string - self::assertEmpty($instance->get(-123456)); + $instance->get(-123456); } /** From 470ba8b61bbaf4254dab257f2af615651d6c4895 Mon Sep 17 00:00:00 2001 From: Philipp Date: Sat, 7 Aug 2021 22:07:47 +0200 Subject: [PATCH 04/19] Fixup Database behaviour --- src/Model/Storage/Database.php | 2 +- tests/src/Model/Storage/DatabaseStorageTest.php | 6 ++---- tests/src/Model/Storage/FilesystemStorageTest.php | 5 ++--- tests/src/Model/Storage/StorageTest.php | 6 +++--- 4 files changed, 8 insertions(+), 11 deletions(-) diff --git a/src/Model/Storage/Database.php b/src/Model/Storage/Database.php index 9edfe21708..a77e268117 100644 --- a/src/Model/Storage/Database.php +++ b/src/Model/Storage/Database.php @@ -101,7 +101,7 @@ class Database implements ISelectableStorage public function delete(string $reference) { try { - if (!$this->dba->delete('storage', ['id' => $reference])) { + if (!$this->dba->delete('storage', ['id' => $reference]) || $this->dba->affectedRows() === 0) { throw new ReferenceStorageException(sprintf('Database storage failed to delete %s', $reference)); } } catch (Exception $exception) { diff --git a/tests/src/Model/Storage/DatabaseStorageTest.php b/tests/src/Model/Storage/DatabaseStorageTest.php index 5e61ea13a4..62c8e7f116 100644 --- a/tests/src/Model/Storage/DatabaseStorageTest.php +++ b/tests/src/Model/Storage/DatabaseStorageTest.php @@ -21,16 +21,14 @@ namespace Friendica\Test\src\Model\Storage; -use Friendica\Core\L10n; use Friendica\Factory\ConfigFactory; use Friendica\Model\Storage\Database; -use Friendica\Model\Storage\IStorage; +use Friendica\Model\Storage\ISelectableStorage; use Friendica\Test\DatabaseTestTrait; use Friendica\Test\Util\Database\StaticDatabase; use Friendica\Test\Util\VFSTrait; use Friendica\Util\ConfigFileLoader; use Friendica\Util\Profiler; -use Mockery\MockInterface; use Psr\Log\NullLogger; class DatabaseStorageTest extends StorageTest @@ -65,7 +63,7 @@ class DatabaseStorageTest extends StorageTest return new Database($dba); } - protected function assertOption(IStorage $storage) + protected function assertOption(ISelectableStorage $storage) { self::assertEmpty($storage->getOptions()); } diff --git a/tests/src/Model/Storage/FilesystemStorageTest.php b/tests/src/Model/Storage/FilesystemStorageTest.php index 3319ea3fa4..a0f267d183 100644 --- a/tests/src/Model/Storage/FilesystemStorageTest.php +++ b/tests/src/Model/Storage/FilesystemStorageTest.php @@ -24,13 +24,12 @@ namespace Friendica\Test\src\Model\Storage; use Friendica\Core\Config\IConfig; use Friendica\Core\L10n; use Friendica\Model\Storage\Filesystem; -use Friendica\Model\Storage\IStorage; +use Friendica\Model\Storage\ISelectableStorage; use Friendica\Model\Storage\StorageException; use Friendica\Test\Util\VFSTrait; use Friendica\Util\Profiler; use Mockery\MockInterface; use org\bovigo\vfs\vfsStream; -use Psr\Log\NullLogger; class FilesystemStorageTest extends StorageTest { @@ -65,7 +64,7 @@ class FilesystemStorageTest extends StorageTest return new Filesystem($this->config, $l10n); } - protected function assertOption(IStorage $storage) + protected function assertOption(ISelectableStorage $storage) { self::assertEquals([ 'storagepath' => [ diff --git a/tests/src/Model/Storage/StorageTest.php b/tests/src/Model/Storage/StorageTest.php index eadddcf02d..4001bfc68b 100644 --- a/tests/src/Model/Storage/StorageTest.php +++ b/tests/src/Model/Storage/StorageTest.php @@ -21,17 +21,17 @@ namespace Friendica\Test\src\Model\Storage; +use Friendica\Model\Storage\ISelectableStorage; use Friendica\Model\Storage\IStorage; use Friendica\Model\Storage\ReferenceStorageException; -use Friendica\Model\Storage\StorageException; use Friendica\Test\MockedTest; abstract class StorageTest extends MockedTest { - /** @return IStorage */ + /** @return ISelectableStorage */ abstract protected function getInstance(); - abstract protected function assertOption(IStorage $storage); + abstract protected function assertOption(ISelectableStorage $storage); /** * Test if the instance is "really" implementing the interface From 813db7956f360120588e7d0bf412a85b6e438324 Mon Sep 17 00:00:00 2001 From: Philipp Date: Sat, 7 Aug 2021 22:34:09 +0200 Subject: [PATCH 05/19] Update AddonStorageBackend.md --- doc/AddonStorageBackend.md | 57 ++++++++++++++++++++++++++++++-------- 1 file changed, 46 insertions(+), 11 deletions(-) diff --git a/doc/AddonStorageBackend.md b/doc/AddonStorageBackend.md index 950a2ad88d..72e1467793 100644 --- a/doc/AddonStorageBackend.md +++ b/doc/AddonStorageBackend.md @@ -10,12 +10,12 @@ A storage backend is implemented as a class, and the plugin register the class t The class must live in `Friendica\Addon\youraddonname` namespace, where `youraddonname` the folder name of your addon. -The class must implement `Friendica\Model\Storage\IStorage` interface. All method in the interface must be implemented: +The class must implement `Friendica\Model\Storage\ISelectableStorage` interface. All method in the interface must be implemented: -namespace Friendica\Model\Storage; +namespace Friendica\Model\ISelectableStorage; ```php -interface IStorage +interface ISelectableStorage { public function get(string $reference); public function put(string $data, string $reference = ''); @@ -79,7 +79,7 @@ Each label should be translatable ]; -See doxygen documentation of `IStorage` interface for details about each method. +See doxygen documentation of `ISelectableStorage` interface for details about each method. ## Register a storage backend class @@ -106,7 +106,7 @@ Add a new test class which's naming convention is `StorageClassTest`, which exte Override the two necessary instances: ```php -use Friendica\Model\Storage\IStorage; +use Friendica\Model\Storage\ISelectableStorage; abstract class StorageTest { @@ -114,13 +114,48 @@ abstract class StorageTest abstract protected function getInstance(); // Assertion for the option array you return for your new StorageClass - abstract protected function assertOption(IStorage $storage); + abstract protected function assertOption(ISelectableStorage $storage); } ``` +## Exception handling + +There are two intended types of exceptions for storages + +### `StorageException` + +This is the common exception in case unexpected errors happen using the storage backend. +If there's a predecessor to this exception (e.g. you caught an exception and are throwing this execption), you should add the predecessor for transparency reasons. + +Example: + +```php +use Friendica\Model\Storage\ISelectableStorage; + +class ExampleStorage implements ISelectableStorage +{ + public function get(string $reference) : string + { + try { + throw new Exception('a real bad exception'); + } catch (Exception $exception) { + throw new \Friendica\Model\Storage\StorageException(sprintf('The Example Storage throws an exception for reference %s', $reference), 500, $exception); + } + } +} +``` + +### `ReferenceStorageExecption` + +This storage exception should be used in case the caller tries to use an invalid references. +This could happen in case the caller tries to delete or update an unknown reference. +The implementation of the storage backend must not ignore invalid references. + +Avoid throwing the common `StorageExecption` instead of the `ReferenceStorageException` at this particular situation! + ## Example -Here an hypotetical addon which register an unusefull storage backend. +Here an hypotetical addon which register a useless storage backend. Let's call it `samplestorage`. This backend will discard all data we try to save and will return always the same image when we ask for some data. @@ -133,12 +168,12 @@ The file will be `addon/samplestorage/SampleStorageBackend.php`: assertEquals([ 'filename' => [ From 57438afbb398940d2a63afd2ce3730d1e127b10c Mon Sep 17 00:00:00 2001 From: Philipp Date: Tue, 10 Aug 2021 21:30:40 +0200 Subject: [PATCH 06/19] - Moved the description for the specific storage exception first - Introduced exceptions for try to get invalid storage - ReferenceStorageException now extends StorageException --- doc/AddonStorageBackend.md | 16 +++--- src/Core/StorageManager.php | 50 +++++++++++-------- src/Model/Storage/ISelectableStorage.php | 3 ++ .../Storage/ReferenceStorageException.php | 4 +- 4 files changed, 42 insertions(+), 31 deletions(-) diff --git a/doc/AddonStorageBackend.md b/doc/AddonStorageBackend.md index 72e1467793..b9b7182471 100644 --- a/doc/AddonStorageBackend.md +++ b/doc/AddonStorageBackend.md @@ -122,6 +122,14 @@ abstract class StorageTest There are two intended types of exceptions for storages +### `ReferenceStorageExecption` + +This storage exception should be used in case the caller tries to use an invalid references. +This could happen in case the caller tries to delete or update an unknown reference. +The implementation of the storage backend must not ignore invalid references. + +Avoid throwing the common `StorageExecption` instead of the `ReferenceStorageException` at this particular situation! + ### `StorageException` This is the common exception in case unexpected errors happen using the storage backend. @@ -145,14 +153,6 @@ class ExampleStorage implements ISelectableStorage } ``` -### `ReferenceStorageExecption` - -This storage exception should be used in case the caller tries to use an invalid references. -This could happen in case the caller tries to delete or update an unknown reference. -The implementation of the storage backend must not ignore invalid references. - -Avoid throwing the common `StorageExecption` instead of the `ReferenceStorageException` at this particular situation! - ## Example Here an hypotetical addon which register a useless storage backend. diff --git a/src/Core/StorageManager.php b/src/Core/StorageManager.php index 03f89f312a..4603b0fded 100644 --- a/src/Core/StorageManager.php +++ b/src/Core/StorageManager.php @@ -25,7 +25,7 @@ use Exception; use Friendica\Core\Config\IConfig; use Friendica\Database\Database; use Friendica\Model\Storage; -use Friendica\Network\IHTTPRequest; +use Friendica\Network\HTTPException\InternalServerErrorException; use Psr\Log\LoggerInterface; @@ -98,11 +98,12 @@ class StorageManager /** * Returns a selectable storage backend class by registered name * - * @param string|null $name Backend name + * @param string $name Backend name * - * @return Storage\ISelectableStorage|null null if no backend registered at $name + * @return Storage\ISelectableStorage * - * @throws \Friendica\Network\HTTPException\InternalServerErrorException + * @throws Storage\ReferenceStorageException in case there's no backend class for the name + * @throws Storage\StorageException in case of an unexpected failure during the hook call */ public function getSelectableStorageByName(string $name = null) { @@ -127,16 +128,20 @@ class StorageManager 'name' => $name, 'storage' => null, ]; - Hook::callAll('storage_instance', $data); - if (($data['storage'] ?? null) instanceof Storage\ISelectableStorage) { - $this->backendInstances[$data['name'] ?? $name] = $data['storage']; - } else { - return null; + try { + Hook::callAll('storage_instance', $data); + if (($data['storage'] ?? null) instanceof Storage\ISelectableStorage) { + $this->backendInstances[$data['name'] ?? $name] = $data['storage']; + } else { + throw new Storage\ReferenceStorageException(sprintf('Backend %s was not found', $name)); + } + } catch (InternalServerErrorException $exception) { + throw new Storage\StorageException(sprintf('Failed calling hook::storage_instance for backend %s', $name), $exception); } break; } } else { - return null; + throw new Storage\ReferenceStorageException(sprintf('Backend %s is not valid', $name)); } } @@ -146,13 +151,14 @@ class StorageManager /** * Return storage backend class by registered name * - * @param string|null $name Backend name + * @param string $name Backend name * - * @return Storage\IStorage|null null if no backend registered at $name + * @return Storage\IStorage * - * @throws \Friendica\Network\HTTPException\InternalServerErrorException + * @throws Storage\ReferenceStorageException in case there's no backend class for the name + * @throws Storage\StorageException in case of an unexpected failure during the hook call */ - public function getByName(string $name = null) + public function getByName(string $name) { // @todo 2020.09 Remove this call after 2 releases $name = $this->checkLegacyBackend($name); @@ -182,16 +188,20 @@ class StorageManager 'name' => $name, 'storage' => null, ]; - Hook::callAll('storage_instance', $data); - if (($data['storage'] ?? null) instanceof Storage\IStorage) { - $this->backendInstances[$data['name'] ?? $name] = $data['storage']; - } else { - return null; + try { + Hook::callAll('storage_instance', $data); + if (($data['storage'] ?? null) instanceof Storage\IStorage) { + $this->backendInstances[$data['name'] ?? $name] = $data['storage']; + } else { + throw new Storage\ReferenceStorageException(sprintf('Backend %s was not found', $name)); + } + } catch (InternalServerErrorException $exception) { + throw new Storage\StorageException(sprintf('Failed calling hook::storage_instance for backend %s', $name), $exception); } break; } } else { - return null; + throw new Storage\ReferenceStorageException(sprintf('Backend %s is not valid', $name)); } } diff --git a/src/Model/Storage/ISelectableStorage.php b/src/Model/Storage/ISelectableStorage.php index f1477a79cf..d3b7547774 100644 --- a/src/Model/Storage/ISelectableStorage.php +++ b/src/Model/Storage/ISelectableStorage.php @@ -23,6 +23,9 @@ namespace Friendica\Model\Storage; /** * Interface for selectable storage backends + * + * Used for storages with CRUD functionality, mainly used for user data (e.g. photos, attachements). + * There's only one active, selectable storage possible and can be selected by the current administrator */ interface ISelectableStorage extends IStorage { diff --git a/src/Model/Storage/ReferenceStorageException.php b/src/Model/Storage/ReferenceStorageException.php index c4de1f879e..fcfd3ab59d 100644 --- a/src/Model/Storage/ReferenceStorageException.php +++ b/src/Model/Storage/ReferenceStorageException.php @@ -21,11 +21,9 @@ namespace Friendica\Model\Storage; -use Exception; - /** * Storage Exception in case of invalid references */ -class ReferenceStorageException extends Exception +class ReferenceStorageException extends StorageException { } From 51ebb1541a62c0fd691d995326d61775cc7d61d3 Mon Sep 17 00:00:00 2001 From: Philipp Date: Tue, 10 Aug 2021 21:34:16 +0200 Subject: [PATCH 07/19] - Revert HTTPSignature change --- src/Util/HTTPSignature.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Util/HTTPSignature.php b/src/Util/HTTPSignature.php index c8ff99eba0..e2de810a60 100644 --- a/src/Util/HTTPSignature.php +++ b/src/Util/HTTPSignature.php @@ -409,7 +409,7 @@ class HTTPSignature * 'nobody' => only return the header * 'cookiejar' => path to cookie jar file * - * @return CurlResult|void CurlResult + * @return CurlResult CurlResult * @throws \Friendica\Network\HTTPException\InternalServerErrorException */ public static function fetchRaw($request, $uid = 0, $opts = ['accept_content' => 'application/activity+json, application/ld+json']) From eb035771f118e1f289f2ebbbb8a474aa2765c81e Mon Sep 17 00:00:00 2001 From: Philipp Date: Tue, 10 Aug 2021 21:39:29 +0200 Subject: [PATCH 08/19] Revert Photo::getImageDataForPhoto return-type change --- src/Model/Photo.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Model/Photo.php b/src/Model/Photo.php index f74c68da58..a38743d9d6 100644 --- a/src/Model/Photo.php +++ b/src/Model/Photo.php @@ -185,7 +185,7 @@ class Photo * * @param array $photo Photo data. Needs at least 'id', 'type', 'backend-class', 'backend-ref' * - * @return \Friendica\Object\Image|string + * @return \Friendica\Object\Image * @throws \Friendica\Network\HTTPException\InternalServerErrorException * @throws \ImagickException * @throws StorageException @@ -201,7 +201,7 @@ class Photo // legacy data storage in "data" column $i = self::selectFirst(['data'], ['id' => $photo['id']]); if ($i === false) { - return ''; + return null; } $data = $i['data']; } else { @@ -210,7 +210,7 @@ class Photo $data = $backendClass->get($backendRef); } catch (ReferenceStorageException $referenceStorageException) { DI::logger()->debug('No data found for photo', ['photo' => $photo, 'exception' => $referenceStorageException]); - return ''; + return null; } } return $data; From d0536ebea70465e275404541dae1627961d6c3cc Mon Sep 17 00:00:00 2001 From: Philipp Date: Tue, 10 Aug 2021 22:07:52 +0200 Subject: [PATCH 09/19] Rename ISelectableStorage to IWritableStorage --- doc/AddonStorageBackend.md | 25 +++++++++--------- src/Console/Storage.php | 2 +- src/Core/StorageManager.php | 26 +++++++++---------- src/DI.php | 4 +-- src/Model/Attach.php | 4 +-- src/Model/Photo.php | 6 ++--- src/Model/Storage/Database.php | 2 +- src/Model/Storage/Filesystem.php | 2 +- ...ctableStorage.php => IWritableStorage.php} | 6 ++--- src/Module/Admin/Storage.php | 16 ++++++------ static/dependencies.config.php | 4 +-- tests/Util/SampleStorageBackend.php | 4 +-- tests/src/Core/StorageManagerTest.php | 12 ++++----- .../src/Model/Storage/DatabaseStorageTest.php | 4 +-- .../Model/Storage/FilesystemStorageTest.php | 4 +-- tests/src/Model/Storage/StorageTest.php | 6 ++--- 16 files changed, 64 insertions(+), 63 deletions(-) rename src/Model/Storage/{ISelectableStorage.php => IWritableStorage.php} (93%) diff --git a/doc/AddonStorageBackend.md b/doc/AddonStorageBackend.md index b9b7182471..e54960252b 100644 --- a/doc/AddonStorageBackend.md +++ b/doc/AddonStorageBackend.md @@ -10,12 +10,12 @@ A storage backend is implemented as a class, and the plugin register the class t The class must live in `Friendica\Addon\youraddonname` namespace, where `youraddonname` the folder name of your addon. -The class must implement `Friendica\Model\Storage\ISelectableStorage` interface. All method in the interface must be implemented: +The class must implement `Friendica\Model\Storage\IWritableStorage` interface. All method in the interface must be implemented: -namespace Friendica\Model\ISelectableStorage; +namespace Friendica\Model\IWritableStorage; ```php -interface ISelectableStorage +interface IWritableStorage { public function get(string $reference); public function put(string $data, string $reference = ''); @@ -79,7 +79,7 @@ Each label should be translatable ]; -See doxygen documentation of `ISelectableStorage` interface for details about each method. +See doxygen documentation of `IWritableStorage` interface for details about each method. ## Register a storage backend class @@ -105,8 +105,9 @@ Each new Storage class should be added to the test-environment at [Storage Tests Add a new test class which's naming convention is `StorageClassTest`, which extend the `StorageTest` in the same directory. Override the two necessary instances: + ```php -use Friendica\Model\Storage\ISelectableStorage; +use Friendica\Model\Storage\IWritableStorage; abstract class StorageTest { @@ -114,7 +115,7 @@ abstract class StorageTest abstract protected function getInstance(); // Assertion for the option array you return for your new StorageClass - abstract protected function assertOption(ISelectableStorage $storage); + abstract protected function assertOption(IWritableStorage $storage); } ``` @@ -138,9 +139,9 @@ If there's a predecessor to this exception (e.g. you caught an exception and are Example: ```php -use Friendica\Model\Storage\ISelectableStorage; +use Friendica\Model\Storage\IWritableStorage; -class ExampleStorage implements ISelectableStorage +class ExampleStorage implements IWritableStorage { public function get(string $reference) : string { @@ -168,12 +169,12 @@ The file will be `addon/samplestorage/SampleStorageBackend.php`: assertEquals([ 'filename' => [ diff --git a/src/Console/Storage.php b/src/Console/Storage.php index 70e8e26302..34419c48cd 100644 --- a/src/Console/Storage.php +++ b/src/Console/Storage.php @@ -132,7 +132,7 @@ HELP; } $name = $this->args[1]; - $class = $this->storageManager->getSelectableStorageByName($name); + $class = $this->storageManager->getWritableStorageByName($name); if (is_null($class)) { $this->out($name . ' is not a registered backend.'); diff --git a/src/Core/StorageManager.php b/src/Core/StorageManager.php index 4603b0fded..9e75f83058 100644 --- a/src/Core/StorageManager.php +++ b/src/Core/StorageManager.php @@ -82,13 +82,13 @@ class StorageManager $currentName = $this->config->get('storage', 'name', ''); // you can only use user backends as a "default" backend, so the second parameter is true - $this->currentBackend = $this->getSelectableStorageByName($currentName); + $this->currentBackend = $this->getWritableStorageByName($currentName); } /** * Return current storage backend class * - * @return Storage\ISelectableStorage|null + * @return Storage\IWritableStorage|null */ public function getBackend() { @@ -96,16 +96,16 @@ class StorageManager } /** - * Returns a selectable storage backend class by registered name + * Returns a writable storage backend class by registered name * * @param string $name Backend name * - * @return Storage\ISelectableStorage + * @return Storage\IWritableStorage * * @throws Storage\ReferenceStorageException in case there's no backend class for the name * @throws Storage\StorageException in case of an unexpected failure during the hook call */ - public function getSelectableStorageByName(string $name = null) + public function getWritableStorageByName(string $name = null) { // @todo 2020.09 Remove this call after 2 releases $name = $this->checkLegacyBackend($name); @@ -130,7 +130,7 @@ class StorageManager ]; try { Hook::callAll('storage_instance', $data); - if (($data['storage'] ?? null) instanceof Storage\ISelectableStorage) { + if (($data['storage'] ?? null) instanceof Storage\IWritableStorage) { $this->backendInstances[$data['name'] ?? $name] = $data['storage']; } else { throw new Storage\ReferenceStorageException(sprintf('Backend %s was not found', $name)); @@ -244,11 +244,11 @@ class StorageManager /** * Set current storage backend class * - * @param Storage\ISelectableStorage $storage The storage class + * @param Storage\IWritableStorage $storage The storage class * * @return boolean True, if the set was successful */ - public function setBackend(Storage\ISelectableStorage $storage) + public function setBackend(Storage\IWritableStorage $storage) { if ($this->config->set('storage', 'name', $storage::getName())) { $this->currentBackend = $storage; @@ -327,15 +327,15 @@ class StorageManager * Copy existing data to destination storage and delete from source. * This method cannot move to legacy in-table `data` field. * - * @param Storage\ISelectableStorage $destination Destination storage class name - * @param array $tables Tables to look in for resources. Optional, defaults to ['photo', 'attach'] - * @param int $limit Limit of the process batch size, defaults to 5000 + * @param Storage\IWritableStorage $destination Destination storage class name + * @param array $tables Tables to look in for resources. Optional, defaults to ['photo', 'attach'] + * @param int $limit Limit of the process batch size, defaults to 5000 * * @return int Number of moved resources * @throws Storage\StorageException * @throws Exception */ - public function move(Storage\ISelectableStorage $destination, array $tables = self::TABLES, int $limit = 5000) + public function move(Storage\IWritableStorage $destination, array $tables = self::TABLES, int $limit = 5000) { if (!$this->isValidBackend($destination, true)) { throw new Storage\StorageException(sprintf("Can't move to storage backend '%s'", $destination::getName())); @@ -354,7 +354,7 @@ class StorageManager while ($resource = $this->dba->fetch($resources)) { $id = $resource['id']; $data = $resource['data']; - $source = $this->getSelectableStorageByName($resource['backend-class']); + $source = $this->getWritableStorageByName($resource['backend-class']); $sourceRef = $resource['backend-ref']; if (!empty($source)) { diff --git a/src/DI.php b/src/DI.php index 9e96ddc015..28ad130b4d 100644 --- a/src/DI.php +++ b/src/DI.php @@ -387,11 +387,11 @@ abstract class DI } /** - * @return Model\Storage\ISelectableStorage + * @return Model\Storage\IWritableStorage */ public static function storage() { - return self::$dice->create(Model\Storage\ISelectableStorage::class); + return self::$dice->create(Model\Storage\IWritableStorage::class); } // diff --git a/src/Model/Attach.php b/src/Model/Attach.php index d3ff5462ca..ac8bca07f1 100644 --- a/src/Model/Attach.php +++ b/src/Model/Attach.php @@ -284,7 +284,7 @@ class Attach $items = self::selectToArray(['backend-class','backend-ref'], $conditions); foreach($items as $item) { - $backend_class = DI::storageManager()->getSelectableStorageByName($item['backend-class'] ?? ''); + $backend_class = DI::storageManager()->getWritableStorageByName($item['backend-class'] ?? ''); if (!empty($backend_class)) { $fields['backend-ref'] = $backend_class->put($img->asString(), $item['backend-ref'] ?? ''); } else { @@ -316,7 +316,7 @@ class Attach $items = self::selectToArray(['backend-class','backend-ref'], $conditions); foreach($items as $item) { - $backend_class = DI::storageManager()->getSelectableStorageByName($item['backend-class'] ?? ''); + $backend_class = DI::storageManager()->getWritableStorageByName($item['backend-class'] ?? ''); if (!empty($backend_class)) { try { $backend_class->delete($item['backend-ref'] ?? ''); diff --git a/src/Model/Photo.php b/src/Model/Photo.php index a38743d9d6..28eb6f3611 100644 --- a/src/Model/Photo.php +++ b/src/Model/Photo.php @@ -347,7 +347,7 @@ class Photo if (DBA::isResult($existing_photo)) { $backend_ref = (string)$existing_photo["backend-ref"]; - $storage = DI::storageManager()->getSelectableStorageByName($existing_photo["backend-class"] ?? ''); + $storage = DI::storageManager()->getWritableStorageByName($existing_photo["backend-class"] ?? ''); } else { $storage = DI::storage(); } @@ -411,7 +411,7 @@ class Photo $photos = DBA::select('photo', ['id', 'backend-class', 'backend-ref'], $conditions); while ($photo = DBA::fetch($photos)) { - $backend_class = DI::storageManager()->getSelectableStorageByName($photo['backend-class'] ?? ''); + $backend_class = DI::storageManager()->getWritableStorageByName($photo['backend-class'] ?? ''); if (!empty($backend_class)) { try { $backend_class->delete($item['backend-ref'] ?? ''); @@ -448,7 +448,7 @@ class Photo $photos = self::selectToArray(['backend-class', 'backend-ref'], $conditions); foreach($photos as $photo) { - $backend_class = DI::storageManager()->getSelectableStorageByName($photo['backend-class'] ?? ''); + $backend_class = DI::storageManager()->getWritableStorageByName($photo['backend-class'] ?? ''); if (!empty($backend_class)) { $fields["backend-ref"] = $backend_class->put($img->asString(), $photo['backend-ref']); } else { diff --git a/src/Model/Storage/Database.php b/src/Model/Storage/Database.php index a77e268117..3457fa4a30 100644 --- a/src/Model/Storage/Database.php +++ b/src/Model/Storage/Database.php @@ -29,7 +29,7 @@ use Friendica\Database\Database as DBA; * * This class manage data stored in database table. */ -class Database implements ISelectableStorage +class Database implements IWritableStorage { const NAME = 'Database'; diff --git a/src/Model/Storage/Filesystem.php b/src/Model/Storage/Filesystem.php index fe5230049f..2d6a352015 100644 --- a/src/Model/Storage/Filesystem.php +++ b/src/Model/Storage/Filesystem.php @@ -36,7 +36,7 @@ use Friendica\Util\Strings; * Each new resource gets a value as reference and is saved in a * folder tree stucture created from that value. */ -class Filesystem implements ISelectableStorage +class Filesystem implements IWritableStorage { const NAME = 'Filesystem'; diff --git a/src/Model/Storage/ISelectableStorage.php b/src/Model/Storage/IWritableStorage.php similarity index 93% rename from src/Model/Storage/ISelectableStorage.php rename to src/Model/Storage/IWritableStorage.php index d3b7547774..650b3246ed 100644 --- a/src/Model/Storage/ISelectableStorage.php +++ b/src/Model/Storage/IWritableStorage.php @@ -22,12 +22,12 @@ namespace Friendica\Model\Storage; /** - * Interface for selectable storage backends + * Interface for writable storage backends * * Used for storages with CRUD functionality, mainly used for user data (e.g. photos, attachements). - * There's only one active, selectable storage possible and can be selected by the current administrator + * There's only one active, writable storage possible. This type of storages are selectable by the current administrator */ -interface ISelectableStorage extends IStorage +interface IWritableStorage extends IStorage { /** * Put data in backend as $ref. If $ref is not defined a new reference is created. diff --git a/src/Module/Admin/Storage.php b/src/Module/Admin/Storage.php index 374d692bf7..e12cecd031 100644 --- a/src/Module/Admin/Storage.php +++ b/src/Module/Admin/Storage.php @@ -23,7 +23,7 @@ namespace Friendica\Module\Admin; use Friendica\Core\Renderer; use Friendica\DI; -use Friendica\Model\Storage\ISelectableStorage; +use Friendica\Model\Storage\IWritableStorage; use Friendica\Module\BaseAdmin; use Friendica\Util\Strings; @@ -37,8 +37,8 @@ class Storage extends BaseAdmin $storagebackend = Strings::escapeTags(trim($parameters['name'] ?? '')); - /** @var ISelectableStorage $newstorage */ - $newstorage = DI::storageManager()->getSelectableStorageByName($storagebackend); + /** @var IWritableStorage $newstorage */ + $newstorage = DI::storageManager()->getWritableStorageByName($storagebackend); // save storage backend form $storage_opts = $newstorage->getOptions(); @@ -68,8 +68,8 @@ class Storage extends BaseAdmin } if (!empty($_POST['submit_save_set'])) { - /** @var ISelectableStorage $newstorage */ - $newstorage = DI::storageManager()->getSelectableStorageByName($storagebackend); + /** @var IWritableStorage $newstorage */ + $newstorage = DI::storageManager()->getWritableStorageByName($storagebackend); if (!DI::storageManager()->setBackend($newstorage)) { notice(DI::l10n()->t('Invalid storage backend setting value.')); @@ -92,7 +92,7 @@ class Storage extends BaseAdmin $storage_form_prefix = preg_replace('|[^a-zA-Z0-9]|', '', $name); $storage_form = []; - foreach (DI::storageManager()->getSelectableStorageByName($name)->getOptions() as $option => $info) { + foreach (DI::storageManager()->getWritableStorageByName($name)->getOptions() as $option => $info) { $type = $info[0]; // Backward compatibilty with yesno field description if ($type == 'yesno') { @@ -111,7 +111,7 @@ class Storage extends BaseAdmin 'name' => $name, 'prefix' => $storage_form_prefix, 'form' => $storage_form, - 'active' => $current_storage_backend instanceof ISelectableStorage && $name === $current_storage_backend::getName(), + 'active' => $current_storage_backend instanceof IWritableStorage && $name === $current_storage_backend::getName(), ]; } @@ -127,7 +127,7 @@ class Storage extends BaseAdmin '$noconfig' => DI::l10n()->t('This backend doesn\'t have custom settings'), '$baseurl' => DI::baseUrl()->get(true), '$form_security_token' => self::getFormSecurityToken("admin_storage"), - '$storagebackend' => $current_storage_backend instanceof ISelectableStorage ? $current_storage_backend::getName() : DI::l10n()->t('Database (legacy)'), + '$storagebackend' => $current_storage_backend instanceof IWritableStorage ? $current_storage_backend::getName() : DI::l10n()->t('Database (legacy)'), '$availablestorageforms' => $available_storage_forms, ]); } diff --git a/static/dependencies.config.php b/static/dependencies.config.php index ea57efca5a..5efe78ddf3 100644 --- a/static/dependencies.config.php +++ b/static/dependencies.config.php @@ -44,7 +44,7 @@ use Friendica\Core\Session\ISession; use Friendica\Core\StorageManager; use Friendica\Database\Database; use Friendica\Factory; -use Friendica\Model\Storage\ISelectableStorage; +use Friendica\Model\Storage\IWritableStorage; use Friendica\Model\User\Cookie; use Friendica\Network; use Friendica\Util; @@ -213,7 +213,7 @@ return [ $_SERVER, $_COOKIE ], ], - ISelectableStorage::class => [ + IWritableStorage::class => [ 'instanceOf' => StorageManager::class, 'call' => [ ['getBackend', [], Dice::CHAIN_CALL], diff --git a/tests/Util/SampleStorageBackend.php b/tests/Util/SampleStorageBackend.php index 1ebd64f6ee..1185a25646 100644 --- a/tests/Util/SampleStorageBackend.php +++ b/tests/Util/SampleStorageBackend.php @@ -22,14 +22,14 @@ namespace Friendica\Test\Util; use Friendica\Core\Hook; -use Friendica\Model\Storage\ISelectableStorage; +use Friendica\Model\Storage\IWritableStorage; use Friendica\Core\L10n; /** * A backend storage example class */ -class SampleStorageBackend implements ISelectableStorage +class SampleStorageBackend implements IWritableStorage { const NAME = 'Sample Storage'; diff --git a/tests/src/Core/StorageManagerTest.php b/tests/src/Core/StorageManagerTest.php index f01640dce4..25ae44b1bc 100644 --- a/tests/src/Core/StorageManagerTest.php +++ b/tests/src/Core/StorageManagerTest.php @@ -178,7 +178,7 @@ class StorageManagerTest extends DatabaseTest $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->l10n, $this->httpRequest); if ($userBackend) { - $storage = $storageManager->getSelectableStorageByName($name); + $storage = $storageManager->getWritableStorageByName($name); } else { $storage = $storageManager->getByName($name); } @@ -230,7 +230,7 @@ class StorageManagerTest extends DatabaseTest self::assertNull($storageManager->getBackend()); if ($userBackend) { - $selBackend = $storageManager->getSelectableStorageByName($name); + $selBackend = $storageManager->getWritableStorageByName($name); $storageManager->setBackend($selBackend); self::assertInstanceOf($assert, $storageManager->getBackend()); @@ -287,7 +287,7 @@ class StorageManagerTest extends DatabaseTest SampleStorageBackend::registerHook(); Hook::loadHooks(); - self::assertTrue($storageManager->setBackend( $storageManager->getSelectableStorageByName(SampleStorageBackend::NAME))); + self::assertTrue($storageManager->setBackend( $storageManager->getWritableStorageByName(SampleStorageBackend::NAME))); self::assertEquals(SampleStorageBackend::NAME, $this->config->get('storage', 'name')); self::assertInstanceOf(SampleStorageBackend::class, $storageManager->getBackend()); @@ -314,7 +314,7 @@ class StorageManagerTest extends DatabaseTest $this->loadFixture(__DIR__ . '/../../datasets/storage/database.fixture.php', $this->dba); $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->l10n); - $storage = $storageManager->getSelectableStorageByName($name); + $storage = $storageManager->getWritableStorageByName($name); $storageManager->move($storage); $photos = $this->dba->select('photo', ['backend-ref', 'backend-class', 'id', 'data']); @@ -333,12 +333,12 @@ class StorageManagerTest extends DatabaseTest /** * Test moving data to a WRONG storage */ - public function testWrongSelectableStorage() + public function testWrongWritableStorage() { $this->expectException(\TypeError::class); $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->l10n); - $storage = $storageManager->getSelectableStorageByName(Storage\SystemResource::getName()); + $storage = $storageManager->getWritableStorageByName(Storage\SystemResource::getName()); $storageManager->move($storage); } } diff --git a/tests/src/Model/Storage/DatabaseStorageTest.php b/tests/src/Model/Storage/DatabaseStorageTest.php index 62c8e7f116..d7b810c1f8 100644 --- a/tests/src/Model/Storage/DatabaseStorageTest.php +++ b/tests/src/Model/Storage/DatabaseStorageTest.php @@ -23,7 +23,7 @@ namespace Friendica\Test\src\Model\Storage; use Friendica\Factory\ConfigFactory; use Friendica\Model\Storage\Database; -use Friendica\Model\Storage\ISelectableStorage; +use Friendica\Model\Storage\IWritableStorage; use Friendica\Test\DatabaseTestTrait; use Friendica\Test\Util\Database\StaticDatabase; use Friendica\Test\Util\VFSTrait; @@ -63,7 +63,7 @@ class DatabaseStorageTest extends StorageTest return new Database($dba); } - protected function assertOption(ISelectableStorage $storage) + protected function assertOption(IWritableStorage $storage) { self::assertEmpty($storage->getOptions()); } diff --git a/tests/src/Model/Storage/FilesystemStorageTest.php b/tests/src/Model/Storage/FilesystemStorageTest.php index a0f267d183..45a7264163 100644 --- a/tests/src/Model/Storage/FilesystemStorageTest.php +++ b/tests/src/Model/Storage/FilesystemStorageTest.php @@ -24,7 +24,7 @@ namespace Friendica\Test\src\Model\Storage; use Friendica\Core\Config\IConfig; use Friendica\Core\L10n; use Friendica\Model\Storage\Filesystem; -use Friendica\Model\Storage\ISelectableStorage; +use Friendica\Model\Storage\IWritableStorage; use Friendica\Model\Storage\StorageException; use Friendica\Test\Util\VFSTrait; use Friendica\Util\Profiler; @@ -64,7 +64,7 @@ class FilesystemStorageTest extends StorageTest return new Filesystem($this->config, $l10n); } - protected function assertOption(ISelectableStorage $storage) + protected function assertOption(IWritableStorage $storage) { self::assertEquals([ 'storagepath' => [ diff --git a/tests/src/Model/Storage/StorageTest.php b/tests/src/Model/Storage/StorageTest.php index 4001bfc68b..340aee8bfd 100644 --- a/tests/src/Model/Storage/StorageTest.php +++ b/tests/src/Model/Storage/StorageTest.php @@ -21,17 +21,17 @@ namespace Friendica\Test\src\Model\Storage; -use Friendica\Model\Storage\ISelectableStorage; +use Friendica\Model\Storage\IWritableStorage; use Friendica\Model\Storage\IStorage; use Friendica\Model\Storage\ReferenceStorageException; use Friendica\Test\MockedTest; abstract class StorageTest extends MockedTest { - /** @return ISelectableStorage */ + /** @return IWritableStorage */ abstract protected function getInstance(); - abstract protected function assertOption(ISelectableStorage $storage); + abstract protected function assertOption(IWritableStorage $storage); /** * Test if the instance is "really" implementing the interface From b798ca2da6d0185f7f1c313e3402313af6338d5f Mon Sep 17 00:00:00 2001 From: Philipp Date: Tue, 10 Aug 2021 22:53:20 +0200 Subject: [PATCH 10/19] Force "Database" as default storage backend --- static/settings.config.php | 7 +++++++ update.php | 17 +++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/static/settings.config.php b/static/settings.config.php index e8c6d09f81..96ae421ff0 100644 --- a/static/settings.config.php +++ b/static/settings.config.php @@ -203,4 +203,11 @@ return [ // Used in the admin settings to lock certain features 'featurelock' => [ ], + + // Storage backend configuration + 'storage' => [ + // name (String) + // The name of the current used backend (default is Database) + 'name' => 'Database', + ], ]; diff --git a/update.php b/update.php index 842fefca88..5d9c76a994 100644 --- a/update.php +++ b/update.php @@ -981,3 +981,20 @@ function update_1429() return Update::SUCCESS; } + +function update_1433() +{ + $name = DI::config()->get('storage', 'name'); + + // in case of an empty config, set "Database" as default storage backend + if (empty($name)) { + DI::config()->set('storage', 'name', Storage\Database::getName()); + } + + // In case of a Using deprecated storage class value, set the right name for it + if (stristr($name, 'Friendica\Model\Storage\\')) { + DI::config()->set('storage', 'name', substr($name, 24)); + } + + return Update::SUCCESS; +} From c17bc55158c63201203a3027f515d505a20529d9 Mon Sep 17 00:00:00 2001 From: Philipp Date: Tue, 10 Aug 2021 23:56:30 +0200 Subject: [PATCH 11/19] Introduce InvalidClassStorageException and adapt the code for it --- src/Console/Storage.php | 19 ++- src/Core/StorageManager.php | 140 ++++++--------- src/Model/Attach.php | 43 ++--- src/Model/Photo.php | 99 +++++------ .../Storage/InvalidClassStorageException.php | 29 ++++ src/Module/Admin/Storage.php | 22 ++- src/Module/Photo.php | 8 + src/Module/Settings/Profile/Photo/Crop.php | 7 + tests/src/Core/StorageManagerTest.php | 159 ++++++++---------- 9 files changed, 259 insertions(+), 267 deletions(-) create mode 100644 src/Model/Storage/InvalidClassStorageException.php diff --git a/src/Console/Storage.php b/src/Console/Storage.php index 34419c48cd..e2aa185c92 100644 --- a/src/Console/Storage.php +++ b/src/Console/Storage.php @@ -23,6 +23,7 @@ namespace Friendica\Console; use Asika\SimpleConsole\CommandArgsException; use Friendica\Core\StorageManager; +use Friendica\Model\Storage\ReferenceStorageException; use Friendica\Model\Storage\StorageException; /** @@ -127,23 +128,23 @@ HELP; protected function doSet() { - if (count($this->args) !== 2) { + if (count($this->args) !== 2 || empty($this->args[1])) { throw new CommandArgsException('Invalid arguments'); } - $name = $this->args[1]; - $class = $this->storageManager->getWritableStorageByName($name); + $name = $this->args[1]; + try { + $class = $this->storageManager->getWritableStorageByName($name); - if (is_null($class)) { + if (!$this->storageManager->setBackend($class)) { + $this->out($class . ' is not a valid backend storage class.'); + return -1; + } + } catch (ReferenceStorageException $exception) { $this->out($name . ' is not a registered backend.'); return -1; } - if (!$this->storageManager->setBackend($class)) { - $this->out($class . ' is not a valid backend storage class.'); - return -1; - } - return 0; } diff --git a/src/Core/StorageManager.php b/src/Core/StorageManager.php index 9e75f83058..132cd64538 100644 --- a/src/Core/StorageManager.php +++ b/src/Core/StorageManager.php @@ -28,7 +28,6 @@ use Friendica\Model\Storage; use Friendica\Network\HTTPException\InternalServerErrorException; use Psr\Log\LoggerInterface; - /** * Manage storage backends * @@ -46,7 +45,7 @@ class StorageManager Storage\Database::NAME => Storage\Database::class, ]; - private $backends = []; + private $backends; /** * @var Storage\IStorage[] A local cache for storage instances @@ -62,7 +61,7 @@ class StorageManager /** @var L10n */ private $l10n; - /** @var Storage\IStorage */ + /** @var Storage\IWritableStorage */ private $currentBackend; /** @@ -70,16 +69,19 @@ class StorageManager * @param IConfig $config * @param LoggerInterface $logger * @param L10n $l10n + * + * @throws Storage\InvalidClassStorageException in case the active backend class is invalid + * @throws Storage\StorageException in case of unexpected errors during the active backend class loading */ public function __construct(Database $dba, IConfig $config, LoggerInterface $logger, L10n $l10n) { - $this->dba = $dba; - $this->config = $config; - $this->logger = $logger; - $this->l10n = $l10n; + $this->dba = $dba; + $this->config = $config; + $this->logger = $logger; + $this->l10n = $l10n; $this->backends = $config->get('storage', 'backends', self::DEFAULT_BACKENDS); - $currentName = $this->config->get('storage', 'name', ''); + $currentName = $this->config->get('storage', 'name'); // you can only use user backends as a "default" backend, so the second parameter is true $this->currentBackend = $this->getWritableStorageByName($currentName); @@ -88,7 +90,7 @@ class StorageManager /** * Return current storage backend class * - * @return Storage\IWritableStorage|null + * @return Storage\IWritableStorage */ public function getBackend() { @@ -102,71 +104,36 @@ class StorageManager * * @return Storage\IWritableStorage * - * @throws Storage\ReferenceStorageException in case there's no backend class for the name + * @throws Storage\InvalidClassStorageException in case there's no backend class for the name * @throws Storage\StorageException in case of an unexpected failure during the hook call */ - public function getWritableStorageByName(string $name = null) + public function getWritableStorageByName(string $name): Storage\IWritableStorage { - // @todo 2020.09 Remove this call after 2 releases - $name = $this->checkLegacyBackend($name); - - // If there's no cached instance create a new instance - if (!isset($this->backendInstances[$name])) { - // If the current name isn't a valid backend (or the SystemResource instance) create it - if ($this->isValidBackend($name, true)) { - switch ($name) { - // Try the filesystem backend - case Storage\Filesystem::getName(): - $this->backendInstances[$name] = new Storage\Filesystem($this->config, $this->l10n); - break; - // try the database backend - case Storage\Database::getName(): - $this->backendInstances[$name] = new Storage\Database($this->dba); - break; - default: - $data = [ - 'name' => $name, - 'storage' => null, - ]; - try { - Hook::callAll('storage_instance', $data); - if (($data['storage'] ?? null) instanceof Storage\IWritableStorage) { - $this->backendInstances[$data['name'] ?? $name] = $data['storage']; - } else { - throw new Storage\ReferenceStorageException(sprintf('Backend %s was not found', $name)); - } - } catch (InternalServerErrorException $exception) { - throw new Storage\StorageException(sprintf('Failed calling hook::storage_instance for backend %s', $name), $exception); - } - break; - } - } else { - throw new Storage\ReferenceStorageException(sprintf('Backend %s is not valid', $name)); - } + $storage = $this->getByName($name, $this->backends); + if ($storage instanceof Storage\IWritableStorage) { + return $storage; + } else { + throw new Storage\InvalidClassStorageException(sprintf('Backend %s is not writable', $name)); } - - return $this->backendInstances[$name]; } /** * Return storage backend class by registered name * - * @param string $name Backend name + * @param string $name Backend name + * @param array|null $validBackends possible, manual override of the valid backends * * @return Storage\IStorage * - * @throws Storage\ReferenceStorageException in case there's no backend class for the name + * @throws Storage\InvalidClassStorageException in case there's no backend class for the name * @throws Storage\StorageException in case of an unexpected failure during the hook call */ - public function getByName(string $name) + public function getByName(string $name, array $validBackends = null): Storage\IStorage { - // @todo 2020.09 Remove this call after 2 releases - $name = $this->checkLegacyBackend($name); - // If there's no cached instance create a new instance if (!isset($this->backendInstances[$name])) { // If the current name isn't a valid backend (or the SystemResource instance) create it - if ($this->isValidBackend($name, false)) { + if ($this->isValidBackend($name, $validBackends)) { switch ($name) { // Try the filesystem backend case Storage\Filesystem::getName(): @@ -193,7 +160,7 @@ class StorageManager if (($data['storage'] ?? null) instanceof Storage\IStorage) { $this->backendInstances[$data['name'] ?? $name] = $data['storage']; } else { - throw new Storage\ReferenceStorageException(sprintf('Backend %s was not found', $name)); + throw new Storage\InvalidClassStorageException(sprintf('Backend %s was not found', $name)); } } catch (InternalServerErrorException $exception) { throw new Storage\StorageException(sprintf('Failed calling hook::storage_instance for backend %s', $name), $exception); @@ -201,7 +168,7 @@ class StorageManager break; } } else { - throw new Storage\ReferenceStorageException(sprintf('Backend %s is not valid', $name)); + throw new Storage\InvalidClassStorageException(sprintf('Backend %s is not valid', $name)); } } @@ -211,34 +178,19 @@ class StorageManager /** * Checks, if the storage is a valid backend * - * @param string|null $name The name or class of the backend - * @param boolean $onlyUserBackend True, if just user backend should get returned (e.g. not SystemResource) + * @param string|null $name The name or class of the backend + * @param array|null $validBackends Possible, valid backends to check * * @return boolean True, if the backend is a valid backend */ - public function isValidBackend(string $name = null, bool $onlyUserBackend = false) + public function isValidBackend(string $name = null, array $validBackends = null): bool { - return array_key_exists($name, $this->backends) || - (!$onlyUserBackend && in_array($name, [Storage\SystemResource::getName(), Storage\ExternalResource::getName()])); - } - - /** - * Check for legacy backend storage class names (= full model class name) - * - * @todo 2020.09 Remove this function after 2 releases, because there shouldn't be any legacy backend classes left - * - * @param string|null $name a potential, legacy storage name ("Friendica\Model\Storage\...") - * - * @return string|null The current storage name - */ - private function checkLegacyBackend(string $name = null) - { - if (stristr($name, 'Friendica\Model\Storage\\')) { - $this->logger->notice('Using deprecated storage class value', ['name' => $name]); - return substr($name, 24); - } - - return $name; + $validBackends = $validBackends ?? array_merge($this->backends, + [ + Storage\SystemResource::getName() => '', + Storage\ExternalResource::getName() => '' + ]); + return array_key_exists($name, $validBackends); } /** @@ -248,7 +200,7 @@ class StorageManager * * @return boolean True, if the set was successful */ - public function setBackend(Storage\IWritableStorage $storage) + public function setBackend(Storage\IWritableStorage $storage): bool { if ($this->config->set('storage', 'name', $storage::getName())) { $this->currentBackend = $storage; @@ -263,7 +215,7 @@ class StorageManager * * @return array */ - public function listBackends() + public function listBackends(): array { return $this->backends; } @@ -277,7 +229,7 @@ class StorageManager * * @return boolean True, if the registration was successful */ - public function register(string $class) + public function register(string $class): bool { if (is_subclass_of($class, Storage\IStorage::class)) { /** @var Storage\IStorage $class */ @@ -303,7 +255,7 @@ class StorageManager * * @return boolean True, if unregistering was successful */ - public function unregister(string $class) + public function unregister(string $class): bool { if (is_subclass_of($class, Storage\IStorage::class)) { /** @var Storage\IStorage $class */ @@ -335,9 +287,9 @@ class StorageManager * @throws Storage\StorageException * @throws Exception */ - public function move(Storage\IWritableStorage $destination, array $tables = self::TABLES, int $limit = 5000) + public function move(Storage\IWritableStorage $destination, array $tables = self::TABLES, int $limit = 5000): int { - if (!$this->isValidBackend($destination, true)) { + if (!$this->isValidBackend($destination, $this->backends)) { throw new Storage\StorageException(sprintf("Can't move to storage backend '%s'", $destination::getName())); } @@ -353,13 +305,19 @@ class StorageManager while ($resource = $this->dba->fetch($resources)) { $id = $resource['id']; - $data = $resource['data']; - $source = $this->getWritableStorageByName($resource['backend-class']); $sourceRef = $resource['backend-ref']; + $source = null; - if (!empty($source)) { + try { + $source = $this->getWritableStorageByName($resource['backend-class'] ?? ''); $this->logger->info('Get data from old backend.', ['oldBackend' => $source, 'oldReference' => $sourceRef]); $data = $source->get($sourceRef); + } catch (Storage\InvalidClassStorageException $exception) { + $this->logger->info('Get data from DB resource field.', ['oldReference' => $sourceRef]); + $data = $resource['data']; + } catch (Storage\ReferenceStorageException $exception) { + $this->logger->info('Invalid source reference.', ['oldBackend' => $source, 'oldReference' => $sourceRef]); + continue; } $this->logger->info('Save data to new backend.', ['newBackend' => $destination::getName()]); diff --git a/src/Model/Attach.php b/src/Model/Attach.php index ac8bca07f1..e11fd01bc3 100644 --- a/src/Model/Attach.php +++ b/src/Model/Attach.php @@ -25,6 +25,7 @@ use Friendica\Core\System; use Friendica\Database\DBA; use Friendica\Database\DBStructure; use Friendica\DI; +use Friendica\Model\Storage\InvalidClassStorageException; use Friendica\Model\Storage\ReferenceStorageException; use Friendica\Object\Image; use Friendica\Util\DateTimeFormat; @@ -164,22 +165,20 @@ class Attach return $item['data']; } - $backendClass = DI::storageManager()->getByName($item['backend-class'] ?? ''); - if (empty($backendClass)) { + try { + $backendClass = DI::storageManager()->getByName($item['backend-class'] ?? ''); + $backendRef = $item['backend-ref']; + return $backendClass->get($backendRef); + } catch (InvalidClassStorageException $storageException) { // legacy data storage in 'data' column $i = self::selectFirst(['data'], ['id' => $item['id']]); if ($i === false) { return null; } return $i['data']; - } else { - $backendRef = $item['backend-ref']; - try { - return $backendClass->get($backendRef); - } catch (ReferenceStorageException $referenceStorageException) { - DI::logger()->debug('No data found for item', ['item' => $item, 'exception' => $referenceStorageException]); - return ''; - } + } catch (ReferenceStorageException $referenceStorageException) { + DI::logger()->debug('No data found for item', ['item' => $item, 'exception' => $referenceStorageException]); + return ''; } } @@ -284,11 +283,13 @@ class Attach $items = self::selectToArray(['backend-class','backend-ref'], $conditions); foreach($items as $item) { - $backend_class = DI::storageManager()->getWritableStorageByName($item['backend-class'] ?? ''); - if (!empty($backend_class)) { + try { + $backend_class = DI::storageManager()->getWritableStorageByName($item['backend-class'] ?? ''); $fields['backend-ref'] = $backend_class->put($img->asString(), $item['backend-ref'] ?? ''); - } else { - $fields['data'] = $img->asString(); + } catch (InvalidClassStorageException $storageException) { + DI::logger()->debug('Storage class not found.', ['conditions' => $conditions, 'exception' => $storageException]); + } catch (ReferenceStorageException $referenceStorageException) { + DI::logger()->debug('Item doesn\'t exist.', ['conditions' => $conditions, 'exception' => $referenceStorageException]); } } } @@ -316,13 +317,13 @@ class Attach $items = self::selectToArray(['backend-class','backend-ref'], $conditions); foreach($items as $item) { - $backend_class = DI::storageManager()->getWritableStorageByName($item['backend-class'] ?? ''); - if (!empty($backend_class)) { - try { - $backend_class->delete($item['backend-ref'] ?? ''); - } catch (ReferenceStorageException $referenceStorageException) { - DI::logger()->debug('Item doesn\'t exist.', ['conditions' => $conditions, 'exception' => $referenceStorageException]); - } + try { + $backend_class = DI::storageManager()->getWritableStorageByName($item['backend-class'] ?? ''); + $backend_class->delete($item['backend-ref'] ?? ''); + } catch (InvalidClassStorageException $storageException) { + DI::logger()->debug('Storage class not found.', ['conditions' => $conditions, 'exception' => $storageException]); + } catch (ReferenceStorageException $referenceStorageException) { + DI::logger()->debug('Item doesn\'t exist.', ['conditions' => $conditions, 'exception' => $referenceStorageException]); } } diff --git a/src/Model/Photo.php b/src/Model/Photo.php index 28eb6f3611..8df9b92f2d 100644 --- a/src/Model/Photo.php +++ b/src/Model/Photo.php @@ -28,6 +28,7 @@ use Friendica\Database\DBA; use Friendica\Database\DBStructure; use Friendica\DI; use Friendica\Model\Storage\ExternalResource; +use Friendica\Model\Storage\InvalidClassStorageException; use Friendica\Model\Storage\ReferenceStorageException; use Friendica\Model\Storage\StorageException; use Friendica\Model\Storage\SystemResource; @@ -186,9 +187,6 @@ class Photo * @param array $photo Photo data. Needs at least 'id', 'type', 'backend-class', 'backend-ref' * * @return \Friendica\Object\Image - * @throws \Friendica\Network\HTTPException\InternalServerErrorException - * @throws \ImagickException - * @throws StorageException */ public static function getImageDataForPhoto(array $photo) { @@ -196,24 +194,36 @@ class Photo return $photo['data']; } - $backendClass = DI::storageManager()->getByName($photo['backend-class'] ?? ''); - if (empty($backendClass)) { - // legacy data storage in "data" column - $i = self::selectFirst(['data'], ['id' => $photo['id']]); - if ($i === false) { - return null; + try { + $backendClass = DI::storageManager()->getByName($photo['backend-class'] ?? ''); + $image = $backendClass->get($photo['backend-ref'] ?? ''); + + if ($image instanceof Image) { + return $image; + } else { + DI::logger()->info('Stored data is not an image', ['photo' => $photo]); } - $data = $i['data']; - } else { - $backendRef = $photo['backend-ref'] ?? ''; + } catch (InvalidClassStorageException $storageException) { try { - $data = $backendClass->get($backendRef); - } catch (ReferenceStorageException $referenceStorageException) { - DI::logger()->debug('No data found for photo', ['photo' => $photo, 'exception' => $referenceStorageException]); - return null; + // legacy data storage in "data" column + $i = self::selectFirst(['data'], ['id' => $photo['id']]); + if ($i !== false) { + return $i['data']; + } else { + DI::logger()->info('Stored legacy data is empty', ['photo' => $photo]); + } + } catch (\Exception $exception) { + DI::logger()->info('Unexpected database exception', ['photo' => $photo, 'exception' => $exception]); } + } catch (ReferenceStorageException $referenceStorageException) { + DI::logger()->debug('Invalid reference for photo', ['photo' => $photo, 'exception' => $referenceStorageException]); + } catch (StorageException $storageException) { + DI::logger()->info('Unexpected storage exception', ['photo' => $photo, 'exception' => $storageException]); + } catch (\ImagickException $imagickException) { + DI::logger()->info('Unexpected imagick exception', ['photo' => $photo, 'exception' => $imagickException]); } - return $data; + + return null; } /** @@ -225,14 +235,9 @@ class Photo * @throws \Friendica\Network\HTTPException\InternalServerErrorException * @throws \ImagickException */ - public static function getImageForPhoto(array $photo) + public static function getImageForPhoto(array $photo): Image { - $data = self::getImageDataForPhoto($photo); - if (empty($data)) { - return null; - } - - return new Image($data, $photo['type']); + return new Image(self::getImageDataForPhoto($photo), $photo['type']); } /** @@ -342,20 +347,20 @@ class Photo // Get defined storage backend. // if no storage backend, we use old "data" column in photo table. // if is an existing photo, reuse same backend - $data = ""; + $data = ""; $backend_ref = ""; + $storage = ""; - if (DBA::isResult($existing_photo)) { - $backend_ref = (string)$existing_photo["backend-ref"]; - $storage = DI::storageManager()->getWritableStorageByName($existing_photo["backend-class"] ?? ''); - } else { - $storage = DI::storage(); - } - - if (empty($storage)) { - $data = $Image->asString(); - } else { + try { + if (DBA::isResult($existing_photo)) { + $backend_ref = (string)$existing_photo["backend-ref"]; + $storage = DI::storageManager()->getWritableStorageByName($existing_photo["backend-class"] ?? ''); + } else { + $storage = DI::storage(); + } $backend_ref = $storage->put($Image->asString(), $backend_ref); + } catch (InvalidClassStorageException $storageException) { + $data = $Image->asString(); } $fields = [ @@ -411,15 +416,15 @@ class Photo $photos = DBA::select('photo', ['id', 'backend-class', 'backend-ref'], $conditions); while ($photo = DBA::fetch($photos)) { - $backend_class = DI::storageManager()->getWritableStorageByName($photo['backend-class'] ?? ''); - if (!empty($backend_class)) { - try { - $backend_class->delete($item['backend-ref'] ?? ''); - // Delete the photos after they had been deleted successfully - DBA::delete("photo", ['id' => $photo['id']]); - } catch (ReferenceStorageException $referenceStorageException) { - DI::logger()->debug('phot doesn\'t exist.', ['conditions' => $conditions, 'exception' => $referenceStorageException]); - } + try { + $backend_class = DI::storageManager()->getWritableStorageByName($photo['backend-class'] ?? ''); + $backend_class->delete($item['backend-ref'] ?? ''); + // Delete the photos after they had been deleted successfully + DBA::delete("photo", ['id' => $photo['id']]); + } catch (InvalidClassStorageException $storageException) { + DI::logger()->debug('Storage class not found.', ['conditions' => $conditions, 'exception' => $storageException]); + } catch (ReferenceStorageException $referenceStorageException) { + DI::logger()->debug('Photo doesn\'t exist.', ['conditions' => $conditions, 'exception' => $referenceStorageException]); } } @@ -448,10 +453,10 @@ class Photo $photos = self::selectToArray(['backend-class', 'backend-ref'], $conditions); foreach($photos as $photo) { - $backend_class = DI::storageManager()->getWritableStorageByName($photo['backend-class'] ?? ''); - if (!empty($backend_class)) { + try { + $backend_class = DI::storageManager()->getWritableStorageByName($photo['backend-class'] ?? ''); $fields["backend-ref"] = $backend_class->put($img->asString(), $photo['backend-ref']); - } else { + } catch (InvalidClassStorageException $storageException) { $fields["data"] = $img->asString(); } } diff --git a/src/Model/Storage/InvalidClassStorageException.php b/src/Model/Storage/InvalidClassStorageException.php new file mode 100644 index 0000000000..9c39b3a60c --- /dev/null +++ b/src/Model/Storage/InvalidClassStorageException.php @@ -0,0 +1,29 @@ +. + * + */ + +namespace Friendica\Model\Storage; + +/** + * Storage Exception in case of invalid storage class + */ +class InvalidClassStorageException extends StorageException +{ +} diff --git a/src/Module/Admin/Storage.php b/src/Module/Admin/Storage.php index e12cecd031..6aac6aace7 100644 --- a/src/Module/Admin/Storage.php +++ b/src/Module/Admin/Storage.php @@ -23,6 +23,7 @@ namespace Friendica\Module\Admin; use Friendica\Core\Renderer; use Friendica\DI; +use Friendica\Model\Storage\InvalidClassStorageException; use Friendica\Model\Storage\IWritableStorage; use Friendica\Module\BaseAdmin; use Friendica\Util\Strings; @@ -37,8 +38,13 @@ class Storage extends BaseAdmin $storagebackend = Strings::escapeTags(trim($parameters['name'] ?? '')); - /** @var IWritableStorage $newstorage */ - $newstorage = DI::storageManager()->getWritableStorageByName($storagebackend); + try { + /** @var IWritableStorage $newstorage */ + $newstorage = DI::storageManager()->getWritableStorageByName($storagebackend); + } catch (InvalidClassStorageException $storageException) { + notice(DI::l10n()->t('Storage backend, %s is invalid.', $storagebackend)); + DI::baseUrl()->redirect('admin/storage'); + } // save storage backend form $storage_opts = $newstorage->getOptions(); @@ -62,16 +68,20 @@ class Storage extends BaseAdmin $storage_form_errors = $newstorage->saveOptions($storage_opts_data); if (count($storage_form_errors)) { foreach ($storage_form_errors as $name => $err) { - notice('Storage backend, ' . $storage_opts[$name][1] . ': ' . $err); + notice(DI::l10n()->t('Storage backend %s error: %s', $storage_opts[$name][1], $err)); } DI::baseUrl()->redirect('admin/storage'); } if (!empty($_POST['submit_save_set'])) { - /** @var IWritableStorage $newstorage */ - $newstorage = DI::storageManager()->getWritableStorageByName($storagebackend); + try { + /** @var IWritableStorage $newstorage */ + $newstorage = DI::storageManager()->getWritableStorageByName($storagebackend); - if (!DI::storageManager()->setBackend($newstorage)) { + if (!DI::storageManager()->setBackend($newstorage)) { + notice(DI::l10n()->t('Invalid storage backend setting value.')); + } + } catch (InvalidClassStorageException $storageException) { notice(DI::l10n()->t('Invalid storage backend setting value.')); } } diff --git a/src/Module/Photo.php b/src/Module/Photo.php index 06a369df77..9d2d31b45d 100644 --- a/src/Module/Photo.php +++ b/src/Module/Photo.php @@ -30,7 +30,11 @@ use Friendica\Model\Photo as MPhoto; use Friendica\Model\Post; use Friendica\Model\Profile; use Friendica\Model\Storage\ExternalResource; +use Friendica\Model\Storage\ReferenceStorageException; +use Friendica\Model\Storage\StorageException; use Friendica\Model\Storage\SystemResource; +use Friendica\Network\HTTPException\InternalServerErrorException; +use Friendica\Network\HTTPException\NotFoundException; use Friendica\Util\Proxy; use Friendica\Object\Image; use Friendica\Util\Images; @@ -105,7 +109,11 @@ class Photo extends BaseModule $cacheable = ($photo["allow_cid"] . $photo["allow_gid"] . $photo["deny_cid"] . $photo["deny_gid"] === "") && (isset($photo["cacheable"]) ? $photo["cacheable"] : true); $stamp = microtime(true); + $imgdata = MPhoto::getImageDataForPhoto($photo); + if (empty($imgdata)) { + throw new NotFoundException(); + } // The mimetype for an external or system resource can only be known reliably after it had been fetched if (in_array($photo['backend-class'], [ExternalResource::NAME, SystemResource::NAME])) { diff --git a/src/Module/Settings/Profile/Photo/Crop.php b/src/Module/Settings/Profile/Photo/Crop.php index 83722c8661..ba96e0032f 100644 --- a/src/Module/Settings/Profile/Photo/Crop.php +++ b/src/Module/Settings/Profile/Photo/Crop.php @@ -61,6 +61,10 @@ class Crop extends BaseSettings $base_image = Photo::selectFirst([], ['resource-id' => $resource_id, 'uid' => local_user(), 'scale' => $scale]); if (DBA::isResult($base_image)) { $Image = Photo::getImageForPhoto($base_image); + if (empty($Image)) { + throw new HTTPException\InternalServerErrorException(); + } + if ($Image->isValid()) { // If setting for the default profile, unset the profile photo flag from any other photos I own DBA::update('photo', ['profile' => 0], ['uid' => local_user()]); @@ -188,6 +192,9 @@ class Crop extends BaseSettings } $Image = Photo::getImageForPhoto($photos[0]); + if (empty($Image)) { + throw new HTTPException\InternalServerErrorException(); + } $imagecrop = [ 'resource-id' => $resource_id, diff --git a/tests/src/Core/StorageManagerTest.php b/tests/src/Core/StorageManagerTest.php index 25ae44b1bc..c4ba9368cf 100644 --- a/tests/src/Core/StorageManagerTest.php +++ b/tests/src/Core/StorageManagerTest.php @@ -34,7 +34,6 @@ use Friendica\Factory\ConfigFactory; use Friendica\Model\Config\Config; use Friendica\Model\Storage; use Friendica\Core\Session; -use Friendica\Model\Storage\StorageException; use Friendica\Network\HTTPRequest; use Friendica\Test\DatabaseTest; use Friendica\Test\Util\Database\StaticDatabase; @@ -47,6 +46,7 @@ use Friendica\Test\Util\SampleStorageBackend; class StorageManagerTest extends DatabaseTest { + use VFSTrait; /** @var Database */ private $dba; /** @var IConfig */ @@ -58,8 +58,6 @@ class StorageManagerTest extends DatabaseTest /** @var HTTPRequest */ private $httpRequest; - use VFSTrait; - protected function setUp(): void { parent::setUp(); @@ -82,6 +80,7 @@ class StorageManagerTest extends DatabaseTest $configModel = new Config($this->dba); $this->config = new PreloadConfig($configCache, $configModel); + $this->config->set('storage', 'name', 'Database'); $this->l10n = \Mockery::mock(L10n::class); @@ -101,34 +100,38 @@ class StorageManagerTest extends DatabaseTest public function dataStorages() { return [ - 'empty' => [ - 'name' => '', - 'assert' => null, - 'assertName' => '', - 'userBackend' => false, + 'empty' => [ + 'name' => '', + 'valid' => false, + 'interface' => Storage\IStorage::class, + 'assert' => null, + 'assertName' => '', ], - 'database' => [ - 'name' => Storage\Database::NAME, - 'assert' => Storage\Database::class, - 'assertName' => Storage\Database::NAME, - 'userBackend' => true, + 'database' => [ + 'name' => Storage\Database::NAME, + 'valid' => true, + 'interface' => Storage\IWritableStorage::class, + 'assert' => Storage\Database::class, + 'assertName' => Storage\Database::NAME, ], - 'filesystem' => [ - 'name' => Storage\Filesystem::NAME, - 'assert' => Storage\Filesystem::class, - 'assertName' => Storage\Filesystem::NAME, - 'userBackend' => true, + 'filesystem' => [ + 'name' => Storage\Filesystem::NAME, + 'valid' => true, + 'interface' => Storage\IWritableStorage::class, + 'assert' => Storage\Filesystem::class, + 'assertName' => Storage\Filesystem::NAME, ], 'systemresource' => [ - 'name' => Storage\SystemResource::NAME, - 'assert' => Storage\SystemResource::class, - 'assertName' => Storage\SystemResource::NAME, - // false here, because SystemResource isn't meant to be a user backend, - // it's for system resources only - 'userBackend' => false, + 'name' => Storage\SystemResource::NAME, + 'valid' => true, + 'interface' => Storage\IStorage::class, + 'assert' => Storage\SystemResource::class, + 'assertName' => Storage\SystemResource::NAME, ], - 'invalid' => [ + 'invalid' => [ 'name' => 'invalid', + 'valid' => false, + 'interface' => null, 'assert' => null, 'assertName' => '', 'userBackend' => false, @@ -136,59 +139,31 @@ class StorageManagerTest extends DatabaseTest ]; } - /** - * Data array for legacy backends - * - * @todo 2020.09 After 2 releases, remove the legacy functionality and these data array with it - * - * @return array - */ - public function dataLegacyBackends() - { - return [ - 'legacyDatabase' => [ - 'name' => 'Friendica\Model\Storage\Database', - 'assert' => Storage\Database::class, - 'assertName' => Storage\Database::NAME, - 'userBackend' => true, - ], - 'legacyFilesystem' => [ - 'name' => 'Friendica\Model\Storage\Filesystem', - 'assert' => Storage\Filesystem::class, - 'assertName' => Storage\Filesystem::NAME, - 'userBackend' => true, - ], - 'legacySystemResource' => [ - 'name' => 'Friendica\Model\Storage\SystemResource', - 'assert' => Storage\SystemResource::class, - 'assertName' => Storage\SystemResource::NAME, - 'userBackend' => false, - ], - ]; - } - /** * Test the getByName() method * * @dataProvider dataStorages - * @dataProvider dataLegacyBackends */ - public function testGetByName($name, $assert, $assertName, $userBackend) + public function testGetByName($name, $valid, $interface, $assert, $assertName) { - $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->l10n, $this->httpRequest); + if (!$valid) { + $this->expectException(Storage\InvalidClassStorageException::class); + } - if ($userBackend) { + if ($interface === Storage\IWritableStorage::class) { + $this->config->set('storage', 'name', $name); + } + + $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->l10n); + + if ($interface === Storage\IWritableStorage::class) { $storage = $storageManager->getWritableStorageByName($name); } else { $storage = $storageManager->getByName($name); } - if (!empty($assert)) { - self::assertInstanceOf(Storage\IStorage::class, $storage); - self::assertInstanceOf($assert, $storage); - } else { - self::assertNull($storage); - } + self::assertInstanceOf($interface, $storage); + self::assertInstanceOf($assert, $storage); self::assertEquals($assertName, $storage); } @@ -197,15 +172,15 @@ class StorageManagerTest extends DatabaseTest * * @dataProvider dataStorages */ - public function testIsValidBackend($name, $assert, $assertName, $userBackend) + public function testIsValidBackend($name, $valid, $interface, $assert, $assertName) { $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->l10n); // true in every of the backends self::assertEquals(!empty($assertName), $storageManager->isValidBackend($name)); - // if userBackend is set to true, filter out e.g. SystemRessource - self::assertEquals($userBackend, $storageManager->isValidBackend($name, true)); + // if it's a IWritableStorage, the valid backend should return true, otherwise false + self::assertEquals($interface === Storage\IWritableStorage::class, $storageManager->isValidBackend($name, StorageManager::DEFAULT_BACKENDS)); } /** @@ -223,37 +198,35 @@ class StorageManagerTest extends DatabaseTest * * @dataProvider dataStorages */ - public function testGetBackend($name, $assert, $assertName, $userBackend) + public function testGetBackend($name, $valid, $interface, $assert, $assertName) { + if ($interface !== Storage\IWritableStorage::class) { + static::markTestSkipped('only works for IWritableStorage'); + } + $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->l10n); - self::assertNull($storageManager->getBackend()); + $selBackend = $storageManager->getWritableStorageByName($name); + $storageManager->setBackend($selBackend); - if ($userBackend) { - $selBackend = $storageManager->getWritableStorageByName($name); - $storageManager->setBackend($selBackend); - - self::assertInstanceOf($assert, $storageManager->getBackend()); - } + self::assertInstanceOf($assert, $storageManager->getBackend()); } /** * Test the method getBackend() with a pre-configured backend * * @dataProvider dataStorages - * @dataProvider dataLegacyBackends */ - public function testPresetBackend($name, $assert, $assertName, $userBackend) + public function testPresetBackend($name, $valid, $interface, $assert, $assertName) { $this->config->set('storage', 'name', $name); + if ($interface !== Storage\IWritableStorage::class) { + $this->expectException(Storage\InvalidClassStorageException::class); + } $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->l10n); - if ($userBackend) { - self::assertInstanceOf($assert, $storageManager->getBackend()); - } else { - self::assertNull($storageManager->getBackend()); - } + self::assertInstanceOf($assert, $storageManager->getBackend()); } /** @@ -266,7 +239,7 @@ class StorageManagerTest extends DatabaseTest public function testRegisterUnregisterBackends() { /// @todo Remove dice once "Hook" is dynamic and mockable - $dice = (new Dice()) + $dice = (new Dice()) ->addRules(include __DIR__ . '/../../../static/dependencies.config.php') ->addRule(Database::class, ['instanceOf' => StaticDatabase::class, 'shared' => true]) ->addRule(ISession::class, ['instanceOf' => Session\Memory::class, 'shared' => true, 'call' => null]); @@ -287,7 +260,7 @@ class StorageManagerTest extends DatabaseTest SampleStorageBackend::registerHook(); Hook::loadHooks(); - self::assertTrue($storageManager->setBackend( $storageManager->getWritableStorageByName(SampleStorageBackend::NAME))); + self::assertTrue($storageManager->setBackend($storageManager->getWritableStorageByName(SampleStorageBackend::NAME))); self::assertEquals(SampleStorageBackend::NAME, $this->config->get('storage', 'name')); self::assertInstanceOf(SampleStorageBackend::class, $storageManager->getBackend()); @@ -305,26 +278,25 @@ class StorageManagerTest extends DatabaseTest * * @dataProvider dataStorages */ - public function testMoveStorage($name, $assert, $assertName, $userBackend) + public function testMoveStorage($name, $valid, $interface, $assert, $assertName) { - if (!$userBackend) { + if ($interface !== Storage\IWritableStorage::class) { self::markTestSkipped("No user backend"); } $this->loadFixture(__DIR__ . '/../../datasets/storage/database.fixture.php', $this->dba); $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->l10n); - $storage = $storageManager->getWritableStorageByName($name); + $storage = $storageManager->getWritableStorageByName($name); $storageManager->move($storage); $photos = $this->dba->select('photo', ['backend-ref', 'backend-class', 'id', 'data']); while ($photo = $this->dba->fetch($photos)) { - self::assertEmpty($photo['data']); $storage = $storageManager->getByName($photo['backend-class']); - $data = $storage->get($photo['backend-ref']); + $data = $storage->get($photo['backend-ref']); self::assertNotEmpty($data); } @@ -335,10 +307,11 @@ class StorageManagerTest extends DatabaseTest */ public function testWrongWritableStorage() { - $this->expectException(\TypeError::class); + $this->expectException(Storage\InvalidClassStorageException::class); + $this->expectExceptionMessage('Backend SystemResource is not valid'); $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->l10n); - $storage = $storageManager->getWritableStorageByName(Storage\SystemResource::getName()); + $storage = $storageManager->getWritableStorageByName(Storage\SystemResource::getName()); $storageManager->move($storage); } } From f4941b5b49ecc3a025b02e435f353f86bac0ea38 Mon Sep 17 00:00:00 2001 From: Philipp Date: Sun, 15 Aug 2021 13:55:04 +0200 Subject: [PATCH 12/19] Update src/Model/Storage/IWritableStorage.php Co-authored-by: Hypolite Petovan --- src/Model/Storage/IWritableStorage.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Model/Storage/IWritableStorage.php b/src/Model/Storage/IWritableStorage.php index 650b3246ed..ee0001a669 100644 --- a/src/Model/Storage/IWritableStorage.php +++ b/src/Model/Storage/IWritableStorage.php @@ -25,7 +25,7 @@ namespace Friendica\Model\Storage; * Interface for writable storage backends * * Used for storages with CRUD functionality, mainly used for user data (e.g. photos, attachements). - * There's only one active, writable storage possible. This type of storages are selectable by the current administrator + * There's only one active writable storage possible. This type of storage is selectable by the current administrator. */ interface IWritableStorage extends IStorage { From 1df62258685572a91d5ed0028a731df20be54c75 Mon Sep 17 00:00:00 2001 From: Philipp Date: Sun, 15 Aug 2021 13:55:09 +0200 Subject: [PATCH 13/19] Update src/Model/Storage/Filesystem.php Co-authored-by: Hypolite Petovan --- src/Model/Storage/Filesystem.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Model/Storage/Filesystem.php b/src/Model/Storage/Filesystem.php index 2d6a352015..c6c939bd46 100644 --- a/src/Model/Storage/Filesystem.php +++ b/src/Model/Storage/Filesystem.php @@ -127,7 +127,6 @@ class Filesystem implements IWritableStorage $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)); } From c06ba3a7c993503da37c4e5ba7a469cf4a3a8f52 Mon Sep 17 00:00:00 2001 From: Philipp Date: Sun, 15 Aug 2021 19:54:22 +0200 Subject: [PATCH 14/19] Update db version --- update.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/update.php b/update.php index 5d9c76a994..536610a00e 100644 --- a/update.php +++ b/update.php @@ -982,7 +982,7 @@ function update_1429() return Update::SUCCESS; } -function update_1433() +function update_1434() { $name = DI::config()->get('storage', 'name'); From 02a4d30f7d44914a2af50f916d61cf5ec219d0ea Mon Sep 17 00:00:00 2001 From: Philipp Date: Sun, 15 Aug 2021 20:40:23 +0200 Subject: [PATCH 15/19] Add todo for later --- src/Model/Photo.php | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/Model/Photo.php b/src/Model/Photo.php index 8df9b92f2d..a59c30aca2 100644 --- a/src/Model/Photo.php +++ b/src/Model/Photo.php @@ -196,13 +196,8 @@ class Photo try { $backendClass = DI::storageManager()->getByName($photo['backend-class'] ?? ''); - $image = $backendClass->get($photo['backend-ref'] ?? ''); - - if ($image instanceof Image) { - return $image; - } else { - DI::logger()->info('Stored data is not an image', ['photo' => $photo]); - } + /// @todo refactoring this returning, because the storage returns a "string" which is casted in different ways - a check "instanceof Image" will fail! + return $backendClass->get($photo['backend-ref'] ?? ''); } catch (InvalidClassStorageException $storageException) { try { // legacy data storage in "data" column From 1901716479a3adcbc778e2a1da640c9c6aa4d3cd Mon Sep 17 00:00:00 2001 From: Philipp Date: Mon, 16 Aug 2021 00:08:06 +0200 Subject: [PATCH 16/19] Lower complexity for valid backends (replace hashmap with a "simple" name array) --- src/Core/StorageManager.php | 67 +++++++++++++++++---------- tests/src/Core/StorageManagerTest.php | 40 +++++++++++++--- 2 files changed, 76 insertions(+), 31 deletions(-) diff --git a/src/Core/StorageManager.php b/src/Core/StorageManager.php index 132cd64538..6e0e0796bd 100644 --- a/src/Core/StorageManager.php +++ b/src/Core/StorageManager.php @@ -40,12 +40,14 @@ class StorageManager const TABLES = ['photo', 'attach']; // Default storage backends + /** @var string[] */ const DEFAULT_BACKENDS = [ - Storage\Filesystem::NAME => Storage\Filesystem::class, - Storage\Database::NAME => Storage\Database::class, + Storage\Filesystem::NAME, + Storage\Database::NAME, ]; - private $backends; + /** @var string[] List of valid backend classes */ + private $validBackends; /** * @var Storage\IStorage[] A local cache for storage instances @@ -79,7 +81,7 @@ class StorageManager $this->config = $config; $this->logger = $logger; $this->l10n = $l10n; - $this->backends = $config->get('storage', 'backends', self::DEFAULT_BACKENDS); + $this->validBackends = $config->get('storage', 'backends', self::DEFAULT_BACKENDS); $currentName = $this->config->get('storage', 'name'); @@ -109,7 +111,7 @@ class StorageManager */ public function getWritableStorageByName(string $name): Storage\IWritableStorage { - $storage = $this->getByName($name, $this->backends); + $storage = $this->getByName($name, $this->validBackends); if ($storage instanceof Storage\IWritableStorage) { return $storage; } else { @@ -121,7 +123,7 @@ class StorageManager * Return storage backend class by registered name * * @param string $name Backend name - * @param array|null $validBackends possible, manual override of the valid backends + * @param string[]|null $validBackends possible, manual override of the valid backends * * @return Storage\IStorage * @@ -178,19 +180,19 @@ class StorageManager /** * Checks, if the storage is a valid backend * - * @param string|null $name The name or class of the backend - * @param array|null $validBackends Possible, valid backends to check + * @param string|null $name The name or class of the backend + * @param string[]|null $validBackends Possible, valid backends to check * * @return boolean True, if the backend is a valid backend */ public function isValidBackend(string $name = null, array $validBackends = null): bool { - $validBackends = $validBackends ?? array_merge($this->backends, + $validBackends = $validBackends ?? array_merge($this->validBackends, [ - Storage\SystemResource::getName() => '', - Storage\ExternalResource::getName() => '' + Storage\SystemResource::getName(), + Storage\ExternalResource::getName(), ]); - return array_key_exists($name, $validBackends); + return in_array($name, $validBackends); } /** @@ -213,11 +215,11 @@ class StorageManager /** * Get registered backends * - * @return array + * @return Storage\IWritableStorage[] */ public function listBackends(): array { - return $this->backends; + return $this->validBackends; } /** @@ -234,11 +236,15 @@ class StorageManager if (is_subclass_of($class, Storage\IStorage::class)) { /** @var Storage\IStorage $class */ - $backends = $this->backends; - $backends[$class::getName()] = $class; + if (array_search($class::getName(), $this->validBackends, true) !== false) { + return true; + } + + $backends = $this->validBackends; + $backends[] = $class::getName(); if ($this->config->set('storage', 'backends', $backends)) { - $this->backends = $backends; + $this->validBackends = $backends; return true; } else { return false; @@ -254,20 +260,33 @@ class StorageManager * @param string $class Backend class name * * @return boolean True, if unregistering was successful + * + * @throws Storage\StorageException */ public function unregister(string $class): bool { if (is_subclass_of($class, Storage\IStorage::class)) { /** @var Storage\IStorage $class */ - unset($this->backends[$class::getName()]); - - if ($this->currentBackend instanceof $class) { - $this->config->set('storage', 'name', null); - $this->currentBackend = null; + if ($this->currentBackend::getName() == $class::getName()) { + throw new Storage\StorageException(sprintf('Cannot unregister %s, because it\'s currently active.', $class::getName())); } - return $this->config->set('storage', 'backends', $this->backends); + $key = array_search($class::getName(), $this->validBackends); + + if ($key !== false) { + $backends = $this->validBackends; + unset($backends[$key]); + $backends = array_values($backends); + if ($this->config->set('storage', 'backends', $backends)) { + $this->validBackends = $backends; + return true; + } else { + return false; + } + } else { + return true; + } } else { return false; } @@ -289,7 +308,7 @@ class StorageManager */ public function move(Storage\IWritableStorage $destination, array $tables = self::TABLES, int $limit = 5000): int { - if (!$this->isValidBackend($destination, $this->backends)) { + if (!$this->isValidBackend($destination, $this->validBackends)) { throw new Storage\StorageException(sprintf("Can't move to storage backend '%s'", $destination::getName())); } diff --git a/tests/src/Core/StorageManagerTest.php b/tests/src/Core/StorageManagerTest.php index c4ba9368cf..9e8e3aa2c2 100644 --- a/tests/src/Core/StorageManagerTest.php +++ b/tests/src/Core/StorageManagerTest.php @@ -250,10 +250,38 @@ class StorageManagerTest extends DatabaseTest self::assertTrue($storageManager->register(SampleStorageBackend::class)); self::assertEquals(array_merge(StorageManager::DEFAULT_BACKENDS, [ - SampleStorageBackend::getName() => SampleStorageBackend::class, + SampleStorageBackend::getName(), ]), $storageManager->listBackends()); self::assertEquals(array_merge(StorageManager::DEFAULT_BACKENDS, [ - SampleStorageBackend::getName() => SampleStorageBackend::class, + SampleStorageBackend::getName() + ]), $this->config->get('storage', 'backends')); + + self::assertTrue($storageManager->unregister(SampleStorageBackend::class)); + self::assertEquals(StorageManager::DEFAULT_BACKENDS, $this->config->get('storage', 'backends')); + self::assertEquals(StorageManager::DEFAULT_BACKENDS, $storageManager->listBackends()); + } + + /** + * tests that an active backend cannot get unregistered + */ + public function testUnregisterActiveBackend() + { + /// @todo Remove dice once "Hook" is dynamic and mockable + $dice = (new Dice()) + ->addRules(include __DIR__ . '/../../../static/dependencies.config.php') + ->addRule(Database::class, ['instanceOf' => StaticDatabase::class, 'shared' => true]) + ->addRule(ISession::class, ['instanceOf' => Session\Memory::class, 'shared' => true, 'call' => null]); + DI::init($dice); + + $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->l10n); + + self::assertTrue($storageManager->register(SampleStorageBackend::class)); + + self::assertEquals(array_merge(StorageManager::DEFAULT_BACKENDS, [ + SampleStorageBackend::getName(), + ]), $storageManager->listBackends()); + self::assertEquals(array_merge(StorageManager::DEFAULT_BACKENDS, [ + SampleStorageBackend::getName() ]), $this->config->get('storage', 'backends')); // inline call to register own class as hook (testing purpose only) @@ -265,12 +293,10 @@ class StorageManagerTest extends DatabaseTest self::assertInstanceOf(SampleStorageBackend::class, $storageManager->getBackend()); - self::assertTrue($storageManager->unregister(SampleStorageBackend::class)); - self::assertEquals(StorageManager::DEFAULT_BACKENDS, $this->config->get('storage', 'backends')); - self::assertEquals(StorageManager::DEFAULT_BACKENDS, $storageManager->listBackends()); + self::expectException(Storage\StorageException::class); + self::expectExceptionMessage('Cannot unregister Sample Storage, because it\'s currently active.'); - self::assertNull($storageManager->getBackend()); - self::assertNull($this->config->get('storage', 'name')); + $storageManager->unregister(SampleStorageBackend::class); } /** From 99239e3d99d8d97419af830c0e22c09faba4c133 Mon Sep 17 00:00:00 2001 From: Philipp Date: Mon, 16 Aug 2021 00:10:37 +0200 Subject: [PATCH 17/19] Fix usage --- src/Console/Storage.php | 2 +- src/Core/StorageManager.php | 2 +- src/Module/Admin/Storage.php | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Console/Storage.php b/src/Console/Storage.php index e2aa185c92..3377f33ddf 100644 --- a/src/Console/Storage.php +++ b/src/Console/Storage.php @@ -106,7 +106,7 @@ HELP; $this->out(sprintf($rowfmt, 'Sel', 'Name')); $this->out('-----------------------'); $isregisterd = false; - foreach ($this->storageManager->listBackends() as $name => $class) { + foreach ($this->storageManager->listBackends() as $name) { $issel = ' '; if ($current && $current::getName() == $name) { $issel = '*'; diff --git a/src/Core/StorageManager.php b/src/Core/StorageManager.php index 6e0e0796bd..985547b0f8 100644 --- a/src/Core/StorageManager.php +++ b/src/Core/StorageManager.php @@ -215,7 +215,7 @@ class StorageManager /** * Get registered backends * - * @return Storage\IWritableStorage[] + * @return string[] */ public function listBackends(): array { diff --git a/src/Module/Admin/Storage.php b/src/Module/Admin/Storage.php index 6aac6aace7..a2172503f5 100644 --- a/src/Module/Admin/Storage.php +++ b/src/Module/Admin/Storage.php @@ -96,7 +96,7 @@ class Storage extends BaseAdmin $current_storage_backend = DI::storage(); $available_storage_forms = []; - foreach (DI::storageManager()->listBackends() as $name => $class) { + foreach (DI::storageManager()->listBackends() as $name) { // build storage config form, $storage_form_prefix = preg_replace('|[^a-zA-Z0-9]|', '', $name); From db6fded5d2a71880decf85698489d23fc356e018 Mon Sep 17 00:00:00 2001 From: Philipp Date: Mon, 16 Aug 2021 23:09:39 +0200 Subject: [PATCH 18/19] Update src/Core/StorageManager.php Co-authored-by: Hypolite Petovan --- src/Core/StorageManager.php | 86 ++++++++++++++++++------------------- 1 file changed, 43 insertions(+), 43 deletions(-) diff --git a/src/Core/StorageManager.php b/src/Core/StorageManager.php index 985547b0f8..f206868885 100644 --- a/src/Core/StorageManager.php +++ b/src/Core/StorageManager.php @@ -77,10 +77,10 @@ class StorageManager */ public function __construct(Database $dba, IConfig $config, LoggerInterface $logger, L10n $l10n) { - $this->dba = $dba; - $this->config = $config; - $this->logger = $logger; - $this->l10n = $l10n; + $this->dba = $dba; + $this->config = $config; + $this->logger = $logger; + $this->l10n = $l10n; $this->validBackends = $config->get('storage', 'backends', self::DEFAULT_BACKENDS); $currentName = $this->config->get('storage', 'name'); @@ -112,11 +112,11 @@ class StorageManager public function getWritableStorageByName(string $name): Storage\IWritableStorage { $storage = $this->getByName($name, $this->validBackends); - if ($storage instanceof Storage\IWritableStorage) { - return $storage; - } else { + if (!$storage instanceof Storage\IWritableStorage) { throw new Storage\InvalidClassStorageException(sprintf('Backend %s is not writable', $name)); } + + return $storage; } /** @@ -135,43 +135,43 @@ class StorageManager // If there's no cached instance create a new instance if (!isset($this->backendInstances[$name])) { // If the current name isn't a valid backend (or the SystemResource instance) create it - if ($this->isValidBackend($name, $validBackends)) { - switch ($name) { - // Try the filesystem backend - case Storage\Filesystem::getName(): - $this->backendInstances[$name] = new Storage\Filesystem($this->config, $this->l10n); - break; - // try the database backend - case Storage\Database::getName(): - $this->backendInstances[$name] = new Storage\Database($this->dba); - break; - // at least, try if there's an addon for the backend - case Storage\SystemResource::getName(): - $this->backendInstances[$name] = new Storage\SystemResource(); - break; - case Storage\ExternalResource::getName(): - $this->backendInstances[$name] = new Storage\ExternalResource(); - break; - default: - $data = [ - 'name' => $name, - 'storage' => null, - ]; - try { - Hook::callAll('storage_instance', $data); - if (($data['storage'] ?? null) instanceof Storage\IStorage) { - $this->backendInstances[$data['name'] ?? $name] = $data['storage']; - } else { - throw new Storage\InvalidClassStorageException(sprintf('Backend %s was not found', $name)); - } - } catch (InternalServerErrorException $exception) { - throw new Storage\StorageException(sprintf('Failed calling hook::storage_instance for backend %s', $name), $exception); - } - break; - } - } else { + if (!$this->isValidBackend($name, $validBackends)) { throw new Storage\InvalidClassStorageException(sprintf('Backend %s is not valid', $name)); } + + switch ($name) { + // Try the filesystem backend + case Storage\Filesystem::getName(): + $this->backendInstances[$name] = new Storage\Filesystem($this->config, $this->l10n); + break; + // try the database backend + case Storage\Database::getName(): + $this->backendInstances[$name] = new Storage\Database($this->dba); + break; + // at least, try if there's an addon for the backend + case Storage\SystemResource::getName(): + $this->backendInstances[$name] = new Storage\SystemResource(); + break; + case Storage\ExternalResource::getName(): + $this->backendInstances[$name] = new Storage\ExternalResource(); + break; + default: + $data = [ + 'name' => $name, + 'storage' => null, + ]; + try { + Hook::callAll('storage_instance', $data); + if (!($data['storage'] ?? null) instanceof Storage\IStorage) { + throw new Storage\InvalidClassStorageException(sprintf('Backend %s was not found', $name)); + } + + $this->backendInstances[$data['name'] ?? $name] = $data['storage']; + } catch (InternalServerErrorException $exception) { + throw new Storage\StorageException(sprintf('Failed calling hook::storage_instance for backend %s', $name), $exception); + } + break; + } } return $this->backendInstances[$name]; @@ -236,7 +236,7 @@ class StorageManager if (is_subclass_of($class, Storage\IStorage::class)) { /** @var Storage\IStorage $class */ - if (array_search($class::getName(), $this->validBackends, true) !== false) { + if ($this->isValidBackend($class::getName(), $this->validBackends)) { return true; } From cbc9d827d10ac7d10b8434c3d28006c9d8618c6c Mon Sep 17 00:00:00 2001 From: Philipp Date: Mon, 16 Aug 2021 23:32:20 +0200 Subject: [PATCH 19/19] Update messages.po --- view/lang/C/messages.po | 98 ++++++++++++++++++----------------------- 1 file changed, 43 insertions(+), 55 deletions(-) diff --git a/view/lang/C/messages.po b/view/lang/C/messages.po index c4b37aee96..ac716a2860 100644 --- a/view/lang/C/messages.po +++ b/view/lang/C/messages.po @@ -8,7 +8,7 @@ msgid "" msgstr "" "Project-Id-Version: 2021.09-dev\n" "Report-Msgid-Bugs-To: \n" -"POT-Creation-Date: 2021-08-16 06:14+0000\n" +"POT-Creation-Date: 2021-08-16 23:28+0200\n" "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n" "Last-Translator: FULL NAME \n" "Language-Team: LANGUAGE \n" @@ -40,10 +40,10 @@ msgstr "" #: include/api.php:4437 mod/photos.php:86 mod/photos.php:195 mod/photos.php:623 #: mod/photos.php:1031 mod/photos.php:1048 mod/photos.php:1594 #: src/Model/User.php:1112 src/Model/User.php:1120 src/Model/User.php:1128 -#: src/Module/Settings/Profile/Photo/Crop.php:97 -#: src/Module/Settings/Profile/Photo/Crop.php:113 -#: src/Module/Settings/Profile/Photo/Crop.php:129 -#: src/Module/Settings/Profile/Photo/Crop.php:175 +#: src/Module/Settings/Profile/Photo/Crop.php:101 +#: src/Module/Settings/Profile/Photo/Crop.php:117 +#: src/Module/Settings/Profile/Photo/Crop.php:133 +#: src/Module/Settings/Profile/Photo/Crop.php:179 #: src/Module/Settings/Profile/Photo/Index.php:95 #: src/Module/Settings/Profile/Photo/Index.php:101 msgid "Profile Photos" @@ -851,7 +851,7 @@ msgstr "" #: src/Module/Search/Directory.php:38 src/Module/Settings/Delegation.php:42 #: src/Module/Settings/Delegation.php:70 src/Module/Settings/Display.php:43 #: src/Module/Settings/Display.php:121 -#: src/Module/Settings/Profile/Photo/Crop.php:154 +#: src/Module/Settings/Profile/Photo/Crop.php:158 #: src/Module/Settings/Profile/Photo/Index.php:112 #: src/Module/Settings/UserExport.php:58 src/Module/Settings/UserExport.php:93 #: src/Module/Settings/UserExport.php:199 @@ -969,7 +969,7 @@ msgid "Edit post" msgstr "" #: mod/editpost.php:91 mod/notes.php:56 src/Content/Text/HTML.php:885 -#: src/Module/Admin/Storage.php:120 src/Module/Filer/SaveTag.php:69 +#: src/Module/Admin/Storage.php:133 src/Module/Filer/SaveTag.php:69 msgid "Save" msgstr "" @@ -2716,7 +2716,7 @@ msgstr "" msgid "File upload failed." msgstr "" -#: mod/wall_upload.php:233 src/Model/Photo.php:1002 +#: mod/wall_upload.php:233 src/Model/Photo.php:1013 msgid "Wall Photos" msgstr "" @@ -4689,39 +4689,17 @@ msgstr "" msgid "OpenWebAuth: %1$s welcomes %2$s" msgstr "" -#: src/Model/Storage/Database.php:74 -#, php-format -msgid "Database storage failed to update %s" -msgstr "" - -#: src/Model/Storage/Database.php:82 -msgid "Database storage failed to insert data" -msgstr "" - -#: src/Model/Storage/Filesystem.php:100 -#, php-format -msgid "" -"Filesystem storage failed to create \"%s\". Check you write permissions." -msgstr "" - -#: src/Model/Storage/Filesystem.php:148 -#, php-format -msgid "" -"Filesystem storage failed to save data to \"%s\". Check your write " -"permissions" -msgstr "" - -#: src/Model/Storage/Filesystem.php:176 +#: src/Model/Storage/Filesystem.php:187 msgid "Storage base path" msgstr "" -#: src/Model/Storage/Filesystem.php:178 +#: src/Model/Storage/Filesystem.php:189 msgid "" "Folder where uploaded files are saved. For maximum security, This should be " "a path outside web server folder tree" msgstr "" -#: src/Model/Storage/Filesystem.php:191 +#: src/Model/Storage/Filesystem.php:202 msgid "Enter a valid existing folder" msgstr "" @@ -5004,7 +4982,7 @@ msgstr "" #: src/Module/Admin/Blocklist/Server.php:88 src/Module/Admin/Federation.php:159 #: src/Module/Admin/Item/Delete.php:65 src/Module/Admin/Logs/Settings.php:80 #: src/Module/Admin/Logs/View.php:64 src/Module/Admin/Queue.php:72 -#: src/Module/Admin/Site.php:498 src/Module/Admin/Storage.php:118 +#: src/Module/Admin/Site.php:498 src/Module/Admin/Storage.php:131 #: src/Module/Admin/Summary.php:232 src/Module/Admin/Themes/Details.php:90 #: src/Module/Admin/Themes/Index.php:111 src/Module/Admin/Tos.php:58 #: src/Module/Admin/Users/Active.php:136 src/Module/Admin/Users/Blocked.php:137 @@ -6463,31 +6441,41 @@ msgstr "" msgid "Start Relocation" msgstr "" -#: src/Module/Admin/Storage.php:72 +#: src/Module/Admin/Storage.php:45 +#, php-format +msgid "Storage backend, %s is invalid." +msgstr "" + +#: src/Module/Admin/Storage.php:71 +#, php-format +msgid "Storage backend %s error: %s" +msgstr "" + +#: src/Module/Admin/Storage.php:82 src/Module/Admin/Storage.php:85 msgid "Invalid storage backend setting value." msgstr "" -#: src/Module/Admin/Storage.php:119 src/Module/BaseAdmin.php:91 +#: src/Module/Admin/Storage.php:132 src/Module/BaseAdmin.php:91 msgid "Storage" msgstr "" -#: src/Module/Admin/Storage.php:121 +#: src/Module/Admin/Storage.php:134 msgid "Save & Activate" msgstr "" -#: src/Module/Admin/Storage.php:122 +#: src/Module/Admin/Storage.php:135 msgid "Activate" msgstr "" -#: src/Module/Admin/Storage.php:123 +#: src/Module/Admin/Storage.php:136 msgid "Save & Reload" msgstr "" -#: src/Module/Admin/Storage.php:124 +#: src/Module/Admin/Storage.php:137 msgid "This backend doesn't have custom settings" msgstr "" -#: src/Module/Admin/Storage.php:127 +#: src/Module/Admin/Storage.php:140 msgid "Database (legacy)" msgstr "" @@ -8700,17 +8688,17 @@ msgstr "" msgid "Visible to:" msgstr "" -#: src/Module/Photo.php:94 +#: src/Module/Photo.php:98 #, php-format msgid "The Photo with id %s is not available." msgstr "" -#: src/Module/Photo.php:124 +#: src/Module/Photo.php:132 #, php-format msgid "Invalid external resource with url %s." msgstr "" -#: src/Module/Photo.php:126 +#: src/Module/Photo.php:134 #, php-format msgid "Invalid photo with id %s." msgstr "" @@ -9465,42 +9453,42 @@ msgid "" "contacts or the Friendica contacts in the selected groups.

" msgstr "" -#: src/Module/Settings/Profile/Photo/Crop.php:102 -#: src/Module/Settings/Profile/Photo/Crop.php:118 -#: src/Module/Settings/Profile/Photo/Crop.php:134 +#: src/Module/Settings/Profile/Photo/Crop.php:106 +#: src/Module/Settings/Profile/Photo/Crop.php:122 +#: src/Module/Settings/Profile/Photo/Crop.php:138 #: src/Module/Settings/Profile/Photo/Index.php:102 #, php-format msgid "Image size reduction [%s] failed." msgstr "" -#: src/Module/Settings/Profile/Photo/Crop.php:139 +#: src/Module/Settings/Profile/Photo/Crop.php:143 msgid "" "Shift-reload the page or clear browser cache if the new photo does not " "display immediately." msgstr "" -#: src/Module/Settings/Profile/Photo/Crop.php:144 +#: src/Module/Settings/Profile/Photo/Crop.php:148 msgid "Unable to process image" msgstr "" -#: src/Module/Settings/Profile/Photo/Crop.php:163 +#: src/Module/Settings/Profile/Photo/Crop.php:167 msgid "Photo not found." msgstr "" -#: src/Module/Settings/Profile/Photo/Crop.php:185 +#: src/Module/Settings/Profile/Photo/Crop.php:189 msgid "Profile picture successfully updated." msgstr "" -#: src/Module/Settings/Profile/Photo/Crop.php:208 -#: src/Module/Settings/Profile/Photo/Crop.php:212 +#: src/Module/Settings/Profile/Photo/Crop.php:215 +#: src/Module/Settings/Profile/Photo/Crop.php:219 msgid "Crop Image" msgstr "" -#: src/Module/Settings/Profile/Photo/Crop.php:209 +#: src/Module/Settings/Profile/Photo/Crop.php:216 msgid "Please adjust the image cropping for optimum viewing." msgstr "" -#: src/Module/Settings/Profile/Photo/Crop.php:211 +#: src/Module/Settings/Profile/Photo/Crop.php:218 msgid "Use Image As Is" msgstr ""