Refactor IStorage

src/Console/Storage.php

@@ -131,10 +131,10 @@ HELP;
 			throw new CommandArgsException('Invalid arguments');
 		}
 
-		$name = $this->args[1];
+		$name  = $this->args[1];
-		$class = $this->storageManager->getByName($name);
+		$class = $this->storageManager->getSelectableStorageByName($name);
 
-		if ($class === '') {
+		if (is_null($class)) {
 			$this->out($name . ' is not a registered backend.');
 			return -1;
 		}

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)) {
+		if ($this->config->set('storage', 'name', $storage::getName())) {
-			return false;
+			$this->currentBackend = $storage;
-		}
-
-		if ($this->config->set('storage', 'name', $name)) {
-			$this->currentBackend = $this->getByName($name, false);
 			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)) {

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

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

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 {

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

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;
 	}
 
 	/**

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 */
+		/** @var ISelectableStorage $newstorage */
-		$newstorage = DI::storageManager()->getByName($storagebackend);
+		$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,
 		]);
 	}

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

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

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