From 971b9c0c29efa352ca8ab9784fd0ad94456b5282 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jordi=20Guti=C3=A9rrez=20Hermoso?= Date: Wed, 17 Jul 2024 13:39:03 -0400 Subject: [PATCH 1/2] config: replace fse read functions with sync variants I need to be able to read the config at module load time, which makes async difficult if not impossible. This will make read config operations synchronous, which is fine. The file is tiny and seldom read. --- app/server/lib/config.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/server/lib/config.ts b/app/server/lib/config.ts index 067198f77e..3bc495fab9 100644 --- a/app/server/lib/config.ts +++ b/app/server/lib/config.ts @@ -2,9 +2,9 @@ import * as fse from "fs-extra"; // Export dependencies for stubbing in tests. export const Deps = { - readFile: fse.readFile, + readFile: fse.readFileSync, writeFile: fse.writeFile, - pathExists: fse.pathExists, + pathExists: fse.pathExistsSync, }; /** From 5f234153172075e652749658b2de3c1fc9430080 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jordi=20Guti=C3=A9rrez=20Hermoso?= Date: Wed, 17 Jul 2024 17:46:28 -0400 Subject: [PATCH 2/2] config: remove all async/await around config read functions Now that reading is synchronous, there's no need to have any more async/await in regards to the those config functions. --- app/server/lib/config.ts | 10 +++++----- app/server/lib/configCore.ts | 4 ++-- app/server/mergedServerMain.ts | 2 +- stubs/app/server/lib/globalConfig.ts | 4 ++-- test/server/lib/config.ts | 12 ++++++------ test/server/lib/configCore.ts | 6 +++--- test/server/lib/configCoreFileFormats.ts | 2 +- 7 files changed, 20 insertions(+), 20 deletions(-) diff --git a/app/server/lib/config.ts b/app/server/lib/config.ts index 3bc495fab9..490cddcf0b 100644 --- a/app/server/lib/config.ts +++ b/app/server/lib/config.ts @@ -56,15 +56,15 @@ export class FileConfig { * @param validator - Validates the contents are in the correct format, and converts to the correct type. * Should throw an error or return null if not valid. */ - public static async create( + public static create( configPath: string, validator: FileContentsValidator - ): Promise> { + ): FileConfig { // Start with empty object, as it can be upgraded to a full config. let rawFileContents: any = {}; - if (await Deps.pathExists(configPath)) { - rawFileContents = JSON.parse(await Deps.readFile(configPath, 'utf8')); + if (Deps.pathExists(configPath)) { + rawFileContents = JSON.parse(Deps.readFile(configPath, 'utf8')); } let fileContents = null; @@ -97,7 +97,7 @@ export class FileConfig { await this.persistToDisk(); } - public async persistToDisk(): Promise { + public async persistToDisk() { await Deps.writeFile(this._filePath, JSON.stringify(this._rawConfig, null, 2)); } } diff --git a/app/server/lib/configCore.ts b/app/server/lib/configCore.ts index 884e2cf742..58ddf627c7 100644 --- a/app/server/lib/configCore.ts +++ b/app/server/lib/configCore.ts @@ -15,8 +15,8 @@ export interface IGristCoreConfig { edition: IWritableConfigValue; } -export async function loadGristCoreConfigFile(configPath?: string): Promise { - const fileConfig = configPath ? await FileConfig.create(configPath, convertToCoreFileContents) : undefined; +export function loadGristCoreConfigFile(configPath?: string): IGristCoreConfig { + const fileConfig = configPath ? FileConfig.create(configPath, convertToCoreFileContents) : undefined; return loadGristCoreConfig(fileConfig); } diff --git a/app/server/mergedServerMain.ts b/app/server/mergedServerMain.ts index f4e4a4a631..466eddafca 100644 --- a/app/server/mergedServerMain.ts +++ b/app/server/mergedServerMain.ts @@ -71,7 +71,7 @@ export async function main(port: number, serverTypes: ServerType[], const includeStatic = serverTypes.includes("static"); const includeApp = serverTypes.includes("app"); - options.settings ??= await getGlobalConfig(); + options.settings ??= getGlobalConfig(); const server = new FlexServer(port, `server(${serverTypes.join(",")})`, options); diff --git a/stubs/app/server/lib/globalConfig.ts b/stubs/app/server/lib/globalConfig.ts index 0c62d85546..9b9c6d4c96 100644 --- a/stubs/app/server/lib/globalConfig.ts +++ b/stubs/app/server/lib/globalConfig.ts @@ -9,10 +9,10 @@ let cachedGlobalConfig: IGristCoreConfig | undefined = undefined; /** * Retrieves the cached grist config, or loads it from the default global path. */ -export async function getGlobalConfig(): Promise { +export function getGlobalConfig(): IGristCoreConfig { if (!cachedGlobalConfig) { log.info(`Loading config file from ${globalConfigPath}`); - cachedGlobalConfig = await loadGristCoreConfigFile(globalConfigPath); + cachedGlobalConfig = loadGristCoreConfigFile(globalConfigPath); } return cachedGlobalConfig; diff --git a/test/server/lib/config.ts b/test/server/lib/config.ts index 711e75ec8c..1c7fe23776 100644 --- a/test/server/lib/config.ts +++ b/test/server/lib/config.ts @@ -18,7 +18,7 @@ describe('FileConfig', () => { const useFakeConfigFile = (contents: string) => { const fakeFile = { contents }; sinon.replace(Deps, 'pathExists', sinon.fake.resolves(true)); - sinon.replace(Deps, 'readFile', sinon.fake((path, encoding: string) => Promise.resolve(fakeFile.contents)) as any); + sinon.replace(Deps, 'readFile', sinon.fake((path, encoding: string) => fakeFile.contents) as any); sinon.replace(Deps, 'writeFile', sinon.fake((path, newContents) => { fakeFile.contents = newContents; return Promise.resolve(); @@ -31,17 +31,17 @@ describe('FileConfig', () => { sinon.restore(); }); - it('throws an error from create if the validator does not return a value', async () => { + it('throws an error from create if the validator does not return a value', () => { useFakeConfigFile(testFileContentsJSON); const validator = () => null; - await assert.isRejected(FileConfig.create("anypath.json", validator)); + assert.throws(() => FileConfig.create("anypath.json", validator)); }); it('persists changes when values are assigned', async () => { const fakeFile = useFakeConfigFile(testFileContentsJSON); // Don't validate - this is guaranteed to be valid above. const validator = (input: any) => input as TestFileContents; - const fileConfig = await FileConfig.create("anypath.json", validator); + const fileConfig = FileConfig.create("anypath.json", validator); await fileConfig.set("myNum", 999); assert.equal(fileConfig.get("myNum"), 999); @@ -58,7 +58,7 @@ describe('FileConfig', () => { const fakeFile = useFakeConfigFile(JSON.stringify(configWithExtraProperties)); // It's entirely possible the validator can damage the extra properties, but that's not in scope for this test. const validator = (input: any) => input as TestFileContents; - const fileConfig = await FileConfig.create("anypath.json", validator); + const fileConfig = FileConfig.create("anypath.json", validator); // Triggering a write to the file await fileConfig.set("myNum", 999); await fileConfig.set("myStr", "Something"); @@ -94,7 +94,7 @@ describe('createConfigValue', () => { assert.equal(accessors.get(), 2); }); - it('initialises with the persistent value if available', async () => { + it('initialises with the persistent value if available', () => { const accessors = makeInMemoryAccessors(22); const configValue = createConfigValue(1, accessors); assert.equal(configValue.get(), 22); diff --git a/test/server/lib/configCore.ts b/test/server/lib/configCore.ts index 3d82ec682b..50259e764d 100644 --- a/test/server/lib/configCore.ts +++ b/test/server/lib/configCore.ts @@ -15,12 +15,12 @@ describe('loadGristCoreConfig', () => { }); it('will function correctly when no config file is present', async () => { - sinon.replace(Deps, 'pathExists', sinon.fake.resolves(false)); - sinon.replace(Deps, 'readFile', sinon.fake.resolves("")); + sinon.replace(Deps, 'pathExists', sinon.fake.returns(false)); + sinon.replace(Deps, 'readFile', sinon.fake.returns("" as any)); const writeFileFake = sinon.fake.resolves(undefined); sinon.replace(Deps, 'writeFile', writeFileFake); - const config = await loadGristCoreConfigFile("doesntmatter.json"); + const config = loadGristCoreConfigFile("doesntmatter.json"); assert.exists(config.edition.get()); await config.edition.set("enterprise"); diff --git a/test/server/lib/configCoreFileFormats.ts b/test/server/lib/configCoreFileFormats.ts index cf05c8ad57..5cb99786e1 100644 --- a/test/server/lib/configCoreFileFormats.ts +++ b/test/server/lib/configCoreFileFormats.ts @@ -2,7 +2,7 @@ import { assert } from 'chai'; import { convertToCoreFileContents, IGristCoreConfigFileLatest } from "app/server/lib/configCoreFileFormats"; describe('convertToCoreFileContents', () => { - it('fails with a malformed config', async () => { + it('fails with a malformed config', () => { const badConfig = { version: "This is a random version number that will never exist", };