diff --git a/lib/IMAP/MessageMapper.php b/lib/IMAP/MessageMapper.php index 6e5d610f3e..6996afc8a9 100644 --- a/lib/IMAP/MessageMapper.php +++ b/lib/IMAP/MessageMapper.php @@ -51,9 +51,12 @@ use function count; use function fclose; use function in_array; +use function is_array; use function iterator_to_array; use function max; use function min; +use function OCA\Mail\array_flat_map; +use function OCA\Mail\chunk_uid_sequence; use function sprintf; class MessageMapper { @@ -239,7 +242,7 @@ static function (int $uid) use ($highestKnownUid) { /** * @param Horde_Imap_Client_Base $client * @param string $mailbox - * @param Horde_Imap_Client_Ids $ids + * @param int[]|Horde_Imap_Client_Ids $ids * @param string $userId * @param bool $loadBody * @return IMAPMessage[] @@ -252,7 +255,7 @@ static function (int $uid) use ($highestKnownUid) { */ public function findByIds(Horde_Imap_Client_Base $client, string $mailbox, - Horde_Imap_Client_Ids $ids, + $ids, string $userId, bool $loadBody = false): array { $query = new Horde_Imap_Client_Fetch_Query(); @@ -267,10 +270,20 @@ public function findByIds(Horde_Imap_Client_Base $client, ] ); - /** @var Horde_Imap_Client_Data_Fetch[] $fetchResults */ - $fetchResults = iterator_to_array($client->fetch($mailbox, $query, [ - 'ids' => $ids, - ]), false); + if (is_array($ids)) { + // Chunk to prevent overly long IMAP commands + /** @var Horde_Imap_Client_Data_Fetch[] $fetchResults */ + $fetchResults = array_flat_map(function ($ids) use ($query, $mailbox, $client) { + return iterator_to_array($client->fetch($mailbox, $query, [ + 'ids' => $ids, + ]), false); + }, chunk_uid_sequence($ids, 10000)); + } else { + /** @var Horde_Imap_Client_Data_Fetch[] $fetchResults */ + $fetchResults = iterator_to_array($client->fetch($mailbox, $query, [ + 'ids' => $ids, + ]), false); + } $fetchResults = array_values(array_filter($fetchResults, static function (Horde_Imap_Client_Data_Fetch $fetchResult) { return $fetchResult->exists(Horde_Imap_Client::FETCH_ENVELOPE); @@ -281,7 +294,11 @@ public function findByIds(Horde_Imap_Client_Base $client, } else { $minFetched = $fetchResults[0]->getUid(); $maxFetched = $fetchResults[count($fetchResults) - 1]->getUid(); - $range = $ids->range_string; + if ($ids instanceof Horde_Imap_Client_Ids) { + $range = $ids->range_string; + } else { + $range = 'literals'; + } $this->logger->debug("findByIds in $mailbox got " . count($ids) . " UIDs ($range) and found " . count($fetchResults) . ". minFetched=$minFetched maxFetched=$maxFetched"); } diff --git a/lib/IMAP/Sync/Synchronizer.php b/lib/IMAP/Sync/Synchronizer.php index 7e4c4dc711..01ace85cee 100644 --- a/lib/IMAP/Sync/Synchronizer.php +++ b/lib/IMAP/Sync/Synchronizer.php @@ -97,8 +97,8 @@ public function sync(Horde_Imap_Client_Base $imapClient, throw $e; } - $newMessages = $this->messageMapper->findByIds($imapClient, $request->getMailbox(), new Horde_Imap_Client_Ids($newUids), $userId); - $changedMessages = $this->messageMapper->findByIds($imapClient, $request->getMailbox(), new Horde_Imap_Client_Ids($changedUids), $userId); + $newMessages = $this->messageMapper->findByIds($imapClient, $request->getMailbox(), $newUids, $userId); + $changedMessages = $this->messageMapper->findByIds($imapClient, $request->getMailbox(), $changedUids, $userId); $vanishedMessageUids = $vanishedUids; return new Response($newMessages, $changedMessages, $vanishedMessageUids); diff --git a/lib/Service/MailManager.php b/lib/Service/MailManager.php index 7eb3fd2e1a..94fa3b2574 100644 --- a/lib/Service/MailManager.php +++ b/lib/Service/MailManager.php @@ -27,7 +27,6 @@ use Horde_Imap_Client; use Horde_Imap_Client_Exception; use Horde_Imap_Client_Exception_NoSupportExtension; -use Horde_Imap_Client_Ids; use Horde_Imap_Client_Socket; use Horde_Mime_Exception; use OCA\Mail\Account; @@ -221,7 +220,7 @@ public function getImapMessagesForScheduleProcessing(Account $account, return $this->imapMessageMapper->findByIds( $client, $mailbox->getName(), - new Horde_Imap_Client_Ids($uids), + $uids, $account->getUserId(), true ); diff --git a/tests/Unit/IMAP/MessageMapperTest.php b/tests/Unit/IMAP/MessageMapperTest.php index d727cad1b7..057d96ca56 100644 --- a/tests/Unit/IMAP/MessageMapperTest.php +++ b/tests/Unit/IMAP/MessageMapperTest.php @@ -142,6 +142,39 @@ public function testGetByIds(): void { $this->assertEquals($expected, $result); } + public function testGetByIdsWithManyMessages(): void { + /** @var Horde_Imap_Client_Socket|MockObject $imapClient */ + $imapClient = $this->createMock(Horde_Imap_Client_Socket::class); + $mailbox = 'inbox'; + $ids = range(1, 10000, 2); + $userId = 'user'; + $loadBody = false; + $fetchResults = new Horde_Imap_Client_Fetch_Results(); + $fetchResult1 = $this->createMock(Horde_Imap_Client_Data_Fetch::class); + $fetchResult2 = $this->createMock(Horde_Imap_Client_Data_Fetch::class); + $imapClient->expects(self::exactly(3)) + ->method('fetch') + ->willReturnOnConsecutiveCalls( + $fetchResults, + $fetchResults, + $fetchResults + ); + $fetchResults[0] = $fetchResult1; + $fetchResults[1] = $fetchResult2; + $fetchResult1->method('getUid') + ->willReturn(1); + $fetchResult2->method('getUid') + ->willReturn(3); + + $this->mapper->findByIds( + $imapClient, + $mailbox, + $ids, + $userId, + $loadBody + ); + } + public function testGetByIdsWithEmpty(): void { /** @var Horde_Imap_Client_Socket|MockObject $imapClient */ $imapClient = $this->createMock(Horde_Imap_Client_Socket::class);