diff options
22 files changed, 221 insertions, 369 deletions
diff --git a/lib/Account.php b/lib/Account.php index a27c44bff..36849c067 100644 --- a/lib/Account.php +++ b/lib/Account.php @@ -34,16 +34,11 @@ namespace OCA\Mail; use Horde_Imap_Client; -use Horde_Imap_Client_Ids; use Horde_Imap_Client_Mailbox; use Horde_Imap_Client_Socket; use Horde_Mail_Rfc822_List; use Horde_Mail_Transport; -use Horde_Mail_Transport_Null; use Horde_Mail_Transport_Smtphorde; -use Horde_Mime_Headers_Date; -use Horde_Mime_Headers_MessageId; -use Horde_Mime_Mail; use JsonSerializable; use OC; use OCA\Mail\Cache\Cache; @@ -51,7 +46,6 @@ use OCA\Mail\Db\Alias; use OCA\Mail\Db\MailAccount; use OCA\Mail\Model\IMessage; use OCA\Mail\Model\Message; -use OCA\Mail\Model\ReplyMessage; use OCP\ICacheFactory; use OCP\IConfig; use OCP\Security\ICrypto; @@ -61,9 +55,6 @@ class Account implements JsonSerializable { /** @var MailAccount */ private $account; - /** @var Mailbox[]|null */ - private $mailboxes; - /** @var Horde_Imap_Client_Socket */ private $client; @@ -84,11 +75,9 @@ class Account implements JsonSerializable { */ public function __construct(MailAccount $account) { $this->account = $account; - $this->mailboxes = null; $this->crypto = OC::$server->getCrypto(); $this->config = OC::$server->getConfig(); $this->memcacheFactory = OC::$server->getMemcacheFactory(); - $this->alias = null; } public function getMailAccount() { @@ -170,7 +159,6 @@ class Account implements JsonSerializable { public function createMailbox($mailBox, $opts = []) { $conn = $this->getImapConnection(); $conn->createMailbox($mailBox, $opts); - $this->mailboxes = null; return $this->getMailbox($mailBox); } @@ -184,7 +172,6 @@ class Account implements JsonSerializable { } $conn = $this->getImapConnection(); $conn->deleteMailbox($mailBox); - $this->mailboxes = null; } /** @@ -284,13 +271,4 @@ class Account implements JsonSerializable { return new Message(); } - /** - * Factory method for creating new reply messages - * - * @return ReplyMessage - */ - public function newReplyMessage() { - return new ReplyMessage(); - } - } diff --git a/lib/Contracts/IMailManager.php b/lib/Contracts/IMailManager.php index fee019836..e82077d7a 100644 --- a/lib/Contracts/IMailManager.php +++ b/lib/Contracts/IMailManager.php @@ -30,11 +30,13 @@ use OCA\Mail\Folder; use OCA\Mail\IMAP\FolderStats; use OCA\Mail\IMAP\Sync\Request as SyncRequest; use OCA\Mail\IMAP\Sync\Response as SyncResponse; +use OCA\Mail\Model\IMAPMessage; interface IMailManager { /** * @param Account $account + * * @return Folder[] */ public function getFolders(Account $account): array; @@ -56,8 +58,20 @@ interface IMailManager { public function getFolderStats(Account $account, string $folderId): FolderStats; /** + * @param Account $account + * @param string $mailbox + * @param int $id + * @param bool $loadBody + * + * @return IMAPMessage + * @throws ServiceException + */ + public function getMessage(Account $account, string $mailbox, int $id, bool $loadBody = false): IMAPMessage; + + /** * @param Account * @param SyncRequest $syncRequest + * * @return SyncResponse * * @throws ClientException diff --git a/lib/Controller/AccountsController.php b/lib/Controller/AccountsController.php index a0b0729cc..23e4316e7 100644 --- a/lib/Controller/AccountsController.php +++ b/lib/Controller/AccountsController.php @@ -268,7 +268,7 @@ class AccountsController extends Controller { * * @throws ServiceException */ - public function send(int $accountId, string $subject = null, string $body, string $to, string $cc, string $bcc, int $draftUID = null, string $folderId = null, int $messageId = null, array $attachments = [], int $aliasId = null): JSONResponse { + public function send(int $accountId, string $subject, string $body, string $to, string $cc, string $bcc, int $draftUID = null, string $folderId = null, int $messageId = null, array $attachments = [], int $aliasId = null): JSONResponse { $account = $this->accountService->find($this->currentUserId, $accountId); $alias = $aliasId ? $this->aliasesService->find($aliasId, $this->currentUserId) : null; @@ -277,7 +277,7 @@ class AccountsController extends Controller { $expandedBcc = $this->groupsIntegration->expand($bcc); $messageData = NewMessageData::fromRequest($account, $expandedTo, $expandedCc, $expandedBcc, $subject, $body, $attachments); - $repliedMessageData = new RepliedMessageData($account, $folderId, $messageId); + $repliedMessageData = new RepliedMessageData($account, $folderId === null ? null : base64_decode($folderId), $messageId); try { $this->mailTransmission->sendMessage($this->currentUserId, $messageData, $repliedMessageData, $alias, $draftUID); @@ -301,7 +301,7 @@ class AccountsController extends Controller { * @param int $uid * @return JSONResponse */ - public function draft(int $accountId, string $subject = null, string $body, string $to, string $cc, string $bcc, int $draftUID = null): JSONResponse { + public function draft(int $accountId, string $subject, string $body, string $to, string $cc, string $bcc, int $draftUID = null): JSONResponse { if (is_null($draftUID)) { $this->logger->info("Saving a new draft in account <$accountId>"); } else { diff --git a/lib/Controller/MessagesController.php b/lib/Controller/MessagesController.php index 04fcac35d..e2b291b9c 100755 --- a/lib/Controller/MessagesController.php +++ b/lib/Controller/MessagesController.php @@ -30,8 +30,10 @@ declare(strict_types=1); namespace OCA\Mail\Controller; +use Exception; use OCA\Mail\Account; use OCA\Mail\Contracts\IMailManager; +use OCA\Mail\Exception\ServiceException; use OCA\Mail\Http\AttachmentDownloadResponse; use OCA\Mail\Http\HtmlResponse; use OCA\Mail\Model\IMAPMessage; @@ -39,6 +41,7 @@ use OCA\Mail\Service\AccountService; use OCA\Mail\Service\IMailBox; use OCP\AppFramework\Controller; use OCP\AppFramework\Db\DoesNotExistException; +use OCP\AppFramework\Http; use OCP\AppFramework\Http\ContentSecurityPolicy; use OCP\AppFramework\Http\JSONResponse; use OCP\AppFramework\Http\Response; @@ -95,10 +98,17 @@ class MessagesController extends Controller { * @param IURLGenerator $urlGenerator * @param ITimeFactory $timeFactory */ - public function __construct(string $appName, IRequest $request, - AccountService $accountService, IMailManager $mailManager, string $UserId, - $userFolder, ILogger $logger, IL10N $l10n, IMimeTypeDetector $mimeTypeDetector, - IURLGenerator $urlGenerator, ITimeFactory $timeFactory) { + public function __construct(string $appName, + IRequest $request, + AccountService $accountService, + IMailManager $mailManager, + string $UserId, + $userFolder, + ILogger $logger, + IL10N $l10n, + IMimeTypeDetector $mimeTypeDetector, + IURLGenerator $urlGenerator, + ITimeFactory $timeFactory) { parent::__construct($appName, $request); $this->accountService = $accountService; @@ -142,15 +152,6 @@ class MessagesController extends Controller { return new JSONResponse($json); } - private function loadMessage(int $accountId, string $folderId, int $id) { - $account = $this->getAccount($accountId); - $mailBox = $account->getMailbox(base64_decode($folderId)); - /* @var $message IMAPMessage */ - $message = $mailBox->getMessage($id); - - return $this->enhanceMessage($accountId, $folderId, $id, $message, $mailBox); - } - /** * @NoAdminRequired * @TrapError @@ -159,9 +160,22 @@ class MessagesController extends Controller { * @param string $folderId * @param int $id * @return JSONResponse + * @throws ServiceException */ public function show(int $accountId, string $folderId, int $id): JSONResponse { - return new JSONResponse($this->loadMessage($accountId, $folderId, $id)); + $account = $this->getAccount($accountId); + if ($account === null) { + return new JSONResponse(null, Http::STATUS_FORBIDDEN); + } + + $json = $this->mailManager->getMessage($account, base64_decode($folderId), $id, true)->getFullMessage(); + if (isset($json['attachments'])) { + $json['attachments'] = array_map(function ($a) use ($accountId, $folderId, $id) { + return $this->enrichDownloadUrl($accountId, $folderId, $id, $a); + }, $json['attachments']); + } + + return new JSONResponse($json); } /** @@ -174,7 +188,7 @@ class MessagesController extends Controller { * @param int $destAccountId * @param string $destFolderId * @return JSONResponse - * @throws \Exception + * @throws Exception */ public function move($accountId, $folderId, $id, $destAccountId, $destFolderId): JSONResponse { $srcAccount = $this->accountService->find($this->currentUserId, $accountId); @@ -193,14 +207,28 @@ class MessagesController extends Controller { * @param int $accountId * @param string $folderId * @param int $messageId + * * @return HtmlResponse|TemplateResponse */ public function getHtmlBody(int $accountId, string $folderId, int $messageId): Response { try { - $mailBox = $this->getFolder($accountId, $folderId); + $account = $this->getAccount($accountId); + if ($account === null) { + return new TemplateResponse( + $this->appName, + 'error', + ['message' => 'Not allowed'], + 'none' + ); + } /** @var IMAPMessage $m */ - $m = $mailBox->getMessage($messageId, true); + $m = $this->mailManager->getMessage( + $account, + base64_decode($folderId), + $messageId, + true + ); $html = $m->getHtmlBody($accountId, $folderId, $messageId, function ($cid) use ($m) { $match = array_filter($m->attachments, @@ -208,7 +236,7 @@ class MessagesController extends Controller { return $a['cid'] === $cid; }); $match = array_shift($match); - if (is_null($match)) { + if ($match === null) { return null; } return $match['id']; @@ -230,9 +258,13 @@ class MessagesController extends Controller { $htmlResponse->addHeader('Pragma', 'cache'); return $htmlResponse; - } catch (\Exception $ex) { - return new TemplateResponse($this->appName, 'error', - ['message' => $ex->getMessage()], 'none'); + } catch (Exception $ex) { + return new TemplateResponse( + $this->appName, + 'error', + ['message' => $ex->getMessage()], + 'none' + ); } } @@ -334,19 +366,18 @@ class MessagesController extends Controller { * @param int $accountId * @param string $folderId * @param int $id + * * @return JSONResponse + * @throws ServiceException */ public function destroy(int $accountId, string $folderId, int $id): JSONResponse { $this->logger->debug("deleting message <$id> of folder <$folderId>, account <$accountId>"); - try { - $account = $this->getAccount($accountId); - $this->mailManager->deleteMessage($account, base64_decode($folderId), $id); - return new JSONResponse(); - } catch (DoesNotExistException $e) { - $this->logger->error("could not delete message <$id> of folder <$folderId>, " - . "account <$accountId> because it does not exist"); - throw $e; + $account = $this->getAccount($accountId); + if ($account === null) { + return new JSONResponse(null, Http::STATUS_FORBIDDEN); } + $this->mailManager->deleteMessage($account, base64_decode($folderId), $id); + return new JSONResponse(); } /** @@ -355,10 +386,12 @@ class MessagesController extends Controller { */ private function getAccount($accountId) { if (!array_key_exists($accountId, $this->accounts)) { - $this->accounts[$accountId] = $this->accountService->find($this->currentUserId, - $accountId); + $this->accounts[$accountId] = $this->accountService->find( + $this->currentUserId, + $accountId + ); } - return $this->accounts[$accountId]; + return $this->accounts[$accountId] ?? null; } /** @@ -421,26 +454,4 @@ class MessagesController extends Controller { return $attachment['mime'] === 'text/calendar'; } - /** - * @param int $accountId - * @param string $folderId - * @param int $id - * @param IMAPMessage $m - * @param IMailBox $mailBox - * @return array - */ - private function enhanceMessage(int $accountId, string $folderId, int $id, IMAPMessage $m, - IMailBox $mailBox): array { - $json = $m->getFullMessage($mailBox->getSpecialRole()); - - if (isset($json['attachments'])) { - $json['attachments'] = array_map(function ($a) use ($accountId, $folderId, $id) { - return $this->enrichDownloadUrl($accountId, $folderId, $id, $a); - }, $json['attachments']); - - return $json; - } - return $json; - } - } diff --git a/lib/IMAP/MessageMapper.php b/lib/IMAP/MessageMapper.php index 92a8b638c..d645dee93 100644 --- a/lib/IMAP/MessageMapper.php +++ b/lib/IMAP/MessageMapper.php @@ -33,8 +33,8 @@ use Horde_Imap_Client_Socket; use Horde_Mime_Mail; use OCA\Mail\Db\Mailbox; use OCA\Mail\Exception\ServiceException; -use OCA\Mail\Folder; use OCA\Mail\Model\IMAPMessage; +use OCP\AppFramework\Db\DoesNotExistException; use OCP\ILogger; class MessageMapper { @@ -47,14 +47,31 @@ class MessageMapper { } /** - * @param Horde_Imap_Client_Base $client - * @param Folder $mailbox - * @param array $ids - * + * @return IMAPMessage + * @throws DoesNotExistException + * @throws Horde_Imap_Client_Exception + */ + public function find(Horde_Imap_Client_Base $client, + string $mailbox, + int $id, + bool $loadBody = false): IMAPMessage { + $result = $this->findByIds($client, $mailbox, [$id], $loadBody); + + if (count($result) === 0) { + throw new DoesNotExistException("Message does not exist"); + } + + return $result[0]; + } + + /** * @return IMAPMessage[] * @throws Horde_Imap_Client_Exception */ - public function findByIds(Horde_Imap_Client_Base $client, $mailbox, array $ids) { + public function findByIds(Horde_Imap_Client_Base $client, + string $mailbox, + array $ids, + bool $loadBody = false): array { $query = new Horde_Imap_Client_Fetch_Query(); $query->envelope(); $query->flags(); @@ -77,8 +94,23 @@ class MessageMapper { 'ids' => new Horde_Imap_Client_Ids($ids), ]), false); - return array_map(function (Horde_Imap_Client_Data_Fetch $fetchResult) use ($client, $mailbox) { - return new IMAPMessage($client, $mailbox, $fetchResult->getUid(), $fetchResult); + return array_map(function (Horde_Imap_Client_Data_Fetch $fetchResult) use ($client, $mailbox, $loadBody) { + if ($loadBody) { + return new IMAPMessage( + $client, + $mailbox, + $fetchResult->getUid(), + null, + $loadBody + ); + } else { + return new IMAPMessage( + $client, + $mailbox, + $fetchResult->getUid(), + $fetchResult + ); + } }, $fetchResults); } diff --git a/lib/Listener/FlagRepliedMessageListener.php b/lib/Listener/FlagRepliedMessageListener.php index 3aa6323bc..280eeb3d1 100644 --- a/lib/Listener/FlagRepliedMessageListener.php +++ b/lib/Listener/FlagRepliedMessageListener.php @@ -67,7 +67,7 @@ class FlagRepliedMessageListener implements IEventListener { try { $mailbox = $this->mailboxMapper->find( $event->getAccount(), - base64_decode($event->getRepliedMessageData()->getFolderId()) + $event->getRepliedMessageData()->getFolderId() ); } catch (DoesNotExistException $e) { $this->logger->logException($e, [ diff --git a/lib/Model/IMAPMessage.php b/lib/Model/IMAPMessage.php index 2d76d2155..f90b79091 100644 --- a/lib/Model/IMAPMessage.php +++ b/lib/Model/IMAPMessage.php @@ -60,9 +60,6 @@ class IMAPMessage implements IMessage, JsonSerializable { */ private $attachmentsToIgnore = ['signature.asc', 'smime.p7s']; - /** @var int */ - private $uid; - /** @var Html|null */ private $htmlService; @@ -127,20 +124,9 @@ class IMAPMessage implements IMessage, JsonSerializable { * @return int */ public function getUid(): int { - if (!is_null($this->uid)) { - return $this->uid; - } return $this->fetch->getUid(); } - public function setUid(int $uid) { - $this->uid = $uid; - $this->attachments = array_map(function ($attachment) use ($uid) { - $attachment['messageId'] = $uid; - return $attachment; - }, $this->attachments); - } - /** * @return array */ @@ -236,19 +222,17 @@ class IMAPMessage implements IMessage, JsonSerializable { /** * Get the ID if available * - * @return int|null + * @return string */ - public function getMessageId() { - $e = $this->getEnvelope(); - return $e->message_id; + public function getMessageId(): string { + return $this->getEnvelope()->message_id; } /** * @return string */ public function getSubject(): string { - $e = $this->getEnvelope(); - return $e->subject; + return $this->getEnvelope()->subject; } /** @@ -414,10 +398,9 @@ class IMAPMessage implements IMessage, JsonSerializable { } /** - * @param string $specialRole * @return array */ - public function getFullMessage($specialRole = null): array { + public function getFullMessage(): array { $mailBody = $this->plainMessage; $data = $this->jsonSerialize(); @@ -426,7 +409,7 @@ class IMAPMessage implements IMessage, JsonSerializable { } else { $mailBody = $this->htmlService->convertLinks($mailBody); list($mailBody, $signature) = $this->htmlService->parseMailBody($mailBody); - $data['body'] = $specialRole === 'drafts' ? $mailBody : nl2br($mailBody); + $data['body'] = $mailBody; $data['signature'] = $signature; } @@ -594,14 +577,14 @@ class IMAPMessage implements IMessage, JsonSerializable { /** * @return IMessage|null */ - public function getRepliedMessage() { + public function getInReplyTo() { throw new Exception('not implemented'); } /** * @param IMessage $message */ - public function setRepliedMessage(IMessage $message) { + public function setInReplyTo(string $message) { throw new Exception('not implemented'); } diff --git a/lib/Model/IMessage.php b/lib/Model/IMessage.php index 8898705d2..86fb012ba 100644 --- a/lib/Model/IMessage.php +++ b/lib/Model/IMessage.php @@ -34,7 +34,7 @@ interface IMessage { /** * Get the ID if available * - * @return int|null + * @return string|null */ public function getMessageId(); @@ -91,14 +91,14 @@ interface IMessage { public function setBcc(AddressList $bcc); /** - * @return IMessage|null + * @return string|null */ - public function getRepliedMessage(); + public function getInReplyTo(); /** - * @param IMessage $message + * @param string $message */ - public function setRepliedMessage(IMessage $message); + public function setInReplyTo(string $id); /** * @return string diff --git a/lib/Model/Message.php b/lib/Model/Message.php index 880280aa1..24ee07128 100644 --- a/lib/Model/Message.php +++ b/lib/Model/Message.php @@ -49,8 +49,8 @@ class Message implements IMessage { /** @var AddressList */ private $bcc; - /** @var IMessage */ - private $repliedMessage = null; + /** @var string|null */ + private $inReplyTo = null; /** @var array */ private $flags = []; @@ -74,7 +74,7 @@ class Message implements IMessage { /** * Get the ID * - * @return int|null + * @return string|null */ public function getMessageId() { return null; @@ -153,17 +153,17 @@ class Message implements IMessage { } /** - * @return IMessage|null + * @return string|null */ - public function getRepliedMessage() { - return $this->repliedMessage; + public function getInReplyTo() { + return $this->inReplyTo; } /** * @param IMessage $message */ - public function setRepliedMessage(IMessage $message) { - $this->repliedMessage = $message; + public function setInReplyTo(string $id) { + $this->inReplyTo = $id; } /** diff --git a/lib/Model/NewMessageData.php b/lib/Model/NewMessageData.php index f59163725..5571b9b26 100644 --- a/lib/Model/NewMessageData.php +++ b/lib/Model/NewMessageData.php @@ -44,7 +44,7 @@ class NewMessageData { /** @var AddressList */ private $bcc; - /** @var string|null */ + /** @var string */ private $subject; /** @var string|null */ @@ -58,7 +58,7 @@ class NewMessageData { * @param AddressList $to * @param AddressList $cc * @param AddressList $bcc - * @param string|null $subject + * @param string $subject * @param string|null $body * @param array $attachments */ @@ -66,7 +66,7 @@ class NewMessageData { AddressList $to, AddressList $cc, AddressList $bcc, - string $subject = null, + string $subject, string $body = null , array $attachments = []) { $this->account = $account; @@ -92,13 +92,13 @@ class NewMessageData { string $to = null, string $cc = null, string $bcc = null, - string $subject = null, + string $subject, string $body = null, array $attachments = []) { $toList = AddressList::parse($to ?: ''); $ccList = AddressList::parse($cc ?: ''); $bccList = AddressList::parse($bcc ?: ''); - $attachmentsArray = is_null($attachments) ? [] : $attachments; + $attachmentsArray = $attachments === null ? [] : $attachments; return new self($account, $toList, $ccList, $bccList, $subject, $body, $attachmentsArray); } @@ -132,9 +132,9 @@ class NewMessageData { } /** - * @return string|null + * @return string */ - public function getSubject() { + public function getSubject(): string { return $this->subject; } diff --git a/lib/Model/ReplyMessage.php b/lib/Model/ReplyMessage.php deleted file mode 100644 index 6f865f749..000000000 --- a/lib/Model/ReplyMessage.php +++ /dev/null @@ -1,45 +0,0 @@ -<?php - -declare(strict_types=1); - -/** - * @author Christoph Wurst <christoph@winzerhof-wurst.at> - * - * Mail - * - * This code is free software: you can redistribute it and/or modify - * it under the terms of the GNU Affero General Public License, version 3, - * as published by the Free Software Foundation. - * - * 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, version 3, - * along with this program. If not, see <http://www.gnu.org/licenses/> - * - */ - -namespace OCA\Mail\Model; - -class ReplyMessage extends Message { - - public function setSubject(string $subject) { - // prevent 'Re: Re:' stacking - if (strcasecmp(substr($subject, 0, 4), 'Re: ') === 0) { - parent::setSubject($subject); - } else if (strcasecmp(substr($subject, 0, 4), 'Aw: ') === 0) { - parent::setSubject($subject); - } else if (strcasecmp(substr($subject, 0, 4), 'Wg: ') === 0) { - parent::setSubject($subject); - } else if (strcasecmp(substr($subject, 0, 4), 'Fw: ') === 0) { - parent::setSubject($subject); - } else if (strcasecmp(substr($subject, 0, 5), 'Fwd: ') === 0) { - parent::setSubject($subject); - } else { - parent::setSubject("Re: $subject"); - } - } - -} diff --git a/lib/Service/MailManager.php b/lib/Service/MailManager.php index 62d43e485..8f523691b 100644 --- a/lib/Service/MailManager.php +++ b/lib/Service/MailManager.php @@ -41,6 +41,7 @@ use OCA\Mail\IMAP\MessageMapper; use OCA\Mail\IMAP\Sync\Request; use OCA\Mail\IMAP\Sync\Response; use OCA\Mail\IMAP\Sync\Synchronizer; +use OCA\Mail\Model\IMAPMessage; use OCP\AppFramework\Db\DoesNotExistException; use OCP\EventDispatcher\IEventDispatcher; @@ -147,6 +148,21 @@ class MailManager implements IMailManager { return $this->folderMapper->getFoldersStatusAsObject($client, $folderId); } + public function getMessage(Account $account, string $mailbox, int $id, bool $loadBody = false): IMAPMessage { + $client = $this->imapClientFactory->getClient($account); + + try { + return $this->messageMapper->find( + $client, + $mailbox, + $id, + $loadBody + ); + } catch (Horde_Imap_Client_Exception|DoesNotExistException $e) { + throw new ServiceException("Could not load message", 0, $e); + } + } + /** * @param Account $sourceAccount * @param string $sourceFolderId @@ -174,9 +190,9 @@ class MailManager implements IMailManager { } /** + * @throws ServiceException * @todo evaluate if we should sync mailboxes first * - * @throws ServiceException */ public function deleteMessage(Account $account, string $mailboxId, diff --git a/lib/Service/MailTransmission.php b/lib/Service/MailTransmission.php index fea83402a..65c39990e 100644 --- a/lib/Service/MailTransmission.php +++ b/lib/Service/MailTransmission.php @@ -26,6 +26,7 @@ namespace OCA\Mail\Service; use Exception; use Horde_Exception; use Horde_Imap_Client; +use Horde_Imap_Client_Exception; use Horde_Imap_Client_Mailbox; use Horde_Mail_Transport_Null; use Horde_Mime_Exception; @@ -148,8 +149,8 @@ class MailTransmission implements IMailTransmission { 'Subject' => $message->getSubject(), ]; - if ($message->getRepliedMessage() !== null) { - $headers['In-Reply-To'] = $message->getRepliedMessage()->getMessageId(); + if ($message->getInReplyTo() !== null) { + $headers['In-Reply-To'] = $message->getInReplyTo(); } $mail = new Horde_Mime_Mail(); @@ -190,7 +191,7 @@ class MailTransmission implements IMailTransmission { $account = $message->getAccount(); $imapMessage = $account->newMessage(); $imapMessage->setTo($message->getTo()); - $imapMessage->setSubject($message->getSubject() ?: ''); + $imapMessage->setSubject($message->getSubject()); $from = new AddressList([ new Address($account->getName(), $account->getEMailAddress()), ]); @@ -244,23 +245,23 @@ class MailTransmission implements IMailTransmission { NewMessageData $messageData, RepliedMessageData $replyData): IMessage { // Reply - $message = $account->newReplyMessage(); + $message = $account->newMessage(); + $message->setSubject($messageData->getSubject()); + $message->setTo($messageData->getTo()); - $mailbox = $account->getMailbox(base64_decode($replyData->getFolderId())); - $repliedMessage = $mailbox->getMessage($replyData->getId()); + $client = $this->imapClientFactory->getClient($account); - if ($messageData->getSubject() === null) { - // No subject set – use the original one - $message->setSubject($repliedMessage->getSubject()); - } else { - $message->setSubject($messageData->getSubject()); - } + try { + $repliedMessage = $this->messageMapper->find( + $client, + $replyData->getFolderId(), + $replyData->getId() + ); - // TODO: old code used - // $message->setTo(Message::parseAddressList($repliedMessage->getToList())); - // when $to was null. Needs investigation whether that is needed or even makes sense. - $message->setTo($messageData->getTo()); - $message->setRepliedMessage($repliedMessage); + $message->setInReplyTo($repliedMessage->getMessageId()); + } catch (DoesNotExistException|Horde_Imap_Client_Exception $e) { + // Nothing to do + } return $message; } @@ -269,7 +270,7 @@ class MailTransmission implements IMailTransmission { // New message $message = $account->newMessage(); $message->setTo($messageData->getTo()); - $message->setSubject($messageData->getSubject() ?: ''); + $message->setSubject($messageData->getSubject()); return $message; } diff --git a/tests/Controller/AccountsControllerTest.php b/tests/Controller/AccountsControllerTest.php index 9cb2f4909..5afb4ffbc 100644 --- a/tests/Controller/AccountsControllerTest.php +++ b/tests/Controller/AccountsControllerTest.php @@ -88,7 +88,7 @@ class AccountsControllerTest extends TestCase { /** @var IMailTransmission|PHPUnit_Framework_MockObject_MockObject */ private $transmission; - + /** @var SetupService|PHPUnit_Framework_MockObject_MockObject */ private $setupService; @@ -355,7 +355,7 @@ class AccountsControllerTest extends TestCase { $this->assertEquals($expectedResponse, $response); } - + public function testUpdateManualFailure() { $autoDetect = false; $id = 135; @@ -416,7 +416,7 @@ class AccountsControllerTest extends TestCase { public function testSendReply() { $account = $this->createMock(Account::class); - $folderId = base64_encode('INBOX'); + $folderId = 'INBOX'; $messageId = 1234; $this->accountService->expects($this->once()) ->method('find') @@ -428,7 +428,7 @@ class AccountsControllerTest extends TestCase { ->with($this->userId, $messageData, $replyData, null, null); $expected = new JSONResponse(); - $resp = $this->controller->send(13, 'sub', 'bod', 'to@d.com', '', '', null, $folderId, $messageId, [], null); + $resp = $this->controller->send(13, 'sub', 'bod', 'to@d.com', '', '', null, base64_encode($folderId), $messageId, [], null); $this->assertEquals($expected, $resp); } diff --git a/tests/Controller/MessagesControllerTest.php b/tests/Controller/MessagesControllerTest.php index b65b20bc5..623f3b5e4 100644 --- a/tests/Controller/MessagesControllerTest.php +++ b/tests/Controller/MessagesControllerTest.php @@ -110,22 +110,16 @@ class MessagesControllerTest extends TestCase { $accountId = 17; $folderId = 'testfolder'; $messageId = 4321; + $message = $this->createMock(IMAPMessage::class); $this->accountService->expects($this->once()) ->method('find') ->with($this->equalTo($this->userId), $this->equalTo($accountId)) ->will($this->returnValue($this->account)); - $this->account->expects($this->once()) - ->method('getMailbox') - ->with($this->equalTo($folderId)) - ->will($this->returnValue($this->mailbox)); - $this->mailbox->expects($this->once()) + $this->mailManager->expects($this->once()) ->method('getMessage') - ->with($this->equalTo($messageId)) - ->will($this->returnValue($this->message)); - $this->message->expects($this->once()) - ->method('getHtmlBody') - ->willReturn(''); + ->with($this->account, $folderId, $messageId, true) + ->willReturn($message); $this->timeFactory->method('getTime')->willReturn(1000); $expectedResponse = new HtmlResponse(''); diff --git a/tests/Integration/Service/MailTransmissionIntegrationTest.php b/tests/Integration/Service/MailTransmissionIntegrationTest.php index 5d6be8a3c..8197366f7 100644 --- a/tests/Integration/Service/MailTransmissionIntegrationTest.php +++ b/tests/Integration/Service/MailTransmissionIntegrationTest.php @@ -170,7 +170,7 @@ class MailTransmissionIntegrationTest extends TestCase { ->finish(); $originalUID = $this->saveMessage('inbox', $originalMessage); - $message = NewMessageData::fromRequest($this->account, 'recipient@domain.com', null, null, null, 'hello there', []); + $message = NewMessageData::fromRequest($this->account, 'recipient@domain.com', null, null, '', 'hello there', []); $reply = new RepliedMessageData($this->account, $inbox, $originalUID); $uid = $this->transmission->sendMessage('ferdinand', $message, $reply); diff --git a/tests/Listener/FlagRepliedMessageListenerTest.php b/tests/Listener/FlagRepliedMessageListenerTest.php index 73f3ca4eb..45bd38278 100644 --- a/tests/Listener/FlagRepliedMessageListenerTest.php +++ b/tests/Listener/FlagRepliedMessageListenerTest.php @@ -134,7 +134,7 @@ class FlagRepliedMessageListenerTest extends TestCase { ->willReturn(true); $repliedMessageData->expects($this->once()) ->method('getFolderId') - ->willReturn(base64_encode('INBOX')); + ->willReturn('INBOX'); $this->mailboxMapper->expects($this->once()) ->method('find') ->with($account, 'INBOX') @@ -171,7 +171,7 @@ class FlagRepliedMessageListenerTest extends TestCase { ->willReturn(true); $repliedMessageData->expects($this->once()) ->method('getFolderId') - ->willReturn(base64_encode('INBOX')); + ->willReturn('INBOX'); $repliedMessageData->expects($this->once()) ->method('getId') ->willReturn(321); diff --git a/tests/Model/IMAPMessageTest.php b/tests/Model/IMAPMessageTest.php index 9f1693090..2eca99d08 100644 --- a/tests/Model/IMAPMessageTest.php +++ b/tests/Model/IMAPMessageTest.php @@ -86,12 +86,12 @@ class IMAPMessageTest extends TestCase { public function testSerialize() { $data = new Horde_Imap_Client_Data_Fetch(); + $data->setUid(1234); $m = new IMAPMessage(null, 'INBOX', 123, $data); - $m->setUid('1234'); $json = $m->jsonSerialize(); - $this->assertEquals('1234', $json['id']); + $this->assertEquals(1234, $json['id']); } } diff --git a/tests/Model/MessageTest.php b/tests/Model/MessageTest.php index f0216e00e..b4e42d0ef 100644 --- a/tests/Model/MessageTest.php +++ b/tests/Model/MessageTest.php @@ -107,10 +107,10 @@ class MessageTest extends TestCase { } public function testRepliedMessage() { - $reply = new Message(); + $reply = '9609171955.AA24342@cmstex2.maths.umanitoba.ca'; - $this->message->setRepliedMessage($reply); - $actual = $this->message->getRepliedMessage(); + $this->message->setInReplyTo($reply); + $actual = $this->message->getInReplyTo(); $this->assertSame($reply, $actual); } diff --git a/tests/Model/RepliedMessageTest.php b/tests/Model/RepliedMessageTest.php index be23ac27a..b4004f07a 100644 --- a/tests/Model/RepliedMessageTest.php +++ b/tests/Model/RepliedMessageTest.php @@ -24,6 +24,7 @@ namespace OCA\Mail\Tests\Model; use ChristophWurst\Nextcloud\Testing\TestCase; use OCA\Mail\Account; use OCA\Mail\Model\RepliedMessageData; +use PHPUnit\Framework\MockObject\MockObject; class RepliedMessageTest extends TestCase { @@ -35,8 +36,9 @@ class RepliedMessageTest extends TestCase { } public function testGetFolderId() { + /** @var Account|MockObject $account */ $account = $this->createMock(Account::class); - $folderId = base64_encode('INBOX'); + $folderId = 'INBOX'; $data = new RepliedMessageData($account, $folderId, null); $this->assertEquals($folderId, $data->getFolderId()); diff --git a/tests/Model/ReplyMessageTest.php b/tests/Model/ReplyMessageTest.php deleted file mode 100644 index 0a05f4bfc..000000000 --- a/tests/Model/ReplyMessageTest.php +++ /dev/null @@ -1,132 +0,0 @@ -<?php - -/** - * @author Christoph Wurst <christoph@winzerhof-wurst.at> - * - * Mail - * - * This code is free software: you can redistribute it and/or modify - * it under the terms of the GNU Affero General Public License, version 3, - * as published by the Free Software Foundation. - * - * 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, version 3, - * along with this program. If not, see <http://www.gnu.org/licenses/> - * - */ -namespace OCA\Mail\Tests\Model; - -use OCA\Mail\Model\ReplyMessage; - -class ReplyMessageTest extends MessageTest { - - protected function setUp() { - parent::setUp(); - - $this->message = new ReplyMessage(); - } - - public function testSubject() { - $subject = 'test message'; - - $this->message->setSubject($subject); - - // "Re: " should be added - $this->assertSame("Re: $subject", $this->message->getSubject()); - } - - public function testSubjectReStacking() { - $subject = 'Re: test message'; - - $this->message->setSubject($subject); - - // Subject shouldn't change - $this->assertSame($subject, $this->message->getSubject()); - } - - public function testSubjectReCaseStacking() { - $subject = 'RE: test message'; - - $this->message->setSubject($subject); - - // Subject shouldn't change - $this->assertSame($subject, $this->message->getSubject()); - } - - public function testSubjectAwStacking() { - $subject = 'Aw: test message'; - - $this->message->setSubject($subject); - - // Subject shouldn't change - $this->assertSame($subject, $this->message->getSubject()); - } - - public function testSubjectAwCaseStacking() { - $subject = 'AW: test message'; - - $this->message->setSubject($subject); - - // Subject shouldn't change - $this->assertSame($subject, $this->message->getSubject()); - } - - public function testSubjectWgStacking() { - $subject = 'Wg: test message'; - - $this->message->setSubject($subject); - - // Subject shouldn't change - $this->assertSame($subject, $this->message->getSubject()); - } - - public function testSubjectWgCaseStacking() { - $subject = 'WG: test message'; - - $this->message->setSubject($subject); - - // Subject shouldn't change - $this->assertSame($subject, $this->message->getSubject()); - } - - public function testSubjectFwStacking() { - $subject = 'Fw: test message'; - - $this->message->setSubject($subject); - - // Subject shouldn't change - $this->assertSame($subject, $this->message->getSubject()); - } - - public function testSubjectFwCaseStacking() { - $subject = 'FW: test message'; - - $this->message->setSubject($subject); - - // Subject shouldn't change - $this->assertSame($subject, $this->message->getSubject()); - } - - public function testSubjectFwdStacking() { - $subject = 'Fwd: test message'; - - $this->message->setSubject($subject); - - // Subject shouldn't change - $this->assertSame($subject, $this->message->getSubject()); - } - - public function testSubjectFwdCaseStacking() { - $subject = 'FWD: test message'; - - $this->message->setSubject($subject); - - // Subject shouldn't change - $this->assertSame($subject, $this->message->getSubject()); - } - -} diff --git a/tests/Service/MailTransmissionTest.php b/tests/Service/MailTransmissionTest.php index 6c520f8eb..f7fdc585e 100644 --- a/tests/Service/MailTransmissionTest.php +++ b/tests/Service/MailTransmissionTest.php @@ -22,23 +22,21 @@ namespace OCA\Mail\Tests\Service; use ChristophWurst\Nextcloud\Testing\TestCase; -use Horde_Imap_Client; +use Horde_Imap_Client_Socket; use Horde_Mail_Transport; use OC\Files\Node\File; use OCA\Mail\Account; -use OCA\Mail\Address; -use OCA\Mail\AddressList; use OCA\Mail\Contracts\IAttachmentService; use OCA\Mail\Db\Alias; use OCA\Mail\Db\MailboxMapper; use OCA\Mail\IMAP\IMAPClientFactory; use OCA\Mail\IMAP\MessageMapper; use OCA\Mail\Mailbox; +use OCA\Mail\Model\IMAPMessage; use OCA\Mail\Model\IMessage; use OCA\Mail\Model\Message; use OCA\Mail\Model\NewMessageData; use OCA\Mail\Model\RepliedMessageData; -use OCA\Mail\Model\ReplyMessage; use OCA\Mail\Service\MailTransmission; use OCA\Mail\SMTP\SmtpClientFactory; use OCP\EventDispatcher\IEventDispatcher; @@ -185,22 +183,22 @@ class MailTransmissionTest extends TestCase { /** @var Account|MockObject $account */ $account = $this->createMock(Account::class); $messageData = NewMessageData::fromRequest($account, 'to@d.com', '', '', 'sub', 'bod'); - $folderId = base64_encode('INBOX'); + $folderId = 'INBOX'; $repliedMessageId = 321; $replyData = new RepliedMessageData($account, $folderId, $repliedMessageId); - $message = new ReplyMessage(); + $message = new Message(); $account->expects($this->once()) - ->method('newReplyMessage') + ->method('newMessage') ->willReturn($message); - $mailbox = $this->createMock(Mailbox::class); - $account->expects($this->once()) - ->method('getMailbox') - ->with(base64_decode($folderId)) - ->willReturn($mailbox); - $repliedMessage = $this->createMock(IMessage::class); - $mailbox->expects($this->once()) - ->method('getMessage') - ->with($repliedMessageId) + $client = $this->createMock(Horde_Imap_Client_Socket::class); + $this->imapClientFactory->expects($this->once()) + ->method('getClient') + ->with($account) + ->willReturn($client); + $repliedMessage = $this->createMock(IMAPMessage::class); + $this->messageMapper->expects($this->once()) + ->method('find') + ->with($client, $folderId, $repliedMessageId) ->willReturn($repliedMessage); $transport = $this->createMock(Horde_Mail_Transport::class); $this->smtpClientFactory->expects($this->once()) @@ -219,7 +217,7 @@ class MailTransmissionTest extends TestCase { $account->expects($this->once()) ->method('newMessage') ->willReturn($message); - $client = $this->createMock(\Horde_Imap_Client_Socket::class); + $client = $this->createMock(Horde_Imap_Client_Socket::class); $this->imapClientFactory->expects($this->once()) ->method('getClient') ->with($account) |