perf: Use getFirstNodeById and pass userId for our internal helper to…

… avoid fetching mountpoints Signed-off-by: Julius Härtl <jus@bitgrid.net>
nextcloud · Jun 20, 2024 · 96a842a · 96a842a
1 parent 5a6305d
commit 96a842a
Show file tree

Hide file tree

Showing 6 changed files with 24 additions and 40 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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,