From bfae6766bf67d77a4fe8ed3efe99d174a06add5d Mon Sep 17 00:00:00 2001 From: nupplaPhil Date: Wed, 8 Jan 2020 22:51:37 +0100 Subject: [PATCH] Implement Hook::callAll('storage_instance') call for addons and add a description for it. - Remove implicit Dice usage - Add concrete instance creating - Adding Hook call for addon instance creating - Updating doc for Hook - Updating tests --- doc/AddonStorageBackend.md | 20 ++++- doc/Addons.md | 8 ++ doc/de/Addons.md | 4 + src/Core/StorageManager.php | 93 +++++++++++++-------- static/dependencies.config.php | 8 -- tests/Util/SampleStorageBackend.php | 12 +++ tests/Util/SampleStorageBackendInstance.php | 18 ++++ tests/src/Core/StorageManagerTest.php | 54 ++++++++---- 8 files changed, 155 insertions(+), 62 deletions(-) create mode 100644 tests/Util/SampleStorageBackendInstance.php diff --git a/doc/AddonStorageBackend.md b/doc/AddonStorageBackend.md index 115bfd0035..ef1b4454b9 100644 --- a/doc/AddonStorageBackend.md +++ b/doc/AddonStorageBackend.md @@ -23,6 +23,7 @@ interface IStorage public function getOptions(); public function saveOptions(array $data); public function __toString(); + public static function getName(); } ``` @@ -85,11 +86,16 @@ See doxygen documentation of `IStorage` interface for details about each method. Each backend must be registered in the system when the plugin is installed, to be aviable. -`DI::facStorage()->register(string $name, string $class)` is used to register the backend class. -The `$name` must be univocal and will be shown to admin. +`DI::facStorage()->register(string $class)` is used to register the backend class. When the plugin is uninstalled, registered backends must be unregistered using -`DI::facStorage()->unregister(string $name)`. +`DI::facStorage()->unregister(string $class)`. + +You have to register a new hook in your addon, listening on `storage_instance(App $a, array $data)`. +In case `$data['name']` is your storage class name, you have to instance a new instance of your `Friendica\Model\Storage\IStorage` class. +Set the instance of your class as `$data['storage']` to pass it back to the backend. + +This is necessary because it isn't always clear, if you need further construction arguments. ## Adding tests @@ -252,6 +258,14 @@ function samplestorage_unistall() // when the plugin is uninstalled, we unregister the backend. DI::facStorage()->unregister(SampleStorageBackend::class); } + +function samplestorage_storage_instance(\Friendica\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()); + } +} ``` **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 d448b026b8..9260ee013c 100644 --- a/doc/Addons.md +++ b/doc/Addons.md @@ -706,6 +706,14 @@ Here is a complete list of all hook callbacks with file locations (as of 24-Sep- Hook::callAll('page_header', DI::page()['nav']); Hook::callAll('nav_info', $nav); +### src/Core/Authentication.php + + Hook::callAll('logged_in', $a->user); + +### src/Core/StorageManager + + Hook::callAll('storage_instance', $data); + ### src/Worker/Directory.php Hook::callAll('globaldir_update', $arr); diff --git a/doc/de/Addons.md b/doc/de/Addons.md index 32add2181e..37bf114d2a 100644 --- a/doc/de/Addons.md +++ b/doc/de/Addons.md @@ -424,6 +424,10 @@ Eine komplette Liste aller Hook-Callbacks mit den zugehörigen Dateien (am 01-Ap ### src/Core/Authentication.php Hook::callAll('logged_in', $a->user); + +### src/Core/StorageManager + + Hook::callAll('storage_instance', $data); ### src/Worker/Directory.php diff --git a/src/Core/StorageManager.php b/src/Core/StorageManager.php index 6a67a06ed7..6a8fac5b80 100644 --- a/src/Core/StorageManager.php +++ b/src/Core/StorageManager.php @@ -2,9 +2,9 @@ namespace Friendica\Core; -use Dice\Dice; use Exception; use Friendica\Core\Config\IConfiguration; +use Friendica\Core\L10n\L10n; use Friendica\Database\Database; use Friendica\Model\Storage; use Psr\Log\LoggerInterface; @@ -29,14 +29,19 @@ class StorageManager private $backends = []; + /** + * @var Storage\IStorage[] A local cache for storage instances + */ + private $backendInstances = []; + /** @var Database */ private $dba; /** @var IConfiguration */ private $config; /** @var LoggerInterface */ private $logger; - /** @var Dice */ - private $dice; + /** @var L10n */ + private $l10n; /** @var Storage\IStorage */ private $currentBackend; @@ -45,23 +50,19 @@ class StorageManager * @param Database $dba * @param IConfiguration $config * @param LoggerInterface $logger - * @param Dice $dice + * @param L10n $l10n */ - public function __construct(Database $dba, IConfiguration $config, LoggerInterface $logger, Dice $dice) + public function __construct(Database $dba, IConfiguration $config, LoggerInterface $logger, L10n $l10n) { $this->dba = $dba; $this->config = $config; $this->logger = $logger; - $this->dice = $dice; + $this->l10n = $l10n; $this->backends = $config->get('storage', 'backends', self::DEFAULT_BACKENDS); $currentName = $this->config->get('storage', 'name', ''); - if ($this->isValidBackend($currentName)) { - $this->currentBackend = $this->dice->create($this->backends[$currentName]); - } else { - $this->currentBackend = null; - } + $this->currentBackend = $this->getByName($currentName); } /** @@ -77,41 +78,65 @@ class StorageManager /** * @brief Return storage backend class by registered name * - * @param string|null $name Backend name + * @param string|null $name Backend name + * @param boolean $userBackend Just return instances in case it's a user backend (e.g. not SystemResource) * * @return Storage\IStorage|null null if no backend registered at $name + * + * @throws \Friendica\Network\HTTPException\InternalServerErrorException */ - public function getByName(string $name = null) + public function getByName(string $name = null, $userBackend = true) { - if (!$this->isValidBackend($name) && - $name !== Storage\SystemResource::getName()) { - return null; + // 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, $userBackend)) { + switch ($name) { + // Try the filesystem backend + case Storage\Filesystem::getName(): + $this->backendInstances[$name] = new Storage\Filesystem($this->config, $this->logger, $this->l10n); + break; + // try the database backend + case Storage\Database::getName(): + $this->backendInstances[$name] = new Storage\Database($this->dba, $this->logger, $this->l10n); + break; + // at least, try if there's an addon for the backend + case Storage\SystemResource::getName(): + $this->backendInstances[$name] = new Storage\SystemResource(); + break; + default: + $data = [ + '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; + } + break; + } + } else { + return null; + } } - /** @var Storage\IStorage $storage */ - $storage = null; - - // If the storage of the file is a system resource, - // create it directly since it isn't listed in the registered backends - if ($name === Storage\SystemResource::getName()) { - $storage = $this->dice->create(Storage\SystemResource::class); - } else { - $storage = $this->dice->create($this->backends[$name]); - } - - return $storage; + return $this->backendInstances[$name]; } /** * Checks, if the storage is a valid backend * - * @param string|null $name The name or class of the backend + * @param string|null $name The name or class of the backend + * @param boolean $userBackend True, if just user backend should get returned (e.g. not SystemResource) * * @return boolean True, if the backend is a valid backend */ - public function isValidBackend(string $name = null) + public function isValidBackend(string $name = null, bool $userBackend = true) { - return array_key_exists($name, $this->backends); + return array_key_exists($name, $this->backends) || + (!$userBackend && $name === Storage\SystemResource::getName()); } /** @@ -128,7 +153,7 @@ class StorageManager } if ($this->config->set('storage', 'name', $name)) { - $this->currentBackend = $this->dice->create($this->backends[$name]); + $this->currentBackend = $this->getByName($name); return true; } else { return false; @@ -146,7 +171,9 @@ class StorageManager } /** - * @brief Register a storage backend class + * Register a storage backend class + * + * You have to register the hook "storage_instance" as well to make this class work! * * @param string $class Backend class name * diff --git a/static/dependencies.config.php b/static/dependencies.config.php index 0bf5a6905c..ec80123aa3 100644 --- a/static/dependencies.config.php +++ b/static/dependencies.config.php @@ -196,15 +196,7 @@ return [ $_SERVER, $_COOKIE ], ], - StorageManager::class => [ - 'constructParams' => [ - [Dice::INSTANCE => Dice::SELF], - ] - ], IStorage::class => [ - // Don't share this class with other creations, because it's possible to switch the backend - // and so we wouldn't be possible to update it - 'shared' => false, 'instanceOf' => StorageManager::class, 'call' => [ ['getBackend', [], Dice::CHAIN_CALL], diff --git a/tests/Util/SampleStorageBackend.php b/tests/Util/SampleStorageBackend.php index 3f4eb90ada..a788c6d3df 100644 --- a/tests/Util/SampleStorageBackend.php +++ b/tests/Util/SampleStorageBackend.php @@ -2,9 +2,12 @@ namespace Friendica\Test\Util; +use Friendica\App; +use Friendica\Core\Hook; use Friendica\Model\Storage\IStorage; use Friendica\Core\L10n\L10n; +use Mockery\MockInterface; /** * A backend storage example class @@ -91,4 +94,13 @@ class SampleStorageBackend implements IStorage { return self::NAME; } + + /** + * This one is a hack to register this class to the hook + */ + public static function registerHook() + { + Hook::register('storage_instance', __DIR__ . '/SampleStorageBackendInstance.php', 'create_instance'); + } } + diff --git a/tests/Util/SampleStorageBackendInstance.php b/tests/Util/SampleStorageBackendInstance.php new file mode 100644 index 0000000000..d55ff04dea --- /dev/null +++ b/tests/Util/SampleStorageBackendInstance.php @@ -0,0 +1,18 @@ +setUpVfsDir(); $this->logger = new NullLogger(); - $this->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]); $profiler = \Mockery::mock(Profiler::class); $profiler->shouldReceive('saveTimestamp')->withAnyArgs()->andReturn(true); @@ -58,6 +63,8 @@ class StorageManagerTest extends DatabaseTest $configModel = new Config($this->dba); $this->config = new PreloadConfiguration($configCache, $configModel); + + $this->l10n = \Mockery::mock(L10n::class); } /** @@ -65,7 +72,7 @@ class StorageManagerTest extends DatabaseTest */ public function testInstance() { - $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->dice); + $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->l10n); $this->assertInstanceOf(StorageManager::class, $storageManager); } @@ -113,11 +120,11 @@ class StorageManagerTest extends DatabaseTest * * @dataProvider dataStorages */ - public function testGetByName($name, $assert, $assertName) + public function testGetByName($name, $assert, $assertName, $userBackend) { - $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->dice); + $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->l10n); - $storage = $storageManager->getByName($name); + $storage = $storageManager->getByName($name, $userBackend); if (!empty($assert)) { $this->assertInstanceOf(Storage\IStorage::class, $storage); @@ -136,7 +143,7 @@ class StorageManagerTest extends DatabaseTest */ public function testIsValidBackend($name, $assert, $assertName, $userBackend) { - $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->dice); + $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->l10n); $this->assertEquals($userBackend, $storageManager->isValidBackend($name)); } @@ -146,7 +153,7 @@ class StorageManagerTest extends DatabaseTest */ public function testListBackends() { - $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->dice); + $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->l10n); $this->assertEquals(StorageManager::DEFAULT_BACKENDS, $storageManager->listBackends()); } @@ -158,7 +165,7 @@ class StorageManagerTest extends DatabaseTest */ public function testGetBackend($name, $assert, $assertName, $userBackend) { - $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->dice); + $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->l10n); $this->assertNull($storageManager->getBackend()); @@ -178,7 +185,7 @@ class StorageManagerTest extends DatabaseTest { $this->config->set('storage', 'name', $name); - $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->dice); + $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->l10n); if ($userBackend) { $this->assertInstanceOf($assert, $storageManager->getBackend()); @@ -196,17 +203,28 @@ class StorageManagerTest extends DatabaseTest */ public function testRegisterUnregisterBackends() { - $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->dice); + /// @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); $this->assertTrue($storageManager->register(SampleStorageBackend::class)); $this->assertEquals(array_merge(StorageManager::DEFAULT_BACKENDS, [ - 'Sample Storage' => SampleStorageBackend::class, + SampleStorageBackend::getName() => SampleStorageBackend::class, ]), $storageManager->listBackends()); $this->assertEquals(array_merge(StorageManager::DEFAULT_BACKENDS, [ - 'Sample Storage' => SampleStorageBackend::class, + SampleStorageBackend::getName() => SampleStorageBackend::class, ]), $this->config->get('storage', 'backends')); + // inline call to register own class as hook (testing purpose only) + SampleStorageBackend::registerHook(); + Hook::loadHooks(); + $this->assertTrue($storageManager->setBackend(SampleStorageBackend::NAME)); $this->assertEquals(SampleStorageBackend::NAME, $this->config->get('storage', 'name')); @@ -233,7 +251,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->dice); + $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->l10n); $storage = $storageManager->getByName($name); $storageManager->move($storage);