From 94b0a0e86337ff9ec7592e1df5fb785286665884 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Mon, 16 Sep 2024 21:48:18 +0200 Subject: [PATCH 1/3] fix: Move storage constructor to specific interface MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit That allows Wrappers to use DI and not care about the constructor Signed-off-by: Côme Chilliet --- .../lib/Config/ConfigAdapter.php | 5 ++++ lib/composer/composer/autoload_classmap.php | 1 + lib/composer/composer/autoload_static.php | 1 + lib/private/Files/Storage/Common.php | 3 ++- lib/private/Files/Storage/StorageFactory.php | 5 ++++ .../Files/Storage/IConstructableStorage.php | 26 +++++++++++++++++++ lib/public/Files/Storage/IStorage.php | 9 +------ 7 files changed, 41 insertions(+), 9 deletions(-) create mode 100644 lib/public/Files/Storage/IConstructableStorage.php diff --git a/apps/files_external/lib/Config/ConfigAdapter.php b/apps/files_external/lib/Config/ConfigAdapter.php index d0437432427ac..b2246226d33ba 100644 --- 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 index 166d5805bdc5d..234cc20b4a478 100644 --- 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 index b455077b958d7..7918621084851 100644 --- 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 index b6f14321f6141..a9721c30d7706 100644 --- 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 index 590425f5b641b..f194228d3c87c 100644 --- 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 new file mode 100644 index 0000000000000..5801b78d0a640 --- /dev/null +++ b/lib/public/Files/Storage/IConstructableStorage.php @@ -0,0 +1,26 @@ + Date: Mon, 23 Sep 2024 11:42:41 +0200 Subject: [PATCH 2/3] chore: use a proper `@param` tag for IConstructableStorage constructor parameter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Kate <26026535+provokateurin@users.noreply.github.com> Signed-off-by: Côme Chilliet <91878298+come-nc@users.noreply.github.com> --- lib/public/Files/Storage/IConstructableStorage.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/public/Files/Storage/IConstructableStorage.php b/lib/public/Files/Storage/IConstructableStorage.php index 5801b78d0a640..57749fa30fa55 100644 --- a/lib/public/Files/Storage/IConstructableStorage.php +++ b/lib/public/Files/Storage/IConstructableStorage.php @@ -18,7 +18,7 @@ */ interface IConstructableStorage { /** - * $parameters is a free form array with the configuration options needed to construct the storage + * @param array $parameters is a free form array with the configuration options needed to construct the storage * * @since 31.0.0 */ From b8ab560bdd1f1f9d80ec6c632323b38a4f007ed7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Mon, 23 Sep 2024 14:50:43 +0200 Subject: [PATCH 3/3] fix(tests): Fix test to remove call to non-existing constructor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- tests/lib/Files/ViewTest.php | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/lib/Files/ViewTest.php b/tests/lib/Files/ViewTest.php index 53da32b24dbcc..636280f89eb6a 100644 --- 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) {