Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
13 changes: 13 additions & 0 deletions lib/Contracts/IMailManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,19 @@ public function getMailbox(string $uid, int $id): Mailbox;
*/
public function getMailboxes(Account $account, bool $forceSync = false): array;

/**
* Like getMailboxes(), but reads only the locally cached mailbox list --
* no live IMAP sync. Useful for callers that need to list an account's
* mailboxes quickly and don't need (or can't afford to wait for) an
* up-to-date view, e.g. a page bootstrap iterating over every connected
* account.
*
* @param Account $account
*
* @return Mailbox[]
*/
public function getCachedMailboxes(Account $account): array;

/**
* @param Account $account
* @param string $name
Expand Down
11 changes: 9 additions & 2 deletions lib/Controller/PageController.php
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,13 @@ public function index(): TemplateResponse {
$this->appManager->getAppVersion('mail'),
);

// Uses getCachedMailboxes() (DB read only) rather than getMailboxes()
// (which does a live IMAP sync first): this loop is synchronous
// across every connected account, so one account with a slow or
// unreachable IMAP server used to delay/block the whole page load
// for every other account too. The existing try/catch below only
// ever helped if the call threw quickly -- it did nothing for a
// hang, since a hang never reaches the catch block at all.
$mailAccounts = $this->accountService->findByUserId($this->userId);
$accountsJson = [];
foreach ($mailAccounts as $mailAccount) {
Expand All @@ -147,7 +154,7 @@ public function index(): TemplateResponse {
$json['aliases'] = $this->aliasesService->findAll($mailAccount->getId(),
$this->userId);
try {
$mailboxes = $this->mailManager->getMailboxes($mailAccount);
$mailboxes = $this->mailManager->getCachedMailboxes($mailAccount);
$json['mailboxes'] = $mailboxes;
} catch (Throwable $ex) {
$this->logger->critical('Could not load account mailboxes: ' . $ex->getMessage(), [
Expand All @@ -166,7 +173,7 @@ public function index(): TemplateResponse {
$json['aliases'] = $this->aliasesService->findAll($delegatedAccount->getId(),
$delegatedAccount->getUserId());
try {
$mailboxes = $this->mailManager->getMailboxes($delegatedAccount);
$mailboxes = $this->mailManager->getCachedMailboxes($delegatedAccount);
$json['mailboxes'] = $mailboxes;
} catch (Throwable $ex) {
$this->logger->critical('Could not load delegated account mailboxes: ' . $ex->getMessage(), [
Expand Down
5 changes: 5 additions & 0 deletions lib/Service/MailManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,11 @@ public function getMailboxes(Account $account, bool $forceSync = false): array {
return $this->mailboxMapper->findAll($account);
}

#[\Override]
public function getCachedMailboxes(Account $account): array {
return $this->mailboxMapper->findAll($account);
}

#[\Override]
public function createMailbox(Account $account, string $name, array $specialUse = []): Mailbox {
$client = $this->imapClientFactory->getClient($account);
Expand Down
2 changes: 1 addition & 1 deletion tests/Unit/Controller/PageControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ public function testIndex(): void {
->with($this->userId)
->willReturn([]);
$this->mailManager->expects($this->exactly(2))
->method('getMailboxes')
->method('getCachedMailboxes')
->withConsecutive(
[$account1],
[$account2]
Expand Down
19 changes: 19 additions & 0 deletions tests/Unit/Service/MailManagerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,25 @@ public function testGetFolders() {
$this->assertSame($mailboxes, $result);
}

public function testGetCachedMailboxesDoesNotSync() {
/** @var Account|MockObject $account */
$account = $this->createStub(Account::class);
$mailboxes = [
$this->createMock(Mailbox::class),
$this->createMock(Mailbox::class),
];
$this->mailboxSync->expects($this->never())
->method('sync');
$this->mailboxMapper->expects($this->once())
->method('findAll')
->with($this->equalTo($account))
->willReturn($mailboxes);

$result = $this->manager->getCachedMailboxes($account);

$this->assertSame($mailboxes, $result);
}

public function testCreateFolder() {
$client = $this->createStub(Horde_Imap_Client_Socket::class);
$account = $this->createStub(Account::class);
Expand Down