Skip to content

feat: store envelopes as list of threads #11529

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.

Sign up for GitHub

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

feat: store envelopes as list of threads #11529

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3dc5cfa
feat: store envelopes as list of threads
hamza221 Aug 19, 2025
0100ddb
fixup! feat: store envelopes as list of threads
hamza221 Aug 28, 2025
6e22993
fixup! feat: store envelopes as list of threads
hamza221 Sep 2, 2025
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
2 changes: 1 addition & 1 deletion lib/Contracts/IMailManager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ public function getImapMessage(Horde_Imap_Client_Socket $client,
*
* @return Message[]
*/
public function getThread(Account $account, string $threadRootId): array;
public function getThread(Account $account, string $threadRootId, string $sortOrder = IMailSearch::ORDER_NEWEST_FIRST): array;

/**
* @param Account $sourceAccount
Expand Down
4 changes: 2 additions & 2 deletions lib/Contracts/IMailSearch.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public function findMessage(Account $account,
* @param string|null $userId
* @param string|null $view
*
* @return Message[]
* @return Message[][]
*
* @throws ClientException
* @throws ServiceException
Expand All @@ -59,7 +59,7 @@ public function findMessages(Account $account,
/**
* Run a search through all mailboxes of a user.
*
* @return Message[]
* @return Message[][]
*
* @throws ClientException
* @throws ServiceException
Expand Down
96 changes: 92 additions & 4 deletions lib/Db/MessageMapper.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -756,10 +756,11 @@
/**
* @param Account $account
* @param string $threadRootId
* @param string $sortOrder
*
* @return Message[]
*/
public function findThread(Account $account, string $threadRootId): array {
public function findThread(Account $account, string $threadRootId, string $sortOrder): array {
$qb = $this->db->getQueryBuilder();
$qb->select('messages.*')
->from($this->getTableName(), 'messages')
Expand All @@ -768,7 +769,7 @@
$qb->expr()->eq('mailboxes.account_id', $qb->createNamedParameter($account->getId(), IQueryBuilder::PARAM_INT)),
$qb->expr()->eq('messages.thread_root_id', $qb->createNamedParameter($threadRootId, IQueryBuilder::PARAM_STR), IQueryBuilder::PARAM_STR)
)
->orderBy('messages.sent_at', 'desc');
->orderBy('messages.sent_at', $sortOrder);

return $this->findRelatedData($this->findEntities($qb), $account->getUserId());
}
Expand Down Expand Up @@ -1273,10 +1274,11 @@
* @param Mailbox $mailbox
* @param string $userId
* @param int[] $ids
* @param string $sortOrder
*
* @return Message[]
*/
public function findByMailboxAndIds(Mailbox $mailbox, string $userId, array $ids): array {
public function findByMailboxAndIds(Mailbox $mailbox, string $userId, array $ids, string $sortOrder): array {
if ($ids === []) {
return [];
}
Expand All @@ -1288,7 +1290,7 @@
$qb->expr()->eq('mailbox_id', $qb->createNamedParameter($mailbox->getId()), IQueryBuilder::PARAM_INT),
$qb->expr()->in('id', $qb->createParameter('ids'))
)
->orderBy('sent_at', 'desc');
->orderBy('sent_at', $sortOrder);

$results = [];
foreach (array_chunk($ids, 1000) as $chunk) {
Expand All @@ -1298,6 +1300,50 @@
return array_merge([], ...$results);
}

/**
* @param Account $account
* @param Mailbox $mailbox
* @param string $userId
* @param int[] $ids
* @param string $sortOrder
* @param bool $threadingEnabled
*
* @return Message[][]

Check failure on line 1311 in lib/Db/MessageMapper.php

View workflow job for this annotation

GitHub Actions / static-psalm-analysis dev-master

InvalidReturnType 

lib/Db/MessageMapper.php:1311:13: InvalidReturnType: The declared return type 'array<array-key, array<array-key, OCA\Mail\Db\Message>>' for OCA\Mail\Db\MessageMapper::findMessageListsByMailboxAndIds is incorrect, got 'array<array-key, OCA\Mail\Db\Message|array<array-key, OCA\Mail\Db\Message|list{OCA\Mail\Db\Message}>>' (see https://psalm.dev/011)
*/
public function findMessageListsByMailboxAndIds(Account $account, Mailbox $mailbox, string $userId, array $ids, string $sortOrder, bool $threadingEnabled = false): array {
if ($ids === []) {
return [];
}

$qb = $this->db->getQueryBuilder();
$qb->select('*')
->from($this->getTableName())
->where(
$qb->expr()->eq('mailbox_id', $qb->createNamedParameter($mailbox->getId()), IQueryBuilder::PARAM_INT),
$qb->expr()->in('id', $qb->createParameter('ids'))
)
->orderBy('sent_at', $sortOrder);
$results = [];
foreach (array_chunk($ids, 1000) as $chunk) {
$qb->setParameter('ids', $chunk, IQueryBuilder::PARAM_INT_ARRAY);
if ($threadingEnabled) {
$res = $qb->executeQuery();
while ($row = $res->fetch()) {
$message = $this->mapRowToEntity($row);
if ($message->getThreadRootId() === null) {
$results[] = [$message];
} else {
$results[] = $this->findThread($account, $message->getThreadRootId(), $sortOrder);

Check failure on line 1336 in lib/Db/MessageMapper.php

View workflow job for this annotation

GitHub Actions / static-psalm-analysis dev-master

PossiblyNullArgument 

lib/Db/MessageMapper.php:1336:48: PossiblyNullArgument: Argument 2 of OCA\Mail\Db\MessageMapper::findThread cannot be null, possibly null value provided (see https://psalm.dev/078)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the n+1 can be avoided with a subquery or self join

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When called from getDatabaseSyncChanges I wonder if it's even necessary to fetch messages of the same thread. We already know the new IDs. Other messages are not relevant, are they?

}
}
$res->closeCursor();
} else {
$results[] = array_map(fn (Message $msg) => [$msg], $this->findRelatedData($this->findEntities($qb), $userId));
}
}
return $threadingEnabled ? $results : array_merge([], ...$results);

Check failure on line 1344 in lib/Db/MessageMapper.php

View workflow job for this annotation

GitHub Actions / static-psalm-analysis dev-master

InvalidReturnStatement 

lib/Db/MessageMapper.php:1344:10: InvalidReturnStatement: The inferred type 'array<array-key, OCA\Mail\Db\Message|array<array-key, OCA\Mail\Db\Message|list{OCA\Mail\Db\Message}>>' does not match the declared return type 'array<array-key, array<array-key, OCA\Mail\Db\Message>>' for OCA\Mail\Db\MessageMapper::findMessageListsByMailboxAndIds (see https://psalm.dev/128)
}

/**
* @param string $userId
* @param int[] $ids
Expand Down Expand Up @@ -1325,6 +1371,48 @@
return array_merge([], ...$results);
}


/**
* @param Account $account
* @param string $userId
* @param int[] $ids
* @param string $sortOrder
*
* @return Message[][]

Check failure on line 1381 in lib/Db/MessageMapper.php

View workflow job for this annotation

GitHub Actions / static-psalm-analysis dev-master

InvalidReturnType 

lib/Db/MessageMapper.php:1381:13: InvalidReturnType: The declared return type 'array<array-key, array<array-key, OCA\Mail\Db\Message>>' for OCA\Mail\Db\MessageMapper::findMessageListsByIds is incorrect, got 'array<array-key, OCA\Mail\Db\Message|array<array-key, OCA\Mail\Db\Message|list{OCA\Mail\Db\Message}>>' (see https://psalm.dev/011)
*/
public function findMessageListsByIds(Account $account, string $userId, array $ids, string $sortOrder, bool $threadingEnabled = false): array {
if ($ids === []) {
return [];
}
$qb = $this->db->getQueryBuilder();
$qb->select('*')
->from($this->getTableName())
->where(
$qb->expr()->in('id', $qb->createParameter('ids'))
)
->orderBy('sent_at', $sortOrder);

$results = [];
foreach (array_chunk($ids, 1000) as $chunk) {
$qb->setParameter('ids', $chunk, IQueryBuilder::PARAM_INT_ARRAY);
if ($threadingEnabled) {
$res = $qb->executeQuery();
while ($row = $res->fetch()) {
$message = $this->mapRowToEntity($row);
if ($message->getThreadRootId() === null) {
$results[] = [$message];
} else {
$results[] = $this->findThread($account, $message->getThreadRootId(), $sortOrder);

Check failure on line 1405 in lib/Db/MessageMapper.php

View workflow job for this annotation

GitHub Actions / static-psalm-analysis dev-master

PossiblyNullArgument 

lib/Db/MessageMapper.php:1405:48: PossiblyNullArgument: Argument 2 of OCA\Mail\Db\MessageMapper::findThread cannot be null, possibly null value provided (see https://psalm.dev/078)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here

}
}
$res->closeCursor();
} else {
$results[] = array_map(fn (Message $msg) => [$msg], $this->findRelatedData($this->findEntities($qb), $userId));
}
}
return $threadingEnabled ? $results : array_merge([], ...$results);

Check failure on line 1413 in lib/Db/MessageMapper.php

View workflow job for this annotation

GitHub Actions / static-psalm-analysis dev-master

InvalidReturnStatement 

lib/Db/MessageMapper.php:1413:10: InvalidReturnStatement: The inferred type 'array<array-key, OCA\Mail\Db\Message|array<array-key, OCA\Mail\Db\Message|list{OCA\Mail\Db\Message}>>' does not match the declared return type 'array<array-key, array<array-key, OCA\Mail\Db\Message>>' for OCA\Mail\Db\MessageMapper::findMessageListsByIds (see https://psalm.dev/128)
}

/**
* @param Message[] $messages
*
Expand Down
4 changes: 2 additions & 2 deletions lib/IMAP/PreviewEnhancer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,12 @@
}

/**
* @param Message[] $messages
* @param Message[][] $messages
*
* @return Message[]
* @return Message[][]

Check failure on line 57 in lib/IMAP/PreviewEnhancer.php

View workflow job for this annotation

GitHub Actions / static-psalm-analysis dev-master

InvalidReturnType 

lib/IMAP/PreviewEnhancer.php:57:13: InvalidReturnType: The declared return type 'array<array-key, array<array-key, OCA\Mail\Db\Message>>' for OCA\Mail\IMAP\PreviewEnhancer::process is incorrect, got 'array<array-key, OCA\Mail\Db\Message|array<array-key, OCA\Mail\Db\Message>>' (see https://psalm.dev/011)
*/
public function process(Account $account, Mailbox $mailbox, array $messages, bool $preLoadAvatars = false, ?string $userId = null): array {
$needAnalyze = array_reduce($messages, static function (array $carry, Message $message) {

Check failure on line 60 in lib/IMAP/PreviewEnhancer.php

View workflow job for this annotation

GitHub Actions / static-psalm-analysis dev-master

InvalidArgument 

lib/IMAP/PreviewEnhancer.php:60:73: InvalidArgument: The second param of the closure passed to array_reduce must take array<array-key, OCA\Mail\Db\Message> but only accepts OCA\Mail\Db\Message (see https://psalm.dev/004)
if ($message->getStructureAnalyzed()) {
// Nothing to do
return $carry;
Expand Down
5 changes: 3 additions & 2 deletions lib/Service/MailManager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
use OCA\Mail\Account;
use OCA\Mail\Attachment;
use OCA\Mail\Contracts\IMailManager;
use OCA\Mail\Contracts\IMailSearch;
use OCA\Mail\Db\Mailbox;
use OCA\Mail\Db\MailboxMapper;
use OCA\Mail\Db\Message;
Expand Down Expand Up @@ -236,8 +237,8 @@ public function getImapMessagesForScheduleProcessing(Account $account,
}

#[\Override]
public function getThread(Account $account, string $threadRootId): array {
return $this->dbMessageMapper->findThread($account, $threadRootId);
public function getThread(Account $account, string $threadRootId, string $sortOrder = IMailSearch::ORDER_NEWEST_FIRST): array {
return $this->dbMessageMapper->findThread($account, $threadRootId, $sortOrder);
}

#[\Override]
Expand Down
31 changes: 19 additions & 12 deletions lib/Service/Search/MailSearch.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ public function findMessage(Account $account,
* @param int|null $limit
* @param string|null $view
*
* @return Message[]
* @return Message[][]
*
* @throws ClientException
* @throws ServiceException
Expand All @@ -102,8 +102,9 @@ public function findMessages(Account $account,
if ($cursor !== null) {
$query->setCursor($cursor);
}
$threadingEnabled = $view === self::VIEW_THREADED;
if ($view !== null) {
$query->setThreaded($view === self::VIEW_THREADED);
$query->setThreaded($threadingEnabled);
}
// In flagged we don't want anything but flagged messages
if ($mailbox->isSpecialUse(Horde_Imap_Client::SPECIALUSE_FLAGGED)) {
Expand All @@ -113,17 +114,23 @@ public function findMessages(Account $account,
if (!$mailbox->isSpecialUse(Horde_Imap_Client::SPECIALUSE_TRASH)) {
$query->addFlag(Flag::not(Flag::DELETED));
}

return $this->previewEnhancer->process(
$account,
$mailbox,
$this->messageMapper->findByIds($account->getUserId(),
$this->getIdsLocally($account, $mailbox, $query, $sortOrder, $limit),
$sortOrder,
),
true,
$userId
$messages = $this->messageMapper->findMessageListsByIds($account, $account->getUserId(),
$this->getIdsLocally($account, $mailbox, $query, $sortOrder, $limit),
$sortOrder,
$threadingEnabled
);
$processedMessages = [];
foreach ($messages as $messageList) {
$processedMessages[] = $this->previewEnhancer->process(
$account,
$mailbox,
$messageList,
true,
$userId
);
}

return $processedMessages;
}

/**
Expand Down
29 changes: 23 additions & 6 deletions lib/Service/Sync/SyncService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
use OCA\Mail\IMAP\Sync\Response;
use OCA\Mail\Service\Search\FilterStringParser;
use OCA\Mail\Service\Search\SearchQuery;
use OCP\IAppConfig;
use Psr\Log\LoggerInterface;
use function array_diff;
use function array_map;
Expand All @@ -50,21 +51,27 @@ class SyncService {
/** @var MailboxSync */
private $mailboxSync;

/** @var IAppConfig */
private $config;

public function __construct(
IMAPClientFactory $clientFactory,
ImapToDbSynchronizer $synchronizer,
FilterStringParser $filterStringParser,
MessageMapper $messageMapper,
PreviewEnhancer $previewEnhancer,
LoggerInterface $logger,
MailboxSync $mailboxSync) {
MailboxSync $mailboxSync,
IAppConfig $config,
) {
$this->clientFactory = $clientFactory;
$this->synchronizer = $synchronizer;
$this->filterStringParser = $filterStringParser;
$this->messageMapper = $messageMapper;
$this->previewEnhancer = $previewEnhancer;
$this->logger = $logger;
$this->mailboxSync = $mailboxSync;
$this->config = $config;
}

/**
Expand Down Expand Up @@ -129,6 +136,7 @@ public function syncMailbox(Account $account,

$this->mailboxSync->syncStats($client, $mailbox);

$threadingEnabled = $this->config->getValueString('mail', 'layout-message-view', 'threaded') === 'threaded';
$client->logout();

$query = $filter === null ? null : $this->filterStringParser->parse($filter);
Expand All @@ -138,7 +146,8 @@ public function syncMailbox(Account $account,
$knownIds ?? [],
$lastMessageTimestamp,
$sortOrder,
$query
$query,
$threadingEnabled
);
}

Expand All @@ -147,6 +156,7 @@ public function syncMailbox(Account $account,
* @param Mailbox $mailbox
* @param int[] $knownIds
* @param SearchQuery $query
* @param bool $threadingEnabled
*
* @return Response
* @todo does not work with text token search queries
Expand All @@ -157,7 +167,8 @@ private function getDatabaseSyncChanges(Account $account,
array $knownIds,
?int $lastMessageTimestamp,
string $sortOrder,
?SearchQuery $query): Response {
?SearchQuery $query,
bool $threadingEnabled): Response {
if ($knownIds === []) {
$newIds = $this->messageMapper->findAllIds($mailbox);
} else {
Expand All @@ -169,7 +180,13 @@ private function getDatabaseSyncChanges(Account $account,
$newUids = $this->messageMapper->findUidsForIds($mailbox, $newIds);
$newIds = $this->messageMapper->findIdsByQuery($mailbox, $query, $order, null, $newUids);
}
$new = $this->messageMapper->findByMailboxAndIds($mailbox, $account->getUserId(), $newIds);

$new = $this->messageMapper->findMessageListsByMailboxAndIds($account, $mailbox, $account->getUserId(), $newIds, $sortOrder, $threadingEnabled);

$newMessages = [];
foreach ($new as $messageList) {
$newMessages[] = $this->previewEnhancer->process($account, $mailbox, $messageList);
}

// TODO: $changed = $this->messageMapper->findChanged($account, $mailbox, $uids);
if ($query !== null) {
Expand All @@ -178,15 +195,15 @@ private function getDatabaseSyncChanges(Account $account,
} else {
$changedIds = $knownIds;
}
$changed = $this->messageMapper->findByMailboxAndIds($mailbox, $account->getUserId(), $changedIds);
$changed = $this->messageMapper->findByMailboxAndIds($mailbox, $account->getUserId(), $changedIds, $sortOrder);

$stillKnownIds = array_map(static function (Message $msg) {
return $msg->getId();
}, $changed);
$vanished = array_values(array_diff($knownIds, $stillKnownIds));

return new Response(
$this->previewEnhancer->process($account, $mailbox, $new),
$newMessages,
$changed,
$vanished,
$mailbox->getStats()
Expand Down
7 changes: 5 additions & 2 deletions src/components/Envelope.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@
<span class="envelope__subtitle__subject"
:class="{'one-line': oneLineLayout }"
dir="auto">
<span class="envelope__subtitle__subject__text" :class="{'one-line': oneLineLayout, draft }" v-html="subjectForSubtitle" />

Check warning on line 80 in src/components/Envelope.vue

View workflow job for this annotation

GitHub Actions / NPM lint 

'v-html' directive can lead to XSS attack
</span>
</div>
<div v-if="data.encrypted || data.previewText"
Expand Down Expand Up @@ -465,7 +465,7 @@
type: Boolean,
default: true,
},
data: {
threadList: {
type: Object,
required: true,
},
Expand Down Expand Up @@ -506,14 +506,17 @@
},
mounted() {
this.onWindowResize()

window.addEventListener('resize', this.onWindowResize)
},
// eslint-disable-next-line vue/order-in-components
computed: {
...mapStores(useMainStore),
...mapState(useMainStore, [
'isSnoozeDisabled',
]),
data() {
return Object.values(this.threadList)[0]
},
messageLongDate() {
return messageDateTime(new Date(this.data.dateInt))
},
Expand Down
Loading
Loading