From 96a842a4dadce60b49826471f82cc863d3f0810e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Fri, 21 Jun 2024 00:53:00 +0200 Subject: [PATCH] perf: Use getFirstNodeById and pass userId for our internal helper to avoid fetching mountpoints MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- lib/DAV/WorkspacePlugin.php | 7 +++---- lib/Middleware/SessionMiddleware.php | 4 ++-- lib/Notification/Notifier.php | 3 +-- lib/Service/ApiService.php | 28 +++++++++------------------- lib/Service/AttachmentService.php | 16 ++++++---------- lib/Service/DocumentService.php | 6 +++--- 6 files changed, 24 insertions(+), 40 deletions(-) diff --git a/lib/DAV/WorkspacePlugin.php b/lib/DAV/WorkspacePlugin.php index 275844a8095..dbe6c198444 100644 --- a/lib/DAV/WorkspacePlugin.php +++ b/lib/DAV/WorkspacePlugin.php @@ -85,12 +85,11 @@ public function propFind(PropFind $propFind, INode $node) { $file = null; $owner = $this->userId ?? $node->getFileInfo()->getStorage()->getOwner(''); - /** @var Folder[] $nodes */ - $nodes = $this->rootFolder->getUserFolder($owner)->getById($node->getId()); - if (count($nodes) > 0) { + $node = $this->rootFolder->getUserFolder($owner)->getFirstNodeById($node->getId()); + if ($node instanceof Folder) { /** @var File $file */ try { - $file = $this->workspaceService->getFile($nodes[0]); + $file = $this->workspaceService->getFile($node); } catch (StorageNotAvailableException $e) { // If a storage is not available we can for the propfind response assume that there is no rich workspace present } diff --git a/lib/Middleware/SessionMiddleware.php b/lib/Middleware/SessionMiddleware.php index e1e8aadbb41..ab1406b5cc7 100644 --- a/lib/Middleware/SessionMiddleware.php +++ b/lib/Middleware/SessionMiddleware.php @@ -120,7 +120,7 @@ private function assertUserOrShareToken(ISessionAwareController $controller): vo $documentId = (int)$this->request->getParam('documentId'); if (null !== $userId = $this->userSession->getUser()?->getUID()) { // Check if user has access to document - if (count($this->rootFolder->getUserFolder($userId)->getById($documentId)) === 0) { + if ($this->rootFolder->getUserFolder($userId)->getFirstNodeById($documentId) === null) { throw new InvalidSessionException(); } $controller->setUserId($userId); @@ -131,7 +131,7 @@ private function assertUserOrShareToken(ISessionAwareController $controller): vo throw new InvalidSessionException(); } // Check if shareToken has access to document - if (count($this->rootFolder->getUserFolder($share->getShareOwner())->getById($documentId)) === 0) { + if ($this->rootFolder->getUserFolder($share->getShareOwner())->getFirstNodeById($documentId) === null) { throw new InvalidSessionException(); } } else { diff --git a/lib/Notification/Notifier.php b/lib/Notification/Notifier.php index 316415f5a30..42d03f4ecb9 100644 --- a/lib/Notification/Notifier.php +++ b/lib/Notification/Notifier.php @@ -67,8 +67,7 @@ public function prepare(INotification $notification, string $languageCode): INot } catch (NotPermittedException|NoUserException $e) { throw new InvalidArgumentException(); } - $nodes = $userFolder->getById($fileId); - $node = array_shift($nodes); + $node = $userFolder->getFirstNodeById($fileId); if ($node === null) { throw new InvalidArgumentException(); diff --git a/lib/Service/ApiService.php b/lib/Service/ApiService.php index 22ebca2378b..2a23f01122c 100644 --- a/lib/Service/ApiService.php +++ b/lib/Service/ApiService.php @@ -32,26 +32,16 @@ use Psr\Log\LoggerInterface; class ApiService { - private IRequest $request; - private SessionService $sessionService; - private DocumentService $documentService; - private LoggerInterface $logger; - private EncodingService $encodingService; - private IL10N $l10n; - public function __construct(IRequest $request, - SessionService $sessionService, - DocumentService $documentService, - EncodingService $encodingService, - LoggerInterface $logger, - IL10N $l10n + public function __construct( + private IRequest $request, + private SessionService $sessionService, + private DocumentService $documentService, + private EncodingService $encodingService, + private LoggerInterface $logger, + private IL10N $l10n, + private ?string $userId, ) { - $this->request = $request; - $this->sessionService = $sessionService; - $this->documentService = $documentService; - $this->logger = $logger; - $this->encodingService = $encodingService; - $this->l10n = $l10n; } public function create(?int $fileId = null, ?string $filePath = null, ?string $baseVersionEtag = null, ?string $token = null, ?string $guestName = null): DataResponse { @@ -72,7 +62,7 @@ public function create(?int $fileId = null, ?string $filePath = null, ?string $b } } elseif ($fileId !== null) { try { - $file = $this->documentService->getFileById($fileId); + $file = $this->documentService->getFileById($fileId, $this->userId); } catch (NotFoundException|NotPermittedException $e) { $this->logger->error('No permission to access this file', [ 'exception' => $e ]); return new DataResponse(['error' => $this->l10n->t('No permission to access this file.')], Http::STATUS_NOT_FOUND); diff --git a/lib/Service/AttachmentService.php b/lib/Service/AttachmentService.php index eb2391a1d32..ff273f02261 100644 --- a/lib/Service/AttachmentService.php +++ b/lib/Service/AttachmentService.php @@ -414,9 +414,8 @@ private function getAttachmentDirectoryForFile(File $textFile, bool $create = fa } $ownerId = $owner->getUID(); $ownerUserFolder = $this->rootFolder->getUserFolder($ownerId); - $ownerTextFile = $ownerUserFolder->getById($textFile->getId()); - if (count($ownerTextFile) > 0) { - $ownerTextFile = $ownerTextFile[0]; + $ownerTextFile = $ownerUserFolder->getFirstNodeById($textFile->getId()); + if ($ownerTextFile !== null) { $ownerParentFolder = $ownerTextFile->getParent(); $attachmentFolderName = '.attachments.' . $textFile->getId(); if ($ownerParentFolder->nodeExists($attachmentFolderName)) { @@ -481,8 +480,7 @@ private function isDownloadDisabled(File $file): bool { */ private function getTextFile(int $documentId, string $userId): File { $userFolder = $this->rootFolder->getUserFolder($userId); - $files = $userFolder->getById($documentId); - $file = array_shift($files); + $file = $userFolder->getFirstNodeById($documentId); if ($file instanceof File && !$this->isDownloadDisabled($file)) { return $file; } @@ -512,8 +510,7 @@ private function getTextFilePublic(?int $documentId, string $shareToken): File { } elseif ($documentId !== null && $share->getNodeType() === 'folder') { $folder = $share->getNode(); if ($folder instanceof Folder) { - $textFile = $folder->getById($documentId); - $textFile = array_shift($textFile); + $textFile = $folder->getFirstNodeById($documentId); if ($textFile instanceof File && !$this->isDownloadDisabled($textFile)) { return $textFile; } @@ -539,9 +536,8 @@ private function getTextFilePublic(?int $documentId, string $shareToken): File { * @throws NoUserException */ public function cleanupAttachments(int $fileId): int { - $textFile = $this->rootFolder->getById($fileId); - if (count($textFile) > 0 && $textFile[0] instanceof File) { - $textFile = $textFile[0]; + $textFile = $this->rootFolder->getFirstNodeById($fileId); + if ($textFile instanceof File) { if ($textFile->getMimeType() === 'text/markdown') { // get IDs of the files inside the attachment dir try { diff --git a/lib/Service/DocumentService.php b/lib/Service/DocumentService.php index 8d3a8bccafb..6c6b9574fce 100644 --- a/lib/Service/DocumentService.php +++ b/lib/Service/DocumentService.php @@ -459,7 +459,7 @@ public function getFileForSession(Session $session, ?string $shareToken = null): $node = $share->getNode(); if ($node instanceof Folder) { - $node = $node->getById($session->getDocumentId())[0]; + $node = $node->getFirstNodeById($session->getDocumentId()); } if ($node instanceof File) { return $node; @@ -617,7 +617,7 @@ public function lock(int $fileId): bool { } try { - $file = $this->getFileById($fileId); + $file = $this->getFileById($fileId, $this->userId); $this->lockManager->lock(new LockContext( $file, ILock::TYPE_APP, @@ -636,7 +636,7 @@ public function unlock(int $fileId): void { } try { - $file = $this->getFileById($fileId); + $file = $this->getFileById($fileId, $this->userId); $this->lockManager->unlock(new LockContext( $file, ILock::TYPE_APP,