diff --git a/src/server/store/drizzle-timelines.ts b/src/server/store/drizzle-timelines.ts index 8a2347b..ed08cbc 100644 --- a/src/server/store/drizzle-timelines.ts +++ b/src/server/store/drizzle-timelines.ts @@ -112,20 +112,21 @@ export function createTimelineStore(database: ModuleDatabase): SignageTimelineSt /** * Rewrites `position` for one timeline from an explicit order. * - * Positions are written in two passes, through a temporary negative range, because the unique + * Positions are written in two passes, through a temporary high range, because the unique * `(timeline_id, position)` index is checked per statement: moving a slot from 3 to 1 while - * another still holds 1 would collide. Negative positions are outside every real value, so the - * intermediate state cannot be mistaken for an order. + * another still holds 1 would collide. Temporary positions sit beyond the final order while + * remaining inside the database's non-negative constraint. */ async function renumber( transaction: ModuleDatabase, timelineId: string, orderedSlotIds: readonly string[], ): Promise { + const temporaryOffset = orderedSlotIds.length + 1; for (const [index, slotId] of orderedSlotIds.entries()) { await transaction .update(timelineSlots) - .set({ position: -1 - index }) + .set({ position: temporaryOffset + index }) .where(eq(timelineSlots.id, slotId)); } for (const [index, slotId] of orderedSlotIds.entries()) { @@ -175,7 +176,7 @@ export function createTimelineStore(database: ModuleDatabase): SignageTimelineSt position === undefined ? existing.length : Math.max(0, Math.min(position, existing.length)); - await transaction.insert(timelineSlots).values(slotValues(slot, -1_000_000)); + await transaction.insert(timelineSlots).values(slotValues(slot, existing.length + 1)); const next = [...existing.slice(0, index), slot.id, ...existing.slice(index)]; await renumber(transaction, slot.timelineId, next); }); diff --git a/tests/drizzle-timelines.test.ts b/tests/drizzle-timelines.test.ts new file mode 100644 index 0000000..07add12 --- /dev/null +++ b/tests/drizzle-timelines.test.ts @@ -0,0 +1,95 @@ +import type { ModuleDatabase } from "@ekklesia/module-sdk"; +import { describe, expect, it } from "vitest"; +import { createTimelineStore } from "../src/server/store/drizzle-timelines.js"; +import type { NewStoredSlot } from "../src/server/store/types.js"; + +const TIMELINE_ID = "00000000-0000-4000-8000-000000000001"; + +function slot(id: string): NewStoredSlot { + return { + durationOverrideMs: null, + eligibleFrom: null, + eligibleUntil: null, + enabled: true, + id, + itemId: "image-1", + kind: "pinned-item", + presentation: { + backgroundColor: null, + backgroundDarkened: null, + backgroundMode: null, + blurPixels: null, + foregroundShadow: null, + }, + providerId: "signage.native-images", + sourceId: null, + timelineId: TIMELINE_ID, + }; +} + +/** + * A narrow database contract double for the production timeline store. + * + * Unlike the service-level memory store, it enforces the shipped PostgreSQL + * `signage_timeline_slots_position_check` on every insert and update. That is the boundary the + * deployed bug crossed: the old collision-avoidance algorithm used negative positions, so the + * actual database rejected a native-image slot before playback ever had content to resolve. + */ +function constraintDatabase(existing: readonly { id: string; position: number }[]): { + readonly database: ModuleDatabase; + readonly positions: number[]; +} { + const positions: number[] = []; + const enforce = (values: { position?: unknown }): void => { + if (typeof values.position === "number") { + expect(values.position).toBeGreaterThanOrEqual(0); + positions.push(values.position); + } + }; + const transaction = { + insert: () => ({ + values: async (values: { position?: unknown }) => enforce(values), + }), + select: () => ({ + from: () => ({ + where: () => ({ + orderBy: async () => existing, + }), + }), + }), + update: () => ({ + set: (values: { position?: unknown }) => ({ + where: async () => enforce(values), + }), + }), + }; + return { + database: { + transaction: async (operation: (value: typeof transaction) => Promise) => operation(transaction), + } as unknown as ModuleDatabase, + positions, + }; +} + +describe("production timeline slot ordering", () => { + it("creates the first pinned native image without violating the database position constraint", async () => { + const { database, positions } = constraintDatabase([]); + + await createTimelineStore(database).addSlot(slot("00000000-0000-4000-8000-000000000002")); + + expect(positions).toEqual([1, 2, 0]); + }); + + it("reorders through a non-negative temporary range before writing the final order", async () => { + const first = "00000000-0000-4000-8000-000000000002"; + const second = "00000000-0000-4000-8000-000000000003"; + const { database, positions } = constraintDatabase([ + { id: first, position: 0 }, + { id: second, position: 1 }, + ]); + + await createTimelineStore(database).reorderSlots(TIMELINE_ID, [second, first]); + + expect(positions).toEqual([3, 4, 0, 1]); + }); +});