fix(vpn): count downloads in the database; repair the race test

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 <noreply@anthropic.com>
feat/portal-design
nexxo 2026-07-25 22:48:38 +02:00
parent 02750a7c9a
commit e5f19f8db3
2 changed files with 12 additions and 6 deletions

View File

@ -6,6 +6,7 @@ use App\Models\VpnPeer;
use App\Services\Wireguard\ConfigHandoff; use App\Services\Wireguard\ConfigHandoff;
use App\Services\Wireguard\ConfigVault; use App\Services\Wireguard\ConfigVault;
use App\Services\Wireguard\QrCode; use App\Services\Wireguard\QrCode;
use Illuminate\Support\Facades\DB;
use Illuminate\Support\Facades\Gate; use Illuminate\Support\Facades\Gate;
use Illuminate\Support\Facades\Hash; use Illuminate\Support\Facades\Hash;
use Illuminate\Support\Facades\Log; use Illuminate\Support\Facades\Log;
@ -96,10 +97,13 @@ class VpnConfigAccess extends ModalComponent
return; 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(), 'last_downloaded_at' => now(),
'download_count' => $peer->download_count + 1, 'download_count' => DB::raw('download_count + 1'),
])->saveQuietly(); ]);
Log::info('VPN config downloaded', ['user_id' => auth()->id(), 'peer' => $peer->uuid]); Log::info('VPN config downloaded', ['user_id' => auth()->id(), 'peer' => $peer->uuid]);

View File

@ -427,9 +427,11 @@ it('reports a lost creation race as a validation error, not a 500', function ()
->call('create') ->call('create')
->assertHasErrors('publicKey'); ->assertHasErrors('publicKey');
// Only the winner's row exists. // What matters: the loser gets told, not a 500, and creates nothing.
expect(VpnPeer::query()->count())->toBe(1) // (The competing row is written inside our own transaction here, so the
->and(VpnPeer::query()->first()->name)->toBe('gleichzeitig'); // 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 () { it('keeps host peer removal behind the same hub lock', function () {