Skip to content

Commit 6f7f9aa

Browse files
committed
Perf: Optimize pages loading (start work on method reading)
Signed-off-by: Kostiantyn Miakshyn <molodchick@gmail.com>
1 parent 3ffab03 commit 6f7f9aa

14 files changed

Lines changed: 434 additions & 57 deletions

appinfo/info.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
- SPDX-License-Identifier: AGPL-3.0-or-later
55
-->
66
<info xmlns:xsi= "http://www.w3.org/2001/XMLSchema-instance"
7-
xsi:noNamespaceSchemaLocation="https://apps.nextcloud.com/schema/apps/info.xsd">
7+
xsi:noNamespaceSchemaLocation="https://apps.nextcloud.com/schema/apps/info.xsd">
88
<id>collectives</id>
99
<name>Collectives</name>
1010
<summary>A place for activist and community projects to build shared knowledge</summary>

lib/Db/FileCacheMapper.php

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
7+
* SPDX-License-Identifier: AGPL-3.0-or-later
8+
*/
9+
10+
namespace OCA\Collectives\Db;
11+
12+
use OCA\Collectives\Model\FileInfo;
13+
use OCP\DB\QueryBuilder\IQueryBuilder;
14+
use OCP\IDBConnection;
15+
16+
class FileCacheMapper {
17+
private const BATCH_SIZE = 1000;
18+
19+
public function __construct(
20+
private readonly IDBConnection $db,
21+
) {
22+
}
23+
24+
/**
25+
* Query filecache for file data by file IDs
26+
*
27+
* @param int[] $fileIds
28+
* @return array<int, FileInfo> Indexed by fileId
29+
*/
30+
public function findByFileIds(array $fileIds): array {
31+
if (empty($fileIds)) {
32+
return [];
33+
}
34+
35+
$fileInfos = [];
36+
foreach (array_chunk($fileIds, self::BATCH_SIZE) as $chunk) {
37+
$qb = $this->db->getQueryBuilder();
38+
$qb->select('fileid', 'storage', 'parent', 'name', 'mimetype', 'mimepart', 'size', 'mtime', 'storage_mtime', 'etag', 'encrypted', 'permissions', 'checksum', 'unencrypted_size')
39+
->from('filecache')
40+
->where($qb->expr()->in('fileid', $qb->createNamedParameter($chunk, IQueryBuilder::PARAM_INT_ARRAY)));
41+
42+
$result = $qb->executeQuery();
43+
while ($row = $result->fetch()) {
44+
$fileInfo = new FileInfo(
45+
fileId: (int)$row['fileid'],
46+
storage: (int)$row['storage'],
47+
parent: (int)$row['parent'],
48+
name: (string)$row['name'],
49+
mimetype: (int)$row['mimetype'],
50+
mimepart: (int)$row['mimepart'],
51+
size: (int)$row['size'],
52+
mtime: (int)$row['mtime'],
53+
storage_mtime: (int)$row['storage_mtime'],
54+
etag: (string)$row['etag'],
55+
encrypted: (int)$row['encrypted'],
56+
permissions: (int)$row['permissions'],
57+
checksum: $row['checksum'] !== null ? (string)$row['checksum'] : null,
58+
unencrypted_size: (int)$row['unencrypted_size'],
59+
);
60+
$fileInfos[$fileInfo->fileId] = $fileInfo;
61+
}
62+
$result->closeCursor();
63+
}
64+
65+
return $fileInfos;
66+
}
67+
}

lib/Db/PageMapper.php

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,26 @@ public function findByFileIds(array $fileIds, bool $trashed = false): array {
110110
return $pagesByFileId;
111111
}
112112

113+
/**
114+
* @return Page[]
115+
*/
116+
public function findByCollectiveId(int $collectiveId, bool $trashed = false): array {
117+
$qb = $this->db->getQueryBuilder();
118+
$andX = [
119+
$qb->expr()->eq('collective_id', $qb->createNamedParameter($collectiveId, IQueryBuilder::PARAM_INT)),
120+
];
121+
// fixme: change index to use timestamp as well?
122+
if ($trashed) {
123+
$andX[] = $qb->expr()->isNotNull('trash_timestamp');
124+
} else {
125+
$andX[] = $qb->expr()->isNull('trash_timestamp');
126+
}
127+
$qb->select('*')
128+
->from($this->tableName)
129+
->where($qb->expr()->andX(...$andX));
130+
return $this->findEntities($qb);
131+
}
132+
113133
/**
114134
* @return Page[]
115135
*/

lib/Fs/NodeHelper.php

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,14 @@
1010
namespace OCA\Collectives\Fs;
1111

1212
use OCA\Collectives\Model\PageInfo;
13+
use OCA\Collectives\Mount\CollectiveStorage;
1314
use OCA\Collectives\Service\NotFoundException;
1415
use OCA\Collectives\Service\NotPermittedException;
1516
use OCP\Files\File;
1617
use OCP\Files\Folder;
1718
use OCP\Files\GenericFileException;
1819
use OCP\Files\InvalidPathException;
20+
use OCP\Files\Node;
1921
use OCP\Files\NotFoundException as FilesNotFoundException;
2022
use OCP\Files\NotPermittedException as FilesNotPermittedException;
2123
use OCP\IDBConnection;
@@ -257,12 +259,11 @@ public static function folderHasSubPage(Folder $folder, string $title): int {
257259
* Only processes collective pages (not versions)
258260
*/
259261
public static function extractCollectiveIdFromPath(string $path): ?int {
260-
// fixme: cover all possible cases with unit test
261262
$parts = explode('/', $path);
262-
if (!str_starts_with($parts[0], 'appdata_')) {
263+
if (!isset($parts[0]) || !str_starts_with($parts[0], 'appdata_')) {
263264
return null;
264265
}
265-
if ($parts[1] !== 'collectives') {
266+
if (!isset($parts[1]) || $parts[1] !== 'collectives') {
266267
return null;
267268
}
268269

@@ -276,4 +277,21 @@ public static function extractCollectiveIdFromPath(string $path): ?int {
276277

277278
return (int)$collectiveId;
278279
}
280+
281+
/**
282+
* Get collective ID from node if it is a markdown file belonging to a CollectiveStorage
283+
*/
284+
public static function getCollectiveIdFromNode(Node $node): ?int {
285+
if (!($node instanceof File) || !self::isPage($node)) {
286+
return null;
287+
}
288+
289+
$storage = $node->getStorage();
290+
if (!$storage->instanceOfStorage(CollectiveStorage::class)) {
291+
return null;
292+
}
293+
294+
/** @var CollectiveStorage $storage */
295+
return $storage->getFolderId();
296+
}
279297
}

lib/Listeners/NodeDeletedListener.php

Lines changed: 3 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,11 @@
1010
namespace OCA\Collectives\Listeners;
1111

1212
use OCA\Collectives\Db\PageMapper;
13-
use OCA\Collectives\Mount\CollectiveStorage;
13+
use OCA\Collectives\Fs\NodeHelper;
1414
use OCA\Collectives\Service\PageService;
1515
use OCP\EventDispatcher\Event;
1616
use OCP\EventDispatcher\IEventListener;
1717
use OCP\Files\Events\Node\NodeDeletedEvent;
18-
use OCP\Files\File;
1918

2019
/** @template-implements IEventListener<Event|NodeDeletedEvent> */
2120
class NodeDeletedListener implements IEventListener {
@@ -30,27 +29,16 @@ public function handle(Event $event): void {
3029
return;
3130
}
3231

33-
// Skip if this deletion originated from Collectives itself
3432
if ($this->pageService->isFromCollectives()) {
3533
return;
3634
}
3735

3836
$node = $event->getNode();
39-
40-
if (!($node instanceof File)) {
41-
return;
42-
}
43-
44-
if ($node->getMimeType() !== 'text/markdown') {
45-
return;
46-
}
47-
48-
$storage = $node->getStorage();
49-
if (!$storage->instanceOfStorage(CollectiveStorage::class)) {
37+
$collectiveId = NodeHelper::getCollectiveIdFromNode($node);
38+
if ($collectiveId === null) {
5039
return;
5140
}
5241

53-
// Delete the page entry from collectives_pages
5442
$this->pageMapper->deleteByFileId($node->getId());
5543
}
5644
}

lib/Listeners/NodeRenamedListener.php

Lines changed: 2 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,10 @@
1111

1212
use OCA\Collectives\Db\PageMapper;
1313
use OCA\Collectives\Fs\NodeHelper;
14-
use OCA\Collectives\Mount\CollectiveStorage;
1514
use OCA\Collectives\Service\PageService;
1615
use OCP\EventDispatcher\Event;
1716
use OCP\EventDispatcher\IEventListener;
1817
use OCP\Files\Events\Node\NodeRenamedEvent;
19-
use OCP\Files\File;
20-
use OCP\Files\Node;
2118

2219
/** @template-implements IEventListener<Event|NodeRenamedEvent> */
2320
class NodeRenamedListener implements IEventListener {
@@ -36,39 +33,20 @@ public function handle(Event $event): void {
3633
return;
3734
}
3835

39-
$source = $event->getSource();
4036
$target = $event->getTarget();
41-
if (!($source instanceof File && $target instanceof File)) {
42-
return;
43-
}
44-
45-
// Only handle markdown files in collectives
46-
if ($target->getMimeType() !== 'text/markdown') {
47-
return;
48-
}
49-
50-
// // File moved out of a collective -> delete page entry
51-
$targetCollectiveId = $this->getCollectiveId($target);
37+
$targetCollectiveId = NodeHelper::getCollectiveIdFromNode($target);
5238
if ($targetCollectiveId === null) {
5339
// fixme: it seems like this case not works
5440
$this->pageMapper->deleteByFileId($target->getId());
5541
return;
5642
}
5743

5844
// File moved into a collective or between collectives - update collectiveId
45+
$source = $event->getSource();
5946
$title = null;
6047
if ($source->getName() !== $target->getName()) {
6148
$title = NodeHelper::getTitleFromFile($target);
6249
}
6350
$this->pageService->updatePage($targetCollectiveId, $target->getId(), null, null, null, $title);
6451
}
65-
66-
private function getCollectiveId(Node|File $node): ?int {
67-
$sourceStorage = $node->getStorage();
68-
if ($sourceStorage->instanceOfStorage(CollectiveStorage::class)) {
69-
/** @var CollectiveStorage $sourceStorage */
70-
return $sourceStorage->getFolderId();
71-
}
72-
return null;
73-
}
7452
}

lib/Listeners/NodeWrittenListener.php

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,10 @@
1313
use OCA\Collectives\Db\PageLinkMapper;
1414
use OCA\Collectives\Fs\MarkdownHelper;
1515
use OCA\Collectives\Fs\NodeHelper;
16-
use OCA\Collectives\Mount\CollectiveStorage;
1716
use OCA\Collectives\Service\PageService;
1817
use OCP\EventDispatcher\Event;
1918
use OCP\EventDispatcher\IEventListener;
2019
use OCP\Files\Events\Node\NodeWrittenEvent;
21-
use OCP\Files\File;
2220
use OCP\Files\Node;
2321
use OCP\IConfig;
2422

@@ -38,16 +36,11 @@ public function handle(Event $event): void {
3836
}
3937

4038
$node = $event->getNode();
41-
$storage = $node->getStorage();
42-
// fixme: handle directory creation in order to create readme file with corresponding collective page
43-
if (!($node instanceof File)
44-
|| $node->getMimeType() !== 'text/markdown'
45-
|| !$node->getStorage()->instanceOfStorage(CollectiveStorage::class)) {
39+
$collectiveId = NodeHelper::getCollectiveIdFromNode($node);
40+
if ($collectiveId === null) {
4641
return;
4742
}
4843

49-
/** @var CollectiveStorage $storage */
50-
$collectiveId = $storage->getFolderId();
5144
$fileId = $node->getId();
5245
$this->updatePageLinks($collectiveId, $node, $fileId);
5346

@@ -58,13 +51,13 @@ public function handle(Event $event): void {
5851
$this->updateCollectivePage($node, $collectiveId, $fileId);
5952
}
6053

61-
private function updatePageLinks(int $collectiveId, Node|File $node, int $fileId): void {
54+
private function updatePageLinks(int $collectiveId, Node $node, int $fileId): void {
6255
$collective = $this->collectiveMapper->idToCollective($collectiveId);
6356
$linkedPageIds = MarkdownHelper::getLinkedPageIds($collective, $node->getContent(), $this->config->getSystemValue('trusted_domains', []));
6457
$this->pageLinkMapper->updateByPageId($fileId, $linkedPageIds);
6558
}
6659

67-
private function updateCollectivePage(Node|File $node, int $collectiveId, int $fileId): void {
60+
private function updateCollectivePage(Node $node, int $collectiveId, int $fileId): void {
6861
$title = NodeHelper::getTitleFromFile($node);
6962
$userId = $node->getOwner()->getUID();
7063
$this->pageService->updatePage($collectiveId, $fileId, $userId, null, null, $title);

lib/Migration/Version030401Date20250331000000.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,8 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt
3131
$changed = true;
3232
}
3333

34-
if (!$table->hasIndex('collectives_pages_collective_id_index')) {
35-
$table->addIndex(['collective_id'], 'collectives_pages_collective_id_index');
34+
if (!$table->hasIndex('collectives_pages_c_id_idx')) {
35+
$table->addIndex(['collective_id'], 'collectives_pages_c_id_idx');
3636
$changed = true;
3737
}
3838

lib/Model/FileInfo.php

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
7+
* SPDX-License-Identifier: AGPL-3.0-or-later
8+
*/
9+
10+
namespace OCA\Collectives\Model;
11+
12+
/**
13+
* FileInfo model with public readonly properties for better SCA
14+
* Represents a file entry from the filecache table
15+
*/
16+
class FileInfo {
17+
public function __construct(
18+
public readonly int $fileId,
19+
public readonly int $storage,
20+
public readonly int $parent,
21+
public readonly string $name,
22+
public readonly int $mimetype,
23+
public readonly int $mimepart,
24+
public readonly int $size,
25+
public readonly int $mtime,
26+
public readonly int $storage_mtime,
27+
public readonly string $etag,
28+
public readonly int $encrypted,
29+
public readonly int $permissions,
30+
public readonly ?string $checksum,
31+
public readonly int $unencrypted_size,
32+
) {
33+
}
34+
35+
/**
36+
* Get the title from filename (strip .md suffix, handle index pages)
37+
*/
38+
public function getTitle(): string {
39+
if ($this->name === PageInfo::INDEX_PAGE_TITLE . PageInfo::SUFFIX) {
40+
return '';
41+
}
42+
return basename($this->name, PageInfo::SUFFIX);
43+
}
44+
45+
/**
46+
* Check if this is an index page (Readme.md)
47+
*/
48+
public function isIndexPage(): bool {
49+
return $this->name === PageInfo::INDEX_PAGE_TITLE . PageInfo::SUFFIX;
50+
}
51+
52+
/**
53+
* Check if this is a page (.md file)
54+
*/
55+
public function isPage(): bool {
56+
return str_ends_with($this->name, PageInfo::SUFFIX);
57+
}
58+
}

0 commit comments

Comments
 (0)