Skip to content

Commit f6b2b11

Browse files
P0lipP0lip
committed
fix: handle more complex references inside of allOfs
1 parent 9152242 commit f6b2b11

File tree

5 files changed

+184
-6
lines changed

5 files changed

+184
-6
lines changed

‎src/tree/__tests__/__fixtures__/complex-allOf-model.json‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
{
2+
"allOf": [
3+
{
4+
"allOf": [
5+
{
6+
"type": "object",
7+
"properties": {
8+
"foo": {
9+
"type": "object",
10+
"properties": {
11+
"user": {
12+
"$ref": "#/allOf/0/allOf/0/properties/foo/definitions/event"
13+
}
14+
},
15+
"definitions": {
16+
"event": {
17+
"allOf": [
18+
{
19+
"type": "object",
20+
"properties": {
21+
"names": {
22+
"items": {
23+
"$ref": "#/allOf/0/allOf/0/properties/foo/definitions/event/allOf/0/properties/name"
24+
}
25+
},
26+
"users": {
27+
"type": "array",
28+
"items": {
29+
"type": "object",
30+
"properties": {
31+
"creation": {
32+
"$ref": "#/allOf/0/allOf/0/properties/foo"
33+
},
34+
"foo": {
35+
"$ref": "#/allOf/0/allOf/0/properties/foo/definitions/event/allOf/0/properties/contacts"
36+
},
37+
"products": {
38+
"$ref": "#/allOf/0/allOf/0/properties/foo/definitions/event/allOf/0/properties/contacts"
39+
}
40+
}
41+
}
42+
}
43+
}
44+
}
45+
]
46+
}
47+
}
48+
}
49+
}
50+
}
51+
]
52+
},
53+
{
54+
"type": "object",
55+
"properties": {
56+
"bar": {
57+
"allOf": [
58+
{
59+
"$ref": "#/allOf/0/allOf/0"
60+
}
61+
]
62+
}
63+
}
64+
}
65+
]
66+
}

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

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
// Jest Snapshot v1, https://goo.gl/fbAQLP
2+
3+
exports[`SchemaTree expanding $refs in allOf given very complex model with circular references, should bail out and display unmerged allOf 1`] = `
4+
"└─ #
5+
├─ type: object
6+
└─ children
7+
├─ 0
8+
│ └─ #/properties/foo
9+
│ ├─ type: object
10+
│ └─ children
11+
│ └─ 0
12+
│ └─ #/properties/foo/properties/user
13+
│ ├─ combiner: allOf
14+
│ └─ children
15+
│ └─ 0
16+
│ └─ #/properties/foo/properties/user/allOf/0
17+
│ ├─ type: object
18+
│ └─ children
19+
│ ├─ 0
20+
│ │ └─ #/properties/foo/properties/user/allOf/0/properties/names
21+
│ │ ├─ type: array
22+
│ │ ├─ subtype: $ref[#/allOf/0/allOf/0/properties/foo/definitions/event/allOf/0/properties/name]
23+
│ │ └─ children
24+
│ └─ 1
25+
│ └─ #/properties/foo/properties/user/allOf/0/properties/users
26+
│ ├─ type: array
27+
│ ├─ subtype: object
28+
│ └─ children
29+
│ ├─ 0
30+
│ │ └─ #/properties/foo/properties/user/allOf/0/properties/users/items/properties/creation
31+
│ │ ├─ $ref: #/allOf/0/allOf/0/properties/foo
32+
│ │ └─ children
33+
│ ├─ 1
34+
│ │ └─ #/properties/foo/properties/user/allOf/0/properties/users/items/properties/foo
35+
│ │ ├─ $ref: #/allOf/0/allOf/0/properties/foo/definitions/event/allOf/0/properties/contacts
36+
│ │ └─ children
37+
│ └─ 2
38+
│ └─ #/properties/foo/properties/user/allOf/0/properties/users/items/properties/products
39+
│ ├─ $ref: #/allOf/0/allOf/0/properties/foo/definitions/event/allOf/0/properties/contacts
40+
│ └─ children
41+
└─ 1
42+
└─ #/properties/bar
43+
├─ type: object
44+
└─ children
45+
└─ 0
46+
└─ #/properties/bar/properties/foo
47+
├─ type: object
48+
└─ children
49+
└─ 0
50+
└─ #/properties/bar/properties/foo/properties/user
51+
├─ combiner: allOf
52+
└─ children
53+
└─ 0
54+
└─ #/properties/bar/properties/foo/properties/user/allOf/0
55+
├─ type: object
56+
└─ children
57+
├─ 0
58+
│ └─ #/properties/bar/properties/foo/properties/user/allOf/0/properties/names
59+
│ ├─ type: array
60+
│ ├─ subtype: $ref[#/allOf/0/allOf/0/properties/foo/definitions/event/allOf/0/properties/name]
61+
│ └─ children
62+
└─ 1
63+
└─ #/properties/bar/properties/foo/properties/user/allOf/0/properties/users
64+
├─ type: array
65+
├─ subtype: object
66+
└─ children
67+
├─ 0
68+
│ └─ #/properties/bar/properties/foo/properties/user/allOf/0/properties/users/items/properties/creation
69+
│ ├─ $ref: #/allOf/0/allOf/0/properties/foo
70+
│ └─ children
71+
├─ 1
72+
│ └─ #/properties/bar/properties/foo/properties/user/allOf/0/properties/users/items/properties/foo
73+
│ ├─ $ref: #/allOf/0/allOf/0/properties/foo/definitions/event/allOf/0/properties/contacts
74+
│ └─ children
75+
└─ 2
76+
└─ #/properties/bar/properties/foo/properties/user/allOf/0/properties/users/items/properties/products
77+
├─ $ref: #/allOf/0/allOf/0/properties/foo/definitions/event/allOf/0/properties/contacts
78+
└─ children
79+
"
80+
`;

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -610,6 +610,21 @@ describe('SchemaTree', () => {
610610
`);
611611
});
612612

613+
test('given very complex model with circular references, should bail out and display unmerged allOf', () => {
614+
const schema = require('./__fixtures__/complex-allOf-model.json');
615+
616+
const tree = new SchemaTree(schema, new SchemaTreeState(), {
617+
expandedDepth: Infinity,
618+
mergeAllOf: true,
619+
resolveRef: void 0,
620+
shouldResolveEagerly: false,
621+
onPopulate: void 0,
622+
});
623+
624+
expect(tree.populate.bind(tree)).not.toThrow();
625+
expect(printTree(tree)).toMatchSnapshot();
626+
});
627+
613628
test('given circular reference pointing at allOf that are not at top-level, should merge top-level allOf normally', () => {
614629
const schema: JSONSchema4 = {
615630
type: 'object',

‎src/tree/utils/mergeAllOf.ts‎

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,13 @@ import { JsonPath } from '@stoplight/types';
33
import { JSONSchema4 } from 'json-schema';
44
import { cloneDeep, compact } from 'lodash';
55
import { ResolvingError } from '../../errors';
6-
import { WalkerRefResolver } from './populateTree';
6+
import { WalkingOptions } from './populateTree';
77

88
const resolveAllOf = require('@stoplight/json-schema-merge-allof');
99

10-
function _mergeAllOf(schema: JSONSchema4, path: JsonPath, resolveRef: WalkerRefResolver) {
10+
const store = new WeakMap<WalkingOptions, WeakMap<JSONSchema4, string[]>>();
11+
12+
function _mergeAllOf(schema: JSONSchema4, path: JsonPath, opts: WalkingOptions) {
1113
return resolveAllOf(cloneDeep(schema), {
1214
deep: false,
1315
resolvers: {
@@ -37,14 +39,29 @@ function _mergeAllOf(schema: JSONSchema4, path: JsonPath, resolveRef: WalkerRefR
3739
throw new ResolvingError('Circular reference detected');
3840
}
3941

40-
return resolveRef(null, $ref);
42+
const allRefs = store.get(opts)!;
43+
const schemaRefs = allRefs.get(schema);
44+
45+
if (schemaRefs === void 0) {
46+
allRefs.set(schema, [$ref]);
47+
} else if (schemaRefs.includes($ref)) {
48+
throw new ResolvingError('Circular reference detected');
49+
} else {
50+
schemaRefs.push($ref);
51+
}
52+
53+
return opts.resolveRef(null, $ref);
4154
},
4255
});
4356
}
4457

45-
export const mergeAllOf = (schema: JSONSchema4, path: JsonPath, resolveRef: WalkerRefResolver) => {
58+
export const mergeAllOf = (schema: JSONSchema4, path: JsonPath, opts: WalkingOptions) => {
4659
try {
47-
return _mergeAllOf(schema, path, resolveRef);
60+
if (!store.has(opts)) {
61+
store.set(opts, new WeakMap());
62+
}
63+
64+
return _mergeAllOf(schema, path, opts);
4865
} catch (ex) {
4966
console.error(ex.message);
5067
throw ex;

‎src/tree/utils/populateTree.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ export const populateTree: Walker = (schema, parent, level, path, options): unde
6363
}
6464
} else if (node.combiner === 'allOf' && options?.mergeAllOf) {
6565
try {
66-
const merged = mergeAllOf(fragment, path, options.resolveRef);
66+
const merged = mergeAllOf(fragment, path, options);
6767
parent.children.pop();
6868
populateTree(merged, parent, level, path, options);
6969
} catch (ex) {

0 commit comments

Comments
 (0)