Merge pull request #5010 from nextcloud/enhancement/migrate-important…

…-to-label1

Add bg job for label sync

lib/BackgroundJob/MigrateImportantJob.php

@@ -0,0 +1,111 @@
+<?php
+/**
+ * @copyright 2021 Anna Larch <[email protected]>
+ *
+ * @author Anna Larch <[email protected]>
+ * @author Roeland Jago Douma <[email protected]>
+ *
+ * @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\BackgroundJob;
+
+use OCA\Mail\Account;
+use OCA\Mail\Db\MailAccountMapper;
+use OCA\Mail\Db\Mailbox;
+use OCA\Mail\Db\MailboxMapper;
+
+use OCA\Mail\Exception\ServiceException;
+use OCA\Mail\Migration\MigrateImportantFromImapAndDb;
+use OCA\Mail\Service\MailManager;
+use OCP\AppFramework\Db\DoesNotExistException;
+use OCP\AppFramework\Utility\ITimeFactory;
+use OCP\BackgroundJob\QueuedJob;
+use Psr\Log\LoggerInterface;
+
+class MigrateImportantJob extends QueuedJob {
+
+	/** @var MailboxMapper */
+	private $mailboxMapper;
+
+	/** @var MailAccountMapper */
+	private $mailAccountMapper;
+
+	/** @var MailManager */
+	private $mailManager;
+
+	/** @var MigrateImportantFromImapAndDb */
+	private $migration;
+
+	/** @var LoggerInterface */
+	private $logger;
+
+	public function __construct(MailboxMapper $mailboxMapper,
+								MailAccountMapper $mailAccountMapper,
+								MailManager $mailManager,
+								MigrateImportantFromImapAndDb $migration,
+								LoggerInterface $logger,
+								ITimeFactory $timeFactory
+								) {
+		parent::__construct($timeFactory);
+		$this->mailboxMapper = $mailboxMapper;
+		$this->mailAccountMapper = $mailAccountMapper;
+		$this->mailManager = $mailManager;
+		$this->migration = $migration;
+		$this->logger = $logger;
+	}
+
+	/**
+	 * @param array $argument
+	 */
+	public function run($argument) {
+		$mailboxId = (int)$argument['mailboxId'];
+		try {
+			/** @var Mailbox $mailbox*/
+			$mailbox = $this->mailboxMapper->findById($mailboxId);
+		} catch (DoesNotExistException $e) {
+			$this->logger->debug('Could not find mailbox <' . $mailboxId . '>');
+			return;
+		}
+
+		$accountId = $mailbox->getAccountId();
+		try {
+			$mailAccount = $this->mailAccountMapper->findById($accountId);
+		} catch (DoesNotExistException $e) {
+			$this->logger->debug('Could not find account <' . $accountId . '>');
+			return;
+		}
+
+		$account = new Account($mailAccount);
+		if ($this->mailManager->isPermflagsEnabled($account, $mailbox->getName()) === false) {
+			$this->logger->debug('Permflags not enabled for <' . $accountId . '>');
+			return;
+		}
+
+		try {
+			$this->migration->migrateImportantOnImap($account, $mailbox);
+		} catch (ServiceException $e) {
+			$this->logger->debug('Could not flag messages on IMAP for mailbox <' . $mailboxId . '>.');
+		}
+
+		try {
+			$this->migration->migrateImportantFromDb($account, $mailbox);
+		} catch (ServiceException $e) {
+			$this->logger->debug('Could not flag messages from DB on IMAP for mailbox <' . $mailboxId . '>.');
+		}
+	}
+}

lib/Db/MailboxMapper.php

@@ -65,6 +65,21 @@ public function findAll(Account $account): array {
 		return $this->findEntities($select);
 	}
 
+	/**
+	 * @return \Generator<int>
+	 */
+	public function findAllIds(): \Generator {
+		$qb = $this->db->getQueryBuilder();
+		$qb->select('id')
+			->from($this->getTableName());
+
+		$cursor = $qb->execute();
+		while ($row = $cursor->fetch()) {
+			yield (int)$row['id'];
+		}
+		$cursor->closeCursor();
+	}
+
 	/**
 	 * @throws DoesNotExistException
 	 * @throws ServiceException
@@ -246,4 +261,27 @@ public function deleteOrphans(): void {
 			->where($qb2->expr()->in('id', $qb2->createNamedParameter($ids, IQueryBuilder::PARAM_INT_ARRAY), IQueryBuilder::PARAM_INT_ARRAY));
 		$query->execute();
 	}
+
+	/**
+	 * Get all UIDS for mail_messages.flag_important = true
+	 *
+	 * @return int[]
+	 */
+	public function findFlaggedImportantUids(int $mailboxId) : array {
+		$qb = $this->db->getQueryBuilder();
+		$query = $qb->select('uid')
+				->from('mail_messages')
+				->where(
+					$qb->expr()->eq('mailbox_id', $qb->createNamedParameter($mailboxId)),
+					$qb->expr()->eq('flag_important', $qb->createNamedParameter(true, IQueryBuilder::PARAM_BOOL))
+				);
+
+		$cursor = $query->execute();
+		$uids = array_map(function (array $row) {
+			return (int)$row['uid'];
+		}, $cursor->fetchAll());
+		$cursor->closeCursor();
+
+		return $uids;
+	}
 }

lib/IMAP/MessageMapper.php

@@ -28,6 +28,7 @@
 use Horde_Imap_Client_Data_Fetch;
 use Horde_Imap_Client_Exception;
 use Horde_Imap_Client_Fetch_Query;
+use Horde_Imap_Client_Search_Query;
 use Horde_Imap_Client_Ids;
 use Horde_Imap_Client_Socket;
 use Horde_Mime_Mail;
@@ -327,12 +328,12 @@ public function save(Horde_Imap_Client_Socket $client,
 	 */
 	public function addFlag(Horde_Imap_Client_Socket $client,
 							Mailbox $mailbox,
-							int $uid,
+							array $uids,
 							string $flag): void {
 		$client->store(
 			$mailbox->getName(),
 			[
-				'ids' => new Horde_Imap_Client_Ids($uid),
+				'ids' => new Horde_Imap_Client_Ids($uids),
 				'add' => [$flag],
 			]
 		);
@@ -343,17 +344,34 @@ public function addFlag(Horde_Imap_Client_Socket $client,
 	 */
 	public function removeFlag(Horde_Imap_Client_Socket $client,
 							   Mailbox $mailbox,
-							   int $uid,
+							   array $uids,
 							   string $flag): void {
 		$client->store(
 			$mailbox->getName(),
 			[
-				'ids' => new Horde_Imap_Client_Ids($uid),
+				'ids' => new Horde_Imap_Client_Ids($uids),
 				'remove' => [$flag],
 			]
 		);
 	}
 
+	/**
+	 * @param Horde_Imap_Client_Socket $client
+	 * @param Mailbox $mailbox
+	 * @param string $flag
+	 * @return int[]
+	 *
+	 * @throws Horde_Imap_Client_Exception
+	 */
+	public function getFlagged(Horde_Imap_Client_Socket $client,
+							   Mailbox $mailbox,
+							   string $flag): array {
+		$query = new Horde_Imap_Client_Search_Query();
+		$query->flag($flag, true);
+		$messages = $client->search($mailbox->getName(), $query);
+		return $messages['match']->ids ?? [];
+	}
+
 	/**
 	 * @param Horde_Imap_Client_Socket $client
 	 * @param string $mailbox

lib/Listener/DeleteDraftListener.php

@@ -96,7 +96,7 @@ private function deleteDraft(Account $account, Message $draft): void {
 			$this->messageMapper->addFlag(
 				$client,
 				$draftsMailbox,
-				$draft->getUid(), // TODO: the UID could be from another mailbox
+				[$draft->getUid()], // TODO: the UID could be from another mailbox
 				Horde_Imap_Client::FLAG_DELETED
 			);
 		} catch (Horde_Imap_Client_Exception $e) {

lib/Listener/FlagRepliedMessageListener.php

@@ -83,7 +83,7 @@ public function handle(Event $event): void {
 			$this->messageMapper->addFlag(
 				$client,
 				$mailbox,
-				$event->getRepliedMessageData()->getMessage()->getUid(),
+				[$event->getRepliedMessageData()->getMessage()->getUid()],
 				Horde_Imap_Client::FLAG_ANSWERED
 			);
 		} catch (Horde_Imap_Client_Exception $e) {

lib/Migration/MigrateImportantFromImapAndDb.php

@@ -0,0 +1,98 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * @copyright 2021 Anna Larch <[email protected]>
+ *
+ * @author 2021 Anna Larch <[email protected]>
+ *
+ * @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/>.
+ *
+ * @link https://github.com/nextcloud/mail/issues/25
+ * @link https://github.com/nextcloud/mail/issues/4780
+ */
+
+namespace OCA\Mail\Migration;
+
+use Horde_Imap_Client_Exception;
+use OCA\Mail\Account;
+use OCA\Mail\Db\Mailbox;
+use OCA\Mail\Db\MailboxMapper;
+use OCA\Mail\Db\Tag;
+use OCA\Mail\Exception\ServiceException;
+use OCA\Mail\IMAP\IMAPClientFactory;
+use OCA\Mail\IMAP\MessageMapper;
+use Psr\Log\LoggerInterface;
+
+class MigrateImportantFromImapAndDb {
+
+	/** @var IMAPClientFactory */
+	private $clientFactory;
+
+	/** @var MessageMapper */
+	private $messageMapper;
+
+	/** @var MailboxMapper */
+	private $mailboxMapper;
+
+	/** @var LoggerInterface */
+	private $logger;
+
+	public function __construct(IMAPClientFactory $clientFactory,
+								MessageMapper $messageMapper,
+								MailboxMapper $mailboxMapper,
+								LoggerInterface $logger
+								) {
+		$this->clientFactory = $clientFactory;
+		$this->messageMapper = $messageMapper;
+		$this->mailboxMapper = $mailboxMapper;
+		$this->logger = $logger;
+	}
+
+	public function migrateImportantOnImap(Account $account, Mailbox $mailbox): void {
+		$client = $this->clientFactory->getClient($account);
+		//get all messages that have an $important label from IMAP
+		try {
+			$uids = $this->messageMapper->getFlagged($client, $mailbox, '$important');
+		} catch (Horde_Imap_Client_Exception $e) {
+			throw new ServiceException("Could not fetch UIDs of important messages: " . $e->getMessage(), 0, $e);
+		}
+		// add $label1 for all that are tagged on IMAP
+		if (!empty($uids)) {
+			try {
+				$this->messageMapper->addFlag($client, $mailbox, $uids, Tag::LABEL_IMPORTANT);
+			} catch (Horde_Imap_Client_Exception $e) {
+				$this->logger->debug('Could not flag messages in mailbox <' . $mailbox->getId() . '>');
+				throw new ServiceException($e->getMessage(), 0, $e);
+			}
+		}
+	}
+
+	public function migrateImportantFromDb(Account $account, Mailbox $mailbox): void {
+		$client = $this->clientFactory->getClient($account);
+		$uids = $this->mailboxMapper->findFlaggedImportantUids($mailbox->getId());
+		// store our data on imap
+		if (!empty($uids)) {
+			try {
+				$this->messageMapper->addFlag($client, $mailbox, $uids, Tag::LABEL_IMPORTANT);
+			} catch (Horde_Imap_Client_Exception $e) {
+				$this->logger->debug('Could not flag messages in mailbox <' . $mailbox->getId() . '>');
+				throw new ServiceException($e->getMessage(), 0, $e);
+			}
+		}
+	}
+}

lib/Migration/Version1100Date20210512142306.php

@@ -0,0 +1,38 @@
+<?php
+
+declare(strict_types=1);
+
+namespace OCA\Mail\Migration;
+
+use Closure;
+use OCA\Mail\BackgroundJob\MigrateImportantJob;
+use OCA\Mail\Db\MailboxMapper;
+use OCP\BackgroundJob\IJobList;
+use OCP\DB\ISchemaWrapper;
+use OCP\Migration\IOutput;
+use OCP\Migration\SimpleMigrationStep;
+
+class Version1100Date20210512142306 extends SimpleMigrationStep {
+
+	/** @var MailboxMapper */
+	private $mailboxMapper;
+
+	/** @var IJobList */
+	private $jobList;
+
+	public function __construct(MailboxMapper $mailboxMapper, IJobList $jobList) {
+		$this->mailboxMapper = $mailboxMapper;
+		$this->jobList = $jobList;
+	}
+
+	/**
+	 * @param IOutput $output
+	 * @param Closure $schemaClosure The `\Closure` returns a `ISchemaWrapper`
+	 * @param array $options
+	 */
+	public function postSchemaChange(IOutput $output, Closure $schemaClosure, array $options): void {
+		foreach ($this->mailboxMapper->findAllIds() as $mailboxId) {
+			$this->jobList->add(MigrateImportantJob::class, ['mailboxId' => $mailboxId]);
+		}
+	}
+}