cleanup fileinfo creation

Signed-off-by: Robin Appelman <robin@icewind.nl>

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'];
+				$files[] = $this->cacheEntryToFileInfo($mount, '', $internalPath, $result);
-				$result['path'] = substr($result['path'], $internalRootLength);
-				$result['storage'] = $storage;
-				$files[] = new \OC\Files\FileInfo($this->path . '/' . $result['path'], $storage, $result['internalPath'], $result, $mount);
 			}
 		}
 
@@ -312,11 +316,7 @@ class Folder extends Node implements \OCP\Files\Folder {
 					$subQueryLimit -= $count;
 
 					foreach ($results as $result) {
-						$result['internalPath'] = $result['path'];
+						$files[] = $this->cacheEntryToFileInfo($mount, $relativeMountPoint, '', $result);
-						$result['path'] = $relativeMountPoint . $result['path'];
-						$result['storage'] = $storage;
-						$files[] = new \OC\Files\FileInfo($this->path . '/' . $result['path'], $storage,
-							$result['internalPath'], $result, $mount);
 					}
 				}
 			}
@@ -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
 	 *

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())
+		$root->method('getUser')
-			->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())
+		$storage->method('getCache')
-			->method('getCache')
 			->willReturn($cache);
 
 		$mount = $this->createMock(IMountPoint::class);
-		$mount->expects($this->once())
+		$mount->method('getStorage')
-			->method('getStorage')
 			->willReturn($storage);
-		$mount->expects($this->once())
+		$mount->method('getInternalPath')
-			->method('getInternalPath')
 			->willReturn('foo');
 
-		$cache->expects($this->once())
+		$cache->method('searchQuery')
-			->method('searchQuery')
 			->willReturn([
 				new CacheEntry(['fileid' => 3, 'path' => 'foo/qwerty', 'name' => 'qwerty', 'size' => 200, 'mtime' => 55, 'mimetype' => 'text/plain']),
 			]);
 
-		$root->expects($this->once())
+		$root->method('getMountsIn')
-			->method('getMountsIn')
 			->with('/bar/foo')
 			->willReturn([]);
 
-		$root->expects($this->once())
+		$root->method('getMount')
-			->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())
+		$mount->method('getStorage')
-			->method('getStorage')
 			->willReturn($storage);
-		$mount->expects($this->once())
+		$mount->method('getInternalPath')
-			->method('getInternalPath')
 			->willReturn('files');
 
-		$storage->expects($this->once())
+		$storage->method('getCache')
-			->method('getCache')
 			->willReturn($cache);
 
-		$cache->expects($this->once())
+		$cache->method('searchQuery')
-			->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())
+		$root->method('getMountsIn')
-			->method('getMountsIn')
 			->with('')
 			->willReturn([]);
 
-		$root->expects($this->once())
+		$root->method('getMount')
-			->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())
+		$root->method('getUser')
-			->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())
+		$mount->method('getStorage')
-			->method('getStorage')
 			->willReturn($storage);
-		$mount->expects($this->once())
+		$mount->method('getInternalPath')
-			->method('getInternalPath')
 			->willReturn('');
 
-		$storage->expects($this->once())
+		$storage->method('getCache')
-			->method('getCache')
 			->willReturn($cache);
 
-		$cache->expects($this->once())
+		$cache->method('searchQuery')
-			->method('searchQuery')
 			->willReturn([
 				new CacheEntry(['fileid' => 3, 'path' => 'foo/qwerty', 'name' => 'qwerty', 'size' => 200, 'mtime' => 55, 'mimetype' => 'text/plain']),
 			]);
 
-		$root->expects($this->once())
+		$root->method('getMountsIn')
-			->method('getMountsIn')
 			->with('/bar')
 			->willReturn([]);
 
-		$root->expects($this->once())
+		$root->method('getMount')
-			->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())
+		$mount->method('getStorage')
-			->method('getStorage')
 			->willReturn($storage);
-		$mount->expects($this->once())
+		$mount->method('getInternalPath')
-			->method('getInternalPath')
 			->willReturn('foo');
 
-		$subMount->expects($this->once())
+		$subMount->method('getStorage')
-			->method('getStorage')
 			->willReturn($subStorage);
 
-		$subMount->expects($this->once())
+		$subMount->method('getMountPoint')
-			->method('getMountPoint')
 			->willReturn('/bar/foo/bar/');
 
-		$storage->expects($this->once())
+		$storage->method('getCache')
-			->method('getCache')
 			->willReturn($cache);
 
-		$subStorage->expects($this->once())
+		$subStorage->method('getCache')
-			->method('getCache')
 			->willReturn($subCache);
 
-		$cache->expects($this->once())
+		$cache->method('searchQuery')
-			->method('searchQuery')
 			->willReturn([
 				new CacheEntry(['fileid' => 3, 'path' => 'foo/qwerty', 'name' => 'qwerty', 'size' => 200, 'mtime' => 55, 'mimetype' => 'text/plain']),
 			]);
 
-		$subCache->expects($this->once())
+		$subCache->method('searchQuery')
-			->method('searchQuery')
 			->willReturn([
 				new CacheEntry(['fileid' => 4, 'path' => 'asd/qweasd', 'name' => 'qweasd', 'size' => 200, 'mtime' => 55, 'mimetype' => 'text/plain']),
 			]);
 
-		$root->expects($this->once())
+		$root->method('getMountsIn')
-			->method('getMountsIn')
 			->with('/bar/foo')
 			->willReturn([$subMount]);
 
-		$root->expects($this->once())
+		$root->method('getMount')
-			->method('getMount')
 			->with('/bar/foo')
 			->willReturn($mount);