diff --git a/apps/server/src/api-data/rundown/__tests__/rundown.utils.test.ts b/apps/server/src/api-data/rundown/__tests__/rundown.utils.test.ts index 0e5d9b349..e3529183b 100644 --- a/apps/server/src/api-data/rundown/__tests__/rundown.utils.test.ts +++ b/apps/server/src/api-data/rundown/__tests__/rundown.utils.test.ts @@ -1,5 +1,7 @@ +import { deepEqual } from 'fast-equals'; import { EndAction, + OntimeEntry, OntimeEvent, OntimeGroup, OntimeMilestone, @@ -8,13 +10,21 @@ import { TimerType, Trigger, } from 'ontime-types'; -import { MILLIS_PER_HOUR, MILLIS_PER_MINUTE, createEvent } from 'ontime-utils'; +import { + MILLIS_PER_HOUR, + MILLIS_PER_MINUTE, + createDelay, + createEvent, + createGroup, + createMilestone, +} from 'ontime-utils'; import { assertType } from 'vitest'; import { makeOntimeEvent, makeOntimeGroup, makeOntimeMilestone, makeRundown } from '../__mocks__/rundown.mocks.js'; import { parseRundown } from '../rundown.parser.js'; import { calculateDayOffset, + cloneEntryData, deleteById, doesInvalidateMetadata, getIntegerAndFraction, @@ -715,3 +725,68 @@ describe('eventDurationMatchGroupTarget()', () => { expect(result).toStrictEqual(null); }); }); + +describe('cloneEntryData()', () => { + const trigger: Trigger = { + id: 'trigger-1', + title: 'Go on air', + trigger: TimerLifeCycle.onStart, + automationId: 'automation-1', + }; + + // the real factories, so these are complete entries exactly as the rundown holds them + const entries: [string, OntimeEntry][] = [ + ['event', createEvent({ custom: { sponsor: 'a value' }, triggers: [trigger] }, 'cue-1') as OntimeEvent], + ['group', createGroup({ id: 'group-1', entries: ['a', 'b'], custom: { sponsor: 'a value' } })], + ['milestone', createMilestone({ id: 'milestone-1', custom: { sponsor: 'a value' } })], + ['delay', createDelay({ id: 'delay-1', duration: 10 })], + ]; + + /** + * Fails if any nested object or array in the clone is the same reference as the source, + * so a field added later that needs a copy of its own is caught here without the clone + * having to enumerate fields. + */ + function expectNoSharedReferences(clone: unknown, source: unknown, path: string) { + if (typeof source !== 'object' || source === null) return; + expect(clone, `${path} is shared with the source`).not.toBe(source); + const cloneRecord = clone as Record; + const sourceRecord = source as Record; + for (const key of Object.keys(sourceRecord)) { + expectNoSharedReferences(cloneRecord[key], sourceRecord[key], `${path}.${key}`); + } + } + + /** the two halves of the structuredClone contract: same value, no shared references */ + it.each(entries)('clones a %s to the same value structuredClone would produce', (_type, entry) => { + expect(cloneEntryData(entry)).toStrictEqual(structuredClone(entry)); + }); + + it.each(entries)('shares no nested object or array with the source %s', (_type, entry) => { + expectNoSharedReferences(cloneEntryData(entry), entry, 'entry'); + }); + + /** + * Regression: normalising an absent container to an empty one makes deepEqual report a + * change on every comparison, which would have the runtime re-broadcast and re-save the + * restore point on every tick. See PR #2178. + */ + it.each([ + ['event', makeOntimeEvent({ id: 'partial' })], + ['group', makeOntimeGroup({ id: 'partial', entries: undefined })], + ])('gives a partial %s exactly the keys structuredClone would, so it stays deep-equal', (_type, entry) => { + const clone = cloneEntryData(entry); + // asserting on keys, not values: `toBeUndefined()` cannot tell an absent key from an own + // key holding undefined, and it is key presence that decides the deepEqual below + expect(Object.keys(clone).sort()).toEqual(Object.keys(structuredClone(entry)).sort()); + // this is the comparison runtime.service.ts uses to decide whether to re-broadcast an + // entry; if the clone gains a key, every tick looks like a change + expect(deepEqual(clone, entry)).toBe(true); + }); + + it('throws on an entry type it does not know how to clone', () => { + expect(() => cloneEntryData({ id: 'x', type: 'unknown' } as unknown as OntimeEvent)).toThrow( + 'Unsupported entry type for cloning', + ); + }); +}); diff --git a/apps/server/src/api-data/rundown/rundown.dao.ts b/apps/server/src/api-data/rundown/rundown.dao.ts index 460031a9d..52c3412b8 100644 --- a/apps/server/src/api-data/rundown/rundown.dao.ts +++ b/apps/server/src/api-data/rundown/rundown.dao.ts @@ -27,6 +27,7 @@ import { isPlayableEvent, } from 'ontime-types'; import { addToRundown, createGroup, customFieldLabelToKey, getInsertAfterId, insertAtIndex } from 'ontime-utils'; +import type { DeepReadonly } from 'ts-essentials'; import { getDataProvider } from '../../classes/data-provider/DataProvider.js'; import { consoleError } from '../../utils/console.js'; @@ -34,6 +35,7 @@ import { ProcessedRundownMetadata, makeRundownMetadata } from './rundown.parser. import type { RundownMetadata } from './rundown.types.js'; import { applyPatchToEntry, + cloneRundown, cloneSimpleRundownEntry, deleteById, doesInvalidateMetadata, @@ -43,9 +45,14 @@ import { } from './rundown.utils.js'; /** - * The currently loaded rundown in cache + * The currently loaded rundown in cache. + * + * Reassigned - never mutated in place - when a different rundown is loaded: the persistence + * layer stores this object by reference, so repurposing it for another rundown would rewrite + * the previously loaded rundown's stored record. Mutating it in place while it represents the + * same rundown (ie. from commit) is intended, and is what keeps the stored record current. */ -const cachedRundown: Rundown = { +let cachedRundown: Rundown = { id: '', title: '', order: [], @@ -79,9 +86,16 @@ export const getRundownMetadata = (): Readonly => rundownMetada export const getProjectCustomFields = (): Readonly => projectCustomFields; export const getEntryWithId = (entryId: EntryId): OntimeEntry | undefined => cachedRundown.entries[entryId]; -type Transaction = { +/** + * @param R the type callers see for `rundown` - a plain, mutable `Rundown` when the + * transaction was opened with `mutableRundown: true`, otherwise a `DeepReadonly` + * so that accidentally mutating an entry (or an order array) on a non-mutable transaction + * - which would silently corrupt the live cache without going through commit() - is a + * compile-time error instead of a runtime bug. + */ +type Transaction = { customFields: CustomFields; - rundown: Rundown; + rundown: R; commit: (shouldProcess?: boolean) => Promise<{ rundown: Readonly; @@ -102,11 +116,17 @@ type TransactionOptions = { rundownId?: string; }; -export function createTransaction(options: TransactionOptions): Transaction { +export function createTransaction(options: TransactionOptions & { mutableRundown: true }): Transaction; +export function createTransaction( + options: TransactionOptions & { mutableRundown?: false }, +): Transaction>; +export function createTransaction( + options: TransactionOptions, +): Transaction | Transaction> { const targetId = options.rundownId ?? cachedRundown.id; const isLoaded = targetId === cachedRundown.id; const sourceRundown: Rundown = isLoaded ? cachedRundown : (getDataProvider().getRundown(targetId) as Rundown); - const rundown = options.mutableRundown ? structuredClone(sourceRundown) : sourceRundown; + const rundown = options.mutableRundown ? cloneRundown(sourceRundown) : sourceRundown; const customFields = options.mutableCustomFields ? structuredClone(projectCustomFields) : projectCustomFields; /** @@ -707,21 +727,25 @@ export const customFieldMutation = { * Expose function to add an initial rundown to the system */ export function init(initialRundown: Readonly, initialCustomFields: Readonly) { - const rundown = structuredClone(initialRundown); + const rundown = cloneRundown(initialRundown); const customFields = structuredClone(initialCustomFields); const processedData = processRundown(rundown, customFields, { mutate: true }); - // update the cache values - cachedRundown.id = rundown.id; - cachedRundown.title = rundown.title; projectCustomFields = customFields; // eslint-disable-next-line @typescript-eslint/no-unused-vars -- we are not interested in the iteration data const { previousEvent, latestEvent, previousEntry, entries, order, ...metadata } = processedData; - cachedRundown.entries = entries; - cachedRundown.order = order; - cachedRundown.flatOrder = metadata.flatEntryOrder; - cachedRundown.revision = rundown.revision; + + // a fresh object, so that the record already stored for a previously loaded rundown keeps + // pointing at that rundown's data - see the note on cachedRundown + cachedRundown = { + id: rundown.id, + title: rundown.title, + entries, + order, + flatOrder: metadata.flatEntryOrder, + revision: rundown.revision, + }; rundownMetadata = metadata; // defer writing to the database diff --git a/apps/server/src/api-data/rundown/rundown.parser.ts b/apps/server/src/api-data/rundown/rundown.parser.ts index e269490e9..14c1721aa 100644 --- a/apps/server/src/api-data/rundown/rundown.parser.ts +++ b/apps/server/src/api-data/rundown/rundown.parser.ts @@ -33,7 +33,7 @@ import { import { makeNewRundown } from '../../models/dataModel.js'; import type { ErrorEmitter } from '../../utils/parserUtils.js'; import { RundownMetadata } from './rundown.types.js'; -import { calculateDayOffset, cleanupCustomFields } from './rundown.utils.js'; +import { calculateDayOffset, cleanupCustomFields, cloneEntryData } from './rundown.utils.js'; /** * Parse a rundowns object along with the project custom fields @@ -234,7 +234,7 @@ export function makeRundownMetadata(customFields: CustomFields, options?: { muta }; function process(entry: T, childOfGroup: EntryId | null): T { - return processEntry(rundownMeta, customFields, mutate ? entry : structuredClone(entry), childOfGroup); + return processEntry(rundownMeta, customFields, mutate ? entry : cloneEntryData(entry), childOfGroup); } function getMetadata(): ProcessedRundownMetadata { diff --git a/apps/server/src/api-data/rundown/rundown.service.ts b/apps/server/src/api-data/rundown/rundown.service.ts index 7eff148ab..a1155dbf9 100644 --- a/apps/server/src/api-data/rundown/rundown.service.ts +++ b/apps/server/src/api-data/rundown/rundown.service.ts @@ -39,6 +39,7 @@ import { import { parseRundown } from './rundown.parser.js'; import type { RundownMetadata } from './rundown.types.js'; import { + cloneRundown, generateEvent, getFirstInsertId, getIntegerAndFraction, @@ -626,7 +627,7 @@ export async function editCustomField( // ... reassign references in the background rundowns for (const rundownId of Object.keys(projectRundowns)) { if (rundownId !== rundown.id) { - const backgroundRundown = structuredClone(projectRundowns[rundownId]); + const backgroundRundown = cloneRundown(projectRundowns[rundownId]); customFieldMutation.renameUsages(backgroundRundown, oldKey, newKey); await updateBackgroundRundown(rundownId, backgroundRundown); } @@ -666,7 +667,7 @@ export async function deleteCustomField(key: CustomFieldKey, projectRundowns: Pr // remove references in the background rundowns for (const rundownId of Object.keys(projectRundowns)) { if (rundownId !== rundown.id) { - const backgroundRundown = structuredClone(projectRundowns[rundownId]); + const backgroundRundown = cloneRundown(projectRundowns[rundownId]); customFieldMutation.removeUsages(backgroundRundown, key); await updateBackgroundRundown(rundownId, backgroundRundown); } @@ -846,7 +847,7 @@ export async function duplicateExistingRundown(id: string) { const dataProvider = getDataProvider(); const rundown = dataProvider.getRundown(id); - const duplicatedRundown: Rundown = structuredClone(rundown); + const duplicatedRundown: Rundown = cloneRundown(rundown); duplicatedRundown.id = generateId(); duplicatedRundown.title = `Copy of ${rundown.title}`; duplicatedRundown.revision = 0; diff --git a/apps/server/src/api-data/rundown/rundown.utils.ts b/apps/server/src/api-data/rundown/rundown.utils.ts index d1d0407db..b027bf455 100644 --- a/apps/server/src/api-data/rundown/rundown.utils.ts +++ b/apps/server/src/api-data/rundown/rundown.utils.ts @@ -329,7 +329,7 @@ export function mergeRundownPreservingFields( const structure = isOntimeGroup(incomingEntry) ? { entries: incomingEntry.entries } : { parent: incomingEntry.parent }; - entries[id] = structuredClone({ ...merged, ...structure }); + entries[id] = cloneEntryData({ ...merged, ...structure }); } return { @@ -499,6 +499,61 @@ export function cloneSimpleRundownEntry(entry: OntimeEntry, newId: EntryId): Ont throw new Error(`Unsupported entry type for cloning: ${entry}`); } +/** + * Fast, shape-aware clone of a single entry, preserving its identity (id, revision, etc). + * Drop-in replacement for `structuredClone(entry)` + */ +export function cloneEntryData(entry: T): T { + switch (entry.type) { + case SupportedEntry.Event: { + const clone: OntimeEvent = { ...entry }; + if (clone.custom) clone.custom = { ...clone.custom }; + if (clone.triggers) clone.triggers = clone.triggers.map((trigger) => ({ ...trigger })); + return clone as T; + } + case SupportedEntry.Group: { + const clone: OntimeGroup = { ...entry }; + if (clone.custom) clone.custom = { ...clone.custom }; + if (clone.entries) clone.entries = clone.entries.slice(); + return clone as T; + } + case SupportedEntry.Milestone: { + const clone: OntimeMilestone = { ...entry }; + if (clone.custom) clone.custom = { ...clone.custom }; + return clone as T; + } + case SupportedEntry.Delay: + return { ...entry } as T; + default: { + // exhaustiveness guard: a new member of `SupportedEntry` is named in the error here + const unhandled: never = entry; + throw new Error(`Unsupported entry type for cloning: ${(unhandled as OntimeEntry).type}`); + } + } +} + +/** + * Fast, shape-aware clone of a whole rundown. + * Drop-in replacement for `structuredClone(rundown)`: every entry (and its nested + * `custom` / `triggers` / `entries` containers) gets its own copy, so callers can mutate + * the result freely without touching the source - same contract as structuredClone, + * at a fraction of the cost since we skip the generic serialization algorithm. + */ +export function cloneRundown(rundown: Readonly): Rundown { + const entries: RundownEntries = {}; + for (const id in rundown.entries) { + entries[id] = cloneEntryData(rundown.entries[id]); + } + return { + id: rundown.id, + title: rundown.title, + revision: rundown.revision, + order: rundown.order.slice(), + flatOrder: rundown.flatOrder.slice(), + entries, + }; +} + /** * Utility for calculating if the current events should have a day offset * @param current the current event under test diff --git a/apps/server/src/api-data/sheets/sheets.service.ts b/apps/server/src/api-data/sheets/sheets.service.ts index e6d7a68de..2cec76b55 100644 --- a/apps/server/src/api-data/sheets/sheets.service.ts +++ b/apps/server/src/api-data/sheets/sheets.service.ts @@ -464,9 +464,8 @@ export async function upload(sheetId: string, options: ImportMap) { sheetOrder.forEach((entryId, index) => { const isGroupEnd = entryId.startsWith('group-end-'); const id = isGroupEnd ? entryId.split('group-end-')[1] : entryId; - const entry = isGroupEnd - ? ({ id: entryId, type: SupportedEntry.Group } as OntimeGroup) - : structuredClone(rundown.entries[id]); + // cellRequestFromEvent only reads the entry to build a cell request, no clone is needed + const entry = isGroupEnd ? ({ id: entryId, type: SupportedEntry.Group } as OntimeGroup) : rundown.entries[id]; updateRundown.push(cellRequestFromEvent(entry, index, worksheetId, sheetMetadata)); }); } catch (e) { diff --git a/apps/server/src/classes/data-provider/DataProvider.ts b/apps/server/src/classes/data-provider/DataProvider.ts index 44cc2aff6..66321b7e4 100644 --- a/apps/server/src/classes/data-provider/DataProvider.ts +++ b/apps/server/src/classes/data-provider/DataProvider.ts @@ -101,8 +101,16 @@ function getCustomFields(): Readonly { return db.data.customFields; } +/** + * Stores a rundown, replacing any existing entry for the same key. + * Takes ownership of `newData` and stores it by reference - the caller must not mutate it + * afterward. Every call site either hands over a freshly-built object it never touches again, + * or (for the loaded rundown) the cache's own long-lived object, which is already the single + * source of truth for that data - aliasing it here costs nothing and avoids a second full + * deep copy of the rundown on every commit. + */ async function setRundown(rundownKey: string, newData: Rundown): ReadonlyPromise { - db.data.rundowns[rundownKey] = structuredClone(newData); + db.data.rundowns[rundownKey] = newData; await persist(); return db.data.rundowns; } diff --git a/apps/server/src/classes/data-provider/DataProvider.utils.ts b/apps/server/src/classes/data-provider/DataProvider.utils.ts index 6a2fbb756..c57bec8b8 100644 --- a/apps/server/src/classes/data-provider/DataProvider.utils.ts +++ b/apps/server/src/classes/data-provider/DataProvider.utils.ts @@ -4,12 +4,17 @@ import { DatabaseModel } from 'ontime-types'; * Merges a partial ontime project into a given ontime project */ export function safeMerge(existing: DatabaseModel, newData: Partial): DatabaseModel { - const deepExisting = structuredClone(existing); - const deepNewData = structuredClone(newData); + // rundowns are merged separately below by reference (only the top-level map is copied, + // same as the other properties here) - deep-cloning them here would be wasted work, + // since a project's rundowns are by far the largest part of this object + const { rundowns: existingRundowns, ...existingRest } = existing; + const { rundowns: newRundowns = {}, ...newDataRest } = newData; + + const deepExisting = structuredClone(existingRest); + const deepNewData = structuredClone(newDataRest); // destructure each property to simplify merging not provided ie: ...{} has no effect const { - rundowns = {}, project = {}, settings = {}, viewSettings = {}, @@ -19,7 +24,7 @@ export function safeMerge(existing: DatabaseModel, newData: Partial