From 2a065ec9fc421f98c777971c53884f99bbafc725 Mon Sep 17 00:00:00 2001 From: Josh Gummersall Date: Thu, 8 Jul 2021 13:02:44 -0700 Subject: [PATCH 1/4] fix: configuration key handling Also adds some tests to ensure future consistency. --- .../src/configuration.ts | 21 ++++++++- .../test/configuration.test.ts | 46 +++++++++++++++++++ 2 files changed, 66 insertions(+), 1 deletion(-) diff --git a/libraries/botbuilder-dialogs-adaptive-runtime/src/configuration.ts b/libraries/botbuilder-dialogs-adaptive-runtime/src/configuration.ts index cefcb5e029..f6efbfdad5 100644 --- a/libraries/botbuilder-dialogs-adaptive-runtime/src/configuration.ts +++ b/libraries/botbuilder-dialogs-adaptive-runtime/src/configuration.ts @@ -9,11 +9,24 @@ import { Provider } from 'nconf'; /** * Configuration implements the [IConfiguration](xref:botbuilder-dialogs-adaptive-runtime-core.IConfiguration) * interface and adds helper methods for setting values, layering sources, and getting type checked values. + * + * @internal */ export class Configuration implements CoreConfiguration { private prefix: string[] = []; private provider = new Provider().use('memory'); + /** + * Create a configuration instance + * + * @param initialValues Optional set of default values to provide + */ + constructor(initialValues?: Record) { + if (initialValues) { + Object.entries(initialValues).forEach(([key, value]) => this.provider.set(key, value)); + } + } + /** * Bind a path to a Configuration instance such that calls to get or set will * automatically include the bound path as a prefix. @@ -41,7 +54,9 @@ export class Configuration implements CoreConfiguration { * @returns the value, or undefined */ get(path: string[] = []): T | undefined { - return this.provider.get(this.key(path)); + // Note: `|| undefined` ensures that empty string yields "undefined" as + // this ensures nconf returns the entire merged configuration. + return this.provider.get(this.key(path) || undefined); } /** @@ -51,6 +66,10 @@ export class Configuration implements CoreConfiguration { * @param value value to set */ set(path: string[], value: unknown): void { + if (!path.length) { + throw new Error('`path` must be non-empty'); + } + this.provider.set(this.key(path), value); } diff --git a/libraries/botbuilder-dialogs-adaptive-runtime/test/configuration.test.ts b/libraries/botbuilder-dialogs-adaptive-runtime/test/configuration.test.ts index f733729c3e..92ffcac6e4 100644 --- a/libraries/botbuilder-dialogs-adaptive-runtime/test/configuration.test.ts +++ b/libraries/botbuilder-dialogs-adaptive-runtime/test/configuration.test.ts @@ -54,4 +54,50 @@ describe('Configuration', function () { assert.strictEqual(layered.get(['root', 'key']), 'layer'); }); }); + + describe('keys', function () { + const configuration = new Configuration({ + key: 'value', + one: { + key: 'value', + two: { + key: 'value', + }, + }, + }); + + describe('non-prefixed', function () { + it('yields a value for a key', function () { + assert.strictEqual(configuration.get(['key']), 'value'); + assert.strictEqual(configuration.get(['one', 'key']), 'value'); + }); + + it('yields all values for a key', function () { + assert.deepStrictEqual(configuration.get(['one']), { key: 'value', two: { key: 'value' } }); + }); + + it('yields all values for no key', function () { + assert.deepStrictEqual(configuration.get(), { + key: 'value', + one: { key: 'value', two: { key: 'value' } }, + }); + }); + }); + + describe('prefixed', function () { + it('yields a value for a key', function () { + assert.strictEqual(configuration.bind(['one']).get(['key']), 'value'); + }); + + it('yields all values for a key', function () { + assert.deepStrictEqual(configuration.bind(['one']).get(['two']), { + key: 'value', + }); + }); + + it('yields all values for no key', function () { + assert.deepStrictEqual(configuration.bind(['one']).get(), { key: 'value', two: { key: 'value' } }); + }); + }); + }); }); From dc298753e682435585375be13aa621ba57e7d3d1 Mon Sep 17 00:00:00 2001 From: Josh Gummersall Date: Thu, 8 Jul 2021 13:41:51 -0700 Subject: [PATCH 2/4] unique values for assertions Co-authored-by: Michael Richardson <40401643+mdrichardson@users.noreply.github.com> --- .../test/configuration.test.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/libraries/botbuilder-dialogs-adaptive-runtime/test/configuration.test.ts b/libraries/botbuilder-dialogs-adaptive-runtime/test/configuration.test.ts index 92ffcac6e4..531189a5a1 100644 --- a/libraries/botbuilder-dialogs-adaptive-runtime/test/configuration.test.ts +++ b/libraries/botbuilder-dialogs-adaptive-runtime/test/configuration.test.ts @@ -59,9 +59,9 @@ describe('Configuration', function () { const configuration = new Configuration({ key: 'value', one: { - key: 'value', + key: 'value-one', two: { - key: 'value', + key: 'value-two', }, }, }); @@ -69,34 +69,34 @@ describe('Configuration', function () { describe('non-prefixed', function () { it('yields a value for a key', function () { assert.strictEqual(configuration.get(['key']), 'value'); - assert.strictEqual(configuration.get(['one', 'key']), 'value'); + assert.strictEqual(configuration.get(['one', 'key']), 'value-one'); }); it('yields all values for a key', function () { - assert.deepStrictEqual(configuration.get(['one']), { key: 'value', two: { key: 'value' } }); + assert.deepStrictEqual(configuration.get(['one']), { key: 'value-one', two: { key: 'value-two' } }); }); it('yields all values for no key', function () { assert.deepStrictEqual(configuration.get(), { key: 'value', - one: { key: 'value', two: { key: 'value' } }, + one: { key: 'value-one', two: { key: 'value-two' } }, }); }); }); describe('prefixed', function () { it('yields a value for a key', function () { - assert.strictEqual(configuration.bind(['one']).get(['key']), 'value'); + assert.strictEqual(configuration.bind(['one']).get(['key']), 'value-one'); }); it('yields all values for a key', function () { assert.deepStrictEqual(configuration.bind(['one']).get(['two']), { - key: 'value', + key: 'value-two', }); }); it('yields all values for no key', function () { - assert.deepStrictEqual(configuration.bind(['one']).get(), { key: 'value', two: { key: 'value' } }); + assert.deepStrictEqual(configuration.bind(['one']).get(), { key: 'value-one', two: { key: 'value-two' } }); }); }); }); From dab4ce9841618763464dd759276607f657c9d395 Mon Sep 17 00:00:00 2001 From: Josh Gummersall Date: Thu, 8 Jul 2021 13:44:19 -0700 Subject: [PATCH 3/4] clarify comment --- .../botbuilder-dialogs-adaptive-runtime/src/configuration.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libraries/botbuilder-dialogs-adaptive-runtime/src/configuration.ts b/libraries/botbuilder-dialogs-adaptive-runtime/src/configuration.ts index f6efbfdad5..8fb0596298 100644 --- a/libraries/botbuilder-dialogs-adaptive-runtime/src/configuration.ts +++ b/libraries/botbuilder-dialogs-adaptive-runtime/src/configuration.ts @@ -54,8 +54,8 @@ export class Configuration implements CoreConfiguration { * @returns the value, or undefined */ get(path: string[] = []): T | undefined { - // Note: `|| undefined` ensures that empty string yields "undefined" as - // this ensures nconf returns the entire merged configuration. + // Note: `|| undefined` ensures that empty string is coerced to undefined + // which ensures nconf returns the entire merged configuration. return this.provider.get(this.key(path) || undefined); } From 36aed45aca5ce00f34d110e71117c68c2662521c Mon Sep 17 00:00:00 2001 From: Josh Gummersall Date: Thu, 8 Jul 2021 13:45:40 -0700 Subject: [PATCH 4/4] fix prettier issue --- .../test/configuration.test.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/libraries/botbuilder-dialogs-adaptive-runtime/test/configuration.test.ts b/libraries/botbuilder-dialogs-adaptive-runtime/test/configuration.test.ts index 531189a5a1..711ea88e44 100644 --- a/libraries/botbuilder-dialogs-adaptive-runtime/test/configuration.test.ts +++ b/libraries/botbuilder-dialogs-adaptive-runtime/test/configuration.test.ts @@ -96,7 +96,10 @@ describe('Configuration', function () { }); it('yields all values for no key', function () { - assert.deepStrictEqual(configuration.bind(['one']).get(), { key: 'value-one', two: { key: 'value-two' } }); + assert.deepStrictEqual(configuration.bind(['one']).get(), { + key: 'value-one', + two: { key: 'value-two' }, + }); }); }); });