Merge pull request #2388 from nextcloud/backport/2360/stable33

[stable33] perf: Replace getById call with getFirstNodeById

Author: miaulalala

lib/ViewInfoCache.php

@@ -1,5 +1,7 @@
 <?php
 
+declare(strict_types=1);
+
 /**
  * SPDX-FileCopyrightText: 2016 ownCloud, Inc.
  * SPDX-License-Identifier: AGPL-3.0-only
@@ -13,24 +15,14 @@
 use OCP\Files\NotFoundException;
 
 class ViewInfoCache {
-	/** @var array */
-	protected $cachePath;
-	/** @var array */
-	protected $cacheId;
-
+	protected array $cacheId = [];
 
 	public function __construct(
 		protected IRootFolder $rootFolder,
 	) {
 	}
 
-	/**
-	 * @param string $user
-	 * @param int $fileId
-	 * @param string $path
-	 * @return array
-	 */
-	public function getInfoById($user, $fileId, $path) {
+	public function getInfoById(string $user, int $fileId, string $path): array {
 		if (isset($this->cacheId[$user][$fileId])) {
 			$cache = $this->cacheId[$user][$fileId];
 			if ($cache['path'] === null) {
@@ -43,12 +35,9 @@ public function getInfoById($user, $fileId, $path) {
 	}
 
 	/**
-	 * @param string $user
-	 * @param int $fileId
-	 * @param string $filePath
-	 * @return array
+	 * @return array{path: string, exists: bool, is_dir: bool, view: string, node?: Node}
 	 */
-	protected function findInfoById($user, $fileId, $filePath) {
+	protected function findInfoById(string $user, int $fileId, string $filePath): array {
 		$cache = [
 			'path' => $filePath,
 			'exists' => false,
@@ -59,39 +48,36 @@ protected function findInfoById($user, $fileId, $filePath) {
 		$notFound = false;
 		try {
 			$userFolder = $this->rootFolder->getUserFolder($user);
-			$entries = $userFolder->getById($fileId);
-			if (empty($entries)) {
+			$entry = $userFolder->getFirstNodeById($fileId);
+			if ($entry === null) {
 				throw new NotFoundException('No entries returned');
 			}
-			/** @var Node $entry */
-			$entry = array_shift($entries);
 
 			$cache['path'] = $userFolder->getRelativePath($entry->getPath());
 			$cache['is_dir'] = $entry instanceof Folder;
 			$cache['exists'] = true;
 			$cache['node'] = $entry;
-		} catch (NotFoundException $e) {
+		} catch (NotFoundException) {
 			// The file was not found in the normal view,
 			// maybe it is in the trashbin?
 			try {
-				/** @var Folder $userTrashBin */
 				$userTrashBin = $this->rootFolder->get('/' . $user . '/files_trashbin');
-				$entries = $userTrashBin->getById($fileId);
-				if (empty($entries)) {
+				if (!$userTrashBin instanceof Folder) {
+					throw new NotFoundException('No trash bin found for user: ' . $user);
+				}
+				$entry = $userTrashBin->getFirstNodeById($fileId);
+				if ($entry === null) {
 					throw new NotFoundException('No entries returned');
 				}
 
-				/** @var Node $entry */
-				$entry = array_shift($entries);
-
 				$cache = [
 					'path' => $userTrashBin->getRelativePath($entry->getPath()),
 					'exists' => true,
 					'is_dir' => $entry instanceof Folder,
 					'view' => 'trashbin',
 					'node' => $entry,
 				];
-			} catch (NotFoundException $e) {
+			} catch (NotFoundException) {
 				$notFound = true;
 			}
 		}

tests/ViewInfoCacheTest.php

@@ -27,7 +27,6 @@
 use OCP\Files\File;
 use OCP\Files\Folder;
 use OCP\Files\IRootFolder;
-use OCP\Files\NotFoundException;
 use PHPUnit\Framework\Attributes\DataProvider;
 use PHPUnit\Framework\MockObject\MockObject;
 
@@ -57,7 +56,7 @@ public function getCache(array $methods = []): ViewInfoCache|MockObject {
 	public static function dataGetInfoById(): array {
 		return [
 			[
-				'user', 23, 'path', [], true, 'findInfoById',
+				'user', 23, 'path', [], true, ['findInfoById'],
 			],
 			[
 				'user',
@@ -69,7 +68,7 @@ public static function dataGetInfoById(): array {
 					]
 				],
 				true,
-				'findInfoById',
+				['findInfoById'],
 			],
 			[
 				'user',
@@ -81,7 +80,7 @@ public static function dataGetInfoById(): array {
 					]
 				],
 				true,
-				'findInfoById',
+				['findInfoById'],
 			],
 			[
 				'user',
@@ -111,15 +110,15 @@ public static function dataGetInfoById(): array {
 	}
 
 	#[DataProvider('dataGetInfoById')]
-	public function testGetInfoById(string $user, int $id, string|array $path, array $cache, bool $callsFind, string|array $expected): void {
+	public function testGetInfoById(string $user, int $id, string|array $path, array $cache, bool $callsFind, array $expected): void {
 		$infoCache = $this->getCache([
 			'findInfoById',
 		]);
 
 		if ($callsFind) {
 			$infoCache->expects($this->once())
 				->method('findInfoById')
-				->willReturn('findInfoById');
+				->willReturn(['findInfoById']);
 		} else {
 			$infoCache->expects($this->never())
 				->method('findInfoById');
@@ -239,9 +238,9 @@ public function testFindInfoById(string $user, int $fileId, string $filename, ?s
 			->willReturn($userFolder);
 		if ($path === null) {
 			$userFolder->expects($this->once())
-				->method('getById')
+				->method('getFirstNodeById')
 				->with($fileId)
-				->willThrowException(new NotFoundException());
+				->willReturn(null);
 
 			$userTrashBin = $this->createMock(Folder::class);
 			$this->rootFolder->expects($this->once())
@@ -250,9 +249,9 @@ public function testFindInfoById(string $user, int $fileId, string $filename, ?s
 				->willReturn($userTrashBin);
 			if ($pathTrash === null) {
 				$userTrashBin->expects($this->once())
-					->method('getById')
+					->method('getFirstNodeById')
 					->with($fileId)
-					->willThrowException(new NotFoundException());
+					->willReturn(null);
 			} else {
 				$node = $this->createMock($isDir ? Folder::class : File::class);
 				$node
@@ -264,9 +263,9 @@ public function testFindInfoById(string $user, int $fileId, string $filename, ?s
 					->willReturn($pathTrash);
 
 				$userTrashBin->expects($this->once())
-					->method('getById')
+					->method('getFirstNodeById')
 					->with($fileId)
-					->willReturn([2 => $node]);
+					->willReturn($node);
 				$expected['node'] = $node;
 				$expectedCache[$user][$fileId]['node'] = $node;
 			}
@@ -281,9 +280,9 @@ public function testFindInfoById(string $user, int $fileId, string $filename, ?s
 				->willReturn($path);
 
 			$userFolder->expects($this->once())
-				->method('getById')
+				->method('getFirstNodeById')
 				->with($fileId)
-				->willReturn([3 => $node]);
+				->willReturn($node);
 			$expected['node'] = $node;
 			$expectedCache[$user][$fileId]['node'] = $node;
 		}