feat(resolver): collect errors in AllOfVisitor hooks

This change is specific to OpenAPI 3.1.0 resolution strategy. Errors are now collected, instead of thrown and visitor traversal is not interrupted. Refs #2808
swagger-api · Feb 1, 2023 · 372cad5 · 372cad5
1 parent a098d85
commit 372cad5
Show file tree

Hide file tree

Showing 4 changed files with 139 additions and 102 deletions.
diff --git a/src/helpers/apidom/reference/dereference/strategies/openapi-3-1-swagger-client/index.js b/src/helpers/apidom/reference/dereference/strategies/openapi-3-1-swagger-client/index.js
@@ -76,15 +76,12 @@ const OpenApi3_1SwaggerClientDereferenceStrategy = OpenApi3_1DereferenceStrategy
 
       // create allOf visitor (if necessary)
       if (this.mode !== 'strict') {
-        const allOfVisitor = AllOfVisitor();
+        const allOfVisitor = AllOfVisitor({ options });
         visitors.push(allOfVisitor);
       }
 
-      // determine the root visitor
-      const rootVisitor =
-        visitors.length === 1
-          ? visitors[0]
-          : mergeAllVisitors(visitors, { nodeTypeGetter: getNodeType });
+      // establish root visitor by visitor merging
+      const rootVisitor = mergeAllVisitors(visitors, { nodeTypeGetter: getNodeType });
 
       const dereferencedElement = await visitAsync(refSet.rootRef.value, rootVisitor, {
         keyMap,

diff --git a/...lpers/apidom/reference/dereference/strategies/openapi-3-1-swagger-client/utils/compose.js b/...lpers/apidom/reference/dereference/strategies/openapi-3-1-swagger-client/utils/compose.js
@@ -0,0 +1,7 @@
+/* eslint-disable camelcase */
+import OpenApi3_1DereferenceStrategy from '@swagger-api/apidom-reference/dereference/strategies/openapi-3-1';
+
+const compose = OpenApi3_1DereferenceStrategy.compose.bind();
+
+export default compose;
+/* eslint-enable camelcase */
diff --git a/...ers/apidom/reference/dereference/strategies/openapi-3-1-swagger-client/visitors/all-of.js b/...ers/apidom/reference/dereference/strategies/openapi-3-1-swagger-client/visitors/all-of.js
@@ -1,61 +1,81 @@
 import { isArrayElement, deepmerge } from '@swagger-api/apidom-core';
 import { isSchemaElement, SchemaElement } from '@swagger-api/apidom-ns-openapi-3-1';
 
-const AllOfVisitor = () => ({
-  SchemaElement: {
-    leave(schemaElement) {
-      // do nothing
-      if (typeof schemaElement.allOf === 'undefined') return undefined;
-      // throw if allOf keyword is not an array
-      if (!isArrayElement(schemaElement.allOf)) {
-        throw new TypeError('allOf must be an array');
-      }
-      // remove allOf keyword if empty
-      if (schemaElement.allOf.isEmpty) {
-        return new SchemaElement(
-          schemaElement.content.filter((memberElement) => memberElement.key.toValue() !== 'allOf'),
-          schemaElement.meta.clone(),
-          schemaElement.attributes.clone()
-        );
-      }
-      // throw if allOf keyword contains anything else than Schema Object
-      schemaElement.allOf.forEach((item) => {
-        if (!isSchemaElement(item)) {
-          throw new TypeError('Elements in allOf must be objects');
+import compose from '../utils/compose.js';
+import toPath from '../utils/to-path.js';
+
+const AllOfVisitor = compose({
+  init({ options }) {
+    this.options = options;
+  },
+  props: {
+    options: null,
+
+    SchemaElement: {
+      leave(schemaElement, key, parent, path, ancestors) {
+        // do nothing
+        if (typeof schemaElement.allOf === 'undefined') return undefined;
+
+        // collect error and return if allOf keyword is not an array
+        if (!isArrayElement(schemaElement.allOf)) {
+          const error = new TypeError('allOf must be an array');
+          error.fullPath = [...toPath([...ancestors, parent, schemaElement]), 'allOf'];
+          this.options.dereference.dereferenceOpts?.errors?.push?.(error);
+          return undefined;
+        }
+
+        // remove allOf keyword if empty
+        if (schemaElement.allOf.isEmpty) {
+          return new SchemaElement(
+            schemaElement.content.filter(
+              (memberElement) => memberElement.key.toValue() !== 'allOf'
+            ),
+            schemaElement.meta.clone(),
+            schemaElement.attributes.clone()
+          );
+        }
+
+        // collect errors if allOf keyword contains anything else than Schema Object
+        const includesSchemaElementOnly = schemaElement.allOf.content.every(isSchemaElement);
+        if (!includesSchemaElementOnly) {
+          const error = new TypeError('Elements in allOf must be objects');
+          error.fullPath = [...toPath([...ancestors, parent, schemaElement]), 'allOf'];
+          this.options.dereference.dereferenceOpts?.errors?.push?.(error);
+          return undefined;
         }
-      });
-
-      const mergedSchemaElement = deepmerge.all([...schemaElement.allOf.content, schemaElement]);
-
-      /**
-       * If there was not an original $$ref value, make sure to remove
-       * any $$ref value that may exist from the result of `allOf` merges.
-       */
-      if (!schemaElement.hasKey('$$ref')) {
-        mergedSchemaElement.remove('$$ref');
-      }
-
-      /**
-       * If there was an example keyword in the original definition,
-       * keep it instead of merging with example from other schema.
-       */
-      if (schemaElement.hasKey('example')) {
-        const member = mergedSchemaElement.getMember('example');
-        member.value = schemaElement.get('example');
-      }
-
-      /**
-       * If there was an examples keyword in the original definition,
-       * keep it instead of merging with examples from other schema.
-       */
-      if (schemaElement.hasKey('examples')) {
-        const member = mergedSchemaElement.getMember('examples');
-        member.value = schemaElement.get('examples');
-      }
-
-      // remove allOf keyword after the merge
-      mergedSchemaElement.remove('allOf');
-      return mergedSchemaElement;
+
+        const mergedSchemaElement = deepmerge.all([...schemaElement.allOf.content, schemaElement]);
+
+        /**
+         * If there was not an original $$ref value, make sure to remove
+         * any $$ref value that may exist from the result of `allOf` merges.
+         */
+        if (!schemaElement.hasKey('$$ref')) {
+          mergedSchemaElement.remove('$$ref');
+        }
+
+        /**
+         * If there was an example keyword in the original definition,
+         * keep it instead of merging with example from other schema.
+         */
+        if (schemaElement.hasKey('example')) {
+          const member = mergedSchemaElement.getMember('example');
+          member.value = schemaElement.get('example');
+        }
+
+        /**
+         * If there was an examples keyword in the original definition,
+         * keep it instead of merging with examples from other schema.
+         */
+        if (schemaElement.hasKey('examples')) {
+          const member = mergedSchemaElement.getMember('examples');
+          member.value = schemaElement.get('examples');
+        }
+
+        // remove allOf keyword after the merge
+        mergedSchemaElement.remove('allOf');
+        return mergedSchemaElement;
+      },
     },
   },
 });

diff --git a/...pidom/reference/dereference/strategies/openapi-3-1-swagger-client/schema-object/all-of.js b/...pidom/reference/dereference/strategies/openapi-3-1-swagger-client/schema-object/all-of.js
@@ -1,10 +1,7 @@
 /* eslint-disable camelcase */
 import { toValue } from '@swagger-api/apidom-core';
 import { mediaTypes, OpenApi3_1Element } from '@swagger-api/apidom-ns-openapi-3-1';
-import {
-  dereferenceApiDOM,
-  DereferenceError,
-} from '@swagger-api/apidom-reference/configuration/empty';
+import { dereferenceApiDOM } from '@swagger-api/apidom-reference/configuration/empty';
 
 import * as jestSetup from '../__utils__/jest.local.setup.js';
 import OpenApi3_1SwaggerClientDereferenceStrategy from '../../../../../../../../src/helpers/apidom/reference/dereference/strategies/openapi-3-1-swagger-client/index.js';
@@ -22,29 +19,37 @@ describe('dereference', () => {
     describe('openapi-3-1-swagger-client', () => {
       describe('Schema Object', () => {
         describe('given allOf is not an array', () => {
-          test('should throw error', async () => {
-            const spec = OpenApi3_1Element.refract({
-              openapi: '3.1.0',
-              components: {
-                schemas: {
-                  User: {
-                    allOf: {},
-                  },
+          const openApiElement = OpenApi3_1Element.refract({
+            openapi: '3.1.0',
+            components: {
+              schemas: {
+                User: {
+                  allOf: {},
                 },
               },
+            },
+          });
+
+          test('should dereference', async () => {
+            const actual = await dereferenceApiDOM(openApiElement, {
+              parse: { mediaType: mediaTypes.latest('json') },
             });
-            const dereferenceThunk = () =>
-              dereferenceApiDOM(spec, {
-                parse: { mediaType: mediaTypes.latest('json') },
-              });
 
-            await expect(dereferenceThunk()).rejects.toThrow(DereferenceError);
-            await expect(dereferenceThunk()).rejects.toMatchObject({
-              cause: {
-                cause: {
-                  message: expect.stringMatching(/^allOf must be an array$/),
-                },
-              },
+            expect(toValue(actual)).toEqual(toValue(openApiElement));
+          });
+
+          test('should collect error', async () => {
+            const errors = [];
+
+            await dereferenceApiDOM(openApiElement, {
+              parse: { mediaType: mediaTypes.latest('json') },
+              dereference: { dereferenceOpts: { errors } },
+            });
+
+            expect(errors).toHaveLength(1);
+            expect(errors[0]).toMatchObject({
+              message: expect.stringMatching(/^allOf must be an array/),
+              fullPath: ['components', 'schemas', 'User', 'allOf'],
             });
           });
         });
@@ -77,29 +82,37 @@ describe('dereference', () => {
         });
 
         describe('give allOf contains non-object item', () => {
-          test('should throw error', async () => {
-            const spec = OpenApi3_1Element.refract({
-              openapi: '3.1.0',
-              components: {
-                schemas: {
-                  User: {
-                    allOf: [{ type: 'string' }, 2],
-                  },
+          const openApiElement = OpenApi3_1Element.refract({
+            openapi: '3.1.0',
+            components: {
+              schemas: {
+                User: {
+                  allOf: [{ type: 'string' }, 2],
                 },
               },
+            },
+          });
+
+          test('should dereference', async () => {
+            const actual = await dereferenceApiDOM(openApiElement, {
+              parse: { mediaType: mediaTypes.latest('json') },
             });
-            const dereferenceThunk = () =>
-              dereferenceApiDOM(spec, {
-                parse: { mediaType: mediaTypes.latest('json') },
-              });
 
-            await expect(dereferenceThunk()).rejects.toThrow(DereferenceError);
-            await expect(dereferenceThunk()).rejects.toMatchObject({
-              cause: {
-                cause: {
-                  message: expect.stringMatching(/^Elements in allOf must be objects$/),
-                },
-              },
+            expect(toValue(actual)).toEqual(toValue(openApiElement));
+          });
+
+          test('should collect error', async () => {
+            const errors = [];
+
+            await dereferenceApiDOM(openApiElement, {
+              parse: { mediaType: mediaTypes.latest('json') },
+              dereference: { dereferenceOpts: { errors } },
+            });
+
+            expect(errors).toHaveLength(1);
+            expect(errors[0]).toMatchObject({
+              message: expect.stringMatching(/^Elements in allOf must be objects/),
+              fullPath: ['components', 'schemas', 'User', 'allOf'],
             });
           });
         });