Merge pull request #57289 from nextcloud/feature/54562/drop-mounts-on-full-or-provider-setup

Feature/54562/drop mounts on full or provider setup

lib/private/Files/SetupManager.php

@@ -40,6 +40,7 @@ use OCP\Files\Config\IRootMountProvider;
 use OCP\Files\Config\IUserMountCache;
 use OCP\Files\Events\BeforeFileSystemSetupEvent;
 use OCP\Files\Events\InvalidateMountCacheEvent;
+use OCP\Files\Events\Node\BeforeNodeRenamedEvent;
 use OCP\Files\Events\Node\FilesystemTornDownEvent;
 use OCP\Files\Mount\IMountManager;
 use OCP\Files\Mount\IMountPoint;
@@ -236,6 +237,8 @@ class SetupManager {
 
 		$this->eventLogger->start('fs:setup:user:full', 'Setup full filesystem for user');
 
+		$this->dropPartialMountsForUser($user);
+
 		$this->setupUserMountProviders[$user->getUID()] ??= [];
 		$previouslySetupProviders = $this->setupUserMountProviders[$user->getUID()];
 
@@ -658,6 +661,7 @@ class SetupManager {
 			$this->eventLogger->end('fs:setup:user:providers');
 			return;
 		} else {
+			$this->dropPartialMountsForUser($user, $providers);
 			$this->setupUserMountProviders[$user->getUID()] = array_merge($setupProviders, $providers);
 			$mounts = $this->mountProviderCollection->getUserMountsForProviderClasses($user, $providers);
 		}
@@ -713,6 +717,16 @@ class SetupManager {
 		$this->eventDispatcher->addListener(ShareCreatedEvent::class, function (ShareCreatedEvent $event) {
 			$this->cache->remove($event->getShare()->getSharedWith());
 		});
+		$this->eventDispatcher->addListener(BeforeNodeRenamedEvent::class, function (BeforeNodeRenamedEvent $event) {
+			// update cache information that is cached by mount point
+			$from = rtrim($event->getSource()->getPath(), '/') . '/';
+			$to = rtrim($event->getTarget()->getPath(), '/') . '/';
+			$existingMount = $this->setupMountProviderPaths[$from] ?? null;
+			if ($existingMount !== null) {
+				$this->setupMountProviderPaths[$to] = $this->setupMountProviderPaths[$from];
+				unset($this->setupMountProviderPaths[$from]);
+			}
+		});
 		$this->eventDispatcher->addListener(InvalidateMountCacheEvent::class, function (InvalidateMountCacheEvent $event,
 		) {
 			if ($user = $event->getUser()) {
@@ -741,4 +755,39 @@ class SetupManager {
 			$this->userMountCache->registerMounts($user, $mounts, $mountProviderClasses);
 		}
 	}
+
+	/**
+	 * Drops partially set-up mounts for the given user
+	 * @param class-string<IMountProvider>[] $providers
+	 */
+	public function dropPartialMountsForUser(IUser $user, array $providers = []): void {
+		// mounts are cached by mount-point
+		$mounts = $this->mountManager->getAll();
+		$partialMounts = array_filter($this->setupMountProviderPaths,
+			static function (string $mountPoint) use (
+				$providers,
+				$user,
+				$mounts
+			) {
+				$isUserMount = str_starts_with($mountPoint, '/' . $user->getUID() . '/files');
+
+				if (!$isUserMount) {
+					return false;
+				}
+
+				$mountProvider = ($mounts[$mountPoint] ?? null)?->getMountProvider();
+
+				return empty($providers)
+					|| \in_array($mountProvider, $providers, true);
+			},
+			ARRAY_FILTER_USE_KEY);
+
+		if (!empty($partialMounts)) {
+			// remove partially set up mounts
+			foreach ($partialMounts as $mountPoint => $_mount) {
+				$this->mountManager->removeMount($mountPoint);
+				unset($this->setupMountProviderPaths[$mountPoint]);
+			}
+		}
+	}
 }

tests/lib/Files/SetupManagerTest.php

@@ -495,6 +495,143 @@ class SetupManagerTest extends TestCase {
 		$this->setupManager->setupForPath($this->path, true);
 	}
 
+	public function testSetupForUserResetsUserPaths(): void {
+		$cachedMount = $this->getCachedMountInfo($this->mountPoint, 42);
+
+		$this->userMountCache->expects($this->once())
+			->method('getMountForPath')
+			->with($this->user, $this->path)
+			->willReturn($cachedMount);
+		$this->userMountCache->expects($this->never())
+			->method('getMountsInPath');
+
+		$this->fileAccess->expects($this->once())
+			->method('getByFileId')
+			->with(42)
+			->willReturn($this->createMock(CacheEntry::class));
+
+		$partialMount = $this->createMock(IMountPoint::class);
+
+		$this->mountProviderCollection->expects($this->once())
+			->method('getUserMountsFromProviderByPath')
+			->with(
+				SetupManagerTestPartialMountProvider::class,
+				$this->path,
+				false,
+				$this->callback(function (array $args) use ($cachedMount) {
+					$this->assertCount(1, $args);
+					$this->assertInstanceOf(IMountProviderArgs::class,
+						$args[0]);
+					$this->assertSame($cachedMount, $args[0]->mountInfo);
+					return true;
+				})
+			)
+			->willReturn([$partialMount]);
+
+		$homeMount = $this->createMock(IMountPoint::class);
+
+		$this->mountProviderCollection->expects($this->once())
+			->method('getHomeMountForUser')
+			->willReturn($homeMount);
+		$this->mountProviderCollection->expects($this->never())
+			->method('getUserMountsForProviderClasses');
+
+		$invokedCount = $this->exactly(2);
+		$addMountExpectations = [
+			1 => $homeMount,
+			2 => $partialMount,
+		];
+		$this->mountManager->expects($invokedCount)
+			->method('addMount')
+			->willReturnCallback($this->getAddMountCheckCallback($invokedCount,
+				$addMountExpectations));
+
+
+		// setting up for $path but then for user should remove the setup path
+		$this->setupManager->setupForPath($this->path, false);
+
+		// note that only the mount known by SetupManrger is removed not the
+		// home mount, because MountManager is mocked
+		$this->mountManager->expects($this->once())
+			->method('removeMount')
+			->with($this->mountPoint);
+
+		$this->setupManager->setupForUser($this->user);
+	}
+
+	/**
+	 * Tests that after a path is setup by a
+	 */
+	public function testSetupForProviderResetsUserProviderPaths(): void {
+		$cachedMount = $this->getCachedMountInfo($this->mountPoint, 42);
+
+		$this->userMountCache->expects($this->once())
+			->method('getMountForPath')
+			->with($this->user, $this->path)
+			->willReturn($cachedMount);
+		$this->userMountCache->expects($this->never())
+			->method('getMountsInPath');
+
+		$this->fileAccess->expects($this->once())
+			->method('getByFileId')
+			->with(42)
+			->willReturn($this->createMock(CacheEntry::class));
+
+		$partialMount = $this->createMock(IMountPoint::class);
+		$partialMount->expects($this->once())->method('getMountProvider')
+			->willReturn(SetupManagerTestFullMountProvider::class);
+
+		$this->mountProviderCollection->expects($this->once())
+			->method('getUserMountsFromProviderByPath')
+			->with(
+				SetupManagerTestPartialMountProvider::class,
+				$this->path,
+				false,
+				$this->callback(function (array $args) use ($cachedMount) {
+					$this->assertCount(1, $args);
+					$this->assertInstanceOf(IMountProviderArgs::class,
+						$args[0]);
+					$this->assertSame($cachedMount, $args[0]->mountInfo);
+					return true;
+				})
+			)
+			->willReturn([$partialMount]);
+
+		$homeMount = $this->createMock(IMountPoint::class);
+
+		$this->mountProviderCollection->expects($this->once())
+			->method('getHomeMountForUser')
+			->willReturn($homeMount);
+
+		$invokedCount = $this->exactly(2);
+		$addMountExpectations = [
+			1 => $homeMount,
+			2 => $partialMount,
+		];
+		$this->mountManager->expects($invokedCount)
+			->method('addMount')
+			->willReturnCallback($this->getAddMountCheckCallback($invokedCount,
+				$addMountExpectations));
+		$this->mountManager->expects($this->once())->method('getAll')
+			->willReturn([$this->mountPoint => $partialMount]);
+
+		// setting up for $path but then for user should remove the setup path
+		$this->setupManager->setupForPath($this->path, false);
+
+		// note that only the mount known by SetupManrger is removed not the
+		// home mount, because MountManager is mocked
+		$this->mountManager->expects($this->once())
+			->method('removeMount')
+			->with($this->mountPoint);
+
+		$this->mountProviderCollection->expects($this->once())
+			->method('getUserMountsForProviderClasses')
+			->with($this->user, [SetupManagerTestFullMountProvider::class]);
+
+		$this->setupManager->setupForProvider($this->path,
+			[SetupManagerTestFullMountProvider::class]);
+	}
+
 	private function getAddMountCheckCallback(InvokedCount $invokedCount, $expectations): \Closure {
 		return function (IMountPoint $actualMount) use ($invokedCount, $expectations) {
 			$expectedMount = $expectations[$invokedCount->numberOfInvocations()] ?? null;