From 20d9df25010a068ef7176187adac780b95084c36 Mon Sep 17 00:00:00 2001 From: Carlos Valente Date: Sun, 23 Jun 2024 22:47:24 +0200 Subject: [PATCH] refactor: escalate errors from parser --- .../src/api-data/excel/excel.service.ts | 5 +- .../services/sheet-service/SheetService.ts | 5 +- apps/server/src/setup/loadDb.ts | 43 ++-- .../server/src/utils/__tests__/parser.test.ts | 161 ++++++------- .../utils/__tests__/parserFunctions.test.ts | 213 ++++++++++++++++-- apps/server/src/utils/parser.ts | 58 +++-- apps/server/src/utils/parserFunctions.ts | 139 ++++++++---- 7 files changed, 440 insertions(+), 184 deletions(-) diff --git a/apps/server/src/api-data/excel/excel.service.ts b/apps/server/src/api-data/excel/excel.service.ts index 929ea6c90..5123c820c 100644 --- a/apps/server/src/api-data/excel/excel.service.ts +++ b/apps/server/src/api-data/excel/excel.service.ts @@ -11,7 +11,7 @@ import { existsSync } from 'fs'; import xlsx from 'node-xlsx'; import { parseExcel } from '../../utils/parser.js'; -import { parseCustomFields, parseRundown } from '../../utils/parserFunctions.js'; +import { parseRundown } from '../../utils/parserFunctions.js'; import { deleteFile } from '../../utils/parserUtils.js'; let excelData: { name: string; data: unknown[][] }[] = []; @@ -42,11 +42,10 @@ export function generateRundownPreview(options: ImportMap): { rundown: OntimeRun const dataFromExcel = parseExcel(data, options); // we run the parsed data through an extra step to ensure the objects shape - const rundown = parseRundown(dataFromExcel); + const { rundown, customFields } = parseRundown(dataFromExcel); if (rundown.length === 0) { throw new Error(`Could not find data to import in the worksheet: ${options.worksheet}`); } - const customFields = parseCustomFields(dataFromExcel); // clear the data excelData = []; diff --git a/apps/server/src/services/sheet-service/SheetService.ts b/apps/server/src/services/sheet-service/SheetService.ts index 0b7950283..870ddae4f 100644 --- a/apps/server/src/services/sheet-service/SheetService.ts +++ b/apps/server/src/services/sheet-service/SheetService.ts @@ -16,7 +16,7 @@ import { ensureDirectory } from '../../utils/fileManagement.js'; import { cellRequestFromEvent, type ClientSecret, getA1Notation, validateClientSecret } from './sheetUtils.js'; import { parseExcel } from '../../utils/parser.js'; import { logger } from '../../classes/Logger.js'; -import { parseCustomFields, parseRundown } from '../../utils/parserFunctions.js'; +import { parseRundown } from '../../utils/parserFunctions.js'; import { getRundown } from '../rundown-service/rundownUtils.js'; const sheetScope = 'https://www.googleapis.com/auth/spreadsheets'; @@ -366,10 +366,9 @@ export async function download( } const dataFromSheet = parseExcel(googleResponse.data.values, options); - const rundown = parseRundown(dataFromSheet); + const { customFields, rundown } = parseRundown(dataFromSheet); if (rundown.length < 1) { throw new Error('Sheet: Could not find data to import in the worksheet'); } - const customFields = parseCustomFields(dataFromSheet); return { rundown, customFields }; } diff --git a/apps/server/src/setup/loadDb.ts b/apps/server/src/setup/loadDb.ts index 4876dbc9b..31199c651 100644 --- a/apps/server/src/setup/loadDb.ts +++ b/apps/server/src/setup/loadDb.ts @@ -1,7 +1,7 @@ import { DatabaseModel } from 'ontime-types'; import { Low } from 'lowdb'; -import { JSONFile } from 'lowdb/node'; +import { JSONFilePreset } from 'lowdb/node'; import { copyFileSync, existsSync } from 'fs'; import { join } from 'path'; @@ -11,6 +11,9 @@ import { dbModel } from '../models/dataModel.js'; import { pathToStartDb, resolveDbDirectory, resolveDbName } from './index.js'; import { parseProjectFile } from '../services/project-service/projectFileUtils.js'; import { parseJson } from '../utils/parser.js'; +import { getErrorMessage } from 'ontime-utils'; +import { appStateService } from '../services/app-state-service/AppStateService.js'; +import { consoleError } from '../utils/console.js'; /** * @description ensures directories exist and populates database @@ -43,36 +46,30 @@ const populateDb = (directory: string, filename: string): string => { return dbPath; }; -/** - * @description parses a json file to the adapter - * It will create an empty file from the model if the parsing fails - */ -const parseDatabase = async (fileToRead: string, adapterToUse: Low) => { - try { - // this will throw if file is not valid - parseProjectFile(fileToRead); - await adapterToUse.read(); - } catch (error) { - adapterToUse.data = dbModel; - } - - return parseJson(adapterToUse.data); -}; - /** * @description loads ontime db */ async function loadDb(directory: string, filename: string) { const dbInDisk = populateDb(directory, filename); - const adapter = new JSONFile(dbInDisk); - const db = new Low(adapter, dbModel); + let newData: DatabaseModel = dbModel; - const data = await parseDatabase(dbInDisk, db); - db.data = data; - await db.write(); + try { + const maybeProjectFile = parseProjectFile(dbInDisk); + const result = parseJson(maybeProjectFile); - return { db, data }; + await appStateService.updateDatabaseConfig(filename); + + newData = result.data; + } catch (error) { + consoleError(`Unable to parse project file: ${getErrorMessage(error)}`); + // we get here if the JSON file is corrupt + } + + const db = await JSONFilePreset(dbInDisk, newData); + db.data = newData; + + return { db, data: newData }; } export let db = {} as Low; diff --git a/apps/server/src/utils/__tests__/parser.test.ts b/apps/server/src/utils/__tests__/parser.test.ts index e459c4619..42db73a8c 100644 --- a/apps/server/src/utils/__tests__/parser.test.ts +++ b/apps/server/src/utils/__tests__/parser.test.ts @@ -22,6 +22,11 @@ import { parseRundown, parseUrlPresets, parseViewSettings } from '../parserFunct import { ImportMap, MILLIS_PER_MINUTE } from 'ontime-utils'; import * as cache from '../../services/rundown-service/rundownCache.js'; +const requiredSettings = { + app: 'ontime', + version: 'any', +}; + describe('test json parser with valid def', () => { const testData: Partial = { rundown: [ @@ -182,19 +187,15 @@ describe('test json parser with valid def', () => { viewSettings: {} as ViewSettings, }; - let parseResponse; - - beforeEach(async () => { - parseResponse = await parseJson(testData); - }); + const { data } = parseJson(testData); it('has 7 events', () => { - const length = parseResponse?.rundown.length; + const length = data.rundown.length; expect(length).toBe(7); }); it('first event is as a match', () => { - const first = parseResponse?.rundown[0]; + const first = data.rundown[0]; const expected = { title: 'Guest Welcoming', type: 'event', @@ -204,7 +205,7 @@ describe('test json parser with valid def', () => { }); it('second event is as a match', () => { - const second = parseResponse?.rundown[1]; + const second = data.rundown[1]; const expected = { title: 'Good Morning', type: 'event', @@ -213,40 +214,36 @@ describe('test json parser with valid def', () => { expect(second).toMatchObject(expected); }); it('third event end action is set as the default value', () => { - const third = parseResponse?.rundown[2]; - expect(third.endAction).toStrictEqual(EndAction.None); + const third = data.rundown[2]; + expect((third as OntimeEvent).endAction).toStrictEqual(EndAction.None); }); it('fourth event timer type is set as the default value', () => { - const fourth = parseResponse?.rundown[3]; - expect(fourth.timerType).toStrictEqual(TimerType.Clock); + const fourth = data.rundown[3]; + expect((fourth as OntimeEvent).timerType).toStrictEqual(TimerType.Clock); }); it('loaded event settings', () => { - const eventTitle = parseResponse?.project?.title; + const eventTitle = data.project.title; expect(eventTitle).toBe('This is a test definition'); }); it('endMessage to exist but be empty', () => { - const endMessage = parseResponse?.viewSettings?.endMessage; + const endMessage = data.viewSettings.endMessage; expect(endMessage).toBeDefined(); expect(endMessage).toBe(''); }); it('settings are for right app and version', () => { - const settings = parseResponse?.settings; + const settings = data.settings; expect(settings.app).toBe('ontime'); expect(settings.version).toEqual(expect.any(String)); }); - - it('missing settings', () => { - const settings = parseResponse?.settings; - expect(settings.osc_port).toBeUndefined(); - }); }); describe('test parser edge cases', () => { - it('stringifies necessary values', async () => { + it('stringifies necessary values', () => { const testData = { + settings: { ...requiredSettings }, rundown: [ { cue: 101, @@ -260,13 +257,14 @@ describe('test parser edge cases', () => { }; // @ts-expect-error -- we know this is wrong, testing imports outside domain - const parseResponse = await parseJson(testData); - expect(typeof (parseResponse.rundown[0] as OntimeEvent).cue).toBe('string'); - expect(typeof (parseResponse.rundown[1] as OntimeEvent).cue).toBe('string'); + const { data } = parseJson(testData); + expect(typeof (data.rundown[0] as OntimeEvent).cue).toBe('string'); + expect(typeof (data.rundown[1] as OntimeEvent).cue).toBe('string'); }); - it('generates missing ids', async () => { + it('generates missing ids', () => { const testData = { + settings: { ...requiredSettings }, rundown: [ { title: 'Test Event', @@ -276,13 +274,14 @@ describe('test parser edge cases', () => { }; // @ts-expect-error -- we know this is wrong, testing imports outside domain - const parseResponse = await parseJson(testData); - expect(parseResponse.rundown[0].id).toBeDefined(); + const { data } = parseJson(testData); + expect(data.rundown[0].id).toBeDefined(); }); - it('detects duplicate Ids', async () => { + it('detects duplicate Ids', () => { console.log = vi.fn(); const testData = { + settings: { ...requiredSettings }, rundown: [ { title: 'Test Event 1', @@ -298,14 +297,15 @@ describe('test parser edge cases', () => { }; //@ts-expect-error -- we know this is wrong, testing imports outside domain - const parseResponse = await parseJson(testData); - expect(console.log).toHaveBeenCalledWith('ERROR: ID collision on import, skipping'); - expect(parseResponse?.rundown.length).toBe(1); + const { data, errors } = parseJson(testData); + expect(data.rundown.length).toBe(1); + expect(errors.length).toBe(7); }); - it('handles incomplete datasets', async () => { + it('handles incomplete datasets', () => { console.log = vi.fn(); const testData = { + settings: { ...requiredSettings }, rundown: [ { title: 'Test Event 1', @@ -319,11 +319,11 @@ describe('test parser edge cases', () => { }; // @ts-expect-error -- we know this is wrong, testing imports outside domain - const parseResponse = await parseJson(testData); - expect(parseResponse?.rundown.length).toBe(0); + const { data } = parseJson(testData); + expect(data.rundown.length).toBe(0); }); - it('skips unknown app and version settings', async () => { + it('skips unknown app and version settings', () => { console.log = vi.fn(); const testData = { settings: { @@ -332,13 +332,12 @@ describe('test parser edge cases', () => { }; // @ts-expect-error -- we know this is wrong, testing imports outside domain - await parseJson(testData); - expect(console.log).toHaveBeenCalledWith('ERROR: unable to parse settings, missing app or version'); + expect(() => parseJson(testData)).toThrow(); }); }); describe('test corrupt data', () => { - it('handles some empty events', async () => { + it('handles some empty events', () => { const emptyEvents = { rundown: [ {}, @@ -376,11 +375,11 @@ describe('test corrupt data', () => { }; // @ts-expect-error -- we know this is wrong, testing imports outside domain - const parsedDef = await parseJson(emptyEvents); - expect(parsedDef.rundown.length).toBe(2); + const { data } = parseJson(emptyEvents); + expect(data.rundown.length).toBe(2); }); - it('handles all empty events', async () => { + it('handles all empty events', () => { const emptyEvents = { rundown: [{}, {}, {}, {}, {}, {}, {}, {}], project: { @@ -401,11 +400,11 @@ describe('test corrupt data', () => { }; // @ts-expect-error -- we know this is wrong, testing imports outside domain - const parsedDef = await parseJson(emptyEvents); - expect(parsedDef.rundown.length).toBe(0); + const { data } = parseJson(emptyEvents); + expect(data.rundown.length).toBe(0); }); - it('handles missing project data', async () => { + it('handles missing project data', () => { const emptyProjectData = { rundown: [{}, {}, {}, {}, {}, {}, {}, {}], project: {}, @@ -419,11 +418,11 @@ describe('test corrupt data', () => { }; // @ts-expect-error -- we know this is wrong, testing imports outside domain - const parsedDef = await parseJson(emptyProjectData); + const { data: parsedDef } = parseJson(emptyProjectData); expect(parsedDef.project).toStrictEqual(dbModel.project); }); - it('handles missing settings', async () => { + it('handles missing settings', () => { const missingSettings = { rundown: [{}, {}, {}, {}, {}, {}, {}, {}], event: {}, @@ -434,16 +433,13 @@ describe('test corrupt data', () => { }; // @ts-expect-error -- we know this is wrong, testing imports outside domain - const parsedDef = await parseJson(missingSettings); - expect(parsedDef.settings).toStrictEqual(dbModel.settings); + const { data } = parseJson(missingSettings); + expect(data.settings).toStrictEqual(dbModel.settings); }); - it('fails with invalid JSON', async () => { - const invalidJSON = 'some random dataset'; - + it('fails with invalid JSON', () => { // @ts-expect-error -- we know this is wrong, testing imports outside domain - const parsedDef = await parseJson(invalidJSON); - expect(parsedDef).toBeNull(); + expect(() => parseJson('some random dataset')).toThrow(); }); }); @@ -485,6 +481,9 @@ describe('test event validator', () => { }; // @ts-expect-error -- we know this is wrong, testing imports outside domain const validated = createEvent(event, 'not-used'); + if (validated === null) { + throw new Error('unexpected value'); + } expect(typeof validated.title).toEqual('string'); expect(typeof validated.note).toEqual('string'); }); @@ -496,6 +495,9 @@ describe('test event validator', () => { }; // @ts-expect-error -- we know this is wrong, testing imports outside domain const validated = createEvent(event); + if (validated === null) { + throw new Error('unexpected value'); + } assertType(validated.timeStart); assertType(validated.timeEnd); assertType(validated.duration); @@ -510,6 +512,9 @@ describe('test event validator', () => { }; // @ts-expect-error -- we know this is wrong, testing imports outside domain const validated = createEvent(event); + if (validated === null) { + throw new Error('unexpected value'); + } expect(typeof validated.title).toEqual('string'); }); }); @@ -588,7 +593,7 @@ describe('test views import', () => { }); describe('test import of v2 datamodel', () => { - it('ignores deprecated fields and generates new ones', async () => { + it('ignores deprecated fields and generates new ones', () => { const v2ProjectFile = { rundown: [ { type: SupportedEvent.Block, title: 'block-title', id: 'block-id' }, @@ -662,7 +667,7 @@ describe('test import of v2 datamodel', () => { }, }; // @ts-expect-error -- we know this is wrong, testing imports outside domain - const parsed = await parseJson(v2ProjectFile); + const { data: parsed, _errors } = parseJson(v2ProjectFile); expect(parsed.rundown.length).toBe(3); expect(parsed.rundown[0]).toMatchObject({ type: SupportedEvent.Block }); expect(parsed.rundown[0]).toEqual( @@ -787,7 +792,7 @@ describe('getCustomFieldData()', () => { }); describe('parseExcel()', () => { - it('parses the example file', async () => { + it('parses the example file', () => { const testdata = [ ['Ontime ┬À Schedule Template'], [], @@ -980,7 +985,7 @@ describe('parseExcel()', () => { expect(parsedData.rundown[1]).toMatchObject(expectedParsedRundown[1]); }); - it('parses a file without custom fields', async () => { + it('parses a file without custom fields', () => { const testdata = [ ['Ontime ┬À Schedule Template'], [], @@ -1293,7 +1298,7 @@ describe('parseExcel()', () => { }; const result = parseExcel(testdata, importMap); expect(result.rundown.length).toBe(2); - expect(result.rundown.at(0).type).toBe(SupportedEvent.Block); + expect((result.rundown.at(0) as OntimeEvent).type).toBe(SupportedEvent.Block); }); it('imports as events if there is no timer type column', () => { @@ -1363,9 +1368,9 @@ describe('parseExcel()', () => { }; const result = parseExcel(testdata, importMap); expect(result.rundown.length).toBe(2); - expect(result.rundown.at(0).type).toBe(SupportedEvent.Event); + expect((result.rundown.at(0) as OntimeEvent).type).toBe(SupportedEvent.Event); expect((result.rundown.at(0) as OntimeEvent).timerType).toBe(TimerType.CountDown); - expect(result.rundown.at(1).type).toBe(SupportedEvent.Event); + expect((result.rundown.at(1) as OntimeEvent).type).toBe(SupportedEvent.Event); expect((result.rundown.at(1) as OntimeEvent).timerType).toBe(TimerType.CountDown); }); @@ -1401,16 +1406,16 @@ describe('parseExcel()', () => { custom: {}, }; const result = parseExcel(testData, importMap); - const rundown = parseRundown(result); + const { rundown } = parseRundown(result); const events = rundown.filter((e) => e.type === SupportedEvent.Event) as OntimeEvent[]; - expect(events.at(0).timeStart).toEqual(16200000); - expect(events.at(1).timeStart).toEqual(35100000); - expect(events.at(2).timeStart).toEqual(59400000); - expect(events.at(3).timeStart).toEqual(78300000); - expect(events.at(4).timeStart).toEqual(16200000); - expect(events.at(5).timeStart).toEqual(35100000); - expect(events.at(6).timeStart).toEqual(59400000); - expect(events.at(7).timeStart).toEqual(78300000); + expect((events.at(0) as OntimeEvent).timeStart).toEqual(16200000); + expect((events.at(1) as OntimeEvent).timeStart).toEqual(35100000); + expect((events.at(2) as OntimeEvent).timeStart).toEqual(59400000); + expect((events.at(3) as OntimeEvent).timeStart).toEqual(78300000); + expect((events.at(4) as OntimeEvent).timeStart).toEqual(16200000); + expect((events.at(5) as OntimeEvent).timeStart).toEqual(35100000); + expect((events.at(6) as OntimeEvent).timeStart).toEqual(59400000); + expect((events.at(7) as OntimeEvent).timeStart).toEqual(78300000); }); it('handle leading and trailing whitespace', () => { @@ -1438,12 +1443,12 @@ describe('parseExcel()', () => { }; const result = parseExcel(testData, importMap); - const rundown = parseRundown(result); + const { rundown } = parseRundown(result); const events = rundown.filter((e) => e.type === SupportedEvent.Event) as OntimeEvent[]; - expect(events.at(0).timeStart).toEqual(16200000); //<--leading white space in MAP - expect(events.at(0).timeEnd).toEqual(16200000); //<--trailing white space in MAP - expect(events.at(0).title).toEqual('A song from the hearth'); //<--leading white space in Excel data - expect(events.at(0).colour).toEqual('#F00'); //<--trailing white space in Excel data + expect((events.at(0) as OntimeEvent).timeStart).toEqual(16200000); //<--leading white space in MAP + expect((events.at(0) as OntimeEvent).timeEnd).toEqual(16200000); //<--trailing white space in MAP + expect((events.at(0) as OntimeEvent).title).toEqual('A song from the hearth'); //<--leading white space in Excel data + expect((events.at(0) as OntimeEvent).colour).toEqual('#F00'); //<--trailing white space in Excel data }); it('link start', () => { @@ -1490,9 +1495,9 @@ describe('parseExcel()', () => { }; const result = parseExcel(testData, importMap); - const initialRundown = parseRundown(result); + const parseResult = parseRundown(result); - cache.init(initialRundown, {}); + cache.init(parseResult.rundown, parseResult.customFields); const { rundown, order } = cache.get(); const firstId = order.at(0); // A @@ -1502,6 +1507,10 @@ describe('parseExcel()', () => { const fifhtId = order.at(4); // Block const sixthId = order.at(5); // G + if (!firstId || !secondId || !thirdId || !fourthId || !fifhtId || !sixthId) { + throw new Error('Unexpected value'); + } + expect((rundown[firstId] as OntimeEvent).timeStart).toEqual(16200000); expect((rundown[secondId] as OntimeEvent).timeStart).toEqual((rundown[firstId] as OntimeEvent).timeEnd); diff --git a/apps/server/src/utils/__tests__/parserFunctions.test.ts b/apps/server/src/utils/__tests__/parserFunctions.test.ts index d270a95e9..a2515c1d6 100644 --- a/apps/server/src/utils/__tests__/parserFunctions.test.ts +++ b/apps/server/src/utils/__tests__/parserFunctions.test.ts @@ -2,27 +2,205 @@ import { CustomFields, DatabaseModel, EndAction, + HttpSettings, HttpSubscription, + OSCSettings, OntimeEvent, OntimeRundown, OscSubscription, + Settings, SupportedEvent, TimeStrategy, TimerType, + URLPreset, } from 'ontime-types'; + import { + parseCustomFields, + parseHttp, + parseOsc, + parseProject, parseRundown, + parseSettings, + parseUrlPresets, + parseViewSettings, sanitiseCustomFields, sanitiseHttpSubscriptions, sanitiseOscSubscriptions, } from '../parserFunctions.js'; -describe('sanitiseOscSubscriptions()', () => { - it('returns an empty array if not an array', () => { - expect(sanitiseOscSubscriptions(undefined)).toEqual([]); +describe('parseRundown()', () => { + it('returns an empty array if no rundown is given', () => { + const errorEmitter = vi.fn(); + const result = parseRundown({}, errorEmitter); + expect(result.rundown).toEqual([]); + expect(result.customFields).toEqual({}); + expect(errorEmitter).toHaveBeenCalledTimes(2); + }); + + it('parses data, skipping invalid results', () => { + const errorEmitter = vi.fn(); + const rundown = [ + { id: '1', type: SupportedEvent.Event, title: 'test', skip: false }, // OK + { id: '1', type: SupportedEvent.Block, title: 'test 2', skip: false }, // duplicate ID + {}, // no data + { id: '2', title: 'test 2', skip: false }, // no type + ] as OntimeRundown; + const { rundown: parsedRundown } = parseRundown({ rundown, customFields: {} }, errorEmitter); + expect(parsedRundown.length).toEqual(1); + expect(parsedRundown.at(0)).toMatchObject({ id: '1', type: SupportedEvent.Event, title: 'test', skip: false }); + expect(errorEmitter).toHaveBeenCalled(); + }); +}); + +describe('parseProject()', () => { + it('returns an a base model if nothing is given', () => { + const errorEmitter = vi.fn(); + const result = parseProject({}, errorEmitter); + expect(result).toBeTypeOf('object'); + expect(errorEmitter).toHaveBeenCalledOnce(); + }); +}); + +describe('parseSettings()', () => { + it('throws if settings object does not exist', () => { + expect(() => parseSettings({})).toThrow(); + }); + + it('returns an a base model as long as we have the app and version', () => { + const minimalSettings = { app: 'ontime', version: '1' } as Settings; + const result = parseSettings({ settings: minimalSettings }); + expect(result).toBeTypeOf('object'); + }); +}); + +describe('parseViewSettings()', () => { + it('returns an a base model if nothing is given', () => { + const errorEmitter = vi.fn(); + const result = parseViewSettings({}, errorEmitter); + expect(result).toBeTypeOf('object'); + expect(errorEmitter).toHaveBeenCalledOnce(); + }); +}); + +describe('parseOsc()', () => { + it('returns an a base model if nothing is given', () => { + const errorEmitter = vi.fn(); + const result = parseOsc({}, errorEmitter); + expect(result).toBeTypeOf('object'); + expect(errorEmitter).toHaveBeenCalledOnce(); + }); + + it('parses data, skipping invalid results', () => { + const errorEmitter = vi.fn(); + const osc = { + subscriptions: [ + { id: '1', cycle: 'onLoad', address: '/test', payload: 'test', enabled: true }, // OK + {}, // no data + { id: '2', cycle: 'onStart', payload: 'test', enabled: true }, // no address + ], + } as OSCSettings; + const result = parseOsc({ osc }, errorEmitter); + expect(result.subscriptions.length).toEqual(1); + expect(result.subscriptions.at(0)).toMatchObject({ + id: '1', + cycle: 'onLoad', + address: '/test', + payload: 'test', + enabled: true, + }); + expect(errorEmitter).toHaveBeenCalled(); + }); +}); + +describe('parseHttp()', () => { + it('returns an a base model if nothing is given', () => { + const errorEmitter = vi.fn(); + const result = parseHttp({}, errorEmitter); + expect(result).toBeTypeOf('object'); + expect(errorEmitter).toHaveBeenCalledOnce(); + }); + + it('parses data, skipping invalid results', () => { + const errorEmitter = vi.fn(); + const http = { + subscriptions: [ + { id: '1', cycle: 'onLoad', message: 'http://', enabled: true }, // OK + {}, // no data + { id: '2', cycle: 'onStart', enabled: true }, // no message + { id: '3', cycle: 'onLoad', message: '/test', enabled: true }, // doesnt start with http + ], + } as HttpSettings; + const result = parseHttp({ http }, errorEmitter); + expect(result.subscriptions.length).toEqual(1); + expect(result.subscriptions.at(0)).toMatchObject({ + id: '1', + cycle: 'onLoad', + message: 'http://', + enabled: true, + }); + expect(errorEmitter).toHaveBeenCalled(); + }); +}); + +describe('parseUrlPresets()', () => { + it('returns an a base model if nothing is given', () => { + const errorEmitter = vi.fn(); + const result = parseUrlPresets({}, errorEmitter); + expect(result).toBeTypeOf('object'); + expect(errorEmitter).toHaveBeenCalledOnce(); + }); + + it('parses data, skipping invalid results', () => { + const errorEmitter = vi.fn(); + const urlPresets = [{ enabled: true, alias: 'alias', pathAndParams: 'ss' }] as URLPreset[]; + const result = parseUrlPresets({ urlPresets }, errorEmitter); + expect(result.length).toEqual(1); + expect(result.at(0)).toMatchObject({ + enabled: true, + alias: 'alias', + pathAndParams: 'ss', + }); + expect(errorEmitter).not.toHaveBeenCalled(); + }); +}); + +describe('parseCustomFields()', () => { + it('returns an a base model if nothing is given', () => { + const errorEmitter = vi.fn(); + const result = parseCustomFields({}, errorEmitter); + expect(result).toBeTypeOf('object'); + expect(errorEmitter).toHaveBeenCalledOnce(); + }); + + it('parses data, skipping invalid results', () => { + const errorEmitter = vi.fn(); // @ts-expect-error -- data is external, we check bad types - expect(sanitiseOscSubscriptions({})).toEqual([]); - expect(sanitiseOscSubscriptions(null)).toEqual([]); + const customFields = { + 1: { label: 'test', type: 'string', colour: 'red' }, // ok + 2: { label: 'test', type: 'string' }, // duplicate label + 3: { label: '', type: 'string' }, // missing colour + 4: { type: 'string', colour: '' }, // missing label + } as CustomFields; + + const result = parseCustomFields({ customFields }, errorEmitter); + expect(result).toMatchObject({ + test: { + label: 'test', + type: 'string', + colour: 'red', + }, + }); + expect(errorEmitter).toHaveBeenCalled(); + }); +}); + +describe('sanitiseOscSubscriptions()', () => { + it('throws if not an array an empty array if not an array', () => { + expect(() => sanitiseOscSubscriptions(undefined)).toThrow(); + // @ts-expect-error -- data is external, we check bad types + expect(() => sanitiseOscSubscriptions({})).toThrow(); + expect(() => sanitiseOscSubscriptions(null)).toThrow(); }); it('returns an array of valid entries', () => { @@ -49,18 +227,18 @@ describe('sanitiseOscSubscriptions()', () => { { id: '4', cycle: 'onStop', enabled: false }, { id: '5', cycle: 'onUpdate', payload: 'test' }, { id: '6', cycle: 'onFinish', payload: 'test', enabled: 'true' }, - ]; - const sanitationResult = sanitiseOscSubscriptions(oscSubscriptions as OscSubscription[]); + ] as OscSubscription[]; + const sanitationResult = sanitiseOscSubscriptions(oscSubscriptions); expect(sanitationResult.length).toBe(0); }); }); describe('sanitiseHttpSubscriptions()', () => { - it('returns an empty array if not an array', () => { - expect(sanitiseHttpSubscriptions(undefined)).toEqual([]); + it('throws if the data is unexpected', () => { + expect(() => sanitiseHttpSubscriptions(undefined)).toThrow(); // @ts-expect-error -- data is external, we check bad types - expect(sanitiseHttpSubscriptions({})).toEqual([]); - expect(sanitiseHttpSubscriptions(null)).toEqual([]); + expect(() => sanitiseHttpSubscriptions({})).toThrow(); + expect(() => sanitiseHttpSubscriptions(null)).toThrow(); }); it('returns an array of valid entries', () => { @@ -148,7 +326,7 @@ describe('sanitiseCustomFields()', () => { expect(sanitationResult).toStrictEqual(expectedCustomFields); }); - it('enforece name cohesion', () => { + it('enforce name cohesion', () => { const customFields: CustomFields = { test: { label: 'New Name', type: 'string', colour: 'red' }, }; @@ -214,6 +392,7 @@ describe('parseRundown() linking', () => { skip: false, } as OntimeEvent, ], + customFields: {}, }; const expected: OntimeRundown = [ @@ -221,10 +400,10 @@ describe('parseRundown() linking', () => { { ...blankEvent, id: '2', cue: '1', linkStart: '1' }, ]; const result = parseRundown(data); - expect(result).toEqual(expected); + expect(result.rundown).toEqual(expected); }); - it('returns unlinkd if no previous', () => { + it('returns unlinked if no previous', () => { const data: Partial = { rundown: [ { @@ -234,11 +413,12 @@ describe('parseRundown() linking', () => { skip: false, } as OntimeEvent, ], + customFields: {}, }; const expected: OntimeRundown = [{ ...blankEvent, id: '2', cue: '0' }]; const result = parseRundown(data); - expect(result).toEqual(expected); + expect(result.rundown).toEqual(expected); }); it('returns linked events past blocks and delays', () => { @@ -272,6 +452,7 @@ describe('parseRundown() linking', () => { skip: false, } as OntimeEvent, ], + customFields: {}, }; const expected: OntimeRundown = [ @@ -282,6 +463,6 @@ describe('parseRundown() linking', () => { { ...blankEvent, id: '3', cue: '2', linkStart: '2' }, ]; const result = parseRundown(data); - expect(result).toEqual(expected); + expect(result.rundown).toEqual(expected); }); }); diff --git a/apps/server/src/utils/parser.ts b/apps/server/src/utils/parser.ts index 95e186d2d..112554c5c 100644 --- a/apps/server/src/utils/parser.ts +++ b/apps/server/src/utils/parser.ts @@ -12,6 +12,7 @@ import { CustomFields, DatabaseModel, EventCustomFields, + LogOrigin, OntimeBlock, OntimeEvent, OntimeRundown, @@ -20,11 +21,10 @@ import { TimeStrategy, } from 'ontime-types'; +import { logger } from '../classes/Logger.js'; import { event as eventDef } from '../models/eventsDefinition.js'; -import { dbModel } from '../models/dataModel.js'; import { makeString } from './parserUtils.js'; import { - parseCustomFields, parseHttp, parseOsc, parseProject, @@ -282,40 +282,48 @@ export const parseExcel = (excelData: unknown[][], options?: Partial) }; }; +export type ParsingError = { + context: string; + message: string; +}; + /** * @description JSON parser function for ontime project file * @param {object} jsonData - project file to be parsed * @returns {object} - parsed object */ -export const parseJson = async (jsonData: Partial): Promise => { +export function parseJson(jsonData: Partial): { data: DatabaseModel; errors: ParsingError[] } { if (!jsonData || typeof jsonData !== 'object') { - return null; + throw new Error('Invalid JSON data'); } - let settings; + // we need to parse settings first to make sure the data is ours + // this may throw + const settings = parseSettings(jsonData); - // check settings first to make sure we can parse it - try { - settings = parseSettings(jsonData); - } catch (error) { - // if we cant parse, return an empty project - console.log('ERROR: unable to parse settings, missing app or version'); - return dbModel; - } - - const returnData: DatabaseModel = { - rundown: parseRundown(jsonData), - project: parseProject(jsonData), - settings, - viewSettings: parseViewSettings(jsonData), - urlPresets: parseUrlPresets(jsonData), - customFields: parseCustomFields(jsonData), - osc: parseOsc(jsonData), - http: parseHttp(jsonData), + const errors: ParsingError[] = []; + const makeEmitError = (context: string) => (message: string) => { + logger.error(LogOrigin.Server, `Error parsing ${context}: ${message}`); + errors.push({ context, message }); }; - return returnData; -}; + // we need to parse the custom fields first so they can be used in validating events + // TODO: can we improve the readability of the error? + const { rundown, customFields } = parseRundown(jsonData, makeEmitError('Rundown')); + + const data: DatabaseModel = { + rundown, + project: parseProject(jsonData, makeEmitError('Project')), + settings, + viewSettings: parseViewSettings(jsonData, makeEmitError('View Settings')), + urlPresets: parseUrlPresets(jsonData, makeEmitError('URL Presets')), + customFields, + osc: parseOsc(jsonData, makeEmitError('OSC')), + http: parseHttp(jsonData, makeEmitError('HTTP')), + }; + + return { data, errors }; +} /** * Function infers strategy for a patch with only partial timer data diff --git a/apps/server/src/utils/parserFunctions.ts b/apps/server/src/utils/parserFunctions.ts index c41267cf6..df4091e90 100644 --- a/apps/server/src/utils/parserFunctions.ts +++ b/apps/server/src/utils/parserFunctions.ts @@ -1,4 +1,5 @@ import { + CustomField, CustomFields, DatabaseModel, HttpSettings, @@ -18,18 +19,27 @@ import { isOntimeDelay, isOntimeEvent, } from 'ontime-types'; -import { generateId, getLastEvent } from 'ontime-utils'; +import { generateId, getErrorMessage, getLastEvent } from 'ontime-utils'; import { dbModel } from '../models/dataModel.js'; import { block as blockDef, delay as delayDef } from '../models/eventsDefinition.js'; import { createEvent } from './parser.js'; +type ErrorEmitter = (message: string) => void; + /** * Parse rundown array of an entry */ -export const parseRundown = (data: Partial): OntimeRundown => { +export function parseRundown( + data: Partial, + emitError?: ErrorEmitter, +): { customFields: CustomFields; rundown: OntimeRundown } { + // check custom fields first + const parsedCustomFields = parseCustomFields(data, emitError); + if (!data.rundown) { - return []; + emitError?.('No data found to import'); + return { customFields: parsedCustomFields, rundown: [] }; } console.log('Found rundown, importing...'); @@ -40,7 +50,7 @@ export const parseRundown = (data: Partial): OntimeRundown => { for (const event of data.rundown) { if (ids.includes(event.id)) { - console.log('ERROR: ID collision on import, skipping'); + emitError?.('ID collision on event import, skipping'); continue; } @@ -52,19 +62,29 @@ export const parseRundown = (data: Partial): OntimeRundown => { const prevId = getLastEvent(rundown).lastEvent?.id ?? null; event.linkStart = prevId; } + newEvent = createEvent(event, eventIndex.toString()); // skip if event is invalid if (newEvent == null) { + emitError?.('Skipping event without payload'); continue; } + // for every field in custom, check that a key exists in customfields + for (const field in newEvent.custom) { + if (!Object.hasOwn(parsedCustomFields, field)) { + emitError?.(`Custom field ${field} not found`); + delete newEvent.custom[field]; + } + } + eventIndex += 1; } else if (isOntimeDelay(event)) { newEvent = { ...delayDef, duration: event.duration, id }; } else if (isOntimeBlock(event)) { newEvent = { ...blockDef, title: event.title, id }; } else { - console.log('ERROR: unknown event type, skipping'); + emitError?.('Unknown event type, skipping'); continue; } @@ -75,14 +95,15 @@ export const parseRundown = (data: Partial): OntimeRundown => { } console.log(`Uploaded rundown with ${rundown.length} entries`); - return rundown; -}; + return { customFields: parsedCustomFields, rundown }; +} /** * Parse event portion of an entry */ -export const parseProject = (data: Partial): ProjectData => { +export function parseProject(data: Partial, emitError?: ErrorEmitter): ProjectData { if (!data.project) { + emitError?.('No data found to import'); return { ...dbModel.project }; } @@ -96,18 +117,14 @@ export const parseProject = (data: Partial): ProjectData => { backstageUrl: data.project.backstageUrl ?? dbModel.project.backstageUrl, backstageInfo: data.project.backstageInfo ?? dbModel.project.backstageInfo, }; -}; +} /** * Parse settings portion of an entry */ -export const parseSettings = (data: Partial): Settings => { - if (!data.settings) { - return { ...dbModel.settings }; - } - +export function parseSettings(data: Partial): Settings { // skip if file definition is missing - if (data.settings?.app !== 'ontime' || data.settings?.version == null) { + if (!data.settings || data.settings?.app !== 'ontime' || data.settings?.version == null) { throw new Error('ERROR: unable to parse settings, missing app or version'); } @@ -122,13 +139,14 @@ export const parseSettings = (data: Partial): Settings => { timeFormat: data.settings.timeFormat ?? '24', language: data.settings.language ?? 'en', }; -}; +} /** * Parse view settings portion of an entry */ -export const parseViewSettings = (data: Partial): ViewSettings => { +export function parseViewSettings(data: Partial, emitError?: ErrorEmitter): ViewSettings { if (!data.viewSettings) { + emitError?.('No data found to import'); return { ...dbModel.viewSettings }; } @@ -142,14 +160,14 @@ export const parseViewSettings = (data: Partial): ViewSettings => overrideStyles: data.viewSettings.overrideStyles ?? dbModel.viewSettings.overrideStyles, warningColor: data.viewSettings.warningColor ?? dbModel.viewSettings.warningColor, }; -}; +} /** * Sanitises an OSC Subscriptions array */ export function sanitiseOscSubscriptions(subscriptions?: OscSubscription[]): OscSubscription[] { if (!Array.isArray(subscriptions)) { - return []; + throw new Error('ERROR: invalid OSC subscriptions'); } return subscriptions.filter( @@ -165,28 +183,41 @@ export function sanitiseOscSubscriptions(subscriptions?: OscSubscription[]): Osc /** * Parse osc portion of an entry */ -export const parseOsc = (data: Partial): OSCSettings => { +export function parseOsc(data: Partial, emitError?: ErrorEmitter): OSCSettings { if (!data.osc) { + emitError?.('No data found to import'); return { ...dbModel.osc }; } + console.log('Found OSC settings, importing...'); + let newSubscriptions: OscSubscription[] = []; + try { + newSubscriptions = sanitiseOscSubscriptions(data.osc.subscriptions); + } catch (error) { + emitError?.(getErrorMessage(error)); + } + + if (newSubscriptions.length !== data.osc.subscriptions.length) { + emitError?.('Skipped invalid subscriptions'); + } + return { portIn: data.osc.portIn ?? dbModel.osc.portIn, portOut: data.osc.portOut ?? dbModel.osc.portOut, targetIP: data.osc.targetIP ?? dbModel.osc.targetIP, enabledIn: data.osc.enabledIn ?? dbModel.osc.enabledIn, enabledOut: data.osc.enabledOut ?? dbModel.osc.enabledOut, - subscriptions: sanitiseOscSubscriptions(data.osc.subscriptions), + subscriptions: newSubscriptions, }; -}; +} /** * Sanitises an HTTP Subscriptions array */ export function sanitiseHttpSubscriptions(subscriptions?: HttpSubscription[]): HttpSubscription[] { if (!Array.isArray(subscriptions)) { - return []; + throw new Error('ERROR: invalid HTTP subscriptions'); } return subscriptions.filter( @@ -202,24 +233,37 @@ export function sanitiseHttpSubscriptions(subscriptions?: HttpSubscription[]): H /** * Parse Http portion of an entry */ -export const parseHttp = (data: Partial): HttpSettings => { +export function parseHttp(data: Partial, emitError?: ErrorEmitter): HttpSettings { if (!data.http) { + emitError?.('No data found to import'); return { ...dbModel.http }; } console.log('Found HTTP settings, importing...'); + let newSubscriptions: HttpSubscription[] = []; + try { + newSubscriptions = sanitiseHttpSubscriptions(data.http.subscriptions); + } catch (error) { + emitError?.(getErrorMessage(error)); + } + + if (newSubscriptions.length !== data.http?.subscriptions.length) { + emitError?.('Skipped invalid subscriptions'); + } + return { enabledOut: data.http.enabledOut ?? dbModel.http.enabledOut, - subscriptions: sanitiseHttpSubscriptions(data.http.subscriptions), + subscriptions: newSubscriptions, }; -}; +} /** * Parse URL preset portion of an entry */ -export const parseUrlPresets = (data: Partial): URLPreset[] => { +export function parseUrlPresets(data: Partial, emitError?: ErrorEmitter): URLPreset[] { if (!data.urlPresets) { + emitError?.('No data found to import'); return []; } @@ -239,31 +283,39 @@ export const parseUrlPresets = (data: Partial): URLPreset[] => { console.log(`Uploaded ${newPresets.length} preset(s)`); return newPresets; -}; +} /** * Parse customFields entry */ -export const parseCustomFields = (data: Partial): CustomFields => { +export function parseCustomFields(data: Partial, emitError?: ErrorEmitter): CustomFields { if (typeof data.customFields !== 'object') { - return { ...dbModel.customFields }; + emitError?.('No data found to import'); + return {}; } console.log('Found Custom Fields, importing...'); - return sanitiseCustomFields(data.customFields); -}; + const customFields = sanitiseCustomFields(data.customFields); + if (Object.keys(customFields).length !== Object.keys(data.customFields).length) { + emitError?.('Skipped invalid custom fields'); + } + return customFields; +} -export const sanitiseCustomFields = (data: object): CustomFields => { +export function sanitiseCustomFields(data: object): CustomFields { const newCustomFields: CustomFields = {}; - for (const fieldLabel in data) { - const field = data[fieldLabel]; - if (!('label' in field) || field.label === '' || !('colour' in field) || typeof field.colour != 'string') { - console.log('ERROR: missing required field, skipping'); + for (const [_key, field] of Object.entries(data)) { + if (!isValidField(field)) { continue; } + // make a new key to avoid mismatches const key = field.label.toLowerCase(); + if (key in newCustomFields) { + continue; + } + newCustomFields[key] = { type: 'string', colour: field.colour, @@ -271,5 +323,16 @@ export const sanitiseCustomFields = (data: object): CustomFields => { }; } + function isValidField(data: unknown): data is CustomField { + return ( + typeof data === 'object' && + data !== null && + 'label' in data && + data.label !== '' && + 'colour' in data && + typeof data.colour === 'string' + ); + } + return newCustomFields; -}; +}