diff --git a/lib/Service/Sync/ImapToDbSynchronizer.php b/lib/Service/Sync/ImapToDbSynchronizer.php index c0aa649ed7..0217bb6b3d 100644 --- a/lib/Service/Sync/ImapToDbSynchronizer.php +++ b/lib/Service/Sync/ImapToDbSynchronizer.php @@ -104,6 +104,9 @@ public function syncAccount(Account $account, $snoozeMailboxId = $account->getMailAccount()->getSnoozeMailboxId(); $sentMailboxId = $account->getMailAccount()->getSentMailboxId(); $trashRetentionDays = $account->getMailAccount()->getTrashRetentionDays(); + + $client = $this->clientFactory->getClient($account); + foreach ($this->mailboxMapper->findAll($account) as $mailbox) { $syncTrash = $trashMailboxId === $mailbox->getId() && $trashRetentionDays !== null; $syncSnooze = $snoozeMailboxId === $mailbox->getId(); @@ -116,6 +119,7 @@ public function syncAccount(Account $account, $logger->debug('Syncing ' . $mailbox->getId()); if ($this->sync( $account, + $client, $mailbox, $logger, $criteria, @@ -126,6 +130,9 @@ public function syncAccount(Account $account, $rebuildThreads = true; } } + + $client->logout(); + $this->dispatcher->dispatchTyped( new SynchronizationEvent( $account, @@ -187,6 +194,7 @@ private function resetCache(Account $account, Mailbox $mailbox): void { * @return bool whether to rebuild threads or not */ public function sync(Account $account, + Horde_Imap_Client_Base $client, Mailbox $mailbox, LoggerInterface $logger, int $criteria = Horde_Imap_Client::SYNC_NEWMSGSUIDS | Horde_Imap_Client::SYNC_FLAGSUIDS | Horde_Imap_Client::SYNC_VANISHEDUIDS, @@ -198,7 +206,6 @@ public function sync(Account $account, return $rebuildThreads; } - $client = $this->clientFactory->getClient($account); $client->login(); // Need to login before fetching capabilities. // There is no partial sync when using QRESYNC. As per RFC the client will always pull @@ -271,8 +278,6 @@ public function sync(Account $account, $logger->debug('Unlocking mailbox ' . $mailbox->getId() . ' from new messages sync'); $this->mailboxMapper->unlockFromNewSync($mailbox); } - - $client->logout(); } if (!$batchSync) { diff --git a/lib/Service/Sync/SyncService.php b/lib/Service/Sync/SyncService.php index c127c52486..e4bed5f96a 100644 --- a/lib/Service/Sync/SyncService.php +++ b/lib/Service/Sync/SyncService.php @@ -19,6 +19,7 @@ use OCA\Mail\Exception\MailboxLockedException; use OCA\Mail\Exception\MailboxNotCachedException; use OCA\Mail\Exception\ServiceException; +use OCA\Mail\IMAP\IMAPClientFactory; use OCA\Mail\IMAP\MailboxSync; use OCA\Mail\IMAP\PreviewEnhancer; use OCA\Mail\IMAP\Sync\Response; @@ -29,6 +30,9 @@ use function array_map; class SyncService { + + private IMAPClientFactory $clientFactory; + /** @var ImapToDbSynchronizer */ private $synchronizer; @@ -50,13 +54,16 @@ class SyncService { /** @var MailboxSync */ private $mailboxSync; - public function __construct(ImapToDbSynchronizer $synchronizer, + public function __construct( + IMAPClientFactory $clientFactory, + ImapToDbSynchronizer $synchronizer, FilterStringParser $filterStringParser, MailboxMapper $mailboxMapper, MessageMapper $messageMapper, PreviewEnhancer $previewEnhancer, LoggerInterface $logger, MailboxSync $mailboxSync) { + $this->clientFactory = $clientFactory; $this->synchronizer = $synchronizer; $this->filterStringParser = $filterStringParser; $this->mailboxMapper = $mailboxMapper; @@ -103,9 +110,12 @@ public function syncMailbox(Account $account, if ($partialOnly && !$mailbox->isCached()) { throw MailboxNotCachedException::from($mailbox); } - + + $client = $this->clientFactory->getClient($account); + $this->synchronizer->sync( $account, + $client, $mailbox, $this->logger, $criteria, @@ -115,6 +125,8 @@ public function syncMailbox(Account $account, $this->mailboxSync->syncStats($account, $mailbox); + $client->logout(); + $query = $filter === null ? null : $this->filterStringParser->parse($filter); return $this->getDatabaseSyncChanges( $account, diff --git a/tests/Unit/Service/Sync/SyncServiceTest.php b/tests/Unit/Service/Sync/SyncServiceTest.php index 2d96f89d33..d811ea4577 100644 --- a/tests/Unit/Service/Sync/SyncServiceTest.php +++ b/tests/Unit/Service/Sync/SyncServiceTest.php @@ -9,11 +9,13 @@ namespace OCA\Mail\Tests\Unit\Service\Sync; +use Horde_Imap_Client_Socket; use OCA\Mail\Account; use OCA\Mail\Db\Mailbox; use OCA\Mail\Db\MailboxMapper; use OCA\Mail\Db\MessageMapper; use OCA\Mail\Exception\MailboxNotCachedException; +use OCA\Mail\IMAP\IMAPClientFactory; use OCA\Mail\IMAP\MailboxStats; use OCA\Mail\IMAP\MailboxSync; use OCA\Mail\IMAP\PreviewEnhancer; @@ -21,10 +23,15 @@ use OCA\Mail\Service\Search\FilterStringParser; use OCA\Mail\Service\Sync\ImapToDbSynchronizer; use OCA\Mail\Service\Sync\SyncService; +use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Psr\Log\LoggerInterface; class SyncServiceTest extends TestCase { + + private IMAPClientFactory&MockObject $clientFactory; + private Horde_Imap_Client_Socket&MockObject $client; + /** @var ImapToDbSynchronizer */ private $synchronizer; @@ -52,6 +59,8 @@ class SyncServiceTest extends TestCase { protected function setUp(): void { parent::setUp(); + $this->clientFactory = $this->createMock(IMAPClientFactory::class); + $this->client = $this->createMock(Horde_Imap_Client_Socket::class); $this->synchronizer = $this->createMock(ImapToDbSynchronizer::class); $this->filterStringParser = $this->createMock(FilterStringParser::class); $this->mailboxMapper = $this->createMock(MailboxMapper::class); @@ -61,6 +70,7 @@ protected function setUp(): void { $this->mailboxSync = $this->createMock(MailboxSync::class); $this->syncService = new SyncService( + $this->clientFactory, $this->synchronizer, $this->filterStringParser, $this->mailboxMapper, @@ -102,7 +112,10 @@ public function testSyncMailboxReturnsFolderStats() { [], new MailboxStats(42, 10, null) ); - + $this->clientFactory + ->method('getClient') + ->with($account) + ->willReturn($this->client); $this->messageMapper ->method('findUidsForIds') ->with($mailbox, []) @@ -111,6 +124,7 @@ public function testSyncMailboxReturnsFolderStats() { ->method('sync') ->with( $account, + $this->client, $mailbox, $this->loggerInterface, 0,