diff options
author | Christoph Wurst <christoph@winzerhof-wurst.at> | 2019-08-30 12:19:54 +0300 |
---|---|---|
committer | Christoph Wurst <christoph@winzerhof-wurst.at> | 2019-09-04 12:47:29 +0300 |
commit | 90275abd7796a85c9107eddb7939ed8f1e569c46 (patch) | |
tree | 44ec8d5adb8c684031bdc3f1456cc9bdaca9f2bc /lib | |
parent | 0c59d64478ee20cd1f8371fdda489836af2ca5ce (diff) |
Clean up the code responsible for deleting a message
Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>
Diffstat (limited to 'lib')
-rw-r--r-- | lib/Account.php | 49 | ||||
-rw-r--r-- | lib/AppInfo/Application.php | 11 | ||||
-rw-r--r-- | lib/Contracts/IMailManager.php | 11 | ||||
-rwxr-xr-x | lib/Controller/MessagesController.php | 2 | ||||
-rw-r--r-- | lib/Db/Mailbox.php | 2 | ||||
-rw-r--r-- | lib/Db/MailboxMapper.php | 51 | ||||
-rw-r--r-- | lib/Events/BeforeMessageDeletedEvent.php | 60 | ||||
-rw-r--r-- | lib/IMAP/MailboxSync.php | 4 | ||||
-rw-r--r-- | lib/IMAP/MessageMapper.php | 73 | ||||
-rw-r--r-- | lib/Listener/MessageDeleteTrashCreatorListener.php | 101 | ||||
-rw-r--r-- | lib/Mailbox.php | 4 | ||||
-rw-r--r-- | lib/Service/MailManager.php | 55 |
12 files changed, 358 insertions, 65 deletions
diff --git a/lib/Account.php b/lib/Account.php index 1a2c009e7..78421c0d2 100644 --- a/lib/Account.php +++ b/lib/Account.php @@ -420,55 +420,6 @@ class Account implements JsonSerializable { } /** - * @param string $sourceFolderId - * @param int $messageId - */ - public function deleteMessage($sourceFolderId, $messageId) { - $mb = $this->getMailbox($sourceFolderId); - $hordeSourceMailBox = $mb->getHordeMailBox(); - // by default we will create a 'Trash' folder if no trash is found - $trashId = "Trash"; - $createTrash = true; - - $trashFolders = $this->getSpecialFolder('trash', true); - - if (count($trashFolders) !== 0) { - $trashId = $trashFolders[0]->getFolderId(); - $createTrash = false; - } else { - // no trash -> guess - $trashes = array_filter($this->getMailboxes(), function($box) { - /** - * @var Mailbox $box - */ - return (stripos($box->getDisplayName(), 'trash') !== false); - }); - if (!empty($trashes)) { - $trashId = array_values($trashes); - $trashId = $trashId[0]->getFolderId(); - $createTrash = false; - } - } - - $hordeMessageIds = new Horde_Imap_Client_Ids($messageId); - $hordeTrashMailBox = new Horde_Imap_Client_Mailbox($trashId); - - if ($sourceFolderId === $trashId) { - $this->getImapConnection()->expunge($hordeSourceMailBox, - array('ids' => $hordeMessageIds, 'delete' => true)); - - OC::$server->getLogger()->info("Message expunged: {message} from mailbox {mailbox}", - array('message' => $messageId, 'mailbox' => $sourceFolderId)); - } else { - $this->getImapConnection()->copy($hordeSourceMailBox, $hordeTrashMailBox, - array('create' => $createTrash, 'move' => true, 'ids' => $hordeMessageIds)); - - OC::$server->getLogger()->info("Message moved to trash: {message} from mailbox {mailbox}", - array('message' => $messageId, 'mailbox' => $sourceFolderId, 'app' => 'mail')); - } - } - - /** * @param int $messageId */ private function deleteDraft($messageId) { diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index 6f12dca77..873eef879 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -29,7 +29,9 @@ use OCA\Mail\Contracts\IAvatarService; use OCA\Mail\Contracts\IMailManager; use OCA\Mail\Contracts\IMailTransmission; use OCA\Mail\Contracts\IUserPreferences; +use OCA\Mail\Events\BeforeMessageDeletedEvent; use OCA\Mail\Http\Middleware\ErrorMiddleware; +use OCA\Mail\Listener\MessageDeleteTrashCreatorListener; use OCA\Mail\Service\Attachment\AttachmentService; use OCA\Mail\Service\AvatarService; use OCA\Mail\Service\Group\IGroupService; @@ -38,6 +40,7 @@ use OCA\Mail\Service\MailManager; use OCA\Mail\Service\MailTransmission; use OCA\Mail\Service\UserPreferenceSevice; use OCP\AppFramework\App; +use OCP\EventDispatcher\IEventDispatcher; use OCP\Util; class Application extends App { @@ -46,6 +49,7 @@ class Application extends App { parent::__construct('mail', $urlParams); $this->initializeAppContainer(); + $this->registerEvents(); } private function initializeAppContainer() { @@ -78,4 +82,11 @@ class Application extends App { $container->registerAlias(IGroupService::class, NextcloudGroupService::class); } + private function registerEvents(): void { + /** @var IEventDispatcher $dispatcher */ + $dispatcher = $this->getContainer()->query(IEventDispatcher::class); + + $dispatcher->addServiceListener(BeforeMessageDeletedEvent::class, MessageDeleteTrashCreatorListener::class); + } + } diff --git a/lib/Contracts/IMailManager.php b/lib/Contracts/IMailManager.php index 6382c3c55..fee019836 100644 --- a/lib/Contracts/IMailManager.php +++ b/lib/Contracts/IMailManager.php @@ -71,11 +71,22 @@ interface IMailManager { * @param int $messageId * @param Account $destinationAccount * @param string $destFolderId + * + * @throws ServiceException */ public function moveMessage(Account $sourceAccount, string $sourceFolderId, int $messageId, Account $destinationAccount, string $destFolderId); /** + * @param Account $account + * @param string $mailboxId + * @param int $messageId + * + * @throws ServiceException + */ + public function deleteMessage(Account $account, string $mailboxId, int $messageId): void; + + /** * Mark all messages of a folder as read * * @param Account $account diff --git a/lib/Controller/MessagesController.php b/lib/Controller/MessagesController.php index 94dce68da..04fcac35d 100755 --- a/lib/Controller/MessagesController.php +++ b/lib/Controller/MessagesController.php @@ -340,7 +340,7 @@ class MessagesController extends Controller { $this->logger->debug("deleting message <$id> of folder <$folderId>, account <$accountId>"); try { $account = $this->getAccount($accountId); - $account->deleteMessage(base64_decode($folderId), $id); + $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>, " diff --git a/lib/Db/Mailbox.php b/lib/Db/Mailbox.php index da911f74c..e3509c6dc 100644 --- a/lib/Db/Mailbox.php +++ b/lib/Db/Mailbox.php @@ -43,7 +43,7 @@ use OCP\AppFramework\Db\Entity; * @method void setUnseen(int $unseen) * @method bool getSelectable() * @method void setSelectable(bool $selectable) - * @method bool getSpecialUse() + * @method string getSpecialUse() * @method void setSpecialUse(string $specialUse) */ class Mailbox extends Entity { diff --git a/lib/Db/MailboxMapper.php b/lib/Db/MailboxMapper.php index 5dd357c97..4cf8e1baf 100644 --- a/lib/Db/MailboxMapper.php +++ b/lib/Db/MailboxMapper.php @@ -24,6 +24,9 @@ namespace OCA\Mail\Db; use OCA\Mail\Account; +use OCA\Mail\Exception\ServiceException; +use OCP\AppFramework\Db\DoesNotExistException; +use OCP\AppFramework\Db\MultipleObjectsReturnedException; use OCP\AppFramework\Db\QBMapper; use OCP\IDBConnection; @@ -48,4 +51,52 @@ class MailboxMapper extends QBMapper { return $this->findEntities($select); } + /** + * @throws DoesNotExistException + * @throws ServiceException + */ + public function find(Account $account, string $name): Mailbox { + $qb = $this->db->getQueryBuilder(); + + $select = $qb->select('*') + ->from($this->getTableName()) + ->where( + $qb->expr()->eq('account_id', $qb->createNamedParameter($account->getId())), + $qb->expr()->eq('name', $qb->createNamedParameter($name)) + ); + + try { + return $this->findEntity($select); + } catch (MultipleObjectsReturnedException $e) { + // Not possible due to DB constraints + throw new ServiceException("The impossible has happened", 42, $e); + } + } + + /** + * @throws DoesNotExistException + */ + public function findSpecial(Account $account, string $specialUse): Mailbox { + $mailboxes = $this->findAll($account); + + // First, let's try to detect by special use attribute + foreach ($mailboxes as $mailbox) { + $specialUses = json_decode($mailbox->getSpecialUse(), true) ?? []; + if (in_array($specialUse, $specialUses, true)) { + return $mailbox; + } + } + + // No luck so far, let's do another round and just guess + foreach ($mailboxes as $mailbox) { + // TODO: also check localized name + if (strtolower($mailbox->getName()) === strtolower($specialUse)) { + return $mailbox; + } + } + + // Give up + throw new DoesNotExistException("Special mailbox $specialUse does not exist"); + } + } diff --git a/lib/Events/BeforeMessageDeletedEvent.php b/lib/Events/BeforeMessageDeletedEvent.php new file mode 100644 index 000000000..928ee8abb --- /dev/null +++ b/lib/Events/BeforeMessageDeletedEvent.php @@ -0,0 +1,60 @@ +<?php declare(strict_types=1); + +/** + * @copyright 2019 Christoph Wurst <christoph@winzerhof-wurst.at> + * + * @author 2019 Christoph Wurst <christoph@winzerhof-wurst.at> + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * 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 + * along with this program. If not, see <http://www.gnu.org/licenses/>. + */ + +namespace OCA\Mail\Events; + +use OCA\Mail\Account; +use OCP\EventDispatcher\Event; + +class BeforeMessageDeletedEvent extends Event { + + /** @var Account */ + private $account; + + /** @var string */ + private $folderId; + + /** @var int */ + private $messageId; + + public function __construct(Account $account, string $folderId, int $messageId) { + parent::__construct(); + $this->account = $account; + $this->folderId = $folderId; + $this->messageId = $messageId; + } + + public function getAccount(): Account { + return $this->account; + } + + public function getFolderId(): string { + return $this->folderId; + } + + public function getMessageId(): int { + return $this->messageId; + } + + +} diff --git a/lib/IMAP/MailboxSync.php b/lib/IMAP/MailboxSync.php index b47774d51..bbd19e796 100644 --- a/lib/IMAP/MailboxSync.php +++ b/lib/IMAP/MailboxSync.php @@ -66,8 +66,8 @@ class MailboxSync { $this->logger = $logger; } - public function sync(Account $account): void { - if ($account->getMailAccount()->getLastMailboxSync() >= ($this->timeFactory->getTime() - 7200)) { + public function sync(Account $account, $force = false): void { + if (!$force && $account->getMailAccount()->getLastMailboxSync() >= ($this->timeFactory->getTime() - 7200)) { $this->logger->debug("account is up to date, skipping mailbox sync"); return; } diff --git a/lib/IMAP/MessageMapper.php b/lib/IMAP/MessageMapper.php index ef4b672a0..81c176cb4 100644 --- a/lib/IMAP/MessageMapper.php +++ b/lib/IMAP/MessageMapper.php @@ -30,15 +30,26 @@ use Horde_Imap_Client_Exception; use Horde_Imap_Client_Exception_NoSupportExtension; use Horde_Imap_Client_Fetch_Query; use Horde_Imap_Client_Ids; +use OCA\Mail\Db\Mailbox; +use OCA\Mail\Exception\ServiceException; use OCA\Mail\Folder; use OCA\Mail\Model\IMAPMessage; +use OCP\ILogger; class MessageMapper { + /** @var ILogger */ + private $logger; + + public function __construct(ILogger $logger) { + $this->logger = $logger; + } + /** * @param Horde_Imap_Client_Base $client * @param Folder $mailbox * @param array $ids + * * @return IMAPMessage[] * @throws Horde_Imap_Client_Exception * @throws Horde_Imap_Client_Exception_NoSupportExtension @@ -76,16 +87,29 @@ class MessageMapper { * @param string $sourceFolderId * @param int $messageId * @param string $destFolderId - * @throws Horde_Imap_Client_Exception - * @throws Horde_Imap_Client_Exception_NoSupportExtension */ - public function move(Horde_Imap_Client_Base $client, string $sourceFolderId, - int $messageId, string $destFolderId) { - $client->copy($sourceFolderId, $destFolderId, - [ - 'ids' => new Horde_Imap_Client_Ids($messageId), - 'move' => true, - ]); + public function move(Horde_Imap_Client_Base $client, + string $sourceFolderId, + int $messageId, + string $destFolderId): void { + try { + $client->copy($sourceFolderId, $destFolderId, + [ + 'ids' => new Horde_Imap_Client_Ids($messageId), + 'move' => true, + ]); + } catch (Horde_Imap_Client_Exception $e) { + $this->logger->logException( + $e, + ['level' => ILogger::DEBUG] + ); + + throw new ServiceException( + "Could not move message $$messageId from $sourceFolderId to $destFolderId", + 0, + $e + ); + } } public function markAllRead(Horde_Imap_Client_Base $client, @@ -97,4 +121,35 @@ class MessageMapper { ]); } + /** + * @throws ServiceException + */ + public function expunge(Horde_Imap_Client_Base $client, + string $mailbox, + int $id): void { + try { + $client->expunge( + $mailbox, + [ + 'ids' => new Horde_Imap_Client_Ids([$id]), + 'delete' => true, + ]); + } catch (Horde_Imap_Client_Exception $e) { + $this->logger->logException( + $e, + ['level' => ILogger::DEBUG] + ); + + throw new ServiceException("Could not expunge message $id", 0, $e); + } + + $this->logger->info( + "Message expunged: {message} from mailbox {mailbox}", + [ + 'message' => $id, + 'mailbox' => $mailbox, + ] + ); + } + } diff --git a/lib/Listener/MessageDeleteTrashCreatorListener.php b/lib/Listener/MessageDeleteTrashCreatorListener.php new file mode 100644 index 000000000..c13240458 --- /dev/null +++ b/lib/Listener/MessageDeleteTrashCreatorListener.php @@ -0,0 +1,101 @@ +<?php declare(strict_types=1); + +/** + * @copyright 2019 Christoph Wurst <christoph@winzerhof-wurst.at> + * + * @author 2019 Christoph Wurst <christoph@winzerhof-wurst.at> + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * 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 + * along with this program. If not, see <http://www.gnu.org/licenses/>. + */ + +namespace OCA\Mail\Listener; + +use Horde_Imap_Client_Exception; +use OCA\Mail\Account; +use OCA\Mail\Db\MailboxMapper; +use OCA\Mail\Events\BeforeMessageDeletedEvent; +use OCA\Mail\IMAP\IMAPClientFactory; +use OCA\Mail\IMAP\MailboxSync; +use OCP\AppFramework\Db\DoesNotExistException; +use OCP\EventDispatcher\Event; +use OCP\EventDispatcher\IEventListener; +use OCP\ILogger; + +class MessageDeleteTrashCreatorListener implements IEventListener { + + /** @var MailboxMapper */ + private $mailboxMapper; + + /** @var IMAPClientFactory */ + private $imapClientFactory; + + /** @var MailboxSync */ + private $mailboxSync; + + /** @var ILogger */ + private $logger; + + public function __construct(MailboxMapper $mailboxMapper, + IMAPClientFactory $imapClientFactory, + MailboxSync $mailboxSync, + ILogger $logger) { + $this->mailboxMapper = $mailboxMapper; + $this->imapClientFactory = $imapClientFactory; + $this->mailboxSync = $mailboxSync; + $this->logger = $logger; + } + + public function handle(Event $event): void { + if (!($event instanceof BeforeMessageDeletedEvent)) { + return; + } + + try { + $this->mailboxMapper->findSpecial( + $event->getAccount(), + "trash" + ); + } catch (DoesNotExistException $e) { + $this->logger->debug("Creating trash mailbox"); + $this->createTrash($event->getAccount()); + $this->logger->debug("Trash mailbox created"); + } + } + + private function createTrash(Account $account): void { + $client = $this->imapClientFactory->getClient($account); + + try { + // TODO: localize mailbox name + $client->createMailbox( + "Trash", + [ + "special_use" => [ + \Horde_Imap_Client::SPECIALUSE_TRASH, + ], + ] + ); + } catch (Horde_Imap_Client_Exception $e) { + $this->logger->logException($e, [ + "message" => "Could not creat trash mailbox", + ]); + } + + // TODO: find a more elegant solution for updating the mailbox cache + $this->mailboxSync->sync($account, true); + } + +} diff --git a/lib/Mailbox.php b/lib/Mailbox.php index 934376e34..506425d55 100644 --- a/lib/Mailbox.php +++ b/lib/Mailbox.php @@ -152,7 +152,7 @@ class Mailbox implements IMailBox { /** * Get message page - * + * * Build the list of UIDs for the current page on the client side * * This is done by fetching a list of *all* UIDs and their data, sorting them @@ -479,7 +479,7 @@ class Mailbox implements IMailBox { /** * @return Horde_Imap_Client_Mailbox */ - public function getHordeMailBox() { + public function getHordeMailBox(): Horde_Imap_Client_Mailbox { return $this->mailBox; } diff --git a/lib/Service/MailManager.php b/lib/Service/MailManager.php index 9c5a865cf..62d43e485 100644 --- a/lib/Service/MailManager.php +++ b/lib/Service/MailManager.php @@ -29,6 +29,7 @@ use OCA\Mail\Account; use OCA\Mail\Contracts\IMailManager; use OCA\Mail\Db\Mailbox; use OCA\Mail\Db\MailboxMapper; +use OCA\Mail\Events\BeforeMessageDeletedEvent; use OCA\Mail\Exception\ClientException; use OCA\Mail\Exception\ServiceException; use OCA\Mail\Folder; @@ -40,6 +41,8 @@ use OCA\Mail\IMAP\MessageMapper; use OCA\Mail\IMAP\Sync\Request; use OCA\Mail\IMAP\Sync\Response; use OCA\Mail\IMAP\Sync\Synchronizer; +use OCP\AppFramework\Db\DoesNotExistException; +use OCP\EventDispatcher\IEventDispatcher; class MailManager implements IMailManager { @@ -61,18 +64,23 @@ class MailManager implements IMailManager { /** @var MessageMapper */ private $messageMapper; + /** @var IEventDispatcher */ + private $eventDispatcher; + public function __construct(IMAPClientFactory $imapClientFactory, MailboxMapper $mailboxMapper, MailboxSync $mailboxSync, FolderMapper $folderMapper, Synchronizer $synchronizer, - MessageMapper $messageMapper) { + MessageMapper $messageMapper, + IEventDispatcher $eventDispatcher) { $this->imapClientFactory = $imapClientFactory; $this->mailboxMapper = $mailboxMapper; $this->mailboxSync = $mailboxSync; $this->folderMapper = $folderMapper; $this->synchronizer = $synchronizer; $this->messageMapper = $messageMapper; + $this->eventDispatcher = $eventDispatcher; } /** @@ -145,6 +153,8 @@ class MailManager implements IMailManager { * @param int $messageId * @param Account $destinationAccount * @param string $destFolderId + * + * @throws ServiceException */ public function moveMessage(Account $sourceAccount, string $sourceFolderId, @@ -164,10 +174,53 @@ class MailManager implements IMailManager { } /** + * @todo evaluate if we should sync mailboxes first + * + * @throws ServiceException + */ + public function deleteMessage(Account $account, + string $mailboxId, + int $messageId): void { + $this->eventDispatcher->dispatch( + BeforeMessageDeletedEvent::class, + new BeforeMessageDeletedEvent($account, $mailboxId, $messageId) + ); + + try { + $sourceFolder = $this->mailboxMapper->find($account, $mailboxId); + } catch (DoesNotExistException $e) { + throw new ServiceException("Source mailbox $mailboxId does not exist", 0, $e); + } + try { + $trashFolder = $this->mailboxMapper->findSpecial($account, 'trash'); + } catch (DoesNotExistException $e) { + throw new ServiceException("No trash folder", 0, $e); + } + + if ($mailboxId === $trashFolder->getName()) { + // Delete inside trash -> expunge + $this->messageMapper->expunge( + $this->imapClientFactory->getClient($account), + $sourceFolder->getName(), + $messageId + ); + } else { + $this->messageMapper->move( + $this->imapClientFactory->getClient($account), + $sourceFolder->getName(), + $messageId, + $trashFolder->getName() + ); + } + } + + /** * @param Account $account * @param string $sourceFolderId * @param string $destFolderId * @param int $messageId + * + * @throws ServiceException */ private function moveMessageOnSameAccount(Account $account, string $sourceFolderId, |