From 423e1581e0602a10ac78ca767b41c082639ca4e0 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Thu, 18 Mar 2021 17:38:31 +0100 Subject: [PATCH] handle limit and offset in folder file search Signed-off-by: Robin Appelman --- lib/private/Files/Node/Folder.php | 57 +++++++- tests/lib/Files/Node/FolderTest.php | 198 ++++++++++++++++++++++------ 2 files changed, 208 insertions(+), 47 deletions(-) diff --git a/lib/private/Files/Node/Folder.php b/lib/private/Files/Node/Folder.php index 90e00e164a..ecfaeb9a47 100644 --- a/lib/private/Files/Node/Folder.php +++ b/lib/private/Files/Node/Folder.php @@ -207,10 +207,8 @@ class Folder extends Node implements \OCP\Files\Folder { } private function queryFromOperator(ISearchOperator $operator, string $uid = null): ISearchQuery { - if (!$uid) { - /** @var IUserSession $session */ - $session = \OC::$server->query(IUserSession::class); - $user = $session->getUser(); + if ($uid === null) { + $user = null; } else { /** @var IUserManager $userManager */ $userManager = \OC::$server->query(IUserManager::class); @@ -230,11 +228,35 @@ class Folder extends Node implements \OCP\Files\Folder { $query = $this->queryFromOperator(new SearchComparison(ISearchComparison::COMPARE_LIKE, 'name', '%' . $query . '%')); } + // assume a setup where the root mount matches 15 items, + // sub mount1 matches 7 items and mount2 matches 1 item + // a search with (0..10) returns 10 results from root with internal offset 0 and limit 10 + // follow up search with (10..20) returns 5 results from root with internal offset 10 and limit 10 + // and 5 results from mount1 with internal offset 0 and limit 5 + // next search with (20..30) return 0 results from root with internal offset 20 and limit 10 + // 2 results from mount1 with internal offset 5[1] and limit 5 + // and 1 result from mount2 with internal offset 0[1] and limit 8 + // + // Because of the difficulty of calculating the offset for a sub-query if the previous one returns no results + // (we don't know how many results the previous sub-query has skipped with it's own offset) + // we instead discard the offset for the sub-queries and filter it afterwards and add the offset to limit. + // this is sub-optimal but shouldn't hurt to much since large offsets are uncommon in practice + $limitToHome = $query->limitToHome(); if ($limitToHome && count(explode('/', $this->path)) !== 3) { throw new \InvalidArgumentException('searching by owner is only allows on the users home folder'); } + $subQueryLimit = $query->getLimit() > 0 ? $query->getLimit() + $query->getOffset() : PHP_INT_MAX; + $subQueryOffset = $query->getOffset(); + $noLimitQuery = new SearchQuery( + $query->getSearchOperation(), + $subQueryLimit, + 0, + $query->getOrder(), + $query->getUser() + ); + $files = []; $rootLength = strlen($this->path); $mount = $this->root->getMount($this->path); @@ -248,7 +270,12 @@ class Folder extends Node implements \OCP\Files\Folder { $cache = $storage->getCache(''); - $results = $cache->searchQuery($query); + $results = $cache->searchQuery($noLimitQuery); + $count = count($results); + $results = array_slice($results, $subQueryOffset, $subQueryLimit); + $subQueryOffset = max(0, $subQueryOffset - $count); + $subQueryLimit = max(0, $subQueryLimit - $count); + foreach ($results as $result) { if ($internalRootLength === 0 or substr($result['path'], 0, $internalRootLength) === $internalPath) { $result['internalPath'] = $result['path']; @@ -261,12 +288,30 @@ class Folder extends Node implements \OCP\Files\Folder { if (!$limitToHome) { $mounts = $this->root->getMountsIn($this->path); foreach ($mounts as $mount) { + if ($subQueryLimit <= 0) { + break; + } + + $subQuery = new SearchQuery( + $query->getSearchOperation(), + $subQueryLimit, + 0, + $query->getOrder(), + $query->getUser() + ); + $storage = $mount->getStorage(); if ($storage) { $cache = $storage->getCache(''); $relativeMountPoint = ltrim(substr($mount->getMountPoint(), $rootLength), '/'); - $results = $cache->searchQuery($query); + $results = $cache->searchQuery($subQuery); + + $count = count($results); + $results = array_slice($results, $subQueryOffset, $subQueryLimit); + $subQueryOffset -= $count; + $subQueryLimit -= $count; + foreach ($results as $result) { $result['internalPath'] = $result['path']; $result['path'] = $relativeMountPoint . $result['path']; diff --git a/tests/lib/Files/Node/FolderTest.php b/tests/lib/Files/Node/FolderTest.php index 6d3367ac09..0dfa9be027 100644 --- a/tests/lib/Files/Node/FolderTest.php +++ b/tests/lib/Files/Node/FolderTest.php @@ -14,13 +14,18 @@ use OC\Files\Config\CachedMountInfo; use OC\Files\FileInfo; use OC\Files\Mount\Manager; use OC\Files\Mount\MountPoint; +use OC\Files\Node\Folder; use OC\Files\Node\Node; use OC\Files\Node\Root; +use OC\Files\Search\SearchComparison; +use OC\Files\Search\SearchQuery; use OC\Files\Storage\Temporary; use OC\Files\Storage\Wrapper\Jail; use OC\Files\View; use OCP\Files\Mount\IMountPoint; use OCP\Files\NotFoundException; +use OCP\Files\Search\ISearchComparison; +use OCP\Files\Search\ISearchQuery; use OCP\Files\Storage; /** @@ -32,7 +37,7 @@ use OCP\Files\Storage; */ class FolderTest extends NodeTest { protected function createTestNode($root, $view, $path) { - return new \OC\Files\Node\Folder($root, $view, $path); + return new Folder($root, $view, $path); } protected function getNodeClass() { @@ -65,10 +70,10 @@ class FolderTest extends NodeTest { ->with('/bar/foo') ->willReturn([ new FileInfo('/bar/foo/asd', null, 'foo/asd', ['fileid' => 2, 'path' => '/bar/foo/asd', 'name' => 'asd', 'size' => 100, 'mtime' => 50, 'mimetype' => 'text/plain'], null), - new FileInfo('/bar/foo/qwerty', null, 'foo/qwerty', ['fileid' => 3, 'path' => '/bar/foo/qwerty', 'name' => 'qwerty', 'size' => 200, 'mtime' => 55, 'mimetype' => 'httpd/unix-directory'], null) + new FileInfo('/bar/foo/qwerty', null, 'foo/qwerty', ['fileid' => 3, 'path' => '/bar/foo/qwerty', 'name' => 'qwerty', 'size' => 200, 'mtime' => 55, 'mimetype' => 'httpd/unix-directory'], null), ]); - $node = new \OC\Files\Node\Folder($root, $view, '/bar/foo'); + $node = new Folder($root, $view, '/bar/foo'); $children = $node->getDirectoryListing(); $this->assertEquals(2, count($children)); $this->assertInstanceOf('\OC\Files\Node\File', $children[0]); @@ -96,7 +101,7 @@ class FolderTest extends NodeTest { ->method('get') ->with('/bar/foo/asd'); - $node = new \OC\Files\Node\Folder($root, $view, '/bar/foo'); + $node = new Folder($root, $view, '/bar/foo'); $node->get('asd'); } @@ -113,14 +118,14 @@ class FolderTest extends NodeTest { ->method('getUser') ->willReturn($this->user); - $child = new \OC\Files\Node\Folder($root, $view, '/bar/foo/asd'); + $child = new Folder($root, $view, '/bar/foo/asd'); $root->expects($this->once()) ->method('get') ->with('/bar/foo/asd') ->willReturn($child); - $node = new \OC\Files\Node\Folder($root, $view, '/bar/foo'); + $node = new Folder($root, $view, '/bar/foo'); $this->assertTrue($node->nodeExists('asd')); } @@ -142,7 +147,7 @@ class FolderTest extends NodeTest { ->with('/bar/foo/asd') ->will($this->throwException(new NotFoundException())); - $node = new \OC\Files\Node\Folder($root, $view, '/bar/foo'); + $node = new Folder($root, $view, '/bar/foo'); $this->assertFalse($node->nodeExists('asd')); } @@ -169,8 +174,8 @@ class FolderTest extends NodeTest { ->with('/bar/foo/asd') ->willReturn(true); - $node = new \OC\Files\Node\Folder($root, $view, '/bar/foo'); - $child = new \OC\Files\Node\Folder($root, $view, '/bar/foo/asd'); + $node = new Folder($root, $view, '/bar/foo'); + $child = new Folder($root, $view, '/bar/foo/asd'); $result = $node->newFolder('asd'); $this->assertEquals($child, $result); } @@ -196,7 +201,7 @@ class FolderTest extends NodeTest { ->with('/bar/foo') ->willReturn($this->getFileInfo(['permissions' => \OCP\Constants::PERMISSION_READ])); - $node = new \OC\Files\Node\Folder($root, $view, '/bar/foo'); + $node = new Folder($root, $view, '/bar/foo'); $node->newFolder('asd'); } @@ -223,7 +228,7 @@ class FolderTest extends NodeTest { ->with('/bar/foo/asd') ->willReturn(true); - $node = new \OC\Files\Node\Folder($root, $view, '/bar/foo'); + $node = new Folder($root, $view, '/bar/foo'); $child = new \OC\Files\Node\File($root, $view, '/bar/foo/asd'); $result = $node->newFile('asd'); $this->assertEquals($child, $result); @@ -250,7 +255,7 @@ class FolderTest extends NodeTest { ->with('/bar/foo') ->willReturn($this->getFileInfo(['permissions' => \OCP\Constants::PERMISSION_READ])); - $node = new \OC\Files\Node\Folder($root, $view, '/bar/foo'); + $node = new Folder($root, $view, '/bar/foo'); $node->newFile('asd'); } @@ -272,7 +277,7 @@ class FolderTest extends NodeTest { ->with('/bar/foo') ->willReturn(100); - $node = new \OC\Files\Node\Folder($root, $view, '/bar/foo'); + $node = new Folder($root, $view, '/bar/foo'); $this->assertEquals(100, $node->getFreeSpace()); } @@ -307,7 +312,7 @@ class FolderTest extends NodeTest { $cache->expects($this->once()) ->method('searchQuery') ->willReturn([ - new CacheEntry(['fileid' => 3, 'path' => 'foo/qwerty', 'name' => 'qwerty', 'size' => 200, 'mtime' => 55, 'mimetype' => 'text/plain']) + new CacheEntry(['fileid' => 3, 'path' => 'foo/qwerty', 'name' => 'qwerty', 'size' => 200, 'mtime' => 55, 'mimetype' => 'text/plain']), ]); $root->expects($this->once()) @@ -320,7 +325,7 @@ class FolderTest extends NodeTest { ->with('/bar/foo') ->willReturn($mount); - $node = new \OC\Files\Node\Folder($root, $view, '/bar/foo'); + $node = new Folder($root, $view, '/bar/foo'); $result = $node->search('qw'); $this->assertEquals(1, count($result)); $this->assertEquals('/bar/foo/qwerty', $result[0]->getPath()); @@ -409,7 +414,7 @@ class FolderTest extends NodeTest { $cache->expects($this->once()) ->method('searchQuery') ->willReturn([ - new CacheEntry(['fileid' => 3, 'path' => 'foo/qwerty', 'name' => 'qwerty', 'size' => 200, 'mtime' => 55, 'mimetype' => 'text/plain']) + new CacheEntry(['fileid' => 3, 'path' => 'foo/qwerty', 'name' => 'qwerty', 'size' => 200, 'mtime' => 55, 'mimetype' => 'text/plain']), ]); $root->expects($this->once()) @@ -422,7 +427,7 @@ class FolderTest extends NodeTest { ->with('/bar') ->willReturn($mount); - $node = new \OC\Files\Node\Folder($root, $view, '/bar'); + $node = new Folder($root, $view, '/bar'); $result = $node->search('qw'); $this->assertEquals(1, count($result)); $this->assertEquals('/bar/foo/qwerty', $result[0]->getPath()); @@ -474,13 +479,13 @@ class FolderTest extends NodeTest { $cache->expects($this->once()) ->method('searchQuery') ->willReturn([ - new CacheEntry(['fileid' => 3, 'path' => 'foo/qwerty', 'name' => 'qwerty', 'size' => 200, 'mtime' => 55, 'mimetype' => 'text/plain']) + new CacheEntry(['fileid' => 3, 'path' => 'foo/qwerty', 'name' => 'qwerty', 'size' => 200, 'mtime' => 55, 'mimetype' => 'text/plain']), ]); $subCache->expects($this->once()) ->method('searchQuery') ->willReturn([ - new CacheEntry(['fileid' => 4, 'path' => 'asd/qweasd', 'name' => 'qweasd', 'size' => 200, 'mtime' => 55, 'mimetype' => 'text/plain']) + new CacheEntry(['fileid' => 4, 'path' => 'asd/qweasd', 'name' => 'qweasd', 'size' => 200, 'mtime' => 55, 'mimetype' => 'text/plain']), ]); $root->expects($this->once()) @@ -494,14 +499,14 @@ class FolderTest extends NodeTest { ->willReturn($mount); - $node = new \OC\Files\Node\Folder($root, $view, '/bar/foo'); + $node = new Folder($root, $view, '/bar/foo'); $result = $node->search('qw'); $this->assertEquals(2, count($result)); } public function testIsSubNode() { $file = new Node(null, null, '/foo/bar'); - $folder = new \OC\Files\Node\Folder(null, null, '/foo'); + $folder = new Folder(null, null, '/foo'); $this->assertTrue($folder->isSubNode($file)); $this->assertFalse($folder->isSubNode($folder)); @@ -557,7 +562,7 @@ class FolderTest extends NodeTest { ->with('/bar/foo') ->willReturn($mount); - $node = new \OC\Files\Node\Folder($root, $view, '/bar/foo'); + $node = new Folder($root, $view, '/bar/foo'); $result = $node->getById(1); $this->assertEquals(1, count($result)); $this->assertEquals('/bar/foo/qwerty', $result[0]->getPath()); @@ -606,7 +611,7 @@ class FolderTest extends NodeTest { ->with('/bar') ->willReturn($mount); - $node = new \OC\Files\Node\Folder($root, $view, '/bar'); + $node = new Folder($root, $view, '/bar'); $result = $node->getById(1); $this->assertEquals(1, count($result)); $this->assertEquals('/bar', $result[0]->getPath()); @@ -660,7 +665,7 @@ class FolderTest extends NodeTest { ->with('/bar/foo') ->willReturn($mount); - $node = new \OC\Files\Node\Folder($root, $view, '/bar/foo'); + $node = new Folder($root, $view, '/bar/foo'); $result = $node->getById(1); $this->assertEquals(0, count($result)); } @@ -706,7 +711,7 @@ class FolderTest extends NodeTest { '/bar/foo/asd/', 1, '' - ) + ), ]); $storage->expects($this->any()) @@ -728,7 +733,7 @@ class FolderTest extends NodeTest { ->with('/bar/foo') ->willReturn($mount1); - $node = new \OC\Files\Node\Folder($root, $view, '/bar/foo'); + $node = new Folder($root, $view, '/bar/foo'); $result = $node->getById(1); $this->assertEquals(2, count($result)); $this->assertEquals('/bar/foo/qwerty', $result[0]->getPath()); @@ -740,7 +745,7 @@ class FolderTest extends NodeTest { // input, existing, expected ['foo', [], 'foo'], ['foo', ['foo'], 'foo (2)'], - ['foo', ['foo', 'foo (2)'], 'foo (3)'] + ['foo', ['foo', 'foo (2)'], 'foo (3)'], ]; } @@ -770,7 +775,7 @@ class FolderTest extends NodeTest { return false; }); - $node = new \OC\Files\Node\Folder($root, $view, $folderPath); + $node = new Folder($root, $view, $folderPath); $this->assertEquals($expected, $node->getNonExistingName($name)); } @@ -805,30 +810,30 @@ class FolderTest extends NodeTest { 'mtime' => $baseTime, 'mimetype' => 'text/plain', 'size' => 3, - 'permissions' => \OCP\Constants::PERMISSION_ALL + 'permissions' => \OCP\Constants::PERMISSION_ALL, ]); $id2 = $cache->put('bar/foo/old.txt', [ 'storage_mtime' => $baseTime - 100, 'mtime' => $baseTime - 100, 'mimetype' => 'text/plain', 'size' => 3, - 'permissions' => \OCP\Constants::PERMISSION_READ + 'permissions' => \OCP\Constants::PERMISSION_READ, ]); $cache->put('bar/asd/outside.txt', [ 'storage_mtime' => $baseTime, 'mtime' => $baseTime, 'mimetype' => 'text/plain', - 'size' => 3 + 'size' => 3, ]); $id3 = $cache->put('bar/foo/older.txt', [ 'storage_mtime' => $baseTime - 600, 'mtime' => $baseTime - 600, 'mimetype' => 'text/plain', 'size' => 3, - 'permissions' => \OCP\Constants::PERMISSION_ALL + 'permissions' => \OCP\Constants::PERMISSION_ALL, ]); - $node = new \OC\Files\Node\Folder($root, $view, $folderPath, $folderInfo); + $node = new Folder($root, $view, $folderPath, $folderInfo); $nodes = $node->getRecent(5); @@ -869,7 +874,7 @@ class FolderTest extends NodeTest { 'mtime' => $baseTime, 'mimetype' => \OCP\Files\FileInfo::MIMETYPE_FOLDER, 'size' => 3, - 'permissions' => 0 + 'permissions' => 0, ]); $id2 = $cache->put('bar/foo/folder/bar.txt', [ 'storage_mtime' => $baseTime, @@ -877,7 +882,7 @@ class FolderTest extends NodeTest { 'mimetype' => 'text/plain', 'size' => 3, 'parent' => $id1, - 'permissions' => \OCP\Constants::PERMISSION_ALL + 'permissions' => \OCP\Constants::PERMISSION_ALL, ]); $id3 = $cache->put('bar/foo/folder/asd.txt', [ 'storage_mtime' => $baseTime - 100, @@ -885,10 +890,10 @@ class FolderTest extends NodeTest { 'mimetype' => 'text/plain', 'size' => 3, 'parent' => $id1, - 'permissions' => \OCP\Constants::PERMISSION_ALL + 'permissions' => \OCP\Constants::PERMISSION_ALL, ]); - $node = new \OC\Files\Node\Folder($root, $view, $folderPath, $folderInfo); + $node = new Folder($root, $view, $folderPath, $folderInfo); $nodes = $node->getRecent(5); @@ -920,7 +925,7 @@ class FolderTest extends NodeTest { $storage = new Temporary(); $jail = new Jail([ 'storage' => $storage, - 'root' => 'folder' + 'root' => 'folder', ]); $mount = new MountPoint($jail, '/bar/foo'); @@ -935,16 +940,16 @@ class FolderTest extends NodeTest { 'mtime' => $baseTime, 'mimetype' => 'text/plain', 'size' => 3, - 'permissions' => \OCP\Constants::PERMISSION_ALL + 'permissions' => \OCP\Constants::PERMISSION_ALL, ]); $cache->put('outside.txt', [ 'storage_mtime' => $baseTime - 100, 'mtime' => $baseTime - 100, 'mimetype' => 'text/plain', - 'size' => 3 + 'size' => 3, ]); - $node = new \OC\Files\Node\Folder($root, $view, $folderPath, $folderInfo); + $node = new Folder($root, $view, $folderPath, $folderInfo); $nodes = $node->getRecent(5); $ids = array_map(function (Node $node) { @@ -952,4 +957,115 @@ class FolderTest extends NodeTest { }, $nodes); $this->assertEquals([$id1], $ids); } + + public function offsetLimitProvider() { + return [ + [0, 10, [10, 11, 12, 13, 14, 15, 16, 17]], + [0, 5, [10, 11, 12, 13, 14]], + [0, 2, [10, 11]], + [3, 2, [13, 14]], + [3, 5, [13, 14, 15, 16, 17]], + [5, 2, [15, 16]], + [6, 2, [16, 17]], + [7, 2, [17]], + [10, 2, []] + ]; + } + + /** + * @dataProvider offsetLimitProvider + */ + public function testSearchSubStoragesLimitOffset(int $offset, int $limit, array $expectedIds) { + $manager = $this->createMock(Manager::class); + /** + * @var \OC\Files\View | \PHPUnit\Framework\MockObject\MockObject $view + */ + $view = $this->createMock(View::class); + $root = $this->getMockBuilder(Root::class) + ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager]) + ->getMock(); + $root->expects($this->any()) + ->method('getUser') + ->willReturn($this->user); + $storage = $this->createMock(Storage::class); + $storage->method('getId')->willReturn(''); + $cache = $this->getMockBuilder(Cache::class)->setConstructorArgs([$storage])->getMock(); + $subCache1 = $this->getMockBuilder(Cache::class)->setConstructorArgs([$storage])->getMock(); + $subStorage1 = $this->createMock(Storage::class); + $subMount1 = $this->getMockBuilder(MountPoint::class)->setConstructorArgs([null, ''])->getMock(); + $subCache2 = $this->getMockBuilder(Cache::class)->setConstructorArgs([$storage])->getMock(); + $subStorage2 = $this->createMock(Storage::class); + $subMount2 = $this->getMockBuilder(MountPoint::class)->setConstructorArgs([null, ''])->getMock(); + + $mount = $this->createMock(IMountPoint::class); + $mount->method('getStorage') + ->willReturn($storage); + $mount->method('getInternalPath') + ->willReturn('foo'); + + $subMount1->method('getStorage') + ->willReturn($subStorage1); + + $subMount1->method('getMountPoint') + ->willReturn('/bar/foo/bar/'); + + $storage->method('getCache') + ->willReturn($cache); + + $subStorage1->method('getCache') + ->willReturn($subCache1); + + $subMount2->method('getStorage') + ->willReturn($subStorage2); + + $subMount2->method('getMountPoint') + ->willReturn('/bar/foo/bar2/'); + + $subStorage2->method('getCache') + ->willReturn($subCache2); + + $cache->method('searchQuery') + ->willReturnCallback(function (ISearchQuery $query) { + return array_slice([ + new CacheEntry(['fileid' => 10, 'path' => 'foo/qwerty', 'name' => 'qwerty', 'size' => 200, 'mtime' => 55, 'mimetype' => 'text/plain']), + new CacheEntry(['fileid' => 11, 'path' => 'foo/qwerty', 'name' => 'qwerty', 'size' => 200, 'mtime' => 55, 'mimetype' => 'text/plain']), + new CacheEntry(['fileid' => 12, 'path' => 'foo/qwerty', 'name' => 'qwerty', 'size' => 200, 'mtime' => 55, 'mimetype' => 'text/plain']), + new CacheEntry(['fileid' => 13, 'path' => 'foo/qwerty', 'name' => 'qwerty', 'size' => 200, 'mtime' => 55, 'mimetype' => 'text/plain']) + ], $query->getOffset(), $query->getOffset() + $query->getLimit()); + }); + + $subCache1->method('searchQuery') + ->willReturnCallback(function (ISearchQuery $query) { + return array_slice([ + new CacheEntry(['fileid' => 14, 'path' => 'foo/qwerty', 'name' => 'qwerty', 'size' => 200, 'mtime' => 55, 'mimetype' => 'text/plain']), + new CacheEntry(['fileid' => 15, 'path' => 'foo/qwerty', 'name' => 'qwerty', 'size' => 200, 'mtime' => 55, 'mimetype' => 'text/plain']), + ], $query->getOffset(), $query->getOffset() + $query->getLimit()); + }); + + $subCache2->method('searchQuery') + ->willReturnCallback(function (ISearchQuery $query) { + return array_slice([ + new CacheEntry(['fileid' => 16, 'path' => 'foo/qwerty', 'name' => 'qwerty', 'size' => 200, 'mtime' => 55, 'mimetype' => 'text/plain']), + new CacheEntry(['fileid' => 17, 'path' => 'foo/qwerty', 'name' => 'qwerty', 'size' => 200, 'mtime' => 55, 'mimetype' => 'text/plain']), + ], $query->getOffset(), $query->getOffset() + $query->getLimit()); + }); + + $root->method('getMountsIn') + ->with('/bar/foo') + ->willReturn([$subMount1, $subMount2]); + + $root->method('getMount') + ->with('/bar/foo') + ->willReturn($mount); + + + $node = new Folder($root, $view, '/bar/foo'); + $comparison = new SearchComparison(ISearchComparison::COMPARE_LIKE, 'name', '%foo%'); + $query = new SearchQuery($comparison, $limit, $offset, []); + $result = $node->search($query); + $ids = array_map(function(Node $info) { + return $info->getId(); + }, $result); + $this->assertEquals($expectedIds, $ids); + } }