From c387260ee7a649a2bc287e8bb9e055b7681a2213 Mon Sep 17 00:00:00 2001 From: Matt Edholm Date: Wed, 6 May 2026 14:21:15 -0400 Subject: [PATCH] fix: include orientation in device 304 cache check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 304 short-circuit at DeviceImageController only compared image IDs, so flipping a device between landscape and portrait would not invalidate the cache: the device kept showing the previously-rendered .bin even after the user changed orientation in the webapp. Now the device row tracks currentImageOrientation — set whenever a 200 binary response is sent — and the 304 path requires both image id AND current orientation to match the device's stored orientation. An orientation flip naturally falls through to the 200 path on the next poll, the freshly-rendered portrait .bin is delivered, and the device redraws. No firmware change: the existing X-Current-Image-Id header from the device is sufficient. Existing devices migrate cleanly — null currentImageOrientation just forces one full re-send on first post- migration poll, which is harmless. Co-Authored-By: Claude Opus 4.7 (1M context) --- migrations/Version20260507210000.php | 26 ++++++++++++++ src/Controller/DeviceImageController.php | 12 ++++++- src/Entity/Device.php | 12 +++++++ .../Controller/DeviceImageControllerTest.php | 35 +++++++++++++++++++ 4 files changed, 84 insertions(+), 1 deletion(-) create mode 100644 migrations/Version20260507210000.php diff --git a/migrations/Version20260507210000.php b/migrations/Version20260507210000.php new file mode 100644 index 0000000..607e21e --- /dev/null +++ b/migrations/Version20260507210000.php @@ -0,0 +1,26 @@ +addSql('ALTER TABLE device ADD current_image_orientation VARCHAR(255) DEFAULT NULL'); + } + + public function down(Schema $schema): void + { + $this->addSql('ALTER TABLE device DROP COLUMN current_image_orientation'); + } +} diff --git a/src/Controller/DeviceImageController.php b/src/Controller/DeviceImageController.php index 05c808e..a8cd102 100644 --- a/src/Controller/DeviceImageController.php +++ b/src/Controller/DeviceImageController.php @@ -69,7 +69,10 @@ class DeviceImageController extends AbstractController } // 304: device already has this image — skip the binary transfer and redraw. - if ($image->getId() === $currentImageId) { + // Both the image and the orientation it was last rendered at must match; + // otherwise the device's cached .bin is stale and we must re-send. + if ($image->getId() === $currentImageId + && $device->getCurrentImageOrientation() === $device->getOrientation()) { $this->logger->info('device.poll.no_change', [ 'device_id' => $device->getId(), 'mac' => $mac, @@ -115,10 +118,17 @@ class DeviceImageController extends AbstractController return $r; } + // Record the orientation we're serving the image at so the next poll's + // 304 check can detect a flip and force a re-fetch. Flushed via the + // controller's EM (markSeen() above already dirties the row). + $device->setCurrentImageOrientation($device->getOrientation()); + $em->flush(); + $this->logger->info('device.poll.served', [ 'device_id' => $device->getId(), 'mac' => $mac, 'image_id' => $image->getId(), + 'orientation' => $device->getOrientation()->value, 'interval_ms' => $intervalMs, 'bytes' => filesize($binPath), ]); diff --git a/src/Entity/Device.php b/src/Entity/Device.php index e25f40e..fa2beb6 100644 --- a/src/Entity/Device.php +++ b/src/Entity/Device.php @@ -65,6 +65,15 @@ class Device #[ORM\Column(nullable: true)] private ?\DateTimeImmutable $lastSeenAt = null; + /** + * Orientation in effect when currentImage was last served as a 200 response. + * Used alongside currentImage's id to decide whether a poll can be answered + * with 304: if the device's orientation has flipped since, the cached image + * on-device is stale and we must re-send the freshly-rendered .bin. + */ + #[ORM\Column(nullable: true, enumType: Orientation::class)] + private ?Orientation $currentImageOrientation = null; + public function __construct() { $this->linkedAt = new \DateTimeImmutable(); @@ -110,4 +119,7 @@ class Device public function getLastSeenAt(): ?\DateTimeImmutable { return $this->lastSeenAt; } public function markSeen(): static { $this->lastSeenAt = new \DateTimeImmutable(); return $this; } + + public function getCurrentImageOrientation(): ?Orientation { return $this->currentImageOrientation; } + public function setCurrentImageOrientation(?Orientation $o): static { $this->currentImageOrientation = $o; return $this; } } diff --git a/tests/Functional/Controller/DeviceImageControllerTest.php b/tests/Functional/Controller/DeviceImageControllerTest.php index f8e7706..6b30d0c 100644 --- a/tests/Functional/Controller/DeviceImageControllerTest.php +++ b/tests/Functional/Controller/DeviceImageControllerTest.php @@ -167,6 +167,9 @@ class DeviceImageControllerTest extends AppWebTestCase $imageId = $image->getId(); $device->setLockedImage($image); + // Simulate the device having already received this image at the current + // orientation — the 304 path now requires this to match too. + $device->setCurrentImageOrientation(Orientation::Landscape); $this->em()->flush(); $this->client->request('GET', '/api/device/' . self::MAC . '/image', [], [], [ @@ -176,6 +179,38 @@ class DeviceImageControllerTest extends AppWebTestCase $this->assertResponseStatusCodeSame(304); } + public function test_orientation_flip_returns_200_even_when_image_id_matches(): void + { + $setup = $this->createTestSetup(); + $device = $setup['device']; + $image = $setup['image']; + $imageId = $image->getId(); + + // Seed: device receives the image at landscape orientation. + $this->client->request('GET', '/api/device/' . self::MAC . '/image'); + $this->assertResponseStatusCodeSame(200); + + // User flips device to portrait and a portrait render is ready. + $this->em()->clear(); + $device = $this->em()->find(Device::class, $setup['device']->getId()); + $device->setOrientation(Orientation::Portrait); + $portraitAsset = (new RenderedAsset()) + ->setImage($this->em()->find(Image::class, $imageId)) + ->setDeviceModel(DeviceModel::V1) + ->setOrientation(Orientation::Portrait) + ->setStatus(RenderStatus::Ready) + ->setFilePath(self::BIN_PATH); + $this->em()->persist($portraitAsset); + $this->em()->flush(); + + // Same image ID, but device's stored orientation (landscape) no longer + // matches the device's current orientation (portrait) → must re-send. + $this->client->request('GET', '/api/device/' . self::MAC . '/image', [], [], [ + 'HTTP_X-Current-Image-Id' => (string) $imageId, + ]); + $this->assertResponseStatusCodeSame(200); + } + public function test_poll_advances_current_image(): void { $setup = $this->createTestSetup();