Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(resolver): collect errors in AllOfVisitor hooks #2809

Merged
merged 1 commit into from
Feb 1, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
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 */
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
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