From cc63c8cce2df4d7f88a5cbf2b1dc0c81f214ebb7 Mon Sep 17 00:00:00 2001 From: Benjamin Nussbaum Date: Mon, 12 May 2025 07:19:03 +0200 Subject: [PATCH] feat(#16): refactor --- .../Commands/ScreenGeneratorCommand.php | 3 +- app/Jobs/GeneratePluginJob.php | 37 ------ app/Jobs/GenerateScreenJob.php | 12 +- app/Models/Plugin.php | 1 - .../ImageGenerationService.php} | 6 +- .../views/livewire/plugins/markup.blade.php | 2 +- routes/api.php | 46 ++++---- tests/Feature/Api/DeviceEndpointsTest.php | 110 +++++++++++++++++- tests/Feature/GenerateScreenJobTest.php | 6 +- 9 files changed, 148 insertions(+), 75 deletions(-) delete mode 100644 app/Jobs/GeneratePluginJob.php rename app/{Jobs/CommonFunctions.php => Services/ImageGenerationService.php} (95%) diff --git a/app/Console/Commands/ScreenGeneratorCommand.php b/app/Console/Commands/ScreenGeneratorCommand.php index baafacb..722c5f2 100644 --- a/app/Console/Commands/ScreenGeneratorCommand.php +++ b/app/Console/Commands/ScreenGeneratorCommand.php @@ -36,8 +36,7 @@ class ScreenGeneratorCommand extends Command return 1; } - - GenerateScreenJob::dispatchSync($deviceId, $markup); + GenerateScreenJob::dispatchSync($deviceId, null, $markup); $this->info('Screen generation job finished.'); diff --git a/app/Jobs/GeneratePluginJob.php b/app/Jobs/GeneratePluginJob.php deleted file mode 100644 index d1e254b..0000000 --- a/app/Jobs/GeneratePluginJob.php +++ /dev/null @@ -1,37 +0,0 @@ -markup); - - Plugin::find($this->pluginId)->update(['current_image' => $newImageUuid]); - \Log::info("Plugin $this->pluginId: updated with new image: $newImageUuid"); - - CommonFunctions::cleanupFolder(); - } -} - diff --git a/app/Jobs/GenerateScreenJob.php b/app/Jobs/GenerateScreenJob.php index 166af8d..61e1305 100644 --- a/app/Jobs/GenerateScreenJob.php +++ b/app/Jobs/GenerateScreenJob.php @@ -3,6 +3,8 @@ namespace App\Jobs; use App\Models\Device; +use App\Models\Plugin; +use App\Services\ImageGenerationService; use Illuminate\Bus\Queueable; use Illuminate\Contracts\Queue\ShouldQueue; use Illuminate\Foundation\Bus\Dispatchable; @@ -18,6 +20,7 @@ class GenerateScreenJob implements ShouldQueue */ public function __construct( private readonly int $deviceId, + private readonly ?int $pluginId, private readonly string $markup ) {} @@ -26,11 +29,16 @@ class GenerateScreenJob implements ShouldQueue */ public function handle(): void { - $newImageUuid = CommonFunctions::generateImage($this->markup); + $newImageUuid = ImageGenerationService::generateImage($this->markup); Device::find($this->deviceId)->update(['current_screen_image' => $newImageUuid]); \Log::info("Device $this->deviceId: updated with new image: $newImageUuid"); - CommonFunctions::cleanupFolder(); + if ($this->pluginId) { + // cache current image + Plugin::find($this->pluginId)->update(['current_image' => $newImageUuid]); + } + + ImageGenerationService::cleanupFolder(); } } diff --git a/app/Models/Plugin.php b/app/Models/Plugin.php index 63ac7ba..fa5dbd6 100644 --- a/app/Models/Plugin.php +++ b/app/Models/Plugin.php @@ -17,7 +17,6 @@ class Plugin extends Model 'data_payload' => 'json', 'data_payload_updated_at' => 'datetime', 'is_native' => 'boolean', - 'current_image' => 'string', ]; protected static function boot() diff --git a/app/Jobs/CommonFunctions.php b/app/Services/ImageGenerationService.php similarity index 95% rename from app/Jobs/CommonFunctions.php rename to app/Services/ImageGenerationService.php index f8ede1b..a9e1b3b 100644 --- a/app/Jobs/CommonFunctions.php +++ b/app/Services/ImageGenerationService.php @@ -1,6 +1,6 @@ toString(); @@ -37,7 +37,7 @@ class CommonFunctions } try { - CommonFunctions::convertToBmpImageMagick($pngPath, $bmpPath); + ImageGenerationService::convertToBmpImageMagick($pngPath, $bmpPath); } catch (\ImagickException $e) { throw new \RuntimeException('Failed to convert image to BMP: '.$e->getMessage(), 0, $e); } diff --git a/resources/views/livewire/plugins/markup.blade.php b/resources/views/livewire/plugins/markup.blade.php index 8b1909c..01ddcd4 100644 --- a/resources/views/livewire/plugins/markup.blade.php +++ b/resources/views/livewire/plugins/markup.blade.php @@ -33,7 +33,7 @@ new class extends Component { try { $rendered = Blade::render($this->blade_code); foreach ($this->checked_devices as $device) { - GenerateScreenJob::dispatchSync($device, $rendered); + GenerateScreenJob::dispatchSync($device, null, $rendered); } } catch (\Exception $e) { $this->addError('error', $e->getMessage()); diff --git a/routes/api.php b/routes/api.php index 50f0d12..8b909de 100644 --- a/routes/api.php +++ b/routes/api.php @@ -1,7 +1,6 @@ mirrorDevice?->current_screen_image) { $refreshTimeOverride = null; - $nextPlaylistItem = $device->getNextPlaylistItem(); // Skip if cloud proxy is enabled for the device - if (! $device->proxy_cloud && $nextPlaylistItem) { - $refreshTimeOverride = $nextPlaylistItem->playlist()->first()->refresh_time; - $plugin = $nextPlaylistItem->plugin; + if (! $device->proxy_cloud || $device->getNextPlaylistItem()) { + $playlistItem = $device->getNextPlaylistItem(); + if ($playlistItem) { + $refreshTimeOverride = $playlistItem->playlist()->first()->refresh_time; + $plugin = $playlistItem->plugin; - // Check and update stale data if needed - if ($plugin->isDataStale() || $plugin->current_image == null) { - $plugin->updateDataPayload(); + // Check and update stale data if needed + if ($plugin->isDataStale() || $plugin->current_image == null) { + $plugin->updateDataPayload(); - if ($plugin->render_markup) { - $markup = Blade::render($plugin->render_markup, ['data' => $plugin->data_payload]); - } elseif ($plugin->render_markup_view) { - $markup = view($plugin->render_markup_view, ['data' => $plugin->data_payload])->render(); + if ($plugin->render_markup) { + $markup = Blade::render($plugin->render_markup, ['data' => $plugin->data_payload]); + } elseif ($plugin->render_markup_view) { + $markup = view($plugin->render_markup_view, ['data' => $plugin->data_payload])->render(); + } + + GenerateScreenJob::dispatchSync($device->id, $plugin->id, $markup); } - GeneratePluginJob::dispatchSync($plugin->id, $markup); - } + $plugin->refresh(); - $plugin->refresh(); - - if ($plugin->current_image != null) - { - $nextPlaylistItem->update(['last_displayed_at' => now()]); - $device->update(['current_screen_image' => $plugin->current_image]); + if ($plugin->current_image != null) { + $playlistItem->update(['last_displayed_at' => now()]); + $device->update(['current_screen_image' => $plugin->current_image]); + } } } @@ -198,11 +198,11 @@ Route::get('/devices', function (Request $request) { 'friendly_id', 'mac_address', 'last_battery_voltage as battery_voltage', - 'last_rssi_level as rssi' + 'last_rssi_level as rssi', ]); return response()->json([ - 'data' => $devices + 'data' => $devices, ]); })->middleware('auth:sanctum'); @@ -217,7 +217,7 @@ Route::post('/display/update', function (Request $request) { $view = Blade::render($request['markup']); - GenerateScreenJob::dispatchSync($deviceId, $view); + GenerateScreenJob::dispatchSync($deviceId, null, $view); response()->json([ 'message' => 'success', diff --git a/tests/Feature/Api/DeviceEndpointsTest.php b/tests/Feature/Api/DeviceEndpointsTest.php index f1b75e8..693d4c1 100644 --- a/tests/Feature/Api/DeviceEndpointsTest.php +++ b/tests/Feature/Api/DeviceEndpointsTest.php @@ -469,7 +469,7 @@ test('authenticated user can fetch their devices', function () { ]); }); -test('plugin doesn\'t update image unless required', function () { +test('plugin caches image until data is stale', function () { // Create source device with a playlist $device = Device::factory()->create([ 'mac_address' => '55:11:22:33:44:55', @@ -479,11 +479,11 @@ test('plugin doesn\'t update image unless required', function () { $plugin = Plugin::factory()->create([ 'name' => 'Zen Quotes', - 'polling_url' => 'https://zenquotes.io/api/today', + 'polling_url' => null, 'data_stale_minutes' => 1, 'data_strategy' => 'polling', 'polling_verb' => 'get', - 'render_markup_view' => 'recipes.zen', + 'render_markup_view' => 'trmnl', 'is_native' => false, 'data_payload_updated_at' => null, ]); @@ -543,3 +543,107 @@ test('plugin doesn\'t update image unless required', function () { expect($thirdResponse['filename']) ->not->toBe($firstResponse['filename']); })->skipOnGitHubActions(); + +test('plugins in playlist are rendered in order', function () { + // Create source device with a playlist + $device = Device::factory()->create([ + 'mac_address' => '55:11:22:33:44:55', + 'api_key' => 'source-api-key', + 'proxy_cloud' => true, + ]); + + // Create two plugins + $firstPlugin = Plugin::factory()->create([ + 'name' => 'First Plugin', + 'polling_url' => null, + 'data_stale_minutes' => 1, + 'data_strategy' => 'polling', + 'polling_verb' => 'get', + 'render_markup_view' => 'trmnl', + 'is_native' => false, + 'data_payload_updated_at' => null, + ]); + + $secondPlugin = Plugin::factory()->create([ + 'name' => 'Second Plugin', + 'polling_url' => null, + 'data_stale_minutes' => 1, + 'data_strategy' => 'polling', + 'polling_verb' => 'get', + 'render_markup_view' => 'trmnl', + 'is_native' => false, + 'data_payload_updated_at' => null, + ]); + + // Create playlist + $playlist = Playlist::factory()->create([ + 'device_id' => $device->id, + 'name' => 'Two Plugins Test', + 'is_active' => true, + 'weekdays' => null, + 'active_from' => null, + 'active_until' => null, + ]); + + // Add plugins to playlist in specific order + PlaylistItem::factory()->create([ + 'playlist_id' => $playlist->id, + 'plugin_id' => $firstPlugin->id, + 'order' => 1, + 'is_active' => true, + 'last_displayed_at' => null, + ]); + + PlaylistItem::factory()->create([ + 'playlist_id' => $playlist->id, + 'plugin_id' => $secondPlugin->id, + 'order' => 2, + 'is_active' => true, + 'last_displayed_at' => null, + ]); + + // First request should show the first plugin + $firstResponse = $this->withHeaders([ + 'id' => $device->mac_address, + 'access-token' => $device->api_key, + ])->get('/api/display'); + + $firstResponse->assertOk(); + $firstImageFilename = $firstResponse['filename']; + expect($firstImageFilename)->not->toBe('setup-logo.bmp'); + + // Get the first plugin's playlist item and verify it was marked as displayed + $firstPluginItem = PlaylistItem::where('plugin_id', $firstPlugin->id)->first(); + expect($firstPluginItem->last_displayed_at)->not->toBeNull(); + + // Second request should show the second plugin + $secondResponse = $this->withHeaders([ + 'id' => $device->mac_address, + 'access-token' => $device->api_key, + 'rssi' => -70, + 'battery_voltage' => 3.8, + 'fw-version' => '1.0.0', + ])->get('/api/display'); + + $secondResponse->assertOk(); + expect($secondResponse['filename']) + ->not->toBe($firstImageFilename) + ->not->toBe('setup-logo.bmp'); + + // Get the second plugin's playlist item and verify it was marked as displayed + $secondPluginItem = PlaylistItem::where('plugin_id', $secondPlugin->id)->first(); + expect($secondPluginItem->last_displayed_at)->not->toBeNull(); + + // Third request should show the first plugin again + $thirdResponse = $this->withHeaders([ + 'id' => $device->mac_address, + 'access-token' => $device->api_key, + 'rssi' => -70, + 'battery_voltage' => 3.8, + 'fw-version' => '1.0.0', + ])->get('/api/display'); + + $thirdResponse->assertOk(); + expect($thirdResponse['filename']) + ->not->toBe($secondResponse['filename']); +})->skipOnGitHubActions(); diff --git a/tests/Feature/GenerateScreenJobTest.php b/tests/Feature/GenerateScreenJobTest.php index e1f24ab..b146424 100644 --- a/tests/Feature/GenerateScreenJobTest.php +++ b/tests/Feature/GenerateScreenJobTest.php @@ -13,7 +13,7 @@ beforeEach(function () { test('it generates screen images and updates device', function () { $device = Device::factory()->create(); - $job = new GenerateScreenJob($device->id, view('trmnl')->render()); + $job = new GenerateScreenJob($device->id, null, view('trmnl')->render()); $job->handle(); // Assert the device was updated with a new image UUID @@ -39,7 +39,7 @@ test('it cleans up unused images', function () { Storage::disk('public')->put('/images/generated/inactive-uuid.bmp', 'test'); // Run a job which will trigger cleanup - $job = new GenerateScreenJob($activeDevice->id, '
Test
'); + $job = new GenerateScreenJob($activeDevice->id, null, '
Test
'); $job->handle(); Storage::disk('public')->assertMissing('/images/generated/uuid-to-be-replaced.png'); @@ -52,7 +52,7 @@ test('it preserves gitignore file during cleanup', function () { Storage::disk('public')->put('/images/generated/.gitignore', '*'); $device = Device::factory()->create(); - $job = new GenerateScreenJob($device->id, '
Test
'); + $job = new GenerateScreenJob($device->id, null, '
Test
'); $job->handle(); Storage::disk('public')->assertExists('/images/generated/.gitignore');