Merge pull request #445 from nextcloud/enh/performance

Build parent path without queries

Author: mejo-

lib/Model/PageInfo.php

@@ -87,16 +87,18 @@ public function jsonSerialize(): array {
 	public function fromFile(File $file, int $parentId, ?string $lastUserId = null, ?string $lastUserDisplayName = null, ?string $emoji = null, ?string $subpageOrder = null): void {
 		$this->setId($file->getId());
 		// Set folder name as title for all index pages except the collective landing page
+		$dirName = dirname($file->getInternalPath());
+		$dirName = $dirName === '.' ? '' : $dirName;
 		if ($parentId !== 0 && 0 === strcmp($file->getName(), self::INDEX_PAGE_TITLE . self::SUFFIX)) {
-			$this->setTitle($file->getParent()->getName());
+			$this->setTitle(basename($dirName));
 		} else {
 			$this->setTitle(basename($file->getName(), self::SUFFIX));
 		}
+		$this->setFilePath($dirName);
 		$this->setTimestamp($file->getMTime());
 		$this->setSize($file->getSize());
 		$this->setFileName($file->getName());
 		$this->setCollectivePath(rtrim(explode('/', $file->getMountPoint()->getMountPoint(), 4)[3], '/'));
-		$this->setFilePath($file->getParent()->getInternalPath());
 		if (null !== $lastUserId) {
 			$this->setLastUserId($lastUserId);
 		}

lib/Mount/CollectiveFolderManager.php

@@ -32,6 +32,9 @@ class CollectiveFolderManager {
 	private IRequest $request;
 	private ?string $rootPath = null;
 
+	/** @var int|null */
+	private $rootFolderStorageId = null;
+
 	/**
 	 * CollectiveFolderManager constructor.
 	 *
@@ -185,14 +188,18 @@ private function getJailPath(int $folderId): string {
 	 * @throws NotFoundException
 	 */
 	private function getRootFolderStorageId(): int {
-		$qb = $this->connection->getQueryBuilder();
+		if ($this->rootFolderStorageId === null) {
+			$qb = $this->connection->getQueryBuilder();
 
-		$qb->select('fileid')
-			->from('filecache')
-			->where($qb->expr()->eq('storage', $qb->createNamedParameter($this->getRootFolder()->getStorage()->getCache()->getNumericStorageId())))
-			->andWhere($qb->expr()->eq('path_hash', $qb->createNamedParameter(md5($this->getRootPath()))));
+			$qb->select('fileid')
+				->from('filecache')
+				->where($qb->expr()->eq('storage', $qb->createNamedParameter($this->getRootFolder()->getStorage()->getCache()->getNumericStorageId())))
+				->andWhere($qb->expr()->eq('path_hash', $qb->createNamedParameter(md5($this->getRootPath()))));
+
+			$this->rootFolderStorageId = (int)$qb->execute()->fetchColumn();
+		}
 
-		return (int)$qb->execute()->fetchColumn();
+		return $this->rootFolderStorageId;
 	}
 
 	/**

lib/Service/PageService.php

@@ -11,6 +11,7 @@
 use OCP\Files\File;
 use OCP\Files\Folder;
 use OCP\Files\InvalidPathException;
+use OCP\Files\Node;
 use OCP\Files\NotPermittedException as FilesNotPermittedException;
 use OCP\Files\NotFoundException as FilesNotFoundException;
 use OCP\IConfig;
@@ -136,35 +137,38 @@ public function getFolder(int $collectiveId, int $fileId, string $userId): Folde
 
 	/**
 	 * @param File $file
+	 * @param Node|null $parent
 	 *
 	 * @return int
 	 * @throws NotFoundException
 	 */
-	public function getParentPageId(File $file): int {
+	private function getParentPageId(File $file, ?Node $parent = null): int {
 		try {
 			if (self::isLandingPage($file)) {
 				// Return `0` for landing page
 				return 0;
 			}
 
+			$parent = $parent ?? $file->getParent();
+
 			if (self::isIndexPage($file)) {
 				// Go down two levels if index page but not landing page
-				return $this->getIndexPageFile($file->getParent()->getParent())->getId();
+				return $this->getIndexPageFile($parent->getParent())->getId();
 			}
 
-			return $this->getIndexPageFile($file->getParent())->getId();
+			return $this->getIndexPageFile($parent)->getId();
 		} catch (InvalidPathException | FilesNotFoundException $e) {
 			throw new NotFoundException($e->getMessage(), 0, $e);
 		}
 	}
 
 	/**
 	 * @param File $file
-	 *
+	 * @param Node|null $parent
 	 * @return PageInfo
 	 * @throws NotFoundException
 	 */
-	private function getPageByFile(File $file): PageInfo {
+	private function getPageByFile(File $file, ?Node $parent = null): PageInfo {
 		$pageInfo = new PageInfo();
 		try {
 			$page = $this->pageMapper->findByFileId($file->getId());
@@ -176,7 +180,7 @@ private function getPageByFile(File $file): PageInfo {
 		$subpageOrder = ($page !== null) ? $page->getSubpageOrder() : null;
 		try {
 			$pageInfo->fromFile($file,
-				$this->getParentPageId($file),
+				$this->getParentPageId($file, $parent),
 				$lastUserId,
 				$lastUserId ? $this->userManager->get($lastUserId)->getDisplayName() : null,
 				$emoji,
@@ -439,7 +443,7 @@ public static function folderHasSubPage(Folder $folder, string $title): int {
 	public function recurseFolder(Folder $folder, string $userId): array {
 		// Find index page or create it if we have subpages, but it doesn't exist
 		try {
-			$indexPage = $this->getPageByFile($this->getIndexPageFile($folder));
+			$indexPage = $this->getPageByFile($this->getIndexPageFile($folder), $folder);
 		} catch (NotFoundException $e) {
 			if (!self::folderHasSubPages($folder)) {
 				return [];
@@ -451,7 +455,7 @@ public function recurseFolder(Folder $folder, string $userId): array {
 		// Add subpages and recurse over subfolders
 		foreach ($folder->getDirectoryListing() as $node) {
 			if ($node instanceof File && self::isPage($node) && !self::isIndexPage($node)) {
-				$pageInfos[] = $this->getPageByFile($node);
+				$pageInfos[] = $this->getPageByFile($node, $folder);
 			} elseif ($node instanceof Folder) {
 				array_push($pageInfos, ...$this->recurseFolder($node, $userId));
 			}

tests/Unit/Model/PageInfoTest.php

@@ -18,6 +18,7 @@ public function testFromFile(): void {
 		$fileMountPoint = '/files/user/Collectives/collective/';
 		$fileCollectivePath = 'Collectives/collective';
 		$parentInternalPath = 'path/to/file';
+		$internalPath = $parentInternalPath . '/' . $fileName;
 		$userId = 'jane';
 
 		$mountPoint = $this->getMockBuilder(MountPoint::class)
@@ -39,6 +40,7 @@ public function testFromFile(): void {
 		$file->method('getName')->willReturn($fileName);
 		$file->method('getMountPoint')->willReturn($mountPoint);
 		$file->method('getParent')->willReturn($parent);
+		$file->method('getInternalPath')->willReturn($internalPath);
 
 		$pageInfo = new PageInfo();
 		$pageInfo->fromFile($file, 1, $userId);

tests/Unit/Service/PageServiceTest.php

@@ -242,6 +242,8 @@ public function testRecurseFolder(): void {
 			->willReturn($folder);
 		$indexFile->method('getMountPoint')
 			->willReturn($mountPoint);
+		$indexFile->method('getInternalPath')
+			->willReturn('Collectives/testfolder/Readme.md');
 		$indexPage = new Page();
 		$this->pageMapper->method('findByFileId')
 			->willReturn($indexPage);
@@ -268,6 +270,9 @@ public function testRecurseFolder(): void {
 				->willReturn($folder);
 			$file->method('getMountPoint')
 				->willReturn($mountPoint);
+			$file->method('getInternalPath')
+				->willReturn('Collectives/testfolder/Readme.md');
+
 			$filesNotJustMd[] = $file;
 
 			// Only add markdown files to $filesJustMd