Skip to content

Commit 0eb4024

Browse files
P0lipP0lip
committed
fix: more relaxed circular $refs handling
1 parent a480185 commit 0eb4024

File tree

3 files changed

+191
-51
lines changed

3 files changed

+191
-51
lines changed

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

Lines changed: 176 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ describe('SchemaTree', () => {
6363
},
6464
},
6565
],
66-
}),
66+
})
6767
);
6868
});
6969

@@ -84,7 +84,7 @@ describe('SchemaTree', () => {
8484
type: 'string',
8585
},
8686
},
87-
}),
87+
})
8888
);
8989
});
9090

@@ -104,7 +104,7 @@ describe('SchemaTree', () => {
104104
type: 'string',
105105
},
106106
},
107-
}),
107+
})
108108
);
109109
});
110110
});
@@ -160,7 +160,7 @@ describe('SchemaTree', () => {
160160
type: 'string',
161161
},
162162
},
163-
}),
163+
})
164164
);
165165
});
166166
});
@@ -206,7 +206,7 @@ describe('SchemaTree', () => {
206206
tree.unwrap(tree.itemAt(1) as TreeListParentNode);
207207
expect(getNodeMetadata(tree.itemAt(2) as TreeListParentNode)).toHaveProperty(
208208
'error',
209-
'Seems like you do not want this to be empty.',
209+
'Seems like you do not want this to be empty.'
210210
);
211211
});
212212
});
@@ -243,13 +243,13 @@ describe('SchemaTree', () => {
243243
tree.unwrap(tree.itemAt(1) as TreeListParentNode);
244244
expect(getNodeMetadata(tree.itemAt(2) as TreeListParentNode)).toHaveProperty(
245245
'error',
246-
'Cannot dereference external references',
246+
'Cannot dereference external references'
247247
);
248248

249249
tree.unwrap(tree.itemAt(3) as TreeListParentNode);
250250
expect(getNodeMetadata(tree.itemAt(4) as TreeListParentNode)).toHaveProperty(
251251
'error',
252-
'Cannot dereference external references',
252+
'Cannot dereference external references'
253253
);
254254
});
255255

@@ -271,13 +271,13 @@ describe('SchemaTree', () => {
271271
tree.unwrap(tree.itemAt(1) as TreeListParentNode);
272272
expect(getNodeMetadata(tree.itemAt(2) as TreeListParentNode)).toHaveProperty(
273273
'error',
274-
'Could not read "../test"',
274+
'Could not read "../test"'
275275
);
276276

277277
tree.unwrap(tree.itemAt(3) as TreeListParentNode);
278278
expect(getNodeMetadata(tree.itemAt(4) as TreeListParentNode)).toHaveProperty(
279279
'error',
280-
'Pointer "#id" is missing',
280+
'Pointer "#id" is missing'
281281
);
282282
});
283283
});
@@ -426,14 +426,14 @@ describe('SchemaTree', () => {
426426
├─ type: object
427427
└─ children
428428
├─ 0
429-
│ └─ #/properties/bar/properties/name
429+
│ └─ #/properties/bar/properties/address
430430
│ └─ type: string
431431
├─ 1
432-
│ └─ #/properties/bar/properties/id
433-
│ └─ type: number
432+
│ └─ #/properties/bar/properties/name
433+
│ └─ type: string
434434
└─ 2
435-
└─ #/properties/bar/properties/address
436-
└─ type: string
435+
└─ #/properties/bar/properties/id
436+
└─ type: number
437437
"
438438
`);
439439
});
@@ -482,25 +482,25 @@ describe('SchemaTree', () => {
482482
│ └─ #/properties/foo
483483
│ ├─ type: undefined
484484
│ └─ children
485-
│ └─ 0
486-
│ └─ #/properties/foo/allOf/0
487-
│ ├─ type: undefined
485+
│ ├─ 0
486+
│ │ └─ #/properties/foo/allOf/0
487+
│ │ ├─ type: undefined
488+
│ │ └─ children
489+
│ └─ 1
490+
│ └─ #/properties/foo/allOf/1
491+
│ ├─ type: object
488492
│ └─ children
493+
│ └─ 0
494+
│ └─ #/properties/foo/allOf/1/properties/name
495+
│ └─ type: string
489496
└─ 1
490497
└─ #/properties/bar
491-
├─ type: undefined
498+
├─ type: object
492499
└─ children
493-
├─ 0
494-
│ └─ #/properties/bar/allOf/0
495-
│ ├─ type: undefined
496-
│ └─ children
497-
└─ 1
498-
└─ #/properties/bar/allOf/1
499-
├─ type: object
500+
└─ 0
501+
└─ #/properties/bar/allOf/0
502+
├─ type: undefined
500503
└─ children
501-
└─ 0
502-
└─ #/properties/bar/allOf/1/properties/name
503-
└─ type: string
504504
"
505505
`);
506506
});
@@ -554,7 +554,7 @@ describe('SchemaTree', () => {
554554
└─ children
555555
├─ 0
556556
│ └─ #/properties/baz
557-
│ ├─ type: undefined
557+
│ ├─ type: object
558558
│ └─ children
559559
│ └─ 0
560560
│ └─ #/properties/baz/allOf/0
@@ -564,25 +564,159 @@ describe('SchemaTree', () => {
564564
│ └─ #/properties/foo
565565
│ ├─ type: undefined
566566
│ └─ children
567-
│ └─ 0
568-
│ └─ #/properties/foo/allOf/0
569-
│ ├─ type: undefined
567+
│ ├─ 0
568+
│ │ └─ #/properties/foo/allOf/0
569+
│ │ ├─ type: undefined
570+
│ │ └─ children
571+
│ └─ 1
572+
│ └─ #/properties/foo/allOf/1
573+
│ ├─ type: object
574+
│ └─ children
575+
│ └─ 0
576+
│ └─ #/properties/foo/allOf/1/properties/name
577+
│ └─ type: string
578+
└─ 2
579+
└─ #/properties/bar
580+
├─ type: object
581+
└─ children
582+
└─ 0
583+
└─ #/properties/bar/allOf/0
584+
├─ type: undefined
585+
└─ children
586+
"
587+
`);
588+
});
589+
590+
test('given circular reference pointing at allOf that are not at top-level, should merge top-level allOf normally', () => {
591+
const schema: JSONSchema4 = {
592+
type: 'object',
593+
properties: {
594+
baz: {
595+
allOf: [
596+
{
597+
$ref: '#/properties/bar',
598+
},
599+
],
600+
},
601+
foo: {
602+
allOf: [
603+
{
604+
$ref: '#/properties/baz',
605+
},
606+
],
607+
},
608+
bar: {
609+
allOf: [
610+
{
611+
type: 'object',
612+
properties: {
613+
id: {
614+
type: 'string',
615+
},
616+
address: {
617+
type: 'object',
618+
properties: {
619+
street: {
620+
$ref: '#/properties/foo',
621+
},
622+
},
623+
},
624+
},
625+
},
626+
{
627+
type: 'object',
628+
properties: {
629+
name: {
630+
type: 'string',
631+
},
632+
},
633+
},
634+
],
635+
},
636+
},
637+
};
638+
639+
const tree = new SchemaTree(schema, new SchemaTreeState(), {
640+
expandedDepth: Infinity,
641+
mergeAllOf: true,
642+
resolveRef: void 0,
643+
});
644+
645+
expect(tree.populate.bind(tree)).not.toThrow();
646+
expect(printTree(tree)).toMatchInlineSnapshot(`
647+
"└─ #
648+
├─ type: object
649+
└─ children
650+
├─ 0
651+
│ └─ #/properties/baz
652+
│ ├─ type: object
653+
│ └─ children
654+
│ ├─ 0
655+
│ │ └─ #/properties/baz/properties/id
656+
│ │ └─ type: string
657+
│ ├─ 1
658+
│ │ └─ #/properties/baz/properties/address
659+
│ │ ├─ type: object
660+
│ │ └─ children
661+
│ │ └─ 0
662+
│ │ └─ #/properties/baz/properties/address/properties/street
663+
│ │ ├─ type: undefined
664+
│ │ └─ children
665+
│ │ └─ 0
666+
│ │ └─ #/properties/baz/properties/address/properties/street/allOf/0
667+
│ │ ├─ type: undefined
668+
│ │ └─ children
669+
│ └─ 2
670+
│ └─ #/properties/baz/properties/name
671+
│ └─ type: string
672+
├─ 1
673+
│ └─ #/properties/foo
674+
│ ├─ type: undefined
675+
│ └─ children
676+
│ ├─ 0
677+
│ │ └─ #/properties/foo/allOf/0
678+
│ │ ├─ type: object
679+
│ │ └─ children
680+
│ │ ├─ 0
681+
│ │ │ └─ #/properties/foo/allOf/0/properties/id
682+
│ │ │ └─ type: string
683+
│ │ └─ 1
684+
│ │ └─ #/properties/foo/allOf/0/properties/address
685+
│ │ ├─ type: object
686+
│ │ └─ children
687+
│ │ └─ 0
688+
│ │ └─ #/properties/foo/allOf/0/properties/address/properties/street
689+
│ │ ├─ type: undefined
690+
│ │ └─ children
691+
│ └─ 1
692+
│ └─ #/properties/foo/allOf/1
693+
│ ├─ type: object
570694
│ └─ children
695+
│ └─ 0
696+
│ └─ #/properties/foo/allOf/1/properties/name
697+
│ └─ type: string
571698
└─ 2
572699
└─ #/properties/bar
573-
├─ type: undefined
700+
├─ type: object
574701
└─ children
575702
├─ 0
576-
│ └─ #/properties/bar/allOf/0
577-
│ ├─ type: undefined
703+
│ └─ #/properties/bar/properties/id
704+
│ └─ type: string
705+
├─ 1
706+
│ └─ #/properties/bar/properties/address
707+
│ ├─ type: object
578708
│ └─ children
579-
└─ 1
580-
└─ #/properties/bar/allOf/1
581-
├─ type: object
582-
└─ children
583-
└─ 0
584-
└─ #/properties/bar/allOf/1/properties/name
585-
└─ type: string
709+
│ └─ 0
710+
│ └─ #/properties/bar/properties/address/properties/street
711+
│ ├─ type: undefined
712+
│ └─ children
713+
│ └─ 0
714+
│ └─ #/properties/bar/properties/address/properties/street/allOf/0
715+
│ ├─ type: undefined
716+
│ └─ children
717+
└─ 2
718+
└─ #/properties/bar/properties/name
719+
└─ type: string
586720
"
587721
`);
588722
});

‎src/tree/utils/populateTree.ts‎

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

‎src/utils/mergeAllOf.ts‎

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
1+
import { pathToPointer } from '@stoplight/json';
2+
import { JsonPath } from '@stoplight/types';
13
import { JSONSchema4 } from 'json-schema';
2-
import { cloneDeep, isObject } from 'lodash';
4+
import { cloneDeep } from 'lodash';
35
import { ResolvingError } from '../errors';
46
import { WalkerRefResolver } from '../tree/utils/populateTree';
57

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

8-
function _mergeAllOf(schema: JSONSchema4, resolveRef: WalkerRefResolver, seen: Set<string>) {
10+
function _mergeAllOf(schema: JSONSchema4, path: JsonPath, resolveRef: WalkerRefResolver) {
911
return resolveAllOf(cloneDeep(schema), {
1012
deep: false,
1113
resolvers: {
@@ -21,16 +23,20 @@ function _mergeAllOf(schema: JSONSchema4, resolveRef: WalkerRefResolver, seen: S
2123
return {};
2224
}
2325

24-
const resolved = resolveRef(null, $ref);
25-
if (seen.has($ref)) {
26+
if (pathToPointer(path).startsWith($ref)) {
2627
throw new ResolvingError('Circular reference detected');
2728
}
2829

29-
seen.add($ref);
30-
return isObject(resolved) && 'allOf' in resolved ? _mergeAllOf(resolved, resolveRef, seen) : resolved;
30+
return resolveRef(null, $ref);
3131
},
3232
});
3333
}
3434

35-
export const mergeAllOf = (schema: JSONSchema4, resolveRef: WalkerRefResolver) =>
36-
_mergeAllOf(schema, resolveRef, new Set());
35+
export const mergeAllOf = (schema: JSONSchema4, path: JsonPath, resolveRef: WalkerRefResolver) => {
36+
try {
37+
return _mergeAllOf(schema, path, resolveRef);
38+
} catch (ex) {
39+
console.error(ex.message);
40+
throw ex;
41+
}
42+
};

0 commit comments

Comments
 (0)