mirror of
https://github.com/cpvalente/ontime.git
synced 2026-08-27 09:59:08 +00:00
refactor(server): replace defensive copies with cow patterns
This commit is contained in:
committed by
Carlos Valente
parent
9806f97d27
commit
d9dd147d4b
@@ -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<string, unknown>;
|
||||
const sourceRecord = source as Record<string, unknown>;
|
||||
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',
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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<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 +116,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,21 +727,25 @@ 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 });
|
||||
|
||||
// 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
|
||||
|
||||
@@ -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,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<T extends OntimeEntry>(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>): 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) {
|
||||
|
||||
@@ -101,8 +101,16 @@ function getCustomFields(): Readonly<CustomFields> {
|
||||
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<ProjectRundowns> {
|
||||
db.data.rundowns[rundownKey] = structuredClone(newData);
|
||||
db.data.rundowns[rundownKey] = 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;
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user