From 1d5cded67b1afa6d55f2e36baeb7e26dc0b786b1 Mon Sep 17 00:00:00 2001 From: Josh Gummersall Date: Wed, 17 Mar 2021 09:50:05 -0700 Subject: [PATCH 1/2] feat: argv config source for runtime --- .../src/index.ts | 29 +++++++++---- .../src/index.ts | 41 +++++++++++++------ libraries/botbuilder-runtime/package.json | 3 +- .../botbuilder-runtime/src/configuration.ts | 18 ++++++++ libraries/botbuilder-runtime/src/index.ts | 1 + .../test/configuration.test.ts | 10 +++++ .../test/settings/base.json | 6 ++- yarn.lock | 5 +++ 8 files changed, 91 insertions(+), 22 deletions(-) diff --git a/libraries/botbuilder-runtime-integration-express/src/index.ts b/libraries/botbuilder-runtime-integration-express/src/index.ts index 47f4b357da..13748443af 100644 --- a/libraries/botbuilder-runtime-integration-express/src/index.ts +++ b/libraries/botbuilder-runtime-integration-express/src/index.ts @@ -9,14 +9,14 @@ import { IServices, ServiceCollection } from 'botbuilder-runtime-core'; const TypedOptions = t.Record({ /** - * Port that server should listen on + * Path that the server will listen to for [Activities](xref:botframework-schema.Activity) */ - port: t.Number, + messagingEndpointPath: t.String, /** - * Path that the server will listen to for [Activities](xref:botframework-schema.Activity) + * Port that server should listen on */ - messagingEndpointPath: t.String, + port: t.Union(t.String, t.Number), }); /** @@ -60,11 +60,23 @@ export async function makeApp( configuration: Configuration, options: Partial = {} ): Promise<[app: express.Application, listen: (callback?: () => void) => http.Server]> { - const { port, messagingEndpointPath } = TypedOptions.check(Object.assign({}, defaultOptions, options)); + const configOverrides: Partial = {}; + + const port = (await Promise.all(['port', 'PORT'].map((key) => configuration.string([key])))).find( + (port) => port !== undefined + ); + + if (port !== undefined) { + configOverrides.port = port; + } + + const validatedOptions = TypedOptions.check(Object.assign({}, defaultOptions, configOverrides, options)); + const { adapter, bot, customAdapters } = await services.mustMakeInstances('adapter', 'bot', 'customAdapters'); + const app = express(); - app.post(messagingEndpointPath, (req, res) => { + app.post(validatedOptions.messagingEndpointPath, (req, res) => { adapter.processActivity(req, res, async (turnContext) => { await bot.run(turnContext); }); @@ -102,7 +114,10 @@ export async function makeApp( (callback) => { // The 'upgrade' event handler for processing WebSocket requests needs to be registered on the Node.js http.Server, not the Express.Application. // In Express the underlying http.Server is made available after the app starts listening for requests. - const server = app.listen(port, callback ?? (() => console.log(`server listening on port ${port}`))); + const server = app.listen( + validatedOptions.port, + callback ?? (() => console.log(`server listening on port ${validatedOptions.port}`)) + ); server.on('upgrade', async (req, socket, head) => { const adapter = await services.mustMakeInstance('adapter'); diff --git a/libraries/botbuilder-runtime-integration-restify/src/index.ts b/libraries/botbuilder-runtime-integration-restify/src/index.ts index e40018dfee..e78ed05ceb 100644 --- a/libraries/botbuilder-runtime-integration-restify/src/index.ts +++ b/libraries/botbuilder-runtime-integration-restify/src/index.ts @@ -8,14 +8,14 @@ import { IServices, ServiceCollection } from 'botbuilder-runtime-core'; const TypedOptions = t.Record({ /** - * Port that server should listen on + * Path that the server will listen to for [Activities](xref:botframework-schema.Activity) */ - port: t.Number, + messagingEndpointPath: t.String, /** - * Path that the server will listen to for [Activities](xref:botframework-schema.Activity) + * Port that server should listen on */ - messagingEndpointPath: t.String, + port: t.Union(t.String, t.Number), }); /** @@ -24,10 +24,24 @@ const TypedOptions = t.Record({ export type Options = t.Static; const defaultOptions: Options = { - port: 3978, messagingEndpointPath: '/api/messages', + port: 3978, }; +async function resolveOptions(options: Partial, configuration: Configuration): Promise { + const configOverrides: Partial = {}; + + const port = (await Promise.all(['port', 'PORT'].map((key) => configuration.string([key])))).find( + (port) => port !== undefined + ); + + if (port !== undefined) { + configOverrides.port = port; + } + + return TypedOptions.check(Object.assign({}, defaultOptions, configOverrides, options)); +} + /** * Start a bot using the runtime restify integration. * @@ -40,14 +54,13 @@ export async function start( settingsDirectory: string, options: Partial = {} ): Promise { - const validatedOptions = TypedOptions.check(Object.assign({}, defaultOptions, options)); const [services, configuration] = await getRuntimeServices(applicationRoot, settingsDirectory); - const server = await makeServer(services, configuration, validatedOptions); + const resolvedOptions = await resolveOptions(options, configuration); - server.listen(validatedOptions.port, () => { - console.log(`server listening on port ${validatedOptions.port}`); - }); + const server = await makeServer(services, configuration, resolvedOptions); + + server.listen(resolvedOptions.port, () => console.log(`server listening on port ${resolvedOptions.port}`)); } /** @@ -63,12 +76,14 @@ export async function makeServer( configuration: Configuration, options: Partial = {} ): Promise { - const { messagingEndpointPath } = TypedOptions.check(Object.assign({}, defaultOptions, options)); - const { adapter, bot, customAdapters } = await services.mustMakeInstances('adapter', 'bot', 'customAdapters'); + const [{ adapter, bot, customAdapters }, resolvedOptions] = await Promise.all([ + services.mustMakeInstances('adapter', 'bot', 'customAdapters'), + resolveOptions(options, configuration), + ]); const server = restify.createServer(); - server.post(messagingEndpointPath, (req, res) => { + server.post(resolvedOptions.messagingEndpointPath, (req, res) => { adapter.processActivity(req, res, async (turnContext) => { await bot.run(turnContext); }); diff --git a/libraries/botbuilder-runtime/package.json b/libraries/botbuilder-runtime/package.json index a7b8f7ac74..9e1e8d89b4 100644 --- a/libraries/botbuilder-runtime/package.json +++ b/libraries/botbuilder-runtime/package.json @@ -40,7 +40,8 @@ "botframework-connector": "4.1.6", "dependency-graph": "^0.10.0", "nconf": "^0.11.2", - "runtypes": "^5.0.1" + "runtypes": "^5.0.1", + "yargs-parser": "^20.2.7" }, "devDependencies": { "@types/nconf": "^0.10.0", diff --git a/libraries/botbuilder-runtime/src/configuration.ts b/libraries/botbuilder-runtime/src/configuration.ts index 9ab3b5a80c..5f7be9609f 100644 --- a/libraries/botbuilder-runtime/src/configuration.ts +++ b/libraries/botbuilder-runtime/src/configuration.ts @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +import yargs from 'yargs-parser'; import { Boolean, Runtype, String, Undefined, ValidationError } from 'runtypes'; import { IConfiguration } from 'botbuilder-runtime-core'; import { Provider } from 'nconf'; @@ -56,6 +57,23 @@ export class Configuration implements IConfiguration { ); } + /** + * Load process.arguments as a configuration source. + * + * @param argv arguments to parse, defaults to `process.argv` + * @returns this for chaining + */ + argv(argv = process.argv.slice(2)): this { + this.provider.argv({ + argv: yargs(argv, { + configuration: { + 'parse-numbers': false, + }, + }), + }); + return this; + } + /** * Load environment variables as a configuration source. * diff --git a/libraries/botbuilder-runtime/src/index.ts b/libraries/botbuilder-runtime/src/index.ts index 57bb4f9d44..6803604f2b 100644 --- a/libraries/botbuilder-runtime/src/index.ts +++ b/libraries/botbuilder-runtime/src/index.ts @@ -267,6 +267,7 @@ export async function getRuntimeServices( let configuration: Configuration; if (typeof configurationOrSettingsDirectory === 'string') { configuration = new Configuration() + .argv() .env() .file(path.join(configurationOrSettingsDirectory, 'appsettings.Development.json')) .file(path.join(configurationOrSettingsDirectory, 'appsettings.json')); diff --git a/libraries/botbuilder-runtime/test/configuration.test.ts b/libraries/botbuilder-runtime/test/configuration.test.ts index 3908190998..b4ed1a4bda 100644 --- a/libraries/botbuilder-runtime/test/configuration.test.ts +++ b/libraries/botbuilder-runtime/test/configuration.test.ts @@ -8,7 +8,14 @@ import { Configuration } from '../src/configuration'; describe('Configuration', () => { const makeConfiguration = (files = ['base.json']) => { const configuration = new Configuration(); + files.forEach((file) => configuration.file(path.join(__dirname, 'settings', file))); + + configuration.argv(['--strings.argv', 'argv']); + + process.env['strings:env'] = 'env'; + configuration.env(); + return configuration; }; @@ -32,6 +39,9 @@ describe('Configuration', () => { assert.strictEqual(await strings.string(['unset']), undefined); strings.set(['unset'], 'set'); assert.strictEqual(await strings.string(['unset']), 'set'); + + assert.strictEqual(await strings.string(['env']), 'env'); + assert.strictEqual(await strings.string(['argv']), 'argv'); }); }); diff --git a/libraries/botbuilder-runtime/test/settings/base.json b/libraries/botbuilder-runtime/test/settings/base.json index 578619e2e6..2dba1777c9 100644 --- a/libraries/botbuilder-runtime/test/settings/base.json +++ b/libraries/botbuilder-runtime/test/settings/base.json @@ -8,7 +8,11 @@ "bad": false, "ok": "howdy" }, + "numbers": { + "bad": "uh oh", + "ok": 10 + }, "root": { "key": "base" } -} \ No newline at end of file +} diff --git a/yarn.lock b/yarn.lock index 60346b0685..873e9ac373 100644 --- a/yarn.lock +++ b/yarn.lock @@ -13225,6 +13225,11 @@ yargs-parser@^20.2.2: resolved "https://registry.yarnpkg.com/yargs-parser/-/yargs-parser-20.2.5.tgz#5d37729146d3f894f39fc94b6796f5b239513186" integrity sha512-jYRGS3zWy20NtDtK2kBgo/TlAoy5YUuhD9/LZ7z7W4j1Fdw2cqD0xEEclf8fxc8xjD6X5Qr+qQQwCEsP8iRiYg== +yargs-parser@^20.2.7: + version "20.2.7" + resolved "https://registry.yarnpkg.com/yargs-parser/-/yargs-parser-20.2.7.tgz#61df85c113edfb5a7a4e36eb8aa60ef423cbc90a" + integrity sha512-FiNkvbeHzB/syOjIUxFDCnhSfzAL8R5vs40MgLFBorXACCOAEaWu0gRZl14vG8MR9AOJIZbmkjhusqBYZ3HTHw== + yargs-parser@^8.0.0: version "8.1.0" resolved "https://registry.yarnpkg.com/yargs-parser/-/yargs-parser-8.1.0.tgz#f1376a33b6629a5d063782944da732631e966950" From 7b2cbb00ee8f99395f8393ade8c9c31faddcb17d Mon Sep 17 00:00:00 2001 From: Josh Gummersall Date: Wed, 17 Mar 2021 17:13:04 -0700 Subject: [PATCH 2/2] fix: remove unnecessary base test config --- libraries/botbuilder-runtime/test/settings/base.json | 4 ---- 1 file changed, 4 deletions(-) diff --git a/libraries/botbuilder-runtime/test/settings/base.json b/libraries/botbuilder-runtime/test/settings/base.json index 2dba1777c9..8e5ee5983a 100644 --- a/libraries/botbuilder-runtime/test/settings/base.json +++ b/libraries/botbuilder-runtime/test/settings/base.json @@ -8,10 +8,6 @@ "bad": false, "ok": "howdy" }, - "numbers": { - "bad": "uh oh", - "ok": 10 - }, "root": { "key": "base" }