From 355ace6769523e2cf54edfb5f8f479706f41caee Mon Sep 17 00:00:00 2001 From: Alexander Mattoni <5110855+mattoni@users.noreply.github.com> Date: Mon, 8 Apr 2024 08:09:12 -0700 Subject: [PATCH 1/3] add converter for oneOf/anyOf with nullable types --- src/RefVisitor.ts | 17 ++++ src/converter.ts | 67 +++++++++++++++ test/converter.spec.ts | 188 +++++++++++++++++++++++++++++++---------- 3 files changed, 228 insertions(+), 44 deletions(-) diff --git a/src/RefVisitor.ts b/src/RefVisitor.ts index f600b5a..da92767 100644 --- a/src/RefVisitor.ts +++ b/src/RefVisitor.ts @@ -126,3 +126,20 @@ export function walkObject(node: object, objectCallback: ObjectVisitor): JsonNod return array; } } + +/** + * Loads the schema/component located at $ref + */ +export function getRefSchema(node: object, ref: RefObject) { + const propertyName = ref.$ref.split('/').reverse()[0]; + if (node.hasOwnProperty('components')) { + const components = node['components']; + if (components != null && typeof components === 'object' && components.hasOwnProperty('schemas')) { + const schemas = components['schemas']; + if (schemas.hasOwnProperty(propertyName)) { + return schemas[propertyName]; + } + } + } + return null; +} diff --git a/src/converter.ts b/src/converter.ts index 07c3358..32112e3 100644 --- a/src/converter.ts +++ b/src/converter.ts @@ -12,6 +12,8 @@ import { JsonNode, RefObject, SchemaObject, + isRef, + getRefSchema, } from './RefVisitor'; /** Lightweight OAS document top-level fields */ @@ -144,6 +146,7 @@ export class Converter { this.convertJsonSchemaContentMediaType(); this.convertConstToEnum(); this.convertNullableTypeArray(); + this.convertMergedNullableType(); this.removeWebhooksObject(); this.removeUnsupportedSchemaKeywords(); if (this.convertSchemaComments) { @@ -244,6 +247,70 @@ export class Converter { visitSchemaObjects(this.openapi30, schemaVisitor); } + /** + * Converts `type: null` merged with other types via anyOf/oneOf to `nullable: true` + */ + convertMergedNullableType() { + const schemaVisitor: SchemaVisitor = (schema: SchemaObject): SchemaObject => { + const nullableOf = ['anyOf', 'oneOf'] as const; + + nullableOf.forEach((of) => { + if (!schema[of]) { + return; + } + + const entries = schema[of]; + + if (!Array.isArray(entries)) { + return; + } + + const typeNullIndex = entries.findIndex((v) => { + if (!v) { + return false; + } + return v.hasOwnProperty('type') && v['type'] === 'null'; + }); + + let isNullable = typeNullIndex > -1; + + // Get the main type of the root object. nullable can't exist with the type property in 3.0.3. + const mainType = entries.reduce((acc, cur) => { + const sub = isRef(cur) ? getRefSchema(this.openapi30, cur) : cur; + + if (sub['type']) { + // if our sub-type...type has null in it, it's still nullable + if (Array.isArray(sub['type'])) { + if (sub['type'].includes('null')) { + isNullable = true; + } + // return the first non-null type. multiple unrelated types aren't + // currently supported. + return sub['type'].filter((_) => _ !== 'null')[0]; + } + // only set non null types + if (sub['type'] !== 'null') { + return sub['type']; + } + } + return acc; + }, null); + + if (isNullable) { + // has a type: null entry in the array + schema['nullable'] = true; + schema['type'] = mainType; + if (typeNullIndex > -1) { + schema[of].splice(typeNullIndex, 1); + } + } + }); + + return this.walkNestedSchemaObjects(schema, schemaVisitor); + }; + visitSchemaObjects(this.openapi30, schemaVisitor); + } + removeWebhooksObject() { if (Object.hasOwnProperty.call(this.openapi30, 'webhooks')) { this.log(`Deleted webhooks object`); diff --git a/test/converter.spec.ts b/test/converter.spec.ts index 0b2f582..5cc26d6 100644 --- a/test/converter.spec.ts +++ b/test/converter.spec.ts @@ -355,7 +355,6 @@ describe('resolver test suite', () => { done(); }); - test('Remove patternProperties keywords', (done) => { const input = { openapi: '3.1.0', @@ -364,12 +363,12 @@ describe('resolver test suite', () => { a: { type: 'object', properties: { - s: { - type: 'string', - }, + s: { + type: 'string', + }, }, patternProperties: { - "^[a-z{2}-[A-Z]{2,3}]$": { + '^[a-z{2}-[A-Z]{2,3}]$': { type: 'object', unevaluatedProperties: false, properties: { @@ -416,7 +415,7 @@ describe('resolver test suite', () => { b: { type: 'string', contentMediaType: 'application/pdf', - maxLength: 5000000 + maxLength: 5000000, }, }, }, @@ -432,7 +431,7 @@ describe('resolver test suite', () => { properties: { b: { type: 'string', - maxLength: 5000000 + maxLength: 5000000, }, }, }, @@ -445,35 +444,34 @@ describe('resolver test suite', () => { done(); }); - - test('Remove webhooks object', (done) => { + test('Remove webhooks object', (done) => { const input = { openapi: '3.1.0', - webhooks: { - newThing: { - post: { - requestBody: { - description: 'Information about a new thing in the system', - content: { - 'application/json': { - schema: { - $ref: '#/components/schemas/newThing' - } - } - } + webhooks: { + newThing: { + post: { + requestBody: { + description: 'Information about a new thing in the system', + content: { + 'application/json': { + schema: { + $ref: '#/components/schemas/newThing', + }, + }, + }, + }, + responses: { + 200: { + description: 'Return a 200 status to indicate that the data was received successfully', }, - responses: { - 200: { - description: 'Return a 200 status to indicate that the data was received successfully' - } - } - } - } - } + }, + }, + }, + }, }; const expected = { - openapi: '3.0.3' + openapi: '3.0.3', }; const converter = new Converter(input, { verbose: true }); @@ -644,6 +642,110 @@ describe('resolver test suite', () => { done(); }); + test('Convert anyOf with null type', (done) => { + const input = { + components: { + schemas: { + a: { + anyOf: [ + { + $ref: '#/components/b', + }, + { + type: 'null', + }, + ], + }, + b: { + type: 'string', + }, + }, + }, + }; + const expected = { + openapi: '3.0.3', + components: { + schemas: { + a: { + anyOf: [ + { + $ref: '#/components/b', + }, + ], + type: 'string', + nullable: true, + }, + b: { + type: 'string', + }, + }, + }, + }; + + const converter = new Converter(input, { verbose: true }); + const converted: any = converter.convert(); + expect(converted).toEqual(expected); + done(); + }); + + test('Convert oneOf with null type', (done) => { + const input = { + components: { + schemas: { + a: { + oneOf: [ + { + $ref: '#/components/b', + }, + { + $ref: '#/components/c', + }, + { + type: 'null', + }, + ], + }, + b: { + type: 'string', + }, + c: { + type: 'string', + }, + }, + }, + }; + const expected = { + openapi: '3.0.3', + components: { + schemas: { + a: { + oneOf: [ + { + $ref: '#/components/b', + }, + { + $ref: '#/components/c', + }, + ], + type: 'string', + nullable: true, + }, + b: { + type: 'string', + }, + c: { + type: 'string', + }, + }, + }, + }; + + const converter = new Converter(input, { verbose: true }); + const converted: any = converter.convert(); + expect(converted).toEqual(expected); + done(); + }); + test('Convert const to enum', (done) => { const input = { components: { @@ -793,11 +895,11 @@ test('binary encoded data with existing binary format', (done) => { const converter = new Converter(input); let caught = false; try { - converter.convert(); + converter.convert(); } catch (e) { caught = true; } - expect(caught).toBeTruthy() + expect(caught).toBeTruthy(); // TODO how to check that Converter logged a specific note? done(); }); @@ -871,7 +973,7 @@ test('contentMediaType with existing binary format', (done) => { binaryEncodedDataWithExistingBinaryFormat: { type: 'string', contentMediaType: 'application/octet-stream', - format: 'binary' + format: 'binary', }, }, }, @@ -894,7 +996,6 @@ test('contentMediaType with existing binary format', (done) => { done(); }); - test('contentMediaType with no existing format', (done) => { const input = { openapi: '3.1.0', @@ -933,21 +1034,20 @@ test('contentMediaType with existing unexpected format', (done) => { binaryEncodedDataWithExistingBinaryFormat: { type: 'string', contentMediaType: 'application/octet-stream', - format: 'byte' + format: 'byte', }, }, }, }; - const converter = new Converter(input); - let caught = false; - try { - converter.convert(); - } catch (e) { - caught = true; - } - expect(caught).toBeTruthy(); + const converter = new Converter(input); + let caught = false; + try { + converter.convert(); + } catch (e) { + caught = true; + } + expect(caught).toBeTruthy(); // TODO how to check that Converter logged to console.warn ? done(); }); - From e01f76924f3094745e1cec69c39541eb8fb792df Mon Sep 17 00:00:00 2001 From: Alexander Mattoni <5110855+mattoni@users.noreply.github.com> Date: Fri, 12 Apr 2024 15:51:13 -0700 Subject: [PATCH 2/3] convert anyOf to allOf --- src/converter.ts | 8 ++++++++ test/converter.spec.ts | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/converter.ts b/src/converter.ts index 32112e3..62063c8 100644 --- a/src/converter.ts +++ b/src/converter.ts @@ -303,6 +303,14 @@ export class Converter { if (typeNullIndex > -1) { schema[of].splice(typeNullIndex, 1); } + + if (entries.length === 1) { + // if only one entry, anyOf/oneOf probably shouldn't be used. + // Instead, convert to allOf with nullable & ref + schema['allOf'] = [schema[of][0]]; + + delete schema[of]; + } } }); diff --git a/test/converter.spec.ts b/test/converter.spec.ts index 5cc26d6..5dcdf04 100644 --- a/test/converter.spec.ts +++ b/test/converter.spec.ts @@ -667,7 +667,7 @@ describe('resolver test suite', () => { components: { schemas: { a: { - anyOf: [ + allOf: [ { $ref: '#/components/b', }, From 866dab89489c3134273bbc952c138a19dd4cf926 Mon Sep 17 00:00:00 2001 From: Alexander Mattoni <5110855+mattoni@users.noreply.github.com> Date: Thu, 18 Jul 2024 17:34:29 -0700 Subject: [PATCH 3/3] Update package.json Lock package versions --- package.json | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/package.json b/package.json index 75bb310..f076afa 100644 --- a/package.json +++ b/package.json @@ -44,24 +44,24 @@ "license": "ISC", "homepage": "https://github.com/apiture/openapi-down-convert#readme", "dependencies": { - "commander": "^9.4.1", - "js-yaml": "^4.1.0", - "typescript": "^4.8.4" + "commander": "9.4.1", + "js-yaml": "4.1.0", + "typescript": "5.5.3" }, "devDependencies": { - "@jest/globals": "^29.2.2", - "@types/commander": "^2.12.2", - "@typescript-eslint/eslint-plugin": "^5.41.0", - "@typescript-eslint/parser": "^5.41.0", - "eslint": "^8.26.0", - "eslint-plugin-import": "^2.25.4", - "eslint-plugin-jsx-a11y": "^6.6.1", - "eslint-plugin-prettier": "^4.2.1", - "eslint-plugin-react": "^7.31.10", - "jest": "^27.5.1", - "pre-push": "^0.1.4", - "prettier": "^2.7.1", - "rimraf": "^4.1.2", - "ts-jest": "^27.1.3" + "@jest/globals": "29.2.2", + "@types/commander": "2.12.2", + "@typescript-eslint/eslint-plugin": "5.41.0", + "@typescript-eslint/parser": "5.41.0", + "eslint": "8.26.0", + "eslint-plugin-import": "2.25.4", + "eslint-plugin-jsx-a11y": "6.6.1", + "eslint-plugin-prettier": "4.2.1", + "eslint-plugin-react": "7.31.10", + "jest": "27.5.1", + "pre-push": "0.1.4", + "prettier": "2.7.1", + "rimraf": "4.1.2", + "ts-jest": "27.1.3" } }