Skip to content

Commit 1fea101

Browse files
committed
src: cache compiled node.config.json schema and align with runtime
1 parent 2ded636 commit 1fea101

4 files changed

Lines changed: 39 additions & 27 deletions

File tree

src/node_config_file.cc

Lines changed: 17 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -258,30 +258,25 @@ ParseResult ConfigReader::ParseConfig(const std::string_view& config_path) {
258258
return ParseResult::InvalidContent;
259259
}
260260

261-
// Validate against JSON Schema after basic parsing succeeds.
262-
// This catches type errors in properties before the option
263-
// parsing loop, which would otherwise produce less clear messages.
261+
// Validate against JSON Schema after basic parsing succeeds. This catches
262+
// unknown namespaces and type errors in properties before the option parsing
263+
// loop, which would otherwise produce less clear messages (or silently skip
264+
// unknown top-level keys). kNodeConfigSchema is a compile-time constant, so
265+
// compile it once on first call.
264266
{
265-
auto schema = ata::compile(kNodeConfigSchema);
266-
if (schema) {
267-
auto result = ata::validate(schema, file_content);
268-
if (!result.valid) {
269-
for (const auto& err : result.errors) {
270-
if (err.code != ata::error_code::additional_property_not_allowed) {
271-
FPrintF(
272-
stderr, "Invalid configuration in %s:\n", config_path.data());
273-
for (const auto& e : result.errors) {
274-
if (e.code != ata::error_code::additional_property_not_allowed) {
275-
FPrintF(stderr,
276-
" %s: %s\n",
277-
e.path.empty() ? "/" : e.path,
278-
e.message);
279-
}
280-
}
281-
return ParseResult::InvalidContent;
282-
}
283-
}
267+
static const ata::schema_ref compiled_schema =
268+
ata::compile(kNodeConfigSchema);
269+
CHECK(compiled_schema);
270+
auto result = ata::validate(compiled_schema, file_content);
271+
if (!result.valid) {
272+
FPrintF(stderr, "Invalid configuration in %s:\n", config_path.data());
273+
for (const auto& err : result.errors) {
274+
FPrintF(stderr,
275+
" %s: %s\n",
276+
err.path.empty() ? "/" : err.path,
277+
err.message);
284278
}
279+
return ParseResult::InvalidContent;
285280
}
286281
}
287282

src/node_config_schema.h

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,12 @@
44
static constexpr const char kNodeConfigSchema[] = R"JSON(
55
{
66
"$schema": "https://json-schema.org/draft/2020-12/schema",
7-
"additionalProperties": false,
87
"required": [],
98
"properties": {
109
"$schema": {
1110
"type": "string"
1211
},
1312
"nodeOptions": {
14-
"additionalProperties": false,
1513
"required": [],
1614
"properties": {
1715
"addons": {
@@ -761,7 +759,6 @@ static constexpr const char kNodeConfigSchema[] = R"JSON(
761759
},
762760
"permission": {
763761
"type": "object",
764-
"additionalProperties": false,
765762
"required": [],
766763
"properties": {
767764
"allow-addons": {
@@ -826,7 +823,6 @@ static constexpr const char kNodeConfigSchema[] = R"JSON(
826823
},
827824
"test": {
828825
"type": "object",
829-
"additionalProperties": false,
830826
"required": [],
831827
"properties": {
832828
"experimental-test-coverage": {
@@ -991,7 +987,6 @@ static constexpr const char kNodeConfigSchema[] = R"JSON(
991987
},
992988
"watch": {
993989
"type": "object",
994-
"additionalProperties": false,
995990
"required": [],
996991
"properties": {
997992
"watch": {
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"nodeOptions": {
3+
"addons": "not-a-boolean",
4+
"max-http-header-size": "not-a-number"
5+
}
6+
}

test/parallel/test-config-file.js

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -684,6 +684,7 @@ describe('JSON Schema validation', () => {
684684
'-p', '"Hello"',
685685
]);
686686
assert.match(result.stderr, /Invalid configuration/);
687+
assert.match(result.stderr, /\/nodeOptions\/addons/);
687688
assert.strictEqual(result.code, 9);
688689
});
689690

@@ -694,6 +695,7 @@ describe('JSON Schema validation', () => {
694695
'-p', '"Hello"',
695696
]);
696697
assert.match(result.stderr, /Invalid configuration/);
698+
assert.match(result.stderr, /\/nodeOptions\/max-http-header-size/);
697699
assert.strictEqual(result.code, 9);
698700
});
699701

@@ -704,6 +706,7 @@ describe('JSON Schema validation', () => {
704706
'-p', '"Hello"',
705707
]);
706708
assert.match(result.stderr, /Invalid configuration/);
709+
assert.match(result.stderr, /\/nodeOptions\/import/);
707710
assert.strictEqual(result.code, 9);
708711
});
709712

@@ -714,6 +717,19 @@ describe('JSON Schema validation', () => {
714717
'-p', '"Hello"',
715718
]);
716719
assert.match(result.stderr, /Invalid configuration/);
720+
assert.match(result.stderr, /\/nodeOptions\/import/);
721+
assert.strictEqual(result.code, 9);
722+
});
723+
724+
test('reports every error when multiple properties fail', async () => {
725+
const result = await spawnPromisified(process.execPath, [
726+
'--experimental-config-file',
727+
fixtures.path('rc/invalid-schema-multiple-errors.json'),
728+
'-p', '"Hello"',
729+
]);
730+
assert.match(result.stderr, /Invalid configuration/);
731+
assert.match(result.stderr, /\/nodeOptions\/addons/);
732+
assert.match(result.stderr, /\/nodeOptions\/max-http-header-size/);
717733
assert.strictEqual(result.code, 9);
718734
});
719735

0 commit comments

Comments
 (0)