mirror of
https://github.com/karatektus/pl_mail.git
synced 2026-10-25 01:54:10 +00:00
feat(imap): mail read anywhere else is read here too, on every provider
plMail never re-read the flags of a message it had already stored. They were captured once at ingest and never again, because incremental sync asks for `lastSeenUid+1:*` and no UID below the high-water mark is ever looked at twice. Mail read on a phone stayed unread here forever; a star set in Roundcube never arrived. Not eventually — never. The folder listing carries flags now. `UID SEARCH ALL` answered presence and nothing else, and a separate flag pass would have cost that SEARCH *plus* a `UID FETCH 1:* (FLAGS)`; folding them together is one command rather than two and strictly cheaper than keeping them apart, at six times the bytes of the half it replaces. It is also one instant instead of two, so "what is here" and "what state is it in" cannot disagree. The sweep's every refusal — a rebuilt UIDVALIDITY, a listing suspicious enough to be a server rebuilding — refuses its flags with it. The echo race is the hard half. The user marks a message read, the row changes first because the database is the source of truth, and until the outbound job lands the server's honest answer is the old one; believing it would revert the user, which the propagator would see as a fresh local change and queue a job for, which the next pass would revert again. Comparing timestamps does not fix that — the local write happens BEFORE the read, so last-write-wins picks the server every time. What the local change is, is unconfirmed, so that is what is recorded: flagsTouchedAt, set where LabelChangePropagator queues the outbound op for all three providers, cleared where the server accepts it. Such a row is skipped whole rather than merged, because a message whose \Seen is in flight may have had its \Flagged changed remotely and a flag list cannot tell those apart. The guard expires after an hour, or a job lost for good would freeze a row's flags against the server permanently — the same bug in a smaller box. It is enforced in ThreadStatusUpdater::applyRemoteFlags(), which is where every provider's inbound flags land, because one rule enforced once is a rule no future caller can forget. That method is the inbound twin of markRead() and star(): the same seenAt, starredAt, thread star, thread recount and JMAP log, and no propagator call, since these changes came from the server. Gmail and Graph had the same gap and now do not. Graph's delta carries isRead and flag on every entry and partition() read only parentFolderId from it, on the stated belief that location was all that could have changed cheaply. Gmail was worse: relabelled ids are sent precisely because they are already stored, and the batch handler dropped every id it already had before fetching anything — re-applying the exact filter GmailApiSyncer documents those ids as bypassing. Idempotency never depended on it; the loop below looks each id up and enriches. The idle supervisor answers `* 4 FETCH (FLAGS (\Seen))` the way it answers EXPUNGE: which message is a sequence number it deliberately does not resolve, and since the flag pass rides on the listing it is the same clock — backdate sweptAt, dispatch, and a star set elsewhere lands in seconds rather than within two fifteen-minute cycles. Not the FLAGS and PERMANENTFLAGS lines every SELECT answers with, which describe what a folder permits and say nothing about a message. The race is pinned from both sides, which is the point: a local change survives a stale read, and a genuine remote change still lands. A guard that passes only the first has switched inbound flag sync off. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Paul LütznerandClaude Fable 5 committed
1 parent
c68e992c85
commit
7ddbe23db9
19 files changed
+2239
-54
No files matched your search
@@ -0,0 +1,54 @@
|
||||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
namespace DoctrineMigrations;
|
||||
|
||||
use Doctrine\DBAL\Schema\Schema;
|
||||
use Doctrine\Migrations\AbstractMigration;
|
||||
|
||||
/**
|
||||
* The column that stops inbound flag sync from undoing the user.
|
||||
*
|
||||
* Reading the server's flags back into plMail introduces a race the outbound
|
||||
* direction never had. The user marks a message read; the local row changes
|
||||
* first, because the database is the source of truth; an ApplyImapFlagsMessage
|
||||
* is queued to tell the server. Between those two moments the server still
|
||||
* says unread — and an inbound pass that believes it would revert the local
|
||||
* change, which would queue a *second* outbound job, which the next pass would
|
||||
* revert again. A flap, driven by nothing but the two directions disagreeing
|
||||
* about who spoke last.
|
||||
*
|
||||
* `flags_touched_at` is the answer: the instant a local flag change was made
|
||||
* that the provider has not yet confirmed. It is written where the outbound job
|
||||
* is queued and cleared where that job reports success, so a non-null value
|
||||
* means exactly "there is a flag change in flight". The inbound pass declines
|
||||
* to touch those rows, and only those.
|
||||
*
|
||||
* Existing rows get NULL, which is the correct answer for every one of them:
|
||||
* nothing is in flight for a message nobody has touched since this deployed.
|
||||
*/
|
||||
final class Version20260811150000 extends AbstractMigration
|
||||
{
|
||||
public function getDescription(): string
|
||||
{
|
||||
return 'Track the instant of an unconfirmed local flag change, so inbound flag sync cannot revert it';
|
||||
}
|
||||
|
||||
public function up(Schema $schema): void
|
||||
{
|
||||
$this->addSql('ALTER TABLE message ADD flags_touched_at TIMESTAMP(0) WITHOUT TIME ZONE DEFAULT NULL');
|
||||
|
||||
// The inbound pass asks "is anything in flight for this row" once per
|
||||
// message it is about to change, and the answer is null for nearly
|
||||
// every row nearly always. A partial index is the whole table's worth
|
||||
// of that question in the few pages where the answer is interesting.
|
||||
$this->addSql('CREATE INDEX idx_message_flags_touched_at ON message (flags_touched_at) WHERE flags_touched_at IS NOT NULL');
|
||||
}
|
||||
|
||||
public function down(Schema $schema): void
|
||||
{
|
||||
$this->addSql('DROP INDEX idx_message_flags_touched_at');
|
||||
$this->addSql('ALTER TABLE message DROP flags_touched_at');
|
||||
}
|
||||
}
|
||||
@@ -212,9 +212,54 @@ class ImapIdleCommand extends Command
|
||||
$io->error(sprintf('[%s] Sweep dispatch failed: %s', date('H:i:s'), $e->getMessage()));
|
||||
}
|
||||
}
|
||||
|
||||
// `* 4 FETCH (FLAGS (\Seen))` — what a server sends an idling
|
||||
// client when another client changes a message's flags. Mail read
|
||||
// on a phone announces itself here, and this loop used to drop the
|
||||
// line exactly as it once dropped EXPUNGE.
|
||||
//
|
||||
// WHICH message is a sequence number, and this deliberately does
|
||||
// not resolve it — the same trigger-versus-authority split the
|
||||
// expunge above is built on. The flag pass rides on the folder
|
||||
// listing, so it is the *same clock*: backdating sweptAt makes the
|
||||
// one `UID FETCH 1:* (FLAGS)` run now, and it answers presence and
|
||||
// flags together. A star set elsewhere lands in seconds rather than
|
||||
// within two fifteen-minute cycles.
|
||||
//
|
||||
// FLAGS as well as FETCH, because an untagged FETCH can carry other
|
||||
// data items and only this one is worth a round trip for.
|
||||
if (true === self::announcesFlagChange($line)) {
|
||||
$this->beat($mailbox, $account, true);
|
||||
$io->text(sprintf('[%s] Flag change seen — refreshing now.', date('H:i:s')));
|
||||
try {
|
||||
$this->mailboxRepository->markSweepDue($mailboxId);
|
||||
$this->bus->dispatch(new SyncImapMailboxMessage($mailboxId));
|
||||
} catch (\Throwable $e) {
|
||||
$io->error(sprintf('[%s] Flag refresh dispatch failed: %s', date('H:i:s'), $e->getMessage()));
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether a line the server pushed is announcing that flags changed.
|
||||
*
|
||||
* `* 4 FETCH (FLAGS (\Seen))` is the shape, and both halves are required.
|
||||
* An untagged FETCH can carry other data items, and a line mentioning
|
||||
* FLAGS without being a FETCH — the FLAGS and PERMANENTFLAGS lines every
|
||||
* SELECT answers with — describes what the folder *permits* rather than
|
||||
* what any message now has. Waking a folder listing for either would be a
|
||||
* round trip that learns nothing.
|
||||
*
|
||||
* Static and public so that which lines count is a stated behaviour with a
|
||||
* test on it rather than a condition buried in a socket loop no test can
|
||||
* reach.
|
||||
*/
|
||||
public static function announcesFlagChange(string $line): bool
|
||||
{
|
||||
return true === str_contains($line, 'FETCH') && true === str_contains($line, 'FLAGS');
|
||||
}
|
||||
|
||||
/**
|
||||
* Record a liveness heartbeat for this mailbox's IDLE process. Lifecycle
|
||||
* beats (connect, timeout re-issue, EXISTS wake) pass $force so they
|
||||
|
||||
@@ -0,0 +1,70 @@
|
||||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
namespace App\Domain\DTO\Mail;
|
||||
|
||||
use App\Domain\Enum\Mail\MessageFlag;
|
||||
use App\Entity\Mail\Message;
|
||||
|
||||
/**
|
||||
* What a provider says one stored message's flags are right now.
|
||||
*
|
||||
* The point of the type is that it is the *server's* statement and not a diff:
|
||||
* it says what is true there, and ThreadStatusUpdater::applyRemoteFlags()
|
||||
* decides what, if anything, that means for the row. Keeping those apart is
|
||||
* what lets the reconciler be tested against a stated server rather than a real
|
||||
* one, the same split RemoteDeletionSyncTest works through.
|
||||
*
|
||||
* $flags is the raw list as the provider spelled it, kept so the row's flags
|
||||
* mirror stays a faithful copy of the server's — including flags plMail has no
|
||||
* column for. $seen and $flagged are the two that map onto columns, normalised
|
||||
* out of that list by the caller because the spelling varies: IMAP writes
|
||||
* `\Seen`, some servers answer `Seen`, and Gmail and Graph do not speak IMAP
|
||||
* flags at all.
|
||||
*/
|
||||
final readonly class RemoteFlagState
|
||||
{
|
||||
/**
|
||||
* @param list<string> $flags the provider's own spelling, verbatim
|
||||
*/
|
||||
public function __construct(
|
||||
public Message $message,
|
||||
public bool $seen,
|
||||
public bool $flagged,
|
||||
public array $flags = [],
|
||||
) {
|
||||
}
|
||||
|
||||
/**
|
||||
* The flag list as plMail stores it: the server's own words when it gave
|
||||
* any, and otherwise the two facts we did establish, spelled the way
|
||||
* MessageFlag spells them.
|
||||
*
|
||||
* The fallback matters for Gmail and Graph, which report read state as a
|
||||
* label and a boolean respectively and never send a flag list at all. Their
|
||||
* rows still carry a flags mirror — GmailMessageBuilder and
|
||||
* GraphMessageBuilder both synthesise one at ingest — so a refresh that
|
||||
* left it alone would leave the mirror saying what was true months ago.
|
||||
*
|
||||
* @return list<string>
|
||||
*/
|
||||
public function storedFlags(): array
|
||||
{
|
||||
if ([] !== $this->flags) {
|
||||
return MessageFlag::canonicalList($this->flags);
|
||||
}
|
||||
|
||||
$flags = [];
|
||||
|
||||
if (true === $this->seen) {
|
||||
$flags[] = MessageFlag::SEEN->value;
|
||||
}
|
||||
|
||||
if (true === $this->flagged) {
|
||||
$flags[] = MessageFlag::FLAGGED->value;
|
||||
}
|
||||
|
||||
return MessageFlag::canonicalList($flags);
|
||||
}
|
||||
}
|
||||
@@ -1,13 +1,66 @@
|
||||
<?php
|
||||
|
||||
namespace App\Domain\Enum\Mail;
|
||||
|
||||
enum MessageFlag: string
|
||||
{
|
||||
case ANSWERED = '\\Answered';
|
||||
case FLAGGED = '\\Flagged';
|
||||
case DELETED = '\\Deleted';
|
||||
case SEEN = '\\Seen';
|
||||
case DRAFT = '\\Draft';
|
||||
case RECENT = '\\Recent';
|
||||
}
|
||||
<?php
|
||||
|
||||
namespace App\Domain\Enum\Mail;
|
||||
|
||||
enum MessageFlag: string
|
||||
{
|
||||
case ANSWERED = '\\Answered';
|
||||
case FLAGGED = '\\Flagged';
|
||||
case DELETED = '\\Deleted';
|
||||
case SEEN = '\\Seen';
|
||||
case DRAFT = '\\Draft';
|
||||
case RECENT = '\\Recent';
|
||||
|
||||
/**
|
||||
* One flag list in the spelling this enum uses, so that two lists can be
|
||||
* compared for meaning rather than for punctuation.
|
||||
*
|
||||
* The backslash on a system flag is IMAP wire syntax and not part of the
|
||||
* name, and servers differ on whether it survives being parsed — webklex
|
||||
* hands back `Seen` from some and `\Seen` from others. Without this, a row
|
||||
* whose mirror was captured from one server would read as changed against
|
||||
* every listing forever, and inbound flag sync would rewrite the whole
|
||||
* folder on every pass and log a JMAP change for each row.
|
||||
*
|
||||
* Unknown flags are kept verbatim: a keyword another client sets is not
|
||||
* plMail's to discard, and this enum is the list of flags that mean
|
||||
* something here, not the list of flags allowed to exist.
|
||||
*
|
||||
* \Recent is dropped. It is not a property of a message but of this
|
||||
* session's relationship to it — no client can set it, and it is false the
|
||||
* moment anybody else looks — so storing it guarantees a spurious
|
||||
* difference on the next comparison.
|
||||
*
|
||||
* Sorted, because two lists that differ only in order say the same thing.
|
||||
*
|
||||
* @param list<string> $flags
|
||||
*
|
||||
* @return list<string>
|
||||
*/
|
||||
public static function canonicalList(array $flags): array
|
||||
{
|
||||
$canonical = [];
|
||||
|
||||
foreach ($flags as $flag) {
|
||||
$bare = ltrim($flag, '\\');
|
||||
|
||||
if ('' === $bare) {
|
||||
continue;
|
||||
}
|
||||
|
||||
$system = self::tryFrom('\\' . ucfirst(strtolower($bare)));
|
||||
|
||||
if (self::RECENT === $system) {
|
||||
continue;
|
||||
}
|
||||
|
||||
$canonical[] = $system->value ?? $flag;
|
||||
}
|
||||
|
||||
$canonical = array_values(array_unique($canonical));
|
||||
|
||||
sort($canonical);
|
||||
|
||||
return $canonical;
|
||||
}
|
||||
}
|
||||
@@ -138,6 +138,30 @@ class Message extends MessageModel
|
||||
#[ORM\Column(nullable: true)]
|
||||
public ?DateTimeImmutable $starredAt = null;
|
||||
|
||||
/**
|
||||
* When a local flag change was made that the provider has not confirmed.
|
||||
*
|
||||
* Null is the resting state and means "the two sides agree, as far as
|
||||
* anybody here knows". A value means a flag mutation happened locally and
|
||||
* an outbound job is queued or in flight to carry it to the server.
|
||||
*
|
||||
* It exists because inbound flag sync reads the server's flags back, and
|
||||
* the server's answer is *stale by construction* for exactly as long as
|
||||
* that job takes: the user marks a message read, the row changes first
|
||||
* because the database is the source of truth, and until the job lands the
|
||||
* server still says unread. An inbound pass that believed it would revert
|
||||
* the user, queue another outbound job to undo the revert, and flap.
|
||||
*
|
||||
* So this is written by LabelChangePropagator — the one place an outbound
|
||||
* flag op is queued, for every provider — and cleared when that op reports
|
||||
* success, or when a pass finds the server already agreeing. Inbound
|
||||
* declines to touch a row while it is set. See ImapFlagReconciler for the
|
||||
* grace window that stops a job lost forever from freezing a row's flags
|
||||
* against the server permanently.
|
||||
*/
|
||||
#[ORM\Column(nullable: true)]
|
||||
public ?DateTimeImmutable $flagsTouchedAt = null;
|
||||
|
||||
#[ORM\Column(type: 'json', nullable: true)]
|
||||
public ?array $inReplyTo = [];
|
||||
|
||||
|
||||
@@ -324,6 +324,16 @@ final class ApplyImapFlagsHandler
|
||||
'delete' => $imapMessage->delete(expunge: true),
|
||||
default => $this->logger->warning('ApplyImapFlagsHandler: unknown action', ['action' => $action]),
|
||||
};
|
||||
|
||||
// The server has the change now, so the inbound pass may read this row
|
||||
// again. Until this line runs, Message::$flagsTouchedAt is what keeps
|
||||
// ImapFlagReconciler from reading the server's still-old answer and
|
||||
// reverting the user — see the entity property, and note that only the
|
||||
// flag actions clear it. A move is not a flag change, and 'delete' is
|
||||
// on its way to having no row at all.
|
||||
if (true === in_array($action, ['flag', 'unflag', 'seen', 'unseen'], true)) {
|
||||
$msg->flagsTouchedAt = null;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -5,6 +5,7 @@ declare(strict_types=1);
|
||||
namespace App\Infrastructure\Messaging\Handler;
|
||||
|
||||
use App\Domain\DTO\Mail\IngestedMessage;
|
||||
use App\Domain\DTO\Mail\RemoteFlagState;
|
||||
use App\Domain\Helper\MessageIdHelper;
|
||||
use App\Entity\Mail\Account;
|
||||
use App\Entity\Mail\Message;
|
||||
@@ -20,6 +21,7 @@ use App\Service\Label\ThreadLabelSynchronizer;
|
||||
use App\Service\Mail\GmailApiClient;
|
||||
use App\Service\Mail\PostIngestPipeline;
|
||||
use App\Service\Mail\SyncNotifier;
|
||||
use App\Service\Mail\ThreadStatusUpdater;
|
||||
use Doctrine\ORM\EntityManagerInterface;
|
||||
use Psr\Log\LoggerInterface;
|
||||
use Symfony\Component\Messenger\Attribute\AsMessageHandler;
|
||||
@@ -46,6 +48,7 @@ final readonly class SyncGmailMessageBatchHandler
|
||||
private StateManager $stateManager,
|
||||
private LoggerInterface $logger,
|
||||
private ThreadLabelSynchronizer $threadLabels,
|
||||
private ThreadStatusUpdater $status,
|
||||
) {}
|
||||
|
||||
public function __invoke(SyncGmailMessageBatchMessage $message): void
|
||||
@@ -64,24 +67,23 @@ final readonly class SyncGmailMessageBatchHandler
|
||||
// Used to attribute Gmailify sent AND received messages to the correct account.
|
||||
$siblingAccounts = $this->buildSiblingAccountMap($account);
|
||||
|
||||
// Dedup inside the batch too — batches can overlap across runs/retries.
|
||||
// USER-scoped, not account-scoped: Gmailify attribution stores messages
|
||||
// under sibling accounts, so a carrier-scoped lookup would miss them
|
||||
// and re-insert on every retry. Enriched IMAP rows get their gmailId
|
||||
// set, so they are covered here too and enrichment runs exactly once.
|
||||
$syncedGmailIds = array_flip(
|
||||
$this->messageRepository->findSyncedGmailIdsForUser($account->usr)
|
||||
);
|
||||
|
||||
$toFetch = [];
|
||||
|
||||
foreach ($message->gmailIds as $gmailId) {
|
||||
if (true === isset($syncedGmailIds[$gmailId])) {
|
||||
continue;
|
||||
}
|
||||
|
||||
$toFetch[] = $gmailId;
|
||||
}
|
||||
// Everything the planner asked for, including ids already stored.
|
||||
//
|
||||
// This used to drop those, and dropping them silently defeated the one
|
||||
// case that most needs them. GmailApiSyncer sends two sets: new ids,
|
||||
// which it has already filtered against what is stored, and RELABELLED
|
||||
// ids, which are stored by definition and are sent precisely because
|
||||
// something about them changed. Its own comment says they "bypass that
|
||||
// filter and the batch handler recognises them and enriches rather than
|
||||
// inserting" — and then this method re-applied the identical filter and
|
||||
// returned before fetching anything. A label change in Gmail, read
|
||||
// state included, reached this handler and stopped here.
|
||||
//
|
||||
// Idempotency does not depend on this filter and never did: the loop
|
||||
// below looks each id up before building, and enriches when it finds a
|
||||
// row. A redelivered batch therefore costs one re-fetch and updates
|
||||
// instead of inserting, which is what it should always have done.
|
||||
$toFetch = array_values($message->gmailIds);
|
||||
|
||||
if (count($toFetch) === 0) {
|
||||
return;
|
||||
@@ -153,7 +155,10 @@ final readonly class SyncGmailMessageBatchHandler
|
||||
$existing = $this->messageRepository->findOneForAccountByMessageId($targetAccount, $rfcMessageId);
|
||||
|
||||
if (null !== $existing) {
|
||||
$this->enrichExisting($existing, $labelIds, $gmailId, $targetAccount, $account);
|
||||
// Gmailify: the row is the sibling's own IMAP copy, and
|
||||
// the IMAP side owns its flags. Enrich the labels, leave
|
||||
// the read state to the folder listing that can see it.
|
||||
$this->enrichExisting($existing, $labelIds, $gmailId, $targetAccount, $account, false);
|
||||
|
||||
$enriched++;
|
||||
$affectedAccounts[(int) $targetAccount->id] = $targetAccount;
|
||||
@@ -177,7 +182,10 @@ final readonly class SyncGmailMessageBatchHandler
|
||||
$known = $this->messageRepository->findOneBy(['gmailId' => $gmailId]);
|
||||
|
||||
if (null !== $known) {
|
||||
$this->enrichExisting($known, $labelIds, $gmailId, $targetAccount, $account);
|
||||
// A native Gmail row: Gmail is the authority on its flags,
|
||||
// and UNREAD/STARRED are in the very labelIds this re-fetch
|
||||
// was made to re-read.
|
||||
$this->enrichExisting($known, $labelIds, $gmailId, $targetAccount, $account, true);
|
||||
|
||||
$enriched++;
|
||||
$affectedAccounts[(int) $targetAccount->id] = $targetAccount;
|
||||
@@ -242,17 +250,30 @@ final readonly class SyncGmailMessageBatchHandler
|
||||
// ── Private ───────────────────────────────────────────────────────────────
|
||||
|
||||
/**
|
||||
* Merge the Gmail copy's knowledge onto an existing IMAP-synced row:
|
||||
* gmailId (covers it under the user-scoped dedup from now on),
|
||||
* gmailLabelIds, and the carrier's labels translated onto the target
|
||||
* account, propagated to the thread. Flags/read state stay untouched —
|
||||
* the IMAP copy owns those.
|
||||
* Merge the Gmail copy's knowledge onto an existing row: gmailId (covers it
|
||||
* under the user-scoped dedup from now on), gmailLabelIds, and the
|
||||
* carrier's labels translated onto the target account, propagated to the
|
||||
* thread.
|
||||
*
|
||||
* Deliberately outside PostIngestPipeline: the row already went through it
|
||||
* on the IMAP side, so re-running would record a second create for an id
|
||||
* JMAP clients hold, and re-apply rules to mail the user may since have
|
||||
* filed by hand. Enrichment adds Gmail's label knowledge, not new content.
|
||||
*
|
||||
* ## Read state, and who owns it
|
||||
*
|
||||
* $ownsFlags is the whole of the difference. Gmail models read and starred
|
||||
* as the labels UNREAD and STARRED, so they arrive in the very labelIds
|
||||
* this re-fetch exists to re-read — and this method used to drop them on
|
||||
* the floor for every row alike. On a native Gmail account that was the
|
||||
* same gap plain IMAP had: mail read on a phone stayed unread in plMail
|
||||
* forever, because nothing ever re-read the state of a message it already
|
||||
* had. GmailMessageBuilder reads both at ingest and then never again.
|
||||
*
|
||||
* False only for the Gmailify case, where the row is a sibling account's
|
||||
* own IMAP copy. There the IMAP folder listing is the authority on flags
|
||||
* and this carrier is a bystander that happens to see the same mail.
|
||||
*
|
||||
* @param list<string> $labelIds
|
||||
*/
|
||||
private function enrichExisting(
|
||||
@@ -261,6 +282,7 @@ final readonly class SyncGmailMessageBatchHandler
|
||||
string $gmailId,
|
||||
Account $target,
|
||||
Account $carrier,
|
||||
bool $ownsFlags,
|
||||
): void {
|
||||
if ('' !== $gmailId) {
|
||||
$existing->gmailId = $gmailId;
|
||||
@@ -270,6 +292,20 @@ final readonly class SyncGmailMessageBatchHandler
|
||||
|
||||
$this->messageBuilder->applyTranslatedLabels($existing, $labelIds, $target, $carrier);
|
||||
|
||||
if (true === $ownsFlags) {
|
||||
// One row at a time because enrichment is one row at a time, and
|
||||
// this is the call that recounts the thread, updates the unread
|
||||
// badge and records the JMAP change — as well as declining to
|
||||
// revert a local change Gmail has not confirmed yet.
|
||||
$this->status->applyRemoteFlags([
|
||||
new RemoteFlagState(
|
||||
$existing,
|
||||
false === in_array('UNREAD', $labelIds, true),
|
||||
true === in_array('STARRED', $labelIds, true),
|
||||
),
|
||||
]);
|
||||
}
|
||||
|
||||
// Enrichment rewrites the message's labels, i.e. Email.mailboxIds — a
|
||||
// change a JMAP client must see. The row already has an id, so this is
|
||||
// safe to record before the batch flush.
|
||||
|
||||
@@ -764,6 +764,42 @@ class MessageRepository extends ServiceEntityRepository
|
||||
->getSingleColumnResult();
|
||||
}
|
||||
|
||||
/**
|
||||
* Every addressable row this mailbox holds, as UID => Message.
|
||||
*
|
||||
* Hydrated, unlike findLocatedUidsById(), and keyed the other way round,
|
||||
* because the caller's question is the other way round: the flag pass holds
|
||||
* the server's answer keyed by UID and needs the row that answer is about,
|
||||
* in order to change it. There is no bulk-update shortcut available to it —
|
||||
* applying a flag means seenAt, starredAt, the thread's unread count and a
|
||||
* JMAP change-log row, which is ThreadStatusUpdater's job and needs
|
||||
* entities.
|
||||
*
|
||||
* The cost is bounded by the same thing that bounds the sweep: this runs
|
||||
* once per folder per SWEEP_INTERVAL_MINUTES, against the folder's located
|
||||
* rows, and the listing it is compared with is the expensive half.
|
||||
*
|
||||
* @return array<int,Message> imapUid => Message
|
||||
*/
|
||||
public function findLocatedByUid(Mailbox $mailbox): array
|
||||
{
|
||||
/** @var list<Message> $rows */
|
||||
$rows = $this->createQueryBuilder('m')
|
||||
->where('m.mailbox = :mailbox')
|
||||
->andWhere('m.imapUid IS NOT NULL')
|
||||
->setParameter('mailbox', $mailbox)
|
||||
->getQuery()
|
||||
->getResult();
|
||||
|
||||
$located = [];
|
||||
|
||||
foreach ($rows as $row) {
|
||||
$located[(int) $row->imapUid] = $row;
|
||||
}
|
||||
|
||||
return $located;
|
||||
}
|
||||
|
||||
/**
|
||||
* Every address this mailbox holds, as row id => UID.
|
||||
*
|
||||
|
||||
@@ -4,6 +4,7 @@ declare(strict_types=1);
|
||||
|
||||
namespace App\Service\Graph;
|
||||
|
||||
use App\Domain\DTO\Mail\RemoteFlagState;
|
||||
use App\Entity\Mail\Account;
|
||||
use App\Entity\Mail\Message;
|
||||
use App\Jmap\State\JmapObjectType;
|
||||
@@ -12,6 +13,8 @@ use App\Infrastructure\Messaging\Message\SyncGraphMessageBatchMessage;
|
||||
use App\Repository\Mail\MessageRepository;
|
||||
use App\Service\Mail\GraphApiClient;
|
||||
use App\Service\Mail\MessageEraser;
|
||||
use App\Service\Mail\ThreadStatusUpdater;
|
||||
use DateTimeImmutable;
|
||||
use Doctrine\ORM\EntityManagerInterface;
|
||||
use Psr\Log\LoggerInterface;
|
||||
use Symfony\Component\Messenger\MessageBusInterface;
|
||||
@@ -55,6 +58,7 @@ final class GraphApiSyncer
|
||||
private readonly MessageBusInterface $bus,
|
||||
private readonly LoggerInterface $logger,
|
||||
private readonly MessageEraser $eraser,
|
||||
private readonly ThreadStatusUpdater $status,
|
||||
) {}
|
||||
|
||||
/**
|
||||
@@ -235,6 +239,12 @@ final class GraphApiSyncer
|
||||
);
|
||||
|
||||
$needsFetch = [];
|
||||
$flagStates = [];
|
||||
|
||||
// Before the loop, because the echo guard is measured from the moment
|
||||
// the provider was asked and the delta was already in hand when this
|
||||
// was called.
|
||||
$readAt = new DateTimeImmutable();
|
||||
|
||||
foreach ($items as $item) {
|
||||
$graphId = (string) ($item['id'] ?? '');
|
||||
@@ -253,14 +263,77 @@ final class GraphApiSyncer
|
||||
continue;
|
||||
}
|
||||
|
||||
// Already synced and still present — the only thing that can have
|
||||
// changed cheaply is where it lives.
|
||||
// Already synced and still present — where it lives, and what state
|
||||
// it is in.
|
||||
$this->attachFolderLabel($account, $graphId, (string) ($item['parentFolderId'] ?? $folderId));
|
||||
|
||||
// Read state used to stop here. The comment above this block said
|
||||
// the only cheap thing that could have changed was location, and it
|
||||
// was wrong: a Graph delta entry carries the whole message resource,
|
||||
// isRead and flag included, so a message read in Outlook announced
|
||||
// itself in this very payload and plMail dropped it. Mail read on a
|
||||
// phone stayed unread here, exactly as it did on IMAP, and for the
|
||||
// same reason — nothing ever re-read the state of a row it already
|
||||
// had.
|
||||
$state = $this->flagStateOf($graphId, $item);
|
||||
|
||||
if (null !== $state) {
|
||||
$flagStates[] = $state;
|
||||
}
|
||||
}
|
||||
|
||||
// One call for the batch: it recounts threads and writes the JMAP
|
||||
// change log, and it is where the echo guard keeps an unconfirmed local
|
||||
// change from being reverted by a delta that predates it.
|
||||
if ([] !== $flagStates) {
|
||||
$this->status->applyRemoteFlags($flagStates, $readAt);
|
||||
}
|
||||
|
||||
return $needsFetch;
|
||||
}
|
||||
|
||||
/**
|
||||
* What one delta entry says about a stored message's flags, or null if it
|
||||
* does not say anything usable.
|
||||
*
|
||||
* Graph has no flag list, so the two booleans it does answer are turned
|
||||
* into one: RemoteFlagState::storedFlags() synthesises the mirror. \Draft
|
||||
* is deliberately not carried here — isDraft is a property of the message
|
||||
* that GraphMessageBuilder sets at ingest and that a delta on a stored
|
||||
* message does not change without the message being rewritten anyway.
|
||||
*
|
||||
* @param array<string,mixed> $item
|
||||
*/
|
||||
private function flagStateOf(string $graphId, array $item): ?RemoteFlagState
|
||||
{
|
||||
if (false === array_key_exists('isRead', $item)) {
|
||||
// A delta entry that does not mention read state is not asserting
|
||||
// that the message is unread. Absence is not an answer — the same
|
||||
// rule the IMAP listing works to.
|
||||
return null;
|
||||
}
|
||||
|
||||
$message = $this->messageRepository->findOneBy(['graphId' => $graphId]);
|
||||
|
||||
if (null === $message) {
|
||||
return null;
|
||||
}
|
||||
|
||||
$flagStatus = $item['flag']['flagStatus'] ?? null;
|
||||
|
||||
return new RemoteFlagState(
|
||||
$message,
|
||||
true === $item['isRead'],
|
||||
null === $flagStatus
|
||||
// Exchange omits `flag` on a message that has never carried
|
||||
// one. That is "not flagged", not "unknown" — unlike isRead,
|
||||
// whose absence would be a delta that simply did not mention
|
||||
// it — so the row's current star is preserved instead.
|
||||
? null !== $message->starredAt
|
||||
: 'notFlagged' !== (string) $flagStatus,
|
||||
);
|
||||
}
|
||||
|
||||
private function attachFolderLabel(Account $account, string $graphId, string $folderId): void
|
||||
{
|
||||
$message = $this->messageRepository->findOneBy(['graphId' => $graphId]);
|
||||
|
||||
@@ -0,0 +1,161 @@
|
||||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
namespace App\Service\Imap;
|
||||
|
||||
use App\Domain\DTO\Mail\RemoteFlagState;
|
||||
use App\Domain\Enum\Mail\MessageFlag;
|
||||
use App\Entity\Mail\Mailbox;
|
||||
use App\Repository\Mail\MessageRepository;
|
||||
use App\Service\Mail\ThreadStatusUpdater;
|
||||
use DateTimeImmutable;
|
||||
use Psr\Log\LoggerInterface;
|
||||
|
||||
/**
|
||||
* Flags the server changed under us, and which of them plMail is allowed to
|
||||
* believe.
|
||||
*
|
||||
* ## The gap this closes
|
||||
*
|
||||
* plMail never re-read the flags of a message it had already stored. They were
|
||||
* captured once, at ingest, in MessageSyncer::buildMessage() — and then never
|
||||
* again, because incremental sync asks for `lastSeenUid+1:*` and no UID below
|
||||
* the high-water mark is ever looked at a second time. Mail read on a phone
|
||||
* stayed unread here forever. A star set in Roundcube never arrived. Not
|
||||
* eventually: never.
|
||||
*
|
||||
* The listing that VanishedMessageReconciler already runs every quarter of an
|
||||
* hour now carries flags as well as presence — see ImapFolderListing for why
|
||||
* that is cheaper than keeping the two apart — so this needs no round trip of
|
||||
* its own. It is handed what that one command said and decides what it means.
|
||||
*
|
||||
* ## The echo race, and the guard
|
||||
*
|
||||
* This is the hard part, and it is a race the outbound direction never had.
|
||||
*
|
||||
* The user marks a message read. The local row changes first, because the
|
||||
* database is the source of truth, and an ApplyImapFlagsMessage is queued to
|
||||
* carry it to the server. That job may be queued, in flight, or waiting behind
|
||||
* a NAS that is asleep. For all of that time the server's honest answer to
|
||||
* "what are this message's flags" is the *old* one — and a pass that believed
|
||||
* it would clear seenAt, which the propagator would notice as a local change
|
||||
* and queue an outbound job for, which the next pass would revert again. Two
|
||||
* directions, each correctly reporting what it sees, flapping forever.
|
||||
*
|
||||
* Note what does *not* fix this: comparing the local change's timestamp against
|
||||
* the moment the server was asked. The local write happens *before* the read —
|
||||
* that is the whole shape of the race — so it is older, and a last-write-wins
|
||||
* on those two timestamps chooses the server every time.
|
||||
*
|
||||
* What the local change actually is, is *unconfirmed*. So that is what is
|
||||
* written down: Message::$flagsTouchedAt is set by LabelChangePropagator where
|
||||
* the outbound job is queued, and cleared by ApplyImapFlagsHandler where the
|
||||
* server accepts it.
|
||||
*
|
||||
* The guard itself is not applied here. It lives in
|
||||
* ThreadStatusUpdater::applyRemoteFlags(), which is the one place every
|
||||
* provider's inbound flags land — Gmail's labels and Graph's isRead reach it
|
||||
* too — because one rule enforced once is a rule no future caller can forget to
|
||||
* apply. What this class contributes is the read time, since the guard is
|
||||
* measured from the moment the server was asked.
|
||||
*
|
||||
* ## Three answers again
|
||||
*
|
||||
* A folder that could not be listed produces no flag pass, exactly as it
|
||||
* produces no sweep: the caller never gets here. Within a listing, a UID the
|
||||
* server did not describe is skipped rather than read as "no flags" — an
|
||||
* absent answer is not the answer "unread and unstarred", and treating it as
|
||||
* one would mark a folder's worth of read mail unread. The same discipline
|
||||
* ImapUidPresence is built on.
|
||||
*/
|
||||
final readonly class ImapFlagReconciler
|
||||
{
|
||||
public function __construct(
|
||||
private MessageRepository $messages,
|
||||
private ThreadStatusUpdater $status,
|
||||
private LoggerInterface $logger,
|
||||
) {
|
||||
}
|
||||
|
||||
/**
|
||||
* Apply one folder listing's flags to the rows that folder holds.
|
||||
*
|
||||
* @param array<int,list<string>> $flagsByUid as the server spelled them
|
||||
* @param DateTimeImmutable $readAt when the server was asked
|
||||
*
|
||||
* @return int how many rows changed
|
||||
*/
|
||||
public function reconcile(Mailbox $mailbox, array $flagsByUid, DateTimeImmutable $readAt): int
|
||||
{
|
||||
if ([] === $flagsByUid) {
|
||||
return 0;
|
||||
}
|
||||
|
||||
$located = $this->messages->findLocatedByUid($mailbox);
|
||||
|
||||
if ([] === $located) {
|
||||
return 0;
|
||||
}
|
||||
|
||||
$states = [];
|
||||
|
||||
foreach ($located as $uid => $message) {
|
||||
$flags = $flagsByUid[$uid] ?? null;
|
||||
|
||||
if (null === $flags) {
|
||||
// The server did not describe this UID. The sweep is what draws
|
||||
// conclusions from absence, with the safety rails; here it is
|
||||
// simply not an answer, and an unanswered UID is left exactly
|
||||
// as it is.
|
||||
continue;
|
||||
}
|
||||
|
||||
$states[] = new RemoteFlagState(
|
||||
$message,
|
||||
$this->hasFlag($flags, 'seen'),
|
||||
$this->hasFlag($flags, 'flagged'),
|
||||
MessageFlag::canonicalList($flags),
|
||||
);
|
||||
}
|
||||
|
||||
// The read time goes with the states rather than being taken there,
|
||||
// because the echo guard is measured from the moment the server was
|
||||
// asked. ThreadStatusUpdater owns that judgement for every provider —
|
||||
// see its applyRemoteFlags() docblock.
|
||||
$changed = $this->status->applyRemoteFlags($states, $readAt);
|
||||
|
||||
if ($changed > 0) {
|
||||
$this->logger->info('Refreshed message flags from the server', [
|
||||
'mailbox' => $mailbox->fullPath,
|
||||
'changed' => $changed,
|
||||
'compared' => count($states),
|
||||
]);
|
||||
}
|
||||
|
||||
return $changed;
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether a flag list contains one flag, whatever the server called it.
|
||||
*
|
||||
* The backslash is part of the IMAP wire syntax for a system flag rather
|
||||
* than part of its name, and servers are inconsistent about whether it
|
||||
* survives into a parsed response — webklex hands back `Seen` from some and
|
||||
* `\Seen` from others. Comparing on the bare lower-cased name is the only
|
||||
* comparison that is right on both.
|
||||
*
|
||||
* @param list<string> $flags
|
||||
*/
|
||||
private function hasFlag(array $flags, string $wanted): bool
|
||||
{
|
||||
foreach ($flags as $flag) {
|
||||
if ($wanted === strtolower(ltrim($flag, '\\'))) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
}
|
||||
@@ -8,6 +8,8 @@ use App\Entity\Mail\Mailbox;
|
||||
use Psr\Log\LoggerInterface;
|
||||
use Throwable;
|
||||
use Webklex\PHPIMAP\Client;
|
||||
use Webklex\PHPIMAP\Connection\Protocols\ImapProtocol;
|
||||
use Webklex\PHPIMAP\Connection\Protocols\ProtocolInterface;
|
||||
use Webklex\PHPIMAP\IMAP;
|
||||
|
||||
/**
|
||||
@@ -27,7 +29,37 @@ use Webklex\PHPIMAP\IMAP;
|
||||
* Learning what left requires asking what is still there. There is no cheaper
|
||||
* question.
|
||||
*
|
||||
* ## Why UID SEARCH ALL and not QRESYNC
|
||||
* ## Why the listing carries flags now
|
||||
*
|
||||
* This used to ask `UID SEARCH ALL`, which answers presence and nothing else —
|
||||
* about seven bytes a message. It now asks `UID FETCH 1:* (FLAGS)`, which
|
||||
* answers presence *and* every message's flags in the same round trip, at
|
||||
* roughly forty to sixty bytes a message.
|
||||
*
|
||||
* That looks like paying six times more for something only half of which the
|
||||
* sweep wanted, and it is the opposite. plMail never re-read the flags of a
|
||||
* stored message — they were captured once at ingest and never again, so mail
|
||||
* read on a phone stayed unread here forever — and fixing that needs a
|
||||
* full-folder flag read on a cadence no matter what. Done separately it would
|
||||
* be `UID SEARCH ALL` *plus* `UID FETCH 1:* (FLAGS)`: this same FETCH, plus the
|
||||
* SEARCH still underneath it. Folding the two together is therefore strictly
|
||||
* cheaper than the alternative that keeps them apart, not more expensive, and
|
||||
* it is one command per folder per cadence instead of two.
|
||||
*
|
||||
* The consistency argument is the better one anyway. Two commands are two
|
||||
* instants, and a message can vanish between them — which would have the sweep
|
||||
* reasoning about a folder the flag pass never saw. One FETCH gives both
|
||||
* answers from one instant, so "what is here" and "what state is it in" cannot
|
||||
* disagree.
|
||||
*
|
||||
* The cost that is real is memory rather than bytes: webklex accumulates the
|
||||
* whole response into an array before returning it, so a fifty-thousand-message
|
||||
* folder builds a fifty-thousand-entry map. That is tens of megabytes at the
|
||||
* top end and it is the honest reason a bigger installation might want this
|
||||
* chunked. It is not chunked here because chunking needs UID ranges to chunk
|
||||
* *by*, and learning the ranges is the very question this command answers.
|
||||
*
|
||||
* ## Why UID FETCH and not QRESYNC
|
||||
*
|
||||
* RFC 7162 is the right answer to this problem and plMail cannot use it:
|
||||
* webklex/php-imap implements neither QRESYNC nor CONDSTORE — there is no
|
||||
@@ -37,11 +69,11 @@ use Webklex\PHPIMAP\IMAP;
|
||||
* great deal more code than this and fails closed on every server that does not
|
||||
* advertise it anyway.
|
||||
*
|
||||
* `EXAMINE` followed by `UID SEARCH ALL` is two round trips, universally
|
||||
* supported since IMAP4rev1, and hands back a few hundred kilobytes for a
|
||||
* folder of fifty thousand. That is too much to do on every poll and nothing at
|
||||
* all to do every quarter of an hour, which is why VanishedMessageReconciler
|
||||
* puts it on a cadence rather than in the poll.
|
||||
* `EXAMINE` followed by `UID FETCH 1:* (FLAGS)` is two round trips, universally
|
||||
* supported since IMAP4rev1, and hands back a couple of megabytes for a folder
|
||||
* of fifty thousand. That is too much to do on every poll and little enough to
|
||||
* do every quarter of an hour, which is why VanishedMessageReconciler puts it
|
||||
* on a cadence rather than in the poll.
|
||||
*
|
||||
* EXAMINE rather than SELECT deliberately: it is the read-only form, so
|
||||
* measuring a folder never clears \Recent on it.
|
||||
@@ -61,9 +93,13 @@ final readonly class ImapFolderListing
|
||||
}
|
||||
|
||||
/**
|
||||
* @return array{uidValidity: int, uids: array<int,true>}|null null when the
|
||||
* server did not answer. `uids` is a set rather than a list: the
|
||||
* caller's only question of it is membership, once per local row.
|
||||
* @return array{uidValidity: int, uids: array<int,true>, flags: array<int,list<string>>, readAt: \DateTimeImmutable}|null
|
||||
* null when the server did not answer. `uids` is a set rather than a
|
||||
* list: the caller's only question of it is membership, once per
|
||||
* local row. `flags` is keyed by the same UIDs, and `readAt` is when
|
||||
* the question was put to the server — which the flag reconciler
|
||||
* needs, because "what the server said" is only meaningful together
|
||||
* with "and when".
|
||||
*/
|
||||
public function read(Client $client, Mailbox $mailbox): ?array
|
||||
{
|
||||
@@ -86,21 +122,46 @@ final readonly class ImapFolderListing
|
||||
return null;
|
||||
}
|
||||
|
||||
$found = $connection->search(['ALL'], IMAP::ST_UID)->validatedData();
|
||||
// Stamped before the command rather than after it. The reconciler
|
||||
// compares this against local flag changes, and the safe direction
|
||||
// to be wrong in is "this answer is older than it really is": that
|
||||
// makes a race resolve in favour of the local write, which is the
|
||||
// one the user made.
|
||||
$readAt = new \DateTimeImmutable();
|
||||
|
||||
$found = $this->listWithFlags($connection);
|
||||
|
||||
if (false === is_array($found)) {
|
||||
return null;
|
||||
}
|
||||
|
||||
$uids = [];
|
||||
$uids = [];
|
||||
$flags = [];
|
||||
|
||||
foreach ($found as $uid) {
|
||||
$uids[(int) $uid] = true;
|
||||
foreach ($found as $uid => $messageFlags) {
|
||||
$uid = (int) $uid;
|
||||
|
||||
$uids[$uid] = true;
|
||||
|
||||
// A message with no flags at all answers with an empty list,
|
||||
// which is a fact — "this message is unread and unstarred" —
|
||||
// and not a missing answer. Anything that is not a list is the
|
||||
// missing answer, and it is left out rather than read as empty.
|
||||
if (false === is_array($messageFlags)) {
|
||||
continue;
|
||||
}
|
||||
|
||||
$flags[$uid] = array_values(array_map(
|
||||
static fn (mixed $flag): string => (string) $flag,
|
||||
$messageFlags,
|
||||
));
|
||||
}
|
||||
|
||||
return [
|
||||
'uidValidity' => (int) $examined['uidvalidity'],
|
||||
'uids' => $uids,
|
||||
'flags' => $flags,
|
||||
'readAt' => $readAt,
|
||||
];
|
||||
} catch (Throwable $e) {
|
||||
$this->logger->info('Could not list a folder in full', [
|
||||
@@ -111,4 +172,63 @@ final readonly class ImapFolderListing
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Every UID in the selected folder with its flags, in as few commands as
|
||||
* this connection allows.
|
||||
*
|
||||
* On the socket client — what plMail actually uses — that is one command:
|
||||
* `UID FETCH 1:* (FLAGS)`, which webklex spells as a $to of INF. With a $to
|
||||
* set at all its single-message filter is skipped, so every FETCH line in
|
||||
* the response is collected and keyed by UID, and that keying is what makes
|
||||
* presence and flags a single answer from a single instant.
|
||||
*
|
||||
* fetch() is on ImapProtocol rather than on ProtocolInterface, so the
|
||||
* narrowing is real and not a formality: webklex also ships LegacyProtocol,
|
||||
* which wraps the ext-imap functions and has no such method. That path
|
||||
* falls back to what the interface does guarantee — a UID SEARCH for
|
||||
* presence, then flags() for the flags — which is the two commands and two
|
||||
* instants this class exists to avoid, and is still better than a
|
||||
* legacy-backed account never learning a flag changed.
|
||||
*
|
||||
* @return array<int|string,mixed>|false false when the server did not
|
||||
* answer in a shape worth reading
|
||||
*/
|
||||
private function listWithFlags(ProtocolInterface $connection): array|false
|
||||
{
|
||||
if (true === $connection instanceof ImapProtocol) {
|
||||
// INF is how webklex spells the open-ended `1:*`, and it says so in
|
||||
// that method's own docblock — which then types the parameter
|
||||
// int|null, a float being the one value that actually reaches the
|
||||
// `$to == INF` branch. The library is inconsistent with itself
|
||||
// here; the call is right.
|
||||
// @phpstan-ignore argument.type
|
||||
$found = $connection->fetch(['FLAGS'], 1, INF, IMAP::ST_UID)->validatedData();
|
||||
|
||||
return is_array($found) ? $found : false;
|
||||
}
|
||||
|
||||
$found = $connection->search(['ALL'], IMAP::ST_UID)->validatedData();
|
||||
|
||||
if (false === is_array($found)) {
|
||||
return false;
|
||||
}
|
||||
|
||||
$uids = array_map(intval(...), array_values($found));
|
||||
|
||||
if ([] === $uids) {
|
||||
return [];
|
||||
}
|
||||
|
||||
$flags = $connection->flags($uids, IMAP::ST_UID)->validatedData();
|
||||
|
||||
if (false === is_array($flags)) {
|
||||
// Presence without flags is still a usable listing: the sweep is
|
||||
// the half that deletes mail, and it loses nothing here. The flag
|
||||
// pass simply has nothing to say this cycle.
|
||||
return array_fill_keys($uids, []);
|
||||
}
|
||||
|
||||
return $flags;
|
||||
}
|
||||
}
|
||||
@@ -140,6 +140,7 @@ final readonly class VanishedMessageReconciler
|
||||
private MessageRepository $messages,
|
||||
private MailboxRepository $mailboxes,
|
||||
private MessageEraser $eraser,
|
||||
private ImapFlagReconciler $flags,
|
||||
private EntityManagerInterface $em,
|
||||
private LoggerInterface $logger,
|
||||
) {
|
||||
@@ -181,7 +182,7 @@ final readonly class VanishedMessageReconciler
|
||||
* MovedMessageReconciliationTest works through, where the test supplies the
|
||||
* answer the probe would have given.
|
||||
*
|
||||
* @param array{uidValidity: int, uids: array<int,true>} $listing
|
||||
* @param array{uidValidity: int, uids: array<int,true>, flags?: array<int,list<string>>, readAt?: DateTimeImmutable} $listing
|
||||
*
|
||||
* @return bool whether the listing was believed and acted on
|
||||
*/
|
||||
@@ -217,6 +218,19 @@ final readonly class VanishedMessageReconciler
|
||||
$marked = $this->messages->markVanished($vanished, $now);
|
||||
$cleared = $this->messages->clearVanished($returned);
|
||||
|
||||
// The other half of what this listing said. A message's presence and
|
||||
// its flags came back in one `UID FETCH 1:* (FLAGS)` — see
|
||||
// ImapFolderListing for why they are one command — so they are believed
|
||||
// together or not at all, and every path above that refuses this
|
||||
// listing refuses its flags with it. A listing suspicious enough not to
|
||||
// conclude a deletion from is not one to conclude a read state from
|
||||
// either.
|
||||
$this->flags->reconcile(
|
||||
$mailbox,
|
||||
$listing['flags'] ?? [],
|
||||
$listing['readAt'] ?? $now,
|
||||
);
|
||||
|
||||
$mailbox->uidValidity = $listing['uidValidity'];
|
||||
$mailbox->sweptAt = $now;
|
||||
|
||||
|
||||
@@ -61,6 +61,8 @@ final readonly class LabelChangePropagator
|
||||
$imapAction = 'flag';
|
||||
}
|
||||
|
||||
$messages = $this->markFlagsInFlight($messages);
|
||||
|
||||
$this->dispatchFlags($messages, $imapAction);
|
||||
$this->dispatchGmail($messages, $this->gmailStarPayload($starred));
|
||||
$this->dispatchGraph($messages);
|
||||
@@ -77,6 +79,8 @@ final readonly class LabelChangePropagator
|
||||
$imapAction = 'seen';
|
||||
}
|
||||
|
||||
$messages = $this->markFlagsInFlight($messages);
|
||||
|
||||
$this->dispatchFlags($messages, $imapAction);
|
||||
$this->dispatchGmail($messages, $this->gmailReadPayload($read));
|
||||
$this->dispatchGraph($messages);
|
||||
@@ -253,6 +257,43 @@ final readonly class LabelChangePropagator
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Write down that these messages carry a flag change the provider has not
|
||||
* confirmed yet, and hand back a list the three dispatchers can each walk.
|
||||
*
|
||||
* This is the write half of the echo-race guard, and it belongs here rather
|
||||
* than in the callers because this is the one place an outbound *flag* op
|
||||
* is queued — for every provider at once. ThreadStatusUpdater and the JMAP
|
||||
* EmailPatchApplier both arrive through star()/markRead(), so neither has
|
||||
* to remember to mark anything, and neither can forget.
|
||||
*
|
||||
* Only star() and markRead() call it. A move is not a flag change: the
|
||||
* inbound flag pass reads \Seen and \Flagged, and an archive that is still
|
||||
* in flight is a question about where the row lives, which the deletion
|
||||
* sweep and SentCopyReconciler already answer between them.
|
||||
*
|
||||
* No flush. Every caller mutates the row and flushes afterwards — that
|
||||
* ordering is the whole contract of this class — so this rides along on the
|
||||
* flush the mutation was already going to do.
|
||||
*
|
||||
* @param iterable<Message> $messages
|
||||
*
|
||||
* @return list<Message>
|
||||
*/
|
||||
private function markFlagsInFlight(iterable $messages): array
|
||||
{
|
||||
$now = new \DateTimeImmutable();
|
||||
$list = [];
|
||||
|
||||
foreach ($messages as $message) {
|
||||
$message->flagsTouchedAt = $now;
|
||||
|
||||
$list[] = $message;
|
||||
}
|
||||
|
||||
return $list;
|
||||
}
|
||||
|
||||
/**
|
||||
* @param iterable<Message> $messages
|
||||
*/
|
||||
|
||||
@@ -4,6 +4,7 @@ declare(strict_types=1);
|
||||
|
||||
namespace App\Service\Mail;
|
||||
|
||||
use App\Domain\DTO\Mail\RemoteFlagState;
|
||||
use App\Domain\Enum\Mail\LabelRole;
|
||||
use App\Domain\Enum\Mail\MessageFlag;
|
||||
use App\Entity\Label\Label;
|
||||
@@ -46,6 +47,24 @@ use Doctrine\ORM\EntityManagerInterface;
|
||||
*/
|
||||
final readonly class ThreadStatusUpdater
|
||||
{
|
||||
/**
|
||||
* How long a local flag change is allowed to stay unconfirmed before the
|
||||
* provider's answer outranks it again.
|
||||
*
|
||||
* The guard has to expire, or it is the same bug in a smaller box: a job
|
||||
* lost for good — a transport wiped, a worker that never returns, an
|
||||
* account whose credentials stopped working — would freeze that row's flags
|
||||
* against the server permanently.
|
||||
*
|
||||
* An hour is generous on purpose. Messenger's retry ladder, a NAS asleep
|
||||
* until morning, a worker restarted mid-deploy: all minutes, none of them
|
||||
* an hour. A change still unconfirmed after one is not in flight, it is
|
||||
* lost, and the row should go back to agreeing with the server. The cost of
|
||||
* being wrong here is one reverted flag; the cost of no bound at all is a
|
||||
* row that never syncs its flags again.
|
||||
*/
|
||||
public const int PENDING_GRACE_MINUTES = 60;
|
||||
|
||||
public function __construct(
|
||||
private EntityManagerInterface $em,
|
||||
private LabelRepository $labels,
|
||||
@@ -212,6 +231,167 @@ final readonly class ThreadStatusUpdater
|
||||
$this->em->flush();
|
||||
}
|
||||
|
||||
/**
|
||||
* Take the provider's word for a set of messages' flags.
|
||||
*
|
||||
* The inbound twin of markRead() and star(), and deliberately in the same
|
||||
* class: what \Seen and \Flagged *mean* locally — a seenAt, a starredAt, a
|
||||
* thread star, a thread unread count, a row in the JMAP change log — is one
|
||||
* answer, and having the inbound direction reimplement it somewhere else is
|
||||
* how the two drift until a message is read in one client and unread in the
|
||||
* other's badge.
|
||||
*
|
||||
* The one thing it does not do is call the propagator, and that is the
|
||||
* whole difference. These changes came *from* the server, so sending them
|
||||
* back is at best a wasted round trip and at worst the echo that makes the
|
||||
* two directions argue.
|
||||
*
|
||||
* ## The echo guard lives here
|
||||
*
|
||||
* The user marks a message read. The row changes first, because the
|
||||
* database is the source of truth, and an outbound job is queued. Until
|
||||
* that job lands, the provider's honest answer is still the *old* one — and
|
||||
* believing it would revert the user, which the propagator would see as a
|
||||
* fresh local change and queue a job for, which the next pass would revert
|
||||
* again. A flap, from two directions each correctly reporting what it sees.
|
||||
*
|
||||
* Note what does not fix it: comparing timestamps. The local write happens
|
||||
* *before* the read — that is the shape of the race — so it is the older of
|
||||
* the two, and last-write-wins picks the server every time. What the local
|
||||
* change actually is, is unconfirmed, so that is what Message::$flagsTouchedAt
|
||||
* records: set where the outbound job is queued, cleared where the provider
|
||||
* accepts it. A row carrying one is left alone entirely, rather than merged,
|
||||
* because a message whose \Seen is in flight may have had its \Flagged
|
||||
* changed remotely meanwhile and a flag list cannot tell the two apart.
|
||||
*
|
||||
* The guard is checked here rather than in each provider's reconciler on
|
||||
* purpose: it is one rule, it applies to IMAP, Gmail and Graph alike, and a
|
||||
* guard that every caller has to remember is one a future caller will
|
||||
* forget.
|
||||
*
|
||||
* Counters are recounted from the thread's own messages rather than
|
||||
* adjusted, because a flag pass can change several messages of one thread
|
||||
* at once and in both directions; markRead()'s cheaper arithmetic works
|
||||
* because the user's action covers the whole selection.
|
||||
*
|
||||
* @param list<RemoteFlagState> $states
|
||||
* @param DateTimeImmutable|null $readAt when the provider was asked. The
|
||||
* echo guard is measured from here rather than from now, because a
|
||||
* pass that took a minute to walk a large folder should judge its
|
||||
* rows against the moment it asked, not the moment it got here.
|
||||
*
|
||||
* @return int how many rows actually changed
|
||||
*/
|
||||
public function applyRemoteFlags(array $states, ?DateTimeImmutable $readAt = null): int
|
||||
{
|
||||
$readAt = $readAt ?? new DateTimeImmutable();
|
||||
$cutoff = $readAt->modify('-' . self::PENDING_GRACE_MINUTES . ' minutes');
|
||||
$changed = [];
|
||||
$threads = [];
|
||||
$expired = false;
|
||||
|
||||
foreach ($states as $state) {
|
||||
$message = $state->message;
|
||||
|
||||
$pending = $message->flagsTouchedAt;
|
||||
|
||||
if (null !== $pending && $pending > $cutoff) {
|
||||
// A local flag change the provider has not confirmed. Its
|
||||
// answer is stale by construction — it predates the change —
|
||||
// and applying it would revert the user and start a flap. See
|
||||
// the method docblock.
|
||||
continue;
|
||||
}
|
||||
|
||||
if (null !== $pending) {
|
||||
// The guard expired. Whatever was carrying that change is not
|
||||
// coming back, and leaving the mark would make this row skip
|
||||
// inbound flag sync for the rest of its life.
|
||||
$message->flagsTouchedAt = null;
|
||||
$expired = true;
|
||||
}
|
||||
|
||||
$wasSeen = null !== $message->seenAt;
|
||||
$wasFlagged = null !== $message->starredAt;
|
||||
|
||||
$storedFlags = $state->storedFlags();
|
||||
|
||||
// Both sides canonicalised, so that a mirror captured from a server
|
||||
// that writes `Seen` is not read as differing from a listing that
|
||||
// writes `\Seen`. Without it the first pass over a folder would
|
||||
// rewrite every row and log a JMAP change for each — see
|
||||
// MessageFlag::canonicalList().
|
||||
if (
|
||||
$wasSeen === $state->seen
|
||||
&& $wasFlagged === $state->flagged
|
||||
&& MessageFlag::canonicalList($message->flags) === $storedFlags
|
||||
) {
|
||||
continue;
|
||||
}
|
||||
|
||||
// The mirror first, so \Answered and \Draft — which the model
|
||||
// carries here and nowhere else — survive the refresh alongside the
|
||||
// two that have columns.
|
||||
$message->flags = $storedFlags;
|
||||
|
||||
if ($wasSeen !== $state->seen) {
|
||||
// Now, not the server's idea of when: IMAP does not record when
|
||||
// a flag was set, so the only honest timestamp is the one at
|
||||
// which plMail learned of it. Null when unread, which is what
|
||||
// every unread query in the codebase asks about.
|
||||
$message->seenAt = true === $state->seen ? new DateTimeImmutable() : null;
|
||||
}
|
||||
|
||||
if ($wasFlagged !== $state->flagged) {
|
||||
$message->starredAt = true === $state->flagged ? new DateTimeImmutable() : null;
|
||||
|
||||
$thread = $message->thread;
|
||||
|
||||
if (null !== $thread) {
|
||||
// A thread is starred as a whole — the same rule star()
|
||||
// works to, from the other direction.
|
||||
$thread->starredAt = $message->starredAt;
|
||||
}
|
||||
}
|
||||
|
||||
$changed[] = $message;
|
||||
|
||||
$thread = $message->thread;
|
||||
|
||||
if (null !== $thread) {
|
||||
$threads[(int) $thread->id] = $thread;
|
||||
}
|
||||
}
|
||||
|
||||
if ([] === $changed) {
|
||||
if (true === $expired) {
|
||||
// Nothing to apply — the server agreed with the row all along —
|
||||
// but an expired guard mark was dropped and that has to reach
|
||||
// the database, or the next pass expires it again forever.
|
||||
$this->em->flush();
|
||||
}
|
||||
|
||||
return 0;
|
||||
}
|
||||
|
||||
foreach ($threads as $thread) {
|
||||
$unread = 0;
|
||||
|
||||
foreach ($thread->messages as $message) {
|
||||
if (null === $message->seenAt) {
|
||||
++$unread;
|
||||
}
|
||||
}
|
||||
|
||||
$thread->unreadCount = $unread;
|
||||
}
|
||||
|
||||
$this->recordJmapUpdates($changed);
|
||||
$this->em->flush();
|
||||
|
||||
return count($changed);
|
||||
}
|
||||
|
||||
/**
|
||||
* Which account a message belongs to.
|
||||
*
|
||||
|
||||
@@ -0,0 +1,77 @@
|
||||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
namespace App\Tests\Command\Imap;
|
||||
|
||||
use App\Command\Imap\ImapIdleCommand;
|
||||
use PHPUnit\Framework\TestCase;
|
||||
|
||||
/**
|
||||
* Which lines off the IDLE socket mean "a flag changed".
|
||||
*
|
||||
* The supervisor answers three untagged responses now: EXISTS for an arrival,
|
||||
* EXPUNGE for a deletion, and FETCH-with-FLAGS for a flag another client
|
||||
* changed. All three are triggers rather than authorities — none of them
|
||||
* resolves the sequence number it carries into a UID, and each simply makes the
|
||||
* folder's own machinery run now instead of on its cadence.
|
||||
*
|
||||
* The distinction pinned here is the one that costs a round trip when it is
|
||||
* wrong: SELECT answers every folder open with FLAGS and PERMANENTFLAGS lines,
|
||||
* which describe what the folder permits and say nothing about any message. A
|
||||
* matcher that woke on those would re-list a folder every time one was opened.
|
||||
*/
|
||||
final class IdleFlagTriggerTest extends TestCase
|
||||
{
|
||||
/** The shape the server actually sends when another client marks mail read. */
|
||||
public function testAnUntaggedFetchCarryingFlagsIsAFlagChange(): void
|
||||
{
|
||||
self::assertTrue(ImapIdleCommand::announcesFlagChange('* 4 FETCH (FLAGS (\\Seen))'));
|
||||
}
|
||||
|
||||
public function testAFetchCarryingFlagsAndAUidIsOneToo(): void
|
||||
{
|
||||
self::assertTrue(ImapIdleCommand::announcesFlagChange('* 12 FETCH (UID 5001 FLAGS (\\Seen \\Flagged))'));
|
||||
}
|
||||
|
||||
/** A star set elsewhere is the same announcement. */
|
||||
public function testAStarSetElsewhereAnnouncesItself(): void
|
||||
{
|
||||
self::assertTrue(ImapIdleCommand::announcesFlagChange('* 7 FETCH (FLAGS (\\Flagged))'));
|
||||
}
|
||||
|
||||
/**
|
||||
* The FLAGS line SELECT answers with. It lists the flags the folder
|
||||
* supports, which has nothing to do with any message's state.
|
||||
*/
|
||||
public function testTheFoldersOwnFlagsLineIsNotAFlagChange(): void
|
||||
{
|
||||
self::assertFalse(
|
||||
ImapIdleCommand::announcesFlagChange('* FLAGS (\\Answered \\Flagged \\Deleted \\Seen \\Draft)'),
|
||||
);
|
||||
}
|
||||
|
||||
public function testThePermanentFlagsLineIsNotOneEither(): void
|
||||
{
|
||||
self::assertFalse(
|
||||
ImapIdleCommand::announcesFlagChange('* OK [PERMANENTFLAGS (\\Answered \\Flagged \\Seen \\*)] Limited'),
|
||||
);
|
||||
}
|
||||
|
||||
/** An arrival is EXISTS, and it has its own branch. */
|
||||
public function testAnExistsLineIsNotAFlagChange(): void
|
||||
{
|
||||
self::assertFalse(ImapIdleCommand::announcesFlagChange('* 23 EXISTS'));
|
||||
}
|
||||
|
||||
/** So is a deletion. */
|
||||
public function testAnExpungeLineIsNotAFlagChange(): void
|
||||
{
|
||||
self::assertFalse(ImapIdleCommand::announcesFlagChange('* 4 EXPUNGE'));
|
||||
}
|
||||
|
||||
public function testTheIdleContinuationIsNotAFlagChange(): void
|
||||
{
|
||||
self::assertFalse(ImapIdleCommand::announcesFlagChange('+ idling'));
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,343 @@
|
||||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
namespace App\Tests\Service\Gmail;
|
||||
|
||||
use App\Domain\Enum\Account\AuthType;
|
||||
use App\Domain\Enum\Mail\LabelRole;
|
||||
use App\Domain\Enum\Mail\MessageFlag;
|
||||
use App\Domain\Enum\Mail\ThreadingMethod;
|
||||
use App\Entity\Label\Label;
|
||||
use App\Entity\Mail\Account;
|
||||
use App\Entity\Mail\Message;
|
||||
use App\Entity\Mail\MessageThread;
|
||||
use App\Entity\User\User;
|
||||
use App\Infrastructure\Messaging\Handler\SyncGmailMessageBatchHandler;
|
||||
use App\Infrastructure\Messaging\Message\SyncGmailMessageBatchMessage;
|
||||
use App\Service\Mail\GmailApiClient;
|
||||
use App\Service\Mail\SyncNotifier;
|
||||
use DateTimeImmutable;
|
||||
use Doctrine\DBAL\Connection;
|
||||
use Doctrine\ORM\EntityManagerInterface;
|
||||
use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase;
|
||||
use Symfony\Component\HttpClient\MockHttpClient;
|
||||
use Symfony\Component\HttpClient\Response\MockResponse;
|
||||
use Symfony\Component\Mercure\HubInterface;
|
||||
|
||||
/**
|
||||
* Read state that arrives as a Gmail label, and whether plMail hears it.
|
||||
*
|
||||
* Gmail models read and starred as the labels UNREAD and STARRED, so both
|
||||
* arrive in the labelIds of a message re-fetched after a label change — which
|
||||
* is exactly what GmailApiSyncer re-fetches a stored message *for*. Two
|
||||
* separate things stopped that working, and both are fixed here:
|
||||
*
|
||||
* 1. The batch handler dropped every id it already had before fetching
|
||||
* anything, which silently undid the planner's deliberate decision to send
|
||||
* relabelled ids through. The planner's own comment says they bypass that
|
||||
* filter; the handler re-applied it.
|
||||
*
|
||||
* 2. Even past that, enrichment mirrored labels and explicitly left read state
|
||||
* alone — so UNREAD coming off in Gmail meant nothing here.
|
||||
*
|
||||
* Together those made Gmail's gap the same as plain IMAP's: read state captured
|
||||
* once at ingest and never re-read. Mail read on a phone stayed unread in
|
||||
* plMail forever.
|
||||
*/
|
||||
final class GmailInboundReadStateTest extends KernelTestCase
|
||||
{
|
||||
private EntityManagerInterface $em;
|
||||
private Connection $connection;
|
||||
private User $user;
|
||||
private Account $account;
|
||||
|
||||
private const string GMAIL_ID = 'gmail-message-1';
|
||||
|
||||
protected function setUp(): void
|
||||
{
|
||||
self::bootKernel();
|
||||
|
||||
$container = self::getContainer();
|
||||
$this->em = $container->get(EntityManagerInterface::class);
|
||||
$this->connection = $container->get(Connection::class);
|
||||
|
||||
$this->connection->beginTransaction();
|
||||
|
||||
$this->user = $this->seedUser();
|
||||
$this->account = $this->seedAccount();
|
||||
}
|
||||
|
||||
protected function tearDown(): void
|
||||
{
|
||||
if (true === $this->connection->isTransactionActive()) {
|
||||
$this->connection->rollBack();
|
||||
}
|
||||
|
||||
parent::tearDown();
|
||||
}
|
||||
|
||||
// ── the truth table ───────────────────────────────────────────────────
|
||||
|
||||
/** UNREAD gone from the labels, stored unread → read. The reported gap. */
|
||||
public function testAMessageReadInGmailBecomesReadHere(): void
|
||||
{
|
||||
$message = $this->seedMessage(seen: false);
|
||||
|
||||
$this->handle(['INBOX']);
|
||||
|
||||
$this->em->refresh($message);
|
||||
|
||||
self::assertNotNull($message->seenAt);
|
||||
self::assertContains(MessageFlag::SEEN->value, $message->flags);
|
||||
}
|
||||
|
||||
/** UNREAD back on, stored read → unread. */
|
||||
public function testAMessageMarkedUnreadInGmailBecomesUnreadHere(): void
|
||||
{
|
||||
$message = $this->seedMessage(seen: true);
|
||||
|
||||
$this->handle(['INBOX', 'UNREAD']);
|
||||
|
||||
$this->em->refresh($message);
|
||||
|
||||
self::assertNull($message->seenAt);
|
||||
self::assertNotContains(MessageFlag::SEEN->value, $message->flags);
|
||||
}
|
||||
|
||||
/** STARRED on, stored unstarred → starred, thread included. */
|
||||
public function testAStarSetInGmailArrivesHere(): void
|
||||
{
|
||||
$message = $this->seedMessage(seen: false);
|
||||
|
||||
$this->handle(['INBOX', 'UNREAD', 'STARRED']);
|
||||
|
||||
$this->em->refresh($message);
|
||||
|
||||
self::assertNotNull($message->starredAt);
|
||||
self::assertNotNull($message->thread->starredAt);
|
||||
}
|
||||
|
||||
/** STARRED gone, stored starred → unstarred. */
|
||||
public function testAStarRemovedInGmailComesOffHere(): void
|
||||
{
|
||||
$message = $this->seedMessage(seen: false, flagged: true);
|
||||
|
||||
$this->handle(['INBOX', 'UNREAD']);
|
||||
|
||||
$this->em->refresh($message);
|
||||
|
||||
self::assertNull($message->starredAt);
|
||||
}
|
||||
|
||||
/**
|
||||
* The first of the two bugs, pinned on its own: an id plMail already holds
|
||||
* must still be fetched, because being already held is the whole reason a
|
||||
* relabelled id is sent.
|
||||
*/
|
||||
public function testAnAlreadyStoredIdIsStillFetchedSoItsLabelsCanBeReRead(): void
|
||||
{
|
||||
$message = $this->seedMessage(seen: false);
|
||||
|
||||
$requests = $this->handle(['INBOX']);
|
||||
|
||||
self::assertSame(1, $requests, 'the batch was actually sent');
|
||||
|
||||
$this->em->refresh($message);
|
||||
|
||||
self::assertNotNull($message->seenAt, 'and the re-read reached the row');
|
||||
}
|
||||
|
||||
/**
|
||||
* The echo race, on Gmail. The guard lives in ThreadStatusUpdater, so it
|
||||
* covers this provider without Gmail having to implement it again.
|
||||
*/
|
||||
public function testAnUnconfirmedLocalChangeIsNotRevertedByStaleLabels(): void
|
||||
{
|
||||
$message = $this->seedMessage(seen: false);
|
||||
|
||||
$message->seenAt = new DateTimeImmutable();
|
||||
$message->flags = [MessageFlag::SEEN->value];
|
||||
$message->flagsTouchedAt = new DateTimeImmutable();
|
||||
$this->em->flush();
|
||||
|
||||
// Gmail has not been told yet, so it still reports UNREAD.
|
||||
$this->handle(['INBOX', 'UNREAD']);
|
||||
|
||||
$this->em->refresh($message);
|
||||
|
||||
self::assertNotNull($message->seenAt, 'the local change is still in flight');
|
||||
}
|
||||
|
||||
// ── helpers ───────────────────────────────────────────────────────────
|
||||
|
||||
/**
|
||||
* Run the batch handler for our one message, with Gmail reporting these
|
||||
* labelIds for it.
|
||||
*
|
||||
* GmailApiClient is final, so it is built for real against a
|
||||
* MockHttpClient — the way GmailApiSyncerHistoryTest does it — and the
|
||||
* response is a genuine multipart/mixed batch body, because that is what
|
||||
* the client parses.
|
||||
*
|
||||
* @param list<string> $labelIds
|
||||
*
|
||||
* @return int how many HTTP requests the client made
|
||||
*/
|
||||
private function handle(array $labelIds): int
|
||||
{
|
||||
$requests = 0;
|
||||
|
||||
$payload = json_encode([
|
||||
'id' => self::GMAIL_ID,
|
||||
'threadId' => 'gmail-thread-1',
|
||||
'labelIds' => $labelIds,
|
||||
'payload' => [
|
||||
'headers' => [
|
||||
['name' => 'Message-ID', 'value' => '<gmail-1@example.test>'],
|
||||
['name' => 'Subject', 'value' => 'Read elsewhere'],
|
||||
['name' => 'From', 'value' => 'sender@example.test'],
|
||||
['name' => 'To', 'value' => 'gmail@example.test'],
|
||||
],
|
||||
],
|
||||
], JSON_THROW_ON_ERROR);
|
||||
|
||||
$boundary = 'batch_plmail_test';
|
||||
|
||||
$body = "--{$boundary}\r\n"
|
||||
. "Content-Type: application/http\r\n"
|
||||
. "Content-ID: <response-{$this->contentId()}>\r\n"
|
||||
. "\r\n"
|
||||
. "HTTP/1.1 200 OK\r\n"
|
||||
. "Content-Type: application/json\r\n"
|
||||
. "\r\n"
|
||||
. $payload . "\r\n"
|
||||
. "--{$boundary}--";
|
||||
|
||||
$httpClient = new MockHttpClient(function () use ($body, $boundary, &$requests): MockResponse {
|
||||
++$requests;
|
||||
|
||||
return new MockResponse($body, [
|
||||
'response_headers' => ['content-type' => 'multipart/mixed; boundary="' . $boundary . '"'],
|
||||
]);
|
||||
});
|
||||
|
||||
$container = self::getContainer();
|
||||
|
||||
$handler = new SyncGmailMessageBatchHandler(
|
||||
$container->get('App\Repository\Mail\MessageRepository'),
|
||||
$container->get('App\Repository\Mail\AccountRepository'),
|
||||
new GmailApiClient($httpClient, $container->get('App\Service\OAuth\OAuthTokenManager')),
|
||||
$container->get('App\Service\Gmail\GmailMessageBuilder'),
|
||||
$container->get('App\Service\Gmail\GmailAddressFilter'),
|
||||
$container->get('App\Service\HarvestContactsService'),
|
||||
// The real notifier publishes to Mercure, which the test kernel has
|
||||
// no signing key for. What it announces is not what is under test.
|
||||
new SyncNotifier($this->createStub(HubInterface::class)),
|
||||
$container->get('messenger.default_bus'),
|
||||
$container->get('App\Service\Mail\PostIngestPipeline'),
|
||||
$this->em,
|
||||
$container->get('App\Jmap\State\StateManager'),
|
||||
new \Psr\Log\NullLogger(),
|
||||
$container->get('App\Service\Label\ThreadLabelSynchronizer'),
|
||||
$container->get('App\Service\Mail\ThreadStatusUpdater'),
|
||||
);
|
||||
|
||||
$handler(new SyncGmailMessageBatchMessage((int) $this->account->id, [self::GMAIL_ID]));
|
||||
|
||||
$this->em->flush();
|
||||
|
||||
return $requests;
|
||||
}
|
||||
|
||||
private function contentId(): string
|
||||
{
|
||||
return self::GMAIL_ID;
|
||||
}
|
||||
|
||||
private function seedMessage(bool $seen, bool $flagged = false): Message
|
||||
{
|
||||
$thread = new MessageThread();
|
||||
$thread->account = $this->account;
|
||||
$thread->subject = 'Read elsewhere';
|
||||
$thread->normalizedSubject = 'read elsewhere';
|
||||
$thread->threadingMethod = ThreadingMethod::SubjectFallback;
|
||||
$thread->lastMessageAt = new DateTimeImmutable('-1 hour');
|
||||
$thread->unreadCount = true === $seen ? 0 : 1;
|
||||
|
||||
$message = new Message();
|
||||
$message->account = $this->account;
|
||||
$message->thread = $thread;
|
||||
$message->subject = 'Read elsewhere';
|
||||
$message->fromAddress = 'sender@example.test';
|
||||
$message->messageId = 'gmail-1@example.test';
|
||||
$message->gmailId = self::GMAIL_ID;
|
||||
$message->receivedAt = new DateTimeImmutable('-1 hour');
|
||||
$message->sentAt = $message->receivedAt;
|
||||
$message->hasAttachments = false;
|
||||
$message->flags = [];
|
||||
$message->syncedAt = new DateTimeImmutable();
|
||||
|
||||
if (true === $seen) {
|
||||
$message->seenAt = new DateTimeImmutable('-30 minutes');
|
||||
$message->flags = [MessageFlag::SEEN->value];
|
||||
}
|
||||
|
||||
if (true === $flagged) {
|
||||
$message->starredAt = new DateTimeImmutable('-30 minutes');
|
||||
$thread->starredAt = $message->starredAt;
|
||||
$message->flags = MessageFlag::canonicalList(
|
||||
array_merge($message->flags, [MessageFlag::FLAGGED->value]),
|
||||
);
|
||||
}
|
||||
|
||||
$thread->addMessage($message);
|
||||
|
||||
$this->em->persist($thread);
|
||||
$this->em->persist($message);
|
||||
$this->em->flush();
|
||||
|
||||
return $message;
|
||||
}
|
||||
|
||||
private function seedAccount(): Account
|
||||
{
|
||||
$account = new Account();
|
||||
$account->usr = $this->user;
|
||||
$account->name = 'Gmail fixture';
|
||||
$account->email = 'gmail@example.test';
|
||||
$account->username = uniqid('gmail-', true);
|
||||
$account->imapHost = 'imap.gmail.com';
|
||||
$account->imapPort = 993;
|
||||
$account->imapEncryption = 'ssl';
|
||||
|
||||
$account->authType = AuthType::OAuth2->value;
|
||||
$account->oauthProvider = 'google';
|
||||
$account->oauthAccessToken = 'test-access-token';
|
||||
$account->oauthTokenExpiry = new DateTimeImmutable('+1 day');
|
||||
$account->isActive = true;
|
||||
|
||||
$this->em->persist($account);
|
||||
$this->em->flush();
|
||||
|
||||
$this->user->addAccount($account);
|
||||
$this->em->flush();
|
||||
|
||||
return $account;
|
||||
}
|
||||
|
||||
private function seedUser(): User
|
||||
{
|
||||
$user = new User();
|
||||
$user->email = 'gmail-read-' . uniqid('', true) . '@example.test';
|
||||
$user->nameFirst = 'Gmail';
|
||||
$user->nameLast = 'ReadState';
|
||||
$user->roles = ['ROLE_USER'];
|
||||
$user->password = 'x';
|
||||
|
||||
$this->em->persist($user);
|
||||
$this->em->flush();
|
||||
|
||||
return $user;
|
||||
}
|
||||
}
|
||||
@@ -263,6 +263,7 @@ final class GraphFolderMoveTest extends KernelTestCase
|
||||
$container->get('messenger.default_bus'),
|
||||
new \Psr\Log\NullLogger(),
|
||||
$container->get('App\Service\Mail\MessageEraser'),
|
||||
$container->get('App\Service\Mail\ThreadStatusUpdater'),
|
||||
);
|
||||
|
||||
$syncer->sync($this->account, array_keys($byFolder));
|
||||
@@ -307,6 +308,7 @@ final class GraphFolderMoveTest extends KernelTestCase
|
||||
$container->get('messenger.default_bus'),
|
||||
new \Psr\Log\NullLogger(),
|
||||
$container->get('App\Service\Mail\MessageEraser'),
|
||||
$container->get('App\Service\Mail\ThreadStatusUpdater'),
|
||||
);
|
||||
|
||||
$syncer->sync($this->account, [$folderId]);
|
||||
|
||||
@@ -0,0 +1,336 @@
|
||||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
namespace App\Tests\Service\Graph;
|
||||
|
||||
use App\Domain\Enum\Account\AuthType;
|
||||
use App\Domain\Enum\Mail\LabelRole;
|
||||
use App\Domain\Enum\Mail\MessageFlag;
|
||||
use App\Domain\Enum\Mail\ThreadingMethod;
|
||||
use App\Entity\Label\Label;
|
||||
use App\Entity\Label\LabelBinding;
|
||||
use App\Entity\Mail\Account;
|
||||
use App\Entity\Mail\Message;
|
||||
use App\Entity\Mail\MessageThread;
|
||||
use App\Entity\User\User;
|
||||
use App\Service\Graph\GraphApiSyncer;
|
||||
use App\Service\Mail\GraphApiClient;
|
||||
use DateTimeImmutable;
|
||||
use Doctrine\DBAL\Connection;
|
||||
use Doctrine\ORM\EntityManagerInterface;
|
||||
use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase;
|
||||
use Symfony\Component\HttpClient\MockHttpClient;
|
||||
use Symfony\Component\HttpClient\Response\MockResponse;
|
||||
|
||||
/**
|
||||
* Read state that arrives on an Exchange delta, and whether plMail hears it.
|
||||
*
|
||||
* It did not. GraphApiSyncer::partition() said, of a message it already had,
|
||||
* that "the only thing that can have changed cheaply is where it lives" — and
|
||||
* that was wrong in the way that matters: a delta entry carries the whole
|
||||
* message resource, isRead and flag included. So a message read in Outlook or
|
||||
* on a phone announced itself in the very payload plMail was reading, and
|
||||
* plMail attached the folder label and threw the rest away. Read state was
|
||||
* captured once at ingest by GraphMessageBuilder and never again, which is the
|
||||
* same gap plain IMAP had.
|
||||
*
|
||||
* The truth table below is the point: each row states what the delta said and
|
||||
* asserts what the row became — including the rows where the answer is "nothing
|
||||
* happened", which are the ones a careless fix breaks.
|
||||
*/
|
||||
final class GraphInboundReadStateTest extends KernelTestCase
|
||||
{
|
||||
private EntityManagerInterface $em;
|
||||
private Connection $connection;
|
||||
private User $user;
|
||||
private Account $account;
|
||||
private Label $inbox;
|
||||
|
||||
private const string INBOX_FOLDER = 'AAMkAD-inbox';
|
||||
private const string GRAPH_ID = 'AAkALgAAmessage';
|
||||
|
||||
protected function setUp(): void
|
||||
{
|
||||
self::bootKernel();
|
||||
|
||||
$container = self::getContainer();
|
||||
$this->em = $container->get(EntityManagerInterface::class);
|
||||
$this->connection = $container->get(Connection::class);
|
||||
|
||||
$this->connection->beginTransaction();
|
||||
|
||||
$this->user = $this->seedUser();
|
||||
$this->account = $this->seedAccount();
|
||||
$this->inbox = $this->seedLabel('Inbox', LabelRole::Inbox, self::INBOX_FOLDER);
|
||||
}
|
||||
|
||||
protected function tearDown(): void
|
||||
{
|
||||
if (true === $this->connection->isTransactionActive()) {
|
||||
$this->connection->rollBack();
|
||||
}
|
||||
|
||||
parent::tearDown();
|
||||
}
|
||||
|
||||
// ── the truth table ───────────────────────────────────────────────────
|
||||
|
||||
/** isRead true, stored unread → read. The reported gap. */
|
||||
public function testAMessageReadInOutlookBecomesReadHere(): void
|
||||
{
|
||||
$message = $this->seedMessage(seen: false);
|
||||
|
||||
$this->syncDelta(['isRead' => true]);
|
||||
|
||||
$this->em->refresh($message);
|
||||
|
||||
self::assertNotNull($message->seenAt);
|
||||
self::assertContains(MessageFlag::SEEN->value, $message->flags);
|
||||
}
|
||||
|
||||
/** isRead false, stored read → unread. */
|
||||
public function testAMessageMarkedUnreadInOutlookBecomesUnreadHere(): void
|
||||
{
|
||||
$message = $this->seedMessage(seen: true);
|
||||
|
||||
$this->syncDelta(['isRead' => false]);
|
||||
|
||||
$this->em->refresh($message);
|
||||
|
||||
self::assertNull($message->seenAt);
|
||||
self::assertNotContains(MessageFlag::SEEN->value, $message->flags);
|
||||
}
|
||||
|
||||
/** flagStatus flagged, stored unstarred → starred, thread included. */
|
||||
public function testAFlagSetInOutlookArrivesAsAStar(): void
|
||||
{
|
||||
$message = $this->seedMessage(seen: false);
|
||||
|
||||
$this->syncDelta(['isRead' => false, 'flag' => ['flagStatus' => 'flagged']]);
|
||||
|
||||
$this->em->refresh($message);
|
||||
|
||||
self::assertNotNull($message->starredAt);
|
||||
self::assertNotNull($message->thread->starredAt);
|
||||
}
|
||||
|
||||
/** flagStatus notFlagged, stored starred → unstarred. */
|
||||
public function testAFlagClearedInOutlookRemovesTheStar(): void
|
||||
{
|
||||
$message = $this->seedMessage(seen: false, flagged: true);
|
||||
|
||||
$this->syncDelta(['isRead' => false, 'flag' => ['flagStatus' => 'notFlagged']]);
|
||||
|
||||
$this->em->refresh($message);
|
||||
|
||||
self::assertNull($message->starredAt);
|
||||
}
|
||||
|
||||
/**
|
||||
* A delta entry with no `flag` key at all is Exchange saying nothing about
|
||||
* flagging, which is not the same as saying "not flagged". The star stays.
|
||||
*/
|
||||
public function testAnAbsentFlagKeyLeavesTheStarAlone(): void
|
||||
{
|
||||
$message = $this->seedMessage(seen: false, flagged: true);
|
||||
|
||||
$this->syncDelta(['isRead' => false]);
|
||||
|
||||
$this->em->refresh($message);
|
||||
|
||||
self::assertNotNull($message->starredAt, 'silence about the flag is not "unflagged"');
|
||||
}
|
||||
|
||||
/**
|
||||
* And an entry with no isRead is not an assertion that the message is
|
||||
* unread. Absence is not an answer — the same rule the IMAP listing works
|
||||
* to, and the rule that keeps a sparse delta from marking a mailbox unread.
|
||||
*/
|
||||
public function testAnAbsentIsReadLeavesTheReadStateAlone(): void
|
||||
{
|
||||
$message = $this->seedMessage(seen: true);
|
||||
|
||||
$this->syncDelta([]);
|
||||
|
||||
$this->em->refresh($message);
|
||||
|
||||
self::assertNotNull($message->seenAt);
|
||||
}
|
||||
|
||||
/**
|
||||
* The echo race, on Graph. The user marks a message read here, the outbound
|
||||
* ApplyGraphChangesMessage has not run, and the delta still says unread.
|
||||
* The guard lives in ThreadStatusUpdater, which is exactly why it covers
|
||||
* this provider without Graph having to implement it again.
|
||||
*/
|
||||
public function testAnUnconfirmedLocalChangeIsNotRevertedByAStaleDelta(): void
|
||||
{
|
||||
$message = $this->seedMessage(seen: false);
|
||||
|
||||
$message->seenAt = new DateTimeImmutable();
|
||||
$message->flags = [MessageFlag::SEEN->value];
|
||||
$message->flagsTouchedAt = new DateTimeImmutable();
|
||||
$this->em->flush();
|
||||
|
||||
$this->syncDelta(['isRead' => false]);
|
||||
|
||||
$this->em->refresh($message);
|
||||
|
||||
self::assertNotNull($message->seenAt, 'the local change is still in flight');
|
||||
}
|
||||
|
||||
// ── helpers ───────────────────────────────────────────────────────────
|
||||
|
||||
/**
|
||||
* Run one folder's delta reporting this message with the given extra
|
||||
* fields, the way GraphFolderMoveTest does it: a hand-built syncer over a
|
||||
* MockHttpClient, because what is under test is what the syncer writes.
|
||||
*
|
||||
* @param array<string,mixed> $extra
|
||||
*/
|
||||
private function syncDelta(array $extra): void
|
||||
{
|
||||
$delta = [
|
||||
'value' => [array_merge([
|
||||
'id' => self::GRAPH_ID,
|
||||
'parentFolderId' => self::INBOX_FOLDER,
|
||||
], $extra)],
|
||||
'@odata.deltaLink' => 'https://graph.microsoft.com/v1.0/me/mailFolders/x/messages/delta?$deltatoken=new',
|
||||
];
|
||||
|
||||
$container = self::getContainer();
|
||||
|
||||
$syncer = new GraphApiSyncer(
|
||||
new GraphApiClient(
|
||||
new MockHttpClient([new MockResponse(json_encode($delta, JSON_THROW_ON_ERROR), [
|
||||
'response_headers' => ['content-type' => 'application/json'],
|
||||
])]),
|
||||
$container->get('App\Service\OAuth\OAuthTokenManager'),
|
||||
),
|
||||
$container->get('App\Service\Graph\GraphFolderResolver'),
|
||||
$container->get('App\Service\Graph\GraphLabelPolicy'),
|
||||
$container->get('App\Repository\Mail\MessageRepository'),
|
||||
$this->em,
|
||||
$container->get('App\Jmap\State\StateManager'),
|
||||
$container->get('messenger.default_bus'),
|
||||
new \Psr\Log\NullLogger(),
|
||||
$container->get('App\Service\Mail\MessageEraser'),
|
||||
$container->get('App\Service\Mail\ThreadStatusUpdater'),
|
||||
);
|
||||
|
||||
$syncer->sync($this->account, [self::INBOX_FOLDER]);
|
||||
|
||||
$this->em->flush();
|
||||
}
|
||||
|
||||
private function seedMessage(bool $seen, bool $flagged = false): Message
|
||||
{
|
||||
$thread = new MessageThread();
|
||||
$thread->account = $this->account;
|
||||
$thread->subject = 'Read elsewhere';
|
||||
$thread->normalizedSubject = 'read elsewhere';
|
||||
$thread->threadingMethod = ThreadingMethod::SubjectFallback;
|
||||
$thread->lastMessageAt = new DateTimeImmutable('-1 hour');
|
||||
$thread->unreadCount = true === $seen ? 0 : 1;
|
||||
|
||||
$message = new Message();
|
||||
$message->account = $this->account;
|
||||
$message->thread = $thread;
|
||||
$message->subject = 'Read elsewhere';
|
||||
$message->fromAddress = 'sender@example.test';
|
||||
$message->graphId = self::GRAPH_ID;
|
||||
$message->receivedAt = new DateTimeImmutable('-1 hour');
|
||||
$message->sentAt = $message->receivedAt;
|
||||
$message->hasAttachments = false;
|
||||
$message->flags = [];
|
||||
$message->syncedAt = new DateTimeImmutable();
|
||||
|
||||
if (true === $seen) {
|
||||
$message->seenAt = new DateTimeImmutable('-30 minutes');
|
||||
$message->flags = [MessageFlag::SEEN->value];
|
||||
}
|
||||
|
||||
if (true === $flagged) {
|
||||
$message->starredAt = new DateTimeImmutable('-30 minutes');
|
||||
$thread->starredAt = $message->starredAt;
|
||||
$message->flags = MessageFlag::canonicalList(
|
||||
array_merge($message->flags, [MessageFlag::FLAGGED->value]),
|
||||
);
|
||||
}
|
||||
|
||||
$message->addLabel($this->inbox);
|
||||
|
||||
$thread->addMessage($message);
|
||||
|
||||
$this->em->persist($thread);
|
||||
$this->em->persist($message);
|
||||
$this->em->flush();
|
||||
|
||||
return $message;
|
||||
}
|
||||
|
||||
private function seedLabel(string $name, ?LabelRole $role, ?string $graphFolderId): Label
|
||||
{
|
||||
$label = new Label();
|
||||
$label->usr = $this->user;
|
||||
$label->name = $name;
|
||||
$label->role = $role;
|
||||
$label->isVisible = true;
|
||||
|
||||
$this->em->persist($label);
|
||||
$this->em->flush();
|
||||
|
||||
if (null !== $graphFolderId) {
|
||||
$binding = new LabelBinding();
|
||||
$binding->label = $label;
|
||||
$binding->account = $this->account;
|
||||
$binding->graphFolderId = $graphFolderId;
|
||||
|
||||
$label->addBinding($binding);
|
||||
|
||||
$this->em->persist($binding);
|
||||
$this->em->flush();
|
||||
}
|
||||
|
||||
return $label;
|
||||
}
|
||||
|
||||
private function seedAccount(): Account
|
||||
{
|
||||
$account = new Account();
|
||||
$account->usr = $this->user;
|
||||
$account->name = 'Graph fixture';
|
||||
$account->email = 'graph@example.test';
|
||||
$account->username = uniqid('graph-', true);
|
||||
$account->imapHost = 'outlook.office365.com';
|
||||
$account->imapPort = 993;
|
||||
$account->imapEncryption = 'ssl';
|
||||
|
||||
$account->authType = AuthType::OAuth2->value;
|
||||
$account->oauthProvider = 'microsoft';
|
||||
$account->oauthAccessToken = 'test-access-token';
|
||||
$account->oauthTokenExpiry = new DateTimeImmutable('+1 day');
|
||||
$account->isActive = true;
|
||||
|
||||
$this->em->persist($account);
|
||||
$this->em->flush();
|
||||
|
||||
return $account;
|
||||
}
|
||||
|
||||
private function seedUser(): User
|
||||
{
|
||||
$user = new User();
|
||||
$user->email = 'graph-read-' . uniqid('', true) . '@example.test';
|
||||
$user->nameFirst = 'Graph';
|
||||
$user->nameLast = 'ReadState';
|
||||
$user->roles = ['ROLE_USER'];
|
||||
$user->password = 'x';
|
||||
|
||||
$this->em->persist($user);
|
||||
$this->em->flush();
|
||||
|
||||
return $user;
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,510 @@
|
||||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
namespace App\Tests\Service\Imap;
|
||||
|
||||
use App\Domain\Enum\Mail\LabelRole;
|
||||
use App\Domain\Enum\Mail\MailboxSpecialUse;
|
||||
use App\Domain\Enum\Mail\MessageFlag;
|
||||
use App\Domain\Enum\Mail\ThreadingMethod;
|
||||
use App\Domain\Helper\MessageIdHelper;
|
||||
use App\Entity\Mail\Account;
|
||||
use App\Entity\Mail\Mailbox;
|
||||
use App\Entity\Mail\Message;
|
||||
use App\Entity\Mail\MessageThread;
|
||||
use App\Entity\User\User;
|
||||
use App\Service\Imap\VanishedMessageReconciler;
|
||||
use App\Service\Label\LabelResolver;
|
||||
use App\Service\Mail\ThreadStatusUpdater;
|
||||
use DateTimeImmutable;
|
||||
use Doctrine\DBAL\Connection;
|
||||
use Doctrine\ORM\EntityManagerInterface;
|
||||
use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase;
|
||||
|
||||
/**
|
||||
* Flags the server changed, and which of them plMail is allowed to believe.
|
||||
*
|
||||
* plMail never re-read the flags of a message it had already stored: they were
|
||||
* captured once at ingest and never again, because incremental sync asks for
|
||||
* `lastSeenUid+1:*` and never looks at an old UID twice. Mail read on a phone
|
||||
* stayed unread here forever, and a star set in another client never arrived.
|
||||
*
|
||||
* These tests state what the server said — the same way RemoteDeletionSyncTest
|
||||
* states what a listing produced — and assert what plMail concluded. The
|
||||
* important half is the echo race, and it is pinned from BOTH sides: a local
|
||||
* change must survive a server read that predates it, and a genuine remote
|
||||
* change must still land. A guard that passes only the first of those is a
|
||||
* guard that has switched inbound flag sync off.
|
||||
*/
|
||||
final class InboundFlagSyncTest extends KernelTestCase
|
||||
{
|
||||
private EntityManagerInterface $em;
|
||||
private Connection $connection;
|
||||
private VanishedMessageReconciler $vanished;
|
||||
private LabelResolver $labels;
|
||||
|
||||
private User $user;
|
||||
private Account $account;
|
||||
private Mailbox $inbox;
|
||||
|
||||
protected function setUp(): void
|
||||
{
|
||||
self::bootKernel();
|
||||
|
||||
$container = self::getContainer();
|
||||
|
||||
$this->em = $container->get(EntityManagerInterface::class);
|
||||
$this->connection = $container->get(Connection::class);
|
||||
$this->vanished = $container->get(VanishedMessageReconciler::class);
|
||||
$this->labels = $container->get(LabelResolver::class);
|
||||
|
||||
$this->connection->beginTransaction();
|
||||
|
||||
$this->seed();
|
||||
}
|
||||
|
||||
protected function tearDown(): void
|
||||
{
|
||||
if (true === $this->connection->isTransactionActive()) {
|
||||
$this->connection->rollBack();
|
||||
}
|
||||
|
||||
parent::tearDown();
|
||||
}
|
||||
|
||||
// ── the gap ───────────────────────────────────────────────────────────
|
||||
|
||||
/**
|
||||
* The reported behaviour. A message stored unread, read afterwards in
|
||||
* another client, is read here once the folder is listed again.
|
||||
*/
|
||||
public function testAMessageReadOnTheServerBecomesReadHere(): void
|
||||
{
|
||||
$message = $this->incoming('Rechnung', 6);
|
||||
|
||||
self::assertNull($message->seenAt, 'it arrived unread');
|
||||
|
||||
$this->sweep([6 => ['\\Seen']], now: '2026-08-11 09:00:00');
|
||||
|
||||
self::assertNotNull($message->seenAt, 'the server says it has been read, and that is news');
|
||||
self::assertContains(MessageFlag::SEEN->value, $message->flags);
|
||||
}
|
||||
|
||||
/**
|
||||
* And the other direction, which is the one a badge notices: a message
|
||||
* marked unread elsewhere goes back to unread here.
|
||||
*/
|
||||
public function testAMessageMarkedUnreadOnTheServerBecomesUnreadHere(): void
|
||||
{
|
||||
$message = $this->incoming('Rechnung', 6, seen: true);
|
||||
|
||||
$this->sweep([6 => []], now: '2026-08-11 09:00:00');
|
||||
|
||||
self::assertNull($message->seenAt, 'the server no longer has \\Seen on it');
|
||||
self::assertNotContains(MessageFlag::SEEN->value, $message->flags);
|
||||
}
|
||||
|
||||
/**
|
||||
* A star set in another client arrives, on the message and on the thread —
|
||||
* the thread is what the list renders, which is the rule star() works to
|
||||
* from the outbound side.
|
||||
*/
|
||||
public function testAStarSetOnTheServerArrivesOnTheMessageAndTheThread(): void
|
||||
{
|
||||
$message = $this->incoming('Rechnung', 6);
|
||||
|
||||
$this->sweep([6 => ['\\Flagged']], now: '2026-08-11 09:00:00');
|
||||
|
||||
self::assertNotNull($message->starredAt);
|
||||
self::assertNotNull($message->thread->starredAt, 'a thread is starred as a whole');
|
||||
}
|
||||
|
||||
public function testAStarRemovedOnTheServerComesOffHere(): void
|
||||
{
|
||||
$message = $this->incoming('Rechnung', 6, flagged: true);
|
||||
|
||||
$this->sweep([6 => []], now: '2026-08-11 09:00:00');
|
||||
|
||||
self::assertNull($message->starredAt);
|
||||
self::assertNull($message->thread->starredAt);
|
||||
}
|
||||
|
||||
/**
|
||||
* The unread badge is a thread counter, and it is recounted from the
|
||||
* thread's own messages rather than adjusted — a listing can change several
|
||||
* messages of one thread at once and in both directions.
|
||||
*/
|
||||
public function testTheThreadsUnreadCountIsRecountedFromItsMessages(): void
|
||||
{
|
||||
$thread = $this->thread('Rechnung');
|
||||
|
||||
$first = $this->rowIn(MessageIdHelper::mint('kunde@example.test'), 6, 'Rechnung', $thread);
|
||||
$second = $this->rowIn(MessageIdHelper::mint('kunde@example.test'), 7, 'Rechnung', $thread);
|
||||
|
||||
$thread->unreadCount = 2;
|
||||
$this->em->flush();
|
||||
|
||||
// One read on the server, one still unread.
|
||||
$this->sweep([6 => ['\\Seen'], 7 => []], now: '2026-08-11 09:00:00');
|
||||
|
||||
self::assertNotNull($first->seenAt);
|
||||
self::assertNull($second->seenAt);
|
||||
self::assertSame(1, $thread->unreadCount, 'recounted, not decremented');
|
||||
}
|
||||
|
||||
/**
|
||||
* \Answered and \Draft have no column of their own — they live in the
|
||||
* flags mirror, which EmailMapper reads to answer the JMAP $answered and
|
||||
* $draft keywords. A refresh that only wrote the two with columns would
|
||||
* leave those saying what was true at ingest.
|
||||
*/
|
||||
public function testFlagsWithoutAColumnStillReachTheMirror(): void
|
||||
{
|
||||
$message = $this->incoming('Rechnung', 6);
|
||||
|
||||
$this->sweep([6 => ['\\Seen', '\\Answered']], now: '2026-08-11 09:00:00');
|
||||
|
||||
self::assertContains(MessageFlag::ANSWERED->value, $message->flags);
|
||||
self::assertContains(MessageFlag::SEEN->value, $message->flags);
|
||||
}
|
||||
|
||||
/**
|
||||
* Servers disagree about whether the backslash on a system flag survives
|
||||
* being parsed — webklex hands back `Seen` from some and `\Seen` from
|
||||
* others — and a mirror captured from one must not read as changed against
|
||||
* a listing from the other on every pass forever.
|
||||
*
|
||||
* The row is left exactly as it is, which is the assertion: a difference
|
||||
* that is only punctuation is not a change, so there is no write, no JMAP
|
||||
* change-log row, and no thread recount. Without this, the first pass over
|
||||
* a fifty-thousand-message folder would rewrite every row in it and wake
|
||||
* every connected client to say nothing.
|
||||
*/
|
||||
public function testTheSpellingOfASystemFlagIsNotAChange(): void
|
||||
{
|
||||
$message = $this->incoming('Rechnung', 6, seen: true);
|
||||
|
||||
$message->flags = ['Seen'];
|
||||
$this->em->flush();
|
||||
|
||||
$this->sweep([6 => ['\\Seen']], now: '2026-08-11 09:00:00');
|
||||
|
||||
self::assertSame(['Seen'], $message->flags, 'not rewritten, because nothing about it differs');
|
||||
self::assertNotNull($message->seenAt, 'and nothing about the read state moved');
|
||||
}
|
||||
|
||||
/**
|
||||
* The same rule the other way: a real change is still a change when the
|
||||
* spelling differs too, and it is written in the canonical form.
|
||||
*/
|
||||
public function testARealChangeIsWrittenCanonically(): void
|
||||
{
|
||||
$message = $this->incoming('Rechnung', 6);
|
||||
|
||||
$message->flags = ['Answered'];
|
||||
$this->em->flush();
|
||||
|
||||
$this->sweep([6 => ['Seen', 'Answered']], now: '2026-08-11 09:00:00');
|
||||
|
||||
self::assertSame(
|
||||
[MessageFlag::ANSWERED->value, MessageFlag::SEEN->value],
|
||||
$message->flags,
|
||||
'canonical and sorted, so the next comparison is cheap',
|
||||
);
|
||||
self::assertNotNull($message->seenAt);
|
||||
}
|
||||
|
||||
/**
|
||||
* A UID the listing did not describe is not a message with no flags. The
|
||||
* sweep is what draws conclusions from absence, with the safety rails.
|
||||
*/
|
||||
public function testAUidTheListingDidNotDescribeIsLeftAlone(): void
|
||||
{
|
||||
$message = $this->incoming('Rechnung', 6, seen: true);
|
||||
|
||||
// The listing produces the UID — so the sweep does not mark it gone —
|
||||
// but says nothing about its flags.
|
||||
$this->vanished->apply(
|
||||
$this->inbox,
|
||||
['uidValidity' => 1, 'uids' => [6 => true], 'flags' => [], 'readAt' => new DateTimeImmutable('2026-08-11 09:00:00')],
|
||||
new DateTimeImmutable('2026-08-11 09:00:00'),
|
||||
);
|
||||
|
||||
self::assertNotNull($message->seenAt, 'silence is not the answer "unread"');
|
||||
}
|
||||
|
||||
// ── the echo race, from both sides ────────────────────────────────────
|
||||
|
||||
/**
|
||||
* THE RACE. The user marks a message read; the outbound job is queued and
|
||||
* has not run; the flag pass reads the server's still-old answer.
|
||||
*
|
||||
* Believing it would clear seenAt — which the propagator would see as a
|
||||
* fresh local change and queue a job for, which the next pass would revert
|
||||
* again. The local change has to survive.
|
||||
*/
|
||||
public function testALocalChangeSurvivesAServerReadThatPredatesIt(): void
|
||||
{
|
||||
$message = $this->incoming('Rechnung', 6);
|
||||
|
||||
// The user marks it read. The row changes first; the job is queued.
|
||||
$message->seenAt = new DateTimeImmutable('2026-08-11 08:59:00');
|
||||
$message->flags = [MessageFlag::SEEN->value];
|
||||
$message->flagsTouchedAt = new DateTimeImmutable('2026-08-11 08:59:00');
|
||||
$this->em->flush();
|
||||
|
||||
// The server has not been told yet, so it still says unread.
|
||||
$this->sweep([6 => []], now: '2026-08-11 09:00:00');
|
||||
|
||||
self::assertNotNull($message->seenAt, 'the user read it; a stale answer does not undo that');
|
||||
self::assertNotNull($message->flagsTouchedAt, 'and it is still waiting to be confirmed');
|
||||
}
|
||||
|
||||
/**
|
||||
* The other side of the same guard, and the one that proves it is a guard
|
||||
* rather than an off switch: with nothing in flight, the server's answer is
|
||||
* the truth and lands.
|
||||
*/
|
||||
public function testAGenuineRemoteChangeStillLandsWhenNothingIsInFlight(): void
|
||||
{
|
||||
$message = $this->incoming('Rechnung', 6);
|
||||
|
||||
self::assertNull($message->flagsTouchedAt, 'nothing queued');
|
||||
|
||||
$this->sweep([6 => ['\\Seen']], now: '2026-08-11 09:00:00');
|
||||
|
||||
self::assertNotNull($message->seenAt, 'no local change to protect, so the server wins');
|
||||
}
|
||||
|
||||
/**
|
||||
* The guard is released by confirmation, not by time: once the outbound job
|
||||
* has told the server, the next listing is authoritative again — including
|
||||
* when it reports something new.
|
||||
*/
|
||||
public function testOnceConfirmedTheServerIsAuthoritativeAgain(): void
|
||||
{
|
||||
$message = $this->incoming('Rechnung', 6);
|
||||
|
||||
$message->seenAt = new DateTimeImmutable('2026-08-11 08:59:00');
|
||||
$message->flags = [MessageFlag::SEEN->value];
|
||||
$message->flagsTouchedAt = new DateTimeImmutable('2026-08-11 08:59:00');
|
||||
$this->em->flush();
|
||||
|
||||
// ApplyImapFlagsHandler applied it and cleared the mark.
|
||||
$message->flagsTouchedAt = null;
|
||||
$this->em->flush();
|
||||
|
||||
// Now somebody marks it unread on another client.
|
||||
$this->sweep([6 => []], now: '2026-08-11 09:00:00');
|
||||
|
||||
self::assertNull($message->seenAt, 'a confirmed row takes the server\'s word again');
|
||||
}
|
||||
|
||||
/**
|
||||
* The guard has to expire, or it is the same bug in a smaller box: a job
|
||||
* lost for good would freeze that row's flags against the server for the
|
||||
* rest of its life.
|
||||
*/
|
||||
public function testAnUnconfirmedChangeStopsWinningOnceItIsPlainlyLost(): void
|
||||
{
|
||||
$message = $this->incoming('Rechnung', 6);
|
||||
|
||||
$message->seenAt = new DateTimeImmutable('2026-08-11 06:00:00');
|
||||
$message->flags = [MessageFlag::SEEN->value];
|
||||
|
||||
// Queued hours ago and never confirmed — well past the grace window.
|
||||
$message->flagsTouchedAt = new DateTimeImmutable('2026-08-11 06:00:00');
|
||||
$this->em->flush();
|
||||
|
||||
$this->sweep([6 => []], now: '2026-08-11 09:00:00');
|
||||
|
||||
self::assertNull($message->seenAt, 'nothing is carrying that change any more');
|
||||
self::assertNull($message->flagsTouchedAt, 'and the mark goes, or it would expire again forever');
|
||||
}
|
||||
|
||||
/**
|
||||
* A row with a change in flight is left alone ENTIRELY rather than merged
|
||||
* flag by flag. A message whose \Seen is queued may have had its \Flagged
|
||||
* changed remotely meanwhile, and a flag list cannot tell the two apart —
|
||||
* so the pass does not guess.
|
||||
*/
|
||||
public function testARowInFlightIsSkippedWholeRatherThanMerged(): void
|
||||
{
|
||||
$message = $this->incoming('Rechnung', 6);
|
||||
|
||||
$message->seenAt = new DateTimeImmutable('2026-08-11 08:59:00');
|
||||
$message->flags = [MessageFlag::SEEN->value];
|
||||
$message->flagsTouchedAt = new DateTimeImmutable('2026-08-11 08:59:00');
|
||||
$this->em->flush();
|
||||
|
||||
// The server says unread AND starred. The star is genuinely new.
|
||||
$this->sweep([6 => ['\\Flagged']], now: '2026-08-11 09:00:00');
|
||||
|
||||
self::assertNotNull($message->seenAt, 'the local read state is protected');
|
||||
self::assertNull($message->starredAt, 'and the star waits for the next pass rather than being guessed at');
|
||||
}
|
||||
|
||||
// ── the guard is one rule, not one per provider ───────────────────────
|
||||
|
||||
/**
|
||||
* The guard lives in ThreadStatusUpdater rather than in each provider's
|
||||
* reconciler, so Gmail's labels and Graph's isRead are covered by the same
|
||||
* rule. Asserted directly, because the alternative is discovering it is
|
||||
* missing on the provider nobody wrote a test for.
|
||||
*/
|
||||
public function testTheGuardIsEnforcedWhereEveryProviderLands(): void
|
||||
{
|
||||
$message = $this->incoming('Rechnung', 6);
|
||||
|
||||
$message->seenAt = new DateTimeImmutable('2026-08-11 08:59:00');
|
||||
$message->flagsTouchedAt = new DateTimeImmutable('2026-08-11 08:59:00');
|
||||
$this->em->flush();
|
||||
|
||||
$status = self::getContainer()->get(ThreadStatusUpdater::class);
|
||||
|
||||
$changed = $status->applyRemoteFlags(
|
||||
[new \App\Domain\DTO\Mail\RemoteFlagState($message, false, false)],
|
||||
new DateTimeImmutable('2026-08-11 09:00:00'),
|
||||
);
|
||||
|
||||
self::assertSame(0, $changed, 'nothing applied while a change is in flight');
|
||||
self::assertNotNull($message->seenAt);
|
||||
}
|
||||
|
||||
// ── helpers ───────────────────────────────────────────────────────────
|
||||
|
||||
/**
|
||||
* Run one folder listing that names these UIDs and their flags — the shape
|
||||
* ImapFolderListing::read() hands back from a single `UID FETCH 1:*
|
||||
* (FLAGS)`, which is where presence and flags become one answer.
|
||||
*
|
||||
* @param array<int,list<string>> $flagsByUid
|
||||
*/
|
||||
private function sweep(array $flagsByUid, string $now): void
|
||||
{
|
||||
$uids = [];
|
||||
|
||||
foreach (array_keys($flagsByUid) as $uid) {
|
||||
$uids[$uid] = true;
|
||||
}
|
||||
|
||||
$this->vanished->apply(
|
||||
$this->inbox,
|
||||
[
|
||||
'uidValidity' => 1,
|
||||
'uids' => $uids,
|
||||
'flags' => $flagsByUid,
|
||||
'readAt' => new DateTimeImmutable($now),
|
||||
],
|
||||
new DateTimeImmutable($now),
|
||||
);
|
||||
}
|
||||
|
||||
private function incoming(string $subject, int $uid, bool $seen = false, bool $flagged = false): Message
|
||||
{
|
||||
$thread = $this->thread($subject);
|
||||
$message = $this->rowIn(MessageIdHelper::mint('kunde@example.test'), $uid, $subject, $thread);
|
||||
|
||||
if (true === $seen) {
|
||||
$message->seenAt = new DateTimeImmutable('2026-08-10 12:00:00');
|
||||
$message->flags = [MessageFlag::SEEN->value];
|
||||
}
|
||||
|
||||
if (true === $flagged) {
|
||||
$message->starredAt = new DateTimeImmutable('2026-08-10 12:00:00');
|
||||
$thread->starredAt = new DateTimeImmutable('2026-08-10 12:00:00');
|
||||
$message->flags = MessageFlag::canonicalList(
|
||||
array_merge($message->flags, [MessageFlag::FLAGGED->value]),
|
||||
);
|
||||
}
|
||||
|
||||
$thread->unreadCount = null === $message->seenAt ? 1 : 0;
|
||||
|
||||
$this->em->flush();
|
||||
|
||||
return $message;
|
||||
}
|
||||
|
||||
private function thread(string $subject): MessageThread
|
||||
{
|
||||
$thread = new MessageThread();
|
||||
$thread->account = $this->account;
|
||||
$thread->subject = $subject;
|
||||
$thread->normalizedSubject = mb_strtolower($subject);
|
||||
$thread->threadingMethod = ThreadingMethod::References;
|
||||
$thread->messageCount = 0;
|
||||
$thread->unreadCount = 0;
|
||||
$thread->attachmentCount = 0;
|
||||
|
||||
$this->em->persist($thread);
|
||||
|
||||
return $thread;
|
||||
}
|
||||
|
||||
private function rowIn(string $messageId, int $uid, string $subject, MessageThread $thread): Message
|
||||
{
|
||||
$message = new Message();
|
||||
$message->account = $this->account;
|
||||
$message->mailbox = $this->inbox;
|
||||
$message->imapUid = $uid;
|
||||
$message->messageId = $messageId;
|
||||
$message->subject = $subject;
|
||||
$message->fromAddress = 'kunde@example.test';
|
||||
$message->bodyText = $subject;
|
||||
$message->hasAttachments = false;
|
||||
$message->receivedAt = new DateTimeImmutable('2026-08-10 09:00:00');
|
||||
$message->sentAt = new DateTimeImmutable('2026-08-10 09:00:00');
|
||||
$message->thread = $thread;
|
||||
$message->flags = [];
|
||||
|
||||
$thread->addMessage($message);
|
||||
|
||||
$this->em->persist($message);
|
||||
|
||||
return $message;
|
||||
}
|
||||
|
||||
private function seed(): void
|
||||
{
|
||||
$this->user = new User();
|
||||
$this->user->email = 'flags-' . uniqid('', true) . '@example.test';
|
||||
$this->user->nameFirst = 'Inbound';
|
||||
$this->user->nameLast = 'Flags';
|
||||
$this->user->roles = ['ROLE_USER'];
|
||||
$this->user->password = 'x';
|
||||
$this->em->persist($this->user);
|
||||
|
||||
$this->account = new Account();
|
||||
$this->account->usr = $this->user;
|
||||
$this->account->email = 'flags-fixture@example.test';
|
||||
$this->account->username = 'flags-fixture@example.test';
|
||||
$this->account->imapHost = 'localhost';
|
||||
$this->account->imapPort = 993;
|
||||
$this->account->imapEncryption = 'ssl';
|
||||
$this->account->smtpHost = 'localhost';
|
||||
$this->account->smtpPort = 587;
|
||||
$this->account->smtpEncryption = 'starttls';
|
||||
$this->account->password = 'x';
|
||||
$this->account->authType = 'password';
|
||||
$this->account->isActive = true;
|
||||
$this->em->persist($this->account);
|
||||
|
||||
$this->inbox = new Mailbox();
|
||||
$this->inbox->account = $this->account;
|
||||
$this->inbox->name = 'INBOX';
|
||||
$this->inbox->fullPath = 'INBOX';
|
||||
$this->inbox->specialUse = MailboxSpecialUse::INBOX;
|
||||
$this->inbox->isSyncEnabled = true;
|
||||
$this->inbox->isIdleEnabled = false;
|
||||
$this->em->persist($this->inbox);
|
||||
|
||||
$this->em->flush();
|
||||
|
||||
$this->user->addAccount($this->account);
|
||||
|
||||
$this->labels->bindMailbox($this->labels->systemLabel(LabelRole::Inbox, $this->account), $this->inbox);
|
||||
|
||||
$this->em->flush();
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user