perf: Use getFirstNodeById as it is cached

Signed-off-by: Julius Härtl <[email protected]>
nextcloud · Jul 15, 2024 · 844622f · 844622f
1 parent 51077f2
commit 844622f
Show file tree

Hide file tree

Showing 10 changed files with 54 additions and 71 deletions.
diff --git a/lib/Controller/AssetsController.php b/lib/Controller/AssetsController.php
@@ -93,13 +93,12 @@ public function get($token) {
 
 		$this->userScopeService->setUserScope($asset->getUid());
 		$userFolder = $this->rootFolder->getUserFolder($asset->getUid());
-		$nodes = $userFolder->getById($asset->getFileid());
+		$node = $userFolder->getFirstNodeById($asset->getFileid());
 
-		if ($nodes === []) {
+		if ($node === null) {
 			return new DataResponse([], Http::STATUS_NOT_FOUND);
 		}
 
-		$node = array_pop($nodes);
 		if (!($node instanceof File)) {
 			return new DataResponse([], Http::STATUS_NOT_FOUND);
 		}

diff --git a/lib/Controller/DirectViewController.php b/lib/Controller/DirectViewController.php
@@ -79,7 +79,7 @@ public function show($token) {
 		$folder = $this->rootFolder->getUserFolder($direct->getUid());
 
 		try {
-			$item = $folder->getById($direct->getFileid())[0];
+			$item = $folder->getFirstNodeById($direct->getFileid());
 			if (!($item instanceof Node)) {
 				throw new \Exception();
 			}
@@ -126,8 +126,7 @@ public function showPublicShare(Direct $direct) {
 
 			$node = $share->getNode();
 			if ($node instanceof Folder) {
-				$nodes = $node->getById($direct->getFileid());
-				$node = array_shift($nodes);
+				$node = $node->getFirstNodeById($direct->getFileid());
 				if ($node === null) {
 					throw new NotFoundException();
 				}

diff --git a/lib/Controller/DocumentAPIController.php b/lib/Controller/DocumentAPIController.php
@@ -142,8 +142,11 @@ public function create(string $mimeType, string $fileName, string $directoryPath
 	#[Http\Attribute\NoAdminRequired]
 	public function openLocal(int $fileId): DataResponse {
 		try {
-			$files = $this->rootFolder->getUserFolder($this->userId)->getById($fileId);
-			$file = array_shift($files);
+			$file = $this->rootFolder->getUserFolder($this->userId)->getFirstNodeById($fileId);
+			if ($file === null) {
+				return new DataResponse([], Http::STATUS_NOT_FOUND);
+			}
+
 			$this->lockManager->unlock(new LockContext(
 				$file,
 				ILock::TYPE_APP,

diff --git a/lib/Controller/DocumentController.php b/lib/Controller/DocumentController.php
@@ -408,8 +408,7 @@ private function getFileForUser(int $fileId, ?string $path = null): File {
 		if ($path !== null) {
 			$node = $folder->get($path);
 		} else {
-			$nodes = $folder->getById($fileId);
-			$node = array_shift($nodes);
+			$node = $folder->getFirstNodeById($fileId);
 		}
 
 		if ($node instanceof File) {
@@ -449,8 +448,7 @@ private function getFileForShare(IShare $share, ?int $fileId, ?string $path = nu
 		if ($path !== null) {
 			$node = $node->get($path);
 		} else {
-			$nodes = $node->getById($fileId);
-			$node = array_shift($nodes);
+			$node = $node->getFirstNodeById($fileId);
 		}
 
 		if ($node instanceof File) {

diff --git a/lib/Controller/OCSController.php b/lib/Controller/OCSController.php
@@ -94,13 +94,12 @@ public function __construct(string $appName,
 	public function createDirect($fileId) {
 		try {
 			$userFolder = $this->rootFolder->getUserFolder($this->userId);
-			$nodes = $userFolder->getById($fileId);
+			$node = $userFolder->getFirstNodeById($fileId);
 
-			if ($nodes === []) {
+			if ($node === null) {
 				throw new OCSNotFoundException();
 			}
 
-			$node = $nodes[0];
 			if ($node instanceof Folder) {
 				throw new OCSBadRequestException('Cannot view folder');
 			}

diff --git a/lib/Controller/WopiController.php b/lib/Controller/WopiController.php
@@ -409,11 +409,10 @@ public function putFile($fileId,
 			if ($isPutRelative) {
 				// the new file needs to be installed in the current user dir
 				$userFolder = $this->rootFolder->getUserFolder($wopi->getEditorUid());
-				$file = $userFolder->getById($fileId);
-				if (count($file) === 0) {
+				$file = $userFolder->getFirstNodeById($fileId);
+				if ($file === null) {
 					return new JSONResponse([], Http::STATUS_NOT_FOUND);
 				}
-				$file = $file[0];
 				$suggested = $this->request->getHeader('X-WOPI-SuggestedTarget');
 				$suggested = mb_convert_encoding($suggested, 'utf-8', 'utf-7');
 
@@ -782,8 +781,7 @@ private function getFileForWopiToken(Wopi $wopi) {
 				return $node;
 			}
 
-			$nodes = $node->getById($wopi->getFileid());
-			return array_shift($nodes);
+			return $node->getFirstNodeById($wopi->getFileid());
 		}
 
 		// Group folders requires an active user to be set in order to apply the proper acl permissions as for anonymous requests it requires share permissions for read access

diff --git a/lib/Reference/OfficeTargetReferenceProvider.php b/lib/Reference/OfficeTargetReferenceProvider.php
@@ -66,8 +66,7 @@ public function resolveReference(string $referenceText): ?IReference {
 
 		try {
 			$userFolder = $this->rootFolder->getUserFolder($this->userId);
-			$files = $userFolder->getById($fileId);
-			$file = array_shift($files);
+			$file = $userFolder->getFirstNodeById($fileId);
 		} catch (Exception $e) {
 			$this->logger->info('Failed to get file for office target reference: ' . $fileId, ['exception' => $e]);
 			return null;

diff --git a/lib/TemplateManager.php b/lib/TemplateManager.php
@@ -162,9 +162,9 @@ public function get(int $fileId) {
 
 		$templateDir = $this->getUserTemplateDir();
 		// finally get the template file
-		$files = $templateDir->getById($fileId);
-		if ($files !== []) {
-			return $files[0];
+		$file = $templateDir->getFirstNodeById($fileId);
+		if ($file !== null) {
+			return $file;
 		}
 
 		throw new NotFoundException();
@@ -575,7 +575,7 @@ public function getTemplateSource(int $fileId): ?File {
 			} catch (NotFoundException $e) {
 				$userFolder = $this->rootFolder->getUserFolder($this->userId);
 				try {
-					$template = $userFolder->getById($templateId);
+					$template = $userFolder->getFirstNodeById($templateId);
 				} catch (NotFoundException $e) {
 					$this->logger->warning('Could not retrieve template source file', ['exception' => $e]);
 					return null;

diff --git a/lib/TokenManager.php b/lib/TokenManager.php
@@ -154,7 +154,13 @@ public function generateWopiToken(string $fileId, ?string $shareToken = null, ?s
 			}
 		}
 		/** @var File $file */
-		$file = $rootFolder->getById($fileId)[0];
+		$file = $rootFolder->getFirstNodeById($fileId);
+
+		// Check node readability (for storage wrapper overwrites like terms of services)
+		if ($file === null || !$file->isReadable()) {
+			throw new NotPermittedException();
+		}
+
 		// If its a public share, use the owner from the share, otherwise check the file object
 		if (is_null($owneruid)) {
 			$owner = $file->getOwner();
@@ -166,11 +172,6 @@ public function generateWopiToken(string $fileId, ?string $shareToken = null, ?s
 			}
 		}
 
-		// Check node readability (for storage wrapper overwrites like terms of services)
-		if (!$file->isReadable()) {
-			throw new NotPermittedException();
-		}
-
 		// Safeguard that users without required group permissions cannot create a token
 		if (!$this->permissionManager->isEnabledForUser($owneruid) && !$this->permissionManager->isEnabledForUser($editoruid)) {
 			throw new NotPermittedException();
@@ -226,8 +227,7 @@ public function generateWopiTokenForTemplate(File $templateFile, ?string $userId
 		$owneruid = $userId;
 		$editoruid = $userId;
 		$rootFolder = $this->rootFolder->getUserFolder($editoruid);
-		$targetFile = $rootFolder->getById($targetFileId);
-		$targetFile = array_shift($targetFile);
+		$targetFile = $rootFolder->getFirstNodeById($targetFileId);
 		if (!$targetFile instanceof File) {
 			throw new NotFoundException();
 		}

diff --git a/tests/psalm-baseline.xml b/tests/psalm-baseline.xml
@@ -1,35 +1,31 @@
 <?xml version="1.0" encoding="UTF-8"?>
-<!--
- - SPDX-FileCopyrightText: 2021 Nextcloud GmbH and Nextcloud contributors
- - SPDX-License-Identifier: AGPL-3.0-or-later
--->
-<files psalm-version="5.21.1@8c473e2437be8b6a8fd8f630f0f11a16b114c494">
+<files psalm-version="5.24.0@462c80e31c34e58cc4f750c656be3927e80e550e">
   <file src="lib/AppConfig.php">
     <InvalidArgument>
-      <code>[]</code>
+      <code><![CDATA[[]]]></code>
     </InvalidArgument>
     <RedundantCondition>
       <code><![CDATA[array_key_exists($key, self::APP_SETTING_TYPES) && self::APP_SETTING_TYPES[$key] === 'array']]></code>
     </RedundantCondition>
   </file>
   <file src="lib/Command/ActivateConfig.php">
     <UndefinedClass>
-      <code>Command</code>
+      <code><![CDATA[Command]]></code>
     </UndefinedClass>
   </file>
   <file src="lib/Command/ConvertToBigInt.php">
     <UndefinedClass>
-      <code>Command</code>
+      <code><![CDATA[Command]]></code>
     </UndefinedClass>
   </file>
   <file src="lib/Command/InstallDefaultFonts.php">
     <UndefinedClass>
-      <code>Command</code>
+      <code><![CDATA[Command]]></code>
     </UndefinedClass>
   </file>
   <file src="lib/Command/UpdateEmptyTemplates.php">
     <UndefinedClass>
-      <code>Command</code>
+      <code><![CDATA[Command]]></code>
     </UndefinedClass>
   </file>
   <file src="lib/Controller/DirectViewController.php">
@@ -39,49 +35,49 @@
   </file>
   <file src="lib/Controller/DocumentAPIController.php">
     <UndefinedClass>
-      <code>\OCA\Files\Helper</code>
+      <code><![CDATA[\OCA\Files\Helper]]></code>
     </UndefinedClass>
   </file>
   <file src="lib/Controller/DocumentController.php">
     <InvalidScalarArgument>
       <code><![CDATA[$node->getId()]]></code>
     </InvalidScalarArgument>
     <RedundantCondition>
-      <code>$app !== ''</code>
+      <code><![CDATA[$app !== '']]></code>
     </RedundantCondition>
   </file>
   <file src="lib/Controller/SettingsController.php">
     <UndefinedClass>
-      <code>NullOutput</code>
-      <code>NullOutput</code>
+      <code><![CDATA[NullOutput]]></code>
+      <code><![CDATA[NullOutput]]></code>
     </UndefinedClass>
   </file>
   <file src="lib/Controller/TemplatesController.php">
     <InvalidReturnStatement>
       <code><![CDATA[$this->fetchPreview($template, $x, $y, $a, $forceIcon, $mode)]]></code>
     </InvalidReturnStatement>
     <InvalidReturnType>
-      <code>DataResponse</code>
+      <code><![CDATA[DataResponse]]></code>
     </InvalidReturnType>
     <InvalidThrow>
-      <code>NotFoundResponse</code>
+      <code><![CDATA[NotFoundResponse]]></code>
     </InvalidThrow>
   </file>
   <file src="lib/Controller/WopiController.php">
     <NullArgument>
-      <code>null</code>
+      <code><![CDATA[null]]></code>
     </NullArgument>
     <TypeDoesNotContainType>
-      <code>$path === ''</code>
+      <code><![CDATA[$path === '']]></code>
     </TypeDoesNotContainType>
     <UndefinedInterfaceMethod>
-      <code>putContent</code>
-      <code>putContent</code>
+      <code><![CDATA[putContent]]></code>
+      <code><![CDATA[putContent]]></code>
     </UndefinedInterfaceMethod>
   </file>
   <file src="lib/Helper.php">
     <InvalidScalarArgument>
-      <code>$time</code>
+      <code><![CDATA[$time]]></code>
     </InvalidScalarArgument>
   </file>
   <file src="lib/PermissionManager.php">
@@ -91,28 +87,28 @@
   </file>
   <file src="lib/Preview/Office.php">
     <MissingDependency>
-      <code>Image</code>
+      <code><![CDATA[Image]]></code>
     </MissingDependency>
     <UndefinedThisPropertyAssignment>
       <code><![CDATA[$this->capabilitites]]></code>
     </UndefinedThisPropertyAssignment>
   </file>
   <file src="lib/Service/ConnectivityService.php">
     <UndefinedClass>
-      <code>OutputInterface</code>
-      <code>OutputInterface</code>
+      <code><![CDATA[OutputInterface]]></code>
+      <code><![CDATA[OutputInterface]]></code>
     </UndefinedClass>
   </file>
   <file src="lib/Service/FederationService.php">
     <TypeDoesNotContainType>
-      <code>is_array($trustedList)</code>
+      <code><![CDATA[is_array($trustedList)]]></code>
     </TypeDoesNotContainType>
     <UndefinedClass>
-      <code>SharingExternalStorage</code>
+      <code><![CDATA[SharingExternalStorage]]></code>
     </UndefinedClass>
     <UndefinedInterfaceMethod>
-      <code>getRemote</code>
-      <code>getToken</code>
+      <code><![CDATA[getRemote]]></code>
+      <code><![CDATA[getToken]]></code>
     </UndefinedInterfaceMethod>
   </file>
   <file src="lib/Template/CollaboraTemplateProvider.php">
@@ -124,15 +120,7 @@
 		}, $collaboraTemplates)]]></code>
     </InvalidReturnStatement>
     <InvalidReturnType>
-      <code>array</code>
-    </InvalidReturnType>
-  </file>
-  <file src="lib/TemplateManager.php">
-    <InvalidReturnStatement>
-      <code>$template</code>
-    </InvalidReturnStatement>
-    <InvalidReturnType>
-      <code>?File</code>
+      <code><![CDATA[array]]></code>
     </InvalidReturnType>
   </file>
   <file src="lib/WOPI/Parser.php">