Fix constructConfigurationObject bug (#288)

* deep copy when constructing object

* use structuredClone

Author: zhiyuanliang-ms

src/appConfigurationImpl.ts

@@ -333,8 +333,8 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration {
             if (current[lastSegment] !== undefined) {
                 throw new InvalidOperationError(`Ambiguity occurs when constructing configuration object from key '${key}', value '${value}'. The key should not be part of another key.`);
             }
-            // set value to the last segment
-            current[lastSegment] = value;
+            // Deep copy object values to avoid mutating the original objects in #configMap via shared references.
+            current[lastSegment] = typeof value === "object" && value !== null ? structuredClone(value) : value;
         }
         return data;
     }
@@ -492,10 +492,8 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration {
         // Use a Map to deduplicate configuration settings by key. When multiple selectors return settings with the same key,
         // the configuration setting loaded by the later selector in the iteration order will override the one from the earlier selector.
         const loadedSettings: Map<string, ConfigurationSetting> = new Map<string, ConfigurationSetting>();
-        // deep copy selectors to avoid modification if current client fails
-        const selectorsToUpdate: PagedSettingsWatcher[] = JSON.parse(
-            JSON.stringify(selectors)
-        );
+        // Deep copy selectors to avoid modification if current client fails.
+        const selectorsToUpdate: PagedSettingsWatcher[] = structuredClone(selectors);
 
         for (const selector of selectorsToUpdate) {
             let settings: ConfigurationSetting[] = [];

test/load.test.ts

@@ -101,6 +101,18 @@ const mockedKVs = [{
     key: "keyWithEmptyTag",
     value: "valueWithEmptyTag",
     tags: {"emptyTag": ""}
+}, {
+    key: "app6_markets",
+    value: JSON.stringify({ gb: { url: "https://example.com/gb" }, dk: { url: "https://example.com/dk" } }),
+    contentType: "application/json"
+}, {
+    key: "app6_markets.markets.dk",
+    value: JSON.stringify({ currency: "DKK" }),
+    contentType: "application/json"
+}, {
+    key: "app6_markets.markets.gb",
+    value: JSON.stringify({ currency: "GBP" }),
+    contentType: "application/json"
 }
 ].map(createMockedKeyValue);
 
@@ -579,6 +591,33 @@ describe("load", function () {
         }).to.throw("Invalid separator '%'. Supported values: '.', ',', ';', '-', '_', '__', '/', ':'.");
     });
 
+    it("should call constructConfigurationObject twice without throwing for JSON content-type keys with overlapping hierarchy", async () => {
+        const connectionString = createMockedConnectionString();
+        const settings = await load(connectionString, {
+            selectors: [{
+                keyFilter: "app6_*"
+            }],
+            trimKeyPrefixes: ["app6_"]
+        });
+        expect(settings).not.undefined;
+
+        // First call should succeed
+        const data1 = settings.constructConfigurationObject({ separator: "." });
+        expect(data1).not.undefined;
+        expect(data1.markets.gb.url).eq("https://example.com/gb");
+        expect(data1.markets.dk.url).eq("https://example.com/dk");
+        expect(data1.markets.markets.dk.currency).eq("DKK");
+        expect(data1.markets.markets.gb.currency).eq("GBP");
+
+        // Second call should also succeed (idempotent)
+        const data2 = settings.constructConfigurationObject({ separator: "." });
+        expect(data2).not.undefined;
+        expect(data2.markets.gb.url).eq("https://example.com/gb");
+        expect(data2.markets.dk.url).eq("https://example.com/dk");
+        expect(data2.markets.markets.dk.currency).eq("DKK");
+        expect(data2.markets.markets.gb.currency).eq("GBP");
+    });
+
     it("should load key values from snapshot", async () => {
         const snapshotName = "Test";
         const snapshotResponses = new Map([