refactor(cache-storage): Make Storage and StorageGlobal psalm strict

Signed-off-by: Carl Schwan <carlschwan@kde.org>

Author: CarlSchwan

apps/files_external/lib/Service/StoragesService.php

@@ -454,29 +454,6 @@ public function removeStorage(int $id) {
 		$this->updateOverwriteHomeFolders();
 	}
 
-	/**
-	 * Construct the storage implementation
-	 *
-	 * @param StorageConfig $storageConfig
-	 * @return int
-	 */
-	private function getStorageId(StorageConfig $storageConfig) {
-		try {
-			$class = $storageConfig->getBackend()->getStorageClass();
-			/** @var \OC\Files\Storage\Storage $storage */
-			$storage = new $class($storageConfig->getBackendOptions());
-
-			// auth mechanism should fire first
-			$storage = $storageConfig->getBackend()->wrapStorage($storage);
-			$storage = $storageConfig->getAuthMechanism()->wrapStorage($storage);
-
-			/** @var \OC\Files\Storage\Storage $storage */
-			return $storage->getStorageCache()->getNumericId();
-		} catch (\Exception $e) {
-			return -1;
-		}
-	}
-
 	public function updateOverwriteHomeFolders(): void {
 		$appIdsList = $this->appConfig->getValueArray(FilesApplication::APP_ID, ConfigLexicon::OVERWRITES_HOME_FOLDERS);
 

build/psalm-baseline.xml

@@ -3480,14 +3480,6 @@
       <code><![CDATA[self::SCAN_RECURSIVE_INCOMPLETE]]></code>
     </InvalidArgument>
   </file>
-  <file src="lib/private/Files/Cache/Storage.php">
-    <InvalidNullableReturnType>
-      <code><![CDATA[array]]></code>
-    </InvalidNullableReturnType>
-    <NullableReturnStatement>
-      <code><![CDATA[self::getGlobalCache()->getStorageInfo($storageId)]]></code>
-    </NullableReturnStatement>
-  </file>
   <file src="lib/private/Files/Config/MountProviderCollection.php">
     <InvalidOperand>
       <code><![CDATA[$user]]></code>

build/rector-strict.php

@@ -18,6 +18,8 @@
 		$nextcloudDir . '/lib/private/Settings/AuthorizedGroupMapper.php',
 		$nextcloudDir . '/apps/settings/lib/Service/AuthorizedGroupService.php',
 		$nextcloudDir . '/lib/private/Files/Storage/Storage.php',
+		$nextcloudDir . '/lib/private/Files/Cache/Storage.php',
+		$nextcloudDir . '/lib/private/Files/Cache/StorageGlobal.php',
 		$nextcloudDir . '/lib/private/Files/Storage/Wrapper/Wrapper.php',
 		$nextcloudDir . '/build/psalm/ITypedQueryBuilderTest.php',
 		$nextcloudDir . '/lib/private/DB/QueryBuilder/TypedQueryBuilder.php',

lib/private/Files/Cache/Storage.php

@@ -1,12 +1,15 @@
 <?php
 
+declare(strict_types=1);
+
 /**
  * SPDX-FileCopyrightText: 2016-2024 Nextcloud GmbH and Nextcloud contributors
  * SPDX-FileCopyrightText: 2016 ownCloud, Inc.
  * SPDX-License-Identifier: AGPL-3.0-only
  */
 namespace OC\Files\Cache;
 
+use OC\DB\Exceptions\DbalException;
 use OCP\DB\QueryBuilder\IQueryBuilder;
 use OCP\Files\Storage\IStorage;
 use OCP\IDBConnection;
@@ -25,55 +28,71 @@
  * @package OC\Files\Cache
  */
 class Storage {
-	/** @var StorageGlobal|null */
-	private static $globalCache = null;
-	private $storageId;
-	private $numericId;
+	private static ?StorageGlobal $globalCache = null;
 
-	/**
-	 * @return StorageGlobal
-	 */
-	public static function getGlobalCache() {
+	private string $storageId;
+
+	private int $numericId;
+
+	public static function getGlobalCache(): StorageGlobal {
 		if (is_null(self::$globalCache)) {
 			self::$globalCache = new StorageGlobal(Server::get(IDBConnection::class));
 		}
+
 		return self::$globalCache;
 	}
 
 	/**
-	 * @param \OC\Files\Storage\Storage|string $storage
-	 * @param bool $isAvailable
 	 * @throws \RuntimeException
 	 */
-	public function __construct($storage, $isAvailable, IDBConnection $connection) {
-		if ($storage instanceof IStorage) {
-			$this->storageId = $storage->getId();
-		} else {
-			$this->storageId = $storage;
-		}
-		$this->storageId = self::adjustStorageId($this->storageId);
+	public function __construct(
+		IStorage|string $storage,
+		bool $isAvailable,
+		IDBConnection $connection,
+	) {
+		$this->storageId = $storage instanceof IStorage ? $storage->getId() : $storage;
+
+        $this->storageId = self::adjustStorageId($this->storageId);
 
 		if ($row = self::getStorageById($this->storageId)) {
-			$this->numericId = (int)$row['numeric_id'];
+			$this->numericId = $row['numeric_id'];
 		} else {
 			$available = $isAvailable ? 1 : 0;
-			if ($connection->insertIfNotExist('*PREFIX*storages', ['id' => $this->storageId, 'available' => $available])) {
-				$this->numericId = $connection->lastInsertId('*PREFIX*storages');
-			} else {
-				if ($row = self::getStorageById($this->storageId)) {
-					$this->numericId = (int)$row['numeric_id'];
-				} else {
-					throw new \RuntimeException('Storage could neither be inserted nor be selected from the database: ' . $this->storageId);
+
+			try {
+				$qb = $connection->getQueryBuilder();
+				$qb->insert('storages')
+					->values([
+						'id' => $qb->createNamedParameter($this->storageId),
+						'available' => $qb->createNamedParameter($available),
+					])
+					->executeStatement();
+				$this->numericId = $qb->getLastInsertId();
+			} catch (DbalException $e) {
+				if ($e->getReason() === DbalException::REASON_UNIQUE_CONSTRAINT_VIOLATION) {
+					$qb = $connection->getQueryBuilder();
+					$result = $qb->select('numeric_id')
+						->from('storage')
+						->where($qb->expr()->eq('id', $qb->createNamedParameter($this->storageId)))
+						->executeQuery();
+					/** @var false|string|int $row */
+					$row = $result->fetchOne();
+					if ($row === false) {
+						throw new \RuntimeException('Storage could neither be inserted nor be selected from the database: ' . $this->storageId, $e->getCode(), $e);
+					}
+
+					$this->numericId = (int)$row;
 				}
+
+				throw $e;
 			}
 		}
 	}
 
 	/**
-	 * @param string $storageId
-	 * @return array
-	 */
-	public static function getStorageById($storageId) {
+     * @return array{id: string, numeric_id: int, available: bool, last_checked: int}|null
+     */
+    public static function getStorageById(string $storageId): ?array {
 		return self::getGlobalCache()->getStorageInfo($storageId);
 	}
 
@@ -83,71 +102,52 @@ public static function getStorageById($storageId) {
 	 * @return string unchanged $storageId if its length is less than 64 characters,
 	 *                else returns the md5 of $storageId
 	 */
-	public static function adjustStorageId($storageId) {
+	public static function adjustStorageId(string $storageId): string {
 		if (strlen($storageId) > 64) {
 			return md5($storageId);
 		}
+
 		return $storageId;
 	}
 
 	/**
 	 * Get the numeric id for the storage
-	 *
-	 * @return int
 	 */
-	public function getNumericId() {
+	public function getNumericId(): int {
 		return $this->numericId;
 	}
 
 	/**
-	 * Get the string id for the storage
-	 *
-	 * @param int $numericId
-	 * @return string|null either the storage id string or null if the numeric id is not known
-	 */
-	public static function getStorageId(int $numericId): ?string {
+     * Get the string id for the storage
+     *
+     * @return string|null either the storage id string or null if the numeric id is not known
+     */
+    public static function getStorageId(int $numericId): ?string {
 		$storage = self::getGlobalCache()->getStorageInfoByNumericId($numericId);
 		return $storage['id'] ?? null;
 	}
 
-	/**
-	 * Get the numeric of the storage with the provided string id
-	 *
-	 * @param $storageId
-	 * @return int|null either the numeric storage id or null if the storage id is not known
-	 */
-	public static function getNumericStorageId($storageId) {
-		$storageId = self::adjustStorageId($storageId);
-
-		if ($row = self::getStorageById($storageId)) {
-			return (int)$row['numeric_id'];
-		} else {
-			return null;
-		}
-	}
-
 	/**
 	 * @return array{available: bool, last_checked: int}
 	 */
-	public function getAvailability() {
+	public function getAvailability(): array {
 		if ($row = self::getStorageById($this->storageId)) {
 			return [
 				'available' => (int)$row['available'] === 1,
 				'last_checked' => $row['last_checked']
 			];
-		} else {
-			return [
+		}
+
+        return [
 				'available' => true,
 				'last_checked' => time(),
 			];
-		}
 	}
 
 	/**
-	 * @param bool $isAvailable
 	 * @param int $delay amount of seconds to delay reconsidering that storage further
 	 */
-	public function setAvailability($isAvailable, int $delay = 0) {
+	public function setAvailability(bool $isAvailable, int $delay = 0): void {
 		$available = $isAvailable ? 1 : 0;
 		if (!$isAvailable) {
 			Server::get(LoggerInterface::class)->info('Storage with ' . $this->storageId . ' marked as unavailable', ['app' => 'lib']);
@@ -162,43 +162,9 @@ public function setAvailability($isAvailable, int $delay = 0) {
 	}
 
 	/**
-	 * Check if a string storage id is known
-	 *
-	 * @param string $storageId
-	 * @return bool
-	 */
-	public static function exists($storageId) {
-		return !is_null(self::getNumericStorageId($storageId));
-	}
-
-	/**
-	 * remove the entry for the storage
-	 *
-	 * @param string $storageId
-	 */
-	public static function remove($storageId) {
-		$storageId = self::adjustStorageId($storageId);
-		$numericId = self::getNumericStorageId($storageId);
-
-		$query = Server::get(IDBConnection::class)->getQueryBuilder();
-		$query->delete('storages')
-			->where($query->expr()->eq('id', $query->createNamedParameter($storageId)));
-		$query->executeStatement();
-
-		if (!is_null($numericId)) {
-			$query = Server::get(IDBConnection::class)->getQueryBuilder();
-			$query->delete('filecache')
-				->where($query->expr()->eq('storage', $query->createNamedParameter($numericId)));
-			$query->executeStatement();
-		}
-	}
-
-	/**
-	 * remove the entry for the storage by the mount id
-	 *
-	 * @param int $mountId
+	 * Remove the entry for the storage by the mount id.
 	 */
-	public static function cleanByMountId(int $mountId) {
+	public static function cleanByMountId(int $mountId): void {
 		$db = Server::get(IDBConnection::class);
 
 		try {
@@ -213,24 +179,24 @@ public static function cleanByMountId(int $mountId) {
 
 			$query = $db->getQueryBuilder();
 			$query->delete('filecache')
-				->where($query->expr()->in('storage', $query->createNamedParameter($storageIds, IQueryBuilder::PARAM_INT_ARRAY)));
-			$query->runAcrossAllShards();
-			$query->executeStatement();
+				->where($query->expr()->in('storage', $query->createNamedParameter($storageIds, IQueryBuilder::PARAM_INT_ARRAY)))
+				->runAcrossAllShards()
+				->executeStatement();
 
 			$query = $db->getQueryBuilder();
 			$query->delete('storages')
-				->where($query->expr()->in('numeric_id', $query->createNamedParameter($storageIds, IQueryBuilder::PARAM_INT_ARRAY)));
-			$query->executeStatement();
+				->where($query->expr()->in('numeric_id', $query->createNamedParameter($storageIds, IQueryBuilder::PARAM_INT_ARRAY)))
+				->executeStatement();
 
 			$query = $db->getQueryBuilder();
 			$query->delete('mounts')
-				->where($query->expr()->eq('mount_id', $query->createNamedParameter($mountId, IQueryBuilder::PARAM_INT)));
-			$query->executeStatement();
+				->where($query->expr()->eq('mount_id', $query->createNamedParameter($mountId, IQueryBuilder::PARAM_INT)))
+				->executeStatement();
 
 			$db->commit();
-		} catch (\Exception $e) {
+		} catch (\Exception $exception) {
 			$db->rollBack();
-			throw $e;
+			throw $exception;
 		}
 	}
 }