From 32d9f12968afa8db4d136eb6829f6222075d3d61 Mon Sep 17 00:00:00 2001
From: Anna Larch <anna@nextcloud.com>
Date: Tue, 12 Apr 2022 23:00:10 +0200
Subject: [PATCH] Fix message attachments not sending

Signed-off-by: Anna Larch <anna@nextcloud.com>
---
 lib/Db/LocalMessageMapper.php                | 29 ++++++++++++++------
 lib/Service/Attachment/AttachmentStorage.php |  9 ++++--
 lib/Service/MailTransmission.php             | 15 +++++-----
 lib/Service/OutboxService.php                |  5 +++-
 4 files changed, 39 insertions(+), 19 deletions(-)

diff --git a/lib/Db/LocalMessageMapper.php b/lib/Db/LocalMessageMapper.php
index 71029e672a..d99550aced 100644
--- a/lib/Db/LocalMessageMapper.php
+++ b/lib/Db/LocalMessageMapper.php
@@ -28,7 +28,6 @@
 use OCP\AppFramework\Db\DoesNotExistException;
 use OCP\DB\Exception as DBException;
 use Throwable;
-use function array_filter;
 use function array_map;
 use OCP\AppFramework\Db\QBMapper;
 use OCP\DB\QueryBuilder\IQueryBuilder;
@@ -133,19 +132,31 @@ public function findDue(int $time): array {
 				$qb->expr()->lte('send_at', $qb->createNamedParameter($time, IQueryBuilder::PARAM_INT), IQueryBuilder::PARAM_INT)
 			);
 		$messages = $this->findEntities($select);
+
+		if (empty($messages)) {
+			return [];
+		}
+
 		$ids = array_map(function (LocalMessage $message) {
 			return $message->getId();
 		}, $messages);
+
 		$attachments = $this->attachmentMapper->findByLocalMessageIds($ids);
 		$recipients = $this->recipientMapper->findByLocalMessageIds($ids);
-		return array_map(static function ($message) use ($attachments, $recipients) {
-			$message->setAttachments(array_filter($attachments, function (LocalAttachment $attachment) use ($message) {
-				return $attachment->getLocalMessageId() === $message->getId();
-			}));
-			$message->setRecipients(array_filter($recipients, function (Recipient $recipient) use ($message) {
-				return $recipient->getLocalMessageId() === $message->getId();
-			}));
-			return $message;
+
+		$recipientMap = [];
+		foreach ($recipients as $r) {
+			$recipientMap[$r->getLocalMessageId()][] = $r;
+		}
+		$attachmentMap = [];
+		foreach ($attachments as $a) {
+			$attachmentMap[$a->getLocalMessageId()][] = $a;
+		}
+
+		return array_map(static function ($localMessage) use ($attachmentMap, $recipientMap) {
+			$localMessage->setAttachments($attachmentMap[$localMessage->getId()] ?? []);
+			$localMessage->setRecipients($recipientMap[$localMessage->getId()] ?? []);
+			return $localMessage;
 		}, $messages);
 	}
 
diff --git a/lib/Service/Attachment/AttachmentStorage.php b/lib/Service/Attachment/AttachmentStorage.php
index 7c31bd842a..fdf7228a23 100644
--- a/lib/Service/Attachment/AttachmentStorage.php
+++ b/lib/Service/Attachment/AttachmentStorage.php
@@ -23,7 +23,6 @@
 
 namespace OCA\Mail\Service\Attachment;
 
-use Exception;
 use OCA\Mail\Exception\AttachmentNotFoundException;
 use OCA\Mail\Exception\UploadException;
 use OCP\Files\IAppData;
@@ -127,6 +126,12 @@ public function retrieve(string $userId, int $attachmentId) {
 	}
 
 	public function delete(string $userId, int $attachmentId): void {
-		throw new Exception('not implemented');
+		$folder = $this->getAttachmentFolder($userId);
+		try {
+			$file = $folder->getFile((string)$attachmentId);
+		} catch (NotFoundException $e) {
+			return;
+		}
+		$file->delete();
 	}
 }
diff --git a/lib/Service/MailTransmission.php b/lib/Service/MailTransmission.php
index f5868ebb9f..66b2d72475 100644
--- a/lib/Service/MailTransmission.php
+++ b/lib/Service/MailTransmission.php
@@ -245,6 +245,13 @@ public function sendLocalMessage(Account $account, LocalMessage $message): void
 			})
 			)
 		);
+		$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());
 		$messageData = new NewMessageData(
 			$account,
 			$to,
@@ -252,13 +259,7 @@ public function sendLocalMessage(Account $account, LocalMessage $message): void
 			$bcc,
 			$message->getSubject(),
 			$message->getBody(),
-			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()),
+			$attachments,
 			$message->isHtml()
 		);
 
diff --git a/lib/Service/OutboxService.php b/lib/Service/OutboxService.php
index 6c7d0470f3..ce37548490 100644
--- a/lib/Service/OutboxService.php
+++ b/lib/Service/OutboxService.php
@@ -177,6 +177,10 @@ public function flush(): void {
 			$this->timeFactory->getTime()
 		);
 
+		if (empty($messages)) {
+			return;
+		}
+
 		$accountIds = array_unique(array_map(function ($message) {
 			return $message->getAccountId();
 		}, $messages));
@@ -187,7 +191,6 @@ public function flush(): void {
 
 		foreach ($messages as $message) {
 			try {
-				// TODO: memoize accounts
 				$this->sendMessage(
 					$message,
 					$accounts[$message->getAccountId()],