Merge pull request #6230 from nextcloud/fix/attachments-not-sent

Fix message attachments not sending
nextcloud · Apr 13, 2022 · 80f38ed · 80f38ed
2 parents 1282378 + 32d9f12
commit 80f38ed
Show file tree

Hide file tree

Showing 4 changed files with 39 additions and 19 deletions.
diff --git 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
@@ -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
@@ -245,20 +245,21 @@ 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,
 			$cc,
 			$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
@@ -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()],