From 0269bc6d28138e94ce2a45406a32220c28820cf7 Mon Sep 17 00:00:00 2001 From: Matthieu Date: Mon, 29 Jun 2026 17:33:59 +0200 Subject: [PATCH] =?UTF-8?q?fix(mail)=20:=20stop=20le=20spam=20GlitchTip=20?= =?UTF-8?q?de=20sync=20(double-encodage=20UTF7=20+=20dossiers=20fant=C3=B4?= =?UTF-8?q?mes)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Deux causes racines généraient ~170 erreurs/cycle (toutes les 10 min) sur la prod : "syncFolder[...] listMessages failed: Folder ... not found". 1. Double-encodage UTF7-IMAP : listFolders() stocke le chemin brut UTF7-IMAP, mais ImapMailProvider rappelait getFolder($path) qui ré-encode UTF8->UTF7-IMAP (webklex Client::getFolderByPath, utf7=false). Le caractère de shift "&" était ré-encodé, rendant introuvables les dossiers à accents/specials. Fix : getFolder($path, null, utf7: true) partout dans ImapMailProvider. 2. Dossiers fantômes jamais purgés : syncFolderStructure() gardait en DB les dossiers disparus du serveur, re-tentés à chaque cycle. Fix : syncFolderStructure() retourne le set des chemins présents sur le serveur ; doSyncAll() skip silencieusement les dossiers DB absents (conservés en DB pour les liens messages/tâches). Fallback historique si listFolders échoue. Test : testSyncAllSkipsFoldersNoLongerPresentOnServer. --- docs/mail-integration.md | 6 +- .../Application/Service/MailSyncService.php | 32 ++++++---- .../Infrastructure/Imap/ImapMailProvider.php | 16 +++-- tests/Unit/Service/MailSyncServiceTest.php | 63 +++++++++++++++++++ 4 files changed, 97 insertions(+), 20 deletions(-) diff --git a/docs/mail-integration.md b/docs/mail-integration.md index 5e4544a..babb7c4 100644 --- a/docs/mail-integration.md +++ b/docs/mail-integration.md @@ -23,10 +23,14 @@ > 7. 139 connexions IMAP (une/dossier) → throttling OVH → réutilisation d'1 connexion (`closeConnection()` sur l'interface) + reconnexion ciblée après dossier en erreur. > - Contrat front/back réaligné dans `frontend/services/mail.ts` (route `/mail/folders/{path}/messages`, mapping `messages→items`, `fromAddress→fromEmail`, détail plat→imbriqué). > +> ### Bugs corrigés 2026-06-29 (spam GlitchTip `syncFolder[...] listMessages failed: Folder ... not found`) +> Deux causes racines, ~170 erreurs/cycle (toutes les 10 min) sur la prod : +> 1. **Double-encodage UTF7-IMAP** : `listFolders()` stocke `$folder->path` = nom **brut UTF7-IMAP** (webklex `Folder::$path`). `ImapMailProvider` appelait ensuite `$client->getFolder($path)` qui ré-encode UTF8→UTF7-IMAP (`Client::getFolderByPath`, `$utf7=false`) → le `&` (shift UTF7) est ré-encodé → dossiers à accents/specials introuvables. **Fix** : `getFolder($path, null, utf7: true)` partout dans `ImapMailProvider` (les paths sont déjà UTF7-IMAP). Résout les ~7 dossiers à encodage spécial qui étaient « skippés ». +> 2. **Dossiers fantômes jamais purgés** : `syncFolderStructure()` gardait en DB les dossiers disparus du serveur (Trash vidé, dossiers RH supprimés) → re-tentés à chaque cycle → `listMessages` → "not found" → log error en boucle. **Fix** : `syncFolderStructure()` retourne le set des chemins **présents sur le serveur** ; `doSyncAll()` skip silencieusement les dossiers DB absents de ce set (gardés en DB pour les liens messages/tâches, mais plus synchronisés). Si `listFolders` échoue (retour `null`), fallback = sync de tous les dossiers connus (comportement historique). +> > ### Points en suspens / à savoir > - **Mise à jour auto** = cron OS lançant `make mail-sync` toutes les 10 min (cf `docs/mail-cron-setup.md`). **Pas configuré en dev** — lancer à la main. > - **Bouton "Actualiser"** : dispatch async Messenger (`MailSyncRequested → async`). Sans worker `messenger:consume async` qui tourne, les demandes s'empilent sans s'exécuter. En prod : supervisor. En dev : lancer un worker. -> - **~7 dossiers/139** à encodage spécial (ex: `INBOX/RH/.../SÉBASTIEN` en UTF7-modifié) ou réponses vides sont skippés proprement et réessayés au cycle suivant. Edge case webklex non bloquant. > - **Dépendance** : `webklex/php-imap ^6.2` tire des paquets Laravel (`illuminate/*` via `carbon ^3`) dans ce projet Symfony — fonctionnel mais à valider en review. > - 6 PHPUnit Notices (mocks sans expectations) non bloquantes. > diff --git a/src/Module/Mail/Application/Service/MailSyncService.php b/src/Module/Mail/Application/Service/MailSyncService.php index ba90b9d..e0ebfc4 100644 --- a/src/Module/Mail/Application/Service/MailSyncService.php +++ b/src/Module/Mail/Application/Service/MailSyncService.php @@ -64,14 +64,22 @@ final class MailSyncService } } - public function syncFolderStructure(): void + /** + * Synchronise the local folder list with the server. + * + * @return null|array the set of folder paths currently present + * on the server, or null when the remote list + * could not be fetched (the caller then falls + * back to syncing every known folder) + */ + public function syncFolderStructure(): ?array { try { $remoteFolders = $this->provider->listFolders(); } catch (MailProviderException $e) { $this->logger->error('syncFolderStructure: listFolders failed: '.$e->getMessage()); - return; + return null; } $remotePathSet = []; @@ -95,16 +103,7 @@ final class MailSyncService $this->entityManager->flush(); - $allDbFolders = $this->folderRepository->findAllOrderedByPath(); - - foreach ($allDbFolders as $dbFolder) { - if (!isset($remotePathSet[$dbFolder->getPath()])) { - $this->logger->warning(sprintf( - 'syncFolderStructure: folder "%s" no longer exists on server — keeping in DB for safety', - $dbFolder->getPath() - )); - } - } + return $remotePathSet; } public function syncFolder(MailFolder $folder): MailSyncReport @@ -259,7 +258,7 @@ final class MailSyncService private function doSyncAll(DateTimeImmutable $startedAt): MailSyncReport { - $this->syncFolderStructure(); + $remotePathSet = $this->syncFolderStructure(); $totalCreated = 0; $totalUpdated = 0; @@ -270,6 +269,13 @@ final class MailSyncService $folders = $this->folderRepository->findAllOrderedByPath(); foreach ($folders as $folder) { + // Skip folders that no longer exist on the server. They are kept in + // the DB (linked messages and tasks still reference them) but retrying + // listMessages every cycle only floods the logs with "Folder not found". + if (null !== $remotePathSet && !isset($remotePathSet[$folder->getPath()])) { + continue; + } + try { $report = $this->syncFolder($folder); $totalCreated += $report->createdCount; diff --git a/src/Module/Mail/Infrastructure/Imap/ImapMailProvider.php b/src/Module/Mail/Infrastructure/Imap/ImapMailProvider.php index 57e9b69..2c48ed3 100644 --- a/src/Module/Mail/Infrastructure/Imap/ImapMailProvider.php +++ b/src/Module/Mail/Infrastructure/Imap/ImapMailProvider.php @@ -103,7 +103,11 @@ final class ImapMailProvider implements MailProviderInterface $client = $this->getClient(); try { - $folder = $client->getFolder($folderPath); + // Folder paths are stored exactly as the server returns them (raw + // UTF7-IMAP). Pass utf7: true so webklex matches them as-is instead of + // re-encoding UTF8 -> UTF7-IMAP, which double-encodes the "&" shift + // character and makes folders with accents/specials unresolvable. + $folder = $client->getFolder($folderPath, null, true); if (null === $folder) { throw MailProviderException::operationFailed('listMessages', sprintf('Folder %s not found', $folderPath)); } @@ -138,7 +142,7 @@ final class ImapMailProvider implements MailProviderInterface $client = $this->getClient(); try { - $folder = $client->getFolder($folderPath); + $folder = $client->getFolder($folderPath, null, true); if (null === $folder) { throw MailProviderException::operationFailed('fetchMessage', sprintf('Folder %s not found', $folderPath)); } @@ -183,7 +187,7 @@ final class ImapMailProvider implements MailProviderInterface $client = $this->getClient(); try { - $folder = $client->getFolder($folderPath); + $folder = $client->getFolder($folderPath, null, true); if (null === $folder) { throw MailProviderException::operationFailed('markRead', sprintf('Folder %s not found', $folderPath)); } @@ -213,7 +217,7 @@ final class ImapMailProvider implements MailProviderInterface $client = $this->getClient(); try { - $folder = $client->getFolder($folderPath); + $folder = $client->getFolder($folderPath, null, true); if (null === $folder) { throw MailProviderException::operationFailed('markFlagged', sprintf('Folder %s not found', $folderPath)); } @@ -243,7 +247,7 @@ final class ImapMailProvider implements MailProviderInterface $client = $this->getClient(); try { - $folder = $client->getFolder($folderPath); + $folder = $client->getFolder($folderPath, null, true); if (null === $folder) { throw MailProviderException::operationFailed('moveMessage', sprintf('Folder %s not found', $folderPath)); } @@ -269,7 +273,7 @@ final class ImapMailProvider implements MailProviderInterface $client = $this->getClient(); try { - $folder = $client->getFolder($folderPath); + $folder = $client->getFolder($folderPath, null, true); if (null === $folder) { throw MailProviderException::operationFailed('fetchAttachment', sprintf('Folder %s not found', $folderPath)); } diff --git a/tests/Unit/Service/MailSyncServiceTest.php b/tests/Unit/Service/MailSyncServiceTest.php index 39c6eeb..42c128a 100644 --- a/tests/Unit/Service/MailSyncServiceTest.php +++ b/tests/Unit/Service/MailSyncServiceTest.php @@ -132,6 +132,69 @@ class MailSyncServiceTest extends TestCase $service->syncFolderStructure(); } + public function testSyncAllSkipsFoldersNoLongerPresentOnServer(): void + { + $config = new MailConfiguration(); + $config->setEnabled(true); + + $configRepo = $this->createMock(MailConfigurationRepositoryInterface::class); + $configRepo->method('findSingleton')->willReturn($config); + + // The server only exposes INBOX; "Trash/STALE" was deleted remotely but + // still lingers in the DB. + $inboxDto = new MailFolderDto( + path: 'INBOX', + displayName: 'Inbox', + parentPath: null, + unreadCount: 0, + totalCount: 0, + ); + + $inboxFolder = new MailFolder(); + $inboxFolder->setPath('INBOX'); + + $staleFolder = new MailFolder(); + $staleFolder->setPath('Trash/STALE'); + + $provider = $this->createMock(MailProviderInterface::class); + $provider->method('listFolders')->willReturn([$inboxDto]); + // listMessages must only ever be called for INBOX, never the stale folder. + $provider->expects(self::once()) + ->method('listMessages') + ->with('INBOX', 5000, 0) + ->willReturn([]) + ; + + $folderRepo = $this->createMock(MailFolderRepositoryInterface::class); + $folderRepo->method('findByPath')->willReturn($inboxFolder); + $folderRepo->method('findAllOrderedByPath')->willReturn([$inboxFolder, $staleFolder]); + + $messageRepo = $this->createMock(MailMessageRepositoryInterface::class); + $messageRepo->method('findMaxUidInFolder')->willReturn(0); + $messageRepo->method('findAllUidsByFolder')->willReturn([]); + $messageRepo->method('findLastNByFolder')->willReturn([]); + + $em = $this->createMock(EntityManagerInterface::class); + $em->method('isOpen')->willReturn(true); + $lockFactory = $this->makeLockFactory(); + + $service = new MailSyncService( + provider: $provider, + configRepository: $configRepo, + folderRepository: $folderRepo, + messageRepository: $messageRepo, + entityManager: $em, + lockFactory: $lockFactory, + logger: new NullLogger(), + managerRegistry: $this->createMock(ManagerRegistry::class), + ); + + $report = $service->syncAll(); + + self::assertSame(1, $report->foldersScanned); + self::assertSame([], $report->errors); + } + public function testSyncFolderAbortsSuppressionWhenOver50Percent(): void { $config = new MailConfiguration();