From aaefcdda497cb4f387dfa8d48ae7c88675e442d6 Mon Sep 17 00:00:00 2001 From: Benjamin Nussbaum Date: Fri, 16 Jan 2026 13:47:01 +0100 Subject: [PATCH] refactor: improve readability of FetchProxyCloudResponses --- app/Jobs/FetchProxyCloudResponses.php | 205 +++++++++-------- .../Feature/FetchProxyCloudResponsesTest.php | 206 +++++++----------- 2 files changed, 197 insertions(+), 214 deletions(-) diff --git a/app/Jobs/FetchProxyCloudResponses.php b/app/Jobs/FetchProxyCloudResponses.php index ac23130..3c8af13 100644 --- a/app/Jobs/FetchProxyCloudResponses.php +++ b/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()) { - 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 { + if ($device->getNextPlaylistItem()) { 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(), + ]); + } + } } diff --git a/tests/Feature/FetchProxyCloudResponsesTest.php b/tests/Feature/FetchProxyCloudResponsesTest.php index 561dc1c..abd3cb9 100644 --- a/tests/Feature/FetchProxyCloudResponsesTest.php +++ b/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 { - config(['services.trmnl.proxy_base_url' => 'https://example.com']); - - // Create a test device with proxy cloud enabled - $device = Device::factory()->create([ +function createTestDevice(array $attributes = []): Device +{ + return Device::factory()->create(array_merge([ '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 - 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 +function assertDeviceHeaders(Device $device): void +{ 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([ - 'proxy_cloud' => true, - 'mac_address' => '00:11:22:33:44:55', - 'api_key' => 'test-api-key', + $device = createTestDevice([ '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; - $job->handle(); + (new FetchProxyCloudResponses)->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([ - 'proxy_cloud' => true, - 'mac_address' => '00:11:22:33:44:55', - ]); + createTestDevice(); Http::fake([ config('services.trmnl.proxy_base_url').'/api/display' => Http::response(null, 500), ]); - $job = new FetchProxyCloudResponses; - - // Job should not throw exception but log error - expect(fn () => $job->handle())->not->toThrow(Exception::class); + expect(fn () => (new FetchProxyCloudResponses)->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; - $job->handle(); + (new FetchProxyCloudResponses)->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 - $job = new FetchProxyCloudResponses; - $job->handle(); + (new FetchProxyCloudResponses)->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) && - $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 + assertDeviceHeaders($device); $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 - $job = new FetchProxyCloudResponses; - $job->handle(); + (new FetchProxyCloudResponses)->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) && - $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 + assertDeviceHeaders($device); $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 - $job = new FetchProxyCloudResponses; - $job->handle(); + (new FetchProxyCloudResponses)->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(); +});