Skip to content

Commit 28a5c69

Browse files
EskibearEskibear
authored
Refactor AzureAppConfiguration with new interfaces (#49)
1 parent 45c8266 commit 28a5c69

File tree

4 files changed

+250
-21
lines changed

4 files changed

+250
-21
lines changed

‎src/AzureAppConfiguration.ts‎

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,4 +16,29 @@ export type AzureAppConfiguration = {
1616
* @param thisArg - Optional. Value to use as `this` when executing callback.
1717
*/
1818
onRefresh(listener: () => any, thisArg?: any): Disposable;
19-
} & ReadonlyMap<string, any>;
19+
} & IGettable & IConfigurationObject;
20+
21+
interface IConfigurationObject {
22+
/**
23+
* Construct configuration object based on Map-styled data structure and hierarchical keys.
24+
* @param options - The options to control the conversion behavior.
25+
*/
26+
constructConfigurationObject(options?: ConfigurationObjectConstructionOptions): Record<string, any>;
27+
}
28+
29+
export interface ConfigurationObjectConstructionOptions {
30+
/**
31+
* The separator to use when converting hierarchical keys to object properties.
32+
* Supported values: '.', ',', ';', '-', '_', '__', '/', ':'.
33+
* If separator is undefined, '.' will be used by default.
34+
*/
35+
separator?: "." | "," | ";" | "-" | "_" | "__" | "/" | ":";
36+
}
37+
38+
interface IGettable {
39+
/**
40+
* Get the value of a key-value from the Map-styled data structure.
41+
* @param key - The key of the key-value to be retrieved.
42+
*/
43+
get<T>(key: string): T | undefined;
44+
}

‎src/AzureAppConfigurationImpl.ts‎

Lines changed: 55 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33

44
import { AppConfigurationClient, ConfigurationSetting, ConfigurationSettingId, GetConfigurationSettingOptions, GetConfigurationSettingResponse, ListConfigurationSettingsOptions } from "@azure/app-configuration";
55
import { RestError } from "@azure/core-rest-pipeline";
6-
import { AzureAppConfiguration } from "./AzureAppConfiguration";
6+
import { AzureAppConfiguration, ConfigurationObjectConstructionOptions } from "./AzureAppConfiguration";
77
import { AzureAppConfigurationOptions } from "./AzureAppConfigurationOptions";
88
import { IKeyValueAdapter } from "./IKeyValueAdapter";
99
import { JsonKeyValueAdapter } from "./JsonKeyValueAdapter";
@@ -15,7 +15,12 @@ import { CorrelationContextHeaderName } from "./requestTracing/constants";
1515
import { createCorrelationContextHeader, requestTracingEnabled } from "./requestTracing/utils";
1616
import { KeyFilter, LabelFilter, SettingSelector } from "./types";
1717

18-
export class AzureAppConfigurationImpl extends Map<string, any> implements AzureAppConfiguration {
18+
export class AzureAppConfigurationImpl implements AzureAppConfiguration {
19+
/**
20+
* Hosting key-value pairs in the configuration store.
21+
*/
22+
#configMap: Map<string, any> = new Map<string, any>();
23+
1924
#adapters: IKeyValueAdapter[] = [];
2025
/**
2126
* Trim key prefixes sorted in descending order.
@@ -40,7 +45,6 @@ export class AzureAppConfigurationImpl extends Map<string, any> implements Azure
4045
client: AppConfigurationClient,
4146
options: AzureAppConfigurationOptions | undefined
4247
) {
43-
super();
4448
this.#client = client;
4549
this.#options = options;
4650

@@ -87,6 +91,9 @@ export class AzureAppConfigurationImpl extends Map<string, any> implements Azure
8791
this.#adapters.push(new JsonKeyValueAdapter());
8892
}
8993

94+
get<T>(key: string): T | undefined {
95+
return this.#configMap.get(key);
96+
}
9097

9198
get #refreshEnabled(): boolean {
9299
return !!this.#options?.refreshOptions?.enabled;
@@ -157,9 +164,9 @@ export class AzureAppConfigurationImpl extends Map<string, any> implements Azure
157164
keyValues.push([key, value]);
158165
}
159166

160-
this.clear(); // clear existing key-values in case of configuration setting deletion
167+
this.#configMap.clear(); // clear existing key-values in case of configuration setting deletion
161168
for (const [k, v] of keyValues) {
162-
this.set(k, v);
169+
this.#configMap.set(k, v);
163170
}
164171
}
165172

@@ -168,11 +175,53 @@ export class AzureAppConfigurationImpl extends Map<string, any> implements Azure
168175
*/
169176
async load() {
170177
await this.#loadSelectedAndWatchedKeyValues();
171-
172178
// Mark all settings have loaded at startup.
173179
this.#isInitialLoadCompleted = true;
174180
}
175181

182+
/**
183+
* Construct hierarchical data object from map.
184+
*/
185+
constructConfigurationObject(options?: ConfigurationObjectConstructionOptions): Record<string, any> {
186+
const separator = options?.separator ?? ".";
187+
const validSeparators = [".", ",", ";", "-", "_", "__", "/", ":"];
188+
if (!validSeparators.includes(separator)) {
189+
throw new Error(`Invalid separator '${separator}'. Supported values: ${validSeparators.map(s => `'${s}'`).join(", ")}.`);
190+
}
191+
192+
// construct hierarchical data object from map
193+
const data: Record<string, any> = {};
194+
for (const [key, value] of this.#configMap) {
195+
const segments = key.split(separator);
196+
let current = data;
197+
// construct hierarchical data object along the path
198+
for (let i = 0; i < segments.length - 1; i++) {
199+
const segment = segments[i];
200+
// undefined or empty string
201+
if (!segment) {
202+
throw new Error(`invalid key: ${key}`);
203+
}
204+
// create path if not exist
205+
if (current[segment] === undefined) {
206+
current[segment] = {};
207+
}
208+
// The path has been occupied by a non-object value, causing ambiguity.
209+
if (typeof current[segment] !== "object") {
210+
throw new Error(`Ambiguity occurs when constructing configuration object from key '${key}', value '${value}'. The path '${segments.slice(0, i + 1).join(separator)}' has been occupied.`);
211+
}
212+
current = current[segment];
213+
}
214+
215+
const lastSegment = segments[segments.length - 1];
216+
if (current[lastSegment] !== undefined) {
217+
throw new Error(`Ambiguity occurs when constructing configuration object from key '${key}', value '${value}'. The key should not be part of another key.`);
218+
}
219+
// set value to the last segment
220+
current[lastSegment] = value;
221+
}
222+
return data;
223+
}
224+
176225
/**
177226
* Refresh the configuration store.
178227
*/

‎test/json.test.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ describe("json", function () {
2525
const connectionString = createMockedConnectionString();
2626
const settings = await load(connectionString);
2727
expect(settings).not.undefined;
28-
const logging = settings.get("json.settings.logging");
28+
const logging = settings.get<any>("json.settings.logging");
2929
expect(logging).not.undefined;
3030
expect(logging.Test).not.undefined;
3131
expect(logging.Test.Level).eq("Debug");
@@ -43,7 +43,7 @@ describe("json", function () {
4343
}
4444
});
4545
expect(settings).not.undefined;
46-
const resolvedSecret = settings.get("TestKey");
46+
const resolvedSecret = settings.get<any>("TestKey");
4747
expect(resolvedSecret).not.undefined;
4848
expect(resolvedSecret.uri).undefined;
4949
expect(typeof resolvedSecret).eq("string");
@@ -74,7 +74,7 @@ describe("json", function () {
7474
const settings = await load(connectionString);
7575
expect(settings).not.undefined;
7676
expect(typeof settings.get("json.settings.object")).eq("object", "is object");
77-
expect(Object.keys(settings.get("json.settings.object")).length).eq(0, "is empty object");
77+
expect(Object.keys(settings.get<any>("json.settings.object")).length).eq(0, "is empty object");
7878
expect(Array.isArray(settings.get("json.settings.array"))).eq(true, "is array");
7979
expect(settings.get("json.settings.number")).eq(8, "is number");
8080
expect(settings.get("json.settings.string")).eq("string", "is string");

‎test/load.test.ts‎

Lines changed: 166 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,18 @@ const mockedKVs = [{
1414
}, {
1515
key: "app.settings.fontSize",
1616
value: "40",
17+
}, {
18+
key: "app/settings/fontColor",
19+
value: "red",
20+
}, {
21+
key: "app/settings/fontSize",
22+
value: "40",
23+
}, {
24+
key: "app%settings%fontColor",
25+
value: "red",
26+
}, {
27+
key: "app%settings%fontSize",
28+
value: "40",
1729
}, {
1830
key: "TestKey",
1931
label: "Test",
@@ -28,7 +40,30 @@ const mockedKVs = [{
2840
}, {
2941
key: "KeyForEmptyValue",
3042
value: "",
31-
}].map(createMockedKeyValue);
43+
}, {
44+
key: "app2.settings",
45+
value: JSON.stringify({ fontColor: "blue", fontSize: 20 }),
46+
contentType: "application/json"
47+
}, {
48+
key: "app3.settings",
49+
value: "placeholder"
50+
}, {
51+
key: "app3.settings.fontColor",
52+
value: "yellow"
53+
}, {
54+
key: "app4.excludedFolders.0",
55+
value: "node_modules"
56+
}, {
57+
key: "app4.excludedFolders.1",
58+
value: "dist"
59+
}, {
60+
key: "app5.settings.fontColor",
61+
value: "yellow"
62+
}, {
63+
key: "app5.settings",
64+
value: "placeholder"
65+
}
66+
].map(createMockedKeyValue);
3267

3368
describe("load", function () {
3469
this.timeout(10000);
@@ -85,11 +120,8 @@ describe("load", function () {
85120
trimKeyPrefixes: ["app.settings."]
86121
});
87122
expect(settings).not.undefined;
88-
expect(settings.has("fontColor")).eq(true);
89123
expect(settings.get("fontColor")).eq("red");
90-
expect(settings.has("fontSize")).eq(true);
91124
expect(settings.get("fontSize")).eq("40");
92-
expect(settings.has("TestKey")).eq(false);
93125
});
94126

95127
it("should trim longest key prefix first", async () => {
@@ -102,20 +134,15 @@ describe("load", function () {
102134
trimKeyPrefixes: ["app.", "app.settings.", "Test"]
103135
});
104136
expect(settings).not.undefined;
105-
expect(settings.has("fontColor")).eq(true);
106137
expect(settings.get("fontColor")).eq("red");
107-
expect(settings.has("fontSize")).eq(true);
108138
expect(settings.get("fontSize")).eq("40");
109-
expect(settings.has("TestKey")).eq(false);
110139
});
111140

112141
it("should support null/empty value", async () => {
113142
const connectionString = createMockedConnectionString();
114143
const settings = await load(connectionString);
115144
expect(settings).not.undefined;
116-
expect(settings.has("KeyForNullValue")).eq(true);
117145
expect(settings.get("KeyForNullValue")).eq(null);
118-
expect(settings.has("KeyForEmptyValue")).eq(true);
119146
expect(settings.get("KeyForEmptyValue")).eq("");
120147
});
121148

@@ -153,7 +180,6 @@ describe("load", function () {
153180
}]
154181
});
155182
expect(settings).not.undefined;
156-
expect(settings.has("TestKey")).eq(true);
157183
expect(settings.get("TestKey")).eq("TestValueForProd");
158184
});
159185

@@ -172,8 +198,137 @@ describe("load", function () {
172198
}]
173199
});
174200
expect(settings).not.undefined;
175-
expect(settings.has("TestKey")).eq(true);
176201
expect(settings.get("TestKey")).eq("TestValueForProd");
177202
});
178203

204+
// access data property
205+
it("should directly access data property", async () => {
206+
const connectionString = createMockedConnectionString();
207+
const settings = await load(connectionString, {
208+
selectors: [{
209+
keyFilter: "app.settings.*"
210+
}]
211+
});
212+
expect(settings).not.undefined;
213+
const data = settings.constructConfigurationObject();
214+
expect(data).not.undefined;
215+
expect(data.app.settings.fontColor).eq("red");
216+
expect(data.app.settings.fontSize).eq("40");
217+
});
218+
219+
it("should access property of JSON object content-type with data accessor", async () => {
220+
const connectionString = createMockedConnectionString();
221+
const settings = await load(connectionString, {
222+
selectors: [{
223+
keyFilter: "app2.*"
224+
}]
225+
});
226+
expect(settings).not.undefined;
227+
const data = settings.constructConfigurationObject();
228+
expect(data).not.undefined;
229+
expect(data.app2.settings.fontColor).eq("blue");
230+
expect(data.app2.settings.fontSize).eq(20);
231+
});
232+
233+
it("should not access property of JSON content-type object with get()", async () => {
234+
const connectionString = createMockedConnectionString();
235+
const settings = await load(connectionString, {
236+
selectors: [{
237+
keyFilter: "app2.*"
238+
}]
239+
});
240+
expect(settings).not.undefined;
241+
expect(settings.get("app2.settings")).not.undefined; // JSON object accessed as a whole
242+
expect(settings.get("app2.settings.fontColor")).undefined;
243+
expect(settings.get("app2.settings.fontSize")).undefined;
244+
});
245+
246+
/**
247+
* Edge case: Hierarchical key-value pairs with overlapped key prefix.
248+
* key: "app3.settings" => value: "placeholder"
249+
* key: "app3.settings.fontColor" => value: "yellow"
250+
*
251+
* get() will return "placeholder" for "app3.settings" and "yellow" for "app3.settings.fontColor", as expected.
252+
* data.app3.settings will return "placeholder" as a whole JSON object, which is not guarenteed to be correct.
253+
*/
254+
it("Edge case 1: Hierarchical key-value pairs with overlapped key prefix.", async () => {
255+
const connectionString = createMockedConnectionString();
256+
const settings = await load(connectionString, {
257+
selectors: [{
258+
keyFilter: "app3.settings*"
259+
}]
260+
});
261+
expect(settings).not.undefined;
262+
expect(() => {
263+
settings.constructConfigurationObject();
264+
}).to.throw("Ambiguity occurs when constructing configuration object from key 'app3.settings.fontColor', value 'yellow'. The path 'app3.settings' has been occupied.");
265+
});
266+
267+
/**
268+
* Edge case: Hierarchical key-value pairs with overlapped key prefix.
269+
* key: "app5.settings.fontColor" => value: "yellow"
270+
* key: "app5.settings" => value: "placeholder"
271+
*
272+
* When ocnstructConfigurationObject() is called, it first constructs from key "app5.settings.fontColor" and then from key "app5.settings".
273+
* An error will be thrown when constructing from key "app5.settings" because there is ambiguity between the two keys.
274+
*/
275+
it("Edge case 1: Hierarchical key-value pairs with overlapped key prefix.", async () => {
276+
const connectionString = createMockedConnectionString();
277+
const settings = await load(connectionString, {
278+
selectors: [{
279+
keyFilter: "app5.settings*"
280+
}]
281+
});
282+
expect(settings).not.undefined;
283+
expect(() => {
284+
settings.constructConfigurationObject();
285+
}).to.throw("Ambiguity occurs when constructing configuration object from key 'app5.settings', value 'placeholder'. The key should not be part of another key.");
286+
});
287+
288+
it("should construct configuration object with array", async () => {
289+
const connectionString = createMockedConnectionString();
290+
const settings = await load(connectionString, {
291+
selectors: [{
292+
keyFilter: "app4.*"
293+
}]
294+
});
295+
expect(settings).not.undefined;
296+
const data = settings.constructConfigurationObject();
297+
expect(data).not.undefined;
298+
// Both { '0': 'node_modules', '1': 'dist' } and ['node_modules', 'dist'] are valid.
299+
expect(data.app4.excludedFolders[0]).eq("node_modules");
300+
expect(data.app4.excludedFolders[1]).eq("dist");
301+
});
302+
303+
it("should construct configuration object with customized separator", async () => {
304+
const connectionString = createMockedConnectionString();
305+
const settings = await load(connectionString, {
306+
selectors: [{
307+
keyFilter: "app/settings/*"
308+
}]
309+
});
310+
expect(settings).not.undefined;
311+
const data = settings.constructConfigurationObject({ separator: "/" });
312+
expect(data).not.undefined;
313+
expect(data.app.settings.fontColor).eq("red");
314+
expect(data.app.settings.fontSize).eq("40");
315+
});
316+
317+
it("should throw error when construct configuration object with invalid separator", async () => {
318+
const connectionString = createMockedConnectionString();
319+
const settings = await load(connectionString, {
320+
selectors: [{
321+
keyFilter: "app%settings%*"
322+
}]
323+
});
324+
expect(settings).not.undefined;
325+
326+
expect(() => {
327+
// Below line will throw error because of type checking, i.e. Type '"%"' is not assignable to type '"/" | "." | "," | ";" | "-" | "_" | "__" | ":" | undefined'.ts(2322)
328+
// Here force to turn if off for testing purpose, as JavaScript does not have type checking.
329+
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
330+
// @ts-ignore
331+
settings.constructConfigurationObject({ separator: "%" });
332+
}).to.throw("Invalid separator '%'. Supported values: '.', ',', ';', '-', '_', '__', '/', ':'.");
333+
});
179334
});

0 commit comments

Comments
 (0)