open-dsi
/
nextcloud-server
mirror of https://github.com/nextcloud/server
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

fix(FileAccessTest): Adress review comments

Browse Source 
Signed-off-by: Marcel Klehr <mklehr@gmx.net>
pull/51810/head
Marcel Klehr 6 months ago
parent 26f6013c1f
commit 131125bbb7
3 changed files with 58 additions and 55 deletions
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Show Stats Download Patch File Download Diff File
  1. 34
      lib/private/Files/Cache/FileAccess.php
  2. 16
      lib/public/Files/Cache/IFileAccess.php
  3. 63
      tests/lib/Files/Cache/FileAccessTest.php

34
lib/private/Files/Cache/FileAccess.php
Unescape View File

@ -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<int> $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<int> $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<string> $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();

16
lib/public/Files/Cache/IFileAccess.php
Unescape View File

@ -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<int> $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<int> $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<string> $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;
}

63
tests/lib/Files/Cache/FileAccessTest.php
Unescape View File

@ -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);

Write Preview
Loading…
Cancel
Save