fixup! Add imip processing

nextcloud · Jul 29, 2022 · 649689e · 649689e
1 parent e8e68af
commit 649689e
Show file tree

Hide file tree

Showing 4 changed files with 131 additions and 52 deletions.
diff --git a/lib/Db/MessageMapper.php b/lib/Db/MessageMapper.php
@@ -282,7 +282,6 @@ public function insertBulk(Account $account, Message ...$messages): void {
 			$qb1->setValue('flag_notjunk', $qb1->createParameter('flag_notjunk'));
 			$qb1->setValue('flag_important', $qb1->createParameter('flag_important'));
 			$qb1->setValue('flag_mdnsent', $qb1->createParameter('flag_mdnsent'));
-			$qb1->setValue('imip_message', $qb1->createParameter('imip_message'));
 			$qb2 = $this->db->getQueryBuilder();
 
 			$qb2->insert('mail_recipients')
@@ -313,8 +312,6 @@ public function insertBulk(Account $account, Message ...$messages): void {
 				$qb1->setParameter('flag_notjunk', $message->getFlagNotjunk(), IQueryBuilder::PARAM_BOOL);
 				$qb1->setParameter('flag_important', $message->getFlagImportant(), IQueryBuilder::PARAM_BOOL);
 				$qb1->setParameter('flag_mdnsent', $message->getFlagMdnsent(), IQueryBuilder::PARAM_BOOL);
-				$qb1->setParameter('imip_message', $message->isImipMessage(), IQueryBuilder::PARAM_BOOL);
-
 				$qb1->execute();
 
 				$messageId = $qb1->getLastInsertId();
@@ -382,6 +379,7 @@ public function updateBulk(Account $account, bool $permflagsEnabled, Message ...
 				->set('flag_mdnsent', $query->createParameter('flag_mdnsent'))
 				->set('flag_important', $query->createParameter('flag_important'))
 				->set('updated_at', $query->createNamedParameter($this->timeFactory->getTime()))
+				->set('imip_message', $query->createParameter('imip_message'))
 				->where($query->expr()->andX(
 					$query->expr()->eq('uid', $query->createParameter('uid')),
 					$query->expr()->eq('mailbox_id', $query->createParameter('mailbox_id'))
@@ -409,6 +407,7 @@ public function updateBulk(Account $account, bool $permflagsEnabled, Message ...
 					$query->setParameter('flag_notjunk', $message->getFlagNotjunk(), IQueryBuilder::PARAM_BOOL);
 					$query->setParameter('flag_mdnsent', $message->getFlagMdnsent(), IQueryBuilder::PARAM_BOOL);
 					$query->setParameter('flag_important', $message->getFlagImportant(), IQueryBuilder::PARAM_BOOL);
+					$query->setParameter('imip_message', $message->isImipMessage(), IQueryBuilder::PARAM_BOOL);
 					$query->execute();
 					$perf->step('Updated message ' . $message->getId());
 				}
@@ -520,6 +519,51 @@ public function updatePreviewDataBulk(Message ...$messages): array {
 		return $messages;
 	}
 
+	/**
+	 * @param Message ...$messages
+	 *
+	 * @return Message[]
+	 */
+	public function updateImipData(Message ...$messages): array {
+		$this->db->beginTransaction();
+
+		try {
+			$query = $this->db->getQueryBuilder();
+			$query->update($this->getTableName())
+				->set('imip_message', $query->createParameter('imip_message'))
+				->set('imip_error', $query->createParameter('imip_error'))
+				->set('imip_processed', $query->createParameter('imip_processed'))
+				->where($query->expr()->andX(
+					$query->expr()->eq('uid', $query->createParameter('uid')),
+					$query->expr()->eq('mailbox_id', $query->createParameter('mailbox_id'))
+				));
+
+			foreach ($messages as $message) {
+				if (empty($message->getUpdatedFields())) {
+					// Micro optimization
+					continue;
+				}
+
+				$query->setParameter('uid', $message->getUid(), IQueryBuilder::PARAM_INT);
+				$query->setParameter('mailbox_id', $message->getMailboxId(), IQueryBuilder::PARAM_INT);
+				$query->setParameter('imip_message', $message->isImipMessage(), IQueryBuilder::PARAM_BOOL);
+				$query->setParameter('imip_error', $message->isImipError(), IQueryBuilder::PARAM_BOOL);
+				$query->setParameter('imip_processed', $message->isImipProcessed(), IQueryBuilder::PARAM_BOOL);
+
+				$query->execute();
+			}
+
+			$this->db->commit();
+		} catch (Throwable $e) {
+			// Make sure to always roll back, otherwise the outer code runs in a failed transaction
+			$this->db->rollBack();
+
+			throw $e;
+		}
+
+		return $messages;
+	}
+
 	public function deleteAll(Mailbox $mailbox): void {
 		$messageIdQuery = $this->db->getQueryBuilder();
 		$messageIdQuery->select('id')
@@ -1231,7 +1275,7 @@ public function resetInReplyTo(): int {
 	 * that haven't been processed yet
 	 * @return Message[]
 	 */
-	public function findIMipMessages(): array {
+	public function findIMipMessagesAscending(): array {
 		$time = $this->timeFactory->getTime() - 60 * 60 * 24 * 14;
 		$qb = $this->db->getQueryBuilder();
 
@@ -1242,7 +1286,7 @@ public function findIMipMessages(): array {
 				$qb->expr()->eq('imip_processed', $qb->createNamedParameter(false, IQueryBuilder::PARAM_BOOL), IQueryBuilder::PARAM_BOOL),
 				$qb->expr()->eq('imip_error', $qb->createNamedParameter(false, IQueryBuilder::PARAM_BOOL), IQueryBuilder::PARAM_BOOL),
 				$qb->expr()->gt('sent_at', $qb->createNamedParameter($time, IQueryBuilder::PARAM_INT)),
-			);
+			)->orderBy('sent_at', 'ASC'); // make sure we don't process newer messages first
 
 		return $this->findEntities($select);
 	}

diff --git a/lib/Model/IMAPMessage.php b/lib/Model/IMAPMessage.php
@@ -194,6 +194,14 @@ private function getRawReferences(): string {
 		return $references->value_single;
 	}
 
+	private function isImipMessage(): bool {
+		$schedulingHeader = $this->fetch->getEnvelope(); // how can I find out if this has a body part 'ics\text' and a method?
+		if($schedulingHeader === null) {
+			return false;
+		}
+//		return true;
+	}
+
 	private function getRawInReplyTo(): string {
 		return $this->fetch->getEnvelope()->in_reply_to;
 	}
@@ -732,6 +740,22 @@ public function setInReplyTo(string $id) {
 		throw new Exception('not implemented');
 	}
 
+	/**
+	 * @return AddressList
+	 */
+	public function getReplyTo() {
+		return AddressList::fromHorde($this->getEnvelope()->reply_to);
+	}
+
+	/**
+	 * @param string $id
+	 *
+	 * @return void
+	 */
+	public function setReplyTo(string $id) {
+		throw new Exception('not implemented');
+	}
+
 	/**
 	 * Cast all values from an IMAP message into the correct DB format
 	 *
@@ -779,9 +803,9 @@ public function toDbMessage(int $mailboxId, MailAccount $account): Message {
 		$msg->setFlagImportant(in_array('$important', $flags, true) || in_array('$labelimportant', $flags, true) || in_array(Tag::LABEL_IMPORTANT, $flags, true));
 		$msg->setFlagAttachments(false);
 		$msg->setFlagMdnsent(in_array(Horde_Imap_Client::FLAG_MDNSENT, $flags, true));
-
-		// @todo needs https://github.com/nextcloud/mail/pull/6782
-		// $msg->setImipMessage(!empty($this->scheduling));
+		if($this->isImipMessage()) {
+			$msg->setImipMessage(true);
+		}
 
 		$allowed = [
 			Horde_Imap_Client::FLAG_ANSWERED,

diff --git a/lib/Service/IMipService.php b/lib/Service/IMipService.php
@@ -61,7 +61,7 @@ public function __construct(
 	}
 
 	public function process(): void {
-		$messages = $this->messageMapper->findIMipMessages();
+		$messages = $this->messageMapper->findIMipMessagesAscending();
 
 		// Collect all mailboxes in memory
 		$mailboxIds = array_unique(array_map(function (Message $message) {
@@ -71,7 +71,7 @@ public function process(): void {
 		$mailboxes = array_combine($mailboxIds, array_map(function (int $mailboxId) {
 			try {
 				return $this->mailboxMapper->findById($mailboxId);
-			} catch (DoesNotExistException $e) {
+			} catch (DoesNotExistException | ServiceException $e) {
 				return null;
 			}
 		}, $mailboxIds));
@@ -89,60 +89,62 @@ public function process(): void {
 			}
 		}, $accountIds));
 
-		// build the updated messages on a per-account basis, so we can bulk update for each account
-		$processedMessages = [];
+		// @todo loop through mailboxes with associated messages instead
 		foreach ($messages as $message) {
 			/** @var Mailbox $mailbox */
 			$mailbox = $mailboxes[$message->getMailboxId()];
+			if( $mailbox === null ) {
+				$message->setImipProcessed(true); // Silently drop from passing to DAV and mark as processed, so we won't run into this message again.
+				continue;
+			}
+
 			/** @var Account $account */
 			$account = $accounts[$mailbox->getAccountId()];
-			// mailbox not in collated array, maybe specal use?
+
+			// Check for accounts or mailboxes that no longer exist,
 			// no processing for drafts and sent items
-			if ($mailbox->isSpecialUse("sent") || $mailbox->isSpecialUse("drafts")) { // does this need more use cases? Also probably won't work? @todo
+			if ($account === null || $mailbox->isSpecialUse("sent") || $mailbox->isSpecialUse("drafts")) { // does this need more use cases? Also probably won't work? @todo
 				$message->setImipProcessed(true); // Silently drop from passing to DAV and mark as processed, so we won't run into this message again.
-				$processedMessages[$account->getId()][] = $message;
 				continue;
 			}
 
-
 			try {
-				$imapMessage = $this->mailManager->getImapMessageForScheduleProcessing($account, $mailbox, $message->getUid());
+				$imapMessages = $this->mailManager->getImapMessagesForScheduleProcessing($account, $mailbox, [$message->getUid()]); // UIDS should return an ordered result, no? @todo
 			} catch (ServiceException $e) {
-				$message->setImipError(true);
-				$processedMessages[$account->getId()][] = $message;
-				continue;
+				$this->logger->error('Could not get IMAP messages form IMIP server', ['exception' => $e]);
+				return;
 			}
 
-			if (empty($imapMessage->scheduling[0])) {
-				// No scheduling info, maybe the DB is wrong
-				$message->setImipError(true);
-				$processedMessages[$account->getId()][] = $message;
-				continue;
-			}
+			foreach($imapMessages as $imapMessage) {
+				if (empty($imapMessage->scheduling[0])) {
+					// No scheduling info, maybe the DB is wrong
+					$message->setImipError(true);
+					continue;
+				}
+
+				$principalUri = 'principals/users/' . $account->getUserId();
+				$sender = $imapMessage->getFrom()->first()->getEmail();
+				$recipient = $account->getEmail();
+				foreach($imapMessage->scheduling as $schedulingInfo) { // an IMAP message could contain more than one iMIP object
+					if ($schedulingInfo['method'] === 'REPLY') {
+						$processed = $this->calendarManager->handleIMipReply($principalUri, $sender, $recipient, $schedulingInfo['contents']);
+						$message->setImipProcessed($processed);
+						$message->setImipError(!$processed);
+					} elseif ($schedulingInfo['method'] === 'CANCEL') {
+						try {
+							$replyTo = $imapMessage->getReplyTo()->first()->getEmail();
+						} catch (\Exception $e) {
+							$replyTo = null;
+						}
+						$processed = $this->calendarManager->handleIMipCancel($principalUri, $sender, $replyTo, $recipient, $schedulingInfo['contents']);
+						$message->setImipProcessed($processed);
+						$message->setImipError(!$processed);
+					}
+				}
 
-			$principalUri = 'principals/users/' . $account->getUserId();
-			$sender = $imapMessage->getFrom()->first()->getEmail();
-			$recipient = $account->getEmail();
-			$processed = false;
-			if ($imapMessage->scheduling[0]['method'] === 'REPLY') {
-				$processed = $this->calendarManager->handleIMipReply($principalUri, $sender, $recipient, $imapMessage->scheduling[0]['contents']);
-			} elseif ($imapMessage->scheduling['method'] === 'CANCEL') {
-				$processed = $this->calendarManager->handleIMipCancel($principalUri, $sender, $recipient, $imapMessage->scheduling[0]['contents']);
 			}
-			$message->setImipProcessed($processed);
-			$message->setImipError(!$processed);
-			$processedMessages[$account->getId()][] = $message;
-		}
 
-		foreach ($accountIds as $accountId) {
-			if (!isset($processedMessages[$accountId])) {
-				continue;
-			}
-			try {
-				$this->messageMapper->updateBulk($accounts[$accountId], false, ...$processedMessages[$accountId]);
-			} catch (\Throwable $e) {
-				$this->logger->error('Could not update iMip messages for account ' . $accountId, ['exception' => $e]);
-			}
 		}
+		$this->messageMapper->updateImipData(...$messages);
 	}
 }
diff --git a/lib/Service/MailManager.php b/lib/Service/MailManager.php
@@ -26,6 +26,7 @@
 use Horde_Imap_Client;
 use Horde_Imap_Client_Exception;
 use Horde_Imap_Client_Exception_NoSupportExtension;
+use Horde_Imap_Client_Ids;
 use Horde_Imap_Client_Socket;
 use OCA\Mail\Account;
 use OCA\Mail\Contracts\IMailManager;
@@ -194,16 +195,24 @@ public function getImapMessage(Account $account,
 		}
 	}
 
-	public function getImapMessageForScheduleProcessing(Account $account,
+	/**
+	 * @param Account $account
+	 * @param Mailbox $mailbox
+	 * @param int[] $uid
+	 * @param bool $loadBody
+	 * @return IMAPMessage[]
+	 * @throws ServiceException
+	 */
+	public function getImapMessagesForScheduleProcessing(Account $account,
 		Mailbox $mailbox,
-		int $uid,
-		bool $loadBody = false): IMAPMessage {
+		array $uid,
+		bool $loadBody = false): array {
 		$client = $this->imapClientFactory->getClient($account);
 		try {
-			return $this->imapMessageMapper->find(
+			return $this->imapMessageMapper->findByIds(
 				$client,
 				$mailbox->getName(),
-				$uid,
+				new Horde_Imap_Client_Ids($uid),
 				true
 			);
 		} catch (Horde_Imap_Client_Exception|DoesNotExistException $e) {