fix: use single connection to sync all mailboxes

Signed-off-by: SebastianKrupinski <[email protected]>
nextcloud · Sep 12, 2024 · ccbfb1b · ccbfb1b
1 parent 927b731
commit ccbfb1b
Show file tree

Hide file tree

Showing 3 changed files with 15 additions and 15 deletions.
diff --git a/lib/Service/Sync/ImapToDbSynchronizer.php b/lib/Service/Sync/ImapToDbSynchronizer.php
@@ -109,10 +109,9 @@ public function syncAccount(Account $account,
 		$snoozeMailboxId = $account->getMailAccount()->getSnoozeMailboxId();
 		$sentMailboxId = $account->getMailAccount()->getSentMailboxId();
 		$trashRetentionDays = $account->getMailAccount()->getTrashRetentionDays();
-		// construct imap client and perform initial connection
+
 		$client = $this->clientFactory->getClient($account);
-		$client->login();
-		// perform mail sync on all mail boxes
+
 		foreach ($this->mailboxMapper->findAll($account) as $mailbox) {
 			$syncTrash = $trashMailboxId === $mailbox->getId() && $trashRetentionDays !== null;
 			$syncSnooze = $snoozeMailboxId === $mailbox->getId();
@@ -136,8 +135,9 @@ public function syncAccount(Account $account,
 				$rebuildThreads = true;
 			}
 		}
-		// disconnect imap client
+
 		$client->logout();
+
 		$this->dispatcher->dispatchTyped(
 			new SynchronizationEvent(
 				$account,
@@ -210,6 +210,9 @@ public function sync(Account $account,
 		if ($mailbox->getSelectable() === false) {
 			return $rebuildThreads;
 		}
+
+		$client->login(); // Need to login before fetching capabilities.
+
 		// There is no partial sync when using QRESYNC. As per RFC the client will always pull
 		// all changes. This is a cheap operation when using QRESYNC as the server keeps track
 		// of a client's state through the sync token. We could just update the sync tokens and

diff --git a/lib/Service/Sync/SyncService.php b/lib/Service/Sync/SyncService.php
@@ -30,8 +30,8 @@
 use function array_map;
 
 class SyncService {
-	/** @var IMAPClientFactory */
-	private $clientFactory;
+
+	private IMAPClientFactory $clientFactory;
 
 	/** @var ImapToDbSynchronizer */
 	private $synchronizer;
@@ -110,10 +110,9 @@ public function syncMailbox(Account $account,
 		if ($partialOnly && !$mailbox->isCached()) {
 			throw MailboxNotCachedException::from($mailbox);
 		}
-		// construct imap client and perform initial connection
+
 		$client = $this->clientFactory->getClient($account);
-		$client->login();
-		// perform mail sync on all mail boxes
+
 		$this->synchronizer->sync(
 			$account,
 			$client,
@@ -125,7 +124,7 @@ public function syncMailbox(Account $account,
 		);
 
 		$this->mailboxSync->syncStats($account, $mailbox);
-		// disconnect imap client
+
 		$client->logout();
 
 		$query = $filter === null ? null : $this->filterStringParser->parse($filter);

diff --git a/tests/Unit/Service/Sync/SyncServiceTest.php b/tests/Unit/Service/Sync/SyncServiceTest.php
@@ -24,15 +24,13 @@
 use OCA\Mail\Service\Sync\ImapToDbSynchronizer;
 use OCA\Mail\Service\Sync\SyncService;
 use PHPUnit\Framework\TestCase;
+use PHPUnit\Framework\MockObject\MockObject;
 use Psr\Log\LoggerInterface;
 
 class SyncServiceTest extends TestCase {
 
-	/** @var IMAPClientFactory */
-	private $clientFactory;
-
-	/** @var Horde_Imap_Client_Socket */
-	private $client;
+	private IMAPClientFactory&MockObject $clientFactory;
+	private Horde_Imap_Client_Socket&MockObject $client;
 
 	/** @var ImapToDbSynchronizer */
 	private $synchronizer;