From 08edeae2f996854d0028d0f73a08a1f3ee7741da Mon Sep 17 00:00:00 2001 From: nupplaPhil Date: Sun, 5 Jan 2020 01:58:49 +0100 Subject: [PATCH 01/14] Make Storage testable & add tests - Making StorageManager dynamic (DI::facStorage()) - Making concrete Storage dynamic (DI::storage()) - Add tests for Storage backend and failure handling - Bumping Level-2/Dice to "dev-master" until new release - Using Storage-Names instead of Storage-Classes in config (includes migration) --- composer.json | 2 +- composer.lock | 90 ++++++-- doc/AddonStorageBackend.md | 6 +- src/Console/Storage.php | 25 ++- src/Core/StorageManager.php | 209 +++++++++++------- src/DI.php | 4 + src/Model/Attach.php | 11 +- src/Model/Photo.php | 11 +- src/Model/Storage/Database.php | 89 ++++++-- src/Model/Storage/Filesystem.php | 133 +++++++---- src/Model/Storage/IStorage.php | 44 ++-- src/Module/Admin/Site.php | 71 +++--- src/Worker/CronJobs.php | 4 +- static/dependencies.config.php | 18 +- .../src/Model/Storage/DatabaseStorageTest.php | 52 +++++ .../Model/Storage/FilesystemStorageTest.php | 109 +++++++++ tests/src/Model/Storage/StorageTest.php | 96 ++++++++ update.php | 12 + 18 files changed, 744 insertions(+), 242 deletions(-) create mode 100644 tests/src/Model/Storage/DatabaseStorageTest.php create mode 100644 tests/src/Model/Storage/FilesystemStorageTest.php create mode 100644 tests/src/Model/Storage/StorageTest.php diff --git a/composer.json b/composer.json index e372547aa..d587b721c 100644 --- a/composer.json +++ b/composer.json @@ -32,7 +32,7 @@ "ezyang/htmlpurifier": "^4.7", "friendica/json-ld": "^1.0", "league/html-to-markdown": "^4.8", - "level-2/dice": "^4", + "level-2/dice": "dev-master", "lightopenid/lightopenid": "dev-master", "michelf/php-markdown": "^1.7", "mobiledetect/mobiledetectlib": "^2.8", diff --git a/composer.lock b/composer.lock index 3a4670b20..ad63fb1e8 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "34ad225ce21474eb84ce78047d9f2c01", + "content-hash": "bf05cd52bc7307f45aff80f1d1fd8214", "packages": [ { "name": "asika/simple-console", @@ -485,6 +485,7 @@ "jsonld.php" ] }, + "notification-url": "https://packagist.org/downloads/", "license": [ "BSD-3-Clause" ], @@ -502,11 +503,11 @@ "description": "A JSON-LD Processor and API implementation in PHP.", "homepage": "https://git.friendi.ca/friendica/php-json-ld", "keywords": [ - "JSON", "JSON-LD", "Linked Data", "RDF", "Semantic Web", + "json", "jsonld" ], "time": "2018-10-08T20:41:00+00:00" @@ -823,16 +824,16 @@ }, { "name": "level-2/dice", - "version": "4.0.1", + "version": "dev-master", "source": { "type": "git", "url": "https://github.com/Level-2/Dice.git", - "reference": "e631f110f0520294fec902814c61cac26566023c" + "reference": "2fea2749a625c3adcc29c402218b0dcaed11586f" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/Level-2/Dice/zipball/e631f110f0520294fec902814c61cac26566023c", - "reference": "e631f110f0520294fec902814c61cac26566023c", + "url": "https://api.github.com/repos/Level-2/Dice/zipball/2fea2749a625c3adcc29c402218b0dcaed11586f", + "reference": "2fea2749a625c3adcc29c402218b0dcaed11586f", "shasum": "" }, "require": { @@ -865,7 +866,7 @@ "di", "ioc" ], - "time": "2019-05-01T12:55:36+00:00" + "time": "2019-10-03T16:08:46+00:00" }, { "name": "lightopenid/lightopenid", @@ -1314,6 +1315,22 @@ "require": { "npm-asset/ev-emitter": ">=1.0.0,<2.0.0" }, + "require-dev": { + "npm-asset/chalk": ">=1.1.1,<2.0.0", + "npm-asset/cheerio": ">=0.19.0,<0.20.0", + "npm-asset/gulp": ">=3.9.0,<4.0.0", + "npm-asset/gulp-jshint": ">=1.11.2,<2.0.0", + "npm-asset/gulp-json-lint": ">=0.1.0,<0.2.0", + "npm-asset/gulp-rename": ">=1.2.2,<2.0.0", + "npm-asset/gulp-replace": ">=0.5.4,<0.6.0", + "npm-asset/gulp-requirejs-optimize": "dev-github:metafizzy/gulp-requirejs-optimize", + "npm-asset/gulp-uglify": ">=1.4.2,<2.0.0", + "npm-asset/gulp-util": ">=3.0.7,<4.0.0", + "npm-asset/highlight.js": ">=8.9.1,<9.0.0", + "npm-asset/marked": ">=0.3.5,<0.4.0", + "npm-asset/minimist": ">=1.2.0,<2.0.0", + "npm-asset/transfob": ">=1.0.0,<2.0.0" + }, "type": "npm-asset-library", "extra": { "npm-asset-bugs": { @@ -1358,6 +1375,14 @@ "url": "https://registry.npmjs.org/jgrowl/-/jgrowl-1.4.6.tgz", "shasum": "2736e332aaee73ccf0a14a5f0066391a0a13f4a3" }, + "require-dev": { + "npm-asset/grunt": "~0.4.2", + "npm-asset/grunt-contrib-cssmin": "~0.9.0", + "npm-asset/grunt-contrib-jshint": "~0.6.3", + "npm-asset/grunt-contrib-less": "~0.11.0", + "npm-asset/grunt-contrib-uglify": "~0.4.0", + "npm-asset/grunt-contrib-watch": "~0.6.1" + }, "type": "npm-asset-library", "extra": { "npm-asset-bugs": { @@ -1390,6 +1415,32 @@ "url": "https://registry.npmjs.org/jquery/-/jquery-2.2.4.tgz", "shasum": "2c89d6889b5eac522a7eea32c14521559c6cbf02" }, + "require-dev": { + "npm-asset/commitplease": "2.0.0", + "npm-asset/core-js": "0.9.17", + "npm-asset/grunt": "0.4.5", + "npm-asset/grunt-babel": "5.0.1", + "npm-asset/grunt-cli": "0.1.13", + "npm-asset/grunt-compare-size": "0.4.0", + "npm-asset/grunt-contrib-jshint": "0.11.2", + "npm-asset/grunt-contrib-uglify": "0.9.2", + "npm-asset/grunt-contrib-watch": "0.6.1", + "npm-asset/grunt-git-authors": "2.0.1", + "npm-asset/grunt-jscs": "2.1.0", + "npm-asset/grunt-jsonlint": "1.0.4", + "npm-asset/grunt-npmcopy": "0.1.0", + "npm-asset/gzip-js": "0.3.2", + "npm-asset/jsdom": "5.6.1", + "npm-asset/load-grunt-tasks": "1.0.0", + "npm-asset/qunit-assert-step": "1.0.3", + "npm-asset/qunitjs": "1.17.1", + "npm-asset/requirejs": "2.1.17", + "npm-asset/sinon": "1.10.3", + "npm-asset/sizzle": "2.2.1", + "npm-asset/strip-json-comments": "1.0.3", + "npm-asset/testswarm": "1.1.0", + "npm-asset/win-spawn": "2.0.0" + }, "type": "npm-asset-library", "extra": { "npm-asset-bugs": { @@ -1537,6 +1588,12 @@ "url": "https://registry.npmjs.org/jquery-mousewheel/-/jquery-mousewheel-3.1.13.tgz", "shasum": "06f0335f16e353a695e7206bf50503cb523a6ee5" }, + "require-dev": { + "npm-asset/grunt": "~0.4.1", + "npm-asset/grunt-contrib-connect": "~0.5.0", + "npm-asset/grunt-contrib-jshint": "~0.7.1", + "npm-asset/grunt-contrib-uglify": "~0.2.7" + }, "type": "npm-asset-library", "extra": { "npm-asset-bugs": { @@ -3335,8 +3392,8 @@ "authors": [ { "name": "Sebastian Bergmann", - "role": "lead", - "email": "sb@sebastian-bergmann.de" + "email": "sb@sebastian-bergmann.de", + "role": "lead" } ], "description": "Library that provides collection, processing, and rendering functionality for PHP code coverage information.", @@ -3603,8 +3660,8 @@ "authors": [ { "name": "Sebastian Bergmann", - "role": "lead", - "email": "sebastian@phpunit.de" + "email": "sebastian@phpunit.de", + "role": "lead" } ], "description": "The PHP Unit Testing framework.", @@ -3777,7 +3834,7 @@ } ], "description": "Provides the functionality to compare PHP values for equality", - "homepage": "http://www.github.com/sebastianbergmann/comparator", + "homepage": "https://github.com/sebastianbergmann/comparator", "keywords": [ "comparator", "compare", @@ -3879,7 +3936,7 @@ } ], "description": "Provides functionality to handle HHVM/PHP environments", - "homepage": "http://www.github.com/sebastianbergmann/environment", + "homepage": "https://github.com/sebastianbergmann/environment", "keywords": [ "Xdebug", "environment", @@ -3947,7 +4004,7 @@ } ], "description": "Provides the functionality to export PHP variables for visualization", - "homepage": "http://www.github.com/sebastianbergmann/exporter", + "homepage": "https://github.com/sebastianbergmann/exporter", "keywords": [ "export", "exporter" @@ -3999,7 +4056,7 @@ } ], "description": "Snapshotting of global state", - "homepage": "http://www.github.com/sebastianbergmann/global-state", + "homepage": "https://github.com/sebastianbergmann/global-state", "keywords": [ "global state" ], @@ -4101,7 +4158,7 @@ } ], "description": "Provides functionality to recursively process PHP variables", - "homepage": "http://www.github.com/sebastianbergmann/recursion-context", + "homepage": "https://github.com/sebastianbergmann/recursion-context", "time": "2016-11-19T07:33:16+00:00" }, { @@ -4360,6 +4417,7 @@ "aliases": [], "minimum-stability": "stable", "stability-flags": { + "level-2/dice": 20, "lightopenid/lightopenid": 20 }, "prefer-stable": false, diff --git a/doc/AddonStorageBackend.md b/doc/AddonStorageBackend.md index d42c8bbbd..d105b0340 100644 --- a/doc/AddonStorageBackend.md +++ b/doc/AddonStorageBackend.md @@ -182,21 +182,21 @@ The file is `addon/samplestorage/samplestorage.php` * Author: Alice */ -use Friendica\Core\StorageManager; use Friendica\Addon\samplestorage\SampleStorageBackend; +use Friendica\DI; function samplestorage_install() { // on addon install, we register our class with name "Sample Storage". // note: we use `::class` property, which returns full class name as string // this save us the problem of correctly escape backslashes in class name - StorageManager::register("Sample Storage", SampleStorageBackend::class); + DI::facStorage()->register("Sample Storage", SampleStorageBackend::class); } function samplestorage_unistall() { // when the plugin is uninstalled, we unregister the backend. - StorageManager::unregister("Sample Storage"); + DI::facStorage()->unregister("Sample Storage"); } ``` diff --git a/src/Console/Storage.php b/src/Console/Storage.php index 30b556782..f4b4de562 100644 --- a/src/Console/Storage.php +++ b/src/Console/Storage.php @@ -13,6 +13,19 @@ class Storage extends \Asika\SimpleConsole\Console { protected $helpOptions = ['h', 'help', '?']; + /** @var StorageManager */ + private $storageManager; + + /** + * @param StorageManager $storageManager + */ + public function __construct(StorageManager $storageManager, array $argv = []) + { + parent::__construct($argv); + + $this->storageManager = $storageManager; + } + protected function getHelp() { $help = <<storageManager->getBackend(); $this->out(sprintf($rowfmt, 'Sel', 'Name')); $this->out('-----------------------'); $isregisterd = false; - foreach (StorageManager::listBackends() as $name => $class) { + foreach ($this->storageManager->listBackends() as $name => $class) { $issel = ' '; if ($current === $class) { $issel = '*'; @@ -100,14 +113,14 @@ HELP; } $name = $this->args[1]; - $class = StorageManager::getByName($name); + $class = $this->storageManager->getByName($name); if ($class === '') { $this->out($name . ' is not a registered backend.'); return -1; } - if (!StorageManager::setBackend($class)) { + if (!$this->storageManager->setBackend($class)) { $this->out($class . ' is not a valid backend storage class.'); return -1; } @@ -130,11 +143,11 @@ HELP; $tables = [$table]; } - $current = StorageManager::getBackend(); + $current = $this->storageManager->getBackend(); $total = 0; do { - $moved = StorageManager::move($current, $tables, $this->getOption('n', 5000)); + $moved = $this->storageManager->move($current, $tables, $this->getOption('n', 5000)); if ($moved) { $this->out(date('[Y-m-d H:i:s] ') . sprintf('Moved %d files', $moved)); } diff --git a/src/Core/StorageManager.php b/src/Core/StorageManager.php index 832d9819c..ede8966f4 100644 --- a/src/Core/StorageManager.php +++ b/src/Core/StorageManager.php @@ -2,8 +2,12 @@ namespace Friendica\Core; -use Friendica\Database\DBA; -use Friendica\Model\Storage\IStorage; +use Dice\Dice; +use Exception; +use Friendica\Core\Config\IConfiguration; +use Friendica\Database\Database; +use Friendica\Model\Storage; +use Psr\Log\LoggerInterface; /** @@ -14,59 +18,108 @@ use Friendica\Model\Storage\IStorage; */ class StorageManager { - private static $default_backends = [ - 'Filesystem' => \Friendica\Model\Storage\Filesystem::class, - 'Database' => \Friendica\Model\Storage\Database::class, + // Default tables to look for data + const TABLES = ['photo', 'attach']; + + // Default storage backends + const DEFAULT_BACKENDS = [ + Storage\Filesystem::NAME => Storage\Filesystem::class, + Storage\Database::NAME => Storage\Database::class, ]; - private static $backends = []; + private $backends = []; - private static function setup() + /** @var Database */ + private $dba; + /** @var IConfiguration */ + private $config; + /** @var LoggerInterface */ + private $logger; + /** @var Dice */ + private $dice; + + /** @var Storage\IStorage */ + private $currentBackend; + + /** + * @param Database $dba + * @param IConfiguration $config + * @param LoggerInterface $logger + */ + public function __construct(Database $dba, IConfiguration $config, LoggerInterface $logger, Dice $dice) { - if (count(self::$backends) == 0) { - self::$backends = Config::get('storage', 'backends', self::$default_backends); + $this->dba = $dba; + $this->config = $config; + $this->logger = $logger; + $this->dice = $dice; + $this->backends = $config->get('storage', 'backends', self::DEFAULT_BACKENDS); + + $currentName = $this->config->get('storage', 'name', ''); + + if ($this->isValidBackend($currentName)) { + $this->currentBackend = $this->dice->create($this->backends[$currentName]); + } else { + $this->currentBackend = null; } } /** * @brief Return current storage backend class * - * @return string - * @throws \Friendica\Network\HTTPException\InternalServerErrorException + * @return Storage\IStorage|null */ - public static function getBackend() + public function getBackend() { - return Config::get('storage', 'class', ''); + return $this->currentBackend; } /** * @brief Return storage backend class by registered name * - * @param string $name Backend name - * @return string Empty if no backend registered at $name exists + * @param string $name Backend name + * + * @return Storage\IStorage|null null if no backend registered at $name */ - public static function getByName($name) + public function getByName(string $name) { - self::setup(); - return self::$backends[$name] ?? ''; + if (!$this->isValidBackend($name)) { + return null; + } + + return $this->dice->create($this->backends[$name]); + } + + /** + * Checks, if the storage is a valid backend + * + * @param string $name The name or class of the backend + * + * @return boolean True, if the backend is a valid backend + */ + public function isValidBackend(string $name) + { + return array_key_exists($name, $this->backends); } /** * @brief Set current storage backend class * - * @param string $class Backend class name - * @return bool - * @throws \Friendica\Network\HTTPException\InternalServerErrorException + * @param string $name Backend class name + * + * @return boolean True, if the set was successful */ - public static function setBackend($class) + public function setBackend(string $name) { - if (!in_array('Friendica\Model\Storage\IStorage', class_implements($class))) { + if (!$this->isValidBackend($name)) { return false; } - Config::set('storage', 'class', $class); - - return true; + if ($this->config->set('storage', 'name', $name)) { + $this->currentBackend = $this->dice->create($this->backends[$name]); + return true; + } else { + return false; + } } /** @@ -74,107 +127,109 @@ class StorageManager * * @return array */ - public static function listBackends() + public function listBackends() { - self::setup(); - return self::$backends; + return $this->backends; } - /** * @brief Register a storage backend class * * @param string $name User readable backend name * @param string $class Backend class name - * @throws \Friendica\Network\HTTPException\InternalServerErrorException + * + * @return boolean True, if the registration was successful */ - public static function register($name, $class) + public function register(string $name, string $class) { - /// @todo Check that $class implements IStorage - self::setup(); - self::$backends[$name] = $class; - Config::set('storage', 'backends', self::$backends); - } + if (!is_subclass_of($class, Storage\IStorage::class)) { + return false; + } + $backends = $this->backends; + $backends[$name] = $class; + + if ($this->config->set('storage', 'backends', $this->backends)) { + $this->backends = $backends; + return true; + } else { + return false; + } + } /** * @brief Unregister a storage backend class * * @param string $name User readable backend name - * @throws \Friendica\Network\HTTPException\InternalServerErrorException + * + * @return boolean True, if unregistering was successful */ - public static function unregister($name) + public function unregister(string $name) { - self::setup(); - unset(self::$backends[$name]); - Config::set('storage', 'backends', self::$backends); + unset($this->backends[$name]); + return $this->config->set('storage', 'backends', $this->backends); } - /** * @brief Move up to 5000 resources to storage $dest * * Copy existing data to destination storage and delete from source. * This method cannot move to legacy in-table `data` field. * - * @param string $destination Storage class name - * @param array|null $tables Tables to look in for resources. Optional, defaults to ['photo', 'attach'] - * @param int $limit Limit of the process batch size, defaults to 5000 + * @param Storage\IStorage $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 + * * @return int Number of moved resources - * @throws \Exception + * @throws Storage\StorageException + * @throws Exception */ - public static function move($destination, $tables = null, $limit = 5000) + public function move(Storage\IStorage $destination, array $tables = self::TABLES, int $limit = 5000) { - if (empty($destination)) { - throw new \Exception('Can\'t move to NULL storage backend'); - } - - if (is_null($tables)) { - $tables = ['photo', 'attach']; + if ($destination === null) { + throw new Storage\StorageException('Can\'t move to NULL storage backend'); } $moved = 0; foreach ($tables as $table) { // Get the rows where backend class is not the destination backend class - $resources = DBA::select( - $table, + $resources = $this->dba->select( + $table, ['id', 'data', 'backend-class', 'backend-ref'], ['`backend-class` IS NULL or `backend-class` != ?', $destination], ['limit' => $limit] ); - while ($resource = DBA::fetch($resources)) { - $id = $resource['id']; - $data = $resource['data']; - /** @var IStorage $backendClass */ - $backendClass = $resource['backend-class']; - $backendRef = $resource['backend-ref']; - if (!empty($backendClass)) { - Logger::log("get data from old backend " . $backendClass . " : " . $backendRef); - $data = $backendClass::get($backendRef); + while ($resource = $this->dba->fetch($resources)) { + $id = $resource['id']; + $data = $resource['data']; + $source = $this->getByName($resource['backend-class']); + $sourceRef = $resource['backend-ref']; + + if (!empty($source)) { + $this->logger->info('Get data from old backend.', ['oldBackend' => $source, 'oldReference' => $sourceRef]); + $data = $source->get($sourceRef); } - Logger::log("save data to new backend " . $destination); - /** @var IStorage $destination */ - $ref = $destination::put($data); - Logger::log("saved data as " . $ref); + $this->logger->info('Save data to new backend.', ['newBackend' => $destination]); + $destinationRef = $destination->put($data); + $this->logger->info('Saved data.', ['newReference' => $destinationRef]); - if ($ref !== '') { - Logger::log("update row"); - if (DBA::update($table, ['backend-class' => $destination, 'backend-ref' => $ref, 'data' => ''], ['id' => $id])) { - if (!empty($backendClass)) { - Logger::log("delete data from old backend " . $backendClass . " : " . $backendRef); - $backendClass::delete($backendRef); + if ($destinationRef !== '') { + $this->logger->info('update row'); + if ($this->dba->update($table, ['backend-class' => $destination, 'backend-ref' => $destinationRef, 'data' => ''], ['id' => $id])) { + if (!empty($source)) { + $this->logger->info('Delete data from old backend.', ['oldBackend' => $source, 'oldReference' => $sourceRef]); + $source->delete($sourceRef); } $moved++; } } } - DBA::close($resources); + $this->dba->close($resources); } return $moved; } } - diff --git a/src/DI.php b/src/DI.php index 7ed581254..152e705a7 100644 --- a/src/DI.php +++ b/src/DI.php @@ -27,6 +27,7 @@ use Psr\Log\LoggerInterface; * @method static Core\L10n\L10n l10n() * @method static Core\Process process() * @method static Core\Session\ISession session() + * @method static Core\StorageManager facStorage() * @method static Database\Database dba() * @method static Factory\Mastodon\Account mstdnAccount() * @method static Factory\Mastodon\FollowRequest mstdnFollowRequest() @@ -34,6 +35,7 @@ use Psr\Log\LoggerInterface; * @method static Model\User\Cookie cookie() * @method static Model\Notify notify() * @method static Repository\Introduction intro() + * @method static Model\Storage\IStorage storage() * @method static Protocol\Activity activity() * @method static Util\ACLFormatter aclFormatter() * @method static Util\DateTimeFormat dtFormat() @@ -64,12 +66,14 @@ abstract class DI 'lock' => Core\Lock\ILock::class, 'process' => Core\Process::class, 'session' => Core\Session\ISession::class, + 'facStorage' => Core\StorageManager::class, 'dba' => Database\Database::class, 'mstdnAccount' => Factory\Mastodon\Account::class, 'mstdnFollowRequest' => Factory\Mastodon\FollowRequest::class, 'mstdnRelationship' => Factory\Mastodon\Relationship::class, 'cookie' => Model\User\Cookie::class, 'notify' => Model\Notify::class, + 'storage' => Model\Storage\IStorage::class, 'intro' => Repository\Introduction::class, 'activity' => Protocol\Activity::class, 'aclFormatter' => Util\ACLFormatter::class, diff --git a/src/Model/Attach.php b/src/Model/Attach.php index c1d5c033b..102e7b1a1 100644 --- a/src/Model/Attach.php +++ b/src/Model/Attach.php @@ -186,13 +186,8 @@ class Attach $filesize = strlen($data); } - /** @var IStorage $backend_class */ - $backend_class = StorageManager::getBackend(); - $backend_ref = ''; - if ($backend_class !== '') { - $backend_ref = $backend_class::put($data); - $data = ''; - } + $backend_ref = DI::storage()->put($data); + $data = ''; $hash = System::createGUID(64); $created = DateTimeFormat::utcNow(); @@ -210,7 +205,7 @@ class Attach 'allow_gid' => $allow_gid, 'deny_cid' => $deny_cid, 'deny_gid' => $deny_gid, - 'backend-class' => $backend_class, + 'backend-class' => (string)DI::storage(), 'backend-ref' => $backend_ref ]; diff --git a/src/Model/Photo.php b/src/Model/Photo.php index c4dbf2b30..7aa6ffdb3 100644 --- a/src/Model/Photo.php +++ b/src/Model/Photo.php @@ -273,18 +273,17 @@ class Photo $data = ""; $backend_ref = ""; - /** @var IStorage $backend_class */ if (DBA::isResult($existing_photo)) { $backend_ref = (string)$existing_photo["backend-ref"]; - $backend_class = (string)$existing_photo["backend-class"]; + $storage = DI::facStorage()->getByName((string)$existing_photo["backend-class"]); } else { - $backend_class = StorageManager::getBackend(); + $storage = DI::storage(); } - if ($backend_class === "") { + if ($storage === null) { $data = $Image->asString(); } else { - $backend_ref = $backend_class::put($Image->asString(), $backend_ref); + $backend_ref = $storage->put($Image->asString(), $backend_ref); } @@ -309,7 +308,7 @@ class Photo "deny_cid" => $deny_cid, "deny_gid" => $deny_gid, "desc" => $desc, - "backend-class" => $backend_class, + "backend-class" => (string)$storage, "backend-ref" => $backend_ref ]; diff --git a/src/Model/Storage/Database.php b/src/Model/Storage/Database.php index 60bd154e6..84dd116b5 100644 --- a/src/Model/Storage/Database.php +++ b/src/Model/Storage/Database.php @@ -6,9 +6,8 @@ namespace Friendica\Model\Storage; -use Friendica\Core\Logger; use Friendica\Core\L10n; -use Friendica\Database\DBA; +use Psr\Log\LoggerInterface; /** * @brief Database based storage system @@ -17,47 +16,93 @@ use Friendica\Database\DBA; */ class Database implements IStorage { - public static function get($ref) + const NAME = 'Database'; + + /** @var \Friendica\Database\Database */ + private $dba; + /** @var LoggerInterface */ + private $logger; + /** @var L10n\L10n */ + private $l10n; + + /** + * @param \Friendica\Database\Database $dba + * @param LoggerInterface $logger + * @param L10n\L10n $l10n + */ + public function __construct(\Friendica\Database\Database $dba, LoggerInterface $logger, L10n\L10n $l10n) { - $r = DBA::selectFirst('storage', ['data'], ['id' => $ref]); - if (!DBA::isResult($r)) { + $this->dba = $dba; + $this->logger = $logger; + $this->l10n = $l10n; + } + + /** + * @inheritDoc + */ + public function get(string $reference) + { + $result = $this->dba->selectFirst('storage', ['data'], ['id' => $reference]); + if (!$this->dba->isResult($result)) { return ''; } - return $r['data']; + return $result['data']; } - public static function put($data, $ref = '') + /** + * @inheritDoc + */ + public function put(string $data, string $reference = '') { - if ($ref !== '') { - $r = DBA::update('storage', ['data' => $data], ['id' => $ref]); - if ($r === false) { - Logger::log('Failed to update data with id ' . $ref . ': ' . DBA::errorNo() . ' : ' . DBA::errorMessage()); - throw new StorageException(L10n::t('Database storage failed to update %s', $ref)); + if ($reference !== '') { + $result = $this->dba->update('storage', ['data' => $data], ['id' => $reference]); + if ($result === false) { + $this->logger->warning('Failed to update data.', ['id' => $reference, 'errorCode' => $this->dba->errorNo(), 'errorMessage' => $this->dba->errorMessage()]); + throw new StorageException($this->l10n->t('Database storage failed to update %s', $reference)); } - return $ref; + + return $reference; } else { - $r = DBA::insert('storage', ['data' => $data]); - if ($r === false) { - Logger::log('Failed to insert data: ' . DBA::errorNo() . ' : ' . DBA::errorMessage()); - throw new StorageException(L10n::t('Database storage failed to insert data')); + $result = $this->dba->insert('storage', ['data' => $data]); + if ($result === false) { + $this->logger->warning('Failed to insert data.', ['errorCode' => $this->dba->errorNo(), 'errorMessage' => $this->dba->errorMessage()]); + throw new StorageException($this->l10n->t('Database storage failed to insert data')); } - return DBA::lastInsertId(); + + return $this->dba->lastInsertId(); } } - public static function delete($ref) + /** + * @inheritDoc + */ + public function delete(string $reference) { - return DBA::delete('storage', ['id' => $ref]); + return $this->dba->delete('storage', ['id' => $reference]); } - public static function getOptions() + /** + * @inheritDoc + */ + public function getOptions() { return []; } - public static function saveOptions($data) + /** + * @inheritDoc + */ + public function saveOptions(array $data) { return []; } + + /** + * @inheritDoc + */ + public function __toString() + { + return self::NAME; + } } diff --git a/src/Model/Storage/Filesystem.php b/src/Model/Storage/Filesystem.php index ff7c59444..e954352bc 100644 --- a/src/Model/Storage/Filesystem.php +++ b/src/Model/Storage/Filesystem.php @@ -6,10 +6,10 @@ namespace Friendica\Model\Storage; -use Friendica\Core\Config; -use Friendica\Core\L10n; -use Friendica\Core\Logger; +use Friendica\Core\Config\IConfiguration; +use Friendica\Core\L10n\L10n; use Friendica\Util\Strings; +use Psr\Log\LoggerInterface; /** * @brief Filesystem based storage backend @@ -23,50 +23,74 @@ use Friendica\Util\Strings; */ class Filesystem implements IStorage { + const NAME = 'Filesystem'; + // Default base folder const DEFAULT_BASE_FOLDER = 'storage'; - private static function getBasePath() + /** @var IConfiguration */ + private $config; + /** @var LoggerInterface */ + private $logger; + /** @var L10n */ + private $l10n; + + /** @var string */ + private $basePath; + + /** + * Filesystem constructor. + * + * @param IConfiguration $config + * @param LoggerInterface $logger + * @param L10n $l10n + */ + public function __construct(IConfiguration $config, LoggerInterface $logger, L10n $l10n) { - $path = Config::get('storage', 'filesystem_path', self::DEFAULT_BASE_FOLDER); - return rtrim($path, '/'); + $this->config = $config; + $this->logger = $logger; + $this->l10n = $l10n; + + $path = $this->config->get('storage', 'filesystem_path', self::DEFAULT_BASE_FOLDER); + $this->basePath = rtrim($path, '/'); } /** * @brief Split data ref and return file path - * @param string $ref Data reference + * + * @param string $reference Data reference + * * @return string */ - private static function pathForRef($ref) + private function pathForRef(string $reference) { - $base = self::getBasePath(); - $fold1 = substr($ref, 0, 2); - $fold2 = substr($ref, 2, 2); - $file = substr($ref, 4); + $fold1 = substr($reference, 0, 2); + $fold2 = substr($reference, 2, 2); + $file = substr($reference, 4); - return implode('/', [$base, $fold1, $fold2, $file]); + return implode('/', [$this->basePath, $fold1, $fold2, $file]); } /** * @brief Create dirctory tree to store file, with .htaccess and index.html files + * * @param string $file Path and filename + * * @throws StorageException */ - private static function createFoldersForFile($file) + private function createFoldersForFile(string $file) { $path = dirname($file); if (!is_dir($path)) { if (!mkdir($path, 0770, true)) { - Logger::log('Failed to create dirs ' . $path); - throw new StorageException(L10n::t('Filesystem storage failed to create "%s". Check you write permissions.', $path)); + $this->logger->warning('Failed to create dir.', ['path' => $path]); + throw new StorageException($this->l10n->t('Filesystem storage failed to create "%s". Check you write permissions.', $path)); } } - $base = self::getBasePath(); - - while ($path !== $base) { + while ($path !== $this->basePath) { if (!is_file($path . '/index.html')) { file_put_contents($path . '/index.html', ''); } @@ -80,9 +104,12 @@ class Filesystem implements IStorage } } - public static function get($ref) + /** + * @inheritDoc + */ + public function get(string $reference) { - $file = self::pathForRef($ref); + $file = $this->pathForRef($reference); if (!is_file($file)) { return ''; } @@ -90,27 +117,33 @@ class Filesystem implements IStorage return file_get_contents($file); } - public static function put($data, $ref = '') + /** + * @inheritDoc + */ + public function put(string $data, string $reference = '') { - if ($ref === '') { - $ref = Strings::getRandomHex(); + if ($reference === '') { + $reference = Strings::getRandomHex(); } - $file = self::pathForRef($ref); + $file = $this->pathForRef($reference); - self::createFoldersForFile($file); + $this->createFoldersForFile($file); - $r = file_put_contents($file, $data); - if ($r === FALSE) { - Logger::log('Failed to write data to ' . $file); - throw new StorageException(L10n::t('Filesystem storage failed to save data to "%s". Check your write permissions', $file)); + if ((file_exists($file) && !is_writable($file)) || !file_put_contents($file, $data)) { + $this->logger->warning('Failed to write data.', ['file' => $file]); + throw new StorageException($this->l10n->t('Filesystem storage failed to save data to "%s". Check your write permissions', $file)); } + chmod($file, 0660); - return $ref; + return $reference; } - public static function delete($ref) + /** + * @inheritDoc + */ + public function delete(string $reference) { - $file = self::pathForRef($ref); + $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; @@ -118,28 +151,42 @@ class Filesystem implements IStorage return unlink($file); } - public static function getOptions() + /** + * @inheritDoc + */ + public function getOptions() { return [ 'storagepath' => [ 'input', - L10n::t('Storage base path'), - self::getBasePath(), - L10n::t('Folder where uploaded files are saved. For maximum security, This should be a path outside web server folder tree') + $this->l10n->t('Storage base path'), + $this->basePath, + $this->l10n->t('Folder where uploaded files are saved. For maximum security, This should be a path outside web server folder tree') ] ]; } - - public static function saveOptions($data) + + /** + * @inheritDoc + */ + public function saveOptions(array $data) { - $storagepath = $data['storagepath'] ?? ''; - if ($storagepath === '' || !is_dir($storagepath)) { + $storagePath = $data['storagepath'] ?? ''; + if ($storagePath === '' || !is_dir($storagePath)) { return [ - 'storagepath' => L10n::t('Enter a valid existing folder') + 'storagepath' => $this->l10n->t('Enter a valid existing folder') ]; }; - Config::set('storage', 'filesystem_path', $storagepath); + $this->config->set('storage', 'filesystem_path', $storagePath); + $this->basePath = $storagePath; return []; } + /** + * @inheritDoc + */ + public function __toString() + { + return self::NAME; + } } diff --git a/src/Model/Storage/IStorage.php b/src/Model/Storage/IStorage.php index 1b0129e5e..a7b7f6f56 100644 --- a/src/Model/Storage/IStorage.php +++ b/src/Model/Storage/IStorage.php @@ -13,26 +13,32 @@ interface IStorage { /** * @brief Get data from backend - * @param string $ref Data reference + * + * @param string $reference Data reference + * * @return string - */ - public static function get($ref); + */ + public function get(string $reference); /** * @brief Put data in backend as $ref. If $ref is not defined a new reference is created. - * @param string $data Data to save - * @param string $ref Data referece. Optional. - * @return string Saved data referece + * + * @param string $data Data to save + * @param string $reference Data reference. Optional. + * + * @return string Saved data reference */ - public static function put($data, $ref = ""); + public function put(string $data, string $reference = ""); /** * @brief Remove data from backend - * @param string $ref Data referece + * + * @param string $reference Data reference + * * @return boolean True on success */ - public static function delete($ref); - + public function delete(string $reference); + /** * @brief Get info about storage options * @@ -71,19 +77,23 @@ interface IStorage * * See https://github.com/friendica/friendica/wiki/Quick-Template-Guide */ - public static function getOptions(); - + public function getOptions(); + /** * @brief Validate and save options * - * @param array $data Array [optionname => value] to be saved + * @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 static function saveOptions($data); - + public function saveOptions(array $data); + + /** + * The name of the backend + * + * @return string + */ + public function __toString(); } - - diff --git a/src/Module/Admin/Site.php b/src/Module/Admin/Site.php index 3302e0dc1..e68f37953 100644 --- a/src/Module/Admin/Site.php +++ b/src/Module/Admin/Site.php @@ -199,42 +199,37 @@ class Site extends BaseAdminModule $relay_user_tags = !empty($_POST['relay_user_tags']); $active_panel = (!empty($_POST['active_panel']) ? "#" . Strings::escapeTags(trim($_POST['active_panel'])) : ''); - /** - * @var $storagebackend \Friendica\Model\Storage\IStorage - */ $storagebackend = Strings::escapeTags(trim($_POST['storagebackend'] ?? '')); // save storage backend form - if (!is_null($storagebackend) && $storagebackend != "") { - if (StorageManager::setBackend($storagebackend)) { - $storage_opts = $storagebackend::getOptions(); - $storage_form_prefix = preg_replace('|[^a-zA-Z0-9]|', '', $storagebackend); - $storage_opts_data = []; - foreach ($storage_opts as $name => $info) { - $fieldname = $storage_form_prefix . '_' . $name; - switch ($info[0]) { // type - case 'checkbox': - case 'yesno': - $value = !empty($_POST[$fieldname]); - break; - default: - $value = $_POST[$fieldname] ?? ''; - } - $storage_opts_data[$name] = $value; + if (DI::facStorage()->setBackend($storagebackend)) { + $storage_opts = DI::storage()->getOptions(); + $storage_form_prefix = preg_replace('|[^a-zA-Z0-9]|', '', $storagebackend); + $storage_opts_data = []; + foreach ($storage_opts as $name => $info) { + $fieldname = $storage_form_prefix . '_' . $name; + switch ($info[0]) { // type + case 'checkbox': + case 'yesno': + $value = !empty($_POST[$fieldname]); + break; + default: + $value = $_POST[$fieldname] ?? ''; } - unset($name); - unset($info); - - $storage_form_errors = $storagebackend::saveOptions($storage_opts_data); - if (count($storage_form_errors)) { - foreach ($storage_form_errors as $name => $err) { - notice('Storage backend, ' . $storage_opts[$name][1] . ': ' . $err); - } - DI::baseUrl()->redirect('admin/site' . $active_panel); - } - } else { - info(L10n::t('Invalid storage backend setting value.')); + $storage_opts_data[$name] = $value; } + unset($name); + unset($info); + + $storage_form_errors = DI::storage()->saveOptions($storage_opts_data); + if (count($storage_form_errors)) { + foreach ($storage_form_errors as $name => $err) { + notice('Storage backend, ' . $storage_opts[$name][1] . ': ' . $err); + } + DI::baseUrl()->redirect('admin/site' . $active_panel); + } + } else { + info(L10n::t('Invalid storage backend setting value.')); } // Has the directory url changed? If yes, then resubmit the existing profiles there @@ -530,29 +525,25 @@ class Site extends BaseAdminModule $optimize_max_tablesize = -1; } - $storage_backends = StorageManager::listBackends(); - /** @var $current_storage_backend \Friendica\Model\Storage\IStorage */ - $current_storage_backend = StorageManager::getBackend(); - + $current_storage_backend = DI::storage(); $available_storage_backends = []; // show legacy option only if it is the current backend: // once changed can't be selected anymore - if ($current_storage_backend == '') { + if ($current_storage_backend == null) { $available_storage_backends[''] = L10n::t('Database (legacy)'); } - foreach ($storage_backends as $name => $class) { - $available_storage_backends[$class] = $name; + foreach (DI::facStorage()->listBackends() as $name => $class) { + $available_storage_backends[$name] = $name; } - unset($storage_backends); // build storage config form, $storage_form_prefix = preg_replace('|[^a-zA-Z0-9]|' ,'', $current_storage_backend); $storage_form = []; if (!is_null($current_storage_backend) && $current_storage_backend != '') { - foreach ($current_storage_backend::getOptions() as $name => $info) { + foreach ($current_storage_backend->getOptions() as $name => $info) { $type = $info[0]; $info[0] = $storage_form_prefix . '_' . $name; $info['type'] = $type; diff --git a/src/Worker/CronJobs.php b/src/Worker/CronJobs.php index 6267bf6f3..6fa09197b 100644 --- a/src/Worker/CronJobs.php +++ b/src/Worker/CronJobs.php @@ -324,8 +324,8 @@ class CronJobs */ private static function moveStorage() { - $current = StorageManager::getBackend(); - $moved = StorageManager::move($current); + $current = DI::storage(); + $moved = DI::facStorage()->move($current); if ($moved) { Worker::add(PRIORITY_LOW, "CronJobs", "move_storage"); diff --git a/static/dependencies.config.php b/static/dependencies.config.php index 6cd077b03..0bf5a6905 100644 --- a/static/dependencies.config.php +++ b/static/dependencies.config.php @@ -8,8 +8,10 @@ use Friendica\Core\L10n\L10n; use Friendica\Core\Lock\ILock; use Friendica\Core\Process; use Friendica\Core\Session\ISession; +use Friendica\Core\StorageManager; use Friendica\Database\Database; use Friendica\Factory; +use Friendica\Model\Storage\IStorage; use Friendica\Model\User\Cookie; use Friendica\Util; use Psr\Log\LoggerInterface; @@ -193,5 +195,19 @@ return [ 'constructParams' => [ $_SERVER, $_COOKIE ], - ] + ], + StorageManager::class => [ + 'constructParams' => [ + [Dice::INSTANCE => Dice::SELF], + ] + ], + IStorage::class => [ + // Don't share this class with other creations, because it's possible to switch the backend + // and so we wouldn't be possible to update it + 'shared' => false, + 'instanceOf' => StorageManager::class, + 'call' => [ + ['getBackend', [], Dice::CHAIN_CALL], + ], + ], ]; diff --git a/tests/src/Model/Storage/DatabaseStorageTest.php b/tests/src/Model/Storage/DatabaseStorageTest.php new file mode 100644 index 000000000..d6dff9932 --- /dev/null +++ b/tests/src/Model/Storage/DatabaseStorageTest.php @@ -0,0 +1,52 @@ +setUpVfsDir(); + + parent::setUp(); + } + + protected function getInstance() + { + $logger = new NullLogger(); + $profiler = \Mockery::mock(Profiler::class); + $profiler->shouldReceive('saveTimestamp')->withAnyArgs()->andReturn(true); + + // load real config to avoid mocking every config-entry which is related to the Database class + $configFactory = new ConfigFactory(); + $loader = new ConfigFileLoader($this->root->url()); + $configCache = $configFactory->createCache($loader); + + $dba = new StaticDatabase($configCache, $profiler, $logger); + + /** @var MockInterface|L10n $l10n */ + $l10n = \Mockery::mock(L10n::class)->makePartial(); + + return new Database($dba, $logger, $l10n); + } + + protected function assertOption(IStorage $storage) + { + $this->assertEmpty($storage->getOptions()); + } +} diff --git a/tests/src/Model/Storage/FilesystemStorageTest.php b/tests/src/Model/Storage/FilesystemStorageTest.php new file mode 100644 index 000000000..2dd467254 --- /dev/null +++ b/tests/src/Model/Storage/FilesystemStorageTest.php @@ -0,0 +1,109 @@ +setUpVfsDir(); + + vfsStream::create(['storage' => []], $this->root); + + parent::setUp(); + } + + protected function getInstance() + { + $logger = new NullLogger(); + $profiler = \Mockery::mock(Profiler::class); + $profiler->shouldReceive('saveTimestamp')->withAnyArgs()->andReturn(true); + + /** @var MockInterface|L10n $l10n */ + $l10n = \Mockery::mock(L10n::class)->makePartial(); + $this->config = \Mockery::mock(IConfiguration::class); + $this->config->shouldReceive('get') + ->with('storage', 'filesystem_path', Filesystem::DEFAULT_BASE_FOLDER) + ->andReturn($this->root->getChild('storage')->url()); + + return new Filesystem($this->config, $logger, $l10n); + } + + protected function assertOption(IStorage $storage) + { + $this->assertEquals([ + 'storagepath' => [ + 'input', 'Storage base path', + $this->root->getChild('storage')->url(), + 'Folder where uploaded files are saved. For maximum security, This should be a path outside web server folder tree' + ] + ], $storage->getOptions()); + } + + /** + * Test the exception in case of missing directorsy permissions + * + * @expectedException \Friendica\Model\Storage\StorageException + * @expectedExceptionMessageRegExp /Filesystem storage failed to create \".*\". Check you write permissions./ + */ + public function testMissingDirPermissions() + { + $this->root->getChild('storage')->chmod(000); + + $instance = $this->getInstance(); + $instance->put('test'); + } + + /** + * Test the exception in case of missing file permissions + * + * @expectedException \Friendica\Model\Storage\StorageException + * @expectedExceptionMessageRegExp /Filesystem storage failed to save data to \".*\". Check your write permissions/ + */ + public function testMissingFilePermissions() + { + vfsStream::create(['storage' => ['f0' => ['c0' => ['k0i0' => '']]]], $this->root); + + $this->root->getChild('storage/f0/c0/k0i0')->chmod(000); + + $instance = $this->getInstance(); + $instance->put('test', 'f0c0k0i0'); + } + + /** + * Test the backend storage of the Filesystem Storage class + */ + public function testDirectoryTree() + { + $instance = $this->getInstance(); + + $instance->put('test', 'f0c0d0i0'); + + $dir = $this->root->getChild('storage/f0/c0')->url(); + $file = $this->root->getChild('storage/f0/c0/d0i0')->url(); + + $this->assertDirectoryExists($dir); + $this->assertFileExists($file); + + $this->assertDirectoryIsWritable($dir); + $this->assertFileIsWritable($file); + + $this->assertEquals('test', file_get_contents($file)); + } +} diff --git a/tests/src/Model/Storage/StorageTest.php b/tests/src/Model/Storage/StorageTest.php new file mode 100644 index 000000000..ae3f8f01f --- /dev/null +++ b/tests/src/Model/Storage/StorageTest.php @@ -0,0 +1,96 @@ +getInstance(); + $this->assertInstanceOf(IStorage::class, $instance); + } + + /** + * Test if the "getOption" is asserted + */ + public function testGetOptions() + { + $instance = $this->getInstance(); + + $this->assertOption($instance); + } + + /** + * Test basic put, get and delete operations + */ + public function testPutGetDelete() + { + $instance = $this->getInstance(); + + $ref = $instance->put('data12345'); + $this->assertNotEmpty($ref); + + $this->assertEquals('data12345', $instance->get($ref)); + + $this->assertTrue($instance->delete($ref)); + } + + /** + * Test a delete with an invalid reference + */ + public function testInvalidDelete() + { + $instance = $this->getInstance(); + + // Even deleting not existing references should return "true" + $this->assertTrue($instance->delete(-1234456)); + } + + /** + * Test a get with an invalid reference + */ + public function testInvalidGet() + { + $instance = $this->getInstance(); + + // Invalid references return an empty string + $this->assertEmpty($instance->get(-123456)); + } + + /** + * Test an update with a given reference + */ + public function testUpdateReference() + { + $instance = $this->getInstance(); + + $ref = $instance->put('data12345'); + $this->assertNotEmpty($ref); + + $this->assertEquals('data12345', $instance->get($ref)); + + $this->assertEquals($ref, $instance->put('data5432', $ref)); + $this->assertEquals('data5432', $instance->get($ref)); + } + + /** + * Test that an invalid update results in an insert + */ + public function testInvalidUpdate() + { + $instance = $this->getInstance(); + + $this->assertEquals(-123, $instance->put('data12345', -123)); + } +} diff --git a/update.php b/update.php index 40f39ebeb..90018b62d 100644 --- a/update.php +++ b/update.php @@ -408,3 +408,15 @@ function update_1327() return Update::SUCCESS; } +function update_1329() +{ + $currStorage = Config::get('storage', 'class', ''); + + if (!empty($currStorage)) { + $storageName = array_key_first(\Friendica\Core\StorageManager::DEFAULT_BACKENDS, $currStorage); + Config::set('storage', 'name', $storageName); + Config::delete('storage', 'class'); + } + + return Update::SUCCESS; +} From cc501439aff799bd7fb400eb8019d58c46786926 Mon Sep 17 00:00:00 2001 From: nupplaPhil Date: Sun, 5 Jan 2020 22:33:27 +0100 Subject: [PATCH 02/14] Update AddonStorageBackend documentation --- doc/AddonStorageBackend.md | 139 ++++++++++++++++++++++++++++--------- 1 file changed, 108 insertions(+), 31 deletions(-) diff --git a/doc/AddonStorageBackend.md b/doc/AddonStorageBackend.md index d105b0340..896ea49bf 100644 --- a/doc/AddonStorageBackend.md +++ b/doc/AddonStorageBackend.md @@ -17,22 +17,23 @@ namespace Friendica\Model\Storage; ```php interface IStorage { - public static function get($ref); - public static function put($data, $ref = ""); - public static function delete($ref); - public static function getOptions(); - public static function saveOptions($data); + public function get(string $reference); + public function put(string $data, string $reference = ''); + public function delete(string $reference); + public function getOptions(); + public function saveOptions(array $data); + public function __toString(); } ``` -- `get($ref)` returns data pointed by `$ref` -- `put($data, $ref)` saves data in `$data` to position `$ref`, or a new position if `$ref` is empty. -- `delete($ref)` delete data pointed by `$ref` +- `get(string $reference)` returns data pointed by `$reference` +- `put(string $data, string $reference)` saves data in `$data` to position `$reference`, or a new position if `$reference` is empty. +- `delete(string $reference)` delete data pointed by `$reference` Each storage backend can have options the admin can set in admin page. - `getOptions()` returns an array with details about each option to build the interface. -- `saveOptions($data)` get `$data` from admin page, validate it and save it. +- `saveOptions(array $data)` get `$data` from admin page, validate it and save it. The array returned by `getOptions()` is defined as: @@ -84,11 +85,33 @@ See doxygen documentation of `IStorage` interface for details about each method. Each backend must be registered in the system when the plugin is installed, to be aviable. -`Friendica\Core\StorageManager::register($name, $class)` is used to register the backend class. +`DI::facStorage()->register(string $name, string $class)` is used to register the backend class. The `$name` must be univocal and will be shown to admin. When the plugin is uninstalled, registered backends must be unregistered using -`Friendica\Core\StorageManager::unregister($class)`. +`DI::facStorage()->unregister(string $name)`. + +## Adding tests + +**Currently testing is limited to core Friendica only, this shows theoretically how tests should work in the future** + +Each new Storage class should be added to the test-environment at [Storage Tests](https://github.com/friendica/friendica/tree/develop/tests/src/Model/Storage/). + +Add a new test class which's naming convention is `StorageClassTest`, which extend the `StorageTest` in the same directory. + +Override the two necessary instances: +```php +use Friendica\Model\Storage\IStorage; + +abstract class StorageTest +{ + // returns an instance of your newly created storage class + abstract protected function getInstance(); + + // Assertion for the option array you return for your new StorageClass + abstract protected function assertOption(IStorage $storage); +} +``` ## Example @@ -112,60 +135,86 @@ use Friendica\Core\L10n; class SampleStorageBackend implements IStorage { - public static function get($ref) + const NAME = 'Sample Storage'; + + /** @var Config\IConfiguration */ + private $config; + /** @var L10n\L10n */ + private $l10n; + + /** + * SampleStorageBackend constructor. + * @param Config\IConfiguration $config The configuration of Friendica + * + * You can add here every dynamic class as dependency you like and add them to a private field + * Friendica automatically creates these classes and passes them as argument to the constructor + */ + public function __construct(Config\IConfiguration $config, L10n\L10n $l10n) { - // we return alwais the same image data. Which file we load is defined by + $this->config = $config; + $this->l10n = $l10n; + } + + public function get(string $reference) + { + // we return always the same image data. Which file we load is defined by // a config key - $filename = Config::get("storage", "samplestorage", "sample.jpg"); + $filename = $this->config->get('storage', 'samplestorage', 'sample.jpg'); return file_get_contents($filename); } - public static function put($data, $ref = "") + public function put(string $data, string $reference = '') { - if ($ref === "") { - $ref = "sample"; + if ($reference === '') { + $reference = 'sample'; } // we don't save $data ! - return $ref; + return $reference; } - public static function delete($ref) + public function delete(string $reference) { // we pretend to delete the data return true; } - public static function getOptions() + public function getOptions() { - $filename = Config::get("storage", "samplestorage", "sample.jpg"); + $filename = $this->config->get('storage', 'samplestorage', 'sample.jpg'); return [ - "filename" => [ - "input", // will use a simple text input - L10n::t("The file to return"), // the label + 'filename' => [ + 'input', // will use a simple text input + $this->l10n->t('The file to return'), // the label $filename, // the current value - L10n::t("Enter the path to a file"), // the help text - // no extra data for "input" type.. + $this->l10n->t('Enter the path to a file'), // the help text + // no extra data for 'input' type.. + ], ]; } - public static function saveOptions($data) + public function saveOptions(array $data) { // the keys in $data are the same keys we defined in getOptions() - $newfilename = trim($data["filename"]); + $newfilename = trim($data['filename']); // this function should always validate the data. // in this example we check if file exists if (!file_exists($newfilename)) { // in case of error we return an array with - // ["optionname" => "error message"] - return ["filename" => "The file doesn't exists"]; + // ['optionname' => 'error message'] + return ['filename' => 'The file doesn\'t exists']; } - Config::set("storage", "samplestorage", $newfilename); + $this->config->set('storage', 'samplestorage', $newfilename); // no errors, return empty array return []; } + + public function __toString() + { + return self::NAME; + } } ``` @@ -200,5 +249,33 @@ function samplestorage_unistall() } ``` +**Theoretically - until tests for Addons are enabled too - create a test class with the name `addon/tests/SampleStorageTest.php`: +```php +use Friendica\Model\Storage\IStorage; +use Friendica\Test\src\Model\Storage\StorageTest; +class SampleStorageTest extends StorageTest +{ + // returns an instance of your newly created storage class + protected function getInstance() + { + // create a new SampleStorageBackend instance with all it's dependencies + // Have a look at DatabaseStorageTest or FilesystemStorageTest for further insights + return new SampleStorageBackend(); + } + + // Assertion for the option array you return for your new StorageClass + protected function assertOption(IStorage $storage) + { + $this->assertEquals([ + 'filename' => [ + 'input', + 'The file to return', + 'sample.jpg', + 'Enter the path to a file' + ], + ], $storage->getOptions()); + } +} +``` From dbd5b5bb6e329ceb216fc2c13540e9f482129ccd Mon Sep 17 00:00:00 2001 From: nupplaPhil Date: Mon, 6 Jan 2020 17:42:28 +0100 Subject: [PATCH 03/14] - Fixing SystemResource - Adding tests for StorageManager - Updating doc --- doc/AddonStorageBackend.md | 9 +- src/Core/StorageManager.php | 70 ++++-- src/Model/Photo.php | 5 +- src/Model/Storage/AbstractStorage.php | 32 +++ src/Model/Storage/Database.php | 33 ++- src/Model/Storage/Filesystem.php | 12 +- src/Model/Storage/IStorage.php | 7 + src/Model/Storage/SystemResource.php | 42 +++- tests/Util/SampleStorageBackend.php | 94 ++++++++ tests/datasets/storage/database.fixture.php | 40 ++++ tests/src/Core/StorageManagerTest.php | 252 ++++++++++++++++++++ update.php | 10 + 12 files changed, 547 insertions(+), 59 deletions(-) create mode 100644 src/Model/Storage/AbstractStorage.php create mode 100644 tests/Util/SampleStorageBackend.php create mode 100644 tests/datasets/storage/database.fixture.php create mode 100644 tests/src/Core/StorageManagerTest.php diff --git a/doc/AddonStorageBackend.md b/doc/AddonStorageBackend.md index 896ea49bf..115bfd003 100644 --- a/doc/AddonStorageBackend.md +++ b/doc/AddonStorageBackend.md @@ -215,6 +215,11 @@ class SampleStorageBackend implements IStorage { return self::NAME; } + + public static function getName() + { + return self::NAME; + } } ``` @@ -239,13 +244,13 @@ function samplestorage_install() // on addon install, we register our class with name "Sample Storage". // note: we use `::class` property, which returns full class name as string // this save us the problem of correctly escape backslashes in class name - DI::facStorage()->register("Sample Storage", SampleStorageBackend::class); + DI::facStorage()->register(SampleStorageBackend::class); } function samplestorage_unistall() { // when the plugin is uninstalled, we unregister the backend. - DI::facStorage()->unregister("Sample Storage"); + DI::facStorage()->unregister(SampleStorageBackend::class); } ``` diff --git a/src/Core/StorageManager.php b/src/Core/StorageManager.php index ede8966f4..6a67a06ed 100644 --- a/src/Core/StorageManager.php +++ b/src/Core/StorageManager.php @@ -45,6 +45,7 @@ class StorageManager * @param Database $dba * @param IConfiguration $config * @param LoggerInterface $logger + * @param Dice $dice */ public function __construct(Database $dba, IConfiguration $config, LoggerInterface $logger, Dice $dice) { @@ -76,27 +77,39 @@ class StorageManager /** * @brief Return storage backend class by registered name * - * @param string $name Backend name + * @param string|null $name Backend name * * @return Storage\IStorage|null null if no backend registered at $name */ - public function getByName(string $name) + public function getByName(string $name = null) { - if (!$this->isValidBackend($name)) { + if (!$this->isValidBackend($name) && + $name !== Storage\SystemResource::getName()) { return null; } - return $this->dice->create($this->backends[$name]); + /** @var Storage\IStorage $storage */ + $storage = null; + + // If the storage of the file is a system resource, + // create it directly since it isn't listed in the registered backends + if ($name === Storage\SystemResource::getName()) { + $storage = $this->dice->create(Storage\SystemResource::class); + } else { + $storage = $this->dice->create($this->backends[$name]); + } + + return $storage; } /** * Checks, if the storage is a valid backend * - * @param string $name The name or class of the backend + * @param string|null $name The name or class of the backend * * @return boolean True, if the backend is a valid backend */ - public function isValidBackend(string $name) + public function isValidBackend(string $name = null) { return array_key_exists($name, $this->backends); } @@ -108,7 +121,7 @@ class StorageManager * * @return boolean True, if the set was successful */ - public function setBackend(string $name) + public function setBackend(string $name = null) { if (!$this->isValidBackend($name)) { return false; @@ -135,23 +148,24 @@ class StorageManager /** * @brief Register a storage backend class * - * @param string $name User readable backend name * @param string $class Backend class name * * @return boolean True, if the registration was successful */ - public function register(string $name, string $class) + public function register(string $class) { - if (!is_subclass_of($class, Storage\IStorage::class)) { - return false; - } + if (is_subclass_of($class, Storage\IStorage::class)) { + /** @var Storage\IStorage $class */ - $backends = $this->backends; - $backends[$name] = $class; + $backends = $this->backends; + $backends[$class::getName()] = $class; - if ($this->config->set('storage', 'backends', $this->backends)) { - $this->backends = $backends; - return true; + if ($this->config->set('storage', 'backends', $backends)) { + $this->backends = $backends; + return true; + } else { + return false; + } } else { return false; } @@ -160,14 +174,26 @@ class StorageManager /** * @brief Unregister a storage backend class * - * @param string $name User readable backend name + * @param string $class Backend class name * * @return boolean True, if unregistering was successful */ - public function unregister(string $name) + public function unregister(string $class) { - unset($this->backends[$name]); - return $this->config->set('storage', 'backends', $this->backends); + if (is_subclass_of($class, Storage\IStorage::class)) { + /** @var Storage\IStorage $class */ + + unset($this->backends[$class::getName()]); + + if ($this->currentBackend instanceof $class) { + $this->config->set('storage', 'name', null); + $this->currentBackend = null; + } + + return $this->config->set('storage', 'backends', $this->backends); + } else { + return false; + } } /** @@ -196,7 +222,7 @@ class StorageManager $resources = $this->dba->select( $table, ['id', 'data', 'backend-class', 'backend-ref'], - ['`backend-class` IS NULL or `backend-class` != ?', $destination], + ['`backend-class` IS NULL or `backend-class` != ?', $destination::getName()], ['limit' => $limit] ); diff --git a/src/Model/Photo.php b/src/Model/Photo.php index 7aa6ffdb3..f8f50824b 100644 --- a/src/Model/Photo.php +++ b/src/Model/Photo.php @@ -16,6 +16,7 @@ use Friendica\Database\DBA; use Friendica\Database\DBStructure; use Friendica\DI; use Friendica\Model\Storage\IStorage; +use Friendica\Model\Storage\SystemResource; use Friendica\Object\Image; use Friendica\Util\DateTimeFormat; use Friendica\Util\Images; @@ -223,7 +224,7 @@ class Photo $values = array_fill(0, count($fields), ""); $photo = array_combine($fields, $values); - $photo["backend-class"] = Storage\SystemResource::class; + $photo["backend-class"] = SystemResource::NAME; $photo["backend-ref"] = $filename; $photo["type"] = $mimetype; $photo["cacheable"] = false; @@ -275,7 +276,7 @@ class Photo if (DBA::isResult($existing_photo)) { $backend_ref = (string)$existing_photo["backend-ref"]; - $storage = DI::facStorage()->getByName((string)$existing_photo["backend-class"]); + $storage = DI::facStorage()->getByName($existing_photo["backend-class"] ?? ''); } else { $storage = DI::storage(); } diff --git a/src/Model/Storage/AbstractStorage.php b/src/Model/Storage/AbstractStorage.php new file mode 100644 index 000000000..270d67562 --- /dev/null +++ b/src/Model/Storage/AbstractStorage.php @@ -0,0 +1,32 @@ +l10n = $l10n; + $this->logger = $logger; + } + + public function __toString() + { + return static::getName(); + } +} diff --git a/src/Model/Storage/Database.php b/src/Model/Storage/Database.php index 84dd116b5..182add861 100644 --- a/src/Model/Storage/Database.php +++ b/src/Model/Storage/Database.php @@ -6,35 +6,32 @@ namespace Friendica\Model\Storage; -use Friendica\Core\L10n; +use Friendica\Core\L10n\L10n; use Psr\Log\LoggerInterface; +use Friendica\Database\Database as DBA; /** * @brief Database based storage system * * This class manage data stored in database table. */ -class Database implements IStorage +class Database extends AbstractStorage { const NAME = 'Database'; - /** @var \Friendica\Database\Database */ + /** @var DBA */ private $dba; - /** @var LoggerInterface */ - private $logger; - /** @var L10n\L10n */ - private $l10n; /** - * @param \Friendica\Database\Database $dba - * @param LoggerInterface $logger - * @param L10n\L10n $l10n + * @param DBA $dba + * @param LoggerInterface $logger + * @param L10n $l10n */ - public function __construct(\Friendica\Database\Database $dba, LoggerInterface $logger, L10n\L10n $l10n) + public function __construct(DBA $dba, LoggerInterface $logger, L10n $l10n) { - $this->dba = $dba; - $this->logger = $logger; - $this->l10n = $l10n; + parent::__construct($l10n, $logger); + + $this->dba = $dba; } /** @@ -58,15 +55,15 @@ class Database implements IStorage if ($reference !== '') { $result = $this->dba->update('storage', ['data' => $data], ['id' => $reference]); if ($result === false) { - $this->logger->warning('Failed to update data.', ['id' => $reference, 'errorCode' => $this->dba->errorNo(), 'errorMessage' => $this->dba->errorMessage()]); + $this->logger->warning('Failed to update data.', ['id' => $reference, 'errorCode' => $this->dba->errorNo(), 'errorMessage' => $this->dba->errorMessage()]); throw new StorageException($this->l10n->t('Database storage failed to update %s', $reference)); - } + } return $reference; } else { $result = $this->dba->insert('storage', ['data' => $data]); if ($result === false) { - $this->logger->warning('Failed to insert data.', ['errorCode' => $this->dba->errorNo(), 'errorMessage' => $this->dba->errorMessage()]); + $this->logger->warning('Failed to insert data.', ['errorCode' => $this->dba->errorNo(), 'errorMessage' => $this->dba->errorMessage()]); throw new StorageException($this->l10n->t('Database storage failed to insert data')); } @@ -101,7 +98,7 @@ class Database implements IStorage /** * @inheritDoc */ - public function __toString() + public static function getName() { return self::NAME; } diff --git a/src/Model/Storage/Filesystem.php b/src/Model/Storage/Filesystem.php index e954352bc..bfc474a5a 100644 --- a/src/Model/Storage/Filesystem.php +++ b/src/Model/Storage/Filesystem.php @@ -21,7 +21,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 implements IStorage +class Filesystem extends AbstractStorage { const NAME = 'Filesystem'; @@ -30,10 +30,6 @@ class Filesystem implements IStorage /** @var IConfiguration */ private $config; - /** @var LoggerInterface */ - private $logger; - /** @var L10n */ - private $l10n; /** @var string */ private $basePath; @@ -47,9 +43,9 @@ class Filesystem implements IStorage */ public function __construct(IConfiguration $config, LoggerInterface $logger, L10n $l10n) { + parent::__construct($l10n, $logger); + $this->config = $config; - $this->logger = $logger; - $this->l10n = $l10n; $path = $this->config->get('storage', 'filesystem_path', self::DEFAULT_BASE_FOLDER); $this->basePath = rtrim($path, '/'); @@ -185,7 +181,7 @@ class Filesystem implements IStorage /** * @inheritDoc */ - public function __toString() + public static function getName() { return self::NAME; } diff --git a/src/Model/Storage/IStorage.php b/src/Model/Storage/IStorage.php index a7b7f6f56..c3ec3197c 100644 --- a/src/Model/Storage/IStorage.php +++ b/src/Model/Storage/IStorage.php @@ -96,4 +96,11 @@ interface IStorage * @return string */ public function __toString(); + + /** + * The name of the backend + * + * @return string + */ + public static function getName(); } diff --git a/src/Model/Storage/SystemResource.php b/src/Model/Storage/SystemResource.php index 3afe8ee6f..43514c1dc 100644 --- a/src/Model/Storage/SystemResource.php +++ b/src/Model/Storage/SystemResource.php @@ -16,10 +16,15 @@ use \BadMethodCallException; */ class SystemResource implements IStorage { + const NAME = 'SystemResource'; + // Valid folders to look for resources const VALID_FOLDERS = ["images"]; - public static function get($filename) + /** + * @inheritDoc + */ + public function get(string $filename) { $folder = dirname($filename); if (!in_array($folder, self::VALID_FOLDERS)) { @@ -31,25 +36,48 @@ class SystemResource implements IStorage return file_get_contents($filename); } - - public static function put($data, $filename = "") + /** + * @inheritDoc + */ + public function put(string $data, string $filename = '') { throw new BadMethodCallException(); } - public static function delete($filename) + public function delete(string $filename) { throw new BadMethodCallException(); } - public static function getOptions() + /** + * @inheritDoc + */ + public function getOptions() { return []; } - public static function saveOptions($data) + /** + * @inheritDoc + */ + public function saveOptions(array $data) { return []; } + + /** + * @inheritDoc + */ + public function __toString() + { + return self::NAME; + } + + /** + * @inheritDoc + */ + public static function getName() + { + return self::NAME; + } } - diff --git a/tests/Util/SampleStorageBackend.php b/tests/Util/SampleStorageBackend.php new file mode 100644 index 000000000..3f4eb90ad --- /dev/null +++ b/tests/Util/SampleStorageBackend.php @@ -0,0 +1,94 @@ + [ + 'input', // will use a simple text input + 'The file to return', // the label + 'sample', // the current value + 'Enter the path to a file', // the help text + // no extra data for 'input' type.. + ], + ]; + /** @var array Just save the data in memory */ + private $data = []; + + /** + * SampleStorageBackend constructor. + * + * @param L10n $l10n The configuration of Friendica + * + * You can add here every dynamic class as dependency you like and add them to a private field + * Friendica automatically creates these classes and passes them as argument to the constructor + */ + public function __construct(L10n $l10n) + { + $this->l10n = $l10n; + } + + public function get(string $reference) + { + // we return always the same image data. Which file we load is defined by + // a config key + return $this->data[$reference] ?? null; + } + + public function put(string $data, string $reference = '') + { + if ($reference === '') { + $reference = 'sample'; + } + + $this->data[$reference] = $data; + + return $reference; + } + + public function delete(string $reference) + { + if (isset($this->data[$reference])) { + unset($this->data[$reference]); + } + + return true; + } + + public function getOptions() + { + return $this->options; + } + + public function saveOptions(array $data) + { + $this->options = $data; + + // no errors, return empty array + return $this->options; + } + + public function __toString() + { + return self::NAME; + } + + public static function getName() + { + return self::NAME; + } +} diff --git a/tests/datasets/storage/database.fixture.php b/tests/datasets/storage/database.fixture.php new file mode 100644 index 000000000..780bfa140 --- /dev/null +++ b/tests/datasets/storage/database.fixture.php @@ -0,0 +1,40 @@ + [ + // move from data-attribute to storage backend + [ + 'id' => 1, + 'backend-class' => null, + 'backend-ref' => 'f0c0d0i2', + 'data' => 'without class', + ], + // move from storage-backend to maybe filesystem backend, skip at database backend + [ + 'id' => 2, + 'backend-class' => 'Database', + 'backend-ref' => 1, + 'data' => '', + ], + // move data if invalid storage + [ + 'id' => 3, + 'backend-class' => 'invalid!', + 'backend-ref' => 'unimported', + 'data' => 'invalid data moved', + ], + // skip everytime because of invalid storage and no data + [ + 'id' => 3, + 'backend-class' => 'invalid!', + 'backend-ref' => 'unimported', + 'data' => '', + ], + ], + 'storage' => [ + [ + 'id' => 1, + 'data' => 'inside database', + ], + ], +]; diff --git a/tests/src/Core/StorageManagerTest.php b/tests/src/Core/StorageManagerTest.php new file mode 100644 index 000000000..82af38b4a --- /dev/null +++ b/tests/src/Core/StorageManagerTest.php @@ -0,0 +1,252 @@ +setUpVfsDir(); + + $this->logger = new NullLogger(); + $this->dice = (new Dice()) + ->addRules(include __DIR__ . '/../../../static/dependencies.config.php') + ->addRule(Database::class, ['instanceOf' => StaticDatabase::class, 'shared' => true]) + ->addRule(ISession::class, ['instanceOf' => Session\Memory::class, 'shared' => true, 'call' => null]); + + $profiler = \Mockery::mock(Profiler::class); + $profiler->shouldReceive('saveTimestamp')->withAnyArgs()->andReturn(true); + + // load real config to avoid mocking every config-entry which is related to the Database class + $configFactory = new ConfigFactory(); + $loader = new ConfigFileLoader($this->root->url()); + $configCache = $configFactory->createCache($loader); + + $this->dba = new StaticDatabase($configCache, $profiler, $this->logger); + + $configModel = new Config($this->dba); + $this->config = new PreloadConfiguration($configCache, $configModel); + } + + /** + * Test plain instancing first + */ + public function testInstance() + { + $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->dice); + + $this->assertInstanceOf(StorageManager::class, $storageManager); + } + + public function dataStorages() + { + return [ + 'empty' => [ + 'name' => '', + 'assert' => null, + 'assertName' => '', + 'userBackend' => false, + ], + 'database' => [ + 'name' => Storage\Database::NAME, + 'assert' => Storage\Database::class, + 'assertName' => Storage\Database::NAME, + 'userBackend' => true, + ], + 'filesystem' => [ + 'name' => Storage\Filesystem::NAME, + 'assert' => Storage\Filesystem::class, + 'assertName' => Storage\Filesystem::NAME, + 'userBackend' => true, + ], + 'systemresource' => [ + 'name' => Storage\SystemResource::NAME, + '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', + 'assert' => null, + 'assertName' => '', + 'userBackend' => false, + ], + ]; + } + + /** + * Test the getByName() method + * + * @dataProvider dataStorages + */ + public function testGetByName($name, $assert, $assertName) + { + $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->dice); + + $storage = $storageManager->getByName($name); + + if (!empty($assert)) { + $this->assertInstanceOf(Storage\IStorage::class, $storage); + $this->assertInstanceOf($assert, $storage); + $this->assertEquals($name, $storage::getName()); + } else { + $this->assertNull($storage); + } + $this->assertEquals($assertName, $storage); + } + + /** + * Test the isValidBackend() method + * + * @dataProvider dataStorages + */ + public function testIsValidBackend($name, $assert, $assertName, $userBackend) + { + $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->dice); + + $this->assertEquals($userBackend, $storageManager->isValidBackend($name)); + } + + /** + * Test the method listBackends() with default setting + */ + public function testListBackends() + { + $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->dice); + + $this->assertEquals(StorageManager::DEFAULT_BACKENDS, $storageManager->listBackends()); + } + + /** + * Test the method getBackend() + * + * @dataProvider dataStorages + */ + public function testGetBackend($name, $assert, $assertName, $userBackend) + { + $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->dice); + + $this->assertNull($storageManager->getBackend()); + + if ($userBackend) { + $storageManager->setBackend($name); + + $this->assertInstanceOf($assert, $storageManager->getBackend()); + } + } + + /** + * Test the method getBackend() with a pre-configured backend + * + * @dataProvider dataStorages + */ + public function testPresetBackend($name, $assert, $assertName, $userBackend) + { + $this->config->set('storage', 'name', $name); + + $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->dice); + + if ($userBackend) { + $this->assertInstanceOf($assert, $storageManager->getBackend()); + } else { + $this->assertNull($storageManager->getBackend()); + } + } + + /** + * Tests the register and unregister methods for a new backend storage class + * + * Uses a sample storage for testing + * + * @see SampleStorageBackend + */ + public function testRegisterUnregisterBackends() + { + $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->dice); + + $this->assertTrue($storageManager->register(SampleStorageBackend::class)); + + $this->assertEquals(array_merge(StorageManager::DEFAULT_BACKENDS, [ + 'Sample Storage' => SampleStorageBackend::class, + ]), $storageManager->listBackends()); + $this->assertEquals(array_merge(StorageManager::DEFAULT_BACKENDS, [ + 'Sample Storage' => SampleStorageBackend::class, + ]), $this->config->get('storage', 'backends')); + + $this->assertTrue($storageManager->setBackend(SampleStorageBackend::NAME)); + $this->assertEquals(SampleStorageBackend::NAME, $this->config->get('storage', 'name')); + + $this->assertInstanceOf(SampleStorageBackend::class, $storageManager->getBackend()); + + $this->assertTrue($storageManager->unregister(SampleStorageBackend::class)); + $this->assertEquals(StorageManager::DEFAULT_BACKENDS, $this->config->get('storage', 'backends')); + $this->assertEquals(StorageManager::DEFAULT_BACKENDS, $storageManager->listBackends()); + + $this->assertNull($storageManager->getBackend()); + $this->assertNull($this->config->get('storage', 'name')); + } + + /** + * Test moving data to a new storage (currently testing db & filesystem) + * + * @dataProvider dataStorages + */ + public function testMoveStorage($name, $assert, $assertName, $userBackend) + { + if (!$userBackend) { + return; + } + + $this->loadFixture(__DIR__ . '/../../datasets/storage/database.fixture.php', $this->dba); + + $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->dice); + $storage = $storageManager->getByName($name); + $storageManager->move($storage); + + $photos = $this->dba->select('photo', ['backend-ref', 'backend-class', 'id', 'data']); + + while ($photo = $this->dba->fetch($photos)) { + + $this->assertEmpty($photo['data']); + + $storage = $storageManager->getByName($photo['backend-class']); + $data = $storage->get($photo['backend-ref']); + + $this->assertNotEmpty($data); + } + } +} diff --git a/update.php b/update.php index 90018b62d..145446bae 100644 --- a/update.php +++ b/update.php @@ -418,5 +418,15 @@ function update_1329() Config::delete('storage', 'class'); } + $photos = DBA::select('photos', ['backend-class', 'id'], ['backend-class IS NOT NULL']); + foreach ($photos as $photo) { + DBA::update('photos', ['backend-class' => $photo['backend-class']::NAME], ['id' => $photo['id']]); + } + + $attachs = DBA::select('attach', ['backend-class', 'id'], ['backend-class IS NOT NULL']); + foreach ($attachs as $attach) { + DBA::update('photos', ['backend-class' => $attach['backend-class']::NAME], ['id' => $attach['id']]); + } + return Update::SUCCESS; } From d6fab6b06b794b91ae635cc406a0dc61dee286b0 Mon Sep 17 00:00:00 2001 From: nupplaPhil Date: Mon, 6 Jan 2020 21:14:01 +0100 Subject: [PATCH 04/14] making update really work ... --- database.sql | 2 +- static/dbstructure.config.php | 2 +- update.php | 27 +++++++++++++++++---------- 3 files changed, 19 insertions(+), 12 deletions(-) diff --git a/database.sql b/database.sql index 6252f8057..8db6c1d6f 100644 --- a/database.sql +++ b/database.sql @@ -1,6 +1,6 @@ -- ------------------------------------------ -- Friendica 2020.03-dev (Dalmatian Bellflower) --- DB_UPDATE_VERSION 1329 +-- DB_UPDATE_VERSION 1330 -- ------------------------------------------ diff --git a/static/dbstructure.config.php b/static/dbstructure.config.php index 20bd93769..49934c63c 100755 --- a/static/dbstructure.config.php +++ b/static/dbstructure.config.php @@ -34,7 +34,7 @@ use Friendica\Database\DBA; if (!defined('DB_UPDATE_VERSION')) { - define('DB_UPDATE_VERSION', 1329); + define('DB_UPDATE_VERSION', 1330); } return [ diff --git a/update.php b/update.php index 145446bae..180cd96ec 100644 --- a/update.php +++ b/update.php @@ -12,6 +12,7 @@ use Friendica\Model\Contact; use Friendica\Model\GContact; use Friendica\Model\Item; use Friendica\Model\User; +use Friendica\Model\Storage; use Friendica\Util\DateTimeFormat; use Friendica\Worker\Delivery; @@ -408,24 +409,30 @@ function update_1327() return Update::SUCCESS; } -function update_1329() +function update_1330() { $currStorage = Config::get('storage', 'class', ''); if (!empty($currStorage)) { $storageName = array_key_first(\Friendica\Core\StorageManager::DEFAULT_BACKENDS, $currStorage); - Config::set('storage', 'name', $storageName); - Config::delete('storage', 'class'); + if (!Config::set('storage', 'name', $storageName) || + !Config::delete('storage', 'class')) { + return Update::FAILED; + }; } - $photos = DBA::select('photos', ['backend-class', 'id'], ['backend-class IS NOT NULL']); - foreach ($photos as $photo) { - DBA::update('photos', ['backend-class' => $photo['backend-class']::NAME], ['id' => $photo['id']]); - } + // Update photos + if (!DBA::update('photo', ['backend-class' => Storage\Filesystem::NAME], ['backend-class' => 'Friendica\Model\Storage\Filesystem']) || + !DBA::update('photo', ['backend-class' => Storage\Database::NAME], ['backend-class' => 'Friendica\Model\Storage\Database']) || + !DBA::update('photo', ['backend-class' => Storage\SystemResource::NAME], ['backend-class' => 'Friendica\Model\Storage\SystemResource'])) { + return Update::FAILED; + }; - $attachs = DBA::select('attach', ['backend-class', 'id'], ['backend-class IS NOT NULL']); - foreach ($attachs as $attach) { - DBA::update('photos', ['backend-class' => $attach['backend-class']::NAME], ['id' => $attach['id']]); + // update attachments + if (!DBA::update('attach', ['backend-class' => Storage\Filesystem::NAME], ['backend-class' => 'Friendica\Model\Storage\Filesystem']) || + !DBA::update('attach', ['backend-class' => Storage\Database::NAME], ['backend-class' => 'Friendica\Model\Storage\Database']) || + !DBA::update('attach', ['backend-class' => Storage\SystemResource::NAME], ['backend-class' => 'Friendica\Model\Storage\SystemResource'])) { + return Update::FAILED; } return Update::SUCCESS; From 1bce3fd0f1aa6b32139ae7a6ff7027254e97a3d0 Mon Sep 17 00:00:00 2001 From: nupplaPhil Date: Mon, 6 Jan 2020 22:07:23 +0100 Subject: [PATCH 05/14] fix Photo model --- src/Model/Photo.php | 42 +++++++++++++++++++----------------------- 1 file changed, 19 insertions(+), 23 deletions(-) diff --git a/src/Model/Photo.php b/src/Model/Photo.php index f8f50824b..8df06565b 100644 --- a/src/Model/Photo.php +++ b/src/Model/Photo.php @@ -173,26 +173,24 @@ class Photo */ public static function getImageForPhoto(array $photo) { - $data = ""; - - if ($photo["backend-class"] == "") { + if (empty($photo['backend-class'])) { // legacy data storage in "data" column - $i = self::selectFirst(["data"], ["id" => $photo["id"]]); + $i = self::selectFirst(['data'], ['id' => $photo['id']]); if ($i === false) { return null; } - $data = $i["data"]; + $data = $i['data']; } else { - $backendClass = $photo["backend-class"]; - $backendRef = $photo["backend-ref"]; - $data = $backendClass::get($backendRef); + $backendClass = DI::facStorage()->getByName($photo['backend-class'] ?? ''); + $backendRef = $photo['backend-ref'] ?? ''; + $data = $backendClass->get($backendRef); } - if ($data === "") { + if (empty($data)) { return null; } - return new Image($data, $photo["type"]); + return new Image($data, $photo['type']); } /** @@ -223,11 +221,11 @@ class Photo $fields = self::getFields(); $values = array_fill(0, count($fields), ""); - $photo = array_combine($fields, $values); - $photo["backend-class"] = SystemResource::NAME; - $photo["backend-ref"] = $filename; - $photo["type"] = $mimetype; - $photo["cacheable"] = false; + $photo = array_combine($fields, $values); + $photo['backend-class'] = SystemResource::NAME; + $photo['backend-ref'] = $filename; + $photo['type'] = $mimetype; + $photo['cacheable'] = false; return $photo; } @@ -340,10 +338,9 @@ class Photo $photos = self::selectToArray(['backend-class', 'backend-ref'], $conditions); foreach($photos as $photo) { - /** @var IStorage $backend_class */ - $backend_class = (string)$photo["backend-class"]; - if ($backend_class !== "") { - $backend_class::delete($photo["backend-ref"]); + $backend_class = DI::facStorage()->getByName($photo['backend-class'] ?? ''); + if ($backend_class !== null) { + $backend_class->delete($photo["backend-ref"] ?? ''); } } @@ -370,10 +367,9 @@ class Photo $photos = self::selectToArray(['backend-class', 'backend-ref'], $conditions); foreach($photos as $photo) { - /** @var IStorage $backend_class */ - $backend_class = (string)$photo["backend-class"]; - if ($backend_class !== "") { - $fields["backend-ref"] = $backend_class::put($img->asString(), $photo["backend-ref"]); + $backend_class = DI::facStorage()->getByName($photo['backend-class'] ?? ''); + if ($backend_class !== null) { + $fields["backend-ref"] = $backend_class->put($img->asString(), $photo['backend-ref']); } else { $fields["data"] = $img->asString(); } From 6f4eee516b2f1c1607c9a7e2d100a08f611f8ea4 Mon Sep 17 00:00:00 2001 From: nupplaPhil Date: Mon, 6 Jan 2020 22:46:22 +0100 Subject: [PATCH 06/14] optimize update script --- update.php | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/update.php b/update.php index 180cd96ec..7d25c6ac0 100644 --- a/update.php +++ b/update.php @@ -413,27 +413,20 @@ function update_1330() { $currStorage = Config::get('storage', 'class', ''); + // set the name of the storage instead of the classpath as config if (!empty($currStorage)) { - $storageName = array_key_first(\Friendica\Core\StorageManager::DEFAULT_BACKENDS, $currStorage); - if (!Config::set('storage', 'name', $storageName) || + /** @var Storage\IStorage $currStorage */ + if (!Config::set('storage', 'name', $currStorage::getName()) || !Config::delete('storage', 'class')) { return Update::FAILED; }; } - // Update photos - if (!DBA::update('photo', ['backend-class' => Storage\Filesystem::NAME], ['backend-class' => 'Friendica\Model\Storage\Filesystem']) || - !DBA::update('photo', ['backend-class' => Storage\Database::NAME], ['backend-class' => 'Friendica\Model\Storage\Database']) || - !DBA::update('photo', ['backend-class' => Storage\SystemResource::NAME], ['backend-class' => 'Friendica\Model\Storage\SystemResource'])) { + // Update attachments and photos + if (!DBA::p("UPDATE `photo` SET `photo`.`backend-class` = SUBSTR(`photo`.`backend-class`, 22) WHERE `photo`.`backend-class` LIKE 'Friendica\\Model\\Storage\\%'") || + !DBA::p("UPDATE `attach` SET `attach`.`backend-class` = SUBSTR(`attach`.`backend-class`, 22) WHERE `attach`.`backend-class` LIKE 'Friendica\\Model\\Storage\\%'")) { return Update::FAILED; }; - // update attachments - if (!DBA::update('attach', ['backend-class' => Storage\Filesystem::NAME], ['backend-class' => 'Friendica\Model\Storage\Filesystem']) || - !DBA::update('attach', ['backend-class' => Storage\Database::NAME], ['backend-class' => 'Friendica\Model\Storage\Database']) || - !DBA::update('attach', ['backend-class' => Storage\SystemResource::NAME], ['backend-class' => 'Friendica\Model\Storage\SystemResource'])) { - return Update::FAILED; - } - return Update::SUCCESS; } From c2ac206379b46c55a951670bafdae289543bc17f Mon Sep 17 00:00:00 2001 From: nupplaPhil Date: Mon, 6 Jan 2020 23:23:44 +0100 Subject: [PATCH 07/14] Fix in case the storage config is set as file. --- update.php | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/update.php b/update.php index 7d25c6ac0..924fac545 100644 --- a/update.php +++ b/update.php @@ -416,10 +416,12 @@ function update_1330() // set the name of the storage instead of the classpath as config if (!empty($currStorage)) { /** @var Storage\IStorage $currStorage */ - if (!Config::set('storage', 'name', $currStorage::getName()) || - !Config::delete('storage', 'class')) { + if (!Config::set('storage', 'name', $currStorage::getName())) { return Update::FAILED; - }; + } + + // try to delete the class since it isn't needed. This won't work with config files + Config::delete('storage', 'class'); } // Update attachments and photos From 1b2ff54f668a2a116e038ec44da0abddc45162ed Mon Sep 17 00:00:00 2001 From: nupplaPhil Date: Mon, 6 Jan 2020 23:58:41 +0100 Subject: [PATCH 08/14] Fix attach model --- src/Model/Attach.php | 14 +++++--------- src/Model/Photo.php | 2 -- 2 files changed, 5 insertions(+), 11 deletions(-) diff --git a/src/Model/Attach.php b/src/Model/Attach.php index 102e7b1a1..7a0446176 100644 --- a/src/Model/Attach.php +++ b/src/Model/Attach.php @@ -6,12 +6,10 @@ */ namespace Friendica\Model; -use Friendica\Core\StorageManager; use Friendica\Core\System; use Friendica\Database\DBA; use Friendica\Database\DBStructure; use Friendica\DI; -use Friendica\Model\Storage\IStorage; use Friendica\Object\Image; use Friendica\Util\DateTimeFormat; use Friendica\Util\Mimetype; @@ -261,10 +259,9 @@ class Attach $items = self::selectToArray(['backend-class','backend-ref'], $conditions); foreach($items as $item) { - /** @var IStorage $backend_class */ - $backend_class = (string)$item['backend-class']; + $backend_class = DI::facStorage()->getByName($item['backend-class'] ?? ''); if ($backend_class !== '') { - $fields['backend-ref'] = $backend_class::put($img->asString(), $item['backend-ref']); + $fields['backend-ref'] = $backend_class->put($img->asString(), $item['backend-ref'] ?? ''); } else { $fields['data'] = $img->asString(); } @@ -294,10 +291,9 @@ class Attach $items = self::selectToArray(['backend-class','backend-ref'], $conditions); foreach($items as $item) { - /** @var IStorage $backend_class */ - $backend_class = (string)$item['backend-class']; - if ($backend_class !== '') { - $backend_class::delete($item['backend-ref']); + $backend_class = DI::facStorage()->getByName($item['backend-class'] ?? ''); + if ($backend_class !== null) { + $backend_class->delete($item['backend-ref'] ?? ''); } } diff --git a/src/Model/Photo.php b/src/Model/Photo.php index 8df06565b..b13aaa79a 100644 --- a/src/Model/Photo.php +++ b/src/Model/Photo.php @@ -10,12 +10,10 @@ use Friendica\Core\Cache; use Friendica\Core\Config; use Friendica\Core\L10n; use Friendica\Core\Logger; -use Friendica\Core\StorageManager; use Friendica\Core\System; use Friendica\Database\DBA; use Friendica\Database\DBStructure; use Friendica\DI; -use Friendica\Model\Storage\IStorage; use Friendica\Model\Storage\SystemResource; use Friendica\Object\Image; use Friendica\Util\DateTimeFormat; From b68d0516ea20ec2705c9071d70493419c87ca5b0 Mon Sep 17 00:00:00 2001 From: nupplaPhil Date: Wed, 8 Jan 2020 02:05:30 +0100 Subject: [PATCH 09/14] remove not working code for checking writable files ("is_writable()" uses a different user) --- src/Model/Storage/Filesystem.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Model/Storage/Filesystem.php b/src/Model/Storage/Filesystem.php index bfc474a5a..9c429cfb3 100644 --- a/src/Model/Storage/Filesystem.php +++ b/src/Model/Storage/Filesystem.php @@ -125,7 +125,7 @@ class Filesystem extends AbstractStorage $this->createFoldersForFile($file); - if ((file_exists($file) && !is_writable($file)) || !file_put_contents($file, $data)) { + if (!file_put_contents($file, $data)) { $this->logger->warning('Failed to write data.', ['file' => $file]); throw new StorageException($this->l10n->t('Filesystem storage failed to save data to "%s". Check your write permissions', $file)); } From abc6a8ce3448e90960f91be9068e44e57918be9f Mon Sep 17 00:00:00 2001 From: nupplaPhil Date: Wed, 8 Jan 2020 21:31:16 +0100 Subject: [PATCH 10/14] skip impossible test-scenario --- tests/src/Model/Storage/FilesystemStorageTest.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/src/Model/Storage/FilesystemStorageTest.php b/tests/src/Model/Storage/FilesystemStorageTest.php index 2dd467254..500fd93bd 100644 --- a/tests/src/Model/Storage/FilesystemStorageTest.php +++ b/tests/src/Model/Storage/FilesystemStorageTest.php @@ -78,6 +78,8 @@ class FilesystemStorageTest extends StorageTest */ public function testMissingFilePermissions() { + $this->markTestIncomplete("Cannot catch file_put_content() error due vfsStream failure"); + vfsStream::create(['storage' => ['f0' => ['c0' => ['k0i0' => '']]]], $this->root); $this->root->getChild('storage/f0/c0/k0i0')->chmod(000); From 5d8e6c33ef31291a3aef6e3912ab450a96ed051c Mon Sep 17 00:00:00 2001 From: nupplaPhil Date: Wed, 8 Jan 2020 21:33:52 +0100 Subject: [PATCH 11/14] revert Dice version bump --- composer.json | 2 +- composer.lock | 90 +++++++++------------------------------------------ 2 files changed, 17 insertions(+), 75 deletions(-) diff --git a/composer.json b/composer.json index d587b721c..e372547aa 100644 --- a/composer.json +++ b/composer.json @@ -32,7 +32,7 @@ "ezyang/htmlpurifier": "^4.7", "friendica/json-ld": "^1.0", "league/html-to-markdown": "^4.8", - "level-2/dice": "dev-master", + "level-2/dice": "^4", "lightopenid/lightopenid": "dev-master", "michelf/php-markdown": "^1.7", "mobiledetect/mobiledetectlib": "^2.8", diff --git a/composer.lock b/composer.lock index ad63fb1e8..3a4670b20 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "bf05cd52bc7307f45aff80f1d1fd8214", + "content-hash": "34ad225ce21474eb84ce78047d9f2c01", "packages": [ { "name": "asika/simple-console", @@ -485,7 +485,6 @@ "jsonld.php" ] }, - "notification-url": "https://packagist.org/downloads/", "license": [ "BSD-3-Clause" ], @@ -503,11 +502,11 @@ "description": "A JSON-LD Processor and API implementation in PHP.", "homepage": "https://git.friendi.ca/friendica/php-json-ld", "keywords": [ + "JSON", "JSON-LD", "Linked Data", "RDF", "Semantic Web", - "json", "jsonld" ], "time": "2018-10-08T20:41:00+00:00" @@ -824,16 +823,16 @@ }, { "name": "level-2/dice", - "version": "dev-master", + "version": "4.0.1", "source": { "type": "git", "url": "https://github.com/Level-2/Dice.git", - "reference": "2fea2749a625c3adcc29c402218b0dcaed11586f" + "reference": "e631f110f0520294fec902814c61cac26566023c" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/Level-2/Dice/zipball/2fea2749a625c3adcc29c402218b0dcaed11586f", - "reference": "2fea2749a625c3adcc29c402218b0dcaed11586f", + "url": "https://api.github.com/repos/Level-2/Dice/zipball/e631f110f0520294fec902814c61cac26566023c", + "reference": "e631f110f0520294fec902814c61cac26566023c", "shasum": "" }, "require": { @@ -866,7 +865,7 @@ "di", "ioc" ], - "time": "2019-10-03T16:08:46+00:00" + "time": "2019-05-01T12:55:36+00:00" }, { "name": "lightopenid/lightopenid", @@ -1315,22 +1314,6 @@ "require": { "npm-asset/ev-emitter": ">=1.0.0,<2.0.0" }, - "require-dev": { - "npm-asset/chalk": ">=1.1.1,<2.0.0", - "npm-asset/cheerio": ">=0.19.0,<0.20.0", - "npm-asset/gulp": ">=3.9.0,<4.0.0", - "npm-asset/gulp-jshint": ">=1.11.2,<2.0.0", - "npm-asset/gulp-json-lint": ">=0.1.0,<0.2.0", - "npm-asset/gulp-rename": ">=1.2.2,<2.0.0", - "npm-asset/gulp-replace": ">=0.5.4,<0.6.0", - "npm-asset/gulp-requirejs-optimize": "dev-github:metafizzy/gulp-requirejs-optimize", - "npm-asset/gulp-uglify": ">=1.4.2,<2.0.0", - "npm-asset/gulp-util": ">=3.0.7,<4.0.0", - "npm-asset/highlight.js": ">=8.9.1,<9.0.0", - "npm-asset/marked": ">=0.3.5,<0.4.0", - "npm-asset/minimist": ">=1.2.0,<2.0.0", - "npm-asset/transfob": ">=1.0.0,<2.0.0" - }, "type": "npm-asset-library", "extra": { "npm-asset-bugs": { @@ -1375,14 +1358,6 @@ "url": "https://registry.npmjs.org/jgrowl/-/jgrowl-1.4.6.tgz", "shasum": "2736e332aaee73ccf0a14a5f0066391a0a13f4a3" }, - "require-dev": { - "npm-asset/grunt": "~0.4.2", - "npm-asset/grunt-contrib-cssmin": "~0.9.0", - "npm-asset/grunt-contrib-jshint": "~0.6.3", - "npm-asset/grunt-contrib-less": "~0.11.0", - "npm-asset/grunt-contrib-uglify": "~0.4.0", - "npm-asset/grunt-contrib-watch": "~0.6.1" - }, "type": "npm-asset-library", "extra": { "npm-asset-bugs": { @@ -1415,32 +1390,6 @@ "url": "https://registry.npmjs.org/jquery/-/jquery-2.2.4.tgz", "shasum": "2c89d6889b5eac522a7eea32c14521559c6cbf02" }, - "require-dev": { - "npm-asset/commitplease": "2.0.0", - "npm-asset/core-js": "0.9.17", - "npm-asset/grunt": "0.4.5", - "npm-asset/grunt-babel": "5.0.1", - "npm-asset/grunt-cli": "0.1.13", - "npm-asset/grunt-compare-size": "0.4.0", - "npm-asset/grunt-contrib-jshint": "0.11.2", - "npm-asset/grunt-contrib-uglify": "0.9.2", - "npm-asset/grunt-contrib-watch": "0.6.1", - "npm-asset/grunt-git-authors": "2.0.1", - "npm-asset/grunt-jscs": "2.1.0", - "npm-asset/grunt-jsonlint": "1.0.4", - "npm-asset/grunt-npmcopy": "0.1.0", - "npm-asset/gzip-js": "0.3.2", - "npm-asset/jsdom": "5.6.1", - "npm-asset/load-grunt-tasks": "1.0.0", - "npm-asset/qunit-assert-step": "1.0.3", - "npm-asset/qunitjs": "1.17.1", - "npm-asset/requirejs": "2.1.17", - "npm-asset/sinon": "1.10.3", - "npm-asset/sizzle": "2.2.1", - "npm-asset/strip-json-comments": "1.0.3", - "npm-asset/testswarm": "1.1.0", - "npm-asset/win-spawn": "2.0.0" - }, "type": "npm-asset-library", "extra": { "npm-asset-bugs": { @@ -1588,12 +1537,6 @@ "url": "https://registry.npmjs.org/jquery-mousewheel/-/jquery-mousewheel-3.1.13.tgz", "shasum": "06f0335f16e353a695e7206bf50503cb523a6ee5" }, - "require-dev": { - "npm-asset/grunt": "~0.4.1", - "npm-asset/grunt-contrib-connect": "~0.5.0", - "npm-asset/grunt-contrib-jshint": "~0.7.1", - "npm-asset/grunt-contrib-uglify": "~0.2.7" - }, "type": "npm-asset-library", "extra": { "npm-asset-bugs": { @@ -3392,8 +3335,8 @@ "authors": [ { "name": "Sebastian Bergmann", - "email": "sb@sebastian-bergmann.de", - "role": "lead" + "role": "lead", + "email": "sb@sebastian-bergmann.de" } ], "description": "Library that provides collection, processing, and rendering functionality for PHP code coverage information.", @@ -3660,8 +3603,8 @@ "authors": [ { "name": "Sebastian Bergmann", - "email": "sebastian@phpunit.de", - "role": "lead" + "role": "lead", + "email": "sebastian@phpunit.de" } ], "description": "The PHP Unit Testing framework.", @@ -3834,7 +3777,7 @@ } ], "description": "Provides the functionality to compare PHP values for equality", - "homepage": "https://github.com/sebastianbergmann/comparator", + "homepage": "http://www.github.com/sebastianbergmann/comparator", "keywords": [ "comparator", "compare", @@ -3936,7 +3879,7 @@ } ], "description": "Provides functionality to handle HHVM/PHP environments", - "homepage": "https://github.com/sebastianbergmann/environment", + "homepage": "http://www.github.com/sebastianbergmann/environment", "keywords": [ "Xdebug", "environment", @@ -4004,7 +3947,7 @@ } ], "description": "Provides the functionality to export PHP variables for visualization", - "homepage": "https://github.com/sebastianbergmann/exporter", + "homepage": "http://www.github.com/sebastianbergmann/exporter", "keywords": [ "export", "exporter" @@ -4056,7 +3999,7 @@ } ], "description": "Snapshotting of global state", - "homepage": "https://github.com/sebastianbergmann/global-state", + "homepage": "http://www.github.com/sebastianbergmann/global-state", "keywords": [ "global state" ], @@ -4158,7 +4101,7 @@ } ], "description": "Provides functionality to recursively process PHP variables", - "homepage": "https://github.com/sebastianbergmann/recursion-context", + "homepage": "http://www.github.com/sebastianbergmann/recursion-context", "time": "2016-11-19T07:33:16+00:00" }, { @@ -4417,7 +4360,6 @@ "aliases": [], "minimum-stability": "stable", "stability-flags": { - "level-2/dice": 20, "lightopenid/lightopenid": 20 }, "prefer-stable": false, From bfae6766bf67d77a4fe8ed3efe99d174a06add5d Mon Sep 17 00:00:00 2001 From: nupplaPhil Date: Wed, 8 Jan 2020 22:51:37 +0100 Subject: [PATCH 12/14] Implement Hook::callAll('storage_instance') call for addons and add a description for it. - Remove implicit Dice usage - Add concrete instance creating - Adding Hook call for addon instance creating - Updating doc for Hook - Updating tests --- doc/AddonStorageBackend.md | 20 ++++- doc/Addons.md | 8 ++ doc/de/Addons.md | 4 + src/Core/StorageManager.php | 93 +++++++++++++-------- static/dependencies.config.php | 8 -- tests/Util/SampleStorageBackend.php | 12 +++ tests/Util/SampleStorageBackendInstance.php | 18 ++++ tests/src/Core/StorageManagerTest.php | 54 ++++++++---- 8 files changed, 155 insertions(+), 62 deletions(-) create mode 100644 tests/Util/SampleStorageBackendInstance.php diff --git a/doc/AddonStorageBackend.md b/doc/AddonStorageBackend.md index 115bfd003..ef1b4454b 100644 --- a/doc/AddonStorageBackend.md +++ b/doc/AddonStorageBackend.md @@ -23,6 +23,7 @@ interface IStorage public function getOptions(); public function saveOptions(array $data); public function __toString(); + public static function getName(); } ``` @@ -85,11 +86,16 @@ See doxygen documentation of `IStorage` interface for details about each method. Each backend must be registered in the system when the plugin is installed, to be aviable. -`DI::facStorage()->register(string $name, string $class)` is used to register the backend class. -The `$name` must be univocal and will be shown to admin. +`DI::facStorage()->register(string $class)` is used to register the backend class. When the plugin is uninstalled, registered backends must be unregistered using -`DI::facStorage()->unregister(string $name)`. +`DI::facStorage()->unregister(string $class)`. + +You have to register a new hook in your addon, listening on `storage_instance(App $a, array $data)`. +In case `$data['name']` is your storage class name, you have to instance a new instance of your `Friendica\Model\Storage\IStorage` class. +Set the instance of your class as `$data['storage']` to pass it back to the backend. + +This is necessary because it isn't always clear, if you need further construction arguments. ## Adding tests @@ -252,6 +258,14 @@ function samplestorage_unistall() // when the plugin is uninstalled, we unregister the backend. DI::facStorage()->unregister(SampleStorageBackend::class); } + +function samplestorage_storage_instance(\Friendica\App $a, array $data) +{ + if ($data['name'] === SampleStorageBackend::getName()) { + // instance a new sample storage instance and pass it back to the core for usage + $data['storage'] = new SampleStorageBackend(DI::config(), DI::l10n(), DI::cache()); + } +} ``` **Theoretically - until tests for Addons are enabled too - create a test class with the name `addon/tests/SampleStorageTest.php`: diff --git a/doc/Addons.md b/doc/Addons.md index d448b026b..9260ee013 100644 --- a/doc/Addons.md +++ b/doc/Addons.md @@ -706,6 +706,14 @@ Here is a complete list of all hook callbacks with file locations (as of 24-Sep- Hook::callAll('page_header', DI::page()['nav']); Hook::callAll('nav_info', $nav); +### src/Core/Authentication.php + + Hook::callAll('logged_in', $a->user); + +### src/Core/StorageManager + + Hook::callAll('storage_instance', $data); + ### src/Worker/Directory.php Hook::callAll('globaldir_update', $arr); diff --git a/doc/de/Addons.md b/doc/de/Addons.md index 32add2181..37bf114d2 100644 --- a/doc/de/Addons.md +++ b/doc/de/Addons.md @@ -424,6 +424,10 @@ Eine komplette Liste aller Hook-Callbacks mit den zugehörigen Dateien (am 01-Ap ### src/Core/Authentication.php Hook::callAll('logged_in', $a->user); + +### src/Core/StorageManager + + Hook::callAll('storage_instance', $data); ### src/Worker/Directory.php diff --git a/src/Core/StorageManager.php b/src/Core/StorageManager.php index 6a67a06ed..6a8fac5b8 100644 --- a/src/Core/StorageManager.php +++ b/src/Core/StorageManager.php @@ -2,9 +2,9 @@ namespace Friendica\Core; -use Dice\Dice; use Exception; use Friendica\Core\Config\IConfiguration; +use Friendica\Core\L10n\L10n; use Friendica\Database\Database; use Friendica\Model\Storage; use Psr\Log\LoggerInterface; @@ -29,14 +29,19 @@ class StorageManager private $backends = []; + /** + * @var Storage\IStorage[] A local cache for storage instances + */ + private $backendInstances = []; + /** @var Database */ private $dba; /** @var IConfiguration */ private $config; /** @var LoggerInterface */ private $logger; - /** @var Dice */ - private $dice; + /** @var L10n */ + private $l10n; /** @var Storage\IStorage */ private $currentBackend; @@ -45,23 +50,19 @@ class StorageManager * @param Database $dba * @param IConfiguration $config * @param LoggerInterface $logger - * @param Dice $dice + * @param L10n $l10n */ - public function __construct(Database $dba, IConfiguration $config, LoggerInterface $logger, Dice $dice) + public function __construct(Database $dba, IConfiguration $config, LoggerInterface $logger, L10n $l10n) { $this->dba = $dba; $this->config = $config; $this->logger = $logger; - $this->dice = $dice; + $this->l10n = $l10n; $this->backends = $config->get('storage', 'backends', self::DEFAULT_BACKENDS); $currentName = $this->config->get('storage', 'name', ''); - if ($this->isValidBackend($currentName)) { - $this->currentBackend = $this->dice->create($this->backends[$currentName]); - } else { - $this->currentBackend = null; - } + $this->currentBackend = $this->getByName($currentName); } /** @@ -77,41 +78,65 @@ class StorageManager /** * @brief Return storage backend class by registered name * - * @param string|null $name Backend name + * @param string|null $name Backend name + * @param boolean $userBackend Just return instances in case it's a user backend (e.g. not SystemResource) * * @return Storage\IStorage|null null if no backend registered at $name + * + * @throws \Friendica\Network\HTTPException\InternalServerErrorException */ - public function getByName(string $name = null) + public function getByName(string $name = null, $userBackend = true) { - if (!$this->isValidBackend($name) && - $name !== Storage\SystemResource::getName()) { - return null; + // 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, $userBackend)) { + switch ($name) { + // Try the filesystem backend + case Storage\Filesystem::getName(): + $this->backendInstances[$name] = new Storage\Filesystem($this->config, $this->logger, $this->l10n); + break; + // try the database backend + case Storage\Database::getName(): + $this->backendInstances[$name] = new Storage\Database($this->dba, $this->logger, $this->l10n); + break; + // at least, try if there's an addon for the backend + case Storage\SystemResource::getName(): + $this->backendInstances[$name] = new Storage\SystemResource(); + break; + default: + $data = [ + 'name' => $name, + 'storage' => null, + ]; + Hook::callAll('storage_instance', $data); + if (($data['storage'] ?? null) instanceof Storage\IStorage) { + $this->backendInstances[$data['name'] ?? $name] = $data['storage']; + } else { + return null; + } + break; + } + } else { + return null; + } } - /** @var Storage\IStorage $storage */ - $storage = null; - - // If the storage of the file is a system resource, - // create it directly since it isn't listed in the registered backends - if ($name === Storage\SystemResource::getName()) { - $storage = $this->dice->create(Storage\SystemResource::class); - } else { - $storage = $this->dice->create($this->backends[$name]); - } - - return $storage; + return $this->backendInstances[$name]; } /** * Checks, if the storage is a valid backend * - * @param string|null $name The name or class of the backend + * @param string|null $name The name or class of the backend + * @param boolean $userBackend True, if just user backend should get returned (e.g. not SystemResource) * * @return boolean True, if the backend is a valid backend */ - public function isValidBackend(string $name = null) + public function isValidBackend(string $name = null, bool $userBackend = true) { - return array_key_exists($name, $this->backends); + return array_key_exists($name, $this->backends) || + (!$userBackend && $name === Storage\SystemResource::getName()); } /** @@ -128,7 +153,7 @@ class StorageManager } if ($this->config->set('storage', 'name', $name)) { - $this->currentBackend = $this->dice->create($this->backends[$name]); + $this->currentBackend = $this->getByName($name); return true; } else { return false; @@ -146,7 +171,9 @@ class StorageManager } /** - * @brief Register a storage backend class + * Register a storage backend class + * + * You have to register the hook "storage_instance" as well to make this class work! * * @param string $class Backend class name * diff --git a/static/dependencies.config.php b/static/dependencies.config.php index 0bf5a6905..ec80123aa 100644 --- a/static/dependencies.config.php +++ b/static/dependencies.config.php @@ -196,15 +196,7 @@ return [ $_SERVER, $_COOKIE ], ], - StorageManager::class => [ - 'constructParams' => [ - [Dice::INSTANCE => Dice::SELF], - ] - ], IStorage::class => [ - // Don't share this class with other creations, because it's possible to switch the backend - // and so we wouldn't be possible to update it - 'shared' => false, 'instanceOf' => StorageManager::class, 'call' => [ ['getBackend', [], Dice::CHAIN_CALL], diff --git a/tests/Util/SampleStorageBackend.php b/tests/Util/SampleStorageBackend.php index 3f4eb90ad..a788c6d3d 100644 --- a/tests/Util/SampleStorageBackend.php +++ b/tests/Util/SampleStorageBackend.php @@ -2,9 +2,12 @@ namespace Friendica\Test\Util; +use Friendica\App; +use Friendica\Core\Hook; use Friendica\Model\Storage\IStorage; use Friendica\Core\L10n\L10n; +use Mockery\MockInterface; /** * A backend storage example class @@ -91,4 +94,13 @@ class SampleStorageBackend implements IStorage { return self::NAME; } + + /** + * This one is a hack to register this class to the hook + */ + public static function registerHook() + { + Hook::register('storage_instance', __DIR__ . '/SampleStorageBackendInstance.php', 'create_instance'); + } } + diff --git a/tests/Util/SampleStorageBackendInstance.php b/tests/Util/SampleStorageBackendInstance.php new file mode 100644 index 000000000..d55ff04de --- /dev/null +++ b/tests/Util/SampleStorageBackendInstance.php @@ -0,0 +1,18 @@ +setUpVfsDir(); $this->logger = new NullLogger(); - $this->dice = (new Dice()) - ->addRules(include __DIR__ . '/../../../static/dependencies.config.php') - ->addRule(Database::class, ['instanceOf' => StaticDatabase::class, 'shared' => true]) - ->addRule(ISession::class, ['instanceOf' => Session\Memory::class, 'shared' => true, 'call' => null]); $profiler = \Mockery::mock(Profiler::class); $profiler->shouldReceive('saveTimestamp')->withAnyArgs()->andReturn(true); @@ -58,6 +63,8 @@ class StorageManagerTest extends DatabaseTest $configModel = new Config($this->dba); $this->config = new PreloadConfiguration($configCache, $configModel); + + $this->l10n = \Mockery::mock(L10n::class); } /** @@ -65,7 +72,7 @@ class StorageManagerTest extends DatabaseTest */ public function testInstance() { - $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->dice); + $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->l10n); $this->assertInstanceOf(StorageManager::class, $storageManager); } @@ -113,11 +120,11 @@ class StorageManagerTest extends DatabaseTest * * @dataProvider dataStorages */ - public function testGetByName($name, $assert, $assertName) + public function testGetByName($name, $assert, $assertName, $userBackend) { - $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->dice); + $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->l10n); - $storage = $storageManager->getByName($name); + $storage = $storageManager->getByName($name, $userBackend); if (!empty($assert)) { $this->assertInstanceOf(Storage\IStorage::class, $storage); @@ -136,7 +143,7 @@ class StorageManagerTest extends DatabaseTest */ public function testIsValidBackend($name, $assert, $assertName, $userBackend) { - $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->dice); + $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->l10n); $this->assertEquals($userBackend, $storageManager->isValidBackend($name)); } @@ -146,7 +153,7 @@ class StorageManagerTest extends DatabaseTest */ public function testListBackends() { - $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->dice); + $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->l10n); $this->assertEquals(StorageManager::DEFAULT_BACKENDS, $storageManager->listBackends()); } @@ -158,7 +165,7 @@ class StorageManagerTest extends DatabaseTest */ public function testGetBackend($name, $assert, $assertName, $userBackend) { - $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->dice); + $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->l10n); $this->assertNull($storageManager->getBackend()); @@ -178,7 +185,7 @@ class StorageManagerTest extends DatabaseTest { $this->config->set('storage', 'name', $name); - $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->dice); + $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->l10n); if ($userBackend) { $this->assertInstanceOf($assert, $storageManager->getBackend()); @@ -196,17 +203,28 @@ class StorageManagerTest extends DatabaseTest */ public function testRegisterUnregisterBackends() { - $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->dice); + /// @todo Remove dice once "Hook" is dynamic and mockable + $dice = (new Dice()) + ->addRules(include __DIR__ . '/../../../static/dependencies.config.php') + ->addRule(Database::class, ['instanceOf' => StaticDatabase::class, 'shared' => true]) + ->addRule(ISession::class, ['instanceOf' => Session\Memory::class, 'shared' => true, 'call' => null]); + DI::init($dice); + + $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->l10n); $this->assertTrue($storageManager->register(SampleStorageBackend::class)); $this->assertEquals(array_merge(StorageManager::DEFAULT_BACKENDS, [ - 'Sample Storage' => SampleStorageBackend::class, + SampleStorageBackend::getName() => SampleStorageBackend::class, ]), $storageManager->listBackends()); $this->assertEquals(array_merge(StorageManager::DEFAULT_BACKENDS, [ - 'Sample Storage' => SampleStorageBackend::class, + SampleStorageBackend::getName() => SampleStorageBackend::class, ]), $this->config->get('storage', 'backends')); + // inline call to register own class as hook (testing purpose only) + SampleStorageBackend::registerHook(); + Hook::loadHooks(); + $this->assertTrue($storageManager->setBackend(SampleStorageBackend::NAME)); $this->assertEquals(SampleStorageBackend::NAME, $this->config->get('storage', 'name')); @@ -233,7 +251,7 @@ class StorageManagerTest extends DatabaseTest $this->loadFixture(__DIR__ . '/../../datasets/storage/database.fixture.php', $this->dba); - $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->dice); + $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->l10n); $storage = $storageManager->getByName($name); $storageManager->move($storage); From 016cfcd84648d8634537822627e370a98005b9ff Mon Sep 17 00:00:00 2001 From: nupplaPhil Date: Thu, 9 Jan 2020 01:48:48 +0100 Subject: [PATCH 13/14] Fixings - Rename "facStorage" to "storageManager" - Fix indentation - Fix tests --- doc/AddonStorageBackend.md | 6 +-- src/DI.php | 66 +++++++++++++-------------- src/Model/Attach.php | 4 +- src/Model/Photo.php | 8 ++-- src/Module/Admin/Site.php | 4 +- src/Worker/CronJobs.php | 2 +- tests/src/Core/StorageManagerTest.php | 1 - 7 files changed, 45 insertions(+), 46 deletions(-) diff --git a/doc/AddonStorageBackend.md b/doc/AddonStorageBackend.md index ef1b4454b..f69dfff45 100644 --- a/doc/AddonStorageBackend.md +++ b/doc/AddonStorageBackend.md @@ -23,7 +23,7 @@ interface IStorage public function getOptions(); public function saveOptions(array $data); public function __toString(); - public static function getName(); + public static function getName(); } ``` @@ -250,13 +250,13 @@ function samplestorage_install() // on addon install, we register our class with name "Sample Storage". // note: we use `::class` property, which returns full class name as string // this save us the problem of correctly escape backslashes in class name - DI::facStorage()->register(SampleStorageBackend::class); + DI::storageManager()->register(SampleStorageBackend::class); } function samplestorage_unistall() { // when the plugin is uninstalled, we unregister the backend. - DI::facStorage()->unregister(SampleStorageBackend::class); + DI::storageManager()->unregister(SampleStorageBackend::class); } function samplestorage_storage_instance(\Friendica\App $a, array $data) diff --git a/src/DI.php b/src/DI.php index 152e705a7..b7be36212 100644 --- a/src/DI.php +++ b/src/DI.php @@ -27,7 +27,7 @@ use Psr\Log\LoggerInterface; * @method static Core\L10n\L10n l10n() * @method static Core\Process process() * @method static Core\Session\ISession session() - * @method static Core\StorageManager facStorage() + * @method static Core\StorageManager storageManager() * @method static Database\Database dba() * @method static Factory\Mastodon\Account mstdnAccount() * @method static Factory\Mastodon\FollowRequest mstdnFollowRequest() @@ -49,40 +49,40 @@ use Psr\Log\LoggerInterface; abstract class DI { const CLASS_MAPPING = [ - 'app' => App::class, - 'auth' => App\Authentication::class, - 'args' => App\Arguments::class, - 'baseUrl' => App\BaseURL::class, - 'mode' => App\Mode::class, - 'module' => App\Module::class, - 'page' => App\Page::class, - 'router' => App\Router::class, - 'contentItem' => Content\Item::class, - 'bbCodeVideo' => Content\Text\BBCode\Video::class, - 'cache' => Core\Cache\ICache::class, - 'config' => Core\Config\IConfiguration::class, - 'pConfig' => Core\Config\IPConfiguration::class, - 'l10n' => Core\L10n\L10n::class, - 'lock' => Core\Lock\ILock::class, - 'process' => Core\Process::class, - 'session' => Core\Session\ISession::class, - 'facStorage' => Core\StorageManager::class, - 'dba' => Database\Database::class, - 'mstdnAccount' => Factory\Mastodon\Account::class, + 'app' => App::class, + 'auth' => App\Authentication::class, + 'args' => App\Arguments::class, + 'baseUrl' => App\BaseURL::class, + 'mode' => App\Mode::class, + 'module' => App\Module::class, + 'page' => App\Page::class, + 'router' => App\Router::class, + 'contentItem' => Content\Item::class, + 'bbCodeVideo' => Content\Text\BBCode\Video::class, + 'cache' => Core\Cache\ICache::class, + 'config' => Core\Config\IConfiguration::class, + 'pConfig' => Core\Config\IPConfiguration::class, + 'l10n' => Core\L10n\L10n::class, + 'lock' => Core\Lock\ILock::class, + 'process' => Core\Process::class, + 'session' => Core\Session\ISession::class, + 'storageManager' => Core\StorageManager::class, + 'dba' => Database\Database::class, + 'mstdnAccount' => Factory\Mastodon\Account::class, 'mstdnFollowRequest' => Factory\Mastodon\FollowRequest::class, 'mstdnRelationship' => Factory\Mastodon\Relationship::class, - 'cookie' => Model\User\Cookie::class, - 'notify' => Model\Notify::class, - 'storage' => Model\Storage\IStorage::class, - 'intro' => Repository\Introduction::class, - 'activity' => Protocol\Activity::class, - 'aclFormatter' => Util\ACLFormatter::class, - 'dtFormat' => Util\DateTimeFormat::class, - 'fs' => Util\FileSystem::class, - 'workerLogger' => Util\Logger\WorkerLogger::class, - 'profiler' => Util\Profiler::class, - 'logger' => LoggerInterface::class, - 'devLogger' => '$devLogger', + 'cookie' => Model\User\Cookie::class, + 'notify' => Model\Notify::class, + 'storage' => Model\Storage\IStorage::class, + 'intro' => Repository\Introduction::class, + 'activity' => Protocol\Activity::class, + 'aclFormatter' => Util\ACLFormatter::class, + 'dtFormat' => Util\DateTimeFormat::class, + 'fs' => Util\FileSystem::class, + 'workerLogger' => Util\Logger\WorkerLogger::class, + 'profiler' => Util\Profiler::class, + 'logger' => LoggerInterface::class, + 'devLogger' => '$devLogger', ]; /** @var Dice */ diff --git a/src/Model/Attach.php b/src/Model/Attach.php index 7a0446176..e98f80a91 100644 --- a/src/Model/Attach.php +++ b/src/Model/Attach.php @@ -259,7 +259,7 @@ class Attach $items = self::selectToArray(['backend-class','backend-ref'], $conditions); foreach($items as $item) { - $backend_class = DI::facStorage()->getByName($item['backend-class'] ?? ''); + $backend_class = DI::storageManager()->getByName($item['backend-class'] ?? ''); if ($backend_class !== '') { $fields['backend-ref'] = $backend_class->put($img->asString(), $item['backend-ref'] ?? ''); } else { @@ -291,7 +291,7 @@ class Attach $items = self::selectToArray(['backend-class','backend-ref'], $conditions); foreach($items as $item) { - $backend_class = DI::facStorage()->getByName($item['backend-class'] ?? ''); + $backend_class = DI::storageManager()->getByName($item['backend-class'] ?? ''); if ($backend_class !== null) { $backend_class->delete($item['backend-ref'] ?? ''); } diff --git a/src/Model/Photo.php b/src/Model/Photo.php index b13aaa79a..9e2671140 100644 --- a/src/Model/Photo.php +++ b/src/Model/Photo.php @@ -179,7 +179,7 @@ class Photo } $data = $i['data']; } else { - $backendClass = DI::facStorage()->getByName($photo['backend-class'] ?? ''); + $backendClass = DI::storageManager()->getByName($photo['backend-class'] ?? ''); $backendRef = $photo['backend-ref'] ?? ''; $data = $backendClass->get($backendRef); } @@ -272,7 +272,7 @@ class Photo if (DBA::isResult($existing_photo)) { $backend_ref = (string)$existing_photo["backend-ref"]; - $storage = DI::facStorage()->getByName($existing_photo["backend-class"] ?? ''); + $storage = DI::storageManager()->getByName($existing_photo["backend-class"] ?? ''); } else { $storage = DI::storage(); } @@ -336,7 +336,7 @@ class Photo $photos = self::selectToArray(['backend-class', 'backend-ref'], $conditions); foreach($photos as $photo) { - $backend_class = DI::facStorage()->getByName($photo['backend-class'] ?? ''); + $backend_class = DI::storageManager()->getByName($photo['backend-class'] ?? ''); if ($backend_class !== null) { $backend_class->delete($photo["backend-ref"] ?? ''); } @@ -365,7 +365,7 @@ class Photo $photos = self::selectToArray(['backend-class', 'backend-ref'], $conditions); foreach($photos as $photo) { - $backend_class = DI::facStorage()->getByName($photo['backend-class'] ?? ''); + $backend_class = DI::storageManager()->getByName($photo['backend-class'] ?? ''); if ($backend_class !== null) { $fields["backend-ref"] = $backend_class->put($img->asString(), $photo['backend-ref']); } else { diff --git a/src/Module/Admin/Site.php b/src/Module/Admin/Site.php index e68f37953..433cddf6f 100644 --- a/src/Module/Admin/Site.php +++ b/src/Module/Admin/Site.php @@ -202,7 +202,7 @@ class Site extends BaseAdminModule $storagebackend = Strings::escapeTags(trim($_POST['storagebackend'] ?? '')); // save storage backend form - if (DI::facStorage()->setBackend($storagebackend)) { + if (DI::storageManager()->setBackend($storagebackend)) { $storage_opts = DI::storage()->getOptions(); $storage_form_prefix = preg_replace('|[^a-zA-Z0-9]|', '', $storagebackend); $storage_opts_data = []; @@ -534,7 +534,7 @@ class Site extends BaseAdminModule $available_storage_backends[''] = L10n::t('Database (legacy)'); } - foreach (DI::facStorage()->listBackends() as $name => $class) { + foreach (DI::storageManager()->listBackends() as $name => $class) { $available_storage_backends[$name] = $name; } diff --git a/src/Worker/CronJobs.php b/src/Worker/CronJobs.php index 6fa09197b..6a8d65464 100644 --- a/src/Worker/CronJobs.php +++ b/src/Worker/CronJobs.php @@ -325,7 +325,7 @@ class CronJobs private static function moveStorage() { $current = DI::storage(); - $moved = DI::facStorage()->move($current); + $moved = DI::storageManager()->move($current); if ($moved) { Worker::add(PRIORITY_LOW, "CronJobs", "move_storage"); diff --git a/tests/src/Core/StorageManagerTest.php b/tests/src/Core/StorageManagerTest.php index 2d131c632..083d12d57 100644 --- a/tests/src/Core/StorageManagerTest.php +++ b/tests/src/Core/StorageManagerTest.php @@ -28,7 +28,6 @@ use Friendica\Test\Util\SampleStorageBackend; * @todo Rework Hook:: methods to dynamic to remove the separated process annotation * * @runTestsInSeparateProcesses - * @preserveGlobalState disabled */ class StorageManagerTest extends DatabaseTest { From d3ab1db29a5a4a31ab6dd0daaebb6d0c4239abce Mon Sep 17 00:00:00 2001 From: nupplaPhil Date: Fri, 10 Jan 2020 13:11:32 +0100 Subject: [PATCH 14/14] Remove annotations and fix test.. --- tests/src/Core/StorageManagerTest.php | 5 ----- 1 file changed, 5 deletions(-) diff --git a/tests/src/Core/StorageManagerTest.php b/tests/src/Core/StorageManagerTest.php index 083d12d57..443f8e786 100644 --- a/tests/src/Core/StorageManagerTest.php +++ b/tests/src/Core/StorageManagerTest.php @@ -24,11 +24,6 @@ use Psr\Log\LoggerInterface; use Psr\Log\NullLogger; use Friendica\Test\Util\SampleStorageBackend; -/** - * @todo Rework Hook:: methods to dynamic to remove the separated process annotation - * - * @runTestsInSeparateProcesses - */ class StorageManagerTest extends DatabaseTest { /** @var Database */