From 60551ced9e4277e6b2f43712c2c323fdfb96ce35 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Thu, 18 Mar 2021 16:22:29 +0100 Subject: [PATCH 01/11] use "newer" node search api directly in unified search Signed-off-by: Robin Appelman --- apps/files/lib/Search/FilesSearchProvider.php | 69 +++++++++++-------- 1 file changed, 40 insertions(+), 29 deletions(-) diff --git a/apps/files/lib/Search/FilesSearchProvider.php b/apps/files/lib/Search/FilesSearchProvider.php index 10ccd76564..189bebaf7b 100644 --- a/apps/files/lib/Search/FilesSearchProvider.php +++ b/apps/files/lib/Search/FilesSearchProvider.php @@ -28,10 +28,13 @@ declare(strict_types=1); namespace OCA\Files\Search; -use OC\Search\Provider\File; -use OC\Search\Result\File as FileResult; +use OC\Files\Search\SearchComparison; +use OC\Files\Search\SearchQuery; +use OCP\Files\FileInfo; use OCP\Files\IMimeTypeDetector; use OCP\Files\IRootFolder; +use OCP\Files\Search\ISearchComparison; +use OCP\Files\Node; use OCP\IL10N; use OCP\IURLGenerator; use OCP\IUser; @@ -42,9 +45,6 @@ use OCP\Search\SearchResultEntry; class FilesSearchProvider implements IProvider { - /** @var File */ - private $fileSearch; - /** @var IL10N */ private $l10n; @@ -57,13 +57,13 @@ class FilesSearchProvider implements IProvider { /** @var IRootFolder */ private $rootFolder; - public function __construct(File $fileSearch, - IL10N $l10n, - IURLGenerator $urlGenerator, - IMimeTypeDetector $mimeTypeDetector, - IRootFolder $rootFolder) { + public function __construct( + IL10N $l10n, + IURLGenerator $urlGenerator, + IMimeTypeDetector $mimeTypeDetector, + IRootFolder $rootFolder + ) { $this->l10n = $l10n; - $this->fileSearch = $fileSearch; $this->urlGenerator = $urlGenerator; $this->mimeTypeDetector = $mimeTypeDetector; $this->rootFolder = $rootFolder; @@ -98,26 +98,37 @@ class FilesSearchProvider implements IProvider { * @inheritDoc */ public function search(IUser $user, ISearchQuery $query): SearchResult { - - // Make sure we setup the users filesystem - $this->rootFolder->getUserFolder($user->getUID()); + $userFolder = $this->rootFolder->getUserFolder($user->getUID()); + $fileQuery = new SearchQuery( + new SearchComparison(ISearchComparison::COMPARE_LIKE, 'name', '%' . $query->getTerm() . '%'), + $query->getLimit(), + (int)$query->getCursor(), + [], + $user + ); return SearchResult::paginated( $this->l10n->t('Files'), - array_map(function (FileResult $result) { + array_map(function (Node $result) use ($userFolder) { // Generate thumbnail url - $thumbnailUrl = $result->has_preview - ? $this->urlGenerator->linkToRouteAbsolute('core.Preview.getPreviewByFileId', ['x' => 32, 'y' => 32, 'fileId' => $result->id]) - : ''; + $thumbnailUrl = $this->urlGenerator->linkToRouteAbsolute('core.Preview.getPreviewByFileId', ['x' => 32, 'y' => 32, 'fileId' => $result->getId()]); + $path = $userFolder->getRelativePath($result->getPath()); + $link = $this->urlGenerator->linkToRoute( + 'files.view.index', + [ + 'dir' => dirname($path), + 'scrollto' => $result->getName(), + ] + ); return new SearchResultEntry( $thumbnailUrl, - $result->name, - $this->formatSubline($result), - $this->urlGenerator->getAbsoluteURL($result->link), - $result->type === 'folder' ? 'icon-folder' : $this->mimeTypeDetector->mimeTypeIcon($result->mime_type) + $result->getName(), + $this->formatSubline($path), + $this->urlGenerator->getAbsoluteURL($link), + $result->getMimetype() === FileInfo::MIMETYPE_FOLDER ? 'icon-folder' : $this->mimeTypeDetector->mimeTypeIcon($result->getMimetype()) ); - }, $this->fileSearch->search($query->getTerm(), $query->getLimit(), (int)$query->getCursor())), + }, $userFolder->search($fileQuery)), $query->getCursor() + $query->getLimit() ); } @@ -125,16 +136,16 @@ class FilesSearchProvider implements IProvider { /** * Format subline for files * - * @param FileResult $result + * @param string $path * @return string */ - private function formatSubline($result): string { + private function formatSubline(string $path): string { // Do not show the location if the file is in root - if ($result->path === '/' . $result->name) { + if (strrpos($path, '/') > 0) { + $path = ltrim(dirname($path), '/'); + return $this->l10n->t('in %s', [$path]); + } else { return ''; } - - $path = ltrim(dirname($result->path), '/'); - return $this->l10n->t('in %s', [$path]); } } From a624243236086ff587a20741dc7cf6462522feb9 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Thu, 18 Mar 2021 16:35:41 +0100 Subject: [PATCH 02/11] unify handling of Folder::search methods into "new" query objects Signed-off-by: Robin Appelman --- lib/private/Files/Node/Folder.php | 106 +++++++++++++++++------------- 1 file changed, 61 insertions(+), 45 deletions(-) diff --git a/lib/private/Files/Node/Folder.php b/lib/private/Files/Node/Folder.php index 134455dd94..858472f851 100644 --- a/lib/private/Files/Node/Folder.php +++ b/lib/private/Files/Node/Folder.php @@ -32,6 +32,8 @@ namespace OC\Files\Node; use OC\DB\QueryBuilder\Literal; +use OC\Files\Search\SearchComparison; +use OC\Files\Search\SearchQuery; use OC\Files\Storage\Wrapper\Jail; use OCA\Files_Sharing\SharedStorage; use OCP\DB\QueryBuilder\IQueryBuilder; @@ -40,7 +42,11 @@ use OCP\Files\FileInfo; use OCP\Files\Mount\IMountPoint; use OCP\Files\NotFoundException; use OCP\Files\NotPermittedException; +use OCP\Files\Search\ISearchComparison; +use OCP\Files\Search\ISearchOperator; use OCP\Files\Search\ISearchQuery; +use OCP\IUserManager; +use OCP\IUserSession; class Folder extends Node implements \OCP\Files\Folder { /** @@ -96,8 +102,8 @@ class Folder extends Node implements \OCP\Files\Folder { /** * get the content of this directory * - * @throws \OCP\Files\NotFoundException * @return Node[] + * @throws \OCP\Files\NotFoundException */ public function getDirectoryListing() { $folderContent = $this->view->getDirectoryContent($this->path); @@ -200,6 +206,19 @@ class Folder extends Node implements \OCP\Files\Folder { throw new NotPermittedException('No create permission for path'); } + private function queryFromOperator(ISearchOperator $operator, string $uid = null): ISearchQuery { + if (!$uid) { + /** @var IUserSession $session */ + $session = \OC::$server->query(IUserSession::class); + $user = $session->getUser(); + } else { + /** @var IUserManager $userManager */ + $userManager = \OC::$server->query(IUserManager::class); + $user = $userManager->get($uid); + } + return new SearchQuery($operator, 0, 0, [], $user); + } + /** * search for files with the name matching $query * @@ -208,40 +227,10 @@ class Folder extends Node implements \OCP\Files\Folder { */ public function search($query) { if (is_string($query)) { - return $this->searchCommon('search', ['%' . $query . '%']); - } else { - return $this->searchCommon('searchQuery', [$query]); + $query = $this->queryFromOperator(new SearchComparison(ISearchComparison::COMPARE_LIKE, 'name', '%' . $query . '%')); } - } - /** - * search for files by mimetype - * - * @param string $mimetype - * @return Node[] - */ - public function searchByMime($mimetype) { - return $this->searchCommon('searchByMime', [$mimetype]); - } - - /** - * search for files by tag - * - * @param string|int $tag name or tag id - * @param string $userId owner of the tags - * @return Node[] - */ - public function searchByTag($tag, $userId) { - return $this->searchCommon('searchByTag', [$tag, $userId]); - } - - /** - * @param string $method cache method - * @param array $args call args - * @return \OC\Files\Node\Node[] - */ - private function searchCommon($method, $args) { - $limitToHome = ($method === 'searchQuery')? $args[0]->limitToHome(): false; + $limitToHome = $query->limitToHome(); if ($limitToHome && count(explode('/', $this->path)) !== 3) { throw new \InvalidArgumentException('searching by owner is only allows on the users home folder'); } @@ -259,7 +248,7 @@ class Folder extends Node implements \OCP\Files\Folder { $cache = $storage->getCache(''); - $results = call_user_func_array([$cache, $method], $args); + $results = $cache->searchQuery($query); foreach ($results as $result) { if ($internalRootLength === 0 or substr($result['path'], 0, $internalRootLength) === $internalPath) { $result['internalPath'] = $result['path']; @@ -277,7 +266,7 @@ class Folder extends Node implements \OCP\Files\Folder { $cache = $storage->getCache(''); $relativeMountPoint = ltrim(substr($mount->getMountPoint(), $rootLength), '/'); - $results = call_user_func_array([$cache, $method], $args); + $results = $cache->searchQuery($query); foreach ($results as $result) { $result['internalPath'] = $result['path']; $result['path'] = $relativeMountPoint . $result['path']; @@ -294,6 +283,33 @@ class Folder extends Node implements \OCP\Files\Folder { }, $files); } + /** + * search for files by mimetype + * + * @param string $mimetype + * @return Node[] + */ + public function searchByMime($mimetype) { + if (strpos($mimetype, '/') === false) { + $query = $this->queryFromOperator(new SearchComparison(ISearchComparison::COMPARE_LIKE, 'mimetype', $mimetype . '/%')); + } else { + $query = $this->queryFromOperator(new SearchComparison(ISearchComparison::COMPARE_EQUAL, 'mimetype', $mimetype)); + } + return $this->search($query); + } + + /** + * search for files by tag + * + * @param string|int $tag name or tag id + * @param string $userId owner of the tags + * @return Node[] + */ + public function searchByTag($tag, $userId) { + $query = $this->queryFromOperator(new SearchComparison(ISearchComparison::COMPARE_EQUAL, 'tagname', $tag), $userId); + return $this->search($query); + } + /** * @param int $id * @return \OC\Files\Node\Node[] @@ -320,7 +336,7 @@ class Folder extends Node implements \OCP\Files\Folder { if (count($mountsContainingFile) === 0) { if ($user === $this->getAppDataDirectoryName()) { - return $this->getByIdInRootMount((int) $id); + return $this->getByIdInRootMount((int)$id); } return []; } @@ -383,11 +399,11 @@ class Folder extends Node implements \OCP\Files\Folder { return [$this->root->createNode( $absolutePath, new \OC\Files\FileInfo( - $absolutePath, - $mount->getStorage(), - $cacheEntry->getPath(), - $cacheEntry, - $mount + $absolutePath, + $mount->getStorage(), + $cacheEntry->getPath(), + $cacheEntry, + $mount ))]; } @@ -518,10 +534,10 @@ class Folder extends Node implements \OCP\Files\Folder { $query->andWhere($storageFilters); $query->andWhere($builder->expr()->orX( - // handle non empty folders separate - $builder->expr()->neq('f.mimetype', $builder->createNamedParameter($folderMimetype, IQueryBuilder::PARAM_INT)), - $builder->expr()->eq('f.size', new Literal(0)) - )) + // handle non empty folders separate + $builder->expr()->neq('f.mimetype', $builder->createNamedParameter($folderMimetype, IQueryBuilder::PARAM_INT)), + $builder->expr()->eq('f.size', new Literal(0)) + )) ->andWhere($builder->expr()->notLike('f.path', $builder->createNamedParameter('files_versions/%'))) ->andWhere($builder->expr()->notLike('f.path', $builder->createNamedParameter('files_trashbin/%'))) ->orderBy('f.mtime', 'DESC') From 4655f6feb1073a799f4be386b7a0b27fc8c252be Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Thu, 18 Mar 2021 17:16:28 +0100 Subject: [PATCH 03/11] only require user to be set in a query that handles tags Signed-off-by: Robin Appelman --- lib/private/Files/Cache/Cache.php | 12 +++++++---- lib/private/Files/Search/SearchQuery.php | 8 +++---- lib/public/Files/Search/ISearchQuery.php | 2 +- tests/lib/Files/Node/FolderTest.php | 27 ++++++++++-------------- 4 files changed, 24 insertions(+), 25 deletions(-) diff --git a/lib/private/Files/Cache/Cache.php b/lib/private/Files/Cache/Cache.php index f1b4b48a1b..b84d9b218b 100644 --- a/lib/private/Files/Cache/Cache.php +++ b/lib/private/Files/Cache/Cache.php @@ -190,10 +190,10 @@ class Cache implements ICache { } $data['permissions'] = (int)$data['permissions']; if (isset($data['creation_time'])) { - $data['creation_time'] = (int) $data['creation_time']; + $data['creation_time'] = (int)$data['creation_time']; } if (isset($data['upload_time'])) { - $data['upload_time'] = (int) $data['upload_time']; + $data['upload_time'] = (int)$data['upload_time']; } return new CacheEntry($data); } @@ -819,6 +819,10 @@ class Cache implements ICache { $query->whereStorageId(); if ($this->querySearchHelper->shouldJoinTags($searchQuery->getSearchOperation())) { + $user = $searchQuery->getUser(); + if ($user === null) { + throw new \InvalidArgumentException("Searching by tag requires the user to be set in the query"); + } $query ->innerJoin('file', 'vcategory_to_object', 'tagmap', $builder->expr()->eq('file.fileid', 'tagmap.objid')) ->innerJoin('tagmap', 'vcategory', 'tag', $builder->expr()->andX( @@ -826,7 +830,7 @@ class Cache implements ICache { $builder->expr()->eq('tagmap.categoryid', 'tag.id') )) ->andWhere($builder->expr()->eq('tag.type', $builder->createNamedParameter('files'))) - ->andWhere($builder->expr()->eq('tag.uid', $builder->createNamedParameter($searchQuery->getUser()->getUID()))); + ->andWhere($builder->expr()->eq('tag.uid', $builder->createNamedParameter($user->getUID()))); } $searchExpr = $this->querySearchHelper->searchOperatorToDBExpr($builder, $searchQuery->getSearchOperation()); @@ -1007,7 +1011,7 @@ class Cache implements ICache { return null; } - return (string) $path; + return (string)$path; } /** diff --git a/lib/private/Files/Search/SearchQuery.php b/lib/private/Files/Search/SearchQuery.php index b7b8b80110..091fe57d21 100644 --- a/lib/private/Files/Search/SearchQuery.php +++ b/lib/private/Files/Search/SearchQuery.php @@ -37,7 +37,7 @@ class SearchQuery implements ISearchQuery { private $offset; /** @var ISearchOrder[] */ private $order; - /** @var IUser */ + /** @var ?IUser */ private $user; private $limitToHome; @@ -48,7 +48,7 @@ class SearchQuery implements ISearchQuery { * @param int $limit * @param int $offset * @param array $order - * @param IUser $user + * @param ?IUser $user * @param bool $limitToHome */ public function __construct( @@ -56,7 +56,7 @@ class SearchQuery implements ISearchQuery { int $limit, int $offset, array $order, - IUser $user, + ?IUser $user = null, bool $limitToHome = false ) { $this->searchOperation = $searchOperation; @@ -96,7 +96,7 @@ class SearchQuery implements ISearchQuery { } /** - * @return IUser + * @return ?IUser */ public function getUser() { return $this->user; diff --git a/lib/public/Files/Search/ISearchQuery.php b/lib/public/Files/Search/ISearchQuery.php index 4d866f8d7b..dd7c901f7f 100644 --- a/lib/public/Files/Search/ISearchQuery.php +++ b/lib/public/Files/Search/ISearchQuery.php @@ -62,7 +62,7 @@ interface ISearchQuery { /** * The user that issued the search * - * @return IUser + * @return ?IUser * @since 12.0.0 */ public function getUser(); diff --git a/tests/lib/Files/Node/FolderTest.php b/tests/lib/Files/Node/FolderTest.php index 1ba052b8de..6d3367ac09 100644 --- a/tests/lib/Files/Node/FolderTest.php +++ b/tests/lib/Files/Node/FolderTest.php @@ -305,10 +305,9 @@ class FolderTest extends NodeTest { ->willReturn('foo'); $cache->expects($this->once()) - ->method('search') - ->with('%qw%') + ->method('searchQuery') ->willReturn([ - ['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()) @@ -358,11 +357,10 @@ class FolderTest extends NodeTest { ->willReturn($cache); $cache->expects($this->once()) - ->method('search') - ->with('%qw%') + ->method('searchQuery') ->willReturn([ - ['fileid' => 3, 'path' => 'files/foo', 'name' => 'qwerty', 'size' => 200, 'mtime' => 55, 'mimetype' => 'text/plain'], - ['fileid' => 3, 'path' => 'files_trashbin/foo2.d12345', 'name' => 'foo2.d12345', 'size' => 200, 'mtime' => 55, 'mimetype' => 'text/plain'], + new CacheEntry(['fileid' => 3, 'path' => 'files/foo', 'name' => 'qwerty', 'size' => 200, 'mtime' => 55, 'mimetype' => 'text/plain']), + new CacheEntry(['fileid' => 3, 'path' => 'files_trashbin/foo2.d12345', 'name' => 'foo2.d12345', 'size' => 200, 'mtime' => 55, 'mimetype' => 'text/plain']), ]); $root->expects($this->once()) @@ -409,10 +407,9 @@ class FolderTest extends NodeTest { ->willReturn($cache); $cache->expects($this->once()) - ->method('search') - ->with('%qw%') + ->method('searchQuery') ->willReturn([ - ['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()) @@ -475,17 +472,15 @@ class FolderTest extends NodeTest { ->willReturn($subCache); $cache->expects($this->once()) - ->method('search') - ->with('%qw%') + ->method('searchQuery') ->willReturn([ - ['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('search') - ->with('%qw%') + ->method('searchQuery') ->willReturn([ - ['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()) From 6a639ec1d0a9c6825ce694d6cc1b975d45c2be53 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Thu, 18 Mar 2021 17:38:31 +0100 Subject: [PATCH 04/11] 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 858472f851..8964b749cf 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); + } } From dbd8b0ae82a25111870d58943ac330e091140ed4 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Thu, 18 Mar 2021 17:44:30 +0100 Subject: [PATCH 05/11] format Signed-off-by: Robin Appelman --- lib/private/Files/Node/Folder.php | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/private/Files/Node/Folder.php b/lib/private/Files/Node/Folder.php index 8964b749cf..30df1892b8 100644 --- a/lib/private/Files/Node/Folder.php +++ b/lib/private/Files/Node/Folder.php @@ -46,7 +46,6 @@ use OCP\Files\Search\ISearchComparison; use OCP\Files\Search\ISearchOperator; use OCP\Files\Search\ISearchQuery; use OCP\IUserManager; -use OCP\IUserSession; class Folder extends Node implements \OCP\Files\Folder { /** From 4e60b9ffe2617d1735dbd8b2b58838e2f6ec4dd7 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Fri, 19 Mar 2021 13:35:37 +0100 Subject: [PATCH 06/11] cleanup fileinfo creation Signed-off-by: Robin Appelman --- lib/private/Files/Node/Folder.php | 25 +++++--- tests/lib/Files/Node/FolderTest.php | 90 ++++++++++------------------- 2 files changed, 46 insertions(+), 69 deletions(-) diff --git a/lib/private/Files/Node/Folder.php b/lib/private/Files/Node/Folder.php index 30df1892b8..2b785ea47a 100644 --- a/lib/private/Files/Node/Folder.php +++ b/lib/private/Files/Node/Folder.php @@ -32,11 +32,14 @@ namespace OC\Files\Node; use OC\DB\QueryBuilder\Literal; +use OC\Files\Mount\MountPoint; use OC\Files\Search\SearchComparison; use OC\Files\Search\SearchQuery; use OC\Files\Storage\Wrapper\Jail; +use OC\Files\Storage\Storage; use OCA\Files_Sharing\SharedStorage; use OCP\DB\QueryBuilder\IQueryBuilder; +use OCP\Files\Cache\ICacheEntry; use OCP\Files\Config\ICachedMountInfo; use OCP\Files\FileInfo; use OCP\Files\Mount\IMountPoint; @@ -227,6 +230,8 @@ class Folder extends Node implements \OCP\Files\Folder { $query = $this->queryFromOperator(new SearchComparison(ISearchComparison::COMPARE_LIKE, 'name', '%' . $query . '%')); } + // Limit+offset for queries without ordering + // // 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 @@ -240,6 +245,8 @@ class Folder extends Node implements \OCP\Files\Folder { // (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 + // + // All of this is only possible for queries without ordering $limitToHome = $query->limitToHome(); if ($limitToHome && count(explode('/', $this->path)) !== 3) { @@ -277,10 +284,7 @@ class Folder extends Node implements \OCP\Files\Folder { foreach ($results as $result) { if ($internalRootLength === 0 or substr($result['path'], 0, $internalRootLength) === $internalPath) { - $result['internalPath'] = $result['path']; - $result['path'] = substr($result['path'], $internalRootLength); - $result['storage'] = $storage; - $files[] = new \OC\Files\FileInfo($this->path . '/' . $result['path'], $storage, $result['internalPath'], $result, $mount); + $files[] = $this->cacheEntryToFileInfo($mount, '', $internalPath, $result); } } @@ -312,11 +316,7 @@ class Folder extends Node implements \OCP\Files\Folder { $subQueryLimit -= $count; foreach ($results as $result) { - $result['internalPath'] = $result['path']; - $result['path'] = $relativeMountPoint . $result['path']; - $result['storage'] = $storage; - $files[] = new \OC\Files\FileInfo($this->path . '/' . $result['path'], $storage, - $result['internalPath'], $result, $mount); + $files[] = $this->cacheEntryToFileInfo($mount, $relativeMountPoint, '', $result); } } } @@ -327,6 +327,13 @@ class Folder extends Node implements \OCP\Files\Folder { }, $files); } + private function cacheEntryToFileInfo(IMountPoint $mount, string $appendRoot, string $trimRoot, ICacheEntry $cacheEntry): FileInfo { + $trimLength = strlen($trimRoot); + $cacheEntry['internalPath'] = $cacheEntry['path']; + $cacheEntry['path'] = $appendRoot . substr($cacheEntry['path'], $trimLength); + return new \OC\Files\FileInfo($this->path . '/' . $cacheEntry['path'], $mount->getStorage(), $cacheEntry['internalPath'], $cacheEntry, $mount); + } + /** * search for files by mimetype * diff --git a/tests/lib/Files/Node/FolderTest.php b/tests/lib/Files/Node/FolderTest.php index 0dfa9be027..339a77edcc 100644 --- a/tests/lib/Files/Node/FolderTest.php +++ b/tests/lib/Files/Node/FolderTest.php @@ -290,38 +290,31 @@ class FolderTest extends NodeTest { $root = $this->getMockBuilder(Root::class) ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager]) ->getMock(); - $root->expects($this->any()) - ->method('getUser') + $root->method('getUser') ->willReturn($this->user); $storage = $this->createMock(Storage::class); $storage->method('getId')->willReturn(''); $cache = $this->getMockBuilder(Cache::class)->setConstructorArgs([$storage])->getMock(); - $storage->expects($this->once()) - ->method('getCache') + $storage->method('getCache') ->willReturn($cache); $mount = $this->createMock(IMountPoint::class); - $mount->expects($this->once()) - ->method('getStorage') + $mount->method('getStorage') ->willReturn($storage); - $mount->expects($this->once()) - ->method('getInternalPath') + $mount->method('getInternalPath') ->willReturn('foo'); - $cache->expects($this->once()) - ->method('searchQuery') + $cache->method('searchQuery') ->willReturn([ new CacheEntry(['fileid' => 3, 'path' => 'foo/qwerty', 'name' => 'qwerty', 'size' => 200, 'mtime' => 55, 'mimetype' => 'text/plain']), ]); - $root->expects($this->once()) - ->method('getMountsIn') + $root->method('getMountsIn') ->with('/bar/foo') ->willReturn([]); - $root->expects($this->once()) - ->method('getMount') + $root->method('getMount') ->with('/bar/foo') ->willReturn($mount); @@ -350,31 +343,25 @@ class FolderTest extends NodeTest { $cache = $this->getMockBuilder(Cache::class)->setConstructorArgs([$storage])->getMock(); $mount = $this->createMock(IMountPoint::class); - $mount->expects($this->once()) - ->method('getStorage') + $mount->method('getStorage') ->willReturn($storage); - $mount->expects($this->once()) - ->method('getInternalPath') + $mount->method('getInternalPath') ->willReturn('files'); - $storage->expects($this->once()) - ->method('getCache') + $storage->method('getCache') ->willReturn($cache); - $cache->expects($this->once()) - ->method('searchQuery') + $cache->method('searchQuery') ->willReturn([ new CacheEntry(['fileid' => 3, 'path' => 'files/foo', 'name' => 'qwerty', 'size' => 200, 'mtime' => 55, 'mimetype' => 'text/plain']), new CacheEntry(['fileid' => 3, 'path' => 'files_trashbin/foo2.d12345', 'name' => 'foo2.d12345', 'size' => 200, 'mtime' => 55, 'mimetype' => 'text/plain']), ]); - $root->expects($this->once()) - ->method('getMountsIn') + $root->method('getMountsIn') ->with('') ->willReturn([]); - $root->expects($this->once()) - ->method('getMount') + $root->method('getMount') ->with('') ->willReturn($mount); @@ -392,38 +379,31 @@ class FolderTest extends NodeTest { $root = $this->getMockBuilder(Root::class) ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager]) ->getMock(); - $root->expects($this->any()) - ->method('getUser') + $root->method('getUser') ->willReturn($this->user); $storage = $this->createMock(Storage::class); $storage->method('getId')->willReturn(''); $cache = $this->getMockBuilder(Cache::class)->setConstructorArgs([$storage])->getMock(); $mount = $this->createMock(IMountPoint::class); - $mount->expects($this->once()) - ->method('getStorage') + $mount->method('getStorage') ->willReturn($storage); - $mount->expects($this->once()) - ->method('getInternalPath') + $mount->method('getInternalPath') ->willReturn(''); - $storage->expects($this->once()) - ->method('getCache') + $storage->method('getCache') ->willReturn($cache); - $cache->expects($this->once()) - ->method('searchQuery') + $cache->method('searchQuery') ->willReturn([ new CacheEntry(['fileid' => 3, 'path' => 'foo/qwerty', 'name' => 'qwerty', 'size' => 200, 'mtime' => 55, 'mimetype' => 'text/plain']), ]); - $root->expects($this->once()) - ->method('getMountsIn') + $root->method('getMountsIn') ->with('/bar') ->willReturn([]); - $root->expects($this->once()) - ->method('getMount') + $root->method('getMount') ->with('/bar') ->willReturn($mount); @@ -453,48 +433,38 @@ class FolderTest extends NodeTest { $subMount = $this->getMockBuilder(MountPoint::class)->setConstructorArgs([null, ''])->getMock(); $mount = $this->createMock(IMountPoint::class); - $mount->expects($this->once()) - ->method('getStorage') + $mount->method('getStorage') ->willReturn($storage); - $mount->expects($this->once()) - ->method('getInternalPath') + $mount->method('getInternalPath') ->willReturn('foo'); - $subMount->expects($this->once()) - ->method('getStorage') + $subMount->method('getStorage') ->willReturn($subStorage); - $subMount->expects($this->once()) - ->method('getMountPoint') + $subMount->method('getMountPoint') ->willReturn('/bar/foo/bar/'); - $storage->expects($this->once()) - ->method('getCache') + $storage->method('getCache') ->willReturn($cache); - $subStorage->expects($this->once()) - ->method('getCache') + $subStorage->method('getCache') ->willReturn($subCache); - $cache->expects($this->once()) - ->method('searchQuery') + $cache->method('searchQuery') ->willReturn([ new CacheEntry(['fileid' => 3, 'path' => 'foo/qwerty', 'name' => 'qwerty', 'size' => 200, 'mtime' => 55, 'mimetype' => 'text/plain']), ]); - $subCache->expects($this->once()) - ->method('searchQuery') + $subCache->method('searchQuery') ->willReturn([ new CacheEntry(['fileid' => 4, 'path' => 'asd/qweasd', 'name' => 'qweasd', 'size' => 200, 'mtime' => 55, 'mimetype' => 'text/plain']), ]); - $root->expects($this->once()) - ->method('getMountsIn') + $root->method('getMountsIn') ->with('/bar/foo') ->willReturn([$subMount]); - $root->expects($this->once()) - ->method('getMount') + $root->method('getMount') ->with('/bar/foo') ->willReturn($mount); From 671b99251e6b1ef9e6ec9630c2b8046f3b78939e Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Fri, 19 Mar 2021 13:41:00 +0100 Subject: [PATCH 07/11] folder filtering in sql Signed-off-by: Robin Appelman --- lib/private/Files/Node/Folder.php | 38 ++++++++++++++++------------- tests/lib/Files/Node/FolderTest.php | 1 - 2 files changed, 21 insertions(+), 18 deletions(-) diff --git a/lib/private/Files/Node/Folder.php b/lib/private/Files/Node/Folder.php index 2b785ea47a..c21950343c 100644 --- a/lib/private/Files/Node/Folder.php +++ b/lib/private/Files/Node/Folder.php @@ -33,6 +33,7 @@ namespace OC\Files\Node; use OC\DB\QueryBuilder\Literal; use OC\Files\Mount\MountPoint; +use OC\Files\Search\SearchBinaryOperator; use OC\Files\Search\SearchComparison; use OC\Files\Search\SearchQuery; use OC\Files\Storage\Wrapper\Jail; @@ -45,6 +46,7 @@ use OCP\Files\FileInfo; use OCP\Files\Mount\IMountPoint; use OCP\Files\NotFoundException; use OCP\Files\NotPermittedException; +use OCP\Files\Search\ISearchBinaryOperator; use OCP\Files\Search\ISearchComparison; use OCP\Files\Search\ISearchOperator; use OCP\Files\Search\ISearchQuery; @@ -253,17 +255,6 @@ class Folder extends Node implements \OCP\Files\Folder { 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); $storage = $mount->getStorage(); @@ -272,20 +263,33 @@ class Folder extends Node implements \OCP\Files\Folder { if ($internalPath !== '') { $internalPath = $internalPath . '/'; } - $internalRootLength = strlen($internalPath); + + $subQueryLimit = $query->getLimit() > 0 ? $query->getLimit() + $query->getOffset() : PHP_INT_MAX; + $subQueryOffset = $query->getOffset(); + $rootQuery = new SearchQuery( + new SearchBinaryOperator(ISearchBinaryOperator::OPERATOR_AND, [ + new SearchComparison(ISearchComparison::COMPARE_LIKE, 'path', $internalPath . '%'), + $query->getSearchOperation(), + ] + ), + $subQueryLimit, + 0, + $query->getOrder(), + $query->getUser() + ); + + $files = []; $cache = $storage->getCache(''); - $results = $cache->searchQuery($noLimitQuery); + $results = $cache->searchQuery($rootQuery); $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) { - $files[] = $this->cacheEntryToFileInfo($mount, '', $internalPath, $result); - } + $files[] = $this->cacheEntryToFileInfo($mount, '', $internalPath, $result); } if (!$limitToHome) { @@ -330,7 +334,7 @@ class Folder extends Node implements \OCP\Files\Folder { private function cacheEntryToFileInfo(IMountPoint $mount, string $appendRoot, string $trimRoot, ICacheEntry $cacheEntry): FileInfo { $trimLength = strlen($trimRoot); $cacheEntry['internalPath'] = $cacheEntry['path']; - $cacheEntry['path'] = $appendRoot . substr($cacheEntry['path'], $trimLength); + $cacheEntry['path'] = $appendRoot . substr($cacheEntry['path'], $trimLength); return new \OC\Files\FileInfo($this->path . '/' . $cacheEntry['path'], $mount->getStorage(), $cacheEntry['internalPath'], $cacheEntry, $mount); } diff --git a/tests/lib/Files/Node/FolderTest.php b/tests/lib/Files/Node/FolderTest.php index 339a77edcc..39dcffe5ae 100644 --- a/tests/lib/Files/Node/FolderTest.php +++ b/tests/lib/Files/Node/FolderTest.php @@ -354,7 +354,6 @@ class FolderTest extends NodeTest { $cache->method('searchQuery') ->willReturn([ new CacheEntry(['fileid' => 3, 'path' => 'files/foo', 'name' => 'qwerty', 'size' => 200, 'mtime' => 55, 'mimetype' => 'text/plain']), - new CacheEntry(['fileid' => 3, 'path' => 'files_trashbin/foo2.d12345', 'name' => 'foo2.d12345', 'size' => 200, 'mtime' => 55, 'mimetype' => 'text/plain']), ]); $root->method('getMountsIn') From 055af29371e7ca7d5edb8071ba702d51330855fe Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Fri, 19 Mar 2021 14:26:44 +0100 Subject: [PATCH 08/11] handle ordering in folder search Signed-off-by: Robin Appelman --- lib/private/Files/Node/Folder.php | 62 +++++++++++------------- lib/private/Files/Search/SearchOrder.php | 25 ++++++++++ lib/public/Files/Search/ISearchOrder.php | 12 +++++ tests/lib/Files/Node/FolderTest.php | 54 +++++++++++++-------- 4 files changed, 98 insertions(+), 55 deletions(-) diff --git a/lib/private/Files/Node/Folder.php b/lib/private/Files/Node/Folder.php index c21950343c..944ba0cb5b 100644 --- a/lib/private/Files/Node/Folder.php +++ b/lib/private/Files/Node/Folder.php @@ -32,7 +32,6 @@ namespace OC\Files\Node; use OC\DB\QueryBuilder\Literal; -use OC\Files\Mount\MountPoint; use OC\Files\Search\SearchBinaryOperator; use OC\Files\Search\SearchComparison; use OC\Files\Search\SearchQuery; @@ -232,23 +231,18 @@ class Folder extends Node implements \OCP\Files\Folder { $query = $this->queryFromOperator(new SearchComparison(ISearchComparison::COMPARE_LIKE, 'name', '%' . $query . '%')); } - // Limit+offset for queries without ordering + // Limit+offset for queries with ordering // - // 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 we currently can't do ordering between the results from different storages in sql + // The only way to do ordering is requesting the $limit number of entries from all storages + // sorting them and returning the first $limit entries. // - // 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 + // For offset we have the same problem, we don't know how many entries from each storage should be skipped + // by a given $offset, so instead we query $offset + $limit from each storage and return entries $offset..($offset+$limit) + // after merging and sorting them. // - // All of this is only possible for queries without ordering + // This is suboptimal but because limit and offset tend to be fairly small in real world use cases it should + // still be significantly better than disabling paging altogether $limitToHome = $query->limitToHome(); if ($limitToHome && count(explode('/', $this->path)) !== 3) { @@ -264,13 +258,12 @@ class Folder extends Node implements \OCP\Files\Folder { $internalPath = $internalPath . '/'; } - $subQueryLimit = $query->getLimit() > 0 ? $query->getLimit() + $query->getOffset() : PHP_INT_MAX; - $subQueryOffset = $query->getOffset(); + $subQueryLimit = $query->getLimit() > 0 ? $query->getLimit() + $query->getOffset() : 0; $rootQuery = new SearchQuery( new SearchBinaryOperator(ISearchBinaryOperator::OPERATOR_AND, [ - new SearchComparison(ISearchComparison::COMPARE_LIKE, 'path', $internalPath . '%'), - $query->getSearchOperation(), - ] + new SearchComparison(ISearchComparison::COMPARE_LIKE, 'path', $internalPath . '%'), + $query->getSearchOperation(), + ] ), $subQueryLimit, 0, @@ -283,11 +276,6 @@ class Folder extends Node implements \OCP\Files\Folder { $cache = $storage->getCache(''); $results = $cache->searchQuery($rootQuery); - $count = count($results); - $results = array_slice($results, $subQueryOffset, $subQueryLimit); - $subQueryOffset = max(0, $subQueryOffset - $count); - $subQueryLimit = max(0, $subQueryLimit - $count); - foreach ($results as $result) { $files[] = $this->cacheEntryToFileInfo($mount, '', $internalPath, $result); } @@ -295,10 +283,6 @@ 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, @@ -313,12 +297,6 @@ class Folder extends Node implements \OCP\Files\Folder { $relativeMountPoint = ltrim(substr($mount->getMountPoint(), $rootLength), '/'); $results = $cache->searchQuery($subQuery); - - $count = count($results); - $results = array_slice($results, $subQueryOffset, $subQueryLimit); - $subQueryOffset -= $count; - $subQueryLimit -= $count; - foreach ($results as $result) { $files[] = $this->cacheEntryToFileInfo($mount, $relativeMountPoint, '', $result); } @@ -326,6 +304,20 @@ class Folder extends Node implements \OCP\Files\Folder { } } + $order = $query->getOrder(); + if ($order) { + usort($files, function (FileInfo $a,FileInfo $b) use ($order) { + foreach ($order as $orderField) { + $cmp = $orderField->sortFileInfo($a, $b); + if ($cmp !== 0) { + return $cmp; + } + } + return 0; + }); + } + $files = array_values(array_slice($files, $query->getOffset(), $query->getLimit() > 0 ? $query->getLimit() : null)); + return array_map(function (FileInfo $file) { return $this->createNode($file->getPath(), $file); }, $files); diff --git a/lib/private/Files/Search/SearchOrder.php b/lib/private/Files/Search/SearchOrder.php index 4bff8ba1b6..deb73baa4c 100644 --- a/lib/private/Files/Search/SearchOrder.php +++ b/lib/private/Files/Search/SearchOrder.php @@ -23,6 +23,7 @@ namespace OC\Files\Search; +use OCP\Files\FileInfo; use OCP\Files\Search\ISearchOrder; class SearchOrder implements ISearchOrder { @@ -55,4 +56,28 @@ class SearchOrder implements ISearchOrder { public function getField() { return $this->field; } + + public function sortFileInfo(FileInfo $a, FileInfo $b): int { + $cmp = $this->sortFileInfoNoDirection($a, $b); + return $cmp * ($this->direction === ISearchOrder::DIRECTION_ASCENDING ? 1 : -1); + } + + private function sortFileInfoNoDirection(FileInfo $a, FileInfo $b): int { + switch ($this->field) { + case 'name': + return $a->getName() <=> $b->getName(); + case 'mimetype': + return $a->getMimetype() <=> $b->getMimetype(); + case 'mtime': + return $a->getMtime() <=> $b->getMtime(); + case 'size': + return $a->getSize() <=> $b->getSize(); + case 'fileid': + return $a->getId() <=> $b->getId(); + case 'permissions': + return $a->getPermissions() <=> $b->getPermissions(); + default: + return 0; + } + } } diff --git a/lib/public/Files/Search/ISearchOrder.php b/lib/public/Files/Search/ISearchOrder.php index fb0137c1ba..8237b1861e 100644 --- a/lib/public/Files/Search/ISearchOrder.php +++ b/lib/public/Files/Search/ISearchOrder.php @@ -24,6 +24,8 @@ namespace OCP\Files\Search; +use OCP\Files\FileInfo; + /** * @since 12.0.0 */ @@ -46,4 +48,14 @@ interface ISearchOrder { * @since 12.0.0 */ public function getField(); + + /** + * Apply the sorting on 2 FileInfo objects + * + * @param FileInfo $a + * @param FileInfo $b + * @return int -1 if $a < $b, 0 if $a = $b, 1 if $a > $b (for ascending, reverse for descending) + * @since 22.0.0 + */ + public function sortFileInfo(FileInfo $a, FileInfo $b): int; } diff --git a/tests/lib/Files/Node/FolderTest.php b/tests/lib/Files/Node/FolderTest.php index 39dcffe5ae..1d541556c0 100644 --- a/tests/lib/Files/Node/FolderTest.php +++ b/tests/lib/Files/Node/FolderTest.php @@ -18,6 +18,7 @@ use OC\Files\Node\Folder; use OC\Files\Node\Node; use OC\Files\Node\Root; use OC\Files\Search\SearchComparison; +use OC\Files\Search\SearchOrder; use OC\Files\Search\SearchQuery; use OC\Files\Storage\Temporary; use OC\Files\Storage\Wrapper\Jail; @@ -25,6 +26,7 @@ use OC\Files\View; use OCP\Files\Mount\IMountPoint; use OCP\Files\NotFoundException; use OCP\Files\Search\ISearchComparison; +use OCP\Files\Search\ISearchOrder; use OCP\Files\Search\ISearchQuery; use OCP\Files\Storage; @@ -929,22 +931,34 @@ class FolderTest extends NodeTest { 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, []] + [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, [], []], + [0, 5, [16, 10, 14, 11, 12], [new SearchOrder(ISearchOrder::DIRECTION_ASCENDING, 'mtime')]], + [3, 2, [11, 12], [new SearchOrder(ISearchOrder::DIRECTION_ASCENDING, 'mtime')]], + [0, 5, [14, 15, 16, 10, 11], [ + new SearchOrder(ISearchOrder::DIRECTION_DESCENDING, 'size'), + new SearchOrder(ISearchOrder::DIRECTION_ASCENDING, 'mtime') + ]], ]; } /** * @dataProvider offsetLimitProvider + * @param int $offset + * @param int $limit + * @param int[] $expectedIds + * @param ISearchOrder[] $ordering + * @throws NotFoundException + * @throws \OCP\Files\InvalidPathException */ - public function testSearchSubStoragesLimitOffset(int $offset, int $limit, array $expectedIds) { + public function testSearchSubStoragesLimitOffset(int $offset, int $limit, array $expectedIds, array $ordering) { $manager = $this->createMock(Manager::class); /** * @var \OC\Files\View | \PHPUnit\Framework\MockObject\MockObject $view @@ -996,26 +1010,26 @@ class FolderTest extends NodeTest { $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']) + new CacheEntry(['fileid' => 10, 'path' => 'foo/qwerty', 'name' => 'qwerty', 'size' => 200, 'mtime' => 10, 'mimetype' => 'text/plain']), + new CacheEntry(['fileid' => 11, 'path' => 'foo/qwerty', 'name' => 'qwerty', 'size' => 200, 'mtime' => 20, 'mimetype' => 'text/plain']), + new CacheEntry(['fileid' => 12, 'path' => 'foo/qwerty', 'name' => 'qwerty', 'size' => 200, 'mtime' => 30, 'mimetype' => 'text/plain']), + new CacheEntry(['fileid' => 13, 'path' => 'foo/qwerty', 'name' => 'qwerty', 'size' => 200, 'mtime' => 40, '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']), + new CacheEntry(['fileid' => 14, 'path' => 'foo/qwerty', 'name' => 'qwerty', 'size' => 300, 'mtime' => 15, 'mimetype' => 'text/plain']), + new CacheEntry(['fileid' => 15, 'path' => 'foo/qwerty', 'name' => 'qwerty', 'size' => 300, 'mtime' => 50, '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']), + new CacheEntry(['fileid' => 16, 'path' => 'foo/qwerty', 'name' => 'qwerty', 'size' => 200, 'mtime' => 5, 'mimetype' => 'text/plain']), + new CacheEntry(['fileid' => 17, 'path' => 'foo/qwerty', 'name' => 'qwerty', 'size' => 200, 'mtime' => 60, 'mimetype' => 'text/plain']), ], $query->getOffset(), $query->getOffset() + $query->getLimit()); }); @@ -1030,9 +1044,9 @@ class FolderTest extends NodeTest { $node = new Folder($root, $view, '/bar/foo'); $comparison = new SearchComparison(ISearchComparison::COMPARE_LIKE, 'name', '%foo%'); - $query = new SearchQuery($comparison, $limit, $offset, []); + $query = new SearchQuery($comparison, $limit, $offset, $ordering); $result = $node->search($query); - $ids = array_map(function(Node $info) { + $ids = array_map(function (Node $info) { return $info->getId(); }, $result); $this->assertEquals($expectedIds, $ids); From 10bf93aa24ec7c48a2ba4218ef5bbba36f8e80dd Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Fri, 19 Mar 2021 14:29:08 +0100 Subject: [PATCH 09/11] pass order from unified search to file search Signed-off-by: Robin Appelman --- apps/files/lib/Search/FilesSearchProvider.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/apps/files/lib/Search/FilesSearchProvider.php b/apps/files/lib/Search/FilesSearchProvider.php index 189bebaf7b..0d35eb053a 100644 --- a/apps/files/lib/Search/FilesSearchProvider.php +++ b/apps/files/lib/Search/FilesSearchProvider.php @@ -29,12 +29,14 @@ declare(strict_types=1); namespace OCA\Files\Search; use OC\Files\Search\SearchComparison; +use OC\Files\Search\SearchOrder; use OC\Files\Search\SearchQuery; use OCP\Files\FileInfo; use OCP\Files\IMimeTypeDetector; use OCP\Files\IRootFolder; use OCP\Files\Search\ISearchComparison; use OCP\Files\Node; +use OCP\Files\Search\ISearchOrder; use OCP\IL10N; use OCP\IURLGenerator; use OCP\IUser; @@ -103,7 +105,9 @@ class FilesSearchProvider implements IProvider { new SearchComparison(ISearchComparison::COMPARE_LIKE, 'name', '%' . $query->getTerm() . '%'), $query->getLimit(), (int)$query->getCursor(), - [], + $query->getSortOrder() === ISearchQuery::SORT_DATE_DESC ? [ + new SearchOrder(ISearchOrder::DIRECTION_DESCENDING, 'mtime'), + ] : [], $user ); From 4ede9dbb079b16e448ade96bf96ff43239e56cd8 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Fri, 19 Mar 2021 14:47:30 +0100 Subject: [PATCH 10/11] use node search api for legacy file search endpoint Signed-off-by: Robin Appelman --- core/Controller/SearchController.php | 1 + lib/private/Search/Provider/File.php | 48 +++++++++++++++++----------- lib/private/Search/Result/File.php | 7 ++-- 3 files changed, 33 insertions(+), 23 deletions(-) diff --git a/core/Controller/SearchController.php b/core/Controller/SearchController.php index 72633630da..42439e9ceb 100644 --- a/core/Controller/SearchController.php +++ b/core/Controller/SearchController.php @@ -55,6 +55,7 @@ class SearchController extends Controller { /** * @NoAdminRequired + * @NoCSRFRequired */ public function search(string $query, array $inApps = [], int $page = 1, int $size = 30): JSONResponse { $results = $this->searcher->searchPaged($query, $inApps, $page, $size); diff --git a/lib/private/Search/Provider/File.php b/lib/private/Search/Provider/File.php index 4125b1f7d7..d42d57b800 100644 --- a/lib/private/Search/Provider/File.php +++ b/lib/private/Search/Provider/File.php @@ -29,7 +29,14 @@ namespace OC\Search\Provider; -use OC\Files\Filesystem; +use OC\Files\Search\SearchComparison; +use OC\Files\Search\SearchOrder; +use OC\Files\Search\SearchQuery; +use OCP\Files\FileInfo; +use OCP\Files\IRootFolder; +use OCP\Files\Search\ISearchComparison; +use OCP\Files\Search\ISearchOrder; +use OCP\IUserSession; use OCP\Search\PagedProvider; /** @@ -48,35 +55,38 @@ class File extends PagedProvider { * @deprecated 20.0.0 */ public function search($query, int $limit = null, int $offset = null) { - if ($offset === null) { - $offset = 0; + /** @var IRootFolder $rootFolder */ + $rootFolder = \OC::$server->query(IRootFolder::class); + /** @var IUserSession $userSession */ + $userSession = \OC::$server->query(IUserSession::class); + $user = $userSession->getUser(); + if (!$user) { + return []; } - \OC_Util::setupFS(); - $files = Filesystem::search($query); + $userFolder = $rootFolder->getUserFolder($user->getUID()); + $fileQuery = new SearchQuery( + new SearchComparison(ISearchComparison::COMPARE_LIKE, 'name', '%' . $query . '%'), + (int)$limit, + (int)$offset, + [ + new SearchOrder(ISearchOrder::DIRECTION_DESCENDING, 'mtime'), + ], + $user + ); + $files = $userFolder->search($fileQuery); $results = []; - if ($limit !== null) { - $files = array_slice($files, $offset, $offset + $limit); - } // edit results foreach ($files as $fileData) { - // skip versions - if (strpos($fileData['path'], '_versions') === 0) { - continue; - } - // skip top-level folder - if ($fileData['name'] === 'files' && $fileData['parent'] === -1) { - continue; - } // create audio result - if ($fileData['mimepart'] === 'audio') { + if ($fileData->getMimePart() === 'audio') { $result = new \OC\Search\Result\Audio($fileData); } // create image result - elseif ($fileData['mimepart'] === 'image') { + elseif ($fileData->getMimePart() === 'image') { $result = new \OC\Search\Result\Image($fileData); } // create folder result - elseif ($fileData['mimetype'] === 'httpd/unix-directory') { + elseif ($fileData->getMimetype() === FileInfo::MIMETYPE_FOLDER) { $result = new \OC\Search\Result\Folder($fileData); } // or create file result diff --git a/lib/private/Search/Result/File.php b/lib/private/Search/Result/File.php index 33e1e97f47..c3b0c4e375 100644 --- a/lib/private/Search/Result/File.php +++ b/lib/private/Search/Result/File.php @@ -97,14 +97,13 @@ class File extends \OCP\Search\Result { public function __construct(FileInfo $data) { $path = $this->getRelativePath($data->getPath()); - $info = pathinfo($path); $this->id = $data->getId(); - $this->name = $info['basename']; + $this->name = $data->getName(); $this->link = \OC::$server->getURLGenerator()->linkToRoute( 'files.view.index', [ - 'dir' => $info['dirname'], - 'scrollto' => $info['basename'], + 'dir' => dirname($path), + 'scrollto' => $data->getName(), ] ); $this->permissions = $data->getPermissions(); From 4b65a1dcd7192c0a32b2fb8b3184c06f788574c7 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Wed, 24 Mar 2021 16:32:38 +0100 Subject: [PATCH 11/11] fix type hints Signed-off-by: Robin Appelman --- lib/private/Files/Cache/Cache.php | 4 ++++ lib/private/Files/Node/Folder.php | 6 +++--- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/lib/private/Files/Cache/Cache.php b/lib/private/Files/Cache/Cache.php index b84d9b218b..be2862cc0f 100644 --- a/lib/private/Files/Cache/Cache.php +++ b/lib/private/Files/Cache/Cache.php @@ -811,6 +811,10 @@ class Cache implements ICache { }, $files); } + /** + * @param ISearchQuery $searchQuery + * @return CacheEntry[] + */ public function searchQuery(ISearchQuery $searchQuery) { $builder = $this->getQueryBuilder(); diff --git a/lib/private/Files/Node/Folder.php b/lib/private/Files/Node/Folder.php index 944ba0cb5b..65854f5054 100644 --- a/lib/private/Files/Node/Folder.php +++ b/lib/private/Files/Node/Folder.php @@ -32,6 +32,7 @@ namespace OC\Files\Node; use OC\DB\QueryBuilder\Literal; +use OC\Files\Cache\CacheEntry; use OC\Files\Search\SearchBinaryOperator; use OC\Files\Search\SearchComparison; use OC\Files\Search\SearchQuery; @@ -39,7 +40,6 @@ use OC\Files\Storage\Wrapper\Jail; use OC\Files\Storage\Storage; use OCA\Files_Sharing\SharedStorage; use OCP\DB\QueryBuilder\IQueryBuilder; -use OCP\Files\Cache\ICacheEntry; use OCP\Files\Config\ICachedMountInfo; use OCP\Files\FileInfo; use OCP\Files\Mount\IMountPoint; @@ -323,7 +323,7 @@ class Folder extends Node implements \OCP\Files\Folder { }, $files); } - private function cacheEntryToFileInfo(IMountPoint $mount, string $appendRoot, string $trimRoot, ICacheEntry $cacheEntry): FileInfo { + private function cacheEntryToFileInfo(IMountPoint $mount, string $appendRoot, string $trimRoot, CacheEntry $cacheEntry): FileInfo { $trimLength = strlen($trimRoot); $cacheEntry['internalPath'] = $cacheEntry['path']; $cacheEntry['path'] = $appendRoot . substr($cacheEntry['path'], $trimLength); @@ -364,7 +364,7 @@ class Folder extends Node implements \OCP\Files\Folder { public function getById($id) { $mountCache = $this->root->getUserMountCache(); if (strpos($this->getPath(), '/', 1) > 0) { - list(, $user) = explode('/', $this->getPath()); + [, $user] = explode('/', $this->getPath()); } else { $user = null; }