From e921d62630660dab9317ade1fb5583929eeb9f8a Mon Sep 17 00:00:00 2001 From: Joachim Van Herwegen Date: Tue, 18 Jul 2023 15:54:45 +0200 Subject: [PATCH] fix: Make all ways to start the server more consistent --- RELEASE_NOTES.md | 2 +- src/init/AppRunner.ts | 159 +++++++++++++-------------- test/integration/ServerFetch.test.ts | 10 +- test/unit/init/AppRunner.test.ts | 76 +++++++------ 4 files changed, 121 insertions(+), 126 deletions(-) diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 2be303a51..f301a4d9c 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -31,7 +31,7 @@ The following changes are relevant for v5 custom configs that replaced certain f These changes are relevant if you wrote custom modules for the server that depend on existing interfaces. -- ... +- The `AppRunner` functions to create and start the server now take a singular arguments object as input. ## v6.0.0 diff --git a/src/init/AppRunner.ts b/src/init/AppRunner.ts index d1018eb75..b9671844f 100644 --- a/src/init/AppRunner.ts +++ b/src/init/AppRunner.ts @@ -30,6 +30,42 @@ const CORE_CLI_PARAMETERS = { const ENV_VAR_PREFIX = 'CSS'; +/** + * Parameters that can be used to instantiate the server through code. + */ +export interface AppRunnerInput { + /** + * Properties that will be used when building the Components.js manager. + * Sets `typeChecking` to false by default as the server components will result in errors otherwise. + */ + loaderProperties: IComponentsManagerBuilderOptions; + /** + * Path to the server config file(s). + */ + config: string | string[]; + /** + * Values to apply to the Components.js variables. + * These are the variables CLI values will be converted to. + * The keys should be the variable URIs. + * E.g.: `{ 'urn:solid-server:default:variable:rootFilePath': '.data' }`. + */ + variableBindings?: VariableBindings; + /** + * CLI argument names and their corresponding values. + * E.g.: `{ rootFilePath: '.data' }`. + * Abbreviated parameter names can not be used, so `{ f: '.data' }` would not work. + * + * In case both `shorthand` and `variableBindings` have entries that would result in a value for the same variable, + * `variableBindings` has priority. + */ + shorthand?: Shorthand; + /** + * An array containing CLI arguments passed to start the process. + * Entries here have the lowest priority for assigning values to variables. + */ + argv?: string[]; +} + /** * A class that can be used to instantiate and start a server based on a Component.js configuration. */ @@ -38,63 +74,48 @@ export class AppRunner { /** * Starts the server with a given config. - * This method can be used to start the server from within another JavaScript application. * - * Keys of the `variableBindings` object should be Components.js variables. - * E.g.: `{ 'urn:solid-server:default:variable:rootFilePath': '.data' }`. - * - * `shorthand` are CLI argument names and their corresponding values. - * E.g.: `{ rootFilePath: '.data' }`. - * Abbreviated parameter names can not be used, so `{ f: '.data' }` would not work. - * - * The values in `variableBindings` take priority over those in `shorthand`. - * - * @param loaderProperties - Components.js loader properties. - * @param configFile - Path to the server config file(s). - * @param variableBindings - Bindings of Components.js variables. - * @param shorthand - Shorthand values that need to be resolved. + * @param input - All values necessary to configure the server. */ - public async run( - loaderProperties: IComponentsManagerBuilderOptions, - configFile: string | string[], - variableBindings?: VariableBindings, - shorthand?: Shorthand, - ): Promise { - const app = await this.create(loaderProperties, configFile, variableBindings, shorthand); + public async run(input: AppRunnerInput): Promise { + const app = await this.create(input); await app.start(); } /** * Returns an App object, created with the given config, that can start and stop the Solid server. * - * Keys of the `variableBindings` object should be Components.js variables. - * E.g.: `{ 'urn:solid-server:default:variable:rootFilePath': '.data' }`. - * - * `shorthand` are CLI argument names and their corresponding values. - * E.g.: `{ rootFilePath: '.data' }`. - * Abbreviated parameter names can not be used, so `{ f: '.data' }` would not work. - * - * The values in `variableBindings` take priority over those in `shorthand`. - * - * @param loaderProperties - Components.js loader properties. - * @param configFile - Path to the server config file(s). - * @param variableBindings - Bindings of Components.js variables. - * @param shorthand - Shorthand values that need to be resolved. + * @param input - All values necessary to configure the server. */ - public async create( - loaderProperties: IComponentsManagerBuilderOptions, - configFile: string | string[], - variableBindings?: VariableBindings, - shorthand?: Shorthand, - ): Promise { - const componentsManager = await this.createComponentsManager(loaderProperties, configFile); + public async create(input: AppRunnerInput): Promise { + const loaderProperties = { + typeChecking: false, + ...input.loaderProperties, + }; + + // Potentially expand file paths as needed + const configs = (Array.isArray(input.config) ? input.config : [ input.config ]).map(resolveAssetPath); + + let componentsManager: ComponentsManager; + try { + componentsManager = await this.createComponentsManager(loaderProperties, configs); + } catch (error: unknown) { + this.resolveError(`Could not build the config files from ${configs}`, error); + } const cliResolver = await this.createCliResolver(componentsManager); - const parsedVariables = await this.resolveShorthand(cliResolver.shorthandResolver, { ...shorthand }); + let extracted: Shorthand = {}; + if (input.argv) { + extracted = await this.extractShorthand(cliResolver.cliExtractor, input.argv); + } + const parsedVariables = await this.resolveShorthand(cliResolver.shorthandResolver, { + ...extracted, + ...input.shorthand, + }); // Create the application using the translated variable values. // `variableBindings` override those resolved from the `shorthand` input. - return this.createApp(componentsManager, { ...parsedVariables, ...variableBindings }); + return this.createApp(componentsManager, { ...parsedVariables, ...input.variableBindings }); } /** @@ -117,6 +138,7 @@ export class AppRunner { /** * Starts the server as a command-line application. + * * @param argv - Command line arguments. */ public async runCli(argv?: CliArgv): Promise { @@ -146,37 +168,23 @@ export class AppRunner { .env(ENV_VAR_PREFIX); const settings = await this.getPackageSettings(); - if (typeof settings !== 'undefined') { yargv = yargv.default(settings); } const params = await yargv.parse(); - const loaderProperties = { + const loaderProperties: IComponentsManagerBuilderOptions = { mainModulePath: resolveAssetPath(params.mainModulePath), - dumpErrorState: true, logLevel: params.loggingLevel, - typeChecking: false, }; - const configs = (params.config as string[]).map(resolveAssetPath); - - // Create the Components.js manager used to build components from the provided config - let componentsManager: ComponentsManager; - try { - componentsManager = await this.createComponentsManager(loaderProperties, configs); - } catch (error: unknown) { - // Print help of the expected core CLI parameters - const help = await yargv.getHelp(); - this.resolveError(`${help}\n\nCould not build the config files from ${configs}`, error); - } - - // Build the CLI components and use them to generate values for the Components.js variables - const variables = await this.cliToVariables(componentsManager, argv, settings); - - // Build and start the actual server application using the generated variable values - return await this.createApp(componentsManager, variables); + return this.create({ + loaderProperties, + config: params.config as string[], + argv, + shorthand: settings, + }); } /** @@ -217,29 +225,15 @@ export class AppRunner { */ public async createComponentsManager( loaderProperties: IComponentsManagerBuilderOptions, - configFile: string | string[], + configs: string[], ): Promise> { const componentsManager = await ComponentsManager.build(loaderProperties); - for (const config of Array.isArray(configFile) ? configFile : [ configFile ]) { + for (const config of configs) { await componentsManager.configRegistry.register(config); } return componentsManager; } - /** - * Handles the first Components.js instantiation. - * Uses it to extract the CLI shorthand values and use those to create variable bindings. - */ - private async cliToVariables( - componentsManager: ComponentsManager, - argv: CliArgv, - settings?: Record, - ): Promise { - const cliResolver = await this.createCliResolver(componentsManager); - const shorthand = await this.extractShorthand(cliResolver.cliExtractor, argv); - return await this.resolveShorthand(cliResolver.shorthandResolver, { ...settings, ...shorthand }); - } - /** * Instantiates the {@link CliResolver}. */ @@ -310,10 +304,7 @@ export class AppRunner { * This is needed for errors that are thrown before the logger is created as we can't log those the standard way. */ private resolveError(message: string, error: unknown): never { - let errorMessage = `${message}\nCause: ${createErrorMessage(error)}\n`; - if (isError(error)) { - errorMessage += `${error.stack}\n`; - } + const errorMessage = `${message}\n${isError(error) ? error.stack : createErrorMessage(error)}\n`; throw new Error(errorMessage); } } diff --git a/test/integration/ServerFetch.test.ts b/test/integration/ServerFetch.test.ts index 44d8c6989..97f80d219 100644 --- a/test/integration/ServerFetch.test.ts +++ b/test/integration/ServerFetch.test.ts @@ -15,15 +15,15 @@ describe('A Solid server', (): void => { beforeAll(async(): Promise => { // Using AppRunner here so it is also tested in an integration test - app = await new AppRunner().create( - { + app = await new AppRunner().create({ + loaderProperties: { mainModulePath: resolveModulePath(''), logLevel: 'error', typeChecking: false, }, - resolveModulePath('config/default.json'), - getDefaultVariables(port, baseUrl), - ); + config: resolveModulePath('config/default.json'), + variableBindings: getDefaultVariables(port, baseUrl), + }); await app.start(); }); diff --git a/test/unit/init/AppRunner.test.ts b/test/unit/init/AppRunner.test.ts index 1edef65de..c4850f72e 100644 --- a/test/unit/init/AppRunner.test.ts +++ b/test/unit/init/AppRunner.test.ts @@ -174,13 +174,15 @@ describe('AppRunner', (): void => { const createdApp = await new AppRunner().create( { - mainModulePath: joinFilePath(__dirname, '../../../'), - dumpErrorState: true, - logLevel: 'info', + loaderProperties: { + mainModulePath: joinFilePath(__dirname, '../../../'), + dumpErrorState: true, + logLevel: 'info', + }, + config: joinFilePath(__dirname, '../../../config/default.json'), + variableBindings: variables, + shorthand, }, - joinFilePath(__dirname, '../../../config/default.json'), - variables, - shorthand, ); expect(createdApp).toBe(app); @@ -189,6 +191,7 @@ describe('AppRunner', (): void => { dumpErrorState: true, logLevel: 'info', mainModulePath: joinFilePath(__dirname, '../../../'), + typeChecking: false, }); expect(manager.configRegistry.register).toHaveBeenCalledTimes(1); expect(manager.configRegistry.register) @@ -220,12 +223,14 @@ describe('AppRunner', (): void => { try { await new AppRunner().create( { - mainModulePath: joinFilePath(__dirname, '../../../'), - dumpErrorState: true, - logLevel: 'info', + loaderProperties: { + mainModulePath: joinFilePath(__dirname, '../../../'), + dumpErrorState: true, + logLevel: 'info', + }, + config: joinFilePath(__dirname, '../../../config/default.json'), + variableBindings: variables, }, - joinFilePath(__dirname, '../../../config/default.json'), - variables, ); } catch (error: unknown) { caughtError = error as Error; @@ -254,12 +259,14 @@ describe('AppRunner', (): void => { try { await new AppRunner().create( { - mainModulePath: joinFilePath(__dirname, '../../../'), - dumpErrorState: true, - logLevel: 'info', + loaderProperties: { + mainModulePath: joinFilePath(__dirname, '../../../'), + dumpErrorState: true, + logLevel: 'info', + }, + config: joinFilePath(__dirname, '../../../config/default.json'), + variableBindings: variables, }, - joinFilePath(__dirname, '../../../config/default.json'), - variables, ); } catch (error: unknown) { caughtError = error as Error; @@ -293,13 +300,15 @@ describe('AppRunner', (): void => { await new AppRunner().run( { - mainModulePath: joinFilePath(__dirname, '../../../'), - dumpErrorState: true, - logLevel: 'info', + loaderProperties: { + mainModulePath: joinFilePath(__dirname, '../../../'), + dumpErrorState: true, + logLevel: 'info', + }, + config: joinFilePath(__dirname, '../../../config/default.json'), + variableBindings: variables, + shorthand, }, - joinFilePath(__dirname, '../../../config/default.json'), - variables, - shorthand, ); expect(ComponentsManager.build).toHaveBeenCalledTimes(1); @@ -307,6 +316,7 @@ describe('AppRunner', (): void => { dumpErrorState: true, logLevel: 'info', mainModulePath: joinFilePath(__dirname, '../../../'), + typeChecking: false, }); expect(manager.configRegistry.register).toHaveBeenCalledTimes(1); expect(manager.configRegistry.register) @@ -331,7 +341,6 @@ describe('AppRunner', (): void => { expect(ComponentsManager.build).toHaveBeenCalledTimes(1); expect(ComponentsManager.build).toHaveBeenCalledWith({ - dumpErrorState: true, logLevel: 'info', mainModulePath: joinFilePath(__dirname, '../../../'), typeChecking: false, @@ -362,7 +371,6 @@ describe('AppRunner', (): void => { expect(ComponentsManager.build).toHaveBeenCalledTimes(1); expect(ComponentsManager.build).toHaveBeenCalledWith({ - dumpErrorState: true, logLevel: 'info', mainModulePath: joinFilePath(__dirname, '../../../'), typeChecking: false, @@ -406,7 +414,6 @@ describe('AppRunner', (): void => { expect(ComponentsManager.build).toHaveBeenCalledTimes(1); expect(ComponentsManager.build).toHaveBeenCalledWith({ - dumpErrorState: true, logLevel: 'debug', mainModulePath: '/var/cwd/module/path', typeChecking: false, @@ -463,7 +470,7 @@ describe('AppRunner', (): void => { caughtError = error as Error; } expect(caughtError.message).toMatch(/^Could not build the config files from .*default\.json/mu); - expect(caughtError.message).toMatch(/^Cause: Fatal/mu); + expect(caughtError.message).toMatch(/^Error: Fatal/mu); expect(write).toHaveBeenCalledTimes(0); expect(exit).toHaveBeenCalledTimes(0); @@ -479,7 +486,7 @@ describe('AppRunner', (): void => { caughtError = error as Error; } expect(caughtError.message).toMatch(/^Could not create the CLI resolver/mu); - expect(caughtError.message).toMatch(/^Cause: Fatal/mu); + expect(caughtError.message).toMatch(/^Error: Fatal/mu); expect(write).toHaveBeenCalledTimes(0); expect(exit).toHaveBeenCalledTimes(0); @@ -495,7 +502,7 @@ describe('AppRunner', (): void => { caughtError = error as Error; } expect(caughtError.message).toMatch(/^Could not parse the CLI parameters/mu); - expect(caughtError.message).toMatch(/^Cause: Fatal/mu); + expect(caughtError.message).toMatch(/^Error: Fatal/mu); expect(write).toHaveBeenCalledTimes(0); expect(exit).toHaveBeenCalledTimes(0); @@ -511,7 +518,7 @@ describe('AppRunner', (): void => { caughtError = error as Error; } expect(caughtError.message).toMatch(/^Could not resolve the shorthand values/mu); - expect(caughtError.message).toMatch(/^Cause: Fatal/mu); + expect(caughtError.message).toMatch(/^Error: Fatal/mu); expect(write).toHaveBeenCalledTimes(0); expect(exit).toHaveBeenCalledTimes(0); @@ -530,7 +537,7 @@ describe('AppRunner', (): void => { caughtError = error as Error; } expect(caughtError.message).toMatch(/^Could not create the server/mu); - expect(caughtError.message).toMatch(/^Cause: Fatal/mu); + expect(caughtError.message).toMatch(/^Error: Fatal/mu); expect(write).toHaveBeenCalledTimes(0); expect(exit).toHaveBeenCalledTimes(0); @@ -545,7 +552,7 @@ describe('AppRunner', (): void => { } catch (error: unknown) { caughtError = error as Error; } - expect(caughtError.message).toMatch(/^Cause: Unknown error: NotAnError$/mu); + expect(caughtError.message).toMatch(/^Unknown error: NotAnError$/mu); expect(write).toHaveBeenCalledTimes(0); expect(exit).toHaveBeenCalledTimes(0); @@ -558,7 +565,6 @@ describe('AppRunner', (): void => { expect(ComponentsManager.build).toHaveBeenCalledTimes(1); expect(ComponentsManager.build).toHaveBeenCalledWith({ - dumpErrorState: true, logLevel: 'info', mainModulePath: joinFilePath(__dirname, '../../../'), typeChecking: false, @@ -591,7 +597,6 @@ describe('AppRunner', (): void => { expect(ComponentsManager.build).toHaveBeenCalledTimes(1); // Check logLevel to be set to debug instead of default `info` expect(ComponentsManager.build).toHaveBeenCalledWith({ - dumpErrorState: true, logLevel: 'debug', mainModulePath: joinFilePath(__dirname, '../../../'), typeChecking: false, @@ -723,7 +728,7 @@ describe('AppRunner', (): void => { caughtError = error as Error; } expect(caughtError.message).toMatch(/^Could not start the server/mu); - expect(caughtError.message).toMatch(/^Cause: Fatal/mu); + expect(caughtError.message).toMatch(/^Error: Fatal/mu); expect(app.start).toHaveBeenCalledTimes(1); @@ -743,7 +748,6 @@ describe('AppRunner', (): void => { expect(ComponentsManager.build).toHaveBeenCalledTimes(1); expect(ComponentsManager.build).toHaveBeenCalledWith({ - dumpErrorState: true, logLevel: 'info', mainModulePath: joinFilePath(__dirname, '../../../'), typeChecking: false, @@ -776,7 +780,7 @@ describe('AppRunner', (): void => { await flushPromises(); expect(write).toHaveBeenCalledTimes(1); - expect(write).toHaveBeenLastCalledWith(expect.stringMatching(/Cause: Fatal/mu)); + expect(write).toHaveBeenLastCalledWith(expect.stringMatching(/Error: Fatal/mu)); expect(exit).toHaveBeenCalledTimes(1); expect(exit).toHaveBeenLastCalledWith(1);