fix: graceful allOf merging handling

P0lip · P0lip · commit a48018577c1c · 2020-04-22T13:27:51.000+02:00
diff --git a/src/errors.ts b/src/errors.ts
@@ -0,0 +1,3 @@
+export class ResolvingError extends ReferenceError {
+  public readonly name = 'ResolvingError';
+}
diff --git a/src/index.ts b/src/index.ts
@@ -2,3 +2,4 @@ export { JsonSchemaViewer, SchemaRow, SchemaTree as SchemaTreeComponent, Propert
 export * from './tree';
 export * from './types';
 export * from './utils';
+export * from './errors';
diff --git a/src/tree/__tests__/tree.spec.ts b/src/tree/__tests__/tree.spec.ts
@@ -1,5 +1,6 @@
 import { TreeListParentNode } from '@stoplight/tree-list';
 import { JSONSchema4 } from 'json-schema';
+import { ResolvingError } from '../../errors';
 import { getNodeMetadata } from '../metadata';
 import { SchemaTree, SchemaTreeState } from '../tree';
 import { printTree } from './utils/printTree';
@@ -62,7 +63,7 @@ describe('SchemaTree', () => {
                 },
               },
             ],
-          })
+          }),
         );
       });
 
@@ -83,7 +84,7 @@ describe('SchemaTree', () => {
                 type: 'string',
               },
             },
-          })
+          }),
         );
       });
 
@@ -103,7 +104,7 @@ describe('SchemaTree', () => {
                 type: 'string',
               },
             },
-          })
+          }),
         );
       });
     });
@@ -159,7 +160,7 @@ describe('SchemaTree', () => {
                 type: 'string',
               },
             },
-          })
+          }),
         );
       });
     });
@@ -196,7 +197,7 @@ describe('SchemaTree', () => {
           expandedDepth: 0,
           mergeAllOf: false,
           resolveRef() {
-            throw new ReferenceError('Seems like you do not want this to be empty.');
+            throw new ResolvingError('Seems like you do not want this to be empty.');
           },
         });
 
@@ -205,7 +206,7 @@ describe('SchemaTree', () => {
         tree.unwrap(tree.itemAt(1) as TreeListParentNode);
         expect(getNodeMetadata(tree.itemAt(2) as TreeListParentNode)).toHaveProperty(
           'error',
-          'Seems like you do not want this to be empty.'
+          'Seems like you do not want this to be empty.',
         );
       });
     });
@@ -242,13 +243,13 @@ describe('SchemaTree', () => {
         tree.unwrap(tree.itemAt(1) as TreeListParentNode);
         expect(getNodeMetadata(tree.itemAt(2) as TreeListParentNode)).toHaveProperty(
           'error',
-          'Cannot dereference external references'
+          'Cannot dereference external references',
         );
 
         tree.unwrap(tree.itemAt(3) as TreeListParentNode);
         expect(getNodeMetadata(tree.itemAt(4) as TreeListParentNode)).toHaveProperty(
           'error',
-          'Cannot dereference external references'
+          'Cannot dereference external references',
         );
       });
 
@@ -258,10 +259,10 @@ describe('SchemaTree', () => {
           mergeAllOf: false,
           resolveRef({ source, pointer }) {
             if (source === '../test') {
-              throw new ReferenceError(`Could not read "${source}"`);
+              throw new ResolvingError(`Could not read "${source}"`);
             }
 
-            throw new ReferenceError(`Pointer "${pointer}" is missing`);
+            throw new ResolvingError(`Pointer "${pointer}" is missing`);
           },
         });
 
@@ -270,13 +271,13 @@ describe('SchemaTree', () => {
         tree.unwrap(tree.itemAt(1) as TreeListParentNode);
         expect(getNodeMetadata(tree.itemAt(2) as TreeListParentNode)).toHaveProperty(
           'error',
-          'Could not read "../test"'
+          'Could not read "../test"',
         );
 
         tree.unwrap(tree.itemAt(3) as TreeListParentNode);
         expect(getNodeMetadata(tree.itemAt(4) as TreeListParentNode)).toHaveProperty(
           'error',
-          'Pointer "#id" is missing'
+          'Pointer "#id" is missing',
         );
       });
     });
@@ -437,7 +438,7 @@ describe('SchemaTree', () => {
         `);
       });
 
-      test('given direct circular reference pointing at allOf, should throw', () => {
+      test('given direct circular reference pointing at allOf, should bail out and display unmerged allOf', () => {
         const schema: JSONSchema4 = {
           type: 'object',
           properties: {
@@ -472,10 +473,39 @@ describe('SchemaTree', () => {
           resolveRef: void 0,
         });
 
-        expect(tree.populate.bind(tree)).toThrowError('Circular reference detected');
+        expect(tree.populate.bind(tree)).not.toThrow();
+        expect(printTree(tree)).toMatchInlineSnapshot(`
+          "└─ #
+             ├─ type: object
+             └─ children
+                ├─ 0
+                │  └─ #/properties/foo
+                │     ├─ type: undefined
+                │     └─ children
+                │        └─ 0
+                │           └─ #/properties/foo/allOf/0
+                │              ├─ type: undefined
+                │              └─ children
+                └─ 1
+                   └─ #/properties/bar
+                      ├─ type: undefined
+                      └─ children
+                         ├─ 0
+                         │  └─ #/properties/bar/allOf/0
+                         │     ├─ type: undefined
+                         │     └─ children
+                         └─ 1
+                            └─ #/properties/bar/allOf/1
+                               ├─ type: object
+                               └─ children
+                                  └─ 0
+                                     └─ #/properties/bar/allOf/1/properties/name
+                                        └─ type: string
+          "
+        `);
       });
 
-      test('given indirect circular reference pointing at allOf, should throw', () => {
+      test('given indirect circular reference pointing at allOf, should bail out and display unmerged allOf', () => {
         const schema: JSONSchema4 = {
           type: 'object',
           properties: {
@@ -517,11 +547,98 @@ describe('SchemaTree', () => {
           resolveRef: void 0,
         });
 
-        expect(tree.populate.bind(tree)).toThrowError('Circular reference detected');
+        expect(tree.populate.bind(tree)).not.toThrow();
+        expect(printTree(tree)).toMatchInlineSnapshot(`
+          "└─ #
+             ├─ type: object
+             └─ children
+                ├─ 0
+                │  └─ #/properties/baz
+                │     ├─ type: undefined
+                │     └─ children
+                │        └─ 0
+                │           └─ #/properties/baz/allOf/0
+                │              ├─ type: undefined
+                │              └─ children
+                ├─ 1
+                │  └─ #/properties/foo
+                │     ├─ type: undefined
+                │     └─ children
+                │        └─ 0
+                │           └─ #/properties/foo/allOf/0
+                │              ├─ type: undefined
+                │              └─ children
+                └─ 2
+                   └─ #/properties/bar
+                      ├─ type: undefined
+                      └─ children
+                         ├─ 0
+                         │  └─ #/properties/bar/allOf/0
+                         │     ├─ type: undefined
+                         │     └─ children
+                         └─ 1
+                            └─ #/properties/bar/allOf/1
+                               ├─ type: object
+                               └─ children
+                                  └─ 0
+                                     └─ #/properties/bar/allOf/1/properties/name
+                                        └─ type: string
+          "
+        `);
       });
     });
   });
 
+  describe('allOf failures', () => {
+    test('given incompatible values, should bail out and display unmerged allOf', () => {
+      const schema: JSONSchema4 = {
+        allOf: [
+          {
+            type: 'string',
+          },
+          {
+            type: 'number',
+          },
+          {
+            type: 'object',
+            properties: {
+              name: {
+                type: 'string',
+              },
+            },
+          },
+        ],
+      };
+
+      const tree = new SchemaTree(schema, new SchemaTreeState(), {
+        expandedDepth: Infinity,
+        mergeAllOf: true,
+        resolveRef: void 0,
+      });
+
+      expect(tree.populate.bind(tree)).not.toThrow();
+      expect(printTree(tree)).toMatchInlineSnapshot(`
+        "└─ #
+           ├─ type: undefined
+           └─ children
+              ├─ 0
+              │  └─ #/allOf/0
+              │     └─ type: string
+              ├─ 1
+              │  └─ #/allOf/1
+              │     └─ type: number
+              └─ 2
+                 └─ #/allOf/2
+                    ├─ type: object
+                    └─ children
+                       └─ 0
+                          └─ #/allOf/2/properties/name
+                             └─ type: string
+        "
+      `);
+    });
+  });
+
   describe('paths generation', () => {
     let schema: JSONSchema4;
     let tree: SchemaTree;
diff --git a/src/tree/tree.ts b/src/tree/tree.ts
@@ -3,6 +3,7 @@ import { Tree, TreeListParentNode, TreeState } from '@stoplight/tree-list';
 import { JsonPath, Optional } from '@stoplight/types';
 import { JSONSchema4 } from 'json-schema';
 import { get as _get, isEqual as _isEqual, isObject as _isObject } from 'lodash';
+import { ResolvingError } from '../errors';
 import { SchemaTreeListNode } from '../types';
 import { generateId } from '../utils/generateId';
 import { hasRefItems, isRefNode } from '../utils/guards';
@@ -143,9 +144,9 @@ export class SchemaTree extends Tree {
     if (this.resolver !== void 0) {
       return this.resolver({ source, pointer }, path, this.schema);
     } else if (source !== null) {
-      throw new ReferenceError('Cannot dereference external references');
+      throw new ResolvingError('Cannot dereference external references');
     } else if (pointer === null) {
-      throw new ReferenceError('The pointer is empty');
+      throw new ResolvingError('The pointer is empty');
     } else {
       return _get(this.schema, pointerToPath(pointer));
     }
@@ -159,7 +160,7 @@ export class SchemaTree extends Tree {
     const schemaFragment = this.resolveRef(path, $ref);
 
     if (!_isObject(schemaFragment)) {
-      throw new ReferenceError(`Could not dereference "${$ref}"`);
+      throw new ResolvingError(`Could not dereference "${$ref}"`);
     }
 
     this.populateTreeFragment(node, schemaFragment, path, false);
diff --git a/src/tree/utils/populateTree.ts b/src/tree/utils/populateTree.ts
@@ -58,8 +58,16 @@ export const populateTree: Walker = (schema, parent, level, path, options): unde
           break;
       }
     } else if (node.combiner === 'allOf' && options?.mergeAllOf) {
-      parent.children.pop();
-      populateTree(mergeAllOf(fragment, options.resolveRef), parent, level, path, options);
+      try {
+        const merged = mergeAllOf(fragment, options.resolveRef);
+        parent.children.pop();
+        populateTree(merged, parent, level, path, options);
+      } catch (ex) {
+        if (Array.isArray(fragment.allOf)) {
+          (treeNode as TreeListParentNode).children = [];
+          bailAllOf(treeNode as TreeListParentNode, fragment, level + 1, [...path, 'allOf'], options);
+        }
+      }
     } else if (_isObject(node.properties)) {
       (treeNode as TreeListParentNode).children = [];
 
@@ -167,3 +175,17 @@ function processObject(
 
   return node;
 }
+
+function bailAllOf(
+  node: TreeListParentNode,
+  schema: JSONSchema4,
+  level: number,
+  path: JsonPath,
+  options: WalkingOptions | null,
+) {
+  if (Array.isArray(schema.allOf)) {
+    for (const [i, item] of schema.allOf.entries()) {
+      populateTree(item, node, level, [...path, i], options);
+    }
+  }
+}
diff --git a/src/utils/mergeAllOf.ts b/src/utils/mergeAllOf.ts
@@ -1,5 +1,6 @@
 import { JSONSchema4 } from 'json-schema';
 import { cloneDeep, isObject } from 'lodash';
+import { ResolvingError } from '../errors';
 import { WalkerRefResolver } from '../tree/utils/populateTree';
 
 const resolveAllOf = require('@stoplight/json-schema-merge-allof');
@@ -22,7 +23,7 @@ function _mergeAllOf(schema: JSONSchema4, resolveRef: WalkerRefResolver, seen: S
 
       const resolved = resolveRef(null, $ref);
       if (seen.has($ref)) {
-        throw new ReferenceError('Circular reference detected');
+        throw new ResolvingError('Circular reference detected');
       }
 
       seen.add($ref);

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+export class ResolvingError extends ReferenceError {`
	`2`	`+ public readonly name = 'ResolvingError';`
	`3`	`+}`