From 43b608510be2fd15ab6025a747497799b4b3d715 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 | 187 +++++++++++++++++++++++++++++++---------- 3 files changed, 228 insertions(+), 43 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 84b6c82..1423eb9 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 915eb57..1c50cd9 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: { @@ -795,11 +897,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(); }); @@ -873,7 +975,7 @@ test('contentMediaType with existing binary format', (done) => { binaryEncodedDataWithExistingBinaryFormat: { type: 'string', contentMediaType: 'application/octet-stream', - format: 'binary' + format: 'binary', }, }, }, @@ -896,7 +998,6 @@ test('contentMediaType with existing binary format', (done) => { done(); }); - test('contentMediaType with no existing format', (done) => { const input = { openapi: '3.1.0', @@ -935,20 +1036,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 9db61b9557b5e7cda1ece3cc7d3453f1ea01fee4 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 1423eb9..0d4309b 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 1c50cd9..73fefe6 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 654309786f68362a6ae7b03f0d76f7e24abcf9f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antoine=20Vandecr=C3=A8me?= Date: Wed, 30 Oct 2024 14:12:17 +0100 Subject: [PATCH 3/3] Fix deep nullable handling --- src/RefVisitor.ts | 5 +- src/converter.ts | 54 +++++++------ test/converter.spec.ts | 172 +++++++++++++++++++++++++++++++++++++++-- 3 files changed, 200 insertions(+), 31 deletions(-) diff --git a/src/RefVisitor.ts b/src/RefVisitor.ts index da92767..b82dee1 100644 --- a/src/RefVisitor.ts +++ b/src/RefVisitor.ts @@ -131,8 +131,9 @@ export function walkObject(node: object, objectCallback: ObjectVisitor): JsonNod * 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 split = ref.$ref.split('/'); + if (split[0] === '#' && split[1] === 'components' && split[2] === 'schemas' && node.hasOwnProperty('components')) { + const propertyName = split[3]; const components = node['components']; if (components != null && typeof components === 'object' && components.hasOwnProperty('schemas')) { const schemas = components['schemas']; diff --git a/src/converter.ts b/src/converter.ts index 0d4309b..ab6714d 100644 --- a/src/converter.ts +++ b/src/converter.ts @@ -56,6 +56,7 @@ export interface ConverterOptions { } export class Converter { + private openapi31: OpenAPI3; private openapi30: OpenAPI3; private verbose = false; private deleteExampleWithId = false; @@ -72,6 +73,7 @@ export class Converter { * @throws Error if the scopeDescriptionFile (if specified) cannot be read or parsed as YAML/JSON */ constructor(openapiDocument: object, options?: ConverterOptions) { + this.openapi31 = openapiDocument as OpenAPI3; this.openapi30 = Converter.deepClone(openapiDocument) as OpenAPI3; this.verbose = Boolean(options?.verbose); this.deleteExampleWithId = Boolean(options?.deleteExampleWithId); @@ -272,40 +274,48 @@ export class Converter { 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; + const nullable = (o, visitedRefs : Set) => { + let obj = o; + if (isRef(o)) { + const ref = o['$ref'] as string; + if (visitedRefs.has(ref)) { + return false; + } + visitedRefs = new Set([...visitedRefs, ref]); + obj = getRefSchema(this.openapi31, o) + } - 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]; + if (obj.hasOwnProperty('type')) { + if (obj['type'] === 'null') { + return true; } - // only set non null types - if (sub['type'] !== 'null') { - return sub['type']; + if (Array.isArray(obj['type'])) { + return obj['type'].some((t) => t === 'null'); } } - return acc; - }, null); + if (obj['anyOf']) { + return obj['anyOf'].some((e) => nullable(e, visitedRefs)); + } + if (obj['oneOf']) { + return obj['oneOf'].some((e) => nullable(e, visitedRefs)); + } + if (obj['allOf']) { + return obj['allOf'].every((e) => nullable(e, visitedRefs)); + } + return false; + } + + const isNullable = typeNullIndex > -1 || entries.some((e) => nullable(e, new Set([]))); if (isNullable) { - // has a type: null entry in the array schema['nullable'] = true; - schema['type'] = mainType; + // If there is a `type: null` entry directly in the array, remove it if (typeNullIndex > -1) { schema[of].splice(typeNullIndex, 1); } if (entries.length === 1) { - // if only one entry, anyOf/oneOf probably shouldn't be used. + // if only one entry remaining, anyOf/oneOf probably shouldn't be used. // Instead, convert to allOf with nullable & ref schema['allOf'] = [schema[of][0]]; diff --git a/test/converter.spec.ts b/test/converter.spec.ts index 73fefe6..ff9f975 100644 --- a/test/converter.spec.ts +++ b/test/converter.spec.ts @@ -649,7 +649,7 @@ describe('resolver test suite', () => { a: { anyOf: [ { - $ref: '#/components/b', + $ref: '#/components/schemas/b', }, { type: 'null', @@ -669,10 +669,9 @@ describe('resolver test suite', () => { a: { allOf: [ { - $ref: '#/components/b', + $ref: '#/components/schemas/b', }, ], - type: 'string', nullable: true, }, b: { @@ -695,10 +694,10 @@ describe('resolver test suite', () => { a: { oneOf: [ { - $ref: '#/components/b', + $ref: '#/components/schemas/b', }, { - $ref: '#/components/c', + $ref: '#/components/schemas/c', }, { type: 'null', @@ -721,21 +720,142 @@ describe('resolver test suite', () => { a: { oneOf: [ { - $ref: '#/components/b', + $ref: '#/components/schemas/b', }, { - $ref: '#/components/c', + $ref: '#/components/schemas/c', }, ], + 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 oneOf with ref nullable type array', (done) => { + const input = { + components: { + schemas: { + a: { + oneOf: [ + { + $ref: '#/components/schemas/b', + }, + { + $ref: '#/components/schemas/c', + }, + ], + }, + b: { + type: ['number', 'null'], + }, + c: { type: 'string', + }, + }, + }, + }; + const expected = { + openapi: '3.0.3', + components: { + schemas: { + a: { + nullable: true, + oneOf: [ + { + $ref: '#/components/schemas/b', + }, + { + $ref: '#/components/schemas/c', + }, + ], + }, + b: { + type: 'number', nullable: true, }, + c: { + type: 'string', + }, + }, + }, + }; + + const converter = new Converter(input, { verbose: true }); + const converted: any = converter.convert(); + expect(converted).toEqual(expected); + done(); + }); + + test('Convert oneOf with deep null type', (done) => { + const input = { + components: { + schemas: { + a: { + oneOf: [ + { + $ref: '#/components/schemas/b', + }, + { + $ref: '#/components/schemas/c', + }, + ], + }, b: { + anyOf: [ {$ref: "#/components/schemas/d"}, { type: 'string'}], + }, + c: { type: 'string', }, + d: { + oneOf: [ + { type: 'null' }, + { type: 'number' } + ] + } + }, + }, + }; + const expected = { + openapi: '3.0.3', + components: { + schemas: { + a: { + nullable: true, + oneOf: [ + { + $ref: '#/components/schemas/b', + }, + { + $ref: '#/components/schemas/c', + }, + ], + }, + b: { + anyOf: [ {$ref: "#/components/schemas/d"}, { type: 'string'}], + nullable: true, + }, c: { type: 'string', }, + d: { + nullable: true, + allOf: [ + { type: 'number' } + ] + } }, }, }; @@ -746,6 +866,44 @@ describe('resolver test suite', () => { done(); }); + test('Convert recursive schema', (done) => { + const input = { + components: { + schemas: { + a: { + anyOf: [ { $ref: "#/components/schemas/b" }, { $ref: "#/components/schemas/c" } ], + }, + b: { + anyOf: [ { $ref: "#/components/schemas/a" }, { $ref: "#/components/schemas/c" } ] + }, + c: { + type: "string", + }, + }, + }, + }; + const expected = { + openapi: '3.0.3', + components: { + schemas: { + a: { + anyOf: [ { $ref: "#/components/schemas/b" }, { $ref: "#/components/schemas/c" } ], + }, + b: { + anyOf: [ { $ref: "#/components/schemas/a" }, { $ref: "#/components/schemas/c" } ] + }, + 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: {