refactor: Call IStorageFactory::addStorageFactory directly

Remove call to deprecated Filesystem api and allow to reuse instead of
the mounts points between addStorageFactory.

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

Author: CarlSchwan

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;

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()]);
+		Filesystem::logWarningWhenAddingStorageWrapper($prevLogging);
 
 		$event = new BeforeFileSystemSetupEvent($user);
 		$this->eventDispatcher->dispatchTyped($event);
 
-		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
 

lib/private/Files/SetupManagerFactory.php

@@ -16,6 +16,7 @@
 use OCP\Files\Config\IMountProviderCollection;
 use OCP\Files\Config\IUserMountCache;
 use OCP\Files\Mount\IMountManager;
+use OCP\Files\Storage\IStorageFactory;
 use OCP\IAppConfig;
 use OCP\ICacheFactory;
 use OCP\IConfig;
@@ -42,6 +43,7 @@ public function __construct(
 		private IAppManager $appManager,
 		private FileAccess $fileAccess,
 		private IAppConfig $appConfig,
+		private IStorageFactory $storageFactory,
 	) {
 		$this->setupManager = null;
 	}
@@ -64,6 +66,7 @@ public function create(IMountManager $mountManager): SetupManager {
 				$this->appManager,
 				$this->fileAccess,
 				$this->appConfig,
+				$this->storageFactory,
 			);
 		}
 		return $this->setupManager;

lib/private/Files/Storage/StorageFactory.php

@@ -16,10 +16,8 @@
 use Psr\Log\LoggerInterface;
 
 class StorageFactory implements IStorageFactory {
-	/**
-	 * @var array[] [$name=>['priority'=>$priority, 'wrapper'=>$callable] $storageWrappers
-	 */
-	private $storageWrappers = [];
+	/** @var array<string, array{wrapper: callable(string $mountPoint, IStorage $storage): IStorage, priority: int}> $storageWrappers */
+	private array $storageWrappers = [];
 
 	#[\Override]
 	public function addStorageWrapper(string $wrapperName, callable $callback, int $priority = 50, array $existingMounts = []): bool {

lib/public/Files/Events/BeforeFileSystemSetupEvent.php

@@ -19,7 +19,7 @@
  * @since 31.0.0
  */
 class BeforeFileSystemSetupEvent extends Event {
-	/** @var array<string, array{wrapper: callable(string $mountPoint, IStorage $storage): IStorage, priority: int}> $storageWrappers */
+	/** @var array<class-string<IStorage>, array{callable: callable(string $mountPoint, IStorage $storage): IStorage, priority: int}> $storageWrappers */
 	private array $storageWrappers = [];
 
 	/**
@@ -42,18 +42,20 @@ public function getUser(): IUser {
 	 * Add a storage wrapper to the file system. This allows apps to inject storage wrappers
 	 * for every mount.
 	 *
+	 * @param class-string<IStorage> $name The identifier of the wrapper.
 	 * @param callable(string $mountPoint, IStorage $storage): IStorage $wrapper
+	 * @param int<0, 100> $priority
 	 * @since 35.0.0
 	 */
 	public function addStorageWrapper(string $name, callable $wrapper, int $priority = 50): void {
-		$this->storageWrappers[$name] = ['wrapper' => $wrapper, 'priority' => $priority];
+		$this->storageWrappers[$name] = ['callable' => $wrapper, 'priority' => $priority];
 	}
 
 	/**
 	 * Get the storage wrappers.
 	 *
+	 * @return array<class-string<IStorage>, array{callable: callable(string $mountPoint, IStorage $storage): IStorage, priority: int}>
 	 * @since 35.0.0
-	 * @return array<string, array{wrapper: callable(string $mountPoint, IStorage $storage): IStorage, priority: int}>
 	 */
 	public function getStorageWrappers(): array {
 		return $this->storageWrappers;

lib/public/Files/Storage/IStorageFactory.php

@@ -8,27 +8,36 @@
 
 namespace OCP\Files\Storage;
 
+use OCP\AppFramework\Attribute\Consumable;
+use OCP\Files\Events\BeforeFileSystemSetupEvent;
 use OCP\Files\Mount\IMountPoint;
 
 /**
  * Creates storage instances and manages and applies storage wrappers
  * @since 8.0.0
  */
+#[Consumable(since: '8.0.0')]
 interface IStorageFactory {
 	/**
-	 * allow modifier storage behaviour by adding wrappers around storages
+	 * Allow modifier storage behaviour by adding wrappers around storages.
 	 *
-	 * $callback should be a function of type (string $mountPoint, Storage $storage) => Storage
+	 * The BeforeFileSystemSetupEvent should be used in most cases instead to add storage wrappers.
+	 *
+	 * @param non-empty-string $wrapperName
+	 * @param callable(string $mountPoint, IStorage $storage, IMountPoint $mountPoint): IStorage $callback
+	 * @param int<0, 100> $priority
+	 * @param IMountPoint[] $existingMounts
 	 *
 	 * @return bool true if the wrapper was added, false if there was already a wrapper with this
 	 *              name registered
 	 * @since 8.0.0
+	 * @see BeforeFileSystemSetupEvent
 	 */
-	public function addStorageWrapper(string $wrapperName, callable $callback);
+	public function addStorageWrapper(string $wrapperName, callable $callback, int $priority = 50, array $existingMounts = []): bool;
 
 	/**
 	 * @return IStorage
 	 * @since 8.0.0
 	 */
-	public function getInstance(IMountPoint $mountPoint, string $class, array $arguments);
+	public function getInstance(IMountPoint $mountPoint, string $class, array $arguments): IStorage;
 }

tests/lib/Files/SetupManagerTest.php

@@ -114,6 +114,7 @@ protected function setUp(): void {
 			$appManager,
 			$this->fileAccess,
 			$this->createMock(IAppConfig::class),
+			$this->createMock(IStorageFactory::class),
 		);
 	}