Merge pull request #7221 from nextcloud/perf/outbox-do-not-call-client

Remove unneccesary IMAP client call from outbox

2 files changed, 78 insertions, 13 deletions

diff --git a/lib/Service/OutboxService.php b/lib/Service/OutboxService.php
index 991f8968b..3b4454073 100644
--- a/lib/Service/OutboxService.php
+++ b/lib/Service/OutboxService.php
@@ -145,6 +145,11 @@ class OutboxService implements ILocalMailboxService {
 		$bccRecipients = self::convertToRecipient($bcc, Recipient::TYPE_BCC);
 		$message = $this->mapper->saveWithRecipients($message, $toRecipients, $ccRecipients, $bccRecipients);
 
+		if (empty($attachments)) {
+			$message->setAttachments($attachments);
+			return $message;
+		}
+
 		$client = $this->clientFactory->getClient($account);
 		try {
 			$attachmentIds = $this->attachmentService->handleAttachments($account, $attachments, $client);
@@ -162,6 +167,11 @@ class OutboxService implements ILocalMailboxService {
 		$bccRecipients = self::convertToRecipient($bcc, Recipient::TYPE_BCC);
 		$message = $this->mapper->updateWithRecipients($message, $toRecipients, $ccRecipients, $bccRecipients);
 
+		if (empty($attachments)) {
+			$message->setAttachments($this->attachmentService->updateLocalMessageAttachments($account->getUserId(), $message, []));
+			return $message;
+		}
+
 		$client = $this->clientFactory->getClient($account);
 		try {
 			$attachmentIds = $this->attachmentService->handleAttachments($account, $attachments, $client);
diff --git a/tests/Unit/Service/OutboxServiceTest.php b/tests/Unit/Service/OutboxServiceTest.php
index f077b9f77..f5262b157 100644
--- a/tests/Unit/Service/OutboxServiceTest.php
+++ b/tests/Unit/Service/OutboxServiceTest.php
@@ -248,6 +248,51 @@ class OutboxServiceTest extends TestCase {
 		$this->outboxService->saveMessage($account, $message, $to, $cc, $bcc, $attachments);
 	}
 
+	public function testSaveMessageNoAttachments(): void {
+		$message = new LocalMessage();
+		$message->setAccountId(1);
+		$message->setSendAt($this->time->getTime());
+		$message->setSubject('Test');
+		$message->setBody('Test Test Test');
+		$message->setHtml(true);
+		$message->setInReplyToMessageId('abcd');
+		$to = [
+			[
+				'label' => 'Lewis',
+				'email' => 'tent-living@startdewvalley.com',
+				'type' => Recipient::TYPE_TO,
+			]
+		];
+		$cc = [];
+		$bcc = [];
+		$attachments = [];
+		$rTo = Recipient::fromParams([
+			'label' => 'Lewis',
+			'email' => 'tent-living@startdewvalley.com',
+			'type' => Recipient::TYPE_TO,
+		]);
+		$message2 = $message;
+		$message2->setId(10);
+		$account = $this->createConfiguredMock(Account::class, [
+			'getUserId' => $this->userId
+		]);
+
+		$this->mapper->expects(self::once())
+			->method('saveWithRecipients')
+			->with($message, [$rTo], $cc, $bcc)
+			->willReturn($message2);
+		$this->clientFactory->expects(self::never())
+			->method('getClient');
+		$this->attachmentService->expects(self::never())
+			->method('handleAttachments');
+		$this->attachmentService->expects(self::never())
+			->method('saveLocalMessageAttachments');
+
+		$result = $this->outboxService->saveMessage($account, $message, $to, $cc, $bcc, $attachments);
+		$this->assertEquals($message2->getId(), $result->getId());
+		$this->assertEmpty($result->getAttachments());
+	}
+
 	public function testUpdateMessage(): void {
 		$message = new LocalMessage();
 		$message->setId(10);
@@ -305,44 +350,54 @@ class OutboxServiceTest extends TestCase {
 		$this->outboxService->updateMessage($account, $message, $to, $cc, $bcc, $attachments);
 	}
 
-
-	public function testSaveMessageNoAttachments(): void {
+	public function testUpdateMessageNoAttachments(): void {
 		$message = new LocalMessage();
+		$message->setId(10);
 		$message->setAccountId(1);
 		$message->setSendAt($this->time->getTime());
 		$message->setSubject('Test');
 		$message->setBody('Test Test Test');
 		$message->setHtml(true);
 		$message->setInReplyToMessageId('abcd');
+		$old = Recipient::fromParams([
+			'label' => 'Pam',
+			'email' => 'BuyMeAnAle@startdewvalley.com',
+			'type' => Recipient::TYPE_TO,
+		]);
+		$message->setRecipients([$old]);
 		$to = [
 			[
-				'label' => 'Pam',
-				'email' => 'BuyMeAnAle@startdewvalley.com',
+				'label' => 'Linus',
+				'email' => 'tent-living@startdewvalley.com',
 				'type' => Recipient::TYPE_TO,
 			]
 		];
 		$cc = [];
 		$bcc = [];
+		$attachments = [];
 		$rTo = Recipient::fromParams([
-			'label' => 'Pam',
-			'email' => 'BuyMeAnAle@startdewvalley.com',
+			'label' => 'Linus',
+			'email' => 'tent-living@startdewvalley.com',
 			'type' => Recipient::TYPE_TO,
 		]);
 		$message2 = $message;
-		$message2->setId(10);
+		$message2->setRecipients([$rTo]);
 		$account = $this->createConfiguredMock(Account::class, [
 			'getUserId' => $this->userId
 		]);
-
 		$this->mapper->expects(self::once())
-			->method('saveWithRecipients')
+			->method('updateWithRecipients')
 			->with($message, [$rTo], $cc, $bcc)
 			->willReturn($message2);
 		$this->attachmentService->expects(self::once())
-			->method('saveLocalMessageAttachments')
-			->with($this->userId, 10, []);
-
-		$this->outboxService->saveMessage($account, $message, $to, $cc, $bcc);
+			->method('updateLocalMessageAttachments')
+			->with($this->userId, $message2, $attachments);
+		$this->clientFactory->expects(self::never())
+			->method('getClient');
+		$this->attachmentService->expects(self::never())
+			->method('handleAttachments');
+		$result = $this->outboxService->updateMessage($account, $message, $to, $cc, $bcc, $attachments);
+		$this->assertEmpty($result->getAttachments());
 	}
 
 	public function testSaveMessageError(): void {