Skip to content

Commit fe29179

Browse files
fantapopfantapop
committed
Simplify generated openapi 3 spec
Simplified the final generated spec by: 1) grouping enums of the same type into a single enum of that type for example: schema: anyOf: - type: string enum: - one - two - type: string enum: - three - type: number enum: - 1 - 2 would be reduced to: schema: anyOf: - type: string enum: - one - two - three - type: number enum: - 1 - 2 2) removing duplicate subschemas from anyOf/allOf for example: schema: anyOf: - type: string - type: string - type: number would be reduced to: schema: anyOf: - type: string - type: number 3) remove some anyOf/allOf constructs when there is only a single element. for example: schema: anyOf: - type: string would be reduced to: schema: type: string Closes: lukeautry#708
1 parent 56884d7 commit fe29179

File tree

4 files changed

+130
-63
lines changed

4 files changed

+130
-63
lines changed

‎packages/cli/src/swagger/specGenerator3.ts‎

Lines changed: 65 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -458,9 +458,54 @@ export class SpecGenerator3 extends SpecGenerator {
458458
return type.dataType === 'enum' && type.enums.length === 1 && type.enums[0] === null;
459459
}
460460

461+
// Join disparate enums with the same type into one.
462+
//
463+
// grouping enums is helpful because it makes the spec more readable and it
464+
// bypasses a failure in openapi-generator caused by using anyOf with
465+
// duplicate types.
466+
private groupEnums(types: Array<Swagger.Schema | Swagger.BaseSchema>) {
467+
const returnTypes: Array<Swagger.Schema | Swagger.BaseSchema> = [];
468+
const enumValuesByType = {};
469+
for (const type of types) {
470+
if (type.enum && type.type) {
471+
for (const enumValue of type.enum) {
472+
if (!enumValuesByType[type.type]) {
473+
enumValuesByType[type.type] = [];
474+
}
475+
enumValuesByType[type.type][enumValue] = enumValue;
476+
}
477+
}
478+
// preserve non-enum types
479+
else {
480+
returnTypes.push(type);
481+
}
482+
}
483+
484+
Object.keys(enumValuesByType).forEach(dataType =>
485+
returnTypes.push({
486+
type: dataType,
487+
enum: Object.values(enumValuesByType[dataType]),
488+
}),
489+
);
490+
491+
return returnTypes;
492+
}
493+
494+
protected removeDuplicateSwaggerTypes(types: Array<Swagger.Schema | Swagger.BaseSchema>) {
495+
if (types.length === 1) {
496+
return types;
497+
} else {
498+
const typesSet = new Set<string>();
499+
for (const type of types) {
500+
typesSet.add(JSON.stringify(type));
501+
}
502+
return Array.from(typesSet).map(typeString => JSON.parse(typeString));
503+
}
504+
}
505+
461506
protected getSwaggerTypeForUnionType(type: Tsoa.UnionType) {
462-
const notNullSwaggerTypes = type.types.filter(x => !this.isNull(x)).map(x => this.getSwaggerType(x));
463-
const nullable = type.types.length !== notNullSwaggerTypes.length;
507+
const notNullSwaggerTypes = this.removeDuplicateSwaggerTypes(this.groupEnums(type.types.filter(x => !this.isNull(x)).map(x => this.getSwaggerType(x))));
508+
const nullable = type.types.some(x => this.isNull(x));
464509

465510
if (nullable && notNullSwaggerTypes.length === 1) {
466511
const [swaggerType] = notNullSwaggerTypes;
@@ -480,9 +525,25 @@ export class SpecGenerator3 extends SpecGenerator {
480525
}
481526

482527
if (nullable) {
483-
return { anyOf: notNullSwaggerTypes, nullable };
528+
if (notNullSwaggerTypes.length === 1) {
529+
const [swaggerType] = notNullSwaggerTypes;
530+
// for ref union with null, use an allOf with a single
531+
// element since you can't attach nullable directly to a ref.
532+
// https://swagger.io/docs/specification/using-ref/#syntax
533+
if (swaggerType.$ref) {
534+
return { allOf: [swaggerType], nullable };
535+
}
536+
537+
return { ...swaggerType, nullable };
538+
} else {
539+
return { anyOf: notNullSwaggerTypes, nullable };
540+
}
484541
} else {
485-
return { anyOf: notNullSwaggerTypes };
542+
if (notNullSwaggerTypes.length === 1) {
543+
return notNullSwaggerTypes[0];
544+
} else {
545+
return { anyOf: notNullSwaggerTypes };
546+
}
486547
}
487548
}
488549

‎tests/fixtures/testModel.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,8 @@ export interface TestModel extends Model {
5959
modelsArray: TestSubModel[];
6060
strLiteralVal: StrLiteral;
6161
strLiteralArr: StrLiteral[];
62-
unionPrimetiveType?: 'String' | 1 | 20.0 | true | false;
62+
unionPrimitiveType?: 'String' | 1 | 20.0 | true | false;
63+
nullableUnionPrimitiveType?: 'String' | 1 | 20.0 | true | false | null;
6364
singleFloatLiteralType?: 3.1415;
6465
dateValue?: Date;
6566
optionalString?: string;

‎tests/unit/swagger/definitionsGeneration/definitions.spec.ts‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -397,7 +397,7 @@ describe('Definition generation', () => {
397397

398398
expect(propertySchema['x-nullable']).to.eq(undefined, `for property ${propertyName}[x-nullable]`);
399399
},
400-
unionPrimetiveType: (propertyName, propertySchema) => {
400+
unionPrimitiveType: (propertyName, propertySchema) => {
401401
expect(propertySchema.type).to.eq('string', `for property ${propertyName}.type`);
402402
expect(propertySchema['x-nullable']).to.eq(false, `for property ${propertyName}[x-nullable]`);
403403
if (!propertySchema.enum) {
@@ -410,6 +410,20 @@ describe('Definition generation', () => {
410410
expect(propertySchema.enum).to.include('true', `for property ${propertyName}.enum`);
411411
expect(propertySchema.enum).to.include('false', `for property ${propertyName}.enum`);
412412
},
413+
nullableUnionPrimitiveType: (propertyName, propertySchema) => {
414+
expect(propertySchema.type).to.eq('string', `for property ${propertyName}.type`);
415+
expect(propertySchema['x-nullable']).to.eq(true, `for property ${propertyName}[x-nullable]`);
416+
if (!propertySchema.enum) {
417+
throw new Error(`There was no 'enum' property on ${propertyName}.`);
418+
}
419+
expect(propertySchema.enum).to.have.length(6, `for property ${propertyName}.enum`);
420+
expect(propertySchema.enum).to.include('String', `for property ${propertyName}.enum`);
421+
expect(propertySchema.enum).to.include('1', `for property ${propertyName}.enum`);
422+
expect(propertySchema.enum).to.include('20', `for property ${propertyName}.enum`);
423+
expect(propertySchema.enum).to.include('true', `for property ${propertyName}.enum`);
424+
expect(propertySchema.enum).to.include('false', `for property ${propertyName}.enum`);
425+
expect(propertySchema.enum).to.include(null, `for property ${propertyName}.enum`);
426+
},
413427
singleFloatLiteralType: (propertyName, propertySchema) => {
414428
expect(propertySchema.type).to.eq('number', `for property ${propertyName}.type`);
415429
expect(propertySchema['x-nullable']).to.eq(false, `for property ${propertyName}[x-nullable]`);

‎tests/unit/swagger/schemaDetails3.spec.ts‎

Lines changed: 48 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -728,15 +728,12 @@ describe('Definition generation for OpenAPI 3.0.0', () => {
728728

729729
const componentSchema = getComponentSchema('StrLiteral', currentSpec);
730730
expect(componentSchema).to.deep.eq({
731-
anyOf: [
732-
{ type: 'string', enum: [''], nullable: false },
733-
{ type: 'string', enum: ['Foo'], nullable: false },
734-
{ type: 'string', enum: ['Bar'], nullable: false },
735-
],
736731
default: undefined,
737732
description: undefined,
733+
enum: ['', 'Foo', 'Bar'],
738734
example: undefined,
739735
format: undefined,
736+
type: 'string',
740737
});
741738
},
742739
strLiteralArr: (propertyName, propertySchema) => {
@@ -746,19 +743,31 @@ describe('Definition generation for OpenAPI 3.0.0', () => {
746743

747744
expect(propertySchema.nullable).to.eq(undefined, `for property ${propertyName}[x-nullable]`);
748745
},
749-
unionPrimetiveType: (propertyName, propertySchema) => {
746+
unionPrimitiveType: (propertyName, propertySchema) => {
750747
expect(propertySchema).to.deep.eq({
751748
anyOf: [
752-
{ type: 'string', enum: ['String'], nullable: false },
753-
{ type: 'number', enum: ['1'], nullable: false },
754-
{ type: 'number', enum: ['20'], nullable: false },
755-
{ type: 'boolean', enum: ['true'], nullable: false },
756-
{ type: 'boolean', enum: ['false'], nullable: false },
749+
{ type: 'string', enum: ['String'] },
750+
{ type: 'number', enum: ['1', '20'] },
751+
{ type: 'boolean', enum: ['true', 'false'] },
757752
],
753+
default: undefined,
754+
description: undefined,
758755
example: undefined,
756+
format: undefined,
757+
});
758+
},
759+
nullableUnionPrimitiveType: (propertyName, propertySchema) => {
760+
expect(propertySchema).to.deep.eq({
761+
anyOf: [
762+
{ type: 'string', enum: ['String'] },
763+
{ type: 'number', enum: ['1', '20'] },
764+
{ type: 'boolean', enum: ['true', 'false'] },
765+
],
759766
default: undefined,
760767
description: undefined,
768+
example: undefined,
761769
format: undefined,
770+
nullable: true,
762771
});
763772
},
764773
singleFloatLiteralType: (propertyName, propertySchema) => {
@@ -899,6 +908,9 @@ describe('Definition generation for OpenAPI 3.0.0', () => {
899908

900909
const definition = getComponentSchema('GenericModel', currentSpec);
901910
expect(definition.properties!.result.type).to.deep.equal('string');
911+
// string | string reduced to just string after removal of duplicate
912+
// types when generating union spec
913+
expect(definition.properties!.union.type).to.deep.equal('string');
902914
expect(definition.properties!.nested.$ref).to.deep.equal('#/components/schemas/GenericRequest_string_');
903915
},
904916
and: (propertyName, propertySchema) => {
@@ -1277,26 +1289,27 @@ describe('Definition generation for OpenAPI 3.0.0', () => {
12771289
);
12781290
expect(excludeLiteral).to.deep.eq(
12791291
{
1280-
anyOf: [
1281-
{ type: 'string', enum: ['id'], nullable: false },
1282-
{ type: 'string', enum: ['enumKeys'], nullable: false },
1283-
{ type: 'string', enum: ['keyInterface'], nullable: false },
1284-
{ type: 'string', enum: ['indexedType'], nullable: false },
1285-
{ type: 'string', enum: ['indexedResponse'], nullable: false },
1286-
{ type: 'string', enum: ['publicStringProperty'], nullable: false },
1287-
{ type: 'string', enum: ['optionalPublicStringProperty'], nullable: false },
1288-
{ type: 'string', enum: ['emailPattern'], nullable: false },
1289-
{ type: 'string', enum: ['stringProperty'], nullable: false },
1290-
{ type: 'string', enum: ['publicConstructorVar'], nullable: false },
1291-
{ type: 'string', enum: ['readonlyConstructorArgument'], nullable: false },
1292-
{ type: 'string', enum: ['optionalPublicConstructorVar'], nullable: false },
1293-
{ type: 'string', enum: ['myIgnoredMethod'], nullable: false },
1294-
{ type: 'string', enum: ['defaultValue1'], nullable: false },
1295-
],
1296-
description: 'Exclude from T those types that are assignable to U',
12971292
default: undefined,
1293+
description: 'Exclude from T those types that are assignable to U',
1294+
enum: [
1295+
'id',
1296+
'enumKeys',
1297+
'keyInterface',
1298+
'indexedType',
1299+
'indexedResponse',
1300+
'publicStringProperty',
1301+
'optionalPublicStringProperty',
1302+
'emailPattern',
1303+
'stringProperty',
1304+
'publicConstructorVar',
1305+
'readonlyConstructorArgument',
1306+
'optionalPublicConstructorVar',
1307+
'myIgnoredMethod',
1308+
'defaultValue1',
1309+
],
12981310
example: undefined,
12991311
format: undefined,
1312+
type: 'string',
13001313
},
13011314
`for a schema linked by property ${propertyName}`,
13021315
);
@@ -1374,12 +1387,10 @@ describe('Definition generation for OpenAPI 3.0.0', () => {
13741387
enumKeys: {
13751388
default: undefined,
13761389
description: undefined,
1377-
format: undefined,
1390+
enum: ['OK', 'KO'],
13781391
example: undefined,
1379-
anyOf: [
1380-
{ enum: ['OK'], nullable: false, type: 'string' },
1381-
{ enum: ['KO'], nullable: false, type: 'string' },
1382-
],
1392+
format: undefined,
1393+
type: 'string',
13831394
},
13841395
id: { type: 'number', format: 'double', default: undefined, description: undefined, example: undefined },
13851396
indexedResponse: {
@@ -1399,40 +1410,20 @@ describe('Definition generation for OpenAPI 3.0.0', () => {
13991410
indexedTypeToInterface: { $ref: '#/components/schemas/IndexedInterface', description: undefined, format: undefined, example: undefined },
14001411
indexedTypeToAlias: { $ref: '#/components/schemas/IndexedInterfaceAlias', description: undefined, format: undefined, example: undefined },
14011412
arrayUnion: {
1402-
anyOf: [
1403-
{
1404-
enum: ['foo'],
1405-
nullable: false,
1406-
type: 'string',
1407-
},
1408-
{
1409-
enum: ['bar'],
1410-
nullable: false,
1411-
type: 'string',
1412-
},
1413-
],
14141413
default: undefined,
14151414
description: undefined,
1415+
enum: ['foo', 'bar'],
14161416
example: undefined,
14171417
format: undefined,
1418+
type: 'string',
14181419
},
14191420
objectUnion: {
1420-
anyOf: [
1421-
{
1422-
enum: ['foo'],
1423-
nullable: false,
1424-
type: 'string',
1425-
},
1426-
{
1427-
enum: ['bar'],
1428-
nullable: false,
1429-
type: 'string',
1430-
},
1431-
],
14321421
default: undefined,
14331422
description: undefined,
1423+
enum: ['foo', 'bar'],
14341424
example: undefined,
14351425
format: undefined,
1426+
type: 'string',
14361427
},
14371428
keyInterface: { type: 'string', default: undefined, description: undefined, format: undefined, example: undefined, enum: ['id'], nullable: false },
14381429
optionalPublicConstructorVar: { type: 'string', default: undefined, description: undefined, format: undefined, example: undefined },

0 commit comments

Comments
 (0)