The installed app says New mail only when mail arrived (#162)

The in-app Web Push device is now told only when mail lands in the Inbox of a mailbox that has finished importing, and Outlook stops recording folder moves for messages that did not move. JMAP clients and FCM phones keep every StateChange. Migration: jmap_push_subscription.arrivals_only, set once for in-app devices.

Takes over #112.

Co-authored-by: Flouss <15911342+Flouss@users.noreply.github.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
authored and GitHub committed 2026-10-15 15:12:06 +02:00
1 parent 55a6d67a48
commit b940bd3a31
22 files changed
+1090 -7

No files matched your search

+22
View File
@@ -8,6 +8,28 @@ The published image tags: `latest` follows the most recent release below,
## Unreleased
**An installed plMail no longer says "New mail" when nothing arrived.** The notification on a phone
or browser was sent for every change to a message — opening a conversation, marking it read,
starring, archiving, a spam move, a copy of something you sent — and again a few minutes later when
Outlook reported your own reading back, and it always read "New mail". A device switched on under
Settings → Notifications is now told when mail arrives in the Inbox of a mailbox that has finished
importing, and about nothing else. Spam, drafts, sent copies and an import in progress no longer
notify, and neither does a message whose arrival time plMail cannot work out from its headers. Reminders from Calendar alerts and the update notice for administrators
are separate pushes and unchanged. Apps that register through JMAP, and an Android phone through
Firebase, are still told about every change: they ask what moved. **Migration:** one boolean column
on `jmap_push_subscription`, false by default; devices registered through the in-app toggle (an id
starting `web-`) are switched to the new behaviour once, so nobody has to turn notifications off and
on. Reversible.
[#162](https://github.com/karatektus/pl_mail/pull/162), by @Flouss.
**Outlook mailboxes stop reporting moves that did not happen.** Microsoft sends a message again
whenever anything about it changes — plMail marking it read is enough — and each of those was
written to the change log as a move into the folder the message was already in. Only a message
whose folder actually changed is recorded now. The effect runs one way: a JMAP client, and an
Android phone through Firebase, are told about fewer Email and Thread changes from an Outlook
mailbox, and none that describe nothing. No migration.
[#162](https://github.com/karatektus/pl_mail/pull/162), by @Flouss.
**Switching category tabs no longer adds an entry to the browser's history.** A tab switch, in the
inbox or on an account's page, replaces the current entry, as Gmail does, so the browser's Back
leaves the inbox instead of stepping back through every tab you looked at. The address still follows
+15 -1
View File
@@ -1,4 +1,4 @@
<!-- translated-from: features/clients.md sha1:56eb1e2bea02c9c442a076466c6e8b19c5fe6ce9 -->
<!-- translated-from: features/clients.md sha1:a74d79f0110b60eefe51388f2743ee644d27ec4f -->
# Andere Clients
@@ -118,6 +118,20 @@ Sich aus demselben Browser erneut anzumelden ersetzt dessen Registrierung, statt
anzuhäufen — Browser rotieren die Endpunkt-URL, und plMail schlüsselt nach einer Id, die der
Browser behält.
**Worüber es benachrichtigt.** Ein hier registriertes Gerät erfährt, wenn Post **im Posteingang**
eines Postfachs **eintrifft**, dessen Import abgeschlossen ist, und sonst von nichts: nicht vom
Lesen, Markieren, Archivieren oder Verschieben, nicht von Spam, nicht von einem Entwurf oder einer
Kopie von etwas, das du gesendet hast, und nicht von Post, die gerade importiert wird. Die
Benachrichtigung sagt nur „New mail“; Absender und Betreff trägt sie nicht. Eine Nachricht, deren
Ankunftszeit plMail aus ihren Headern nicht ermitteln kann, wird nicht gemeldet.
Zwei andere Arten von Push gehen an dieselben Geräte und sind nicht betroffen: eine Erinnerung aus
den [Kalender-Erinnerungen](calendar-alerts.md) und der Hinweis, den ein Administrator bekommt, wenn
eine neuere plMail-Version verfügbar ist. Apps, die sich über JMAP registrieren
(`PushSubscription/set`), und ein Android-Telefon über Firebase sind ebenfalls anders: Sie erfahren
von jeder Änderung, weil sie nachfragen, was sich bewegt hat, und selbst entscheiden, was sie
anzeigen.
**Registrierte Geräte** listet unter dem Schalter jedes Gerät des Kontos auf, nicht nur dieses:
die Handy-App, das Tablet, den anderen Laptop. Jede Zeile sagt, welchen Transportweg das Gerät
benutzt (Web Push oder Firebase), ob der Bestätigungs-Handshake abgeschlossen ist, und was beim
+12
View File
@@ -106,6 +106,18 @@ endpoint provably reaches this user's device.
Re-subscribing from the same browser replaces its registration rather than piling up dead endpoints
— browsers rotate the endpoint URL, and plMail keys on an id the browser keeps.
**What it notifies about.** A device registered here is told when mail **arrives in the Inbox** of a
mailbox that has finished importing, and about nothing else: not reading, starring, archiving or
moving something, not spam, not a draft or a copy of something you sent, and not mail being
imported. The notification says only "New mail"; it carries no sender or subject. A message whose
arrival time plMail cannot work out from its headers is not announced.
Two other kinds of push go to the same devices and are not affected: a reminder from
[Calendar alerts](calendar-alerts.md), and the notice an administrator gets when a newer plMail is
available. Apps that register through JMAP (`PushSubscription/set`), and an Android phone through
Firebase, are different too: they are told about every change, because they ask what moved and
decide for themselves what to show.
**Registered devices** lists every device on the account below the toggle, not just this one: the
phone app, the tablet, the other laptop. Each line says which transport it uses (Web Push or
Firebase), whether the verification handshake completed, and what happened the last time something
+55
View File
@@ -0,0 +1,55 @@
<?php
declare(strict_types=1);
namespace DoctrineMigrations;
use Doctrine\DBAL\Schema\Schema;
use Doctrine\Migrations\AbstractMigration;
/**
* Which Web Push rows want new-mail pushes only: jmap_push_subscription gains
* arrivals_only, see App\Entity\User\PushSubscription::$arrivalsOnly.
*
* The column is NOT NULL with a default of false, in the schema as well as in
* the entity, so every row that exists is a full-stream row until it is told
* otherwise. That is what every row was before this feature.
*
* The backfill is the one line that is not schema. The in-app toggle
* (assets/controllers/settings/web_push_controller.js, _deviceClientId()) mints
* its device ids as `web-<uuid>`, which is the only thing that identifies a
* browser/PWA row today: nothing else records that the row came from the app's
* own registration rather than from a JMAP client. Without the backfill, a
* device that registered before this change would keep receiving every
* StateChange until somebody switched notifications off and on, and the one
* device this feature exists for would look unfixed. The UPDATE narrows those
* rows once, here, and the device does not have to do anything.
*
* It is a one-time migration, not a rule. Dispatch never matches on the `web-`
* prefix; it reads the flag. A row created after this runs gets the flag from
* WebPushController, not from its id, so the prefix is not something the code
* may come to depend on.
*
* Reversible. Going down drops the column, which returns every row to the full
* stream; a device that was narrowed registers again to be narrowed, and there
* is no other side effect.
*/
final class Version20261014100000 extends AbstractMigration
{
public function getDescription(): string
{
return 'Narrow in-app Web Push devices to new-mail pushes: jmap_push_subscription.arrivals_only';
}
public function up(Schema $schema): void
{
$this->addSql('ALTER TABLE jmap_push_subscription ADD arrivals_only BOOLEAN DEFAULT false NOT NULL');
$this->addSql("UPDATE jmap_push_subscription SET arrivals_only = TRUE WHERE transport = 'webpush' AND device_client_id LIKE 'web-%'");
}
public function down(Schema $schema): void
{
$this->addSql('ALTER TABLE jmap_push_subscription DROP arrivals_only');
}
}
@@ -103,6 +103,12 @@ final class WebPushController extends AbstractController
$subscription->pointAt($endpoint);
}
// Both branches above, deliberately: a browser that re-subscribes
// after an endpoint rotation must come back narrowed too. This is the
// only place that narrows a row. A JMAP client registering through
// PushSubscription/set is not marked and keeps the full stream.
$subscription->markArrivalsOnly();
$subscription->p256dh = $p256dh;
$subscription->auth = $auth;
$this->em->flush();
+29
View File
@@ -125,6 +125,25 @@ class PushSubscription
#[ORM\Column]
public int $failureCount = 0;
/**
* Told only when mail ARRIVED, not on every StateChange.
*
* Set by the in-app browser/PWA device's own registration (WebPushController)
* and by nothing else. That device wants "new mail" and no more: a
* StateChange says that an Email changed, which a read, a flag or a sent
* copy also does, and the installed app turned each one into a notification.
* JMAP clients and FCM phones keep the full stream.
*
* Deliberately not part of the JMAP vocabulary: PushSubscription/get does not
* return it and PushSubscription/set cannot set it, so a client cannot opt
* itself out of the stream or into the filter.
*
* Defaulted in the column as well as here, so a row written by anything else
* is a full-stream one, which is the behaviour every row had before this.
*/
#[ORM\Column(name: 'arrivals_only', options: ['default' => false])]
public private(set) bool $arrivalsOnly = false;
private function __construct(User $usr, string $deviceClientId, PushTransport $transport)
{
$this->usr = $usr;
@@ -217,6 +236,16 @@ class PushSubscription
$this->failureCount = 0;
}
/**
* Narrow this row to new-mail pushes. Nothing clears it again: there is no
* setter for false, so a row that should get the full stream back is a
* different row, not an edit.
*/
public function markArrivalsOnly(): void
{
$this->arrivalsOnly = true;
}
public function isExpired(?DateTimeImmutable $now = null): bool
{
if (null === $this->expires) {
@@ -61,6 +61,11 @@ final readonly class JmapPushSubscriber implements EventSubscriberInterface
{
$changed = $this->stateManager->drainDirty();
// Drained even when nothing changed. The arrivals buffer is separate
// from the dirty set, and if it were left behind on an early return it
// would be announced as new mail by whichever unit of work came next.
$arrived = $this->stateManager->drainArrivals();
if (0 === count($changed)) {
return;
}
@@ -70,7 +75,7 @@ final readonly class JmapPushSubscriber implements EventSubscriberInterface
$this->browsers->publish($changed);
try {
$this->dispatcher->dispatch($changed);
$this->dispatcher->dispatch($changed, $arrived);
} catch (\Throwable $exception) {
// Never let a push failure surface as a request or handler error:
// the data change already succeeded and is what matters.
+34 -2
View File
@@ -24,6 +24,13 @@ use Psr\Log\LoggerInterface;
* for. An unconfigured transport skips its own rows and leaves the others
* alone, which is what makes turning Firebase off a decision about Android
* rather than about push.
*
* **A subscription can ask for arrivals only.** The in-app browser/PWA device
* wants "new mail" and nothing else, and a StateChange cannot say which it is:
* it names an Email that changed, and a read, a flag or a sent copy changes one
* too. So such a row is sent only when mail arrived in one of its user's
* accounts since the last drain. Every other row is sent exactly as before, and
* a silenced row records nothing, the same as a row the type filter skips.
*/
final class PushDispatcher
{
@@ -37,14 +44,17 @@ final class PushDispatcher
}
/**
* @param array<int,array<string,string>> $changed accountId => type => state
* @param array<int,array<string,string>> $changed accountId => type => state
* @param list<int> $arrivedAccountIds accounts on which mail arrived since the last drain
*/
public function dispatch(array $changed): void
public function dispatch(array $changed, array $arrivedAccountIds): void
{
if (0 === count($changed) || false === $this->senders->anyConfigured()) {
return;
}
$arrived = array_map('intval', $arrivedAccountIds);
foreach ($this->groupByUser($changed) as $userId => $userChanges) {
$deliverable = $this->subscriptions->findDeliverableForUser($userId);
@@ -60,6 +70,13 @@ final class PushDispatcher
continue;
}
// Judged per user, not per change: a StateChange for this user's
// own mail is what the PWA is waiting for, and another user's
// arrival says nothing about it.
if (true === $subscription->arrivalsOnly && false === $this->anyArrived($userChanges, $arrived)) {
continue;
}
$filtered = $this->filterToWantedTypes($subscription, $userChanges);
if (0 === count($filtered)) {
@@ -100,6 +117,21 @@ final class PushDispatcher
return $byUser;
}
/**
* @param array<string,array<string,string>> $userChanges accountId => type => state, for one user
* @param list<int> $arrived accounts on which mail arrived
*/
private function anyArrived(array $userChanges, array $arrived): bool
{
foreach (array_keys($userChanges) as $accountId) {
if (true === in_array((int) $accountId, $arrived, true)) {
return true;
}
}
return false;
}
/**
* @param array<string,array<string,string>> $changes
*
+7 -1
View File
@@ -120,7 +120,13 @@ column.
relays ciphertext. `FcmSender` sends such a payload sealed or not at all;
`StateChange` and `PushVerification` carry no content and stay readable.
- `Push/PushDispatcher` fans out to a user's devices, per transport. Draining is
driven by `App\Infrastructure\Event\Subscriber\JmapPushSubscriber`.
driven by `App\Infrastructure\Event\Subscriber\JmapPushSubscriber`, which
drains two buffers from `StateManager`: what changed (`drainDirty()`) and on
which accounts mail arrived (`drainArrivals()`). A subscription with
`arrivalsOnly` — the in-app browser/PWA device, set by `WebPushController` —
is sent a StateChange only when its user had an arrival; every other row is
sent every StateChange. "Arrived" is decided by `PostIngestPipeline` (a live
batch, filed in the Inbox), not by an Email being created.
**Appearance glue**
- `Mapper/AppearanceMapper` — `App\Entity\Embeddable\Appearance` → the JMAP
+43
View File
@@ -32,6 +32,21 @@ final class StateManager
*/
private array $dirty = [];
/**
* Accounts on which mail ARRIVED since the last drain, keyed by account id
* so a batch of a hundred messages is one entry.
*
* A separate buffer rather than a flag on $dirty, because an Email being
* CREATED is not the same question as mail ARRIVING: drafts, sent copies
* and imported history are creates too. $dirty answers "did anything
* change" for every JMAP client; this answers "is there new mail" for the
* one device that is only interested in that. Keeping them apart is what
* lets that device be told less without any other client being told less.
*
* @var array<int,bool>
*/
private array $arrivals = [];
public function __construct(
private readonly EntityManagerInterface $entityManager,
private readonly ChangeLogRepository $changeLogRepository,
@@ -97,6 +112,34 @@ final class StateManager
$this->dirty[$accountId][$type->value] = true;
}
/**
* Record that mail arrived in this account's Inbox. Idempotent per account
* within one drain. Writes no change-log row: the Email's own create is
* recorded separately, by the same caller.
*
* Callers must be sure the message is genuinely new mail, not merely a
* created Email. See PostIngestPipeline, the one place that decides.
*/
public function recordArrival(int $accountId): void
{
$this->arrivals[$accountId] = true;
}
/**
* The accounts on which mail arrived since the last drain, emptying that
* buffer. Independent of drainDirty(): draining one leaves the other.
*
* @return list<int>
*/
public function drainArrivals(): array
{
$arrived = array_map('intval', array_keys($this->arrivals));
$this->arrivals = [];
return array_values($arrived);
}
/**
* Whether anything has been recorded since the last drain.
*
+14 -1
View File
@@ -720,7 +720,14 @@ final class GraphApiSyncer
// A folder move on the Microsoft side changes Email.mailboxIds, which
// is the single most visible thing a JMAP client watches for. Without
// this the message silently moves in Outlook and never in ltt.rs.
$this->recordMoved($account, $message);
//
// Only when it did move. Graph re-sends a message in its folder's
// delta for reasons that are not a move — plMail marking it read is
// one — and recording a change for a message that is already where it
// should be announces nothing true to a JMAP client.
if (true === $moved) {
$this->recordMoved($account, $message);
}
// A list shows the thread's labels, and a thread wears a folder while
// any of its messages does. Adding the destination alone left the
@@ -751,6 +758,12 @@ final class GraphApiSyncer
$wasThere = $message->hasLabel($label);
// A removal for a folder the message is not in changes nothing, and
// recording it would announce a move that did not happen.
if (false === $wasThere) {
return false;
}
$message->removeLabel($label);
$this->recordMoved($account, $message);
$this->syncThreadLabels($message);
+19
View File
@@ -68,6 +68,25 @@ final readonly class MailChangeRecorder
}
}
/**
* New mail arrived in the account's Inbox.
*
* A distinct announcement from emailChanged(), though it sounds like one of
* its cases. That call answers "did an Email change", which every JMAP
* client needs and which a draft or a sent copy also answers. This answers
* "did mail arrive", which only one device wants, and it is the only signal
* that device gets. It is not a feature in the sense the class docblock
* refuses: it is a different question with a different audience, and folding
* it into emailChanged() would make every create look like new mail.
*
* Recorded by the caller in addition to emailChanged(created: true), never
* instead of it: the Email still changed for everybody else.
*/
public function mailArrived(int $accountId): void
{
$this->stateManager->recordArrival($accountId);
}
/**
* An Email was really deleted — unlike Email/set destroy, which moves the
* message to Trash and keeps its id resolvable.
+17
View File
@@ -6,6 +6,7 @@ namespace App\Service\Mail;
use App\Domain\DTO\Mail\IngestedMessage;
use App\Domain\DTO\Mail\PostIngestResult;
use App\Domain\Enum\Mail\ArrivalPlace;
use App\Domain\Interface\PostIngestStepInterface;
use App\Entity\Mail\Account;
use App\Repository\Mail\ContactRepository;
@@ -128,6 +129,22 @@ final readonly class PostIngestPipeline
thread: null,
);
// NEW MAIL, as distinct from the create above: an Email created
// here is new mail only when it landed in the Inbox of a mailbox
// that has finished importing. Spam, other folders and unplaced
// messages are creates too, and a message from an import is one
// nobody is watching arrive.
//
// A copy of something you sent that a sync brings back from the
// Sent folder is filed as Elsewhere, and a draft written here is
// stored outside any sync and left unplaced (null); neither counts.
//
// The arrivals buffer is what the in-app PWA's "new mail" push
// reads; see StateManager::recordArrival().
if (true === $live && ArrivalPlace::Inbox === $message->arrivedIn) {
$this->changes->mailArrived($accountId);
}
$message->category = $this->categorizer->categorize($message, $correspondents, $user?->categorySorting);
// BEFORE THREADING, because the thread is where a hold takes
@@ -0,0 +1,218 @@
<?php
declare(strict_types=1);
namespace App\Tests\Controller\Settings;
use App\Entity\User\PushSubscription;
use App\Entity\User\User;
use App\Jmap\Push\PushDeliveryRecorder;
use App\Jmap\Push\WebPushSender;
use App\Repository\User\PushSubscriptionRepository;
use App\Repository\User\UserRepository;
use Doctrine\ORM\EntityManagerInterface;
use Minishlink\WebPush\VAPID;
use Psr\Log\NullLogger;
use Symfony\Bundle\FrameworkBundle\KernelBrowser;
use Symfony\Bundle\FrameworkBundle\Test\WebTestCase;
use Symfony\Component\HttpClient\MockHttpClient;
use Symfony\Component\HttpClient\Response\MockResponse;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Contracts\HttpClient\ResponseInterface;
/**
* The in-app browser/PWA registration is what narrows a row to new-mail pushes,
* and it must do so on BOTH of its branches: a device seen for the first time,
* and a device that re-subscribes with a rotated endpoint and keeps its id.
*
* The request is real. The only thing replaced is the Web Push sender, and only
* its HTTP client: it is built with a throwaway VAPID pair and a MockHttpClient,
* so the verification push the controller sends lands on the mock and never on
* a push service. The endpoint is an IP literal because a hostname would be
* resolved by PrivateNetwork, which is a DNS query.
*
* Each "narrowed" assertion has its absence companion in the same test: the row
* a JMAP client creates through the factory, which PushSubscription/set uses, is
* still on the full stream after the in-app device has been narrowed.
*
* Skips itself without the seeded admin, as the other controller tests do.
*/
final class WebPushSubscribeArrivalsTest extends WebTestCase
{
private const string ADMIN_EMAIL = 'e2e-admin@plmail.test';
/** A public address written as a literal, so no name is resolved. */
private const string ENDPOINT = 'https://93.184.215.14/wpush/v2/';
private KernelBrowser $client;
private EntityManagerInterface $em;
private PushSubscriptionRepository $subscriptions;
private User $user;
/** @var list<string> the URLs the verification pushes were sent to */
private array $pushes = [];
protected function setUp(): void
{
$this->client = static::createClient();
// The rows are staged inside a rolled-back transaction, which a kernel
// reboot between requests would not be able to see. Same as
// PushDeviceRemovalTest.
$this->client->disableReboot();
$container = static::getContainer();
$this->em = $container->get(EntityManagerInterface::class);
$this->subscriptions = $container->get(PushSubscriptionRepository::class);
$user = $container->get(UserRepository::class)->findOneBy(['email' => self::ADMIN_EMAIL]);
if (false === $user instanceof User) {
self::markTestSkipped('run `app:test:seed-user --admin` first');
}
$this->user = $user;
$keys = VAPID::createVapidKeys();
$http = new MockHttpClient(function (string $method, string $url): ResponseInterface {
$this->pushes[] = $url;
return new MockResponse('', ['http_code' => 201]);
});
// Built by hand with the public constructor, which is what a test may do
// with a final class. The controller takes whatever the container holds.
$container->set(WebPushSender::class, new WebPushSender(
$this->em,
new PushDeliveryRecorder($this->em, new NullLogger()),
new NullLogger(),
'mailto:admin@example.test',
$keys['publicKey'],
$keys['privateKey'],
$http,
));
$this->client->loginUser($user);
$container->get(EntityManagerInterface::class)->getConnection()->beginTransaction();
}
protected function tearDown(): void
{
$connection = static::getContainer()->get(EntityManagerInterface::class)->getConnection();
if (true === $connection->isTransactionActive()) {
$connection->rollBack();
}
parent::tearDown();
}
public function testANewInAppDeviceIsNarrowedAndAJmapClientRowIsNot(): void
{
$jmapClient = PushSubscription::webPush($this->user, 'a-jmap-client', self::ENDPOINT . 'jmap');
$this->em->persist($jmapClient);
$this->em->flush();
$this->subscribe('web-new-device', self::ENDPOINT . 'new');
self::assertNotEmpty($this->pushes, 'the verification push went to the mock, not the network');
$this->em->clear();
self::assertTrue($this->stored('web-new-device')->arrivalsOnly, 'presence: the in-app device is narrowed');
self::assertFalse($this->stored('a-jmap-client')->arrivalsOnly, 'absence: a JMAP client row keeps the full stream');
}
public function testAnExistingInAppDeviceIsNarrowedWhenItResubscribesWithARotatedEndpoint(): void
{
$existing = PushSubscription::webPush($this->user, 'web-existing-device', self::ENDPOINT . 'old');
$this->em->persist($existing);
$this->em->flush();
self::assertFalse($existing->arrivalsOnly, 'the row starts on the full stream, so the test can see it change');
$this->subscribe('web-existing-device', self::ENDPOINT . 'rotated');
$this->em->clear();
$row = $this->stored('web-existing-device');
self::assertSame(self::ENDPOINT . 'rotated', $row->url, 'presence: the rotated endpoint was taken, so this is the re-subscribe branch');
self::assertTrue($row->arrivalsOnly, 'presence: the re-subscribed device is narrowed');
}
private function subscribe(string $deviceClientId, string $endpoint): void
{
$body = [
'endpoint' => $endpoint,
'keys' => [
'p256dh' => $this->p256dh(),
'auth' => $this->authSecret(),
],
'deviceClientId' => $deviceClientId,
];
$this->client->request(
'POST',
'/settings/push/subscribe',
server: [
'CONTENT_TYPE' => 'application/json',
'HTTP_X_CSRF_TOKEN' => $this->csrfToken('web-push'),
],
content: json_encode($body, JSON_THROW_ON_ERROR),
);
self::assertResponseIsSuccessful();
}
private function stored(string $deviceClientId): PushSubscription
{
$subscription = $this->subscriptions->findOneByDeviceClientId($this->user, $deviceClientId);
self::assertNotNull($subscription, sprintf('no row for %s', $deviceClientId));
return $subscription;
}
/**
* A real P-256 point, the shape a browser hands over: the library encrypts
* the verification payload to it. VAPID's public key has that shape too.
*/
private function p256dh(): string
{
return VAPID::createVapidKeys()['publicKey'];
}
private function authSecret(): string
{
return rtrim(strtr(base64_encode(random_bytes(16)), '+/', '-_'), '=');
}
/**
* The token manager reads the session off the current request, and there is
* no current request between two client calls, so the page the form lives on
* is fetched and its session pushed onto the stack. As PushDeviceRemovalTest
* does it.
*/
private function csrfToken(string $tokenId): string
{
$this->client->request('GET', '/settings?section=notifications');
$stack = static::getContainer()->get('request_stack');
$carrier = new Request();
$carrier->setSession($this->client->getRequest()->getSession());
$stack->push($carrier);
try {
return static::getContainer()
->get('security.csrf.token_manager')
->getToken($tokenId)
->getValue();
} finally {
$this->client->getRequest()->getSession()->save();
$stack->pop();
}
}
}
@@ -0,0 +1,40 @@
<?php
declare(strict_types=1);
namespace App\Tests\Entity\User;
use App\Entity\User\PushSubscription;
use App\Entity\User\User;
use PHPUnit\Framework\TestCase;
/**
* The flag is what narrows a row, and the only thing that sets it is the
* in-app registration. A row made the way PushSubscription/set makes one, by
* the webPush() factory, must stay on the full stream.
*
* This is the entity half of the claim. That WebPushController marks its rows
* is WebPushSubscribeArrivalsTest's, which posts to the real endpoint with the
* push client mocked.
*/
final class PushSubscriptionArrivalsTest extends TestCase
{
public function testAJmapClientsRowIsNotNarrowed(): void
{
$subscription = PushSubscription::webPush(new User(), 'a-jmap-client', 'https://push.example.test/c');
self::assertFalse($subscription->arrivalsOnly, 'a row made by the factory keeps the full stream');
}
public function testMarkingARowNarrowsItAndOnlyThatRow(): void
{
$user = new User();
$marked = PushSubscription::webPush($user, 'pwa-device', 'https://push.example.test/p');
$other = PushSubscription::webPush($user, 'a-jmap-client', 'https://push.example.test/c');
$marked->markArrivalsOnly();
self::assertTrue($marked->arrivalsOnly, 'presence: the marked row is narrowed');
self::assertFalse($other->arrivalsOnly, 'the other row is not narrowed by marking this one');
}
}
@@ -0,0 +1,77 @@
<?php
declare(strict_types=1);
namespace App\Tests\Infrastructure\Event\Subscriber;
use App\Domain\Enum\PushTransport;
use App\Entity\User\PushSubscription;
use App\Infrastructure\Event\Subscriber\JmapPushSubscriber;
use App\Jmap\Protocol\StateChangeBuilder;
use App\Jmap\Push\PushDispatcher;
use App\Jmap\Push\PushSenderRegistry;
use App\Jmap\State\JmapObjectType;
use App\Jmap\State\StateManager;
use App\Repository\User\PushSubscriptionRepository;
use App\Repository\User\UserRepository;
use App\Service\Mail\MailChangeNotifier;
use App\Tests\Jmap\JmapTestCase;
use App\Tests\Jmap\Push\RecordingArrivalsSender;
use Psr\Log\NullLogger;
/**
* The flush is where arrivals meet the dispatcher. An arrival recorded in one
* unit of work must reach that unit's push, and must not be carried into the
* next one, where it would announce mail that did not arrive there.
*
* Drives onWorkerMessage() because it is the public entry point that needs no
* event object; it runs the same flush as kernel.terminate.
*/
final class JmapPushSubscriberArrivalsTest extends JmapTestCase
{
public function testAnArrivalReachesTheDispatcherOnceAndDoesNotLeakIntoTheNextFlush(): void
{
$container = self::getContainer();
$state = $container->get(StateManager::class);
$accountId = (int) $this->account->id;
$subscription = PushSubscription::webPush($this->user, 'pwa-device', 'https://push.example.test/p');
$subscription->markArrivalsOnly();
$subscription->verify((string) $subscription->verificationCode);
$this->em->persist($subscription);
$this->em->flush();
$sender = new RecordingArrivalsSender(PushTransport::WebPush);
$subscriber = new JmapPushSubscriber(
$state,
new PushDispatcher(
$container->get(PushSubscriptionRepository::class),
$container->get(UserRepository::class),
new PushSenderRegistry([$sender]),
$container->get(StateChangeBuilder::class),
new NullLogger(),
),
$container->get(MailChangeNotifier::class),
new NullLogger(),
);
$state->recordUpdated($accountId, JmapObjectType::Email, '9');
$this->em->flush();
$subscriber->onWorkerMessage();
self::assertCount(0, $sender->sent, 'an update-only flush does not mark the PWA as arrived');
$state->recordArrival($accountId);
$state->recordUpdated($accountId, JmapObjectType::Email, '10');
$this->em->flush();
$subscriber->onWorkerMessage();
self::assertCount(1, $sender->sent, 'presence: the arrival reached the dispatcher');
$state->recordUpdated($accountId, JmapObjectType::Email, '11');
$this->em->flush();
$subscriber->onWorkerMessage();
self::assertCount(1, $sender->sent, 'the arrival was drained by the flush above and did not leak into this one');
}
}
@@ -0,0 +1,188 @@
<?php
declare(strict_types=1);
namespace App\Tests\Jmap\Push;
use App\Domain\Enum\PushTransport;
use App\Domain\Interface\PushSenderInterface;
use App\Entity\Mail\Account;
use App\Entity\User\PushSubscription;
use App\Entity\User\User;
use App\Jmap\Protocol\StateChangeBuilder;
use App\Jmap\Push\PushDispatcher;
use App\Jmap\Push\PushSenderRegistry;
use App\Repository\User\PushSubscriptionRepository;
use App\Repository\User\UserRepository;
use App\Tests\Jmap\JmapTestCase;
use Psr\Log\NullLogger;
/**
* The in-app browser/PWA device wants "new mail" and nothing else, so its row
* is marked arrivals-only and is told only when mail actually arrived in one of
* its user's accounts. Every other subscription keeps receiving every change.
*
* Each absence below has a presence companion in the same test, so an empty
* send list cannot pass for a dispatcher that dropped everything.
*/
final class PushDispatcherArrivalsTest extends JmapTestCase
{
public function testAnArrivalsOnlyBrowserIsSilentOnAnUpdateAndToldOnAnArrival(): void
{
$this->subscribe($this->arrivalsOnlyBrowser('pwa-device'));
$webPush = new RecordingArrivalsSender(PushTransport::WebPush);
$this->dispatch($webPush, [$this->ownAccountId()], updated: [$this->ownAccountId()], arrived: []);
self::assertCount(0, $webPush->sent, 'an update is not new mail');
$this->dispatch($webPush, [$this->ownAccountId()], updated: [$this->ownAccountId()], arrived: [$this->ownAccountId()]);
self::assertCount(1, $webPush->sent, 'presence: mail arrived, so the PWA is told');
self::assertSame('pwa-device', $webPush->sent[0]->deviceClientId);
}
public function testAJmapClientBrowserIsStillToldOfAnUpdate(): void
{
$this->subscribe(PushSubscription::webPush($this->user, 'a-jmap-client', 'https://push.example.test/c'));
$webPush = new RecordingArrivalsSender(PushTransport::WebPush);
$this->dispatch($webPush, [$this->ownAccountId()], updated: [$this->ownAccountId()], arrived: []);
self::assertCount(1, $webPush->sent, 'a client that did not opt in still gets every StateChange');
self::assertSame('a-jmap-client', $webPush->sent[0]->deviceClientId);
}
public function testAnFcmPhoneIsStillToldOfAnUpdate(): void
{
$this->subscribe(PushSubscription::fcm($this->user, 'a-phone', 'a-device-token'));
$fcm = new RecordingArrivalsSender(PushTransport::Fcm);
$this->dispatch($fcm, [$this->ownAccountId()], updated: [$this->ownAccountId()], arrived: []);
self::assertCount(1, $fcm->sent, 'the phone keeps every StateChange');
self::assertSame('a-phone', $fcm->sent[0]->deviceClientId);
}
public function testArrivalsForAnotherUsersAccountDoNotNotifyThisUser(): void
{
$this->subscribe($this->arrivalsOnlyBrowser('pwa-device'));
$otherAccountId = $this->seedOtherUsersAccount();
$webPush = new RecordingArrivalsSender(PushTransport::WebPush);
$this->dispatch($webPush, [$otherAccountId], updated: [$otherAccountId], arrived: [$otherAccountId]);
self::assertCount(0, $webPush->sent, 'mail for someone else is not this user\'s new mail');
$this->dispatch($webPush, [$this->ownAccountId()], updated: [$this->ownAccountId()], arrived: [$this->ownAccountId()]);
self::assertCount(1, $webPush->sent, 'presence: this user\'s own arrival still notifies the PWA');
}
/**
* The case the test above cannot catch: this user's own mail changed in the
* same drain in which somebody else's mail arrived. An arrival anywhere is
* not an arrival in this user's accounts, and judging it that way would send
* every user's PWA a "New mail" for every other user's mail.
*/
public function testSomeoneElsesArrivalDoesNotNotifyAUserWhoseOwnMailOnlyChanged(): void
{
$this->subscribe($this->arrivalsOnlyBrowser('pwa-device'));
$otherAccountId = $this->seedOtherUsersAccount();
$webPush = new RecordingArrivalsSender(PushTransport::WebPush);
$this->dispatch($webPush, [$this->ownAccountId()], updated: [$this->ownAccountId()], arrived: [$otherAccountId]);
self::assertCount(0, $webPush->sent, 'a stranger\'s mail arrived; this user\'s mail only changed');
$this->dispatch($webPush, [$this->ownAccountId()], updated: [$this->ownAccountId()], arrived: [$this->ownAccountId()]);
self::assertCount(1, $webPush->sent, 'presence: their own arrival notifies the PWA');
}
/**
* @param list<int> $accountIds the accounts whose state changed
* @param list<int> $updated the accounts a change was recorded for; the
* dispatcher receives the same ids for both
* @param list<int> $arrived the accounts mail arrived in
*/
private function dispatch(PushSenderInterface $sender, array $accountIds, array $updated, array $arrived): void
{
$container = self::getContainer();
$changed = [];
foreach ($accountIds as $accountId) {
if (true === in_array($accountId, $updated, true)) {
$changed[$accountId] = ['Email' => '9'];
}
}
new PushDispatcher(
$container->get(PushSubscriptionRepository::class),
$container->get(UserRepository::class),
new PushSenderRegistry([$sender]),
$container->get(StateChangeBuilder::class),
new NullLogger(),
)->dispatch($changed, $arrived);
}
/** This user's account id as the int the dispatcher is handed; JmapTestCase::accountId() is the JMAP string. */
private function ownAccountId(): int
{
return (int) $this->account->id;
}
private function arrivalsOnlyBrowser(string $deviceClientId): PushSubscription
{
$subscription = PushSubscription::webPush($this->user, $deviceClientId, 'https://push.example.test/p');
$subscription->markArrivalsOnly();
return $subscription;
}
/** Verified, because an unverified subscription is deliverable to nothing. */
private function subscribe(PushSubscription $subscription): void
{
$subscription->verify((string) $subscription->verificationCode);
$this->em->persist($subscription);
$this->em->flush();
}
/**
* A second user with one account, so an arrival can belong to someone else.
*/
private function seedOtherUsersAccount(): int
{
$user = new User();
$user->email = 'push-arrivals-' . uniqid('', true) . '@example.test';
$user->nameFirst = 'Other';
$user->nameLast = 'Owner';
$user->roles = ['ROLE_USER'];
$user->password = 'x';
$this->em->persist($user);
$account = new Account();
$account->usr = $user;
$account->email = 'Other Owner';
$account->username = 'other-owner@example.test';
$account->imapHost = 'localhost';
$account->imapPort = 993;
$account->imapEncryption = 'ssl';
$account->smtpHost = 'localhost';
$account->smtpPort = 587;
$account->smtpEncryption = 'starttls';
$account->password = 'x';
$account->authType = 'password';
$account->isActive = true;
$this->em->persist($account);
$this->em->flush();
return (int) $account->id;
}
}
+1 -1
View File
@@ -77,7 +77,7 @@ final class PushDispatcherFanOutTest extends JmapTestCase
new PushSenderRegistry($senders),
$container->get(StateChangeBuilder::class),
new NullLogger(),
)->dispatch([(int) $this->account->id => ['Email' => '9']]);
)->dispatch([(int) $this->account->id => ['Email' => '9']], []);
}
/** Verified, because an unverified subscription is deliverable to nothing. */
@@ -0,0 +1,42 @@
<?php
declare(strict_types=1);
namespace App\Tests\Jmap\Push;
use App\Domain\Enum\PushTransport;
use App\Domain\Interface\PushSenderInterface;
use App\Entity\User\PushSubscription;
/**
* A sender that records rather than delivers. Named apart from the one in
* PushDispatcherFanOutTest because both files declare a class in this namespace,
* and a second declaration of the same name is a fatal error in one run.
*/
final class RecordingArrivalsSender implements PushSenderInterface
{
/** @var list<PushSubscription> */
public array $sent = [];
public function __construct(
private readonly PushTransport $transport,
private readonly bool $configured = true,
) {}
public function transport(): PushTransport
{
return $this->transport;
}
public function isConfigured(): bool
{
return $this->configured;
}
public function send(PushSubscription $subscription, array $payload): bool
{
$this->sent[] = $subscription;
return true;
}
}
+59
View File
@@ -0,0 +1,59 @@
<?php
declare(strict_types=1);
namespace App\Tests\Jmap\State;
use App\Jmap\State\JmapObjectType;
use App\Jmap\State\StateManager;
use App\Tests\Jmap\JmapTestCase;
/**
* Arrivals are a second pending buffer beside the dirty set, and the two must
* stay apart: a create is not an arrival, and an arrival is not a change that
* a JMAP client needs to hear about by its own state token.
*
* Each test asserts the presence of the thing it means as well as the absence
* of the other, so an empty result can only mean the buffer was empty.
*/
final class StateArrivalsTest extends JmapTestCase
{
public function testAnArrivalIsRecordedOncePerAccountAndDrainingEmptiesIt(): void
{
$state = self::getContainer()->get(StateManager::class);
$accountId = (int) $this->account->id;
$state->recordArrival($accountId);
$state->recordArrival($accountId);
self::assertSame([$accountId], $state->drainArrivals(), 'the account arrived once');
self::assertSame([], $state->drainArrivals(), 'a second drain finds the buffer empty');
}
public function testArrivalsAndDirtyChangesAreDrainedIndependently(): void
{
$state = self::getContainer()->get(StateManager::class);
$accountId = (int) $this->account->id;
$state->recordArrival($accountId);
$state->recordUpdated($accountId, JmapObjectType::Email, '9');
$this->em->flush();
$dirty = $state->drainDirty();
self::assertArrayHasKey($accountId, $dirty, 'the update is still drained as dirty');
self::assertSame([$accountId], $state->drainArrivals(), 'the arrival is still drained as an arrival');
}
public function testAnUpdateIsNotAnArrival(): void
{
$state = self::getContainer()->get(StateManager::class);
$accountId = (int) $this->account->id;
$state->recordUpdated($accountId, JmapObjectType::Email, '9');
$this->em->flush();
self::assertSame([], $state->drainArrivals(), 'an update alone records no arrival');
self::assertArrayHasKey($accountId, $state->drainDirty(), 'presence: the update is dirty');
}
}
+106
View File
@@ -400,6 +400,100 @@ final class GraphFolderMoveTest extends KernelTestCase
);
}
// ── re-reported, not moved ───────────────────────────────────────────────
/**
* Graph re-sends a message in its folder's delta whenever anything about it
* changes, and most of those changes are not a move. Writing a change-log
* row for a message that did not move makes the server announce new mail
* to the phone, so a message already wearing its folder's label must leave
* no trace in the change log at all.
*/
public function testAMessageAlreadyInItsFolderWritesNoChangeLog(): void
{
$message = $this->seedMessage($this->trash);
$this->syncFolder(self::TRASH_FOLDER, $message->graphId);
self::assertSame(['Trash'], $this->labelNamesOf($message));
self::assertSame(0, $this->changeLogRows('Email', 'updated'));
self::assertSame(0, $this->changeLogRows('Thread', 'updated'));
}
/**
* The production shape, not the tidy one. A delta entry for a message plMail
* has just marked read carries `isRead` alongside its folder, and the flag
* state that comes with it is already what plMail stores for an unread
* message, so nothing moved and nothing changed.
*/
public function testAReReportedMessageWithAnUnchangedReadStateWritesNoChangeLog(): void
{
$message = $this->seedMessage($this->trash);
$this->syncFolders([self::TRASH_FOLDER => [[
'id' => $message->graphId,
'parentFolderId' => self::TRASH_FOLDER,
'isRead' => false,
]]]);
self::assertSame(['Trash'], $this->labelNamesOf($message));
self::assertSame(0, $this->changeLogRows('Email', 'updated'));
self::assertSame(0, $this->changeLogRows('Thread', 'updated'));
}
/**
* The counterpart to the two tests above: the same syncer, the same
* assertion helper, a real move. Without it, zero rows could mean the
* counting query never sees the table.
*/
public function testAMessageThatMovedWritesOneEmailAndOneThreadChangeLogRow(): void
{
$message = $this->seedMessage($this->drafts);
$this->syncFolder(self::TRASH_FOLDER, $message->graphId);
self::assertSame(1, $this->changeLogRows('Email', 'updated'));
self::assertSame(1, $this->changeLogRows('Thread', 'updated'));
}
/**
* A removal reported under a folder the message is not in is a stale or
* misdirected delta entry. There is nothing to take off, so nothing may be
* written and the message stays where it is.
*/
public function testARemovalFromAFolderTheMessageIsNotInWritesNoChangeLog(): void
{
$message = $this->seedMessage($this->trash);
$rowId = (int) $message->id;
$this->syncFolders([self::DRAFTS_FOLDER => [$this->removedItem($message->graphId)]]);
self::assertNotNull($this->em->find(Message::class, $rowId));
self::assertSame(['Trash'], $this->labelNamesOf($message));
self::assertSame(0, $this->changeLogRows('Email', 'updated'));
self::assertSame(0, $this->changeLogRows('Thread', 'updated'));
}
/**
* The counterpart to the removal test above: the message does wear the
* label being removed, and it still has another folder label to keep it
* from being read as a deletion, so this is a real change and must be
* recorded.
*/
public function testARemovalFromAFolderTheMessageIsInWritesAChangeLogRow(): void
{
$message = $this->seedMessage($this->drafts, $this->trash);
$rowId = (int) $message->id;
$this->syncFolders([
self::DRAFTS_FOLDER => [$this->removedItem($message->graphId)],
self::TRASH_FOLDER => [['id' => $message->graphId, 'parentFolderId' => self::TRASH_FOLDER]],
]);
self::assertNotNull($this->em->find(Message::class, $rowId));
self::assertGreaterThanOrEqual(1, $this->changeLogRows('Email', 'updated'));
}
// ── Fixtures ─────────────────────────────────────────────────────────────
/**
@@ -530,6 +624,18 @@ final class GraphFolderMoveTest extends KernelTestCase
return $names;
}
/**
* How many change-log rows this account has of one object type and change.
* The fixtures write none, so after a sync the count is what the sync wrote.
*/
private function changeLogRows(string $objectType, string $changeType): int
{
return (int) $this->connection->fetchOne(
'SELECT COUNT(*) FROM jmap_change_log WHERE account_id = ? AND object_type = ? AND change_type = ?',
[$this->account->id, $objectType, $changeType],
);
}
/**
* The thread's labels as a list shows them, read back from the database.
*
@@ -6,7 +6,9 @@ namespace App\Tests\Service\Mail;
use App\Domain\DTO\Mail\IngestedMessage;
use App\Domain\DTO\Mail\PostIngestResult;
use App\Domain\Enum\Mail\ArrivalPlace;
use App\Domain\Interface\PostIngestStepInterface;
use App\Jmap\State\StateManager;
use App\Entity\Mail\Account;
use App\Entity\Mail\Mailbox;
use App\Entity\Mail\Message;
@@ -224,8 +226,86 @@ final class PostIngestPipelineTest extends KernelTestCase
self::assertFalse($ran);
}
/**
* "New mail" means mail that ARRIVED in the Inbox, not any Email created: a
* spam, filed or unplaced message is a create too, and must not reach the
* arrivals buffer. The Inbox case is the presence companion for the three
* absences, in the same test.
*/
public function testOnlyAnInboxArrivalInALiveBatchIsRecordedAsAnArrival(): void
{
$state = self::getContainer()->get(StateManager::class);
$state->drainArrivals();
// Not importing any more, so the batch is live. The fixture mailbox
// starts out syncing, which is an import in progress.
$this->mailbox->isSyncEnabled = false;
$this->em->flush();
$this->pipeline()->run($this->account, [
new IngestedMessage($this->seedArrival(ArrivalPlace::Spam), $this->account),
]);
self::assertSame([], $state->drainArrivals(), 'spam is not new mail');
$this->pipeline()->run($this->account, [
new IngestedMessage($this->seedArrival(ArrivalPlace::Elsewhere), $this->account),
]);
self::assertSame([], $state->drainArrivals(), 'mail filed elsewhere is not new mail');
$this->pipeline()->run($this->account, [
new IngestedMessage($this->seedArrival(null), $this->account),
]);
self::assertSame([], $state->drainArrivals(), 'a message with no arrival place is not new mail');
$this->pipeline()->run($this->account, [
new IngestedMessage($this->seedArrival(ArrivalPlace::Inbox), $this->account),
]);
self::assertSame([(int) $this->account->id], $state->drainArrivals(), 'an Inbox arrival is new mail');
}
/**
* An import is tens of thousands of messages nobody is watching arrive.
* The same Inbox message in a batch that is not live must not be announced
* as new mail; once the mailbox is done importing it must be.
*/
public function testAnInboxMessageDuringAnImportIsNotAnArrivalButAfterItIs(): void
{
$state = self::getContainer()->get(StateManager::class);
$state->drainArrivals();
$this->mailbox->isSyncEnabled = true;
$this->em->flush();
$this->pipeline()->run($this->account, [
new IngestedMessage($this->seedArrival(ArrivalPlace::Inbox), $this->account),
]);
self::assertSame([], $state->drainArrivals(), 'an import is not new mail');
$this->mailbox->isSyncEnabled = false;
$this->em->flush();
$this->pipeline()->run($this->account, [
new IngestedMessage($this->seedArrival(ArrivalPlace::Inbox), $this->account),
]);
self::assertSame([(int) $this->account->id], $state->drainArrivals(), 'presence: once the import is done, it is');
}
// ── Fixtures ──────────────────────────────────────────────────────────
/**
* A persisted message whose arrival place is set the way ArrivalStamper
* would set it at persist. The stamper itself needs provider headers this
* fixture does not carry, so the place is written here.
*/
private function seedArrival(?ArrivalPlace $arrivedIn): Message
{
$message = $this->seedMessage('arrival-' . uniqid('', true));
$message->arrivedIn = $arrivedIn;
$this->em->flush();
return $message;
}
/**
* Built by hand rather than pulled from the container so the test can pass
* its own steps — the real service takes them from a tagged iterator.