Skip to content

Perf: Optimize pages loading#2390

Draft
Koc wants to merge 3 commits intomainfrom
feature/optimize-pages-loading
Draft

Perf: Optimize pages loading#2390
Koc wants to merge 3 commits intomainfrom
feature/optimize-pages-loading

Conversation

@Koc
Copy link
Copy Markdown
Contributor

@Koc Koc commented Mar 31, 2026 •
edited
Loading

📝 Summary

🚧 Work in progress

🖼️ Screenshots

🏚️ Before 🏡 After
B A

🚧 TODO

  • Migration ✔️ ❓
    • Do we want create composite index collective_id, trash_timestamp instead of just collective_id ❓
    • Create new column ✔️
    • Populate new column for already existent pages ✔️
  • Operations via Collective ✔️
    • Create new collective -> collectiveId set for root page ✔️
    • Create new page -> collectiveId set for created page ✔️
    • Copy page to another collective -> new collectiveId set for copied page ✔️
    • Move page to another collective -> new collectiveId set for moved page ✔️
  • Operations via Files 🚧
    • Create new file -> a new page with collectiveId is created ✔️
    • Copy file to another collective -> a new page with new collectiveId and slug is created ✔️
    • Move file to another collective -> collectiveId is updated ✔️
    • Move file to another collective with file renaming -> collectiveId and slug are updated ⏳
    • Move file to non-collective directory -> page should be removed 🔴 not works for some reason
    • Delete file -> page should be removed ✔️
    • Restore file -> page should be recreated ❓ Should we trash/untrash pages in order to preserve id to prevent broken links
    • Directory creation -> create a new page with Readme.md -> not needed ❌
  • Collectives loading 🚧 ❓
    • I would like to get rid of pages creation at runtime at all. Methods for reading data shouldn't mutate anything. WDYT ❓
    • Load collective by shared link \OCA\Collectives\Controller\PublicPageController::index 🚧
    • Load collective by shared link filtered by page \OCA\Collectives\Controller\PublicPageController::index 🚧
    • Load collective \OCA\Collectives\Controller\PageController::index ⏳
    • Other places ❓

🏁 Checklist

  • Code is properly formatted (npm run lint / npm run stylelint / composer run cs:check)
  • Sign-off message is added to all commits
  • Tests (unit, integration and/or end-to-end) passing and the changes are covered with tests
  • Documentation (README or documentation) has been updated or is not required

@Koc Koc force-pushed the feature/optimize-pages-loading branch 3 times, most recently from 77be0ae to 235649a Compare March 31, 2026 20:43
@Koc Koc changed the title Perf: Optimize pages loading (closes #2380) Perf: Optimize pages loading Mar 31, 2026
@Koc Koc force-pushed the feature/optimize-pages-loading branch 5 times, most recently from bd18443 to 073c47b Compare March 31, 2026 22:58
@Koc Koc mentioned this pull request Mar 31, 2026
Open
mejo-
mejo- reviewed Apr 1, 2026
View reviewed changes
lib/Service/PageService.php
$this->getParentPageId($newFile),
$userId,
$this->userManager->getDisplayName($userId));
$slug = $title ? $this->slugger->slug($title)->toString() : null;
Copy link
Copy Markdown
Member

@mejo- mejo- Apr 1, 2026

Choose a reason for hiding this comment

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

I wonder whether we had the condition to not set slugs for the landing page.

Copy link
Copy Markdown
Member

@mejo- mejo- Apr 1, 2026

Choose a reason for hiding this comment

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

Ah sorry, just saw later that you moved the slug creation into updatePage(). Still better to check if it still works exected when updating the landing page.

mejo-
mejo- reviewed Apr 1, 2026
View reviewed changes
mejo-
mejo- reviewed Apr 1, 2026
View reviewed changes
psalm.xml Outdated
<referencedClass name="OCA\Richdocuments\Db\WopiMapper" />
</errorLevel>
</UndefinedClass>
<UndefinedInterfaceMethod>
Copy link
Copy Markdown
Member

@mejo- mejo- Apr 1, 2026

Choose a reason for hiding this comment

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

Interesting, I wonder why this is necessary. You didn't add a call to Node::getContent() in your commit and it is part of the public API 🤔

mejo-
mejo- reviewed Apr 1, 2026
View reviewed changes
mejo-
mejo- reviewed Apr 1, 2026
View reviewed changes
@mejo-
Copy link
Copy Markdown
Member

mejo- commented Apr 1, 2026

Directory creation -> create a new page with Readme.md ❓

I'd say no, we don't want new directories to become "pages" automatically.

@mejo-
Copy link
Copy Markdown
Member

mejo- commented Apr 1, 2026

I would like to get rid of pages creation at runtime at all. Methods for reading data shouldn't mutate anything. WDYT ❓

Can you elaborate on that?

@Koc Koc force-pushed the feature/optimize-pages-loading branch 2 times, most recently from fa4618a to 1ab198f Compare April 1, 2026 21:36
Koc added 2 commits April 1, 2026 23:37
@Koc
Perf: Optimize pages loading (closes #2380)
68d820c 
Signed-off-by: Kostiantyn Miakshyn <molodchick@gmail.com>
@Koc
Perf: Optimize pages loading (add listeners)
3ffab03 
Signed-off-by: Kostiantyn Miakshyn <molodchick@gmail.com>
@Koc Koc force-pushed the feature/optimize-pages-loading branch 4 times, most recently from 6f7f9aa to 91142d7 Compare April 1, 2026 22:18
Koc
Koc commented Apr 1, 2026
View reviewed changes
lib/Listeners/NodeDeletedListener.php
return;
}

$this->pageMapper->deleteByFileId($node->getId());
Copy link
Copy Markdown
Contributor Author

@Koc Koc Apr 1, 2026

Choose a reason for hiding this comment

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

@mejo- should we hard delete page or just set trashed timestamp to use softdelete? I guess it's better to preserve page id in order to not break references

@Koc Koc force-pushed the feature/optimize-pages-loading branch 3 times, most recently from 203b325 to 52e8de1 Compare April 1, 2026 23:24
Koc
Koc commented Apr 1, 2026
View reviewed changes
lib/Migration/Version030401Date20250331000000.php
Comment on lines +34 to +35
if (!$table->hasIndex('collectives_pages_c_id_idx')) {
$table->addIndex(['collective_id'], 'collectives_pages_c_id_idx');
Copy link
Copy Markdown
Contributor Author

@Koc Koc Apr 1, 2026

Choose a reason for hiding this comment

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

@mejo- WDYT about adding composite collective_id, trash_timestamp instead of just collective_id?

@Koc Koc force-pushed the feature/optimize-pages-loading branch from 52e8de1 to 0c0aad0 Compare April 1, 2026 23:44
@Koc
Perf: Optimize pages loading (start work on method reading)
40cede4 
Signed-off-by: Kostiantyn Miakshyn <molodchick@gmail.com>
@Koc Koc force-pushed the feature/optimize-pages-loading branch from 0c0aad0 to 40cede4 Compare April 1, 2026 23:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mejo- mejo- mejo- left review comments

@max-nextcloud max-nextcloud Awaiting requested review from max-nextcloud max-nextcloud will be requested when the pull request is marked ready for review max-nextcloud is a code owner

@silverkszlo silverkszlo Awaiting requested review from silverkszlo silverkszlo will be requested when the pull request is marked ready for review silverkszlo is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Koc @mejo-