nextcloud
/
mail
зеркало из
1
0
Форкнуть 0
Код Задачи Пакеты Проекты Релизы Вики Активность

Refactor and fix issues with draft handling 

Signed-off-by: Anna Larch <anna@nextcloud.com>
This commit is contained in:
Anna Larch 2022-11-24 14:27:08 +01:00
Родитель 41cce03ecd
Коммит 30f0cc9287
10 изменённых файлов: 226 добавлений и 134 удалений
Показать статистику Скачать Patch файл Скачать Diff файл Показать все файлы Свернуть все файлы

4
lib/BackgroundJob/IMipMessageJob.php
Просмотреть файл

@ -33,12 +33,12 @@ class IMipMessageJob extends TimedJob {
private IMipService $iMipService;
public function __construct(ITimeFactory $time,
IMipService $draftsService) {
IMipService $iMipService) {
parent::__construct($time);
// Run once per hour
$this->setInterval(60 * 60);
$this->iMipService = $draftsService;
$this->iMipService = $iMipService;
}
protected function run($argument): void {

86
lib/Contracts/ILocalMailboxService.php
Просмотреть файл

@ -1,86 +0,0 @@
<?php
declare(strict_types=1);
/**
* Mail App
*
* @copyright 2022 Anna Larch <anna.larch@gmx.net>
*
* @author Anna Larch <anna.larch@gmx.net>
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU AFFERO GENERAL PUBLIC LICENSE
* License as published by the Free Software Foundation; either
* version 3 of the License, or any later version.
*
* This library 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 library. If not, see <http://www.gnu.org/licenses/>.
*
*/
namespace OCA\Mail\Contracts;
use OCA\Mail\Account;
use OCA\Mail\Db\LocalMessage;
use OCA\Mail\Db\Recipient;
use OCA\Mail\Exception\ClientException;
use OCA\Mail\Exception\ServiceException;
interface ILocalMailboxService {
/**
* @param string $userId
* @return mixed
*/
public function getMessages(string $userId): array;
/**
* @param int $id
*
* @return LocalMessage
*
* @throws ServiceException
*/
public function getMessage(int $id, string $userId): LocalMessage;
/**
* @param Account $account
* @param LocalMessage $message
* @param Recipient[] $to
* @param Recipient[] $cc
* @param Recipient[] $bcc
* @param array $attachments
* @return LocalMessage
*/
public function saveMessage(Account $account, LocalMessage $message, array $to, array $cc, array $bcc, array $attachments = []): LocalMessage;
/**
* @param LocalMessage $message
* @param Recipient[] $to
* @param Recipient[] $cc
* @param Recipient[] $bcc
* @param array $attachments
* @return LocalMessage
*/
public function updateMessage(Account $account, LocalMessage $message, array $to, array $cc, array $bcc, array $attachments = []): LocalMessage;
/**
* @param LocalMessage $message
* @param string $userId
*/
public function deleteMessage(string $userId, LocalMessage $message): void;
/**
* @param LocalMessage $message
* @param Account $account
* @throws ClientException
* @throws ServiceException
* @return void
*/
public function sendMessage(LocalMessage $message, Account $account): void;
}

11
lib/Contracts/IMailTransmission.php
Просмотреть файл

@ -57,7 +57,16 @@ interface IMailTransmission {
* @throws ServiceException
* @return void
*/
public function sendLocalMessage(Account $account, LocalMessage $message, bool $isDraft = false): void;
public function sendLocalMessage(Account $account, LocalMessage $message): void;
/**
* @param Account $account
* @param LocalMessage $message
* @throws ClientException
* @throws ServiceException
* @return void
*/
public function saveLocalDraft(Account $account, LocalMessage $message): void;
/**
* Save a message draft

12
lib/Controller/DraftsController.php
Просмотреть файл

@ -63,9 +63,9 @@ class DraftsController extends Controller {
* @param string $body
* @param string $editorBody
* @param bool $isHtml
* @param array $to i. e. [['label' => 'Linus', 'email' => 'tent@stardewvalley.com'], ['label' => 'Pierre', 'email' => 'generalstore@stardewvalley.com']]
* @param array $cc
* @param array $bcc
* @param array<int, string[]> $to i. e. [['label' => 'Linus', 'email' => 'tent@stardewvalley.com'], ['label' => 'Pierre', 'email' => 'generalstore@stardewvalley.com']]
* @param array<int, string[]> $cc
* @param array<int, string[]> $bcc
* @param array $attachments
* @param int|null $aliasId
* @param string|null $inReplyToMessageId
@ -121,9 +121,9 @@ class DraftsController extends Controller {
* @param string $editorBody
* @param bool $isHtml
* @param bool $failed
* @param array $to i. e. [['label' => 'Linus', 'email' => 'tent@stardewvalley.com'], ['label' => 'Pierre', 'email' => 'generalstore@stardewvalley.com']]
* @param array $cc
* @param array $bcc
* @param array<int, string[]> $to i. e. [['label' => 'Linus', 'email' => 'tent@stardewvalley.com'], ['label' => 'Pierre', 'email' => 'generalstore@stardewvalley.com']]
* @param array<int, string[]> $cc
* @param array<int, string[]> $bcc
* @param array $attachments
* @param int|null $aliasId
* @param string|null $inReplyToMessageId

6
lib/Controller/OutboxController.php
Просмотреть файл

@ -82,9 +82,9 @@ class OutboxController extends Controller {
* @param string $body
* @param string $editorBody
* @param bool $isHtml
* @param array $to i. e. [['label' => 'Linus', 'email' => 'tent@stardewvalley.com'], ['label' => 'Pierre', 'email' => 'generalstore@stardewvalley.com']]
* @param array $cc
* @param array $bcc
* @param array<int, string[]> $to i. e. [['label' => 'Linus', 'email' => 'tent@stardewvalley.com'], ['label' => 'Pierre', 'email' => 'generalstore@stardewvalley.com']]
* @param array<int, string[]> $cc
* @param array<int, string[]> $bcc
* @param array $attachments
* @param int|null $draftId
* @param int|null $aliasId

42
lib/Service/DraftsService.php
Просмотреть файл

@ -27,7 +27,6 @@ declare(strict_types=1);
namespace OCA\Mail\Service;
use OCA\Mail\Account;
use OCA\Mail\Contracts\ILocalMailboxService;
use OCA\Mail\Contracts\IMailManager;
use OCA\Mail\Contracts\IMailTransmission;
use OCA\Mail\Db\LocalMessage;
@ -35,7 +34,6 @@ use OCA\Mail\Db\LocalMessageMapper;
use OCA\Mail\Db\Recipient;
use OCA\Mail\Events\DraftMessageCreatedEvent;
use OCA\Mail\Exception\ClientException;
use OCA\Mail\Exception\NotImplemented;
use OCA\Mail\Exception\ServiceException;
use OCA\Mail\IMAP\IMAPClientFactory;
use OCA\Mail\Service\Attachment\AttachmentService;
@ -45,7 +43,7 @@ use OCP\EventDispatcher\IEventDispatcher;
use Psr\Log\LoggerInterface;
use Throwable;
class DraftsService implements ILocalMailboxService {
class DraftsService {
private IMailTransmission $transmission;
private LocalMessageMapper $mapper;
private AttachmentService $attachmentService;
@ -91,12 +89,25 @@ class DraftsService implements ILocalMailboxService {
}, $recipients);
}
/**
* @param string $userId
* @param LocalMessage $message
* @return void
*/
public function deleteMessage(string $userId, LocalMessage $message): void {
$this->attachmentService->deleteLocalMessageAttachments($userId, $message->getId());
$this->mapper->deleteWithRecipients($message);
}
/**
* @param Account $account
* @param LocalMessage $message
* @param array<int, string[]> $to
* @param array<int, string[]> $cc
* @param array<int, string[]> $bcc
* @param array $attachments
* @return LocalMessage
*/
public function saveMessage(Account $account, LocalMessage $message, array $to, array $cc, array $bcc, array $attachments = []): LocalMessage {
$toRecipients = self::convertToRecipient($to, Recipient::TYPE_TO);
$ccRecipients = self::convertToRecipient($cc, Recipient::TYPE_CC);
@ -126,6 +137,15 @@ class DraftsService implements ILocalMailboxService {
return $message;
}
/**
* @param Account $account
* @param LocalMessage $message
* @param array<int, string[]> $to
* @param array<int, string[]> $cc
* @param array<int, string[]> $bcc
* @param array $attachments
* @return LocalMessage
*/
public function updateMessage(Account $account, LocalMessage $message, array $to, array $cc, array $bcc, array $attachments = []): LocalMessage {
$toRecipients = self::convertToRecipient($to, Recipient::TYPE_TO);
$ccRecipients = self::convertToRecipient($cc, Recipient::TYPE_CC);
@ -168,7 +188,7 @@ class DraftsService implements ILocalMailboxService {
*/
public function sendMessage(LocalMessage $message, Account $account): void {
try {
$this->transmission->sendLocalMessage($account, $message, true);
$this->transmission->saveLocalDraft($account, $message);
} catch (ClientException|ServiceException $e) {
$this->logger->error('Could not move draft to IMAP', ['exception' => $e]);
// Mark as failed so the message is not moved repeatedly in background
@ -181,6 +201,10 @@ class DraftsService implements ILocalMailboxService {
}
/**
*
* @param int $id
* @param string $userId
* @return LocalMessage
* @throws DoesNotExistException
*/
public function getMessage(int $id, string $userId): LocalMessage {
@ -188,12 +212,8 @@ class DraftsService implements ILocalMailboxService {
}
/**
* @throws NotImplemented
* @return void
*/
public function getMessages(string $userId): array {
throw new NotImplemented('Not implemented');
}
public function flush() {
$messages = $this->mapper->findDueDrafts($this->time->getTime());
if (empty($messages)) {
@ -228,7 +248,7 @@ class DraftsService implements ILocalMailboxService {
'id' => $message->getId(),
]);
} catch (Throwable $e) {
// Failure of one message should not stop sending other messages
// Failure of one message should not stop other messages
// Log and continue
$this->logger->warning('Could not move draft {id} to IMAP: ' . $e->getMessage(), [
'id' => $message->getId(),

140
lib/Service/MailTransmission.php
Просмотреть файл

@ -252,7 +252,7 @@ class MailTransmission implements IMailTransmission {
);
}
public function sendLocalMessage(Account $account, LocalMessage $message, bool $isDraft = false): void {
public function sendLocalMessage(Account $account, LocalMessage $message): void {
$to = new AddressList(
array_map(
static function ($recipient) {
@ -308,17 +308,83 @@ class MailTransmission implements IMailTransmission {
$alias = $this->aliasesService->find($message->getAliasId(), $account->getUserId());
}
if ($isDraft) {
$this->saveDraft($messageData);
} else {
try {
$this->sendMessage($messageData, $message->getInReplyToMessageId(), $alias ?? null);
} catch (SentMailboxNotSetException $e) {
throw new ClientException('Could not send message' . $e->getMessage(), $e->getCode(), $e);
}
try {
$this->sendMessage($messageData, $message->getInReplyToMessageId(), $alias ?? null);
} catch (SentMailboxNotSetException $e) {
throw new ClientException('Could not send message' . $e->getMessage(), $e->getCode(), $e);
}
}
public function saveLocalDraft(Account $account, LocalMessage $message): void {
$messageData = $this->getNewMessageData($message, $account);
$perfLogger = $this->performanceLogger->start('save local draft');
$account = $messageData->getAccount();
$imapMessage = $account->newMessage();
$imapMessage->setTo($messageData->getTo());
$imapMessage->setSubject($messageData->getSubject());
$from = new AddressList([
Address::fromRaw($account->getName(), $account->getEMailAddress()),
]);
$imapMessage->setFrom($from);
$imapMessage->setCC($messageData->getCc());
$imapMessage->setBcc($messageData->getBcc());
$imapMessage->setContent($messageData->getBody());
// build mime body
$headers = [
'From' => $imapMessage->getFrom()->first()->toHorde(),
'To' => $imapMessage->getTo()->toHorde(),
'Cc' => $imapMessage->getCC()->toHorde(),
'Bcc' => $imapMessage->getBCC()->toHorde(),
'Subject' => $imapMessage->getSubject(),
'Date' => Horde_Mime_Headers_Date::create(),
];
$mail = new Horde_Mime_Mail();
$mail->addHeaders($headers);
if ($message->isHtml()) {
$mail->setHtmlBody($imapMessage->getContent());
} else {
$mail->setBody($imapMessage->getContent());
}
$mail->addHeaderOb(Horde_Mime_Headers_MessageId::create());
$perfLogger->step('build local draft message');
// 'Send' the message
$client = $this->imapClientFactory->getClient($account);
try {
$transport = new Horde_Mail_Transport_Null();
$mail->send($transport, false, false);
$perfLogger->step('create IMAP draft message');
// save the message in the drafts folder
$draftsMailboxId = $account->getMailAccount()->getDraftsMailboxId();
if ($draftsMailboxId === null) {
throw new ClientException('No drafts mailbox configured');
}
$draftsMailbox = $this->mailboxMapper->findById($draftsMailboxId);
$this->messageMapper->save(
$client,
$draftsMailbox,
$mail,
[Horde_Imap_Client::FLAG_DRAFT]
);
$perfLogger->step('save local draft message on IMAP');
} catch (DoesNotExistException $e) {
throw new ServiceException('Drafts mailbox does not exist', 0, $e);
} catch (Horde_Exception $e) {
throw new ServiceException('Could not save draft message', 0, $e);
} finally {
$client->logout();
}
$this->eventDispatcher->dispatchTyped(new DraftSavedEvent($account, $messageData, null));
$perfLogger->step('emit post local draft save event');
$perfLogger->end();
}
/**
* @param NewMessageData $message
* @param Message|null $previousDraft
@ -666,4 +732,60 @@ class MailTransmission implements IMailTransmission {
throw new ServiceException('Unable to send mdn for message "' . $message->getId() . '" caused by: ' . $e->getMessage(), 0, $e);
}
}
/**
* @param LocalMessage $message
* @param Account $account
* @return NewMessageData
*/
private function getNewMessageData(LocalMessage $message, Account $account): NewMessageData {
$to = new AddressList(
array_map(
static function ($recipient) {
return Address::fromRaw($recipient->getLabel() ?? $recipient->getEmail(), $recipient->getEmail());
},
$this->groupsIntegration->expand(array_filter($message->getRecipients(), static function (Recipient $recipient) {
return $recipient->getType() === Recipient::TYPE_TO;
}))
)
);
$cc = new AddressList(
array_map(
static function ($recipient) {
return Address::fromRaw($recipient->getLabel() ?? $recipient->getEmail(), $recipient->getEmail());
},
$this->groupsIntegration->expand(array_filter($message->getRecipients(), static function (Recipient $recipient) {
return $recipient->getType() === Recipient::TYPE_CC;
}))
)
);
$bcc = new AddressList(
array_map(
static function ($recipient) {
return Address::fromRaw($recipient->getLabel() ?? $recipient->getEmail(), $recipient->getEmail());
},
$this->groupsIntegration->expand(array_filter($message->getRecipients(), static function (Recipient $recipient) {
return $recipient->getType() === Recipient::TYPE_BCC;
}))
)
);
$attachments = array_map(function (LocalAttachment $attachment) {
// Convert to the untyped nested array used in \OCA\Mail\Controller\AccountsController::send
return [
'type' => 'local',
'id' => $attachment->getId(),
];
}, $message->getAttachments());
return new NewMessageData(
$account,
$to,
$cc,
$bcc,
$message->getSubject(),
$message->getBody(),
$attachments,
$message->isHtml()
);
}
}

41
lib/Service/OutboxService.php
Просмотреть файл

@ -27,7 +27,6 @@ declare(strict_types=1);
namespace OCA\Mail\Service;
use OCA\Mail\Account;
use OCA\Mail\Contracts\ILocalMailboxService;
use OCA\Mail\Contracts\IMailManager;
use OCA\Mail\Contracts\IMailTransmission;
use OCA\Mail\Db\LocalMessage;
@ -44,7 +43,7 @@ use OCP\AppFramework\Utility\ITimeFactory;
use Psr\Log\LoggerInterface;
use Throwable;
class OutboxService implements ILocalMailboxService {
class OutboxService {
/** @var IMailTransmission */
private $transmission;
@ -121,11 +120,23 @@ class OutboxService implements ILocalMailboxService {
return $this->mapper->findById($id, $userId);
}
/**
* @param string $userId
* @param LocalMessage $message
* @return void
*/
public function deleteMessage(string $userId, LocalMessage $message): void {
$this->attachmentService->deleteLocalMessageAttachments($userId, $message->getId());
$this->mapper->deleteWithRecipients($message);
}
/**
* @param LocalMessage $message
* @param Account $account
* @return void
* @throws ClientException
* @throws ServiceException
*/
public function sendMessage(LocalMessage $message, Account $account): void {
try {
$this->transmission->sendLocalMessage($account, $message);
@ -139,6 +150,15 @@ class OutboxService implements ILocalMailboxService {
$this->mapper->deleteWithRecipients($message);
}
/**
* @param Account $account
* @param LocalMessage $message
* @param array<int, string[]> $to
* @param array<int, string[]> $cc
* @param array<int, string[]> $bcc
* @param array $attachments
* @return LocalMessage
*/
public function saveMessage(Account $account, LocalMessage $message, array $to, array $cc, array $bcc, array $attachments = []): LocalMessage {
$toRecipients = self::convertToRecipient($to, Recipient::TYPE_TO);
$ccRecipients = self::convertToRecipient($cc, Recipient::TYPE_CC);
@ -161,6 +181,15 @@ class OutboxService implements ILocalMailboxService {
return $message;
}
/**
* @param Account $account
* @param LocalMessage $message
* @param array<int, string[]> $to
* @param array<int, string[]> $cc
* @param array<int, string[]> $bcc
* @param array $attachments
* @return LocalMessage
*/
public function updateMessage(Account $account, LocalMessage $message, array $to, array $cc, array $bcc, array $attachments = []): LocalMessage {
$toRecipients = self::convertToRecipient($to, Recipient::TYPE_TO);
$ccRecipients = self::convertToRecipient($cc, Recipient::TYPE_CC);
@ -182,6 +211,11 @@ class OutboxService implements ILocalMailboxService {
return $message;
}
/**
* @param Account $account
* @param int $draftId
* @return void
*/
public function handleDraft(Account $account, int $draftId): void {
$message = $this->mailManager->getMessage($account->getUserId(), $draftId);
$this->eventDispatcher->dispatch(
@ -190,6 +224,9 @@ class OutboxService implements ILocalMailboxService {
);
}
/**
* @return void
*/
public function flush(): void {
$messages = $this->mapper->findDue(
$this->timeFactory->getTime()

10
tests/Unit/Service/DraftsServiceTest.php
Просмотреть файл

@ -38,7 +38,6 @@ use OCA\Mail\Db\Message;
use OCA\Mail\Db\Recipient;
use OCA\Mail\Events\DraftMessageCreatedEvent;
use OCA\Mail\Exception\ClientException;
use OCA\Mail\Exception\NotImplemented;
use OCA\Mail\IMAP\IMAPClientFactory;
use OCA\Mail\Service\AccountService;
use OCA\Mail\Service\Attachment\AttachmentService;
@ -112,11 +111,6 @@ class DraftsServiceTest extends TestCase {
$this->time = $this->createMock(ITimeFactory::class);
}
public function testGetMessages(): void {
$this->expectException(NotImplemented::class);
$this->draftsService->getMessages($this->userId);
}
public function testGetMessage(): void {
$message = new LocalMessage();
$message->setAccountId(1);
@ -531,7 +525,7 @@ class DraftsServiceTest extends TestCase {
]);
$this->transmission->expects(self::once())
->method('sendLocalMessage')
->method('saveLocalDraft')
->with($account, $message);
$this->attachmentService->expects(self::once())
->method('deleteLocalMessageAttachments')
@ -563,7 +557,7 @@ class DraftsServiceTest extends TestCase {
]);
$this->transmission->expects(self::once())
->method('sendLocalMessage')
->method('saveLocalDraft')
->with($account, $message)
->willThrowException(new ClientException());
$this->attachmentService->expects(self::never())

8
tests/Unit/Service/MailTransmissionTest.php
Просмотреть файл

@ -556,15 +556,11 @@ class MailTransmissionTest extends TestCase {
'name' => 'Emily',
'alias' => 'Emmerlie'
]);
$this->aliasService->expects(self::once())
->method('find')
->with($message->getAliasId(), $mailAccount->getUserId())
->willReturn($alias);
$replyMessage = new DbMessage();
$replyMessage->setMessageId('abc');
$this->transmission->sendLocalMessage(new Account($mailAccount), $message, true);
$this->transmission->saveLocalDraft(new Account($mailAccount), $message);
}
public function testSendLocalDraftNoDraftsMailbox(): void {
@ -605,6 +601,6 @@ class MailTransmissionTest extends TestCase {
$replyMessage->setMessageId('abc');
$this->expectException(ClientException::class);
$this->transmission->sendLocalMessage(new Account($mailAccount), $message, true);
$this->transmission->sendLocalMessage(new Account($mailAccount), $message);
}
}