perf: only load a single mount at a time when checking for share conflicts

Signed-off-by: Robin Appelman <robin@icewind.nl>

apps/files_sharing/lib/Repair/CleanupShareTarget.php

@@ -107,7 +107,7 @@ class CleanupShareTarget implements IRepairStep {
 					(int)$shareInfo['file_source'],
 					$absoluteNewTarget,
 					$targetParentNode->getMountPoint(),
-					$userMounts,
+					fn ($path) => $userMounts[$path] ?? null,
 				);
 				$newTarget = $userFolder->getRelativePath($absoluteNewTarget);
 

apps/files_sharing/lib/ShareRecipientUpdater.php

@@ -51,7 +51,7 @@ class ShareRecipientUpdater {
 			$mountKey = $parentShare->getNodeId() . '::' . $mountPoint;
 			if (!isset($cachedMounts[$mountKey])) {
 				$mountsChanged = true;
-				$this->shareTargetValidator->verifyMountPoint($user, $parentShare, $mountsByPath, $groupedShares);
+				$this->shareTargetValidator->verifyMountPoint($user, $parentShare, fn ($path) => $mountsByPath[$path] ?? null, $groupedShares);
 			}
 		}
 
@@ -67,11 +67,7 @@ class ShareRecipientUpdater {
 	 * Validate a single received share for a user
 	 */
 	public function updateForAddedShare(IUser $user, IShare $share): void {
-		$cachedMounts = $this->userMountCache->getMountsForUser($user);
-		$mountPoints = array_map(fn (ICachedMountInfo $mount) => $mount->getMountPoint(), $cachedMounts);
-		$mountsByPath = array_combine($mountPoints, $cachedMounts);
-
-		$target = $this->shareTargetValidator->verifyMountPoint($user, $share, $mountsByPath, [$share]);
+		$target = $this->shareTargetValidator->verifyMountPoint($user, $share, fn ($path) => $this->userMountCache->getMountAtPath($user, $path), [$share]);
 		$mountPoint = $this->getMountPointFromTarget($user, $target);
 
 		$this->userMountCache->addMount($user, $mountPoint, $share->getNode()->getData(), MountProvider::class);

apps/files_sharing/lib/ShareTargetValidator.php

@@ -45,14 +45,14 @@ class ShareTargetValidator {
 	/**
 	 * check if the parent folder exists otherwise move the mount point up
 	 *
-	 * @param array<string, ICachedMountInfo> $allCachedMounts Other mounts for the user, indexed by path
+	 * @param callable(string):?ICachedMountInfo $getMountByPath
 	 * @param IShare[] $childShares
 	 * @return string
 	 */
 	public function verifyMountPoint(
 		IUser $user,
 		IShare &$share,
-		array $allCachedMounts,
+		callable $getMountByPath,
 		array $childShares,
 	): string {
 		$mountPoint = basename($share->getTarget());
@@ -94,7 +94,7 @@ class ShareTargetValidator {
 			$share->getNodeId(),
 			Filesystem::normalizePath($absoluteParent . '/' . $mountPoint),
 			$parentMount,
-			$allCachedMounts,
+			$getMountByPath,
 		);
 
 		/** @psalm-suppress InternalMethod */
@@ -112,13 +112,13 @@ class ShareTargetValidator {
 
 
 	/**
-	 * @param ICachedMountInfo[] $allCachedMounts
+	 * @param callable(string):?ICachedMountInfo $getMountByPath
 	 */
 	public function generateUniqueTarget(
 		int $shareNodeId,
 		string $absolutePath,
 		IMountPoint $parentMount,
-		array $allCachedMounts,
+		callable $getMountByPath,
 	): string {
 		$pathInfo = pathinfo($absolutePath);
 		$ext = isset($pathInfo['extension']) ? '.' . $pathInfo['extension'] : '';
@@ -128,7 +128,7 @@ class ShareTargetValidator {
 		$i = 2;
 		$parentCache = $parentMount->getStorage()->getCache();
 		$internalPath = $parentMount->getInternalPath($absolutePath);
-		while ($parentCache->inCache($internalPath) || $this->hasConflictingMount($shareNodeId, $allCachedMounts, $absolutePath)) {
+		while ($parentCache->inCache($internalPath) || $this->hasConflictingMount($shareNodeId, $getMountByPath, $absolutePath)) {
 			$absolutePath = Filesystem::normalizePath($dir . '/' . $name . ' (' . $i . ')' . $ext);
 			$internalPath = $parentMount->getInternalPath($absolutePath);
 			$i++;
@@ -138,14 +138,14 @@ class ShareTargetValidator {
 	}
 
 	/**
-	 * @param ICachedMountInfo[] $allCachedMounts
+	 * @param callable(string):?ICachedMountInfo $getMountByPath
 	 */
-	private function hasConflictingMount(int $shareNodeId, array $allCachedMounts, string $absolutePath): bool {
-		if (!isset($allCachedMounts[$absolutePath . '/'])) {
+	private function hasConflictingMount(int $shareNodeId, callable $getMountByPath, string $absolutePath): bool {
+		$mount = $getMountByPath($absolutePath . '/');
+		if ($mount === null) {
 			return false;
 		}
 
-		$mount = $allCachedMounts[$absolutePath . '/'];
 		if ($mount->getMountProvider() === MountProvider::class && $mount->getRootId() === $shareNodeId) {
 			// "conflicting" mount is a mount for the current share
 			return false;

apps/files_sharing/tests/ShareRecipientUpdaterTest.php

@@ -61,7 +61,7 @@ class ShareRecipientUpdaterTest extends \Test\TestCase {
 			->willReturn([]);
 
 		$this->shareTargetValidator->method('verifyMountPoint')
-			->with($user1, $share, [], [$share])
+			->with($user1, $share, fn ($path) => null, [$share])
 			->willReturn('/new-target');
 
 		$this->userMountCache->expects($this->exactly(1))
@@ -122,7 +122,7 @@ class ShareRecipientUpdaterTest extends \Test\TestCase {
 		$this->setCachedMounts($user1, []);
 
 		$this->shareTargetValidator->method('verifyMountPoint')
-			->with($user1, $share, [], [$share])
+			->with($user1, $share, fn ($path) => null, [$share])
 			->willReturn('/new-target');
 
 		$this->userMountCache->expects($this->exactly(1))

apps/files_sharing/tests/ShareTargetValidatorTest.php

@@ -83,7 +83,7 @@ class ShareTargetValidatorTest extends TestCase {
 		$share = $this->shareManager->getShareById($share->getFullId());
 		$this->assertSame('/foo/bar' . $this->folder, $share->getTarget());
 
-		$this->targetValidator->verifyMountPoint($this->user2, $share, [], [$share]);
+		$this->targetValidator->verifyMountPoint($this->user2, $share, fn ($path) => null, [$share]);
 
 		$share = $this->shareManager->getShareById($share->getFullId());
 		$this->assertSame($this->folder, $share->getTarget());
@@ -117,7 +117,7 @@ class ShareTargetValidatorTest extends TestCase {
 
 		$share = $this->shareManager->getShareById($share->getFullId());
 
-		$this->targetValidator->verifyMountPoint($this->user2, $share, [], [$share]);
+		$this->targetValidator->verifyMountPoint($this->user2, $share, fn ($path) => null, [$share]);
 
 		$share = $this->shareManager->getShareById($share->getFullId());
 		$this->assertSame('/bar (2)', $share->getTarget());
@@ -142,9 +142,10 @@ class ShareTargetValidatorTest extends TestCase {
 		$this->shareManager->acceptShare($share2, self::TEST_FILES_SHARING_API_USER2);
 
 		$conflictingMount = $this->createMock(ICachedMountInfo::class);
-		$this->targetValidator->verifyMountPoint($this->user2, $share2, [
+		$conflictingMounts = [
 			'/' . $this->user2->getUID() . '/files' . $this->folder2 . '/' => $conflictingMount
-		], [$share2]);
+		];
+		$this->targetValidator->verifyMountPoint($this->user2, $share2, fn ($path) => $conflictingMounts[$path] ?? null, [$share2]);
 
 		$share2 = $this->shareManager->getShareById($share2->getFullId());
 
@@ -179,7 +180,7 @@ class ShareTargetValidatorTest extends TestCase {
 		$this->eventDispatcher->addListener(VerifyMountPointEvent::class, function (VerifyMountPointEvent $event): void {
 			$event->setCreateParent(true);
 		});
-		$this->targetValidator->verifyMountPoint($this->user2, $share, [], [$share]);
+		$this->targetValidator->verifyMountPoint($this->user2, $share, fn ($path) => null, [$share]);
 
 		$share = $this->shareManager->getShareById($share->getFullId());
 		$this->assertSame('/foo/bar' . $this->folder, $share->getTarget());

lib/private/Files/Config/UserMountCache.php

@@ -5,6 +5,7 @@
  * SPDX-FileCopyrightText: 2016 ownCloud, Inc.
  * SPDX-License-Identifier: AGPL-3.0-only
  */
+
 namespace OC\Files\Config;
 
 use OC\DB\Exceptions\DbalException;
@@ -33,11 +34,13 @@ class UserMountCache implements IUserMountCache {
 
 	/**
 	 * Cached mount info.
+	 *
 	 * @var CappedMemoryCache<ICachedMountInfo[]>
 	 **/
 	private CappedMemoryCache $mountsForUsers;
 	/**
 	 * fileid => internal path mapping for cached mount info.
+	 *
 	 * @var CappedMemoryCache<string>
 	 **/
 	private CappedMemoryCache $internalPathCache;
@@ -73,7 +76,9 @@ class UserMountCache implements IUserMountCache {
 
 		$cachedMounts = $this->getMountsForUser($user);
 		if (is_array($mountProviderClasses)) {
-			$cachedMounts = array_filter($cachedMounts, function (ICachedMountInfo $mountInfo) use ($mountProviderClasses, $newMounts) {
+			$cachedMounts = array_filter($cachedMounts, function (
+				ICachedMountInfo $mountInfo,
+			) use ($mountProviderClasses, $newMounts) {
 				// for existing mounts that didn't have a mount provider set
 				// we still want the ones that map to new mounts
 				if ($mountInfo->getMountProvider() === '' && isset($newMounts[$mountInfo->getKey()])) {
@@ -536,7 +541,13 @@ class UserMountCache implements IUserMountCache {
 		}
 	}
 
-	public function addMount(IUser $user, string $mountPoint, ICacheEntry $rootCacheEntry, string $mountProvider, ?int $mountId = null): void {
+	public function addMount(
+		IUser $user,
+		string $mountPoint,
+		ICacheEntry $rootCacheEntry,
+		string $mountProvider,
+		?int $mountId = null,
+	): void {
 		$query = $this->connection->getQueryBuilder();
 		$query->insert('mounts')
 			->values([
@@ -567,4 +578,26 @@ class UserMountCache implements IUserMountCache {
 		$this->internalPathCache = new CappedMemoryCache();
 		$this->mountsForUsers = new CappedMemoryCache();
 	}
+
+	public function getMountAtPath(IUser $user, string $mountPoint): ?ICachedMountInfo {
+		if (isset($this->mountsForUsers[$user->getUID()])) {
+			foreach ($this->mountsForUsers[$user->getUID()] as $mount) {
+				if ($mount->getMountPoint() === $mountPoint) {
+					return $mount;
+				}
+			}
+			return null;
+		}
+
+		$builder = $this->connection->getQueryBuilder();
+		$query = $builder->select('storage_id', 'root_id', 'user_id', 'mount_point', 'mount_id', 'f.path', 'mount_provider_class')
+			->from('mounts', 'm')
+			->innerJoin('m', 'filecache', 'f', $builder->expr()->eq('m.root_id', 'f.fileid'))
+			->where($builder->expr()->eq('user_id', $builder->createNamedParameter($user->getUID())))
+			->andWhere($builder->expr()->eq('mount_point_hash', $builder->createNamedParameter(hash('xxh128', $mountPoint))))
+			->setMaxResults(1);
+
+		$row = $query->executeQuery()->fetch();
+		return $row ? $this->dbRowToMountInfo($row) : null;
+	}
 }

lib/public/Files/Config/IUserMountCache.php

@@ -113,7 +113,10 @@ interface IUserMountCache {
 	public function clear(): void;
 
 	/**
-	 * Get all cached mounts for a user
+	 * Get the cached mount for a path
+	 *
+	 * This walks up the directly tree until a mount is found, if you only want
+	 * to get the mount at the specific path, use `getMountAtPath` instead.
 	 *
 	 * @param IUser $user
 	 * @param string $path
@@ -147,4 +150,11 @@ interface IUserMountCache {
 	 * @since 33.0.0
 	 */
 	public function addMount(IUser $user, string $mountPoint, ICacheEntry $rootCacheEntry, string $mountProvider, ?int $mountId = null): void;
+
+	/**
+	 * Get the mount at the specified path, if any
+	 *
+	 * @since 33.0.2
+	 */
+	public function getMountAtPath(IUser $user, string $mountPoint): ?ICachedMountInfo;
 }