Skip to content

Commit

Permalink
config: remove all async/await around config functions
Browse files Browse the repository at this point in the history
Now that I/O is synchronous, there's no need to have any more
async/await in regards to the config functions.
  • Loading branch information
jordigh committed Jul 17, 2024
1 parent fda2240 commit ed1416c
Show file tree
Hide file tree
Showing 7 changed files with 36 additions and 37 deletions.
16 changes: 8 additions & 8 deletions app/server/lib/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,15 +50,15 @@ export class FileConfig<FileContents> {
* @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<CreateConfigFileContents>(
public static create<CreateConfigFileContents>(
configPath: string,
validator: FileContentsValidator<CreateConfigFileContents>
): Promise<FileConfig<CreateConfigFileContents>> {
): FileConfig<CreateConfigFileContents> {
// 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;
Expand Down Expand Up @@ -86,13 +86,13 @@ export class FileConfig<FileContents> {
return this._rawConfig[key];
}

public async set<Key extends keyof FileContents>(key: Key, value: FileContents[Key]) {
public set<Key extends keyof FileContents>(key: Key, value: FileContents[Key]) {
this._rawConfig[key] = value;
await this.persistToDisk();
this.persistToDisk();
}

public async persistToDisk(): Promise<void> {
await Deps.writeFile(this._filePath, JSON.stringify(this._rawConfig, null, 2));
public persistToDisk() {
Deps.writeFile(this._filePath, JSON.stringify(this._rawConfig, null, 2));
}
}

Expand Down
4 changes: 2 additions & 2 deletions app/server/lib/configCore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ export interface IGristCoreConfig {
edition: IConfigValue<Edition>;
}

export async function loadGristCoreConfigFile(configPath?: string): Promise<IGristCoreConfig> {
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);
}

Expand Down
2 changes: 1 addition & 1 deletion app/server/mergedServerMain.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
4 changes: 2 additions & 2 deletions stubs/app/server/lib/globalConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<IGristCoreConfig> {
export function getGlobalConfig(): IGristCoreConfig {
if (!cachedGlobalConfig) {
log.info(`Loading config file from ${globalConfigPath}`);
cachedGlobalConfig = await loadGristCoreConfigFile(globalConfigPath);
cachedGlobalConfig = loadGristCoreConfigFile(globalConfigPath);
}

return cachedGlobalConfig;
Expand Down
31 changes: 15 additions & 16 deletions test/server/lib/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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<TestFileContents>("anypath.json", validator));
assert.throws(() => FileConfig.create<TestFileContents>("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",
Expand All @@ -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);
Expand All @@ -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);
Expand Down
14 changes: 7 additions & 7 deletions test/server/lib/configCore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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));
it('will function correctly when no config file is present', () => {
sinon.replace(Deps, 'pathExists', sinon.fake.returns(false));
sinon.replace(Deps, 'readFile', sinon.fake.resolves(""));
const writeFileFake = sinon.fake.resolves(undefined);
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);
});
Expand Down
2 changes: 1 addition & 1 deletion test/server/lib/configCoreFileFormats.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
};
Expand Down

0 comments on commit ed1416c

Please sign in to comment.