Merge pull request #43426 from nextcloud/usermountcache-lazy-path

only get the path for the users cached mount info when we use it

lib/composer/composer/autoload_classmap.php

@@ -1360,6 +1360,7 @@
     'OC\\Files\\Cache\\Wrapper\\JailPropagator' => $baseDir . '/lib/private/Files/Cache/Wrapper/JailPropagator.php',
     'OC\\Files\\Config\\CachedMountFileInfo' => $baseDir . '/lib/private/Files/Config/CachedMountFileInfo.php',
     'OC\\Files\\Config\\CachedMountInfo' => $baseDir . '/lib/private/Files/Config/CachedMountInfo.php',
+    'OC\\Files\\Config\\LazyPathCachedMountInfo' => $baseDir . '/lib/private/Files/Config/LazyPathCachedMountInfo.php',
     'OC\\Files\\Config\\LazyStorageMountInfo' => $baseDir . '/lib/private/Files/Config/LazyStorageMountInfo.php',
     'OC\\Files\\Config\\MountProviderCollection' => $baseDir . '/lib/private/Files/Config/MountProviderCollection.php',
     'OC\\Files\\Config\\UserMountCache' => $baseDir . '/lib/private/Files/Config/UserMountCache.php',

lib/composer/composer/autoload_static.php

@@ -1393,6 +1393,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
         'OC\\Files\\Cache\\Wrapper\\JailPropagator' => __DIR__ . '/../../..' . '/lib/private/Files/Cache/Wrapper/JailPropagator.php',
         'OC\\Files\\Config\\CachedMountFileInfo' => __DIR__ . '/../../..' . '/lib/private/Files/Config/CachedMountFileInfo.php',
         'OC\\Files\\Config\\CachedMountInfo' => __DIR__ . '/../../..' . '/lib/private/Files/Config/CachedMountInfo.php',
+        'OC\\Files\\Config\\LazyPathCachedMountInfo' => __DIR__ . '/../../..' . '/lib/private/Files/Config/LazyPathCachedMountInfo.php',
         'OC\\Files\\Config\\LazyStorageMountInfo' => __DIR__ . '/../../..' . '/lib/private/Files/Config/LazyStorageMountInfo.php',
         'OC\\Files\\Config\\MountProviderCollection' => __DIR__ . '/../../..' . '/lib/private/Files/Config/MountProviderCollection.php',
         'OC\\Files\\Config\\UserMountCache' => __DIR__ . '/../../..' . '/lib/private/Files/Config/UserMountCache.php',

lib/private/Files/Config/LazyPathCachedMountInfo.php

@@ -0,0 +1,63 @@
+<?php
+
+declare(strict_types=1);
+/**
+ * @copyright Copyright (c) 2022 Robin Appelman <robin@icewind.nl>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+namespace OC\Files\Config;
+
+use OCP\IUser;
+
+class LazyPathCachedMountInfo extends CachedMountInfo {
+	// we don't allow \ in paths so it makes a great placeholder
+	private const PATH_PLACEHOLDER = '\\PLACEHOLDER\\';
+
+	/** @var callable(CachedMountInfo): string */
+	protected $rootInternalPathCallback;
+
+	/**
+	 * @param IUser $user
+	 * @param int $storageId
+	 * @param int $rootId
+	 * @param string $mountPoint
+	 * @param string $mountProvider
+	 * @param int|null $mountId
+	 * @param callable(CachedMountInfo): string $rootInternalPathCallback
+	 * @throws \Exception
+	 */
+	public function __construct(
+		IUser $user,
+		int $storageId,
+		int $rootId,
+		string $mountPoint,
+		string $mountProvider,
+		int $mountId = null,
+		callable $rootInternalPathCallback,
+	) {
+		parent::__construct($user, $storageId, $rootId, $mountPoint, $mountProvider, $mountId, self::PATH_PLACEHOLDER);
+		$this->rootInternalPathCallback = $rootInternalPathCallback;
+	}
+
+	public function getRootInternalPath(): string {
+		if ($this->rootInternalPath === self::PATH_PLACEHOLDER) {
+			$this->rootInternalPath = ($this->rootInternalPathCallback)($this);
+		}
+		return $this->rootInternalPath;
+	}
+}

lib/private/Files/Config/UserMountCache.php

@@ -52,6 +52,11 @@ class UserMountCache implements IUserMountCache {
 	 * @var CappedMemoryCache<ICachedMountInfo[]>
 	 **/
 	private CappedMemoryCache $mountsForUsers;
+	/**
+	 * fileid => internal path mapping for cached mount info.
+	 * @var CappedMemoryCache<string>
+	 **/
+	private CappedMemoryCache $internalPathCache;
 	private LoggerInterface $logger;
 	/** @var CappedMemoryCache<array> */
 	private CappedMemoryCache $cacheInfoCache;
@@ -71,6 +76,7 @@ public function __construct(
 		$this->logger = $logger;
 		$this->eventLogger = $eventLogger;
 		$this->cacheInfoCache = new CappedMemoryCache();
+		$this->internalPathCache = new CappedMemoryCache();
 		$this->mountsForUsers = new CappedMemoryCache();
 	}
 
@@ -204,7 +210,12 @@ private function removeFromCache(ICachedMountInfo $mount) {
 		$query->execute();
 	}
 
-	private function dbRowToMountInfo(array $row) {
+	/**
+	 * @param array $row
+	 * @param (callable(CachedMountInfo): string)|null $pathCallback
+	 * @return CachedMountInfo|null
+	 */
+	private function dbRowToMountInfo(array $row, ?callable $pathCallback = null): ?ICachedMountInfo {
 		$user = $this->userManager->get($row['user_id']);
 		if (is_null($user)) {
 			return null;
@@ -213,15 +224,27 @@ private function dbRowToMountInfo(array $row) {
 		if (!is_null($mount_id)) {
 			$mount_id = (int)$mount_id;
 		}
-		return new CachedMountInfo(
-			$user,
-			(int)$row['storage_id'],
-			(int)$row['root_id'],
-			$row['mount_point'],
-			$row['mount_provider_class'] ?? '',
-			$mount_id,
-			$row['path'] ?? '',
-		);
+		if ($pathCallback) {
+			return new LazyPathCachedMountInfo(
+				$user,
+				(int)$row['storage_id'],
+				(int)$row['root_id'],
+				$row['mount_point'],
+				$row['mount_provider_class'] ?? '',
+				$mount_id,
+				$pathCallback,
+			);
+		} else {
+			return new CachedMountInfo(
+				$user,
+				(int)$row['storage_id'],
+				(int)$row['root_id'],
+				$row['mount_point'],
+				$row['mount_provider_class'] ?? '',
+				$mount_id,
+				$row['path'] ?? '',
+			);
+		}
 	}
 
 	/**
@@ -232,27 +255,39 @@ public function getMountsForUser(IUser $user) {
 		$userUID = $user->getUID();
 		if (!isset($this->mountsForUsers[$userUID])) {
 			$builder = $this->connection->getQueryBuilder();
-			$query = $builder->select('storage_id', 'root_id', 'user_id', 'mount_point', 'mount_id', 'f.path', 'mount_provider_class')
+			$query = $builder->select('storage_id', 'root_id', 'user_id', 'mount_point', 'mount_id', '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->createPositionalParameter($userUID)));
 
 			$result = $query->execute();
 			$rows = $result->fetchAll();
 			$result->closeCursor();
 
-			$this->mountsForUsers[$userUID] = [];
 			/** @var array<string, ICachedMountInfo> $mounts */
+			$mounts = [];
 			foreach ($rows as $row) {
-				$mount = $this->dbRowToMountInfo($row);
+				$mount = $this->dbRowToMountInfo($row, [$this, 'getInternalPathForMountInfo']);
 				if ($mount !== null) {
-					$this->mountsForUsers[$userUID][$mount->getKey()] = $mount;
+					$mounts[$mount->getKey()] = $mount;
 				}
 			}
+			$this->mountsForUsers[$userUID] = $mounts;
 		}
 		return $this->mountsForUsers[$userUID];
 	}
 
+	public function getInternalPathForMountInfo(CachedMountInfo $info): string {
+		$cached = $this->internalPathCache->get($info->getRootId());
+		if ($cached !== null) {
+			return $cached;
+		}
+		$builder = $this->connection->getQueryBuilder();
+		$query = $builder->select('path')
+			->from('filecache')
+			->where($builder->expr()->eq('fileid', $builder->createPositionalParameter($info->getRootId())));
+		return $query->executeQuery()->fetchOne() ?: '';
+	}
+
 	/**
 	 * @param int $numericStorageId
 	 * @param string|null $user limit the results to a single user

tests/lib/Files/Config/UserMountCacheTest.php

@@ -84,8 +84,8 @@ protected function tearDown(): void {
 		}
 	}
 
-	private function getStorage($storageId) {
-		$rootId = $this->createCacheEntry('', $storageId);
+	private function getStorage($storageId, $rootInternalPath = '') {
+		$rootId = $this->createCacheEntry($rootInternalPath, $storageId);
 
 		$storageCache = $this->getMockBuilder('\OC\Files\Cache\Storage')
 			->disableOriginalConstructor()
@@ -237,7 +237,7 @@ public function testGetMountsForUser() {
 		$user3 = $this->userManager->get('u3');
 
 		[$storage1, $id1] = $this->getStorage(1);
-		[$storage2, $id2] = $this->getStorage(2);
+		[$storage2, $id2] = $this->getStorage(2, 'foo/bar');
 		$mount1 = new MountPoint($storage1, '/foo/');
 		$mount2 = new MountPoint($storage2, '/bar/');
 
@@ -256,11 +256,13 @@ public function testGetMountsForUser() {
 		$this->assertEquals($user1, $cachedMounts[$this->keyForMount($mount1)]->getUser());
 		$this->assertEquals($id1, $cachedMounts[$this->keyForMount($mount1)]->getRootId());
 		$this->assertEquals(1, $cachedMounts[$this->keyForMount($mount1)]->getStorageId());
+		$this->assertEquals('', $cachedMounts[$this->keyForMount($mount1)]->getRootInternalPath());
 
 		$this->assertEquals('/bar/', $cachedMounts[$this->keyForMount($mount2)]->getMountPoint());
 		$this->assertEquals($user1, $cachedMounts[$this->keyForMount($mount2)]->getUser());
 		$this->assertEquals($id2, $cachedMounts[$this->keyForMount($mount2)]->getRootId());
 		$this->assertEquals(2, $cachedMounts[$this->keyForMount($mount2)]->getStorageId());
+		$this->assertEquals('foo/bar', $cachedMounts[$this->keyForMount($mount2)]->getRootInternalPath());
 
 		$cachedMounts = $this->cache->getMountsForUser($user3);
 		$this->assertEmpty($cachedMounts);