From ca3d6ce95726e1ee2c93656ef5788d6f9d55a45b Mon Sep 17 00:00:00 2001 From: Richard Steinmetz Date: Tue, 10 Sep 2024 10:04:15 +0200 Subject: [PATCH] feat: implement periodic full sync job to repair cache inconsistencies Signed-off-by: Richard Steinmetz --- lib/BackgroundJob/SyncRepairJob.php | 90 ++++ lib/Controller/MailboxesController.php | 14 + lib/IMAP/IMAPClientFactory.php | 12 - lib/Migration/FixBackgroundJobs.php | 2 + lib/Service/Sync/ImapToDbSynchronizer.php | 52 +- .../Sync/ImapToDbSynchronizer.php.orig | 488 ++++++++++++++++++ lib/Service/Sync/SyncService.php | 12 + src/components/NavigationMailbox.vue | 27 +- src/service/MailboxService.js | 8 + .../Sync/ImapToDbSynchronizerTest.php | 158 ++++++ 10 files changed, 849 insertions(+), 14 deletions(-) create mode 100644 lib/BackgroundJob/SyncRepairJob.php create mode 100644 lib/Service/Sync/ImapToDbSynchronizer.php.orig create mode 100644 tests/Integration/Sync/ImapToDbSynchronizerTest.php diff --git a/lib/BackgroundJob/SyncRepairJob.php b/lib/BackgroundJob/SyncRepairJob.php new file mode 100644 index 0000000000..fc604042c3 --- /dev/null +++ b/lib/BackgroundJob/SyncRepairJob.php @@ -0,0 +1,90 @@ +setInterval(3600 * 24 * 7); + $this->setTimeSensitivity(self::TIME_INSENSITIVE); + } + + protected function run($argument): void { + $accountId = (int)$argument['accountId']; + + try { + $account = $this->accountService->findById($accountId); + } catch (DoesNotExistException $e) { + $this->logger->debug('Could not find account <' . $accountId . '> removing from jobs'); + $this->jobList->remove(self::class, $argument); + return; + } + + if(!$account->getMailAccount()->canAuthenticateImap()) { + $this->logger->debug('No authentication on IMAP possible, skipping background sync job'); + return; + } + + $user = $this->userManager->get($account->getUserId()); + if ($user === null || !$user->isEnabled()) { + $this->logger->debug(sprintf( + 'Account %d of user %s could not be found or was disabled, skipping background sync', + $account->getId(), + $account->getUserId() + )); + return; + } + + $rebuildThreads = false; + $trashMailboxId = $account->getMailAccount()->getTrashMailboxId(); + $snoozeMailboxId = $account->getMailAccount()->getSnoozeMailboxId(); + $sentMailboxId = $account->getMailAccount()->getSentMailboxId(); + $junkMailboxId = $account->getMailAccount()->getJunkMailboxId(); + foreach ($this->mailboxMapper->findAll($account) as $mailbox) { + $isTrash = $trashMailboxId === $mailbox->getId(); + $isSnooze = $snoozeMailboxId === $mailbox->getId(); + $isSent = $sentMailboxId === $mailbox->getId(); + $isJunk = $junkMailboxId === $mailbox->getId(); + if ($isTrash || $isSnooze || $isSent || $isJunk) { + continue; + } + + if ($this->syncService->repairSync($account, $mailbox) > 0) { + $rebuildThreads = true; + } + } + + $this->dispatcher->dispatchTyped( + new SynchronizationEvent($account, $this->logger, $rebuildThreads), + ); + } +} diff --git a/lib/Controller/MailboxesController.php b/lib/Controller/MailboxesController.php index e45cb49d4b..11d3fc5f8f 100644 --- a/lib/Controller/MailboxesController.php +++ b/lib/Controller/MailboxesController.php @@ -23,6 +23,7 @@ use OCA\Mail\Service\Sync\SyncService; use OCP\AppFramework\Controller; use OCP\AppFramework\Http; +use OCP\AppFramework\Http\Attribute\NoAdminRequired; use OCP\AppFramework\Http\Attribute\OpenAPI; use OCP\AppFramework\Http\JSONResponse; use OCP\IRequest; @@ -286,4 +287,17 @@ public function clearMailbox(int $id): JSONResponse { $this->mailManager->clearMailbox($account, $mailbox); return new JSONResponse(); } + + /** + * Delete all vanished mails that are still cached. + */ + #[TrapError] + #[NoAdminRequired] + public function repair(int $id): JSONResponse { + $mailbox = $this->mailManager->getMailbox($this->currentUserId, $id); + $account = $this->accountService->find($this->currentUserId, $mailbox->getAccountId()); + + $this->syncService->repairSync($account, $mailbox); + return new JsonResponse(); + } } diff --git a/lib/IMAP/IMAPClientFactory.php b/lib/IMAP/IMAPClientFactory.php index 1560228397..c6ca0b5565 100644 --- a/lib/IMAP/IMAPClientFactory.php +++ b/lib/IMAP/IMAPClientFactory.php @@ -121,18 +121,6 @@ public function getClient(Account $account, bool $useCache = true): Horde_Imap_C $this->cacheFactory->createDistributed(md5((string)$account->getId())), ), ]; - } else { - // WARNING: This is very dangerous! We **will** miss changes when using QRESYNC without - // actually persisting changes to the cache. Especially vanished messages will - // be missed. - /** - * If we don't use a cache we use a null cache to trick Horde into - * using QRESYNC/CONDSTORE if they are available - * @see \Horde_Imap_Client_Socket::_loginTasks - */ - $params['cache'] = [ - 'backend' => new Horde_Imap_Client_Cache_Backend_Null(), - ]; } if ($this->config->getSystemValue('debug', false)) { $params['debug'] = $this->config->getSystemValue('datadirectory') . '/horde_imap.log'; diff --git a/lib/Migration/FixBackgroundJobs.php b/lib/Migration/FixBackgroundJobs.php index 91621e300c..d134d9c971 100644 --- a/lib/Migration/FixBackgroundJobs.php +++ b/lib/Migration/FixBackgroundJobs.php @@ -11,6 +11,7 @@ use OCA\Mail\BackgroundJob\PreviewEnhancementProcessingJob; use OCA\Mail\BackgroundJob\QuotaJob; use OCA\Mail\BackgroundJob\SyncJob; +use OCA\Mail\BackgroundJob\SyncRepairJob; use OCA\Mail\BackgroundJob\TrainImportanceClassifierJob; use OCA\Mail\Db\MailAccount; use OCA\Mail\Db\MailAccountMapper; @@ -43,6 +44,7 @@ public function run(IOutput $output) { $output->startProgress(count($accounts)); foreach ($accounts as $account) { $this->jobList->add(SyncJob::class, ['accountId' => $account->getId()]); + $this->jobList->add(SyncRepairJob::class, ['accountId' => $account->getId()]); $this->jobList->add(TrainImportanceClassifierJob::class, ['accountId' => $account->getId()]); $this->jobList->add(PreviewEnhancementProcessingJob::class, ['accountId' => $account->getId()]); $this->jobList->add(QuotaJob::class, ['accountId' => $account->getId()]); diff --git a/lib/Service/Sync/ImapToDbSynchronizer.php b/lib/Service/Sync/ImapToDbSynchronizer.php index 68f35c5d49..8c3b751686 100644 --- a/lib/Service/Sync/ImapToDbSynchronizer.php +++ b/lib/Service/Sync/ImapToDbSynchronizer.php @@ -12,6 +12,7 @@ use Horde_Imap_Client; use Horde_Imap_Client_Base; use Horde_Imap_Client_Exception; +use Horde_Imap_Client_Ids; use OCA\Mail\Account; use OCA\Mail\Contracts\IMailManager; use OCA\Mail\Db\Mailbox; @@ -42,7 +43,7 @@ class ImapToDbSynchronizer { /** @var int */ - public const MAX_NEW_MESSAGES = 5000; + public const MAX_NEW_MESSAGES = 5000000; /** @var DatabaseMessageMapper */ private $dbMapper; @@ -485,4 +486,53 @@ private function runPartialSync( return $newOrVanished; } + + /** + * Run a (rather costly) sync to delete cached messages which are not present on IMAP anymore. + * + * @return int Number of detected vanished phantom messages + * + * @throws MailboxLockedException + * @throws ServiceException + */ + public function repairSync( + Account $account, + Mailbox $mailbox, + LoggerInterface $logger, + ): int { + $this->mailboxMapper->lockForVanishedSync($mailbox); + + $perf = $this->performanceLogger->startWithLogger( + 'Repair sync for ' . $account->getId() . ':' . $mailbox->getName(), + $logger, + ); + + // Need to use a client without a cache here (to disable QRESYNC entirely) + $client = $this->clientFactory->getClient($account, false); + try { + $knownUids = $this->dbMapper->findAllUids($mailbox); + $hordeMailbox = new \Horde_Imap_Client_Mailbox($mailbox->getName()); + $phantomVanishedUids = $client->vanished($hordeMailbox, 0, [ + 'ids' => new Horde_Imap_Client_Ids($knownUids), + ]); + if (count($phantomVanishedUids) > 0) { + $this->dbMapper->deleteByUid($mailbox, ...$phantomVanishedUids); + } + } catch (Throwable $e) { + $message = sprintf( + 'Repair sync failed for %d:%s: %s', + $account->getId(), + $mailbox->getName(), + $e->getMessage(), + ); + throw new ServiceException($message, 0, $e); + } finally { + $this->mailboxMapper->unlockFromVanishedSync($mailbox); + $client->logout(); + } + + $perf->end(); + + return count($phantomVanishedUids); + } } diff --git a/lib/Service/Sync/ImapToDbSynchronizer.php.orig b/lib/Service/Sync/ImapToDbSynchronizer.php.orig new file mode 100644 index 0000000000..68f35c5d49 --- /dev/null +++ b/lib/Service/Sync/ImapToDbSynchronizer.php.orig @@ -0,0 +1,488 @@ +dbMapper = $dbMapper; + $this->clientFactory = $clientFactory; + $this->imapMapper = $imapMapper; + $this->mailboxMapper = $mailboxMapper; + $this->messageMapper = $messageMapper; + $this->synchronizer = $synchronizer; + $this->dispatcher = $dispatcher; + $this->performanceLogger = $performanceLogger; + $this->logger = $logger; + $this->mailManager = $mailManager; + } + + /** + * @throws ClientException + * @throws ServiceException + */ + public function syncAccount(Account $account, + LoggerInterface $logger, + bool $force = false, + int $criteria = Horde_Imap_Client::SYNC_NEWMSGSUIDS | Horde_Imap_Client::SYNC_FLAGSUIDS | Horde_Imap_Client::SYNC_VANISHEDUIDS): void { + $rebuildThreads = false; + $trashMailboxId = $account->getMailAccount()->getTrashMailboxId(); + $snoozeMailboxId = $account->getMailAccount()->getSnoozeMailboxId(); + $sentMailboxId = $account->getMailAccount()->getSentMailboxId(); + $trashRetentionDays = $account->getMailAccount()->getTrashRetentionDays(); + foreach ($this->mailboxMapper->findAll($account) as $mailbox) { + $syncTrash = $trashMailboxId === $mailbox->getId() && $trashRetentionDays !== null; + $syncSnooze = $snoozeMailboxId === $mailbox->getId(); + $syncSent = $sentMailboxId === $mailbox->getId() || $mailbox->isSpecialUse('sent'); + + if (!$syncTrash && !$mailbox->isInbox() && !$syncSnooze && !$mailbox->getSyncInBackground() && !$syncSent) { + $logger->debug('Skipping mailbox sync for ' . $mailbox->getId()); + continue; + } + $logger->debug('Syncing ' . $mailbox->getId()); + if ($this->sync( + $account, + $mailbox, + $logger, + $criteria, + null, + $force, + true + )) { + $rebuildThreads = true; + } + } + $this->dispatcher->dispatchTyped( + new SynchronizationEvent( + $account, + $logger, + $rebuildThreads, + ) + ); + } + + /** + * Clear all cached data of a mailbox + * + * @param Account $account + * @param Mailbox $mailbox + * + * @throws MailboxLockedException + * @throws ServiceException + */ + public function clearCache(Account $account, + Mailbox $mailbox): void { + $id = $account->getId() . ':' . $mailbox->getName(); + try { + $this->mailboxMapper->lockForNewSync($mailbox); + $this->mailboxMapper->lockForChangeSync($mailbox); + $this->mailboxMapper->lockForVanishedSync($mailbox); + + $this->resetCache($account, $mailbox); + } catch (Throwable $e) { + throw new ServiceException("Could not clear mailbox cache for $id: " . $e->getMessage(), 0, $e); + } finally { + $this->mailboxMapper->unlockFromNewSync($mailbox); + $this->mailboxMapper->unlockFromChangedSync($mailbox); + $this->mailboxMapper->unlockFromVanishedSync($mailbox); + } + } + + /** + * Wipe all cached messages of a mailbox from the database + * + * Warning: the caller has to ensure the mailbox is locked + * + * @param Account $account + * @param Mailbox $mailbox + */ + private function resetCache(Account $account, Mailbox $mailbox): void { + $id = $account->getId() . ':' . $mailbox->getName(); + $this->messageMapper->deleteAll($mailbox); + $this->logger->debug("All messages of $id cleared"); + $mailbox->setSyncNewToken(null); + $mailbox->setSyncChangedToken(null); + $mailbox->setSyncVanishedToken(null); + $this->mailboxMapper->update($mailbox); + } + + /** + * @throws ClientException + * @throws MailboxNotCachedException + * @throws ServiceException + * @return bool whether to rebuild threads or not + */ + public function sync(Account $account, + Mailbox $mailbox, + LoggerInterface $logger, + int $criteria = Horde_Imap_Client::SYNC_NEWMSGSUIDS | Horde_Imap_Client::SYNC_FLAGSUIDS | Horde_Imap_Client::SYNC_VANISHEDUIDS, + ?array $knownUids = null, + bool $force = false, + bool $batchSync = false): bool { + $rebuildThreads = true; + if ($mailbox->getSelectable() === false) { + return $rebuildThreads; + } + + $client = $this->clientFactory->getClient($account); + $client->login(); // Need to login before fetching capabilities. + + // There is no partial sync when using QRESYNC. As per RFC the client will always pull + // all changes. This is a cheap operation when using QRESYNC as the server keeps track + // of a client's state through the sync token. We could just update the sync tokens and + // call it a day because Horde caches unrelated/unrequested changes until the next + // operation. However, our cache is not reliable as some instance might use APCu which + // isn't shared between cron and web requests. + if ($client->capability->isEnabled('QRESYNC')) { + $this->logger->debug('Forcing full sync due to QRESYNC'); + $criteria |= Horde_Imap_Client::SYNC_NEWMSGSUIDS + | Horde_Imap_Client::SYNC_FLAGSUIDS + | Horde_Imap_Client::SYNC_VANISHEDUIDS; + } + + if ($force || ($criteria & Horde_Imap_Client::SYNC_NEWMSGSUIDS)) { + $logger->debug('Locking mailbox ' . $mailbox->getId() . ' for new messages sync'); + $this->mailboxMapper->lockForNewSync($mailbox); + } + if ($force || ($criteria & Horde_Imap_Client::SYNC_FLAGSUIDS)) { + $logger->debug('Locking mailbox ' . $mailbox->getId() . ' for changed messages sync'); + $this->mailboxMapper->lockForChangeSync($mailbox); + } + if ($force || ($criteria & Horde_Imap_Client::SYNC_VANISHEDUIDS)) { + $logger->debug('Locking mailbox ' . $mailbox->getId() . ' for vanished messages sync'); + $this->mailboxMapper->lockForVanishedSync($mailbox); + } + + try { + if ($force + || $mailbox->getSyncNewToken() === null + || $mailbox->getSyncChangedToken() === null + || $mailbox->getSyncVanishedToken() === null) { + $logger->debug('Running initial sync for ' . $mailbox->getId()); + $this->runInitialSync($client, $account, $mailbox, $logger); + } else { + try { + $logger->debug('Running partial sync for ' . $mailbox->getId()); + // Only rebuild threads if there were new or vanished messages + $rebuildThreads = $this->runPartialSync($client, $account, $mailbox, $logger, $criteria, $knownUids); + } catch (UidValidityChangedException $e) { + $logger->warning('Mailbox UID validity changed. Wiping cache and performing full sync for ' . $mailbox->getId()); + $this->resetCache($account, $mailbox); + $logger->debug('Running initial sync for ' . $mailbox->getId() . ' after cache reset'); + $this->runInitialSync($client, $account, $mailbox, $logger); + } catch (MailboxDoesNotSupportModSequencesException $e) { + $logger->warning('Mailbox does not support mod-sequences error occured. Wiping cache and performing full sync for ' . $mailbox->getId(), [ + 'exception' => $e, + ]); + $this->resetCache($account, $mailbox); + $logger->debug('Running initial sync for ' . $mailbox->getId() . ' after cache reset - no mod-sequences error'); + $this->runInitialSync($client, $account, $mailbox, $logger); + } + } + } catch (ServiceException $e) { + // Just rethrow, don't wrap into another exception + throw $e; + } catch (Throwable $e) { + throw new ServiceException('Sync failed for ' . $account->getId() . ':' . $mailbox->getName() . ': ' . $e->getMessage(), 0, $e); + } finally { + if ($force || ($criteria & Horde_Imap_Client::SYNC_VANISHEDUIDS)) { + $logger->debug('Unlocking mailbox ' . $mailbox->getId() . ' from vanished messages sync'); + $this->mailboxMapper->unlockFromVanishedSync($mailbox); + } + if ($force || ($criteria & Horde_Imap_Client::SYNC_FLAGSUIDS)) { + $logger->debug('Unlocking mailbox ' . $mailbox->getId() . ' from changed messages sync'); + $this->mailboxMapper->unlockFromChangedSync($mailbox); + } + if ($force || ($criteria & Horde_Imap_Client::SYNC_NEWMSGSUIDS)) { + $logger->debug('Unlocking mailbox ' . $mailbox->getId() . ' from new messages sync'); + $this->mailboxMapper->unlockFromNewSync($mailbox); + } + + $client->logout(); + } + + if (!$batchSync) { + $this->dispatcher->dispatchTyped( + new SynchronizationEvent( + $account, + $this->logger, + $rebuildThreads, + ) + ); + } + + return $rebuildThreads; + } + + /** + * @throws ServiceException + * @throws IncompleteSyncException + */ + private function runInitialSync( + Horde_Imap_Client_Base $client, + Account $account, + Mailbox $mailbox, + LoggerInterface $logger): void { + $perf = $this->performanceLogger->startWithLogger( + 'Initial sync ' . $account->getId() . ':' . $mailbox->getName(), + $logger + ); + + // Need a client without a cache + $client->logout(); + $client = $this->clientFactory->getClient($account, false); + + $highestKnownUid = $this->dbMapper->findHighestUid($mailbox); + try { + $imapMessages = $this->imapMapper->findAll( + $client, + $mailbox->getName(), + self::MAX_NEW_MESSAGES, + $highestKnownUid ?? 0, + $logger, + $perf, + $account->getUserId(), + ); + $perf->step(sprintf('fetch %d messages from IMAP', count($imapMessages))); + } catch (Horde_Imap_Client_Exception $e) { + throw new ServiceException('Can not get messages from mailbox ' . $mailbox->getName() . ': ' . $e->getMessage(), 0, $e); + } + + foreach (array_chunk($imapMessages['messages'], 500) as $chunk) { + $messages = array_map(static function (IMAPMessage $imapMessage) use ($mailbox, $account) { + return $imapMessage->toDbMessage($mailbox->getId(), $account->getMailAccount()); + }, $chunk); + $this->dbMapper->insertBulk($account, ...$messages); + $perf->step(sprintf('persist %d messages in database', count($chunk))); + // Free the memory + unset($messages); + } + + if (!$imapMessages['all']) { + // We might need more attempts to fill the cache + $loggingMailboxId = $account->getId() . ':' . $mailbox->getName(); + $total = $imapMessages['total']; + $cached = count($this->messageMapper->findAllUids($mailbox)); + $perf->step('find number of cached UIDs'); + + $perf->end(); + throw new IncompleteSyncException("Initial sync is not complete for $loggingMailboxId ($cached of $total messages cached)."); + } + + $mailbox->setSyncNewToken($client->getSyncToken($mailbox->getName())); + $mailbox->setSyncChangedToken($client->getSyncToken($mailbox->getName())); + $mailbox->setSyncVanishedToken($client->getSyncToken($mailbox->getName())); + $this->mailboxMapper->update($mailbox); + + $perf->end(); + } + + /** + * @param int[] $knownUids + * + * @throws ServiceException + * @throws UidValidityChangedException + * @return bool whether there are new or vanished messages + */ + private function runPartialSync( + Horde_Imap_Client_Base $client, + Account $account, + Mailbox $mailbox, + LoggerInterface $logger, + int $criteria, + ?array $knownUids = null): bool { + $newOrVanished = false; + $perf = $this->performanceLogger->startWithLogger( + 'partial sync ' . $account->getId() . ':' . $mailbox->getName(), + $logger + ); + + $uids = $knownUids ?? $this->dbMapper->findAllUids($mailbox); + $perf->step('get all known UIDs'); + + if ($criteria & Horde_Imap_Client::SYNC_NEWMSGSUIDS) { + $response = $this->synchronizer->sync( + $client, + new Request( + $mailbox->getName(), + $mailbox->getSyncNewToken(), + $uids + ), + $account->getUserId(), + Horde_Imap_Client::SYNC_NEWMSGSUIDS + ); + $perf->step('get new messages via Horde'); + + $highestKnownUid = $this->dbMapper->findHighestUid($mailbox); + if ($highestKnownUid === null) { + // Everything is relevant + $newMessages = $response->getNewMessages(); + } else { + // Filter out anything that is already in the DB. Ideally this never happens, but if there is an error + // during a consecutive chunk INSERT, the sync token won't be updated. In that case the same message(s) + // will be seen as *new* and therefore cause conflicts. + $newMessages = array_filter($response->getNewMessages(), static function (IMAPMessage $imapMessage) use ($highestKnownUid) { + return $imapMessage->getUid() > $highestKnownUid; + }); + } + + foreach (array_chunk($newMessages, 500) as $chunk) { + $dbMessages = array_map(static function (IMAPMessage $imapMessage) use ($mailbox, $account) { + return $imapMessage->toDbMessage($mailbox->getId(), $account->getMailAccount()); + }, $chunk); + + $this->dbMapper->insertBulk($account, ...$dbMessages); + + $this->dispatcher->dispatch( + NewMessagesSynchronized::class, + new NewMessagesSynchronized($account, $mailbox, $dbMessages) + ); + $perf->step('classified a chunk of new messages'); + } + $perf->step('persist new messages'); + + $mailbox->setSyncNewToken($client->getSyncToken($mailbox->getName())); + $newOrVanished = $newMessages !== []; + } + if ($criteria & Horde_Imap_Client::SYNC_FLAGSUIDS) { + $response = $this->synchronizer->sync( + $client, + new Request( + $mailbox->getName(), + $mailbox->getSyncChangedToken(), + $uids + ), + $account->getUserId(), + Horde_Imap_Client::SYNC_FLAGSUIDS + ); + $perf->step('get changed messages via Horde'); + + $permflagsEnabled = $this->mailManager->isPermflagsEnabled($client, $account, $mailbox->getName()); + + foreach (array_chunk($response->getChangedMessages(), 500) as $chunk) { + $this->dbMapper->updateBulk($account, $permflagsEnabled, ...array_map(static function (IMAPMessage $imapMessage) use ($mailbox, $account) { + return $imapMessage->toDbMessage($mailbox->getId(), $account->getMailAccount()); + }, $chunk)); + } + $perf->step('persist changed messages'); + + // If a list of UIDs was *provided* (as opposed to loaded from the DB, + // we can not assume that all changes were detected, hence this is kinda + // a silent sync and we don't update the change token until the next full + // mailbox sync + if ($knownUids === null) { + $mailbox->setSyncChangedToken($client->getSyncToken($mailbox->getName())); + } + } + if ($criteria & Horde_Imap_Client::SYNC_VANISHEDUIDS) { + $response = $this->synchronizer->sync( + $client, + new Request( + $mailbox->getName(), + $mailbox->getSyncVanishedToken(), + $uids + ), + $account->getUserId(), + Horde_Imap_Client::SYNC_VANISHEDUIDS + ); + $perf->step('get vanished messages via Horde'); + + foreach (array_chunk($response->getVanishedMessageUids(), 500) as $chunk) { + $this->dbMapper->deleteByUid($mailbox, ...$chunk); + } + $perf->step('delete vanished messages'); + + // If a list of UIDs was *provided* (as opposed to loaded from the DB, + // we can not assume that all changes were detected, hence this is kinda + // a silent sync and we don't update the vanish token until the next full + // mailbox sync + if ($knownUids === null) { + $mailbox->setSyncVanishedToken($client->getSyncToken($mailbox->getName())); + } + $newOrVanished = $newOrVanished || !empty($response->getVanishedMessageUids()); + } + $this->mailboxMapper->update($mailbox); + $perf->end(); + + return $newOrVanished; + } +} diff --git a/lib/Service/Sync/SyncService.php b/lib/Service/Sync/SyncService.php index c127c52486..f79523f7bd 100644 --- a/lib/Service/Sync/SyncService.php +++ b/lib/Service/Sync/SyncService.php @@ -78,6 +78,18 @@ public function clearCache(Account $account, $this->synchronizer->clearCache($account, $mailbox); } + /** + * Run a (rather costly) sync to delete cached messages which are not present on IMAP anymore. + * + * @return int Number of detected vanished phantom messages + * + * @throws MailboxLockedException + * @throws ServiceException + */ + public function repairSync(Account $account, Mailbox $mailbox): int { + return $this->synchronizer->repairSync($account, $mailbox, $this->logger); + } + /** * @param Account $account * @param Mailbox $mailbox diff --git a/src/components/NavigationMailbox.vue b/src/components/NavigationMailbox.vue index 004616443a..78a2810235 100644 --- a/src/components/NavigationMailbox.vue +++ b/src/components/NavigationMailbox.vue @@ -111,6 +111,14 @@ {{ t('mail', 'Move mailbox') }} + + + {{ t('mail', 'Repair mailbox') }} + diff --git a/src/service/MailboxService.js b/src/service/MailboxService.js index 98c2e83b4c..bd49000dc9 100644 --- a/src/service/MailboxService.js +++ b/src/service/MailboxService.js @@ -66,3 +66,11 @@ export const clearMailbox = async (id) => { await axios.post(url) } + +export const repairMailbox = async (id) => { + const url = generateUrl('/apps/mail/api/mailboxes/{id}/repair', { + id, + }) + + await axios.post(url) +} diff --git a/tests/Integration/Sync/ImapToDbSynchronizerTest.php b/tests/Integration/Sync/ImapToDbSynchronizerTest.php new file mode 100644 index 0000000000..0554106c3d --- /dev/null +++ b/tests/Integration/Sync/ImapToDbSynchronizerTest.php @@ -0,0 +1,158 @@ +synchronizer = Server::get(ImapToDbSynchronizer::class); + $this->account = $this->createTestAccount(); + } + + public function testRepairSync(): void { + // Create some test messages + $mailbox = 'INBOX'; + $message = $this->getMessageBuilder() + ->from('ralph@buffington@domain.tld') + ->to('user@domain.tld') + ->subject('Message 1') + ->finish(); + $uid1 = $this->saveMessage($mailbox, $message, $this->account); + $message = $this->getMessageBuilder() + ->from('ralph@buffington@domain.tld') + ->to('user@domain.tld') + ->subject('Message 2') + ->finish(); + $uid2 = $this->saveMessage($mailbox, $message, $this->account); + $message = $this->getMessageBuilder() + ->from('ralph@buffington@domain.tld') + ->to('user@domain.tld') + ->subject('Message 3') + ->finish(); + $uid3 = $this->saveMessage($mailbox, $message, $this->account); + + // Retrieve mailbox object + $mailManager = Server::get(IMailManager::class); + $mailBoxes = $mailManager->getMailboxes(new Account($this->account)); + $inbox = null; + foreach ($mailBoxes as $mailBox) { + if ($mailBox->getName() === 'INBOX') { + $inbox = $mailBox; + break; + } + } + + // Do an initial sync to pull in all created messages + $syncService = Server::get(SyncService::class); + $syncService->syncMailbox( + new Account($this->account), + $inbox, + Horde_Imap_Client::SYNC_NEWMSGSUIDS | Horde_Imap_Client::SYNC_FLAGSUIDS | Horde_Imap_Client::SYNC_VANISHEDUIDS, + false, + null, + [], + ); + + // Assert that there are 3 messages and nothing changes when deleting a message externally + $dbMessageMapper = Server::get(DbMessageMapper::class); + self::assertCount(3, $dbMessageMapper->findAllUids($inbox)); + $this->deleteMessagesExternally($mailbox, [$uid3]); + self::assertCount(3, $dbMessageMapper->findAllUids($inbox)); + + // Do a repair sync to get rid of the vanished message that is still in the cache + $this->synchronizer->repairSync( + new Account($this->account), + $inbox, + Server::get(LoggerInterface::class), + ); + + // Assert that the cached state has been reconciled with IMAP + self::assertEquals([$uid1, $uid2], $dbMessageMapper->findAllUids($inbox)); + } + + public function testRepairSyncNoopIfNoneVanished(): void { + // Create some test messages + $mailbox = 'INBOX'; + $message = $this->getMessageBuilder() + ->from('ralph@buffington@domain.tld') + ->to('user@domain.tld') + ->subject('Message 1') + ->finish(); + $uid1 = $this->saveMessage($mailbox, $message, $this->account); + $message = $this->getMessageBuilder() + ->from('ralph@buffington@domain.tld') + ->to('user@domain.tld') + ->subject('Message 2') + ->finish(); + $uid2 = $this->saveMessage($mailbox, $message, $this->account); + $message = $this->getMessageBuilder() + ->from('ralph@buffington@domain.tld') + ->to('user@domain.tld') + ->subject('Message 3') + ->finish(); + $uid3 = $this->saveMessage($mailbox, $message, $this->account); + + // Retrieve mailbox object + $mailManager = Server::get(IMailManager::class); + $mailBoxes = $mailManager->getMailboxes(new Account($this->account)); + $inbox = null; + foreach ($mailBoxes as $mailBox) { + if ($mailBox->getName() === 'INBOX') { + $inbox = $mailBox; + break; + } + } + + // Do an initial sync to pull in all created messages + $syncService = Server::get(SyncService::class); + $syncService->syncMailbox( + new Account($this->account), + $inbox, + Horde_Imap_Client::SYNC_NEWMSGSUIDS | Horde_Imap_Client::SYNC_FLAGSUIDS | Horde_Imap_Client::SYNC_VANISHEDUIDS, + false, + null, + [], + ); + + // Assert that there are 3 messages and nothing changes when deleting a message externally + $dbMessageMapper = Server::get(DbMessageMapper::class); + self::assertCount(3, $dbMessageMapper->findAllUids($inbox)); + + // Do a repair sync to get rid of the vanished message that is still in the cache + $this->synchronizer->repairSync( + new Account($this->account), + $inbox, + Server::get(LoggerInterface::class), + ); + + // Assert that the cached state has been reconciled with IMAP + self::assertEquals([$uid1, $uid2, $uid3], $dbMessageMapper->findAllUids($inbox)); + } +}