From c5c0401ac4b3da59c163a0535c675216358065ff Mon Sep 17 00:00:00 2001 From: Alex Christoffer Rasmussen Date: Sat, 30 Dec 2023 10:03:02 +0100 Subject: [PATCH] refactor: switch out steno for lowdb (#682) * refactor: switch out steno for lowdb --- apps/server/package.json | 1 - apps/server/src/app.ts | 2 +- apps/server/src/services/RestoreService.ts | 58 +++---- .../services/__tests__/RestoreService.test.ts | 20 +-- pnpm-lock.yaml | 141 +++++++++++++++++- 5 files changed, 165 insertions(+), 57 deletions(-) diff --git a/apps/server/package.json b/apps/server/package.json index c8fadb11b..a76bf6d10 100644 --- a/apps/server/package.json +++ b/apps/server/package.json @@ -20,7 +20,6 @@ "ontime-utils": "workspace:*", "passport": "^0.6.0", "passport-local": "~1.0.0", - "steno": "^3.1.0", "ws": "^8.13.0" }, "devDependencies": { diff --git a/apps/server/src/app.ts b/apps/server/src/app.ts index 2a3e3683c..fc17225f8 100644 --- a/apps/server/src/app.ts +++ b/apps/server/src/app.ts @@ -147,7 +147,7 @@ export const startServer = async () => { eventLoader.init(); // load restore point if it exists - const maybeRestorePoint = restoreService.load(); + const maybeRestorePoint = await restoreService.load(); if (maybeRestorePoint) { logger.info(LogOrigin.Server, 'Found resumable state'); diff --git a/apps/server/src/services/RestoreService.ts b/apps/server/src/services/RestoreService.ts index 0edbf47a5..8af01a6ff 100644 --- a/apps/server/src/services/RestoreService.ts +++ b/apps/server/src/services/RestoreService.ts @@ -1,8 +1,6 @@ import { Playback } from 'ontime-types'; -import { readFileSync } from 'fs'; -import { Writer } from 'steno'; - +import { JSONFile } from 'lowdb/node'; import { resolveRestoreFile } from '../setup.js'; export type RestorePoint = { @@ -58,32 +56,24 @@ export function isRestorePoint(obj: unknown): obj is RestorePoint { */ export class RestoreService { private readonly filePath: string | null; - + private readonly file: JSONFile; private lastStore: string | null; - private file: Writer | null; private failedCreateAttempts: number; constructor(filePath: string) { this.filePath = filePath; this.lastStore = null; - this.file = null; + this.file = new JSONFile(this.filePath); this.failedCreateAttempts = 0; } - /** - * Utility, creates a restore file - */ - create() { - this.file = new Writer(this.filePath); - } - /** * Utility, reads from file * @private */ - private read() { - return readFileSync(this.filePath, 'utf-8'); + private async read() { + return this.file.read(); } /** @@ -91,13 +81,8 @@ export class RestoreService { * @throws * @param stringifiedState */ - private async write(stringifiedState: string) { - // Create a file if it doesnt exist - if (!this.file) { - this.create(); - } - // steno is async, and it uses a queue to avoid unnecessary re-writes - await this.file.write(stringifiedState); + private async write(data: RestorePoint) { + await this.file.write(data); } /** @@ -113,10 +98,10 @@ export class RestoreService { const stringifiedStore = JSON.stringify(newState); if (stringifiedStore !== this.lastStore) { try { - await this.write(stringifiedStore); + await this.write(newState); this.lastStore = stringifiedStore; this.failedCreateAttempts = 0; - } catch (_err) { + } catch (_error) { this.failedCreateAttempts += 1; } } @@ -126,34 +111,27 @@ export class RestoreService { * Attempts reading a restore point from a given file path * Returns null if none found, restore point otherwise */ - load(): RestorePoint | null { + async load(): Promise { try { - const data = this.read(); - const maybeRestorePoint = JSON.parse(data); - - if (!isRestorePoint(maybeRestorePoint)) { - return null; + const maybeRestorePoint = await this.read(); + if (isRestorePoint(maybeRestorePoint)) { + return maybeRestorePoint; } - - return maybeRestorePoint; } catch (_error) { // no need to notify the user - return null; } + return null; } /** * Clears the restore file */ async clear() { - if (this.file && this.failedCreateAttempts <= 3) { - try { - await this.file.write(''); - } catch (_error) { - // nothing to do - } + try { + await this.write(null); + } catch (_error) { + // nothing to do } - this.file = undefined; } } diff --git a/apps/server/src/services/__tests__/RestoreService.test.ts b/apps/server/src/services/__tests__/RestoreService.test.ts index 3fc207392..afd62acec 100644 --- a/apps/server/src/services/__tests__/RestoreService.test.ts +++ b/apps/server/src/services/__tests__/RestoreService.test.ts @@ -61,7 +61,7 @@ describe('isRestorePoint()', () => { describe('RestoreService()', () => { describe('load()', () => { - it('loads working file with times', () => { + it('loads working file with times', async () => { const expected = { playback: Playback.Play, selectedEventId: 'da5b4', @@ -71,13 +71,13 @@ describe('RestoreService()', () => { }; const restoreService = new RestoreService('/path/to/restore/file'); - vi.spyOn(restoreService, 'read').mockImplementation(() => JSON.stringify(expected)); + vi.spyOn(restoreService, 'read').mockImplementation(() => expected); - const testLoad = restoreService.load(); + const testLoad = await restoreService.load(); expect(testLoad).toStrictEqual(expected); }); - it('loads working file without times', () => { + it('loads working file without times', async () => { const expected = { playback: Playback.Stop, selectedEventId: null, @@ -87,13 +87,13 @@ describe('RestoreService()', () => { }; const restoreService = new RestoreService('/path/to/restore/file'); - vi.spyOn(restoreService, 'read').mockImplementation(() => JSON.stringify(expected)); + vi.spyOn(restoreService, 'read').mockImplementation(() => expected); - const testLoad = restoreService.load(); + const testLoad = await restoreService.load(); expect(testLoad).toStrictEqual(expected); }); - it('does not load wrong play state', () => { + it('does not load wrong play state', async () => { const expected = { playback: 'does-not-exist', selectedEventId: 'da5b4', @@ -103,9 +103,9 @@ describe('RestoreService()', () => { }; const restoreService = new RestoreService('/path/to/restore/file'); - vi.spyOn(restoreService, 'read').mockImplementation(() => JSON.stringify(expected)); + vi.spyOn(restoreService, 'read').mockImplementation(() => expected); - const testLoad = restoreService.load(); + const testLoad = await restoreService.load(); expect(testLoad).toBe(null); }); }); @@ -123,7 +123,7 @@ describe('RestoreService()', () => { const restoreService = new RestoreService('/path/to/restore/file'); const writeSpy = vi.spyOn(restoreService, 'write').mockImplementation(() => undefined); await restoreService.save(testData); - expect(writeSpy).toHaveBeenCalledWith(JSON.stringify(testData)); + expect(writeSpy).toHaveBeenCalledWith(testData); }); }); }); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 396eb3805..c68757b0f 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -291,9 +291,6 @@ importers: passport-local: specifier: ~1.0.0 version: 1.0.0 - steno: - specifier: ^3.1.0 - version: 3.1.0 ws: specifier: ^8.13.0 version: 8.13.0 @@ -324,7 +321,7 @@ importers: version: 8.53.0 eslint-plugin-prettier: specifier: ^5.0.1 - version: 5.0.1(eslint-config-prettier@9.0.0)(eslint@8.53.0)(prettier@3.0.3) + version: 5.0.1(eslint@8.53.0)(prettier@3.0.3) nodemon: specifier: ^2.0.20 version: 2.0.20 @@ -345,7 +342,7 @@ importers: version: 5.2.2 vitest: specifier: ^1.0.4 - version: 1.0.4(@types/node@18.11.18)(jsdom@21.1.0)(sass@1.57.1) + version: 1.0.4(@types/node@18.11.18) packages/types: devDependencies: @@ -5150,6 +5147,26 @@ packages: synckit: 0.8.5 dev: true + /eslint-plugin-prettier@5.0.1(eslint@8.53.0)(prettier@3.0.3): + resolution: {integrity: sha512-m3u5RnR56asrwV/lDC4GHorlW75DsFfmUcjfCYylTUs85dBRnB7VM6xG8eCMJdeDRnppzmxZVf1GEPJvl1JmNg==} + engines: {node: ^14.18.0 || >=16.0.0} + peerDependencies: + '@types/eslint': '>=8.0.0' + eslint: '>=8.0.0' + eslint-config-prettier: '*' + prettier: '>=3.0.0' + peerDependenciesMeta: + '@types/eslint': + optional: true + eslint-config-prettier: + optional: true + dependencies: + eslint: 8.53.0 + prettier: 3.0.3 + prettier-linter-helpers: 1.0.0 + synckit: 0.8.5 + dev: true + /eslint-plugin-react-hooks@4.6.0(eslint@8.53.0): resolution: {integrity: sha512-oFc7Itz9Qxh2x4gNHStv3BqJq54ExXmfC+a1NjAta66IAN87Wu0R/QArgIS9qKzX3dXKPI9H5crl9QchNMY9+g==} engines: {node: '>=10'} @@ -8844,6 +8861,27 @@ packages: dev: true optional: true + /vite-node@1.0.4(@types/node@18.11.18): + resolution: {integrity: sha512-9xQQtHdsz5Qn8hqbV7UKqkm8YkJhzT/zr41Dmt5N7AlD8hJXw/Z7y0QiD5I8lnTthV9Rvcvi0QW7PI0Fq83ZPg==} + engines: {node: ^18.0.0 || >=20.0.0} + hasBin: true + dependencies: + cac: 6.7.14 + debug: 4.3.4 + pathe: 1.1.1 + picocolors: 1.0.0 + vite: 5.0.10(@types/node@18.11.18) + transitivePeerDependencies: + - '@types/node' + - less + - lightningcss + - sass + - stylus + - sugarss + - supports-color + - terser + dev: true + /vite-node@1.0.4(@types/node@18.11.18)(sass@1.57.1): resolution: {integrity: sha512-9xQQtHdsz5Qn8hqbV7UKqkm8YkJhzT/zr41Dmt5N7AlD8hJXw/Z7y0QiD5I8lnTthV9Rvcvi0QW7PI0Fq83ZPg==} engines: {node: ^18.0.0 || >=20.0.0} @@ -8905,6 +8943,42 @@ packages: - typescript dev: true + /vite@5.0.10(@types/node@18.11.18): + resolution: {integrity: sha512-2P8J7WWgmc355HUMlFrwofacvr98DAjoE52BfdbwQtyLH06XKwaL/FMnmKM2crF0iX4MpmMKoDlNCB1ok7zHCw==} + engines: {node: ^18.0.0 || >=20.0.0} + hasBin: true + peerDependencies: + '@types/node': ^18.0.0 || >=20.0.0 + less: '*' + lightningcss: ^1.21.0 + sass: '*' + stylus: '*' + sugarss: '*' + terser: ^5.4.0 + peerDependenciesMeta: + '@types/node': + optional: true + less: + optional: true + lightningcss: + optional: true + sass: + optional: true + stylus: + optional: true + sugarss: + optional: true + terser: + optional: true + dependencies: + '@types/node': 18.11.18 + esbuild: 0.19.10 + postcss: 8.4.32 + rollup: 4.9.1 + optionalDependencies: + fsevents: 2.3.3 + dev: true + /vite@5.0.10(@types/node@18.11.18)(sass@1.57.1): resolution: {integrity: sha512-2P8J7WWgmc355HUMlFrwofacvr98DAjoE52BfdbwQtyLH06XKwaL/FMnmKM2crF0iX4MpmMKoDlNCB1ok7zHCw==} engines: {node: ^18.0.0 || >=20.0.0} @@ -8942,6 +9016,63 @@ packages: fsevents: 2.3.3 dev: true + /vitest@1.0.4(@types/node@18.11.18): + resolution: {integrity: sha512-s1GQHp/UOeWEo4+aXDOeFBJwFzL6mjycbQwwKWX2QcYfh/7tIerS59hWQ20mxzupTJluA2SdwiBuWwQHH67ckg==} + engines: {node: ^18.0.0 || >=20.0.0} + hasBin: true + peerDependencies: + '@edge-runtime/vm': '*' + '@types/node': ^18.0.0 || >=20.0.0 + '@vitest/browser': ^1.0.0 + '@vitest/ui': ^1.0.0 + happy-dom: '*' + jsdom: '*' + peerDependenciesMeta: + '@edge-runtime/vm': + optional: true + '@types/node': + optional: true + '@vitest/browser': + optional: true + '@vitest/ui': + optional: true + happy-dom: + optional: true + jsdom: + optional: true + dependencies: + '@types/node': 18.11.18 + '@vitest/expect': 1.0.4 + '@vitest/runner': 1.0.4 + '@vitest/snapshot': 1.0.4 + '@vitest/spy': 1.0.4 + '@vitest/utils': 1.0.4 + acorn-walk: 8.3.1 + cac: 6.7.14 + chai: 4.3.10 + debug: 4.3.4 + execa: 8.0.1 + local-pkg: 0.5.0 + magic-string: 0.30.5 + pathe: 1.1.1 + picocolors: 1.0.0 + std-env: 3.6.0 + strip-literal: 1.3.0 + tinybench: 2.5.1 + tinypool: 0.8.1 + vite: 5.0.10(@types/node@18.11.18) + vite-node: 1.0.4(@types/node@18.11.18) + why-is-node-running: 2.2.2 + transitivePeerDependencies: + - less + - lightningcss + - sass + - stylus + - sugarss + - supports-color + - terser + dev: true + /vitest@1.0.4(@types/node@18.11.18)(jsdom@21.1.0)(sass@1.57.1): resolution: {integrity: sha512-s1GQHp/UOeWEo4+aXDOeFBJwFzL6mjycbQwwKWX2QcYfh/7tIerS59hWQ20mxzupTJluA2SdwiBuWwQHH67ckg==} engines: {node: ^18.0.0 || >=20.0.0}