Skip to content

Commit 7d1bc9d

Browse files
P0lipP0lip
authored
perf: improve handling of under compound keyword combo (#20)
1 parent a14a678 commit 7d1bc9d

File tree

7 files changed

+252
-34
lines changed

7 files changed

+252
-34
lines changed

‎package.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@
3737
"peerDependencies": {},
3838
"dependencies": {
3939
"@stoplight/json": "^3.12.0",
40-
"@stoplight/json-schema-merge-allof": "^0.7.7",
40+
"@stoplight/json-schema-merge-allof": "^0.7.8",
4141
"@stoplight/lifecycle": "^2.3.2",
4242
"@types/json-schema": "^7.0.7",
4343
"magic-error": "0.0.1"

‎src/__tests__/__fixtures__/combiners/allOfs/nested-refs.json

+60
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
{
2+
"required": ["limit", "order"],
3+
"type": "object",
4+
"properties": {
5+
"dimensions": { "type": "array", "items": { "type": "string" } },
6+
"measures": { "type": "array", "items": { "type": "string" } },
7+
"limit": {
8+
"maximum": 2000,
9+
"minimum": 0,
10+
"type": "integer",
11+
"format": "int32"
12+
},
13+
"offset": {
14+
"minimum": 0,
15+
"type": "integer",
16+
"format": "int32",
17+
"maximum": 2147483647
18+
},
19+
"filters": {
20+
"type": "array",
21+
"items": { "oneOf": [{ "$ref": "#/$defs/Logical" }, { "$ref": "#/$defs/Plain" }] }
22+
},
23+
"timeDimensions": {
24+
"maxItems": 1,
25+
"minItems": 0,
26+
"type": "array",
27+
"items": { "$ref": "#/$defs/TimeDimension" }
28+
},
29+
"order": { "type": "object", "additionalProperties": { "type": "string", "enum": ["ASC", "DESC"] } },
30+
"nextToken": { "type": "string" }
31+
},
32+
"$defs": {
33+
"Logical": { "type": "object", "allOf": [{ "$ref": "#/$defs/Filter" }] },
34+
"Filter": { "type": "object", "anyOf": [{ "$ref": "#/$defs/Logical" }, { "$ref": "#/$defs/Plain" }] },
35+
"Plain": {
36+
"required": ["member", "operator"],
37+
"type": "object",
38+
"allOf": [
39+
{ "$ref": "#/$defs/Filter" },
40+
{
41+
"type": "object",
42+
"properties": {
43+
"member": { "type": "string" },
44+
"operator": { "pattern": "equals|notEquals|gt|gte|lt|lte|set|notSet|inDateRange", "type": "string" },
45+
"values": { "type": "array", "items": { "type": "object" } }
46+
}
47+
}
48+
]
49+
},
50+
"TimeDimension": {
51+
"required": ["dateRange", "dimension", "granularity"],
52+
"type": "object",
53+
"properties": {
54+
"dimension": { "type": "string" },
55+
"granularity": { "pattern": "second|minute|hour|day", "type": "string" },
56+
"dateRange": { "type": "object", "example": ["2022-04-19T16:00:00.000Z", "2022-04-19T17:00:00.000Z"] }
57+
}
58+
}
59+
}
60+
}

‎src/__tests__/__snapshots__/tree.spec.ts.snap

+138
Original file line numberDiff line numberDiff line change
@@ -694,6 +694,144 @@ exports[`SchemaTree output should generate valid tree for combiners/allOfs/compl
694694
"
695695
`;
696696

697+
exports[`SchemaTree output should generate valid tree for combiners/allOfs/nested-refs.json 1`] = `
698+
"└─ #
699+
├─ types
700+
│ └─ 0: object
701+
├─ primaryType: object
702+
└─ children
703+
├─ 0
704+
│ └─ #/properties/dimensions
705+
│ ├─ types
706+
│ │ └─ 0: array
707+
│ ├─ primaryType: array
708+
│ └─ children
709+
│ └─ 0
710+
│ └─ #/properties/dimensions/items
711+
│ ├─ types
712+
│ │ └─ 0: string
713+
│ └─ primaryType: string
714+
├─ 1
715+
│ └─ #/properties/measures
716+
│ ├─ types
717+
│ │ └─ 0: array
718+
│ ├─ primaryType: array
719+
│ └─ children
720+
│ └─ 0
721+
│ └─ #/properties/measures/items
722+
│ ├─ types
723+
│ │ └─ 0: string
724+
│ └─ primaryType: string
725+
├─ 2
726+
│ └─ #/properties/limit
727+
│ ├─ types
728+
│ │ └─ 0: integer
729+
│ └─ primaryType: integer
730+
├─ 3
731+
│ └─ #/properties/offset
732+
│ ├─ types
733+
│ │ └─ 0: integer
734+
│ └─ primaryType: integer
735+
├─ 4
736+
│ └─ #/properties/filters
737+
│ ├─ types
738+
│ │ └─ 0: array
739+
│ ├─ primaryType: array
740+
│ └─ children
741+
│ └─ 0
742+
│ └─ #/properties/filters/items
743+
│ ├─ combiners
744+
│ │ └─ 0: oneOf
745+
│ └─ children
746+
│ ├─ 0
747+
│ │ └─ #/properties/filters/items/oneOf/0
748+
│ │ ├─ types
749+
│ │ │ └─ 0: object
750+
│ │ ├─ primaryType: object
751+
│ │ ├─ combiners
752+
│ │ │ └─ 0: anyOf
753+
│ │ └─ children
754+
│ │ ├─ 0
755+
│ │ │ └─ #/properties/filters/items/oneOf/0/anyOf/0
756+
│ │ │ └─ mirrors: #/properties/filters/items/oneOf/0
757+
│ │ └─ 1
758+
│ │ └─ #/properties/filters/items/oneOf/0/anyOf/1
759+
│ │ ├─ types
760+
│ │ │ └─ 0: object
761+
│ │ ├─ primaryType: object
762+
│ │ ├─ combiners
763+
│ │ │ └─ 0: anyOf
764+
│ │ └─ children
765+
│ │ ├─ 0
766+
│ │ │ └─ #/properties/filters/items/oneOf/0/anyOf/1/anyOf/0
767+
│ │ │ └─ mirrors: #/properties/filters/items/oneOf/0
768+
│ │ ├─ 1
769+
│ │ │ └─ #/properties/filters/items/oneOf/0/anyOf/1/anyOf/1
770+
│ │ │ └─ mirrors: #/properties/filters/items/oneOf/0/anyOf/1
771+
│ │ ├─ 2
772+
│ │ │ └─ #/properties/filters/items/oneOf/0/anyOf/1/properties/member
773+
│ │ │ ├─ types
774+
│ │ │ │ └─ 0: string
775+
│ │ │ └─ primaryType: string
776+
│ │ ├─ 3
777+
│ │ │ └─ #/properties/filters/items/oneOf/0/anyOf/1/properties/operator
778+
│ │ │ ├─ types
779+
│ │ │ │ └─ 0: string
780+
│ │ │ └─ primaryType: string
781+
│ │ └─ 4
782+
│ │ └─ #/properties/filters/items/oneOf/0/anyOf/1/properties/values
783+
│ │ ├─ types
784+
│ │ │ └─ 0: array
785+
│ │ ├─ primaryType: array
786+
│ │ └─ children
787+
│ │ └─ 0
788+
│ │ └─ #/properties/filters/items/oneOf/0/anyOf/1/properties/values/items
789+
│ │ ├─ types
790+
│ │ │ └─ 0: object
791+
│ │ └─ primaryType: object
792+
│ └─ 1
793+
│ └─ #/properties/filters/items/oneOf/1
794+
│ └─ mirrors: #/properties/filters/items/oneOf/0/anyOf/1
795+
├─ 5
796+
│ └─ #/properties/timeDimensions
797+
│ ├─ types
798+
│ │ └─ 0: array
799+
│ ├─ primaryType: array
800+
│ └─ children
801+
│ └─ 0
802+
│ └─ #/properties/timeDimensions/items
803+
│ ├─ types
804+
│ │ └─ 0: object
805+
│ ├─ primaryType: object
806+
│ └─ children
807+
│ ├─ 0
808+
│ │ └─ #/properties/timeDimensions/items/properties/dimension
809+
│ │ ├─ types
810+
│ │ │ └─ 0: string
811+
│ │ └─ primaryType: string
812+
│ ├─ 1
813+
│ │ └─ #/properties/timeDimensions/items/properties/granularity
814+
│ │ ├─ types
815+
│ │ │ └─ 0: string
816+
│ │ └─ primaryType: string
817+
│ └─ 2
818+
│ └─ #/properties/timeDimensions/items/properties/dateRange
819+
│ ├─ types
820+
│ │ └─ 0: object
821+
│ └─ primaryType: object
822+
├─ 6
823+
│ └─ #/properties/order
824+
│ ├─ types
825+
│ │ └─ 0: object
826+
│ └─ primaryType: object
827+
└─ 7
828+
└─ #/properties/nextToken
829+
├─ types
830+
│ └─ 0: string
831+
└─ primaryType: string
832+
"
833+
`;
834+
697835
exports[`SchemaTree output should generate valid tree for combiners/allOfs/todo-full.json 1`] = `
698836
"└─ #
699837
├─ types

‎src/mergers/mergeAllOf.ts

+28-10
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { pathToPointer, stringify } from '@stoplight/json';
1+
import { pathToPointer } from '@stoplight/json';
22

33
import { ResolvingError } from '../errors';
44
import type { SchemaFragment } from '../types';
@@ -8,8 +8,18 @@ const resolveAllOf = require('@stoplight/json-schema-merge-allof');
88

99
const store = new WeakMap<WalkerRefResolver, WeakMap<SchemaFragment, string[]>>();
1010

11-
function _mergeAllOf(fragment: SchemaFragment, path: string[], resolveRef: WalkerRefResolver | null): SchemaFragment {
12-
return resolveAllOf(fragment, {
11+
function _mergeAllOf(
12+
fragment: SchemaFragment,
13+
path: string[],
14+
resolveRef: WalkerRefResolver | null,
15+
seen: WeakMap<SchemaFragment, SchemaFragment>,
16+
): SchemaFragment {
17+
const cached = seen.get(fragment);
18+
if (cached !== void 0) {
19+
return cached;
20+
}
21+
22+
const merged = resolveAllOf(fragment, {
1323
deep: false,
1424
resolvers: resolveAllOf.stoplightResolvers,
1525
...(resolveRef !== null
@@ -30,8 +40,8 @@ function _mergeAllOf(fragment: SchemaFragment, path: string[], resolveRef: Walke
3040
schemaRefs = [$ref];
3141
allRefs.set(fragment, schemaRefs);
3242
} else if (schemaRefs.includes($ref)) {
33-
const safelyResolved = JSON.parse(stringify(resolveRef(null, $ref)));
34-
return 'allOf' in safelyResolved ? _mergeAllOf(safelyResolved, path, resolveRef) : safelyResolved;
43+
const resolved = resolveRef(null, $ref);
44+
return 'allOf' in resolved ? _mergeAllOf(resolved, path, resolveRef, seen) : resolved;
3545
} else {
3646
schemaRefs.push($ref);
3747
}
@@ -52,17 +62,25 @@ function _mergeAllOf(fragment: SchemaFragment, path: string[], resolveRef: Walke
5262
}
5363
: null),
5464
});
65+
66+
seen.set(fragment, merged);
67+
return merged;
5568
}
5669

57-
export function mergeAllOf(fragment: SchemaFragment, path: string[], walkingOptions: WalkingOptions) {
70+
export function mergeAllOf(
71+
fragment: SchemaFragment,
72+
path: string[],
73+
walkingOptions: WalkingOptions,
74+
seen: WeakMap<SchemaFragment, SchemaFragment>,
75+
) {
5876
if (walkingOptions.resolveRef !== null && !store.has(walkingOptions.resolveRef)) {
5977
store.set(walkingOptions.resolveRef, new WeakMap());
6078
}
6179

62-
let merged = _mergeAllOf(fragment, path, walkingOptions.resolveRef);
63-
while ('allOf' in merged) {
64-
merged = _mergeAllOf(merged, path, walkingOptions.resolveRef);
65-
}
80+
let merged = fragment;
81+
do {
82+
merged = _mergeAllOf(merged, path, walkingOptions.resolveRef, seen);
83+
} while ('allOf' in merged);
6684

6785
return merged;
6886
}

‎src/mergers/mergeOneOrAnyOf.ts

+15-17
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ export function mergeOneOrAnyOf(
77
fragment: SchemaFragment,
88
path: string[],
99
walkingOptions: WalkingOptions,
10+
mergedAllOfs: WeakMap<SchemaFragment, SchemaFragment>,
1011
): SchemaFragment[] {
1112
const combiner = SchemaCombinerName.OneOf in fragment ? SchemaCombinerName.OneOf : SchemaCombinerName.AnyOf;
1213
const items = fragment[combiner];
@@ -15,7 +16,7 @@ export function mergeOneOrAnyOf(
1516

1617
const merged: SchemaFragment[] = [];
1718

18-
if (Array.isArray(fragment.allOf) && Array.isArray(items)) {
19+
if (Array.isArray(fragment.allOf)) {
1920
for (const item of items) {
2021
merged.push({
2122
allOf: [...fragment.allOf, item],
@@ -24,26 +25,23 @@ export function mergeOneOrAnyOf(
2425

2526
return merged;
2627
} else {
27-
for (const item of items) {
28-
const prunedSchema = { ...fragment };
29-
delete prunedSchema[combiner];
28+
const prunedSchema = { ...fragment };
29+
delete prunedSchema[combiner];
3030

31+
for (const item of items) {
3132
if (Object.keys(prunedSchema).length === 0) {
3233
merged.push(item);
3334
} else {
34-
const resolvedItem =
35-
typeof item.$ref === 'string' && walkingOptions.resolveRef !== null
36-
? walkingOptions.resolveRef(null, item.$ref)
37-
: item;
38-
const mergedSchema = {
39-
allOf: [prunedSchema, resolvedItem],
40-
};
41-
42-
try {
43-
merged.push(mergeAllOf(mergedSchema, path, walkingOptions));
44-
} catch {
45-
merged.push(mergedSchema);
46-
}
35+
merged.push(
36+
mergeAllOf(
37+
{
38+
allOf: [prunedSchema, item],
39+
},
40+
path,
41+
walkingOptions,
42+
mergedAllOfs,
43+
),
44+
);
4745
}
4846
}
4947
}

‎src/walker/walker.ts

+6-2
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,9 @@ export class Walker extends EventEmitter<WalkerEmitter> {
2828
protected fragment: SchemaFragment;
2929
protected schemaNode: RegularNode | RootNode;
3030

31+
private mergedAllOfs: WeakMap<SchemaFragment, SchemaFragment>;
3132
private processedFragments: WeakMap<ProcessedFragment, SchemaNode>;
33+
3234
private readonly hooks: Partial<Dictionary<WalkerHookHandler, WalkerHookAction>>;
3335

3436
constructor(protected readonly root: RootNode, protected readonly walkingOptions: WalkingOptions) {
@@ -39,6 +41,7 @@ export class Walker extends EventEmitter<WalkerEmitter> {
3941
this.fragment = root.fragment;
4042
this.schemaNode = root;
4143
this.processedFragments = new WeakMap<SchemaFragment, SchemaNode>();
44+
this.mergedAllOfs = new WeakMap();
4245

4346
this.hooks = {};
4447
}
@@ -49,6 +52,7 @@ export class Walker extends EventEmitter<WalkerEmitter> {
4952
this.fragment = this.root.fragment;
5053
this.schemaNode = this.root;
5154
this.processedFragments = new WeakMap<SchemaFragment, RegularNode | ReferenceNode>();
55+
this.mergedAllOfs = new WeakMap();
5256
}
5357

5458
public loadSnapshot(snapshot: WalkerSnapshot) {
@@ -299,7 +303,7 @@ export class Walker extends EventEmitter<WalkerEmitter> {
299303
initialFragment = fragment.allOf;
300304
}
301305

302-
fragment = mergeAllOf(fragment, path, walkingOptions);
306+
fragment = mergeAllOf(fragment, path, walkingOptions, this.mergedAllOfs);
303307
} catch (ex) {
304308
initialFragment = fragment;
305309
super.emit('error', createMagicError(new MergingError(ex?.message ?? 'Unknown merging error')));
@@ -309,7 +313,7 @@ export class Walker extends EventEmitter<WalkerEmitter> {
309313

310314
if (SchemaCombinerName.OneOf in fragment || SchemaCombinerName.AnyOf in fragment) {
311315
try {
312-
const merged = mergeOneOrAnyOf(fragment, path, walkingOptions);
316+
const merged = mergeOneOrAnyOf(fragment, path, walkingOptions, this.mergedAllOfs);
313317
if (merged.length === 1) {
314318
return [new RegularNode(merged[0], { originalFragment }), initialFragment];
315319
} else {

‎yarn.lock

+4-4
Original file line numberDiff line numberDiff line change
@@ -936,10 +936,10 @@
936936
dependencies:
937937
eslint-config-prettier "^7.1.0"
938938

939-
"@stoplight/json-schema-merge-allof@^0.7.7":
940-
version "0.7.7"
941-
resolved "https://registry.yarnpkg.com/@stoplight/json-schema-merge-allof/-/json-schema-merge-allof-0.7.7.tgz#d79ca8729aa5d6420e40cc545b1854b518b9240f"
942-
integrity sha512-3QrZ+2hhTvsPAjl8HD9rQI3MPHSCl/kCf0rEkKM4YjLumDsfX1TSrhESDIt4lSzpSdNyT2oXSJx0ADVGE2gTyA==
939+
"@stoplight/json-schema-merge-allof@^0.7.8":
940+
version "0.7.8"
941+
resolved "https://registry.yarnpkg.com/@stoplight/json-schema-merge-allof/-/json-schema-merge-allof-0.7.8.tgz#7efe5e0086dff433eb011f617e82f7295c3de061"
942+
integrity sha512-JTDt6GYpCWQSb7+UW1P91IAp/pcLWis0mmEzWVFcLsrNgtUYK7JLtYYz0ZPSR4QVL0fJ0YQejM+MPq5iNDFO4g==
943943
dependencies:
944944
compute-lcm "^1.1.0"
945945
json-schema-compare "^0.2.2"

0 commit comments

Comments
 (0)