Skip to content

Commit

Permalink
fixup! fix(imap): persist vanished messages immediately on EXAMINE co…
Browse files Browse the repository at this point in the history
…mmands
  • Loading branch information
st3iny committed Sep 11, 2024
1 parent 45f2854 commit 67a12d7
Show file tree
Hide file tree
Showing 9 changed files with 224 additions and 485 deletions.
512 changes: 87 additions & 425 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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,21 +12,21 @@
use OCA\Mail\Account;
use OCA\Mail\Db\MailboxMapper;
use OCA\Mail\Db\MessageMapper;
use OCP\ICache;

class CacheFactory {
class HordeCacheFactory {
public function __construct(
private MailboxMapper $mailboxMapper,
private MessageMapper $messageMapper,
private HordeSyncTokenParser $syncTokenParser,
) {
}

public function newCache(Account $account, ICache $cache): Cache {
public function newCache(Account $account): Cache {
return new Cache(
$this->messageMapper,
$this->mailboxMapper,
$this->syncTokenParser,
$account,
$cache,
);
}
}
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 @@ -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.
Expand All @@ -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,
Expand Down Expand Up @@ -76,12 +65,4 @@ protected function _login() {
throw $e;
}
}

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

parent::logout();
}
}
43 changes: 12 additions & 31 deletions lib/IMAP/IMAPClientFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -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\CacheFactory;
use OCA\Mail\Cache\HordeCacheFactory;
use OCA\Mail\Events\BeforeImapClientCreated;
use OCP\AppFramework\Utility\ITimeFactory;
use OCP\EventDispatcher\IEventDispatcher;
Expand All @@ -24,7 +23,6 @@
use function hash;
use function implode;
use function json_encode;
use function md5;

class IMAPClientFactory {
/** @var ICrypto */
Expand All @@ -40,14 +38,14 @@ class IMAPClientFactory {
private $eventDispatcher;

private ITimeFactory $timeFactory;
private CacheFactory $hordeCacheFactory;

public function __construct(ICrypto $crypto,
IConfig $config,
ICacheFactory $cacheFactory,
IEventDispatcher $eventDispatcher,
ITimeFactory $timeFactory,
CacheFactory $hordeCacheFactory) {
private HordeCacheFactory $hordeCacheFactory;

public function __construct(ICrypto $crypto,
IConfig $config,
ICacheFactory $cacheFactory,
IEventDispatcher $eventDispatcher,
ITimeFactory $timeFactory,
HordeCacheFactory $hordeCacheFactory) {
$this->crypto = $crypto;
$this->config = $config;
$this->cacheFactory = $cacheFactory;
Expand Down Expand Up @@ -114,26 +112,9 @@ public function getClient(Account $account, bool $useCache = true): Horde_Imap_C
json_encode($params)
]),
);
if ($useCache && $this->cacheFactory->isAvailable()) {
$params['cache'] = [
'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
* @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
4 changes: 2 additions & 2 deletions tests/Integration/Framework/Caching.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
namespace OCA\Mail\Tests\Integration\Framework;

use OC\Memcache\Factory;
use OCA\Mail\Cache\CacheFactory;
use OCA\Mail\Cache\HordeCacheFactory;
use OCA\Mail\IMAP\IMAPClientFactory;
use OCP\AppFramework\Utility\ITimeFactory;
use OCP\EventDispatcher\IEventDispatcher;
Expand Down Expand Up @@ -43,7 +43,7 @@ public static function getImapClientFactoryAndConfiguredCacheFactory(?ICrypto $c
$cacheFactory,
Server::get(IEventDispatcher::class),
Server::get(ITimeFactory::class),
Server::get(CacheFactory::class),
Server::get(HordeCacheFactory::class),
);
return [$imapClient, $cacheFactory];
}
Expand Down
6 changes: 3 additions & 3 deletions tests/Integration/IMAP/IMAPClientFactoryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
use Horde_Imap_Client_Socket;
use OC\Memcache\Redis;
use OCA\Mail\Account;
use OCA\Mail\Cache\CacheFactory;
use OCA\Mail\Cache\HordeCacheFactory;
use OCA\Mail\Db\MailAccount;
use OCA\Mail\IMAP\HordeImapClient;
use OCA\Mail\IMAP\IMAPClientFactory;
Expand All @@ -41,7 +41,7 @@ class IMAPClientFactoryTest extends TestCase {
private $factory;
private IEventDispatcher|MockObject $eventDispatcher;
private ITimeFactory|MockObject $timeFactory;
private CacheFactory|MockObject $hordeCacheFactory;
private HordeCacheFactory|MockObject $hordeCacheFactory;

protected function setUp(): void {
parent::setUp();
Expand All @@ -51,7 +51,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(CacheFactory::class);
$this->hordeCacheFactory = $this->createMock(HordeCacheFactory::class);

$this->factory = new IMAPClientFactory(
$this->crypto,
Expand Down

0 comments on commit 67a12d7

Please sign in to comment.