From 06f5f2035f11603f712b41a641a631cd4336aa6d Mon Sep 17 00:00:00 2001
From: Ovidiu U <nonreal@gmail.com>
Date: Wed, 29 Apr 2026 20:21:21 +0100
Subject: [PATCH] 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>
---
 app/Services/FuelPriceService.php | 96 ++++++++++++++-----------------
 1 file changed, 43 insertions(+), 53 deletions(-)

diff --git a/app/Services/FuelPriceService.php b/app/Services/FuelPriceService.php
index c7ab1e3..de5ec99 100644
--- a/app/Services/FuelPriceService.php
+++ b/app/Services/FuelPriceService.php
@@ -67,57 +67,15 @@ class FuelPriceService
      */
     public function pollPrices(): int
     {
-        $token = $this->getAccessToken();
-        $inserted = 0;
-        $batch = 1;
         $pollStartedAt = now();
         $since = Cache::get(self::LAST_PRICE_POLL_CACHE_KEY);
-        $completedCleanly = false;
+        $sinceCarbon = $since instanceof CarbonInterface ? $since : null;
 
-        do {
-            try {
-                $baseUrl = config('services.fuel_finder.base_url').'/pfs/fuel-prices';
-                $params = ['batch-number' => $batch];
-
-                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);
+        [$inserted, $completedCleanly] = $this->iterateBatches(
+            '/pfs/fuel-prices',
+            $sinceCarbon,
+            fn (array $stations): int => $this->processPriceBatch($stations),
+        );
 
         if ($completedCleanly) {
             Cache::forever(self::LAST_PRICE_POLL_CACHE_KEY, $pollStartedAt);
@@ -131,25 +89,53 @@ class FuelPriceService
      * Called on full daily refresh before pollPrices().
      */
     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();
+        $baseUrl = config('services.fuel_finder.base_url').$endpoint;
+        $total = 0;
         $batch = 1;
+        $completedCleanly = false;
 
         do {
             try {
-                $baseUrl = config('services.fuel_finder.base_url').'/pfs';
                 $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);
                 $response = $this->apiLogger->send('fuel_finder', 'GET', $logUrl, fn () => Http::timeout(30)
                     ->withToken($token)
                     ->get($baseUrl, $params));
 
                 if ($response->notFound()) {
-                    break; // No more batches
+                    $completedCleanly = true;
+                    break;
                 }
 
                 if (! $response->successful()) {
-                    Log::error('FuelPriceService: station batch returned error', [
+                    Log::error('FuelPriceService: batch returned error', [
+                        'endpoint' => $endpoint,
                         'batch' => $batch,
                         'status' => $response->status(),
                     ]);
@@ -158,7 +144,8 @@ class FuelPriceService
 
                 $stations = $response->json() ?? [];
             } catch (Throwable $e) {
-                Log::error('FuelPriceService: station batch fetch failed', [
+                Log::error('FuelPriceService: batch fetch failed', [
+                    'endpoint' => $endpoint,
                     'batch' => $batch,
                     'error' => $e->getMessage(),
                 ]);
@@ -166,12 +153,15 @@ class FuelPriceService
             }
 
             if (empty($stations)) {
+                $completedCleanly = true;
                 break;
             }
 
-            $this->upsertStations($stations);
+            $total += $process($stations);
             $batch++;
         } while (true);
+
+        return [$total, $completedCleanly];
     }
 
     /** @param array<int, array<string, mixed>> $apiStations */