Introduce InvalidClassStorageException and adapt the code for it

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,22 +128,22 @@ 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];
+		try {
 			$class = $this->storageManager->getWritableStorageByName($name);
 
-		if (is_null($class)) {
-			$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;
 			}
+		} catch (ReferenceStorageException $exception) {
+			$this->out($name . ' is not a registered backend.');
+			return -1;
+		}
 
 		return 0;
 	}

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,6 +69,9 @@ 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)
 	{
@@ -79,7 +81,7 @@ class StorageManager
 		$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
+		$storage = $this->getByName($name, $this->backends);
-		$name = $this->checkLegacyBackend($name);
+		if ($storage instanceof Storage\IWritableStorage) {
-
+			return $storage;
-		// 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));
+			throw new Storage\InvalidClassStorageException(sprintf('Backend %s is not writable', $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));
-			}
-		}
-
-		return $this->backendInstances[$name];
 	}
 
 	/**
 	 * Return storage backend class by registered 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));
 			}
 		}
 
@@ -212,33 +179,18 @@ 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 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) ||
+		$validBackends = $validBackends ?? array_merge($this->backends,
-		       (!$onlyUserBackend && in_array($name, [Storage\SystemResource::getName(), Storage\ExternalResource::getName()]));
+				[
-	}
+					Storage\SystemResource::getName()   => '',
-
+					Storage\ExternalResource::getName() => ''
-	/**
+				]);
-	 * Check for legacy backend storage class names (= full model class name)
+		return array_key_exists($name, $validBackends);
-	 *
-	 * @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;
 	}
 
 	/**
@@ -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()]);

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,24 +165,22 @@ class Attach
 			return $item['data'];
 		}
 
+		try {
 			$backendClass = DI::storageManager()->getByName($item['backend-class'] ?? '');
-		if (empty($backendClass)) {
+			$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 '';
 		}
 	}
-	}
 
 	/**
 	 * Store new file metadata in db and binary in default backend
@@ -284,11 +283,13 @@ class Attach
 			$items = self::selectToArray(['backend-class','backend-ref'], $conditions);
 
 			foreach($items as $item) {
+				try {
 					$backend_class         = DI::storageManager()->getWritableStorageByName($item['backend-class'] ?? '');
-				if (!empty($backend_class)) {
 					$fields['backend-ref'] = $backend_class->put($img->asString(), $item['backend-ref'] ?? '');
-				} else {
+				} catch (InvalidClassStorageException $storageException) {
-					$fields['data'] = $img->asString();
+					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,15 +317,15 @@ 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'] ?? '');
 				$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]);
 			}
 		}
-		}
 
 		return DBA::delete('attach', $conditions, $options);
 	}

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,25 +194,37 @@ class Photo
 			return $photo['data'];
 		}
 
+		try {
 			$backendClass = DI::storageManager()->getByName($photo['backend-class'] ?? '');
-		if (empty($backendClass)) {
+			$image        = $backendClass->get($photo['backend-ref'] ?? '');
+
+			if ($image instanceof Image) {
+				return $image;
+			} else {
+				DI::logger()->info('Stored data is not an image', ['photo' => $photo]);
+			}
+		} catch (InvalidClassStorageException $storageException) {
+			try {
 				// legacy data storage in "data" column
 				$i = self::selectFirst(['data'], ['id' => $photo['id']]);
-			if ($i === false) {
+				if ($i !== false) {
-				return null;
+					return $i['data'];
-			}
-			$data = $i['data'];
 				} else {
-			$backendRef = $photo['backend-ref'] ?? '';
+					DI::logger()->info('Stored legacy data is empty', ['photo' => $photo]);
-			try {
+				}
-				$data = $backendClass->get($backendRef);
+			} catch (\Exception $exception) {
+				DI::logger()->info('Unexpected database exception', ['photo' => $photo, 'exception' => $exception]);
+			}
 		} catch (ReferenceStorageException $referenceStorageException) {
-				DI::logger()->debug('No data found for photo', ['photo' => $photo, 'exception' => $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 null;
 	}
-		}
-		return $data;
-	}
 
 	/**
 	 * Get Image object for given row id. null if row id does not exist
@@ -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);
+		return new Image(self::getImageDataForPhoto($photo), $photo['type']);
-		if (empty($data)) {
-			return null;
-		}
-
-		return new Image($data, $photo['type']);
 	}
 
 	/**
@@ -344,18 +349,18 @@ class Photo
 		// if is an existing photo, reuse same backend
 		$data        = "";
 		$backend_ref = "";
+		$storage     = "";
 
+		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();
 			}
-
-		if (empty($storage)) {
-			$data = $Image->asString();
-		} else {
 			$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 = 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('phot doesn\'t exist.', ['conditions' => $conditions, 'exception' => $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) {
+				try {
 					$backend_class         = DI::storageManager()->getWritableStorageByName($photo['backend-class'] ?? '');
-				if (!empty($backend_class)) {
 					$fields["backend-ref"] = $backend_class->put($img->asString(), $photo['backend-ref']);
-				} else {
+				} catch (InvalidClassStorageException $storageException) {
 					$fields["data"] = $img->asString();
 				}
 			}

src/Model/Storage/InvalidClassStorageException.php

@@ -0,0 +1,29 @@
+<?php
+/**
+ * @copyright Copyright (C) 2010-2021, the Friendica project
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <https://www.gnu.org/licenses/>.
+ *
+ */
+
+namespace Friendica\Model\Storage;
+
+/**
+ * Storage Exception in case of invalid storage class
+ */
+class InvalidClassStorageException extends StorageException
+{
+}

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'] ?? ''));
 
+		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,18 +68,22 @@ 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'])) {
+			try {
 				/** @var IWritableStorage $newstorage */
 				$newstorage = DI::storageManager()->getWritableStorageByName($storagebackend);
 
 				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.'));
+			}
 		}
 
 		DI::baseUrl()->redirect('admin/storage');

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])) {

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,

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);
 
@@ -103,32 +102,36 @@ class StorageManagerTest extends DatabaseTest
 		return [
 			'empty' => [
 				'name'       => '',
+				'valid'      => false,
+				'interface'  => Storage\IStorage::class,
 				'assert'     => null,
 				'assertName' => '',
-				'userBackend' => false,
 			],
 			'database' => [
 				'name'       => Storage\Database::NAME,
+				'valid'      => true,
+				'interface'  => Storage\IWritableStorage::class,
 				'assert'     => Storage\Database::class,
 				'assertName' => Storage\Database::NAME,
-				'userBackend' => true,
 			],
 			'filesystem' => [
 				'name'       => Storage\Filesystem::NAME,
+				'valid'      => true,
+				'interface'  => Storage\IWritableStorage::class,
 				'assert'     => Storage\Filesystem::class,
 				'assertName' => Storage\Filesystem::NAME,
-				'userBackend' => true,
 			],
 			'systemresource' => [
 				'name'       => Storage\SystemResource::NAME,
+				'valid'      => true,
+				'interface'  => Storage\IStorage::class,
 				'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,
 			],
 			'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($interface, $storage);
-			self::assertInstanceOf(Storage\IStorage::class, $storage);
 		self::assertInstanceOf($assert, $storage);
-		} else {
-			self::assertNull($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
+		// if it's a IWritableStorage, the valid backend should return true, otherwise false
-		self::assertEquals($userBackend, $storageManager->isValidBackend($name, true));
+		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());
-
-		if ($userBackend) {
 		$selBackend = $storageManager->getWritableStorageByName($name);
 		$storageManager->setBackend($selBackend);
 
 		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());
-		}
 	}
 
 	/**
@@ -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,9 +278,9 @@ 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");
 		}
 
@@ -320,7 +293,6 @@ class StorageManagerTest extends DatabaseTest
 		$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']);
@@ -335,7 +307,8 @@ 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());