From 4e60b9ffe2617d1735dbd8b2b58838e2f6ec4dd7 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Fri, 19 Mar 2021 13:35:37 +0100 Subject: [PATCH] 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);