From a71340156267a62c20fed32d5f0379af94854a5b Mon Sep 17 00:00:00 2001 From: nexxo Date: Mon, 3 Aug 2026 16:25:25 +0200 Subject: [PATCH] Der Takt, und die Falle mit den Versuchen MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SendQueuedMailable kennt keine middleware() — nachgesehen im Framework. Der Weg fuehrt ueber einen eigenen Auftrag, den Mailable::newQueuedJob() ueber den Container erzeugt; eine Bindung tauscht ihn fuer alle Mails aus, ohne dass eine Absendestelle sich aendert. mail-wichtig bekommt 30 je 5 Minuten, mail-ruhig 20 je 10, mail-direkt gar keine Drossel — dort wartet gerade ein Mensch. Der Arbeiter laeuft mit --tries=3, und eine gedrosselte Rueckstellung zaehlt als Versuch. Ohne retryUntil() waere jede Rechnung nach dem dritten Drosseln gescheitert statt verschickt. Der tragende Test faehrt den echten Arbeiter gegen ein zu kleines Kontingent und belegt beides: failed_jobs bleibt leer, und der hoechste Versuchszaehler ist vier — der Lauf hat die Linie wirklich ueberschritten. Die Bindung aendert den Klassennamen des eingereihten Auftrags, deshalb ziehen zwei bestehende Zusicherungen in MailLaneRoutingTest und SenderAddressTest nach: QueueFake legt Auftraege unter ihrem exakten Klassennamen ab. Co-Authored-By: Claude Opus 5 --- app/Jobs/PacedMail.php | 71 ++++++ app/Providers/MailPaceServiceProvider.php | 49 +++++ bootstrap/providers.php | 1 + tests/Feature/Mail/MailLaneRoutingTest.php | 13 +- tests/Feature/Mail/MailPaceTest.php | 241 +++++++++++++++++++++ tests/Feature/Mail/SenderAddressTest.php | 7 +- 6 files changed, 378 insertions(+), 4 deletions(-) create mode 100644 app/Jobs/PacedMail.php create mode 100644 app/Providers/MailPaceServiceProvider.php create mode 100644 tests/Feature/Mail/MailPaceTest.php diff --git a/app/Jobs/PacedMail.php b/app/Jobs/PacedMail.php new file mode 100644 index 0000000..1264f69 --- /dev/null +++ b/app/Jobs/PacedMail.php @@ -0,0 +1,71 @@ + */ + public function middleware(): array + { + if (! Settings::bool('mail.pace.enabled', true)) { + return []; + } + + // Die Schlange am Mailable, nicht `MailLane::for(...)`: das hier ist + // der Name, den `Mailable::queue()` an `pushOn()` gegeben hat — die + // Spur, in der dieser Auftrag TATSÄCHLICH liegt. `MailLane::for(...)` + // läge zwischen Einreihen und Abarbeiten eine Zuordnung später: würde + // der Betreiber die Rechnung von ruhig auf wichtig schieben, während + // zweihundert davon in der ruhigen Schlange warten, drosselte sie ab + // diesem Moment das falsche Kontingent. + // + // `?? null` statt eines nackten Zugriffs, weil `$this->mailable` nur + // die Schnittstelle zusichert: eine Mailklasse, die den Trait nicht + // benutzt, hat die Eigenschaft womöglich gar nicht — und liegt dann + // auch in keiner Spur, gehört also nicht gedrosselt. + return match ($this->mailable->queue ?? null) { + MailLane::URGENT => [new RateLimited(MailLane::URGENT)], + MailLane::CALM => [new RateLimited(MailLane::CALM)], + default => [], + }; + } + + /** + * Sechs Stunden statt eines Versuchszählers. + * + * Der Arbeiter prüft `retryUntil` VOR `--tries`: liegt sie in der Zukunft, + * steigt er aus, bevor er `attempts()` gegen die Versuchszahl hält. Ein + * Zähler dagegen könnte nicht zwischen „dreimal zurückgelegt" und „dreimal + * gescheitert" unterscheiden — die Rechnung, die am Abend zweihundertmal an + * der Drossel ansteht, wäre nach dem dritten Mal endgültig gescheitert. + * + * Weit genug, dass auch ein Lauf über die ganze Nacht durchkommt, und eng + * genug, dass eine Mail, die nach sechs Stunden noch nicht draußen ist, + * nicht am nächsten Tag zwischen den neuen auftaucht. + */ + public function retryUntil(): DateTimeInterface + { + return now()->addHours(6); + } +} diff --git a/app/Providers/MailPaceServiceProvider.php b/app/Providers/MailPaceServiceProvider.php new file mode 100644 index 0000000..a592c31 --- /dev/null +++ b/app/Providers/MailPaceServiceProvider.php @@ -0,0 +1,49 @@ +app->bind(SendQueuedMailable::class, PacedMail::class); + } + + public function boot(): void + { + // Die Einstellungen werden im Rückruf gelesen, nicht hier: der Rückruf + // läuft im Arbeiter je Auftrag, und eine Änderung des Betreibers wirkt + // damit ab dem nächsten Auftrag statt erst ab dem nächsten Neustart. + RateLimiter::for(MailLane::URGENT, fn () => Limit::perMinutes( + (int) Settings::get('mail.pace.urgent.minutes', 5), + (int) Settings::get('mail.pace.urgent.count', 30), + )); + + RateLimiter::for(MailLane::CALM, fn () => Limit::perMinutes( + (int) Settings::get('mail.pace.calm.minutes', 10), + (int) Settings::get('mail.pace.calm.count', 20), + )); + } +} diff --git a/bootstrap/providers.php b/bootstrap/providers.php index 0ad9c57..d221dbe 100644 --- a/bootstrap/providers.php +++ b/bootstrap/providers.php @@ -3,4 +3,5 @@ return [ App\Providers\AppServiceProvider::class, App\Providers\FortifyServiceProvider::class, + App\Providers\MailPaceServiceProvider::class, ]; diff --git a/tests/Feature/Mail/MailLaneRoutingTest.php b/tests/Feature/Mail/MailLaneRoutingTest.php index e32ff2b..f6fa6cf 100644 --- a/tests/Feature/Mail/MailLaneRoutingTest.php +++ b/tests/Feature/Mail/MailLaneRoutingTest.php @@ -1,5 +1,6 @@ queue(new InvoiceMail(invoiceForTest(), 'Muster')); - Queue::assertPushedOn(MailLane::CALM, Illuminate\Mail\SendQueuedMailable::class); + Queue::assertPushedOn(MailLane::CALM, PacedMail::class); }); it('reiht eine Direkt-Mail in die Direkt-Spur ein', function () { @@ -62,7 +69,7 @@ it('reiht eine Direkt-Mail in die Direkt-Spur ein', function () { Mail::to('kunde@example.test')->queue(new ResetPasswordMail($user, 'https://example.test/reset', 60)); - Queue::assertPushedOn(MailLane::DIRECT, Illuminate\Mail\SendQueuedMailable::class); + Queue::assertPushedOn(MailLane::DIRECT, PacedMail::class); }); it('folgt einer verschobenen Zuordnung', function () { @@ -71,7 +78,7 @@ it('folgt einer verschobenen Zuordnung', function () { Mail::to('kunde@example.test')->queue(new InvoiceMail(invoiceForTest(), 'Muster')); - Queue::assertPushedOn(MailLane::URGENT, Illuminate\Mail\SendQueuedMailable::class); + Queue::assertPushedOn(MailLane::URGENT, PacedMail::class); }); /** diff --git a/tests/Feature/Mail/MailPaceTest.php b/tests/Feature/Mail/MailPaceTest.php new file mode 100644 index 0000000..d61a50a --- /dev/null +++ b/tests/Feature/Mail/MailPaceTest.php @@ -0,0 +1,241 @@ +nummer); + } + + public function content(): Content + { + return new Content(htmlString: '

Takt

'); + } +} + +/** + * Der Auftrag, so wie ihn das Einreihen erzeugt. + * + * Nicht `new PacedMail(...)`: dass überhaupt ein PacedMail entsteht, ist die + * halbe Aufgabe — die Bindung im Container tauscht ihn für jede Mail aus, ohne + * dass eine Absendestelle davon weiß. Und die Spur steht erst da, weil der + * Trait sie VOR `newQueuedJob()` setzt; ein von Hand gebauter Auftrag hätte sie + * nie gesehen und der Test bewiese die falsche Sache. + */ +function taktAuftrag(Mailable $mailable): object +{ + Queue::fake(); + + Mail::to('kunde@example.test')->queue($mailable); + + return Queue::pushed(PacedMail::class)->first() + ?? throw new RuntimeException('Es wurde kein PacedMail eingereiht.'); +} + +/** + * Der Name der Drossel, an der dieser Auftrag hängt. + * + * `RateLimited` hält ihn geschützt und bietet keinen Weg heraus. Die Alternative + * wäre, die Spur am Verhalten abzulesen — zwei Kontingente setzen und schauen, + * welches greift; das prüft aber die Drossel des Frameworks mit und nicht die + * eine Entscheidung, um die es hier geht: welche Spur welche Drossel bekommt. + */ +function drosselName(RateLimited $middleware): string +{ + return (new ReflectionProperty($middleware, 'limiterName'))->getValue($middleware); +} + +/** + * Die Drossel darf keine Mail verlieren. + * + * Der Arbeiter läuft mit --tries=3, und eine zurückgelegte Mail zählt als + * Versuch. Ohne Vorkehrung wäre jede Rechnung nach dem dritten Drosseln + * gescheitert statt verschickt — still, im Fehlerprotokoll, ohne dass jemand + * etwas merkt. Das ist der Test, der vor der ersten Zeile Drossel steht. + * + * Über den ECHTEN Arbeiter, nicht über das Verhalten der Zwischenschicht: die + * Falle sitzt nicht in der Drossel, sondern in `Worker::markJobAsFailedIf…`, + * das `--tries` gegen `attempts()` hält. Nur ein Lauf, der wirklich durch diese + * Stelle geht, kann belegen, dass sie nicht zuschlägt. Die Schlange ist deshalb + * `database` statt `sync` — `sync` legt nicht zurück, sondern lässt eine + * gedrosselte Mail spurlos verschwinden, und der Test wäre grün ohne Grund. + * + * Acht Mails auf zwei je Fenster: die letzten beiden werden dreimal + * zurückgelegt und kommen im vierten Anlauf dran — ein Versuch mehr, als + * `--tries=3` durchgehen ließe. Genau das sichert die letzte Zusicherung ab, + * damit der Test nicht eines Tages grün bleibt, weil gar nicht mehr gedrosselt + * wird. + */ +it('verliert keine Mail, wenn das Kontingent kleiner ist als der Lauf', function () { + Settings::set('mail.pace.calm.count', 2); + Settings::set('mail.pace.calm.minutes', 1); + MailLane::assign(TaktProbeMail::class, MailLane::CALM); + + config(['queue.default' => 'database']); + + $versuche = []; + Event::listen(JobProcessing::class, function (JobProcessing $event) use (&$versuche) { + $versuche[] = $event->job->attempts(); + }); + + foreach (range(1, 8) as $nummer) { + Mail::to("kunde{$nummer}@example.test")->queue(new TaktProbeMail($nummer)); + } + + // Der Arbeiter, so wie er im Betrieb läuft: eine Mail je Aufruf, --tries=3. + // Liegt nichts Fälliges mehr da, wird die Uhr auf den nächsten + // Fälligkeitszeitpunkt gestellt — die Drossel legt zurück, sie wirft nicht, + // und ohne Uhrstellen stünde der Test eine Minute lang still. + $notbremse = 100; + + while (DB::table('jobs')->count() > 0) { + if (--$notbremse <= 0) { + throw new RuntimeException('Die Schlange leert sich nicht — Abbruch statt Endlosschleife.'); + } + + if (! DB::table('jobs')->where('available_at', '<=', now()->getTimestamp())->exists()) { + $this->travelTo(Carbon::createFromTimestamp((int) DB::table('jobs')->min('available_at'))); + + continue; + } + + Artisan::call('queue:work', [ + '--once' => true, + '--tries' => 3, + '--sleep' => 0, + '--queue' => implode(',', [MailLane::DIRECT, MailLane::URGENT, MailLane::CALM]), + ]); + } + + expect(DB::table('failed_jobs')->count())->toBe(0) + ->and(Mail::mailer()->getSymfonyTransport()->messages())->toHaveCount(8) + ->and(max($versuche))->toBeGreaterThan(3); +}); + +/** + * Was der Arbeiter liest, ist der Auftrag in der Schlange, nicht das Objekt. + * + * `retryUntil` wird beim Einreihen ausgerechnet und in die Nutzlast + * geschrieben; von dort holt `Worker::markJobAsFailedIfAlreadyExceedsMaxAttempts` + * sie und steigt aus, bevor `--tries` überhaupt zur Sprache kommt. Steht die + * Zeile nicht in der Nutzlast, hilft die schönste `retryUntil()`-Methode nichts. + * + * `maxTries` bleibt leer: ein eigener Versuchszähler am Auftrag würde den des + * Arbeiters ersetzen und die Falle nur an eine andere Zahl hängen. + */ +it('schreibt die zeitliche Grenze in den Auftrag, den der Arbeiter liest', function () { + config(['queue.default' => 'database']); + + Mail::to('kunde@example.test')->queue(new TaktProbeMail(1)); + + $nutzlast = json_decode((string) DB::table('jobs')->value('payload'), true); + + expect($nutzlast['retryUntil'])->toBe(now()->addHours(6)->getTimestamp()) + ->and($nutzlast['maxTries'])->toBeNull(); +}); + +it('erzeugt für jede Mail den gedrosselten Auftrag', function () { + expect(taktAuftrag(new TaktProbeMail(1)))->toBeInstanceOf(PacedMail::class); +}); + +it('hängt jede gedrosselte Spur an ihre eigene Drossel', function (string $spur) { + MailLane::assign(TaktProbeMail::class, $spur); + + $middleware = taktAuftrag(new TaktProbeMail(1))->middleware(); + + expect($middleware)->toHaveCount(1) + ->and($middleware[0])->toBeInstanceOf(RateLimited::class) + ->and(drosselName($middleware[0]))->toBe($spur); +})->with([MailLane::URGENT, MailLane::CALM]); + +/** + * Auf der direkten Spur wartet gerade ein Mensch, der eben geklickt hat. + * Drosseln nützt dort nichts — die Mails entstehen einzeln und können gar + * keinen Schub bilden — und kostet einen Supportfall je verzögertem Kennwort. + */ +it('drosselt die direkte Spur nicht', function () { + $mail = new ResetPasswordMail(User::factory()->create(), 'https://example.test/reset', 60); + + expect(taktAuftrag($mail)->middleware())->toBe([]); +}); + +it('meldet für die direkte Spur gar keine Drossel an', function () { + expect(RateLimiter::limiter(MailLane::DIRECT))->toBeNull(); +}); + +/** + * `Limit::perMinutes($minuten, $anzahl)` — Minuten zuerst. Die Reihenfolge ist + * anders herum, als man sie liest, und ein vertauschtes Paar wäre ein Takt von + * fünf Mails in dreißig Minuten statt dreißig in fünf. Deshalb stehen hier + * beide Zahlen einzeln, und zwar in Sekunden, wie die Drossel sie sieht. + */ +it('nimmt beide Kontingente aus den Einstellungen', function () { + Settings::set('mail.pace.urgent.count', 12); + Settings::set('mail.pace.urgent.minutes', 3); + Settings::set('mail.pace.calm.count', 7); + Settings::set('mail.pace.calm.minutes', 9); + + $wichtig = RateLimiter::limiter(MailLane::URGENT)(null); + $ruhig = RateLimiter::limiter(MailLane::CALM)(null); + + expect($wichtig->maxAttempts)->toBe(12) + ->and($wichtig->decaySeconds)->toBe(180) + ->and($ruhig->maxAttempts)->toBe(7) + ->and($ruhig->decaySeconds)->toBe(540); +}); + +it('taktet ohne Einstellung dreißig je fünf und zwanzig je zehn Minuten', function () { + $wichtig = RateLimiter::limiter(MailLane::URGENT)(null); + $ruhig = RateLimiter::limiter(MailLane::CALM)(null); + + expect($wichtig->maxAttempts)->toBe(30) + ->and($wichtig->decaySeconds)->toBe(300) + ->and($ruhig->maxAttempts)->toBe(20) + ->and($ruhig->decaySeconds)->toBe(600); +}); + +/** + * Der Notschalter für den Abend, an dem etwas anderes klemmt und zweihundert + * Rechnungen trotzdem heute raus müssen. + */ +it('lässt sich mit dem Notschalter ganz abstellen', function () { + Settings::set('mail.pace.enabled', false); + MailLane::assign(TaktProbeMail::class, MailLane::CALM); + + expect(taktAuftrag(new TaktProbeMail(1))->middleware())->toBe([]); +}); diff --git a/tests/Feature/Mail/SenderAddressTest.php b/tests/Feature/Mail/SenderAddressTest.php index 129b943..e6efc81 100644 --- a/tests/Feature/Mail/SenderAddressTest.php +++ b/tests/Feature/Mail/SenderAddressTest.php @@ -1,5 +1,6 @@ mailable->mailer === 'cp_maintenance'; }); });