Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: config argv source, runtime port handling #3406

Merged
merged 3 commits into from
Mar 18, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 22 additions & 7 deletions libraries/botbuilder-runtime-integration-express/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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),
});

/**
Expand Down Expand Up @@ -60,11 +60,23 @@ export async function makeApp(
configuration: Configuration,
options: Partial<Options> = {}
): Promise<[app: express.Application, listen: (callback?: () => void) => http.Server]> {
const { port, messagingEndpointPath } = TypedOptions.check(Object.assign({}, defaultOptions, options));
const configOverrides: Partial<Options> = {};

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);
});
Expand Down Expand Up @@ -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');
Expand Down
41 changes: 28 additions & 13 deletions libraries/botbuilder-runtime-integration-restify/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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),
});

/**
Expand All @@ -24,10 +24,24 @@ const TypedOptions = t.Record({
export type Options = t.Static<typeof TypedOptions>;

const defaultOptions: Options = {
port: 3978,
messagingEndpointPath: '/api/messages',
port: 3978,
};

async function resolveOptions(options: Partial<Options>, configuration: Configuration): Promise<Options> {
const configOverrides: Partial<Options> = {};

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.
*
Expand All @@ -40,14 +54,13 @@ export async function start(
settingsDirectory: string,
options: Partial<Options> = {}
): Promise<void> {
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}`));
}

/**
Expand All @@ -63,12 +76,14 @@ export async function makeServer(
configuration: Configuration,
options: Partial<Options> = {}
): Promise<restify.Server> {
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);
});
Expand Down
3 changes: 2 additions & 1 deletion libraries/botbuilder-runtime/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
18 changes: 18 additions & 0 deletions libraries/botbuilder-runtime/src/configuration.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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.
*
Expand Down
1 change: 1 addition & 0 deletions libraries/botbuilder-runtime/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'));
Expand Down
10 changes: 10 additions & 0 deletions libraries/botbuilder-runtime/test/configuration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};

Expand All @@ -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');
});
});

Expand Down
2 changes: 1 addition & 1 deletion libraries/botbuilder-runtime/test/settings/base.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,4 @@
"root": {
"key": "base"
}
}
}
5 changes: 5 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -13230,6 +13230,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"
Expand Down