diff --git a/lib/private/Files/Cache/QuerySearchHelper.php b/lib/private/Files/Cache/QuerySearchHelper.php index 9beabd0716..b8d5be6af8 100644 --- a/lib/private/Files/Cache/QuerySearchHelper.php +++ b/lib/private/Files/Cache/QuerySearchHelper.php @@ -243,7 +243,11 @@ class QuerySearchHelper { */ public function addSearchOrdersToQuery(IQueryBuilder $query, array $orders) { foreach ($orders as $order) { - $query->addOrderBy($order->getField(), $order->getDirection()); + $field = $order->getField(); + if ($field === 'fileid') { + $field = 'file.fileid'; + } + $query->addOrderBy($field, $order->getDirection()); } } diff --git a/lib/private/Files/Node/Folder.php b/lib/private/Files/Node/Folder.php index 31c040907b..bf8cca8351 100644 --- a/lib/private/Files/Node/Folder.php +++ b/lib/private/Files/Node/Folder.php @@ -31,11 +31,9 @@ namespace OC\Files\Node; -use OC\DB\QueryBuilder\Literal; use OC\Files\Cache\QuerySearchHelper; use OC\Files\Search\SearchBinaryOperator; use OC\Files\Cache\Wrapper\CacheJail; -use OC\Files\Search\SearchBinaryOperator; use OC\Files\Search\SearchComparison; use OC\Files\Search\SearchOrder; use OC\Files\Search\SearchQuery; @@ -251,7 +249,12 @@ class Folder extends Node implements \OCP\Files\Folder { // collect all caches for this folder, indexed by their mountpoint relative to this folder // and save the mount which is needed later to construct the FileInfo objects - $caches = ['' => new CacheJail($storage->getCache(''), $internalPath)]; // a temporary CacheJail is used to handle filtering down the results to within this folder + if ($internalPath !== '') { + // a temporary CacheJail is used to handle filtering down the results to within this folder + $caches = ['' => new CacheJail($storage->getCache(''), $internalPath)]; + } else { + $caches = ['' => $storage->getCache('')]; + } $mountByMountPoint = ['' => $mount]; if (!$limitToHome) { @@ -271,13 +274,27 @@ class Folder extends Node implements \OCP\Files\Folder { $resultsPerCache = $searchHelper->searchInCaches($query, $caches); // loop trough all results per-cache, constructing the FileInfo object from the CacheEntry and merge them all - $files = array_merge(...array_map(function(array $results, $relativeMountPoint) use ($mountByMountPoint) { + $files = array_merge(...array_map(function (array $results, $relativeMountPoint) use ($mountByMountPoint) { $mount = $mountByMountPoint[$relativeMountPoint]; - return array_map(function(ICacheEntry $result) use ($relativeMountPoint, $mount) { + return array_map(function (ICacheEntry $result) use ($relativeMountPoint, $mount) { return $this->cacheEntryToFileInfo($mount, $relativeMountPoint, $result); }, $results); }, array_values($resultsPerCache), array_keys($resultsPerCache))); + // since results were returned per-cache, they are no longer fully sorted + $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; + }); + } + return array_map(function (FileInfo $file) { return $this->createNode($file->getPath(), $file); }, $files); diff --git a/tests/lib/Files/Node/FolderTest.php b/tests/lib/Files/Node/FolderTest.php index 36398171b1..72bf70a68b 100644 --- a/tests/lib/Files/Node/FolderTest.php +++ b/tests/lib/Files/Node/FolderTest.php @@ -23,11 +23,11 @@ use OC\Files\Search\SearchQuery; use OC\Files\Storage\Temporary; use OC\Files\Storage\Wrapper\Jail; use OC\Files\View; +use OCP\Files\Cache\ICacheEntry; 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; /** @@ -294,9 +294,10 @@ class FolderTest extends NodeTest { ->getMock(); $root->method('getUser') ->willReturn($this->user); - $storage = $this->createMock(Storage::class); - $storage->method('getId')->willReturn(''); - $cache = $this->getMockBuilder(Cache::class)->setConstructorArgs([$storage])->getMock(); + /** @var Storage\IStorage $storage */ + $storage = $this->createMock(Storage\IStorage::class); + $storage->method('getId')->willReturn('test::1'); + $cache = new Cache($storage); $storage->method('getCache') ->willReturn($cache); @@ -307,10 +308,8 @@ class FolderTest extends NodeTest { $mount->method('getInternalPath') ->willReturn('foo'); - $cache->method('searchQuery') - ->willReturn([ - new CacheEntry(['fileid' => 3, 'path' => 'foo/qwerty', 'name' => 'qwerty', 'size' => 200, 'mtime' => 55, 'mimetype' => 'text/plain']), - ]); + $cache->insert('foo', ['size' => 200, 'mtime' => 55, 'mimetype' => ICacheEntry::DIRECTORY_MIMETYPE]); + $cache->insert('foo/qwerty', ['size' => 200, 'mtime' => 55, 'mimetype' => 'text/plain']); $root->method('getMountsIn') ->with('/bar/foo') @@ -322,6 +321,7 @@ class FolderTest extends NodeTest { $node = new Folder($root, $view, '/bar/foo'); $result = $node->search('qw'); + $cache->clear(); $this->assertEquals(1, count($result)); $this->assertEquals('/bar/foo/qwerty', $result[0]->getPath()); } @@ -341,8 +341,8 @@ class FolderTest extends NodeTest { ->willReturn($this->user); /** @var \PHPUnit\Framework\MockObject\MockObject|Storage $storage */ $storage = $this->createMock(Storage::class); - $storage->method('getId')->willReturn(''); - $cache = $this->getMockBuilder(Cache::class)->setConstructorArgs([$storage])->getMock(); + $storage->method('getId')->willReturn('test::2'); + $cache = new Cache($storage); $mount = $this->createMock(IMountPoint::class); $mount->method('getStorage') @@ -353,10 +353,8 @@ class FolderTest extends NodeTest { $storage->method('getCache') ->willReturn($cache); - $cache->method('searchQuery') - ->willReturn([ - new CacheEntry(['fileid' => 3, 'path' => 'files/foo', 'name' => 'qwerty', 'size' => 200, 'mtime' => 55, 'mimetype' => 'text/plain']), - ]); + $cache->insert('files', ['size' => 200, 'mtime' => 55, 'mimetype' => ICacheEntry::DIRECTORY_MIMETYPE]); + $cache->insert('files/foo', ['size' => 200, 'mtime' => 55, 'mimetype' => 'text/plain']); $root->method('getMountsIn') ->with('') @@ -366,7 +364,8 @@ class FolderTest extends NodeTest { ->with('') ->willReturn($mount); - $result = $root->search('qw'); + $result = $root->search('foo'); + $cache->clear(); $this->assertEquals(1, count($result)); $this->assertEquals('/foo', $result[0]->getPath()); } @@ -383,8 +382,8 @@ class FolderTest extends NodeTest { $root->method('getUser') ->willReturn($this->user); $storage = $this->createMock(Storage::class); - $storage->method('getId')->willReturn(''); - $cache = $this->getMockBuilder(Cache::class)->setConstructorArgs([$storage])->getMock(); + $storage->method('getId')->willReturn('test::1'); + $cache = new Cache($storage); $mount = $this->createMock(IMountPoint::class); $mount->method('getStorage') @@ -395,10 +394,9 @@ class FolderTest extends NodeTest { $storage->method('getCache') ->willReturn($cache); - $cache->method('searchQuery') - ->willReturn([ - new CacheEntry(['fileid' => 3, 'path' => 'foo/qwerty', 'name' => 'qwerty', 'size' => 200, 'mtime' => 55, 'mimetype' => 'text/plain']), - ]); + $cache->insert('foo', ['size' => 200, 'mtime' => 55, 'mimetype' => ICacheEntry::DIRECTORY_MIMETYPE]); + $cache->insert('foo/qwerty', ['size' => 200, 'mtime' => 55, 'mimetype' => 'text/plain']); + $root->method('getMountsIn') ->with('/bar') @@ -410,6 +408,7 @@ class FolderTest extends NodeTest { $node = new Folder($root, $view, '/bar'); $result = $node->search('qw'); + $cache->clear(); $this->assertEquals(1, count($result)); $this->assertEquals('/bar/foo/qwerty', $result[0]->getPath()); } @@ -427,10 +426,11 @@ class FolderTest extends NodeTest { ->method('getUser') ->willReturn($this->user); $storage = $this->createMock(Storage::class); - $storage->method('getId')->willReturn(''); - $cache = $this->getMockBuilder(Cache::class)->setConstructorArgs([$storage])->getMock(); - $subCache = $this->getMockBuilder(Cache::class)->setConstructorArgs([$storage])->getMock(); + $storage->method('getId')->willReturn('test::1'); + $cache = new Cache($storage); $subStorage = $this->createMock(Storage::class); + $subStorage->method('getId')->willReturn('test::2'); + $subCache = new Cache($subStorage); $subMount = $this->getMockBuilder(MountPoint::class)->setConstructorArgs([null, ''])->getMock(); $mount = $this->createMock(IMountPoint::class); @@ -451,15 +451,12 @@ class FolderTest extends NodeTest { $subStorage->method('getCache') ->willReturn($subCache); - $cache->method('searchQuery') - ->willReturn([ - new CacheEntry(['fileid' => 3, 'path' => 'foo/qwerty', 'name' => 'qwerty', 'size' => 200, 'mtime' => 55, 'mimetype' => 'text/plain']), - ]); + $cache->insert('foo', ['size' => 200, 'mtime' => 55, 'mimetype' => ICacheEntry::DIRECTORY_MIMETYPE]); + $cache->insert('foo/qwerty', ['size' => 200, 'mtime' => 55, 'mimetype' => 'text/plain']); + + $subCache->insert('asd', ['size' => 200, 'mtime' => 55, 'mimetype' => ICacheEntry::DIRECTORY_MIMETYPE]); + $subCache->insert('asd/qwerty', ['size' => 200, 'mtime' => 55, 'mimetype' => 'text/plain']); - $subCache->method('searchQuery') - ->willReturn([ - new CacheEntry(['fileid' => 4, 'path' => 'asd/qweasd', 'name' => 'qweasd', 'size' => 200, 'mtime' => 55, 'mimetype' => 'text/plain']), - ]); $root->method('getMountsIn') ->with('/bar/foo') @@ -472,6 +469,8 @@ class FolderTest extends NodeTest { $node = new Folder($root, $view, '/bar/foo'); $result = $node->search('qw'); + $cache->clear(); + $subCache->clear(); $this->assertEquals(2, count($result)); } @@ -944,18 +943,18 @@ 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], []], + [0, 10, ['/bar/foo/foo1', '/bar/foo/foo2', '/bar/foo/foo3', '/bar/foo/foo4', '/bar/foo/sub1/foo5', '/bar/foo/sub1/foo6', '/bar/foo/sub2/foo7', '/bar/foo/sub2/foo8'], []], + [0, 5, ['/bar/foo/foo1', '/bar/foo/foo2', '/bar/foo/foo3', '/bar/foo/foo4', '/bar/foo/sub1/foo5'], []], + [0, 2, ['/bar/foo/foo1', '/bar/foo/foo2'], []], + [3, 2, ['/bar/foo/foo4', '/bar/foo/sub1/foo5'], []], + [3, 5, ['/bar/foo/foo4', '/bar/foo/sub1/foo5', '/bar/foo/sub1/foo6', '/bar/foo/sub2/foo7', '/bar/foo/sub2/foo8'], []], + [5, 2, ['/bar/foo/sub1/foo6', '/bar/foo/sub2/foo7'], []], + [6, 2, ['/bar/foo/sub2/foo7', '/bar/foo/sub2/foo8'], []], + [7, 2, ['/bar/foo/sub2/foo8'], []], [10, 2, [], []], - [0, 5, [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], [ + [0, 5, ['/bar/foo/sub2/foo7', '/bar/foo/foo1', '/bar/foo/sub1/foo5', '/bar/foo/foo2', '/bar/foo/foo3'], [new SearchOrder(ISearchOrder::DIRECTION_ASCENDING, 'mtime')]], + [3, 2, ['/bar/foo/foo2', '/bar/foo/foo3'], [new SearchOrder(ISearchOrder::DIRECTION_ASCENDING, 'mtime')]], + [0, 5, ['/bar/foo/sub1/foo5', '/bar/foo/sub1/foo6', '/bar/foo/sub2/foo7', '/bar/foo/foo1', '/bar/foo/foo2'], [ new SearchOrder(ISearchOrder::DIRECTION_DESCENDING, 'size'), new SearchOrder(ISearchOrder::DIRECTION_ASCENDING, 'mtime') ]], @@ -966,12 +965,16 @@ class FolderTest extends NodeTest { * @dataProvider offsetLimitProvider * @param int $offset * @param int $limit - * @param int[] $expectedIds + * @param string[] $expectedPaths * @param ISearchOrder[] $ordering * @throws NotFoundException * @throws \OCP\Files\InvalidPathException */ - public function testSearchSubStoragesLimitOffset(int $offset, int $limit, array $expectedIds, array $ordering) { + public function testSearchSubStoragesLimitOffset(int $offset, int $limit, array $expectedPaths, array $ordering) { + if (!$ordering) { + $ordering = [new SearchOrder(ISearchOrder::DIRECTION_ASCENDING, 'fileid')]; + } + $manager = $this->createMock(Manager::class); /** * @var \OC\Files\View | \PHPUnit\Framework\MockObject\MockObject $view @@ -984,13 +987,15 @@ class FolderTest extends NodeTest { ->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(); + $storage->method('getId')->willReturn('test::1'); + $cache = new Cache($storage); $subStorage1 = $this->createMock(Storage::class); + $subStorage1->method('getId')->willReturn('test::2'); + $subCache1 = new Cache($subStorage1); $subMount1 = $this->getMockBuilder(MountPoint::class)->setConstructorArgs([null, ''])->getMock(); - $subCache2 = $this->getMockBuilder(Cache::class)->setConstructorArgs([$storage])->getMock(); $subStorage2 = $this->createMock(Storage::class); + $subStorage2->method('getId')->willReturn('test::3'); + $subCache2 = new Cache($subStorage2); $subMount2 = $this->getMockBuilder(MountPoint::class)->setConstructorArgs([null, ''])->getMock(); $mount = $this->createMock(IMountPoint::class); @@ -1003,7 +1008,7 @@ class FolderTest extends NodeTest { ->willReturn($subStorage1); $subMount1->method('getMountPoint') - ->willReturn('/bar/foo/bar/'); + ->willReturn('/bar/foo/sub1/'); $storage->method('getCache') ->willReturn($cache); @@ -1015,36 +1020,21 @@ class FolderTest extends NodeTest { ->willReturn($subStorage2); $subMount2->method('getMountPoint') - ->willReturn('/bar/foo/bar2/'); + ->willReturn('/bar/foo/sub2/'); $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' => 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()); - }); + $cache->insert('foo/foo1', ['size' => 200, 'mtime' => 10, 'mimetype' => 'text/plain']); + $cache->insert('foo/foo2', ['size' => 200, 'mtime' => 20, 'mimetype' => 'text/plain']); + $cache->insert('foo/foo3', ['size' => 200, 'mtime' => 30, 'mimetype' => 'text/plain']); + $cache->insert('foo/foo4', ['size' => 200, 'mtime' => 40, 'mimetype' => 'text/plain']); - $subCache1->method('searchQuery') - ->willReturnCallback(function (ISearchQuery $query) { - return array_slice([ - 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()); - }); + $subCache1->insert('foo5', ['size' => 300, 'mtime' => 15, 'mimetype' => 'text/plain']); + $subCache1->insert('foo6', ['size' => 300, 'mtime' => 50, 'mimetype' => 'text/plain']); - $subCache2->method('searchQuery') - ->willReturnCallback(function (ISearchQuery $query) { - return array_slice([ - 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()); - }); + $subCache2->insert('foo7', ['size' => 200, 'mtime' => 5, 'mimetype' => 'text/plain']); + $subCache2->insert('foo8', ['size' => 200, 'mtime' => 60, 'mimetype' => 'text/plain']); $root->method('getMountsIn') ->with('/bar/foo') @@ -1054,14 +1044,16 @@ class FolderTest extends NodeTest { ->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, $ordering); $result = $node->search($query); + $cache->clear(); + $subCache1->clear(); + $subCache2->clear(); $ids = array_map(function (Node $info) { - return $info->getId(); + return $info->getPath(); }, $result); - $this->assertEquals($expectedIds, $ids); + $this->assertEquals($expectedPaths, $ids); } }