fix(imap): pass through $loadBody in ImapMessageConnector::fetchMessages

.github/workflows/test.yml

@@ -171,8 +171,15 @@ jobs:
               image: stalwartlabs/stalwart:v0.15.5
               env:
                   STALWART_ADMIN_PASSWORD: ${{ env.STALWART_PWD }}
+              # Pin a stable container hostname. Stalwart derives the authority of
+              # its published JMAP discovery URLs (apiUrl/downloadUrl/uploadUrl) from
+              # the container hostname + internal port, which is otherwise a random
+              # container id the runner cannot resolve. With a known hostname we can
+              # map it to loopback via /etc/hosts and publish the internal port 1:1
+              # so the advertised http://stalwart:8080/jmap/ is reachable.
+              options: --hostname stalwart
               ports:
-                  - 10080:8080
+                  - 8080:8080
                   - 10025:25
                   - 10143:143
                   - 10993:993
@@ -213,13 +220,15 @@ jobs:
               ports:
                 - 6379:6379
       steps:
+          - name: Map Stalwart hostname to loopback
+            run: echo "127.0.0.1 stalwart" | sudo tee -a /etc/hosts
           - name: Create domain and account in Stalwart
             run: |
-              curl -sf -X POST http://localhost:10080/api/principal \
+              curl -sf -X POST http://stalwart:8080/api/principal \
                 -u "admin:${{ env.STALWART_PWD }}" \
                 -H 'Content-Type: application/json' \
                 -d '{"type":"domain","name":"example.com"}'
-              curl -sf -X POST http://localhost:10080/api/principal \
+              curl -sf -X POST http://stalwart:8080/api/principal \
                 -u "admin:${{ env.STALWART_PWD }}" \
                 -H 'Content-Type: application/json' \
                 -d '{"type":"individual","name":"user@example.com","secrets":["mypassword"],"emails":["user@example.com"],"roles":["user"]}'

composer.json

@@ -43,7 +43,7 @@
 		"phpmailer/dkimvalidator": "^0.3.1",
 		"rubix/ml": "2.5.5",
 		"sabberworm/php-css-parser": "^9.4.0",
-		"sebastiankrupinski/jmap-client-php": "^2.0.0",
+		"sebastiankrupinski/jmap-client-php": "^2.1.0",
 		"wamania/php-stemmer": "4.0 as 3.0",
 		"youthweb/urllinker": "^2.1.0"
 	},

lib/AppInfo/Application.php

@@ -17,7 +17,6 @@
 use OCA\Mail\Contracts\IAvatarService;
 use OCA\Mail\Contracts\IDkimService;
 use OCA\Mail\Contracts\IDkimValidator;
-use OCA\Mail\Contracts\IMailManager;
 use OCA\Mail\Contracts\IMailSearch;
 use OCA\Mail\Contracts\IMailTransmission;
 use OCA\Mail\Contracts\ITrustedSenderService;
@@ -63,7 +62,6 @@
 use OCA\Mail\Service\AvatarService;
 use OCA\Mail\Service\DkimService;
 use OCA\Mail\Service\DkimValidator;
-use OCA\Mail\Service\MailManager;
 use OCA\Mail\Service\MailTransmission;
 use OCA\Mail\Service\Search\MailSearch;
 use OCA\Mail\Service\TrustedSenderService;
@@ -121,7 +119,6 @@ public function register(IRegistrationContext $context): void {
 
 		$context->registerServiceAlias(IAvatarService::class, AvatarService::class);
 		$context->registerServiceAlias(IAttachmentService::class, AttachmentService::class);
-		$context->registerServiceAlias(IMailManager::class, MailManager::class);
 		$context->registerServiceAlias(IMailSearch::class, MailSearch::class);
 		$context->registerServiceAlias(IMailTransmission::class, MailTransmission::class);
 		$context->registerServiceAlias(ITrustedSenderService::class, TrustedSenderService::class);

lib/BackgroundJob/ContextChat/SubmitContentJob.php

@@ -15,7 +15,6 @@
 use OCA\Mail\Db\MessageMapper;
 use OCA\Mail\Exception\ServiceException;
 use OCA\Mail\Exception\SmimeDecryptException;
-use OCA\Mail\IMAP\IMAPClientFactory;
 use OCA\Mail\Service\AccountService;
 use OCA\Mail\Service\ContextChat\TaskService;
 use OCA\Mail\Service\MailManager;
@@ -36,7 +35,6 @@ public function __construct(
 		private AccountService $accountService,
 		private MailManager $mailManager,
 		private MessageMapper $messageMapper,
-		private IMAPClientFactory $clientFactory,
 		private ContextChatProvider $contextChatProvider,
 		private IContentManager $contentManager,
 		private LoggerInterface $logger,
@@ -108,50 +106,39 @@ protected function run($argument): void {
 			return;
 		}
 
-		$client = $this->clientFactory->getClient($account);
 		$items = [];
 
-		try {
-			$startTime = $this->time->getTime();
-			foreach ($messages as $message) {
-				if ($this->time->getTime() - $startTime > ContextChatProvider::CONTEXT_CHAT_JOB_INTERVAL) {
-					break;
-				}
-				try {
-					$imapMessage = $this->mailManager->getImapMessage($client, $account, $mailbox, $message->getUid(), true);
-				} catch (ServiceException $e) {
-					// couldn't load message, let's skip it. Retrying would be too costly
-					continue;
-				} catch (SmimeDecryptException $e) {
-					// encryption problem, skip this message
-					continue;
-				}
-
-				// Skip encrypted messages
-				if ($imapMessage->isEncrypted()) {
-					continue;
-				}
-
-				$fullMessage = $imapMessage->getFullMessage($imapMessage->getUid(), true);
-
-				$items[] = new ContentItem(
-					"{$mailbox->getId()}:{$message->getId()}",
-					$this->contextChatProvider->getId(),
-					$imapMessage->getSubject(),
-					$fullMessage['body'] ?? '',
-					'E-Mail',
-					$imapMessage->getSentDate(),
-					[$account->getUserId()],
-				);
+		$startTime = $this->time->getTime();
+		foreach ($messages as $message) {
+			if ($this->time->getTime() - $startTime > ContextChatProvider::CONTEXT_CHAT_JOB_INTERVAL) {
+				break;
 			}
-		} catch (\Throwable $e) {
-			$this->logger->warning('Exception occurred when trying to fetch messages for context chat', ['exception' => $e]);
-		} finally {
 			try {
-				$client->close();
-			} catch (\Horde_Imap_Client_Exception $e) {
-				$this->logger->debug('Failed to close IMAP client', ['exception' => $e]);
+				$imapMessage = $this->mailManager->getImapMessage($account, $mailbox, $message, true);
+			} catch (ServiceException $e) {
+				// couldn't load message, let's skip it. Retrying would be too costly
+				continue;
+			} catch (SmimeDecryptException $e) {
+				// encryption problem, skip this message
+				continue;
+			}
+
+			// Skip encrypted messages
+			if ($imapMessage->isEncrypted()) {
+				continue;
 			}
+
+			$fullMessage = $imapMessage->getFullMessage($imapMessage->getUid(), true);
+
+			$items[] = new ContentItem(
+				"{$mailbox->getId()}:{$message->getId()}",
+				$this->contextChatProvider->getId(),
+				$imapMessage->getSubject(),
+				$fullMessage['body'] ?? '',
+				'E-Mail',
+				$imapMessage->getSentDate(),
+				[$account->getUserId()],
+			);
 		}
 
 		if (count($items) > 0) {

lib/BackgroundJob/FollowUpClassifierJob.php

@@ -9,13 +9,13 @@
 
 namespace OCA\Mail\BackgroundJob;
 
-use OCA\Mail\Contracts\IMailManager;
 use OCA\Mail\Db\Message;
 use OCA\Mail\Db\ThreadMapper;
 use OCA\Mail\Exception\ClientException;
 use OCA\Mail\Exception\ServiceException;
 use OCA\Mail\Service\AccountService;
 use OCA\Mail\Service\AiIntegrations\AiIntegrationsService;
+use OCA\Mail\Service\MailManager;
 use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\BackgroundJob\QueuedJob;
 use OCP\DB\Exception;
@@ -31,7 +31,7 @@ public function __construct(
 		ITimeFactory $time,
 		private LoggerInterface $logger,
 		private AccountService $accountService,
-		private IMailManager $mailManager,
+		private MailManager $mailManager,
 		private AiIntegrationsService $aiService,
 		private ThreadMapper $threadMapper,
 	) {
@@ -104,12 +104,12 @@ public function run($argument): void {
 
 		$this->logger->debug("Message requires follow-up: {$message->getId()}");
 		$tag = $this->mailManager->createTag('Follow up', '#d77000', $userId);
-		$this->mailManager->tagMessage(
+		$this->mailManager->tagMessages(
 			$account,
-			$mailbox->getName(),
-			$message,
+			$mailbox,
 			$tag,
 			true,
+			$message,
 		);
 	}
 }

lib/BackgroundJob/MigrateImportantJob.php

@@ -13,8 +13,8 @@
 use OCA\Mail\Db\MailAccountMapper;
 use OCA\Mail\Db\MailboxMapper;
 use OCA\Mail\Exception\ServiceException;
-use OCA\Mail\IMAP\IMAPClientFactory;
 use OCA\Mail\Migration\MigrateImportantFromImapAndDb;
+use OCA\Mail\Protocol\ProtocolFactory;
 use OCA\Mail\Service\MailManager;
 use OCP\AppFramework\Db\DoesNotExistException;
 use OCP\AppFramework\Utility\ITimeFactory;
@@ -29,7 +29,7 @@ public function __construct(
 		private MigrateImportantFromImapAndDb $migration,
 		private LoggerInterface $logger,
 		ITimeFactory $timeFactory,
-		private IMAPClientFactory $imapClientFactory,
+		private ProtocolFactory $protocolFactory,
 	) {
 		parent::__construct($timeFactory);
 	}
@@ -58,10 +58,10 @@ public function run($argument) {
 		}
 
 		$account = new Account($mailAccount);
-		$client = $this->imapClientFactory->getClient($account);
+		$client = $this->protocolFactory->imapClient($account);
 
 		try {
-			if ($this->mailManager->isPermflagsEnabled($client, $account, $mailbox->getName()) === false) {
+			if ($this->mailManager->isPermflagsEnabled($account, $mailbox) === false) {
 				$this->logger->debug("Permflags not enabled for <{$accountId}>");
 				return;
 			}

lib/BackgroundJob/QuotaJob.php

@@ -8,8 +8,8 @@
 
 namespace OCA\Mail\BackgroundJob;
 
-use OCA\Mail\Contracts\IMailManager;
 use OCA\Mail\Service\AccountService;
+use OCA\Mail\Service\MailManager;
 use OCP\AppFramework\Db\DoesNotExistException;
 use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\BackgroundJob\IJobList;
@@ -28,7 +28,7 @@ public function __construct(
 		ITimeFactory $time,
 		IUserManager $userManager,
 		private AccountService $accountService,
-		private IMailManager $mailManager,
+		private MailManager $mailManager,
 		IManager $notificationManager,
 		private LoggerInterface $logger,
 		IJobList $jobList,

lib/BackgroundJob/RepairSyncJob.php

@@ -9,8 +9,10 @@
 
 namespace OCA\Mail\BackgroundJob;
 
+use OCA\Mail\Db\MailAccount;
 use OCA\Mail\Db\MailboxMapper;
 use OCA\Mail\Events\SynchronizationEvent;
+use OCA\Mail\Protocol\ProtocolFactory;
 use OCA\Mail\Service\AccountService;
 use OCA\Mail\Service\Sync\SyncService;
 use OCP\AppFramework\Db\DoesNotExistException;
@@ -25,6 +27,7 @@ class RepairSyncJob extends TimedJob {
 	public function __construct(
 		ITimeFactory $time,
 		private SyncService $syncService,
+		private ProtocolFactory $protocolFactory,
 		private AccountService $accountService,
 		private IUserManager $userManager,
 		private MailboxMapper $mailboxMapper,
@@ -55,6 +58,15 @@ protected function run($argument): void {
 			return;
 		}
 
+		if ($account->getMailAccount()->getProtocol() !== MailAccount::PROTOCOL_IMAP) {
+			$this->logger->debug(sprintf(
+				'Account %d uses %s, skipping IMAP repair sync after mailbox refresh',
+				$account->getId(),
+				$account->getMailAccount()->getProtocol(),
+			));
+			return;
+		}
+
 		$user = $this->userManager->get($account->getUserId());
 		if ($user === null || !$user->isEnabled()) {
 			$this->logger->debug(sprintf(
@@ -65,6 +77,10 @@ protected function run($argument): void {
 			return;
 		}
 
+		$this->protocolFactory
+			->mailboxConnector($account)
+			->syncAll($account, true);
+
 		$rebuildThreads = false;
 		$trashMailboxId = $account->getMailAccount()->getTrashMailboxId();
 		$snoozeMailboxId = $account->getMailAccount()->getSnoozeMailboxId();

lib/BackgroundJob/SyncJob.php

@@ -10,11 +10,11 @@
 
 use Horde_Imap_Client_Exception;
 use OCA\Mail\AppInfo\Application;
+use OCA\Mail\Db\MailAccount;
 use OCA\Mail\Exception\IncompleteSyncException;
 use OCA\Mail\Exception\ServiceException;
-use OCA\Mail\IMAP\MailboxSync;
+use OCA\Mail\Protocol\ProtocolFactory;
 use OCA\Mail\Service\AccountService;
-use OCA\Mail\Service\Sync\ImapToDbSynchronizer;
 use OCP\AppFramework\Db\DoesNotExistException;
 use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\BackgroundJob\IJobList;
@@ -38,8 +38,7 @@ public function __construct(
 		ITimeFactory $time,
 		IUserManager $userManager,
 		private AccountService $accountService,
-		private MailboxSync $mailboxSync,
-		private ImapToDbSynchronizer $syncService,
+		private ProtocolFactory $protocolFactory,
 		private LoggerInterface $logger,
 		IJobList $jobList,
 		private readonly IConfig $config,
@@ -87,7 +86,8 @@ protected function run($argument) {
 			return;
 		}
 
-		if (!$account->getMailAccount()->canAuthenticateImap()) {
+		if ($account->getMailAccount()->getProtocol() === MailAccount::PROTOCOL_IMAP
+			&& !$account->getMailAccount()->canAuthenticateImap()) {
 			$this->logger->debug('No authentication on IMAP possible, skipping background sync job');
 			return;
 		}
@@ -128,8 +128,12 @@ protected function run($argument) {
 		}
 
 		try {
-			$this->mailboxSync->sync($account, $this->logger, true);
-			$this->syncService->syncAccount($account, $this->logger);
+			$this->protocolFactory
+				->mailboxConnector($account)
+				->syncAll($account, true);
+			$this->protocolFactory
+				->messageConnector($account)
+				->syncAll($account, false);
 		} catch (IncompleteSyncException $e) {
 			$this->logger->warning($e->getMessage(), [
 				'exception' => $e,