Skip to content

test(files): make FolderTest multi-storage pagination coverage deterministic #61579

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
joshtrichards wants to merge 2 commits into
base: master
Choose a base branch
from
+130 −60
Draft

test(files): make FolderTest multi-storage pagination coverage deterministic #61579

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ac92581
refactor(files): simplify Folder::search result handling and validation
joshtrichards Jun 24, 2026
02a6b9f
test(files): fix flaky sub-storage search pagination assertions
joshtrichards Jun 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 27 additions & 21 deletions lib/private/Files/Node/Folder.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,46 +206,52 @@ private function queryFromOperator(ISearchOperator $operator, ?string $uid = nul
}

/**
* search for files with the name matching $query
*
* @param string|ISearchQuery $query
* @return \OC\Files\Node\Node[]
* @return Node[]
*/
#[\Override]
public function search($query) {
if (is_string($query)) {
$query = $this->queryFromOperator(new SearchComparison(ISearchComparison::COMPARE_LIKE, 'name', '%' . $query . '%'));
}

// search is handled by a single query covering all caches that this folder contains
// this is done by collect

$currentPath = $this->path;
$limitToHome = $query->limitToHome();
if ($limitToHome && count(explode('/', $this->path)) !== 3) {
throw new \InvalidArgumentException('searching by owner is only allowed in the users home folder');
$pathParts = explode('/', trim($currentPath, '/'));
$isUserHomeFolder = count($pathParts) === 2 && $pathParts[1] === 'files';
if ($limitToHome && !$isUserHomeFolder) {
throw new \InvalidArgumentException('Searching by owner is only allowed in a user home folder');
}

/** @var QuerySearchHelper $searchHelper */
$searchHelper = Server::get(QuerySearchHelper::class);
[$caches, $mountByMountPoint] = $searchHelper->getCachesAndMountPointsForSearch($this->root, $this->path, $limitToHome);

// Execute one logical search across all caches reachable from this folder.
// The helper returns results grouped by mount point; we merge them and apply ordering below.
[$caches, $mountByMountPoint] = $searchHelper->getCachesAndMountPointsForSearch(
$this->root,
$currentPath,
$limitToHome,
);
$resultsPerCache = $searchHelper->searchInCaches($query, $caches);

// loop through all results per-cache, constructing the FileInfo object from the CacheEntry and merge them all
$files = array_merge(...array_map(function (array $results, string $relativeMountPoint) use ($mountByMountPoint) {
// Flatten per-cache results into FileInfo objects using their mount context.
$files = [];
foreach ($resultsPerCache as $relativeMountPoint => $results) {
$mount = $mountByMountPoint[$relativeMountPoint];
return array_map(function (ICacheEntry $result) use ($relativeMountPoint, $mount) {
return $this->cacheEntryToFileInfo($mount, $relativeMountPoint, $result);
}, $results);
}, array_values($resultsPerCache), array_keys($resultsPerCache)));

// don't include this folder in the results
$files = array_values(array_filter($files, function (FileInfo $file) {
return $file->getPath() !== $this->getPath();
foreach ($results as $result) {
$files[] = $this->cacheEntryToFileInfo($mount, $relativeMountPoint, $result);
}
}

// Exclude the folder being searched itself from the result set.
$files = array_values(array_filter($files, function (FileInfo $file) use ($currentPath) {
return $file->getPath() !== $currentPath;
}));

// since results were returned per-cache, they are no longer fully sorted
// Apply the requested ordering to the merged result set.
$order = $query->getOrder();
if ($order) {
if ($order && count($files) > 1) {
usort($files, function (FileInfo $a, FileInfo $b) use ($order) {
foreach ($order as $orderField) {
$cmp = $orderField->sortFileInfo($a, $b);
Expand Down
142 changes: 103 additions & 39 deletions tests/lib/Files/Node/FolderTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -934,40 +934,102 @@ public function testRecentJail(): void {
$this->assertEquals([$id1], $ids);
}

public static function offsetLimitProvider(): array {
public static function defaultOffsetLimitProvider(): array {
return [
[0, 10, ['/bar/foo/foo1', '/bar/foo/foo2', '/bar/foo/foo3', '/bar/foo/foo4', '/bar/foo/sub1/foo5', '/bar/foo/sub1/foo6', '/bar/foo/sub2/foo7', '/bar/foo/sub2/foo8'], []],
[0, 5, ['/bar/foo/foo1', '/bar/foo/foo2', '/bar/foo/foo3', '/bar/foo/foo4', '/bar/foo/sub1/foo5'], []],
[0, 2, ['/bar/foo/foo1', '/bar/foo/foo2'], []],
[3, 2, ['/bar/foo/foo4', '/bar/foo/sub1/foo5'], []],
[3, 5, ['/bar/foo/foo4', '/bar/foo/sub1/foo5', '/bar/foo/sub1/foo6', '/bar/foo/sub2/foo7', '/bar/foo/sub2/foo8'], []],
[5, 2, ['/bar/foo/sub1/foo6', '/bar/foo/sub2/foo7'], []],
[6, 2, ['/bar/foo/sub2/foo7', '/bar/foo/sub2/foo8'], []],
[7, 2, ['/bar/foo/sub2/foo8'], []],
[10, 2, [], []],
[0, 5, ['/bar/foo/sub2/foo7', '/bar/foo/foo1', '/bar/foo/sub1/foo5', '/bar/foo/foo2', '/bar/foo/foo3'], [new SearchOrder(ISearchOrder::DIRECTION_ASCENDING, 'mtime')]],
[3, 2, ['/bar/foo/foo2', '/bar/foo/foo3'], [new SearchOrder(ISearchOrder::DIRECTION_ASCENDING, 'mtime')]],
[0, 5, ['/bar/foo/sub1/foo5', '/bar/foo/sub1/foo6', '/bar/foo/sub2/foo7', '/bar/foo/foo1', '/bar/foo/foo2'], [
new SearchOrder(ISearchOrder::DIRECTION_DESCENDING, 'size'),
new SearchOrder(ISearchOrder::DIRECTION_ASCENDING, 'mtime')
]],
[0, 10, 8],
[0, 5, 5],
[0, 2, 2],
[3, 2, 2],
[3, 5, 5],
[5, 2, 2],
[6, 2, 2],
[7, 2, 1],
[10, 2, 0],
];
}

/**
* @param int $offset
* @param int $limit
* @param string[] $expectedPaths
* @param ISearchOrder[] $ordering
* @throws NotFoundException
* @throws InvalidPathException
* @return array<int, array{0: int, 1: int, 2: string[], 3: SearchOrder[]}>
*/
#[\PHPUnit\Framework\Attributes\DataProvider('offsetLimitProvider')]
public function testSearchSubStoragesLimitOffset(int $offset, int $limit, array $expectedPaths, array $ordering): void {
if (!$ordering) {
$ordering = [new SearchOrder(ISearchOrder::DIRECTION_ASCENDING, 'fileid')];
public static function orderedOffsetLimitProvider(): array {
return [
[
0, 5, ['/bar/foo/sub2/foo7', '/bar/foo/foo1', '/bar/foo/sub1/foo5', '/bar/foo/foo2', '/bar/foo/foo3'],
[
new SearchOrder(ISearchOrder::DIRECTION_ASCENDING, 'mtime'),
],
],
[
3, 2, ['/bar/foo/foo2', '/bar/foo/foo3'],
[
new SearchOrder(ISearchOrder::DIRECTION_ASCENDING, 'mtime'),
],
],
[
0, 5, ['/bar/foo/sub1/foo5', '/bar/foo/sub1/foo6', '/bar/foo/sub2/foo7', '/bar/foo/foo1', '/bar/foo/foo2'],
[
new SearchOrder(ISearchOrder::DIRECTION_DESCENDING, 'size'),
new SearchOrder(ISearchOrder::DIRECTION_ASCENDING, 'mtime'),
],
],
];
}

#[\PHPUnit\Framework\Attributes\DataProvider('defaultOffsetLimitProvider')]
public function testSearchSubStoragesLimitOffsetDefaultOrdering(int $offset, int $limit, int $expectedCount): void {
[$node, $cache, $subCache1, $subCache2] = $this->createFolderForSubStorageSearch();

try {
$query = $this->createSubStorageSearchQuery($limit, $offset, []);
$result = $node->search($query);

$paths = array_map(fn (Node $info) => $info->getPath(), $result);

$this->assertCount($expectedCount, $paths);
$this->assertSame($expectedCount, count(array_unique($paths)));

$allExpectedPaths = [
'/bar/foo/foo1',
'/bar/foo/foo2',
'/bar/foo/foo3',
'/bar/foo/foo4',
'/bar/foo/sub1/foo5',
'/bar/foo/sub1/foo6',
'/bar/foo/sub2/foo7',
'/bar/foo/sub2/foo8',
];

foreach ($paths as $path) {
$this->assertContains($path, $allExpectedPaths);
}
} finally {
$cache->clear();
$subCache1->clear();
$subCache2->clear();
}
}

#[\PHPUnit\Framework\Attributes\DataProvider('orderedOffsetLimitProvider')]
public function testSearchSubStoragesLimitOffsetWithExplicitOrdering(int $offset, int $limit, array $expectedPaths, array $ordering): void {
[$node, $cache, $subCache1, $subCache2] = $this->createFolderForSubStorageSearch();

try {
$query = $this->createSubStorageSearchQuery($limit, $offset, $ordering);
$result = $node->search($query);

$paths = array_map(fn (Node $info) => $info->getPath(), $result);
$this->assertSame($expectedPaths, $paths);
} finally {
$cache->clear();
$subCache1->clear();
$subCache2->clear();
}
}

/**
* @return array{0: Folder, 1: Cache, 2: Cache, 3: Cache}
*/
private function createFolderForSubStorageSearch(): array {
$manager = $this->createMock(Manager::class);
$view = $this->getRootViewMock();
$root = $this->getMockBuilder(Root::class)
Expand All @@ -976,13 +1038,16 @@ public function testSearchSubStoragesLimitOffset(int $offset, int $limit, array
$root->expects($this->any())
->method('getUser')
->willReturn($this->user);

$storage = $this->createMock(IStorage::class);
$storage->method('getId')->willReturn('test::1');
$cache = new Cache($storage);

$subStorage1 = $this->createMock(IStorage::class);
$subStorage1->method('getId')->willReturn('test::2');
$subCache1 = new Cache($subStorage1);
$subMount1 = $this->getMockBuilder(MountPoint::class)->setConstructorArgs([Temporary::class, ''])->getMock();

$subStorage2 = $this->createMock(IStorage::class);
$subStorage2->method('getId')->willReturn('test::3');
$subCache2 = new Cache($subStorage2);
Expand All @@ -996,7 +1061,6 @@ public function testSearchSubStoragesLimitOffset(int $offset, int $limit, array

$subMount1->method('getStorage')
->willReturn($subStorage1);

$subMount1->method('getMountPoint')
->willReturn('/bar/foo/sub1/');

Expand All @@ -1012,7 +1076,6 @@ public function testSearchSubStoragesLimitOffset(int $offset, int $limit, array

$subMount2->method('getStorage')
->willReturn($subStorage2);

$subMount2->method('getMountPoint')
->willReturn('/bar/foo/sub2/');

Expand Down Expand Up @@ -1044,21 +1107,22 @@ public function testSearchSubStoragesLimitOffset(int $offset, int $limit, array
->with('/bar/foo')
->willReturn($mount);

$node = new Folder($root, $view, '/bar/foo');
return [new Folder($root, $view, '/bar/foo'), $cache, $subCache1, $subCache2];
}

/**
* @param SearchOrder[] $ordering
*/
private function createSubStorageSearchQuery(int $limit, int $offset, array $ordering): SearchQuery {
$comparison = new SearchComparison(ISearchComparison::COMPARE_LIKE, 'name', '%foo%');
$operator = new SearchBinaryOperator(ISearchBinaryOperator::OPERATOR_AND, [
$comparison,
new SearchBinaryOperator(ISearchBinaryOperator::OPERATOR_NOT, [new SearchComparison(ISearchComparison::COMPARE_EQUAL, 'mimetype', ICacheEntry::DIRECTORY_MIMETYPE)]),
new SearchBinaryOperator(ISearchBinaryOperator::OPERATOR_NOT, [
new SearchComparison(ISearchComparison::COMPARE_EQUAL, 'mimetype', ICacheEntry::DIRECTORY_MIMETYPE),
]),
]);
$query = new SearchQuery($operator, $limit, $offset, $ordering);
$result = $node->search($query);
$cache->clear();
$subCache1->clear();
$subCache2->clear();
$ids = array_map(function (Node $info) {
return $info->getPath();
}, $result);
$this->assertEquals($expectedPaths, $ids);

return new SearchQuery($operator, $limit, $offset, $ordering);
}

public static function dataGetOrCreateFolder(): \Generator {
Expand Down
Loading