From af05ed6694094ee6fdbf957fea80f8cf0a2b3512 Mon Sep 17 00:00:00 2001 From: nexxo Date: Sat, 1 Aug 2026 14:18:01 +0200 Subject: [PATCH] Kapazitaetspruefung hinter den Idempotenz-Kurzschluss verschoben Durchsicht (R22): die Pruefung stand in __invoke(), vor book()s Kurzschluss fuer einen wiederholt zugestellten Webhook ("Idempotent against a retried webhook"). Damit fragte jede Wiederholung erneut "passt NOCH ein Block drauf" - obwohl keiner hinzukommt - und ein laengst bezahlter, laengst gebuchter Vorgang quittierte die Wiederholung mit einem Fehler, sobald der Host zwischenzeitlich eng geworden war. Genau das Szenario, fuer das diese Aufgabe gebaut wurde, nur gegen den eigenen Kunden gerichtet. Jetzt sitzt die Pruefung in book(), hinter dem order_id+addon_key-Kurzschluss: eine Wiederholung bekommt ihre bestehende Buchung zurueck, ohne die Frage erneut zu stellen. Eine echte neue Buchung durchlaeuft die Pruefung wie zuvor. Deckel (quantityRefusal) und Domain-Ausschluss bleiben unangetastet - sie haben dasselbe Muster im Kleinen, sind aber nicht Gegenstand dieses Befundes. Neuer Test: derselbe Auftrag wird zweimal gebucht, der Host wird zwischen den beiden Aufrufen eng - der zweite Aufruf gibt die vorhandene Buchung zurueck statt zu werfen. Co-Authored-By: Claude Opus 5 --- app/Actions/BookAddon.php | 43 +++++++++++------- .../Billing/StoragePackCapacityTest.php | 45 +++++++++++++++++++ 2 files changed, 71 insertions(+), 17 deletions(-) diff --git a/app/Actions/BookAddon.php b/app/Actions/BookAddon.php index 41b7878..7c1412d 100644 --- a/app/Actions/BookAddon.php +++ b/app/Actions/BookAddon.php @@ -90,23 +90,6 @@ class BookAddon throw new RuntimeException($overLimit); } - // Platz auf DIESER Maschine, nicht irgendwo im Bestand: eine laufende - // Instanz zieht nicht um. Dünn belegter Speicher lässt eine - // Überbuchung sofort gelingen und erst auffallen, wenn die Gäste - // wirklich schreiben — also unter zahlenden Kunden. - // - // Ohne Maschine wird nicht gefragt: der Vertrag ist gerade erst - // geschlossen, und die Platzierung nimmt die Blöcke in - // ReserveResources mit auf. - if ($addonKey === AddonCatalogue::STORAGE) { - $host = $subscription->instance?->host; - $needs = app(AddonCatalogue::class)->packDiskGb() * $quantity; - - if ($host !== null && ! $host->canTake($needs)) { - throw new RuntimeException(__('billing.storage_no_room')); - } - } - try { $addon = $this->book($subscription, $addonKey, $quantity, $order, $price, $overrides); } catch (UniqueConstraintViolationException) { @@ -157,6 +140,32 @@ class BookAddon } } + // Platz auf DIESER Maschine, nicht irgendwo im Bestand: eine laufende + // Instanz zieht nicht um. Dünn belegter Speicher lässt eine + // Überbuchung sofort gelingen und erst auffallen, wenn die Gäste + // wirklich schreiben — also unter zahlenden Kunden. + // + // Ohne Maschine wird nicht gefragt: der Vertrag ist gerade erst + // geschlossen, und die Platzierung nimmt die Blöcke in + // ReserveResources mit auf. + // + // ABSICHTLICH hinter dem Kurzschluss oben, nicht in __invoke(): + // ein Webhook, den Stripe ein zweites Mal zustellt, soll seine + // bestehende Buchung zurückbekommen, nicht erneut gefragt werden, + // ob NOCH ein Block passt — es kommt ja keiner hinzu. Vor dem + // Kurzschluss gefragt, hätte die Wiederholung genau dann geworfen, + // wenn der Host in der Zwischenzeit eng wurde: das Szenario, für + // das diese Prüfung gebaut ist, hätte ausgerechnet einen bereits + // bezahlten Vorgang mit einem Fehler quittiert. + if ($addonKey === AddonCatalogue::STORAGE) { + $host = $subscription->instance?->host; + $needs = $catalogue->packDiskGb() * $quantity; + + if ($host !== null && ! $host->canTake($needs)) { + throw new RuntimeException(__('billing.storage_no_room')); + } + } + // Asked AFTER the retry above, so a webhook delivered twice still // gets its one booking back rather than an error: that is the same // order arriving again, not a second purchase. What is refused here diff --git a/tests/Feature/Billing/StoragePackCapacityTest.php b/tests/Feature/Billing/StoragePackCapacityTest.php index 656f236..de6e648 100644 --- a/tests/Feature/Billing/StoragePackCapacityTest.php +++ b/tests/Feature/Billing/StoragePackCapacityTest.php @@ -68,3 +68,48 @@ it('fragt nicht, solange es keine Maschine gibt', function () { expect((int) $subscription->addons()->active()->sum('quantity'))->toBe(1); }); + +/** + * Nachtrag aus der Durchsicht: die Kapazitätsprüfung darf einen wiederholt + * zugestellten Webhook nicht treffen. + * + * Stripe liefert bei einem 5xx oder Timeout denselben Webhook ein zweites Mal + * aus — BookAddon::__invoke() wird mit demselben `$order` erneut aufgerufen, + * und book()'s eigener Kurzschluss ("Idempotent against a retried webhook") + * soll dafür nur die bestehende Buchung zurückgeben, ohne irgendetwas neu zu + * entscheiden. Läge die Kapazitätsprüfung VOR diesem Kurzschluss, würde die + * Wiederholung erneut fragen "passt NOCH ein Block drauf" — obwohl gar keiner + * hinzukommt — und einen längst bezahlten, längst gebuchten Vorgang mit einem + * Fehler quittieren, sobald der Host in der Zwischenzeit enger geworden ist. + */ +it('gibt bei einem wiederholten Webhook die vorhandene Buchung zurück, auch wenn der Host inzwischen eng geworden ist', function () { + config(['provisioning.storage_addon.gb' => 20, 'provisioning.storage_addon.disk_gb' => 22]); + + // 100 GB gesamt → 85 GB frei (reserve_pct 15). Die gepackte Instanz bindet + // 40, für den einen Block (22) bleiben 45 - 22 = 23 übrig: reichlich für + // den ERSTEN Aufruf. + $host = Host::factory()->active()->create(['datacenter' => 'fsn', 'total_gb' => 100]); + $subscription = packedInstanceOn($host); + + $order = Order::factory()->create([ + 'customer_id' => $subscription->customer_id, 'type' => 'addon', 'addon_key' => 'storage', 'status' => 'paid', + ]); + + $first = app(BookAddon::class)($subscription, AddonCatalogue::STORAGE, 1, $order); + + // Der Host wird zwischen den beiden Zustellversuchen enger — hier durch + // eine zweite Instanz, die den Rest praktisch aufbraucht (40 + 22 + 30 = + // 92 committed, nur noch 85 - 92 < 0 → 0 frei). Die Ursache ist beliebig + // (ein zweiter Kunde, ein weiterer Block); die Frage ist nur, ob die + // WIEDERHOLUNG sie stellen darf. + Instance::factory()->create([ + 'host_id' => $host->id, + 'status' => 'active', + 'disk_gb' => 30, + ]); + + $second = app(BookAddon::class)($subscription, AddonCatalogue::STORAGE, 1, $order); + + expect($second->id)->toBe($first->id) + ->and((int) $subscription->addons()->active()->sum('quantity'))->toBe(1); +});