Skip to content

Commit

Permalink
add sorting capabilities to mail
Browse files Browse the repository at this point in the history
Signed-off-by: hamza221 <[email protected]>
  • Loading branch information
hamza221 committed Nov 8, 2023
1 parent a91bdc0 commit f6fa141
Show file tree
Hide file tree
Showing 20 changed files with 299 additions and 139 deletions.
4 changes: 4 additions & 0 deletions lib/Contracts/IMailSearch.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@
use OCP\IUser;

interface IMailSearch {
public const ORDER_NEWEST_FIRST = 'DESC';
public const ORDER_OLDEST_FIRST = 'ASC';
/**
* @throws DoesNotExistException
* @throws ClientException
Expand All @@ -46,6 +48,7 @@ public function findMessage(Account $account,
/**
* @param Account $account
* @param Mailbox $mailbox
* @param string $sortOrder
* @param string|null $filter
* @param int|null $cursor
* @param int|null $limit
Expand All @@ -57,6 +60,7 @@ public function findMessage(Account $account,
*/
public function findMessages(Account $account,
Mailbox $mailbox,
string $sortOrder,
?string $filter,
?int $cursor,
?int $limit): array;
Expand Down
1 change: 1 addition & 0 deletions lib/Controller/AccountsController.php
Original file line number Diff line number Diff line change
Expand Up @@ -461,6 +461,7 @@ public function draft(int $id,
$draftsMailbox,
Horde_Imap_Client::SYNC_NEWMSGSUIDS,
[],
null,
false
);
return new JSONResponse([
Expand Down
6 changes: 5 additions & 1 deletion lib/Controller/MailboxesController.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@

use Horde_Imap_Client;
use OCA\Mail\Contracts\IMailManager;
use OCA\Mail\Contracts\IMailSearch;
use OCA\Mail\Exception\ClientException;
use OCA\Mail\Exception\IncompleteSyncException;
use OCA\Mail\Exception\MailboxNotCachedException;
Expand Down Expand Up @@ -145,9 +146,10 @@ public function patch(int $id,
* @throws ServiceException
*/
#[TrapError]
public function sync(int $id, array $ids = [], bool $init = false, string $query = null): JSONResponse {
public function sync(int $id, array $ids = [], ?int $lastMessageTimestamp, bool $init = false, string $sortOrder = 'newest', string $query = null): JSONResponse {
$mailbox = $this->mailManager->getMailbox($this->currentUserId, $id);
$account = $this->accountService->find($this->currentUserId, $mailbox->getAccountId());
$order = $sortOrder === 'newest' ? IMailSearch::ORDER_NEWEST_FIRST: IMailSearch::ORDER_OLDEST_FIRST;

try {
$syncResponse = $this->syncService->syncMailbox(
Expand All @@ -157,7 +159,9 @@ public function sync(int $id, array $ids = [], bool $init = false, string $query
array_map(static function ($id) {
return (int)$id;
}, $ids),
$lastMessageTimestamp,
!$init,
$order,
$query
);
} catch (MailboxNotCachedException $e) {
Expand Down
8 changes: 7 additions & 1 deletion lib/Controller/MessagesController.php
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
use OCA\Mail\Contracts\IMailSearch;
use OCA\Mail\Contracts\IMailTransmission;
use OCA\Mail\Contracts\ITrustedSenderService;
use OCA\Mail\Contracts\IUserPreferences;
use OCA\Mail\Db\Message;
use OCA\Mail\Exception\ClientException;
use OCA\Mail\Exception\ServiceException;
Expand Down Expand Up @@ -87,6 +88,7 @@ class MessagesController extends Controller {
private SmimeService $smimeService;
private IMAPClientFactory $clientFactory;
private IDkimService $dkimService;
private IUserPreferences $preferences;
private SnoozeService $snoozeService;

public function __construct(string $appName,
Expand All @@ -107,6 +109,7 @@ public function __construct(string $appName,
SmimeService $smimeService,
IMAPClientFactory $clientFactory,
IDkimService $dkimService,
IUserPreferences $preferences,
SnoozeService $snoozeService) {
parent::__construct($appName, $request);
$this->accountService = $accountService;
Expand All @@ -125,6 +128,7 @@ public function __construct(string $appName,
$this->smimeService = $smimeService;
$this->clientFactory = $clientFactory;
$this->dkimService = $dkimService;
$this->preferences = $preferences;
$this->snoozeService = $snoozeService;
}

Expand Down Expand Up @@ -153,12 +157,14 @@ public function index(int $mailboxId,
return new JSONResponse([], Http::STATUS_FORBIDDEN);
}

$this->logger->debug("loading messages of folder <$mailboxId>");
$this->logger->debug("loading messages of mailbox <$mailboxId>");

$order = $this->preferences->getPreference($this->currentUserId, 'sort-order', 'newest') === 'newest' ? 'DESC': 'ASC';
return new JSONResponse(
$this->mailSearch->findMessages(
$account,
$mailbox,
$order,
$filter === '' ? null : $filter,
$cursor,
$limit
Expand Down
5 changes: 5 additions & 0 deletions lib/Controller/PageController.php
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,11 @@ public function index(): TemplateResponse {
$this->tagMapper->getAllTagsForUser($this->currentUserId)
);

$this->initialStateService->provideInitialState(
'sort-order',
$this->preferences->getPreference($this->currentUserId, 'sort-order', 'newest')
);

try {
$password = $this->credentialStore->getLoginCredentials()->getPassword();
$passwordIsUnavailable = $password === null || $password === '';
Expand Down
62 changes: 44 additions & 18 deletions lib/Db/MessageMapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
use OCA\Mail\Account;
use OCA\Mail\Address;
use OCA\Mail\AddressList;
use OCA\Mail\Contracts\IMailSearch;
use OCA\Mail\IMAP\Threading\DatabaseMessage;
use OCA\Mail\Service\Search\Flag;
use OCA\Mail\Service\Search\FlagExpression;
Expand Down Expand Up @@ -700,7 +701,7 @@ public function findByMessageId(Account $account, string $messageId): array {
*
* @return int[]
*/
public function findIdsByQuery(Mailbox $mailbox, SearchQuery $query, ?int $limit, array $uids = null): array {
public function findIdsByQuery(Mailbox $mailbox, SearchQuery $query, string $sortOrder, ?int $limit, array $uids = null): array {
$qb = $this->db->getQueryBuilder();

if ($this->needDistinct($query)) {
Expand Down Expand Up @@ -828,10 +829,14 @@ public function findIdsByQuery(Mailbox $mailbox, SearchQuery $query, ?int $limit
);
}

if ($query->getCursor() !== null) {
if ($query->getCursor() !== null && $sortOrder === IMailSearch::ORDER_NEWEST_FIRST) {
$select->andWhere(
$qb->expr()->lt('m.sent_at', $qb->createNamedParameter($query->getCursor(), IQueryBuilder::PARAM_INT))
);
} elseif ($query->getCursor() !== null && $sortOrder === IMailSearch::ORDER_OLDEST_FIRST) {
$select->andWhere(
$qb->expr()->gt('m.sent_at', $qb->createNamedParameter($query->getCursor(), IQueryBuilder::PARAM_INT))
);
}

// createParameter
Expand Down Expand Up @@ -861,7 +866,11 @@ public function findIdsByQuery(Mailbox $mailbox, SearchQuery $query, ?int $limit

$select->andWhere($qb->expr()->isNull('m2.id'));

$select->orderBy('m.sent_at', 'desc');
if ($sortOrder === 'ASC') {
$select->orderBy('sent_at', $sortOrder);
} else {
$select->orderBy('sent_at', 'DESC');
}

if ($limit !== null) {
$select->setMaxResults($limit);
Expand All @@ -876,9 +885,10 @@ public function findIdsByQuery(Mailbox $mailbox, SearchQuery $query, ?int $limit
}, array_chunk($uids, 1000));
}

return array_map(static function (Message $message) {
$result = array_map(static function (Message $message) {
return $message->getId();
}, $this->findEntities($select));
return $result;
}

public function findIdsGloballyByQuery(IUser $user, SearchQuery $query, ?int $limit, array $uids = null): array {
Expand Down Expand Up @@ -1115,21 +1125,21 @@ public function findByMailboxAndIds(Mailbox $mailbox, string $userId, array $ids
/**
* @param string $userId
* @param int[] $ids
* @param string $sortOrder
*
* @return Message[]
*/
public function findByIds(string $userId, array $ids): array {
public function findByIds(string $userId, array $ids, string $sortOrder): array {
if ($ids === []) {
return [];
}

$qb = $this->db->getQueryBuilder();
$qb->select('*')
->from($this->getTableName())
->where(
$qb->expr()->in('id', $qb->createParameter('ids'))
)
->orderBy('sent_at', 'desc');
->orderBy('sent_at', $sortOrder);

$results = [];
foreach (array_chunk($ids, 1000) as $chunk) {
Expand Down Expand Up @@ -1215,14 +1225,19 @@ public function findRelatedData(array $messages, string $userId): array {
/**
* @param Mailbox $mailbox
* @param array $ids
* @param int|null $lastMessageTimestamp
* @param IMailSearch::ORDER_* $sortOrder
*
* @return int[]
*/
public function findNewIds(Mailbox $mailbox, array $ids): array {
public function findNewIds(Mailbox $mailbox, array $ids, ?int $lastMessageTimestamp, string $sortOrder): array {
$select = $this->db->getQueryBuilder();
$subSelect = $this->db->getQueryBuilder();

$subSelect
->select($subSelect->func()->min('sent_at'))
->select($sortOrder === IMailSearch::ORDER_NEWEST_FIRST ?
$subSelect->func()->min('sent_at') :
$subSelect->func()->max('sent_at'))
->from($this->getTableName())
->where(
$subSelect->expr()->eq('mailbox_id', $select->createNamedParameter($mailbox->getId(), IQueryBuilder::PARAM_INT)),
Expand All @@ -1234,20 +1249,31 @@ public function findNewIds(Mailbox $mailbox, array $ids): array {
$selfJoin = $select->expr()->andX(
$select->expr()->eq('m.mailbox_id', 'm2.mailbox_id', IQueryBuilder::PARAM_INT),
$select->expr()->eq('m.thread_root_id', 'm2.thread_root_id', IQueryBuilder::PARAM_INT),
$select->expr()->lt('m.sent_at', 'm2.sent_at', IQueryBuilder::PARAM_INT)
$sortOrder === IMailSearch::ORDER_NEWEST_FIRST ?
$select->expr()->lt('m.sent_at', 'm2.sent_at', IQueryBuilder::PARAM_INT) :
$select->expr()->gt('m.sent_at', 'm2.sent_at', IQueryBuilder::PARAM_INT)
);
$wheres = [$select->expr()->eq('m.mailbox_id', $select->createNamedParameter($mailbox->getId(), IQueryBuilder::PARAM_INT)),
$select->expr()->andX($subSelect->expr()->notIn('m.id', $select->createParameter('ids'), IQueryBuilder::PARAM_INT_ARRAY)),
$select->expr()->isNull('m2.id'),
];
if ($sortOrder === IMailSearch::ORDER_NEWEST_FIRST) {
$wheres[] = $select->expr()->gt('m.sent_at', $select->createFunction('(' . $subSelect->getSQL() . ')'), IQueryBuilder::PARAM_INT);
} else {
$wheres[] = $select->expr()->lt('m.sent_at', $select->createFunction('(' . $subSelect->getSQL() . ')'), IQueryBuilder::PARAM_INT);
}

if ($lastMessageTimestamp !== null && $sortOrder === IMailSearch::ORDER_OLDEST_FIRST) {
// Don't consider old "new messages" as new when their UID has already been seen before
$wheres[] = $select->expr()->lt('m.sent_at', $select->createNamedParameter($lastMessageTimestamp, IQueryBuilder::PARAM_INT));
}

$select
->select('m.id')
->select(['m.id', 'm.sent_at'])
->from($this->getTableName(), 'm')
->leftJoin('m', $this->getTableName(), 'm2', $selfJoin)
->where(
$select->expr()->eq('m.mailbox_id', $select->createNamedParameter($mailbox->getId(), IQueryBuilder::PARAM_INT)),
$select->expr()->andX($subSelect->expr()->notIn('m.id', $select->createParameter('ids'), IQueryBuilder::PARAM_INT_ARRAY)),
$select->expr()->isNull('m2.id'),
$select->expr()->gt('m.sent_at', $select->createFunction('(' . $subSelect->getSQL() . ')'), IQueryBuilder::PARAM_INT)
)
->orderBy('m.sent_at', 'desc');
->where(...$wheres)
->orderBy('m.sent_at', $sortOrder === IMailSearch::ORDER_NEWEST_FIRST ? 'desc' : 'asc');

$results = [];
foreach (array_chunk($ids, 1000) as $chunk) {
Expand Down
2 changes: 1 addition & 1 deletion lib/IMAP/Sync/Synchronizer.php
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ public function sync(Horde_Imap_Client_Base $imapClient,
$changedMessages = $this->messageMapper->findByIds($imapClient, $request->getMailbox(), $changedUids, $userId);
$vanishedMessageUids = $vanishedUids;

return new Response($newMessages, $changedMessages, $vanishedMessageUids);
return new Response($newMessages, $changedMessages, $vanishedMessageUids, null);
}

/**
Expand Down
14 changes: 9 additions & 5 deletions lib/Service/Search/MailSearch.php
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ public function findMessage(Account $account,
/**
* @param Account $account
* @param Mailbox $mailbox
* @param string $sortOrder
* @param string|null $filter
* @param int|null $cursor
* @param int|null $limit
Expand All @@ -103,6 +104,7 @@ public function findMessage(Account $account,
*/
public function findMessages(Account $account,
Mailbox $mailbox,
string $sortOrder,
?string $filter,
?int $cursor,
?int $limit): array {
Expand Down Expand Up @@ -130,7 +132,8 @@ public function findMessages(Account $account,
$account,
$mailbox,
$this->messageMapper->findByIds($account->getUserId(),
$this->getIdsLocally($account, $mailbox, $query, $limit)
$this->getIdsLocally($account, $mailbox, $query, $sortOrder, $limit),
$sortOrder,
)
);
}
Expand All @@ -155,7 +158,8 @@ public function findMessagesGlobally(IUser $user,
}

return $this->messageMapper->findByIds($user->getUID(),
$this->getIdsGlobally($user, $query, $limit)
$this->getIdsGlobally($user, $query, $limit),
'DESC'
);
}

Expand All @@ -164,17 +168,17 @@ public function findMessagesGlobally(IUser $user,
*
* @throws ServiceException
*/
private function getIdsLocally(Account $account, Mailbox $mailbox, SearchQuery $query, ?int $limit): array {
private function getIdsLocally(Account $account, Mailbox $mailbox, SearchQuery $query, string $sortOrder, ?int $limit): array {
if (empty($query->getBodies())) {
return $this->messageMapper->findIdsByQuery($mailbox, $query, $limit);
return $this->messageMapper->findIdsByQuery($mailbox, $query, $sortOrder, $limit);
}

$fromImap = $this->imapSearchProvider->findMatches(
$account,
$mailbox,
$query
);
return $this->messageMapper->findIdsByQuery($mailbox, $query, $limit, $fromImap);
return $this->messageMapper->findIdsByQuery($mailbox, $query, $sortOrder, $limit, $fromImap);
}

/**
Expand Down
15 changes: 11 additions & 4 deletions lib/Service/Sync/SyncService.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
namespace OCA\Mail\Service\Sync;

use OCA\Mail\Account;
use OCA\Mail\Contracts\IMailSearch;
use OCA\Mail\Db\Mailbox;
use OCA\Mail\Db\MailboxMapper;
use OCA\Mail\Db\Message;
Expand Down Expand Up @@ -111,7 +112,9 @@ public function syncMailbox(Account $account,
Mailbox $mailbox,
int $criteria,
array $knownIds = null,
?int $lastMessageTimestamp,
bool $partialOnly,
string $sortOrder = IMailSearch::ORDER_NEWEST_FIRST,
string $filter = null): Response {
if ($partialOnly && !$mailbox->isCached()) {
throw MailboxNotCachedException::from($mailbox);
Expand All @@ -133,6 +136,8 @@ public function syncMailbox(Account $account,
$account,
$mailbox,
$knownIds ?? [],
$lastMessageTimestamp,
$sortOrder,
$query
);
}
Expand All @@ -150,24 +155,26 @@ public function syncMailbox(Account $account,
private function getDatabaseSyncChanges(Account $account,
Mailbox $mailbox,
array $knownIds,
?int $lastMessageTimestamp,
string $sortOrder,
?SearchQuery $query): Response {
if ($knownIds === []) {
$newIds = $this->messageMapper->findAllIds($mailbox);
} else {
$newIds = $this->messageMapper->findNewIds($mailbox, $knownIds);
$newIds = $this->messageMapper->findNewIds($mailbox, $knownIds, $lastMessageTimestamp, $sortOrder);
}

$order = $sortOrder === 'oldest' ? IMailSearch::ORDER_OLDEST_FIRST : IMailSearch::ORDER_NEWEST_FIRST;
if ($query !== null) {
// Filter new messages to those that also match the current filter
$newUids = $this->messageMapper->findUidsForIds($mailbox, $newIds);
$newIds = $this->messageMapper->findIdsByQuery($mailbox, $query, null, $newUids);
$newIds = $this->messageMapper->findIdsByQuery($mailbox, $query, $order, null, $newUids);
}
$new = $this->messageMapper->findByMailboxAndIds($mailbox, $account->getUserId(), $newIds);

// TODO: $changed = $this->messageMapper->findChanged($account, $mailbox, $uids);
if ($query !== null) {
$changedUids = $this->messageMapper->findUidsForIds($mailbox, $knownIds);
$changedIds = $this->messageMapper->findIdsByQuery($mailbox, $query, null, $changedUids);
$changedIds = $this->messageMapper->findIdsByQuery($mailbox, $query, $order, null, $changedUids);
} else {
$changedIds = $knownIds;
}
Expand Down
Loading

0 comments on commit f6fa141

Please sign in to comment.