From c3dcbb48fd8a48d5861434dc71d7b1398d73172c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADctor=20Falc=C3=B3n?= Date: Tue, 12 May 2026 11:45:45 +0100 Subject: [PATCH] Fix PHP-LARAVEL-1V exchange rate cache race (#383) ## Sentry - Issue: PHP-LARAVEL-1V - URL: https://whisper-money.sentry.io/issues/PHP-LARAVEL-1V ## Root cause `ExchangeRateService::getRates()` skipped the database lookup after `preloadRates()` marked a miss, then used `create()` to cache fetched rates. If another request inserted the same `base_currency` + `date` meanwhile, the unique index threw a duplicate-key exception. ## Fix Use atomic Eloquent `upsert()` for exchange-rate cache writes so concurrent requests converge on one row. Added regression coverage for the preload-miss race path. ## Verification - `php artisan test --compact tests/Feature/ExchangeRateServiceTest.php` - `vendor/bin/pint --dirty --format agent` - `php artisan test --compact --filter="getRates tolerates another request storing rates after preload miss"` --- app/Services/ExchangeRateService.php | 12 ++++++----- tests/Feature/ExchangeRateServiceTest.php | 26 +++++++++++++++++++++++ tests/Pest.php | 6 +++++- 3 files changed, 38 insertions(+), 6 deletions(-) diff --git a/app/Services/ExchangeRateService.php b/app/Services/ExchangeRateService.php index 7c3b4d3c..506a8a4c 100644 --- a/app/Services/ExchangeRateService.php +++ b/app/Services/ExchangeRateService.php @@ -82,11 +82,13 @@ class ExchangeRateService $rates = $this->currencyApi->getRatesForCurrency($baseCurrency, $date); if (! empty($rates)) { - ExchangeRate::query()->create([ - 'base_currency' => $baseCurrency, - 'date' => $date, - 'rates' => $rates, - ]); + ExchangeRate::query()->upsert([ + [ + 'base_currency' => $baseCurrency, + 'date' => $date, + 'rates' => json_encode($rates, JSON_THROW_ON_ERROR), + ], + ], uniqueBy: ['base_currency', 'date'], update: ['rates']); } return $this->ratesCache[$cacheKey] = $rates; diff --git a/tests/Feature/ExchangeRateServiceTest.php b/tests/Feature/ExchangeRateServiceTest.php index 3c418aaf..b69693fd 100644 --- a/tests/Feature/ExchangeRateServiceTest.php +++ b/tests/Feature/ExchangeRateServiceTest.php @@ -218,6 +218,32 @@ test('getRates fetches and stores when not cached', function () { expect($stored->rates['usd'])->toBe(1.08); }); +test('getRates tolerates another request storing rates after preload miss', function () { + Http::fake([ + 'cdn.jsdelivr.net/*currencies/eur*' => Http::response([ + 'eur' => [ + 'usd' => 1.08, + ], + ]), + ]); + + $service = app(ExchangeRateService::class); + $service->preloadRates('eur', ['2026-03-27']); + + ExchangeRate::factory()->create([ + 'base_currency' => 'eur', + 'date' => '2026-03-27', + 'rates' => [ + 'usd' => 1.07, + ], + ]); + + $rates = $service->getRates('eur', '2026-03-27'); + + expect($rates['usd'])->toBe(1.08); + expect(ExchangeRate::where('base_currency', 'eur')->where('date', '2026-03-27')->count())->toBe(1); +}); + test('getRates caps future dates to today', function () { $today = now()->toDateString(); diff --git a/tests/Pest.php b/tests/Pest.php index 2c4b4d28..e892483d 100644 --- a/tests/Pest.php +++ b/tests/Pest.php @@ -83,13 +83,17 @@ function performanceSeedUser(): User $categories = Category::factory(5)->create(['user_id' => $user->id]); Label::factory(3)->create(['user_id' => $user->id]); - $accounts = Account::factory(3)->create(['user_id' => $user->id]); + $accounts = Account::factory(3)->create([ + 'user_id' => $user->id, + 'currency_code' => $user->currency_code, + ]); foreach ($accounts as $index => $account) { Transaction::factory(10)->plaintext()->create([ 'user_id' => $user->id, 'account_id' => $account->id, 'category_id' => $categories->random()->id, + 'currency_code' => $user->currency_code, ]); for ($i = 0; $i < 5; $i++) {