refactor: don't join on filecache in usermountcache

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

lib/private/Files/Config/UserMountCache.php

@@ -26,8 +26,10 @@
  * along with this program. If not, see <http://www.gnu.org/licenses/>
  *
  */
+
 namespace OC\Files\Config;
 
+use OC\Files\Cache\FileAccess;
 use OC\User\LazyUser;
 use OCP\Cache\CappedMemoryCache;
 use OCP\DB\QueryBuilder\IQueryBuilder;
@@ -45,37 +47,31 @@ use Psr\Log\LoggerInterface;
  * Cache mounts points per user in the cache so we can easily look them up
  */
 class UserMountCache implements IUserMountCache {
-	private IDBConnection $connection;
-	private IUserManager $userManager;
-
 	/**
 	 * Cached mount info.
+	 *
 	 * @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;
-	private IEventLogger $eventLogger;
 
 	/**
 	 * UserMountCache constructor.
 	 */
 	public function __construct(
-		IDBConnection $connection,
-		IUserManager $userManager,
-		LoggerInterface $logger,
-		IEventLogger $eventLogger
+		private IDBConnection   $connection,
+		private IUserManager    $userManager,
+		private LoggerInterface $logger,
+		private IEventLogger    $eventLogger,
+		private FileAccess      $cacheAccess,
 	) {
-		$this->connection = $connection;
-		$this->userManager = $userManager;
-		$this->logger = $logger;
-		$this->eventLogger = $eventLogger;
 		$this->cacheInfoCache = new CappedMemoryCache();
 		$this->internalPathCache = new CappedMemoryCache();
 		$this->mountsForUsers = new CappedMemoryCache();
@@ -282,11 +278,8 @@ class UserMountCache implements IUserMountCache {
 		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() ?: '';
+		$entry = $this->cacheAccess->getByFileIdInStorage($info->getRootId(), $info->getStorageId());
+		return $entry ? $entry->getPath() : '';
 	}
 
 	/**
@@ -294,11 +287,10 @@ class UserMountCache implements IUserMountCache {
 	 * @param string|null $user limit the results to a single user
 	 * @return CachedMountInfo[]
 	 */
-	public function getMountsForStorageId($numericStorageId, $user = null) {
+	public function getMountsForStorageId($numericStorageId, $user = null, bool $preloadPaths = false) {
 		$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('storage_id', $builder->createPositionalParameter($numericStorageId, IQueryBuilder::PARAM_INT)));
 
 		if ($user) {
@@ -309,7 +301,21 @@ class UserMountCache implements IUserMountCache {
 		$rows = $result->fetchAll();
 		$result->closeCursor();
 
-		return array_filter(array_map([$this, 'dbRowToMountInfo'], $rows));
+		if ($preloadPaths) {
+			$fileIds = array_map(fn (array $row) => $row['root_id'], $rows);
+			$files = $this->cacheAccess->getByFileIds($fileIds);
+
+			foreach ($rows as &$row) {
+				$mountFileId = $row['root_id'];
+				if (isset($files[$mountFileId])) {
+					$row['path'] = $files[$mountFileId]->getPath();
+				}
+			}
+		}
+
+		return array_filter(array_map(function (array $row) use ($preloadPaths) {
+			return $this->dbRowToMountInfo($row, $preloadPaths ? null : [$this, 'getInternalPathForMountInfo']);
+		}, $rows));
 	}
 
 	/**
@@ -318,45 +324,17 @@ class UserMountCache implements IUserMountCache {
 	 */
 	public function getMountsForRootId($rootFileId) {
 		$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('root_id', $builder->createPositionalParameter($rootFileId, IQueryBuilder::PARAM_INT)));
 
 		$result = $query->execute();
 		$rows = $result->fetchAll();
 		$result->closeCursor();
 
-		return array_filter(array_map([$this, 'dbRowToMountInfo'], $rows));
-	}
-
-	/**
-	 * @param $fileId
-	 * @return array{int, string, int}
-	 * @throws \OCP\Files\NotFoundException
-	 */
-	private function getCacheInfoFromFileId($fileId): array {
-		if (!isset($this->cacheInfoCache[$fileId])) {
-			$builder = $this->connection->getQueryBuilder();
-			$query = $builder->select('storage', 'path', 'mimetype')
-				->from('filecache')
-				->where($builder->expr()->eq('fileid', $builder->createNamedParameter($fileId, IQueryBuilder::PARAM_INT)));
-
-			$result = $query->execute();
-			$row = $result->fetch();
-			$result->closeCursor();
-
-			if (is_array($row)) {
-				$this->cacheInfoCache[$fileId] = [
-					(int)$row['storage'],
-					(string)$row['path'],
-					(int)$row['mimetype']
-				];
-			} else {
-				throw new NotFoundException('File with id "' . $fileId . '" not found');
-			}
-		}
-		return $this->cacheInfoCache[$fileId];
+		return array_filter(array_map(function (array $row) {
+			return $this->dbRowToMountInfo($row, [$this, 'getInternalPathForMountInfo']);
+		}, $rows));
 	}
 
 	/**
@@ -366,12 +344,13 @@ class UserMountCache implements IUserMountCache {
 	 * @since 9.0.0
 	 */
 	public function getMountsForFileId($fileId, $user = null) {
-		try {
-			[$storageId, $internalPath] = $this->getCacheInfoFromFileId($fileId);
-		} catch (NotFoundException $e) {
+		$cacheEntry = $this->cacheAccess->getByFileId($fileId);
+		if (!$cacheEntry) {
 			return [];
 		}
-		$mountsForStorage = $this->getMountsForStorageId($storageId, $user);
+		$internalPath = $cacheEntry->getPath();
+
+		$mountsForStorage = $this->getMountsForStorageId($cacheEntry->getStorageId(), $user, true);
 
 		// filter mounts that are from the same storage but not a parent of the file we care about
 		$filteredMounts = array_filter($mountsForStorage, function (ICachedMountInfo $mount) use ($internalPath, $fileId) {
@@ -449,13 +428,8 @@ class UserMountCache implements IUserMountCache {
 			return $user->getUID();
 		}, $users);
 
-		$query = $builder->select('m.user_id', 'f.size')
+		$query = $builder->select('m.user_id', 'storage_id')
 			->from('mounts', 'm')
-			->innerJoin('m', 'filecache', 'f',
-				$builder->expr()->andX(
-					$builder->expr()->eq('m.storage_id', 'f.storage'),
-					$builder->expr()->eq('f.path_hash', $builder->createNamedParameter(md5('files')))
-				))
 			->where($builder->expr()->eq('m.mount_point', $mountPoint))
 			->andWhere($builder->expr()->in('m.user_id', $builder->createNamedParameter($userIds, IQueryBuilder::PARAM_STR_ARRAY)));
 
@@ -463,12 +437,17 @@ class UserMountCache implements IUserMountCache {
 
 		$results = [];
 		while ($row = $result->fetch()) {
-			$results[$row['user_id']] = $row['size'];
+			$results[$row['user_id']] = $this->getSizeForHomeStorage($row['storage_id']);
 		}
 		$result->closeCursor();
 		return $results;
 	}
 
+	private function getSizeForHomeStorage(int $storage): int {
+		$entry = $this->cacheAccess->getByPathInStorage('files', $storage);
+		return $entry->getSize();
+	}
+
 	public function clear(): void {
 		$this->cacheInfoCache = new CappedMemoryCache();
 		$this->mountsForUsers = new CappedMemoryCache();

tests/lib/Files/Config/UserMountCacheTest.php

@@ -8,11 +8,14 @@
 
 namespace Test\Files\Config;
 
+use OC\DB\Exceptions\DbalException;
 use OC\DB\QueryBuilder\Literal;
+use OC\Files\Cache\FileAccess;
 use OC\Files\Mount\MountPoint;
 use OC\Files\Storage\Storage;
 use OC\User\Manager;
 use OCP\Cache\CappedMemoryCache;
+use OCP\DB\QueryBuilder\IQueryBuilder;
 use OCP\Diagnostics\IEventLogger;
 use OCP\EventDispatcher\IEventDispatcher;
 use OCP\Files\Config\ICachedMountInfo;
@@ -20,6 +23,7 @@ use OCP\ICacheFactory;
 use OCP\IConfig;
 use OCP\IDBConnection;
 use OCP\IUserManager;
+use OCP\Server;
 use Psr\Log\LoggerInterface;
 use Test\TestCase;
 use Test\Util\User\Dummy;
@@ -28,10 +32,8 @@ use Test\Util\User\Dummy;
  * @group DB
  */
 class UserMountCacheTest extends TestCase {
-	/**
-	 * @var IDBConnection
-	 */
-	private $connection;
+	private IDBConnection $connection;
+	private FileAccess $cacheAccess;
 
 	/**
 	 * @var IUserManager
@@ -49,7 +51,8 @@ class UserMountCacheTest extends TestCase {
 		parent::setUp();
 
 		$this->fileIds = [];
-		$this->connection = \OC::$server->getDatabaseConnection();
+		$this->connection = Server::get(IDBConnection::class);
+		$this->cacheAccess = Server::get(FileAccess::class);
 		$config = $this->getMockBuilder(IConfig::class)
 			->disableOriginalConstructor()
 			->getMock();
@@ -67,7 +70,13 @@ class UserMountCacheTest extends TestCase {
 		$userBackend->createUser('u2', '');
 		$userBackend->createUser('u3', '');
 		$this->userManager->registerBackend($userBackend);
-		$this->cache = new \OC\Files\Config\UserMountCache($this->connection, $this->userManager, $this->createMock(LoggerInterface::class), $this->createMock(IEventLogger::class));
+		$this->cache = new \OC\Files\Config\UserMountCache(
+			$this->connection,
+			$this->userManager,
+			$this->createMock(LoggerInterface::class),
+			$this->createMock(IEventLogger::class),
+			$this->cacheAccess,
+		);
 	}
 
 	protected function tearDown(): void {
@@ -336,31 +345,36 @@ class UserMountCacheTest extends TestCase {
 
 	private function createCacheEntry($internalPath, $storageId, $size = 0) {
 		$internalPath = trim($internalPath, '/');
-		$inserted = $this->connection->insertIfNotExist('*PREFIX*filecache', [
-			'storage' => $storageId,
-			'path' => $internalPath,
-			'path_hash' => md5($internalPath),
-			'parent' => -1,
-			'name' => basename($internalPath),
-			'mimetype' => 0,
-			'mimepart' => 0,
-			'size' => $size,
-			'storage_mtime' => 0,
-			'encrypted' => 0,
-			'unencrypted_size' => 0,
-			'etag' => '',
-			'permissions' => 31
-		], ['storage', 'path_hash']);
-		if ($inserted) {
-			$id = (int)$this->connection->lastInsertId('*PREFIX*filecache');
+		$query = $this->connection->getQueryBuilder();
+		$query->insert('filecache')
+			->values([
+				'storage' => $query->createNamedParameter($storageId, IQueryBuilder::PARAM_INT),
+				'path' => $query->createNamedParameter($internalPath),
+				'path_hash' => $query->createNamedParameter(md5($internalPath)),
+				'parent' => $query->createNamedParameter(-1, IQueryBuilder::PARAM_INT),
+				'name' => $query->createNamedParameter(basename($internalPath)),
+				'mimetype' => $query->createNamedParameter(0, IQueryBuilder::PARAM_INT),
+				'mimepart' => $query->createNamedParameter(0, IQueryBuilder::PARAM_INT),
+				'size' => $query->createNamedParameter($size, IQueryBuilder::PARAM_INT),
+				'storage_mtime' => $query->createNamedParameter(0, IQueryBuilder::PARAM_INT),
+				'encrypted' => $query->createNamedParameter(0, IQueryBuilder::PARAM_INT),
+				'unencrypted_size' => $query->createNamedParameter(0, IQueryBuilder::PARAM_INT),
+				'etag' => $query->createNamedParameter(''),
+				'permissions' => $query->createNamedParameter(31, IQueryBuilder::PARAM_INT),
+			]);
+		try {
+			$query->executeStatement();
+			$id = (int)$query->getLastInsertId();
 			$this->fileIds[] = $id;
-		} else {
-			$sql = 'SELECT `fileid` FROM `*PREFIX*filecache` WHERE `storage` = ? AND `path_hash` =?';
-			$query = $this->connection->prepare($sql);
-			$query->execute([$storageId, md5($internalPath)]);
-			return (int)$query->fetchOne();
+			return $id;
+		} catch (DbalException $e) {
+			$query = $this->connection->getQueryBuilder();
+			$query->select('fileid')
+				->from('filecache')
+				->where($query->expr()->eq('storage', $query->createNamedParameter($storageId, IQueryBuilder::PARAM_INT)))
+				->andWhere($query->expr()->eq('path_hash', $query->createNamedParameter(md5($internalPath))));
+			return (int)$query->executeQuery()->fetchOne();
 		}
-		return $id;
 	}
 
 	public function testGetMountsForFileIdRootId() {