From 08ba92c303749fd476603c9ef30e0b948e027a45 Mon Sep 17 00:00:00 2001 From: Ansh Chaturvedi Date: Mon, 22 Jul 2024 22:23:21 -0400 Subject: [PATCH 1/4] feat: initial impl. of union consolidation Ticket: DX-614 --- packages/openapi-generator/src/ir.ts | 7 ++- .../openapi-generator/src/knownImports.ts | 42 +++++++++++--- packages/openapi-generator/src/optimize.ts | 55 +++++++++++++++---- 3 files changed, 84 insertions(+), 20 deletions(-) diff --git a/packages/openapi-generator/src/ir.ts b/packages/openapi-generator/src/ir.ts index c51e0a2c..4a729275 100644 --- a/packages/openapi-generator/src/ir.ts +++ b/packages/openapi-generator/src/ir.ts @@ -82,4 +82,9 @@ export type SchemaMetadata = Omit< | 'externalDocs' >; -export type Schema = BaseSchema & HasComment & SchemaMetadata; +type ExtendedSchemaMetadata = SchemaMetadata & { + primitive?: boolean; + decodedType?: string; +}; + +export type Schema = BaseSchema & HasComment & ExtendedSchemaMetadata; diff --git a/packages/openapi-generator/src/knownImports.ts b/packages/openapi-generator/src/knownImports.ts index 0fa6b484..fa77183b 100644 --- a/packages/openapi-generator/src/knownImports.ts +++ b/packages/openapi-generator/src/knownImports.ts @@ -46,10 +46,10 @@ export const KNOWN_IMPORTS: KnownImports = { }, }, 'io-ts': { - string: () => E.right({ type: 'string' }), - number: () => E.right({ type: 'number' }), + string: () => E.right({ type: 'string', primitive: true }), + number: () => E.right({ type: 'number', primitive: true }), bigint: () => E.right({ type: 'number' }), - boolean: () => E.right({ type: 'boolean' }), + boolean: () => E.right({ type: 'boolean', primitive: true }), null: () => E.right({ type: 'null' }), nullType: () => E.right({ type: 'null' }), undefined: () => E.right({ type: 'undefined' }), @@ -143,11 +143,13 @@ export const KNOWN_IMPORTS: KnownImports = { type: 'string', format: 'number', pattern: '^\\d+$', + decodedType: 'number', }), NaturalFromString: () => E.right({ type: 'string', format: 'number', + decodedType: 'number', }), Negative: () => E.right({ @@ -161,6 +163,7 @@ export const KNOWN_IMPORTS: KnownImports = { format: 'number', maximum: 0, exclusiveMaximum: true, + decodedType: 'number', }), NegativeInt: () => E.right({ @@ -174,6 +177,7 @@ export const KNOWN_IMPORTS: KnownImports = { format: 'number', maximum: 0, exclusiveMaximum: true, + decodedType: 'number', }), NonNegative: () => E.right({ @@ -185,6 +189,7 @@ export const KNOWN_IMPORTS: KnownImports = { type: 'string', format: 'number', minimum: 0, + decodedType: 'number', }), NonNegativeInt: () => E.right({ @@ -195,6 +200,7 @@ export const KNOWN_IMPORTS: KnownImports = { E.right({ type: 'string', format: 'number', + decodedType: 'number', }), NonPositive: () => E.right({ @@ -206,6 +212,7 @@ export const KNOWN_IMPORTS: KnownImports = { type: 'string', format: 'number', maximum: 0, + decodedType: 'number', }), NonPositiveInt: () => E.right({ @@ -217,6 +224,7 @@ export const KNOWN_IMPORTS: KnownImports = { type: 'string', format: 'number', maximum: 0, + decodedType: 'number', }), NonZero: () => E.right({ @@ -226,6 +234,7 @@ export const KNOWN_IMPORTS: KnownImports = { E.right({ type: 'string', format: 'number', + decodedType: 'number', }), NonZeroInt: () => E.right({ @@ -235,6 +244,7 @@ export const KNOWN_IMPORTS: KnownImports = { E.right({ type: 'string', format: 'number', + decodedType: 'number', }), Positive: () => E.right({ @@ -248,6 +258,7 @@ export const KNOWN_IMPORTS: KnownImports = { format: 'number', minimum: 0, exclusiveMinimum: true, + decodedType: 'number', }), Zero: () => E.right({ @@ -257,6 +268,7 @@ export const KNOWN_IMPORTS: KnownImports = { E.right({ type: 'string', format: 'number', + decodedType: 'number', }), }, 'io-ts-bigint': { @@ -264,6 +276,7 @@ export const KNOWN_IMPORTS: KnownImports = { E.right({ type: 'string', format: 'number', + decodedType: 'bigint', }), NegativeBigInt: () => E.right({ @@ -275,6 +288,7 @@ export const KNOWN_IMPORTS: KnownImports = { type: 'string', format: 'number', maximum: -1, + decodedType: 'bigint', }), NonEmptyString: () => E.right({ type: 'string', minLength: 1 }), NonNegativeBigInt: () => E.right({ type: 'number', minimum: 0 }), @@ -283,6 +297,7 @@ export const KNOWN_IMPORTS: KnownImports = { type: 'string', format: 'number', maximum: 0, + decodedType: 'bigint', }), NonPositiveBigInt: () => E.right({ @@ -294,12 +309,14 @@ export const KNOWN_IMPORTS: KnownImports = { type: 'string', format: 'number', maximum: 0, + decodedType: 'bigint', }), NonZeroBigInt: () => E.right({ type: 'number' }), NonZeroBigIntFromString: () => E.right({ type: 'string', format: 'number', + decodedType: 'bigint', }), PositiveBigInt: () => E.right({ type: 'number', minimum: 1 }), PositiveBigIntFromString: () => @@ -307,15 +324,21 @@ export const KNOWN_IMPORTS: KnownImports = { type: 'string', format: 'number', minimum: 1, + decodedType: 'bigint', }), ZeroBigInt: () => E.right({ type: 'number' }), - ZeroBigIntFromString: () => E.right({ type: 'string', format: 'number' }), + ZeroBigIntFromString: () => + E.right({ type: 'string', format: 'number', decodedType: 'bigint' }), }, 'io-ts-types': { - NumberFromString: () => E.right({ type: 'string', format: 'number' }), - BigIntFromString: () => E.right({ type: 'string', format: 'number' }), - BooleanFromNumber: () => E.right({ type: 'number', enum: [0, 1] }), - BooleanFromString: () => E.right({ type: 'string', enum: ['true', 'false'] }), + NumberFromString: () => + E.right({ type: 'string', format: 'number', decodedType: 'number' }), + BigIntFromString: () => + E.right({ type: 'string', format: 'number', decodedType: 'bigint' }), + BooleanFromNumber: () => + E.right({ type: 'number', enum: [0, 1], decodedType: 'boolean' }), + BooleanFromString: () => + E.right({ type: 'string', enum: ['true', 'false'], decodedType: 'boolean' }), DateFromISOString: () => E.right({ type: 'string', format: 'date-time', title: 'ISO Date String' }), DateFromNumber: () => @@ -332,7 +355,8 @@ export const KNOWN_IMPORTS: KnownImports = { format: 'number', description: 'Number of seconds since the Unix epoch', }), - IntFromString: () => E.right({ type: 'string', format: 'integer' }), + IntFromString: () => + E.right({ type: 'string', format: 'integer', decodedType: 'number' }), JsonFromString: () => E.right({ type: 'string', title: 'JSON String' }), nonEmptyArray: (_, innerSchema) => E.right({ type: 'array', items: innerSchema, minItems: 1 }), diff --git a/packages/openapi-generator/src/optimize.ts b/packages/openapi-generator/src/optimize.ts index 510aad68..dbf8e483 100644 --- a/packages/openapi-generator/src/optimize.ts +++ b/packages/openapi-generator/src/optimize.ts @@ -31,10 +31,48 @@ export function foldIntersection(schema: Schema, optimize: OptimizeFn): Schema { return result; } +function consolidateUnion(schema: Schema): Schema { + if (schema.type !== 'union') return schema; + if (schema.schemas.length === 1) return schema.schemas[0]!; + if (schema.schemas.length === 0) return { type: 'undefined' }; + + const consolidatableTypes = ['boolean', 'number', 'string']; + const innerSchemas = schema.schemas.map(optimize); + + const isConsolidatableType = (s: Schema): boolean => { + return ( + (s.primitive && consolidatableTypes.includes(s.type)) || + (s.decodedType !== undefined && consolidatableTypes.includes(s.decodedType)) + ); + }; + + /** + * We need to check three things: + * 1. All the schemas satisfy isConsolidatableType + * 2. All the schemas have the same decodedType type (aka type at runtime, or the `A` type of the codec) + * 3. At least one of the schemas is a primitive type + * + * If all these conditions are satisfied, we can prove to ourselves that this is a union that + * we can consolidate to the decodedType (runtime) type. + */ + + const allConsolidatable = innerSchemas.every(isConsolidatableType); + const hasPrimitive = innerSchemas.some((s: Schema) => s.primitive); + + const innerSchemaTypes = new Set(innerSchemas.map((s) => s.decodedType || s.type)); + const areSameRuntimeType = innerSchemaTypes.size === 1; + + if (allConsolidatable && areSameRuntimeType && hasPrimitive) { + return { type: Array.from(innerSchemaTypes)[0] as Primitive['type'] }; + } else { + return schema; + } +} + function mergeUnions(schema: Schema): Schema { if (schema.type !== 'union') return schema; - else if (schema.schemas.length === 1) return schema.schemas[0]!; - else if (schema.schemas.length === 0) return { type: 'undefined' }; + if (schema.schemas.length === 1) return schema.schemas[0]!; + if (schema.schemas.length === 0) return { type: 'undefined' }; // Stringified schemas (i.e. hashes of the schemas) to avoid duplicates const resultingSchemas: Set = new Set(); @@ -75,13 +113,9 @@ function mergeUnions(schema: Schema): Schema { } export function simplifyUnion(schema: Schema, optimize: OptimizeFn): Schema { - if (schema.type !== 'union') { - return schema; - } else if (schema.schemas.length === 1) { - return schema.schemas[0]!; - } else if (schema.schemas.length === 0) { - return { type: 'undefined' }; - } + if (schema.type !== 'union') return schema; + if (schema.schemas.length === 1) return schema.schemas[0]!; + if (schema.schemas.length === 0) return { type: 'undefined' }; const innerSchemas = schema.schemas.map(optimize); @@ -176,7 +210,8 @@ export function optimize(schema: Schema): Schema { } return newSchema; } else if (schema.type === 'union') { - const simplified = simplifyUnion(schema, optimize); + const consolidated = consolidateUnion(schema); + const simplified = simplifyUnion(consolidated, optimize); const merged = mergeUnions(simplified); if (schema.comment) { From eb71404246eb29d9125730b64db96218c0646305 Mon Sep 17 00:00:00 2001 From: Ansh Chaturvedi Date: Tue, 23 Jul 2024 11:35:18 -0400 Subject: [PATCH 2/4] fix: add openapi generation test --- .../openapi-generator/test/openapi.test.ts | 118 ++++++++++++++++++ 1 file changed, 118 insertions(+) diff --git a/packages/openapi-generator/test/openapi.test.ts b/packages/openapi-generator/test/openapi.test.ts index f9026652..12a9511f 100644 --- a/packages/openapi-generator/test/openapi.test.ts +++ b/packages/openapi-generator/test/openapi.test.ts @@ -3689,3 +3689,121 @@ testCase("route with array examples", ROUTE_WITH_ARRAY_EXAMPLE, { schemas: {} } }); + +const ROUTE_WITH_CONSOLIDATABLE_UNION_SCHEMAS = ` +import * as t from 'io-ts'; +import * as h from '@api-ts/io-ts-http'; +import { BooleanFromString, BooleanFromNumber, NumberFromString } from 'io-ts-types'; + +export const route = h.httpRoute({ + path: '/foo', + method: 'GET', + request: h.httpRequest({ + query: { + // are not consolidatable + firstUnion: t.union([t.string, t.number]), + secondUnion: t.union([BooleanFromString, NumberFromString]), + thirdUnion: t.union([t.string, BooleanFromString]), + firstNonUnion: BooleanFromString, + secondNonUnion: NumberFromString, + thirdNonUnion: t.string, + }, + }), + response: { + 200: { + // are consolidatable + fourthUnion: t.union([t.boolean, BooleanFromNumber]), + fifthUnion: h.optional(t.union([t.boolean, t.boolean, BooleanFromNumber, BooleanFromString])), + sixthUnion: t.union([t.number, NumberFromString]), + } + }, +}); +`; + +testCase("route with consolidatable union schemas", ROUTE_WITH_CONSOLIDATABLE_UNION_SCHEMAS, { + openapi: '3.0.3', + info: { + title: 'Test', + version: '1.0.0' + }, + paths: { + '/foo': { + get: { + parameters: [ + { + name: 'firstUnion', + in: 'query', + required: true, + schema: { + oneOf: [ + { type: 'string' }, + { type: 'number' } + ] + } + }, + { + name: 'secondUnion', + in: 'query', + required: true, + schema: { + oneOf: [ + { type: 'string', format: 'number' }, + { type: 'string', enum: [ 'true', 'false' ] } + ] + } + }, + { + name: 'thirdUnion', + in: 'query', + required: true, + schema: { + oneOf: [ + { type: 'string' }, + { type: 'string', enum: [ 'true', 'false' ] } + ] + } + }, + { + name: 'firstNonUnion', + in: 'query', + required: true, + schema: { type: 'string', enum: [ 'true', 'false' ] } + }, + { + name: 'secondNonUnion', + in: 'query', + required: true, + schema: { type: 'string', format: 'number' } + }, + { + name: 'thirdNonUnion', + in: 'query', + required: true, + schema: { type: 'string' } + } + ], + responses: { + '200': { + description: 'OK', + content: { + 'application/json': { + schema: { + type: 'object', + properties: { + fourthUnion: { type: 'boolean' }, + fifthUnion: { type: 'boolean' }, + sixthUnion: { type: 'number' } + }, + required: [ 'fourthUnion', 'sixthUnion' ] + } + } + } + } + } + } + } + }, + components: { + schemas: {} + } +}); From edc41cf0f6505150c00b5c1fb583eb8968a1441f Mon Sep 17 00:00:00 2001 From: Ansh Chaturvedi Date: Tue, 23 Jul 2024 17:06:56 -0400 Subject: [PATCH 3/4] fix: fix tests to match `primitive` field Ticket: DX-614 --- .../openapi-generator/test/apiSpec.test.ts | 10 +- packages/openapi-generator/test/codec.test.ts | 92 ++++++++++--------- .../test/externalModule.test.ts | 4 + .../openapi-generator/test/resolve.test.ts | 6 +- packages/openapi-generator/test/route.test.ts | 43 +++++---- 5 files changed, 85 insertions(+), 70 deletions(-) diff --git a/packages/openapi-generator/test/apiSpec.test.ts b/packages/openapi-generator/test/apiSpec.test.ts index 6c9958ab..08c96ccd 100644 --- a/packages/openapi-generator/test/apiSpec.test.ts +++ b/packages/openapi-generator/test/apiSpec.test.ts @@ -80,7 +80,7 @@ testCase('simple api spec', SIMPLE, '/index.ts', { path: '/test', method: 'GET', parameters: [], - response: { 200: { type: 'string' } }, + response: { 200: { type: 'string', primitive: true } }, }, ], }); @@ -112,7 +112,7 @@ testCase('const route reference', ROUTE_REF, '/index.ts', { path: '/test', method: 'GET', parameters: [], - response: { 200: { type: 'string' } }, + response: { 200: { type: 'string', primitive: true } }, }, ], }); @@ -144,7 +144,7 @@ testCase('const action reference', ACTION_REF, '/index.ts', { path: '/test', method: 'GET', parameters: [], - response: { 200: { type: 'string' } }, + response: { 200: { type: 'string', primitive: true } }, }, ], }); @@ -182,7 +182,7 @@ testCase('spread api spec', SPREAD, '/index.ts', { path: '/test', method: 'GET', parameters: [], - response: { 200: { type: 'string' } }, + response: { 200: { type: 'string', primitive: true } }, }, ], }); @@ -216,7 +216,7 @@ testCase('computed property api spec', COMPUTED_PROPERTY, '/index.ts', { path: '/test', method: 'GET', parameters: [], - response: { 200: { type: 'string' } }, + response: { 200: { type: 'string', primitive: true } }, }, ], }); diff --git a/packages/openapi-generator/test/codec.test.ts b/packages/openapi-generator/test/codec.test.ts index cf690994..e833c04d 100644 --- a/packages/openapi-generator/test/codec.test.ts +++ b/packages/openapi-generator/test/codec.test.ts @@ -46,7 +46,7 @@ export const FOO = t.number; `; testCase('simple codec is parsed', SIMPLE, { - FOO: { type: 'number' }, + FOO: { type: 'number', primitive: true }, }); const DIRECT = ` @@ -55,7 +55,7 @@ export const FOO = number; `; testCase('direct import is parsed', DIRECT, { - FOO: { type: 'number' }, + FOO: { type: 'number', primitive: true }, }); const TYPE = ` @@ -66,7 +66,7 @@ export const FOO = t.type({ foo: t.number }); testCase('type is parsed', TYPE, { FOO: { type: 'object', - properties: { foo: { type: 'number' } }, + properties: { foo: { type: 'number', primitive: true } }, required: ['foo'], }, }); @@ -79,7 +79,7 @@ export const FOO = t.partial({ foo: t.number }); testCase('partial type is parsed', PARTIAL, { FOO: { type: 'object', - properties: { foo: { type: 'number' } }, + properties: { foo: { type: 'number', primitive: true } }, required: [], }, }); @@ -91,10 +91,10 @@ export const FOO = t.type({ bar }); `; testCase('shorthand property is parsed', SHORTHAND_PROPERTY, { - bar: { type: 'number' }, + bar: { type: 'number', primitive: true }, FOO: { type: 'object', - properties: { bar: { type: 'number' } }, + properties: { bar: { type: 'number', primitive: true } }, required: ['bar'], }, }); @@ -110,14 +110,14 @@ export const TEST = t.type({ ...foo, bar: t.string }); testCase('spread property is parsed', SPREAD_PROPERTY, { foo: { type: 'object', - properties: { foo: { type: 'number' } }, + properties: { foo: { type: 'number', primitive: true } }, required: ['foo'], }, TEST: { type: 'object', properties: { - foo: { type: 'number' }, - bar: { type: 'string' }, + foo: { type: 'number', primitive: true }, + bar: { type: 'string', primitive: true }, }, required: ['foo', 'bar'], }, @@ -132,12 +132,12 @@ export const FOO = t.type(props); testCase('const assertion is parsed', CONST_ASSERTION, { FOO: { type: 'object', - properties: { foo: { type: 'number' } }, + properties: { foo: { type: 'number', primitive: true } }, required: ['foo'], }, props: { type: 'object', - properties: { foo: { type: 'number' } }, + properties: { foo: { type: 'number', primitive: true } }, required: ['foo'], }, }); @@ -153,12 +153,12 @@ export const FOO = t.type({ testCase('spread const assertion is parsed', SPREAD_CONST_ASSERTION, { FOO: { type: 'object', - properties: { foo: { type: 'number' } }, + properties: { foo: { type: 'number', primitive: true } }, required: ['foo'], }, props: { type: 'object', - properties: { foo: { type: 'number' } }, + properties: { foo: { type: 'number', primitive: true } }, required: ['foo'], }, }); @@ -172,12 +172,12 @@ export const FOO = t.type(props); testCase('as assertion is parsed', AS_ASSERTION, { FOO: { type: 'object', - properties: { foo: { type: 'number' } }, + properties: { foo: { type: 'number', primitive: true } }, required: ['foo'], }, props: { type: 'object', - properties: { foo: { type: 'number' } }, + properties: { foo: { type: 'number', primitive: true } }, required: ['foo'], }, }); @@ -193,12 +193,12 @@ export const FOO = t.type({ testCase('spread const assertion is parsed', SPREAD_AS_ASSERTION, { FOO: { type: 'object', - properties: { foo: { type: 'number' } }, + properties: { foo: { type: 'number', primitive: true } }, required: ['foo'], }, props: { type: 'object', - properties: { foo: { type: 'number' } }, + properties: { foo: { type: 'number', primitive: true } }, required: ['foo'], }, }); @@ -209,7 +209,7 @@ export const FOO = t.array(t.number); `; testCase('array type is parsed', ARRAY, { - FOO: { type: 'array', items: { type: 'number' } }, + FOO: { type: 'array', items: { type: 'number', primitive: true } }, }); const UNION = ` @@ -220,7 +220,7 @@ export const FOO = t.union([t.string, t.number]); testCase('union type is parsed', UNION, { FOO: { type: 'union', - schemas: [{ type: 'string' }, { type: 'number' }], + schemas: [{ type: 'string', primitive: true }, { type: 'number', primitive: true }], }, }); @@ -235,11 +235,11 @@ export const FOO = t.union([...common, t.number]); testCase('union type with spread is parsed', UNION_SPREAD, { FOO: { type: 'union', - schemas: [{ type: 'string' }, { type: 'number' }], + schemas: [{ type: 'string', primitive: true }, { type: 'number', primitive: true }], }, common: { type: 'tuple', - schemas: [{ type: 'string' }], + schemas: [{ type: 'string', primitive: true }], }, }); @@ -252,7 +252,7 @@ export const FOO = t.union([...[t.string], t.number]); testCase('union type with inline spread is parsed', UNION_INLINE_SPREAD, { FOO: { type: 'union', - schemas: [{ type: 'string' }, { type: 'number' }], + schemas: [{ type: 'string', primitive: true }, { type: 'number', primitive: true }], }, }); @@ -267,12 +267,12 @@ testCase('intersection type is parsed', INTERSECTION, { schemas: [ { type: 'object', - properties: { foo: { type: 'number' } }, + properties: { foo: { type: 'number', primitive: true } }, required: ['foo'], }, { type: 'object', - properties: { bar: { type: 'string' } }, + properties: { bar: { type: 'string', primitive: true } }, required: [], }, ], @@ -285,7 +285,7 @@ export const FOO = t.record(t.string, t.number); `; testCase('record type is parsed', RECORD, { - FOO: { type: 'record', codomain: { type: 'number' } }, + FOO: { type: 'record', codomain: { type: 'number', primitive: true } }, }); const ENUM = ` @@ -381,7 +381,7 @@ export const FOO = test; `; testCase('aliased import is parsed', ALIAS, { - FOO: { type: 'string' }, + FOO: { type: 'string', primitive: true }, }); const BRAND = ` @@ -394,7 +394,7 @@ export const FOO = t.brand(t.string, (s): s is FooBranded => s === 'foo', 'Foo') `; testCase('brand type is parsed', BRAND, { - FOO: { type: 'string' }, + FOO: { type: 'string', primitive: true }, }); const LOCAL_REF = ` @@ -406,7 +406,7 @@ export const BAR = t.type({ bar: FOO }); testCase('local ref is parsed', LOCAL_REF, { FOO: { type: 'object', - properties: { foo: { type: 'number' } }, + properties: { foo: { type: 'number', primitive: true } }, required: ['foo'], }, BAR: { @@ -425,7 +425,7 @@ export const BAR = t.type({ bar: FOO }); testCase('local exported ref is parsed', LOCAL_EXPORTED_REF, { FOO: { type: 'object', - properties: { foo: { type: 'number' } }, + properties: { foo: { type: 'number', primitive: true } }, required: ['foo'], }, BAR: { @@ -476,6 +476,7 @@ export const FOO = t.number; testCase('declaration comment is parsed', DECLARATION_COMMENT, { FOO: { type: 'number', + primitive: true, comment: { description: 'Test codec', tags: [], @@ -553,7 +554,8 @@ testCase( DECLARATION_COMMENT_WITHOUT_LINE_BREAK, { FOO: { - type: 'number', + type: 'number', + primitive: true, comment: { description: 'Test codec', tags: [], @@ -633,6 +635,7 @@ testCase('first property comment is parsed', FIRST_PROPERTY_COMMENT, { properties: { foo: { type: 'number', + primitive: true, comment: { description: 'this is a comment', problems: [], @@ -677,9 +680,10 @@ testCase('second property comment is parsed', SECOND_PROPERTY_COMMENT, { FOO: { type: 'object', properties: { - foo: { type: 'number' }, + foo: { type: 'number', primitive: true }, bar: { - type: 'string', + type: 'string', + primitive: true, comment: { description: 'this is a comment', problems: [], @@ -720,7 +724,7 @@ export const FOO = h.optional(t.string); testCase('optional combinator is parsed', OPTIONAL_COMBINATOR, { FOO: { type: 'union', - schemas: [{ type: 'string' }, { type: 'undefined' }], + schemas: [{ type: 'string', primitive: true }, { type: 'undefined' }], }, }); @@ -737,10 +741,10 @@ testCase('optionalized combinator is parsed', OPTIONALIZED_COMBINATOR, { FOO: { type: 'object', properties: { - foo: { type: 'string' }, + foo: { type: 'string', primitive: true }, bar: { type: 'union', - schemas: [{ type: 'number' }, { type: 'undefined' }], + schemas: [{ type: 'number', primitive: true }, { type: 'undefined' }], }, }, required: ['foo'], @@ -766,7 +770,7 @@ testCase('httpRequest combinator is parsed', HTTP_REQUEST_COMBINATOR, { properties: { params: { type: 'object', - properties: { foo: { type: 'string' } }, + properties: { foo: { type: 'string', primitive: true } }, required: ['foo'], }, query: { @@ -774,7 +778,7 @@ testCase('httpRequest combinator is parsed', HTTP_REQUEST_COMBINATOR, { properties: { bar: { type: 'union', - schemas: [{ type: 'number' }, { type: 'undefined' }], + schemas: [{ type: 'number', primitive: true }, { type: 'undefined' }], }, }, required: [], @@ -801,15 +805,15 @@ testCase('object property is parsed', OBJECT_PROPERTY, { FOO: { type: 'object', properties: { - baz: { type: 'number' }, + baz: { type: 'number', primitive: true }, }, required: ['baz'], }, props: { type: 'object', properties: { - foo: { type: 'number' }, - bar: { type: 'string' }, + foo: { type: 'number', primitive: true }, + bar: { type: 'string', primitive: true }, }, required: ['foo', 'bar'], }, @@ -830,16 +834,16 @@ testCase('object assign is parsed', OBJECT_ASSIGN, { FOO: { type: 'object', properties: { - foo: { type: 'number' }, - bar: { type: 'string' }, + foo: { type: 'number', primitive: true }, + bar: { type: 'string', primitive: true }, }, required: ['foo', 'bar'], }, props: { type: 'object', properties: { - foo: { type: 'number' }, - bar: { type: 'string' }, + foo: { type: 'number', primitive: true }, + bar: { type: 'string', primitive: true }, }, required: ['foo', 'bar'], }, diff --git a/packages/openapi-generator/test/externalModule.test.ts b/packages/openapi-generator/test/externalModule.test.ts index 38efa569..4a09985a 100644 --- a/packages/openapi-generator/test/externalModule.test.ts +++ b/packages/openapi-generator/test/externalModule.test.ts @@ -69,9 +69,11 @@ const FoobarObject: Schema = { properties: { foo: { type: 'string', + primitive: true, }, bar: { type: 'number', + primitive: true, }, }, required: ['foo', 'bar'], @@ -82,9 +84,11 @@ const RandomTypeObject: Schema = { properties: { random: { type: 'string', + primitive: true, }, type: { type: 'number', + primitive: true, }, }, required: ['random', 'type'], diff --git a/packages/openapi-generator/test/resolve.test.ts b/packages/openapi-generator/test/resolve.test.ts index fe11bddb..b6a11d91 100644 --- a/packages/openapi-generator/test/resolve.test.ts +++ b/packages/openapi-generator/test/resolve.test.ts @@ -63,7 +63,7 @@ testCase( { FOO: { type: 'object', - properties: { foo: { type: 'string' } }, + properties: { foo: { type: 'string', primitive: true } }, required: ['foo'], }, }, @@ -85,7 +85,7 @@ testCase( { FOO: { type: 'union', - schemas: [{ type: 'string' }, { type: 'number' }], + schemas: [{ type: 'string', primitive: true }, { type: 'number', primitive: true }], }, }, ['Unimplemented initializer type ArrayExpression'], @@ -544,7 +544,7 @@ testCase( FOO: { type: 'object', properties: { - foo: { type: 'number' }, + foo: { type: 'number', primitive: true }, }, required: ['foo'], }, diff --git a/packages/openapi-generator/test/route.test.ts b/packages/openapi-generator/test/route.test.ts index 3cfa3b08..efe777b3 100644 --- a/packages/openapi-generator/test/route.test.ts +++ b/packages/openapi-generator/test/route.test.ts @@ -85,12 +85,14 @@ testCase('simple route', SIMPLE, { required: true, schema: { type: 'string', + primitive: true, }, }, ], response: { 200: { type: 'string', + primitive: true, }, }, }, @@ -124,6 +126,7 @@ testCase('path params route', PATH_PARAMS, { required: true, schema: { type: 'string', + primitive: true, }, }, ], @@ -165,13 +168,14 @@ testCase('optional query param route', OPTIONAL_QUERY_PARAM, { required: false, schema: { type: 'union', - schemas: [{ type: 'string' }, { type: 'undefined' }], + schemas: [{ type: 'string', primitive: true }, { type: 'undefined' }], }, }, ], response: { 200: { type: 'string', + primitive: true, }, }, }, @@ -201,6 +205,7 @@ testCase('const request route', REQUEST_REF, { response: { 200: { type: 'string', + primitive: true, }, }, }, @@ -228,7 +233,7 @@ testCase('const response route', RESPONSE_REF, { method: 'GET', parameters: [], response: { - 200: { type: 'string' }, + 200: { type: 'string', primitive: true }, }, }, }); @@ -273,14 +278,14 @@ testCase('query param union route', QUERY_PARAM_UNION, { { type: 'object', properties: { - foo: { type: 'string' }, + foo: { type: 'string', primitive: true }, }, required: ['foo'], }, { type: 'object', properties: { - bar: { type: 'string' }, + bar: { type: 'string', primitive: true }, }, required: ['bar'], }, @@ -289,7 +294,7 @@ testCase('query param union route', QUERY_PARAM_UNION, { }, ], response: { - 200: { type: 'string' }, + 200: { type: 'string', primitive: true }, }, }, }); @@ -340,14 +345,14 @@ testCase('path param union route', PATH_PARAM_UNION, { { type: 'object', properties: { - foo: { type: 'string' }, + foo: { type: 'string', primitive: true }, }, required: ['foo'], }, { type: 'object', properties: { - bar: { type: 'string' }, + bar: { type: 'string', primitive: true }, }, required: ['bar'], }, @@ -358,11 +363,11 @@ testCase('path param union route', PATH_PARAM_UNION, { type: 'path', name: 'id', required: true, - schema: { type: 'string' }, + schema: { type: 'string', primitive: true }, }, ], response: { - 200: { type: 'string' }, + 200: { type: 'string', primitive: true }, }, }, }); @@ -402,21 +407,21 @@ testCase('body union route', BODY_UNION, { { type: 'object', properties: { - foo: { type: 'string' }, + foo: { type: 'string', primitive: true }, }, required: ['foo'], }, { type: 'object', properties: { - bar: { type: 'string' }, + bar: { type: 'string', primitive: true }, }, required: ['bar'], }, ], }, response: { - 200: { type: 'string' }, + 200: { type: 'string', primitive: true }, }, }, }); @@ -454,17 +459,17 @@ testCase('request intersection route', REQUEST_INTERSECTION, { type: 'query', name: 'foo', required: true, - schema: { type: 'string' }, + schema: { type: 'string', primitive: true }, }, { type: 'query', name: 'bar', required: true, - schema: { type: 'string' }, + schema: { type: 'string', primitive: true }, }, ], response: { - 200: { type: 'string' }, + 200: { type: 'string', primitive: true }, }, }, }); @@ -504,21 +509,21 @@ testCase('request body intersection route', BODY_INTERSECTION, { { type: 'object', properties: { - foo: { type: 'string' }, + foo: { type: 'string', primitive: true }, }, required: ['foo'], }, { type: 'object', properties: { - bar: { type: 'string' }, + bar: { type: 'string', primitive: true }, }, required: ['bar'], }, ], }, response: { - 200: { type: 'string' }, + 200: { type: 'string', primitive: true }, }, }, }); @@ -557,12 +562,14 @@ testCase('route with operationId', WITH_OPERATION_ID, { required: true, schema: { type: 'string', + primitive: true, }, }, ], response: { 200: { type: 'string', + primitive: true, }, }, comment: { From 8d610adad2fc91da7c97fd6e7867e8b479ffceb2 Mon Sep 17 00:00:00 2001 From: Ansh Chaturvedi Date: Tue, 23 Jul 2024 17:27:21 -0400 Subject: [PATCH 4/4] fix: add tests to test union `optimize` fn --- .../openapi-generator/test/optimize.test.ts | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/packages/openapi-generator/test/optimize.test.ts b/packages/openapi-generator/test/optimize.test.ts index ad0a7fbf..eaa58af7 100644 --- a/packages/openapi-generator/test/optimize.test.ts +++ b/packages/openapi-generator/test/optimize.test.ts @@ -144,3 +144,39 @@ test('comments are exposed in objects', () => { assert.deepEqual(optimize(input), expected); }); + +test('consolidatable unions are consolidated to single primitive type', () => { + const input: Schema = { + type: 'union', + schemas: [ + { type: 'string', enum: ['true', 'false'], decodedType: 'boolean' }, + { type: 'boolean', primitive: true }, + ], + required: [], + }; + + const expected: Schema = { type: 'boolean', }; + + assert.deepEqual(optimize(input), expected); +}); + +test('non-consolidatable unions are not consolidated', () => { + const input: Schema = { + type: 'union', + schemas: [ + { type: 'string', enum: ['true', 'false'], decodedType: 'boolean' }, + { type: 'string', primitive: true }, + ], + required: [], + }; + + const expected: Schema = { + type: 'union', + schemas: [ + { type: 'string', primitive: true }, + { type: 'string', enum: [ 'true', 'false' ] }, + ] + }; + + assert.deepEqual(optimize(input), expected); +});