fix(rotation): isDue() compares wakeTime boundary in UTC, not device-local tz
CI / test (push) Has been cancelled
CI / test (push) Has been cancelled
Symptom: wakeTimes schedules silently never fire on non-UTC devices.
Reported live by Matt's EDT frame: wakeTimes=[12:30 PM NY] saved,
12:30 came and went, no rotation. Same bug pattern would fire
*every* poll on east-of-UTC tzs.
Root cause: device_image_history.served_at is `timestamp without time
zone`, written by `new DateTimeImmutable()` so it stores UTC
components ("2026-05-08 16:28:50"). The boundary in isDue() was
bound through Doctrine with the device's local tz still attached,
so Doctrine's format() emitted local-tz components ("12:30:00").
Postgres compared the strings literally — for west-of-UTC tzs the
UTC timestamp is numerically larger than the local-tz boundary, so
every same-day row falsely satisfied `servedAt >= :wakeTime` and
isDue returned false.
Fix: $boundary->setTimezone(UTC) before binding. Both sides now
format in UTC components, so Postgres's literal compare is correct.
Regression test ID-TZ-01: device in America/New_York, wakeTimes
[12:30 PM NY], history at 12:00 PM NY (= 16:00 UTC). With the fix
isDue returns true; without it the test falsely-matches and fails.
Skipped before 13:00 NY since the assertion needs the wake slot to
have already passed today.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -53,13 +53,24 @@ class RotationService
|
|||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// device_image_history.served_at is `timestamp without time zone`
|
||||||
|
// and entries are written by `new DateTimeImmutable()` (UTC). The
|
||||||
|
// boundary above carries the device's local tz, which Doctrine
|
||||||
|
// would format with local-tz components — leading to a string
|
||||||
|
// comparison against UTC-formatted rows in PostgreSQL. For
|
||||||
|
// anything other than UTC that mismatch quietly breaks `isDue`:
|
||||||
|
// west-of-UTC tzs falsely match every same-day row (rotation
|
||||||
|
// never fires); east-of-UTC tzs falsely miss every row (rotation
|
||||||
|
// fires every poll). Bind in UTC so both sides agree.
|
||||||
|
$boundaryUtc = $boundary->setTimezone(new \DateTimeZone('UTC'));
|
||||||
|
|
||||||
$entry = $this->em->createQueryBuilder()
|
$entry = $this->em->createQueryBuilder()
|
||||||
->select('h')
|
->select('h')
|
||||||
->from(DeviceImageHistory::class, 'h')
|
->from(DeviceImageHistory::class, 'h')
|
||||||
->where('h.device = :device')
|
->where('h.device = :device')
|
||||||
->andWhere('h.servedAt >= :wakeTime')
|
->andWhere('h.servedAt >= :wakeTime')
|
||||||
->setParameter('device', $device)
|
->setParameter('device', $device)
|
||||||
->setParameter('wakeTime', $boundary)
|
->setParameter('wakeTime', $boundaryUtc)
|
||||||
->setMaxResults(1)
|
->setMaxResults(1)
|
||||||
->getQuery()
|
->getQuery()
|
||||||
->getOneOrNullResult();
|
->getOneOrNullResult();
|
||||||
|
|||||||
@@ -322,6 +322,53 @@ class RotationServiceTest extends AppKernelTestCase
|
|||||||
self::em()->flush();
|
self::em()->flush();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ID-TZ-01: Regression — isDue() in a non-UTC device timezone.
|
||||||
|
//
|
||||||
|
// Bug: device_image_history.served_at is `timestamp without time zone`
|
||||||
|
// storing UTC components, while the boundary in isDue() was bound with
|
||||||
|
// the device's local-tz components. Postgres compared the strings
|
||||||
|
// literally, so for tzs west of UTC every same-day row falsely matched
|
||||||
|
// the `>= boundary` predicate and isDue returned false — wakeTimes
|
||||||
|
// schedules silently never fired in EDT/PST/CST/etc.
|
||||||
|
//
|
||||||
|
// Real-world repro (Matt's EDT frame, 2026-05-08): wakeTimes=[12:30 PM
|
||||||
|
// NY], history at 12:28:50 EDT = 16:28:50 UTC. Boundary serialized as
|
||||||
|
// "12:30:00" (NY components), history stored as "16:28:50" (UTC):
|
||||||
|
// "16:28:50" >= "12:30:00" lexically → false-match → bug. Fix: bind
|
||||||
|
// the boundary in UTC so both sides agree.
|
||||||
|
//
|
||||||
|
// Test windowing: needs NY local to be past 12:30 PM (so the wake slot
|
||||||
|
// qualifies as "today's most-recent past"). Skipped before 13:00 NY.
|
||||||
|
public function test_id_tz_01_isDue_correct_in_non_utc_timezone(): void
|
||||||
|
{
|
||||||
|
$nyHour = (int) (new \DateTimeImmutable('now', new \DateTimeZone('America/New_York')))->format('G');
|
||||||
|
if ($nyHour < 13) {
|
||||||
|
$this->markTestSkipped('Test exercises a 12:30 PM NY-local boundary; skip until after 13:00 NY');
|
||||||
|
}
|
||||||
|
|
||||||
|
[$device, $images] = $this->setupDeviceAndImages('tz-edt@example.com', [
|
||||||
|
'2024-01-01',
|
||||||
|
]);
|
||||||
|
$device->setTimezone('America/New_York');
|
||||||
|
$device->setWakeTimes([12 * 60 + 30]); // 12:30 PM NY-local
|
||||||
|
self::em()->flush();
|
||||||
|
|
||||||
|
// Record a history row at 12:00 PM NY today, stored as UTC components
|
||||||
|
// (16:00:00 in EDT, 17:00:00 in EST). The buggy comparison sees
|
||||||
|
// "16:00:00" >= "12:30:00" (boundary was NY-formatted) → falsely
|
||||||
|
// matches → isDue returns false. The fixed comparison binds the
|
||||||
|
// boundary as "16:30:00" UTC → "16:00:00" < "16:30:00" → no match →
|
||||||
|
// isDue returns true.
|
||||||
|
$historyUtc = (new \DateTimeImmutable('today 12:00:00', new \DateTimeZone('America/New_York')))
|
||||||
|
->setTimezone(new \DateTimeZone('UTC'));
|
||||||
|
$this->recordHistoryAt($device, $images[0], $historyUtc->format('Y-m-d H:i:s'));
|
||||||
|
|
||||||
|
$this->assertTrue(
|
||||||
|
$this->rotation->isDue($device),
|
||||||
|
'isDue must use a tz-consistent comparison; the regression is invisible on UTC-tz devices but breaks every other tz',
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
// RM-01: NewestUpload picks the most recent upload.
|
// RM-01: NewestUpload picks the most recent upload.
|
||||||
public function test_newest_upload_mode_picks_newest(): void
|
public function test_newest_upload_mode_picks_newest(): void
|
||||||
{
|
{
|
||||||
|
|||||||
Reference in New Issue
Block a user