diff --git a/meteor/server/api/deviceTriggers/StudioObserver.ts b/meteor/server/api/deviceTriggers/StudioObserver.ts index ae3b1263c9..5f18675eda 100644 --- a/meteor/server/api/deviceTriggers/StudioObserver.ts +++ b/meteor/server/api/deviceTriggers/StudioObserver.ts @@ -212,6 +212,8 @@ export class StudioObserver extends EventEmitter { this.nextProps = undefined const { activePlaylistId, activationId } = this.currentProps + const rundownContentChanged = this.#rundownContentChanged + const pieceInstancesChanged = this.#pieceInstancesChanged this.showStyleBaseId = showStyleBaseId @@ -219,7 +221,7 @@ export class StudioObserver extends EventEmitter { logger.silly(`Creating new RundownContentObserver`) const obs1 = await RundownContentObserver.create(activePlaylistId, showStyleBaseId, rundownIds, (cache) => { - return this.#rundownContentChanged(showStyleBaseId, cache) + return rundownContentChanged(showStyleBaseId, cache) }) return () => { @@ -228,11 +230,9 @@ export class StudioObserver extends EventEmitter { }) this.#pieceInstancesLiveQuery = await PieceInstancesObserver.create(activationId, showStyleBaseId, (cache) => { - const cleanupChanges = this.#pieceInstancesChanged(showStyleBaseId, cache) + const cleanupChanges = pieceInstancesChanged(showStyleBaseId, cache) - return () => { - cleanupChanges?.() - } + return () => cleanupChanges?.() }) if (this.#disposed) { diff --git a/meteor/server/api/deviceTriggers/__tests__/StudioObserver.test.ts b/meteor/server/api/deviceTriggers/__tests__/StudioObserver.test.ts new file mode 100644 index 0000000000..b2ebaeecc0 --- /dev/null +++ b/meteor/server/api/deviceTriggers/__tests__/StudioObserver.test.ts @@ -0,0 +1,156 @@ +import { protectString } from '@sofie-automation/corelib/dist/protectedString' +import { + RundownId, + RundownPlaylistActivationId, + RundownPlaylistId, + ShowStyleBaseId, + StudioId, +} from '@sofie-automation/corelib/dist/dataModel/Ids' + +import type { ContentCache as RundownContentCache } from '../reactiveContentCache' +import type { ContentCache as PieceInstancesContentCache } from '../reactiveContentCacheForPieceInstances' +import { runAllTimers } from '../../../../__mocks__/helpers/jest' + +type OnChangedRundown = (cache: RundownContentCache) => () => void +type OnChangedPieceInstances = (cache: PieceInstancesContentCache) => () => void + +let capturedRundownContentOnChanged: OnChangedRundown | undefined +let capturedPieceInstancesOnChanged: OnChangedPieceInstances | undefined + +jest.mock('../../../publications/lib/observerChain', () => { + const fakeHandle = { stop: jest.fn() } + const chain: any = { + next: jest.fn(() => chain), + end: jest.fn(() => fakeHandle), + } + return { + observerChain: jest.fn(() => chain), + } +}) + +jest.mock('../RundownsObserver', () => { + return { + RundownsObserver: { + create: jest.fn( + async (_playlistId: RundownPlaylistId, onChanged: (ids: RundownId[]) => Promise<() => void>) => { + // Immediately drive the callback once, to emulate initial observer execution + await onChanged([protectString('r0')]) + return { stop: jest.fn() } + } + ), + }, + } +}) + +jest.mock('../RundownContentObserver', () => { + return { + RundownContentObserver: { + create: jest.fn( + async ( + _playlistId: RundownPlaylistId, + _showStyleBaseId: ShowStyleBaseId, + _rundownIds: RundownId[], + onChanged: OnChangedRundown + ) => { + capturedRundownContentOnChanged = onChanged + return { stop: jest.fn() } + } + ), + }, + } +}) + +jest.mock('../PieceInstancesObserver', () => { + return { + PieceInstancesObserver: { + create: jest.fn( + async ( + _activationId: RundownPlaylistActivationId, + _showStyleBaseId: ShowStyleBaseId, + onChanged: OnChangedPieceInstances + ) => { + capturedPieceInstancesOnChanged = onChanged + return { stop: jest.fn() } + } + ), + }, + } +}) + +describe('StudioObserver', () => { + beforeEach(() => { + jest.useFakeTimers() + capturedRundownContentOnChanged = undefined + capturedPieceInstancesOnChanged = undefined + }) + + test('rundown deactivation regression: observer callbacks must not depend on `this` (private fields)', async () => { + // Import after mocks are in place + const { StudioObserver } = await import('../StudioObserver') + + const studioId = protectString('studio0') + const playlistId = protectString('playlist0') + const activationId = protectString('activation0') + const rundownId = protectString('rundown0') + const showStyleBaseId = protectString('showStyleBase0') + + const rundownCleanup = jest.fn() + const pieceCleanup = jest.fn() + + const onRundownContentChanged = jest.fn( + (_ssbId: ShowStyleBaseId, _cache: RundownContentCache) => rundownCleanup + ) + const onPieceInstancesChanged = jest.fn( + (_ssbId: ShowStyleBaseId, _cache: PieceInstancesContentCache) => pieceCleanup + ) + + const observer = new StudioObserver(studioId, onRundownContentChanged, onPieceInstancesChanged) + + // Prime state so updateShowStyle goes down the creation path + ;(observer as any).nextProps = { + activePlaylistId: playlistId, + activationId, + currentRundownId: rundownId, + } + + const state = { + currentRundown: { _id: rundownId, showStyleBaseId }, + showStyleBase: { _id: showStyleBaseId }, + } + + // Trigger the debounced execution + const ps: Promise = (observer as any).updateShowStyle.call(state) + + // Flush debounce timers and any queued promises + await jest.advanceTimersByTimeAsync(25) + await runAllTimers() + await ps + + // Ensure we captured callbacks from the two observers + expect(capturedRundownContentOnChanged).toBeTruthy() + expect(capturedPieceInstancesOnChanged).toBeTruthy() + + const mockRundownCache = {} as any as RundownContentCache + const mockPieceInstancesCache = {} as any as PieceInstancesContentCache + + // Regression: invoke callbacks without a bound `this` (simulates lost context) + expect(() => capturedRundownContentOnChanged!(mockRundownCache)).not.toThrow() + expect(() => capturedPieceInstancesOnChanged!(mockPieceInstancesCache)).not.toThrow() + + // They should return cleanup fns + const cleanup1 = capturedRundownContentOnChanged!(mockRundownCache) + const cleanup2 = capturedPieceInstancesOnChanged!(mockPieceInstancesCache) + expect(typeof cleanup1).toBe('function') + expect(typeof cleanup2).toBe('function') + + // Ensure our handlers were called with expected args + expect(onRundownContentChanged).toHaveBeenCalledWith(showStyleBaseId, mockRundownCache) + expect(onPieceInstancesChanged).toHaveBeenCalledWith(showStyleBaseId, mockPieceInstancesCache) + + // Ensure returned cleanup fns are callable + expect(() => cleanup1()).not.toThrow() + expect(() => cleanup2()).not.toThrow() + + observer.stop() + }) +})