refactor: improve readability of FetchProxyCloudResponses

app/Jobs/FetchProxyCloudResponses.php

@@ -7,6 +7,7 @@ use Exception;
 use Illuminate\Bus\Queueable;
 use Illuminate\Contracts\Queue\ShouldQueue;
 use Illuminate\Foundation\Bus\Dispatchable;
+use Illuminate\Http\Client\Response;
 use Illuminate\Queue\InteractsWithQueue;
 use Illuminate\Queue\SerializesModels;
 use Illuminate\Support\Facades\Http;
@@ -18,100 +19,126 @@ class FetchProxyCloudResponses implements ShouldQueue
 {
     use Dispatchable, InteractsWithQueue, Queueable, SerializesModels;
 
-    /**
-     * Execute the job.
-     */
     public function handle(): void
     {
         Device::where('proxy_cloud', true)->each(function ($device): void {
-            if (! $device->getNextPlaylistItem()) {
+            if ($device->getNextPlaylistItem()) {
-                try {
-                    $response = Http::withHeaders([
-                        'id' => $device->mac_address,
-                        'access-token' => $device->api_key,
-                        'width' => 800,
-                        'height' => 480,
-                        'rssi' => $device->last_rssi_level,
-                        'battery_voltage' => $device->last_battery_voltage,
-                        'refresh-rate' => $device->default_refresh_interval,
-                        'fw-version' => $device->last_firmware_version,
-                        'accept-encoding' => 'identity;q=1,chunked;q=0.1,*;q=0',
-                        'user-agent' => 'ESP32HTTPClient',
-                    ])->get(config('services.trmnl.proxy_base_url').'/api/display');
-
-                    $device->update([
-                        'proxy_cloud_response' => $response->json(),
-                    ]);
-
-                    $imageUrl = $response->json('image_url');
-                    $filename = $response->json('filename');
-
-                    parse_str(parse_url($imageUrl)['query'] ?? '', $queryParams);
-                    $imageType = urldecode($queryParams['response-content-type'] ?? 'image/bmp');
-                    $imageExtension = $imageType === 'image/png' ? 'png' : 'bmp';
-
-                    if (Str::contains($imageUrl, '.png')) {
-                        $imageExtension = 'png';
-                    }
-
-                    \Log::info("Response data: $imageUrl. Image Extension: $imageExtension");
-                    if (isset($imageUrl)) {
-                        try {
-                            $imageContents = Http::get($imageUrl)->body();
-                            if (! Storage::disk('public')->exists("images/generated/{$filename}.{$imageExtension}")) {
-                                Storage::disk('public')->put(
-                                    "images/generated/{$filename}.{$imageExtension}",
-                                    $imageContents
-                                );
-                            }
-
-                            $device->update([
-                                'current_screen_image' => $filename,
-                            ]);
-                        } catch (Exception $e) {
-                            Log::error("Failed to download and save image for device: {$device->mac_address}", [
-                                'error' => $e->getMessage(),
-                            ]);
-                        }
-                    }
-
-                    Log::info("Successfully updated proxy cloud response for device: {$device->mac_address}");
-
-                    if ($device->last_log_request) {
-                        try {
-                            Http::withHeaders([
-                                'id' => $device->mac_address,
-                                'access-token' => $device->api_key,
-                                'width' => 800,
-                                'height' => 480,
-                                'rssi' => $device->last_rssi_level,
-                                'battery_voltage' => $device->last_battery_voltage,
-                                'refresh-rate' => $device->default_refresh_interval,
-                                'fw-version' => $device->last_firmware_version,
-                                'accept-encoding' => 'identity;q=1,chunked;q=0.1,*;q=0',
-                                'user-agent' => 'ESP32HTTPClient',
-                            ])->post(config('services.trmnl.proxy_base_url').'/api/log', $device->last_log_request);
-
-                            // Only clear the pending log request if the POST succeeded
-                            $device->update([
-                                'last_log_request' => null,
-                            ]);
-                        } catch (Exception $e) {
-                            // Do not fail the entire proxy fetch if the log upload fails
-                            Log::error("Failed to upload device log for device: {$device->mac_address}", [
-                                'error' => $e->getMessage(),
-                            ]);
-                        }
-                    }
-
-                } catch (Exception $e) {
-                    Log::error("Failed to fetch proxy cloud response for device: {$device->mac_address}", [
-                        'error' => $e->getMessage(),
-                    ]);
-                }
-            } else {
                 Log::info("Skipping device: {$device->mac_address} as it has a pending playlist item.");
+
+                return;
+            }
+
+            try {
+                $response = $this->fetchDisplayResponse($device);
+                $device->update([
+                    'proxy_cloud_response' => $response->json(),
+                ]);
+
+                $this->processImage($device, $response);
+                $this->uploadLogRequest($device);
+
+                Log::info("Successfully updated proxy cloud response for device: {$device->mac_address}");
+            } catch (Exception $e) {
+                Log::error("Failed to fetch proxy cloud response for device: {$device->mac_address}", [
+                    'error' => $e->getMessage(),
+                ]);
             }
         });
     }
+
+    private function fetchDisplayResponse(Device $device): Response
+    {
+        /** @var Response $response */
+        $response = Http::withHeaders($this->getDeviceHeaders($device))
+            ->get(config('services.trmnl.proxy_base_url').'/api/display');
+
+        return $response;
+    }
+
+    private function getDeviceHeaders(Device $device): array
+    {
+        return [
+            'id' => $device->mac_address,
+            'access-token' => $device->api_key,
+            'width' => 800,
+            'height' => 480,
+            'rssi' => $device->last_rssi_level,
+            'battery_voltage' => $device->last_battery_voltage,
+            'refresh-rate' => $device->default_refresh_interval,
+            'fw-version' => $device->last_firmware_version,
+            'accept-encoding' => 'identity;q=1,chunked;q=0.1,*;q=0',
+            'user-agent' => 'ESP32HTTPClient',
+        ];
+    }
+
+    private function processImage(Device $device, Response $response): void
+    {
+        $imageUrl = $response->json('image_url');
+        $filename = $response->json('filename');
+
+        if ($imageUrl === null) {
+            return;
+        }
+
+        $imageExtension = $this->determineImageExtension($imageUrl);
+        Log::info("Response data: $imageUrl. Image Extension: $imageExtension");
+
+        try {
+            $imageContents = Http::get($imageUrl)->body();
+            $filePath = "images/generated/{$filename}.{$imageExtension}";
+
+            if (! Storage::disk('public')->exists($filePath)) {
+                Storage::disk('public')->put($filePath, $imageContents);
+            }
+
+            $device->update([
+                'current_screen_image' => $filename,
+            ]);
+        } catch (Exception $e) {
+            Log::error("Failed to download and save image for device: {$device->mac_address}", [
+                'error' => $e->getMessage(),
+            ]);
+        }
+    }
+
+    private function determineImageExtension(?string $imageUrl): string
+    {
+        if ($imageUrl === null) {
+            return 'bmp';
+        }
+
+        if (Str::contains($imageUrl, '.png')) {
+            return 'png';
+        }
+
+        $parsedUrl = parse_url($imageUrl);
+        if ($parsedUrl === false || ! isset($parsedUrl['query'])) {
+            return 'bmp';
+        }
+
+        parse_str($parsedUrl['query'], $queryParams);
+        $imageType = urldecode($queryParams['response-content-type'] ?? 'image/bmp');
+
+        return $imageType === 'image/png' ? 'png' : 'bmp';
+    }
+
+    private function uploadLogRequest(Device $device): void
+    {
+        if (! $device->last_log_request) {
+            return;
+        }
+
+        try {
+            Http::withHeaders($this->getDeviceHeaders($device))
+                ->post(config('services.trmnl.proxy_base_url').'/api/log', $device->last_log_request);
+
+            $device->update([
+                'last_log_request' => null,
+            ]);
+        } catch (Exception $e) {
+            Log::error("Failed to upload device log for device: {$device->mac_address}", [
+                'error' => $e->getMessage(),
+            ]);
+        }
+    }
 }

tests/Feature/FetchProxyCloudResponsesTest.php

@@ -16,13 +16,12 @@ beforeEach(function (): void {
         'https://trmnl.app/api/log' => Http::response([], 200),
         'https://example.com/api/log' => Http::response([], 200),
     ]);
+    config(['services.trmnl.proxy_base_url' => 'https://example.com']);
 });
 
-test('it fetches and processes proxy cloud responses for devices', function (): void {
+function createTestDevice(array $attributes = []): Device
-    config(['services.trmnl.proxy_base_url' => 'https://example.com']);
+{
-
+    return Device::factory()->create(array_merge([
-    // Create a test device with proxy cloud enabled
-    $device = Device::factory()->create([
         'proxy_cloud' => true,
         'mac_address' => '00:11:22:33:44:55',
         'api_key' => 'test-api-key',
@@ -30,35 +29,11 @@ test('it fetches and processes proxy cloud responses for devices', function ():
         'last_battery_voltage' => 3.7,
         'default_refresh_interval' => 300,
         'last_firmware_version' => '1.0.0',
-    ]);
+    ], $attributes));
+}
 
-    // Mock the API response
+function assertDeviceHeaders(Device $device): void
-    Http::fake([
+{
-        config('services.trmnl.proxy_base_url').'/api/display' => Http::response([
-            'image_url' => 'https://example.com/test-image.bmp',
-            'filename' => 'test-image',
-        ]),
-        'https://example.com/test-image.bmp' => Http::response('fake-image-content'),
-    ]);
-
-    Http::withHeaders([
-        'id' => $device->mac_address,
-        'access-token' => $device->api_key,
-        'width' => 800,
-        'height' => 480,
-        'rssi' => $device->last_rssi_level,
-        'battery_voltage' => $device->last_battery_voltage,
-        'refresh-rate' => $device->default_refresh_interval,
-        'fw-version' => $device->last_firmware_version,
-        'accept-encoding' => 'identity;q=1,chunked;q=0.1,*;q=0',
-        'user-agent' => 'ESP32HTTPClient',
-    ])->get(config('services.trmnl.proxy_base_url').'/api/display');
-
-    // Run the job
-    $job = new FetchProxyCloudResponses;
-    $job->handle();
-
-    // Assert HTTP requests were made with correct headers
     Http::assertSent(fn ($request): bool => $request->hasHeader('id', $device->mac_address) &&
         $request->hasHeader('access-token', $device->api_key) &&
         $request->hasHeader('width', 800) &&
@@ -67,7 +42,22 @@ test('it fetches and processes proxy cloud responses for devices', function ():
         $request->hasHeader('battery_voltage', $device->last_battery_voltage) &&
         $request->hasHeader('refresh-rate', $device->default_refresh_interval) &&
         $request->hasHeader('fw-version', $device->last_firmware_version));
-    // Assert the device was updated
+}
+
+test('it fetches and processes proxy cloud responses for devices', function (): void {
+    $device = createTestDevice();
+
+    Http::fake([
+        config('services.trmnl.proxy_base_url').'/api/display' => Http::response([
+            'image_url' => 'https://example.com/test-image.bmp',
+            'filename' => 'test-image',
+        ]),
+        'https://example.com/test-image.bmp' => Http::response('fake-image-content'),
+    ]);
+
+    (new FetchProxyCloudResponses)->handle();
+
+    assertDeviceHeaders($device);
     $device->refresh();
 
     expect($device->current_screen_image)->toBe('test-image')
@@ -76,15 +66,11 @@ test('it fetches and processes proxy cloud responses for devices', function ():
             'filename' => 'test-image',
         ]);
 
-    // Assert the image was saved
     Storage::disk('public')->assertExists('images/generated/test-image.bmp');
 });
 
 test('it handles log requests when present', function (): void {
-    $device = Device::factory()->create([
+    $device = createTestDevice([
-        'proxy_cloud' => true,
-        'mac_address' => '00:11:22:33:44:55',
-        'api_key' => 'test-api-key',
         'last_log_request' => ['message' => 'test log'],
     ]);
 
@@ -97,33 +83,24 @@ test('it handles log requests when present', function (): void {
         config('services.trmnl.proxy_base_url').'/api/log' => Http::response(null, 200),
     ]);
 
-    $job = new FetchProxyCloudResponses;
+    (new FetchProxyCloudResponses)->handle();
-    $job->handle();
 
-    // Assert log request was sent
     Http::assertSent(fn ($request): bool => $request->url() === config('services.trmnl.proxy_base_url').'/api/log' &&
         $request->hasHeader('id', $device->mac_address) &&
         $request->body() === json_encode(['message' => 'test log']));
 
-    // Assert log request was cleared
     $device->refresh();
     expect($device->last_log_request)->toBeNull();
 });
 
 test('it handles API errors gracefully', function (): void {
-    $device = Device::factory()->create([
+    createTestDevice();
-        'proxy_cloud' => true,
-        'mac_address' => '00:11:22:33:44:55',
-    ]);
 
     Http::fake([
         config('services.trmnl.proxy_base_url').'/api/display' => Http::response(null, 500),
     ]);
 
-    $job = new FetchProxyCloudResponses;
+    expect(fn () => (new FetchProxyCloudResponses)->handle())->not->toThrow(Exception::class);
-
-    // Job should not throw exception but log error
-    expect(fn () => $job->handle())->not->toThrow(Exception::class);
 });
 
 test('it only processes proxy cloud enabled devices', function (): void {
@@ -131,30 +108,15 @@ test('it only processes proxy cloud enabled devices', function (): void {
     $enabledDevice = Device::factory()->create(['proxy_cloud' => true]);
     $disabledDevice = Device::factory()->create(['proxy_cloud' => false]);
 
-    $job = new FetchProxyCloudResponses;
+    (new FetchProxyCloudResponses)->handle();
-    $job->handle();
 
-    // Assert request was only made for enabled device
     Http::assertSent(fn ($request) => $request->hasHeader('id', $enabledDevice->mac_address));
-
     Http::assertNotSent(fn ($request) => $request->hasHeader('id', $disabledDevice->mac_address));
 });
 
 test('it fetches and processes proxy cloud responses for devices with BMP images', function (): void {
-    config(['services.trmnl.proxy_base_url' => 'https://example.com']);
+    $device = createTestDevice();
 
-    // Create a test device with proxy cloud enabled
-    $device = Device::factory()->create([
-        'proxy_cloud' => true,
-        'mac_address' => '00:11:22:33:44:55',
-        'api_key' => 'test-api-key',
-        'last_rssi_level' => -70,
-        'last_battery_voltage' => 3.7,
-        'default_refresh_interval' => 300,
-        'last_firmware_version' => '1.0.0',
-    ]);
-
-    // Mock the API response with BMP image
     Http::fake([
         config('services.trmnl.proxy_base_url').'/api/display' => Http::response([
             'image_url' => 'https://example.com/test-image.bmp?response-content-type=image/bmp',
@@ -163,21 +125,9 @@ test('it fetches and processes proxy cloud responses for devices with BMP images
         'https://example.com/test-image.bmp?response-content-type=image/bmp' => Http::response('fake-image-content'),
     ]);
 
-    // Run the job
+    (new FetchProxyCloudResponses)->handle();
-    $job = new FetchProxyCloudResponses;
-    $job->handle();
 
-    // Assert HTTP requests were made with correct headers
+    assertDeviceHeaders($device);
-    Http::assertSent(fn ($request): bool => $request->hasHeader('id', $device->mac_address) &&
-        $request->hasHeader('access-token', $device->api_key) &&
-        $request->hasHeader('width', 800) &&
-        $request->hasHeader('height', 480) &&
-        $request->hasHeader('rssi', $device->last_rssi_level) &&
-        $request->hasHeader('battery_voltage', $device->last_battery_voltage) &&
-        $request->hasHeader('refresh-rate', $device->default_refresh_interval) &&
-        $request->hasHeader('fw-version', $device->last_firmware_version));
-
-    // Assert the device was updated
     $device->refresh();
 
     expect($device->current_screen_image)->toBe('test-image')
@@ -186,26 +136,13 @@ test('it fetches and processes proxy cloud responses for devices with BMP images
             'filename' => 'test-image',
         ]);
 
-    // Assert the image was saved with BMP extension
     expect(Storage::disk('public')->exists('images/generated/test-image.bmp'))->toBeTrue();
     expect(Storage::disk('public')->exists('images/generated/test-image.png'))->toBeFalse();
 });
 
 test('it fetches and processes proxy cloud responses for devices with PNG images', function (): void {
-    config(['services.trmnl.proxy_base_url' => 'https://example.com']);
+    $device = createTestDevice();
 
-    // Create a test device with proxy cloud enabled
-    $device = Device::factory()->create([
-        'proxy_cloud' => true,
-        'mac_address' => '00:11:22:33:44:55',
-        'api_key' => 'test-api-key',
-        'last_rssi_level' => -70,
-        'last_battery_voltage' => 3.7,
-        'default_refresh_interval' => 300,
-        'last_firmware_version' => '1.0.0',
-    ]);
-
-    // Mock the API response with PNG image
     Http::fake([
         config('services.trmnl.proxy_base_url').'/api/display' => Http::response([
             'image_url' => 'https://example.com/test-image.png?response-content-type=image/png',
@@ -214,21 +151,9 @@ test('it fetches and processes proxy cloud responses for devices with PNG images
         'https://example.com/test-image.png?response-content-type=image/png' => Http::response('fake-image-content'),
     ]);
 
-    // Run the job
+    (new FetchProxyCloudResponses)->handle();
-    $job = new FetchProxyCloudResponses;
-    $job->handle();
 
-    // Assert HTTP requests were made with correct headers
+    assertDeviceHeaders($device);
-    Http::assertSent(fn ($request): bool => $request->hasHeader('id', $device->mac_address) &&
-        $request->hasHeader('access-token', $device->api_key) &&
-        $request->hasHeader('width', 800) &&
-        $request->hasHeader('height', 480) &&
-        $request->hasHeader('rssi', $device->last_rssi_level) &&
-        $request->hasHeader('battery_voltage', $device->last_battery_voltage) &&
-        $request->hasHeader('refresh-rate', $device->default_refresh_interval) &&
-        $request->hasHeader('fw-version', $device->last_firmware_version));
-
-    // Assert the device was updated
     $device->refresh();
 
     expect($device->current_screen_image)->toBe('test-image')
@@ -237,22 +162,13 @@ test('it fetches and processes proxy cloud responses for devices with PNG images
             'filename' => 'test-image',
         ]);
 
-    // Assert the image was saved with PNG extension
     expect(Storage::disk('public')->exists('images/generated/test-image.png'))->toBeTrue();
     expect(Storage::disk('public')->exists('images/generated/test-image.bmp'))->toBeFalse();
 });
 
 test('it handles missing content type in image URL gracefully', function (): void {
-    config(['services.trmnl.proxy_base_url' => 'https://example.com']);
+    $device = createTestDevice();
 
-    // Create a test device with proxy cloud enabled
-    $device = Device::factory()->create([
-        'proxy_cloud' => true,
-        'mac_address' => '00:11:22:33:44:55',
-        'api_key' => 'test-api-key',
-    ]);
-
-    // Mock the API response with no content type in URL
     Http::fake([
         config('services.trmnl.proxy_base_url').'/api/display' => Http::response([
             'image_url' => 'https://example.com/test-image.bmp',
@@ -261,11 +177,8 @@ test('it handles missing content type in image URL gracefully', function (): voi
         'https://example.com/test-image.bmp' => Http::response('fake-image-content'),
     ]);
 
-    // Run the job
+    (new FetchProxyCloudResponses)->handle();
-    $job = new FetchProxyCloudResponses;
-    $job->handle();
 
-    // Assert the device was updated
     $device->refresh();
 
     expect($device->current_screen_image)->toBe('test-image')
@@ -274,7 +187,50 @@ test('it handles missing content type in image URL gracefully', function (): voi
             'filename' => 'test-image',
         ]);
 
-    // Assert the image was saved with default BMP extension
     expect(Storage::disk('public')->exists('images/generated/test-image.bmp'))->toBeTrue();
     expect(Storage::disk('public')->exists('images/generated/test-image.png'))->toBeFalse();
 });
+
+test('it handles null image URL gracefully', function (): void {
+    $device = createTestDevice();
+
+    Http::fake([
+        config('services.trmnl.proxy_base_url').'/api/display' => Http::response([
+            'image_url' => null,
+            'filename' => 'test-image',
+        ]),
+    ]);
+
+    expect(fn () => (new FetchProxyCloudResponses)->handle())->not->toThrow(TypeError::class);
+
+    $device->refresh();
+    expect($device->proxy_cloud_response)->toBe([
+        'image_url' => null,
+        'filename' => 'test-image',
+    ]);
+
+    expect($device->current_screen_image)->toBeNull();
+    expect(Storage::disk('public')->exists('images/generated/test-image.bmp'))->toBeFalse();
+    expect(Storage::disk('public')->exists('images/generated/test-image.png'))->toBeFalse();
+});
+
+test('it handles malformed image URL gracefully', function (): void {
+    $device = createTestDevice();
+
+    Http::fake([
+        config('services.trmnl.proxy_base_url').'/api/display' => Http::response([
+            'image_url' => 'not-a-valid-url://',
+            'filename' => 'test-image',
+        ]),
+    ]);
+
+    expect(fn () => (new FetchProxyCloudResponses)->handle())->not->toThrow(TypeError::class);
+
+    $device->refresh();
+    expect($device->proxy_cloud_response)->toBe([
+        'image_url' => 'not-a-valid-url://',
+        'filename' => 'test-image',
+    ]);
+
+    expect($device->current_screen_image)->toBeNull();
+});