Fix lower bound when fetching a range of IMAP messages for the sync
Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>
This commit is contained in:
Christoph Wurst
Родитель
823a25b41c
Коммит
8fafd264b2
|
|
@ -71,18 +71,18 @@ class MessageMapper {
|
|||
|
||||
/**
|
||||
* @param Horde_Imap_Client_Socket $client
|
||||
* @param Mailbox $mailbox
|
||||
* @param string $mailbox
|
||||
*
|
||||
* @param int $maxResults
|
||||
* @param int|null $highestKnownUid
|
||||
* @param int $highestKnownUid
|
||||
*
|
||||
* @return array
|
||||
* @throws Horde_Imap_Client_Exception
|
||||
*/
|
||||
public function findAll(Horde_Imap_Client_Socket $client,
|
||||
Mailbox $mailbox,
|
||||
string $mailbox,
|
||||
int $maxResults,
|
||||
?int $highestKnownUid = 0): array {
|
||||
int $highestKnownUid): array {
|
||||
/**
|
||||
* To prevent memory exhaustion, we don't want to just ask for a list of
|
||||
* all UIDs and limit them client-side. Instead we can (hopefully
|
||||
|
|
@ -94,7 +94,7 @@ class MessageMapper {
|
|||
*/
|
||||
|
||||
$metaResults = $client->search(
|
||||
$mailbox->getName(),
|
||||
$mailbox,
|
||||
null,
|
||||
[
|
||||
'results' => [
|
||||
|
|
@ -125,11 +125,17 @@ class MessageMapper {
|
|||
// +1 is added to fetch all messages with the rare case of strictly
|
||||
// continuous UIDs and fractions
|
||||
$estimatedPageSize = (int)(($totalRange / $total) * $maxResults) + 1;
|
||||
// Determine min UID to fetch, but don't exceed the known maximum
|
||||
$lower = max(
|
||||
$min,
|
||||
($highestKnownUid ?? 0) + 1
|
||||
);
|
||||
// Determine max UID to fetch, but don't exceed the known maximum
|
||||
$upper = min(
|
||||
$max,
|
||||
$highestKnownUid + $estimatedPageSize
|
||||
$lower + $estimatedPageSize
|
||||
);
|
||||
$this->logger->debug("Built range for findAll: min=$min max=$max total=$total totalRange=$totalRange estimatedPageSize=$estimatedPageSize lower=$lower upper=$upper highestKnownUid=$highestKnownUid");
|
||||
|
||||
$query = new Horde_Imap_Client_Fetch_Query();
|
||||
$query->uid();
|
||||
|
|
@ -140,10 +146,10 @@ class MessageMapper {
|
|||
return $data->getUid();
|
||||
},
|
||||
iterator_to_array($client->fetch(
|
||||
$mailbox->getName(),
|
||||
$mailbox,
|
||||
$query,
|
||||
[
|
||||
'ids' => new Horde_Imap_Client_Ids(($highestKnownUid + 1) . ':' . $upper)
|
||||
'ids' => new Horde_Imap_Client_Ids($lower . ':' . $upper)
|
||||
]
|
||||
))
|
||||
),
|
||||
|
|
@ -159,7 +165,7 @@ class MessageMapper {
|
|||
return [
|
||||
'messages' => $this->findByIds(
|
||||
$client,
|
||||
$mailbox->getName(),
|
||||
$mailbox,
|
||||
$uidsToFetch
|
||||
),
|
||||
'all' => $upper === $max,
|
||||
|
|
|
|||
|
|
@ -175,7 +175,12 @@ class ImapToDbSynchronizer {
|
|||
$highestKnownUid = $this->dbMapper->findHighestUid($mailbox);
|
||||
$client = $this->clientFactory->getClient($account);
|
||||
try {
|
||||
$imapMessages = $this->imapMapper->findAll($client, $mailbox, self::MAX_NEW_MESSAGES, $highestKnownUid);
|
||||
$imapMessages = $this->imapMapper->findAll(
|
||||
$client,
|
||||
$mailbox->getName(),
|
||||
self::MAX_NEW_MESSAGES,
|
||||
$highestKnownUid ?? 0
|
||||
);
|
||||
$perf->step('fetch all messages from IMAP');
|
||||
} catch (Horde_Imap_Client_Exception $e) {
|
||||
throw new ServiceException('Can not get messages from mailbox ' . $mailbox->getName() . ': ' . $e->getMessage(), 0, $e);
|
||||
|
|
|
|||
|
|
@ -1,5 +1,7 @@
|
|||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
/**
|
||||
* @author Christoph Wurst <christoph@winzerhof-wurst.at>
|
||||
*
|
||||
|
|
@ -22,9 +24,12 @@
|
|||
namespace OCA\Mail\Tests\Unit\IMAP;
|
||||
|
||||
use ChristophWurst\Nextcloud\Testing\TestCase;
|
||||
use Horde_Imap_Client_Base;
|
||||
use Horde_Imap_Client;
|
||||
use Horde_Imap_Client_Data_Fetch;
|
||||
use Horde_Imap_Client_Fetch_Query;
|
||||
use Horde_Imap_Client_Fetch_Results;
|
||||
use Horde_Imap_Client_Ids;
|
||||
use Horde_Imap_Client_Socket;
|
||||
use OCA\Mail\IMAP\MessageMapper;
|
||||
use OCA\Mail\Model\IMAPMessage;
|
||||
use OCP\ILogger;
|
||||
|
|
@ -49,7 +54,8 @@ class MessageMapperTest extends TestCase {
|
|||
}
|
||||
|
||||
public function testGetByIds() {
|
||||
$imapClient = $this->createMock(Horde_Imap_Client_Base::class);
|
||||
/** @var Horde_Imap_Client_Socket|MockObject $imapClient */
|
||||
$imapClient = $this->createMock(Horde_Imap_Client_Socket::class);
|
||||
$mailbox = 'inbox';
|
||||
$ids = [1, 3];
|
||||
|
||||
|
|
@ -78,4 +84,139 @@ class MessageMapperTest extends TestCase {
|
|||
|
||||
$this->assertEquals($expected, $result);
|
||||
}
|
||||
|
||||
public function testFindAllEmptyMailbox(): void {
|
||||
/** @var Horde_Imap_Client_Socket|MockObject $client */
|
||||
$client = $this->createMock(Horde_Imap_Client_Socket::class);
|
||||
$mailbox = 'inbox';
|
||||
$client->expects($this->once())
|
||||
->method('search')
|
||||
->with(
|
||||
$mailbox,
|
||||
null,
|
||||
[
|
||||
'results' => [
|
||||
Horde_Imap_Client::SEARCH_RESULTS_MIN,
|
||||
Horde_Imap_Client::SEARCH_RESULTS_MAX,
|
||||
Horde_Imap_Client::SEARCH_RESULTS_COUNT,
|
||||
]
|
||||
]
|
||||
)
|
||||
->willReturn([
|
||||
'min' => 0,
|
||||
'max' => 0,
|
||||
'count' => 0,
|
||||
]);
|
||||
$client->expects($this->never())
|
||||
->method('fetch');
|
||||
|
||||
$result = $this->mapper->findAll(
|
||||
$client,
|
||||
$mailbox,
|
||||
5000,
|
||||
0
|
||||
);
|
||||
|
||||
$this->assertSame(
|
||||
[
|
||||
'messages' => [],
|
||||
'all' => true,
|
||||
],
|
||||
$result
|
||||
);
|
||||
}
|
||||
|
||||
public function testFindAllNoKnownUid(): void {
|
||||
/** @var Horde_Imap_Client_Socket|MockObject $client */
|
||||
$client = $this->createMock(Horde_Imap_Client_Socket::class);
|
||||
$mailbox = 'inbox';
|
||||
$client->expects($this->once())
|
||||
->method('search')
|
||||
->with(
|
||||
$mailbox,
|
||||
null,
|
||||
[
|
||||
'results' => [
|
||||
Horde_Imap_Client::SEARCH_RESULTS_MIN,
|
||||
Horde_Imap_Client::SEARCH_RESULTS_MAX,
|
||||
Horde_Imap_Client::SEARCH_RESULTS_COUNT,
|
||||
]
|
||||
]
|
||||
)
|
||||
->willReturn([
|
||||
'min' => 123,
|
||||
'max' => 321,
|
||||
'count' => 50,
|
||||
]);
|
||||
$query = new Horde_Imap_Client_Fetch_Query();
|
||||
$query->uid();
|
||||
$uidResults = new Horde_Imap_Client_Fetch_Results();
|
||||
$client->expects($this->at(1))
|
||||
->method('fetch')
|
||||
->with(
|
||||
$mailbox,
|
||||
$query,
|
||||
[
|
||||
'ids' => new Horde_Imap_Client_Ids('123:321'),
|
||||
]
|
||||
)->willReturn($uidResults);
|
||||
$bodyResults = new Horde_Imap_Client_Fetch_Results();
|
||||
$client->expects($this->at(2))
|
||||
->method('fetch')
|
||||
->willReturn($bodyResults);
|
||||
|
||||
$this->mapper->findAll(
|
||||
$client,
|
||||
$mailbox,
|
||||
5000,
|
||||
0
|
||||
);
|
||||
}
|
||||
|
||||
public function testFindAllWithKnownUid(): void {
|
||||
/** @var Horde_Imap_Client_Socket|MockObject $client */
|
||||
$client = $this->createMock(Horde_Imap_Client_Socket::class);
|
||||
$mailbox = 'inbox';
|
||||
$client->expects($this->once())
|
||||
->method('search')
|
||||
->with(
|
||||
$mailbox,
|
||||
null,
|
||||
[
|
||||
'results' => [
|
||||
Horde_Imap_Client::SEARCH_RESULTS_MIN,
|
||||
Horde_Imap_Client::SEARCH_RESULTS_MAX,
|
||||
Horde_Imap_Client::SEARCH_RESULTS_COUNT,
|
||||
]
|
||||
]
|
||||
)
|
||||
->willReturn([
|
||||
'min' => 123,
|
||||
'max' => 321,
|
||||
'count' => 50,
|
||||
]);
|
||||
$query = new Horde_Imap_Client_Fetch_Query();
|
||||
$query->uid();
|
||||
$uidResults = new Horde_Imap_Client_Fetch_Results();
|
||||
$client->expects($this->at(1))
|
||||
->method('fetch')
|
||||
->with(
|
||||
$mailbox,
|
||||
$query,
|
||||
[
|
||||
'ids' => new Horde_Imap_Client_Ids('301:321'),
|
||||
]
|
||||
)->willReturn($uidResults);
|
||||
$bodyResults = new Horde_Imap_Client_Fetch_Results();
|
||||
$client->expects($this->at(2))
|
||||
->method('fetch')
|
||||
->willReturn($bodyResults);
|
||||
|
||||
$this->mapper->findAll(
|
||||
$client,
|
||||
$mailbox,
|
||||
5000,
|
||||
300
|
||||
);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Загрузка…
Ссылка в новой задаче