From c0897ee1b88742baf6eb8960e49f6f27984ce115 Mon Sep 17 00:00:00 2001
From: Josh Gummersall <jgummersall@microsoft.com>
Date: Thu, 8 Jul 2021 14:01:25 -0700
Subject: [PATCH] fix: configuration key handling (#3855)

* fix: configuration key handling

Also adds some tests to ensure future consistency.

* unique values for assertions

Co-authored-by: Michael Richardson <40401643+mdrichardson@users.noreply.github.com>

* clarify comment

* fix prettier issue

Co-authored-by: Michael Richardson <40401643+mdrichardson@users.noreply.github.com>
---
 .../src/configuration.ts                      | 21 +++++++-
 .../test/configuration.test.ts                | 49 +++++++++++++++++++
 2 files changed, 69 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..8fb0596298 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<string, unknown>) {
+        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<T = unknown>(path: string[] = []): T | undefined {
-        return this.provider.get(this.key(path));
+        // 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);
     }
 
     /**
@@ -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..711ea88e44 100644
--- a/libraries/botbuilder-dialogs-adaptive-runtime/test/configuration.test.ts
+++ b/libraries/botbuilder-dialogs-adaptive-runtime/test/configuration.test.ts
@@ -54,4 +54,53 @@ describe('Configuration', function () {
             assert.strictEqual(layered.get(['root', 'key']), 'layer');
         });
     });
+
+    describe('keys', function () {
+        const configuration = new Configuration({
+            key: 'value',
+            one: {
+                key: 'value-one',
+                two: {
+                    key: 'value-two',
+                },
+            },
+        });
+
+        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-one');
+            });
+
+            it('yields all values for a key', function () {
+                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-one', two: { key: 'value-two' } },
+                });
+            });
+        });
+
+        describe('prefixed', function () {
+            it('yields a value for a key', function () {
+                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-two',
+                });
+            });
+
+            it('yields all values for no key', function () {
+                assert.deepStrictEqual(configuration.bind(['one']).get(), {
+                    key: 'value-one',
+                    two: { key: 'value-two' },
+                });
+            });
+        });
+    });
 });