Skip to content
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.

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

fix: perform bulk message actions #10280

Draft
wants to merge 9 commits into
base: main
Choose a base branch
from
28 changes: 26 additions & 2 deletions lib/Controller/MessagesController.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,14 @@
use OCA\Mail\Service\AccountService;
use OCA\Mail\Service\AiIntegrations\AiIntegrationsService;
use OCA\Mail\Service\ItineraryService;
use OCA\Mail\Service\MessageOperationService;
use OCA\Mail\Service\SmimeService;
use OCA\Mail\Service\SnoozeService;
use OCP\AppFramework\Controller;
use OCP\AppFramework\Db\DoesNotExistException;
use OCP\AppFramework\Http;
use OCP\AppFramework\Http\Attribute\FrontpageRoute;
use OCP\AppFramework\Http\Attribute\NoAdminRequired;
use OCP\AppFramework\Http\Attribute\OpenAPI;
use OCP\AppFramework\Http\ContentSecurityPolicy;
use OCP\AppFramework\Http\JSONResponse;
Expand Down Expand Up @@ -74,7 +77,8 @@
private SnoozeService $snoozeService;
private AiIntegrationsService $aiIntegrationService;

public function __construct(string $appName,
public function __construct(
string $appName,
IRequest $request,
AccountService $accountService,
IMailManager $mailManager,
Expand All @@ -94,7 +98,9 @@
IDkimService $dkimService,
IUserPreferences $preferences,
SnoozeService $snoozeService,
AiIntegrationsService $aiIntegrationService) {
AiIntegrationsService $aiIntegrationService,
private MessageOperationService $messageOperationService,
) {
parent::__construct($appName, $request);
$this->accountService = $accountService;
$this->mailManager = $mailManager;
Expand Down Expand Up @@ -782,6 +788,24 @@
return new JSONResponse();
}

/**
*
* @NoAdminRequired
*
* @param array<int,int> $identifiers
* @param array<string,bool> $flags
*
* @return JSONResponse
*/
#[FrontpageRoute(verb: 'PUT', url: '/api/messages/flags')]

Check warning on line 800 in lib/Controller/MessagesController.php

View check run for this annotation

Codecov / codecov/patch

lib/Controller/MessagesController.php#L800

Added line #L800 was not covered by tests
#[NoAdminRequired]
#[TrapError]
public function changeFlags(array $identifiers, array $flags): JSONResponse {
return new JSONResponse(
$this->messageOperationService->changeFlags($this->currentUserId, $identifiers, $flags)
);

Check warning on line 806 in lib/Controller/MessagesController.php

View check run for this annotation

Codecov / codecov/patch

lib/Controller/MessagesController.php#L804-L806

Added lines #L804 - L806 were not covered by tests
}

/**
* @NoAdminRequired
*
Expand Down Expand Up @@ -882,7 +906,7 @@
#[TrapError]
public function smartReply(int $messageId):JSONResponse {
try {
$message = $this->mailManager->getMessage($this->currentUserId, $messageId);

Check failure on line 909 in lib/Controller/MessagesController.php

View workflow job for this annotation

GitHub Actions / static-psalm-analysis dev-master

PossiblyNullArgument

lib/Controller/MessagesController.php:909:46: PossiblyNullArgument: Argument 1 of OCA\Mail\Contracts\IMailManager::getMessage cannot be null, possibly null value provided (see https://psalm.dev/078)
$mailbox = $this->mailManager->getMailbox($this->currentUserId, $message->getMailboxId());
$account = $this->accountService->find($this->currentUserId, $mailbox->getAccountId());
} catch (DoesNotExistException $e) {
Expand Down
23 changes: 23 additions & 0 deletions lib/Db/MailAccountMapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,29 @@
return $this->findEntity($query);
}

/**
* Finds all mail accounts by account ids
*
* @param string $userId
* @param array<int,int> $identifiers
*
* @return array<int,MailAccount>
*/
public function findByIds(string $userId, array $identifiers): array {

Check warning on line 77 in lib/Db/MailAccountMapper.php

View check run for this annotation

Codecov / codecov/patch

lib/Db/MailAccountMapper.php#L77

Added line #L77 was not covered by tests

$qb = $this->db->getQueryBuilder();
$qb->select('*')
->from($this->getTableName())
->where(
$qb->expr()->eq('user_id', $qb->createNamedParameter($userId, IQueryBuilder::PARAM_STR), IQueryBuilder::PARAM_STR)
)
->andWhere(
$qb->expr()->in('id', $qb->createNamedParameter($identifiers, IQueryBuilder::PARAM_STR_ARRAY), IQueryBuilder::PARAM_STR_ARRAY)
);

Check warning on line 87 in lib/Db/MailAccountMapper.php

View check run for this annotation

Codecov / codecov/patch

lib/Db/MailAccountMapper.php#L79-L87

Added lines #L79 - L87 were not covered by tests

return $this->findEntities($qb);

Check warning on line 89 in lib/Db/MailAccountMapper.php

View check run for this annotation

Codecov / codecov/patch

lib/Db/MailAccountMapper.php#L89

Added line #L89 was not covered by tests
}

/**
* Finds all Mail Accounts by user id existing for this user
*
Expand Down
4 changes: 3 additions & 1 deletion lib/Db/MailboxMapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,9 @@ public function findById(int $id): Mailbox {
}

/**
* @return Mailbox[]
* @param array<int> $ids
*
* @return array<Mailbox>
*
* @throws Exception
*/
Expand Down
22 changes: 22 additions & 0 deletions lib/Db/MessageMapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,28 @@
}, array_chunk($ids, 1000));
}

/**
* @param array<int,int> $identifiers
*
* @return array
*/
public function findMailboxAndUid(array $identifiers): array {

Check warning on line 186 in lib/Db/MessageMapper.php

View check run for this annotation

Codecov / codecov/patch

lib/Db/MessageMapper.php#L186

Added line #L186 was not covered by tests

if ($identifiers === []) {
return [];

Check warning on line 189 in lib/Db/MessageMapper.php

View check run for this annotation

Codecov / codecov/patch

lib/Db/MessageMapper.php#L188-L189

Added lines #L188 - L189 were not covered by tests
}

$qb = $this->db->getQueryBuilder();
$qb->select('id', 'mailbox_id', 'uid')
->from($this->getTableName())
->where(
$qb->expr()->in('id', $qb->createNamedParameter($identifiers, IQueryBuilder::PARAM_STR_ARRAY), IQueryBuilder::PARAM_STR_ARRAY)
);

Check warning on line 197 in lib/Db/MessageMapper.php

View check run for this annotation

Codecov / codecov/patch

lib/Db/MessageMapper.php#L192-L197

Added lines #L192 - L197 were not covered by tests

return $qb->executeQuery()->fetchAll();

Check warning on line 199 in lib/Db/MessageMapper.php

View check run for this annotation

Codecov / codecov/patch

lib/Db/MessageMapper.php#L199

Added line #L199 was not covered by tests

}

/**
* @param Account $account
*
Expand Down
38 changes: 38 additions & 0 deletions lib/IMAP/MessageMapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -439,6 +439,44 @@
);
}

/**
* @throws Horde_Imap_Client_Exception
*/
public function setFlags(

Check warning on line 445 in lib/IMAP/MessageMapper.php

View check run for this annotation

Codecov / codecov/patch

lib/IMAP/MessageMapper.php#L445

Added line #L445 was not covered by tests
Horde_Imap_Client_Socket $client,
Mailbox $mailbox,
array $uids,
array $addFlags = [],
array $removeFlags = [],
array $replaceFlags = [],
): array {

if (count($uids) === 0) {
return [];

Check warning on line 455 in lib/IMAP/MessageMapper.php

View check run for this annotation

Codecov / codecov/patch

lib/IMAP/MessageMapper.php#L454-L455

Added lines #L454 - L455 were not covered by tests
}

$cmd = [];

Check warning on line 458 in lib/IMAP/MessageMapper.php

View check run for this annotation

Codecov / codecov/patch

lib/IMAP/MessageMapper.php#L458

Added line #L458 was not covered by tests

if (count($replaceFlags) > 0) {
$cmd['replace'] = $replaceFlags;

Check warning on line 461 in lib/IMAP/MessageMapper.php

View check run for this annotation

Codecov / codecov/patch

lib/IMAP/MessageMapper.php#L460-L461

Added lines #L460 - L461 were not covered by tests
} else {
if (count($addFlags) > 0) {
$cmd['add'] = $addFlags;

Check warning on line 464 in lib/IMAP/MessageMapper.php

View check run for this annotation

Codecov / codecov/patch

lib/IMAP/MessageMapper.php#L463-L464

Added lines #L463 - L464 were not covered by tests
}
if (count($removeFlags) > 0) {
$cmd['remove'] = $removeFlags;

Check warning on line 467 in lib/IMAP/MessageMapper.php

View check run for this annotation

Codecov / codecov/patch

lib/IMAP/MessageMapper.php#L466-L467

Added lines #L466 - L467 were not covered by tests
}
}

if (count($cmd) > 0) {
$cmd['ids'] = new Horde_Imap_Client_Ids($uids);
$result = $client->store($mailbox->getName(), $cmd);
return $result->ids;

Check warning on line 474 in lib/IMAP/MessageMapper.php

View check run for this annotation

Codecov / codecov/patch

lib/IMAP/MessageMapper.php#L471-L474

Added lines #L471 - L474 were not covered by tests
}

return [];

Check warning on line 477 in lib/IMAP/MessageMapper.php

View check run for this annotation

Codecov / codecov/patch

lib/IMAP/MessageMapper.php#L477

Added line #L477 was not covered by tests
}

/**
* @param Horde_Imap_Client_Socket $client
* @param Mailbox $mailbox
Expand Down
147 changes: 147 additions & 0 deletions lib/Service/MessageOperationService.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,147 @@
<?php

declare(strict_types=1);

/**
* SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/

namespace OCA\Mail\Service;

use OCA\Mail\Account;
use OCA\Mail\Db\MailAccountMapper;
use OCA\Mail\Db\MailboxMapper;
use OCA\Mail\Db\MessageMapper;
use OCA\Mail\IMAP\IMAPClientFactory;
use OCA\Mail\IMAP\MessageMapper as ImapMessageMapper;
use Throwable;

class MessageOperationService {

public function __construct(

Check warning on line 22 in lib/Service/MessageOperationService.php

View check run for this annotation

Codecov / codecov/patch

lib/Service/MessageOperationService.php#L22

Added line #L22 was not covered by tests
protected IMAPClientFactory $clientFactory,
protected MailAccountMapper $accountMapper,
protected MailboxMapper $mailboxMapper,
protected MessageMapper $messageMapper,
protected MailManager $mailManager,
protected ImapMessageMapper $imapMessageMapper,
) {
}

Check warning on line 30 in lib/Service/MessageOperationService.php

View check run for this annotation

Codecov / codecov/patch

lib/Service/MessageOperationService.php#L30

Added line #L30 was not covered by tests

/**
* convert message collection to grouped collections by mailbox id
*
* [[mailbox_id, uid, id]] to [mailbox_id => [[id, uid]]]
*
* @param array<array{0:int,1:int,2:int}> $collection
*
* @return array<int,array<int,array{0:int,1:int,2:int}>>
*/
protected function groupByMailbox(array $collection): array {
return array_reduce($collection, function ($carry, $pair) {

Check failure on line 42 in lib/Service/MessageOperationService.php

View workflow job for this annotation

GitHub Actions / static-psalm-analysis dev-master

InvalidReturnStatement

lib/Service/MessageOperationService.php:42:10: InvalidReturnStatement: The inferred type 'array<array-key, mixed|non-empty-list<array{id: int, uid: int}>>|mixed' does not match the declared return type 'array<int, array<int, array{0: int, 1: int, 2: int}>>' for OCA\Mail\Service\MessageOperationService::groupByMailbox (see https://psalm.dev/128)
if (!isset($carry[$pair['mailbox_id']])) {
$carry[$pair['mailbox_id']] = [];

Check warning on line 44 in lib/Service/MessageOperationService.php

View check run for this annotation

Codecov / codecov/patch

lib/Service/MessageOperationService.php#L41-L44

Added lines #L41 - L44 were not covered by tests
}
$carry[(int)$pair['mailbox_id']][] = ['id' => (int)$pair['id'], 'uid' => (int)$pair['uid']];
return $carry;
}, []);

Check warning on line 48 in lib/Service/MessageOperationService.php

View check run for this annotation

Codecov / codecov/patch

lib/Service/MessageOperationService.php#L46-L48

Added lines #L46 - L48 were not covered by tests
}

/**
* convert mailbox collection to grouped collections by account id
*
* [mailbox] to [account_id => [mailbox]]
*
* @param array<\OCA\Mail\Db\Mailbox> $collection
*
* @return array<int,array<\OCA\Mail\Db\Mailbox>>
*/
protected function groupByAccount(array $collection) {
return array_reduce($collection, function ($carry, $entry) {
if (!isset($carry[$entry->getAccountId()])) {
$carry[$entry->getAccountId()] = [];

Check warning on line 63 in lib/Service/MessageOperationService.php

View check run for this annotation

Codecov / codecov/patch

lib/Service/MessageOperationService.php#L60-L63

Added lines #L60 - L63 were not covered by tests
}
$carry[$entry->getAccountId()][] = $entry;
return $carry;
}, []);

Check warning on line 67 in lib/Service/MessageOperationService.php

View check run for this annotation

Codecov / codecov/patch

lib/Service/MessageOperationService.php#L65-L67

Added lines #L65 - L67 were not covered by tests
}

/**
* generates operation status responses for each message
*
* @param array<int,bool> &$results
* @param bool $value
* @param array<\OCA\Mail\Db\Mailbox> $mailboxes
* @param array<int,array<array{id:int,uid:int}>> $messages
*/
protected function generateResult(array &$results, bool $value, array $mailboxes, array $messages) {
foreach ($mailboxes as $mailbox) {
foreach ($messages[$mailbox->getId()] as $message) {
$results[$message['id']] = $value;

Check warning on line 81 in lib/Service/MessageOperationService.php

View check run for this annotation

Codecov / codecov/patch

lib/Service/MessageOperationService.php#L78-L81

Added lines #L78 - L81 were not covered by tests
}
}
}

/**
* Set/Unset system flags or keywords
*
* @param string $userId system user id
* @param array<int> $identifiers message ids
* @param array<string,bool> $flags message flags
*
* @return array<int,bool> operation results
*/
public function changeFlags(string $userId, array $identifiers, array $flags): array {

Check warning on line 95 in lib/Service/MessageOperationService.php

View check run for this annotation

Codecov / codecov/patch

lib/Service/MessageOperationService.php#L95

Added line #L95 was not covered by tests

// retrieve message meta data [uid, mailbox_id] for all messages and group by mailbox id
$messages = $this->groupByMailbox($this->messageMapper->findMailboxAndUid($identifiers));

Check warning on line 98 in lib/Service/MessageOperationService.php

View check run for this annotation

Codecov / codecov/patch

lib/Service/MessageOperationService.php#L98

Added line #L98 was not covered by tests
// retrieve all mailboxes and group by account
$mailboxes = $this->groupByAccount($this->mailboxMapper->findByIds(array_keys($messages)));

Check warning on line 100 in lib/Service/MessageOperationService.php

View check run for this annotation

Codecov / codecov/patch

lib/Service/MessageOperationService.php#L100

Added line #L100 was not covered by tests
// retrieve all accounts
$accounts = $this->accountMapper->findByIds($userId, array_keys($mailboxes));

Check warning on line 102 in lib/Service/MessageOperationService.php

View check run for this annotation

Codecov / codecov/patch

lib/Service/MessageOperationService.php#L102

Added line #L102 was not covered by tests
// process every account
$results = [];
foreach ($accounts as $account) {
$account = new Account($account);
$client = $this->clientFactory->getClient($account);

Check warning on line 107 in lib/Service/MessageOperationService.php

View check run for this annotation

Codecov / codecov/patch

lib/Service/MessageOperationService.php#L104-L107

Added lines #L104 - L107 were not covered by tests
// process every mailbox
foreach ($mailboxes[$account->getId()] as $mailbox) {

Check warning on line 109 in lib/Service/MessageOperationService.php

View check run for this annotation

Codecov / codecov/patch

lib/Service/MessageOperationService.php#L109

Added line #L109 was not covered by tests
try {
// check if specific flags are supported and group them by action
$addFlags = [];
$removeFlags = [];
foreach ($flags as $flag => $value) {
$value = filter_var($value, FILTER_VALIDATE_BOOLEAN);
$imapFlags = $this->mailManager->filterFlags($client, $account, $flag, $mailbox->getName());
if (empty($imapFlags)) {
continue;

Check warning on line 118 in lib/Service/MessageOperationService.php

View check run for this annotation

Codecov / codecov/patch

lib/Service/MessageOperationService.php#L112-L118

Added lines #L112 - L118 were not covered by tests
}
if ($value) {
$addFlags = array_merge($addFlags, $imapFlags);

Check warning on line 121 in lib/Service/MessageOperationService.php

View check run for this annotation

Codecov / codecov/patch

lib/Service/MessageOperationService.php#L120-L121

Added lines #L120 - L121 were not covered by tests
} else {
$removeFlags = array_merge($removeFlags, $imapFlags);

Check warning on line 123 in lib/Service/MessageOperationService.php

View check run for this annotation

Codecov / codecov/patch

lib/Service/MessageOperationService.php#L123

Added line #L123 was not covered by tests
}
}
// apply flags to messages on server
$this->imapMessageMapper->setFlags(
$client,
$mailbox,
array_column($messages[$mailbox->getId()], 'uid'),
$addFlags,
$removeFlags
);

Check warning on line 133 in lib/Service/MessageOperationService.php

View check run for this annotation

Codecov / codecov/patch

lib/Service/MessageOperationService.php#L127-L133

Added lines #L127 - L133 were not covered by tests
// add messages to results as successful
$this->generateResult($results, true, [$mailbox], $messages);

Check failure on line 135 in lib/Service/MessageOperationService.php

View workflow job for this annotation

GitHub Actions / static-psalm-analysis dev-master

InvalidArgument

lib/Service/MessageOperationService.php:135:56: InvalidArgument: Argument 4 of OCA\Mail\Service\MessageOperationService::generateResult expects array<int, array<array-key, array{id: int, uid: int}>>, but array<int, array<int, array{0: int, 1: int, 2: int}>> provided (see https://psalm.dev/004)
} catch (Throwable $e) {

Check warning on line 136 in lib/Service/MessageOperationService.php

View check run for this annotation

Codecov / codecov/patch

lib/Service/MessageOperationService.php#L135-L136

Added lines #L135 - L136 were not covered by tests
// add messages to results as failed
$this->generateResult($results, false, [$mailbox], $messages);

Check failure on line 138 in lib/Service/MessageOperationService.php

View workflow job for this annotation

GitHub Actions / static-psalm-analysis dev-master

InvalidArgument

lib/Service/MessageOperationService.php:138:57: InvalidArgument: Argument 4 of OCA\Mail\Service\MessageOperationService::generateResult expects array<int, array<array-key, array{id: int, uid: int}>>, but array<int, array<int, array{0: int, 1: int, 2: int}>> provided (see https://psalm.dev/004)

Check warning on line 138 in lib/Service/MessageOperationService.php

View check run for this annotation

Codecov / codecov/patch

lib/Service/MessageOperationService.php#L138

Added line #L138 was not covered by tests
}
}
$client->logout();

Check warning on line 141 in lib/Service/MessageOperationService.php

View check run for this annotation

Codecov / codecov/patch

lib/Service/MessageOperationService.php#L141

Added line #L141 was not covered by tests
}

return $results;

Check warning on line 144 in lib/Service/MessageOperationService.php

View check run for this annotation

Codecov / codecov/patch

lib/Service/MessageOperationService.php#L144

Added line #L144 was not covered by tests
}

}
Loading
Loading