From bfa045b13f90e7bbbbc45c6e38544bee516152d8 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] config: remove all async/await around config functions Now that I/O is synchronous, there's no need to have any more async/await in regards to the config functions. --- app/server/lib/config.ts | 16 ++++++------ app/server/lib/configCore.ts | 4 +-- app/server/mergedServerMain.ts | 2 +- stubs/app/server/lib/globalConfig.ts | 4 +-- test/server/lib/config.ts | 31 ++++++++++++------------ test/server/lib/configCore.ts | 16 ++++++------ test/server/lib/configCoreFileFormats.ts | 2 +- 7 files changed, 37 insertions(+), 38 deletions(-) diff --git a/app/server/lib/config.ts b/app/server/lib/config.ts index 612e8b0a0ab..8751366ab66 100644 --- a/app/server/lib/config.ts +++ b/app/server/lib/config.ts @@ -50,15 +50,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; @@ -86,13 +86,13 @@ export class FileConfig { return this._rawConfig[key]; } - public async set(key: Key, value: FileContents[Key]) { + public set(key: Key, value: FileContents[Key]) { this._rawConfig[key] = value; - await this.persistToDisk(); + this.persistToDisk(); } - public async persistToDisk(): Promise { - await Deps.writeFile(this._filePath, JSON.stringify(this._rawConfig, null, 2)); + public persistToDisk() { + 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 983c7e970e8..8fa64aa8674 100644 --- a/app/server/lib/configCore.ts +++ b/app/server/lib/configCore.ts @@ -15,8 +15,8 @@ export interface IGristCoreConfig { edition: IConfigValue; } -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 f4e4a4a631d..466eddafca7 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 0c62d855461..9b9c6d4c964 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 711e75ec8c6..938e1fcee96 100644 --- a/test/server/lib/config.ts +++ b/test/server/lib/config.ts @@ -18,10 +18,9 @@ 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(); })); return fakeFile; @@ -31,25 +30,25 @@ 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 () => { + it('persists changes when values are assigned', () => { 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); - await fileConfig.set("myNum", 999); + const fileConfig = FileConfig.create("anypath.json", validator); + fileConfig.set("myNum", 999); assert.equal(fileConfig.get("myNum"), 999); assert.equal(JSON.parse(fakeFile.contents).myNum, 999); }); // Avoid removing extra properties from the file, in case another edition of grist is doing something. - it('does not remove extra values from the file', async () => { + it('does not remove extra values from the file', () => { const configWithExtraProperties = { ...testFileContentsExample, someProperty: "isPresent", @@ -58,10 +57,10 @@ 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"); + fileConfig.set("myNum", 999); + fileConfig.set("myStr", "Something"); const newContents = JSON.parse(fakeFile.contents); assert.equal(newContents.myNum, 999); @@ -79,22 +78,22 @@ describe('createConfigValue', () => { }; }; - it('works without persistence', async () => { + it('works without persistence', () => { const configValue = createConfigValue(1); assert.equal(configValue.get(), 1); - await configValue.set(2); + configValue.set(2); assert.equal(configValue.get(), 2); }); - it('writes to persistence when saved', async () => { + it('writes to persistence when saved', () => { const accessors = makeInMemoryAccessors(1); const configValue = createConfigValue(1, accessors); assert.equal(accessors.get(), 1); - await configValue.set(2); + configValue.set(2); 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 adfa921fca8..f984c6837b5 100644 --- a/test/server/lib/configCore.ts +++ b/test/server/lib/configCore.ts @@ -8,22 +8,22 @@ describe('loadGristCoreConfig', () => { sinon.restore(); }); - it('can be used with an in-memory store if no file config is provided', async () => { + it('can be used with an in-memory store if no file config is provided', () => { const config = loadGristCoreConfig(); - await config.edition.set("enterprise"); + config.edition.set("enterprise"); assert.equal(config.edition.get(), "enterprise"); }); - 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("")); - const writeFileFake = sinon.fake.resolves(undefined); + it('will function correctly when no config file is present', () => { + sinon.replace(Deps, 'pathExists', sinon.fake.returns(false)); + sinon.replace(Deps, 'readFile', sinon.fake.returns("" as any)); + const writeFileFake = sinon.fake.returns(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"); + config.edition.set("enterprise"); // Make sure that the change was written back to the file. assert.isTrue(writeFileFake.calledOnce); }); diff --git a/test/server/lib/configCoreFileFormats.ts b/test/server/lib/configCoreFileFormats.ts index cf05c8ad578..5cb99786e10 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", };