Skip to content

Commit 269e4e2

Browse files
jdesrosiersjdesrosiers
committed
Fix unevaluatedProperties performance problem
1 parent 7144c66 commit 269e4e2

File tree

5 files changed

+99
-96
lines changed

5 files changed

+99
-96
lines changed

‎lib/keywords/unevaluatedItems.js‎

Lines changed: 19 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -1,31 +1,17 @@
1-
import { canonicalUri, Validation } from "../experimental.js";
1+
import { Validation } from "../experimental.js";
22
import * as Instance from "../instance.js";
33

44

55
const id = "https://json-schema.org/keyword/unevaluatedItems";
66

7-
const compile = async (schema, ast, parentSchema) => {
8-
return [canonicalUri(parentSchema), await Validation.compile(schema, ast)];
9-
};
7+
const compile = (schema, ast) => Validation.compile(schema, ast);
108

11-
const interpret = ([schemaUrl, unevaluatedItems], instance, context) => {
9+
const interpret = (unevaluatedItems, instance, context) => {
1210
if (Instance.typeOf(instance) !== "array") {
1311
return true;
1412
}
1513

16-
// Because order matters, we re-evaluate this schema skipping this keyword
17-
// just to collect all the evalauted items.
18-
if (context.rootSchema === schemaUrl) {
19-
return true;
20-
}
21-
const evaluatedItemsPlugin = new EvaluatedItemsPlugin(schemaUrl);
22-
if (!Validation.interpret(schemaUrl, instance, {
23-
...context,
24-
plugins: [...context.ast.plugins, evaluatedItemsPlugin]
25-
})) {
26-
return true;
27-
}
28-
const evaluatedItems = evaluatedItemsPlugin.evaluatedItems;
14+
const evaluatedItems = context.schemaEvaluatedItems;
2915

3016
let isValid = true;
3117
let index = 0;
@@ -46,38 +32,30 @@ const interpret = ([schemaUrl, unevaluatedItems], instance, context) => {
4632

4733
const simpleApplicator = true;
4834

49-
class EvaluatedItemsPlugin {
50-
constructor(rootSchema) {
51-
this.rootSchema = rootSchema;
52-
}
53-
35+
const plugin = {
5436
beforeSchema(_url, _instance, context) {
5537
context.evaluatedItems ??= new Set();
56-
context.schemaEvaluatedItems ??= new Set();
57-
}
38+
context.schemaEvaluatedItems = new Set();
39+
},
5840

59-
beforeKeyword(_node, _instance, context) {
60-
context.rootSchema = this.rootSchema;
41+
beforeKeyword(_node, _instance, context, schemaContext) {
6142
context.evaluatedItems = new Set();
62-
}
43+
context.schemaEvaluatedItems = schemaContext.schemaEvaluatedItems;
44+
},
6345

64-
afterKeyword(_node, _instance, context, valid, schemaContext) {
65-
if (valid) {
66-
for (const index of context.evaluatedItems) {
67-
schemaContext.schemaEvaluatedItems.add(index);
68-
}
46+
afterKeyword(_node, _instance, context, _valid, schemaContext) {
47+
for (const property of context.evaluatedItems) {
48+
schemaContext.schemaEvaluatedItems.add(property);
6949
}
70-
}
50+
},
7151

72-
afterSchema(_url, _instance, context, valid) {
52+
afterSchema(_node, _instance, context, valid) {
7353
if (valid) {
74-
for (const index of context.schemaEvaluatedItems) {
75-
context.evaluatedItems.add(index);
54+
for (const property of context.schemaEvaluatedItems) {
55+
context.evaluatedItems.add(property);
7656
}
7757
}
78-
79-
this.evaluatedItems = context.evaluatedItems;
8058
}
81-
}
59+
};
8260

83-
export default { id, compile, interpret, simpleApplicator };
61+
export default { id, compile, interpret, simpleApplicator, plugin };

‎lib/keywords/unevaluatedProperties.js‎

Lines changed: 17 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -1,31 +1,17 @@
1-
import { Validation, canonicalUri } from "../experimental.js";
1+
import { Validation } from "../experimental.js";
22
import * as Instance from "../instance.js";
33

44

55
const id = "https://json-schema.org/keyword/unevaluatedProperties";
66

7-
const compile = async (schema, ast, parentSchema) => {
8-
return [canonicalUri(parentSchema), await Validation.compile(schema, ast)];
9-
};
7+
const compile = (schema, ast) => Validation.compile(schema, ast);
108

11-
const interpret = ([schemaUrl, unevaluatedProperties], instance, context) => {
9+
const interpret = (unevaluatedProperties, instance, context) => {
1210
if (Instance.typeOf(instance) !== "object") {
1311
return true;
1412
}
1513

16-
// Because order matters, we re-evaluate this schema skipping this keyword
17-
// just to collect all the evalauted properties.
18-
if (context.rootSchema === schemaUrl) {
19-
return true;
20-
}
21-
const evaluatedPropertiesPlugin = new EvaluatedPropertiesPlugin(schemaUrl);
22-
if (!Validation.interpret(schemaUrl, instance, {
23-
...context,
24-
plugins: [...context.ast.plugins, evaluatedPropertiesPlugin]
25-
})) {
26-
return true;
27-
}
28-
const evaluatedProperties = evaluatedPropertiesPlugin.evaluatedProperties;
14+
const evaluatedProperties = context.schemaEvaluatedProperties;
2915

3016
let isValid = true;
3117
for (const [propertyNameNode, property] of Instance.entries(instance)) {
@@ -46,38 +32,30 @@ const interpret = ([schemaUrl, unevaluatedProperties], instance, context) => {
4632

4733
const simpleApplicator = true;
4834

49-
class EvaluatedPropertiesPlugin {
50-
constructor(rootSchema) {
51-
this.rootSchema = rootSchema;
52-
}
53-
35+
const plugin = {
5436
beforeSchema(_url, _instance, context) {
5537
context.evaluatedProperties ??= new Set();
56-
context.schemaEvaluatedProperties ??= new Set();
57-
}
38+
context.schemaEvaluatedProperties = new Set();
39+
},
5840

59-
beforeKeyword(_node, _instance, context) {
60-
context.rootSchema = this.rootSchema;
41+
beforeKeyword(_node, _instance, context, schemaContext) {
6142
context.evaluatedProperties = new Set();
62-
}
43+
context.schemaEvaluatedProperties = schemaContext.schemaEvaluatedProperties;
44+
},
6345

64-
afterKeyword(_node, _instance, context, valid, schemaContext) {
65-
if (valid) {
66-
for (const property of context.evaluatedProperties) {
67-
schemaContext.schemaEvaluatedProperties.add(property);
68-
}
46+
afterKeyword(_node, _instance, context, _valid, schemaContext) {
47+
for (const property of context.evaluatedProperties) {
48+
schemaContext.schemaEvaluatedProperties.add(property);
6949
}
70-
}
50+
},
7151

72-
afterSchema(_url, _instance, context, valid) {
52+
afterSchema(_node, _instance, context, valid) {
7353
if (valid) {
7454
for (const property of context.schemaEvaluatedProperties) {
7555
context.evaluatedProperties.add(property);
7656
}
7757
}
78-
79-
this.evaluatedProperties = context.evaluatedProperties;
8058
}
81-
}
59+
};
8260

83-
export default { id, compile, interpret, simpleApplicator };
61+
export default { id, compile, interpret, simpleApplicator, plugin };

‎lib/keywords/validation.js‎

Lines changed: 25 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -28,23 +28,36 @@ const compile = async (schema, ast) => {
2828
throw Error(`No schema found at '${url}'`);
2929
}
3030

31-
ast[url] = typeof schemaValue === "boolean" ? schemaValue : await pipe(
32-
entries(schema),
33-
asyncMap(async ([keyword, keywordSchema]) => {
34-
const keywordHandler = getKeywordByName(keyword, schema.document.dialectId);
35-
if (keywordHandler.plugin) {
36-
ast.plugins.add(keywordHandler.plugin);
37-
}
38-
const keywordAst = await keywordHandler.compile(keywordSchema, ast, schema);
39-
return [keywordHandler.id, pointerAppend(keyword, canonicalUri(schema)), keywordAst];
40-
}),
41-
asyncCollectArray
42-
);
31+
if (typeof schemaValue === "boolean") {
32+
ast[url] = schemaValue;
33+
} else {
34+
ast[url] = await pipe(
35+
entries(schema),
36+
asyncMap(async ([keyword, keywordSchema]) => {
37+
const keywordHandler = getKeywordByName(keyword, schema.document.dialectId);
38+
if (keywordHandler.plugin) {
39+
ast.plugins.add(keywordHandler.plugin);
40+
}
41+
const keywordAst = await keywordHandler.compile(keywordSchema, ast, schema);
42+
return [keywordHandler.id, pointerAppend(keyword, canonicalUri(schema)), keywordAst];
43+
}),
44+
asyncCollectArray
45+
);
46+
47+
// Keyword order shouldn't matter, but the unevaluated keywords are an exception :-(
48+
ast[url].sort(keywordComparator);
49+
}
4350
}
4451

4552
return url;
4653
};
4754

55+
const lastKeywords = new Set([
56+
"https://json-schema.org/keyword/unevaluatedProperties",
57+
"https://json-schema.org/keyword/unevaluatedItems"
58+
]);
59+
const keywordComparator = (_a, b) => lastKeywords.has(b[0]) ? -1 : 1;
60+
4861
const interpret = (url, instance, context) => {
4962
let valid = true;
5063

‎lib/output-basic.spec.ts‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -645,7 +645,7 @@ describe("Basic Output Format", () => {
645645
});
646646
});
647647

648-
test("invalid - doesn't apply if the schema fails", async () => {
648+
test("invalid - with sibling property declarations", async () => {
649649
registerSchema({
650650
properties: {
651651
foo: true,
@@ -662,6 +662,11 @@ describe("Basic Output Format", () => {
662662
keyword: "https://json-schema.org/evaluation/validate",
663663
absoluteKeywordLocation: `${schemaUri}#/properties/bar`,
664664
instanceLocation: "#/bar"
665+
},
666+
{
667+
keyword: "https://json-schema.org/evaluation/validate",
668+
absoluteKeywordLocation: `${schemaUri}#/unevaluatedProperties`,
669+
instanceLocation: "#/baz"
665670
}
666671
]
667672
});
@@ -727,7 +732,7 @@ describe("Basic Output Format", () => {
727732
});
728733
});
729734

730-
test("invalid - doesn't apply if the schema fails", async () => {
735+
test("invalid - with sibling property declarations", async () => {
731736
registerSchema({
732737
prefixItems: [true, false],
733738
unevaluatedItems: false
@@ -741,6 +746,11 @@ describe("Basic Output Format", () => {
741746
keyword: "https://json-schema.org/evaluation/validate",
742747
absoluteKeywordLocation: `${schemaUri}#/prefixItems/1`,
743748
instanceLocation: "#/1"
749+
},
750+
{
751+
keyword: "https://json-schema.org/evaluation/validate",
752+
absoluteKeywordLocation: `${schemaUri}#/unevaluatedItems`,
753+
instanceLocation: "#/2"
744754
}
745755
]
746756
});

‎lib/output-detailed.spec.ts‎

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -788,7 +788,7 @@ describe("Detailed Output Format", () => {
788788
});
789789
});
790790

791-
test("invalid - doesn't apply if the schema fails", async () => {
791+
test("invalid - with sibling property declarations", async () => {
792792
registerSchema({
793793
properties: {
794794
foo: true,
@@ -812,6 +812,18 @@ describe("Detailed Output Format", () => {
812812
instanceLocation: "#/bar"
813813
}
814814
]
815+
},
816+
{
817+
keyword: "https://json-schema.org/keyword/unevaluatedProperties",
818+
absoluteKeywordLocation: `${schemaUri}#/unevaluatedProperties`,
819+
instanceLocation: "#",
820+
errors: [
821+
{
822+
keyword: "https://json-schema.org/evaluation/validate",
823+
absoluteKeywordLocation: `${schemaUri}#/unevaluatedProperties`,
824+
instanceLocation: "#/baz"
825+
}
826+
]
815827
}
816828
]
817829
});
@@ -873,7 +885,7 @@ describe("Detailed Output Format", () => {
873885
});
874886
});
875887

876-
test("invalid - doesn't apply if the schema fails", async () => {
888+
test("invalid - with sibling property declarations", async () => {
877889
registerSchema({
878890
prefixItems: [true, false],
879891
unevaluatedItems: false
@@ -894,6 +906,18 @@ describe("Detailed Output Format", () => {
894906
instanceLocation: "#/1"
895907
}
896908
]
909+
},
910+
{
911+
keyword: "https://json-schema.org/keyword/unevaluatedItems",
912+
absoluteKeywordLocation: "schema:main#/unevaluatedItems",
913+
instanceLocation: "#",
914+
errors: [
915+
{
916+
keyword: "https://json-schema.org/evaluation/validate",
917+
absoluteKeywordLocation: "schema:main#/unevaluatedItems",
918+
instanceLocation: "#/2"
919+
}
920+
]
897921
}
898922
]
899923
});

0 commit comments

Comments
 (0)