From c75dfa0371ad145987dd40d543dd0bbc169dd016 Mon Sep 17 00:00:00 2001 From: matthieu Date: Wed, 20 May 2026 08:21:02 +0200 Subject: [PATCH] fix(mail) : synchro multi-dossiers fiable contre OVH MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Trois causes racines révélées par une vraie synchro complète (139 dossiers) : - contrainte UNIQUE globale sur message_id : fausse pour IMAP (un même Message-ID existe dans plusieurs dossiers) → violation → fermeture de l'EntityManager → cascade qui tuait tous les dossiers suivants. Migration : index simple à la place. - 139 connexions IMAP (une par dossier) → throttling OVH (failed to authenticate) : réutilisation d'une seule connexion (closeConnection() ajouté à l'interface). - état de connexion corrompu après un dossier en erreur (must be in SELECTED state) : reconnexion ciblée après chaque dossier en échec. - garde anti-cascade : reset du ManagerRegistry + arrêt propre si l'EM se ferme. Résultat : 456 messages sur 57 dossiers (avant : 188/30 puis crash). Les rares dossiers à encodage spécial sont skippés proprement et réessayés au cycle suivant. Co-Authored-By: Claude Opus 4.7 (1M context) --- migrations/Version20260520061736.php | 31 ++++++++++++++++ src/Entity/MailMessage.php | 3 +- src/Mail/ImapMailProvider.php | 41 +++++++++++++--------- src/Mail/MailProviderInterface.php | 6 ++++ src/Service/MailSyncService.php | 22 ++++++++++++ tests/Unit/Service/MailSyncServiceTest.php | 5 +++ 6 files changed, 91 insertions(+), 17 deletions(-) create mode 100644 migrations/Version20260520061736.php diff --git a/migrations/Version20260520061736.php b/migrations/Version20260520061736.php new file mode 100644 index 0000000..ebc57ee --- /dev/null +++ b/migrations/Version20260520061736.php @@ -0,0 +1,31 @@ +addSql('DROP INDEX uniq_6c00b110537a1329'); + $this->addSql('CREATE INDEX idx_mail_message_message_id ON mail_message (message_id)'); + } + + public function down(Schema $schema): void + { + $this->addSql('DROP INDEX idx_mail_message_message_id'); + $this->addSql('CREATE UNIQUE INDEX uniq_6c00b110537a1329 ON mail_message (message_id)'); + } +} diff --git a/src/Entity/MailMessage.php b/src/Entity/MailMessage.php index 55524f5..8efee5f 100644 --- a/src/Entity/MailMessage.php +++ b/src/Entity/MailMessage.php @@ -13,6 +13,7 @@ use Doctrine\ORM\Mapping as ORM; #[ORM\UniqueConstraint(name: 'uq_mail_message_folder_uid', columns: ['folder_id', 'uid'])] #[ORM\Index(columns: ['sent_at'], name: 'idx_mail_message_sent_at')] #[ORM\Index(columns: ['is_read'], name: 'idx_mail_message_is_read')] +#[ORM\Index(columns: ['message_id'], name: 'idx_mail_message_message_id')] class MailMessage { #[ORM\Id] @@ -20,7 +21,7 @@ class MailMessage #[ORM\Column] private ?int $id = null; - #[ORM\Column(length: 500, unique: true)] + #[ORM\Column(length: 500)] private string $messageId; #[ORM\ManyToOne(targetEntity: MailFolder::class)] diff --git a/src/Mail/ImapMailProvider.php b/src/Mail/ImapMailProvider.php index e146d5a..9ef7c45 100644 --- a/src/Mail/ImapMailProvider.php +++ b/src/Mail/ImapMailProvider.php @@ -21,19 +21,37 @@ use Webklex\PHPIMAP\IMAP; final class ImapMailProvider implements MailProviderInterface { + private ?Client $client = null; + public function __construct( private readonly MailConfigurationRepository $configRepository, private readonly TokenEncryptor $tokenEncryptor, private readonly LoggerInterface $logger, ) {} + /** + * Closes the reused IMAP connection. Call once at the end of a batch + * synchronisation to release the socket; HTTP requests can ignore it + * (the connection dies with the process). + */ + public function closeConnection(): void + { + if (null !== $this->client && $this->client->isConnected()) { + try { + $this->client->disconnect(); + } catch (Throwable) { + // best effort + } + } + $this->client = null; + } + public function testConnection(): int { $client = $this->getClient(requireEnabled: false); try { $folders = $client->getFolders(false); - $client->disconnect(); return count($folders); } catch (Throwable $e) { @@ -69,8 +87,6 @@ final class ImapMailProvider implements MailProviderInterface ); } - $client->disconnect(); - return $result; } catch (MailProviderException $e) { throw $e; @@ -106,8 +122,6 @@ final class ImapMailProvider implements MailProviderInterface $result[] = $this->buildHeaderDto($message, withSnippet: false); } - $client->disconnect(); - return $result; } catch (MailProviderException $e) { throw $e; @@ -148,8 +162,6 @@ final class ImapMailProvider implements MailProviderInterface ); } - $client->disconnect(); - return new MailMessageDetailDto( header: $header, bodyHtml: $bodyHtml, @@ -186,8 +198,6 @@ final class ImapMailProvider implements MailProviderInterface } else { $message->unsetFlag('Seen'); } - - $client->disconnect(); } catch (MailProviderException $e) { throw $e; } catch (Throwable $e) { @@ -218,8 +228,6 @@ final class ImapMailProvider implements MailProviderInterface } else { $message->unsetFlag('Flagged'); } - - $client->disconnect(); } catch (MailProviderException $e) { throw $e; } catch (Throwable $e) { @@ -246,7 +254,6 @@ final class ImapMailProvider implements MailProviderInterface } $message->moveToFolder($targetFolder); - $client->disconnect(); } catch (MailProviderException $e) { throw $e; } catch (Throwable $e) { @@ -274,14 +281,10 @@ final class ImapMailProvider implements MailProviderInterface foreach ($message->getAttachments() as $att) { if ((string) ($att->part_number ?? '1') === $partNumber) { - $client->disconnect(); - return (string) $att->getContent(); } } - $client->disconnect(); - throw MailProviderException::operationFailed('fetchAttachment', sprintf('Part %s not found in UID %d', $partNumber, $uid)); } catch (MailProviderException $e) { throw $e; @@ -294,6 +297,10 @@ final class ImapMailProvider implements MailProviderInterface private function getClient(bool $requireEnabled = true): Client { + if (null !== $this->client && $this->client->isConnected()) { + return $this->client; + } + $config = $this->configRepository->findSingleton(); if (null === $config) { @@ -335,6 +342,8 @@ final class ImapMailProvider implements MailProviderInterface } } + $this->client = $client; + return $client; } diff --git a/src/Mail/MailProviderInterface.php b/src/Mail/MailProviderInterface.php index 9392f85..c233846 100644 --- a/src/Mail/MailProviderInterface.php +++ b/src/Mail/MailProviderInterface.php @@ -20,6 +20,12 @@ interface MailProviderInterface */ public function testConnection(): int; + /** + * Releases any reused network connection held by the provider. + * Safe to call multiple times; a no-op if nothing is open. + */ + public function closeConnection(): void; + /** * Returns the full folder tree of the configured mailbox. * diff --git a/src/Service/MailSyncService.php b/src/Service/MailSyncService.php index af06b20..2d0e329 100644 --- a/src/Service/MailSyncService.php +++ b/src/Service/MailSyncService.php @@ -14,6 +14,7 @@ use App\Repository\MailFolderRepository; use App\Repository\MailMessageRepository; use DateTimeImmutable; use Doctrine\ORM\EntityManagerInterface; +use Doctrine\Persistence\ManagerRegistry; use Psr\Log\LoggerInterface; use Symfony\Component\Lock\LockFactory; use Throwable; @@ -32,6 +33,7 @@ final class MailSyncService private readonly EntityManagerInterface $entityManager, private readonly LockFactory $lockFactory, private readonly LoggerInterface $logger, + private readonly ManagerRegistry $managerRegistry, ) {} public function syncAll(): MailSyncReport @@ -57,6 +59,7 @@ final class MailSyncService try { return $this->doSyncAll($startedAt); } finally { + $this->provider->closeConnection(); $lock->release(); } } @@ -274,9 +277,28 @@ final class MailSyncService $totalDeleted += $report->deletedCount; ++$totalFolders; $allErrors = array_merge($allErrors, $report->errors); + // A folder error can leave the reused IMAP connection in a bad + // selection state ("must be in SELECTED state", "empty response"). + // Drop it so the next folder reconnects on a clean session. + if ([] !== $report->errors) { + $this->provider->closeConnection(); + } } catch (Throwable $e) { $this->logger->error(sprintf('doSyncAll: syncFolder[%s] threw: %s', $folder->getPath(), $e->getMessage())); $allErrors[] = $e->getMessage(); + $this->provider->closeConnection(); + } + + // A failed flush closes the Doctrine EntityManager; without a reset + // every subsequent folder would fail with "EntityManager is closed". + // Reset it via the registry and stop the run cleanly — the next cron + // cycle resumes incrementally from where we left off. + if (!$this->entityManager->isOpen()) { + $this->logger->error('doSyncAll: EntityManager was closed mid-sync, resetting and aborting this run'); + $this->managerRegistry->resetManager(); + $allErrors[] = 'EntityManager closed mid-sync — run aborted, will resume next cycle'; + + break; } } diff --git a/tests/Unit/Service/MailSyncServiceTest.php b/tests/Unit/Service/MailSyncServiceTest.php index 7e4dd63..76abdf4 100644 --- a/tests/Unit/Service/MailSyncServiceTest.php +++ b/tests/Unit/Service/MailSyncServiceTest.php @@ -13,6 +13,7 @@ use App\Repository\MailFolderRepository; use App\Repository\MailMessageRepository; use App\Service\MailSyncService; use Doctrine\ORM\EntityManagerInterface; +use Doctrine\Persistence\ManagerRegistry; use PHPUnit\Framework\TestCase; use Psr\Log\NullLogger; use Symfony\Component\Lock\LockFactory; @@ -45,6 +46,7 @@ class MailSyncServiceTest extends TestCase entityManager: $em, lockFactory: $lockFactory, logger: new NullLogger(), + managerRegistry: $this->createMock(ManagerRegistry::class), ); $report = $service->syncAll(); @@ -77,6 +79,7 @@ class MailSyncServiceTest extends TestCase entityManager: $em, lockFactory: $lockFactory, logger: new NullLogger(), + managerRegistry: $this->createMock(ManagerRegistry::class), ); $report = $service->syncAll(); @@ -123,6 +126,7 @@ class MailSyncServiceTest extends TestCase entityManager: $em, lockFactory: $lockFactory, logger: new NullLogger(), + managerRegistry: $this->createMock(ManagerRegistry::class), ); $service->syncFolderStructure(); @@ -161,6 +165,7 @@ class MailSyncServiceTest extends TestCase entityManager: $em, lockFactory: $lockFactory, logger: new NullLogger(), + managerRegistry: $this->createMock(ManagerRegistry::class), ); $report = $service->syncFolder($folder);