Merge pull request #10036 from nextcloud/fix/imap/persist-deleted-mes…

…sages

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

lib/Cache/CachedMailbox.php

@@ -0,0 +1,48 @@
+<?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;
+
+class CachedMailbox {
+	/** @var int[]|null */
+	private ?array $uids = null;
+
+	private ?int $uidValidity = null;
+	private ?int $highestModSeq = null;
+
+	/**
+	 * @return int[]|null
+	 */
+	public function getUids(): ?array {
+		return $this->uids;
+	}
+
+	/**
+	 * @param int[]|null $uids
+	 */
+	public function setUids(?array $uids): void {
+		$this->uids = $uids;
+	}
+
+	public function getUidValidity(): ?int {
+		return $this->uidValidity;
+	}
+
+	public function setUidValidity(?int $uidvalid): void {
+		$this->uidValidity = $uidvalid;
+	}
+
+	public function getHighestModSeq(): ?int {
+		return $this->highestModSeq;
+	}
+
+	public function setHighestModSeq(?int $highestModSeq): void {
+		$this->highestModSeq = $highestModSeq;
+	}
+}

lib/Cache/HordeCacheFactory.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;
+
+class HordeCacheFactory {
+	public function __construct(
+		private MailboxMapper $mailboxMapper,
+		private MessageMapper $messageMapper,
+		private HordeSyncTokenParser $syncTokenParser,
+	) {
+	}
+
+	public function newCache(Account $account): Cache {
+		return new Cache(
+			$this->messageMapper,
+			$this->mailboxMapper,
+			$this->syncTokenParser,
+			$account,
+		);
+	}
+}

lib/Cache/HordeSyncToken.php

@@ -0,0 +1,31 @@
+<?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;
+
+class HordeSyncToken {
+	public function __construct(
+		private ?int $nextUid,
+		private ?int $uidValidity,
+		private ?int $highestModSeq,
+	) {
+	}
+
+	public function getNextUid(): ?int {
+		return $this->nextUid;
+	}
+
+	public function getUidValidity(): ?int {
+		return $this->uidValidity;
+	}
+
+	public function getHighestModSeq(): ?int {
+		return $this->highestModSeq;
+	}
+}

lib/Cache/HordeSyncTokenParser.php

@@ -0,0 +1,36 @@
+<?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;
+
+class HordeSyncTokenParser {
+	public function parseSyncToken(string $token): HordeSyncToken {
+		$decodedToken = base64_decode($token, true);
+		$parts = explode(',', $decodedToken);
+
+		$nextUid = null;
+		$uidValidity = null;
+		$highestModSeq = null;
+		foreach ($parts as $part) {
+			if (str_starts_with($part, 'U')) {
+				$nextUid = (int)substr($part, 1);
+			}
+
+			if (str_starts_with($part, 'V')) {
+				$uidValidity = (int)substr($part, 1);
+			}
+
+			if (str_starts_with($part, 'H')) {
+				$highestModSeq = (int)substr($part, 1);
+			}
+		}
+
+		return new HordeSyncToken($nextUid, $uidValidity, $highestModSeq);
+	}
+}

lib/IMAP/HordeImapClient.php

@@ -11,14 +11,12 @@
 
 use Horde_Imap_Client_Exception;
 use Horde_Imap_Client_Socket;
-use OCA\Mail\Cache\Cache;
 use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\IMemcache;
 use function floor;
 
 /**
- * "Decorator" around Horde's IMAP client to add auth error rate limiting and save the cache on
- * logout.
+ * "Decorator" around Horde's IMAP client to add auth error rate limiting.
  *
  * This is not a real decorator because the component to decorate doesn't have
  * an interface, making it hard to base a decorator on composition.
@@ -28,15 +26,6 @@ class HordeImapClient extends Horde_Imap_Client_Socket {
 	private ?IMemcache $rateLimiterCache = null;
 	private ?ITimeFactory $timeFactory = null;
 	private ?string $hash = null;
-	private ?Cache $cacheBackend = null;
-
-	public function __construct(array $params) {
-		if (isset($params['cache']['backend']) && $params['cache']['backend'] instanceof Cache) {
-			$this->cacheBackend = $params['cache']['backend'];
-		}
-
-		parent::__construct($params);
-	}
 
 	public function enableRateLimiter(
 		IMemcache $cache,
@@ -76,12 +65,4 @@ protected function _login() {
 			throw $e;
 		}
 	}
-
-	public function logout() {
-		if ($this->cacheBackend !== null) {
-			$this->cacheBackend->save();
-		}
-
-		parent::logout();
-	}
 }

lib/IMAP/IMAPClientFactory.php

@@ -9,11 +9,10 @@
 
 namespace OCA\Mail\IMAP;
 
-use Horde_Imap_Client_Cache_Backend_Null;
 use Horde_Imap_Client_Password_Xoauth2;
 use Horde_Imap_Client_Socket;
 use OCA\Mail\Account;
-use OCA\Mail\Cache\Cache;
+use OCA\Mail\Cache\HordeCacheFactory;
 use OCA\Mail\Events\BeforeImapClientCreated;
 use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\EventDispatcher\IEventDispatcher;
@@ -24,7 +23,6 @@
 use function hash;
 use function implode;
 use function json_encode;
-use function md5;
 
 class IMAPClientFactory {
 	/** @var ICrypto */
@@ -40,17 +38,20 @@ class IMAPClientFactory {
 	private $eventDispatcher;
 
 	private ITimeFactory $timeFactory;
+	private HordeCacheFactory $hordeCacheFactory;
 
 	public function __construct(ICrypto $crypto,
 		IConfig $config,
 		ICacheFactory $cacheFactory,
 		IEventDispatcher $eventDispatcher,
-		ITimeFactory $timeFactory) {
+		ITimeFactory $timeFactory,
+		HordeCacheFactory $hordeCacheFactory) {
 		$this->crypto = $crypto;
 		$this->config = $config;
 		$this->cacheFactory = $cacheFactory;
 		$this->eventDispatcher = $eventDispatcher;
 		$this->timeFactory = $timeFactory;
+		$this->hordeCacheFactory = $hordeCacheFactory;
 	}
 
 	/**
@@ -111,21 +112,9 @@ public function getClient(Account $account, bool $useCache = true): Horde_Imap_C
 				json_encode($params)
 			]),
 		);
-		if ($useCache && $this->cacheFactory->isAvailable()) {
-			$params['cache'] = [
-				'backend' => new Cache([
-					'cacheob' => $this->cacheFactory->createDistributed(md5((string)$account->getId())),
-				])];
-		} else {
-			/**
-			 * 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(),
-			];
-		}
+		$params['cache'] = [
+			'backend' => $this->hordeCacheFactory->newCache($account),
+		];
 		if ($this->config->getSystemValue('debug', false)) {
 			$params['debug'] = $this->config->getSystemValue('datadirectory') . '/horde_imap.log';
 		}

tests/Integration/Framework/ImapTest.php

@@ -211,6 +211,31 @@ public function deleteMessage($mailbox, $id, ?MailAccount $account = null) {
 		}
 	}
 
+	/**
+	 * Delete a message without informing Horde or the db cache. This simulates another client
+	 * deleting a message on IMAP.
+	 *
+	 * @param int[] $uids
+	 */
+	public function deleteMessagesExternally(string $mailbox, array $uids): void {
+		$client = new Horde_Imap_Client_Socket([
+			'username' => '[email protected]',
+			'password' => 'mypassword',
+			'hostspec' => '127.0.0.1',
+			'port' => 993,
+			'secure' => 'ssl',
+		]);
+		$ids = new Horde_Imap_Client_Ids($uids);
+		try {
+			$client->expunge($mailbox, [
+				'ids' => $ids,
+				'delete' => true,
+			]);
+		} finally {
+			$client->logout();
+		}
+	}
+
 	/**
 	 * @param Horde_Imap_Client_Socket $client
 	 * @param string $mailbox

tests/Integration/IMAP/IMAPClientFactoryTest.php

@@ -14,6 +14,7 @@
 use Horde_Imap_Client_Socket;
 use OC\Memcache\Redis;
 use OCA\Mail\Account;
+use OCA\Mail\Cache\HordeCacheFactory;
 use OCA\Mail\Db\MailAccount;
 use OCA\Mail\IMAP\HordeImapClient;
 use OCA\Mail\IMAP\IMAPClientFactory;
@@ -40,6 +41,7 @@ class IMAPClientFactoryTest extends TestCase {
 	private $factory;
 	private IEventDispatcher|MockObject $eventDispatcher;
 	private ITimeFactory|MockObject $timeFactory;
+	private HordeCacheFactory|MockObject $hordeCacheFactory;
 
 	protected function setUp(): void {
 		parent::setUp();
@@ -49,13 +51,15 @@ protected function setUp(): void {
 		$this->cacheFactory = Server::get(ICacheFactory::class);
 		$this->eventDispatcher = $this->createMock(IEventDispatcher::class);
 		$this->timeFactory = $this->createMock(ITimeFactory::class);
+		$this->hordeCacheFactory = $this->createMock(HordeCacheFactory::class);
 
 		$this->factory = new IMAPClientFactory(
 			$this->crypto,
 			$this->config,
 			$this->cacheFactory,
 			$this->eventDispatcher,
 			$this->timeFactory,
+			$this->hordeCacheFactory,
 		);
 	}