Fix Storage Exceptions

src/Model/Storage/Database.php

@@ -57,9 +57,13 @@ class Database implements ISelectableStorage
 
 			return $result['data'];
 		} catch (Exception $exception) {
+			if ($exception instanceof ReferenceStorageException) {
+				throw $exception;
+			} else {
 				throw new StorageException(sprintf('Database storage failed to get %s', $reference), $exception->getCode(), $exception);
 			}
 		}
+	}
 
 	/**
 	 * @inheritDoc
@@ -101,9 +105,13 @@ class Database implements ISelectableStorage
 				throw new ReferenceStorageException(sprintf('Database storage failed to delete %s', $reference));
 			}
 		} catch (Exception $exception) {
+			if ($exception instanceof ReferenceStorageException) {
+				throw $exception;
+			} else {
 				throw new StorageException(sprintf('Database storage failed to delete %s', $reference), $exception->getCode(), $exception);
 			}
 		}
+	}
 
 	/**
 	 * @inheritDoc

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');
 	}
 }
-

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(\TypeError::class);
-		$this->expectException(StorageException::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);
 	}

tests/src/Model/Storage/DatabaseStorageTest.php

@@ -62,10 +62,7 @@ class DatabaseStorageTest extends StorageTest
 
 		$dba = new StaticDatabase($configCache, $profiler, $logger);
 
-		/** @var MockInterface|L10n $l10n */
+		return new Database($dba);
-		$l10n = \Mockery::mock(L10n::class)->makePartial();
-
-		return new Database($dba, $logger, $l10n);
 	}
 
 	protected function assertOption(IStorage $storage)

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)

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"
+		$instance->delete(-1234456);
-		self::assertTrue($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
+		$instance->get(-123456);
-		self::assertEmpty($instance->get(-123456));
 	}
 
 	/**