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 0e9cc694c..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,3 +1,4 @@ +import { deepEqual } from 'fast-equals'; import { EndAction, OntimeEntry, @@ -770,9 +771,17 @@ describe('cloneEntryData()', () => { * change on every comparison, which would have the runtime re-broadcast and re-save the * restore point on every tick. See PR #2178. */ - it('leaves an absent triggers or entries value absent', () => { - expect(cloneEntryData(makeOntimeEvent({ id: 'partial' })).triggers).toBeUndefined(); - expect(cloneEntryData(makeOntimeGroup({ id: 'partial', entries: undefined })).entries).toBeUndefined(); + 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', () => { diff --git a/apps/server/src/api-data/rundown/rundown.dao.ts b/apps/server/src/api-data/rundown/rundown.dao.ts index b0f6828e4..52c3412b8 100644 --- a/apps/server/src/api-data/rundown/rundown.dao.ts +++ b/apps/server/src/api-data/rundown/rundown.dao.ts @@ -45,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: [], @@ -726,17 +731,21 @@ export function init(initialRundown: Readonly, initialCustomFields: Rea 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.utils.ts b/apps/server/src/api-data/rundown/rundown.utils.ts index 1ba93e068..70b1b478a 100644 --- a/apps/server/src/api-data/rundown/rundown.utils.ts +++ b/apps/server/src/api-data/rundown/rundown.utils.ts @@ -506,17 +506,29 @@ export function cloneSimpleRundownEntry(entry: OntimeEntry, newId: EntryId): Ont * `custom` and `entries` hold primitives, so copying the container is enough. `triggers` * holds objects, which are copied too - otherwise a mutable transaction could edit a trigger * of the cached rundown before commit. - * An absent container is left absent rather than normalised to an empty one, so that a clone - * still compares equal to the entry it came from. + * A container is only replaced when the entry actually carries it, so the clone has exactly + * the same keys as the source. Adding a key here would make the clone compare unequal to the + * entry it came from, and the runtime uses that comparison to decide whether to re-broadcast. */ export function cloneEntryData(entry: T): T { switch (entry.type) { - case SupportedEntry.Event: - return { ...entry, custom: { ...entry.custom }, triggers: entry.triggers?.map((t) => ({ ...t })) } 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.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: {