From 065b46c72156a52b595b054be30fdd7b6796e76b Mon Sep 17 00:00:00 2001 From: Philipp Date: Mon, 4 Oct 2021 10:25:29 +0200 Subject: [PATCH 1/6] Split Storage usage and Storage configuration --- src/Core/StorageManager.php | 40 +++++++- src/Model/Storage/Database.php | 16 --- src/Model/Storage/Filesystem.php | 52 +--------- src/Model/Storage/FilesystemConfig.php | 99 +++++++++++++++++++ src/Model/Storage/IStorageConfiguration.php | 78 +++++++++++++++ src/Model/Storage/IWritableStorage.php | 50 ---------- src/Module/Admin/Storage.php | 82 ++++++++------- .../src/Model/Storage/DatabaseStorageTest.php | 13 +-- .../Storage/FilesystemStorageConfigTest.php | 67 +++++++++++++ .../Model/Storage/FilesystemStorageTest.php | 35 +------ tests/src/Model/Storage/StorageConfigTest.php | 43 ++++++++ tests/src/Model/Storage/StorageTest.php | 12 --- 12 files changed, 379 insertions(+), 208 deletions(-) create mode 100644 src/Model/Storage/FilesystemConfig.php create mode 100644 src/Model/Storage/IStorageConfiguration.php create mode 100644 tests/src/Model/Storage/FilesystemStorageConfigTest.php create mode 100644 tests/src/Model/Storage/StorageConfigTest.php diff --git a/src/Core/StorageManager.php b/src/Core/StorageManager.php index f206868885..61dd8d375d 100644 --- a/src/Core/StorageManager.php +++ b/src/Core/StorageManager.php @@ -119,6 +119,43 @@ class StorageManager return $storage; } + /** + * Return storage backend configuration by registered name + * + * @param string $name Backend name + * + * @return Storage\IStorageConfiguration|false + * + * @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 getConfigurationByName(string $name) + { + switch ($name) { + // Try the filesystem backend + case Storage\Filesystem::getName(): + return new Storage\FilesystemConfig($this->config, $this->l10n); + // try the database backend + case Storage\Database::getName(): + return false; + default: + $data = [ + 'name' => $name, + 'storage_config' => null, + ]; + try { + Hook::callAll('storage_config', $data); + if (!($data['storage_config'] ?? null) instanceof Storage\IStorageConfiguration) { + throw new Storage\InvalidClassStorageException(sprintf('Configuration for backend %s was not found', $name)); + } + + return $data['storage_config']; + } catch (InternalServerErrorException $exception) { + throw new Storage\StorageException(sprintf('Failed calling hook::storage_config for backend %s', $name), $exception); + } + } + } + /** * Return storage backend class by registered name * @@ -142,7 +179,8 @@ class StorageManager switch ($name) { // Try the filesystem backend case Storage\Filesystem::getName(): - $this->backendInstances[$name] = new Storage\Filesystem($this->config, $this->l10n); + $storageConfig = new Storage\FilesystemConfig($this->config, $this->l10n); + $this->backendInstances[$name] = new Storage\Filesystem($storageConfig->getStoragePath()); break; // try the database backend case Storage\Database::getName(): diff --git a/src/Model/Storage/Database.php b/src/Model/Storage/Database.php index 3457fa4a30..7b90d87890 100644 --- a/src/Model/Storage/Database.php +++ b/src/Model/Storage/Database.php @@ -113,22 +113,6 @@ class Database implements IWritableStorage } } - /** - * @inheritDoc - */ - public function getOptions(): array - { - return []; - } - - /** - * @inheritDoc - */ - public function saveOptions(array $data): array - { - return []; - } - /** * @inheritDoc */ diff --git a/src/Model/Storage/Filesystem.php b/src/Model/Storage/Filesystem.php index c6c939bd46..5a4ea1f0d3 100644 --- a/src/Model/Storage/Filesystem.php +++ b/src/Model/Storage/Filesystem.php @@ -22,8 +22,6 @@ namespace Friendica\Model\Storage; use Exception; -use Friendica\Core\Config\IConfig; -use Friendica\Core\L10n; use Friendica\Util\Strings; /** @@ -40,30 +38,17 @@ class Filesystem implements IWritableStorage { const NAME = 'Filesystem'; - // Default base folder - const DEFAULT_BASE_FOLDER = 'storage'; - - /** @var IConfig */ - private $config; - /** @var string */ private $basePath; - /** @var L10n */ - private $l10n; - /** * Filesystem constructor. * - * @param IConfig $config - * @param L10n $l10n + * @param string $filesystemPath */ - public function __construct(IConfig $config, L10n $l10n) + public function __construct(string $filesystemPath = FilesystemConfig::DEFAULT_BASE_FOLDER) { - $this->config = $config; - $this->l10n = $l10n; - - $path = $this->config->get('storage', 'filesystem_path', self::DEFAULT_BASE_FOLDER); + $path = $filesystemPath; $this->basePath = rtrim($path, '/'); } @@ -176,37 +161,6 @@ class Filesystem implements IWritableStorage } } - /** - * @inheritDoc - */ - public function getOptions(): array - { - return [ - 'storagepath' => [ - 'input', - $this->l10n->t('Storage base path'), - $this->basePath, - $this->l10n->t('Folder where uploaded files are saved. For maximum security, This should be a path outside web server folder tree') - ] - ]; - } - - /** - * @inheritDoc - */ - public function saveOptions(array $data): array - { - $storagePath = $data['storagepath'] ?? ''; - if ($storagePath === '' || !is_dir($storagePath)) { - return [ - 'storagepath' => $this->l10n->t('Enter a valid existing folder') - ]; - }; - $this->config->set('storage', 'filesystem_path', $storagePath); - $this->basePath = $storagePath; - return []; - } - /** * @inheritDoc */ diff --git a/src/Model/Storage/FilesystemConfig.php b/src/Model/Storage/FilesystemConfig.php new file mode 100644 index 0000000000..735808e822 --- /dev/null +++ b/src/Model/Storage/FilesystemConfig.php @@ -0,0 +1,99 @@ +. + * + */ + +namespace Friendica\Model\Storage; + +use Friendica\Core\Config\IConfig; +use Friendica\Core\L10n; + +/** + * Filesystem based storage backend configuration + */ +class FilesystemConfig implements IStorageConfiguration +{ + // Default base folder + const DEFAULT_BASE_FOLDER = 'storage'; + + /** @var IConfig */ + private $config; + + /** @var string */ + private $storagePath; + + /** @var L10n */ + private $l10n; + + /** + * Returns the current storage path + * + * @return string + */ + public function getStoragePath(): string + { + return $this->storagePath; + } + + /** + * Filesystem constructor. + * + * @param IConfig $config + * @param L10n $l10n + */ + public function __construct(IConfig $config, L10n $l10n) + { + $this->config = $config; + $this->l10n = $l10n; + + $path = $this->config->get('storage', 'filesystem_path', self::DEFAULT_BASE_FOLDER); + $this->storagePath = rtrim($path, '/'); + } + + /** + * @inheritDoc + */ + public function getOptions(): array + { + return [ + 'storagepath' => [ + 'input', + $this->l10n->t('Storage base path'), + $this->storagePath, + $this->l10n->t('Folder where uploaded files are saved. For maximum security, This should be a path outside web server folder tree') + ] + ]; + } + + /** + * @inheritDoc + */ + public function saveOptions(array $data): array + { + $storagePath = $data['storagepath'] ?? ''; + if ($storagePath === '' || !is_dir($storagePath)) { + return [ + 'storagepath' => $this->l10n->t('Enter a valid existing folder') + ]; + }; + $this->config->set('storage', 'filesystem_path', $storagePath); + $this->storagePath = $storagePath; + return []; + } +} diff --git a/src/Model/Storage/IStorageConfiguration.php b/src/Model/Storage/IStorageConfiguration.php new file mode 100644 index 0000000000..c589f5ed5c --- /dev/null +++ b/src/Model/Storage/IStorageConfiguration.php @@ -0,0 +1,78 @@ +. + * + */ + +namespace Friendica\Model\Storage; + +/** + * The interface to use for configurable storage backends + */ +interface IStorageConfiguration +{ + /** + * Get info about storage options + * + * @return array + * + * This method return an array with information 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/IWritableStorage.php b/src/Model/Storage/IWritableStorage.php index ee0001a669..118f4b2561 100644 --- a/src/Model/Storage/IWritableStorage.php +++ b/src/Model/Storage/IWritableStorage.php @@ -50,54 +50,4 @@ interface IWritableStorage extends IStorage * @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/Module/Admin/Storage.php b/src/Module/Admin/Storage.php index c6bc3878ef..6b22d905cf 100644 --- a/src/Module/Admin/Storage.php +++ b/src/Module/Admin/Storage.php @@ -24,6 +24,7 @@ namespace Friendica\Module\Admin; use Friendica\Core\Renderer; use Friendica\DI; use Friendica\Model\Storage\InvalidClassStorageException; +use Friendica\Model\Storage\IStorageConfiguration; use Friendica\Model\Storage\IWritableStorage; use Friendica\Module\BaseAdmin; use Friendica\Util\Strings; @@ -39,38 +40,40 @@ class Storage extends BaseAdmin $storagebackend = Strings::escapeTags(trim($parameters['name'] ?? '')); try { - /** @var IWritableStorage $newstorage */ - $newstorage = DI::storageManager()->getWritableStorageByName($storagebackend); + /** @var IStorageConfiguration|false $newStorageConfig */ + $newStorageConfig = DI::storageManager()->getConfigurationByName($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(); - $storage_form_prefix = preg_replace('|[^a-zA-Z0-9]|', '', $storagebackend); - $storage_opts_data = []; - foreach ($storage_opts as $name => $info) { - $fieldname = $storage_form_prefix . '_' . $name; - switch ($info[0]) { // type - case 'checkbox': - case 'yesno': - $value = !empty($_POST[$fieldname]); - break; - default: - $value = $_POST[$fieldname] ?? ''; + if ($newStorageConfig !== false) { + // save storage backend form + $storage_opts = $newStorageConfig->getOptions(); + $storage_form_prefix = preg_replace('|[^a-zA-Z0-9]|', '', $storagebackend); + $storage_opts_data = []; + foreach ($storage_opts as $name => $info) { + $fieldname = $storage_form_prefix . '_' . $name; + switch ($info[0]) { // type + case 'checkbox': + case 'yesno': + $value = !empty($_POST[$fieldname]); + break; + default: + $value = $_POST[$fieldname] ?? ''; + } + $storage_opts_data[$name] = $value; } - $storage_opts_data[$name] = $value; - } - unset($name); - unset($info); + unset($name); + unset($info); - $storage_form_errors = $newstorage->saveOptions($storage_opts_data); - if (count($storage_form_errors)) { - foreach ($storage_form_errors as $name => $err) { - notice(DI::l10n()->t('Storage backend %s error: %s', $storage_opts[$name][1], $err)); + $storage_form_errors = $newStorageConfig->saveOptions($storage_opts_data); + if (count($storage_form_errors)) { + foreach ($storage_form_errors as $name => $err) { + notice(DI::l10n()->t('Storage backend %s error: %s', $storage_opts[$name][1], $err)); + } + DI::baseUrl()->redirect('admin/storage'); } - DI::baseUrl()->redirect('admin/storage'); } if (!empty($_POST['submit_save_set'])) { @@ -101,20 +104,25 @@ class Storage extends BaseAdmin // build storage config form, $storage_form_prefix = preg_replace('|[^a-zA-Z0-9]|', '', $name); - $storage_form = []; - foreach (DI::storageManager()->getWritableStorageByName($name)->getOptions() as $option => $info) { - $type = $info[0]; - // Backward compatibilty with yesno field description - if ($type == 'yesno') { - $type = 'checkbox'; - // Remove translated labels Yes No from field info - unset($info[4]); - } + $storage_form = []; + $storageConfig = DI::storageManager()->getConfigurationByName($name); - $info[0] = $storage_form_prefix . '_' . $option; - $info['type'] = $type; - $info['field'] = 'field_' . $type . '.tpl'; - $storage_form[$option] = $info; + if ($storageConfig !== false) { + foreach ($storageConfig->getOptions() as $option => $info) { + + $type = $info[0]; + // Backward compatibilty with yesno field description + if ($type == 'yesno') { + $type = 'checkbox'; + // Remove translated labels Yes No from field info + unset($info[4]); + } + + $info[0] = $storage_form_prefix . '_' . $option; + $info['type'] = $type; + $info['field'] = 'field_' . $type . '.tpl'; + $storage_form[$option] = $info; + } } $available_storage_forms[] = [ diff --git a/tests/src/Model/Storage/DatabaseStorageTest.php b/tests/src/Model/Storage/DatabaseStorageTest.php index aa25d1cc7f..e1dfef9a18 100644 --- a/tests/src/Model/Storage/DatabaseStorageTest.php +++ b/tests/src/Model/Storage/DatabaseStorageTest.php @@ -23,11 +23,9 @@ namespace Friendica\Test\src\Model\Storage; use Friendica\Factory\ConfigFactory; use Friendica\Model\Storage\Database; -use Friendica\Model\Storage\IWritableStorage; use Friendica\Test\DatabaseTestTrait; use Friendica\Test\Util\Database\StaticDatabase; use Friendica\Test\Util\VFSTrait; -use Friendica\Util\ConfigFileLoader; use Friendica\Util\Profiler; use Psr\Log\NullLogger; @@ -47,7 +45,7 @@ class DatabaseStorageTest extends StorageTest protected function getInstance() { - $logger = new NullLogger(); + $logger = new NullLogger(); $profiler = \Mockery::mock(Profiler::class); $profiler->shouldReceive('startRecording'); $profiler->shouldReceive('stopRecording'); @@ -55,19 +53,14 @@ class DatabaseStorageTest extends StorageTest // load real config to avoid mocking every config-entry which is related to the Database class $configFactory = new ConfigFactory(); - $loader = (new ConfigFactory())->createConfigFileLoader($this->root->url(), []); - $configCache = $configFactory->createCache($loader); + $loader = (new ConfigFactory())->createConfigFileLoader($this->root->url(), []); + $configCache = $configFactory->createCache($loader); $dba = new StaticDatabase($configCache, $profiler, $logger); return new Database($dba); } - protected function assertOption(IWritableStorage $storage) - { - self::assertEmpty($storage->getOptions()); - } - protected function tearDown(): void { $this->tearDownDb(); diff --git a/tests/src/Model/Storage/FilesystemStorageConfigTest.php b/tests/src/Model/Storage/FilesystemStorageConfigTest.php new file mode 100644 index 0000000000..5821309386 --- /dev/null +++ b/tests/src/Model/Storage/FilesystemStorageConfigTest.php @@ -0,0 +1,67 @@ +. + * + */ + +namespace Friendica\Test\src\Model\Storage; + +use Friendica\Core\Config\IConfig; +use Friendica\Core\L10n; +use Friendica\Model\Storage\FilesystemConfig; +use Friendica\Model\Storage\IStorageConfiguration; +use Friendica\Test\Util\VFSTrait; +use Mockery\MockInterface; +use org\bovigo\vfs\vfsStream; + +class FilesystemStorageConfigTest extends StorageConfigTest +{ + use VFSTrait; + + protected function setUp(): void + { + $this->setUpVfsDir(); + + vfsStream::create(['storage' => []], $this->root); + + parent::setUp(); + } + + protected function getInstance() + { + /** @var MockInterface|L10n $l10n */ + $l10n = \Mockery::mock(L10n::class)->makePartial(); + $config = \Mockery::mock(IConfig::class); + $config->shouldReceive('get') + ->with('storage', 'filesystem_path', FilesystemConfig::DEFAULT_BASE_FOLDER) + ->andReturn($this->root->getChild('storage')->url()); + + return new FilesystemConfig($config, $l10n); + } + + protected function assertOption(IStorageConfiguration $storage) + { + self::assertEquals([ + 'storagepath' => [ + 'input', 'Storage base path', + $this->root->getChild('storage')->url(), + 'Folder where uploaded files are saved. For maximum security, This should be a path outside web server folder tree' + ] + ], $storage->getOptions()); + } +} diff --git a/tests/src/Model/Storage/FilesystemStorageTest.php b/tests/src/Model/Storage/FilesystemStorageTest.php index 45a7264163..d3204c6829 100644 --- a/tests/src/Model/Storage/FilesystemStorageTest.php +++ b/tests/src/Model/Storage/FilesystemStorageTest.php @@ -21,23 +21,15 @@ namespace Friendica\Test\src\Model\Storage; -use Friendica\Core\Config\IConfig; -use Friendica\Core\L10n; use Friendica\Model\Storage\Filesystem; -use Friendica\Model\Storage\IWritableStorage; use Friendica\Model\Storage\StorageException; use Friendica\Test\Util\VFSTrait; -use Friendica\Util\Profiler; -use Mockery\MockInterface; use org\bovigo\vfs\vfsStream; class FilesystemStorageTest extends StorageTest { use VFSTrait; - /** @var MockInterface|IConfig */ - protected $config; - protected function setUp(): void { $this->setUpVfsDir(); @@ -49,30 +41,7 @@ class FilesystemStorageTest extends StorageTest protected function getInstance() { - $profiler = \Mockery::mock(Profiler::class); - $profiler->shouldReceive('startRecording'); - $profiler->shouldReceive('stopRecording'); - $profiler->shouldReceive('saveTimestamp')->withAnyArgs()->andReturn(true); - - /** @var MockInterface|L10n $l10n */ - $l10n = \Mockery::mock(L10n::class)->makePartial(); - $this->config = \Mockery::mock(IConfig::class); - $this->config->shouldReceive('get') - ->with('storage', 'filesystem_path', Filesystem::DEFAULT_BASE_FOLDER) - ->andReturn($this->root->getChild('storage')->url()); - - return new Filesystem($this->config, $l10n); - } - - protected function assertOption(IWritableStorage $storage) - { - self::assertEquals([ - 'storagepath' => [ - 'input', 'Storage base path', - $this->root->getChild('storage')->url(), - 'Folder where uploaded files are saved. For maximum security, This should be a path outside web server folder tree' - ] - ], $storage->getOptions()); + return new Filesystem($this->root->getChild('storage')->url()); } /** @@ -116,7 +85,7 @@ class FilesystemStorageTest extends StorageTest $instance->put('test', 'f0c0d0i0'); - $dir = $this->root->getChild('storage/f0/c0')->url(); + $dir = $this->root->getChild('storage/f0/c0')->url(); $file = $this->root->getChild('storage/f0/c0/d0i0')->url(); self::assertDirectoryExists($dir); diff --git a/tests/src/Model/Storage/StorageConfigTest.php b/tests/src/Model/Storage/StorageConfigTest.php new file mode 100644 index 0000000000..80cbbaeb0a --- /dev/null +++ b/tests/src/Model/Storage/StorageConfigTest.php @@ -0,0 +1,43 @@ +. + * + */ + +namespace Friendica\Test\src\Model\Storage; + +use Friendica\Model\Storage\IStorageConfiguration; +use Friendica\Test\MockedTest; + +abstract class StorageConfigTest extends MockedTest +{ + /** @return IStorageConfiguration */ + abstract protected function getInstance(); + + abstract protected function assertOption(IStorageConfiguration $storage); + + /** + * Test if the "getOption" is asserted + */ + public function testGetOptions() + { + $instance = $this->getInstance(); + + $this->assertOption($instance); + } +} diff --git a/tests/src/Model/Storage/StorageTest.php b/tests/src/Model/Storage/StorageTest.php index 340aee8bfd..7433747575 100644 --- a/tests/src/Model/Storage/StorageTest.php +++ b/tests/src/Model/Storage/StorageTest.php @@ -31,8 +31,6 @@ abstract class StorageTest extends MockedTest /** @return IWritableStorage */ abstract protected function getInstance(); - abstract protected function assertOption(IWritableStorage $storage); - /** * Test if the instance is "really" implementing the interface */ @@ -42,16 +40,6 @@ abstract class StorageTest extends MockedTest self::assertInstanceOf(IStorage::class, $instance); } - /** - * Test if the "getOption" is asserted - */ - public function testGetOptions() - { - $instance = $this->getInstance(); - - $this->assertOption($instance); - } - /** * Test basic put, get and delete operations */ From 08d1dcf14c3c1db6baaa97ccc9074c14a5ef4964 Mon Sep 17 00:00:00 2001 From: Philipp Date: Mon, 4 Oct 2021 10:37:31 +0200 Subject: [PATCH 2/6] Adapt documentation --- doc/AddonStorageBackend.md | 121 ++++++++++++++++++++++++++----------- doc/Addons.md | 1 + doc/de/Addons.md | 1 + 3 files changed, 88 insertions(+), 35 deletions(-) diff --git a/doc/AddonStorageBackend.md b/doc/AddonStorageBackend.md index e54960252b..19e956ce9e 100644 --- a/doc/AddonStorageBackend.md +++ b/doc/AddonStorageBackend.md @@ -10,18 +10,20 @@ 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. +There are two different interfaces you need to implement. + +### `IWritableStorage` + The class must implement `Friendica\Model\Storage\IWritableStorage` interface. All method in the interface must be implemented: -namespace Friendica\Model\IWritableStorage; - ```php +namespace Friendica\Model\Storage\IWritableStorage; + interface IWritableStorage { public function get(string $reference); public function put(string $data, string $reference = ''); public function delete(string $reference); - public function getOptions(); - public function saveOptions(array $data); public function __toString(); public static function getName(); } @@ -31,7 +33,22 @@ interface IWritableStorage - `put(string $data, string $reference)` saves data in `$data` to position `$reference`, or a new position if `$reference` is empty. - `delete(string $reference)` delete data pointed by `$reference` +### `IStorageConfiguration` + Each storage backend can have options the admin can set in admin page. +To make the options possible, you need to implement the `Friendica\Model\Storage\IStorageConfiguration` interface. + +All methods in the interface mus be implemented: + +```php +namespace Friendica\Model\Storage\IStorageConfiguration; + +interface IStorageConfiguration +{ + public function getOptions(); + public function saveOptions(array $data); +} +``` - `getOptions()` returns an array with details about each option to build the interface. - `saveOptions(array $data)` get `$data` from admin page, validate it and save it. @@ -156,7 +173,7 @@ class ExampleStorage implements IWritableStorage ## Example -Here an hypotetical addon which register a useless storage backend. +Here is a hypothetical 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. @@ -178,30 +195,25 @@ class SampleStorageBackend implements IWritableStorage { const NAME = 'Sample Storage'; - /** @var IConfig */ - private $config; - /** @var L10n */ - private $l10n; + /** @var string */ + private $filename; /** * SampleStorageBackend constructor. - * @param IConfig $config The configuration of Friendica - * + * * You can add here every dynamic class as dependency you like and add them to a private field * Friendica automatically creates these classes and passes them as argument to the constructor */ - public function __construct(IConfig $config, L10n $l10n) + public function __construct(string $filename) { - $this->config = $config; - $this->l10n = $l10n; + $this->filename = $filename; } public function get(string $reference) { // we return always the same image data. Which file we load is defined by // a config key - $filename = $this->config->get('storage', 'samplestorage', 'sample.jpg'); - return file_get_contents($filename); + return file_get_contents($this->filename); } public function put(string $data, string $reference = '') @@ -219,6 +231,51 @@ class SampleStorageBackend implements IWritableStorage return true; } + public function __toString() + { + return self::NAME; + } + + public static function getName() + { + return self::NAME; + } +} +``` + +```php +config = $config; + $this->l10n = $l10n; + } + + public function getFileName(): string + { + return $this->config->get('storage', 'samplestorage', 'sample.jpg'); + } + public function getOptions() { $filename = $this->config->get('storage', 'samplestorage', 'sample.jpg'); @@ -252,15 +309,6 @@ class SampleStorageBackend implements IWritableStorage return []; } - public function __toString() - { - return self::NAME; - } - - public static function getName() - { - return self::NAME; - } } ``` @@ -278,29 +326,32 @@ The file is `addon/samplestorage/samplestorage.php` */ use Friendica\Addon\samplestorage\SampleStorageBackend; +use Friendica\Addon\samplestorage\SampleStorageBackendConfig; use Friendica\DI; function samplestorage_install() { - // on addon install, we register our class with name "Sample Storage". - // note: we use `::class` property, which returns full class name as string - // this save us the problem of correctly escape backslashes in class name + Hook::register('storage_instance' , __FILE__, 'samplestorage_storage_instance'); + Hook::register('storage_config' , __FILE__, 'samplestorage_storage_config'); DI::storageManager()->register(SampleStorageBackend::class); } -function samplestorage_unistall() +function webdav_storage_uninstall() { - // when the plugin is uninstalled, we unregister the backend. DI::storageManager()->unregister(SampleStorageBackend::class); } -function samplestorage_storage_instance(\Friendica\App $a, array $data) +function webdav_storage_instance(App $a, array &$data) { - if ($data['name'] === SampleStorageBackend::getName()) { - // instance a new sample storage instance and pass it back to the core for usage - $data['storage'] = new SampleStorageBackend(DI::config(), DI::l10n(), DI::cache()); - } + $config = new SampleStorageBackendConfig(DI::l10n(), DI::config()); + $data['storage'] = new SampleStorageBackendConfig($config->getFileName()); } + +function webdav_storage_config(App $a, array &$data) +{ + $data['storage_config'] = new WebDavConfig(DI::l10n(), DI::config()); +} + ``` **Theoretically - until tests for Addons are enabled too - create a test class with the name `addon/tests/SampleStorageTest.php`: diff --git a/doc/Addons.md b/doc/Addons.md index 6fa87c953c..e40027fa9e 100644 --- a/doc/Addons.md +++ b/doc/Addons.md @@ -802,6 +802,7 @@ Here is a complete list of all hook callbacks with file locations (as of 24-Sep- ### src/Core/StorageManager Hook::callAll('storage_instance', $data); + Hook::callAll('storage_config', $data); ### src/Worker/Directory.php diff --git a/doc/de/Addons.md b/doc/de/Addons.md index 64e3e37ba6..a20cf4a594 100644 --- a/doc/de/Addons.md +++ b/doc/de/Addons.md @@ -425,6 +425,7 @@ Eine komplette Liste aller Hook-Callbacks mit den zugehörigen Dateien (am 01-Ap ### src/Core/StorageManager Hook::callAll('storage_instance', $data); + Hook::callAll('storage_config', $data); ### src/Module/PermissionTooltip.php From bb64f1cb77630bf8ea770cfcf7a5e6bcea05ac6d Mon Sep 17 00:00:00 2001 From: Philipp Date: Tue, 5 Oct 2021 18:35:50 +0200 Subject: [PATCH 3/6] Update doc/AddonStorageBackend.md Co-authored-by: Hypolite Petovan --- doc/AddonStorageBackend.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/AddonStorageBackend.md b/doc/AddonStorageBackend.md index 19e956ce9e..a10d5b0000 100644 --- a/doc/AddonStorageBackend.md +++ b/doc/AddonStorageBackend.md @@ -38,7 +38,7 @@ interface IWritableStorage Each storage backend can have options the admin can set in admin page. To make the options possible, you need to implement the `Friendica\Model\Storage\IStorageConfiguration` interface. -All methods in the interface mus be implemented: +All methods in the interface must be implemented: ```php namespace Friendica\Model\Storage\IStorageConfiguration; From aebbbbba287f432f9e0dd66612717edae79d24e2 Mon Sep 17 00:00:00 2001 From: Philipp Date: Tue, 5 Oct 2021 18:35:57 +0200 Subject: [PATCH 4/6] Update doc/Addons.md Co-authored-by: Hypolite Petovan --- doc/Addons.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/Addons.md b/doc/Addons.md index e40027fa9e..0d38b63925 100644 --- a/doc/Addons.md +++ b/doc/Addons.md @@ -802,7 +802,7 @@ Here is a complete list of all hook callbacks with file locations (as of 24-Sep- ### src/Core/StorageManager Hook::callAll('storage_instance', $data); - Hook::callAll('storage_config', $data); + Hook::callAll('storage_config', $data); ### src/Worker/Directory.php From ccd88952373f2003887cf40c86a8e5f3e3e2b281 Mon Sep 17 00:00:00 2001 From: Philipp Date: Tue, 5 Oct 2021 19:06:13 +0200 Subject: [PATCH 5/6] Adress feedback :) --- doc/AddonStorageBackend.md | 8 ++++---- doc/Addons.md | 16 ++++++++++++++++ src/Model/Storage/Filesystem.php | 6 ++++++ 3 files changed, 26 insertions(+), 4 deletions(-) diff --git a/doc/AddonStorageBackend.md b/doc/AddonStorageBackend.md index a10d5b0000..f4773acb89 100644 --- a/doc/AddonStorageBackend.md +++ b/doc/AddonStorageBackend.md @@ -336,20 +336,20 @@ function samplestorage_install() DI::storageManager()->register(SampleStorageBackend::class); } -function webdav_storage_uninstall() +function samplestorage_storage_uninstall() { DI::storageManager()->unregister(SampleStorageBackend::class); } -function webdav_storage_instance(App $a, array &$data) +function samplestorage_storage_instance(App $a, array &$data) { $config = new SampleStorageBackendConfig(DI::l10n(), DI::config()); $data['storage'] = new SampleStorageBackendConfig($config->getFileName()); } -function webdav_storage_config(App $a, array &$data) +function samplestorage_storage_config(App $a, array &$data) { - $data['storage_config'] = new WebDavConfig(DI::l10n(), DI::config()); + $data['storage_config'] = new SampleStorageBackendConfig(DI::l10n(), DI::config()); } ``` diff --git a/doc/Addons.md b/doc/Addons.md index 0d38b63925..debdc89dd4 100644 --- a/doc/Addons.md +++ b/doc/Addons.md @@ -538,6 +538,22 @@ Hook data: - **uid** (input): the user id to revoke the block for. - **result** (output): a boolean value indicating wether the operation was successful or not. +### storage_instance + +Called when a custom storage is used (e.g. webdav_storage) + +Hook data: +- **name** (input): the name of the used storage backend +- **data['storage']** (output): the storage instance to use (**must** implement `\Friendica\Model\Storage\IWritableStorage`) + +### storage_config + +Called when the admin of the node wants to configure a custom storage (e.g. webdav_storage) + +Hook data: +- **name** (input): the name of the used storage backend +- **data['storage_config']** (output): the storage configuration instance to use (**must** implement `\Friendica\Model\Storage\IStorageConfiguration`) + ## Complete list of hook callbacks Here is a complete list of all hook callbacks with file locations (as of 24-Sep-2018). Please see the source for details of any hooks not documented above. diff --git a/src/Model/Storage/Filesystem.php b/src/Model/Storage/Filesystem.php index 5a4ea1f0d3..3165565b0b 100644 --- a/src/Model/Storage/Filesystem.php +++ b/src/Model/Storage/Filesystem.php @@ -45,11 +45,17 @@ class Filesystem implements IWritableStorage * Filesystem constructor. * * @param string $filesystemPath + * + * @throws StorageException in case the path doesn't exist or isn't writeable */ public function __construct(string $filesystemPath = FilesystemConfig::DEFAULT_BASE_FOLDER) { $path = $filesystemPath; $this->basePath = rtrim($path, '/'); + + if (!is_dir($this->basePath) || !is_writable($this->basePath)) { + throw new StorageException(sprintf('Path %s does not exist or is not writeable', $this->basePath)); + } } /** From 7471b7698b479d150cd4fc01f8e0e0112af9a8ef Mon Sep 17 00:00:00 2001 From: Philipp Date: Tue, 5 Oct 2021 19:59:13 +0200 Subject: [PATCH 6/6] Adapt filesystem tests --- src/Model/Storage/Filesystem.php | 2 +- tests/src/Core/StorageManagerTest.php | 11 ++++++++ .../Model/Storage/FilesystemStorageTest.php | 27 ++++++++++++++----- 3 files changed, 33 insertions(+), 7 deletions(-) diff --git a/src/Model/Storage/Filesystem.php b/src/Model/Storage/Filesystem.php index 3165565b0b..8b5078f548 100644 --- a/src/Model/Storage/Filesystem.php +++ b/src/Model/Storage/Filesystem.php @@ -54,7 +54,7 @@ class Filesystem implements IWritableStorage $this->basePath = rtrim($path, '/'); if (!is_dir($this->basePath) || !is_writable($this->basePath)) { - throw new StorageException(sprintf('Path %s does not exist or is not writeable', $this->basePath)); + throw new StorageException(sprintf('Path "%s" does not exist or is not writeable.', $this->basePath)); } } diff --git a/tests/src/Core/StorageManagerTest.php b/tests/src/Core/StorageManagerTest.php index b0def99b70..77e4946bdf 100644 --- a/tests/src/Core/StorageManagerTest.php +++ b/tests/src/Core/StorageManagerTest.php @@ -40,6 +40,7 @@ use Friendica\Test\Util\Database\StaticDatabase; use Friendica\Test\Util\VFSTrait; use Friendica\Util\ConfigFileLoader; use Friendica\Util\Profiler; +use org\bovigo\vfs\vfsStream; use Psr\Log\LoggerInterface; use Psr\Log\NullLogger; use Friendica\Test\Util\SampleStorageBackend; @@ -64,6 +65,8 @@ class StorageManagerTest extends DatabaseTest $this->setUpVfsDir(); + vfsStream::newDirectory(Storage\FilesystemConfig::DEFAULT_BASE_FOLDER, 0777)->at($this->root); + $this->logger = new NullLogger(); $profiler = \Mockery::mock(Profiler::class); @@ -81,12 +84,20 @@ class StorageManagerTest extends DatabaseTest $configModel = new Config($this->dba); $this->config = new PreloadConfig($configCache, $configModel); $this->config->set('storage', 'name', 'Database'); + $this->config->set('storage', 'filesystem_path', $this->root->getChild(Storage\FilesystemConfig::DEFAULT_BASE_FOLDER)->url()); $this->l10n = \Mockery::mock(L10n::class); $this->httpRequest = \Mockery::mock(HTTPClient::class); } + protected function tearDown(): void + { + $this->root->removeChild(Storage\FilesystemConfig::DEFAULT_BASE_FOLDER); + + parent::tearDown(); + } + /** * Test plain instancing first */ diff --git a/tests/src/Model/Storage/FilesystemStorageTest.php b/tests/src/Model/Storage/FilesystemStorageTest.php index d3204c6829..837c166819 100644 --- a/tests/src/Model/Storage/FilesystemStorageTest.php +++ b/tests/src/Model/Storage/FilesystemStorageTest.php @@ -22,6 +22,7 @@ namespace Friendica\Test\src\Model\Storage; use Friendica\Model\Storage\Filesystem; +use Friendica\Model\Storage\FilesystemConfig; use Friendica\Model\Storage\StorageException; use Friendica\Test\Util\VFSTrait; use org\bovigo\vfs\vfsStream; @@ -41,20 +42,34 @@ class FilesystemStorageTest extends StorageTest protected function getInstance() { - return new Filesystem($this->root->getChild('storage')->url()); + return new Filesystem($this->root->getChild(FilesystemConfig::DEFAULT_BASE_FOLDER)->url()); } /** - * Test the exception in case of missing directorsy permissions + * Test the exception in case of missing directory permissions during put new files + */ + public function testMissingDirPermissionsDuringPut() + { + $this->expectException(StorageException::class); + $this->expectExceptionMessageMatches("/Filesystem storage failed to create \".*\". Check you write permissions./"); + $this->root->getChild(FilesystemConfig::DEFAULT_BASE_FOLDER)->chmod(0777); + + $instance = $this->getInstance(); + + $this->root->getChild(FilesystemConfig::DEFAULT_BASE_FOLDER)->chmod(0000); + $instance->put('test'); + } + + /** + * Test the exception in case the directory isn't writeable */ public function testMissingDirPermissions() { $this->expectException(StorageException::class); - $this->expectExceptionMessageMatches("/Filesystem storage failed to create \".*\". Check you write permissions./"); - $this->root->getChild('storage')->chmod(000); + $this->expectExceptionMessageMatches("/Path \".*\" does not exist or is not writeable./"); + $this->root->getChild(FilesystemConfig::DEFAULT_BASE_FOLDER)->chmod(0000); - $instance = $this->getInstance(); - $instance->put('test'); + $this->getInstance(); } /**