diff --git a/app/Services/CurrencyConversionService.php b/app/Services/CurrencyConversionService.php index 94673b04..ae2dda0c 100644 --- a/app/Services/CurrencyConversionService.php +++ b/app/Services/CurrencyConversionService.php @@ -2,10 +2,11 @@ namespace App\Services; +use Illuminate\Http\Client\ConnectionException; use Illuminate\Support\Carbon; +use Illuminate\Support\Facades\Cache; use Illuminate\Support\Facades\Http; use Illuminate\Support\Facades\Log; -use RuntimeException; class CurrencyConversionService { @@ -15,6 +16,16 @@ class CurrencyConversionService private const HISTORICAL_LOOKBACK_DAYS = 7; + private const HTTP_CONNECT_TIMEOUT_SECONDS = 3; + + private const HTTP_TIMEOUT_SECONDS = 5; + + private const CACHE_TTL_HISTORICAL_SECONDS = 60 * 60 * 24 * 30; + + private const CACHE_TTL_LATEST_SECONDS = 60 * 60 * 6; + + private const CACHE_TTL_UNAVAILABLE_SECONDS = 60 * 10; + /** @var array> Keyed by "{currency}:{date}" */ private array $rateCache = []; @@ -67,7 +78,15 @@ class CurrencyConversionService return $this->rateCache[$cacheKey]; } - $rates = $this->fetchRates($currency, $date); + $persistentKey = "currency-rates:{$cacheKey}"; + + $rates = Cache::get($persistentKey); + + if ($rates === null) { + $rates = $this->fetchRates($currency, $date); + Cache::put($persistentKey, $rates, $this->cacheTtlFor($date, $rates)); + } + $this->rateCache[$cacheKey] = $rates; return $rates; @@ -76,35 +95,58 @@ class CurrencyConversionService /** * Fetch rates from CDN with fallback. * + * A missing release (404) walks back to earlier historical dates, but an + * unreachable source (connection refused or timeout) aborts the walk: the + * same timeout would repeat for every candidate date and risk exhausting + * the request's execution time. Failures degrade to an empty rate map + * rather than throwing, so a slow CDN never crashes the calling endpoint. + * * @return array */ private function fetchRates(string $currency, string $date): array { - $lastException = null; + $sourceUnreachable = false; foreach ($this->candidateDates($date) as $candidateDate) { foreach ($this->rateUrls($currency, $candidateDate) as $url) { try { - $response = Http::timeout(10)->get($url); + $response = Http::connectTimeout(self::HTTP_CONNECT_TIMEOUT_SECONDS) + ->timeout(self::HTTP_TIMEOUT_SECONDS) + ->get($url); + } catch (ConnectionException $e) { + $sourceUnreachable = true; - if ($response->notFound()) { - continue; - } + Log::warning('Currency rate source unreachable', [ + 'currency' => $currency, + 'date' => $candidateDate, + 'url' => $url, + 'error' => $e->getMessage(), + ]); - $response->throw(); - - return $response->json($currency) ?? []; - } catch (\Throwable $e) { - $lastException = $e; + continue; } + + if ($response->notFound()) { + continue; + } + + if ($response->successful()) { + return $response->json($currency) ?? []; + } + + Log::warning('Currency rate source returned an error', [ + 'currency' => $currency, + 'date' => $candidateDate, + 'status' => $response->status(), + ]); + } + + if ($sourceUnreachable) { + break; } } - if ($lastException !== null) { - throw new RuntimeException("Failed to fetch currency rates for {$currency} on {$date}: {$lastException->getMessage()}", 0, $lastException); - } - - Log::warning('Currency rates unavailable, all sources returned 404', [ + Log::warning('Currency rates unavailable', [ 'currency' => $currency, 'date' => $date, ]); @@ -112,6 +154,26 @@ class CurrencyConversionService return []; } + /** + * Resolve the cache lifetime for a fetched rate map. + * + * Historical releases are immutable, so cache them long. The "latest" + * release changes daily. An empty result means the sources were missing or + * unreachable; cache it briefly so a transient outage recovers quickly. + * + * @param array $rates + */ + private function cacheTtlFor(string $date, array $rates): int + { + if ($rates === []) { + return self::CACHE_TTL_UNAVAILABLE_SECONDS; + } + + return $date === 'latest' + ? self::CACHE_TTL_LATEST_SECONDS + : self::CACHE_TTL_HISTORICAL_SECONDS; + } + /** * @return array */ diff --git a/tests/Feature/CurrencyConversionServiceTest.php b/tests/Feature/CurrencyConversionServiceTest.php index aa3581c2..9c9e9e2c 100644 --- a/tests/Feature/CurrencyConversionServiceTest.php +++ b/tests/Feature/CurrencyConversionServiceTest.php @@ -1,8 +1,14 @@ Http::response([ @@ -63,15 +69,49 @@ test('uses fallback URL when primary fails', function () { expect($result)->toBe(1.0 / 0.00002); }); -test('throws when both primary and fallback fail', function () { +test('degrades to zero when both primary and fallback fail', function () { Http::fake([ 'cdn.jsdelivr.net/*' => Http::response('Server Error', 500), 'currency-api.pages.dev/*' => Http::response('Server Error', 500), ]); $service = new CurrencyConversionService; - $service->convert('BTC', 'EUR', 1.0, '2026-01-15'); -})->throws(RuntimeException::class); + $result = $service->convert('BTC', 'EUR', 1.0, '2026-01-15'); + + expect($result)->toBe(0.0); +}); + +test('degrades to zero and stops walking dates when the source times out', function () { + $attempts = 0; + + Http::fake(function () use (&$attempts) { + $attempts++; + + throw new ConnectionException('cURL error 28: Operation timed out'); + }); + + $service = new CurrencyConversionService; + $result = $service->convert('BTC', 'EUR', 1.0, '2026-01-15'); + + expect($result)->toBe(0.0); + + // Only the first candidate date is attempted (primary + fallback); a timing + // out source must not be retried across every historical lookback date. + expect($attempts)->toBe(2); +}); + +test('caches rates across service instances so repeat lookups skip HTTP', function () { + Http::fake([ + 'cdn.jsdelivr.net/*currencies/eur*' => Http::response([ + 'eur' => ['btc' => 0.000015], + ]), + ]); + + (new CurrencyConversionService)->convert('BTC', 'EUR', 1.0, '2026-01-15'); + (new CurrencyConversionService)->convert('BTC', 'EUR', 1.0, '2026-01-15'); + + Http::assertSentCount(1); +}); test('returns zero when base currency is unknown and all sources return 404', function () { Http::fake([