Der Deckel hielt außerhalb der Transaktion, das Fenster versprach einen Block zu viel
Zwei Befunde der Codex-Durchsicht an derselben Grenze — höchstens drei Blöcke je Vertrag. Die Mengenprüfung stand in __invoke(), also außerhalb der Buchungstransaktion, und für Speicher wurde der Vertrag absichtlich nicht gesperrt. Zwei Speicher-Bestellungen desselben Vertrags nahe der Grenze lasen damit beide denselben alten Stand und fügten beide ein; der eindeutige Index über (order_id, addon_key) greift dabei nicht, weil es zwei verschiedene Bestellungen sind. Die Prüfung zählt jetzt in book(), unter der Sperre — und die vorhandene Sperre gilt zusätzlich für gedeckelte Module, statt eine zweite daneben zu nehmen. Der Idempotenz-Kurzschluss bleibt davor: eine wiederholte Zustellung derselben Bestellung bekommt ihre Buchung zurück, statt ausgerechnet am ausgefüllten Deckel zu scheitern (dieselbe Reihenfolge wie bei der Kapazitätsprüfung). Und im Bestätigungsfenster machte max(1, min($max, $packs)) aus null buchbaren Blöcken wieder einen — dieselbe Falle, die in Billing::purchase() schon behoben war. Bei null buchbaren Blöcken zeigt das Fenster jetzt die Absage in dem Satz, den der Kunde zu dieser Grenze überall sonst liest, statt einen Kauf anzubieten, den purchase() danach ablehnt. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>main
nexxo
parent
055eb90876
commit
da11d70037
|
|
@ -90,16 +90,6 @@ class BookAddon
|
|||
throw new RuntimeException($closed);
|
||||
}
|
||||
|
||||
// Der Deckel, an der Stelle, die ihn halten muss. Gezählt wird über
|
||||
// ALLE laufenden Buchungen dieses Moduls, nicht je Buchung: drei
|
||||
// Bestellungen à einem Block sind drei Blöcke, und eine Prüfung je
|
||||
// Bestellung hätte jede einzeln durchgewinkt.
|
||||
$overLimit = app(AddonCatalogue::class)->quantityRefusal($subscription, $addonKey, $quantity);
|
||||
|
||||
if ($overLimit !== null) {
|
||||
throw new RuntimeException($overLimit);
|
||||
}
|
||||
|
||||
try {
|
||||
$addon = $this->book($subscription, $addonKey, $quantity, $order, $price, $overrides);
|
||||
} catch (UniqueConstraintViolationException) {
|
||||
|
|
@ -129,12 +119,28 @@ class BookAddon
|
|||
$catalogue = app(AddonCatalogue::class);
|
||||
|
||||
// Held while we look and write, for the same reason the shop holds
|
||||
// the customer row: two clicks in flight would otherwise both find
|
||||
// nothing booked and both book, which is precisely the double
|
||||
// charge the check below exists to prevent. Only for a module of
|
||||
// which there may be one — a second storage pack is meant to
|
||||
// succeed, and serialising those would buy nothing.
|
||||
if ($catalogue->isEntitlement($addonKey)) {
|
||||
// the customer row: two bookings in flight would otherwise both
|
||||
// find the old state and both write. ONE lock, taken for both
|
||||
// reasons a booking can be refused on what the contract already
|
||||
// holds — taking a second one further down would be a second wait
|
||||
// for a row this transaction is already holding, and the two could
|
||||
// only ever be taken in the wrong order.
|
||||
//
|
||||
// A module of which there may be one: the short-circuit and
|
||||
// duplicateRefusal() below are read-then-write, and two clicks
|
||||
// would both find nothing booked and both book.
|
||||
//
|
||||
// A CAPPED module: a second storage pack is meant to succeed, so
|
||||
// this is deliberately not about serialising the booking — it is
|
||||
// about the count below. Without the lock two different storage
|
||||
// ORDERS near the cap both read the old total and both insert, and
|
||||
// the unique index on (order_id, addon_key) never sees it: two
|
||||
// orders are two different rows. That is a contract over the cap,
|
||||
// written by two ordinary purchases.
|
||||
//
|
||||
// A module with neither property is left unlocked; there is
|
||||
// nothing here that two of them could get wrong.
|
||||
if ($catalogue->isEntitlement($addonKey) || $catalogue->maxQuantity($addonKey) !== null) {
|
||||
Subscription::query()->whereKey($subscription->getKey())->lockForUpdate()->first();
|
||||
}
|
||||
|
||||
|
|
@ -150,6 +156,29 @@ class BookAddon
|
|||
}
|
||||
}
|
||||
|
||||
// Der Deckel, an der Stelle, die ihn halten muss. Gezählt wird über
|
||||
// ALLE laufenden Buchungen dieses Moduls, nicht je Buchung: drei
|
||||
// Bestellungen à einem Block sind drei Blöcke, und eine Prüfung je
|
||||
// Bestellung hätte jede einzeln durchgewinkt.
|
||||
//
|
||||
// HIER und nicht in __invoke(): gezählt werden darf erst unter der
|
||||
// Sperre oben, sonst lesen zwei gleichzeitige Bestellungen
|
||||
// denselben alten Stand und fügen beide ein — der eindeutige Index
|
||||
// über (order_id, addon_key) fängt das nicht, weil es zwei
|
||||
// verschiedene Bestellungen sind.
|
||||
//
|
||||
// Und wie die Kapazitätsprüfung darunter ABSICHTLICH hinter dem
|
||||
// Kurzschluss: ein zweites Mal zugestellter Webhook bekommt seine
|
||||
// bestehende Buchung zurück, statt gefragt zu werden, ob NOCH ein
|
||||
// Block passt — es kommt ja keiner hinzu. Davor gefragt, hätte die
|
||||
// Wiederholung ausgerechnet die Buchung abgelehnt, die den Deckel
|
||||
// gerade ausgefüllt hat, also jede Buchung des letzten Blocks.
|
||||
$overLimit = $catalogue->quantityRefusal($subscription, $addonKey, $quantity);
|
||||
|
||||
if ($overLimit !== null) {
|
||||
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
|
||||
|
|
|
|||
|
|
@ -28,6 +28,9 @@ class ConfirmBookStorage extends ModalComponent
|
|||
|
||||
public int $packGb = 0;
|
||||
|
||||
/** Warum es hier nichts zu kaufen gibt. Leer, solange es etwas gibt. */
|
||||
public string $refusal = '';
|
||||
|
||||
public function mount(int $packs = 1): void
|
||||
{
|
||||
$catalogue = app(AddonCatalogue::class);
|
||||
|
|
@ -44,13 +47,43 @@ class ConfirmBookStorage extends ModalComponent
|
|||
// absoluten Grenze: ein Vertrag, der schon einen Block hält, darf im
|
||||
// Dialog nicht mehr versprechen, als purchase() nachher wirklich in
|
||||
// den Warenkorb legt.
|
||||
//
|
||||
// Kein `max(1, …)` mehr darum herum: bei null buchbaren Blöcken —
|
||||
// Deckel erreicht oder Modul für dieses Paket nicht vorgesehen — machte
|
||||
// das daraus wieder einen. Dieselbe Falle, die in purchase() schon
|
||||
// behoben ist ($lines floorte auf eine Bestellzeile), hatte im Fenster
|
||||
// überlebt: ein direkt geöffnetes oder veraltetes Fenster versprach
|
||||
// einen Block und schickte eine Bestellung los, die purchase() danach
|
||||
// ablehnt. Nach oben wird weiter geklemmt, nach unten nur noch auf
|
||||
// null.
|
||||
$max = $catalogue->bookableQuantity($subscription, AddonCatalogue::STORAGE);
|
||||
$this->packs = max(1, min($max, $packs));
|
||||
$this->packs = min($max, max(1, $packs));
|
||||
$this->packGb = $catalogue->packGb();
|
||||
|
||||
// Absage statt geschlossenem Fenster: der Kunde hat gerade auf eine
|
||||
// Karte geklickt, die ihm diesen Ausweg angeboten hat, und ein Dialog,
|
||||
// der sich wortlos wieder schließt, ist von einem kaputten Knopf nicht
|
||||
// zu unterscheiden. In dem Satz, den er zu dieser Grenze überall sonst
|
||||
// liest — derselbe, mit dem purchase() und BookAddon ablehnen.
|
||||
$this->refusal = $this->packs > 0 ? '' : (string) (
|
||||
$catalogue->availabilityRefusal($subscription, AddonCatalogue::STORAGE)
|
||||
?? $catalogue->quantityRefusal($subscription, AddonCatalogue::STORAGE, 1)
|
||||
);
|
||||
}
|
||||
|
||||
public function proceed(): void
|
||||
{
|
||||
// Nichts buchbar, nichts losgeschickt — der Knopf, der hier ankommt,
|
||||
// ist bei einer Absage gar nicht gezeichnet. Die Menge steht in einer
|
||||
// öffentlichen Eigenschaft und damit im Browser, deshalb ist das hier
|
||||
// nicht die Grenze, sondern nur die Ehrlichkeit des Fensters: gehalten
|
||||
// wird sie in purchase() und in BookAddon, wo das Geld fließt.
|
||||
if ($this->packs < 1) {
|
||||
$this->closeModal();
|
||||
|
||||
return;
|
||||
}
|
||||
|
||||
$this->dispatch('storage-packs-confirmed', packs: $this->packs);
|
||||
$this->closeModal();
|
||||
}
|
||||
|
|
|
|||
|
|
@ -136,6 +136,7 @@ return [
|
|||
'storage_includes_packs' => 'Darin enthalten: :count × Zusatzspeicher (+:gb GB).',
|
||||
'storage_confirm_title' => ':count × Zusatzspeicher buchen?',
|
||||
'storage_confirm_body' => 'Damit werden :count Speicherpakete (+:gb GB) vorgemerkt. Das sind :total netto pro Monat, monatlich kündbar.',
|
||||
'storage_confirm_blocked_title' => 'Kein weiterer Zusatzspeicher buchbar',
|
||||
'storage_confirm_cancel' => 'Abbrechen',
|
||||
'storage_confirm_cta' => 'Vormerken',
|
||||
'storage_remeasured' => 'Ihr Speicherverbrauch wurde neu gemessen.',
|
||||
|
|
|
|||
|
|
@ -135,6 +135,7 @@ return [
|
|||
'storage_includes_packs' => 'Includes :count × extra storage (+:gb GB).',
|
||||
'storage_confirm_title' => 'Add :count × extra storage?',
|
||||
'storage_confirm_body' => 'This places :count storage packs (+:gb GB) in your cart. That is :total net per month, cancellable monthly.',
|
||||
'storage_confirm_blocked_title' => 'No further storage can be booked',
|
||||
'storage_confirm_cancel' => 'Cancel',
|
||||
'storage_confirm_cta' => 'Add to cart',
|
||||
'storage_remeasured' => 'Your storage usage has been measured again.',
|
||||
|
|
|
|||
|
|
@ -4,25 +4,44 @@
|
|||
$loc = app()->getLocale();
|
||||
$priceCents = (int) config('provisioning.storage_addon.price_cents', 0);
|
||||
@endphp
|
||||
<div class="flex items-start gap-3">
|
||||
<span class="grid size-10 shrink-0 place-items-center rounded-lg bg-accent-subtle text-accent-active">
|
||||
<x-ui.icon name="database" class="size-5" />
|
||||
</span>
|
||||
<div class="min-w-0">
|
||||
<h3 class="text-base font-semibold text-ink">{{ __('billing.storage_confirm_title', ['count' => $packs]) }}</h3>
|
||||
<p class="mt-1 text-sm text-muted">
|
||||
{{ __('billing.storage_confirm_body', [
|
||||
'count' => $packs,
|
||||
'gb' => $packs * $packGb,
|
||||
'total' => Number::currency($packs * $priceCents / 100, in: 'EUR', locale: $loc),
|
||||
]) }}
|
||||
</p>
|
||||
{{-- Null buchbare Blöcke: Absage statt Kaufzusage. Das Fenster wird von
|
||||
einer Karte geöffnet, deren Zahl älter sein kann als der Vertrag — und
|
||||
ein Dialog, der einen Block verspricht, den purchase() danach ablehnt,
|
||||
ist eine Falle. --}}
|
||||
@if ($packs < 1)
|
||||
<div class="flex items-start gap-3">
|
||||
<span class="grid size-10 shrink-0 place-items-center rounded-lg bg-warning-bg text-warning">
|
||||
<x-ui.icon name="alert-triangle" class="size-5" />
|
||||
</span>
|
||||
<div class="min-w-0">
|
||||
<h3 class="text-base font-semibold text-ink">{{ __('billing.storage_confirm_blocked_title') }}</h3>
|
||||
<p class="mt-1 text-sm text-muted">{{ $refusal }}</p>
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
<div class="mt-6 flex justify-end gap-3">
|
||||
<x-ui.button variant="secondary" x-on:click="Livewire.dispatch('closeModal')">{{ __('billing.storage_confirm_cancel') }}</x-ui.button>
|
||||
<x-ui.button variant="primary" wire:click="proceed" wire:loading.attr="disabled">
|
||||
<x-ui.icon name="database" class="size-4" />{{ __('billing.storage_confirm_cta') }}
|
||||
</x-ui.button>
|
||||
</div>
|
||||
<div class="mt-6 flex justify-end">
|
||||
<x-ui.button variant="secondary" x-on:click="Livewire.dispatch('closeModal')">{{ __('common.close') }}</x-ui.button>
|
||||
</div>
|
||||
@else
|
||||
<div class="flex items-start gap-3">
|
||||
<span class="grid size-10 shrink-0 place-items-center rounded-lg bg-accent-subtle text-accent-active">
|
||||
<x-ui.icon name="database" class="size-5" />
|
||||
</span>
|
||||
<div class="min-w-0">
|
||||
<h3 class="text-base font-semibold text-ink">{{ __('billing.storage_confirm_title', ['count' => $packs]) }}</h3>
|
||||
<p class="mt-1 text-sm text-muted">
|
||||
{{ __('billing.storage_confirm_body', [
|
||||
'count' => $packs,
|
||||
'gb' => $packs * $packGb,
|
||||
'total' => Number::currency($packs * $priceCents / 100, in: 'EUR', locale: $loc),
|
||||
]) }}
|
||||
</p>
|
||||
</div>
|
||||
</div>
|
||||
<div class="mt-6 flex justify-end gap-3">
|
||||
<x-ui.button variant="secondary" x-on:click="Livewire.dispatch('closeModal')">{{ __('billing.storage_confirm_cancel') }}</x-ui.button>
|
||||
<x-ui.button variant="primary" wire:click="proceed" wire:loading.attr="disabled">
|
||||
<x-ui.icon name="database" class="size-4" />{{ __('billing.storage_confirm_cta') }}
|
||||
</x-ui.button>
|
||||
</div>
|
||||
@endif
|
||||
</div>
|
||||
|
|
|
|||
|
|
@ -8,6 +8,7 @@ use App\Models\Order;
|
|||
use App\Models\Subscription;
|
||||
use App\Models\User;
|
||||
use App\Services\Billing\AddonCatalogue;
|
||||
use Illuminate\Support\Facades\DB;
|
||||
use Livewire\Livewire;
|
||||
|
||||
/**
|
||||
|
|
@ -32,6 +33,14 @@ function limitUser(Subscription $subscription): User
|
|||
return User::factory()->create(['email' => $subscription->customer->email]);
|
||||
}
|
||||
|
||||
/** Eine bezahlte Speicher-Bestellung dieses Kunden — eine Buchung hat eine. */
|
||||
function limitOrder(Subscription $subscription): Order
|
||||
{
|
||||
return Order::factory()->create([
|
||||
'customer_id' => $subscription->customer_id, 'type' => 'addon', 'addon_key' => 'storage', 'status' => 'paid',
|
||||
]);
|
||||
}
|
||||
|
||||
it('lehnt den vierten Block auf einmal ab', function () {
|
||||
$subscription = limitContract();
|
||||
|
||||
|
|
@ -120,3 +129,104 @@ it('klemmt das Bestätigungs-Modal an der Restmenge, nicht an der absoluten Gren
|
|||
->call('proceed')
|
||||
->assertDispatched('storage-packs-confirmed', packs: 2);
|
||||
});
|
||||
|
||||
/**
|
||||
* Nachtrag aus der Durchsicht (Befund 1): der Deckel stand außerhalb der
|
||||
* Transaktion, und für Speicher wurde der Vertrag gar nicht gesperrt.
|
||||
*
|
||||
* Zwei Speicher-Bestellungen desselben Vertrags, gleichzeitig verarbeitet,
|
||||
* lasen damit beide denselben alten Stand und fügten beide ein — der eindeutige
|
||||
* Index über (order_id, addon_key) greift nicht, weil es zwei verschiedene
|
||||
* Bestellungen sind. Ergebnis: ein Vertrag über dem Deckel.
|
||||
*
|
||||
* WAS DIESER TEST BEWEIST: dass der Vertrag auch für ein Mengen-Modul gesperrt
|
||||
* wird, dass die Zählung NACH dieser Sperre kommt und dass beide innerhalb der
|
||||
* Buchungstransaktion laufen — die Reihenfolge, ohne die eine Sperre nichts
|
||||
* bewirkt.
|
||||
*
|
||||
* WAS ER NICHT BEWEIST: dass zwei gleichzeitige Buchungen einander wirklich
|
||||
* ausschließen. Die Suite läuft auf SQLite, dort ist `lockForUpdate()` ein
|
||||
* No-op (SQLiteGrammar::compileLock() gibt '' zurück) und ein zweiter Prozess
|
||||
* ist im selben PHP-Aufruf nicht herstellbar. Echte Nebenläufigkeit hängt an
|
||||
* MariaDB und ist hier nicht nachgestellt; nachgewiesen wird die
|
||||
* Voraussetzung dafür, nicht die Wirkung.
|
||||
*/
|
||||
it('zählt den Deckel unter der Vertragssperre und in derselben Transaktion, die bucht', function () {
|
||||
$subscription = limitContract();
|
||||
$order = limitOrder($subscription);
|
||||
|
||||
// RefreshDatabase hält den ganzen Test in einer Transaktion. Die Tiefe VOR
|
||||
// dem Aufruf ist deshalb der Nullpunkt, gegen den "innerhalb der
|
||||
// Buchungstransaktion" gemessen wird — absolut wäre 1 schon außerhalb.
|
||||
$baseline = DB::transactionLevel();
|
||||
$log = [];
|
||||
|
||||
DB::listen(function ($query) use (&$log) {
|
||||
$log[] = ['sql' => $query->sql, 'level' => DB::transactionLevel()];
|
||||
});
|
||||
|
||||
app(BookAddon::class)($subscription, AddonCatalogue::STORAGE, 1, $order);
|
||||
|
||||
// Die Sperre ist die erste Anweisung der Transaktion und die einzige
|
||||
// Abfrage auf `subscriptions` in diesem Pfad; gezählt wird über
|
||||
// sum(quantity) auf den Buchungen.
|
||||
$lock = collect($log)->search(fn (array $statement) => str_contains($statement['sql'], 'from "subscriptions"'));
|
||||
$count = collect($log)->search(fn (array $statement) => str_contains($statement['sql'], 'sum("quantity")'));
|
||||
|
||||
expect($lock)->not->toBeFalse()
|
||||
->and($count)->not->toBeFalse()
|
||||
->and($count)->toBeGreaterThan($lock)
|
||||
->and($log[$lock]['level'])->toBeGreaterThan($baseline)
|
||||
->and($log[$count]['level'])->toBeGreaterThan($baseline);
|
||||
});
|
||||
|
||||
/**
|
||||
* Und die andere Hälfte derselben Verschiebung: der Kurzschluss bleibt vorn.
|
||||
*
|
||||
* Dieselbe Lektion wie bei der Kapazitätsprüfung eine Runde zuvor (siehe
|
||||
* StoragePackCapacityTest): Stripe stellt denselben Webhook nach einem Timeout
|
||||
* ein zweites Mal zu. Läge die Mengenprüfung vor dem Kurzschluss, würde die
|
||||
* Wiederholung fragen "passt NOCH ein Block drauf" — obwohl gar keiner
|
||||
* hinzukommt — und einen längst bezahlten, längst gebuchten Vorgang genau dann
|
||||
* mit einem Fehler quittieren, wenn die Buchung selbst den Deckel erreicht hat.
|
||||
*/
|
||||
it('gibt bei einem wiederholten Webhook die vorhandene Buchung zurück, auch am erreichten Deckel', function () {
|
||||
$subscription = limitContract();
|
||||
app(BookAddon::class)($subscription, AddonCatalogue::STORAGE, 2, limitOrder($subscription));
|
||||
|
||||
$order = limitOrder($subscription);
|
||||
|
||||
// Der dritte Block füllt den Deckel genau aus.
|
||||
$first = app(BookAddon::class)($subscription, AddonCatalogue::STORAGE, 1, $order);
|
||||
$second = app(BookAddon::class)($subscription, AddonCatalogue::STORAGE, 1, $order);
|
||||
|
||||
expect($second->id)->toBe($first->id)
|
||||
->and((int) $subscription->addons()->active()->sum('quantity'))->toBe(3);
|
||||
});
|
||||
|
||||
/**
|
||||
* Nachtrag aus der Durchsicht (Befund 3): dieselbe `max(1, …)`-Falle, die in
|
||||
* purchase() schon behoben war, hatte im Fenster überlebt.
|
||||
*
|
||||
* Bei null buchbaren Blöcken machte `max(1, min($max, $packs))` daraus wieder
|
||||
* einen — das Fenster versprach einen Block und schickte eine Bestellung los,
|
||||
* die purchase() danach ablehnt. Ein Knopf, der etwas anbietet, das die Aktion
|
||||
* dahinter ablehnt, ist eine Falle.
|
||||
*/
|
||||
it('bietet am erreichten Deckel keinen Kauf aus dem Bestätigungsfenster an', function () {
|
||||
$subscription = limitContract();
|
||||
app(BookAddon::class)($subscription, AddonCatalogue::STORAGE, 3);
|
||||
|
||||
Livewire::actingAs(limitUser($subscription))
|
||||
->test(ConfirmBookStorage::class, ['packs' => 1])
|
||||
->assertSet('packs', 0)
|
||||
// Keine Kaufzusage, sondern die Absage in dem Satz, den der Kunde
|
||||
// überall sonst zu dieser Grenze liest.
|
||||
->assertDontSee(__('billing.storage_confirm_cta'))
|
||||
->assertSee(__('billing.addon_limit_reached', [
|
||||
'module' => app(AddonCatalogue::class)->name(AddonCatalogue::STORAGE),
|
||||
'max' => 3,
|
||||
]))
|
||||
->call('proceed')
|
||||
->assertNotDispatched('storage-packs-confirmed');
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in New Issue