From ce7d0426cb4da984868199ed5dc9e78154e42803 Mon Sep 17 00:00:00 2001 From: nexxo Date: Sat, 1 Aug 2026 16:21:46 +0200 Subject: [PATCH] Fix-Runde: preferredDatacenter() blind fuer Reservierung, Loeschen hob sie auf MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit preferredDatacenter() waehlte das Rechenzentrum ueber Host::availableGb() ohne ->unreserved() - ein Rechenzentrum mit einem grossen, aber komplett reservierten Host sah geraeumiger aus als eines mit echtem allgemeinem Bestand. Der Checkout haette die Bestellung dorthin gelegt, placeableIn() haette dort niemanden gefunden, und sie waere geparkt, obwohl anderswo Platz war. Jetzt ->unreserved(), derselbe Bestand wie largestPlaceableGb(). Die Migration nutzte nullOnDelete() und tat damit das Gegenteil der eigenen Vorgabe: die Reservierung sollte einen Kundenaustritt nicht stillschweigend ueberleben, loeste sich mit nullOnDelete() aber genau so auf, sobald der Kunde verschwindet. restrictOnDelete() macht das Loeschen eines Kunden mit eigener Maschine zum Fehler, bis ein Operator die Reservierung von Hand gelöst hat - Migration und Modellkommentar sagen jetzt dasselbe. Co-Authored-By: Claude Opus 5 --- app/Models/Host.php | 8 +-- app/Services/Provisioning/HostCapacity.php | 8 +++ ..._08_01_000003_add_reservation_to_hosts.php | 9 +++- tests/Feature/Admin/HostReservationTest.php | 51 +++++++++++++++++++ 4 files changed, 72 insertions(+), 4 deletions(-) diff --git a/app/Models/Host.php b/app/Models/Host.php index 5834dd9..f821cb9 100644 --- a/app/Models/Host.php +++ b/app/Models/Host.php @@ -106,9 +106,11 @@ class Host extends Model implements ProvisioningSubject /** * Wem diese Maschine als "eigener Server" versprochen ist — null heißt - * allgemeiner Bestand. Bewusst ohne `nullOnDelete`-Kaskade im Modell: die - * Migration löscht die Reservierung schon auf Datenbankebene, wenn der - * Kunde verschwindet (siehe deren Kopfkommentar). + * allgemeiner Bestand. Die Migration setzt `restrictOnDelete()`, nicht + * `nullOnDelete()`: ein Kunde, dem noch eine Maschine gehört, lässt sich + * nicht löschen, solange niemand die Reservierung von Hand gelöst hat + * (siehe deren Kopfkommentar) — sonst verschwände sie kommentarlos aus + * dem Bestand, ohne dass ein Operator sie tatsächlich freigegeben hat. */ public function reservedFor(): BelongsTo { diff --git a/app/Services/Provisioning/HostCapacity.php b/app/Services/Provisioning/HostCapacity.php index fa661c8..8218c57 100644 --- a/app/Services/Provisioning/HostCapacity.php +++ b/app/Services/Provisioning/HostCapacity.php @@ -217,6 +217,13 @@ class HostCapacity * whose roomiest host has the most left, which is where the order will * actually roll out fastest. * + * ->unreserved(): this runs before a customer exists (Stripe checkout + * metadata), so nobody to reserve FOR is known yet — the same "general + * stock" question largestPlaceableGb() asks. Without it, a datacenter + * whose biggest host is entirely somebody else's "eigener Server" looks + * roomiest, the order is sent there, and placeableIn() then finds nobody + * — parking an order that another datacenter had real room for. + * * Falls back to the first configured datacenter, and then to the same * literal the Stripe webhook has always defaulted to, so a checkout is * never blocked by an estate that has not been built yet. @@ -225,6 +232,7 @@ class HostCapacity { $best = Host::query() ->where('status', 'active') + ->unreserved() ->get() ->sortByDesc(fn (Host $host) => $host->availableGb()) ->first(); diff --git a/database/migrations/2026_08_01_000003_add_reservation_to_hosts.php b/database/migrations/2026_08_01_000003_add_reservation_to_hosts.php index 6ff929b..428938a 100644 --- a/database/migrations/2026_08_01_000003_add_reservation_to_hosts.php +++ b/database/migrations/2026_08_01_000003_add_reservation_to_hosts.php @@ -16,6 +16,13 @@ use Illuminate\Support\Facades\Schema; * ein Host überlebt seinen Mieter, und die Reservierung wird beim Auszug von * Hand gelöst — sonst stünde die Maschine still im Bestand, ohne dass jemand * sie angeboten hätte. + * + * Deshalb `restrictOnDelete()`, nicht `nullOnDelete()`: die Datenbank selbst + * darf die Reservierung nicht stillschweigend aufheben, wenn der + * Kundendatensatz verschwindet (Löschung nach DSGVO wäre der naheliegende + * Weg dahin). Ein Kunde, dem noch eine Maschine gehört, lässt sich nicht + * löschen, bis ein Operator sie von Hand freigegeben hat — das Lösen bleibt + * eine bewusste Handlung, nicht ein Nebeneffekt einer anderen Löschung. */ return new class extends Migration { @@ -23,7 +30,7 @@ return new class extends Migration { Schema::table('hosts', function (Blueprint $table) { $table->foreignId('reserved_for_customer_id')->nullable()->after('datacenter') - ->constrained('customers')->nullOnDelete(); + ->constrained('customers')->restrictOnDelete(); }); } diff --git a/tests/Feature/Admin/HostReservationTest.php b/tests/Feature/Admin/HostReservationTest.php index 511736d..5abe5b5 100644 --- a/tests/Feature/Admin/HostReservationTest.php +++ b/tests/Feature/Admin/HostReservationTest.php @@ -45,6 +45,57 @@ it('lässt einen freien Host frei', function () { expect(app(HostCapacity::class)->canPlace('fsn', 40))->toBeTrue(); }); +/** + * preferredDatacenter() ruft placeableIn() nicht auf — sie fragt selbst nach + * dem Rechenzentrum mit dem meisten freien Platz, für einen Checkout, der + * noch keinen Kunden kennt. Ohne ->unreserved() sieht ein Rechenzentrum mit + * einem großen, aber komplett reservierten Host geräumiger aus als eines mit + * echtem allgemeinem Bestand — der Checkout legt die neue Bestellung dorthin, + * placeableIn() findet dort dann niemanden, und die Bestellung parkt, obwohl + * anderswo echter Platz war. + */ +it('bevorzugt kein Rechenzentrum, dessen größter Host nur reservierter Platz ist', function () { + App\Models\Datacenter::factory()->create(['code' => 'fsn4']); + App\Models\Datacenter::factory()->create(['code' => 'hel4']); + + $mieter = Customer::factory()->create(); + // fsn4: der größere Host, aber vollständig einem Kunden versprochen. + Host::factory()->active()->create(['datacenter' => 'fsn4', 'total_gb' => 4000, 'reserved_for_customer_id' => $mieter->id]); + // hel4: kleiner, aber echter allgemeiner Bestand. + Host::factory()->active()->create(['datacenter' => 'hel4', 'total_gb' => 500]); + + expect(app(HostCapacity::class)->preferredDatacenter())->toBe('hel4'); +}); + +/** + * Die Reservierung überlebt den Kunden — sie löst sich nicht von selbst. + * + * Vorgabe: "Die Beziehung löscht nicht mit dem Kunden … die Reservierung + * wird beim Auszug von Hand gelöst — sonst stünde die Maschine still im + * Bestand, ohne dass jemand sie angeboten hätte." Ein `nullOnDelete()` täte + * genau das Verbotene: die Datenbank hebt die Reservierung von sich aus auf, + * sobald der Kunde verschwindet (z. B. eine DSGVO-Löschung). `restrictOnDelete()` + * macht daraus stattdessen eine bewusste Handlung — das Löschen selbst + * scheitert, solange die Maschine noch jemandem gehört. + */ +it('verhindert das Löschen eines Kunden, dem noch ein Host gehört', function () { + $mieter = Customer::factory()->create(); + Host::factory()->active()->create(['reserved_for_customer_id' => $mieter->id]); + + expect(fn () => $mieter->delete())->toThrow(\Illuminate\Database\QueryException::class); + + // Nichts wurde stillschweigend aufgelöst — der Kunde steht noch da. + expect(Customer::query()->whereKey($mieter->id)->exists())->toBeTrue(); +}); + +it('lässt einen Kunden ohne Host normal löschen', function () { + $customer = Customer::factory()->create(); + + $customer->delete(); + + expect(Customer::query()->whereKey($customer->id)->exists())->toBeFalse(); +}); + /** * Die Konsole: setzen und lösen. *