Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[stable4.0] fix(imap): persist vanished messages immediately on EXAMINE commands #10200

Merged
merged 1 commit into from
Sep 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
506 changes: 76 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 @@ -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();
}
}
27 changes: 8 additions & 19 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\Cache;
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,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;
}

/**
Expand Down Expand Up @@ -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';
}
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 @@ -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' => '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
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 @@ -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;
Expand All @@ -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();
Expand All @@ -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,
);
}

Expand Down
Loading
Loading