Refactor IStorage

diff --git a/src/Console/Storage.php b/src/Console/Storage.php
index 93bbb26579a28db8b72110d29b69ccc7b844b659..70e8e263024d8af6bc9b0b79c456f8501ee127ad 100644 (file)
--- a/src/Console/Storage.php
+++ b/src/Console/Storage.php
@@ -131,10 +131,10 @@ HELP;
                        throw new CommandArgsException('Invalid arguments');
                }
 
-               $name = $this->args[1];
-               $class = $this->storageManager->getByName($name);
+               $name  = $this->args[1];
+               $class = $this->storageManager->getSelectableStorageByName($name);
 
-               if ($class === '') {
+               if (is_null($class)) {
                        $this->out($name . ' is not a registered backend.');
                        return -1;
                }

diff --git a/src/Core/StorageManager.php b/src/Core/StorageManager.php
index 64e53c10b9f1f62883fa28c23919809774c8947e..03f89f312a6e3d4faffafd683086bfa32f30d2e7 100644 (file)
--- a/src/Core/StorageManager.php
+++ b/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,42 +71,88 @@ 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()
        {
                return $this->currentBackend;
        }
 
+       /**
+        * Returns a selectable storage backend class by registered name
+        *
+        * @param string|null $name            Backend name
+        *
+        * @return Storage\ISelectableStorage|null null if no backend registered at $name
+        *
+        * @throws \Friendica\Network\HTTPException\InternalServerErrorException
+        */
+       public function getSelectableStorageByName(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, 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,
+                                               ];
+                                               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
-        * @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
         *
         * @throws \Friendica\Network\HTTPException\InternalServerErrorException
         */
-       public function getByName(string $name = null, $onlyUserBackend = false)
+       public function getByName(string $name = null)
        {
                // @todo 2020.09 Remove this call after 2 releases
                $name = $this->checkLegacyBackend($name);
@@ -116,22 +160,22 @@ 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, false)) {
                                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;
                                        // 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)) {
-                       return false;
-               }
-
-               if ($this->config->set('storage', 'name', $name)) {
-                       $this->currentBackend = $this->getByName($name, false);
+               if ($this->config->set('storage', 'name', $storage::getName())) {
+                       $this->currentBackend = $storage;
                        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)) {

diff --git a/src/DI.php b/src/DI.php
index 02620ea11eac21ce38e78c87dccf06ee02675694..9e96ddc0158cd824e8d4cd97f6be4e357c4dbab3 100644 (file)
--- a/src/DI.php
+++ b/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);
        }
 
        //

diff --git a/src/Model/Attach.php b/src/Model/Attach.php
index 6182727c9762ad85d63c6365941bc8636129c5aa..d3ff5462ca69ad5f6d54ca9f64cf03d1adf6d0f4 100644 (file)
--- a/src/Model/Attach.php
+++ b/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]);
+                               }
                        }
                }
 

diff --git a/src/Model/Photo.php b/src/Model/Photo.php
index 26369a3540f0f4c5153188c1430f32547a92ed80..f74c68da583c770111212e125245be4c23bafaec 100644 (file)
--- a/src/Model/Photo.php
+++ b/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 {

diff --git a/src/Model/Storage/ExternalResource.php b/src/Model/Storage/ExternalResource.php
index 402619e67545d2bbfc67b92773fb81c137ea97c2..918bcf8ac315b9d1f867e93ad2c1a38ccdd9ee26 100644 (file)
--- a/src/Model/Storage/ExternalResource.php
+++ b/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()));
                }
        }
 

diff --git a/src/Model/Storage/Filesystem.php b/src/Model/Storage/Filesystem.php
index 7760b4a5304b276c9a4fdaaaf1114ec4b241e479..fe5230049f8d18f4af16625c35b17ff01ba6fc55 100644 (file)
--- a/src/Model/Storage/Filesystem.php
+++ b/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;
        }
 
        /**

diff --git a/src/Module/Admin/Storage.php b/src/Module/Admin/Storage.php
index 101070712450c534e700e08053277603918440aa..374d692bf7330273b7c680494990d7e1feda6748 100644 (file)
--- a/src/Module/Admin/Storage.php
+++ b/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 */
-               $newstorage = DI::storageManager()->getByName($storagebackend);
+               /** @var ISelectableStorage $newstorage */
+               $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,
                ]);
        }

diff --git a/src/Worker/MoveStorage.php b/src/Worker/MoveStorage.php
index ac70c63545f129d80287f8243812b58767bf83af..34ecb5c147f2b384c620d8461f5d925b7bc34567 100644 (file)
--- a/src/Worker/MoveStorage.php
+++ b/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');

diff --git a/static/dependencies.config.php b/static/dependencies.config.php
index 90fa13684ca98a31e28cf4a303b872da198bbbc0..ea57efca5ab86034f9fd96a6b87c5f32f0adee27 100644 (file)
--- a/static/dependencies.config.php
+++ b/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],

diff --git a/tests/src/Core/StorageManagerTest.php b/tests/src/Core/StorageManagerTest.php
index deb9c4b11fc716b69758732d09525bdce065cde9..537fd841e43a21e922c9d38d019a95e0ba43809c 100644 (file)
--- a/tests/src/Core/StorageManagerTest.php
+++ b/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);
        }
 }