Skip to content

Commit

Permalink
feat: implement periodic full sync job to repair cache inconsistencies
Browse files Browse the repository at this point in the history
Signed-off-by: Richard Steinmetz <[email protected]>
  • Loading branch information
st3iny committed Sep 24, 2024
1 parent d42f23a commit 89dd295
Show file tree
Hide file tree
Showing 10 changed files with 382 additions and 4 deletions.
5 changes: 5 additions & 0 deletions appinfo/routes.php
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,11 @@
'url' => '/api/mailboxes/{id}/stats',
'verb' => 'GET'
],
[
'name' => 'mailboxes#repair',
'url' => '/api/mailboxes/{id}/repair',
'verb' => 'POST'
],
[
'name' => 'messages#downloadAttachment',
'url' => '/api/messages/{id}/attachment/{attachmentId}',
Expand Down
90 changes: 90 additions & 0 deletions lib/BackgroundJob/RepairSyncJob.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
<?php

declare(strict_types=1);

/**
* SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/

namespace OCA\Mail\BackgroundJob;

use OCA\Mail\Db\MailboxMapper;
use OCA\Mail\Events\SynchronizationEvent;
use OCA\Mail\Service\AccountService;
use OCA\Mail\Service\Sync\SyncService;
use OCP\AppFramework\Db\DoesNotExistException;
use OCP\AppFramework\Utility\ITimeFactory;
use OCP\BackgroundJob\IJobList;
use OCP\BackgroundJob\TimedJob;
use OCP\EventDispatcher\IEventDispatcher;
use OCP\IUserManager;
use Psr\Log\LoggerInterface;

class RepairSyncJob extends TimedJob {
public function __construct(
ITimeFactory $time,
private SyncService $syncService,
private AccountService $accountService,
private IUserManager $userManager,
private MailboxMapper $mailboxMapper,
private IJobList $jobList,
private LoggerInterface $logger,
private IEventDispatcher $dispatcher,
) {
parent::__construct($time);

$this->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),
);
}
}
14 changes: 14 additions & 0 deletions lib/Controller/MailboxesController.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);

Check failure on line 297 in lib/Controller/MailboxesController.php

View workflow job for this annotation

GitHub Actions / static-psalm-analysis dev-master

PossiblyNullArgument

lib/Controller/MailboxesController.php:297:45: PossiblyNullArgument: Argument 1 of OCA\Mail\Contracts\IMailManager::getMailbox cannot be null, possibly null value provided (see https://psalm.dev/078)
$account = $this->accountService->find($this->currentUserId, $mailbox->getAccountId());

$this->syncService->repairSync($account, $mailbox);
return new JsonResponse();
}
}
8 changes: 5 additions & 3 deletions lib/IMAP/IMAPClientFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -112,9 +112,11 @@ public function getClient(Account $account, bool $useCache = true): Horde_Imap_C
json_encode($params)
]),
);
$params['cache'] = [
'backend' => $this->hordeCacheFactory->newCache($account),
];
if ($useCache) {
$params['cache'] = [
'backend' => $this->hordeCacheFactory->newCache($account),
];
}
if ($this->config->getSystemValue('debug', false)) {
$params['debug'] = $this->config->getSystemValue('datadirectory') . '/horde_imap.log';
}
Expand Down
2 changes: 2 additions & 0 deletions lib/Migration/FixBackgroundJobs.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

use OCA\Mail\BackgroundJob\PreviewEnhancementProcessingJob;
use OCA\Mail\BackgroundJob\QuotaJob;
use OCA\Mail\BackgroundJob\RepairSyncJob;
use OCA\Mail\BackgroundJob\SyncJob;
use OCA\Mail\BackgroundJob\TrainImportanceClassifierJob;
use OCA\Mail\Db\MailAccount;
Expand Down Expand Up @@ -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(RepairSyncJob::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()]);
Expand Down
46 changes: 46 additions & 0 deletions lib/Service/Sync/ImapToDbSynchronizer.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -485,4 +486,49 @@ private function runPartialSync(

return $newOrVanished;
}

/**
* Run a (rather costly) sync to delete cached messages which are not present on IMAP anymore.
*
* @throws MailboxLockedException
* @throws ServiceException
*/
public function repairSync(
Account $account,
Mailbox $mailbox,
LoggerInterface $logger,
): void {
$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),
])->ids;
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();
}
}
10 changes: 10 additions & 0 deletions lib/Service/Sync/SyncService.php
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,16 @@ 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.
*
* @throws MailboxLockedException
* @throws ServiceException
*/
public function repairSync(Account $account, Mailbox $mailbox): void {
$this->synchronizer->repairSync($account, $mailbox, $this->logger);
}

/**
* @param Account $account
* @param Mailbox $mailbox
Expand Down
39 changes: 38 additions & 1 deletion src/components/NavigationMailbox.vue
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,14 @@
</template>
{{ t('mail', 'Move mailbox') }}
</ActionButton>
<ActionButton v-if="!account.isUnified && mailbox.specialRole !== 'flagged'"
:disabled="repairing"
@click="repair">
<template #icon>
<IconWrench :size="16" />
</template>
{{ t('mail', 'Repair mailbox') }}
</ActionButton>
<ActionButton v-if="debug && !account.isUnified && mailbox.specialRole !== 'flagged'"
:name="t('mail', 'Clear cache')"
:disabled="clearingCache"
Expand Down Expand Up @@ -195,11 +203,12 @@ import IconAllInboxes from 'vue-material-design-icons/InboxMultiple.vue'
import EraserVariant from 'vue-material-design-icons/EraserVariant.vue'
import ImportantIcon from './icons/ImportantIcon.vue'
import IconSend from 'vue-material-design-icons/Send.vue'
import IconWrench from 'vue-material-design-icons/Wrench.vue'
import MoveMailboxModal from './MoveMailboxModal.vue'
import { PRIORITY_INBOX_ID, UNIFIED_INBOX_ID } from '../store/constants.js'
import { mailboxHasRights } from '../util/acl.js'
import { clearCache } from '../service/MessageService.js'
import { getMailboxStatus } from '../service/MailboxService.js'
import { getMailboxStatus, repairMailbox } from '../service/MailboxService.js'
import logger from '../logger.js'
import { translatePlural as n } from '@nextcloud/l10n'
import { translate as translateMailboxName } from '../i18n/MailboxTranslator.js'
Expand Down Expand Up @@ -233,6 +242,7 @@ export default {
IconArchive,
IconJunk,
IconInbox,
IconWrench,
EraserVariant,
ImportantIcon,
IconLoading,
Expand Down Expand Up @@ -276,6 +286,7 @@ export default {
hasDelimiter: !!this.mailbox.delimiter,
UNIFIED_INBOX_ID,
createMailboxName: '',
repairing: false,
}
},
computed: {
Expand Down Expand Up @@ -659,6 +670,32 @@ export default {
})
}
},
/**
* Delete all vanished emails that are still cached.
*
* @return {Promise<void>}
*/
async repair() {
this.repairing = true
const mailboxId = this.mailbox.databaseId
try {
await repairMailbox(mailboxId)
// Reload the page to start with a clean mailbox state
await this.$router.push({
name: 'mailbox',
params: {
mailboxId: this.$route.params.mailboxId,
},
})
window.location.reload()
} catch (error) {
// Only reset state in case of an error because the page will be reloaded anyway
this.repairing = false
throw error
}
},
},
}
</script>
Expand Down
14 changes: 14 additions & 0 deletions src/service/MailboxService.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,3 +66,17 @@ export const clearMailbox = async (id) => {

await axios.post(url)
}

/**
* Delete all vanished emails that are still cached.
*
* @param {number} id Mailbox database id
* @return {Promise<void>}
*/
export const repairMailbox = async (id) => {
const url = generateUrl('/apps/mail/api/mailboxes/{id}/repair', {
id,
})

await axios.post(url)
}
Loading

0 comments on commit 89dd295

Please sign in to comment.