Merge pull request #48111 from nextcloud/fix/move-storage-constructor…

…-to-specific-interface fix: Move storage constructor to specific interface
nextcloud · Sep 23, 2024 · 40b404c · 40b404c
2 parents 8927510 + b8ab560
commit 40b404c
Show file tree

Hide file tree

Showing 8 changed files with 41 additions and 10 deletions.
diff --git a/apps/files_external/lib/Config/ConfigAdapter.php b/apps/files_external/lib/Config/ConfigAdapter.php
@@ -15,11 +15,13 @@
 use OCA\Files_External\Service\UserStoragesService;
 use OCP\Files\Config\IMountProvider;
 use OCP\Files\ObjectStore\IObjectStore;
+use OCP\Files\Storage\IConstructableStorage;
 use OCP\Files\Storage\IStorage;
 use OCP\Files\Storage\IStorageFactory;
 use OCP\Files\StorageNotAvailableException;
 use OCP\IUser;
 use Psr\Clock\ClockInterface;
+use Psr\Log\LoggerInterface;
 
 /**
  * Make the old files_external config work with the new public mount config api
@@ -62,6 +64,9 @@ private function prepareStorageConfig(StorageConfig &$storage, IUser $user): voi
 	 */
 	private function constructStorage(StorageConfig $storageConfig): IStorage {
 		$class = $storageConfig->getBackend()->getStorageClass();
+		if (!$class instanceof IConstructableStorage) {
+			\OCP\Server::get(LoggerInterface::class)->warning('Building a storage not implementing IConstructableStorage is deprecated since 31.0.0', ['class' => $class]);
+		}
 		$storage = new $class($storageConfig->getBackendOptions());
 
 		// auth mechanism should fire first

diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php
@@ -428,6 +428,7 @@
     'OCP\\Files\\StorageNotAvailableException' => $baseDir . '/lib/public/Files/StorageNotAvailableException.php',
     'OCP\\Files\\StorageTimeoutException' => $baseDir . '/lib/public/Files/StorageTimeoutException.php',
     'OCP\\Files\\Storage\\IChunkedFileWrite' => $baseDir . '/lib/public/Files/Storage/IChunkedFileWrite.php',
+    'OCP\\Files\\Storage\\IConstructableStorage' => $baseDir . '/lib/public/Files/Storage/IConstructableStorage.php',
     'OCP\\Files\\Storage\\IDisableEncryptionStorage' => $baseDir . '/lib/public/Files/Storage/IDisableEncryptionStorage.php',
     'OCP\\Files\\Storage\\ILockingStorage' => $baseDir . '/lib/public/Files/Storage/ILockingStorage.php',
     'OCP\\Files\\Storage\\INotifyStorage' => $baseDir . '/lib/public/Files/Storage/INotifyStorage.php',

diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php
@@ -461,6 +461,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
         'OCP\\Files\\StorageNotAvailableException' => __DIR__ . '/../../..' . '/lib/public/Files/StorageNotAvailableException.php',
         'OCP\\Files\\StorageTimeoutException' => __DIR__ . '/../../..' . '/lib/public/Files/StorageTimeoutException.php',
         'OCP\\Files\\Storage\\IChunkedFileWrite' => __DIR__ . '/../../..' . '/lib/public/Files/Storage/IChunkedFileWrite.php',
+        'OCP\\Files\\Storage\\IConstructableStorage' => __DIR__ . '/../../..' . '/lib/public/Files/Storage/IConstructableStorage.php',
         'OCP\\Files\\Storage\\IDisableEncryptionStorage' => __DIR__ . '/../../..' . '/lib/public/Files/Storage/IDisableEncryptionStorage.php',
         'OCP\\Files\\Storage\\ILockingStorage' => __DIR__ . '/../../..' . '/lib/public/Files/Storage/ILockingStorage.php',
         'OCP\\Files\\Storage\\INotifyStorage' => __DIR__ . '/../../..' . '/lib/public/Files/Storage/INotifyStorage.php',

diff --git a/lib/private/Files/Storage/Common.php b/lib/private/Files/Storage/Common.php
@@ -21,6 +21,7 @@
 use OCP\Files\GenericFileException;
 use OCP\Files\IFilenameValidator;
 use OCP\Files\InvalidPathException;
+use OCP\Files\Storage\IConstructableStorage;
 use OCP\Files\Storage\ILockingStorage;
 use OCP\Files\Storage\IStorage;
 use OCP\Files\Storage\IWriteStreamStorage;
@@ -41,7 +42,7 @@
  * Some \OC\Files\Storage\Common methods call functions which are first defined
  * in classes which extend it, e.g. $this->stat() .
  */
-abstract class Common implements Storage, ILockingStorage, IWriteStreamStorage {
+abstract class Common implements Storage, ILockingStorage, IWriteStreamStorage, IConstructableStorage {
 	use LocalTempFileTrait;
 
 	protected ?Cache $cache = null;

diff --git a/lib/private/Files/Storage/StorageFactory.php b/lib/private/Files/Storage/StorageFactory.php
@@ -8,8 +8,10 @@
 namespace OC\Files\Storage;
 
 use OCP\Files\Mount\IMountPoint;
+use OCP\Files\Storage\IConstructableStorage;
 use OCP\Files\Storage\IStorage;
 use OCP\Files\Storage\IStorageFactory;
+use Psr\Log\LoggerInterface;
 
 class StorageFactory implements IStorageFactory {
 	/**
@@ -62,6 +64,9 @@ public function removeStorageWrapper($wrapperName) {
 	 * @return IStorage
 	 */
 	public function getInstance(IMountPoint $mountPoint, $class, $arguments) {
+		if (!($class instanceof IConstructableStorage)) {
+			\OCP\Server::get(LoggerInterface::class)->warning('Building a storage not implementing IConstructableStorage is deprecated since 31.0.0', ['class' => $class]);
+		}
 		return $this->wrap($mountPoint, new $class($arguments));
 	}
 

diff --git a/lib/public/Files/Storage/IConstructableStorage.php b/lib/public/Files/Storage/IConstructableStorage.php
@@ -0,0 +1,26 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-only
+ */
+// use OCP namespace for all classes that are considered public.
+// This means that they should be used by apps instead of the internal Nextcloud classes
+
+namespace OCP\Files\Storage;
+
+/**
+ * Marks a storage as constructable. Allows to pass the storage as a string to a mounpoint and let it build the instance.
+ *
+ * @since 31.0.0
+ */
+interface IConstructableStorage {
+	/**
+	 * @param array $parameters is a free form array with the configuration options needed to construct the storage
+	 *
+	 * @since 31.0.0
+	 */
+	public function __construct(array $parameters);
+}
diff --git a/lib/public/Files/Storage/IStorage.php b/lib/public/Files/Storage/IStorage.php
@@ -23,16 +23,9 @@
  * All paths passed to the storage are relative to the storage and should NOT have a leading slash.
  *
  * @since 9.0.0
+ * @since 31.0.0 Moved the constructor to IConstructableStorage so that wrappers can use DI
  */
 interface IStorage {
-	/**
-	 * $parameters is a free form array with the configuration options needed to construct the storage
-	 *
-	 * @param array $parameters
-	 * @since 9.0.0
-	 */
-	public function __construct($parameters);
-
 	/**
 	 * Get the identifier for the storage,
 	 * the returned id should be the same for every storage object that is created with the same parameters

diff --git a/tests/lib/Files/ViewTest.php b/tests/lib/Files/ViewTest.php
@@ -1589,7 +1589,6 @@ private function createTestMovableMountPoints($mountPoints) {
 		foreach ($mountPoints as $mountPoint) {
 			$storage = $this->getMockBuilder(Storage::class)
 				->setMethods([])
-				->setConstructorArgs([[]])
 				->getMock();
 			$storage->method('getId')->willReturn('non-null-id');
 			$storage->method('getStorageCache')->willReturnCallback(function () use ($storage) {