diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index c64d6130b9..aeb49f13c4 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -32,6 +32,7 @@ use OCA\Mail\Contracts\IUserPreferences; use OCA\Mail\Events\BeforeMessageDeletedEvent; use OCA\Mail\Events\DraftSavedEvent; +use OCA\Mail\Events\SynchronizationEvent; use OCA\Mail\Events\MessageDeletedEvent; use OCA\Mail\Events\MessageFlaggedEvent; use OCA\Mail\Events\MessageSentEvent; @@ -44,6 +45,7 @@ use OCA\Mail\Listener\DraftMailboxCreatorListener; use OCA\Mail\Listener\FlagRepliedMessageListener; use OCA\Mail\Listener\InteractionListener; +use OCA\Mail\Listener\AccountSynchronizedThreadUpdaterListener; use OCA\Mail\Listener\MessageCacheUpdaterListener; use OCA\Mail\Listener\NewMessageClassificationListener; use OCA\Mail\Listener\SaveSentMessageListener; @@ -111,6 +113,7 @@ private function registerEvents(IAppContainer $container): void { $dispatcher->addServiceListener(MessageSentEvent::class, SaveSentMessageListener::class); $dispatcher->addServiceListener(NewMessagesSynchronized::class, NewMessageClassificationListener::class); $dispatcher->addServiceListener(SaveDraftEvent::class, DraftMailboxCreatorListener::class); + $dispatcher->addServiceListener(SynchronizationEvent::class, AccountSynchronizedThreadUpdaterListener::class); $dispatcher->addServiceListener(UserDeletedEvent::class, UserDeletedListener::class); } } diff --git a/lib/Db/MessageMapper.php b/lib/Db/MessageMapper.php index 1c1b57407a..b85830893e 100644 --- a/lib/Db/MessageMapper.php +++ b/lib/Db/MessageMapper.php @@ -26,8 +26,10 @@ namespace OCA\Mail\Db; use Horde_Imap_Client; +use OCA\Mail\Account; use OCA\Mail\Address; use OCA\Mail\AddressList; +use OCA\Mail\IMAP\Threading\DatabaseMessage; use OCA\Mail\Service\Search\SearchQuery; use OCP\AppFramework\Db\QBMapper; use OCP\AppFramework\Utility\ITimeFactory; @@ -86,6 +88,34 @@ public function findAllUids(Mailbox $mailbox): array { return $this->findUids($query); } + public function findThreadingData(Account $account): array { + $mailboxesQuery = $this->db->getQueryBuilder(); + $messagesQuery = $this->db->getQueryBuilder(); + + $mailboxesQuery->select('id') + ->from('mail_mailboxes') + ->where($mailboxesQuery->expr()->eq('account_id', $messagesQuery->createNamedParameter($account->getId(), IQueryBuilder::PARAM_INT), IQueryBuilder::PARAM_INT)); + $messagesQuery->select('id', 'subject', 'message_id', 'in_reply_to', 'references', 'thread_root_id') + ->from($this->getTableName()) + ->where($messagesQuery->expr()->in('mailbox_id', $messagesQuery->createFunction($mailboxesQuery->getSQL()), IQueryBuilder::PARAM_INT_ARRAY)) + ->andWhere($messagesQuery->expr()->isNotNull('message_id')); + + $result = $messagesQuery->execute(); + $uids = array_map(function (array $row) { + return DatabaseMessage::fromRowData( + (int) $row['id'], + $row['subject'], + $row['message_id'], + $row['in_reply_to'], + $row['references'], + $row['thread_root_id'] + ); + }, $result->fetchAll()); + $result->closeCursor(); + + return $uids; + } + public function insertBulk(Message ...$messages): void { $this->db->beginTransaction(); diff --git a/lib/Events/SynchronizationEvent.php b/lib/Events/SynchronizationEvent.php new file mode 100644 index 0000000000..12e51968a7 --- /dev/null +++ b/lib/Events/SynchronizationEvent.php @@ -0,0 +1,45 @@ + + * + * @author 2020 Christoph Wurst + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +namespace OCA\Mail\Events; + +use OCA\Mail\Account; +use OCP\EventDispatcher\Event; + +class SynchronizationEvent extends Event { + + /** @var Account */ + private $account; + + public function __construct(Account $account) { + parent::__construct(); + + $this->account = $account; + } + + public function getAccount(): Account { + return $this->account; + } +} diff --git a/lib/IMAP/Threading/DatabaseMessage.php b/lib/IMAP/Threading/DatabaseMessage.php new file mode 100644 index 0000000000..7d53473686 --- /dev/null +++ b/lib/IMAP/Threading/DatabaseMessage.php @@ -0,0 +1,89 @@ + + * + * @author 2020 Christoph Wurst + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +namespace OCA\Mail\IMAP\Threading; + +use Horde_Mail_Rfc822_Identification; +use function json_decode; + +class DatabaseMessage extends Message { + + /** @var int */ + private $databaseId; + + /** @var string|null */ + private $threadRootId; + + /** @var bool */ + private $dirty = false; + + public function __construct(int $databaseId, + string $subject, + string $id, + array $references, + ?string $threadRootId) { + parent::__construct($subject, $id, $references); + + $this->databaseId = $databaseId; + $this->threadRootId = $threadRootId; + } + + public static function fromRowData(int $id, + string $subject, + ?string $messageId, + ?string $references, + ?string $inReplyTo, + ?string $threadRootId): self { + $referencesForThreading = $references !== null ? json_decode($references, true) : []; + if (!empty($inReplyTo)) { + $referencesForThreading[] = $inReplyTo; + } + + return new self( + $id, + $subject, + $messageId, + $referencesForThreading, + $threadRootId + ); + } + + public function getDatabaseId(): int { + return $this->databaseId; + } + + public function getThreadRootId(): ?string { + return $this->threadRootId; + } + + public function setThreadRootId(?string $threadRootId): void { + $this->threadRootId = $threadRootId; + $this->dirty = true; + } + + public function isDirty(): bool { + return $this->dirty; + } +} diff --git a/lib/IMAP/Threading/ThreadBuilder.php b/lib/IMAP/Threading/ThreadBuilder.php index dee6c471a4..ae5f906685 100644 --- a/lib/IMAP/Threading/ThreadBuilder.php +++ b/lib/IMAP/Threading/ThreadBuilder.php @@ -25,32 +25,48 @@ namespace OCA\Mail\IMAP\Threading; +use OCA\Mail\Support\PerformanceLogger; use function array_key_exists; use function count; class ThreadBuilder { + /** @var PerformanceLogger */ + private $performanceLogger; + + public function __construct(PerformanceLogger $performanceLogger) { + $this->performanceLogger = $performanceLogger; + } + /** * @param Message[] $messages * * @return Container[] */ public function build(array $messages): array { + $log = $this->performanceLogger->start('Threading ' . count($messages) . ' messages'); + // Step 1 $idTable = $this->buildIdTable($messages); + $log->step('build ID table'); // Step 2 $rootContainer = $this->buildRootContainer($idTable); + $log->step('build root container'); // Step 3 unset($idTable); + $log->step('free ID table'); // Step 4 $this->pruneContainers($rootContainer); + $log->step('prune containers'); // Step 5 $this->groupBySubject($rootContainer); + $log->step('group by subject'); + $log->end(); // Return the children with reset numeric keys return array_values($rootContainer->getChildren()); } @@ -81,7 +97,9 @@ private function buildIdTable($messages): array { if ($refContainer === null) { $refContainer = $idTable[$reference] = Container::empty(); } - if (!$refContainer->hasParent()) { + if (!$refContainer->hasParent() + && !($parent !== null && !$parent->hasAncestor($refContainer)) + && !($parent !== null && !$refContainer->hasAncestor($parent))) { // TODO: Do not add a link if adding that link would introduce a loop: that is, before asserting A->B, search down the children of B to see if A is reachable, and also search down the children of A to see if B is reachable. If either is already reachable as a child of the other, don't add the link. $refContainer->setParent($parent); } diff --git a/lib/Listener/AccountSynchronizedThreadUpdaterListener.php b/lib/Listener/AccountSynchronizedThreadUpdaterListener.php new file mode 100644 index 0000000000..f0fc2ba8e4 --- /dev/null +++ b/lib/Listener/AccountSynchronizedThreadUpdaterListener.php @@ -0,0 +1,57 @@ + + * + * @author 2020 Christoph Wurst + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +namespace OCA\Mail\Listener; + +use OCA\Mail\Db\MessageMapper; +use OCA\Mail\Events\SynchronizationEvent; +use OCA\Mail\IMAP\Threading\ThreadBuilder; +use OCP\EventDispatcher\Event; +use OCP\EventDispatcher\IEventListener; + +class AccountSynchronizedThreadUpdaterListener implements IEventListener { + + /** @var MessageMapper */ + private $mapper; + + /** @var ThreadBuilder */ + private $builder; + + public function __construct(MessageMapper $mapper, + ThreadBuilder $builder) { + $this->mapper = $mapper; + $this->builder = $builder; + } + + public function handle(Event $event): void { + if (!($event instanceof SynchronizationEvent)) { + // Unrelated + return; + } + + $messages = $this->mapper->findThreadingData($event->getAccount()); + $threads = $this->builder->build($messages); + } +} diff --git a/lib/Service/Sync/ImapToDbSynchronizer.php b/lib/Service/Sync/ImapToDbSynchronizer.php index 2f17ed779c..b1a643b0dc 100644 --- a/lib/Service/Sync/ImapToDbSynchronizer.php +++ b/lib/Service/Sync/ImapToDbSynchronizer.php @@ -31,6 +31,7 @@ use OCA\Mail\Db\Mailbox; use OCA\Mail\Db\MailboxMapper; use OCA\Mail\Db\MessageMapper as DatabaseMessageMapper; +use OCA\Mail\Events\SynchronizationEvent; use OCA\Mail\Events\NewMessagesSynchronized; use OCA\Mail\Exception\ClientException; use OCA\Mail\Exception\IncompleteSyncException; @@ -115,9 +116,15 @@ public function syncAccount(Account $account, $mailbox, $criteria, null, - $force + $force, + true ); } + $this->dispatcher->dispatchTyped( + new SynchronizationEvent( + $account + ) + ); } /** @@ -166,8 +173,6 @@ private function resetCache(Account $account, Mailbox $mailbox): void { } /** - * @param int[] $knownUids - * * @throws ClientException * @throws MailboxNotCachedException * @throws ServiceException @@ -176,7 +181,8 @@ public function sync(Account $account, Mailbox $mailbox, int $criteria = Horde_Imap_Client::SYNC_NEWMSGSUIDS | Horde_Imap_Client::SYNC_FLAGSUIDS | Horde_Imap_Client::SYNC_VANISHEDUIDS, array $knownUids = null, - bool $force = false): void { + bool $force = false, + bool $batchSync = false): void { if ($mailbox->getSelectable() === false) { return; } @@ -222,6 +228,14 @@ public function sync(Account $account, $this->mailboxMapper->unlockFromNewSync($mailbox); } } + + if (!$batchSync) { + $this->dispatcher->dispatchTyped( + new SynchronizationEvent( + $account + ) + ); + } } /** diff --git a/tests/Unit/IMAP/Threading/ContainerTest.php b/tests/Unit/IMAP/Threading/ContainerTest.php index 65a56361ab..de29391755 100644 --- a/tests/Unit/IMAP/Threading/ContainerTest.php +++ b/tests/Unit/IMAP/Threading/ContainerTest.php @@ -81,6 +81,29 @@ public function testReSetParent(): void { $this->assertCount(1, $parent2->getChildren()); } + public function testHasNoAncestor(): void { + $unrelated = Container::empty(); + $container = Container::empty(); + + $hasAncestor = $container->hasAncestor($unrelated); + + $this->assertFalse($hasAncestor); + } + + public function testHasAncestor(): void { + $grandmother = Container::empty(); + $mother = Container::empty(); + $container = Container::empty(); + $container->setParent($mother); + $mother->setParent($grandmother); + + $hasMother = $container->hasAncestor($mother); + $hasGrandMother = $container->hasAncestor($grandmother); + + $this->assertTrue($hasMother); + $this->assertTrue($hasGrandMother); + } + public function testUnlink(): void { $parent = Container::empty(); $container = Container::empty(); diff --git a/tests/Unit/IMAP/Threading/ThreadBuilderTest.php b/tests/Unit/IMAP/Threading/ThreadBuilderTest.php index 4f15fc4329..87189ed029 100644 --- a/tests/Unit/IMAP/Threading/ThreadBuilderTest.php +++ b/tests/Unit/IMAP/Threading/ThreadBuilderTest.php @@ -29,16 +29,25 @@ use OCA\Mail\IMAP\Threading\Container; use OCA\Mail\IMAP\Threading\Message; use OCA\Mail\IMAP\Threading\ThreadBuilder; +use OCA\Mail\Support\PerformanceLogger; +use PHPUnit\Framework\MockObject\MockObject; class ThreadBuilderTest extends TestCase { + /** @var PerformanceLogger|MockObject */ + private $performanceLogger; + /** @var ThreadBuilder */ private $builder; protected function setUp(): void { parent::setUp(); - $this->builder = new ThreadBuilder(); + $this->performanceLogger = $this->createMock(PerformanceLogger::class); + + $this->builder = new ThreadBuilder( + $this->performanceLogger + ); } /**