From 2b02997fd1a6639d870b615cc5c211842f47b579 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Mon, 15 Jul 2024 07:46:51 +0200 Subject: [PATCH] perf: Use getFirstNodeById as it is cached MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- REUSE.toml | 2 +- lib/Controller/AssetsController.php | 5 +- lib/Controller/DirectViewController.php | 5 +- lib/Controller/DocumentAPIController.php | 7 ++- lib/Controller/DocumentController.php | 6 +- lib/Controller/OCSController.php | 5 +- lib/Controller/WopiController.php | 8 +-- .../OfficeTargetReferenceProvider.php | 3 +- lib/TemplateManager.php | 8 +-- lib/TokenManager.php | 16 ++--- tests/psalm-baseline.xml | 62 ++++++++----------- 11 files changed, 55 insertions(+), 72 deletions(-) diff --git a/REUSE.toml b/REUSE.toml index acbdd57624..18fc90295f 100644 --- a/REUSE.toml +++ b/REUSE.toml @@ -18,7 +18,7 @@ SPDX-FileCopyrightText = "2016 Nextcloud contributors" SPDX-License-Identifier = "AGPL-3.0-or-later" [[annotations]] -path = ["js/**.js.map", "js/**.js", "js/**.mjs", "js/**.mjs.map", "js/templates/**.handlebars", "emptyTemplates/**", "cypress/fixtures/**", "tests/features/**.feature"] +path = ["js/**.js.map", "js/**.js", "js/**.mjs", "js/**.mjs.map", "js/templates/**.handlebars", "emptyTemplates/**", "cypress/fixtures/**", "tests/features/**.feature", "tests/psalm-baseline.xml"] precedence = "aggregate" SPDX-FileCopyrightText = "2016 Nextcloud GmbH and Nextcloud contributors" SPDX-License-Identifier = "AGPL-3.0-or-later" diff --git a/lib/Controller/AssetsController.php b/lib/Controller/AssetsController.php index 1d6712a1dc..d7792a6017 100644 --- 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 index 663770599e..fc0c941a82 100644 --- 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 index db13a9a17a..3aa7edab3e 100644 --- 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 index 118a8a910c..acd834287c 100644 --- 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 index e889d40f10..b9d15d6008 100644 --- 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 index 0ad169a1a2..ce8606b1a6 100644 --- 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 index 30496cb4cb..68fc1a1f78 100644 --- 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 index a6f0d16d4a..bc6b578a96 100644 --- 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 index 6b536c493b..77d5ce787b 100644 --- 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 index e14f949556..5da7af4264 100644 --- a/tests/psalm-baseline.xml +++ b/tests/psalm-baseline.xml @@ -1,12 +1,8 @@ - - + - [] + @@ -14,22 +10,22 @@ - Command + - Command + - Command + - Command + @@ -39,7 +35,7 @@ - \OCA\Files\Helper + @@ -47,13 +43,13 @@ getId()]]> - $app !== '' + - NullOutput - NullOutput + + @@ -61,27 +57,27 @@ fetchPreview($template, $x, $y, $a, $forceIcon, $mode)]]> - DataResponse + - NotFoundResponse + - null + - $path === '' + - putContent - putContent + + - $time + @@ -91,7 +87,7 @@ - Image + capabilitites]]> @@ -99,20 +95,20 @@ - OutputInterface - OutputInterface + + - is_array($trustedList) + - SharingExternalStorage + - getRemote - getToken + + @@ -124,15 +120,7 @@ }, $collaboraTemplates)]]> - array - - - - - $template - - - ?File +