fix(imap): persist vanished messages immediately on EXAMINE commands

Signed-off-by: Richard Steinmetz <[email protected]>

lib/Cache/Cache.php

@@ -13,20 +13,17 @@
 use Exception;
 use Horde_Imap_Client_Cache_Backend;
 use Horde_Imap_Client_Exception;
-use InvalidArgumentException;
+use OCA\Mail\Account;
+use OCA\Mail\Db\MailboxMapper;
+use OCA\Mail\Db\MessageMapper;
 use OCP\ICache;
 
 /**
  * This class is inspired by Horde_Imap_Client_Cache_Backend_Cache of the Horde Project
  */
 class Cache extends Horde_Imap_Client_Cache_Backend {
 	/** Cache structure version. */
-	public const VERSION = 3;
-
-	/**
-	 * The cache object.
-	 */
-	protected ICache $_cache;
+	public const VERSION = 4;
 
 	/**
 	 * The working data for the current pageload.  All changes take place to
@@ -52,26 +49,24 @@ class Cache extends Horde_Imap_Client_Cache_Backend {
 	 */
 	protected array $_update = [];
 
-	/**
-	 * Constructor.
-	 *
-	 * @param array $params Configuration parameters:
-	 */
-	public function __construct(array $params = []) {
-		// Default parameters.
-		$params = array_merge([
-			'lifetime' => 604800,
-			'slicesize' => 50
-		], array_filter($params));
-
-		if (!isset($params['cacheob'])) {
-			throw new InvalidArgumentException('Missing cacheob parameter.');
-		}
-
-		foreach (['lifetime', 'slicesize'] as $val) {
-			$params[$val] = intval($params[$val]);
-		}
-
+	/** @var int[]|null **/
+	private ?array $cachedUids = null;
+
+	private ?int $uidvalid = null;
+
+	public function __construct(
+		private MessageMapper $messageMapper,
+		private MailboxMapper $mailboxMapper,
+		private Account $account,
+		private ICache $_cache,
+		int $lifetime = 604800,
+		int $slicesize = 50,
+	) {
+		$params = [
+			'lifetime' => $lifetime,
+			'slicesize' => $slicesize,
+			'cacheob' => $this->_cache,
+		];
 		parent::__construct($params);
 	}
 
@@ -164,13 +159,23 @@ public function get($mailbox, $uids, $fields, $uidvalid) {
 		return $ret;
 	}
 
-	/** {@inheritDoc} */
-	public function getCachedUids($mailbox, $uidvalid) {
-		$this->_loadSliceMap($mailbox, $uidvalid);
-		return array_unique(array_merge(
-			array_keys($this->_slicemap[$mailbox]['s']),
-			(isset($this->_update[$mailbox]) ? $this->_update[$mailbox]['add'] : [])
-		));
+	public function getCachedUids($mailbox, $uidvalid): array {
+		// Delete cached data of mailbox if uidvalid has changed
+		if ($this->uidvalid !== null && $uidvalid !== null && $this->uidvalid !== $uidvalid) {
+			$this->_deleteMailbox($mailbox);
+		}
+
+		// Refresh cached uids lazily
+		if ($this->cachedUids === null) {
+			$mailboxEntity = $this->mailboxMapper->find($this->account, $mailbox);
+			$this->cachedUids = $this->messageMapper->findAllUids($mailboxEntity);
+		}
+
+		if ($this->uidvalid === null && $uidvalid !== null) {
+			$this->uidvalid = $uidvalid;
+		}
+
+		return array_merge([], $this->cachedUids);
 	}
 
 	/**
@@ -235,12 +240,13 @@ public function setMetaData($mailbox, $data) {
 		$this->_toUpdate($mailbox, 'slicemap', true);
 	}
 
-	/**
-	 * {@inheritDoc}
-	 *
-	 * @return void
-	 */
-	public function deleteMsgs($mailbox, $uids) {
+	public function deleteMsgs($mailbox, $uids): void {
+		// Delete uids from the db cache
+		$mailboxEntity = $this->mailboxMapper->find($this->account, $mailbox);
+		$this->messageMapper->deleteByUid($mailboxEntity, ...$uids);
+		$this->cachedUids = array_diff($this->cachedUids, $uids);
+
+		// Delete uids from the memory cache
 		$this->_loadSliceMap($mailbox);
 
 		$slicemap = &$this->_slicemap[$mailbox];
@@ -298,6 +304,8 @@ public function deleteMailbox($mailbox) {
 	public function clear($lifetime) {
 		$this->_cache->clear();
 		$this->_data = $this->_loaded = $this->_slicemap = $this->_update = [];
+		$this->cachedUids = null;
+		$this->uidvalid = null;
 	}
 
 	/**
@@ -339,6 +347,9 @@ protected function _deleteMailbox($mbox): void {
 			$this->_slicemap[$mbox],
 			$this->_update[$mbox]
 		);
+
+		$this->cachedUids = null;
+		$this->uidvalid = null;
 	}
 
 	/**

lib/Cache/CacheFactory.php

@@ -0,0 +1,32 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace OCA\Mail\Cache;
+
+use OCA\Mail\Account;
+use OCA\Mail\Db\MailboxMapper;
+use OCA\Mail\Db\MessageMapper;
+use OCP\ICache;
+
+class CacheFactory {
+	public function __construct(
+		private readonly MailboxMapper $mailboxMapper,
+		private readonly MessageMapper $messageMapper,
+	) {
+	}
+
+	public function newCache(Account $account, ICache $cache): Cache {
+		return new Cache(
+			$this->messageMapper,
+			$this->mailboxMapper,
+			$account,
+			$cache,
+		);
+	}
+}

lib/IMAP/IMAPClientFactory.php

@@ -13,7 +13,7 @@
 use Horde_Imap_Client_Password_Xoauth2;
 use Horde_Imap_Client_Socket;
 use OCA\Mail\Account;
-use OCA\Mail\Cache\Cache;
+use OCA\Mail\Cache\CacheFactory;
 use OCA\Mail\Events\BeforeImapClientCreated;
 use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\EventDispatcher\IEventDispatcher;
@@ -40,17 +40,20 @@ class IMAPClientFactory {
 	private $eventDispatcher;
 
 	private ITimeFactory $timeFactory;
+	private CacheFactory $hordeCacheFactory;
 
 	public function __construct(ICrypto $crypto,
 		IConfig $config,
 		ICacheFactory $cacheFactory,
 		IEventDispatcher $eventDispatcher,
-		ITimeFactory $timeFactory) {
+		ITimeFactory $timeFactory,
+		CacheFactory $hordeCacheFactory) {
 		$this->crypto = $crypto;
 		$this->config = $config;
 		$this->cacheFactory = $cacheFactory;
 		$this->eventDispatcher = $eventDispatcher;
 		$this->timeFactory = $timeFactory;
+		$this->hordeCacheFactory = $hordeCacheFactory;
 	}
 
 	/**
@@ -113,10 +116,15 @@ public function getClient(Account $account, bool $useCache = true): Horde_Imap_C
 		);
 		if ($useCache && $this->cacheFactory->isAvailable()) {
 			$params['cache'] = [
-				'backend' => new Cache([
-					'cacheob' => $this->cacheFactory->createDistributed(md5((string)$account->getId())),
-				])];
+				'backend' => $this->hordeCacheFactory->newCache(
+					$account,
+					$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