From d274366bd63fe0ee9d2919ba2fbefa852127c87a Mon Sep 17 00:00:00 2001 From: Zhiyuan Liang Date: Thu, 31 Jul 2025 14:34:02 +0800 Subject: [PATCH 1/3] support comment in json key value --- package-lock.json | 199 ++++++++++++++++++++++++++++++++++--- package.json | 3 +- rollup.config.mjs | 1 + src/JsonKeyValueAdapter.ts | 14 ++- test/json.test.ts | 128 +++++++++++++++++++++++- 5 files changed, 327 insertions(+), 18 deletions(-) diff --git a/package-lock.json b/package-lock.json index 550e7f3d..32a78e26 100644 --- a/package-lock.json +++ b/package-lock.json @@ -11,7 +11,8 @@ "dependencies": { "@azure/app-configuration": "^1.6.1", "@azure/identity": "^4.2.1", - "@azure/keyvault-secrets": "^4.7.0" + "@azure/keyvault-secrets": "^4.7.0", + "jsonc-parser": "^3.3.1" }, "devDependencies": { "@rollup/plugin-typescript": "^11.1.2", @@ -1229,9 +1230,9 @@ } }, "node_modules/brace-expansion": { - "version": "1.1.11", - "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.11.tgz", - "integrity": "sha512-iCuPHDFgrHX7H2vEI/5xpz07zSHB00TpugqhmYtVmMO6518mCuRMoOYFldEBl0g187ufozdaHgWKcYFb61qGiA==", + "version": "1.1.12", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.12.tgz", + "integrity": "sha512-9T9UjW3r0UW5c1Q7GTwllptXwhvYmEzFhzMfZ9H7FQWt+uZePjZPjBP/W1ZEyZ1twGWom5/56TF4lPcqjnDHcg==", "dev": true, "dependencies": { "balanced-match": "^1.0.0", @@ -1261,6 +1262,18 @@ "resolved": "https://registry.npmjs.org/buffer-equal-constant-time/-/buffer-equal-constant-time-1.0.1.tgz", "integrity": "sha512-zRpUiDwd/xk6ADqPMATG8vc9VPrkck7T07OIx0gnjmJAnHnTVXNQG3vfvWNuiZIkwu9KrKdA1iJKfsfTVxE6NA==" }, + "node_modules/call-bind-apply-helpers": { + "version": "1.0.2", + "resolved": "https://registry.npmjs.org/call-bind-apply-helpers/-/call-bind-apply-helpers-1.0.2.tgz", + "integrity": "sha512-Sp1ablJ0ivDkSzjcaJdxEunN5/XvksFJ2sMBFfq6x0ryhQV/2b/KwFe21cMpmHtPOSij8K99/wSfoEuTObmuMQ==", + "dependencies": { + "es-errors": "^1.3.0", + "function-bind": "^1.1.2" + }, + "engines": { + "node": ">= 0.4" + } + }, "node_modules/callsites": { "version": "3.1.0", "resolved": "https://registry.npmjs.org/callsites/-/callsites-3.1.0.tgz", @@ -1548,6 +1561,19 @@ "url": "https://github.com/motdotla/dotenv?sponsor=1" } }, + "node_modules/dunder-proto": { + "version": "1.0.1", + "resolved": "https://registry.npmjs.org/dunder-proto/-/dunder-proto-1.0.1.tgz", + "integrity": "sha512-KIN/nDJBQRcXw0MLVhZE9iQHmG68qAVIBg9CqmUYjmQIhgij9U5MFvrqkUL5FbtyyzZuOeOt0zdeRe4UY7ct+A==", + "dependencies": { + "call-bind-apply-helpers": "^1.0.1", + "es-errors": "^1.3.0", + "gopd": "^1.2.0" + }, + "engines": { + "node": ">= 0.4" + } + }, "node_modules/eastasianwidth": { "version": "0.2.0", "resolved": "https://registry.npmjs.org/eastasianwidth/-/eastasianwidth-0.2.0.tgz", @@ -1568,6 +1594,47 @@ "integrity": "sha512-MSjYzcWNOA0ewAHpz0MxpYFvwg6yjy1NG3xteoqz644VCo/RPgnr1/GGt+ic3iJTzQ8Eu3TdM14SawnVUmGE6A==", "dev": true }, + "node_modules/es-define-property": { + "version": "1.0.1", + "resolved": "https://registry.npmjs.org/es-define-property/-/es-define-property-1.0.1.tgz", + "integrity": "sha512-e3nRfgfUZ4rNGL232gUgX06QNyyez04KdjFrF+LTRoOXmrOgFKDg4BCdsjW8EnT69eqdYGmRpJwiPVYNrCaW3g==", + "engines": { + "node": ">= 0.4" + } + }, + "node_modules/es-errors": { + "version": "1.3.0", + "resolved": "https://registry.npmjs.org/es-errors/-/es-errors-1.3.0.tgz", + "integrity": "sha512-Zf5H2Kxt2xjTvbJvP2ZWLEICxA6j+hAmMzIlypy4xcBg1vKVnx89Wy0GbS+kf5cwCVFFzdCFh2XSCFNULS6csw==", + "engines": { + "node": ">= 0.4" + } + }, + "node_modules/es-object-atoms": { + "version": "1.1.1", + "resolved": "https://registry.npmjs.org/es-object-atoms/-/es-object-atoms-1.1.1.tgz", + "integrity": "sha512-FGgH2h8zKNim9ljj7dankFPcICIK9Cp5bm+c2gQSYePhpaG5+esrLODihIorn+Pe6FGJzWhXQotPv73jTaldXA==", + "dependencies": { + "es-errors": "^1.3.0" + }, + "engines": { + "node": ">= 0.4" + } + }, + "node_modules/es-set-tostringtag": { + "version": "2.1.0", + "resolved": "https://registry.npmjs.org/es-set-tostringtag/-/es-set-tostringtag-2.1.0.tgz", + "integrity": "sha512-j6vWzfrGVfyXxge+O0x5sh6cvxAog0a/4Rdd2K36zCMV5eJ+/+tOAngRO8cODMNWbVRdVlmGZQL2YS3yR8bIUA==", + "dependencies": { + "es-errors": "^1.3.0", + "get-intrinsic": "^1.2.6", + "has-tostringtag": "^1.0.2", + "hasown": "^2.0.2" + }, + "engines": { + "node": ">= 0.4" + } + }, "node_modules/escalade": { "version": "3.1.1", "resolved": "https://registry.npmjs.org/escalade/-/escalade-3.1.1.tgz", @@ -1900,12 +1967,14 @@ } }, "node_modules/form-data": { - "version": "4.0.0", - "resolved": "https://registry.npmjs.org/form-data/-/form-data-4.0.0.tgz", - "integrity": "sha512-ETEklSGi5t0QMZuiXoA/Q6vcnxcLQP5vdugSpuAyi6SVGi2clPPp+xgEhuMaHC+zGgn31Kd235W35f7Hykkaww==", + "version": "4.0.4", + "resolved": "https://registry.npmjs.org/form-data/-/form-data-4.0.4.tgz", + "integrity": "sha512-KrGhL9Q4zjj0kiUt5OO4Mr/A/jlI2jDYs5eHBpYHPcBEVSiipAvn2Ko2HnPe20rmcuuvMHNdZFp+4IlGTMF0Ow==", "dependencies": { "asynckit": "^0.4.0", "combined-stream": "^1.0.8", + "es-set-tostringtag": "^2.1.0", + "hasown": "^2.0.2", "mime-types": "^2.1.12" }, "engines": { @@ -1932,6 +2001,14 @@ "node": "^8.16.0 || ^10.6.0 || >=11.0.0" } }, + "node_modules/function-bind": { + "version": "1.1.2", + "resolved": "https://registry.npmjs.org/function-bind/-/function-bind-1.1.2.tgz", + "integrity": "sha512-7XHNxH7qX9xG5mIwxkhumTox/MIRNcOgDrxWsMt2pAr23WHp6MrRlN7FBSFpCpr+oVO0F744iUgR82nJMfG2SA==", + "funding": { + "url": "https://github.com/sponsors/ljharb" + } + }, "node_modules/get-caller-file": { "version": "2.0.5", "resolved": "https://registry.npmjs.org/get-caller-file/-/get-caller-file-2.0.5.tgz", @@ -1950,6 +2027,41 @@ "node": "*" } }, + "node_modules/get-intrinsic": { + "version": "1.3.0", + "resolved": "https://registry.npmjs.org/get-intrinsic/-/get-intrinsic-1.3.0.tgz", + "integrity": "sha512-9fSjSaos/fRIVIp+xSJlE6lfwhES7LNtKaCBIamHsjr2na1BiABJPo0mOjjz8GJDURarmCPGqaiVg5mfjb98CQ==", + "dependencies": { + "call-bind-apply-helpers": "^1.0.2", + "es-define-property": "^1.0.1", + "es-errors": "^1.3.0", + "es-object-atoms": "^1.1.1", + "function-bind": "^1.1.2", + "get-proto": "^1.0.1", + "gopd": "^1.2.0", + "has-symbols": "^1.1.0", + "hasown": "^2.0.2", + "math-intrinsics": "^1.1.0" + }, + "engines": { + "node": ">= 0.4" + }, + "funding": { + "url": "https://github.com/sponsors/ljharb" + } + }, + "node_modules/get-proto": { + "version": "1.0.1", + "resolved": "https://registry.npmjs.org/get-proto/-/get-proto-1.0.1.tgz", + "integrity": "sha512-sTSfBjoXBp89JvIKIefqw7U2CCebsc74kiY6awiGogKtoSGbgjYE/G/+l9sF3MWFPNc9IcoOC4ODfKHfxFmp0g==", + "dependencies": { + "dunder-proto": "^1.0.1", + "es-object-atoms": "^1.0.0" + }, + "engines": { + "node": ">= 0.4" + } + }, "node_modules/glob": { "version": "7.2.0", "resolved": "https://registry.npmjs.org/glob/-/glob-7.2.0.tgz", @@ -2017,6 +2129,17 @@ "url": "https://github.com/sponsors/sindresorhus" } }, + "node_modules/gopd": { + "version": "1.2.0", + "resolved": "https://registry.npmjs.org/gopd/-/gopd-1.2.0.tgz", + "integrity": "sha512-ZUKRh6/kUFoAiTAtTYPZJ3hw9wNxx+BIBOijnlG9PnrJsCcSjs1wyyD6vJpaYtgnzDrKYRSqf3OO6Rfa93xsRg==", + "engines": { + "node": ">= 0.4" + }, + "funding": { + "url": "https://github.com/sponsors/ljharb" + } + }, "node_modules/graphemer": { "version": "1.4.0", "resolved": "https://registry.npmjs.org/graphemer/-/graphemer-1.4.0.tgz", @@ -2041,6 +2164,42 @@ "node": ">=8" } }, + "node_modules/has-symbols": { + "version": "1.1.0", + "resolved": "https://registry.npmjs.org/has-symbols/-/has-symbols-1.1.0.tgz", + "integrity": "sha512-1cDNdwJ2Jaohmb3sg4OmKaMBwuC48sYni5HUw2DvsC8LjGTLK9h+eb1X6RyuOHe4hT0ULCW68iomhjUoKUqlPQ==", + "engines": { + "node": ">= 0.4" + }, + "funding": { + "url": "https://github.com/sponsors/ljharb" + } + }, + "node_modules/has-tostringtag": { + "version": "1.0.2", + "resolved": "https://registry.npmjs.org/has-tostringtag/-/has-tostringtag-1.0.2.tgz", + "integrity": "sha512-NqADB8VjPFLM2V0VvHUewwwsw0ZWBaIdgo+ieHtK3hasLz4qeCRjYcqfB6AQrBggRKppKF8L52/VqdVsO47Dlw==", + "dependencies": { + "has-symbols": "^1.0.3" + }, + "engines": { + "node": ">= 0.4" + }, + "funding": { + "url": "https://github.com/sponsors/ljharb" + } + }, + "node_modules/hasown": { + "version": "2.0.2", + "resolved": "https://registry.npmjs.org/hasown/-/hasown-2.0.2.tgz", + "integrity": "sha512-0hJU9SCPvmMzIBdZFqNPXWa6dqh7WdH0cII9y+CyS8rG3nL48Bclra9HmKhVVUHyPWNH5Y7xDwAB7bfgSjkUMQ==", + "dependencies": { + "function-bind": "^1.1.2" + }, + "engines": { + "node": ">= 0.4" + } + }, "node_modules/he": { "version": "1.2.0", "resolved": "https://registry.npmjs.org/he/-/he-1.2.0.tgz", @@ -2316,6 +2475,11 @@ "integrity": "sha512-ZClg6AaYvamvYEE82d3Iyd3vSSIjQ+odgjaTzRuO3s7toCdFKczob2i0zCh7JE8kWn17yvAWhUVxvqGwUalsRA==", "dev": true }, + "node_modules/jsonc-parser": { + "version": "3.3.1", + "resolved": "https://registry.npmjs.org/jsonc-parser/-/jsonc-parser-3.3.1.tgz", + "integrity": "sha512-HUgH65KyejrUFPvHFPbqOY0rsFip3Bo5wb4ngvdi1EpCYWUQDC5V+Y7mZws+DLkr4M//zQJoanu1SP+87Dv1oQ==" + }, "node_modules/jsonwebtoken": { "version": "9.0.2", "resolved": "https://registry.npmjs.org/jsonwebtoken/-/jsonwebtoken-9.0.2.tgz", @@ -2513,6 +2677,14 @@ "node": ">=12" } }, + "node_modules/math-intrinsics": { + "version": "1.1.0", + "resolved": "https://registry.npmjs.org/math-intrinsics/-/math-intrinsics-1.1.0.tgz", + "integrity": "sha512-/IXtbwEk5HTPyEwyKX6hGkYXxM9nbj64B+ilVJnC/R6B0pH5G4V3b0pVbL7DBj4tkhBAppbQUlf6F6Xl9LHu1g==", + "engines": { + "node": ">= 0.4" + } + }, "node_modules/merge2": { "version": "1.4.1", "resolved": "https://registry.npmjs.org/merge2/-/merge2-1.4.1.tgz", @@ -2613,11 +2785,10 @@ } }, "node_modules/mocha/node_modules/brace-expansion": { - "version": "2.0.1", - "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-2.0.1.tgz", - "integrity": "sha512-XnAIvQ8eM+kC6aULx6wuQiwVsnzsi9d3WxzV3FpWTGA19F621kwdbsAcFKXgKUHZWsy+mY6iL1sHTxWEFCytDA==", + "version": "2.0.2", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-2.0.2.tgz", + "integrity": "sha512-Jt0vHyM+jmUBqojB7E1NIYadt0vI0Qxjxd2TErW94wDz+E2LAm5vKMXXwg6ZZBTHPuUlDgQHKXvjGBdfcF1ZDQ==", "dev": true, - "license": "MIT", "dependencies": { "balanced-match": "^1.0.0" } @@ -3041,9 +3212,9 @@ } }, "node_modules/rimraf/node_modules/brace-expansion": { - "version": "2.0.1", - "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-2.0.1.tgz", - "integrity": "sha512-XnAIvQ8eM+kC6aULx6wuQiwVsnzsi9d3WxzV3FpWTGA19F621kwdbsAcFKXgKUHZWsy+mY6iL1sHTxWEFCytDA==", + "version": "2.0.2", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-2.0.2.tgz", + "integrity": "sha512-Jt0vHyM+jmUBqojB7E1NIYadt0vI0Qxjxd2TErW94wDz+E2LAm5vKMXXwg6ZZBTHPuUlDgQHKXvjGBdfcF1ZDQ==", "dev": true, "dependencies": { "balanced-match": "^1.0.0" diff --git a/package.json b/package.json index 4ac941b2..57e76127 100644 --- a/package.json +++ b/package.json @@ -57,6 +57,7 @@ "dependencies": { "@azure/app-configuration": "^1.6.1", "@azure/identity": "^4.2.1", - "@azure/keyvault-secrets": "^4.7.0" + "@azure/keyvault-secrets": "^4.7.0", + "jsonc-parser": "^3.3.1" } } diff --git a/rollup.config.mjs b/rollup.config.mjs index 16224a2f..0df0e168 100644 --- a/rollup.config.mjs +++ b/rollup.config.mjs @@ -11,6 +11,7 @@ export default [ "@azure/identity", "crypto", "dns/promises", + "jsonc-parser", "@microsoft/feature-management" ], input: "src/index.ts", diff --git a/src/JsonKeyValueAdapter.ts b/src/JsonKeyValueAdapter.ts index 84266069..c15bad5e 100644 --- a/src/JsonKeyValueAdapter.ts +++ b/src/JsonKeyValueAdapter.ts @@ -2,6 +2,7 @@ // Licensed under the MIT license. import { ConfigurationSetting, featureFlagContentType, secretReferenceContentType } from "@azure/app-configuration"; +import { stripComments } from "jsonc-parser"; import { parseContentType, isJsonContentType } from "./common/contentType.js"; import { IKeyValueAdapter } from "./IKeyValueAdapter.js"; @@ -26,9 +27,18 @@ export class JsonKeyValueAdapter implements IKeyValueAdapter { let parsedValue: unknown; if (setting.value !== undefined) { try { - parsedValue = JSON.parse(setting.value); + let cleanJsonValue = setting.value; + if (setting.value) { + cleanJsonValue = stripComments(setting.value); + } + parsedValue = JSON.parse(cleanJsonValue); } catch (error) { - parsedValue = setting.value; + if (error instanceof SyntaxError) { + parsedValue = setting.value; + } else { + // If the error is not a SyntaxError, rethrow it + throw error; + } } } else { parsedValue = setting.value; diff --git a/test/json.test.ts b/test/json.test.ts index a2b57907..cb829a94 100644 --- a/test/json.test.ts +++ b/test/json.test.ts @@ -60,7 +60,7 @@ describe("json", function () { createMockedJsonKeyValue("json.settings.object", "{}"), createMockedJsonKeyValue("json.settings.array", "[]"), createMockedJsonKeyValue("json.settings.number", "8"), - createMockedJsonKeyValue("json.settings.string", "string"), + createMockedJsonKeyValue("json.settings.string", "\"string\""), createMockedJsonKeyValue("json.settings.false", "false"), createMockedJsonKeyValue("json.settings.true", "true"), createMockedJsonKeyValue("json.settings.null", "null"), @@ -88,4 +88,130 @@ describe("json", function () { expect(settings.get("json.settings.emptyString")).eq("", "is empty string"); expect(settings.get("json.settings.illegalString")).eq("[unclosed", "is illegal string"); }); + + it("should parse json with single-line comments", async () => { + const jsoncValue = `{ + // This is a single-line comment + "database": { + "host": "localhost", // Another comment + "port": 5432 + }, + "debug": true + }`; + const jsoncKeyValue = createMockedJsonKeyValue("jsonc.settings.withComments", jsoncValue); + mockAppConfigurationClientListConfigurationSettings([[jsoncKeyValue]]); + + const connectionString = createMockedConnectionString(); + const settings = await load(connectionString); + expect(settings).not.undefined; + const config = settings.get("jsonc.settings.withComments"); + expect(config).not.undefined; + expect(config.database).not.undefined; + expect(config.database.host).eq("localhost"); + expect(config.database.port).eq(5432); + expect(config.debug).eq(true); + }); + + it("should parse json with multi-line comments", async () => { + const jsoncValue = `{ + /* + * This is a multi-line comment + * describing the configuration + */ + "app": { + "name": "TestApp", + /* inline multi-line comment */ "version": "1.0.0" + }, + /* + "disabled": "this entire section is commented out" + */ + "enabled": true + }`; + const jsoncKeyValue = createMockedJsonKeyValue("jsonc.settings.multilineComments", jsoncValue); + mockAppConfigurationClientListConfigurationSettings([[jsoncKeyValue]]); + + const connectionString = createMockedConnectionString(); + const settings = await load(connectionString); + expect(settings).not.undefined; + const config = settings.get("jsonc.settings.multilineComments"); + expect(config).not.undefined; + expect(config.app).not.undefined; + expect(config.app.name).eq("TestApp"); + expect(config.app.version).eq("1.0.0"); + expect(config.enabled).eq(true); + expect(config.disabled).undefined; // Should be undefined as it's commented out + }); + + it("should parse json with mixed comment types", async () => { + const jsoncValue = `{ + // Configuration for the application + "application": { + "name": "Azure App Config Test", // Application name + "version": "2.0.0", + /* + * Environment settings + * These can be overridden per environment + */ + "environment": { + "development": { + "debug": true, + "logLevel": "debug" + }, + "production": { + "debug": false, + "logLevel": "error" + } + } + }, + // Features configuration + "features": [ + "authentication", + "logging", + /* "experimental-feature", */ // Commented out feature + "monitoring" + ] + }`; + const jsoncKeyValue = createMockedJsonKeyValue("jsonc.settings.complex", jsoncValue); + mockAppConfigurationClientListConfigurationSettings([[jsoncKeyValue]]); + + const connectionString = createMockedConnectionString(); + const settings = await load(connectionString); + expect(settings).not.undefined; + const config = settings.get("jsonc.settings.complex"); + expect(config).not.undefined; + expect(config.application).not.undefined; + expect(config.application.name).eq("Azure App Config Test"); + expect(config.application.version).eq("2.0.0"); + expect(config.application.environment).not.undefined; + expect(config.application.environment.development).not.undefined; + expect(config.application.environment.development.debug).eq(true); + expect(config.application.environment.development.logLevel).eq("debug"); + expect(config.application.environment.production).not.undefined; + expect(config.application.environment.production.debug).eq(false); + expect(config.application.environment.production.logLevel).eq("error"); + expect(config.features).not.undefined; + expect(Array.isArray(config.features)).eq(true); + expect(config.features.length).eq(3); + expect(config.features[0]).eq("authentication"); + expect(config.features[1]).eq("logging"); + expect(config.features[2]).eq("monitoring"); + // Should not contain the commented out "experimental-feature" + expect(config.features.includes("experimental-feature")).eq(false); + }); + + it("should fallback to string value if json with comments parsing fails", async () => { + const invalidJsoncValue = `{ + // This is invalid JSON with unclosed bracket + "test": "value"`; + const jsoncKeyValue = createMockedJsonKeyValue("jsonc.settings.invalid", invalidJsoncValue); + mockAppConfigurationClientListConfigurationSettings([[jsoncKeyValue]]); + + const connectionString = createMockedConnectionString(); + const settings = await load(connectionString); + expect(settings).not.undefined; + const config = settings.get("jsonc.settings.invalid"); + expect(config).not.undefined; + expect(typeof config).eq("string", "should fallback to string value"); + expect(config).eq(invalidJsoncValue); + }); }); From 8c838bc66a93707f749e900879ba3fd5f7de64a4 Mon Sep 17 00:00:00 2001 From: zhiyuanliang Date: Fri, 1 Aug 2025 10:52:16 +0800 Subject: [PATCH 2/3] update testcase --- src/JsonKeyValueAdapter.ts | 6 +- test/json.test.ts | 183 ++++++++++++++----------------------- 2 files changed, 73 insertions(+), 116 deletions(-) diff --git a/src/JsonKeyValueAdapter.ts b/src/JsonKeyValueAdapter.ts index c15bad5e..725ff6de 100644 --- a/src/JsonKeyValueAdapter.ts +++ b/src/JsonKeyValueAdapter.ts @@ -27,11 +27,11 @@ export class JsonKeyValueAdapter implements IKeyValueAdapter { let parsedValue: unknown; if (setting.value !== undefined) { try { - let cleanJsonValue = setting.value; + let rawJsonValue = setting.value; if (setting.value) { - cleanJsonValue = stripComments(setting.value); + rawJsonValue = stripComments(setting.value); } - parsedValue = JSON.parse(cleanJsonValue); + parsedValue = JSON.parse(rawJsonValue); } catch (error) { if (error instanceof SyntaxError) { parsedValue = setting.value; diff --git a/test/json.test.ts b/test/json.test.ts index cb829a94..fbb5f03e 100644 --- a/test/json.test.ts +++ b/test/json.test.ts @@ -89,129 +89,86 @@ describe("json", function () { expect(settings.get("json.settings.illegalString")).eq("[unclosed", "is illegal string"); }); - it("should parse json with single-line comments", async () => { - const jsoncValue = `{ - // This is a single-line comment - "database": { - "host": "localhost", // Another comment - "port": 5432 + it("should load json values with comments", async () => { + // Test various comment styles and positions + const mixedCommentStylesValue = `{ + // Single line comment at start + "ApiSettings": { + "BaseUrl": "https://api.example.com", // Inline single line + /* Multi-line comment + spanning multiple lines */ + "ApiKey": "secret-key", + "Endpoints": [ + // Comment before array element + "/users", + /* Comment between elements */ + "/orders", + "/products" // Comment after element + ] }, - "debug": true + // Test edge cases + "StringWithSlashes": "This is not a // comment", + "StringWithStars": "This is not a /* comment */", + "UrlValue": "https://example.com/path", // This is a real comment + "EmptyComment": "value", // + /**/ + "AfterEmptyComment": "value2" + /* Final multi-line comment */ }`; - const jsoncKeyValue = createMockedJsonKeyValue("jsonc.settings.withComments", jsoncValue); - mockAppConfigurationClientListConfigurationSettings([[jsoncKeyValue]]); - const connectionString = createMockedConnectionString(); - const settings = await load(connectionString); - expect(settings).not.undefined; - const config = settings.get("jsonc.settings.withComments"); - expect(config).not.undefined; - expect(config.database).not.undefined; - expect(config.database.host).eq("localhost"); - expect(config.database.port).eq(5432); - expect(config.debug).eq(true); - }); + // Test invalid JSON with comments + const invalidJsonWithCommentsValue = `// This is a comment + { invalid json structure + // Another comment + missing quotes and braces`; - it("should parse json with multi-line comments", async () => { - const jsoncValue = `{ - /* - * This is a multi-line comment - * describing the configuration - */ - "app": { - "name": "TestApp", - /* inline multi-line comment */ "version": "1.0.0" - }, - /* - "disabled": "this entire section is commented out" - */ - "enabled": true - }`; - const jsoncKeyValue = createMockedJsonKeyValue("jsonc.settings.multilineComments", jsoncValue); - mockAppConfigurationClientListConfigurationSettings([[jsoncKeyValue]]); + // Test only comments (should be invalid JSON) + const onlyCommentsValue = ` + // Just comments + /* No actual content */ + `; - const connectionString = createMockedConnectionString(); - const settings = await load(connectionString); - expect(settings).not.undefined; - const config = settings.get("jsonc.settings.multilineComments"); - expect(config).not.undefined; - expect(config.app).not.undefined; - expect(config.app.name).eq("TestApp"); - expect(config.app.version).eq("1.0.0"); - expect(config.enabled).eq(true); - expect(config.disabled).undefined; // Should be undefined as it's commented out - }); + const keyValues = [ + createMockedJsonKeyValue("MixedCommentStyles", mixedCommentStylesValue), + createMockedJsonKeyValue("InvalidJsonWithComments", invalidJsonWithCommentsValue), + createMockedJsonKeyValue("OnlyComments", onlyCommentsValue) + ]; - it("should parse json with mixed comment types", async () => { - const jsoncValue = `{ - // Configuration for the application - "application": { - "name": "Azure App Config Test", // Application name - "version": "2.0.0", - /* - * Environment settings - * These can be overridden per environment - */ - "environment": { - "development": { - "debug": true, - "logLevel": "debug" - }, - "production": { - "debug": false, - "logLevel": "error" - } - } - }, - // Features configuration - "features": [ - "authentication", - "logging", - /* "experimental-feature", */ // Commented out feature - "monitoring" - ] - }`; - const jsoncKeyValue = createMockedJsonKeyValue("jsonc.settings.complex", jsoncValue); - mockAppConfigurationClientListConfigurationSettings([[jsoncKeyValue]]); + mockAppConfigurationClientListConfigurationSettings([keyValues]); const connectionString = createMockedConnectionString(); const settings = await load(connectionString); expect(settings).not.undefined; - const config = settings.get("jsonc.settings.complex"); - expect(config).not.undefined; - expect(config.application).not.undefined; - expect(config.application.name).eq("Azure App Config Test"); - expect(config.application.version).eq("2.0.0"); - expect(config.application.environment).not.undefined; - expect(config.application.environment.development).not.undefined; - expect(config.application.environment.development.debug).eq(true); - expect(config.application.environment.development.logLevel).eq("debug"); - expect(config.application.environment.production).not.undefined; - expect(config.application.environment.production.debug).eq(false); - expect(config.application.environment.production.logLevel).eq("error"); - expect(config.features).not.undefined; - expect(Array.isArray(config.features)).eq(true); - expect(config.features.length).eq(3); - expect(config.features[0]).eq("authentication"); - expect(config.features[1]).eq("logging"); - expect(config.features[2]).eq("monitoring"); - // Should not contain the commented out "experimental-feature" - expect(config.features.includes("experimental-feature")).eq(false); - }); - it("should fallback to string value if json with comments parsing fails", async () => { - const invalidJsoncValue = `{ - // This is invalid JSON with unclosed bracket - "test": "value"`; - const jsoncKeyValue = createMockedJsonKeyValue("jsonc.settings.invalid", invalidJsoncValue); - mockAppConfigurationClientListConfigurationSettings([[jsoncKeyValue]]); - - const connectionString = createMockedConnectionString(); - const settings = await load(connectionString); - expect(settings).not.undefined; - const config = settings.get("jsonc.settings.invalid"); - expect(config).not.undefined; - expect(typeof config).eq("string", "should fallback to string value"); - expect(config).eq(invalidJsoncValue); + // Verify mixed comment styles are properly parsed + const mixedConfig = settings.get("MixedCommentStyles"); + expect(mixedConfig).not.undefined; + expect(mixedConfig.ApiSettings).not.undefined; + expect(mixedConfig.ApiSettings.BaseUrl).eq("https://api.example.com"); + expect(mixedConfig.ApiSettings.ApiKey).eq("secret-key"); + expect(mixedConfig.ApiSettings.Endpoints).not.undefined; + expect(Array.isArray(mixedConfig.ApiSettings.Endpoints)).eq(true); + expect(mixedConfig.ApiSettings.Endpoints[0]).eq("/users"); + expect(mixedConfig.ApiSettings.Endpoints[1]).eq("/orders"); + expect(mixedConfig.ApiSettings.Endpoints[2]).eq("/products"); + + // Verify edge cases where comment-like text appears in strings + expect(mixedConfig.StringWithSlashes).eq("This is not a // comment"); + expect(mixedConfig.StringWithStars).eq("This is not a /* comment */"); + expect(mixedConfig.UrlValue).eq("https://example.com/path"); + expect(mixedConfig.EmptyComment).eq("value"); + expect(mixedConfig.AfterEmptyComment).eq("value2"); + + // Invalid JSON should fall back to string value + const invalidConfig = settings.get("InvalidJsonWithComments"); + expect(invalidConfig).not.undefined; + expect(typeof invalidConfig).eq("string"); + expect(invalidConfig).eq(invalidJsonWithCommentsValue); + + // Only comments should be treated as string value (invalid JSON) + const onlyCommentsConfig = settings.get("OnlyComments"); + expect(onlyCommentsConfig).not.undefined; + expect(typeof onlyCommentsConfig).eq("string"); + expect(onlyCommentsConfig).eq(onlyCommentsValue); }); }); From 1ea1ff0a9695ef2398c486dae74c383a737db79c Mon Sep 17 00:00:00 2001 From: Zhiyuan Liang Date: Wed, 6 Aug 2025 11:22:35 +0800 Subject: [PATCH 3/3] strip comment only when normal parse failed --- src/JsonKeyValueAdapter.ts | 34 +++++++++++++++++++++------------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/src/JsonKeyValueAdapter.ts b/src/JsonKeyValueAdapter.ts index 725ff6de..75ecaffc 100644 --- a/src/JsonKeyValueAdapter.ts +++ b/src/JsonKeyValueAdapter.ts @@ -26,22 +26,19 @@ export class JsonKeyValueAdapter implements IKeyValueAdapter { async processKeyValue(setting: ConfigurationSetting): Promise<[string, unknown]> { let parsedValue: unknown; if (setting.value !== undefined) { - try { - let rawJsonValue = setting.value; - if (setting.value) { - rawJsonValue = stripComments(setting.value); - } - parsedValue = JSON.parse(rawJsonValue); - } catch (error) { - if (error instanceof SyntaxError) { - parsedValue = setting.value; + const parseResult = this.#tryParseJson(setting.value); + if (parseResult.success) { + parsedValue = parseResult.result; + } else { + // Try parsing with comments stripped + const parseWithoutCommentsResult = this.#tryParseJson(stripComments(setting.value)); + if (parseWithoutCommentsResult.success) { + parsedValue = parseWithoutCommentsResult.result; } else { - // If the error is not a SyntaxError, rethrow it - throw error; + // If still not valid JSON, return the original value + parsedValue = setting.value; } } - } else { - parsedValue = setting.value; } return [setting.key, parsedValue]; } @@ -49,4 +46,15 @@ export class JsonKeyValueAdapter implements IKeyValueAdapter { async onChangeDetected(): Promise { return; } + + #tryParseJson(value: string): { success: true; result: unknown } | { success: false } { + try { + return { success: true, result: JSON.parse(value) }; + } catch (error) { + if (error instanceof SyntaxError) { + return { success: false }; + } + throw error; + } + } }