From e5f19f8db369b8a940e0a6aabf84fa9224dcd0d0 Mon Sep 17 00:00:00 2001 From: nexxo Date: Sat, 25 Jul 2026 22:48:38 +0200 Subject: [PATCH] fix(vpn): count downloads in the database; repair the race test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The download counter was read-modify-written, so two concurrent retrievals recorded one. It is an audit trail — under-reporting is the failure mode it must not have. The creation-race test broke when issuing moved into a transaction: its simulated competitor writes inside our transaction, so the rollback removes that row too. It now asserts what actually matters (the loser is told, and creates nothing) and states what a single-connection sqlite test cannot show. This test was red in the previous commit — my mistake for committing before reading the run. Co-Authored-By: Claude Opus 4.8 --- app/Livewire/Admin/VpnConfigAccess.php | 10 +++++++--- tests/Feature/Admin/VpnTest.php | 8 +++++--- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/app/Livewire/Admin/VpnConfigAccess.php b/app/Livewire/Admin/VpnConfigAccess.php index 67c833b..c9179fb 100644 --- a/app/Livewire/Admin/VpnConfigAccess.php +++ b/app/Livewire/Admin/VpnConfigAccess.php @@ -6,6 +6,7 @@ use App\Models\VpnPeer; use App\Services\Wireguard\ConfigHandoff; use App\Services\Wireguard\ConfigVault; use App\Services\Wireguard\QrCode; +use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Gate; use Illuminate\Support\Facades\Hash; use Illuminate\Support\Facades\Log; @@ -96,10 +97,13 @@ class VpnConfigAccess extends ModalComponent return; } - $peer->forceFill([ + // Counted in the database, not read-modify-written here: two concurrent + // retrievals would otherwise record one. This is an audit trail, so + // under-reporting is the one failure mode it must not have. + VpnPeer::query()->whereKey($peer->getKey())->update([ 'last_downloaded_at' => now(), - 'download_count' => $peer->download_count + 1, - ])->saveQuietly(); + 'download_count' => DB::raw('download_count + 1'), + ]); Log::info('VPN config downloaded', ['user_id' => auth()->id(), 'peer' => $peer->uuid]); diff --git a/tests/Feature/Admin/VpnTest.php b/tests/Feature/Admin/VpnTest.php index 44df093..290b138 100644 --- a/tests/Feature/Admin/VpnTest.php +++ b/tests/Feature/Admin/VpnTest.php @@ -427,9 +427,11 @@ it('reports a lost creation race as a validation error, not a 500', function () ->call('create') ->assertHasErrors('publicKey'); - // Only the winner's row exists. - expect(VpnPeer::query()->count())->toBe(1) - ->and(VpnPeer::query()->first()->name)->toBe('gleichzeitig'); + // What matters: the loser gets told, not a 500, and creates nothing. + // (The competing row is written inside our own transaction here, so the + // rollback removes it too — a real competitor's row would survive, which a + // single-connection sqlite test cannot reproduce.) + expect(VpnPeer::withTrashed()->where('name', 'verloren')->count())->toBe(0); }); it('keeps host peer removal behind the same hub lock', function () {