Merge pull request #1562 from iffa/fix/openapi-3-enum-nullable

Fix invalid implementation of nullable enum

Author: WoH

packages/cli/src/swagger/specGenerator3.ts

@@ -1,14 +1,14 @@
-import { merge as mergeAnything } from 'merge-anything'
+import { Swagger, Tsoa, assertNever } from '@tsoa/runtime';
+import { merge as mergeAnything } from 'merge-anything';
 import { merge as deepMerge } from 'ts-deepmerge';
-import { Tsoa, assertNever, Swagger } from '@tsoa/runtime';
 
 import { ExtendedSpecConfig } from '../cli';
 import { isVoidType } from '../utils/isVoidType';
+import { UnspecifiedObject } from '../utils/unspecifiedObject';
+import { shouldIncludeValidatorInSchema } from '../utils/validatorUtils';
 import { convertColonPathParams, normalisePath } from './../utils/pathUtils';
 import { DEFAULT_REQUEST_MEDIA_TYPE, DEFAULT_RESPONSE_MEDIA_TYPE, getValue } from './../utils/swaggerUtils';
 import { SpecGenerator } from './specGenerator';
-import { UnspecifiedObject } from '../utils/unspecifiedObject';
-import { shouldIncludeValidatorInSchema } from '../utils/validatorUtils';
 
 /**
  * TODO:
@@ -20,7 +20,10 @@ import { shouldIncludeValidatorInSchema } from '../utils/validatorUtils';
  * Also accept OpenAPI 3.0.0 metadata, like components/securitySchemes instead of securityDefinitions
  */
 export class SpecGenerator3 extends SpecGenerator {
-  constructor(protected readonly metadata: Tsoa.Metadata, protected readonly config: ExtendedSpecConfig) {
+  constructor(
+    protected readonly metadata: Tsoa.Metadata,
+    protected readonly config: ExtendedSpecConfig,
+  ) {
     super(metadata, config);
   }
 
@@ -660,6 +663,13 @@ export class SpecGenerator3 extends SpecGenerator {
         if (swaggerType.$ref) {
           return { allOf: [swaggerType], nullable };
         }
+
+        // Note that null must be explicitly included in the list of enum values. Using nullable: true alone is not enough here.
+        // https://swagger.io/docs/specification/data-models/enums/
+        if (swaggerType.enum) {
+          swaggerType.enum.push(null);
+        }
+
         return { ...(title && { title }), ...swaggerType, nullable };
       } else {
         return { ...(title && { title }), anyOf: actualSwaggerTypes, nullable };

tests/fixtures/controllers/getController.ts

@@ -1,24 +1,24 @@
 ///<reference path="../tsoaTestModule.d.ts" />
+import { Controller, Example, Get, OperationId, Queries, Query, Request, Res, Response, Route, SuccessResponse, Tags, TsoaResponse } from '@tsoa/runtime';
 import { Readable } from 'stream';
-import { Controller, Example, Get, OperationId, Query, Request, Response, Route, SuccessResponse, Tags, Res, TsoaResponse, Queries } from '@tsoa/runtime';
+import TsoaTest from 'tsoaTest';
 import '../duplicateTestModel';
 import {
   GenericModel,
   GetterClass,
   GetterInterface,
   GetterInterfaceHerited,
-  TestClassModel,
-  TestModel,
-  TestSubModel,
-  SimpleClassWithToJSON,
   IndexedValue,
-  IndexedValueReference,
-  ParenthesizedIndexedValue,
   IndexedValueGeneric,
+  IndexedValueReference,
   IndexedValueTypeReference,
+  ParenthesizedIndexedValue,
+  SimpleClassWithToJSON,
+  TestClassModel,
+  TestModel,
+  TestSubModel,
 } from '../testModel';
 import { ModelService } from './../services/modelService';
-import TsoaTest from 'tsoaTest';
 
 export const PathFromConstant = 'PathFromConstantValue';
 export enum EnumPaths {
@@ -66,6 +66,7 @@ export class GetTestController extends Controller {
     strLiteralVal: 'Foo',
     stringArray: ['string one', 'string two'],
     stringValue: 'a string',
+    nullableStringLiteral: 'NULLABLE_LIT_1',
     undefineableUnionPrimitiveType: undefined,
     undefinedValue: undefined,
   })

tests/fixtures/testModel.ts

@@ -67,6 +67,7 @@ export interface TestModel extends Model {
   modelsArray: TestSubModel[];
   strLiteralVal: StrLiteral;
   strLiteralArr: StrLiteral[];
+  nullableStringLiteral?: 'NULLABLE_LIT_1' | 'NULLABLE_LIT_2' | null;
   unionPrimitiveType?: 'String' | 1 | 20.0 | true | false;
   nullableUnionPrimitiveType?: 'String' | 1 | 20.0 | true | false | null;
   undefineableUnionPrimitiveType: 'String' | 1 | 20.0 | true | false | undefined;

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

@@ -4,10 +4,10 @@ import { SpecGenerator2 } from '@tsoa/cli/swagger/specGenerator2';
 import { Swagger } from '@tsoa/runtime';
 import { expect } from 'chai';
 import 'mocha';
+import * as os from 'os';
 import { versionMajorMinor } from 'typescript';
 import { getDefaultOptions } from '../../../fixtures/defaultOptions';
 import { TestModel } from '../../../fixtures/testModel';
-import * as os from 'os';
 
 describe('Definition generation', () => {
   const metadata = new MetadataGenerator('./fixtures/controllers/getController.ts').Generate();
@@ -2853,6 +2853,20 @@ describe('Definition generation', () => {
             const mappedTypedConditionalSchema = getValidatedDefinition('Partial_Conditional_string.string._a-number_.never__', currentSpec);
             expect(mappedTypedConditionalSchema).to.deep.eq(mappedConditionalSchema, `for property ${propertyName}.mappedTypedConditional`);
           },
+          nullableStringLiteral: (propertyName, propertySchema) => {
+            expect(propertySchema).to.not.haveOwnProperty('additionalProperties', `for property ${propertyName}`);
+            expect(propertySchema['x-nullable']).to.eq(true, `for property ${propertyName}[x-nullable]`);
+
+            expect(propertySchema).to.deep.eq({
+              type: 'string',
+              enum: ['NULLABLE_LIT_1', 'NULLABLE_LIT_2', null],
+              ['x-nullable']: true,
+              description: undefined,
+              example: undefined,
+              format: undefined,
+              default: undefined,
+            });
+          },
           typeOperators: (propertyName, propertySchema) => {
             expect(propertySchema?.properties?.keysOfAny?.$ref).to.eq('#/definitions/KeysMember', `for property ${propertyName}`);
             expect(propertySchema?.properties?.keysOfInterface?.$ref).to.eq('#/definitions/KeysMember_NestedTypeLiteral_', `for property ${propertyName}`);

tests/unit/swagger/schemaDetails3.spec.ts

@@ -4,10 +4,10 @@ import { SpecGenerator3 } from '@tsoa/cli/swagger/specGenerator3';
 import { Swagger, Tsoa } from '@tsoa/runtime';
 import { expect } from 'chai';
 import 'mocha';
+import * as os from 'os';
 import { versionMajorMinor } from 'typescript';
 import { getDefaultExtendedOptions } from '../../fixtures/defaultOptions';
 import { TestModel } from '../../fixtures/testModel';
-import * as os from 'os';
 
 describe('Definition generation for OpenAPI 3.0.0', () => {
   const metadataGet = new MetadataGenerator('./fixtures/controllers/getController.ts').Generate();
@@ -4425,6 +4425,20 @@ describe('Definition generation for OpenAPI 3.0.0', () => {
               `for property ${propertyName}.separateField3.omitted`,
             );
           },
+          nullableStringLiteral: (propertyName, propertySchema) => {
+            expect(propertySchema).to.not.haveOwnProperty('additionalProperties', `for property ${propertyName}`);
+            expect(propertySchema.nullable).to.eq(true, `for property ${propertyName}[x-nullable]`);
+
+            expect(propertySchema).to.deep.eq({
+              type: 'string',
+              enum: ['NULLABLE_LIT_1', 'NULLABLE_LIT_2', null],
+              nullable: true,
+              description: undefined,
+              example: undefined,
+              format: undefined,
+              default: undefined,
+            });
+          },
         };
 
         const testModel = currentSpec.spec.components.schemas[interfaceModelName];