From 6f4e70dbb928a9cb64e83a71954ea607a0bdb1a0 Mon Sep 17 00:00:00 2001 From: Joachim Van Herwegen Date: Thu, 21 Apr 2022 10:54:06 +0200 Subject: [PATCH] fix: Change YargsCliExtractor structure to avoid Components.js issues --- .componentsignore | 2 +- config/app/variables/cli/cli.json | 138 +++++++++++------- config/https-file-cli.json | 32 ++-- .../interaction/FixedInteractionHandler.ts | 2 +- src/init/cli/YargsCliExtractor.ts | 41 ++++-- test/unit/init/cli/YargsCliExtractor.test.ts | 29 +--- 6 files changed, 148 insertions(+), 96 deletions(-) diff --git a/.componentsignore b/.componentsignore index 0d03afee8..f1bc4f5ba 100644 --- a/.componentsignore +++ b/.componentsignore @@ -23,5 +23,5 @@ "VariableBindings", "UnionHandler", "WinstonLogger", - "YargsArgOptions" + "YargsOptions" ] diff --git a/config/app/variables/cli/cli.json b/config/app/variables/cli/cli.json index 2d59f7016..578d5bcbe 100644 --- a/config/app/variables/cli/cli.json +++ b/config/app/variables/cli/cli.json @@ -5,65 +5,105 @@ "comment": "Extracts CLI arguments into a key/value object. Config and mainModulePath are only defined here so their description is returned.", "@id": "urn:solid-server-app-setup:default:CliExtractor", "@type": "YargsCliExtractor", - "parameters": { - "config": { - "alias": "c", - "requiresArg": true, - "type": "string", - "describe": "The configuration for the server. The default only stores data in memory; to persist to your filesystem, use @css:config/file.json." + "parameters": [ + { + "@type": "YargsParameter", + "name": "config", + "options": { + "alias": "c", + "requiresArg": true, + "type": "string", + "describe": "The configuration for the server. The default only stores data in memory; to persist to your filesystem, use @css:config/file.json." + } }, - "mainModulePath": { - "alias": "m", - "requiresArg": true, - "type": "string", - "describe": "Path from where Components.js will start its lookup when initializing configurations." + { + "@type": "YargsParameter", + "name": "mainModulePath", + "options": { + "alias": "m", + "requiresArg": true, + "type": "string", + "describe": "Path from where Components.js will start its lookup when initializing configurations." + } }, - "loggingLevel": { - "alias": "l", - "requiresArg": true, - "type": "string", - "describe": "The detail level of logging; useful for debugging problems." + { + "@type": "YargsParameter", + "name": "loggingLevel", + "options": { + "alias": "l", + "requiresArg": true, + "type": "string", + "describe": "The detail level of logging; useful for debugging problems." + } }, - "baseUrl": { - "alias": "b", - "requiresArg": true, - "type": "string", - "describe": "The public URL of your server." + { + "@type": "YargsParameter", + "name": "baseUrl", + "options": { + "alias": "b", + "requiresArg": true, + "type": "string", + "describe": "The public URL of your server." + } }, - "port": { - "alias": "p", - "requiresArg": true, - "type": "number", - "describe": "The TCP port on which the server runs." + { + "@type": "YargsParameter", + "name": "port", + "options": { + "alias": "p", + "requiresArg": true, + "type": "number", + "describe": "The TCP port on which the server runs." + } }, - "rootFilePath": { - "alias": "f", - "requiresArg": true, - "type": "string", - "describe": "Root folder of the server, when using a file-based configuration." + { + "@type": "YargsParameter", + "name": "rootFilePath", + "options": { + "alias": "f", + "requiresArg": true, + "type": "string", + "describe": "Root folder of the server, when using a file-based configuration." + } }, - "showStackTrace": { - "alias": "t", - "type": "boolean", - "describe": "Enables detailed logging on error pages." + { + "@type": "YargsParameter", + "name": "showStackTrace", + "options": { + "alias": "t", + "type": "boolean", + "describe": "Enables detailed logging on error pages." + } }, - "sparqlEndpoint": { - "alias": "s", - "requiresArg": true, - "type": "string", - "describe": "URL of the SPARQL endpoint, when using a quadstore-based configuration." + { + "@type": "YargsParameter", + "name": "sparqlEndpoint", + "options": { + "alias": "s", + "requiresArg": true, + "type": "string", + "describe": "URL of the SPARQL endpoint, when using a quadstore-based configuration." + } }, - "podConfigJson": { - "requiresArg": true, - "type": "string", - "describe": "Path to the file that keeps track of dynamic Pod configurations." + { + "@type": "YargsParameter", + "name": "podConfigJson", + "options": { + "requiresArg": true, + "type": "string", + "describe": "Path to the file that keeps track of dynamic Pod configurations." + } }, - "seededPodConfigJson": { - "requiresArg": true, - "type": "string", - "describe": "Path to the file that will be used to seed pods." + { + "@type": "YargsParameter", + "name": "seededPodConfigJson", + "options": { + "requiresArg": true, + "type": "string", + "describe": "Path to the file that will be used to seed pods." + } } - }, + ], "options": { "usage": "node ./bin/server.js [args]" } diff --git a/config/https-file-cli.json b/config/https-file-cli.json index eaac04cd6..901b00a90 100644 --- a/config/https-file-cli.json +++ b/config/https-file-cli.json @@ -42,20 +42,28 @@ { "@id": "urn:solid-server-app-setup:default:CliExtractor", "@type": "YargsCliExtractor", - "extendedParameters": { - "httpsKey": { - "demandOption": true, - "requiresArg": true, - "type": "string", - "describe": "File path to the HTTPS key." + "parameters": [ + { + "@type": "YargsParameter", + "name": "httpsKey", + "options": { + "demandOption": true, + "requiresArg": true, + "type": "string", + "describe": "File path to the HTTPS key." + } }, - "httpsCert": { - "demandOption": true, - "requiresArg": true, - "type": "string", - "describe": "File path to the HTTPS certificate." + { + "@type": "YargsParameter", + "name": "httpsCert", + "options": { + "demandOption": true, + "requiresArg": true, + "type": "string", + "describe": "File path to the HTTPS certificate." + } } - } + ] }, { "comment": "Adds resolvers to assign the CLI values to the Components.js variables.", diff --git a/src/identity/interaction/FixedInteractionHandler.ts b/src/identity/interaction/FixedInteractionHandler.ts index c3deecf3c..5237de868 100644 --- a/src/identity/interaction/FixedInteractionHandler.ts +++ b/src/identity/interaction/FixedInteractionHandler.ts @@ -15,7 +15,7 @@ export class FixedInteractionHandler extends InteractionHandler { /** * @param response - @range {json} */ - public constructor(response: unknown) { + public constructor(response: Record) { super(); this.response = JSON.stringify(response); } diff --git a/src/init/cli/YargsCliExtractor.ts b/src/init/cli/YargsCliExtractor.ts index 9d41750c9..26ee5f701 100644 --- a/src/init/cli/YargsCliExtractor.ts +++ b/src/init/cli/YargsCliExtractor.ts @@ -3,7 +3,28 @@ import type { Arguments, Argv, Options } from 'yargs'; import yargs from 'yargs'; import { CliExtractor } from './CliExtractor'; -export type YargsArgOptions = Record; +// This type exists to prevent Components.js from erroring on an unknown type +export type YargsOptions = Options; + +/** + * This class exists as wrapper around a yargs Options object, + * thereby allowing us to create these in a Components.js configuration. + * + * Format details can be found at https://yargs.js.org/docs/#api-reference-optionskey-opt + */ +export class YargsParameter { + public readonly name: string; + public readonly options: YargsOptions; + + /** + * @param name - Name of the parameter. Corresponds to the first parameter passed to the `yargs.options` function. + * @param options - Options for a single parameter that should be parsed. @range {json} + */ + public constructor(name: string, options: Record) { + this.name = name; + this.options = options; + } +} export interface CliOptions { // Usage string printed in case of CLI errors @@ -24,21 +45,21 @@ export interface CliOptions { * Specific settings can be enabled through the provided options. */ export class YargsCliExtractor extends CliExtractor { - protected readonly yargsArgOptions: YargsArgOptions; + protected readonly yargsArgOptions: Record; protected readonly yargvOptions: CliOptions; /** - * @param parameters - Parameters that should be parsed from the CLI. @range {json} - * Format details can be found at https://yargs.js.org/docs/#api-reference-optionskey-opt + * @param parameters - Parameters that should be parsed from the CLI. * @param options - Additional options to configure yargs. @range {json} - * @param extendedParameters - The same as @parameters. Separate variable so in Components.js - * we can have both a default set and a user-added version. @range {json} + * + * JSON parameters cannot be optional due to https://github.com/LinkedSoftwareDependencies/Components-Generator.js/issues/87 */ - public constructor(parameters: YargsArgOptions = {}, options: CliOptions = {}, - extendedParameters: YargsArgOptions = {}) { + public constructor(parameters: YargsParameter[], options: CliOptions) { super(); - this.yargsArgOptions = { ...parameters, ...extendedParameters }; - this.yargvOptions = options; + this.yargsArgOptions = Object.fromEntries( + parameters.map((entry): [string, YargsOptions] => [ entry.name, entry.options ]), + ); + this.yargvOptions = { ...options }; } public async handle(argv: readonly string[]): Promise { diff --git a/test/unit/init/cli/YargsCliExtractor.test.ts b/test/unit/init/cli/YargsCliExtractor.test.ts index e23b77487..7bcc3a39c 100644 --- a/test/unit/init/cli/YargsCliExtractor.test.ts +++ b/test/unit/init/cli/YargsCliExtractor.test.ts @@ -1,19 +1,18 @@ -import type { YargsArgOptions } from '../../../../src/init/cli/YargsCliExtractor'; -import { YargsCliExtractor } from '../../../../src/init/cli/YargsCliExtractor'; +import { YargsParameter, YargsCliExtractor } from '../../../../src/init/cli/YargsCliExtractor'; const error = jest.spyOn(console, 'error').mockImplementation(jest.fn()); const log = jest.spyOn(console, 'log').mockImplementation(jest.fn()); const exit = jest.spyOn(process, 'exit').mockImplementation(jest.fn() as any); describe('A YargsCliExtractor', (): void => { - const parameters: YargsArgOptions = { - baseUrl: { alias: 'b', requiresArg: true, type: 'string' }, - port: { alias: 'p', requiresArg: true, type: 'number' }, - }; + const parameters: YargsParameter[] = [ + new YargsParameter('baseUrl', { alias: 'b', requiresArg: true, type: 'string' }), + new YargsParameter('port', { alias: 'p', requiresArg: true, type: 'number' }), + ]; let extractor: YargsCliExtractor; beforeEach(async(): Promise => { - extractor = new YargsCliExtractor(parameters); + extractor = new YargsCliExtractor(parameters, {}); }); afterEach(async(): Promise => { @@ -36,22 +35,6 @@ describe('A YargsCliExtractor', (): void => { })); }); - it('defaults to no parameters if none are provided.', async(): Promise => { - extractor = new YargsCliExtractor(); - const argv = [ 'node', 'script', '-b', 'http://localhost:3000/', '-p', '3000' ]; - await expect(extractor.handle(argv)).resolves.toEqual(expect.objectContaining({})); - }); - - it('combines parameters and extra parameters.', async(): Promise => { - extractor = new YargsCliExtractor(parameters, {}, { test: { alias: 't', requiresArg: true, type: 'string' }}); - const argv = [ 'node', 'script', '-b', 'http://localhost:3000/', '-p', '3000', '-t', 'test' ]; - await expect(extractor.handle(argv)).resolves.toEqual(expect.objectContaining({ - baseUrl: 'http://localhost:3000/', - port: 3000, - test: 'test', - })); - }); - it('prints usage if defined.', async(): Promise => { extractor = new YargsCliExtractor(parameters, { usage: 'node ./bin/server.js [args]' }); const argv = [ 'node', 'script', '--help' ];