diff --git a/migrations/Version20260826020000.php b/migrations/Version20260826020000.php new file mode 100644 index 00000000..deb3ac7a --- /dev/null +++ b/migrations/Version20260826020000.php @@ -0,0 +1,39 @@ +addSql('ALTER TABLE account ADD full_resync_count INT DEFAULT 0 NOT NULL'); + $this->addSql('ALTER TABLE account ADD last_full_resync_at TIMESTAMP(0) WITHOUT TIME ZONE DEFAULT NULL'); + } + + public function down(Schema $schema): void + { + $this->addSql('ALTER TABLE account DROP full_resync_count'); + $this->addSql('ALTER TABLE account DROP last_full_resync_at'); + } +} diff --git a/src/Domain/DTO/Health/HealthFact.php b/src/Domain/DTO/Health/HealthFact.php index 9d4cb760..f6ae22de 100644 --- a/src/Domain/DTO/Health/HealthFact.php +++ b/src/Domain/DTO/Health/HealthFact.php @@ -49,6 +49,18 @@ final readonly class HealthFact * teach the reader that the colour means nothing. */ public bool $alarmWhenPast = false, + /** + * Placeholders for the label, the way HealthIssue carries titleParams. + * + * A label is usually a constant — "Watch expires" — and needed none. + * "Re-listed the whole mailbox 6 times" is the same kind of fact and + * cannot be one: the number IS the evidence, and a fact that said + * "re-listed the whole mailbox" without it would be a sentence nobody + * could act on. + * + * @var array + */ + public array $labelParams = [], ) { } diff --git a/src/Entity/Mail/Account.php b/src/Entity/Mail/Account.php index d6d17f31..6c4f1a1a 100644 --- a/src/Entity/Mail/Account.php +++ b/src/Entity/Mail/Account.php @@ -219,6 +219,39 @@ class Account extends AccountModel #[ORM\Column(type: 'text', nullable: true)] public ?string $imapServerAlert = null; + /** + * How many times this account has had to re-enumerate its whole mailbox. + * + * Both providers hand out a sync cursor that can expire — Gmail's historyId + * after about thirty days, Graph's delta link on its own schedule — and the + * answer to either is to list everything again and work out what is missing. + * That recovery is correct and normal, occasionally. + * + * It is not normal often. A cursor that keeps expiring means the account is + * not being synced inside the window the provider keeps, or a worker is + * dying mid-batch and never committing the new one — and the symptom of + * that is a mailbox that is intermittently behind, which looks like nothing + * in particular from the outside. + * + * Recorded rather than surfaced on its own: one re-sync is housekeeping and + * a card about it would be noise. It rides along as evidence on the card + * that fires when syncing is actually failing — see HealthFact, which + * exists so somebody can check the reasoning rather than take a verdict on + * faith. + */ + #[ORM\Column(options: ['default' => 0])] + public int $fullResyncCount = 0; + + #[ORM\Column(nullable: true)] + public ?DateTimeImmutable $lastFullResyncAt = null; + + /** A cursor expired and the whole mailbox had to be listed again. */ + public function recordFullResync(): void + { + ++$this->fullResyncCount; + $this->lastFullResyncAt = new DateTimeImmutable(); + } + #[ORM\Column(length: 255, nullable: true)] public ?string $gmailHistoryId = null; diff --git a/src/Service/Gmail/GmailApiSyncer.php b/src/Service/Gmail/GmailApiSyncer.php index 11cb452a..2259ff8d 100644 --- a/src/Service/Gmail/GmailApiSyncer.php +++ b/src/Service/Gmail/GmailApiSyncer.php @@ -243,6 +243,12 @@ final class GmailApiSyncer $account->backfillRanAt = null; $account->backfillAttempts = 0; + // Counted, because once is housekeeping and repeatedly is a + // fault: a cursor that keeps expiring means this account is not + // being synced inside the thirty days Gmail keeps history for, + // or a worker is dying before it can commit the new one. + $account->recordFullResync(); + $this->em->flush(); $this->initialSync($account); diff --git a/src/Service/Graph/GraphApiSyncer.php b/src/Service/Graph/GraphApiSyncer.php index 57513631..00019985 100644 --- a/src/Service/Graph/GraphApiSyncer.php +++ b/src/Service/Graph/GraphApiSyncer.php @@ -109,6 +109,14 @@ final class GraphApiSyncer unset($deltaLinks[$folderId]); $account->graphDeltaLinks = $deltaLinks; + + // Same count as the Gmail path keeps, and it matters more here: + // a Graph re-enumeration re-reads the whole folder and, on a + // mailbox without immutable ids, re-downloads every body only + // to discard them as duplicates. Once is nothing; every hour is + // an account quietly costing a great deal. + $account->recordFullResync(); + $this->em->flush(); try { diff --git a/src/Service/Health/AccountHealthInspector.php b/src/Service/Health/AccountHealthInspector.php index 935652f4..770520e1 100644 --- a/src/Service/Health/AccountHealthInspector.php +++ b/src/Service/Health/AccountHealthInspector.php @@ -480,10 +480,42 @@ final readonly class AccountHealthInspector // behaviour is to wait for the next scheduled sync to fail too. // The detail below is what someone can actually act on. repairs: [], + // The evidence, not a second verdict. A mailbox that fails to sync + // and has re-listed itself six times this week is a different story + // from one that has never had to — the first is falling behind the + // window its provider keeps, and that is somewhere to look. + facts: $this->resyncFacts($account), detail: $account->lastSyncError, ); } + /** + * How often this account has had to start its sync over, if ever. + * + * Nothing when it never has, which is the common case and reads better as + * absence than as "0 times". HealthFact's own docblock makes the opposite + * argument for dates — a null there renders as "never" rather than a blank, + * because an empty cell reads as a bug — and that holds because the fact is + * always relevant once its card is up. This one is only relevant when it + * has happened. + * + * @return list + */ + private function resyncFacts(Account $account): array + { + if (0 === $account->fullResyncCount) { + return []; + } + + return [ + new HealthFact( + labelKey: 'settings.health.fact.last_full_resync', + at: $account->lastFullResyncAt, + labelParams: ['%count%' => $account->fullResyncCount], + ), + ]; + } + /** * What the mail server asked us to tell the user. * diff --git a/templates/settings/_health.html.twig b/templates/settings/_health.html.twig index 9d8d9bc2..3c449770 100644 --- a/templates/settings/_health.html.twig +++ b/templates/settings/_health.html.twig @@ -135,7 +135,7 @@ {% if issue.facts is not empty %}
{% for fact in issue.facts %} -
{{ fact.labelKey|trans }}
+
{{ fact.labelKey|trans(fact.labelParams) }}
{# Only a deadline that has gone by is tinted — see HealthFact $alarmWhenPast. Every other date diff --git a/tests/Controller/Settings/AccountHealthQuieterProblemsTest.php b/tests/Controller/Settings/AccountHealthQuieterProblemsTest.php index b2538aee..80326be1 100644 --- a/tests/Controller/Settings/AccountHealthQuieterProblemsTest.php +++ b/tests/Controller/Settings/AccountHealthQuieterProblemsTest.php @@ -496,6 +496,64 @@ final class AccountHealthQuieterProblemsTest extends WebTestCase self::assertSame(0, $card->filter('form')->count()); } + /** + * A repeatedly re-listed mailbox shows how often, as evidence. + * + * Both providers hand out a sync cursor that expires, and the answer is to + * list the whole mailbox again and work out what is missing. Once is + * housekeeping. Repeatedly means the account is not being synced inside the + * window the provider keeps, or a worker is dying before it can commit the + * new cursor — and the symptom of that is a mailbox that is intermittently + * behind, which looks like nothing at all from outside. + * + * It rides on the card that fires when syncing is actually failing, rather + * than raising one of its own: a card about routine housekeeping is noise, + * and HealthFact exists precisely so a verdict can show what it was read + * off. + */ + public function testARepeatedlyReListedMailboxShowsHowOften(): void + { + $client = static::createClient(); + $user = $this->boot($client); + $account = $this->gmailAccount($user, 'falling-behind@joder.dev'); + + $account->recordSyncFailure('IMAP: connection refused'); + $account->recordSyncFailure('IMAP: connection refused'); + $account->recordSyncFailure('IMAP: connection refused'); + + $account->recordFullResync(); + $account->recordFullResync(); + $this->em->flush(); + + $crawler = $client->request('GET', '/settings?section=health'); + $card = $crawler->filter('[data-health-issue="account-sync-' . $account->id . '"]'); + + self::assertSame(1, $card->count()); + self::assertStringContainsString('2', $card->filter('dt')->text(), 'the count is the evidence and it is missing'); + } + + /** And an account that has never had to says nothing about it. */ + public function testAMailboxThatNeverReListedShowsNoSuchFact(): void + { + $client = static::createClient(); + $user = $this->boot($client); + $account = $this->gmailAccount($user, 'steady@joder.dev'); + + $account->recordSyncFailure('IMAP: connection refused'); + $account->recordSyncFailure('IMAP: connection refused'); + $account->recordSyncFailure('IMAP: connection refused'); + $this->em->flush(); + + $crawler = $client->request('GET', '/settings?section=health'); + $card = $crawler->filter('[data-health-issue="account-sync-' . $account->id . '"]'); + + self::assertSame(1, $card->count()); + + // Absent rather than "0 times": a fact that has not happened reads + // better as nothing than as a zero somebody has to interpret. + self::assertSame(0, $card->filter('[data-health-fact="settings.health.fact.last_full_resync"]')->count()); + } + /** The reconnect this account's repair button should point at. */ private function urlFor(Account $account): string {