Merge pull request #54384 from nextcloud/getpath-node

perf: use more optimized node-by-id logic in View::getPath

apps/files/lib/Service/OwnershipTransferService.php

@@ -333,15 +333,10 @@ class OwnershipTransferService {
 				if ($path !== "$sourceUid/files") {
 					$sharePage = array_filter($sharePage, function (IShare $share) use ($view, $normalizedPath) {
 						try {
-							$relativePath = $view->getPath($share->getNodeId());
-							$singleFileTranfer = $view->is_file($normalizedPath);
-							if ($singleFileTranfer) {
-								return Filesystem::normalizePath($relativePath) === $normalizedPath;
-							}
+							$sourceNode = $share->getNode();
+							$relativePath = $view->getRelativePath($sourceNode->getPath());
 
-							return mb_strpos(
-								Filesystem::normalizePath($relativePath . '/', false),
-								$normalizedPath . '/') === 0;
+							return str_starts_with($relativePath . '/', $normalizedPath . '/');
 						} catch (Exception $e) {
 							return false;
 						}
@@ -357,7 +352,7 @@ class OwnershipTransferService {
 
 		return array_values(array_filter(array_map(function (IShare $share) use ($view, $normalizedPath, $output, $sourceUid) {
 			try {
-				$nodePath = $view->getPath($share->getNodeId());
+				$nodePath = $view->getRelativePath($share->getNode()->getPath());
 			} catch (NotFoundException $e) {
 				$output->writeln("<error>Failed to find path for shared file {$share->getNodeId()} for user $sourceUid, skipping</error>");
 				return null;

lib/private/Files/Node/Root.php

@@ -415,7 +415,7 @@ class Root extends Folder implements IRootFolder {
 	 */
 	public function getByIdInPath(int $id, string $path): array {
 		$mountCache = $this->getUserMountCache();
-		if (strpos($path, '/', 1) > 0) {
+		if ($path !== '' && strpos($path, '/', 1) > 0) {
 			[, $user] = explode('/', $path);
 		} else {
 			$user = null;

lib/private/Files/View.php

@@ -1828,43 +1828,25 @@ class View {
 	 * @return string
 	 * @throws NotFoundException
 	 */
-	public function getPath($id, ?int $storageId = null) {
+	public function getPath($id, ?int $storageId = null): string {
 		$id = (int)$id;
-		$manager = Filesystem::getMountManager();
-		$mounts = $manager->findIn($this->fakeRoot);
-		$mounts[] = $manager->find($this->fakeRoot);
-		$mounts = array_filter($mounts);
-		// reverse the array, so we start with the storage this view is in
-		// which is the most likely to contain the file we're looking for
-		$mounts = array_reverse($mounts);
-
-		// put non-shared mounts in front of the shared mount
-		// this prevents unneeded recursion into shares
-		usort($mounts, function (IMountPoint $a, IMountPoint $b) {
-			return $a instanceof SharedMount && (!$b instanceof SharedMount) ? 1 : -1;
-		});
+		$rootFolder = Server::get(Files\IRootFolder::class);
 
-		if (!is_null($storageId)) {
-			$mounts = array_filter($mounts, function (IMountPoint $mount) use ($storageId) {
-				return $mount->getNumericStorageId() === $storageId;
-			});
+		$node = $rootFolder->getFirstNodeByIdInPath($id, $this->getRoot());
+		if ($node) {
+			if ($storageId === null || $storageId === $node->getStorage()->getCache()->getNumericStorageId()) {
+				return $this->getRelativePath($node->getPath()) ?? '';
+			}
+		} else {
+			throw new NotFoundException(sprintf('File with id "%s" has not been found.', $id));
 		}
 
-		foreach ($mounts as $mount) {
-			/**
-			 * @var \OC\Files\Mount\MountPoint $mount
-			 */
-			if ($mount->getStorage()) {
-				$cache = $mount->getStorage()->getCache();
-				$internalPath = $cache->getPathById($id);
-				if (is_string($internalPath)) {
-					$fullPath = $mount->getMountPoint() . $internalPath;
-					if (!is_null($path = $this->getRelativePath($fullPath))) {
-						return $path;
-					}
-				}
+		foreach ($rootFolder->getByIdInPath($id, $this->getRoot()) as $node) {
+			if ($storageId === $node->getStorage()->getCache()->getNumericStorageId()) {
+				return $this->getRelativePath($node->getPath()) ?? '';
 			}
 		}
+
 		throw new NotFoundException(sprintf('File with id "%s" has not been found.', $id));
 	}
 

tests/lib/Files/ViewTest.php

@@ -23,6 +23,7 @@ use OCP\Cache\CappedMemoryCache;
 use OCP\Constants;
 use OCP\Files\Config\IMountProvider;
 use OCP\Files\Config\IMountProviderCollection;
+use OCP\Files\Config\IUserMountCache;
 use OCP\Files\FileInfo;
 use OCP\Files\ForbiddenException;
 use OCP\Files\GenericFileException;
@@ -258,28 +259,36 @@ class ViewTest extends \Test\TestCase {
 	 * @medium
 	 */
 	public function testGetPath(): void {
+		$user = $this->createMock(IUser::class);
+		$user->method('getUID')
+			->willReturn('test');
 		$storage1 = $this->getTestStorage();
 		$storage2 = $this->getTestStorage();
 		$storage3 = $this->getTestStorage();
 
-		Filesystem::mount($storage1, [], '/');
-		Filesystem::mount($storage2, [], '/substorage');
-		Filesystem::mount($storage3, [], '/folder/anotherstorage');
+		Filesystem::mount($storage1, [], '/test/files');
+		Filesystem::mount($storage2, [], '/test/files/substorage');
+		Filesystem::mount($storage3, [], '/test/files/folder/anotherstorage');
 
-		$rootView = new View('');
+		$userMountCache = Server::get(IUserMountCache::class);
+		$userMountCache->registerMounts($user, [
+			new MountPoint($storage1, '/test/files'),
+			new MountPoint($storage2, '/test/files/substorage'),
+			new MountPoint($storage3, '/test/files/folder/anotherstorage'),
+		]);
+
+		$rootView = new View('/test/files');
 
 
 		$cachedData = $rootView->getFileInfo('/foo.txt');
-		/** @var int $id1 */
-		$id1 = $cachedData['fileid'];
+		$id1 = $cachedData->getId();
 		$this->assertEquals('/foo.txt', $rootView->getPath($id1));
 
 		$cachedData = $rootView->getFileInfo('/substorage/foo.txt');
-		/** @var int $id2 */
-		$id2 = $cachedData['fileid'];
+		$id2 = $cachedData->getId();
 		$this->assertEquals('/substorage/foo.txt', $rootView->getPath($id2));
 
-		$folderView = new View('/substorage');
+		$folderView = new View('/test/files/substorage');
 		$this->assertEquals('/foo.txt', $folderView->getPath($id2));
 	}