Skip to content

Perf: Optimize pages loading (Filecache path like approach) #2549

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Koc wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Perf: Optimize pages loading (Filecache path like approach) #2549

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions lib/Db/PageLinkMapper.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
namespace OCA\Collectives\Db;

use OCP\DB\Exception;
use OCP\DB\QueryBuilder\IQueryBuilder;
use OCP\IDBConnection;

class PageLinkMapper {
Expand All @@ -31,6 +32,31 @@ public function findByPageId(int $pageId): array {
return $qb->executeQuery()->fetchAll(\PDO::FETCH_COLUMN);
}

/**
* @param int[] $pageIds
* @return array<int, int[]> Linked page ids indexed by page id
* @throws Exception
*/
public function findByPageIds(array $pageIds): array {
if (empty($pageIds)) {
return [];
}

$qb = $this->db->getQueryBuilder();
$qb->select('page_id', 'linked_page_id')
->from(self::TABLE_NAME)
->where($qb->expr()->in('page_id', $qb->createNamedParameter($pageIds, IQueryBuilder::PARAM_INT_ARRAY)));

$linkedPageIds = [];
$result = $qb->executeQuery();
while ($row = $result->fetch()) {
$linkedPageIds[(int)$row['page_id']][] = (int)$row['linked_page_id'];
}
$result->closeCursor();

return $linkedPageIds;
}

/**
* @throws Exception
*/
Expand Down
44 changes: 44 additions & 0 deletions lib/Model/FileInfo.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
<?php

declare(strict_types=1);

/*
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/

namespace OCA\Collectives\Model;

/**
* Lightweight representation of a file entry from the `filecache` table.
*
* `path` is relative to the collective root folder (e.g. `Readme.md` or
* `subfolder/page.md`), so it matches the semantics of `File::getInternalPath()`
* within the jailed collective storage.
*/
class FileInfo {

@mejo- mejo- Jun 9, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe better to call this CollectiveFileInfo to not confuse it with the public FileInfo API from server.

public function __construct(
public readonly int $fileId,
public readonly int $storage,
public readonly string $path,
public readonly int $parent,
public readonly string $name,
public readonly int $mimetype,
public readonly int $mimepart,
public readonly int $size,
public readonly int $mtime,
public readonly int $storageMtime,
public readonly int $encrypted,
public readonly string $etag,
public readonly int $permissions,
) {
}

public function isPage(): bool {
return str_ends_with($this->name, PageInfo::SUFFIX);
}

public function isIndexPage(): bool {
return $this->name === PageInfo::INDEX_PAGE_TITLE . PageInfo::SUFFIX;
}
}
64 changes: 64 additions & 0 deletions lib/Model/PageInfo.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,70 @@ public function jsonSerialize(): array {
];
}

/**
* Build the page info from a lightweight filecache entry (see PageService::getPagesFromFolderV2).
*/
public function fromFileInfo(

@mejo- mejo- Jun 9, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This copies much of the logic of fromFile(). Maybe worth refactoring to have less code duplication?

FileInfo $fileInfo,
int $parentId,
?string $collectivePath = null,
?string $lastUserId = null,
?string $lastUserDisplayName = null,
?string $emoji = null,
?string $subpageOrder = null,
?bool $fullWidth = false,
?string $slug = null,
?string $tags = null,
?array $linkedPageIds = null,
): void {
$this->setId($fileInfo->fileId);
$dirName = dirname($fileInfo->path);
$dirName = $dirName === '.' ? '' : $dirName;
if ($fileInfo->isIndexPage()) {
if ($parentId === 0) {
// Landing page
$this->setTitle(Server::get(IFactory::class)->get('collectives')->t('Landing page'));
} else {
// Index page
$this->setTitle(basename($dirName));
}
} else {
$this->setTitle(basename($fileInfo->name, self::SUFFIX));
}
$this->setFilePath($dirName);
$this->setTimestamp($fileInfo->mtime);
$this->setSize($fileInfo->size);
$this->setFileName($fileInfo->name);
if ($collectivePath !== null) {

@mejo- mejo- Jun 9, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess these checks can be omitted as the default is null anyway, so setting them to null again doesn't hurt.

$this->setCollectivePath($collectivePath);
}
if ($lastUserId !== null) {
$this->setLastUserId($lastUserId);
}
if ($lastUserDisplayName !== null) {
$this->setLastUserDisplayName($lastUserDisplayName);
}
if ($emoji !== null) {
$this->setEmoji($emoji);
}
if ($fullWidth !== null) {
$this->setFullWidth($fullWidth);
}
if ($subpageOrder !== null) {
$this->setSubpageOrder($subpageOrder);
}
if ($slug !== null) {
$this->setSlug($slug);
}
if ($tags !== null) {
$this->setTags($tags);
}
if ($linkedPageIds !== null) {
$this->setLinkedPageIds($linkedPageIds);
}
$this->setParentId($parentId);
}

/**
* @throws InvalidPathException
* @throws NotFoundException
Expand Down
48 changes: 48 additions & 0 deletions lib/Mount/CollectiveFolderManager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
use OC\Files\Storage\Wrapper\Jail;
use OC\Files\Storage\Wrapper\PermissionsMask;
use OCA\Collectives\ACL\ACLStorageWrapper;
use OCA\Collectives\Model\FileInfo;
use OCP\DB\QueryBuilder\IQueryBuilder;
use OCP\Files\Cache\ICacheEntry;
use OCP\Files\Folder;
Expand Down Expand Up @@ -250,6 +251,53 @@ public function getFolderFileCachePerCollectiveId(array $ids): array {
return $result;
}

/**
* Load all filecache entries (files and folders) of a collective folder in a single query.
*
* @return FileInfo[] Indexed by file id, with paths relative to the collective root folder
*
* @throws NotFoundException
* @throws \OCP\DB\Exception
*/
public function getFileCacheForCollective(int $collectiveId): array {

@mejo- mejo- Jun 9, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it be an option to pass the folder name here as well and further narrow down the query to only contain files in this subfolder? This would make the query less heavy when getPagesFromFolderV2() is called with a subfolder and not for the whole collective folder.

@mejo- mejo- Jun 9, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would be particularly helpful for building the templateFolder page tree, as that one doesn't need to process all the files from the collective rood folder.

$jailPath = $this->getJailPath($collectiveId);
$storageId = $this->getRootFolderStorageId();

$qb = $this->connection->getQueryBuilder();
$qb->select('fileid', 'storage', 'path', 'parent', 'name', 'mimetype', 'mimepart',
Comment thread
mejo- marked this conversation as resolved.
'size', 'mtime', 'storage_mtime', 'encrypted', 'etag', 'permissions')
->from('filecache')
->where($qb->expr()->eq('storage', $qb->createNamedParameter($storageId, IQueryBuilder::PARAM_INT)))

@mejo- mejo- Jun 9, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason to not further filter on mimetype here and only get folders and Markdown files? If I understand the code further down correctly, we only process folders and Markdown files anyway, right?

@Koc Koc Jun 9, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My goal was to introduce a new index that will cover columns used in filter. So, in this case we should extend it with mime type then

// Trailing slash matters: it restricts to descendants of this collective and
// avoids matching sibling collectives (e.g. `16` must not match `160`).
->andWhere($qb->expr()->like('path', $qb->createNamedParameter($this->connection->escapeLikeParameter($jailPath . '/') . '%')));

$prefixLength = strlen($jailPath . '/');
$result = [];
$cursor = $qb->executeQuery();
while ($row = $cursor->fetch()) {
$relativePath = substr($row['path'], $prefixLength);
$result[(int)$row['fileid']] = new FileInfo(
fileId: (int)$row['fileid'],
storage: (int)$row['storage'],
path: $relativePath,
parent: (int)$row['parent'],
name: (string)$row['name'],
mimetype: (int)$row['mimetype'],
mimepart: (int)$row['mimepart'],
size: (int)$row['size'],
mtime: (int)$row['mtime'],
storageMtime: (int)$row['storage_mtime'],
encrypted: (int)$row['encrypted'],
etag: (string)$row['etag'],
permissions: (int)$row['permissions'],
);
}
$cursor->closeCursor();

return $result;
}

/**
* @throws InvalidPathException
* @throws NotFoundException
Expand Down
Loading
Loading