From 4a0a174eb96efa5e4606fd787967980d50d94450 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 4 Jul 2026 10:01:02 +0000 Subject: [PATCH] refactor: extract broadcast diffing into a testable function The state diffing and gating logic moves verbatim from the broadcastResult decorator into collectRuntimeStateChanges in runtime.utils.ts. The decorator becomes a thin shell: snapshot, diff, batch the changed keys, save the restore point, send. The entire gating matrix is now unit tested against plain state objects (see runtime.utils.test.ts), including the characterised entry-diffing short-circuit, without sockets or module mocking. The broadcast-contract tests remain the end-to-end lock and are unchanged. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01PVTnCfesGNGwPQJwJ9FwoD --- .../__tests__/runtime.utils.test.ts | 107 +++++++++++++++++- .../runtime-service/runtime.service.ts | 95 ++-------------- .../services/runtime-service/runtime.utils.ts | 101 +++++++++++++++++ 3 files changed, 213 insertions(+), 90 deletions(-) diff --git a/apps/server/src/services/runtime-service/__tests__/runtime.utils.test.ts b/apps/server/src/services/runtime-service/__tests__/runtime.utils.test.ts index 16188b768..f40f629cb 100644 --- a/apps/server/src/services/runtime-service/__tests__/runtime.utils.test.ts +++ b/apps/server/src/services/runtime-service/__tests__/runtime.utils.test.ts @@ -2,9 +2,18 @@ * Characterisation tests for the change-detection predicates that gate * what gets broadcast to clients on every tick */ -import { Offset, OffsetMode, Playback, TimerPhase, TimerState, TimerType } from 'ontime-types'; +import { Offset, OffsetMode, PlayableEvent, Playback, TimerPhase, TimerState, TimerType } from 'ontime-types'; -import { getShouldClockUpdate, getShouldOffsetUpdate, getShouldTimerUpdate, isNewSecond } from '../runtime.utils.js'; +import { makeOntimeEvent } from '../../../api-data/rundown/__mocks__/rundown.mocks.js'; +import { makeRuntimeStateData } from '../../../stores/__mocks__/runtimeState.mocks.js'; +import type { RuntimeState } from '../../../stores/runtimeState.js'; +import { + collectRuntimeStateChanges, + getShouldClockUpdate, + getShouldOffsetUpdate, + getShouldTimerUpdate, + isNewSecond, +} from '../runtime.utils.js'; const baseTimer: TimerState = { addedTime: 0, @@ -142,3 +151,97 @@ describe('getShouldOffsetUpdate()', () => { expect(getShouldOffsetUpdate(baseOffset, current, true)).toBe(true); }); }); + +describe('collectRuntimeStateChanges()', () => { + test('the very first run emits everything and counts as an immediate change', () => { + const previousState = {} as RuntimeState; + const state = makeRuntimeStateData(); + + const { patch, hasImmediateChanges } = collectRuntimeStateChanges(previousState, state); + + expect(Object.keys(patch).sort()).toStrictEqual(['clock', 'offset', 'rundown', 'timer']); + expect(hasImmediateChanges).toBe(true); + // the emitted keys are persisted into the previous state for the next diff + expect(previousState.timer).toStrictEqual(state.timer); + expect(previousState.offset).toStrictEqual(state.offset); + }); + + test('an unchanged state emits nothing', () => { + const previousState = makeRuntimeStateData(); + const state = makeRuntimeStateData(); + + const { patch, hasImmediateChanges } = collectRuntimeStateChanges(previousState, state); + + expect(patch).toStrictEqual({}); + expect(hasImmediateChanges).toBe(false); + }); + + test('a playback change emits timer and clock together', () => { + const previousState = makeRuntimeStateData(); + const state = makeRuntimeStateData({ timer: { playback: Playback.Armed } }); + + const { patch, hasImmediateChanges } = collectRuntimeStateChanges(previousState, state); + + expect(Object.keys(patch).sort()).toStrictEqual(['clock', 'timer']); + expect(hasImmediateChanges).toBe(true); + }); + + test('an offset change is gated on a timer/clock dependency', () => { + // nothing else changed: the offset is held back + const previousState = makeRuntimeStateData(); + const state = makeRuntimeStateData({ offset: { absolute: 1000 } }); + + const gated = collectRuntimeStateChanges(previousState, state); + expect(gated.patch).toStrictEqual({}); + + // with a clock rollover, the offset rides along + const stateWithClock = makeRuntimeStateData({ clock: 1000, offset: { absolute: 1000 } }); + const emitted = collectRuntimeStateChanges(previousState, stateWithClock); + expect(Object.keys(emitted.patch).sort()).toStrictEqual(['clock', 'offset']); + }); + + test('!!! characterised bug: changed entries drip out one per call', () => { + const previousState = makeRuntimeStateData(); + const eventNow = makeOntimeEvent({ id: 'now' }) as PlayableEvent; + const eventNext = makeOntimeEvent({ id: 'next' }) as PlayableEvent; + const state = makeRuntimeStateData({ eventNow, eventNext }); + + // both entries changed, but the ||= short-circuit only diffs the first + const first = collectRuntimeStateChanges(previousState, state); + expect(Object.keys(first.patch).sort()).toStrictEqual(['eventNow']); + + // the second call flushes the next pending entry + const second = collectRuntimeStateChanges(previousState, state); + expect(Object.keys(second.patch).sort()).toStrictEqual(['eventNext']); + + // from here on, nothing is pending + const third = collectRuntimeStateChanges(previousState, state); + expect(third.patch).toStrictEqual({}); + }); + + test('rundown data changes are emitted on deep difference', () => { + const previousState = makeRuntimeStateData(); + const state = makeRuntimeStateData({ rundown: { actualStart: 1000 } }); + + const { patch } = collectRuntimeStateChanges(previousState, state); + expect(Object.keys(patch).sort()).toStrictEqual(['rundown']); + }); + + test('addedTime changes count as immediate and emit the timer', () => { + const previousState = makeRuntimeStateData(); + const state = makeRuntimeStateData({ timer: { addedTime: 60_000 } }); + + const { patch, hasImmediateChanges } = collectRuntimeStateChanges(previousState, state); + expect(Object.keys(patch).sort()).toStrictEqual(['clock', 'timer']); + expect(hasImmediateChanges).toBe(true); + }); + + test('an offset mode change is immediate and bypasses the dependency gate', () => { + const previousState = makeRuntimeStateData(); + const state = makeRuntimeStateData({ offset: { mode: OffsetMode.Relative } }); + + const { patch, hasImmediateChanges } = collectRuntimeStateChanges(previousState, state); + expect(Object.keys(patch).sort()).toStrictEqual(['offset']); + expect(hasImmediateChanges).toBe(true); + }); +}); diff --git a/apps/server/src/services/runtime-service/runtime.service.ts b/apps/server/src/services/runtime-service/runtime.service.ts index d64564ea2..b5294c487 100644 --- a/apps/server/src/services/runtime-service/runtime.service.ts +++ b/apps/server/src/services/runtime-service/runtime.service.ts @@ -1,4 +1,3 @@ -import { deepEqual } from 'fast-equals'; import { EndAction, EntryId, @@ -30,13 +29,12 @@ import { restoreService } from '../restore-service/restore.service.js'; import type { RestorePoint } from '../restore-service/restore.type.js'; import { skippedOutOfEvent } from '../timerUtils.js'; import { + collectRuntimeStateChanges, findNextPlayableId, findNextPlayableWithCue, findPreviousPlayableId, getEventAtIndex, getShouldClockUpdate, - getShouldOffsetUpdate, - getShouldTimerUpdate, isNewSecond, } from './runtime.utils.js'; @@ -651,8 +649,6 @@ const eventTimer = new EventTimer({ }); export const runtimeService = new RuntimeService(eventTimer); -type EntryUpdateKeys = keyof Pick; - /** * Decorator manages side effects from updating the runtime * This should only be applied to functions that are exposed for consumption @@ -665,90 +661,13 @@ function broadcastResult(_target: any, _propertyKey: string, descriptor: Propert // call the original method and get the state const result = originalMethod.apply(this, args); const state = runtimeState.getState(); + + // diff against the previously broadcast state to find what to send + const { patch, hasImmediateChanges } = collectRuntimeStateChanges(RuntimeService.previousState, state); + const batch = eventStore.createBatch(); - - // we do the comparison by explicitly for each property - // to apply custom logic for different datasets - - // Update the entry if they have changed - let entryChanged = false; - entryChanged ||= updateMaybeEntryIfChanged('eventNow'); - entryChanged ||= updateMaybeEntryIfChanged('eventNext'); - entryChanged ||= updateMaybeEntryIfChanged('eventFlag'); - entryChanged ||= updateMaybeEntryIfChanged('groupNow'); - - // for the very fist run there will be nothing in the previousState so we force an update - const justStarted = !RuntimeService.previousState?.timer; - - // offset mode has been changed - const offsetModeChanged = RuntimeService.previousState?.offset?.mode !== state.offset.mode; - - // if playback changes most things should update - const hasChangedPlayback = RuntimeService.previousState.timer?.playback !== state.timer.playback; - - const addedTimeChanged = !justStarted && RuntimeService.previousState?.timer.addedTime !== state.timer.addedTime; - - // combine all big changes - const hasImmediateChanges = - entryChanged || justStarted || hasChangedPlayback || offsetModeChanged || addedTimeChanged; - - /** - * if any values have changed. - * values that have the possibility to tick are updated when the seconds roll over - */ - const updateTimer = getShouldTimerUpdate(RuntimeService.previousState?.timer, state.timer); - if (updateTimer) { - batch.add('timer', state.timer); - RuntimeService.previousState.timer = { ...state.timer }; - } - - /** - * clock has changed by a second or more. - * or the timer updated so we ensure that the timer and clock ticks are in sync - */ - const updateClock = updateTimer || getShouldClockUpdate(RuntimeService.previousState.clock, state.clock); - if (updateClock) { - batch.add('clock', state.clock); - RuntimeService.previousState.clock = state.clock; - } - - /** - * if any values have changed. - * values that have the possibility to tick are modulated by `updateClock || hasImmediateChanges` - */ - const updateRuntime = getShouldOffsetUpdate( - RuntimeService.previousState?.offset, - state.offset, - updateClock || hasImmediateChanges, - ); - if (updateRuntime) { - batch.add('offset', state.offset); - RuntimeService.previousState.offset = structuredClone(state.offset); - } - - /** - * if any values have changed. - */ - const updateRundownData = !deepEqual(RuntimeService.previousState.rundown, state.rundown); - if (updateRundownData) { - batch.add('rundown', state.rundown); - RuntimeService.previousState.rundown = structuredClone(state.rundown); - } - - function updateMaybeEntryIfChanged(key: K) { - const previousEntry = RuntimeService.previousState[key]; - const currentEntry = state[key]; - - if (!previousEntry && !currentEntry) return false; // if both are null -> skip - - // if they have the same id the check if the contents have changed - if (previousEntry?.id === currentEntry?.id) { - if (deepEqual(previousEntry, currentEntry)) return false; // contents are the same -> skip - } - // 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); - return true; + for (const key of Object.keys(patch) as (keyof RuntimeStore)[]) { + batch.add(key, patch[key] as RuntimeStore[typeof key]); } // save the restore state diff --git a/apps/server/src/services/runtime-service/runtime.utils.ts b/apps/server/src/services/runtime-service/runtime.utils.ts index 9961a78fb..2296e9828 100644 --- a/apps/server/src/services/runtime-service/runtime.utils.ts +++ b/apps/server/src/services/runtime-service/runtime.utils.ts @@ -5,6 +5,7 @@ import { Offset, OntimeEvent, Rundown, + RuntimeStore, TimerState, TimerType, isOntimeEvent, @@ -12,6 +13,8 @@ import { } from 'ontime-types'; import { millisToSeconds } from 'ontime-utils'; +import type { RuntimeState } from '../../stores/runtimeState.js'; + export function isNewSecond( previousValue: MaybeNumber | undefined, currentValue: MaybeNumber | undefined, @@ -68,6 +71,104 @@ export function getShouldOffsetUpdate( return didDependencyUpdate && !deepEqual(previousValue, currentValue); } +type EntryUpdateKeys = keyof Pick; + +/** + * Diffs a runtime state snapshot against the previously broadcast state + * and collects the store keys that should be sent to clients + * + * !!! mutates previousState in place: emitted keys are persisted so the + * next diff compares against what clients last received + */ +export function collectRuntimeStateChanges( + previousState: RuntimeState, + state: Readonly, +): { patch: Partial; hasImmediateChanges: boolean } { + const patch: Partial = {}; + + // we do the comparison by explicitly for each property + // to apply custom logic for different datasets + + // Update the entry if they have changed + let entryChanged = false; + entryChanged ||= updateMaybeEntryIfChanged('eventNow'); + entryChanged ||= updateMaybeEntryIfChanged('eventNext'); + entryChanged ||= updateMaybeEntryIfChanged('eventFlag'); + entryChanged ||= updateMaybeEntryIfChanged('groupNow'); + + // for the very fist run there will be nothing in the previousState so we force an update + const justStarted = !previousState?.timer; + + // offset mode has been changed + const offsetModeChanged = previousState?.offset?.mode !== state.offset.mode; + + // if playback changes most things should update + const hasChangedPlayback = previousState.timer?.playback !== state.timer.playback; + + const addedTimeChanged = !justStarted && previousState?.timer.addedTime !== state.timer.addedTime; + + // combine all big changes + const hasImmediateChanges = + entryChanged || justStarted || hasChangedPlayback || offsetModeChanged || addedTimeChanged; + + /** + * if any values have changed. + * values that have the possibility to tick are updated when the seconds roll over + */ + const updateTimer = getShouldTimerUpdate(previousState?.timer, state.timer); + if (updateTimer) { + patch.timer = state.timer; + previousState.timer = { ...state.timer }; + } + + /** + * clock has changed by a second or more. + * or the timer updated so we ensure that the timer and clock ticks are in sync + */ + const updateClock = updateTimer || getShouldClockUpdate(previousState.clock, state.clock); + if (updateClock) { + patch.clock = state.clock; + previousState.clock = state.clock; + } + + /** + * if any values have changed. + * values that have the possibility to tick are modulated by `updateClock || hasImmediateChanges` + */ + const updateRuntime = getShouldOffsetUpdate(previousState?.offset, state.offset, updateClock || hasImmediateChanges); + if (updateRuntime) { + patch.offset = state.offset; + previousState.offset = structuredClone(state.offset); + } + + /** + * if any values have changed. + */ + const updateRundownData = !deepEqual(previousState.rundown, state.rundown); + if (updateRundownData) { + patch.rundown = state.rundown; + previousState.rundown = structuredClone(state.rundown); + } + + function updateMaybeEntryIfChanged(key: K) { + const previousEntry = previousState[key]; + const currentEntry = state[key]; + + if (!previousEntry && !currentEntry) return false; // if both are null -> skip + + // if they have the same id the check if the contents have changed + if (previousEntry?.id === currentEntry?.id) { + if (deepEqual(previousEntry, currentEntry)) return false; // contents are the same -> skip + } + // at this point we know that either the id or the contents has changed + patch[key] = currentEntry as RuntimeStore[K]; // we know that there is the necessary overlap in the types to cast this + previousState[key] = structuredClone(currentEntry); + return true; + } + + return { patch, hasImmediateChanges }; +} + /** * finds the previous playable event, if it exists */