fix: preserve valid timeline slot positions
This commit is contained in:
@@ -112,20 +112,21 @@ export function createTimelineStore(database: ModuleDatabase): SignageTimelineSt
|
|||||||
/**
|
/**
|
||||||
* Rewrites `position` for one timeline from an explicit order.
|
* 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
|
* `(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
|
* another still holds 1 would collide. Temporary positions sit beyond the final order while
|
||||||
* intermediate state cannot be mistaken for an order.
|
* remaining inside the database's non-negative constraint.
|
||||||
*/
|
*/
|
||||||
async function renumber(
|
async function renumber(
|
||||||
transaction: ModuleDatabase,
|
transaction: ModuleDatabase,
|
||||||
timelineId: string,
|
timelineId: string,
|
||||||
orderedSlotIds: readonly string[],
|
orderedSlotIds: readonly string[],
|
||||||
): Promise<void> {
|
): Promise<void> {
|
||||||
|
const temporaryOffset = orderedSlotIds.length + 1;
|
||||||
for (const [index, slotId] of orderedSlotIds.entries()) {
|
for (const [index, slotId] of orderedSlotIds.entries()) {
|
||||||
await transaction
|
await transaction
|
||||||
.update(timelineSlots)
|
.update(timelineSlots)
|
||||||
.set({ position: -1 - index })
|
.set({ position: temporaryOffset + index })
|
||||||
.where(eq(timelineSlots.id, slotId));
|
.where(eq(timelineSlots.id, slotId));
|
||||||
}
|
}
|
||||||
for (const [index, slotId] of orderedSlotIds.entries()) {
|
for (const [index, slotId] of orderedSlotIds.entries()) {
|
||||||
@@ -175,7 +176,7 @@ export function createTimelineStore(database: ModuleDatabase): SignageTimelineSt
|
|||||||
position === undefined
|
position === undefined
|
||||||
? existing.length
|
? existing.length
|
||||||
: Math.max(0, Math.min(position, 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)];
|
const next = [...existing.slice(0, index), slot.id, ...existing.slice(index)];
|
||||||
await renumber(transaction, slot.timelineId, next);
|
await renumber(transaction, slot.timelineId, next);
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -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<unknown>) => 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]);
|
||||||
|
});
|
||||||
|
});
|
||||||
Reference in New Issue
Block a user