From 90c99520bb2dd4817799da81e39f0043eefb171c Mon Sep 17 00:00:00 2001 From: Philipp Date: Sun, 1 Aug 2021 14:31:57 +0200 Subject: [PATCH] Fix Storage Exceptions --- src/Model/Storage/Database.php | 12 ++++++-- tests/Util/SampleStorageBackend.php | 17 +++++------ tests/src/Core/StorageManagerTest.php | 30 +++++++++++-------- .../src/Model/Storage/DatabaseStorageTest.php | 5 +--- .../Model/Storage/FilesystemStorageTest.php | 3 +- tests/src/Model/Storage/StorageTest.php | 14 +++++---- 6 files changed, 46 insertions(+), 35 deletions(-) diff --git a/src/Model/Storage/Database.php b/src/Model/Storage/Database.php index 5e83ca8568..9edfe21708 100644 --- a/src/Model/Storage/Database.php +++ b/src/Model/Storage/Database.php @@ -57,7 +57,11 @@ class Database implements ISelectableStorage return $result['data']; } catch (Exception $exception) { - throw new StorageException(sprintf('Database storage failed to get %s', $reference), $exception->getCode(), $exception); + if ($exception instanceof ReferenceStorageException) { + throw $exception; + } else { + throw new StorageException(sprintf('Database storage failed to get %s', $reference), $exception->getCode(), $exception); + } } } @@ -101,7 +105,11 @@ class Database implements ISelectableStorage throw new ReferenceStorageException(sprintf('Database storage failed to delete %s', $reference)); } } catch (Exception $exception) { - throw new StorageException(sprintf('Database storage failed to delete %s', $reference), $exception->getCode(), $exception); + if ($exception instanceof ReferenceStorageException) { + throw $exception; + } else { + throw new StorageException(sprintf('Database storage failed to delete %s', $reference), $exception->getCode(), $exception); + } } } diff --git a/tests/Util/SampleStorageBackend.php b/tests/Util/SampleStorageBackend.php index 3d5789b0de..1ebd64f6ee 100644 --- a/tests/Util/SampleStorageBackend.php +++ b/tests/Util/SampleStorageBackend.php @@ -22,14 +22,14 @@ namespace Friendica\Test\Util; use Friendica\Core\Hook; -use Friendica\Model\Storage\IStorage; +use Friendica\Model\Storage\ISelectableStorage; use Friendica\Core\L10n; /** * A backend storage example class */ -class SampleStorageBackend implements IStorage +class SampleStorageBackend implements ISelectableStorage { const NAME = 'Sample Storage'; @@ -62,14 +62,14 @@ class SampleStorageBackend implements IStorage $this->l10n = $l10n; } - public function get(string $reference) + public function get(string $reference): string { // we return always the same image data. Which file we load is defined by // a config key - return $this->data[$reference] ?? null; + return $this->data[$reference] ?? ''; } - public function put(string $data, string $reference = '') + public function put(string $data, string $reference = ''): string { if ($reference === '') { $reference = 'sample'; @@ -89,12 +89,12 @@ class SampleStorageBackend implements IStorage return true; } - public function getOptions() + public function getOptions(): array { return $this->options; } - public function saveOptions(array $data) + public function saveOptions(array $data): array { $this->options = $data; @@ -107,7 +107,7 @@ class SampleStorageBackend implements IStorage return self::NAME; } - public static function getName() + public static function getName(): string { return self::NAME; } @@ -120,4 +120,3 @@ class SampleStorageBackend implements IStorage Hook::register('storage_instance', __DIR__ . '/SampleStorageBackendInstance.php', 'create_instance'); } } - diff --git a/tests/src/Core/StorageManagerTest.php b/tests/src/Core/StorageManagerTest.php index 537fd841e4..f01640dce4 100644 --- a/tests/src/Core/StorageManagerTest.php +++ b/tests/src/Core/StorageManagerTest.php @@ -177,7 +177,11 @@ class StorageManagerTest extends DatabaseTest { $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->l10n, $this->httpRequest); - $storage = $storageManager->getByName($name, $userBackend); + if ($userBackend) { + $storage = $storageManager->getSelectableStorageByName($name); + } else { + $storage = $storageManager->getByName($name); + } if (!empty($assert)) { self::assertInstanceOf(Storage\IStorage::class, $storage); @@ -195,7 +199,7 @@ class StorageManagerTest extends DatabaseTest */ public function testIsValidBackend($name, $assert, $assertName, $userBackend) { - $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->l10n, $this->httpRequest); + $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->l10n); // true in every of the backends self::assertEquals(!empty($assertName), $storageManager->isValidBackend($name)); @@ -209,7 +213,7 @@ class StorageManagerTest extends DatabaseTest */ public function testListBackends() { - $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->l10n, $this->httpRequest); + $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->l10n); self::assertEquals(StorageManager::DEFAULT_BACKENDS, $storageManager->listBackends()); } @@ -221,12 +225,13 @@ class StorageManagerTest extends DatabaseTest */ public function testGetBackend($name, $assert, $assertName, $userBackend) { - $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->l10n, $this->httpRequest); + $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->l10n); self::assertNull($storageManager->getBackend()); if ($userBackend) { - $storageManager->setBackend($name); + $selBackend = $storageManager->getSelectableStorageByName($name); + $storageManager->setBackend($selBackend); self::assertInstanceOf($assert, $storageManager->getBackend()); } @@ -242,7 +247,7 @@ class StorageManagerTest extends DatabaseTest { $this->config->set('storage', 'name', $name); - $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->l10n, $this->httpRequest); + $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->l10n); if ($userBackend) { self::assertInstanceOf($assert, $storageManager->getBackend()); @@ -267,7 +272,7 @@ class StorageManagerTest extends DatabaseTest ->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->httpRequest); + $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->l10n); self::assertTrue($storageManager->register(SampleStorageBackend::class)); @@ -282,7 +287,7 @@ class StorageManagerTest extends DatabaseTest SampleStorageBackend::registerHook(); Hook::loadHooks(); - self::assertTrue($storageManager->setBackend(SampleStorageBackend::NAME)); + self::assertTrue($storageManager->setBackend( $storageManager->getSelectableStorageByName(SampleStorageBackend::NAME))); self::assertEquals(SampleStorageBackend::NAME, $this->config->get('storage', 'name')); self::assertInstanceOf(SampleStorageBackend::class, $storageManager->getBackend()); @@ -308,7 +313,7 @@ class StorageManagerTest extends DatabaseTest $this->loadFixture(__DIR__ . '/../../datasets/storage/database.fixture.php', $this->dba); - $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->l10n, $this->httpRequest); + $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->l10n); $storage = $storageManager->getSelectableStorageByName($name); $storageManager->move($storage); @@ -328,12 +333,11 @@ class StorageManagerTest extends DatabaseTest /** * Test moving data to a WRONG storage */ - public function testMoveStorageWrong() + public function testWrongSelectableStorage() { - $this->expectExceptionMessage("Can't move to storage backend 'SystemResource'"); - $this->expectException(StorageException::class); + $this->expectException(\TypeError::class); - $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->l10n, $this->httpRequest); + $storageManager = new StorageManager($this->dba, $this->config, $this->logger, $this->l10n); $storage = $storageManager->getSelectableStorageByName(Storage\SystemResource::getName()); $storageManager->move($storage); } diff --git a/tests/src/Model/Storage/DatabaseStorageTest.php b/tests/src/Model/Storage/DatabaseStorageTest.php index 21d6e18652..5e61ea13a4 100644 --- a/tests/src/Model/Storage/DatabaseStorageTest.php +++ b/tests/src/Model/Storage/DatabaseStorageTest.php @@ -62,10 +62,7 @@ class DatabaseStorageTest extends StorageTest $dba = new StaticDatabase($configCache, $profiler, $logger); - /** @var MockInterface|L10n $l10n */ - $l10n = \Mockery::mock(L10n::class)->makePartial(); - - return new Database($dba, $logger, $l10n); + return new Database($dba); } protected function assertOption(IStorage $storage) diff --git a/tests/src/Model/Storage/FilesystemStorageTest.php b/tests/src/Model/Storage/FilesystemStorageTest.php index ebe29f0c4c..3319ea3fa4 100644 --- a/tests/src/Model/Storage/FilesystemStorageTest.php +++ b/tests/src/Model/Storage/FilesystemStorageTest.php @@ -50,7 +50,6 @@ class FilesystemStorageTest extends StorageTest protected function getInstance() { - $logger = new NullLogger(); $profiler = \Mockery::mock(Profiler::class); $profiler->shouldReceive('startRecording'); $profiler->shouldReceive('stopRecording'); @@ -63,7 +62,7 @@ class FilesystemStorageTest extends StorageTest ->with('storage', 'filesystem_path', Filesystem::DEFAULT_BASE_FOLDER) ->andReturn($this->root->getChild('storage')->url()); - return new Filesystem($this->config, $logger, $l10n); + return new Filesystem($this->config, $l10n); } protected function assertOption(IStorage $storage) diff --git a/tests/src/Model/Storage/StorageTest.php b/tests/src/Model/Storage/StorageTest.php index d978f013d9..eadddcf02d 100644 --- a/tests/src/Model/Storage/StorageTest.php +++ b/tests/src/Model/Storage/StorageTest.php @@ -22,6 +22,8 @@ namespace Friendica\Test\src\Model\Storage; use Friendica\Model\Storage\IStorage; +use Friendica\Model\Storage\ReferenceStorageException; +use Friendica\Model\Storage\StorageException; use Friendica\Test\MockedTest; abstract class StorageTest extends MockedTest @@ -62,7 +64,7 @@ abstract class StorageTest extends MockedTest self::assertEquals('data12345', $instance->get($ref)); - self::assertTrue($instance->delete($ref)); + $instance->delete($ref); } /** @@ -70,10 +72,11 @@ abstract class StorageTest extends MockedTest */ public function testInvalidDelete() { + self::expectException(ReferenceStorageException::class); + $instance = $this->getInstance(); - // Even deleting not existing references should return "true" - self::assertTrue($instance->delete(-1234456)); + $instance->delete(-1234456); } /** @@ -81,10 +84,11 @@ abstract class StorageTest extends MockedTest */ public function testInvalidGet() { + self::expectException(ReferenceStorageException::class); + $instance = $this->getInstance(); - // Invalid references return an empty string - self::assertEmpty($instance->get(-123456)); + $instance->get(-123456); } /**