Keep every Gmail message in the mailbox it is in, Spam and Trash included

`app:gmail:skipped` on a real mailbox (#90) showed what the ownership
filter was dropping: the account owner's own mail.

  202  sent from "send mail as" addresses (From: pl@pmdevelopment.de)
   89  forwarded in from an address not connected here
   43  Gmail drafts
   14  received, with no recipient header naming anyone
    4  addressed to the googlemail.com spelling of the account
    6  chats

resolveOwningAccount() kept a message only when its headers named the
account. It now keeps one for the account unless another connected
account has the better claim (a Gmailify sibling named in its headers):

  SENT or DRAFT   the sibling the From names, else this account
  received        resolveReceivedAccount()'s answer, else this account;
                  with Gmailify attribution switched off, still only mail
                  addressed to this account
  CHAT            nobody, and recorded as before

GmailAddressFilter reads googlemail.com as gmail.com.

The listing never asked for Spam and Trash although its comment said
it did: messages.list leaves both out without includeSpamTrash, and
getProfile's messagesTotal, the import's total, counts them. How much
of the 11,200 that mailbox was short of this accounts for is not known
yet: its import was still running when it was counted.

A finished import does not list again, so the migration clears the
record of declined messages (chats aside) and every Gmail account's
backfill markers. Each lists its mailbox once more, fetching only what
it does not hold.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Paul LütznerandClaude Opus 5.5 committed 2026-10-14 08:54:12 +02:00
1 parent ff87ee92d7
commit 72d3956b0d
9 files changed
+251 -61

No files matched your search

+15
View File
@@ -6,6 +6,21 @@ so anything that changes the schema irreversibly is called out explicitly.
The published image tags: `latest` follows the most recent release below,
`main` follows the tip of the default branch, and `sha-…` pins one commit.
## Unreleased
**A Gmail import takes in everything in the mailbox, not only mail addressed to the account.**
plMail kept a Gmail message only when its headers named the account, and on a real mailbox
(`app:gmail:skipped`) what it dropped was the account owner's own mail: 202 messages sent from
"send mail as" addresses, 89 forwarded in from an address not connected here, 43 drafts and 4
addressed to the `googlemail.com` spelling of the address. A message in a Gmail mailbox is now that
account's unless another connected account has a better claim to it; drafts arrive as drafts, and
`googlemail.com` is read as `gmail.com`. Only chats are still left out. The import also fetches
Spam and Trash now, which it said it did and never asked Google for — the reason an import could
not reach Gmail's own total. Every Gmail account lists its mailbox once more after the upgrade to
take these in, and shows as importing until that pass is done. **Migration:** clears the record of
skipped Gmail messages except chats, and the backfill markers of Gmail
accounts. [#90](https://github.com/karatektus/pl_mail/issues/90), reported by @karamanliev.
## v0.3.5 — 2026-10-13
**Browsers and password managers kept offering your logins in client-secret fields.** Opening
+11 -4
View File
@@ -1,4 +1,4 @@
<!-- translated-from: internals/mail-ingest.md sha1:116b8d5828c7cc61a9936ec251013995f87652dd -->
<!-- translated-from: internals/mail-ingest.md sha1:1c9c9d3d0cc4ce9e95d18b4f296c61ddfb7dc752 -->
# Mail-Ingest
Vom Anbieter in die Datenbank: wie aus Bytes eine Zeile `Message` wird, was mit ihr geschieht,
@@ -106,10 +106,17 @@ die Vorgeschichte gehört `App\Service\Mail\MailImporter`, auf der Warteschlange
in der Datenbank, sodass alle Worker aus einem Kontingent schöpfen. Eine Ablehnung ist auf
diesem Weg nirgends ein Fehler: Ein Stapel, eine Import-Seite oder ein Sync, der auf eine
trifft, wird zurückgestellt und läuft, wenn die Minute des Kontos vorbei ist.
- **Eine Gmail-Nachricht gehört zu dem Postfach, in dem sie liegt.** Der Stapel-Handler gibt
eine Nachricht nur dann einem anderen verbundenen Konto, wenn dieses den besseren Anspruch
hat (Gmailify: ein Geschwisterkonto, das in ihren Headern genannt ist); alles andere im
Postfach gehört dem Gmail-Konto, auch von einer „Senden als“-Adresse verschickte Mail, von
einer hier nicht verbundenen Adresse weitergeleitete Mail und Gmail-Entwürfe, die als
Entwürfe ankommen. Die Auflistung fragt auch nach Spam und Papierkorb (`includeSpamTrash`),
die `messages.list` sonst auslässt und `messagesTotal` mitzählt.
- **Eine Gmail-Nachricht, die angesehen und nicht gespeichert wird, wird notiert.** Der
Stapel-Handler lehnt eine Nachricht ab, deren Header sie keinem Konto hier zuordnen — einen
Entwurf, einen Chat, eine von einer „Senden als“-Adresse verschickte Mail — und kann eine
nicht speichern, die Gmail auflistet und mit 404 beantwortet. Jede bekommt eine Zeile in
Stapel-Handler lehnt einen Chat ab — die einzige Art, für die es keinen Platz gibt — und
kann eine Nachricht nicht speichern, die Gmail auflistet und mit 404 beantwortet. Jede
bekommt eine Zeile in
`gmail_skipped_message` (`App\Entity\Mail\GmailSkippedMessage`) mit dem Grund, ihren Labels
und ihren Adress-Headern. Die Auflistung behandelt eine notierte id wie eine gespeicherte, die
Nachricht wird also nicht jede Stunde erneut abgerufen, und ein Import, bei dem jede
+8 -2
View File
@@ -93,9 +93,15 @@ The two are separate now. A sync arranges for new mail to be seen and stops; the
refused are in a database-backed cache pool, so every worker draws on one allowance. A
refusal is not an error anywhere on this path: a batch, an import page or a sync that meets
one is put back to run when the account's minute is over.
- **A Gmail message belongs to the mailbox it is in.** The batch handler gives a message to
another connected account only when that account has the better claim (Gmailify: a sibling
named in its headers); everything else in the mailbox is the Gmail account's, including mail
sent from a "send mail as" address, mail forwarded in from an address not connected here and
Gmail drafts, which arrive as drafts. The listing asks for Spam and Trash as well
(`includeSpamTrash`), which `messages.list` otherwise leaves out and `messagesTotal` counts.
- **A Gmail message that is looked at and not stored is written down.** The batch handler
declines a message whose headers tie it to no account here — a draft, a chat, mail sent from a
"send mail as" address — and cannot store one Gmail lists and answers 404 for. Each gets a row
declines a chat — the one kind left with nowhere to go — and cannot store a message Gmail
lists and answers 404 for. Each gets a row
in `gmail_skipped_message` (`App\Entity\Mail\GmailSkippedMessage`) with the reason, its labels
and its address headers. The listing treats a recorded id like a stored one, so the message
is not fetched again every hour and an import whose every outstanding message has an answer
+63
View File
@@ -0,0 +1,63 @@
<?php
declare(strict_types=1);
namespace DoctrineMigrations;
use Doctrine\DBAL\Schema\Schema;
use Doctrine\Migrations\AbstractMigration;
/**
* Have every Gmail account list its mailbox once more, under the new rules.
*
* Two things changed about what a Gmail import takes in (#90): a message in
* the mailbox now belongs to that account unless another connected account
* claims it — sent mail from "send mail as" addresses, mail forwarded in from
* an address not connected here, Gmail drafts — and the listing now includes
* Spam and Trash, which it never asked for.
*
* Neither reaches an account whose import has finished: it does not list its
* mailbox again. So:
*
* - the record of declined messages is cleared, except chats, which are still
* declined — the rest are fetched again and stored;
* - the backfill markers are removed from every Gmail account, which makes
* its next sync start one listing. It fetches only what is not stored, so a
* mailbox that lost nothing costs its list pages and the messages above.
*
* Until that listing has finished the account shows as importing, which is
* what it is.
*/
final class Version20261014090000 extends AbstractMigration
{
public function getDescription(): string
{
return 'Re-list every Gmail mailbox once, so declined mail and Spam and Trash are taken in';
}
public function up(Schema $schema): void
{
$this->addSql(<<<'SQL'
DELETE FROM gmail_skipped_message
WHERE reason = 'unattributable'
AND NOT (label_ids::jsonb @> '["CHAT"]'::jsonb)
SQL);
$this->addSql(<<<'SQL'
UPDATE account
SET settings = settings
- 'sync.backfill_target'
- 'sync.backfill_ran_at'
- 'sync.backfill_attempts'
- 'sync.backfill_page_token'
- 'sync.backfill_pending'
WHERE auth_type = 'oauth2' AND oauth_provider = 'google'
SQL);
}
public function down(Schema $schema): void
{
// Nothing to put back: what was deleted is a record the next listing
// rebuilds, and an import that has run again is no less finished.
}
}
@@ -537,20 +537,32 @@ final readonly class SyncGmailMessageBatchHandler
/**
* Determine which account owns this message.
*
* Received mail (no SENT label):
* - Without Gmailify attribution: only mail genuinely addressed to
* this account is considered.
* - With it: GmailAddressFilter::resolveReceivedAccount decides
* between carrier and siblings, To/Cc outranking a
* carrier-matching Delivered-To (Gmail stamps its own address on
* fetched mail). The loop then decides between enriching the
* sibling's existing IMAP row and importing a new one.
* The message is in THIS account's Gmail mailbox, so it is this account's
* unless another connected account has a better claim to it. That is the
* rule; everything below is the "better claim".
*
* Sent mail (SENT label):
* - From matches this account's own address → this account.
* - From matches a Gmailify sibling AND the carrier has
* gmailSyncGmailifyEnabled → the sibling.
* - Otherwise skip (return null).
* It used to be the other way round: a message was stored only when its
* headers named this account, and anything else was dropped. Measured on
* a real mailbox (#90, `app:gmail:skipped`), what that dropped was 202
* messages from the user's own Sent — written from "send mail as"
* addresses — 89 delivered to a forwarding address that is not connected
* here, 43 drafts and 4 addressed to the `googlemail.com` spelling of the
* account. All of it is in Gmail, in the folders the user would look for
* it in, and none of it belongs to any other account.
*
* Received mail (no SENT, no DRAFT label):
* - With Gmailify attribution, GmailAddressFilter::resolveReceivedAccount
* picks a sibling when one is named; the carrier otherwise.
* - Without it, mail genuinely addressed to this account only. Switching
* Gmailify attribution off is a deliberate "only what was sent to this
* address", and it stays that.
*
* Sent mail and drafts (SENT or DRAFT):
* - From a Gmailify sibling, with attribution on → the sibling.
* - Otherwise → this account. A message in this account's Sent was sent
* by its owner, whichever of their addresses it went out as.
*
* Chats (CHAT) → nobody. A chat is not mail and has nowhere to go here.
*
* @param list<string> $labelIds
* @param array<string,string> $headers lower-cased header name → value
@@ -562,44 +574,36 @@ final readonly class SyncGmailMessageBatchHandler
Account $account,
array $siblingAccounts,
): ?Account {
$isSent = true === in_array('SENT', $labelIds, true);
if (true === in_array('CHAT', $labelIds, true)) {
return null;
}
$written = true === in_array('SENT', $labelIds, true) || true === in_array('DRAFT', $labelIds, true);
$gmailifyEnabled = true === $account->getSetting('gmailSyncGmailifyEnabled', true);
if (false === $isSent) {
if (false === $written) {
if (false === $gmailifyEnabled) {
if (true === $this->addressFilter->isAddressedToAccount($headers, $account)) {
return $account;
}
return null;
return true === $this->addressFilter->isAddressedToAccount($headers, $account) ? $account : null;
}
return $this->addressFilter->resolveReceivedAccount($headers, $account, $siblingAccounts);
return $this->addressFilter->resolveReceivedAccount($headers, $account, $siblingAccounts) ?? $account;
}
// Sent mail — check From against this account first.
if (true === $this->addressFilter->isSentByAccount($headers, $account)) {
if (true === $this->addressFilter->isSentByAccount($headers, $account) || false === $gmailifyEnabled) {
return $account;
}
// Gmailify sent: check if From matches a sibling account.
if (false === $gmailifyEnabled) {
return null;
}
$from = $headers['from'] ?? '';
if ('' === $from) {
return null;
if ('' !== $from) {
$normFrom = $this->addressFilter->normalise($from);
if (true === isset($siblingAccounts[$normFrom])) {
return $siblingAccounts[$normFrom];
}
}
$normFrom = $this->addressFilter->normalise($from);
if (true === isset($siblingAccounts[$normFrom])) {
return $siblingAccounts[$normFrom];
}
return null;
return $account;
}
/**
+9
View File
@@ -13,6 +13,7 @@ use App\Entity\Mail\Account;
* 1. Lowercase the entire address.
* 2. Strip the +tag suffix from the local part (foo+bar@gmail.com → foo).
* 3. Remove all dots from the local part (f.o.o@gmail.com → foo).
* 4. Read googlemail.com as gmail.com (foo@googlemail.com → foo@gmail.com).
*
* After normalisation karatektus@gmail.com, kara.te.ktus@gmail.com and
* karatektus+newsletter@gmail.com all reduce to the same canonical form.
@@ -171,6 +172,14 @@ final class GmailAddressFilter
$local = substr($raw, 0, $atPos);
$domain = substr($raw, $atPos + 1);
// One mailbox, two spellings: Gmail delivers to name@googlemail.com
// and name@gmail.com alike, and accounts from where the googlemail
// domain was the only one still receive on it. Compared as different
// addresses, mail to the old spelling belonged to nobody (#90).
if ('googlemail.com' === $domain) {
$domain = 'gmail.com';
}
// Strip +tag
$plusPos = strpos($local, '+');
if (false !== $plusPos) {
+11 -2
View File
@@ -202,8 +202,17 @@ final class GmailApiSyncer
$this->em->flush();
}
// No labelIds filter — fetch all mail (inbox, sent, spam, trash, …).
$listed = $this->apiClient->listMessagesPage($account, ['maxResults' => self::PAGE_SIZE], $pageToken);
// No labelIds filter — all mail — and Spam and Trash asked for by
// name. The comment here said "inbox, sent, spam, trash", and the
// request never included the last two: messages.list leaves both out
// unless includeSpamTrash is set. getProfile's messagesTotal, which
// the progress line counts towards, includes them, so an import of a
// mailbox with a full Trash could never reach its own total.
$listed = $this->apiClient->listMessagesPage(
$account,
['maxResults' => self::PAGE_SIZE, 'includeSpamTrash' => 'true'],
$pageToken,
);
$pending = $this->newGmailIds($account, $listed['messages']);
@@ -71,6 +71,24 @@ final class GmailApiSyncerBackfillTest extends TestCase
self::assertCount(1, $this->requests);
}
/**
* messages.list leaves Spam and Trash out unless asked, and Gmail's
* messagesTotal — what the import line counts towards — includes them.
* The comment above this call said "inbox, sent, spam, trash" for a
* request that never asked for the last two.
*/
public function testTheListingAsksForSpamAndTrashToo(): void
{
$this->synced();
$this->syncer(['a'])->backfill($this->account());
$listings = array_values(array_filter($this->requests, static fn (string $url): bool => str_contains($url, '/messages?')));
self::assertNotSame([], $listings);
self::assertStringContainsString('includeSpamTrash=true', $listings[0]);
}
public function testBackfillIsSkippedOnceTheWholeMailboxIsIn(): void
{
$account = $this->account();
+75 -16
View File
@@ -7,6 +7,7 @@ namespace App\Tests\Service\Gmail;
use App\Domain\Enum\Account\AuthType;
use App\Domain\Enum\Mail\GmailSkipReason;
use App\Entity\Mail\Account;
use App\Entity\Mail\GmailSkippedMessage;
use App\Entity\Mail\Message;
use App\Entity\User\User;
use App\Infrastructure\Messaging\Handler\SyncGmailMessageBatchHandler;
@@ -73,10 +74,10 @@ final class GmailSkippedMessageTest extends KernelTestCase
parent::tearDown();
}
/** A draft: no Sent label, addressed to somebody else, never delivered anywhere. */
/** A chat: in the mailbox, and not mail. The one kind that is still declined. */
public function testADeclinedMessageIsRecordedWithWhatTheDecisionWasMadeOn(): void
{
$this->handle(['DRAFT'], to: 'Rike <rike@elsewhere.test>');
$this->handle(['CHAT'], to: 'Rike <rike@elsewhere.test>');
self::assertSame(0, $this->storedCount(), 'declined, as before');
@@ -85,7 +86,7 @@ final class GmailSkippedMessageTest extends KernelTestCase
self::assertCount(1, $rows);
self::assertSame(self::GMAIL_ID, $rows[0]->gmailId);
self::assertSame(GmailSkipReason::Unattributable, $rows[0]->reason);
self::assertSame(['DRAFT'], $rows[0]->labelIds);
self::assertSame(['CHAT'], $rows[0]->labelIds);
self::assertSame('Rike <rike@elsewhere.test>', $rows[0]->headers['to'] ?? null);
self::assertSame('sender@example.test', $rows[0]->headers['from'] ?? null);
self::assertArrayNotHasKey('subject', $rows[0]->headers, 'why it was declined, not a copy of the mail');
@@ -94,11 +95,11 @@ final class GmailSkippedMessageTest extends KernelTestCase
/** The point of the record: the listing's next batch does not ask Gmail for it at all. */
public function testADeclinedMessageIsNotFetchedAgainByTheNextListing(): void
{
$this->handle(['DRAFT'], to: 'rike@elsewhere.test', onlyMissing: true);
$this->handle(['CHAT'], to: 'rike@elsewhere.test', onlyMissing: true);
self::assertSame(1, $this->requests);
$this->handle(['DRAFT'], to: 'rike@elsewhere.test', onlyMissing: true);
$this->handle(['CHAT'], to: 'rike@elsewhere.test', onlyMissing: true);
self::assertSame(1, $this->requests, 'nothing was asked for the second time');
self::assertSame(1, $this->skipped->countForAccount($this->account));
@@ -107,8 +108,8 @@ final class GmailSkippedMessageTest extends KernelTestCase
/** A label change re-fetches it deliberately; that must not write a second row. */
public function testDecliningTheSameMessageTwiceIsOneRecord(): void
{
$this->handle(['DRAFT'], to: 'rike@elsewhere.test');
$this->handle(['DRAFT', 'STARRED'], to: 'rike@elsewhere.test');
$this->handle(['CHAT'], to: 'rike@elsewhere.test');
$this->handle(['CHAT', 'STARRED'], to: 'rike@elsewhere.test');
self::assertSame(2, $this->requests);
self::assertSame(1, $this->skipped->countForAccount($this->account));
@@ -126,19 +127,70 @@ final class GmailSkippedMessageTest extends KernelTestCase
self::assertSame([], $rows[0]->headers);
}
/** Declined once is not declined for good: when it can be attributed after all, it is stored and the record goes. */
/**
* Declined once is not declined for good. A message recorded under the
* old rule — which dropped anything whose headers did not name this
* account — is stored when it is next looked at, and its record goes.
* This is the upgrade path for every mailbox the old rule ran over.
*/
public function testAMessageThatBecomesAttributableIsStoredAndComesOffTheRecord(): void
{
$this->em->persist(new GmailSkippedMessage($this->account, self::GMAIL_ID, GmailSkipReason::Unattributable, ['INBOX']));
$this->em->flush();
$this->handle(['INBOX'], to: 'somebody-else@elsewhere.test');
self::assertSame(1, $this->skipped->countForAccount($this->account));
$this->handle(['INBOX'], to: 'gmail@example.test');
self::assertSame(1, $this->storedCount());
self::assertSame(0, $this->skipped->countForAccount($this->account));
}
// ── who a message belongs to (#90) ───────────────────────────────────────
/**
* The four kinds `app:gmail:skipped` found on a real mailbox, which the
* old rule dropped and none of which belongs to any other account.
*
* @return iterable<string, array{0: list<string>, 1: string, 2: string, 3: ?string}>
*/
public static function mailInThisMailbox(): iterable
{
yield 'sent from a "send mail as" address' => [['SENT'], 'me@work.test', 'client@elsewhere.test', null];
yield 'delivered to a forwarding address not connected here' => [['INBOX'], 'sender@example.test', 'me@old-isp.test', 'me@old-isp.test'];
yield 'addressed to the googlemail spelling of the account' => [['INBOX'], 'sender@example.test', 'gmail@googlemail.com', null];
yield 'with no recipient headers at all' => [['INBOX'], 'sender@example.test', '', null];
}
/**
* @param list<string> $labelIds
*/
#[\PHPUnit\Framework\Attributes\DataProvider('mailInThisMailbox')]
public function testMailInThisMailboxThatNoOtherAccountClaimsIsThisAccounts(array $labelIds, string $from, string $to, ?string $deliveredTo): void
{
$this->handle($labelIds, to: $to, from: $from, deliveredTo: $deliveredTo);
self::assertSame(1, $this->storedCount());
self::assertSame(0, $this->skipped->countForAccount($this->account));
}
public function testMailToTheGooglemailSpellingOfAGmailAddressIsThatAddresses(): void
{
$filter = self::getContainer()->get(\App\Service\Gmail\GmailAddressFilter::class);
self::assertSame($filter->normalise('Kara.Tektus@gmail.com'), $filter->normalise('karatektus@googlemail.com'));
self::assertNotSame($filter->normalise('karatektus@gmail.com'), $filter->normalise('karatektus@googlemail.de'));
}
/** A Gmail draft is a draft here: in Drafts, flagged as one. */
public function testAGmailDraftIsStoredAsADraft(): void
{
$this->handle(['DRAFT'], to: 'rike@elsewhere.test');
$draft = $this->em->getRepository(Message::class)->findOneBy(['account' => $this->account]);
self::assertNotNull($draft);
self::assertTrue($draft->isDraft());
}
/**
* 21,964 of 24,199 with nothing still to come. The total is Google's count
* of the mailbox, so what was looked at and declined has to count as done.
@@ -152,7 +204,7 @@ final class GmailSkippedMessageTest extends KernelTestCase
self::assertSame(0, $progress->forUser($this->user)[0]->done);
$this->handle(['DRAFT'], to: 'rike@elsewhere.test');
$this->handle(['CHAT'], to: 'rike@elsewhere.test');
self::assertSame(1, $progress->forUser($this->user)[0]->done);
}
@@ -171,8 +223,14 @@ final class GmailSkippedMessageTest extends KernelTestCase
*
* @param list<string> $labelIds
*/
private function handle(array $labelIds, string $to, int $status = 200, bool $onlyMissing = false): void
{
private function handle(
array $labelIds,
string $to,
int $status = 200,
bool $onlyMissing = false,
string $from = 'sender@example.test',
?string $deliveredTo = null,
): void {
$payload = json_encode([
'id' => self::GMAIL_ID,
'threadId' => 'gmail-skipped-thread',
@@ -182,8 +240,9 @@ final class GmailSkippedMessageTest extends KernelTestCase
'headers' => array_values(array_filter([
['name' => 'Message-ID', 'value' => '<gmail-skipped-1@example.test>'],
['name' => 'Subject', 'value' => 'About the invoice'],
['name' => 'From', 'value' => 'sender@example.test'],
['name' => 'From', 'value' => $from],
'' === $to ? null : ['name' => 'To', 'value' => $to],
null === $deliveredTo ? null : ['name' => 'Delivered-To', 'value' => $deliveredTo],
])),
],
], JSON_THROW_ON_ERROR);