From 845b78086bfe117e87253fdefb12e8e19a57c640 Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Mon, 31 Mar 2025 13:41:53 +0200 Subject: [PATCH 01/18] feat(IFileAccess#getByAncestorInStorage): Add new method to retrieve all files in a mount Signed-off-by: Marcel Klehr --- lib/private/Files/Cache/FileAccess.php | 67 ++++++++++++++++++++++++++ lib/public/Files/Cache/IFileAccess.php | 20 ++++++++ 2 files changed, 87 insertions(+) diff --git a/lib/private/Files/Cache/FileAccess.php b/lib/private/Files/Cache/FileAccess.php index 11a95b5d897..6835fb948d6 100644 --- a/lib/private/Files/Cache/FileAccess.php +++ b/lib/private/Files/Cache/FileAccess.php @@ -10,6 +10,7 @@ namespace OC\Files\Cache; use OC\FilesMetadata\FilesMetadataManager; use OC\SystemConfig; +use OCP\DB\Exception; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\Files\Cache\IFileAccess; use OCP\Files\IMimeTypeLoader; @@ -94,4 +95,70 @@ class FileAccess implements IFileAccess { $rows = $query->executeQuery()->fetchAll(); return $this->rowsToEntries($rows); } + + /** + * Retrieves files stored in a specific storage that have a specified ancestor in the file hierarchy. + * Allows filtering by mime types, encryption status, and limits the number of results. + * + * @param int $storageId The ID of the storage to search within. + * @param int $rootId The file ID of the ancestor to base the search on. + * @param int $lastFileId The last processed file ID. Only files with a higher ID will be included. Defaults to 0. + * @param array $mimeTypes An array of mime types to filter the results. If empty, no mime type filtering will be applied. + * @param bool $endToEndEncrypted Whether to include EndToEndEncrypted files + * @param bool $serverSideEncrypted Whether to include ServerSideEncrypted files + * @param int $maxResults The maximum number of results to retrieve. If set to 0, all matching files will be retrieved. + * @return \Generator A generator yielding matching files as cache entries. + * @throws Exception + */ + public function getByAncestorInStorage(int $storageId, int $rootId, int $lastFileId = 0, array $mimeTypes = [], bool $endToEndEncrypted = true, bool $serverSideEncrypted = true, int $maxResults = 100): \Generator { + $qb = $this->getQuery(); + $qb->selectFileCache(); + $qb->andWhere($qb->expr()->eq('filecache.fileid', $qb->createNamedParameter($rootId, IQueryBuilder::PARAM_INT))); + $result = $qb->executeQuery(); + /** @var array{path:string}|false $root */ + $root = $result->fetch(); + $result->closeCursor(); + + if ($root === false) { + throw new Exception('Could not fetch storage root'); + } + + $qb = $this->getQuery(); + + $path = $root['path'] === '' ? '' : $root['path'] . '/'; + + $qb->select('*') + ->from('filecache', 'filecache') + ->andWhere($qb->expr()->like('filecache.path', $qb->createNamedParameter($path . '%'))) + ->andWhere($qb->expr()->eq('filecache.storage', $qb->createNamedParameter($storageId))) + ->andWhere($qb->expr()->gt('filecache.fileid', $qb->createNamedParameter($lastFileId))); + + if (!$endToEndEncrypted) { + $qb->innerJoin('filecache', 'filecache', 'p', $qb->expr()->eq('filecache.parent', 'p.fileid')); + $qb->andWhere($qb->expr()->eq('p.encrypted', $qb->createNamedParameter(0, IQueryBuilder::PARAM_INT))); + } + + if (!$serverSideEncrypted) { + $qb->andWhere($qb->expr()->eq('filecache.encrypted', $qb->createNamedParameter(0, IQueryBuilder::PARAM_INT))); + } + + if (count($mimeTypes) > 0) { + $qb->andWhere($qb->expr()->in('filecache.mimetype', $qb->createNamedParameter($mimeTypes, IQueryBuilder::PARAM_INT_ARRAY))); + } + + if ($maxResults !== 0) { + $qb->setMaxResults($maxResults); + } + $files = $qb->orderBy('filecache.fileid', 'ASC') + ->executeQuery(); + + while ( + /** @var array */ + $row = $files->fetch() + ) { + yield Cache::cacheEntryFromData($row, $this->mimeTypeLoader); + } + + $files->closeCursor(); + } } diff --git a/lib/public/Files/Cache/IFileAccess.php b/lib/public/Files/Cache/IFileAccess.php index 51945b55a25..2a7cf2b27ae 100644 --- a/lib/public/Files/Cache/IFileAccess.php +++ b/lib/public/Files/Cache/IFileAccess.php @@ -8,6 +8,8 @@ declare(strict_types=1); */ namespace OCP\Files\Cache; +use OCP\DB\Exception; + /** * Low level access to the file cache. * @@ -79,4 +81,22 @@ interface IFileAccess { * @since 29.0.0 */ public function getByFileIdsInStorage(array $fileIds, int $storageId): array; + + /** + * Retrieves files stored in a specific storage that have a specified ancestor in the file hierarchy. + * Allows filtering by mime types, encryption status, and limits the number of results. + * + * @param int $storageId The ID of the storage to search within. + * @param int $rootId The file ID of the ancestor to base the search on. + * @param int $lastFileId The last processed file ID. Only files with a higher ID will be included. Defaults to 0. + * @param array $mimeTypes An array of mime types to filter the results. If empty, no mime type filtering will be applied. + * @param bool $endToEndEncrypted Whether to include EndToEndEncrypted files + * @param bool $serverSideEncrypted Whether to include ServerSideEncrypted files + * @param int $maxResults The maximum number of results to retrieve. If set to 0, all matching files will be retrieved. + * @return \Generator A generator yielding matching files as cache entries. + * @throws Exception + * + * @since 32.0.0 + */ + public function getByAncestorInStorage(int $storageId, int $rootId, int $lastFileId = 0, array $mimeTypes = [], bool $endToEndEncrypted = true, bool $serverSideEncrypted = true, int $maxResults = 100): \Generator; } From 3eef614769bec38ffe989f0c8f3f6f1824f21e29 Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Mon, 31 Mar 2025 15:35:36 +0200 Subject: [PATCH 02/18] feat(IFileAccess#getMounts): Add new method to retrieve all distinct mounts Signed-off-by: Marcel Klehr --- lib/private/Files/Cache/FileAccess.php | 63 +++++++++++++++++++++++++- lib/public/Files/Cache/IFileAccess.php | 15 ++++++ 2 files changed, 77 insertions(+), 1 deletion(-) diff --git a/lib/private/Files/Cache/FileAccess.php b/lib/private/Files/Cache/FileAccess.php index 6835fb948d6..863f17437c1 100644 --- a/lib/private/Files/Cache/FileAccess.php +++ b/lib/private/Files/Cache/FileAccess.php @@ -103,7 +103,7 @@ class FileAccess implements IFileAccess { * @param int $storageId The ID of the storage to search within. * @param int $rootId The file ID of the ancestor to base the search on. * @param int $lastFileId The last processed file ID. Only files with a higher ID will be included. Defaults to 0. - * @param array $mimeTypes An array of mime types to filter the results. If empty, no mime type filtering will be applied. + * @param list $mimeTypes An array of mime types to filter the results. If empty, no mime type filtering will be applied. * @param bool $endToEndEncrypted Whether to include EndToEndEncrypted files * @param bool $serverSideEncrypted Whether to include ServerSideEncrypted files * @param int $maxResults The maximum number of results to retrieve. If set to 0, all matching files will be retrieved. @@ -161,4 +161,65 @@ class FileAccess implements IFileAccess { $files->closeCursor(); } + + /** + * Retrieves a list of all distinct mounts. + * Allows filtering by specific mount providers and excluding certain mount points. + * Optionally rewrites home directory root paths to avoid cache and trashbin. + * + * @param list $mountProviders An array of mount provider class names to filter. If empty, all providers will be included. + * @param string|false $excludeMountPoints A string pattern to exclude mount points. Set to false to not exclude any mount points. + * @param bool $rewriteHomeDirectories Whether to rewrite the root path IDs for home directories to only include user files. + * @return \Generator A generator yielding mount configurations as an array containing 'storage_id', 'root_id', and 'override_root'. + * @throws Exception + */ + public function getDistinctMounts(array $mountProviders = [], string|false $excludeMountPoints = false, bool $rewriteHomeDirectories = true): \Generator { + $qb = $this->connection->getQueryBuilder(); + $qb->selectDistinct(['root_id', 'storage_id', 'mount_provider_class']) + ->from('mounts'); + if (count($mountProviders) > 0) { + $qb->where($qb->expr()->in('mount_provider_class', $qb->createPositionalParameter($mountProviders, IQueryBuilder::PARAM_STR_ARRAY))); + } + if ($excludeMountPoints !== false) { + $qb->andWhere($qb->expr()->notLike('mount_point', $qb->createPositionalParameter($excludeMountPoints))); + } + $result = $qb->executeQuery(); + + + while ( + /** @var array{storage_id:int, root_id:int,mount_provider_class:string} $row */ + $row = $result->fetch() + ) { + $storageId = (int)$row['storage_id']; + $rootId = (int)$row['root_id']; + $overrideRoot = $rootId; + if (in_array($row['mount_provider_class'], [ + OC\Files\Mount\LocalHomeMountProvider::class, + OC\Files\Mount\ObjectHomeMountProvider::class, + ])) { + // Only crawl files, not cache or trashbin + $qb = $this->getQuery(); + try { + $qb->selectFileCache(); + /** @var array|false $root */ + $root = $qb + ->andWhere($qb->expr()->eq('filecache.storage', $qb->createNamedParameter($storageId, IQueryBuilder::PARAM_INT))) + ->andWhere($qb->expr()->eq('filecache.path', $qb->createNamedParameter('files'))) + ->executeQuery()->fetch(); + if ($root !== false) { + $overrideRoot = intval($root['fileid']); + } + } catch (Exception $e) { + $this->logger->error('Could not fetch home storage files root for storage ' . $storageId, ['exception' => $e]); + continue; + } + } + yield [ + 'storage_id' => $storageId, + 'root_id' => $rootId, + 'override_root' => $overrideRoot, + ]; + } + $result->closeCursor(); + } } diff --git a/lib/public/Files/Cache/IFileAccess.php b/lib/public/Files/Cache/IFileAccess.php index 2a7cf2b27ae..4e819fcf942 100644 --- a/lib/public/Files/Cache/IFileAccess.php +++ b/lib/public/Files/Cache/IFileAccess.php @@ -99,4 +99,19 @@ interface IFileAccess { * @since 32.0.0 */ public function getByAncestorInStorage(int $storageId, int $rootId, int $lastFileId = 0, array $mimeTypes = [], bool $endToEndEncrypted = true, bool $serverSideEncrypted = true, int $maxResults = 100): \Generator; + + /** + * Retrieves a list of all distinct mounts. + * Allows filtering by specific mount providers and excluding certain mount points. + * Optionally rewrites home directory root paths to avoid cache and trashbin. + * + * @param list $mountProviders An array of mount provider class names to filter. If empty, all providers will be included. + * @param string|false $excludeMountPoints A string pattern to exclude mount points. Set to false to not exclude any mount points. + * @param bool $rewriteHomeDirectories Whether to rewrite the root path IDs for home directories to only include user files. + * @return \Generator A generator yielding mount configurations as an array containing 'storage_id', 'root_id', and 'override_root'. + * @throws Exception + * + * @since 32.0.0 + */ + public function getDistinctMounts(array $mountProviders = [], string|false $excludeMountPoints = false, bool $rewriteHomeDirectories = true): \Generator; } From 611d83aa6a98c9ac274b6f15a90427dcf75d3728 Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Mon, 31 Mar 2025 15:55:38 +0200 Subject: [PATCH 03/18] fix: Fix psalm issues Signed-off-by: Marcel Klehr --- lib/private/Files/Cache/FileAccess.php | 4 ++-- lib/public/Files/Cache/IFileAccess.php | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/private/Files/Cache/FileAccess.php b/lib/private/Files/Cache/FileAccess.php index 863f17437c1..a71e6e99d9e 100644 --- a/lib/private/Files/Cache/FileAccess.php +++ b/lib/private/Files/Cache/FileAccess.php @@ -194,8 +194,8 @@ class FileAccess implements IFileAccess { $rootId = (int)$row['root_id']; $overrideRoot = $rootId; if (in_array($row['mount_provider_class'], [ - OC\Files\Mount\LocalHomeMountProvider::class, - OC\Files\Mount\ObjectHomeMountProvider::class, + \OC\Files\Mount\LocalHomeMountProvider::class, + \OC\Files\Mount\ObjectHomeMountProvider::class, ])) { // Only crawl files, not cache or trashbin $qb = $this->getQuery(); diff --git a/lib/public/Files/Cache/IFileAccess.php b/lib/public/Files/Cache/IFileAccess.php index 4e819fcf942..734af8b6126 100644 --- a/lib/public/Files/Cache/IFileAccess.php +++ b/lib/public/Files/Cache/IFileAccess.php @@ -89,7 +89,7 @@ interface IFileAccess { * @param int $storageId The ID of the storage to search within. * @param int $rootId The file ID of the ancestor to base the search on. * @param int $lastFileId The last processed file ID. Only files with a higher ID will be included. Defaults to 0. - * @param array $mimeTypes An array of mime types to filter the results. If empty, no mime type filtering will be applied. + * @param list $mimeTypes An array of mime types to filter the results. If empty, no mime type filtering will be applied. * @param bool $endToEndEncrypted Whether to include EndToEndEncrypted files * @param bool $serverSideEncrypted Whether to include ServerSideEncrypted files * @param int $maxResults The maximum number of results to retrieve. If set to 0, all matching files will be retrieved. From d8c6f8d18ecac7cc1bb5e42b8c31d64e41b99f52 Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Tue, 1 Apr 2025 10:39:32 +0200 Subject: [PATCH 04/18] fix(FileAccess): Address review comments Signed-off-by: Marcel Klehr --- lib/private/Files/Cache/FileAccess.php | 16 +++++++++------- lib/public/Files/Cache/IFileAccess.php | 6 +++--- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/lib/private/Files/Cache/FileAccess.php b/lib/private/Files/Cache/FileAccess.php index a71e6e99d9e..c212c193a43 100644 --- a/lib/private/Files/Cache/FileAccess.php +++ b/lib/private/Files/Cache/FileAccess.php @@ -108,7 +108,7 @@ class FileAccess implements IFileAccess { * @param bool $serverSideEncrypted Whether to include ServerSideEncrypted files * @param int $maxResults The maximum number of results to retrieve. If set to 0, all matching files will be retrieved. * @return \Generator A generator yielding matching files as cache entries. - * @throws Exception + * @throws \OCP\DB\Exception */ public function getByAncestorInStorage(int $storageId, int $rootId, int $lastFileId = 0, array $mimeTypes = [], bool $endToEndEncrypted = true, bool $serverSideEncrypted = true, int $maxResults = 100): \Generator { $qb = $this->getQuery(); @@ -134,11 +134,13 @@ class FileAccess implements IFileAccess { ->andWhere($qb->expr()->gt('filecache.fileid', $qb->createNamedParameter($lastFileId))); if (!$endToEndEncrypted) { + // End to end encrypted files are descendants of a folder with encrypted=1 $qb->innerJoin('filecache', 'filecache', 'p', $qb->expr()->eq('filecache.parent', 'p.fileid')); $qb->andWhere($qb->expr()->eq('p.encrypted', $qb->createNamedParameter(0, IQueryBuilder::PARAM_INT))); } if (!$serverSideEncrypted) { + // Server side encrypted files have encrypted=1 directly $qb->andWhere($qb->expr()->eq('filecache.encrypted', $qb->createNamedParameter(0, IQueryBuilder::PARAM_INT))); } @@ -168,10 +170,10 @@ class FileAccess implements IFileAccess { * Optionally rewrites home directory root paths to avoid cache and trashbin. * * @param list $mountProviders An array of mount provider class names to filter. If empty, all providers will be included. - * @param string|false $excludeMountPoints A string pattern to exclude mount points. Set to false to not exclude any mount points. + * @param string|false $excludeMountPoints A string containing an SQL LIKE pattern to exclude mount points. Set to false to not exclude any mount points. * @param bool $rewriteHomeDirectories Whether to rewrite the root path IDs for home directories to only include user files. * @return \Generator A generator yielding mount configurations as an array containing 'storage_id', 'root_id', and 'override_root'. - * @throws Exception + * @throws \OCP\DB\Exception */ public function getDistinctMounts(array $mountProviders = [], string|false $excludeMountPoints = false, bool $rewriteHomeDirectories = true): \Generator { $qb = $this->connection->getQueryBuilder(); @@ -193,10 +195,10 @@ class FileAccess implements IFileAccess { $storageId = (int)$row['storage_id']; $rootId = (int)$row['root_id']; $overrideRoot = $rootId; - if (in_array($row['mount_provider_class'], [ - \OC\Files\Mount\LocalHomeMountProvider::class, - \OC\Files\Mount\ObjectHomeMountProvider::class, - ])) { + if ($rewriteHomeDirectories && in_array($row['mount_provider_class'], [ + \OC\Files\Mount\LocalHomeMountProvider::class, + \OC\Files\Mount\ObjectHomeMountProvider::class, + ], true)) { // Only crawl files, not cache or trashbin $qb = $this->getQuery(); try { diff --git a/lib/public/Files/Cache/IFileAccess.php b/lib/public/Files/Cache/IFileAccess.php index 734af8b6126..021befa188d 100644 --- a/lib/public/Files/Cache/IFileAccess.php +++ b/lib/public/Files/Cache/IFileAccess.php @@ -94,7 +94,7 @@ interface IFileAccess { * @param bool $serverSideEncrypted Whether to include ServerSideEncrypted files * @param int $maxResults The maximum number of results to retrieve. If set to 0, all matching files will be retrieved. * @return \Generator A generator yielding matching files as cache entries. - * @throws Exception + * @throws \OCP\DB\Exception * * @since 32.0.0 */ @@ -106,10 +106,10 @@ interface IFileAccess { * Optionally rewrites home directory root paths to avoid cache and trashbin. * * @param list $mountProviders An array of mount provider class names to filter. If empty, all providers will be included. - * @param string|false $excludeMountPoints A string pattern to exclude mount points. Set to false to not exclude any mount points. + * @param string|false $excludeMountPoints A string containing an SQL LIKE pattern to exclude mount points. Set to false to not exclude any mount points. * @param bool $rewriteHomeDirectories Whether to rewrite the root path IDs for home directories to only include user files. * @return \Generator A generator yielding mount configurations as an array containing 'storage_id', 'root_id', and 'override_root'. - * @throws Exception + * @throws \OCP\DB\Exception * * @since 32.0.0 */ From 5689af5a94a2b8c6a2a74dd90f8c8a3fed1fd28f Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Tue, 1 Apr 2025 10:50:55 +0200 Subject: [PATCH 05/18] fix(FileAccess): Run cs:fix Signed-off-by: Marcel Klehr --- lib/private/Files/Cache/FileAccess.php | 6 +++--- lib/public/Files/Cache/IFileAccess.php | 2 -- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/lib/private/Files/Cache/FileAccess.php b/lib/private/Files/Cache/FileAccess.php index c212c193a43..a57fa42dd93 100644 --- a/lib/private/Files/Cache/FileAccess.php +++ b/lib/private/Files/Cache/FileAccess.php @@ -196,9 +196,9 @@ class FileAccess implements IFileAccess { $rootId = (int)$row['root_id']; $overrideRoot = $rootId; if ($rewriteHomeDirectories && in_array($row['mount_provider_class'], [ - \OC\Files\Mount\LocalHomeMountProvider::class, - \OC\Files\Mount\ObjectHomeMountProvider::class, - ], true)) { + \OC\Files\Mount\LocalHomeMountProvider::class, + \OC\Files\Mount\ObjectHomeMountProvider::class, + ], true)) { // Only crawl files, not cache or trashbin $qb = $this->getQuery(); try { diff --git a/lib/public/Files/Cache/IFileAccess.php b/lib/public/Files/Cache/IFileAccess.php index 021befa188d..be66c5a9a09 100644 --- a/lib/public/Files/Cache/IFileAccess.php +++ b/lib/public/Files/Cache/IFileAccess.php @@ -8,8 +8,6 @@ declare(strict_types=1); */ namespace OCP\Files\Cache; -use OCP\DB\Exception; - /** * Low level access to the file cache. * From d67c877ac51f762b65de86b8e18b52e5d1a8d32f Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Tue, 1 Apr 2025 17:56:05 +0200 Subject: [PATCH 06/18] fix(FileAccess): Add tests Signed-off-by: Marcel Klehr --- lib/private/Files/Cache/FileAccess.php | 18 +- tests/lib/Files/Cache/FileAccessTest.php | 438 +++++++++++++++++++++++ 2 files changed, 447 insertions(+), 9 deletions(-) create mode 100644 tests/lib/Files/Cache/FileAccessTest.php diff --git a/lib/private/Files/Cache/FileAccess.php b/lib/private/Files/Cache/FileAccess.php index a57fa42dd93..2ddd18f1601 100644 --- a/lib/private/Files/Cache/FileAccess.php +++ b/lib/private/Files/Cache/FileAccess.php @@ -127,31 +127,31 @@ class FileAccess implements IFileAccess { $path = $root['path'] === '' ? '' : $root['path'] . '/'; - $qb->select('*') - ->from('filecache', 'filecache') - ->andWhere($qb->expr()->like('filecache.path', $qb->createNamedParameter($path . '%'))) - ->andWhere($qb->expr()->eq('filecache.storage', $qb->createNamedParameter($storageId))) - ->andWhere($qb->expr()->gt('filecache.fileid', $qb->createNamedParameter($lastFileId))); + $qb->selectDistinct('*') + ->from('filecache', 'f') + ->andWhere($qb->expr()->like('f.path', $qb->createNamedParameter($path . '%'))) + ->andWhere($qb->expr()->eq('f.storage', $qb->createNamedParameter($storageId))) + ->andWhere($qb->expr()->gt('f.fileid', $qb->createNamedParameter($lastFileId))); if (!$endToEndEncrypted) { // End to end encrypted files are descendants of a folder with encrypted=1 - $qb->innerJoin('filecache', 'filecache', 'p', $qb->expr()->eq('filecache.parent', 'p.fileid')); + $qb->leftJoin('f', 'filecache', 'p', $qb->expr()->eq('f.parent', 'p.fileid')); $qb->andWhere($qb->expr()->eq('p.encrypted', $qb->createNamedParameter(0, IQueryBuilder::PARAM_INT))); } if (!$serverSideEncrypted) { // Server side encrypted files have encrypted=1 directly - $qb->andWhere($qb->expr()->eq('filecache.encrypted', $qb->createNamedParameter(0, IQueryBuilder::PARAM_INT))); + $qb->andWhere($qb->expr()->eq('f.encrypted', $qb->createNamedParameter(0, IQueryBuilder::PARAM_INT))); } if (count($mimeTypes) > 0) { - $qb->andWhere($qb->expr()->in('filecache.mimetype', $qb->createNamedParameter($mimeTypes, IQueryBuilder::PARAM_INT_ARRAY))); + $qb->andWhere($qb->expr()->in('f.mimetype', $qb->createNamedParameter($mimeTypes, IQueryBuilder::PARAM_INT_ARRAY))); } if ($maxResults !== 0) { $qb->setMaxResults($maxResults); } - $files = $qb->orderBy('filecache.fileid', 'ASC') + $files = $qb->orderBy('f.fileid', 'ASC') ->executeQuery(); while ( diff --git a/tests/lib/Files/Cache/FileAccessTest.php b/tests/lib/Files/Cache/FileAccessTest.php new file mode 100644 index 00000000000..2078412ad73 --- /dev/null +++ b/tests/lib/Files/Cache/FileAccessTest.php @@ -0,0 +1,438 @@ +dbConnection = \OC::$server->get(IDBConnection::class); + + // Ensure FileAccess is instantiated with the real connection + $this->fileAccess = new FileAccess( + $this->dbConnection, + \OC::$server->get(\OC\SystemConfig::class), + \OC::$server->get(LoggerInterface::class), + \OC::$server->get(\OC\FilesMetadata\FilesMetadataManager::class), + \OC::$server->get(\OCP\Files\IMimeTypeLoader::class) + ); + + // Clear and prepare `filecache` table for tests + $queryBuilder = $this->dbConnection->getQueryBuilder(); + $queryBuilder->delete('filecache')->executeStatement(); + + // Clean up potential leftovers from other tests + $queryBuilder = $this->dbConnection->getQueryBuilder(); + $queryBuilder->delete('mounts')->executeStatement(); + + + $this->setUpTestDatabaseForGetDistinctMounts(); + $this->setUpTestDatabaseForGetByAncestorInStorage(); + } + + private function setUpTestDatabaseForGetDistinctMounts(): void { + $queryBuilder = $this->dbConnection->getQueryBuilder(); + + // Insert test data + $queryBuilder->insert('mounts') + ->values([ + 'storage_id' => $queryBuilder->createNamedParameter(1, IQueryBuilder::PARAM_INT), + 'root_id' => $queryBuilder->createNamedParameter(10, IQueryBuilder::PARAM_INT), + 'mount_provider_class' => $queryBuilder->createNamedParameter('TestProviderClass1'), + 'mount_point' => $queryBuilder->createNamedParameter('/files'), + 'user_id' => $queryBuilder->createNamedParameter('test'), + ]) + ->executeStatement(); + + $queryBuilder->insert('mounts') + ->values([ + 'storage_id' => $queryBuilder->createNamedParameter(2, IQueryBuilder::PARAM_INT), + 'root_id' => $queryBuilder->createNamedParameter(20, IQueryBuilder::PARAM_INT), + 'mount_provider_class' => $queryBuilder->createNamedParameter('TestProviderClass2'), + 'mount_point' => $queryBuilder->createNamedParameter('/cache'), + 'user_id' => $queryBuilder->createNamedParameter('test'), + ]) + ->executeStatement(); + + $queryBuilder->insert('mounts') + ->values([ + 'storage_id' => $queryBuilder->createNamedParameter(3, IQueryBuilder::PARAM_INT), + 'root_id' => $queryBuilder->createNamedParameter(30, IQueryBuilder::PARAM_INT), + 'mount_provider_class' => $queryBuilder->createNamedParameter('TestProviderClass1'), + 'mount_point' => $queryBuilder->createNamedParameter('/trashbin'), + 'user_id' => $queryBuilder->createNamedParameter('test'), + ]) + ->executeStatement(); + } + + /** + * Test that getDistinctMounts returns all mounts without filters + */ + public function testGetDistinctMountsWithoutFilters(): void { + $result = iterator_to_array($this->fileAccess->getDistinctMounts()); + + $this->assertCount(3, $result); + + $this->assertEquals([ + 'storage_id' => 1, + 'root_id' => 10, + 'override_root' => 10, + ], $result[0]); + + $this->assertEquals([ + 'storage_id' => 2, + 'root_id' => 20, + 'override_root' => 20, + ], $result[1]); + + $this->assertEquals([ + 'storage_id' => 3, + 'root_id' => 30, + 'override_root' => 30, + ], $result[2]); + } + + /** + * Test that getDistinctMounts applies filtering by mount providers + */ + public function testGetDistinctMountsWithMountProviderFilter(): void { + $result = iterator_to_array($this->fileAccess->getDistinctMounts(['TestProviderClass1'])); + + $this->assertCount(2, $result); + + $this->assertEquals([ + 'storage_id' => 1, + 'root_id' => 10, + 'override_root' => 10, + ], $result[0]); + + $this->assertEquals([ + 'storage_id' => 3, + 'root_id' => 30, + 'override_root' => 30, + ], $result[1]); + } + + /** + * Test that getDistinctMounts excludes certain mount points + */ + public function testGetDistinctMountsWithExclusionFilter(): void { + $result = iterator_to_array($this->fileAccess->getDistinctMounts([], '/cache')); + + $this->assertCount(2, $result); + + $this->assertEquals([ + 'storage_id' => 1, + 'root_id' => 10, + 'override_root' => 10, + ], $result[0]); + + $this->assertEquals([ + 'storage_id' => 3, + 'root_id' => 30, + 'override_root' => 30, + ], $result[1]); + } + + /** + * Test that getDistinctMounts rewrites home directory paths + */ + public function testGetDistinctMountsWithRewriteHomeDirectories(): void { + // Add additional test data for a home directory mount + $queryBuilder = $this->dbConnection->getQueryBuilder(); + $queryBuilder->insert('mounts') + ->values([ + 'storage_id' => $queryBuilder->createNamedParameter(4, IQueryBuilder::PARAM_INT), + 'root_id' => $queryBuilder->createNamedParameter(40, IQueryBuilder::PARAM_INT), + 'mount_provider_class' => $queryBuilder->createNamedParameter(\OC\Files\Mount\LocalHomeMountProvider::class), + 'mount_point' => $queryBuilder->createNamedParameter('/home/user'), + 'user_id' => $queryBuilder->createNamedParameter('test'), + ]) + ->executeStatement(); + + // Simulate adding a "files" directory to the filecache table + $queryBuilder->delete('filecache')->executeStatement(); + $queryBuilder->insert('filecache') + ->values([ + 'fileid' => $queryBuilder->createNamedParameter(99, IQueryBuilder::PARAM_INT), + 'storage' => $queryBuilder->createNamedParameter(4, IQueryBuilder::PARAM_INT), + 'path' => $queryBuilder->createNamedParameter('files'), + ]) + ->executeStatement(); + + $result = iterator_to_array($this->fileAccess->getDistinctMounts()); + + $this->assertEquals([ + 'storage_id' => 4, + 'root_id' => 40, + 'override_root' => 99, + ], end($result)); + } + + private function setUpTestDatabaseForGetByAncestorInStorage(): void { + // prepare `filecache` table for tests + $queryBuilder = $this->dbConnection->getQueryBuilder(); + + $queryBuilder->insert('filecache') + ->values([ + 'fileid' => 1, + 'parent' => 0, + 'path' => $queryBuilder->createNamedParameter('files'), + 'path_hash' => $queryBuilder->createNamedParameter(md5('files')), + 'storage' => $queryBuilder->createNamedParameter(1), + 'name' => $queryBuilder->createNamedParameter('files'), + 'mimetype' => 1, + 'encrypted' => 0, + ]) + ->executeStatement(); + + $queryBuilder->insert('filecache') + ->values([ + 'fileid' => 2, + 'parent' => 1, + 'path' => $queryBuilder->createNamedParameter('files/documents'), + 'path_hash' => $queryBuilder->createNamedParameter(md5('files/documents')), + 'storage' => $queryBuilder->createNamedParameter(1), + 'name' => $queryBuilder->createNamedParameter('documents'), + 'mimetype' => 2, + 'encrypted' => 1, + ]) + ->executeStatement(); + + $queryBuilder->insert('filecache') + ->values([ + 'fileid' => 3, + 'parent' => 1, + 'path' => $queryBuilder->createNamedParameter('files/photos'), + 'path_hash' => $queryBuilder->createNamedParameter(md5('files/photos')), + 'storage' => $queryBuilder->createNamedParameter(1), + 'name' => $queryBuilder->createNamedParameter('photos'), + 'mimetype' => 3, + 'encrypted' => 1, + ]) + ->executeStatement(); + + $queryBuilder->insert('filecache') + ->values([ + 'fileid' => 4, + 'parent' => 3, + 'path' => $queryBuilder->createNamedParameter('files/photos/endtoendencrypted'), + 'path_hash' => $queryBuilder->createNamedParameter(md5('files/photos/endtoendencrypted')), + 'storage' => $queryBuilder->createNamedParameter(1), + 'name' => $queryBuilder->createNamedParameter('endtoendencrypted'), + 'mimetype' => 4, + 'encrypted' => 0, + ]) + ->executeStatement(); + + $queryBuilder->insert('filecache') + ->values([ + 'fileid' => 5, + 'parent' => 0, + 'path' => $queryBuilder->createNamedParameter('files/serversideencrypted'), + 'path_hash' => $queryBuilder->createNamedParameter(md5('files/serversideencrypted')), + 'storage' => $queryBuilder->createNamedParameter(1), + 'name' => $queryBuilder->createNamedParameter('serversideencrypted'), + 'mimetype' => 4, + 'encrypted' => 1, + ]) + ->executeStatement(); + + $queryBuilder->insert('filecache') + ->values([ + 'fileid' => 6, + 'parent' => 0, + 'path' => $queryBuilder->createNamedParameter('files/storage2'), + 'path_hash' => $queryBuilder->createNamedParameter(md5('files/storage2')), + 'storage' => $queryBuilder->createNamedParameter(2), + 'name' => $queryBuilder->createNamedParameter('storage2'), + 'mimetype' => 5, + 'encrypted' => 0, + ]) + ->executeStatement(); + + $queryBuilder->insert('filecache') + ->values([ + 'fileid' => 7, + 'parent' => 6, + 'path' => $queryBuilder->createNamedParameter('files/storage2/file'), + 'path_hash' => $queryBuilder->createNamedParameter(md5('files/storage2/file')), + 'storage' => $queryBuilder->createNamedParameter(2), + 'name' => $queryBuilder->createNamedParameter('file'), + 'mimetype' => 6, + 'encrypted' => 0, + ]) + ->executeStatement(); + } + + /** + * Test fetching files by ancestor in storage. + */ + public function testGetByAncestorInStorage(): void { + $generator = $this->fileAccess->getByAncestorInStorage( + 1, // storageId + 1, // rootId + 0, // lastFileId + [], // mimeTypes + true, // include end-to-end encrypted files + true, // include server-side encrypted files + 10 // maxResults + ); + + $result = iterator_to_array($generator); + + $this->assertCount(4, $result); + + $paths = array_map(fn(CacheEntry $entry) => $entry->getPath(), $result); + $this->assertEquals([ + 'files/documents', + 'files/photos', + 'files/photos/endtoendencrypted', + 'files/serversideencrypted', + ], $paths); + } + + /** + * Test filtering by mime types. + */ + public function testGetByAncestorInStorageWithMimeTypes(): void { + $generator = $this->fileAccess->getByAncestorInStorage( + 1, + 1, + 0, + [2], // Only include documents (mimetype=2) + true, + true, + 10 + ); + + $result = iterator_to_array($generator); + + $this->assertCount(1, $result); + $this->assertEquals('files/documents', $result[0]->getPath()); + } + + /** + * Test excluding end-to-end encrypted files. + */ + public function testGetByAncestorInStorageWithoutEndToEndEncrypted(): void { + $generator = $this->fileAccess->getByAncestorInStorage( + 1, + 1, + 0, + [], + false, // exclude end-to-end encrypted files + true, + 10 + ); + + $result = iterator_to_array($generator); + + var_dump($result); + + $this->assertCount(1, $result); + $this->assertEquals('files/serversideencrypted', $result[0]->getPath()); + } + + /** + * Test excluding server-side encrypted files. + */ + public function testGetByAncestorInStorageWithoutServerSideEncrypted(): void { + $generator = $this->fileAccess->getByAncestorInStorage( + 1, + 1, + 0, + [], + true, + false, // exclude server-side encrypted files + 10 + ); + + $result = iterator_to_array($generator); + + $this->assertCount(1, $result); + $this->assertEquals('files/photos/endtoendencrypted', $result[0]->getPath()); + } + + /** + * Test max result limits. + */ + public function testGetByAncestorInStorageWithMaxResults(): void { + $generator = $this->fileAccess->getByAncestorInStorage( + 1, + 1, + 0, + [], + true, + true, + 1 // Limit to 1 result + ); + + $result = iterator_to_array($generator); + + $this->assertCount(1, $result); + $this->assertEquals('files/documents', $result[0]->getPath()); + } + + /** + * Test rootId filter + */ + public function testGetByAncestorInStorageWithRootIdFilter(): void { + $generator = $this->fileAccess->getByAncestorInStorage( + 1, + 3, // Filter by rootId + 0, + [], + true, + true, + 10 + ); + + $result = iterator_to_array($generator); + + $this->assertCount(1, $result); + $this->assertEquals('files/photos/endtoendencrypted', $result[0]->getPath()); + } + + /** + * Test rootId filter + */ + public function testGetByAncestorInStorageWithStorageFilter(): void { + $generator = $this->fileAccess->getByAncestorInStorage( + 2, // Filter by storage + 6, // and by rootId + 0, + [], + true, + true, + 10 + ); + + $result = iterator_to_array($generator); + + $this->assertCount(1, $result); + $this->assertEquals('files/storage2/file', $result[0]->getPath()); + } +} From 10cc43041bb6a0fd7470ec13e7e48f871aa1895e Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Thu, 3 Apr 2025 11:31:34 +0200 Subject: [PATCH 07/18] fix(FileAccess#getByAncestorInStorage): Use a subquery to fix tests Signed-off-by: Marcel Klehr --- lib/private/Files/Cache/FileAccess.php | 16 ++++++++++++---- tests/lib/Files/Cache/FileAccessTest.php | 9 ++++----- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/lib/private/Files/Cache/FileAccess.php b/lib/private/Files/Cache/FileAccess.php index 2ddd18f1601..43aa9224b34 100644 --- a/lib/private/Files/Cache/FileAccess.php +++ b/lib/private/Files/Cache/FileAccess.php @@ -135,8 +135,16 @@ class FileAccess implements IFileAccess { if (!$endToEndEncrypted) { // End to end encrypted files are descendants of a folder with encrypted=1 - $qb->leftJoin('f', 'filecache', 'p', $qb->expr()->eq('f.parent', 'p.fileid')); - $qb->andWhere($qb->expr()->eq('p.encrypted', $qb->createNamedParameter(0, IQueryBuilder::PARAM_INT))); + // Use a subquery to check the `encrypted` status of the parent folder + $subQuery = $this->getQuery()->select('p.encrypted') + ->from('filecache', 'p') + ->andWhere($qb->expr()->eq('p.fileid', 'f.parent')) + ->setMaxResults(1) + ->getSQL(); + + $qb->andWhere( + $qb->expr()->eq($qb->createFunction(sprintf('(%s)', $subQuery)), $qb->createNamedParameter(0, IQueryBuilder::PARAM_INT)) + ); } if (!$serverSideEncrypted) { @@ -151,8 +159,8 @@ class FileAccess implements IFileAccess { if ($maxResults !== 0) { $qb->setMaxResults($maxResults); } - $files = $qb->orderBy('f.fileid', 'ASC') - ->executeQuery(); + $qb->orderBy('f.fileid', 'ASC'); + $files = $qb->executeQuery(); while ( /** @var array */ diff --git a/tests/lib/Files/Cache/FileAccessTest.php b/tests/lib/Files/Cache/FileAccessTest.php index 2078412ad73..3dec6e8893c 100644 --- a/tests/lib/Files/Cache/FileAccessTest.php +++ b/tests/lib/Files/Cache/FileAccessTest.php @@ -250,7 +250,7 @@ class FileAccessTest extends TestCase { $queryBuilder->insert('filecache') ->values([ 'fileid' => 5, - 'parent' => 0, + 'parent' => 1, 'path' => $queryBuilder->createNamedParameter('files/serversideencrypted'), 'path_hash' => $queryBuilder->createNamedParameter(md5('files/serversideencrypted')), 'storage' => $queryBuilder->createNamedParameter(1), @@ -350,10 +350,9 @@ class FileAccessTest extends TestCase { $result = iterator_to_array($generator); - var_dump($result); - - $this->assertCount(1, $result); - $this->assertEquals('files/serversideencrypted', $result[0]->getPath()); + $this->assertCount(3, $result); + $paths = array_map(fn(CacheEntry $entry) => $entry->getPath(), $result); + $this->assertEquals(['files/documents', 'files/photos', 'files/serversideencrypted'], $paths); } /** From 34b3f7553bd1955e73a45270213a202aefa619c5 Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Fri, 4 Apr 2025 10:15:23 +0200 Subject: [PATCH 08/18] fix(FileAccess#getDistinctMounts): Order results deterministically Signed-off-by: Marcel Klehr --- lib/private/Files/Cache/FileAccess.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/private/Files/Cache/FileAccess.php b/lib/private/Files/Cache/FileAccess.php index 43aa9224b34..a1bb70a7f14 100644 --- a/lib/private/Files/Cache/FileAccess.php +++ b/lib/private/Files/Cache/FileAccess.php @@ -193,6 +193,7 @@ class FileAccess implements IFileAccess { if ($excludeMountPoints !== false) { $qb->andWhere($qb->expr()->notLike('mount_point', $qb->createPositionalParameter($excludeMountPoints))); } + $qb->orderBy('root_id', 'ASC'); $result = $qb->executeQuery(); @@ -217,7 +218,7 @@ class FileAccess implements IFileAccess { ->andWhere($qb->expr()->eq('filecache.path', $qb->createNamedParameter('files'))) ->executeQuery()->fetch(); if ($root !== false) { - $overrideRoot = intval($root['fileid']); + $overrideRoot = (int)$root['fileid']; } } catch (Exception $e) { $this->logger->error('Could not fetch home storage files root for storage ' . $storageId, ['exception' => $e]); From 3941622059454a6ff69673aee1f62e2bb4627360 Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Fri, 4 Apr 2025 11:08:45 +0200 Subject: [PATCH 09/18] fix(FileAccessTest): Make sure path_hash is not NULL Signed-off-by: Marcel Klehr --- tests/lib/Files/Cache/FileAccessTest.php | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/lib/Files/Cache/FileAccessTest.php b/tests/lib/Files/Cache/FileAccessTest.php index 3dec6e8893c..f2e7ef8f510 100644 --- a/tests/lib/Files/Cache/FileAccessTest.php +++ b/tests/lib/Files/Cache/FileAccessTest.php @@ -179,6 +179,7 @@ class FileAccessTest extends TestCase { 'fileid' => $queryBuilder->createNamedParameter(99, IQueryBuilder::PARAM_INT), 'storage' => $queryBuilder->createNamedParameter(4, IQueryBuilder::PARAM_INT), 'path' => $queryBuilder->createNamedParameter('files'), + 'path_hash' => $queryBuilder->createNamedParameter(md5('files')), ]) ->executeStatement(); From 895160a1d35601f3e4246bdba27df6ac8b97291f Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Fri, 4 Apr 2025 14:10:52 +0200 Subject: [PATCH 10/18] fix(FileAccessTest): Do not use LIMIT in subquery Signed-off-by: Marcel Klehr --- lib/private/Files/Cache/FileAccess.php | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/private/Files/Cache/FileAccess.php b/lib/private/Files/Cache/FileAccess.php index a1bb70a7f14..c24cc75ce00 100644 --- a/lib/private/Files/Cache/FileAccess.php +++ b/lib/private/Files/Cache/FileAccess.php @@ -139,7 +139,6 @@ class FileAccess implements IFileAccess { $subQuery = $this->getQuery()->select('p.encrypted') ->from('filecache', 'p') ->andWhere($qb->expr()->eq('p.fileid', 'f.parent')) - ->setMaxResults(1) ->getSQL(); $qb->andWhere( From 26f6013c1f70078afa10d9f4d72024e750cd94fa Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Fri, 4 Apr 2025 14:47:52 +0200 Subject: [PATCH 11/18] fix(FileAccessTest): Make it work on sharded instance Signed-off-by: Marcel Klehr --- tests/lib/Files/Cache/FileAccessTest.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/lib/Files/Cache/FileAccessTest.php b/tests/lib/Files/Cache/FileAccessTest.php index f2e7ef8f510..435a233a57e 100644 --- a/tests/lib/Files/Cache/FileAccessTest.php +++ b/tests/lib/Files/Cache/FileAccessTest.php @@ -40,7 +40,7 @@ class FileAccessTest extends TestCase { ); // Clear and prepare `filecache` table for tests - $queryBuilder = $this->dbConnection->getQueryBuilder(); + $queryBuilder = $this->dbConnection->getQueryBuilder()->runAcrossAllShards(); $queryBuilder->delete('filecache')->executeStatement(); // Clean up potential leftovers from other tests @@ -173,7 +173,9 @@ class FileAccessTest extends TestCase { ->executeStatement(); // Simulate adding a "files" directory to the filecache table + $queryBuilder = $this->dbConnection->getQueryBuilder()->runAcrossAllShards(); $queryBuilder->delete('filecache')->executeStatement(); + $queryBuilder = $this->dbConnection->getQueryBuilder(); $queryBuilder->insert('filecache') ->values([ 'fileid' => $queryBuilder->createNamedParameter(99, IQueryBuilder::PARAM_INT), From 131125bbb7c6a865314e547e9a477ced1e57862d Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Thu, 10 Apr 2025 15:08:50 +0200 Subject: [PATCH 12/18] fix(FileAccessTest): Adress review comments Signed-off-by: Marcel Klehr --- lib/private/Files/Cache/FileAccess.php | 34 +++++++------ lib/public/Files/Cache/IFileAccess.php | 16 +++--- tests/lib/Files/Cache/FileAccessTest.php | 63 ++++++++++++------------ 3 files changed, 58 insertions(+), 55 deletions(-) diff --git a/lib/private/Files/Cache/FileAccess.php b/lib/private/Files/Cache/FileAccess.php index c24cc75ce00..3f997053c76 100644 --- a/lib/private/Files/Cache/FileAccess.php +++ b/lib/private/Files/Cache/FileAccess.php @@ -101,19 +101,19 @@ class FileAccess implements IFileAccess { * Allows filtering by mime types, encryption status, and limits the number of results. * * @param int $storageId The ID of the storage to search within. - * @param int $rootId The file ID of the ancestor to base the search on. - * @param int $lastFileId The last processed file ID. Only files with a higher ID will be included. Defaults to 0. - * @param list $mimeTypes An array of mime types to filter the results. If empty, no mime type filtering will be applied. + * @param int $folderId The file ID of the ancestor to base the search on. + * @param int $fileIdCursor The last processed file ID. Only files with a higher ID will be included. Defaults to 0. + * @param int $maxResults The maximum number of results to retrieve. If set to 0, all matching files will be retrieved. + * @param list $mimeTypeIds An array of mime types to filter the results. If empty, no mime type filtering will be applied. * @param bool $endToEndEncrypted Whether to include EndToEndEncrypted files * @param bool $serverSideEncrypted Whether to include ServerSideEncrypted files - * @param int $maxResults The maximum number of results to retrieve. If set to 0, all matching files will be retrieved. * @return \Generator A generator yielding matching files as cache entries. * @throws \OCP\DB\Exception */ - public function getByAncestorInStorage(int $storageId, int $rootId, int $lastFileId = 0, array $mimeTypes = [], bool $endToEndEncrypted = true, bool $serverSideEncrypted = true, int $maxResults = 100): \Generator { + public function getByAncestorInStorage(int $storageId, int $folderId, int $fileIdCursor = 0, int $maxResults = 100, array $mimeTypeIds = [], bool $endToEndEncrypted = true, bool $serverSideEncrypted = true): \Generator { $qb = $this->getQuery(); $qb->selectFileCache(); - $qb->andWhere($qb->expr()->eq('filecache.fileid', $qb->createNamedParameter($rootId, IQueryBuilder::PARAM_INT))); + $qb->andWhere($qb->expr()->eq('filecache.fileid', $qb->createNamedParameter($folderId, IQueryBuilder::PARAM_INT))); $result = $qb->executeQuery(); /** @var array{path:string}|false $root */ $root = $result->fetch(); @@ -131,7 +131,7 @@ class FileAccess implements IFileAccess { ->from('filecache', 'f') ->andWhere($qb->expr()->like('f.path', $qb->createNamedParameter($path . '%'))) ->andWhere($qb->expr()->eq('f.storage', $qb->createNamedParameter($storageId))) - ->andWhere($qb->expr()->gt('f.fileid', $qb->createNamedParameter($lastFileId))); + ->andWhere($qb->expr()->gt('f.fileid', $qb->createNamedParameter($fileIdCursor, IQueryBuilder::PARAM_INT))); if (!$endToEndEncrypted) { // End to end encrypted files are descendants of a folder with encrypted=1 @@ -151,8 +151,8 @@ class FileAccess implements IFileAccess { $qb->andWhere($qb->expr()->eq('f.encrypted', $qb->createNamedParameter(0, IQueryBuilder::PARAM_INT))); } - if (count($mimeTypes) > 0) { - $qb->andWhere($qb->expr()->in('f.mimetype', $qb->createNamedParameter($mimeTypes, IQueryBuilder::PARAM_INT_ARRAY))); + if (count($mimeTypeIds) > 0) { + $qb->andWhere($qb->expr()->in('f.mimetype', $qb->createNamedParameter($mimeTypeIds, IQueryBuilder::PARAM_INT_ARRAY))); } if ($maxResults !== 0) { @@ -177,20 +177,20 @@ class FileAccess implements IFileAccess { * Optionally rewrites home directory root paths to avoid cache and trashbin. * * @param list $mountProviders An array of mount provider class names to filter. If empty, all providers will be included. - * @param string|false $excludeMountPoints A string containing an SQL LIKE pattern to exclude mount points. Set to false to not exclude any mount points. + * @param bool $excludeTrashbinMounts Whether to exclude mounts that mount directories in a user's trashbin. * @param bool $rewriteHomeDirectories Whether to rewrite the root path IDs for home directories to only include user files. * @return \Generator A generator yielding mount configurations as an array containing 'storage_id', 'root_id', and 'override_root'. * @throws \OCP\DB\Exception */ - public function getDistinctMounts(array $mountProviders = [], string|false $excludeMountPoints = false, bool $rewriteHomeDirectories = true): \Generator { + public function getDistinctMounts(array $mountProviders = [], bool $excludeTrashbinMounts = true, bool $rewriteHomeDirectories = true): \Generator { $qb = $this->connection->getQueryBuilder(); $qb->selectDistinct(['root_id', 'storage_id', 'mount_provider_class']) ->from('mounts'); if (count($mountProviders) > 0) { $qb->where($qb->expr()->in('mount_provider_class', $qb->createPositionalParameter($mountProviders, IQueryBuilder::PARAM_STR_ARRAY))); } - if ($excludeMountPoints !== false) { - $qb->andWhere($qb->expr()->notLike('mount_point', $qb->createPositionalParameter($excludeMountPoints))); + if ($excludeTrashbinMounts === true) { + $qb->andWhere($qb->expr()->notLike('mount_point', $qb->createPositionalParameter('/%/files_trashbin/%'))); } $qb->orderBy('root_id', 'ASC'); $result = $qb->executeQuery(); @@ -203,6 +203,8 @@ class FileAccess implements IFileAccess { $storageId = (int)$row['storage_id']; $rootId = (int)$row['root_id']; $overrideRoot = $rootId; + // LocalHomeMountProvider is the default provider for user home directories + // ObjectHomeMountProvider is the home directory provider for when S3 primary storage is used if ($rewriteHomeDirectories && in_array($row['mount_provider_class'], [ \OC\Files\Mount\LocalHomeMountProvider::class, \OC\Files\Mount\ObjectHomeMountProvider::class, @@ -214,7 +216,8 @@ class FileAccess implements IFileAccess { /** @var array|false $root */ $root = $qb ->andWhere($qb->expr()->eq('filecache.storage', $qb->createNamedParameter($storageId, IQueryBuilder::PARAM_INT))) - ->andWhere($qb->expr()->eq('filecache.path', $qb->createNamedParameter('files'))) + ->andWhere($qb->expr()->eq('filecache.parent', $qb->createNamedParameter($rootId, IQueryBuilder::PARAM_INT))) + ->andWhere($qb->expr()->eq('filecache.name', $qb->createNamedParameter('files'))) ->executeQuery()->fetch(); if ($root !== false) { $overrideRoot = (int)$root['fileid']; @@ -224,10 +227,11 @@ class FileAccess implements IFileAccess { continue; } } + // Reference to root_id is still necessary even if we have the overridden_root_id, because storage_id and root_id uniquely identify a mount yield [ 'storage_id' => $storageId, 'root_id' => $rootId, - 'override_root' => $overrideRoot, + 'overridden_root' => $overrideRoot, ]; } $result->closeCursor(); diff --git a/lib/public/Files/Cache/IFileAccess.php b/lib/public/Files/Cache/IFileAccess.php index be66c5a9a09..6f27950a0b0 100644 --- a/lib/public/Files/Cache/IFileAccess.php +++ b/lib/public/Files/Cache/IFileAccess.php @@ -85,18 +85,16 @@ interface IFileAccess { * Allows filtering by mime types, encryption status, and limits the number of results. * * @param int $storageId The ID of the storage to search within. - * @param int $rootId The file ID of the ancestor to base the search on. - * @param int $lastFileId The last processed file ID. Only files with a higher ID will be included. Defaults to 0. - * @param list $mimeTypes An array of mime types to filter the results. If empty, no mime type filtering will be applied. + * @param int $folderId The file ID of the ancestor to base the search on. + * @param int $fileIdCursor The last processed file ID. Only files with a higher ID will be included. Defaults to 0. + * @param int $maxResults The maximum number of results to retrieve. If set to 0, all matching files will be retrieved. + * @param list $mimeTypeIds An array of mime types to filter the results. If empty, no mime type filtering will be applied. * @param bool $endToEndEncrypted Whether to include EndToEndEncrypted files * @param bool $serverSideEncrypted Whether to include ServerSideEncrypted files - * @param int $maxResults The maximum number of results to retrieve. If set to 0, all matching files will be retrieved. * @return \Generator A generator yielding matching files as cache entries. * @throws \OCP\DB\Exception - * - * @since 32.0.0 */ - public function getByAncestorInStorage(int $storageId, int $rootId, int $lastFileId = 0, array $mimeTypes = [], bool $endToEndEncrypted = true, bool $serverSideEncrypted = true, int $maxResults = 100): \Generator; + public function getByAncestorInStorage(int $storageId, int $folderId, int $fileIdCursor = 0, int $maxResults = 100, array $mimeTypeIds = [], bool $endToEndEncrypted = true, bool $serverSideEncrypted = true): \Generator; /** * Retrieves a list of all distinct mounts. @@ -104,12 +102,12 @@ interface IFileAccess { * Optionally rewrites home directory root paths to avoid cache and trashbin. * * @param list $mountProviders An array of mount provider class names to filter. If empty, all providers will be included. - * @param string|false $excludeMountPoints A string containing an SQL LIKE pattern to exclude mount points. Set to false to not exclude any mount points. + * @param bool $excludeTrashbinMounts Whether to include mounts that mount a directory in a user's trashbin. * @param bool $rewriteHomeDirectories Whether to rewrite the root path IDs for home directories to only include user files. * @return \Generator A generator yielding mount configurations as an array containing 'storage_id', 'root_id', and 'override_root'. * @throws \OCP\DB\Exception * * @since 32.0.0 */ - public function getDistinctMounts(array $mountProviders = [], string|false $excludeMountPoints = false, bool $rewriteHomeDirectories = true): \Generator; + public function getDistinctMounts(array $mountProviders = [], bool $excludeTrashbinMounts = true, bool $rewriteHomeDirectories = true): \Generator; } diff --git a/tests/lib/Files/Cache/FileAccessTest.php b/tests/lib/Files/Cache/FileAccessTest.php index 435a233a57e..8b796d86f9a 100644 --- a/tests/lib/Files/Cache/FileAccessTest.php +++ b/tests/lib/Files/Cache/FileAccessTest.php @@ -71,7 +71,7 @@ class FileAccessTest extends TestCase { 'storage_id' => $queryBuilder->createNamedParameter(2, IQueryBuilder::PARAM_INT), 'root_id' => $queryBuilder->createNamedParameter(20, IQueryBuilder::PARAM_INT), 'mount_provider_class' => $queryBuilder->createNamedParameter('TestProviderClass2'), - 'mount_point' => $queryBuilder->createNamedParameter('/cache'), + 'mount_point' => $queryBuilder->createNamedParameter('/foobar/files_trashbin/test'), 'user_id' => $queryBuilder->createNamedParameter('test'), ]) ->executeStatement(); @@ -81,7 +81,7 @@ class FileAccessTest extends TestCase { 'storage_id' => $queryBuilder->createNamedParameter(3, IQueryBuilder::PARAM_INT), 'root_id' => $queryBuilder->createNamedParameter(30, IQueryBuilder::PARAM_INT), 'mount_provider_class' => $queryBuilder->createNamedParameter('TestProviderClass1'), - 'mount_point' => $queryBuilder->createNamedParameter('/trashbin'), + 'mount_point' => $queryBuilder->createNamedParameter('/documents'), 'user_id' => $queryBuilder->createNamedParameter('test'), ]) ->executeStatement(); @@ -93,25 +93,19 @@ class FileAccessTest extends TestCase { public function testGetDistinctMountsWithoutFilters(): void { $result = iterator_to_array($this->fileAccess->getDistinctMounts()); - $this->assertCount(3, $result); + $this->assertCount(2, $result); $this->assertEquals([ 'storage_id' => 1, 'root_id' => 10, - 'override_root' => 10, + 'overridden_root' => 10, ], $result[0]); - $this->assertEquals([ - 'storage_id' => 2, - 'root_id' => 20, - 'override_root' => 20, - ], $result[1]); - $this->assertEquals([ 'storage_id' => 3, 'root_id' => 30, - 'override_root' => 30, - ], $result[2]); + 'overridden_root' => 30, + ], $result[1]); } /** @@ -125,13 +119,13 @@ class FileAccessTest extends TestCase { $this->assertEquals([ 'storage_id' => 1, 'root_id' => 10, - 'override_root' => 10, + 'overridden_root' => 10, ], $result[0]); $this->assertEquals([ 'storage_id' => 3, 'root_id' => 30, - 'override_root' => 30, + 'overridden_root' => 30, ], $result[1]); } @@ -139,21 +133,27 @@ class FileAccessTest extends TestCase { * Test that getDistinctMounts excludes certain mount points */ public function testGetDistinctMountsWithExclusionFilter(): void { - $result = iterator_to_array($this->fileAccess->getDistinctMounts([], '/cache')); + $result = iterator_to_array($this->fileAccess->getDistinctMounts([], false)); - $this->assertCount(2, $result); + $this->assertCount(3, $result); $this->assertEquals([ 'storage_id' => 1, 'root_id' => 10, - 'override_root' => 10, + 'overridden_root' => 10, ], $result[0]); + $this->assertEquals([ + 'storage_id' => 2, + 'root_id' => 20, + 'overridden_root' => 20, + ], $result[1]); + $this->assertEquals([ 'storage_id' => 3, 'root_id' => 30, - 'override_root' => 30, - ], $result[1]); + 'overridden_root' => 30, + ], $result[2]); } /** @@ -180,8 +180,9 @@ class FileAccessTest extends TestCase { ->values([ 'fileid' => $queryBuilder->createNamedParameter(99, IQueryBuilder::PARAM_INT), 'storage' => $queryBuilder->createNamedParameter(4, IQueryBuilder::PARAM_INT), - 'path' => $queryBuilder->createNamedParameter('files'), - 'path_hash' => $queryBuilder->createNamedParameter(md5('files')), + 'parent' => $queryBuilder->createNamedParameter(40), + 'name' => $queryBuilder->createNamedParameter('files'), + 'path_hash' => $queryBuilder->createNamedParameter(md5('/home/user/files')), ]) ->executeStatement(); @@ -190,7 +191,7 @@ class FileAccessTest extends TestCase { $this->assertEquals([ 'storage_id' => 4, 'root_id' => 40, - 'override_root' => 99, + 'overridden_root' => 99, ], end($result)); } @@ -298,17 +299,17 @@ class FileAccessTest extends TestCase { 1, // storageId 1, // rootId 0, // lastFileId + 10, // maxResults [], // mimeTypes true, // include end-to-end encrypted files true, // include server-side encrypted files - 10 // maxResults ); $result = iterator_to_array($generator); $this->assertCount(4, $result); - $paths = array_map(fn(CacheEntry $entry) => $entry->getPath(), $result); + $paths = array_map(fn (CacheEntry $entry) => $entry->getPath(), $result); $this->assertEquals([ 'files/documents', 'files/photos', @@ -325,10 +326,10 @@ class FileAccessTest extends TestCase { 1, 1, 0, + 10, [2], // Only include documents (mimetype=2) true, true, - 10 ); $result = iterator_to_array($generator); @@ -345,16 +346,16 @@ class FileAccessTest extends TestCase { 1, 1, 0, + 10, [], false, // exclude end-to-end encrypted files true, - 10 ); $result = iterator_to_array($generator); $this->assertCount(3, $result); - $paths = array_map(fn(CacheEntry $entry) => $entry->getPath(), $result); + $paths = array_map(fn (CacheEntry $entry) => $entry->getPath(), $result); $this->assertEquals(['files/documents', 'files/photos', 'files/serversideencrypted'], $paths); } @@ -366,10 +367,10 @@ class FileAccessTest extends TestCase { 1, 1, 0, + 10, [], true, false, // exclude server-side encrypted files - 10 ); $result = iterator_to_array($generator); @@ -386,10 +387,10 @@ class FileAccessTest extends TestCase { 1, 1, 0, + 1, // Limit to 1 result [], true, true, - 1 // Limit to 1 result ); $result = iterator_to_array($generator); @@ -406,10 +407,10 @@ class FileAccessTest extends TestCase { 1, 3, // Filter by rootId 0, + 10, [], true, true, - 10 ); $result = iterator_to_array($generator); @@ -426,10 +427,10 @@ class FileAccessTest extends TestCase { 2, // Filter by storage 6, // and by rootId 0, + 10, [], true, true, - 10 ); $result = iterator_to_array($generator); From 7e986988fe6f82c91a0bdbb1661c7fcc3c3fd252 Mon Sep 17 00:00:00 2001 From: Julien Veyssier Date: Wed, 28 May 2025 12:49:16 +0200 Subject: [PATCH 13/18] fix(FileAccess*): Adress review comments Signed-off-by: Julien Veyssier --- lib/private/Files/Cache/FileAccess.php | 39 +++++------------------- lib/public/Files/Cache/IFileAccess.php | 6 ++-- tests/lib/Files/Cache/FileAccessTest.php | 20 +++++------- 3 files changed, 20 insertions(+), 45 deletions(-) diff --git a/lib/private/Files/Cache/FileAccess.php b/lib/private/Files/Cache/FileAccess.php index 3f997053c76..2c897b5b6c4 100644 --- a/lib/private/Files/Cache/FileAccess.php +++ b/lib/private/Files/Cache/FileAccess.php @@ -96,24 +96,11 @@ class FileAccess implements IFileAccess { return $this->rowsToEntries($rows); } - /** - * Retrieves files stored in a specific storage that have a specified ancestor in the file hierarchy. - * Allows filtering by mime types, encryption status, and limits the number of results. - * - * @param int $storageId The ID of the storage to search within. - * @param int $folderId The file ID of the ancestor to base the search on. - * @param int $fileIdCursor The last processed file ID. Only files with a higher ID will be included. Defaults to 0. - * @param int $maxResults The maximum number of results to retrieve. If set to 0, all matching files will be retrieved. - * @param list $mimeTypeIds An array of mime types to filter the results. If empty, no mime type filtering will be applied. - * @param bool $endToEndEncrypted Whether to include EndToEndEncrypted files - * @param bool $serverSideEncrypted Whether to include ServerSideEncrypted files - * @return \Generator A generator yielding matching files as cache entries. - * @throws \OCP\DB\Exception - */ public function getByAncestorInStorage(int $storageId, int $folderId, int $fileIdCursor = 0, int $maxResults = 100, array $mimeTypeIds = [], bool $endToEndEncrypted = true, bool $serverSideEncrypted = true): \Generator { $qb = $this->getQuery(); - $qb->selectFileCache(); - $qb->andWhere($qb->expr()->eq('filecache.fileid', $qb->createNamedParameter($folderId, IQueryBuilder::PARAM_INT))); + $qb->select('path') + ->from('filecache'); + $qb->where($qb->expr()->eq('fileid', $qb->createNamedParameter($folderId, IQueryBuilder::PARAM_INT))); $result = $qb->executeQuery(); /** @var array{path:string}|false $root */ $root = $result->fetch(); @@ -171,17 +158,6 @@ class FileAccess implements IFileAccess { $files->closeCursor(); } - /** - * Retrieves a list of all distinct mounts. - * Allows filtering by specific mount providers and excluding certain mount points. - * Optionally rewrites home directory root paths to avoid cache and trashbin. - * - * @param list $mountProviders An array of mount provider class names to filter. If empty, all providers will be included. - * @param bool $excludeTrashbinMounts Whether to exclude mounts that mount directories in a user's trashbin. - * @param bool $rewriteHomeDirectories Whether to rewrite the root path IDs for home directories to only include user files. - * @return \Generator A generator yielding mount configurations as an array containing 'storage_id', 'root_id', and 'override_root'. - * @throws \OCP\DB\Exception - */ public function getDistinctMounts(array $mountProviders = [], bool $excludeTrashbinMounts = true, bool $rewriteHomeDirectories = true): \Generator { $qb = $this->connection->getQueryBuilder(); $qb->selectDistinct(['root_id', 'storage_id', 'mount_provider_class']) @@ -212,12 +188,13 @@ class FileAccess implements IFileAccess { // Only crawl files, not cache or trashbin $qb = $this->getQuery(); try { - $qb->selectFileCache(); + $qb->select('fileid') + ->from('filecache'); /** @var array|false $root */ $root = $qb - ->andWhere($qb->expr()->eq('filecache.storage', $qb->createNamedParameter($storageId, IQueryBuilder::PARAM_INT))) - ->andWhere($qb->expr()->eq('filecache.parent', $qb->createNamedParameter($rootId, IQueryBuilder::PARAM_INT))) - ->andWhere($qb->expr()->eq('filecache.name', $qb->createNamedParameter('files'))) + ->andWhere($qb->expr()->eq('storage', $qb->createNamedParameter($storageId, IQueryBuilder::PARAM_INT))) + ->andWhere($qb->expr()->eq('parent', $qb->createNamedParameter($rootId, IQueryBuilder::PARAM_INT))) + ->andWhere($qb->expr()->eq('name', $qb->createNamedParameter('files'))) ->executeQuery()->fetch(); if ($root !== false) { $overrideRoot = (int)$root['fileid']; diff --git a/lib/public/Files/Cache/IFileAccess.php b/lib/public/Files/Cache/IFileAccess.php index 6f27950a0b0..61e28318034 100644 --- a/lib/public/Files/Cache/IFileAccess.php +++ b/lib/public/Files/Cache/IFileAccess.php @@ -91,8 +91,10 @@ interface IFileAccess { * @param list $mimeTypeIds An array of mime types to filter the results. If empty, no mime type filtering will be applied. * @param bool $endToEndEncrypted Whether to include EndToEndEncrypted files * @param bool $serverSideEncrypted Whether to include ServerSideEncrypted files - * @return \Generator A generator yielding matching files as cache entries. + * @return \Generator A generator yielding matching files as cache entries. * @throws \OCP\DB\Exception + * + * @since 32.0.0 */ public function getByAncestorInStorage(int $storageId, int $folderId, int $fileIdCursor = 0, int $maxResults = 100, array $mimeTypeIds = [], bool $endToEndEncrypted = true, bool $serverSideEncrypted = true): \Generator; @@ -104,7 +106,7 @@ interface IFileAccess { * @param list $mountProviders An array of mount provider class names to filter. If empty, all providers will be included. * @param bool $excludeTrashbinMounts Whether to include mounts that mount a directory in a user's trashbin. * @param bool $rewriteHomeDirectories Whether to rewrite the root path IDs for home directories to only include user files. - * @return \Generator A generator yielding mount configurations as an array containing 'storage_id', 'root_id', and 'override_root'. + * @return \Generator A generator yielding mount configurations as an array containing 'storage_id', 'root_id', and 'override_root'. * @throws \OCP\DB\Exception * * @since 32.0.0 diff --git a/tests/lib/Files/Cache/FileAccessTest.php b/tests/lib/Files/Cache/FileAccessTest.php index 8b796d86f9a..4cad5299966 100644 --- a/tests/lib/Files/Cache/FileAccessTest.php +++ b/tests/lib/Files/Cache/FileAccessTest.php @@ -1,7 +1,6 @@ dbConnection = \OC::$server->get(IDBConnection::class); + $this->dbConnection = \OCP\Server::get(IDBConnection::class); // Ensure FileAccess is instantiated with the real connection $this->fileAccess = new FileAccess( $this->dbConnection, - \OC::$server->get(\OC\SystemConfig::class), - \OC::$server->get(LoggerInterface::class), - \OC::$server->get(\OC\FilesMetadata\FilesMetadataManager::class), - \OC::$server->get(\OCP\Files\IMimeTypeLoader::class) + \OCP\Server::get(\OC\SystemConfig::class), + \OCP\Server::get(LoggerInterface::class), + \OCP\Server::get(\OC\FilesMetadata\FilesMetadataManager::class), + \OCP\Server::get(\OCP\Files\IMimeTypeLoader::class) ); // Clear and prepare `filecache` table for tests From 28dc4a299e17c5194bd4a81acecf4802bb1c2bce Mon Sep 17 00:00:00 2001 From: Julien Veyssier Date: Mon, 2 Jun 2025 11:52:11 +0200 Subject: [PATCH 14/18] fix(FileAccess): exclude trashbin nodes on the oc_filecache query, there is no trashbin mount Signed-off-by: Julien Veyssier --- lib/private/Files/Cache/FileAccess.php | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/private/Files/Cache/FileAccess.php b/lib/private/Files/Cache/FileAccess.php index 2c897b5b6c4..11eeb347c1f 100644 --- a/lib/private/Files/Cache/FileAccess.php +++ b/lib/private/Files/Cache/FileAccess.php @@ -165,9 +165,6 @@ class FileAccess implements IFileAccess { if (count($mountProviders) > 0) { $qb->where($qb->expr()->in('mount_provider_class', $qb->createPositionalParameter($mountProviders, IQueryBuilder::PARAM_STR_ARRAY))); } - if ($excludeTrashbinMounts === true) { - $qb->andWhere($qb->expr()->notLike('mount_point', $qb->createPositionalParameter('/%/files_trashbin/%'))); - } $qb->orderBy('root_id', 'ASC'); $result = $qb->executeQuery(); @@ -190,12 +187,15 @@ class FileAccess implements IFileAccess { try { $qb->select('fileid') ->from('filecache'); - /** @var array|false $root */ - $root = $qb - ->andWhere($qb->expr()->eq('storage', $qb->createNamedParameter($storageId, IQueryBuilder::PARAM_INT))) + $qb->andWhere($qb->expr()->eq('storage', $qb->createNamedParameter($storageId, IQueryBuilder::PARAM_INT))) ->andWhere($qb->expr()->eq('parent', $qb->createNamedParameter($rootId, IQueryBuilder::PARAM_INT))) - ->andWhere($qb->expr()->eq('name', $qb->createNamedParameter('files'))) - ->executeQuery()->fetch(); + ->andWhere($qb->expr()->eq('name', $qb->createNamedParameter('files'))); + if ($excludeTrashbinMounts === true) { + $qb->andWhere($qb->expr()->notLike('path', $qb->createPositionalParameter('files_trashbin/%'))) + ->andWhere($qb->expr()->notLike('path', $qb->createPositionalParameter('__groupfolders/trash/%'))); + } + /** @var array|false $root */ + $root = $qb->executeQuery()->fetch(); if ($root !== false) { $overrideRoot = (int)$root['fileid']; } From a2aeec0f4b155e86c7dde2a580d542c58a86d50d Mon Sep 17 00:00:00 2001 From: Julien Veyssier Date: Tue, 3 Jun 2025 11:57:09 +0200 Subject: [PATCH 15/18] fix(FileAccess*): Adress review comments Signed-off-by: Julien Veyssier --- lib/private/Files/Cache/FileAccess.php | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/lib/private/Files/Cache/FileAccess.php b/lib/private/Files/Cache/FileAccess.php index 11eeb347c1f..0a3f1983d27 100644 --- a/lib/private/Files/Cache/FileAccess.php +++ b/lib/private/Files/Cache/FileAccess.php @@ -99,8 +99,8 @@ class FileAccess implements IFileAccess { public function getByAncestorInStorage(int $storageId, int $folderId, int $fileIdCursor = 0, int $maxResults = 100, array $mimeTypeIds = [], bool $endToEndEncrypted = true, bool $serverSideEncrypted = true): \Generator { $qb = $this->getQuery(); $qb->select('path') - ->from('filecache'); - $qb->where($qb->expr()->eq('fileid', $qb->createNamedParameter($folderId, IQueryBuilder::PARAM_INT))); + ->from('filecache') + ->where($qb->expr()->eq('fileid', $qb->createNamedParameter($folderId, IQueryBuilder::PARAM_INT))); $result = $qb->executeQuery(); /** @var array{path:string}|false $root */ $root = $result->fetch(); @@ -116,7 +116,7 @@ class FileAccess implements IFileAccess { $qb->selectDistinct('*') ->from('filecache', 'f') - ->andWhere($qb->expr()->like('f.path', $qb->createNamedParameter($path . '%'))) + ->where($qb->expr()->like('f.path', $qb->createNamedParameter($this->connection->escapeLikeParameter($path) . '%'))) ->andWhere($qb->expr()->eq('f.storage', $qb->createNamedParameter($storageId))) ->andWhere($qb->expr()->gt('f.fileid', $qb->createNamedParameter($fileIdCursor, IQueryBuilder::PARAM_INT))); @@ -168,7 +168,6 @@ class FileAccess implements IFileAccess { $qb->orderBy('root_id', 'ASC'); $result = $qb->executeQuery(); - while ( /** @var array{storage_id:int, root_id:int,mount_provider_class:string} $row */ $row = $result->fetch() @@ -186,13 +185,13 @@ class FileAccess implements IFileAccess { $qb = $this->getQuery(); try { $qb->select('fileid') - ->from('filecache'); - $qb->andWhere($qb->expr()->eq('storage', $qb->createNamedParameter($storageId, IQueryBuilder::PARAM_INT))) + ->from('filecache') + ->where($qb->expr()->eq('storage', $qb->createNamedParameter($storageId, IQueryBuilder::PARAM_INT))) ->andWhere($qb->expr()->eq('parent', $qb->createNamedParameter($rootId, IQueryBuilder::PARAM_INT))) ->andWhere($qb->expr()->eq('name', $qb->createNamedParameter('files'))); if ($excludeTrashbinMounts === true) { - $qb->andWhere($qb->expr()->notLike('path', $qb->createPositionalParameter('files_trashbin/%'))) - ->andWhere($qb->expr()->notLike('path', $qb->createPositionalParameter('__groupfolders/trash/%'))); + $qb->andWhere($qb->expr()->notLike('path', $qb->createNamedParameter('files_trashbin/%'))) + ->andWhere($qb->expr()->notLike('path', $qb->createNamedParameter('__groupfolders/trash/%'))); } /** @var array|false $root */ $root = $qb->executeQuery()->fetch(); From cb221c8211c5859deb476cfe1e368466a901585c Mon Sep 17 00:00:00 2001 From: Julien Veyssier Date: Tue, 3 Jun 2025 15:40:44 +0200 Subject: [PATCH 16/18] fix(FileAccess*): Change the way home dir root is found, remove the excludeTrashbinMounts param of getDistinctMounts Signed-off-by: Julien Veyssier --- lib/private/Files/Cache/FileAccess.php | 8 ++--- lib/public/Files/Cache/IFileAccess.php | 5 ++-- tests/lib/Files/Cache/FileAccessTest.php | 37 ------------------------ 3 files changed, 4 insertions(+), 46 deletions(-) diff --git a/lib/private/Files/Cache/FileAccess.php b/lib/private/Files/Cache/FileAccess.php index 0a3f1983d27..a7a6e3a222f 100644 --- a/lib/private/Files/Cache/FileAccess.php +++ b/lib/private/Files/Cache/FileAccess.php @@ -158,7 +158,7 @@ class FileAccess implements IFileAccess { $files->closeCursor(); } - public function getDistinctMounts(array $mountProviders = [], bool $excludeTrashbinMounts = true, bool $rewriteHomeDirectories = true): \Generator { + public function getDistinctMounts(array $mountProviders = [], bool $rewriteHomeDirectories = true): \Generator { $qb = $this->connection->getQueryBuilder(); $qb->selectDistinct(['root_id', 'storage_id', 'mount_provider_class']) ->from('mounts'); @@ -188,11 +188,7 @@ class FileAccess implements IFileAccess { ->from('filecache') ->where($qb->expr()->eq('storage', $qb->createNamedParameter($storageId, IQueryBuilder::PARAM_INT))) ->andWhere($qb->expr()->eq('parent', $qb->createNamedParameter($rootId, IQueryBuilder::PARAM_INT))) - ->andWhere($qb->expr()->eq('name', $qb->createNamedParameter('files'))); - if ($excludeTrashbinMounts === true) { - $qb->andWhere($qb->expr()->notLike('path', $qb->createNamedParameter('files_trashbin/%'))) - ->andWhere($qb->expr()->notLike('path', $qb->createNamedParameter('__groupfolders/trash/%'))); - } + ->andWhere($qb->expr()->eq('path', $qb->createNamedParameter('files'))); /** @var array|false $root */ $root = $qb->executeQuery()->fetch(); if ($root !== false) { diff --git a/lib/public/Files/Cache/IFileAccess.php b/lib/public/Files/Cache/IFileAccess.php index 61e28318034..be6257382ad 100644 --- a/lib/public/Files/Cache/IFileAccess.php +++ b/lib/public/Files/Cache/IFileAccess.php @@ -100,16 +100,15 @@ interface IFileAccess { /** * Retrieves a list of all distinct mounts. - * Allows filtering by specific mount providers and excluding certain mount points. + * Allows filtering by specific mount providers. * Optionally rewrites home directory root paths to avoid cache and trashbin. * * @param list $mountProviders An array of mount provider class names to filter. If empty, all providers will be included. - * @param bool $excludeTrashbinMounts Whether to include mounts that mount a directory in a user's trashbin. * @param bool $rewriteHomeDirectories Whether to rewrite the root path IDs for home directories to only include user files. * @return \Generator A generator yielding mount configurations as an array containing 'storage_id', 'root_id', and 'override_root'. * @throws \OCP\DB\Exception * * @since 32.0.0 */ - public function getDistinctMounts(array $mountProviders = [], bool $excludeTrashbinMounts = true, bool $rewriteHomeDirectories = true): \Generator; + public function getDistinctMounts(array $mountProviders = [], bool $rewriteHomeDirectories = true): \Generator; } diff --git a/tests/lib/Files/Cache/FileAccessTest.php b/tests/lib/Files/Cache/FileAccessTest.php index 4cad5299966..ed82674d4c5 100644 --- a/tests/lib/Files/Cache/FileAccessTest.php +++ b/tests/lib/Files/Cache/FileAccessTest.php @@ -62,16 +62,6 @@ class FileAccessTest extends TestCase { ]) ->executeStatement(); - $queryBuilder->insert('mounts') - ->values([ - 'storage_id' => $queryBuilder->createNamedParameter(2, IQueryBuilder::PARAM_INT), - 'root_id' => $queryBuilder->createNamedParameter(20, IQueryBuilder::PARAM_INT), - 'mount_provider_class' => $queryBuilder->createNamedParameter('TestProviderClass2'), - 'mount_point' => $queryBuilder->createNamedParameter('/foobar/files_trashbin/test'), - 'user_id' => $queryBuilder->createNamedParameter('test'), - ]) - ->executeStatement(); - $queryBuilder->insert('mounts') ->values([ 'storage_id' => $queryBuilder->createNamedParameter(3, IQueryBuilder::PARAM_INT), @@ -125,33 +115,6 @@ class FileAccessTest extends TestCase { ], $result[1]); } - /** - * Test that getDistinctMounts excludes certain mount points - */ - public function testGetDistinctMountsWithExclusionFilter(): void { - $result = iterator_to_array($this->fileAccess->getDistinctMounts([], false)); - - $this->assertCount(3, $result); - - $this->assertEquals([ - 'storage_id' => 1, - 'root_id' => 10, - 'overridden_root' => 10, - ], $result[0]); - - $this->assertEquals([ - 'storage_id' => 2, - 'root_id' => 20, - 'overridden_root' => 20, - ], $result[1]); - - $this->assertEquals([ - 'storage_id' => 3, - 'root_id' => 30, - 'overridden_root' => 30, - ], $result[2]); - } - /** * Test that getDistinctMounts rewrites home directory paths */ From 3a96f8e53345dd578b00119f9bdd541adec12a8e Mon Sep 17 00:00:00 2001 From: Julien Veyssier Date: Tue, 3 Jun 2025 17:20:13 +0200 Subject: [PATCH 17/18] fix(FileAccess*): fix tests Signed-off-by: Julien Veyssier --- tests/lib/Files/Cache/FileAccessTest.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/lib/Files/Cache/FileAccessTest.php b/tests/lib/Files/Cache/FileAccessTest.php index ed82674d4c5..e856488f176 100644 --- a/tests/lib/Files/Cache/FileAccessTest.php +++ b/tests/lib/Files/Cache/FileAccessTest.php @@ -141,7 +141,8 @@ class FileAccessTest extends TestCase { 'storage' => $queryBuilder->createNamedParameter(4, IQueryBuilder::PARAM_INT), 'parent' => $queryBuilder->createNamedParameter(40), 'name' => $queryBuilder->createNamedParameter('files'), - 'path_hash' => $queryBuilder->createNamedParameter(md5('/home/user/files')), + 'path' => $queryBuilder->createNamedParameter('files'), + 'path_hash' => $queryBuilder->createNamedParameter(md5('files')), ]) ->executeStatement(); From 43be97de0846c25b5098d0b355ef1aa8408dbca5 Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Mon, 7 Jul 2025 15:54:30 +0200 Subject: [PATCH 18/18] fix(FileAccess): Use one param for rewriting home dirs and excluding non-user files mounts Signed-off-by: Marcel Klehr --- lib/private/Files/Cache/FileAccess.php | 17 +++++++-- lib/public/Files/Cache/IFileAccess.php | 4 +-- tests/lib/Files/Cache/FileAccessTest.php | 45 +++++++++++++++++++++--- 3 files changed, 57 insertions(+), 9 deletions(-) diff --git a/lib/private/Files/Cache/FileAccess.php b/lib/private/Files/Cache/FileAccess.php index a7a6e3a222f..c3f3614f3ca 100644 --- a/lib/private/Files/Cache/FileAccess.php +++ b/lib/private/Files/Cache/FileAccess.php @@ -158,12 +158,23 @@ class FileAccess implements IFileAccess { $files->closeCursor(); } - public function getDistinctMounts(array $mountProviders = [], bool $rewriteHomeDirectories = true): \Generator { + public function getDistinctMounts(array $mountProviders = [], bool $onlyUserFilesMounts = true): \Generator { $qb = $this->connection->getQueryBuilder(); $qb->selectDistinct(['root_id', 'storage_id', 'mount_provider_class']) ->from('mounts'); + if ($onlyUserFilesMounts) { + $qb->andWhere( + $qb->expr()->orX( + $qb->expr()->like('mount_point', $qb->createNamedParameter('/%/files/%')), + $qb->expr()->in('mount_provider_class', $qb->createNamedParameter([ + \OC\Files\Mount\LocalHomeMountProvider::class, + \OC\Files\Mount\ObjectHomeMountProvider::class, + ], IQueryBuilder::PARAM_STR_ARRAY)) + ) + ); + } if (count($mountProviders) > 0) { - $qb->where($qb->expr()->in('mount_provider_class', $qb->createPositionalParameter($mountProviders, IQueryBuilder::PARAM_STR_ARRAY))); + $qb->andWhere($qb->expr()->in('mount_provider_class', $qb->createNamedParameter($mountProviders, IQueryBuilder::PARAM_STR_ARRAY))); } $qb->orderBy('root_id', 'ASC'); $result = $qb->executeQuery(); @@ -177,7 +188,7 @@ class FileAccess implements IFileAccess { $overrideRoot = $rootId; // LocalHomeMountProvider is the default provider for user home directories // ObjectHomeMountProvider is the home directory provider for when S3 primary storage is used - if ($rewriteHomeDirectories && in_array($row['mount_provider_class'], [ + if ($onlyUserFilesMounts && in_array($row['mount_provider_class'], [ \OC\Files\Mount\LocalHomeMountProvider::class, \OC\Files\Mount\ObjectHomeMountProvider::class, ], true)) { diff --git a/lib/public/Files/Cache/IFileAccess.php b/lib/public/Files/Cache/IFileAccess.php index be6257382ad..7a993d81e7a 100644 --- a/lib/public/Files/Cache/IFileAccess.php +++ b/lib/public/Files/Cache/IFileAccess.php @@ -104,11 +104,11 @@ interface IFileAccess { * Optionally rewrites home directory root paths to avoid cache and trashbin. * * @param list $mountProviders An array of mount provider class names to filter. If empty, all providers will be included. - * @param bool $rewriteHomeDirectories Whether to rewrite the root path IDs for home directories to only include user files. + * @param bool $onlyUserFilesMounts Whether to rewrite the root IDs for home directories to only include user files and to only consider mounts with mount points in the user files. * @return \Generator A generator yielding mount configurations as an array containing 'storage_id', 'root_id', and 'override_root'. * @throws \OCP\DB\Exception * * @since 32.0.0 */ - public function getDistinctMounts(array $mountProviders = [], bool $rewriteHomeDirectories = true): \Generator; + public function getDistinctMounts(array $mountProviders = [], bool $onlyUserFilesMounts = true): \Generator; } diff --git a/tests/lib/Files/Cache/FileAccessTest.php b/tests/lib/Files/Cache/FileAccessTest.php index e856488f176..59fa2494ea8 100644 --- a/tests/lib/Files/Cache/FileAccessTest.php +++ b/tests/lib/Files/Cache/FileAccessTest.php @@ -1,4 +1,5 @@ $queryBuilder->createNamedParameter('test'), ]) ->executeStatement(); + + $queryBuilder->insert('mounts') + ->values([ + 'storage_id' => $queryBuilder->createNamedParameter(4, IQueryBuilder::PARAM_INT), + 'root_id' => $queryBuilder->createNamedParameter(31, IQueryBuilder::PARAM_INT), + 'mount_provider_class' => $queryBuilder->createNamedParameter('TestProviderClass2'), + 'mount_point' => $queryBuilder->createNamedParameter('/foobar'), + 'user_id' => $queryBuilder->createNamedParameter('test'), + ]) + ->executeStatement(); } /** * Test that getDistinctMounts returns all mounts without filters */ public function testGetDistinctMountsWithoutFilters(): void { - $result = iterator_to_array($this->fileAccess->getDistinctMounts()); + $result = iterator_to_array($this->fileAccess->getDistinctMounts([], false)); - $this->assertCount(2, $result); + $this->assertCount(3, $result); $this->assertEquals([ 'storage_id' => 1, @@ -92,13 +103,19 @@ class FileAccessTest extends TestCase { 'root_id' => 30, 'overridden_root' => 30, ], $result[1]); + + $this->assertEquals([ + 'storage_id' => 4, + 'root_id' => 31, + 'overridden_root' => 31, + ], $result[2]); } /** * Test that getDistinctMounts applies filtering by mount providers */ public function testGetDistinctMountsWithMountProviderFilter(): void { - $result = iterator_to_array($this->fileAccess->getDistinctMounts(['TestProviderClass1'])); + $result = iterator_to_array($this->fileAccess->getDistinctMounts(['TestProviderClass1'], false)); $this->assertCount(2, $result); @@ -131,6 +148,18 @@ class FileAccessTest extends TestCase { ]) ->executeStatement(); + // Add a mount that is mounted in the home directory + $queryBuilder = $this->dbConnection->getQueryBuilder(); + $queryBuilder->insert('mounts') + ->values([ + 'storage_id' => $queryBuilder->createNamedParameter(5, IQueryBuilder::PARAM_INT), + 'root_id' => $queryBuilder->createNamedParameter(41, IQueryBuilder::PARAM_INT), + 'mount_provider_class' => $queryBuilder->createNamedParameter('TestMountProvider3'), + 'mount_point' => $queryBuilder->createNamedParameter('/test/files/foobar'), + 'user_id' => $queryBuilder->createNamedParameter('test'), + ]) + ->executeStatement(); + // Simulate adding a "files" directory to the filecache table $queryBuilder = $this->dbConnection->getQueryBuilder()->runAcrossAllShards(); $queryBuilder->delete('filecache')->executeStatement(); @@ -148,11 +177,19 @@ class FileAccessTest extends TestCase { $result = iterator_to_array($this->fileAccess->getDistinctMounts()); + $this->assertCount(2, $result); + $this->assertEquals([ 'storage_id' => 4, 'root_id' => 40, 'overridden_root' => 99, - ], end($result)); + ], $result[0]); + + $this->assertEquals([ + 'storage_id' => 5, + 'root_id' => 41, + 'overridden_root' => 41, + ], $result[1]); } private function setUpTestDatabaseForGetByAncestorInStorage(): void {