refactor: Call IStorageFactory::addStorageFactory directly

CarlSchwan · CarlSchwan · commit 415aeeb2f775 · 2026-06-22T16:40:09.000+02:00
Remove call to deprecated Filesystem api and allow to reuse instead of
the mounts points between addStorageFactory.

Signed-off-by: Carl Schwan &lt;carlschwan@kde.org&gt;
diff --git a/apps/dav/appinfo/v1/publicwebdav.php b/apps/dav/appinfo/v1/publicwebdav.php
@@ -21,6 +21,7 @@
 use OCP\EventDispatcher\IEventDispatcher;
 use OCP\Files\IRootFolder;
 use OCP\Files\Mount\IMountManager;
+use OCP\Files\Storage\IStorage;
 use OCP\IConfig;
 use OCP\IDBConnection;
 use OCP\IPreview;
@@ -109,14 +110,14 @@ function (\Sabre\DAV\Server $server) use (
 
 		// FIXME: should not add storage wrappers outside of preSetup, need to find a better way
 		$previousLog = Filesystem::logWarningWhenAddingStorageWrapper(false);
-		Filesystem::addStorageWrapper('sharePermissions', function ($mountPoint, $storage) use ($share) {
+		Filesystem::addStorageWrapper('sharePermissions', function (string $mountPoint, IStorage $storage) use ($share) {
 			return new DirPermissionsMask([
 				'storage' => $storage,
 				'mask' => $share->getPermissions() | Constants::PERMISSION_SHARE,
 				'path' => 'files'
 			]);
 		});
-		Filesystem::addStorageWrapper('shareOwner', function ($mountPoint, $storage) use ($share) {
+		Filesystem::addStorageWrapper('shareOwner', function (string $mountPoint, IStorage $storage) use ($share) {
 			return new PublicOwnerWrapper(['storage' => $storage, 'owner' => $share->getShareOwner()]);
 		});
 		Filesystem::logWarningWhenAddingStorageWrapper($previousLog);
diff --git a/apps/dav/appinfo/v2/publicremote.php b/apps/dav/appinfo/v2/publicremote.php
@@ -26,6 +26,7 @@
 use OCP\Files\IHomeStorage;
 use OCP\Files\IRootFolder;
 use OCP\Files\Mount\IMountManager;
+use OCP\Files\Storage\IStorage;
 use OCP\ICacheFactory;
 use OCP\IConfig;
 use OCP\IDBConnection;
@@ -120,7 +121,7 @@
 	$previousLog = Filesystem::logWarningWhenAddingStorageWrapper(false);
 
 	/** @psalm-suppress MissingClosureParamType */
-	Filesystem::addStorageWrapper('sharePermissions', function ($mountPoint, $storage) use ($requestUri, $baseuri, $share) {
+	Filesystem::addStorageWrapper('sharePermissions', function (string $mountPoint, IStorage $storage) use ($requestUri, $baseuri, $share) {
 		$mask = $share->getPermissions() | Constants::PERMISSION_SHARE;
 
 		// For chunked uploads it is necessary to have read and delete permission,
@@ -141,13 +142,13 @@
 	});
 
 	/** @psalm-suppress MissingClosureParamType */
-	Filesystem::addStorageWrapper('shareOwner', function ($mountPoint, $storage) use ($share) {
+	Filesystem::addStorageWrapper('shareOwner', function (string $mountPoint, IStorage $storage) use ($share) {
 		return new PublicOwnerWrapper(['storage' => $storage, 'owner' => $share->getShareOwner()]);
 	});
 
 	// Ensure that also private shares have the `getShare` method
 	/** @psalm-suppress MissingClosureParamType */
-	Filesystem::addStorageWrapper('getShare', function ($mountPoint, $storage) use ($share) {
+	Filesystem::addStorageWrapper('getShare', function (string $mountPoint, IStorage $storage) use ($share) {
 		return new PublicShareWrapper(['storage' => $storage, 'share' => $share]);
 	}, 0);
 
diff --git a/apps/files_sharing/tests/DeleteOrphanedSharesJobTest.php b/apps/files_sharing/tests/DeleteOrphanedSharesJobTest.php
@@ -11,6 +11,7 @@
 use OC\Files\Filesystem;
 use OC\SystemConfig;
 use OCA\Files_Sharing\DeleteOrphanedSharesJob;
+use OCA\Files_Trashbin\Storage;
 use OCP\App\IAppManager;
 use OCP\Constants;
 use OCP\IDBConnection;
@@ -60,7 +61,7 @@ public static function setUpBeforeClass(): void {
 		$appManager->disableApp('files_trashbin');
 
 		// just in case...
-		Filesystem::getLoader()->removeStorageWrapper('oc_trashbin');
+		Filesystem::getLoader()->removeStorageWrapper(Storage::class);
 	}
 
 	public static function tearDownAfterClass(): void {
diff --git a/apps/files_sharing/tests/UpdaterTest.php b/apps/files_sharing/tests/UpdaterTest.php
@@ -13,6 +13,7 @@
 use OC\Files\View;
 use OCA\Files_Sharing\Helper;
 use OCA\Files_Trashbin\AppInfo\Application;
+use OCA\Files_Trashbin\Storage;
 use OCP\App\IAppManager;
 use OCP\AppFramework\Bootstrap\IBootContext;
 use OCP\Constants;
@@ -121,7 +122,7 @@ public function testDeleteParentFolder(): void {
 			$appManager->disableApp('files_trashbin');
 		}
 
-		Filesystem::getLoader()->removeStorageWrapper('oc_trashbin');
+		Filesystem::getLoader()->removeStorageWrapper(Storage::class);
 	}
 
 	public static function shareFolderProvider() {
diff --git a/apps/files_trashbin/lib/Listener/EventListener.php b/apps/files_trashbin/lib/Listener/EventListener.php
@@ -10,24 +10,33 @@
 namespace OCA\Files_Trashbin\Listener;
 
 use OCA\Files_Trashbin\Storage;
+use OCA\Files_Trashbin\Trash\ITrashManager;
 use OCA\Files_Trashbin\Trashbin;
 use OCP\EventDispatcher\Event;
+use OCP\EventDispatcher\IEventDispatcher;
 use OCP\EventDispatcher\IEventListener;
 use OCP\Files\Events\BeforeFileSystemSetupEvent;
 use OCP\Files\Events\Node\NodeWrittenEvent;
 use OCP\Files\Folder;
 use OCP\Files\IRootFolder;
 use OCP\Files\NotFoundException;
 use OCP\Files\NotPermittedException;
+use OCP\Files\Storage\IStorage;
+use OCP\IRequest;
 use OCP\IUserManager;
 use OCP\User\Events\BeforeUserDeletedEvent;
+use Psr\Log\LoggerInterface;
 
 /** @template-implements IEventListener<NodeWrittenEvent|BeforeUserDeletedEvent|BeforeFileSystemSetupEvent> */
 class EventListener implements IEventListener {
 	public function __construct(
-		private IUserManager $userManager,
-		private IRootFolder $rootFolder,
-		private ?string $userId = null,
+		private readonly IUserManager $userManager,
+		private readonly IRootFolder $rootFolder,
+		private readonly IRequest $request,
+		private readonly IEventDispatcher $eventDispatcher,
+		private readonly LoggerInterface $logger,
+		private readonly ITrashManager $trashManager,
+		private readonly ?string $userId = null,
 	) {
 	}
 
@@ -57,7 +66,20 @@ public function handle(Event $event): void {
 		}
 
 		if ($event instanceof BeforeFileSystemSetupEvent) {
-			Storage::setupStorage();
+			$event->addStorageWrapper(
+				Storage::class,
+				function (string $mountPoint, IStorage $storage): Storage {
+					return new Storage(
+						['storage' => $storage, 'mountPoint' => $mountPoint],
+						$this->trashManager,
+						$this->userManager,
+						$this->logger,
+						$this->eventDispatcher,
+						$this->rootFolder,
+						$this->request,
+					);
+				},
+				1);
 		}
 	}
 }
diff --git a/apps/files_trashbin/lib/Storage.php b/apps/files_trashbin/lib/Storage.php
@@ -159,7 +159,7 @@ public static function setupStorage(): void {
 		$rootFolder = Server::get(IRootFolder::class);
 		$request = Server::get(IRequest::class);
 		Filesystem::addStorageWrapper(
-			'oc_trashbin',
+			Storage::class,
 			function (string $mountPoint, IStorage $storage) use ($trashManager, $userManager, $logger, $eventDispatcher, $rootFolder, $request) {
 				return new Storage(
 					['storage' => $storage, 'mountPoint' => $mountPoint],
diff --git a/apps/files_trashbin/tests/StorageTest.php b/apps/files_trashbin/tests/StorageTest.php
@@ -96,7 +96,7 @@ protected function setUp(): void {
 	}
 
 	protected function tearDown(): void {
-		Filesystem::getLoader()->removeStorageWrapper('oc_trashbin');
+		Filesystem::getLoader()->removeStorageWrapper(Storage::class);
 		$this->logout();
 		$user = Server::get(IUserManager::class)->get($this->user);
 		if ($user !== null) {
@@ -502,7 +502,7 @@ public function testKeepFileAndVersionsWhenMovingFileBetweenStorages(): void {
 		$storage2 = new Temporary([]);
 		Filesystem::mount($storage2, [], $this->user . '/files/substorage');
 
-		// trigger a version (multiple would not work because of the expire logic)
+		// trigger a version (multiple would not work because of the expiration logic)
 		$this->userView->file_put_contents('test.txt', 'v1');
 
 		$results = $this->rootView->getDirectoryContent($this->user . '/files_trashbin/files');
diff --git a/apps/files_trashbin/tests/TrashbinTest.php b/apps/files_trashbin/tests/TrashbinTest.php
@@ -19,6 +19,7 @@
 use OCA\Files_Trashbin\AppInfo\Application as TrashbinApplication;
 use OCA\Files_Trashbin\Expiration;
 use OCA\Files_Trashbin\Helper;
+use OCA\Files_Trashbin\Storage;
 use OCA\Files_Trashbin\Trashbin;
 use OCP\App\IAppManager;
 use OCP\AppFramework\Utility\ITimeFactory;
@@ -96,7 +97,7 @@ public static function tearDownAfterClass(): void {
 
 		\OC_Hook::clear();
 
-		Filesystem::getLoader()->removeStorageWrapper('oc_trashbin');
+		Filesystem::getLoader()->removeStorageWrapper(Storage::class);
 
 		if (self::$trashBinStatus) {
 			Server::get(IAppManager::class)->enableApp('files_trashbin');
diff --git a/lib/private/Encryption/Manager.php b/lib/private/Encryption/Manager.php
@@ -203,11 +203,11 @@ public function getDefaultEncryptionModuleId() {
 	/**
 	 * Add storage wrapper
 	 */
-	public function setupStorage() {
+	public function setupStorage(): void {
 		// If encryption is disabled and there are no loaded modules it makes no sense to load the wrapper
 		if (!empty($this->encryptionModules) || $this->isEnabled()) {
 			$encryptionWrapper = new EncryptionWrapper($this->arrayCache, $this, $this->logger);
-			Filesystem::addStorageWrapper('oc_encryption', [$encryptionWrapper, 'wrapStorage'], 2);
+			Filesystem::addStorageWrapper('oc_encryption', $encryptionWrapper->wrapStorage(...), 2);
 		}
 	}
 
diff --git a/lib/private/Files/Mount/Manager.php b/lib/private/Files/Mount/Manager.php
@@ -207,9 +207,6 @@ public function findByStorageId(string $id): array {
 		return $result;
 	}
 
-	/**
-	 * @return IMountPoint[]
-	 */
 	#[\Override]
 	public function getAll(): array {
 		return $this->mounts;
diff --git a/lib/private/Files/SetupManager.php b/lib/private/Files/SetupManager.php
@@ -49,6 +49,7 @@
 use OCP\Files\Mount\IMountPoint;
 use OCP\Files\NotFoundException;
 use OCP\Files\Storage\IStorage;
+use OCP\Files\Storage\IStorageFactory;
 use OCP\Group\Events\UserAddedEvent;
 use OCP\Group\Events\UserRemovedEvent;
 use OCP\HintException;
@@ -115,6 +116,7 @@ public function __construct(
 		private IAppManager $appManager,
 		private FileAccess $fileAccess,
 		private IAppConfig $appConfig,
+		private IStorageFactory $storageFactory,
 	) {
 		$this->cache = $cacheFactory->createDistributed('setupmanager::');
 		$this->listeningForProviders = false;
@@ -154,28 +156,30 @@ private function isPathSetup(string $path): bool {
 		return false;
 	}
 
-	private function setupBuiltinWrappers(): void {
+	/**
+	 * @param IMountPoint[] $existingMounts
+	 */
+	private function setupBuiltinWrappers(array $existingMounts): void {
 		if ($this->setupBuiltinWrappersDone) {
 			return;
 		}
 		$this->setupBuiltinWrappersDone = true;
 
 		// load all filesystem apps before, so no setup-hook gets lost
 		$this->appManager->loadApps(['filesystem']);
-		$prevLogging = Filesystem::logWarningWhenAddingStorageWrapper(false);
 
-		Filesystem::addStorageWrapper('mount_options', function ($mountPoint, IStorage $storage, IMountPoint $mount) {
+		$this->storageFactory->addStorageWrapper('mount_options', function (string $mountPoint, IStorage $storage, IMountPoint $mount): IStorage {
 			if ($storage->instanceOfStorage(Common::class)) {
 				$options = array_merge($mount->getOptions(), ['mount_point' => $mountPoint]);
 				$storage->setMountOptions($options);
 			}
 			return $storage;
-		});
+		}, 50, $existingMounts);
 
 		$reSharingEnabled = $this->appConfig->getValueBool('core', 'shareapi_allow_resharing', true);
 		$user = $this->userSession->getUser();
 		$sharingEnabledForUser = $user ? !$this->shareDisableChecker->sharingDisabledForUser($user->getUID()) : true;
-		Filesystem::addStorageWrapper(
+		$this->storageFactory->addStorageWrapper(
 			'sharing_mask',
 			function ($mountPoint, IStorage $storage, IMountPoint $mount) use ($reSharingEnabled, $sharingEnabledForUser) {
 				$sharingEnabledForMount = $mount->getOption('enable_sharing', true);
@@ -187,27 +191,26 @@ function ($mountPoint, IStorage $storage, IMountPoint $mount) use ($reSharingEna
 					]);
 				}
 				return $storage;
-			}
-		);
+			}, 50, $existingMounts);
 
 		// install storage availability wrapper, before most other wrappers
-		Filesystem::addStorageWrapper('oc_availability', function ($mountPoint, IStorage $storage, IMountPoint $mount) {
+		$this->storageFactory->addStorageWrapper(Availability::class, function (string $mountPoint, IStorage $storage, IMountPoint $mount) {
 			$externalMount = $mount instanceof ExternalMountPoint || $mount instanceof Mount;
 			if ($externalMount && !$storage->isLocal()) {
 				return new Availability(['storage' => $storage]);
 			}
 			return $storage;
-		});
+		}, 50, $existingMounts);
 
-		Filesystem::addStorageWrapper('oc_encoding', function ($mountPoint, IStorage $storage, IMountPoint $mount) {
+		$this->storageFactory->addStorageWrapper(Encoding::class, function (string $mountPoint, IStorage $storage, IMountPoint $mount) {
 			if ($mount->getOption('encoding_compatibility', false) && !$mount instanceof SharedMount) {
 				return new Encoding(['storage' => $storage]);
 			}
 			return $storage;
-		});
+		}, 50, $existingMounts);
 
 		$quotaIncludeExternal = $this->config->getSystemValue('quota_include_external_storage', false);
-		Filesystem::addStorageWrapper('oc_quota', function ($mountPoint, $storage, IMountPoint $mount) use ($quotaIncludeExternal) {
+		$this->storageFactory->addStorageWrapper(Quota::class, function ($mountPoint, $storage, IMountPoint $mount) use ($quotaIncludeExternal) {
 			// set up quota for home storages, even for other users
 			// which can happen when using sharing
 			if ($mount instanceof HomeMountPoint) {
@@ -218,9 +221,9 @@ function ($mountPoint, IStorage $storage, IMountPoint $mount) use ($reSharingEna
 			}
 
 			return $storage;
-		});
+		}, 50, $existingMounts);
 
-		Filesystem::addStorageWrapper('readonly', function ($mountPoint, IStorage $storage, IMountPoint $mount) {
+		$this->storageFactory->addStorageWrapper('readonly', function (string $mountPoint, IStorage $storage, IMountPoint $mount) {
 			/*
 			 * Do not allow any operations that modify the storage
 			 */
@@ -235,9 +238,7 @@ function ($mountPoint, IStorage $storage, IMountPoint $mount) use ($reSharingEna
 				]);
 			}
 			return $storage;
-		});
-
-		Filesystem::logWarningWhenAddingStorageWrapper($prevLogging);
+		}, 50, $existingMounts);
 	}
 
 	/**
@@ -299,7 +300,7 @@ public function setupForUser(IUser $user): void {
 	/**
 	 * Part of the user setup that is run only once per user.
 	 */
-	private function oneTimeUserSetup(IUser $user) {
+	private function oneTimeUserSetup(IUser $user): void {
 		if ($this->isSetupStarted($user)) {
 			return;
 		}
@@ -309,22 +310,24 @@ private function oneTimeUserSetup(IUser $user) {
 
 		$this->eventLogger->start('fs:setup:user:onetime', 'Onetime filesystem for user');
 
-		$this->setupBuiltinWrappers();
-
-		$prevLogging = Filesystem::logWarningWhenAddingStorageWrapper(false);
+		$mounts = $this->mountManager->getAll();
+		$this->setupBuiltinWrappers($mounts);
 
 		// TODO remove hook
+		$prevLogging = Filesystem::logWarningWhenAddingStorageWrapper(false);
 		OC_Hook::emit('OC_Filesystem', 'preSetup', ['user' => $user->getUID()]);
 
 		$event = new BeforeFileSystemSetupEvent($user);
 		$this->eventDispatcher->dispatchTyped($event);
+		Filesystem::logWarningWhenAddingStorageWrapper($prevLogging);
 
-		foreach ($event->getStorageWrappers() as $name => $wrapper) {
-			Filesystem::addStorageWrapper($name, $wrapper['wrapper'], $wrapper['priority']);
+		$storageWrappers = $event->getStorageWrappers();
+		if ($storageWrappers !== []) {
+			foreach ($storageWrappers as $wrapperName => $wrapper) {
+				$this->storageFactory->addStorageWrapper($wrapperName, $wrapper['callable'], $wrapper['priority'], $mounts);
+			}
 		}
 
-		Filesystem::logWarningWhenAddingStorageWrapper($prevLogging);
-
 		$userDir = '/' . $user->getUID() . '/files';
 
 		Filesystem::initInternal($userDir);
@@ -430,7 +433,8 @@ public function setupRoot(): void {
 			return;
 		}
 
-		$this->setupBuiltinWrappers();
+		$mounts = $this->mountManager->getAll();
+		$this->setupBuiltinWrappers($mounts);
 
 		$this->rootSetup = true;
 
@@ -776,7 +780,7 @@ private function listenForNewMountProviders() {
 		}
 	}
 
-	private function setupListeners() {
+	private function setupListeners(): void {
 		// note that this event handling is intentionally pessimistic
 		// clearing the cache to often is better than not enough
 
diff --git a/lib/private/Files/SetupManagerFactory.php b/lib/private/Files/SetupManagerFactory.php
diff --git a/lib/private/Files/Storage/StorageFactory.php b/lib/private/Files/Storage/StorageFactory.php
diff --git a/lib/public/Files/Events/BeforeFileSystemSetupEvent.php b/lib/public/Files/Events/BeforeFileSystemSetupEvent.php
diff --git a/lib/public/Files/Storage/IStorageFactory.php b/lib/public/Files/Storage/IStorageFactory.php
diff --git a/tests/lib/Files/SetupManagerTest.php b/tests/lib/Files/SetupManagerTest.php

Original file line number	Diff line number	Diff line change
`@@ -13,6 +13,7 @@`
`13`	`13`	`use OC\Files\View;`
`14`	`14`	`use OCA\Files_Sharing\Helper;`
`15`	`15`	`use OCA\Files_Trashbin\AppInfo\Application;`
	`16`	`+use OCA\Files_Trashbin\Storage;`
`16`	`17`	`use OCP\App\IAppManager;`
`17`	`18`	`use OCP\AppFramework\Bootstrap\IBootContext;`
`18`	`19`	`use OCP\Constants;`
`@@ -121,7 +122,7 @@ public function testDeleteParentFolder(): void {`
`121`	`122`	`$appManager->disableApp('files_trashbin');`
`122`	`123`	`}`
`123`	`124`
`124`		`- Filesystem::getLoader()->removeStorageWrapper('oc_trashbin');`
	`125`	`+ Filesystem::getLoader()->removeStorageWrapper(Storage::class);`
`125`	`126`	`}`
`126`	`127`
`127`	`128`	`public static function shareFolderProvider() {`
Original file line number	Diff line number	Diff line change
`@@ -203,11 +203,11 @@ public function getDefaultEncryptionModuleId() {`
`203`	`203`	`/**`
`204`	`204`	`* Add storage wrapper`
`205`	`205`	`*/`
`206`		`- public function setupStorage() {`
	`206`	`+ public function setupStorage(): void {`
`207`	`207`	`// If encryption is disabled and there are no loaded modules it makes no sense to load the wrapper`
`208`	`208`	`if (!empty($this->encryptionModules) \|\| $this->isEnabled()) {`
`209`	`209`	`$encryptionWrapper = new EncryptionWrapper($this->arrayCache, $this, $this->logger);`
`210`		`- Filesystem::addStorageWrapper('oc_encryption', [$encryptionWrapper, 'wrapStorage'], 2);`
	`210`	`+ Filesystem::addStorageWrapper('oc_encryption', $encryptionWrapper->wrapStorage(...), 2);`
`211`	`211`	`}`
`212`	`212`	`}`
`213`	`213`
Original file line number	Diff line number	Diff line change
`@@ -207,9 +207,6 @@ public function findByStorageId(string $id): array {`
`207`	`207`	`return $result;`
`208`	`208`	`}`
`209`	`209`
`210`		`- /**`
`211`		`- * @return IMountPoint[]`
`212`		`- */`
`213`	`210`	`#[\Override]`
`214`	`211`	`public function getAll(): array {`
`215`	`212`	`return $this->mounts;`