From 1866e5e78dc02e4f875894da564a868d802b789b Mon Sep 17 00:00:00 2001 From: Carlos Valente Date: Sat, 6 Sep 2025 09:41:04 +0200 Subject: [PATCH] refactor: simplify restore point --- apps/server/src/app.ts | 3 +- apps/server/src/services/RestoreService.ts | 146 ------------------ .../services/__tests__/RestoreService.test.ts | 139 ----------------- .../__tests__/restore.parser.test.ts | 63 ++++++++ .../__tests__/restore.service.test.ts | 124 +++++++++++++++ .../restore-service/restore.parser.ts | 53 +++++++ .../restore-service/restore.service.ts | 106 +++++++++++++ .../services/restore-service/restore.type.ts | 10 ++ .../runtime-service/runtime.service.ts | 3 +- apps/server/src/stores/runtimeState.ts | 2 +- 10 files changed, 361 insertions(+), 288 deletions(-) delete mode 100644 apps/server/src/services/RestoreService.ts delete mode 100644 apps/server/src/services/__tests__/RestoreService.test.ts create mode 100644 apps/server/src/services/restore-service/__tests__/restore.parser.test.ts create mode 100644 apps/server/src/services/restore-service/__tests__/restore.service.test.ts create mode 100644 apps/server/src/services/restore-service/restore.parser.ts create mode 100644 apps/server/src/services/restore-service/restore.service.ts create mode 100644 apps/server/src/services/restore-service/restore.type.ts diff --git a/apps/server/src/app.ts b/apps/server/src/app.ts index f47daf928..933745b05 100644 --- a/apps/server/src/app.ts +++ b/apps/server/src/app.ts @@ -34,7 +34,8 @@ import { populateTranslation } from './setup/loadTranslations.js'; import { populateStyles } from './setup/loadStyles.js'; import { eventStore } from './stores/EventStore.js'; import { runtimeService } from './services/runtime-service/runtime.service.js'; -import { RestorePoint, restoreService } from './services/RestoreService.js'; +import { restoreService } from './services/restore-service/restore.service.js'; +import type { RestorePoint } from './services/restore-service/restore.type.js'; import * as messageService from './services/message-service/message.service.js'; import { getState } from './stores/runtimeState.js'; import { initialiseProject } from './services/project-service/ProjectService.js'; diff --git a/apps/server/src/services/RestoreService.ts b/apps/server/src/services/RestoreService.ts deleted file mode 100644 index 11c749181..000000000 --- a/apps/server/src/services/RestoreService.ts +++ /dev/null @@ -1,146 +0,0 @@ -import { MaybeNumber, MaybeString, Playback } from 'ontime-types'; - -import { JSONFile } from 'lowdb/node'; -import { deepEqual } from 'fast-equals'; - -import { publicFiles } from '../setup/index.js'; - -export type RestorePoint = { - playback: Playback; - selectedEventId: MaybeString; - startedAt: MaybeNumber; - addedTime: number; - pausedAt: MaybeNumber; - firstStart: MaybeNumber; -}; - -/** - * Utility validates a RestorePoint - * @param obj - * @return boolean - */ -export function isRestorePoint(obj: unknown): obj is RestorePoint { - if (!obj) { - return false; - } - - const restorePoint = obj as RestorePoint; - - if (typeof restorePoint.playback !== 'string' || !Object.values(Playback).includes(restorePoint.playback)) { - return false; - } - - if (typeof restorePoint.selectedEventId !== 'string' && restorePoint.selectedEventId !== null) { - return false; - } - - if (typeof restorePoint.startedAt !== 'number' && restorePoint.startedAt !== null) { - return false; - } - - if (typeof restorePoint.addedTime !== 'number') { - return false; - } - - if (typeof restorePoint.pausedAt !== 'number' && restorePoint.pausedAt !== null) { - return false; - } - - if (typeof restorePoint.firstStart !== 'number' && restorePoint.firstStart !== null) { - return false; - } - - return true; -} - -/** - * Utility interface to allow dependency injection during test - */ - -/** - * Service manages saving of application state - * that can then be restored when reopening - */ -export class RestoreService { - private readonly filePath: MaybeString; - private readonly file: JSONFile; - private failedCreateAttempts: number; - private savedState: RestorePoint | null; - - constructor(filePath: string) { - this.filePath = filePath; - - this.savedState = null; - this.file = new JSONFile(this.filePath); - this.failedCreateAttempts = 0; - } - - /** - * Utility, reads from file - * @private - */ - private async read() { - return this.file.read(); - } - - /** - * Utility writes payload to file - * @throws - * @param stringifiedState - */ - private async write(data: RestorePoint) { - await this.file.write(data); - } - - /** - * Saves runtime data to restore file - * @param newState RestorePoint - */ - async save(newState: RestorePoint) { - // after three failed attempts, mark the service as unavailable - if (this.failedCreateAttempts > 3) { - return; - } - - if (deepEqual(newState, this.savedState)) { - return; - } - - try { - await this.write(newState); - this.savedState = { ...newState }; - this.failedCreateAttempts = 0; - } catch (_error) { - this.failedCreateAttempts += 1; - } - } - - /** - * Attempts reading a restore point from a given file path - * Returns null if none found, restore point otherwise - */ - async load(): Promise { - try { - const maybeRestorePoint = await this.read(); - if (isRestorePoint(maybeRestorePoint)) { - return maybeRestorePoint; - } - } catch (_error) { - // no need to notify the user - } - return null; - } - - /** - * Clears the restore file - */ - async clear() { - try { - await this.file.write(null); - } catch (_error) { - // nothing to do - } - } -} - -export const restoreService = new RestoreService(publicFiles.restoreFile); diff --git a/apps/server/src/services/__tests__/RestoreService.test.ts b/apps/server/src/services/__tests__/RestoreService.test.ts deleted file mode 100644 index 42341f3ac..000000000 --- a/apps/server/src/services/__tests__/RestoreService.test.ts +++ /dev/null @@ -1,139 +0,0 @@ -/* eslint-disable @typescript-eslint/no-explicit-any */ -import { describe, expect, it, vi } from 'vitest'; - -import { Playback } from 'ontime-types'; - -import { isRestorePoint, RestorePoint, RestoreService } from '../RestoreService.js'; - -describe('isRestorePoint()', () => { - it('validates a well defined object', () => { - let restorePoint: RestorePoint = { - playback: Playback.Roll, - selectedEventId: '123', - startedAt: 1, - addedTime: 2, - pausedAt: 3, - firstStart: 1, - }; - expect(isRestorePoint(restorePoint)).toBe(true); - - restorePoint = { - playback: Playback.Roll, - selectedEventId: '123', - startedAt: null, - addedTime: 0, - pausedAt: null, - firstStart: 1, - }; - expect(isRestorePoint(restorePoint)).toBe(true); - }); - - describe('rejects a badly formatted file', () => { - it('with invalid playback value', () => { - const restorePoint = { - playback: 'unknown', - selectedEventId: '123', - startedAt: null, - addedTime: 0, - pausedAt: null, - groupStartAt: 10, - }; - expect(isRestorePoint(restorePoint)).toBe(false); - }); - it('with missing playback value', () => { - const restorePoint = { - selectedEventId: '123', - startedAt: null, - addedTime: 0, - pausedAt: null, - groupStartAt: 10, - }; - expect(isRestorePoint(restorePoint)).toBe(false); - }); - it('with incorrect value', () => { - const restorePoint = { - playback: Playback.Roll, - selectedEventId: '123', - startedAt: 'testing', - addedTime: 0, - pausedAt: null, - groupStartAt: 10, - }; - expect(isRestorePoint(restorePoint)).toBe(false); - }); - }); -}); - -describe('RestoreService()', () => { - describe('load()', () => { - it('loads working file with times', async () => { - const expected: RestorePoint = { - playback: Playback.Play, - selectedEventId: 'da5b4', - startedAt: 1234, - addedTime: 5678, - pausedAt: 9087, - firstStart: 1234, - }; - - const restoreService = new RestoreService('/path/to/restore/file'); - vi.spyOn(restoreService, 'read').mockImplementation(() => expected); - - const testLoad = await restoreService.load(); - expect(testLoad).toStrictEqual(expected); - }); - - it('loads working file without times', async () => { - const expected: RestorePoint = { - playback: Playback.Stop, - selectedEventId: null, - startedAt: null, - addedTime: 0, - pausedAt: null, - firstStart: 1234, - }; - - const restoreService = new RestoreService('/path/to/restore/file'); - vi.spyOn(restoreService, 'read').mockImplementation(() => expected); - - const testLoad = await restoreService.load(); - expect(testLoad).toStrictEqual(expected); - }); - - it('does not load wrong play state', async () => { - const expected = { - playback: 'does-not-exist', - selectedEventId: 'da5b4', - startedAt: 1234, - addedTime: 1234, - pausedAt: 1234, - firstStart: 1234, - groupStartAt: 10, - }; - - const restoreService = new RestoreService('/path/to/restore/file'); - vi.spyOn(restoreService, 'read').mockImplementation(() => expected); - - const testLoad = await restoreService.load(); - expect(testLoad).toBe(null); - }); - }); - - describe('save()', () => { - it('saves data to file', async () => { - const testData: RestorePoint = { - playback: Playback.Play, - selectedEventId: '1234', - startedAt: 1234, - addedTime: 1234, - pausedAt: 1234, - firstStart: 1234, - }; - - const restoreService = new RestoreService('/path/to/restore/file'); - const writeSpy = vi.spyOn(restoreService, 'write').mockImplementation(() => undefined); - await restoreService.save(testData); - expect(writeSpy).toHaveBeenCalledWith(testData); - }); - }); -}); diff --git a/apps/server/src/services/restore-service/__tests__/restore.parser.test.ts b/apps/server/src/services/restore-service/__tests__/restore.parser.test.ts new file mode 100644 index 000000000..f10f890de --- /dev/null +++ b/apps/server/src/services/restore-service/__tests__/restore.parser.test.ts @@ -0,0 +1,63 @@ +import { Playback } from 'ontime-types'; + +import { isRestorePoint } from '../restore.parser.js'; +import { RestorePoint } from '../restore.type.js'; + +describe('isRestorePoint()', () => { + it('validates a well defined object', () => { + let restorePoint: RestorePoint = { + playback: Playback.Roll, + selectedEventId: '123', + startedAt: 1, + addedTime: 2, + pausedAt: 3, + firstStart: 1, + }; + expect(isRestorePoint(restorePoint)).toBe(true); + + restorePoint = { + playback: Playback.Roll, + selectedEventId: '123', + startedAt: null, + addedTime: 0, + pausedAt: null, + firstStart: 1, + }; + expect(isRestorePoint(restorePoint)).toBe(true); + }); + + describe('rejects a badly formatted file', () => { + it('with invalid playback value', () => { + const restorePoint = { + playback: 'unknown', + selectedEventId: '123', + startedAt: null, + addedTime: 0, + pausedAt: null, + groupStartAt: 10, + }; + expect(isRestorePoint(restorePoint)).toBe(false); + }); + it('with missing playback value', () => { + const restorePoint = { + selectedEventId: '123', + startedAt: null, + addedTime: 0, + pausedAt: null, + groupStartAt: 10, + }; + expect(isRestorePoint(restorePoint)).toBe(false); + }); + it('with incorrect value', () => { + const restorePoint = { + playback: Playback.Roll, + selectedEventId: '123', + startedAt: 'testing', + addedTime: 0, + pausedAt: null, + groupStartAt: 10, + }; + expect(isRestorePoint(restorePoint)).toBe(false); + }); + }); +}); diff --git a/apps/server/src/services/restore-service/__tests__/restore.service.test.ts b/apps/server/src/services/restore-service/__tests__/restore.service.test.ts new file mode 100644 index 000000000..8316e1d47 --- /dev/null +++ b/apps/server/src/services/restore-service/__tests__/restore.service.test.ts @@ -0,0 +1,124 @@ +/* eslint-disable @typescript-eslint/no-explicit-any */ +import { Playback } from 'ontime-types'; + +import { vi } from 'vitest'; + +import { RestorePoint } from '../restore.type.js'; +import { restoreService } from '../restore.service.js'; + +describe('restoreService', () => { + describe('load()', () => { + it('loads working file with times', async () => { + const expected: RestorePoint = { + playback: Playback.Play, + selectedEventId: 'da5b4', + startedAt: 1234, + addedTime: 5678, + pausedAt: 9087, + firstStart: 1234, + }; + + const mockRead = vi.fn().mockResolvedValue(expected); + + const testLoad = await restoreService.load(mockRead); + expect(testLoad).toStrictEqual(expected); + expect(mockRead).toHaveBeenCalledOnce(); + }); + + it('loads working file without times', async () => { + const expected: RestorePoint = { + playback: Playback.Stop, + selectedEventId: null, + startedAt: null, + addedTime: 0, + pausedAt: null, + firstStart: 1234, + }; + + const mockRead = vi.fn().mockResolvedValue(expected); + + const testLoad = await restoreService.load(mockRead); + expect(testLoad).toStrictEqual(expected); + expect(mockRead).toHaveBeenCalledOnce(); + }); + + it('does not load wrong play state', async () => { + const expected = { + // Missing required field 'firstStart' to make validation fail + playback: 'does-not-exist', + selectedEventId: 'da5b4', + startedAt: 1234, + addedTime: 1234, + pausedAt: 1234, + groupStartAt: 10, + }; + + const mockRead = vi.fn().mockResolvedValue(expected); + + const testLoad = await restoreService.load(mockRead); + // Should return null because isRestorePoint validation fails + expect(testLoad).toBe(null); + expect(mockRead).toHaveBeenCalledOnce(); + }); + + it('returns null when file read fails', async () => { + const mockRead = vi.fn().mockRejectedValue(new Error('File not found')); + + const testLoad = await restoreService.load(mockRead); + expect(testLoad).toBe(null); + expect(mockRead).toHaveBeenCalledOnce(); + }); + }); + + describe('save()', () => { + it('saves data to file', async () => { + const testData: RestorePoint = { + playback: Playback.Play, + selectedEventId: '1234', + startedAt: 1234, + addedTime: 1234, + pausedAt: 1234, + firstStart: 1234, + }; + + const mockWrite = vi.fn().mockResolvedValue(undefined); + + await restoreService.save(testData, mockWrite); + expect(mockWrite).toHaveBeenCalledWith(testData); + }); + + it('handles write failures gracefully', async () => { + const testData: RestorePoint = { + playback: Playback.Pause, + selectedEventId: '5678', + startedAt: 5678, + addedTime: 5678, + pausedAt: 5678, + firstStart: 5678, + }; + + const mockWrite = vi.fn().mockRejectedValue(new Error('Write failed')); + + // Should not throw, and should still call write + await expect(restoreService.save(testData, mockWrite)).resolves.toBeUndefined(); + expect(mockWrite).toHaveBeenCalledWith(testData); + }); + }); + + describe('clear()', () => { + it('clears the restore file', async () => { + const mockWrite = vi.fn().mockResolvedValue(undefined); + + await restoreService.clear(mockWrite); + expect(mockWrite).toHaveBeenCalledWith(null); + }); + + it('handles clear failures gracefully', async () => { + const mockWrite = vi.fn().mockRejectedValue(new Error('Clear failed')); + + // Should not throw + await expect(restoreService.clear(mockWrite)).resolves.toBeUndefined(); + expect(mockWrite).toHaveBeenCalledWith(null); + }); + }); +}); diff --git a/apps/server/src/services/restore-service/restore.parser.ts b/apps/server/src/services/restore-service/restore.parser.ts new file mode 100644 index 000000000..e1f16bb55 --- /dev/null +++ b/apps/server/src/services/restore-service/restore.parser.ts @@ -0,0 +1,53 @@ +import { Playback } from 'ontime-types'; + +import { is } from '../../utils/is.js'; + +import type { RestorePoint } from './restore.type.js'; + +/** + * Utility validates a RestorePoint + */ +export function isRestorePoint(restorePoint: unknown): restorePoint is RestorePoint { + if (!is.object(restorePoint)) { + return false; + } + + if ( + !is.objectWithKeys(restorePoint, [ + 'playback', + 'selectedEventId', + 'startedAt', + 'addedTime', + 'pausedAt', + 'firstStart', + ]) + ) { + return false; + } + + if (!is.string(restorePoint.playback) && !Object.values(Playback).includes(restorePoint.playback as Playback)) { + return false; + } + + if (!is.string(restorePoint.selectedEventId) && restorePoint.selectedEventId !== null) { + return false; + } + + if (!is.number(restorePoint.startedAt) && restorePoint.startedAt !== null) { + return false; + } + + if (!is.number(restorePoint.addedTime)) { + return false; + } + + if (!is.number(restorePoint.pausedAt) && restorePoint.pausedAt !== null) { + return false; + } + + if (!is.number(restorePoint.firstStart) && restorePoint.firstStart !== null) { + return false; + } + + return true; +} diff --git a/apps/server/src/services/restore-service/restore.service.ts b/apps/server/src/services/restore-service/restore.service.ts new file mode 100644 index 000000000..15d647fdf --- /dev/null +++ b/apps/server/src/services/restore-service/restore.service.ts @@ -0,0 +1,106 @@ +import { JSONFile } from 'lowdb/node'; +import { deepEqual } from 'fast-equals'; + +import { publicFiles } from '../../setup/index.js'; + +import { isRestorePoint } from './restore.parser.js'; +import type { RestorePoint } from './restore.type.js'; + +let failedCreateAttempts = 0; +let savedState: RestorePoint | null = null; +let fileRef: JSONFile | null = null; + +/** + * Service manages saving snapshot of application state + * that can then be restored when reopening + */ +export const restoreService = { + save, + load, + clear, +}; + +/** + * Saves a restore point + * @param [writeFn=write] - allows overriding the write function for testing + * @public + */ +async function save(data: RestorePoint, writeFn = write) { + // after three failed attempts, mark the service as unavailable + if (failedCreateAttempts > 3) { + return; + } + + if (deepEqual(data, savedState)) { + return; + } + + try { + await writeFn(data); + savedState = { ...data }; + failedCreateAttempts = 0; + } catch (_error) { + failedCreateAttempts += 1; + } +} + +/** + * Attempts reading a restore point from a given file path + * Returns null if none found, restore point otherwise + * @param [readFn=read] - allows overriding the read function for testing + * @public + */ +async function load(readFn = read): Promise { + try { + const maybeRestorePoint = await readFn(); + if (isRestorePoint(maybeRestorePoint)) { + return maybeRestorePoint; + } + } catch (_error) { + // no need to notify the user + } + return null; +} + +/** + * Clears the restore file + * @param [writeFn=write] - allows overriding the write function for testing + * @public + */ +async function clear(writeFn = write) { + try { + await writeFn(null); + } catch (_error) { + // nothing to do + } +} + +/** + * Initialised file reference + * @private + */ +async function init(): Promise> { + if (fileRef) return fileRef; + + fileRef = new JSONFile(publicFiles.restoreFile); + return fileRef; +} + +/** + * Reads from an intialized file reference + * @private + */ +async function read(): Promise { + const file = await init(); + return file.read(); +} + +/** + * Writes to an intialized file reference + * @throws - if writing fails + * @private + */ +async function write(data: RestorePoint | null) { + const file = await init(); + return file.write(data); +} diff --git a/apps/server/src/services/restore-service/restore.type.ts b/apps/server/src/services/restore-service/restore.type.ts new file mode 100644 index 000000000..018612f46 --- /dev/null +++ b/apps/server/src/services/restore-service/restore.type.ts @@ -0,0 +1,10 @@ +import { Playback, MaybeString, MaybeNumber } from 'ontime-types'; + +export type RestorePoint = { + playback: Playback; + selectedEventId: MaybeString; + startedAt: MaybeNumber; + addedTime: number; + pausedAt: MaybeNumber; + firstStart: MaybeNumber; +}; diff --git a/apps/server/src/services/runtime-service/runtime.service.ts b/apps/server/src/services/runtime-service/runtime.service.ts index 2f49eb824..a5b7b2bb7 100644 --- a/apps/server/src/services/runtime-service/runtime.service.ts +++ b/apps/server/src/services/runtime-service/runtime.service.ts @@ -25,7 +25,8 @@ import { triggerAutomations } from '../../api-data/automation/automation.service import { getCurrentRundown, getEntryWithId, getRundownMetadata } from '../../api-data/rundown/rundown.dao.js'; import { EventTimer } from '../EventTimer.js'; -import { RestorePoint, restoreService } from '../RestoreService.js'; +import type { RestorePoint } from '../restore-service/restore.type.js'; +import { restoreService } from '../restore-service/restore.service.js'; import { skippedOutOfEvent } from '../timerUtils.js'; import { diff --git a/apps/server/src/stores/runtimeState.ts b/apps/server/src/stores/runtimeState.ts index 1b4993f0c..27e6e88a5 100644 --- a/apps/server/src/stores/runtimeState.ts +++ b/apps/server/src/stores/runtimeState.ts @@ -24,7 +24,7 @@ import { } from 'ontime-utils'; import { timeNow } from '../utils/time.js'; -import type { RestorePoint } from '../services/RestoreService.js'; +import type { RestorePoint } from '../services/restore-service/restore.type.js'; import { getCurrent, getExpectedFinish, getRuntimeOffset, getTimerPhase } from '../services/timerUtils.js'; import { loadRoll, normaliseRollStart } from '../services/rollUtils.js'; import { timerConfig } from '../setup/config.js';