Skip to content

Commit

Permalink
Merge pull request #8494 from nextcloud/perf/imap/reduce-mailbox-stat…
Browse files Browse the repository at this point in the history 
…us-background-sync

perf(imap): Reduce number of STATUS commands for background mailbox sync
  • Loading branch information
@miaulalala
miaulalala authored Jul 10, 2023
2 parents 2c9f157 + f4b01d9 commit 0d77363
Show file tree
Hide file tree
Showing 7 changed files with 135 additions and 122 deletions.
2 changes: 1 addition & 1 deletion lib/Db/Mailbox.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ public function hasLocks(int $now): bool {
* @return MailboxStats
*/
public function getStats(): MailboxStats {
return new MailboxStats($this->getMessages(), $this->getUnseen(), $this->getMyAcls());
return new MailboxStats($this->getMessages(), $this->getUnseen());
}

#[ReturnTypeWillChange]
Expand Down
8 changes: 4 additions & 4 deletions lib/Folder.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ class Folder {
/** @var string */
private $delimiter;

/** @var array */
/** @var null|array */
private $status;

/** @var string[] */
Expand All @@ -50,7 +50,7 @@ public function __construct(int $accountId,
Horde_Imap_Client_Mailbox $mailbox,
array $attributes,
?string $delimiter,
array $status) {
?array $status) {
$this->accountId = $accountId;
$this->mailbox = $mailbox;
$this->attributes = $attributes;
Expand All @@ -76,9 +76,9 @@ public function getAttributes(): array {
}

/**
* @return array
* @return null|array
*/
public function getStatus(): array {
public function getStatus(): ?array {
return $this->status;
}

Expand Down
42 changes: 19 additions & 23 deletions lib/IMAP/FolderMapper.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,28 +59,25 @@ public function getFolders(Account $account, Horde_Imap_Client_Socket $client,
'delimiter' => true,
'attributes' => true,
'special_use' => true,
'status' => Horde_Imap_Client::STATUS_ALL,
]);

return array_filter(array_map(static function (array $mailbox) use ($account) {
$toPersist = array_filter($mailboxes, function (array $mailbox) {
$attributes = array_flip(array_map('strtolower', $mailbox['attributes']));
if (isset($attributes['\\nonexistent'])) {
// Ignore mailbox that does not exist, similar to \Horde_Imap_Client::MBOX_SUBSCRIBED_EXISTS mode
return null;
return false;
}
if (in_array($mailbox['mailbox']->utf8, self::DOVECOT_SIEVE_FOLDERS, true)) {
// This is a special folder that must not be shown
return null;
}

// This is a special folder that must not be shown
return !in_array($mailbox['mailbox']->utf8, self::DOVECOT_SIEVE_FOLDERS, true);
});
return array_map(static function (array $mailbox) use ($account) {
return new Folder(
$account->getId(),
$mailbox['mailbox'],
$mailbox['attributes'],
$mailbox['delimiter'],
$mailbox['status'],
null,
);
}, $mailboxes));
}, $toPersist);
}

public function createFolder(Horde_Imap_Client_Socket $client,
Expand Down Expand Up @@ -137,21 +134,20 @@ public function fetchFolderAcls(array $folders,
*
* @throws Horde_Imap_Client_Exception
*
* @return MailboxStats
* @return MailboxStats[]
*/
public function getFoldersStatusAsObject(Horde_Imap_Client_Socket $client,
string $mailbox): MailboxStats {
$status = $client->status($mailbox);
$acls = null;
if ($client->capability->query('ACL')) {
$acls = (string)$client->getMyACLRights($mailbox);
}
array $mailboxes): array {
$multiStatus = $client->status($mailboxes);

return new MailboxStats(
$status['messages'],
$status['unseen'],
$acls
);
$statuses = [];
foreach ($multiStatus as $mailbox => $status) {
$statuses[$mailbox] = new MailboxStats(
$status['messages'],
$status['unseen'],
);
}
return $statuses;
}

/**
Expand Down
9 changes: 1 addition & 8 deletions lib/IMAP/MailboxStats.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,10 @@
class MailboxStats implements JsonSerializable {
private int $total;
private int $unread;
private ?string $myAcls;

public function __construct(int $total, int $unread, ?string $myAcls) {
public function __construct(int $total, int $unread) {
$this->total = $total;
$this->unread = $unread;
$this->myAcls = $myAcls;
}

public function getTotal(): int {
Expand All @@ -48,16 +46,11 @@ public function getUnread(): int {
return $this->unread;
}

public function getMyAcls(): ?string {
return $this->myAcls;
}

#[ReturnTypeWillChange]
public function jsonSerialize() {
return [
'total' => $this->total,
'unread' => $this->unread,
'myAcls' => $this->myAcls,
];
}
}
87 changes: 70 additions & 17 deletions lib/IMAP/MailboxSync.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,11 @@
use Psr\Log\LoggerInterface;
use function array_combine;
use function array_map;
use function array_reduce;
use function array_slice;
use function in_array;
use function json_encode;
use function shuffle;
use function sprintf;
use function str_starts_with;

Expand Down Expand Up @@ -103,14 +106,16 @@ public function sync(Account $account,
$namespaces = $client->getNamespaces([], [
'ob_return' => true,
]);
$personalNamespace = $this->getPersonalNamespace($namespaces);
$account->getMailAccount()->setPersonalNamespace(
$this->getPersonalNamespace($namespaces)
$personalNamespace
);
} catch (Horde_Imap_Client_Exception $e) {
$logger->debug('Getting namespaces for account ' . $account->getId() . ' failed: ' . $e->getMessage(), [
'exception' => $e,
]);
$namespaces = null;
$personalNamespace = null;
}

try {
Expand All @@ -125,7 +130,7 @@ public function sync(Account $account,
}
$this->folderMapper->detectFolderSpecialUse($folders);

$this->atomic(function () use ($account, $folders, $namespaces) {
$mailboxes = $this->atomic(function () use ($account, $folders, $namespaces) {
$old = $this->mailboxMapper->findAll($account);
$indexedOld = array_combine(
array_map(static function (Mailbox $mb) {
Expand All @@ -134,9 +139,11 @@ public function sync(Account $account,
$old
);

$this->persist($account, $folders, $indexedOld, $namespaces);
return $this->persist($account, $folders, $indexedOld, $namespaces);
}, $this->dbConnection);

$this->syncMailboxStatus($mailboxes, $personalNamespace, $client);

$this->dispatcher->dispatchTyped(
new MailboxesSynchronizedEvent($account)
);
Expand All @@ -156,7 +163,7 @@ public function sync(Account $account,
public function syncStats(Account $account, Mailbox $mailbox): void {
$client = $this->imapClientFactory->getClient($account);
try {
$stats = $this->folderMapper->getFoldersStatusAsObject($client, $mailbox->getName());
$stats = $this->folderMapper->getFoldersStatusAsObject($client, [$mailbox->getName()])[$mailbox->getName()];
} catch (Horde_Imap_Client_Exception $e) {
$id = $mailbox->getId();
throw new ServiceException(
Expand All @@ -173,19 +180,23 @@ public function syncStats(Account $account, Mailbox $mailbox): void {
$this->mailboxMapper->update($mailbox);
}

/**
* @return Mailbox[]
*/
private function persist(Account $account,
array $folders,
array $existing,
?Horde_Imap_Client_Namespace_List $namespaces): void {
?Horde_Imap_Client_Namespace_List $namespaces): array {
$mailboxes = [];
foreach ($folders as $folder) {
if (isset($existing[$folder->getMailbox()])) {
$this->updateMailboxFromFolder(
$mailboxes[] = $this->updateMailboxFromFolder(
$folder,
$existing[$folder->getMailbox()],
$namespaces,
);
} else {
$this->createMailboxFromFolder(
$mailboxes[] = $this->createMailboxFromFolder(
$account,
$folder,
$namespaces,
Expand All @@ -201,44 +212,49 @@ private function persist(Account $account,

$account->getMailAccount()->setLastMailboxSync($this->timeFactory->getTime());
$this->mailAccountMapper->update($account->getMailAccount());

return $mailboxes;
}

private function getPersonalNamespace(Horde_Imap_Client_Namespace_List $namespaces): ?string {
foreach ($namespaces as $namespace) {
/** @var Horde_Imap_Client_Data_Namespace $namespace */
if ($namespace->type === Horde_Imap_Client::NS_PERSONAL) {
return $namespace->name;
return $namespace->name !== "" ? $namespace->name : null;
}
$namespace->name;
}
return null;
}

private function updateMailboxFromFolder(Folder $folder, Mailbox $mailbox, ?Horde_Imap_Client_Namespace_List $namespaces): void {
private function updateMailboxFromFolder(Folder $folder, Mailbox $mailbox, ?Horde_Imap_Client_Namespace_List $namespaces): Mailbox {
$mailbox->setAttributes(json_encode($folder->getAttributes()));
$mailbox->setDelimiter($folder->getDelimiter());
$mailbox->setMessages($folder->getStatus()['messages'] ?? 0);
$mailbox->setUnseen($folder->getStatus()['unseen'] ?? 0);
$status = $folder->getStatus();
if ($status !== null) {
$mailbox->setMessages($status['messages'] ?? 0);
$mailbox->setUnseen($status['unseen'] ?? 0);
}
$mailbox->setSelectable(!in_array('\noselect', $folder->getAttributes()));
$mailbox->setSpecialUse(json_encode($folder->getSpecialUse()));
$mailbox->setMyAcls($folder->getMyAcls());
$mailbox->setShared($this->isMailboxShared($namespaces, $mailbox));
$this->mailboxMapper->update($mailbox);
return $this->mailboxMapper->update($mailbox);
}

private function createMailboxFromFolder(Account $account, Folder $folder, ?Horde_Imap_Client_Namespace_List $namespaces): void {
private function createMailboxFromFolder(Account $account, Folder $folder, ?Horde_Imap_Client_Namespace_List $namespaces): Mailbox {
$mailbox = new Mailbox();
$mailbox->setName($folder->getMailbox());
$mailbox->setAccountId($account->getId());
$mailbox->setAttributes(json_encode($folder->getAttributes()));
$mailbox->setDelimiter($folder->getDelimiter());
$mailbox->setMessages($folder->getStatus()['messages'] ?? 0);
$mailbox->setUnseen($folder->getStatus()['unseen'] ?? 0);
$status = $folder->getStatus();
$mailbox->setMessages($status['messages'] ?? 0);
$mailbox->setUnseen($status['unseen'] ?? 0);
$mailbox->setSelectable(!in_array('\noselect', $folder->getAttributes()));
$mailbox->setSpecialUse(json_encode($folder->getSpecialUse()));
$mailbox->setMyAcls($folder->getMyAcls());
$mailbox->setShared($this->isMailboxShared($namespaces, $mailbox));
$this->mailboxMapper->insert($mailbox);
return $this->mailboxMapper->insert($mailbox);
}

private function isMailboxShared(?Horde_Imap_Client_Namespace_List $namespaces, Mailbox $mailbox): bool {
Expand All @@ -250,4 +266,41 @@ private function isMailboxShared(?Horde_Imap_Client_Namespace_List $namespaces,
}
return false;
}

private function syncMailboxStatus(mixed $mailboxes, ?string $personalNamespace, \Horde_Imap_Client_Socket $client): void {
/** @var array{0: Mailbox[], 1: Mailbox[]} */
[$sync, $doNotSync] = array_reduce($mailboxes, function (array $carry, Mailbox $mailbox) use ($personalNamespace): array {
/** @var array{0: Mailbox[], 1: Mailbox[]} $carry */
[$sync, $doNotSync] = $carry;
$inboxName = $personalNamespace === null ? "INBOX" : ($personalNamespace . $mailbox->getDelimiter() . "INBOX");
if ($inboxName === $mailbox->getName() || $mailbox->getSyncInBackground()) {
return [
array_merge($sync, [$mailbox]),
$doNotSync,
];
}
return [
$sync,
array_merge($doNotSync, [$mailbox]),
];
}, [[], []]);

// Synchronize the mailboxes selected for sync and keep the rest updated occasionally
shuffle($doNotSync);
/** @var Mailbox[] $syncStatus */
$syncStatus = [...$sync, ...array_slice($doNotSync, 0, 5)];
$statuses = $this->folderMapper->getFoldersStatusAsObject($client, array_map(function (Mailbox $mailbox) {
return $mailbox->getName();
}, $syncStatus));
foreach ($syncStatus as $mailbox) {
$status = $statuses[$mailbox->getName()];
$mailbox->setMessages($status->getTotal());
$mailbox->setUnseen($status->getUnread());
}
$this->atomic(function () use ($syncStatus) {
foreach ($syncStatus as $mailbox) {
$this->mailboxMapper->update($mailbox);
}
}, $this->dbConnection);
}
}
Loading

0 comments on commit 0d77363

Please sign in to comment.