From a2c9e8cdf6abfe3648e17decaa82407878ffd178 Mon Sep 17 00:00:00 2001 From: Richard Steinmetz Date: Wed, 21 Aug 2024 14:18:22 +0200 Subject: [PATCH] fix(imap): persist vanished messages immediately on EXAMINE commands Signed-off-by: Richard Steinmetz --- lib/Account.php | 12 +- lib/Cache/Cache.php | 507 +++--------------- lib/Cache/CachedMailbox.php | 48 ++ lib/Cache/HordeCacheFactory.php | 32 ++ lib/Cache/HordeSyncToken.php | 31 ++ lib/Cache/HordeSyncTokenParser.php | 36 ++ lib/IMAP/HordeImapClient.php | 21 +- lib/IMAP/IMAPClientFactory.php | 27 +- tests/Integration/Framework/ImapTest.php | 25 + .../IMAP/IMAPClientFactoryTest.php | 4 + .../MailboxSynchronizationTest.php | 65 +++ 11 files changed, 334 insertions(+), 474 deletions(-) create mode 100644 lib/Cache/CachedMailbox.php create mode 100644 lib/Cache/HordeCacheFactory.php create mode 100644 lib/Cache/HordeSyncToken.php create mode 100644 lib/Cache/HordeSyncTokenParser.php diff --git a/lib/Account.php b/lib/Account.php index b1d0197b3d..8cd9a9e060 100644 --- a/lib/Account.php +++ b/lib/Account.php @@ -38,7 +38,7 @@ use Horde_Imap_Client_Socket; use JsonSerializable; use OC; -use OCA\Mail\Cache\Cache; +use OCA\Mail\Cache\HordeCacheFactory; use OCA\Mail\Db\Alias; use OCA\Mail\Db\MailAccount; use OCA\Mail\Exception\ServiceException; @@ -48,6 +48,7 @@ use OCP\ICacheFactory; use OCP\IConfig; use OCP\Security\ICrypto; +use OCP\Server; use ReturnTypeWillChange; class Account implements JsonSerializable { @@ -69,6 +70,8 @@ class Account implements JsonSerializable { /** @var Alias */ private $alias; + private HordeCacheFactory $hordeCacheFactory; + /** * @param MailAccount $account */ @@ -77,6 +80,7 @@ public function __construct(MailAccount $account) { $this->crypto = OC::$server->getCrypto(); $this->config = OC::$server->getConfig(); $this->memcacheFactory = OC::$server->getMemcacheFactory(); + $this->hordeCacheFactory = Server::get(HordeCacheFactory::class); } public function __destruct() { @@ -153,10 +157,8 @@ public function getImapConnection() { if ($this->config->getSystemValue('app.mail.server-side-cache.enabled', true)) { if ($this->memcacheFactory->isAvailable()) { $params['cache'] = [ - 'backend' => new Cache([ - 'cacheob' => $this->memcacheFactory - ->createDistributed(md5($this->getId() . $this->getEMailAddress())) - ])]; + 'backend' => $this->hordeCacheFactory->newCache($this), + ]; } } $this->client = new \Horde_Imap_Client_Socket($params); diff --git a/lib/Cache/Cache.php b/lib/Cache/Cache.php index af05a758f8..d6e2c673b0 100644 --- a/lib/Cache/Cache.php +++ b/lib/Cache/Cache.php @@ -5,6 +5,7 @@ /** * @author Lukas Reschke * @author Thomas Müller + * @author 2024 Richard Steinmetz * * Mail * @@ -24,483 +25,129 @@ namespace OCA\Mail\Cache; -use Exception; use Horde_Imap_Client_Cache_Backend; -use Horde_Imap_Client_Exception; -use InvalidArgumentException; -use OCP\ICache; +use OCA\Mail\Account; +use OCA\Mail\Db\MailboxMapper; +use OCA\Mail\Db\MessageMapper; /** - * This class is inspired by Horde_Imap_Client_Cache_Backend_Cache of the Horde Project + * This class passes the minimum amount of data from the db cache to Horde to make QRESYNC work + * reliably. */ class Cache extends Horde_Imap_Client_Cache_Backend { - /** Cache structure version. */ - public const VERSION = 3; + /** @var CachedMailbox[] */ + private array $cachedMailboxes = []; - /** - * The cache object. - */ - protected ICache $_cache; - - /** - * The working data for the current pageload. All changes take place to - * this data. - */ - protected array $_data = []; - - /** - * The list of cache slices loaded. - */ - protected array $_loaded = []; - - /** - * The mapping of UIDs to slices. - */ - protected array $_slicemap = []; - - /** - * The list of items to update: - * - add: (array) List of IDs that were added. - * - slice: (array) List of slices that were modified. - * - slicemap: (boolean) Was slicemap info changed? - */ - 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]); - } - - parent::__construct($params); + public function __construct( + private MessageMapper $dbMessageMapper, + private MailboxMapper $mailboxMapper, + private HordeSyncTokenParser $syncTokenParser, + private Account $account, + ) { + parent::__construct(); } - /** - * * Initialization tasks. - * - * @return void - */ - protected function _initOb() { - $this->_cache = $this->_params['cacheob']; + public function get($mailbox, $uids, $fields, $uidvalid) { + // Don't forward any data related to individual messages + return []; } - /** - * Updates the cache. - */ - public function save(): void { - $lifetime = $this->_params['lifetime']; - - foreach ($this->_update as $mbox => $val) { - $s = &$this->_slicemap[$mbox]; - - if (!empty($val['add'])) { - if ($s['c'] <= $this->_params['slicesize']) { - $val['slice'][] = $s['i']; - $this->_loadSlice($mbox, $s['i']); - } - $val['slicemap'] = true; - - foreach (array_keys(array_flip($val['add'])) as $uid) { - if ($this->_params['slicesize'] < $s['c']++) { - $s['c'] = 0; - $val['slice'][] = ++$s['i']; - $this->_loadSlice($mbox, $s['i']); - } - $s['s'][$uid] = $s['i']; - } - } - - if (!empty($val['slice'])) { - $d = &$this->_data[$mbox]; - $val['slicemap'] = true; - - foreach (array_keys(array_flip($val['slice'])) as $slice) { - $data = []; - foreach (array_keys($s['s'], $slice) as $uid) { - /** @var int $uid */ - $data[$uid] = is_array($d[$uid]) - ? serialize($d[$uid]) - : $d[$uid]; - } - $this->_cache->set($this->_getCid($mbox, $slice), serialize($data), $lifetime); - } - } - - if (!empty($val['slicemap'])) { - $this->_cache->set($this->_getCid($mbox, 'slicemap'), serialize($s), $lifetime); - } + private function getOrInsertCachedMailbox(string $mailbox): CachedMailbox { + if (!isset($this->cachedMailboxes[$mailbox])) { + $this->cachedMailboxes[$mailbox] = new CachedMailbox(); } - $this->_update = []; + return $this->cachedMailboxes[$mailbox]; } - /** {@inheritDoc} */ - public function get($mailbox, $uids, $fields, $uidvalid) { - $ret = []; - $this->_loadUids($mailbox, $uids, $uidvalid); - - if (empty($this->_data[$mailbox])) { - return $ret; - } + public function getCachedUids($mailbox, $uidvalid) { + $cachedMailbox = $this->getOrInsertCachedMailbox($mailbox); - if (!empty($fields)) { - $fields = array_flip($fields); + // Delete cached data of mailbox if uidvalid has changed + $cachedUidvalid = $cachedMailbox->getUidValidity(); + if ($uidvalid !== null + && $cachedUidvalid !== null + && $cachedUidvalid !== (int)$uidvalid + ) { + $this->deleteMailbox($mailbox); + $cachedMailbox = $this->getOrInsertCachedMailbox($mailbox); } - $ptr = &$this->_data[$mailbox]; - - foreach (array_intersect($uids, array_keys($ptr)) as $val) { - if (is_string($ptr[$val])) { - try { - $ptr[$val] = @unserialize($ptr[$val]); - } catch (Exception $e) { - } - } - $ret[$val] = (empty($fields) || empty($ptr[$val])) - ? $ptr[$val] - : array_intersect_key($ptr[$val], $fields); + // Refresh cached uids lazily + $cachedUids = $cachedMailbox->getUids(); + if ($cachedUids === null) { + $mailboxEntity = $this->mailboxMapper->find($this->account, $mailbox); + $cachedUids = $this->dbMessageMapper->findAllUids($mailboxEntity); + $cachedMailbox->setUids($cachedUids); } - return $ret; + // Copy the array because we don't know whether horde will mutate it + return array_merge([], $cachedUids); } - /** {@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'] : []) - )); - } - - /** - * {@inheritDoc} - * - * @return void - */ public function set($mailbox, $data, $uidvalid) { - $update = array_keys($data); - - try { - $this->_loadUids($mailbox, $update, $uidvalid); - } catch (Horde_Imap_Client_Exception $e) { - // Ignore invalidity - just start building the new cache - } - - $d = &$this->_data[$mailbox]; - $s = &$this->_slicemap[$mailbox]['s']; - $add = $updated = []; + // Don't mutate any data related to individual messages + } - foreach ($data as $k => $v) { - if (isset($d[$k])) { - if (is_string($d[$k])) { - try { - $d[$k] = @unserialize($d[$k]); - } catch (Exception $e) { - } + public function getMetaData($mailbox, $uidvalid, $entries) { + $cachedMailbox = $this->getOrInsertCachedMailbox($mailbox); + + // Ensure that uidvalid is always set (see the default null cache implementation) + $md = ['uidvalid' => 0]; + + // Lazily load uidvalid and highestmodseq values from the database + if ($cachedMailbox->getUidValidity() === null || $cachedMailbox->getHighestModSeq() === null) { + $mailboxEntity = $this->mailboxMapper->find($this->account, $mailbox); + $syncToken = $mailboxEntity->getSyncNewToken(); + if ($syncToken !== null) { + $parsedToken = $this->syncTokenParser->parseSyncToken($syncToken); + if ($parsedToken->getUidValidity()) { + $cachedMailbox->setUidValidity($parsedToken->getUidValidity()); } - $d[$k] = is_array($d[$k]) - ? array_merge($d[$k], $v) - : $v; - if (isset($s[$k])) { - $updated[$s[$k]] = true; + if ($parsedToken->getHighestModSeq()) { + $cachedMailbox->setHighestModSeq($parsedToken->getHighestModSeq()); } - } else { - $d[$k] = $v; - $add[] = $k; } } - $this->_toUpdate($mailbox, 'add', $add); - $this->_toUpdate($mailbox, 'slice', array_keys($updated)); - } + if ($cachedMailbox->getUidValidity() !== null) { + $md['uidvalid'] = $cachedMailbox->getUidValidity(); + } - /** {@inheritDoc} */ - public function getMetaData($mailbox, $uidvalid, $entries) { - $this->_loadSliceMap($mailbox, $uidvalid); + if ($cachedMailbox->getHighestModSeq() !== null) { + $md['_m'] = $cachedMailbox->getHighestModSeq(); + } - return empty($entries) - ? $this->_slicemap[$mailbox]['d'] - : array_intersect_key($this->_slicemap[$mailbox]['d'], array_flip($entries)); + return $md; } - /** - * {@inheritDoc} - * - * @return void - */ public function setMetaData($mailbox, $data) { - $this->_loadSliceMap($mailbox, isset($data['uidvalid']) ? $data['uidvalid'] : null); - $this->_slicemap[$mailbox]['d'] = array_merge($this->_slicemap[$mailbox]['d'], $data); - $this->_toUpdate($mailbox, 'slicemap', true); + // Don't mutate any metadata. + // The data will be refreshed once the new sync token is written to the db. } - /** - * {@inheritDoc} - * - * @return void - */ public function deleteMsgs($mailbox, $uids) { - $this->_loadSliceMap($mailbox); + $mailboxEntity = $this->mailboxMapper->find($this->account, $mailbox); + $this->dbMessageMapper->deleteByUid($mailboxEntity, ...$uids); - $slicemap = &$this->_slicemap[$mailbox]; - $deleted = array_intersect_key($slicemap['s'], array_flip($uids)); - - if (isset($this->_update[$mailbox])) { - $this->_update[$mailbox]['add'] = array_diff( - $this->_update[$mailbox]['add'], - $uids - ); - } - - if (empty($deleted)) { + if (!isset($this->cachedMailboxes[$mailbox])) { return; } - $this->_loadUids($mailbox, array_keys($deleted)); - $d = &$this->_data[$mailbox]; - - foreach (array_keys($deleted) as $id) { - unset($d[$id], $slicemap['s'][$id]); + $cachedMailbox = $this->cachedMailboxes[$mailbox]; + $cachedUids = $cachedMailbox->getUids(); + if ($cachedUids === null) { + return; } - foreach (array_unique($deleted) as $slice) { - /* Get rid of slice if less than 10% of capacity. */ - if (($slice !== $slicemap['i']) && - ($slice_uids = array_keys($slicemap['s'], $slice)) && - ($this->_params['slicesize'] * 0.1) > count($slice_uids)) { - $this->_toUpdate($mailbox, 'add', $slice_uids); - $this->_cache->remove($this->_getCid($mailbox, $slice)); - foreach ($slice_uids as $val) { - unset($slicemap['s'][$val]); - } - } else { - $this->_toUpdate($mailbox, 'slice', [$slice]); - } - } + $cachedMailbox->setUids(array_diff($cachedUids, $uids)); } - /** - * {@inheritDoc} - * - * @return void - */ public function deleteMailbox($mailbox) { - $this->_loadSliceMap($mailbox); - $this->_deleteMailbox($mailbox); + unset($this->cachedMailboxes[$mailbox]); } - /** - * {@inheritDoc} - * - * @return void - */ public function clear($lifetime) { - $this->_cache->clear(); - $this->_data = $this->_loaded = $this->_slicemap = $this->_update = []; - } - - /** - * Create the unique ID used to store the data in the cache. - * - * @param string $mailbox The mailbox to cache. - * @param string $slice The cache slice. - * - * @return string The cache ID. - */ - protected function _getCid($mailbox, $slice) { - return implode('|', [ - 'horde_imap_client', - $this->_params['username'], - $mailbox, - $this->_params['hostspec'], - $this->_params['port'], - $slice, - self::VERSION - ]); - } - - /** - * Delete a mailbox from the cache. - * - * @param string $mbox The mailbox to delete. - * - * @return void - */ - protected function _deleteMailbox($mbox): void { - foreach (array_merge(array_keys(array_flip($this->_slicemap[$mbox]['s'])), ['slicemap']) as $slice) { - $cid = $this->_getCid($mbox, $slice); - $this->_cache->remove($cid); - unset($this->_loaded[$cid]); - } - - unset( - $this->_data[$mbox], - $this->_slicemap[$mbox], - $this->_update[$mbox] - ); - } - - /** - * Load UIDs by regenerating from the cache. - * - * @param string $mailbox The mailbox to load. - * @param array $uids The UIDs to load. - * @param integer $uidvalid The IMAP uidvalidity value of the mailbox. - * - * @return void - */ - protected function _loadUids($mailbox, $uids, $uidvalid = null): void { - if (!isset($this->_data[$mailbox])) { - $this->_data[$mailbox] = []; - } - - $this->_loadSliceMap($mailbox, $uidvalid); - - if (!empty($uids)) { - foreach (array_unique(array_intersect_key($this->_slicemap[$mailbox]['s'], array_flip($uids))) as $slice) { - $this->_loadSlice($mailbox, $slice); - } - } - } - - /** - * Load UIDs from a cache slice. - * - * @param string $mailbox The mailbox to load. - * @param integer $slice The slice to load. - * - * @return void - */ - protected function _loadSlice($mailbox, $slice) { - $cache_id = $this->_getCid($mailbox, (string)$slice); - - if (!empty($this->_loaded[$cache_id])) { - return; - } - - if (($data = $this->_cache->get($cache_id)) !== false) { - try { - if (is_string($data)) { - $data = @unserialize($data); - } - } catch (Exception $e) { - } - } - - if (($data !== false) && is_array($data)) { - $this->_data[$mailbox] += $data; - $this->_loaded[$cache_id] = true; - } else { - $ptr = &$this->_slicemap[$mailbox]; - - // Slice data is corrupt; remove from slicemap. - foreach (array_keys($ptr['s'], $slice) as $val) { - unset($ptr['s'][$val]); - } - - if ($slice === $ptr['i']) { - $ptr['c'] = 0; - } - } - } - - /** - * Load the slicemap for a given mailbox. The slicemap contains - * the uidvalidity information, the UIDs->slice lookup table, and any - * metadata that needs to be saved for the mailbox. - * - * @param string $mailbox The mailbox. - * @param integer $uidvalid The IMAP uidvalidity value of the mailbox. - * - * @return void - */ - protected function _loadSliceMap($mailbox, $uidvalid = null) { - if (!isset($this->_slicemap[$mailbox]) && - (($data = $this->_cache->get($this->_getCid($mailbox, 'slicemap'))) !== false)) { - try { - if (is_string($data) && - ($slice = @unserialize($data)) && - is_array($slice)) { - $this->_slicemap[$mailbox] = $slice; - } - } catch (Exception $e) { - } - } - - if (isset($this->_slicemap[$mailbox])) { - $ptr = &$this->_slicemap[$mailbox]; - if (is_null($ptr['d']['uidvalid'])) { - $ptr['d']['uidvalid'] = $uidvalid; - return; - } elseif (!is_null($uidvalid) && - ($ptr['d']['uidvalid'] !== $uidvalid)) { - $this->_deleteMailbox($mailbox); - } else { - return; - } - } - - $this->_slicemap[$mailbox] = [ - // Tracking count for purposes of determining slices - 'c' => 0, - // Metadata storage - // By default includes UIDVALIDITY of mailbox. - 'd' => ['uidvalid' => $uidvalid], - // The ID of the last slice. - 'i' => 0, - // The slice list. - 's' => [] - ]; - } - - /** - * Add update entry for a mailbox. - * - * @param string $mailbox The mailbox. - * @param string $type 'add', 'slice', or 'slicemap'. - * @param mixed $data The data to update. - * - * @return void - */ - protected function _toUpdate($mailbox, $type, $data): void { - if (!isset($this->_update[$mailbox])) { - $this->_update[$mailbox] = [ - 'add' => [], - 'slice' => [] - ]; - } - - $this->_update[$mailbox][$type] = ($type === 'slicemap') - ? $data - : array_merge($this->_update[$mailbox][$type], $data); - } - - /* Serializable methods. */ - - /** - */ - public function serialize() { - $this->save(); - return parent::serialize(); - } - - public function __serialize(): array { - $this->save(); - return parent::__serialize(); + $this->cachedMailboxes = []; } } diff --git a/lib/Cache/CachedMailbox.php b/lib/Cache/CachedMailbox.php new file mode 100644 index 0000000000..64a2b21435 --- /dev/null +++ b/lib/Cache/CachedMailbox.php @@ -0,0 +1,48 @@ +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; + } +} diff --git a/lib/Cache/HordeCacheFactory.php b/lib/Cache/HordeCacheFactory.php new file mode 100644 index 0000000000..902e701912 --- /dev/null +++ b/lib/Cache/HordeCacheFactory.php @@ -0,0 +1,32 @@ +messageMapper, + $this->mailboxMapper, + $this->syncTokenParser, + $account, + ); + } +} diff --git a/lib/Cache/HordeSyncToken.php b/lib/Cache/HordeSyncToken.php new file mode 100644 index 0000000000..2fbfb5bf8c --- /dev/null +++ b/lib/Cache/HordeSyncToken.php @@ -0,0 +1,31 @@ +nextUid; + } + + public function getUidValidity(): ?int { + return $this->uidValidity; + } + + public function getHighestModSeq(): ?int { + return $this->highestModSeq; + } +} diff --git a/lib/Cache/HordeSyncTokenParser.php b/lib/Cache/HordeSyncTokenParser.php new file mode 100644 index 0000000000..d1af892ff3 --- /dev/null +++ b/lib/Cache/HordeSyncTokenParser.php @@ -0,0 +1,36 @@ +cacheBackend = $params['cache']['backend']; - } - - parent::__construct($params); - } public function enableRateLimiter( IMemcache $cache, @@ -93,12 +82,4 @@ protected function _login() { throw $e; } } - - public function logout() { - if ($this->cacheBackend !== null) { - $this->cacheBackend->save(); - } - - parent::logout(); - } } diff --git a/lib/IMAP/IMAPClientFactory.php b/lib/IMAP/IMAPClientFactory.php index 3b1b97908a..69438ea5f9 100644 --- a/lib/IMAP/IMAPClientFactory.php +++ b/lib/IMAP/IMAPClientFactory.php @@ -24,11 +24,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; @@ -39,7 +38,6 @@ use function hash; use function implode; use function json_encode; -use function md5; class IMAPClientFactory { /** @var ICrypto */ @@ -55,17 +53,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; } /** @@ -126,21 +127,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'; } diff --git a/tests/Integration/Framework/ImapTest.php b/tests/Integration/Framework/ImapTest.php index aad747ba5f..0c1cb00471 100644 --- a/tests/Integration/Framework/ImapTest.php +++ b/tests/Integration/Framework/ImapTest.php @@ -225,6 +225,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' => 'user@domain.tld', + '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 diff --git a/tests/Integration/IMAP/IMAPClientFactoryTest.php b/tests/Integration/IMAP/IMAPClientFactoryTest.php index 8e6c682406..28fd34c53a 100644 --- a/tests/Integration/IMAP/IMAPClientFactoryTest.php +++ b/tests/Integration/IMAP/IMAPClientFactoryTest.php @@ -28,6 +28,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; @@ -54,6 +55,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(); @@ -63,6 +65,7 @@ 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, @@ -70,6 +73,7 @@ protected function setUp(): void { $this->cacheFactory, $this->eventDispatcher, $this->timeFactory, + $this->hordeCacheFactory, ); } diff --git a/tests/Integration/MailboxSynchronizationTest.php b/tests/Integration/MailboxSynchronizationTest.php index 31f0faa7f5..e0fcdc6a94 100644 --- a/tests/Integration/MailboxSynchronizationTest.php +++ b/tests/Integration/MailboxSynchronizationTest.php @@ -28,12 +28,15 @@ use OCA\Mail\Account; use OCA\Mail\Contracts\IMailManager; use OCA\Mail\Controller\MailboxesController; +use OCA\Mail\Db\MessageMapper as DbMessageMapper; use OCA\Mail\Service\AccountService; +use OCA\Mail\Service\Sync\ImapToDbSynchronizer; use OCA\Mail\Service\Sync\SyncService; use OCA\Mail\Tests\Integration\Framework\ImapTest; use OCA\Mail\Tests\Integration\Framework\ImapTestAccount; use OCP\IRequest; use OCP\Server; +use Psr\Log\LoggerInterface; class MailboxSynchronizationTest extends TestCase { use ImapTest, @@ -234,4 +237,66 @@ public function testSyncVanishedMessage() { // self::assertCount(0, $syncJson['changedMessages']); // self::assertCount(1, $syncJson['vanishedMessages'], 'Message does not show as vanished, possibly because UID and ID are mixed up above.'); } + + public function testUnsolicitedVanishedMessage() { + $mailbox = 'INBOX'; + $message = $this->getMessageBuilder() + ->from('ralph@buffington@domain.tld') + ->to('user@domain.tld') + ->subject('Msg 1') + ->finish(); + $uid1 = $this->saveMessage($mailbox, $message, $this->account); + $message = $this->getMessageBuilder() + ->from('ralph@buffington@domain.tld') + ->to('user@domain.tld') + ->subject('Msg 2') + ->finish(); + $uid2 = $this->saveMessage($mailbox, $message, $this->account); + /** @var IMailManager $mailManager */ + $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; + } + } + /** @var SyncService $syncService */ + $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, + [] + ); + + // Trigger partial sync to warm the cache. + $synchronizer = Server::get(ImapToDbSynchronizer::class); + $synchronizer->syncAccount( + new Account($this->account), + Server::get(LoggerInterface::class), + ); + + // Assert that there are 2 messages and nothing changes when deleting a message externally + $dbMessageMapper = Server::get(DbMessageMapper::class); + self::assertCount(2, $dbMessageMapper->findAllUids($inbox)); + $this->deleteMessagesExternally($mailbox, [$uid1]); + self::assertCount(2, $dbMessageMapper->findAllUids($inbox)); + + // Receive unsolicited vanished uid + $client = $this->getClient($this->account); + $mailManager->getSource( + $client, + new Account($this->account), + $mailbox, + $uid2, + ); + $client->logout(); + + // Assert that the unsolicited change was synced to the db + self::assertCount(1, $dbMessageMapper->findAllUids($inbox)); + } }