Skip to content

Commit

Permalink
fix(imap): persist vanished messages immediately on EXAMINE commands
Browse files Browse the repository at this point in the history
Signed-off-by: Richard Steinmetz <[email protected]>

[skip ci]
  • Loading branch information
st3iny committed Oct 7, 2024
1 parent 15fe30f commit 1015644
Show file tree
Hide file tree
Showing 10 changed files with 327 additions and 469 deletions.
507 changes: 77 additions & 430 deletions lib/Cache/Cache.php

Large diffs are not rendered by default.

48 changes: 48 additions & 0 deletions lib/Cache/CachedMailbox.php
Original file line number Diff line number Diff line change
@@ -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;
}
}
32 changes: 32 additions & 0 deletions lib/Cache/HordeCacheFactory.php
Original file line number Diff line number Diff line change
@@ -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,
);
}
}
31 changes: 31 additions & 0 deletions lib/Cache/HordeSyncToken.php
Original file line number Diff line number Diff line change
@@ -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;
}
}
36 changes: 36 additions & 0 deletions lib/Cache/HordeSyncTokenParser.php
Original file line number Diff line number Diff line change
@@ -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);
}
}
21 changes: 1 addition & 20 deletions lib/IMAP/HordeImapClient.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,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.
Expand All @@ -45,15 +43,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,
Expand Down Expand Up @@ -93,12 +82,4 @@ protected function _login() {
throw $e;
}
}

public function logout() {
if ($this->cacheBackend !== null) {
$this->cacheBackend->save();
}

parent::logout();
}
}
27 changes: 8 additions & 19 deletions lib/IMAP/IMAPClientFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -39,7 +38,6 @@
use function hash;
use function implode;
use function json_encode;
use function md5;

class IMAPClientFactory {
/** @var ICrypto */
Expand All @@ -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;
}

/**
Expand Down Expand Up @@ -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';
}
Expand Down
25 changes: 25 additions & 0 deletions tests/Integration/Framework/ImapTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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' => '[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
Expand Down
4 changes: 4 additions & 0 deletions tests/Integration/IMAP/IMAPClientFactoryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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();
Expand All @@ -63,13 +65,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,
);
}

Expand Down
Loading

0 comments on commit 1015644

Please sign in to comment.