diff --git a/apps/client/src/common/api/constants.ts b/apps/client/src/common/api/constants.ts index 25723da4b..cd6bccd22 100644 --- a/apps/client/src/common/api/constants.ts +++ b/apps/client/src/common/api/constants.ts @@ -21,6 +21,7 @@ export const CSS_OVERRIDE = ['cssOverride']; export const CLIENT_LIST = ['clientList']; export const REPORT = ['report']; export const REPORT_RUNS = ['report', 'runs']; +export const REPORT_OPEN_RUN = ['report', 'runs', 'open']; export const TRANSLATION = ['translation']; // API URLs diff --git a/apps/client/src/common/api/report.ts b/apps/client/src/common/api/report.ts index 31b5c4c59..7bc7e836e 100644 --- a/apps/client/src/common/api/report.ts +++ b/apps/client/src/common/api/report.ts @@ -1,5 +1,5 @@ import axios from 'axios'; -import { OntimeReport, ShowRun, ShowRunSummary } from 'ontime-types'; +import { OntimeReport, OpenRun, ShowRun, ShowRunSummary } from 'ontime-types'; import { ontimeQueryClient } from '../../common/queryClient'; import { REPORT, apiEntryUrl } from './constants'; @@ -45,7 +45,23 @@ export async function fetchRun(id: string, options?: RequestOptions): Promise { + try { + const res = await axios.get(`${reportUrl}/runs/open`, { signal: options?.signal }); + return res.data; + } catch (error) { + if (axios.isAxiosError(error) && error.response?.status === 404) { + return null; + } + throw error; + } +} + +/** + * HTTP request to finish the run in progress, which is what writes it to history */ export async function finishRun(): Promise { await axios.post(`${reportUrl}/runs/finish`); diff --git a/apps/client/src/common/hooks-query/useRuns.ts b/apps/client/src/common/hooks-query/useRuns.ts index 16228de0e..387d1e875 100644 --- a/apps/client/src/common/hooks-query/useRuns.ts +++ b/apps/client/src/common/hooks-query/useRuns.ts @@ -1,9 +1,9 @@ import { useQuery } from '@tanstack/react-query'; -import { ShowRun, ShowRunSummary } from 'ontime-types'; +import { OpenRun, ShowRun, ShowRunSummary } from 'ontime-types'; import { MILLIS_PER_HOUR } from 'ontime-utils'; -import { REPORT_RUNS } from '../api/constants'; -import { fetchRun, fetchRuns } from '../api/report'; +import { REPORT_OPEN_RUN, REPORT_RUNS } from '../api/constants'; +import { fetchOpenRun, fetchRun, fetchRuns } from '../api/report'; /** * Run history for the current project, optionally scoped to a rundown @@ -33,8 +33,18 @@ export function useRun(id: string | null) { return { data, status }; } -/** The run currently in progress, if any */ -export function useOpenRun(): ShowRunSummary | null { - const { data } = useRuns(); - return data.find((run) => run.endedAt === null) ?? null; +/** + * The run currently being recorded, if any. + * A run in progress lives in memory on the server until it is finished, so it + * cannot be found in the run history and needs its own query. + */ +export function useOpenRun(): OpenRun | null { + const { data } = useQuery({ + queryKey: REPORT_OPEN_RUN, + queryFn: ({ signal }) => fetchOpenRun({ signal }), + placeholderData: (previousData, _previousQuery) => previousData, + staleTime: MILLIS_PER_HOUR, + }); + + return data ?? null; } diff --git a/apps/client/src/features/app-settings/panel/feature-panel/ReportSettings.tsx b/apps/client/src/features/app-settings/panel/feature-panel/ReportSettings.tsx index 363a9457f..df43d70ac 100644 --- a/apps/client/src/features/app-settings/panel/feature-panel/ReportSettings.tsx +++ b/apps/client/src/features/app-settings/panel/feature-panel/ReportSettings.tsx @@ -42,8 +42,8 @@ export default function ReportSettings() { - A run is created the first time an event is started, and is added to history once playback stops. Every - run keeps its own snapshot of the schedule, so editing the rundown later does not change past reports. + Recording starts with the first event of a show and is saved here when you finish the run. Every report + keeps its own snapshot of the schedule, so editing the rundown later does not change past reports. diff --git a/apps/client/src/features/app-settings/panel/feature-panel/composite/RunsList.tsx b/apps/client/src/features/app-settings/panel/feature-panel/composite/RunsList.tsx index 7c7866066..0fee56ef1 100644 --- a/apps/client/src/features/app-settings/panel/feature-panel/composite/RunsList.tsx +++ b/apps/client/src/features/app-settings/panel/feature-panel/composite/RunsList.tsx @@ -6,7 +6,6 @@ import { deleteRun, renameRun } from '../../../../../common/api/report'; import { maybeAxiosError } from '../../../../../common/api/utils'; import IconButton from '../../../../../common/components/buttons/IconButton'; import Input from '../../../../../common/components/input/input/Input'; -import Tag from '../../../../../common/components/tag/Tag'; import { preventEscape } from '../../../../../common/utils/keyEvent'; import { cx } from '../../../../../common/utils/styleUtils'; import * as Panel from '../../../panel-utils/PanelUtils'; @@ -77,14 +76,13 @@ export default function RunsList({ runs, selectedRunId, onSelect }: RunsListProp Started Drift - {runs.length === 0 && ( )} {runs.map((run) => { @@ -123,7 +121,6 @@ export default function RunsList({ runs, selectedRunId, onSelect }: RunsListProp {run.rundownTitle} {new Date(run.startedAt).toLocaleString()} {formatDrift(run.summary.drift, run.summary.eventsRun)} - {run.endedAt === null && Ongoing} event.stopPropagation()}> {!isRenaming && ( <> diff --git a/apps/server/src/api-data/report/__tests__/report.service.test.ts b/apps/server/src/api-data/report/__tests__/report.service.test.ts index e5b91501b..f11ecd1fb 100644 --- a/apps/server/src/api-data/report/__tests__/report.service.test.ts +++ b/apps/server/src/api-data/report/__tests__/report.service.test.ts @@ -7,8 +7,8 @@ import { makeRuntimeStateData } from '../../../stores/__mocks__/runtimeState.moc // in-memory stand-in for the sidecar store, verified separately in report.store.test.ts let runs: ShowRun[] = []; -/** write options seen by the store, so we can assert what reaches the disk and when */ -let writeOptions: ({ debounce?: boolean } | undefined)[] = []; +/** how many times the store was asked to write, to prove the cue path stays clean */ +let writes = 0; vi.mock('../../../services/report-service/report.store.js', () => ({ // isolation between tests comes from the top-level beforeEach resetting `runs`, @@ -16,8 +16,8 @@ vi.mock('../../../services/report-service/report.store.js', () => ({ loadReports: vi.fn(async () => ({ runs })), getRuns: vi.fn(() => runs), getRun: vi.fn((id: string) => runs.find((run) => run.id === id)), - upsertRun: vi.fn(async (run: ShowRun, options?: { debounce?: boolean }) => { - writeOptions.push(options); + upsertRun: vi.fn(async (run: ShowRun) => { + writes += 1; const index = runs.findIndex((candidate) => candidate.id === run.id); if (index === -1) { runs.unshift(run); @@ -25,16 +25,6 @@ vi.mock('../../../services/report-service/report.store.js', () => ({ runs[index] = run; } }), - upsertRuns: vi.fn(async (updated: ShowRun[]) => { - for (const run of updated) { - const index = runs.findIndex((candidate) => candidate.id === run.id); - if (index === -1) { - runs.unshift(run); - } else { - runs[index] = run; - } - } - }), deleteRun: vi.fn(async (id: string) => { const index = runs.findIndex((run) => run.id === id); if (index === -1) return false; @@ -77,6 +67,7 @@ const { initReports, listRuns, getRun, + getOpenRun, renameRun, deleteRun, deleteAllRuns, @@ -87,7 +78,7 @@ const eventB = makeOntimeEvent({ id: 'event-b', timeStart: 10000, timeEnd: 20000 beforeEach(async () => { runs = []; - writeOptions = []; + writes = 0; currentRundown = makeRundown({ id: 'rundown-1', title: 'Test rundown', @@ -155,59 +146,59 @@ describe('triggerReportEntry()', () => { expect(generate()).toEqual({}); }); - it('persists a run to history on the first event stop', async () => { - const start = makeRuntimeStateData({ eventNow: eventA, timer: { startedAt: 0 }, clock: 0 }); - triggerReportEntry(TimerLifeCycle.onStart, start); - const stop = makeRuntimeStateData({ eventNow: eventA, timer: { startedAt: 0 }, clock: 10000 }); - triggerReportEntry(TimerLifeCycle.onStop, stop); - - // persistence happens off the event loop - await new Promise((resolve) => setImmediate(resolve)); - - expect(listRuns()).toHaveLength(1); - expect(listRuns()[0].rundownId).toBe('rundown-1'); - }); }); -describe('write pressure on the show critical path', () => { +describe('nothing is written on the cue path', () => { function runEvent(clock: number) { const state = makeRuntimeStateData({ eventNow: eventA, timer: { startedAt: 0 }, clock, _startEpoch: showEpoch }); triggerReportEntry(TimerLifeCycle.onStart, state); triggerReportEntry(TimerLifeCycle.onStop, { ...state, clock: clock + 1000 } as typeof state); } - it('coalesces writes while a run is in progress', async () => { - // the sidecar holds the whole project history, so writing per event would - // put work proportional to everything that ever ran onto the cue path + it('keeps a run in progress out of history until it is finished', async () => { runEvent(0); runEvent(2000); runEvent(4000); await new Promise((resolve) => setImmediate(resolve)); - expect(writeOptions.length).toBeGreaterThan(0); - expect(writeOptions.every((options) => options?.debounce === true)).toBe(true); + // the run exists and is accumulating, but is not a report yet + expect(getOpenRun()).not.toBeNull(); + expect(Object.keys(generate())).toHaveLength(1); + expect(listRuns()).toHaveLength(0); + expect(writes).toBe(0); }); - it('writes immediately when the run is finished', async () => { + it('writes exactly once, when the run is finished', async () => { runEvent(0); - await new Promise((resolve) => setImmediate(resolve)); - writeOptions = []; + runEvent(2000); + expect(writes).toBe(0); await closeRun(); - expect(writeOptions).toHaveLength(1); - expect(writeOptions[0]?.debounce).toBeFalsy(); + expect(writes).toBe(1); + expect(listRuns()).toHaveLength(1); + expect(getOpenRun()).toBeNull(); }); - it('writes immediately when a user renames a run', async () => { + it('stamps the finished run with an end', async () => { + const before = Date.now(); runEvent(0); await closeRun(); - const runId = listRuns()[0].id; - writeOptions = []; - await renameRun(runId, 'Dress rehearsal'); + const run = listRuns()[0]; + expect(run.endedAt).toBeGreaterThanOrEqual(before); + expect(run.endedAt).toBeGreaterThanOrEqual(run.startedAt); + }); - expect(writeOptions[0]?.debounce).toBeFalsy(); + it('discards an unfinished run when the project changes', async () => { + runEvent(0); + expect(getOpenRun()).not.toBeNull(); + + await initReports('project-b'); + + expect(getOpenRun()).toBeNull(); + expect(listRuns()).toHaveLength(0); + expect(writes).toBe(0); }); }); @@ -224,7 +215,7 @@ describe('run timestamps', () => { }); triggerReportEntry(TimerLifeCycle.onStart, state); triggerReportEntry(TimerLifeCycle.onStop, { ...state, clock: 44200000 } as typeof state); - await new Promise((resolve) => setImmediate(resolve)); + await closeRun(); const run = listRuns()[0]; expect(run.startedAt).toBe(showEpoch); @@ -236,7 +227,7 @@ describe('run timestamps', () => { const state = makeRuntimeStateData({ eventNow: eventA, timer: { startedAt: 0 }, clock: 0, _startEpoch: null }); triggerReportEntry(TimerLifeCycle.onStart, state); triggerReportEntry(TimerLifeCycle.onStop, { ...state, clock: 10000 } as typeof state); - await new Promise((resolve) => setImmediate(resolve)); + await closeRun(); const run = listRuns()[0]; expect(run.startedAt).toBeGreaterThanOrEqual(before); @@ -277,16 +268,14 @@ describe('closeRun()', () => { triggerReportEntry(TimerLifeCycle.onStop, { ...start, clock: 10000 } as typeof start); await closeRun(); - await new Promise((resolve) => setImmediate(resolve)); expect(listRuns()).toHaveLength(1); - expect(listRuns()[0].endedAt).toBe(10000); - // a new start after closing opens a second run rather than reusing the first + // a new start after finishing opens a second run rather than reusing the first const secondStart = makeRuntimeStateData({ eventNow: eventB, timer: { startedAt: 20000 }, clock: 20000 }); triggerReportEntry(TimerLifeCycle.onStart, secondStart); triggerReportEntry(TimerLifeCycle.onStop, { ...secondStart, clock: 30000 } as typeof secondStart); - await new Promise((resolve) => setImmediate(resolve)); + await closeRun(); expect(listRuns()).toHaveLength(2); }); @@ -297,81 +286,6 @@ describe('closeRun()', () => { }); }); -describe('initReports()', () => { - it('closes a dangling run left open by a crash or shutdown', async () => { - runs = [ - { - id: 'dangling', - rundownId: 'rundown-1', - rundownTitle: 'Test rundown', - label: 'unfinished', - startedAt: 0, - endedAt: null, - report: { - [eventA.id]: { - startedAt: 0, - endedAt: 9000, - scheduledStart: 0, - scheduledDuration: 10000, - playCount: 1, - }, - }, - summary: { - eventsRun: 1, - eventsPlanned: 2, - scheduledDuration: 10000, - actualDuration: 9000, - drift: -1000, - eventsOver: 0, - eventsUnder: 1, - eventsOnTime: 0, - worstOverrun: null, - }, - }, - ]; - - await initReports('project-a'); - - const recovered = getRun('dangling'); - expect(recovered?.endedAt).toBe(9000); - }); - - it('closes every dangling run, not only the first', async () => { - runs = [makeDanglingRun('first', 4000), makeDanglingRun('second', 7000)]; - - await initReports('project-a'); - - expect(getRun('first')?.endedAt).toBe(4000); - expect(getRun('second')?.endedAt).toBe(7000); - expect(listRuns().every((run) => run.endedAt !== null)).toBe(true); - }); - - function makeDanglingRun(id: string, endedAt: number): ShowRun { - return { - id, - rundownId: 'rundown-1', - rundownTitle: 'Test rundown', - label: id, - startedAt: showEpoch, - endedAt: null, - report: { - [eventA.id]: { startedAt: 0, endedAt, scheduledStart: 0, scheduledDuration: 10000, playCount: 1 }, - }, - summary: { - eventsRun: 1, - eventsPlanned: 2, - scheduledDuration: 10000, - actualDuration: endedAt, - drift: endedAt - 10000, - eventsOver: 0, - eventsUnder: 1, - eventsOnTime: 0, - worstOverrun: null, - }, - }; - } -}); - describe('summary is measured against the run\'s own rundown', () => { it('counts planned events from the rundown the run belongs to', async () => { // a three event rundown that is not the loaded one @@ -390,29 +304,26 @@ describe('summary is measured against the run\'s own rundown', () => { triggerReportEntry(TimerLifeCycle.onStart, state); currentRundown = storedRundowns['rundown-1']; - triggerReportEntry(TimerLifeCycle.onStop, { ...state, clock: 10000 } as typeof state); - await new Promise((resolve) => setImmediate(resolve)); + await closeRun(); // three, from rundown-2, not two from the now loaded rundown-1 expect(listRuns()[0].summary.eventsPlanned).toBe(3); }); - it('keeps the last known count when the rundown has been deleted', async () => { + it('falls back to what ran when the rundown has been deleted', async () => { + // the run belongs to a rundown that is neither loaded nor in storage currentRundown = makeRundown({ id: 'gone', title: 'Deleted rundown', entries: {}, order: [], flatOrder: [] }); const state = makeRuntimeStateData({ eventNow: eventA, timer: { startedAt: 0 }, clock: 0, _startEpoch: showEpoch }); triggerReportEntry(TimerLifeCycle.onStart, state); triggerReportEntry(TimerLifeCycle.onStop, { ...state, clock: 10000 } as typeof state); - await new Promise((resolve) => setImmediate(resolve)); - - // rundown disappears from both the loaded slot and storage - const runId = listRuns()[0].id; - runs[0] = { ...runs[0], summary: { ...runs[0].summary, eventsPlanned: 7 } }; currentRundown = storedRundowns['rundown-1']; - triggerReportEntry(TimerLifeCycle.onStop, { ...state, clock: 20000 } as typeof state); - await new Promise((resolve) => setImmediate(resolve)); + await closeRun(); - expect(getRun(runId)?.summary.eventsPlanned).toBe(7); + // one event ran, so planned reads as one rather than zero of one + const summary = listRuns()[0].summary; + expect(summary.eventsPlanned).toBe(1); + expect(summary.eventsRun).toBe(1); }); }); @@ -456,17 +367,6 @@ describe('run history queries and edits', () => { expect(getRun('run-a')).toBeUndefined(); }); - it('clears the in-progress report when the open run is deleted', async () => { - const start = makeRuntimeStateData({ eventNow: eventA, timer: { startedAt: 0 }, clock: 0 }); - triggerReportEntry(TimerLifeCycle.onStart, start); - triggerReportEntry(TimerLifeCycle.onStop, { ...start, clock: 10000 } as typeof start); - await new Promise((resolve) => setImmediate(resolve)); - - const openRunId = listRuns()[0].id; - await deleteRun(openRunId); - - expect(generate()).toEqual({}); - }); it('deletes all run history', async () => { await makeClosedRun('run-a'); diff --git a/apps/server/src/api-data/report/report.router.ts b/apps/server/src/api-data/report/report.router.ts index bdc53aa78..d34508444 100644 --- a/apps/server/src/api-data/report/report.router.ts +++ b/apps/server/src/api-data/report/report.router.ts @@ -16,13 +16,27 @@ router.get('/', (_req: Request, res: Response) => { }); /** - * Run history, most recent first. `?rundownId=` scopes the list to one rundown. + * Finished reports, most recent first. `?rundownId=` scopes the list to one + * rundown. A run in progress is not a report and does not appear here. */ router.get('/runs', validateRundownIdQuery, (req: Request, res: Response) => { const { rundownId } = req.query as { rundownId?: string }; res.status(200).json(report.listRuns(rundownId)); }); +/** + * The run currently being recorded, which exists in memory only. + * Registered ahead of /runs/:id so "open" is not read as an id. + */ +router.get('/runs/open', (_req: Request, res: Response) => { + const run = report.getOpenRun(); + if (!run) { + res.status(404).send(); + return; + } + res.status(200).json(run); +}); + /** * Closes the run in progress and writes it to history immediately. * Registered ahead of /runs/:id so "finish" is not read as an id. diff --git a/apps/server/src/api-data/report/report.service.ts b/apps/server/src/api-data/report/report.service.ts index b374987c4..fa7ffae62 100644 --- a/apps/server/src/api-data/report/report.service.ts +++ b/apps/server/src/api-data/report/report.service.ts @@ -2,6 +2,7 @@ import { EntryId, OntimeEventReport, OntimeReport, + OpenRun, RefetchKey, Rundown, ShowRun, @@ -23,8 +24,8 @@ const report = new Map(); let formattedReport: OntimeReport | null = null; -/** metadata for the run in progress, null when no run is open */ -let openRun: Omit | null = null; +/** the run being recorded, held in memory only until it is finished */ +let openRun: OpenRun | null = null; /** * generates a full report @@ -48,7 +49,6 @@ export function clear(id?: string) { } else { report.clear(); } - void persistOpenRun(); } /** @@ -102,7 +102,8 @@ export function triggerReportEntry( playCount: previous?.playCount ?? 1, }); formattedReport = null; - void persistOpenRun(); + // deliberately not written here: a run is kept in memory until it is + // finished, so nothing touches the disk on the cue path sendRefetch(RefetchKey.Report); } } @@ -122,7 +123,7 @@ export async function closeRun(): Promise { // detach the run before the write so a start arriving in between opens a // new run instead of appending to the one we are closing - const closing = { ...openRun, endedAt: lastEndedAtIn(generate()) }; + const closing = { ...openRun, endedAt: timeCore.now() }; openRun = null; const closed = await persistRun(closing, generate()); @@ -130,6 +131,11 @@ export async function closeRun(): Promise { return closed; } +/** The run being recorded, if any. Held in memory until it is finished. */ +export function getOpenRun(): OpenRun | null { + return openRun; +} + /** * Opens a run on the first event start after the previous run closed. * The in progress report is reset here rather than on close, so the rundown @@ -156,7 +162,6 @@ function openRunIfNeeded(state: DeepReadonly) { rundownTitle: rundown.title, label: makeRunLabel(startedAt), startedAt, - endedAt: null, }; // let the editor show that a run is being recorded without waiting @@ -181,29 +186,21 @@ function makeRunLabel(startedAt: number): string { * Persisting on every event stop means an interrupted show still leaves a record. * @private */ -async function persistOpenRun(): Promise { - if (openRun === null) { - return; - } - // debounced: this runs as events stop, which is the show critical path - await persistRun(openRun, generate(), { debounce: true }); -} - /** - * Writes a run and its derived summary to the sidecar + * Writes a finished run and its derived summary to the sidecar * @private */ async function persistRun( run: Omit, currentReport: OntimeReport, - options?: reportStore.WriteOptions, ): Promise { const rundown = getRundownForRun(run.rundownId); - // a rundown deleted mid-run leaves nothing to count against, so we keep - // whatever the run was last written with rather than reporting zero + // a rundown deleted mid-run leaves nothing to count against. Falling back to + // what actually ran keeps the report honest, where zero would read as if the + // whole show had been missed const eventsPlanned = rundown ? countPlannedEvents(rundown.entries, rundown.flatOrder) - : (reportStore.getRun(run.id)?.summary.eventsPlanned ?? 0); + : Object.keys(currentReport).length; const persisted: ShowRun = { ...run, @@ -211,7 +208,7 @@ async function persistRun( summary: getRunSummary(currentReport, eventsPlanned), }; - await reportStore.upsertRun(persisted, options); + await reportStore.upsertRun(persisted); return persisted; } @@ -236,8 +233,10 @@ function getRundownForRun(rundownId: string): Readonly | null { /** * Prepares reporting for a newly loaded project. - * Any run left open by a crash or shutdown is closed against its own data - * so it cannot absorb events from the next show. + * + * There is nothing to recover here: a run only reaches the sidecar once it + * has been finished, so the stored history never contains a partial run. An + * unfinished run is discarded along with the project it belonged to. */ export async function initReports(projectFilename: string): Promise { report.clear(); @@ -245,29 +244,6 @@ export async function initReports(projectFilename: string): Promise { openRun = null; await reportStore.loadReports(projectFilename); - - // close every run left open, not just the first: a run stuck with a null - // endedAt reads as ongoing forever and is skipped by getLatestRun - const closed: ShowRun[] = []; - for (const run of reportStore.getRuns()) { - if (run.endedAt !== null) continue; - closed.push({ ...run, endedAt: lastEndedAtIn(run.report) }); - } - await reportStore.upsertRuns(closed); -} - -/** - * Timestamp of the last event to finish within a stored report - * @private - */ -function lastEndedAtIn(storedReport: OntimeReport): number | null { - let latest: number | null = null; - for (const entry of Object.values(storedReport)) { - if (entry.endedAt !== null && (latest === null || entry.endedAt > latest)) { - latest = entry.endedAt; - } - } - return latest; } /** Run history for the current project, without per event data */ @@ -295,13 +271,10 @@ export async function renameRun(id: string, label: string): Promise { + // only finished reports are in the store, so this can never target the + // run in progress const didDelete = await reportStore.deleteRun(id); if (didDelete) { - if (openRun?.id === id) { - openRun = null; - report.clear(); - formattedReport = null; - } sendRefetch(RefetchKey.Report); } return didDelete; diff --git a/apps/server/src/app.ts b/apps/server/src/app.ts index 12c7e8dbd..29d9d17a6 100644 --- a/apps/server/src/app.ts +++ b/apps/server/src/app.ts @@ -28,7 +28,6 @@ import { ONTIME_VERSION } from './ONTIME_VERSION.js'; import { getShowWelcomeDialog } from './services/app-state-service/AppStateService.js'; import * as messageService from './services/message-service/message.service.js'; import { initialiseProject } from './services/project-service/ProjectService.js'; -import { flush as flushReports } from './services/report-service/report.store.js'; import { restoreService } from './services/restore-service/restore.service.js'; import type { RestorePoint } from './services/restore-service/restore.type.js'; import { runtimeService } from './services/runtime-service/runtime.service.js'; @@ -332,14 +331,6 @@ async function performShutdown(exitCode: number): Promise { shutdownTimeout, ); - // report writes are coalesced during a run, make sure none are outstanding - await withTimeout( - flushReports().catch((_error) => { - /** nothing do to here */ - }), - shutdownTimeout, - ); - // clear the restore file if it was a normal exit // 0 means it was a SIGNAL // 1 means crash -> keep the file diff --git a/apps/server/src/services/report-service/__tests__/report.store.test.ts b/apps/server/src/services/report-service/__tests__/report.store.test.ts index 8f7d9c258..8026e9d6c 100644 --- a/apps/server/src/services/report-service/__tests__/report.store.test.ts +++ b/apps/server/src/services/report-service/__tests__/report.store.test.ts @@ -44,7 +44,6 @@ const { getRuns, getRun, upsertRun, - upsertRuns, deleteRun, deleteRunsForRundown, deleteAllRuns, @@ -52,8 +51,6 @@ const { renameReportsForProject, resetStore, getPathToReports, - flush, - hasUnwrittenChanges, } = await import('../report.store.js'); function makeRun(patch: Partial = {}): ShowRun { @@ -143,80 +140,7 @@ describe('upsertRun() / getRun() / getRuns()', () => { }); }); -describe('upsertRuns()', () => { - beforeEach(async () => { - await loadReports('project-a'); - }); - it('applies several runs in a single write', async () => { - await upsertRuns([makeRun({ id: 'a' }), makeRun({ id: 'b' })]); - - expect(getRuns().map((run) => run.id)).toEqual(['b', 'a']); - const written = files.get(getPathToReports('project-a')) as { runs: ShowRun[] }; - expect(written.runs).toHaveLength(2); - }); - - it('mixes inserts and replacements', async () => { - await upsertRun(makeRun({ id: 'existing', label: 'before' })); - await upsertRuns([makeRun({ id: 'existing', label: 'after' }), makeRun({ id: 'fresh' })]); - - expect(getRuns()).toHaveLength(2); - expect(getRun('existing')?.label).toBe('after'); - }); - - it('does not write when given nothing to do', async () => { - await upsertRun(makeRun({ id: 'a' })); - await upsertRuns([]); - - expect(getRuns()).toHaveLength(1); - }); -}); - -describe('write coalescing', () => { - beforeEach(async () => { - await loadReports('project-a'); - }); - - it('keeps a debounced write off the disk until flushed', async () => { - await upsertRun(makeRun({ id: 'a' }), { debounce: true }); - - expect(hasUnwrittenChanges()).toBe(true); - expect(files.has(getPathToReports('project-a'))).toBe(false); - - await flush(); - - expect(hasUnwrittenChanges()).toBe(false); - const written = files.get(getPathToReports('project-a')) as { runs: ShowRun[] }; - expect(written.runs).toHaveLength(1); - }); - - it('collapses repeated debounced writes into one flush', async () => { - await upsertRun(makeRun({ id: 'a' }), { debounce: true }); - await upsertRun(makeRun({ id: 'b' }), { debounce: true }); - await upsertRun(makeRun({ id: 'c' }), { debounce: true }); - await flush(); - - const written = files.get(getPathToReports('project-a')) as { runs: ShowRun[] }; - expect(written.runs).toHaveLength(3); - }); - - it('writes straight through when not debounced', async () => { - await upsertRun(makeRun({ id: 'a' })); - - expect(hasUnwrittenChanges()).toBe(false); - expect(files.has(getPathToReports('project-a'))).toBe(true); - }); - - it('flushes pending work before switching project', async () => { - await upsertRun(makeRun({ id: 'a' }), { debounce: true }); - - await loadReports('project-b'); - - const written = files.get(getPathToReports('project-a')) as { runs: ShowRun[] }; - expect(written.runs).toHaveLength(1); - expect(getRuns()).toEqual([]); - }); -}); describe('deleteRun()', () => { beforeEach(async () => { diff --git a/apps/server/src/services/report-service/report.store.ts b/apps/server/src/services/report-service/report.store.ts index e7342c259..180809ed2 100644 --- a/apps/server/src/services/report-service/report.store.ts +++ b/apps/server/src/services/report-service/report.store.ts @@ -25,19 +25,9 @@ function emptyStore(): ProjectReports { return { runs: [] }; } -/** - * How long writes are coalesced for while a run is in progress. - * A run's events stop frequently and the sidecar holds the project's whole - * history, so writing per event would put work proportional to everything - * that ever ran onto the show critical path. - */ -const writeDebounce = 5000; - let fileRef: JSONFile | null = null; let cache: ProjectReports = emptyStore(); let failedWriteAttempts = 0; -let writeTimer: NodeJS.Timeout | null = null; -let hasPendingWrite = false; /** * Resolves the sidecar path for a given project file name @@ -52,9 +42,6 @@ export function getPathToReports(projectFilename: string): string { * project changes. */ export async function loadReports(projectFilename: string): Promise { - // the outgoing project may have a coalesced write outstanding - await flush(); - fileRef = new JSONFile(getPathToReports(projectFilename)); failedWriteAttempts = 0; @@ -80,45 +67,20 @@ export function getRun(id: string): ShowRun | undefined { return cache.runs.find((run) => run.id === id); } -export type WriteOptions = { - /** coalesce this write instead of hitting disk straight away */ - debounce?: boolean; -}; - /** - * Inserts or replaces a run, keeping the list ordered newest first - */ -export async function upsertRun(run: ShowRun, options?: WriteOptions): Promise { - return upsertRuns([run], options); -} - -/** - * Inserts or replaces several runs in one pass. - * Batching matters: these all live in a single file, so writing per run would - * mean concurrent writes racing on the same path. + * Inserts or replaces a run, keeping the list ordered newest first. * - * Writes are immediate unless the caller opts into debouncing. Anything driven - * by a user action should stay immediate so it cannot be lost. + * Every write here is a whole finished report, which only happens when a run + * is finished or edited. Nothing writes on the cue path. */ -export async function upsertRuns(updated: ShowRun[], options?: WriteOptions): Promise { - if (updated.length === 0) { - return; +export async function upsertRun(run: ShowRun): Promise { + const index = cache.runs.findIndex((candidate) => candidate.id === run.id); + if (index === -1) { + cache.runs.unshift(run); + } else { + cache.runs[index] = run; } - - for (const run of updated) { - const index = cache.runs.findIndex((candidate) => candidate.id === run.id); - if (index === -1) { - cache.runs.unshift(run); - } else { - cache.runs[index] = run; - } - } - - if (options?.debounce) { - scheduleWrite(); - return; - } - await flush(); + await persist(); } /** @@ -132,7 +94,7 @@ export async function deleteRun(id: string): Promise { } cache.runs.splice(index, 1); - await flush(); + await persist(); return true; } @@ -146,7 +108,7 @@ export async function deleteRunsForRundown(rundownId: string): Promise { const removed = before - cache.runs.length; if (removed > 0) { - await flush(); + await persist(); } return removed; } @@ -156,7 +118,7 @@ export async function deleteRunsForRundown(rundownId: string): Promise { */ export async function deleteAllRuns(): Promise { cache.runs = []; - await flush(); + await persist(); } /** @@ -196,34 +158,11 @@ export async function renameReportsForProject(originalFilename: string, newFilen } /** - * Marks the cache dirty and arms the coalescing timer. + * Writes the cache to disk. + * Gives up after repeated failures so a broken disk cannot stall the runtime. * @private */ -function scheduleWrite(): void { - hasPendingWrite = true; - if (writeTimer !== null) { - return; - } - - writeTimer = setTimeout(() => { - writeTimer = null; - void flush(); - }, writeDebounce); - // a pending report write is never a reason to keep the process alive - writeTimer.unref?.(); -} - -/** - * Writes any outstanding changes to disk now. - * Called on every immediate write, when the project changes, and on shutdown. - */ -export async function flush(): Promise { - if (writeTimer !== null) { - clearTimeout(writeTimer); - writeTimer = null; - } - hasPendingWrite = false; - +async function persist(): Promise { if (fileRef === null || failedWriteAttempts > 3) { return; } @@ -236,20 +175,10 @@ export async function flush(): Promise { } } -/** Whether a coalesced write is still outstanding, for tests and diagnostics */ -export function hasUnwrittenChanges(): boolean { - return hasPendingWrite; -} - /** * Resets in-memory state, used when no project is loaded and in tests */ export function resetStore(): void { - if (writeTimer !== null) { - clearTimeout(writeTimer); - writeTimer = null; - } - hasPendingWrite = false; fileRef = null; cache = emptyStore(); failedWriteAttempts = 0; diff --git a/packages/types/src/definitions/core/Report.type.ts b/packages/types/src/definitions/core/Report.type.ts index ad85b0332..e25101016 100644 --- a/packages/types/src/definitions/core/Report.type.ts +++ b/packages/types/src/definitions/core/Report.type.ts @@ -49,10 +49,11 @@ export type ShowRun = { */ startedAt: number; /** - * Time of day the last event in the run finished, or null while the run is - * open. Only ever compared against the per event times in the same run. + * Wall clock instant the run was finished. + * Always set: a run only becomes a report once it has been finished, so + * there is no such thing as a stored run still in progress. */ - endedAt: MaybeNumber; + endedAt: number; report: OntimeReport; summary: RunSummary; }; @@ -60,6 +61,13 @@ export type ShowRun = { /** A run without its per event data, for list views */ export type ShowRunSummary = Omit; +/** + * The run currently being recorded. + * Lives in memory only until it is finished, so it carries no report data + * and has no end. + */ +export type OpenRun = Omit; + /** Contents of a project's report sidecar file */ export type ProjectReports = { runs: ShowRun[]; diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index c82d1d124..3ceecbdc1 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -27,6 +27,7 @@ export type { Day, Duration, Instant, TimeOfDay } from './definitions/core/Tempo export type { OntimeReport, OntimeEventReport, + OpenRun, ProjectReports, RunSummary, ShowRun,