refactor: extract iterateBatches helper in FuelPriceService
Audit item #9. pollPrices() and refreshStations() shared the same do/while batch loop with token + try/catch + Log::error + clean-exit detection but different bodies. Extracted to a private iterateBatches() that takes the endpoint, optional since timestamp, and a process callable. Returns [total_processed, completedCleanly] so pollPrices can still gate its incremental-poll cache update on a clean exhaustion. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -67,57 +67,15 @@ class FuelPriceService
|
|||||||
*/
|
*/
|
||||||
public function pollPrices(): int
|
public function pollPrices(): int
|
||||||
{
|
{
|
||||||
$token = $this->getAccessToken();
|
|
||||||
$inserted = 0;
|
|
||||||
$batch = 1;
|
|
||||||
$pollStartedAt = now();
|
$pollStartedAt = now();
|
||||||
$since = Cache::get(self::LAST_PRICE_POLL_CACHE_KEY);
|
$since = Cache::get(self::LAST_PRICE_POLL_CACHE_KEY);
|
||||||
$completedCleanly = false;
|
$sinceCarbon = $since instanceof CarbonInterface ? $since : null;
|
||||||
|
|
||||||
do {
|
[$inserted, $completedCleanly] = $this->iterateBatches(
|
||||||
try {
|
'/pfs/fuel-prices',
|
||||||
$baseUrl = config('services.fuel_finder.base_url').'/pfs/fuel-prices';
|
$sinceCarbon,
|
||||||
$params = ['batch-number' => $batch];
|
fn (array $stations): int => $this->processPriceBatch($stations),
|
||||||
|
);
|
||||||
if ($since instanceof CarbonInterface) {
|
|
||||||
$params['effective-start-timestamp'] = $since->format('Y-m-d H:i:s');
|
|
||||||
}
|
|
||||||
|
|
||||||
$logUrl = $baseUrl.'?'.http_build_query($params);
|
|
||||||
$response = $this->apiLogger->send('fuel_finder', 'GET', $logUrl, fn () => Http::timeout(30)
|
|
||||||
->withToken($token)
|
|
||||||
->get($baseUrl, $params));
|
|
||||||
|
|
||||||
if ($response->notFound()) {
|
|
||||||
$completedCleanly = true;
|
|
||||||
break;
|
|
||||||
}
|
|
||||||
|
|
||||||
if (! $response->successful()) {
|
|
||||||
Log::error('FuelPriceService: price batch returned error', [
|
|
||||||
'batch' => $batch,
|
|
||||||
'status' => $response->status(),
|
|
||||||
]);
|
|
||||||
break;
|
|
||||||
}
|
|
||||||
|
|
||||||
$stations = $response->json() ?? [];
|
|
||||||
} catch (Throwable $e) {
|
|
||||||
Log::error('FuelPriceService: price batch fetch failed', [
|
|
||||||
'batch' => $batch,
|
|
||||||
'error' => $e->getMessage(),
|
|
||||||
]);
|
|
||||||
break;
|
|
||||||
}
|
|
||||||
|
|
||||||
if (empty($stations)) {
|
|
||||||
$completedCleanly = true;
|
|
||||||
break;
|
|
||||||
}
|
|
||||||
|
|
||||||
$inserted += $this->processPriceBatch($stations);
|
|
||||||
$batch++;
|
|
||||||
} while (true);
|
|
||||||
|
|
||||||
if ($completedCleanly) {
|
if ($completedCleanly) {
|
||||||
Cache::forever(self::LAST_PRICE_POLL_CACHE_KEY, $pollStartedAt);
|
Cache::forever(self::LAST_PRICE_POLL_CACHE_KEY, $pollStartedAt);
|
||||||
@@ -131,25 +89,53 @@ class FuelPriceService
|
|||||||
* Called on full daily refresh before pollPrices().
|
* Called on full daily refresh before pollPrices().
|
||||||
*/
|
*/
|
||||||
public function refreshStations(): void
|
public function refreshStations(): void
|
||||||
|
{
|
||||||
|
$this->iterateBatches('/pfs', null, function (array $stations): int {
|
||||||
|
$this->upsertStations($stations);
|
||||||
|
|
||||||
|
return 0;
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Drive a paginated fuel-finder endpoint until exhausted, calling
|
||||||
|
* $process on each non-empty batch. Returns the sum of $process return
|
||||||
|
* values plus a flag indicating the loop exited cleanly (404 or empty
|
||||||
|
* body) rather than via an HTTP error or thrown exception. Callers use
|
||||||
|
* the flag to decide whether to update incremental-poll bookkeeping.
|
||||||
|
*
|
||||||
|
* @param callable(array<int, array<string, mixed>>): int $process
|
||||||
|
* @return array{0: int, 1: bool}
|
||||||
|
*/
|
||||||
|
private function iterateBatches(string $endpoint, ?CarbonInterface $since, callable $process): array
|
||||||
{
|
{
|
||||||
$token = $this->getAccessToken();
|
$token = $this->getAccessToken();
|
||||||
|
$baseUrl = config('services.fuel_finder.base_url').$endpoint;
|
||||||
|
$total = 0;
|
||||||
$batch = 1;
|
$batch = 1;
|
||||||
|
$completedCleanly = false;
|
||||||
|
|
||||||
do {
|
do {
|
||||||
try {
|
try {
|
||||||
$baseUrl = config('services.fuel_finder.base_url').'/pfs';
|
|
||||||
$params = ['batch-number' => $batch];
|
$params = ['batch-number' => $batch];
|
||||||
|
|
||||||
|
if ($since !== null) {
|
||||||
|
$params['effective-start-timestamp'] = $since->format('Y-m-d H:i:s');
|
||||||
|
}
|
||||||
|
|
||||||
$logUrl = $baseUrl.'?'.http_build_query($params);
|
$logUrl = $baseUrl.'?'.http_build_query($params);
|
||||||
$response = $this->apiLogger->send('fuel_finder', 'GET', $logUrl, fn () => Http::timeout(30)
|
$response = $this->apiLogger->send('fuel_finder', 'GET', $logUrl, fn () => Http::timeout(30)
|
||||||
->withToken($token)
|
->withToken($token)
|
||||||
->get($baseUrl, $params));
|
->get($baseUrl, $params));
|
||||||
|
|
||||||
if ($response->notFound()) {
|
if ($response->notFound()) {
|
||||||
break; // No more batches
|
$completedCleanly = true;
|
||||||
|
break;
|
||||||
}
|
}
|
||||||
|
|
||||||
if (! $response->successful()) {
|
if (! $response->successful()) {
|
||||||
Log::error('FuelPriceService: station batch returned error', [
|
Log::error('FuelPriceService: batch returned error', [
|
||||||
|
'endpoint' => $endpoint,
|
||||||
'batch' => $batch,
|
'batch' => $batch,
|
||||||
'status' => $response->status(),
|
'status' => $response->status(),
|
||||||
]);
|
]);
|
||||||
@@ -158,7 +144,8 @@ class FuelPriceService
|
|||||||
|
|
||||||
$stations = $response->json() ?? [];
|
$stations = $response->json() ?? [];
|
||||||
} catch (Throwable $e) {
|
} catch (Throwable $e) {
|
||||||
Log::error('FuelPriceService: station batch fetch failed', [
|
Log::error('FuelPriceService: batch fetch failed', [
|
||||||
|
'endpoint' => $endpoint,
|
||||||
'batch' => $batch,
|
'batch' => $batch,
|
||||||
'error' => $e->getMessage(),
|
'error' => $e->getMessage(),
|
||||||
]);
|
]);
|
||||||
@@ -166,12 +153,15 @@ class FuelPriceService
|
|||||||
}
|
}
|
||||||
|
|
||||||
if (empty($stations)) {
|
if (empty($stations)) {
|
||||||
|
$completedCleanly = true;
|
||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
|
|
||||||
$this->upsertStations($stations);
|
$total += $process($stations);
|
||||||
$batch++;
|
$batch++;
|
||||||
} while (true);
|
} while (true);
|
||||||
|
|
||||||
|
return [$total, $completedCleanly];
|
||||||
}
|
}
|
||||||
|
|
||||||
/** @param array<int, array<string, mixed>> $apiStations */
|
/** @param array<int, array<string, mixed>> $apiStations */
|
||||||
|
|||||||
Reference in New Issue
Block a user