From 26b5f3ceb1ad5f502fce8512360dc9d272df9b4e Mon Sep 17 00:00:00 2001 From: Benjamin Nussbaum Date: Fri, 27 Feb 2026 17:38:28 +0100 Subject: [PATCH] feat(#194): refactor cache to be device specific --- app/Jobs/GenerateScreenJob.php | 9 +- app/Models/Plugin.php | 2 + app/Services/ImageGenerationService.php | 104 +++++++++--- ...urrent_image_metadata_to_plugins_table.php | 28 +++ routes/api.php | 15 +- tests/Feature/GenerateScreenJobTest.php | 25 +++ tests/Feature/ImageGenerationServiceTest.php | 160 +++++++++++++----- .../Services/ImageGenerationServiceTest.php | 45 +---- 8 files changed, 278 insertions(+), 110 deletions(-) create mode 100644 database/migrations/2026_02_27_153837_add_current_image_metadata_to_plugins_table.php diff --git a/app/Jobs/GenerateScreenJob.php b/app/Jobs/GenerateScreenJob.php index b9661cc..4af43dd 100644 --- a/app/Jobs/GenerateScreenJob.php +++ b/app/Jobs/GenerateScreenJob.php @@ -34,8 +34,13 @@ class GenerateScreenJob implements ShouldQueue Device::find($this->deviceId)->update(['current_screen_image' => $newImageUuid]); if ($this->pluginId) { - // cache current image - Plugin::find($this->pluginId)->update(['current_image' => $newImageUuid]); + $plugin = Plugin::find($this->pluginId); + $update = ['current_image' => $newImageUuid]; + if ($plugin->plugin_type === 'recipe') { + $device = Device::with(['deviceModel', 'deviceModel.palette'])->find($this->deviceId); + $update['current_image_metadata'] = ImageGenerationService::buildImageMetadataFromDevice($device); + } + $plugin->update($update); } ImageGenerationService::cleanupFolder(); diff --git a/app/Models/Plugin.php b/app/Models/Plugin.php index 3f84bc5..d99d988 100644 --- a/app/Models/Plugin.php +++ b/app/Models/Plugin.php @@ -49,6 +49,7 @@ class Plugin extends Model 'preferred_renderer' => 'string', 'plugin_type' => 'string', 'alias' => 'boolean', + 'current_image_metadata' => 'array', ]; protected static function boot() @@ -71,6 +72,7 @@ class Plugin extends Model 'render_markup_shared', ])) { $model->current_image = null; + $model->current_image_metadata = null; } }); diff --git a/app/Services/ImageGenerationService.php b/app/Services/ImageGenerationService.php index fa6f325..75f374e 100644 --- a/app/Services/ImageGenerationService.php +++ b/app/Services/ImageGenerationService.php @@ -331,36 +331,88 @@ class ImageGenerationService } } - public static function resetIfNotCacheable(?Plugin $plugin): void + /** + * Ensure plugin image cache is valid for the current context. No-op for image_webhook. + * When deviceOrModel is provided (recipe only), clears cache if stored metadata does not match. + */ + public static function resetIfNotCacheable(?Plugin $plugin, Device|DeviceModel|null $deviceOrModel = null): void { - if ($plugin?->id) { - // Image webhook plugins have finalized images that shouldn't be reset - if ($plugin->plugin_type === 'image_webhook') { - return; - } - // Check if any devices have custom dimensions or use non-standard DeviceModels - $hasCustomDimensions = Device::query() - ->where(function ($query): void { - $query->where('width', '!=', 800) - ->orWhere('height', '!=', 480) - ->orWhere('rotate', '!=', 0); - }) - ->orWhereHas('deviceModel', function ($query): void { - // Only allow caching if all device models have standard dimensions (800x480, rotation=0) - $query->where(function ($subQuery): void { - $subQuery->where('width', '!=', 800) - ->orWhere('height', '!=', 480) - ->orWhere('rotation', '!=', 0); - }); - }) - ->exists(); + if (! $plugin?->id || $plugin->plugin_type === 'image_webhook') { + return; + } + if ($deviceOrModel === null || $plugin->plugin_type !== 'recipe') { + return; + } + if ($plugin->current_image === null) { + return; + } + if (self::imageMetadataMatches($plugin->current_image_metadata, $deviceOrModel)) { + return; + } + $plugin->update([ + 'current_image' => null, + 'current_image_metadata' => null, + ]); + Log::debug("Plugin {$plugin->id}: cleared image cache due to metadata mismatch"); + } - if ($hasCustomDimensions) { - // TODO cache image per device - $plugin->update(['current_image' => null]); - Log::debug('Skip cache as devices with custom dimensions or non-standard DeviceModels exist'); + /** + * Build canonical image metadata from a Device for cache comparison. + * + * @return array{width: int, height: int, rotation: int, palette_id: int|null, mime_type: string} + */ + public static function buildImageMetadataFromDevice(Device $device): array + { + $device->loadMissing(['deviceModel', 'deviceModel.palette']); + $settings = self::getImageSettings($device); + $paletteId = $device->palette_id ?? $device->deviceModel?->palette_id; + + return [ + 'width' => $settings['width'], + 'height' => $settings['height'], + 'rotation' => $settings['rotation'] ?? 0, + 'palette_id' => $paletteId, + 'mime_type' => $settings['mime_type'], + ]; + } + + /** + * Build canonical image metadata from a DeviceModel for cache comparison. + * + * @return array{width: int, height: int, rotation: int, palette_id: int|null, mime_type: string} + */ + public static function buildImageMetadataFromDeviceModel(DeviceModel $model): array + { + return [ + 'width' => $model->width, + 'height' => $model->height, + 'rotation' => $model->rotation ?? 0, + 'palette_id' => $model->palette_id, + 'mime_type' => $model->mime_type, + ]; + } + + /** + * Check if stored metadata matches the current device or device model. + * Returns false if stored is null or empty so cache is regenerated and metadata is stored. + */ + public static function imageMetadataMatches(?array $stored, Device|DeviceModel $deviceOrModel): bool + { + if ($stored === null || $stored === []) { + return false; + } + + $current = $deviceOrModel instanceof Device + ? self::buildImageMetadataFromDevice($deviceOrModel) + : self::buildImageMetadataFromDeviceModel($deviceOrModel); + + foreach (['width', 'height', 'rotation', 'palette_id', 'mime_type'] as $key) { + if (($stored[$key] ?? null) !== ($current[$key] ?? null)) { + return false; } } + + return true; } /** diff --git a/database/migrations/2026_02_27_153837_add_current_image_metadata_to_plugins_table.php b/database/migrations/2026_02_27_153837_add_current_image_metadata_to_plugins_table.php new file mode 100644 index 0000000..d212fe7 --- /dev/null +++ b/database/migrations/2026_02_27_153837_add_current_image_metadata_to_plugins_table.php @@ -0,0 +1,28 @@ +json('current_image_metadata')->nullable()->after('current_image'); + }); + } + + /** + * Reverse the migrations. + */ + public function down(): void + { + Schema::table('plugins', function (Blueprint $table) { + $table->dropColumn('current_image_metadata'); + }); + } +}; diff --git a/routes/api.php b/routes/api.php index c759d61..8fc040c 100644 --- a/routes/api.php +++ b/routes/api.php @@ -88,8 +88,8 @@ Route::get('/display', function (Request $request) { $refreshTimeOverride = $playlistItem->playlist()->first()->refresh_time; $plugin = $playlistItem->plugin; - // Reset cache if Devices with different dimensions exist - ImageGenerationService::resetIfNotCacheable($plugin); + ImageGenerationService::resetIfNotCacheable($plugin, $device); + $plugin->refresh(); // Check and update stale data if needed if ($plugin->isDataStale() || $plugin->current_image === null) { @@ -699,6 +699,9 @@ Route::get('/display/{uuid}/alias', function (Request $request, string $uuid) { ], 404); } + ImageGenerationService::resetIfNotCacheable($plugin, $deviceModel); + $plugin->refresh(); + // Check if we can use cached image (only for og_png and if data is not stale) $useCache = $deviceModelName === 'og_png' && ! $plugin->isDataStale() && $plugin->current_image !== null; @@ -744,9 +747,13 @@ Route::get('/display/{uuid}/alias', function (Request $request, string $uuid) { palette: $deviceModel->palette ); - // Update plugin cache if using og_png + // Update plugin cache if using og_png (recipes only get metadata for cache comparison) if ($deviceModelName === 'og_png') { - $plugin->update(['current_image' => $imageUuid]); + $update = ['current_image' => $imageUuid]; + if ($plugin->plugin_type === 'recipe') { + $update['current_image_metadata'] = ImageGenerationService::buildImageMetadataFromDeviceModel($deviceModel); + } + $plugin->update($update); } // Return the generated image diff --git a/tests/Feature/GenerateScreenJobTest.php b/tests/Feature/GenerateScreenJobTest.php index 115fb51..9482e8c 100644 --- a/tests/Feature/GenerateScreenJobTest.php +++ b/tests/Feature/GenerateScreenJobTest.php @@ -2,6 +2,8 @@ use App\Jobs\GenerateScreenJob; use App\Models\Device; +use App\Models\DeviceModel; +use App\Models\Plugin; use Bnussbau\TrmnlPipeline\TrmnlPipeline; use Illuminate\Support\Facades\Storage; @@ -58,3 +60,26 @@ test('it preserves gitignore file during cleanup', function (): void { Storage::disk('public')->assertExists('/images/generated/.gitignore'); }); + +test('it saves current_image_metadata for recipe plugins', function (): void { + $deviceModel = DeviceModel::factory()->create([ + 'width' => 800, + 'height' => 480, + 'rotation' => 0, + 'mime_type' => 'image/png', + 'palette_id' => null, + ]); + $device = Device::factory()->create(['device_model_id' => $deviceModel->id]); + $plugin = Plugin::factory()->create(['plugin_type' => 'recipe']); + + $job = new GenerateScreenJob($device->id, $plugin->id, '
Test
'); + $job->handle(); + + $plugin->refresh(); + expect($plugin->current_image)->not->toBeNull(); + expect($plugin->current_image_metadata)->toBeArray(); + expect($plugin->current_image_metadata)->toHaveKeys(['width', 'height', 'rotation', 'palette_id', 'mime_type']); + expect($plugin->current_image_metadata['width'])->toBe(800); + expect($plugin->current_image_metadata['height'])->toBe(480); + expect($plugin->current_image_metadata['mime_type'])->toBe('image/png'); +}); diff --git a/tests/Feature/ImageGenerationServiceTest.php b/tests/Feature/ImageGenerationServiceTest.php index 07bb6a6..9d26eb6 100644 --- a/tests/Feature/ImageGenerationServiceTest.php +++ b/tests/Feature/ImageGenerationServiceTest.php @@ -8,6 +8,7 @@ use App\Models\DeviceModel; use App\Services\ImageGenerationService; use Bnussbau\TrmnlPipeline\TrmnlPipeline; use Illuminate\Foundation\Testing\RefreshDatabase; +use Illuminate\Support\Facades\Schema; use Illuminate\Support\Facades\Storage; uses(RefreshDatabase::class); @@ -22,6 +23,10 @@ afterEach(function (): void { TrmnlPipeline::restore(); }); +it('plugins table has current_image_metadata column', function (): void { + expect(Schema::hasColumn('plugins', 'current_image_metadata'))->toBeTrue(); +}); + it('generates image for device without device model', function (): void { // Create a device without a DeviceModel (legacy behavior) $device = Device::factory()->create([ @@ -270,39 +275,15 @@ it('cleanupFolder preserves .gitignore', function (): void { Storage::disk('public')->assertExists('/images/generated/.gitignore'); }); -it('resetIfNotCacheable resets when device models exist', function (): void { - // Create a plugin - $plugin = App\Models\Plugin::factory()->create(['current_image' => 'test-uuid']); +it('resetIfNotCacheable does not reset recipe cache based on other devices', function (): void { + // Cache validity is now determined at use-time via current_image_metadata + $plugin = App\Models\Plugin::factory()->create(['current_image' => 'test-uuid', 'plugin_type' => 'recipe']); - // Create a device with DeviceModel (should trigger cache reset) - Device::factory()->create([ - 'device_model_id' => DeviceModel::factory()->create()->id, - ]); - - // Run reset check + Device::factory()->create(['device_model_id' => DeviceModel::factory()->create()->id]); ImageGenerationService::resetIfNotCacheable($plugin); - // Assert plugin image was reset $plugin->refresh(); - expect($plugin->current_image)->toBeNull(); -}); - -it('resetIfNotCacheable resets when custom dimensions exist', function (): void { - // Create a plugin - $plugin = App\Models\Plugin::factory()->create(['current_image' => 'test-uuid']); - - // Create a device with custom dimensions (should trigger cache reset) - Device::factory()->create([ - 'width' => 1024, // Different from default 800 - 'height' => 768, // Different from default 480 - ]); - - // Run reset check - ImageGenerationService::resetIfNotCacheable($plugin); - - // Assert plugin image was reset - $plugin->refresh(); - expect($plugin->current_image)->toBeNull(); + expect($plugin->current_image)->toBe('test-uuid'); }); it('resetIfNotCacheable preserves image for standard devices', function (): void { @@ -325,27 +306,122 @@ it('resetIfNotCacheable preserves image for standard devices', function (): void }); it('cache is reset when plugin markup changes', function (): void { - // Create a plugin with cached image + // Create a plugin with cached image and metadata $plugin = App\Models\Plugin::factory()->create([ 'current_image' => 'cached-uuid', + 'current_image_metadata' => ['width' => 800, 'height' => 480, 'rotation' => 0, 'palette_id' => null, 'mime_type' => 'image/png'], 'render_markup' => '
Original markup
', ]); - // Create devices with standard dimensions (cacheable) - Device::factory()->count(2)->create([ - 'width' => 800, - 'height' => 480, - 'rotate' => 0, - ]); + $plugin->update(['render_markup' => '
Updated markup
']); - // Update the plugin markup - $plugin->update([ - 'render_markup' => '
Updated markup
', - ]); - - // Assert cache was reset when markup changed $plugin->refresh(); expect($plugin->current_image)->toBeNull(); + expect($plugin->current_image_metadata)->toBeNull(); +}); + +it('buildImageMetadataFromDevice returns canonical metadata shape', function (): void { + $deviceModel = DeviceModel::factory()->create([ + 'width' => 800, + 'height' => 480, + 'rotation' => 0, + 'mime_type' => 'image/png', + 'palette_id' => null, + ]); + $device = Device::factory()->create(['device_model_id' => $deviceModel->id]); + + $meta = ImageGenerationService::buildImageMetadataFromDevice($device); + + expect($meta)->toHaveKeys(['width', 'height', 'rotation', 'palette_id', 'mime_type']); + expect($meta['width'])->toBe(800); + expect($meta['height'])->toBe(480); + expect($meta['rotation'])->toBe(0); + expect($meta['mime_type'])->toBe('image/png'); +}); + +it('buildImageMetadataFromDeviceModel returns canonical metadata shape', function (): void { + $model = DeviceModel::factory()->create([ + 'width' => 1024, + 'height' => 768, + 'rotation' => 90, + 'mime_type' => 'image/bmp', + 'palette_id' => null, + ]); + + $meta = ImageGenerationService::buildImageMetadataFromDeviceModel($model); + + expect($meta)->toHaveKeys(['width', 'height', 'rotation', 'palette_id', 'mime_type']); + expect($meta['width'])->toBe(1024); + expect($meta['height'])->toBe(768); + expect($meta['rotation'])->toBe(90); + expect($meta['mime_type'])->toBe('image/bmp'); +}); + +it('imageMetadataMatches returns false when stored is null or empty', function (): void { + $device = Device::factory()->create(['width' => 800, 'height' => 480, 'rotate' => 0]); + + expect(ImageGenerationService::imageMetadataMatches(null, $device))->toBeFalse(); + expect(ImageGenerationService::imageMetadataMatches([], $device))->toBeFalse(); +}); + +it('imageMetadataMatches returns true when metadata matches device', function (): void { + $deviceModel = DeviceModel::factory()->create([ + 'width' => 800, + 'height' => 480, + 'rotation' => 0, + 'mime_type' => 'image/png', + 'palette_id' => null, + ]); + $device = Device::factory()->create(['device_model_id' => $deviceModel->id]); + $stored = ImageGenerationService::buildImageMetadataFromDevice($device); + + expect(ImageGenerationService::imageMetadataMatches($stored, $device))->toBeTrue(); +}); + +it('imageMetadataMatches returns false when metadata differs', function (): void { + $device = Device::factory()->create(['width' => 800, 'height' => 480, 'rotate' => 0]); + $stored = ['width' => 800, 'height' => 480, 'rotation' => 0, 'palette_id' => null, 'mime_type' => 'image/png']; + + $device->update(['width' => 1024]); + $device->refresh(); + + expect(ImageGenerationService::imageMetadataMatches($stored, $device))->toBeFalse(); +}); + +it('resetIfNotCacheable clears recipe cache when metadata does not match', function (): void { + $plugin = App\Models\Plugin::factory()->create([ + 'plugin_type' => 'recipe', + 'current_image' => 'cached-uuid', + 'current_image_metadata' => ['width' => 800, 'height' => 480, 'rotation' => 0, 'palette_id' => null, 'mime_type' => 'image/png'], + ]); + $device = Device::factory()->create(['width' => 1024, 'height' => 768, 'rotate' => 0]); + + ImageGenerationService::resetIfNotCacheable($plugin, $device); + + $plugin->refresh(); + expect($plugin->current_image)->toBeNull(); + expect($plugin->current_image_metadata)->toBeNull(); +}); + +it('resetIfNotCacheable preserves cache when metadata matches', function (): void { + $deviceModel = DeviceModel::factory()->create([ + 'width' => 800, + 'height' => 480, + 'rotation' => 0, + 'mime_type' => 'image/png', + ]); + $device = Device::factory()->create(['device_model_id' => $deviceModel->id]); + $meta = ImageGenerationService::buildImageMetadataFromDevice($device); + $plugin = App\Models\Plugin::factory()->create([ + 'plugin_type' => 'recipe', + 'current_image' => 'cached-uuid', + 'current_image_metadata' => $meta, + ]); + + ImageGenerationService::resetIfNotCacheable($plugin, $device); + + $plugin->refresh(); + expect($plugin->current_image)->toBe('cached-uuid'); }); it('determines correct image format from device model', function (): void { diff --git a/tests/Unit/Services/ImageGenerationServiceTest.php b/tests/Unit/Services/ImageGenerationServiceTest.php index 5e3dc47..da9bef9 100644 --- a/tests/Unit/Services/ImageGenerationServiceTest.php +++ b/tests/Unit/Services/ImageGenerationServiceTest.php @@ -176,37 +176,15 @@ it('cleanup_folder identifies active images correctly', function (): void { expect($activeImageUuids)->not->toContain(null); }); -it('reset_if_not_cacheable detects device models', function (): void { - // Create a plugin - $plugin = App\Models\Plugin::factory()->create(['current_image' => 'test-uuid']); +it('reset_if_not_cacheable does not reset recipe cache when other devices exist', function (): void { + // Cache validity is now determined at use-time via metadata + $plugin = App\Models\Plugin::factory()->create(['current_image' => 'test-uuid', 'plugin_type' => 'recipe']); + Device::factory()->create(['device_model_id' => DeviceModel::factory()->create()->id]); - // Create a device with DeviceModel - Device::factory()->create([ - 'device_model_id' => DeviceModel::factory()->create()->id, - ]); - - // Test that the method detects DeviceModels and resets cache ImageGenerationService::resetIfNotCacheable($plugin); $plugin->refresh(); - expect($plugin->current_image)->toBeNull(); -}); - -it('reset_if_not_cacheable detects custom dimensions', function (): void { - // Create a plugin - $plugin = App\Models\Plugin::factory()->create(['current_image' => 'test-uuid']); - - // Create a device with custom dimensions - Device::factory()->create([ - 'width' => 1024, // Different from default 800 - 'height' => 768, // Different from default 480 - ]); - - // Test that the method detects custom dimensions and resets cache - ImageGenerationService::resetIfNotCacheable($plugin); - - $plugin->refresh(); - expect($plugin->current_image)->toBeNull(); + expect($plugin->current_image)->toBe('test-uuid'); }); it('reset_if_not_cacheable preserves cache for standard devices', function (): void { @@ -258,26 +236,21 @@ it('reset_if_not_cacheable preserves cache for og_png and og_plus device models' expect($plugin->current_image)->toBe('test-uuid'); }); -it('reset_if_not_cacheable resets cache for non-standard device models', function (): void { - // Create a plugin - $plugin = App\Models\Plugin::factory()->create(['current_image' => 'test-uuid']); - - // Create a non-standard device model (e.g., kindle) +it('reset_if_not_cacheable does not reset cache for non-standard device models', function (): void { + // Cache is now validated at use-time via metadata comparison + $plugin = App\Models\Plugin::factory()->create(['current_image' => 'test-uuid', 'plugin_type' => 'recipe']); $kindleModel = DeviceModel::factory()->create([ 'name' => 'test_amazon_kindle_2024', 'width' => 1400, 'height' => 840, 'rotation' => 90, ]); - - // Create a device with the non-standard device model Device::factory()->create(['device_model_id' => $kindleModel->id]); - // Test that the method resets cache for non-standard device models ImageGenerationService::resetIfNotCacheable($plugin); $plugin->refresh(); - expect($plugin->current_image)->toBeNull(); + expect($plugin->current_image)->toBe('test-uuid'); }); it('reset_if_not_cacheable handles null plugin', function (): void {