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

@@ -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.
+`DI::facStorage()->register(string $class)` is used to register the backend class.
-The `$name` must be univocal and will be shown to admin.
 
 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`:

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

doc/de/Addons.md

@@ -425,6 +425,10 @@ Eine komplette Liste aller Hook-Callbacks mit den zugehörigen Dateien (am 01-Ap
 
     Hook::callAll('logged_in', $a->user);
     
+### src/Core/StorageManager
+
+    Hook::callAll('storage_instance', $data);
+
 ### src/Worker/Directory.php
 
     Hook::callAll('globaldir_update', $arr);

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 */
+	/** @var L10n */
-	private $dice;
+	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->getByName($currentName);
-			$this->currentBackend = $this->dice->create($this->backends[$currentName]);
-		} else {
-			$this->currentBackend = null;
-		}
 	}
 
 	/**
@@ -78,40 +79,64 @@ class StorageManager
 	 * @brief Return storage backend class by registered 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) &&
+		// If there's no cached instance create a new instance
-		    $name !== Storage\SystemResource::getName()) {
+		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;
-		/** @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 null;
+			}
 		}
 
-		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 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
 	 *

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],

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');
 	}
+}
+

tests/Util/SampleStorageBackendInstance.php

@@ -0,0 +1,18 @@
+<?php
+
+// contains a test-hook call for creating a storage instance
+
+use Friendica\App;
+use Friendica\Core\L10n\L10n;
+use Friendica\Test\Util\SampleStorageBackend;
+use Mockery\MockInterface;
+
+function create_instance(App $a, &$data)
+{
+	/** @var L10n|MockInterface $l10n */
+	$l10n = \Mockery::mock(L10n::class);
+
+	if ($data['name'] == SampleStorageBackend::getName()) {
+		$data['storage'] = new SampleStorageBackend($l10n);
+	}
+}

tests/src/Core/StorageManagerTest.php

@@ -5,9 +5,12 @@ namespace Friendica\Test\src\Core;
 use Dice\Dice;
 use Friendica\Core\Config\IConfiguration;
 use Friendica\Core\Config\PreloadConfiguration;
+use Friendica\Core\Hook;
+use Friendica\Core\L10n\L10n;
 use Friendica\Core\Session\ISession;
 use Friendica\Core\StorageManager;
 use Friendica\Database\Database;
+use Friendica\DI;
 use Friendica\Factory\ConfigFactory;
 use Friendica\Model\Config\Config;
 use Friendica\Model\Storage;
@@ -21,6 +24,12 @@ use Psr\Log\LoggerInterface;
 use Psr\Log\NullLogger;
 use Friendica\Test\Util\SampleStorageBackend;
 
+/**
+ * @todo Rework Hook:: methods to dynamic to remove the separated process annotation
+ *
+ * @runTestsInSeparateProcesses
+ * @preserveGlobalState disabled
+ */
 class StorageManagerTest extends DatabaseTest
 {
 	/** @var Database */
@@ -29,8 +38,8 @@ class StorageManagerTest extends DatabaseTest
 	private $config;
 	/** @var LoggerInterface */
 	private $logger;
-	/** @var Dice */
+	/** @var L10n */
-	private $dice;
+	private $l10n;
 
 	use VFSTrait;
 
@@ -41,10 +50,6 @@ class StorageManagerTest extends DatabaseTest
 		$this->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);