fix(page): use cached mailboxes for the initial bootstrap#13207
Draft
ktogias wants to merge 2 commits into
Draft
fix(page): use cached mailboxes for the initial bootstrap#13207ktogias wants to merge 2 commits into
ktogias wants to merge 2 commits into
Conversation
PageController::index() loops over every connected account synchronously in one request to build the initial page state, calling getMailboxes() for each -- which does a live IMAP sync before reading the cached list. One account with a slow or unreachable IMAP server was therefore enough to delay or block the whole bootstrap for every other account too, since this loop is synchronous. The existing try/catch only helps if the call throws quickly; it does nothing for a hang, since a hang never reaches the catch block. Adds getCachedMailboxes() to IMailManager/MailManager, a DB-only read with no live IMAP touch, and switches both account loops in PageController::index() to use it. The bootstrap's speed is now fully decoupled from any account's live IMAP health; periodic background sync jobs and opening an account's own mailbox view still do the live sync work and keep things up to date. Updated the existing PageControllerTest expectation to match, and added a unit test for the new method confirming it doesn't touch MailboxSync at all. Signed-off-by: Konstantinos Togias <info@ktogias.gr>
ChristophWurst
requested changes
Jul 3, 2026
ChristophWurst
left a comment
Member
There was a problem hiding this comment.
Thanks for the patch
This also appears to be the result of agentic AI. You need to disclose that and honor CONTRIBUTING.md and AGENTS.md
Explain in your words what this change fixes. Perhaps you can demo it with before/after data flows.
Author
|
You're right, and I'm sorry. I missed the guideline about the disclosure. Step by step, what this fixes:
Fix: add |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
PageController::index()loops over every connected account synchronously in a single request to build the initial page bootstrap state, callinggetMailboxes()for each one:getMailboxes()callsmailboxSync->sync()first (a live IMAP touch) before reading the cached list. One account with a slow-to-respond or temporarily unreachable IMAP server was therefore enough to delay/block the bootstrap for every other account too, since this loop is synchronous. The existingtry/catchonly helps if the call throws quickly — it does nothing for a hang, since the loop never reaches thecatchuntil the call actually returns or errors.Fix
Adds
getCachedMailboxes()toIMailManager/MailManager— a DB-only read with no live IMAP touch — and switches both account loops inPageController::index()(regular and delegated accounts) to use it. The bootstrap's speed is now fully decoupled from any account's live IMAP health. Periodic background sync jobs and opening an account's own mailbox view still do the live sync work and keep things up to date; the bootstrap just no longer needs to wait on it.Test plan
PageControllerTest::testIndex()mock expectation fromgetMailboxestogetCachedMailboxes.MailManagerTest::testGetCachedMailboxesDoesNotSync()confirming the new method never callsMailboxSync::sync().php -lclean on all changed files. I don't have a local PHP/Composer/PHPUnit environment to run the full suite in this session — happy to iterate if CI surfaces anything.