Split IStorage and ISelectableStorage and make their behaviour homogenous

src/Model/Storage/Database.php

@@ -21,8 +21,7 @@
 
 namespace Friendica\Model\Storage;
 
-use Friendica\Core\L10n;
+use Exception;
-use Psr\Log\LoggerInterface;
 use Friendica\Database\Database as DBA;
 
 /**
@@ -30,7 +29,7 @@ use Friendica\Database\Database as DBA;
  *
  * This class manage data stored in database table.
  */
-class Database extends AbstractStorage
+class Database implements ISelectableStorage
 {
 	const NAME = 'Database';
 
@@ -39,47 +38,53 @@ class Database extends AbstractStorage
 
 	/**
 	 * @param DBA             $dba
-	 * @param LoggerInterface $logger
-	 * @param L10n            $l10n
 	 */
-	public function __construct(DBA $dba, LoggerInterface $logger, L10n $l10n)
+	public function __construct(DBA $dba)
 	{
-		parent::__construct($l10n, $logger);
-
 		$this->dba = $dba;
 	}
 
 	/**
 	 * @inheritDoc
 	 */
-	public function get(string $reference)
+	public function get(string $reference): string
 	{
-		$result = $this->dba->selectFirst('storage', ['data'], ['id' => $reference]);
+		try {
-		if (!$this->dba->isResult($result)) {
+			$result = $this->dba->selectFirst('storage', ['data'], ['id' => $reference]);
-			return '';
+			if (!$this->dba->isResult($result)) {
-		}
+				throw new ReferenceStorageException(sprintf('Database storage cannot find data for reference %s', $reference));
+			}
 
-		return $result['data'];
+			return $result['data'];
+		} catch (Exception $exception) {
+			throw new StorageException(sprintf('Database storage failed to get %s', $reference), $exception->getCode(), $exception);
+		}
 	}
 
 	/**
 	 * @inheritDoc
 	 */
-	public function put(string $data, string $reference = '')
+	public function put(string $data, string $reference = ''): string
 	{
 		if ($reference !== '') {
-			$result = $this->dba->update('storage', ['data' => $data], ['id' => $reference]);
+			try {
+				$result = $this->dba->update('storage', ['data' => $data], ['id' => $reference]);
+			} catch (Exception $exception) {
+				throw new StorageException(sprintf('Database storage failed to update %s', $reference), $exception->getCode(), $exception);
+			}
 			if ($result === false) {
-				$this->logger->warning('Failed to update data.', ['id' => $reference, 'errorCode' => $this->dba->errorNo(), 'errorMessage' => $this->dba->errorMessage()]);
+				throw new StorageException(sprintf('Database storage failed to update %s', $reference), 500, new Exception($this->dba->errorMessage(), $this->dba->errorNo()));
-				throw new StorageException($this->l10n->t('Database storage failed to update %s', $reference));
 			}
 
 			return $reference;
 		} else {
-			$result = $this->dba->insert('storage', ['data' => $data]);
+			try {
+				$result = $this->dba->insert('storage', ['data' => $data]);
+			} catch (Exception $exception) {
+				throw new StorageException(sprintf('Database storage failed to insert %s', $reference), $exception->getCode(), $exception);
+			}
 			if ($result === false) {
-				$this->logger->warning('Failed to insert data.', ['errorCode' => $this->dba->errorNo(), 'errorMessage' => $this->dba->errorMessage()]);
+				throw new StorageException(sprintf('Database storage failed to update %s', $reference), 500, new Exception($this->dba->errorMessage(), $this->dba->errorNo()));
-				throw new StorageException($this->l10n->t('Database storage failed to insert data'));
 			}
 
 			return $this->dba->lastInsertId();
@@ -91,13 +96,19 @@ class Database extends AbstractStorage
 	 */
 	public function delete(string $reference)
 	{
-		return $this->dba->delete('storage', ['id' => $reference]);
+		try {
+			if (!$this->dba->delete('storage', ['id' => $reference])) {
+				throw new ReferenceStorageException(sprintf('Database storage failed to delete %s', $reference));
+			}
+		} catch (Exception $exception) {
+			throw new StorageException(sprintf('Database storage failed to delete %s', $reference), $exception->getCode(), $exception);
+		}
 	}
 
 	/**
 	 * @inheritDoc
 	 */
-	public function getOptions()
+	public function getOptions(): array
 	{
 		return [];
 	}
@@ -105,7 +116,7 @@ class Database extends AbstractStorage
 	/**
 	 * @inheritDoc
 	 */
-	public function saveOptions(array $data)
+	public function saveOptions(array $data): array
 	{
 		return [];
 	}
@@ -113,8 +124,13 @@ class Database extends AbstractStorage
 	/**
 	 * @inheritDoc
 	 */
-	public static function getName()
+	public static function getName(): string
 	{
 		return self::NAME;
 	}
+
+	public function __toString()
+	{
+		return self::getName();
+	}
 }

src/Model/Storage/ExternalResource.php

@@ -21,9 +21,8 @@
 
 namespace Friendica\Model\Storage;
 
-use BadMethodCallException;
+use Exception;
 use Friendica\Util\HTTPSignature;
-use Friendica\Network\IHTTPRequest;
 
 /**
  * External resource storage class
@@ -35,66 +34,33 @@ class ExternalResource implements IStorage
 {
 	const NAME = 'ExternalResource';
 
-	/** @var IHTTPRequest */
-	private $httpRequest;
-
-	public function __construct(IHTTPRequest $httpRequest)
-	{
-		$this->httpRequest = $httpRequest;
-	}
-
 	/**
 	 * @inheritDoc
 	 */
-	public function get(string $reference)
+	public function get(string $reference): string
 	{
 		$data = json_decode($reference);
 		if (empty($data->url)) {
-			return "";
+			throw new ReferenceStorageException(sprintf('Invalid reference %s, cannot retrieve URL', $reference));
 		}
 
 		$parts = parse_url($data->url);
 		if (empty($parts['scheme']) || empty($parts['host'])) {
-			return "";
+			throw new ReferenceStorageException(sprintf('Invalid reference %s, cannot extract scheme and host', $reference));
 		}
 
-		$fetchResult = HTTPSignature::fetchRaw($data->url, $data->uid, ['accept_content' => '']);
+		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);
+		}
 		if ($fetchResult->isSuccess()) {
 			return $fetchResult->getBody();
 		} else {
-			return "";
+			throw new StorageException(sprintf('External resource failed to get %s', $reference), $fetchResult->getReturnCode(), new Exception($fetchResult->getBody()));
 		}
 	}
 
-	/**
-	 * @inheritDoc
-	 */
-	public function put(string $data, string $reference = '')
-	{
-		throw new BadMethodCallException();
-	}
-
-	public function delete(string $reference)
-	{
-		throw new BadMethodCallException();
-	}
-
-	/**
-	 * @inheritDoc
-	 */
-	public function getOptions()
-	{
-		return [];
-	}
-
-	/**
-	 * @inheritDoc
-	 */
-	public function saveOptions(array $data)
-	{
-		return [];
-	}
-
 	/**
 	 * @inheritDoc
 	 */
@@ -106,7 +72,7 @@ class ExternalResource implements IStorage
 	/**
 	 * @inheritDoc
 	 */
-	public static function getName()
+	public static function getName(): string
 	{
 		return self::NAME;
 	}

src/Model/Storage/Filesystem.php

@@ -21,10 +21,10 @@
 
 namespace Friendica\Model\Storage;
 
+use Exception;
 use Friendica\Core\Config\IConfig;
 use Friendica\Core\L10n;
 use Friendica\Util\Strings;
-use Psr\Log\LoggerInterface;
 
 /**
  * Filesystem based storage backend
@@ -36,7 +36,7 @@ use Psr\Log\LoggerInterface;
  * Each new resource gets a value as reference and is saved in a
  * folder tree stucture created from that value.
  */
-class Filesystem extends AbstractStorage
+class Filesystem implements ISelectableStorage
 {
 	const NAME = 'Filesystem';
 
@@ -49,18 +49,19 @@ class Filesystem extends AbstractStorage
 	/** @var string */
 	private $basePath;
 
+	/** @var L10n */
+	private $l10n;
+
 	/**
 	 * Filesystem constructor.
 	 *
 	 * @param IConfig         $config
-	 * @param LoggerInterface $logger
 	 * @param L10n            $l10n
 	 */
-	public function __construct(IConfig $config, LoggerInterface $logger, L10n $l10n)
+	public function __construct(IConfig $config, L10n $l10n)
 	{
-		parent::__construct($l10n, $logger);
-
 		$this->config = $config;
+		$this->l10n   = $l10n;
 
 		$path           = $this->config->get('storage', 'filesystem_path', self::DEFAULT_BASE_FOLDER);
 		$this->basePath = rtrim($path, '/');
@@ -73,7 +74,7 @@ class Filesystem extends AbstractStorage
 	 *
 	 * @return string
 	 */
-	private function pathForRef(string $reference)
+	private function pathForRef(string $reference): string
 	{
 		$fold1 = substr($reference, 0, 2);
 		$fold2 = substr($reference, 2, 2);
@@ -84,7 +85,7 @@ class Filesystem extends AbstractStorage
 
 
 	/**
-	 * Create dirctory tree to store file, with .htaccess and index.html files
+	 * Create directory tree to store file, with .htaccess and index.html files
 	 *
 	 * @param string $file Path and filename
 	 *
@@ -96,8 +97,7 @@ class Filesystem extends AbstractStorage
 
 		if (!is_dir($path)) {
 			if (!mkdir($path, 0770, true)) {
-				$this->logger->warning('Failed to create dir.', ['path' => $path]);
+				throw new StorageException(sprintf('Filesystem storage failed to create "%s". Check you write permissions.', $path));
-				throw new StorageException($this->l10n->t('Filesystem storage failed to create "%s". Check you write permissions.', $path));
 			}
 		}
 
@@ -118,23 +118,32 @@ class Filesystem extends AbstractStorage
 	/**
 	 * @inheritDoc
 	 */
-	public function get(string $reference)
+	public function get(string $reference): string
 	{
 		$file = $this->pathForRef($reference);
 		if (!is_file($file)) {
-			return '';
+			throw new ReferenceStorageException(sprintf('Filesystem storage failed to get the file %s, The file is invalid', $reference));
 		}
 
-		return file_get_contents($file);
+		$result = file_get_contents($file);
+
+		// just in case the result is REALLY false, not zero or empty or anything else, throw the exception
+		if ($result === false) {
+			throw new StorageException(sprintf('Filesystem storage failed to get data to "%s". Check your write permissions', $file));
+		}
 	}
 
 	/**
 	 * @inheritDoc
 	 */
-	public function put(string $data, string $reference = '')
+	public function put(string $data, string $reference = ''): string
 	{
 		if ($reference === '') {
-			$reference = Strings::getRandomHex();
+			try {
+				$reference = Strings::getRandomHex();
+			} catch (Exception $exception) {
+				throw new StorageException('Filesystem storage failed to generate a random hex', $exception->getCode(), $exception);
+			}
 		}
 		$file = $this->pathForRef($reference);
 
@@ -144,8 +153,7 @@ class Filesystem extends AbstractStorage
 
 		// just in case the result is REALLY false, not zero or empty or anything else, throw the exception
 		if ($result === false) {
-			$this->logger->warning('Failed to write data.', ['file' => $file]);
+			throw new StorageException(sprintf('Filesystem storage failed to save data to "%s". Check your write permissions', $file));
-			throw new StorageException($this->l10n->t('Filesystem storage failed to save data to "%s". Check your write permissions', $file));
 		}
 
 		chmod($file, 0660);
@@ -158,17 +166,19 @@ class Filesystem extends AbstractStorage
 	public function delete(string $reference)
 	{
 		$file = $this->pathForRef($reference);
-		// return true if file doesn't exists. we want to delete it: success with zero work!
 		if (!is_file($file)) {
-			return true;
+			throw new ReferenceStorageException(sprintf('File with reference "%s" doesn\'t exist', $reference));
+		}
+
+		if (!unlink($file)) {
+			throw new StorageException(sprintf('Cannot delete with file with reference "%s"', $reference));
 		}
-		return unlink($file);
 	}
 
 	/**
 	 * @inheritDoc
 	 */
-	public function getOptions()
+	public function getOptions(): array
 	{
 		return [
 			'storagepath' => [
@@ -183,7 +193,7 @@ class Filesystem extends AbstractStorage
 	/**
 	 * @inheritDoc
 	 */
-	public function saveOptions(array $data)
+	public function saveOptions(array $data): array
 	{
 		$storagePath = $data['storagepath'] ?? '';
 		if ($storagePath === '' || !is_dir($storagePath)) {
@@ -199,8 +209,13 @@ class Filesystem extends AbstractStorage
 	/**
 	 * @inheritDoc
 	 */
-	public static function getName()
+	public static function getName(): string
 	{
 		return self::NAME;
 	}
+
+	public function __toString()
+	{
+		return self::getName();
+	}
 }

src/Model/Storage/ISelectableStorage.php

@@ -0,0 +1,100 @@
+<?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;
+
+/**
+ * Interface for selectable storage backends
+ */
+interface ISelectableStorage extends IStorage
+{
+	/**
+	 * Put data in backend as $ref. If $ref is not defined a new reference is created.
+	 *
+	 * @param string $data      Data to save
+	 * @param string $reference Data reference. Optional.
+	 *
+	 * @return string Saved data reference
+	 *
+	 * @throws StorageException in case there's an unexpected error
+	 */
+	public function put(string $data, string $reference = ""): string;
+
+	/**
+	 * Remove data from backend
+	 *
+	 * @param string $reference Data reference
+	 *
+	 * @throws StorageException in case there's an unexpected error
+	 * @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;
+}

src/Model/Storage/IStorage.php

@@ -22,9 +22,7 @@
 namespace Friendica\Model\Storage;
 
 /**
- * Interface for storage backends
+ * Interface for basic storage backends
- *
- * @todo Split this interface into "IStorage" for get() operations (including Resource fetching) and "IUserStorage" for real user backends including put/delete/options
  */
 interface IStorage
 {
@@ -34,77 +32,11 @@ interface IStorage
 	 * @param string $reference Data reference
 	 *
 	 * @return string
+	 *
+	 * @throws StorageException in case there's an unexpected error
+	 * @throws ReferenceStorageException in case the reference doesn't exist
 	 */
-	public function get(string $reference);
+	public function get(string $reference): string;
-
-	/**
-	 * Put data in backend as $ref. If $ref is not defined a new reference is created.
-	 *
-	 * @param string $data      Data to save
-	 * @param string $reference Data reference. Optional.
-	 *
-	 * @return string Saved data reference
-	 */
-	public function put(string $data, string $reference = "");
-
-	/**
-	 * Remove data from backend
-	 *
-	 * @param string $reference Data reference
-	 *
-	 * @return boolean  True on success
-	 */
-	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();
-
-	/**
-	 * 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);
 
 	/**
 	 * The name of the backend
@@ -118,5 +50,5 @@ interface IStorage
 	 *
 	 * @return string
 	 */
-	public static function getName();
+	public static function getName(): string;
 }

src/Model/Storage/ReferenceStorageException.php

@@ -21,31 +21,11 @@
 
 namespace Friendica\Model\Storage;
 
-use Friendica\Core\L10n;
+use Exception;
-use Psr\Log\LoggerInterface;
 
 /**
- * A general storage class which loads common dependencies and implements common methods
+ * Storage Exception in case of invalid references
  */
-abstract class AbstractStorage implements IStorage
+class ReferenceStorageException extends Exception
 {
-	/** @var L10n */
-	protected $l10n;
-	/** @var LoggerInterface */
-	protected $logger;
-
-	/**
-	 * @param L10n            $l10n
-	 * @param LoggerInterface $logger
-	 */
-	public function __construct(L10n $l10n, LoggerInterface $logger)
-	{
-		$this->l10n   = $l10n;
-		$this->logger = $logger;
-	}
-
-	public function __toString()
-	{
-		return static::getName();
-	}
 }

src/Model/Storage/StorageException.php

@@ -21,9 +21,11 @@
 
 namespace Friendica\Model\Storage;
 
+use Exception;
+
 /**
- * Storage Exception
+ * Storage Exception for unexpected failures
  */
-class StorageException extends \Exception
+class StorageException extends Exception
 {
 }

src/Model/Storage/SystemResource.php

@@ -21,8 +21,6 @@
 
 namespace Friendica\Model\Storage;
 
-use \BadMethodCallException;
-
 /**
  * System resource storage class
  *
@@ -39,45 +37,22 @@ class SystemResource implements IStorage
 	/**
 	 * @inheritDoc
 	 */
-	public function get(string $filename)
+	public function get(string $reference): string
 	{
-		$folder = dirname($filename);
+		$folder = dirname($reference);
 		if (!in_array($folder, self::VALID_FOLDERS)) {
-			return "";
+			throw new ReferenceStorageException(sprintf('System Resource is invalid for reference %s, no valid folder found', $reference));
 		}
-		if (!file_exists($filename)) {
+		if (!file_exists($reference)) {
-			return "";
+			throw new StorageException(sprintf('System Resource is invalid for reference %s, the file doesn\'t exist', $reference));
 		}
-		return file_get_contents($filename);
+		$content = file_get_contents($reference);
-	}
 
-	/**
+		if ($content === false) {
-	 * @inheritDoc
+			throw new StorageException(sprintf('Cannot get content for reference %s', $reference));
-	 */
+		}
-	public function put(string $data, string $filename = '')
-	{
-		throw new BadMethodCallException();
-	}
 
-	public function delete(string $filename)
+		return $content;
-	{
-		throw new BadMethodCallException();
-	}
-
-	/**
-	 * @inheritDoc
-	 */
-	public function getOptions()
-	{
-		return [];
-	}
-
-	/**
-	 * @inheritDoc
-	 */
-	public function saveOptions(array $data)
-	{
-		return [];
 	}
 
 	/**
@@ -91,7 +66,7 @@ class SystemResource implements IStorage
 	/**
 	 * @inheritDoc
 	 */
-	public static function getName()
+	public static function getName(): string
 	{
 		return self::NAME;
 	}

src/Util/HTTPSignature.php

@@ -28,6 +28,7 @@ use Friendica\DI;
 use Friendica\Model\APContact;
 use Friendica\Model\Contact;
 use Friendica\Model\User;
+use Friendica\Network\CurlResult;
 
 /**
  * Implements HTTP Signatures per draft-cavage-http-signatures-07.
@@ -408,7 +409,7 @@ class HTTPSignature
 	 *                         'nobody' => only return the header
 	 *                         'cookiejar' => path to cookie jar file
 	 *
-	 * @return object CurlResult
+	 * @return CurlResult|void CurlResult
 	 * @throws \Friendica\Network\HTTPException\InternalServerErrorException
 	 */
 	public static function fetchRaw($request, $uid = 0, $opts = ['accept_content' => 'application/activity+json, application/ld+json'])