Compare commits

..

3 Commits

Author SHA1 Message Date
Claude 45487985be Make cloneEntryData fail to compile when an entry shape changes
cloneEntryData relies on a spread plus a hand-written list of the nested
containers to copy. Nothing tied that list to the real types, so adding a
reference-typed field to an entry - or a new entry type - would silently
produce a clone that aliases the new field back to the source, which is the
exact bug the custom clone exists to avoid.

Two compile-time guards, both verified by temporarily mutating the shared
types:

- ClonedReferenceFields declares, per entry type, the reference-typed fields
  the switch copies. UnclonedReferenceFields diffs that against the fields
  the types actually have, computed via ReferenceKeys. Adding
  `attachments: string[]` to OntimeEvent now fails with
  `Type '"attachments"' does not satisfy the constraint 'never'`, naming the
  offending field. Adding a primitive field stays silent, since the spread
  already copies it by value and no action is needed.

- A default branch in the switch asserts the entry is never. Adding a
  SupportedEntry member fails with `Type 'OntimeMarker' is not assignable to
  type 'never'`, and separately at the ClonedReferenceFields index, which is
  no longer total.

Branded primitives such as Day (number & Brand<'day'>) correctly classify as
primitives, so they are not flagged.

Typecheck, lint, format and the full suite (695 tests) pass unchanged.
2026-08-23 19:59:56 +00:00
Claude c6c25cebcc Make createTransaction's rundown deep-readonly when not mutable
Overload createTransaction() on the literal mutableRundown option: with
mutableRundown: true it returns rundown: Rundown as before; otherwise it
returns rundown: DeepReadonly<Rundown> (ts-essentials, already a convention
in this codebase for read-only snapshots).

Previously a non-mutable transaction's rundown was typed as plain Rundown
even though it's the live cachedRundown reference itself (or a background
rundown read straight from disk) - nothing stopped a future mutation
function from writing into it outside the commit() flow, since
Readonly<T> (used elsewhere for the same purpose) only blocks top-level
reassignment, not nested writes like array.push() or entry.field = x.

Scoped to rundown.dao.ts only: every existing call site in rundown.service.ts
passes a literal mutableRundown: true, so this changes no call-site types.
The one mutableRundown: false site doesn't destructure rundown at all.
Verified with a throwaway probe file (removed) that mutating a non-mutable
transaction's rundown is now a compile error, and that a mutable one still
compiles as before. Typecheck, lint and full test suite (695 tests) pass
unchanged.
2026-08-23 18:19:18 +00:00
Claude b1059e7bff Replace structuredClone with shape-aware clones in rundown/data hot paths
structuredClone's generic serialization algorithm does far more work than
plain object spreads need for these known shapes. Adds cloneEntryData()
and cloneRundown() as drop-in replacements (same "independent copy" contract,
same call sites) and swaps them in everywhere a rundown or a single entry
was being deep-cloned via structuredClone:

- createTransaction()/init() in rundown.dao.ts - the main per-mutation clone
- DataProvider.setRundown() - was re-cloning the whole rundown a second time
  on every single commit
- rundown.service.ts background-rundown clones (custom field rename/remove,
  duplicateExistingRundown)
- the per-entry clone in processRundown's non-mutating path (rundown.parser.ts)
- mergeRundownPreservingFields's per-entry clone

Also:
- safeMerge() (DataProvider.utils.ts) was deep-cloning the entire DatabaseModel,
  including all rundowns, just to read a handful of small config properties
  that never touch rundowns - it now only clones the properties it actually
  merges.
- sheets.service.ts's per-row clone before building a (read-only) Google
  Sheets cell request was unnecessary and is removed.
- runtime.service.ts's previous-state snapshot for eventNow/eventNext/
  eventFlag/groupNow now uses cloneEntryData.

Benchmarked on a synthetic 1000-event rundown: a cue-only edit (no
reprocessing needed) went from ~5ms to ~1.2ms end to end, including the
DataProvider clone. Verified against the existing test suite (695 passing)
plus typecheck and lint.
2026-08-23 16:21:53 +00:00
8 changed files with 133 additions and 20 deletions
@@ -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,
@@ -79,9 +81,16 @@ export const getRundownMetadata = (): Readonly<RundownMetadata> => rundownMetada
export const getProjectCustomFields = (): Readonly<CustomFields> => 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<Rundown>`
* 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<R> = {
customFields: CustomFields;
rundown: Rundown;
rundown: R;
commit: (shouldProcess?: boolean) => Promise<{
rundown: Readonly<Rundown>;
@@ -102,11 +111,17 @@ type TransactionOptions = {
rundownId?: string;
};
export function createTransaction(options: TransactionOptions): Transaction {
export function createTransaction(options: TransactionOptions & { mutableRundown: true }): Transaction<Rundown>;
export function createTransaction(
options: TransactionOptions & { mutableRundown?: false },
): Transaction<DeepReadonly<Rundown>>;
export function createTransaction(
options: TransactionOptions,
): Transaction<Rundown> | Transaction<DeepReadonly<Rundown>> {
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,7 +722,7 @@ export const customFieldMutation = {
* Expose function to add an initial rundown to the system
*/
export function init(initialRundown: Readonly<Rundown>, initialCustomFields: Readonly<CustomFields>) {
const rundown = structuredClone(initialRundown);
const rundown = cloneRundown(initialRundown);
const customFields = structuredClone(initialCustomFields);
const processedData = processRundown(rundown, customFields, { mutate: true });
@@ -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<T extends OntimeEntry>(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 {
@@ -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;
@@ -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,97 @@ export function cloneSimpleRundownEntry(entry: OntimeEntry, newId: EntryId): Ont
throw new Error(`Unsupported entry type for cloning: ${entry}`);
}
type Primitive = string | number | boolean | bigint | symbol | null | undefined;
/**
* The keys of `T` holding a reference (object or array) rather than a primitive: exactly the
* fields a shallow spread aliases instead of copying, and so exactly the fields
* `cloneEntryData` has to give a fresh copy to.
* Branded primitives (eg. `Day`) resolve as primitives, which is what we want - they are
* plain numbers at runtime.
*/
type ReferenceKeys<T> = { [K in keyof T]-?: T[K] extends Primitive ? never : K }[keyof T];
/** The member of the `OntimeEntry` union carrying a given `type` tag */
type EntryOfType<K extends SupportedEntry> = Extract<OntimeEntry, { type: K }>;
/**
* The reference-typed fields that `cloneEntryData` below gives a fresh copy to, per entry type.
* This is the one place to update when an entry gains or loses such a field - the shape is
* checked against the real types by `UnclonedReferenceFields`.
*/
type ClonedReferenceFields = {
[SupportedEntry.Event]: 'custom' | 'triggers';
[SupportedEntry.Group]: 'custom' | 'entries';
[SupportedEntry.Milestone]: 'custom';
[SupportedEntry.Delay]: never;
};
/**
* Every reference-typed field `cloneEntryData` would alias instead of copy. Stays `never`
* while the clone is complete, so `AssertEntryClonesEveryReference` fails the build when:
* - an entry type gains a reference-typed field -> the field name resolves here, and is
* named in the error, until it is copied in the switch and listed above
* - `SupportedEntry` gains a member -> indexing `ClonedReferenceFields` fails here
* A new primitive field needs no action: the spread already copies it by value.
*/
type UnclonedReferenceFields = {
[K in SupportedEntry]: Exclude<ReferenceKeys<EntryOfType<K>>, ClonedReferenceFields[K]>;
}[SupportedEntry];
type AssertNever<T extends never> = T;
/**
* Compile-time guard only, with no runtime meaning - see `UnclonedReferenceFields`.
* Exported because `noUnusedLocals` rejects an unreferenced local type.
*/
export type AssertEntryClonesEveryReference = AssertNever<UnclonedReferenceFields>;
/**
* Fast, shape-aware clone of a single entry, preserving its identity (id, revision, etc).
* Drop-in replacement for `structuredClone(entry)`: the generic structured-clone
* algorithm does far more work than the plain object spreads an entry this shallow needs.
*/
export function cloneEntryData<T extends OntimeEntry>(entry: T): T {
switch (entry.type) {
case SupportedEntry.Event:
return { ...entry, custom: { ...entry.custom }, triggers: entry.triggers?.slice() ?? [] } as T;
case SupportedEntry.Group:
return { ...entry, custom: { ...entry.custom }, entries: entry.entries?.slice() ?? [] } as T;
case SupportedEntry.Milestone:
return { ...entry, custom: { ...entry.custom } } 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>): 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
@@ -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) {
@@ -12,6 +12,7 @@ import {
ViewSettings,
} from 'ontime-types';
import { cloneRundown } from '../../api-data/rundown/rundown.utils.js';
import { isTest } from '../../setup/environment.js';
import { shouldCrashDev } from '../../utils/development.js';
import { isPath } from '../../utils/fileManagement.js';
@@ -102,7 +103,7 @@ function getCustomFields(): Readonly<CustomFields> {
}
async function setRundown(rundownKey: string, newData: Rundown): ReadonlyPromise<ProjectRundowns> {
db.data.rundowns[rundownKey] = structuredClone(newData);
db.data.rundowns[rundownKey] = cloneRundown(newData);
await persist();
return db.data.rundowns;
}
@@ -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>): 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<DatabaseMode
} = deepNewData;
return {
rundowns: { ...existing.rundowns, ...rundowns },
rundowns: { ...existingRundowns, ...newRundowns },
project: { ...deepExisting.project, ...project },
settings: { ...deepExisting.settings, ...settings },
viewSettings: { ...deepExisting.viewSettings, ...viewSettings },
@@ -20,6 +20,7 @@ import { triggerAutomations } from '../../api-data/automation/automation.service
import { triggerReportEntry } from '../../api-data/report/report.service.js';
import { getCurrentRundown, getEntryWithId, getRundownMetadata } from '../../api-data/rundown/rundown.dao.js';
import { RundownMetadata } from '../../api-data/rundown/rundown.types.js';
import { cloneEntryData } from '../../api-data/rundown/rundown.utils.js';
import { logger } from '../../classes/Logger.js';
import { timerConfig } from '../../setup/config.js';
import { eventStore } from '../../stores/EventStore.js';
@@ -754,7 +755,7 @@ function broadcastResult(_target: any, _propertyKey: string, descriptor: Propert
}
// at this point we know that either the id or the contents has changed
batch.add(key, currentEntry as RuntimeStore[K]); // we know that there is the necessary overlap in the types to cast this
RuntimeService.previousState[key] = structuredClone(currentEntry);
RuntimeService.previousState[key] = currentEntry ? cloneEntryData(currentEntry) : null;
return true;
}