Skip to content

Commit

Permalink
feat(resolver): collect errors in AllOfVisitor hooks (#2809)
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@char0n
char0n authored Feb 1, 2023
1 parent a098d85 commit 627ee8d
Show file tree
Hide file tree
Showing 4 changed files with 139 additions and 102 deletions.
9 changes: 3 additions & 6 deletions src/helpers/apidom/reference/dereference/strategies/openapi-3-1-swagger-client/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
7 changes: 7 additions & 0 deletions ...lpers/apidom/reference/dereference/strategies/openapi-3-1-swagger-client/utils/compose.js
View file
Original file line number Diff line number Diff line change
@@ -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 */
128 changes: 74 additions & 54 deletions ...ers/apidom/reference/dereference/strategies/openapi-3-1-swagger-client/visitors/all-of.js
View file
Original file line number Diff line number Diff line change
@@ -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;
},
},
},
});
Expand Down
97 changes: 55 additions & 42 deletions ...pidom/reference/dereference/strategies/openapi-3-1-swagger-client/schema-object/all-of.js
View file
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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'],
});
});
});
Expand Down Expand Up @@ -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'],
});
});
});
Expand Down

0 comments on commit 627ee8d

Please sign in to comment.