Repository: nestjs/swagger
Pull request #2870 (2024-03-09 16:17:14)

fix(swagger): Query Enums with isArray and enumName have type 'array' in schema
## PR Checklist
Please check if your PR fulfills the following requirements:

- [x] The commit message follows our guidelines: https://github.com/nestjs/nest/blob/master/CONTRIBUTING.md
- [x] Tests for the changes have been added (for bug fixes / features)
- [ ] Docs have been added / updated (for bug fixes / features)

## PR Type
What kind of change does this PR introduce?

<!-- Please check the one that applies to this PR using "x". -->
- [x] Bugfix
- [ ] Feature
- [ ] Code style update (formatting, local variables)
- [ ] Refactoring (no functional changes, no api changes)
- [ ] Build related changes
- [ ] CI related changes
- [ ] Other... Please describe:

## What is the current behavior?

I have a reproduction repository: https://github.com/arkraft/enum-array-repro

If a define a query like this:

```
enum LettersEnum {
  A = 'a',
  B = 'b',
  C = 'c',
}

@ApiQuery({
    name: 'letters',
    enum: LettersEnum,
    enumName: 'LettersEnum',
    isArray: true,
  })
  getHello(@Query('letters') letters: LettersEnum[]): string {
```

the final array should be of type string and the query param should be of type array. Instead, both of them are strings:

<img width="385" alt="image" src="https://github.com/nestjs/swagger/assets/7426376/cf06b9b9-a3aa-46b3-a12f-8e504d547e03">


```
"components": {
        "schemas": {
            "LettersEnum": {
                "type": "array",
                "enum": [
                    "a",
                    "b",
                    "c"
                ]
            }
        }
    }
```

## What is the new behavior?

With this PR, the extracted enum will have the correct enum type

```
"components": {
        "schemas": {
            "LettersEnum": {
                "type": "string",
                "enum": [
                    "a",
                    "b",
                    "c"
                ]
            }
        }
    }
```

## Does this PR introduce a breaking change?
- [ ] Yes
- [x] No

<!-- If this PR contains a breaking change, please describe the impact and migration path for existing applications below. -->


## Other information

I edited the enum tests to include the `@Query' and '@Param` in the request itself, because I believe, this is the correct use case for them. I would also revert the change, if it is not wanted



Patch:
diff --git a/lib/services/schema-object-factory.ts b/lib/services/schema-object-factory.ts
index 8c72bd98..a5bb2be7 100644
--- a/lib/services/schema-object-factory.ts
+++ b/lib/services/schema-object-factory.ts
@@ -167,14 +167,24 @@ export class SchemaObjectFactory {
 
       const schemaCombinators = ['oneOf', 'anyOf', 'allOf'];
       let keyOfCombinators = '';
-      if (schemaCombinators.some((_key) => { keyOfCombinators = _key; return _key in property; })) {
-          if (((property as SchemaObjectMetadata)?.type === 'array' || (property as SchemaObjectMetadata).isArray) && keyOfCombinators) {
-              (property as SchemaObjectMetadata).items = {};
-              (property as SchemaObjectMetadata).items[keyOfCombinators] = property[keyOfCombinators];
-              delete property[keyOfCombinators];
-          } else {
-              delete (property as SchemaObjectMetadata).type;
-          }
+      if (
+        schemaCombinators.some((_key) => {
+          keyOfCombinators = _key;
+          return _key in property;
+        })
+      ) {
+        if (
+          ((property as SchemaObjectMetadata)?.type === 'array' ||
+            (property as SchemaObjectMetadata).isArray) &&
+          keyOfCombinators
+        ) {
+          (property as SchemaObjectMetadata).items = {};
+          (property as SchemaObjectMetadata).items[keyOfCombinators] =
+            property[keyOfCombinators];
+          delete property[keyOfCombinators];
+        } else {
+          delete (property as SchemaObjectMetadata).type;
+        }
       }
       return property as ParameterObject;
     });
@@ -203,7 +213,8 @@ export class SchemaObjectFactory {
     if (!propertiesWithType) {
       return '';
     }
-    const extensionProperties = Reflect.getMetadata(DECORATORS.API_EXTENSION, type) || {};
+    const extensionProperties =
+      Reflect.getMetadata(DECORATORS.API_EXTENSION, type) || {};
     const typeDefinition: SchemaObject = {
       type: 'object',
       properties: mapValues(keyBy(propertiesWithType, 'name'), (property) =>
@@ -273,7 +284,10 @@ export class SchemaObjectFactory {
         : undefined;
 
       schemas[enumName] = {
-        type: param.schema?.['type'] ?? 'string',
+        type:
+          (param.isArray
+            ? param.schema?.['items']?.['type']
+            : param.schema?.['type']) ?? 'string',
         enum: _enum
       };
     }
@@ -301,15 +315,17 @@ export class SchemaObjectFactory {
     const $ref = getSchemaPath(enumName);
 
     // Allow given fields to be part of the referenced enum schema
-    const additionalParams = ['description', 'deprecated', 'default']
-    const additionalFields = additionalParams.reduce((acc, param) =>
-      ({...acc, ...(metadata[param] && { [param]: metadata[param] })}), {});
-
-    const enumType: string = (
-      metadata.isArray
-      ? metadata.items['type']
-      : metadata.type
-    ) ?? 'string';
+    const additionalParams = ['description', 'deprecated', 'default'];
+    const additionalFields = additionalParams.reduce(
+      (acc, param) => ({
+        ...acc,
+        ...(metadata[param] && { [param]: metadata[param] })
+      }),
+      {}
+    );
+
+    const enumType: string =
+      (metadata.isArray ? metadata.items['type'] : metadata.type) ?? 'string';
 
     schemas[enumName] = {
       type: enumType,


Test patch:
diff --git a/test/explorer/swagger-explorer.spec.ts b/test/explorer/swagger-explorer.spec.ts
index 2870f792..123e1b90 100644
--- a/test/explorer/swagger-explorer.spec.ts
+++ b/test/explorer/swagger-explorer.spec.ts
@@ -938,7 +938,7 @@ describe('SwaggerExplorer', () => {
       })
       @ApiQuery({ name: 'order', enum: QueryEnum })
       @ApiQuery({ name: 'page', enum: ['d', 'e', 'f'], isArray: true })
-      find(): Promise<Foo[]> {
+      find(@Param('objectId') objectId: ParamEnum): Promise<Foo[]> {
         return Promise.resolve([]);
       }
     }
@@ -952,7 +952,7 @@ describe('SwaggerExplorer', () => {
       })
       @ApiQuery({ name: 'order', enum: QueryEnum })
       @ApiQuery({ name: 'page', enum: ['d', 'e', 'f'] })
-      find(): Promise<Foo[]> {
+      find(@Param('objectId') objectId: ParamEnum, @Query('order') order: QueryEnum, @Query('page') page: 'd' | 'e' | 'f'): Promise<Foo[]> {
         return Promise.resolve([]);
       }
     }
@@ -972,7 +972,7 @@ describe('SwaggerExplorer', () => {
         enumName: 'QueryEnum',
         isArray: true
       })
-      findBar(): Promise<Foo> {
+      findBar(@Param('objectId') objectId: ParamEnum, @Query('order') order: QueryEnum, @Query('page') page: QueryEnum[]): Promise<Foo> {
         return Promise.resolve(null);
       }
     }
@@ -985,7 +985,7 @@ describe('SwaggerExplorer', () => {
         enum: [1, 2, 3],
         enumName: 'NumberEnum'
       })
-      findBar(): Promise<Foo> {
+      findBar(@Param('objectId') objectId: number): Promise<Foo> {
         return Promise.resolve(null);
       }
     }
@@ -1010,6 +1010,15 @@ describe('SwaggerExplorer', () => {
         '/globalPrefix/v3/modulePath/foos/{objectId}'
       );
       expect(routes[0].root!.parameters).toEqual([
+        {
+          in: 'path',
+          name: 'objectId',
+          required: true,
+          schema: {
+            type: 'string',
+            enum: ['a', 'b', 'c']
+          }
+        },
         {
           in: 'query',
           name: 'page',
@@ -1030,15 +1039,6 @@ describe('SwaggerExplorer', () => {
             type: 'number',
             enum: [1, 2, 3]
           }
-        },
-        {
-          in: 'path',
-          name: 'objectId',
-          required: true,
-          schema: {
-            type: 'string',
-            enum: ['a', 'b', 'c']
-          }
         }
       ]);
     });
@@ -1056,12 +1056,12 @@ describe('SwaggerExplorer', () => {
 
       expect(routes[0].root!.parameters).toEqual([
         {
-          in: 'query',
-          name: 'page',
+          in: 'path',
+          name: 'objectId',
           required: true,
           schema: {
             type: 'string',
-            enum: ['d', 'e', 'f']
+            enum: ['a', 'b', 'c']
           }
         },
         {
@@ -1074,12 +1074,12 @@ describe('SwaggerExplorer', () => {
           }
         },
         {
-          in: 'path',
-          name: 'objectId',
+          in: 'query',
+          name: 'page',
           required: true,
           schema: {
             type: 'string',
-            enum: ['a', 'b', 'c']
+            enum: ['d', 'e', 'f']
           }
         }
       ]);
@@ -1099,14 +1099,11 @@ describe('SwaggerExplorer', () => {
 
       expect(routes[0].root!.parameters).toEqual([
         {
-          in: 'query',
-          name: 'page',
+          in: 'path',
+          name: 'objectId',
           required: true,
           schema: {
-            type: 'array',
-            items: {
-              $ref: '#/components/schemas/QueryEnum'
-            }
+            $ref: '#/components/schemas/ParamEnum'
           }
         },
         {
@@ -1118,11 +1115,14 @@ describe('SwaggerExplorer', () => {
           }
         },
         {
-          in: 'path',
-          name: 'objectId',
+          in: 'query',
+          name: 'page',
           required: true,
           schema: {
-            $ref: '#/components/schemas/ParamEnum'
+            type: 'array',
+            items: {
+              $ref: '#/components/schemas/QueryEnum'
+            }
           }
         }
       ]);
diff --git a/test/services/schema-object-factory.spec.ts b/test/services/schema-object-factory.spec.ts
index 2a754673..3f4514bc 100644
--- a/test/services/schema-object-factory.spec.ts
+++ b/test/services/schema-object-factory.spec.ts
@@ -1,9 +1,13 @@
 import { ApiExtension, ApiProperty } from '../../lib/decorators';
-import { SchemasObject } from '../../lib/interfaces/open-api-spec.interface';
+import {
+  BaseParameterObject,
+  SchemasObject
+} from '../../lib/interfaces/open-api-spec.interface';
 import { ModelPropertiesAccessor } from '../../lib/services/model-properties-accessor';
 import { SchemaObjectFactory } from '../../lib/services/schema-object-factory';
 import { SwaggerTypesMapper } from '../../lib/services/swagger-types-mapper';
 import { CreateUserDto } from './fixtures/create-user.dto';
+import { ParamWithTypeMetadata } from '../../lib/services/parameter-metadata-accessor';
 
 describe('SchemaObjectFactory', () => {
   let modelPropertiesAccessor: ModelPropertiesAccessor;
@@ -328,4 +332,44 @@ describe('SchemaObjectFactory', () => {
       expect(schemas).toEqual({ MyEnum: { enum: [1, 2, 3], type: 'number' } });
     });
   });
+
+  describe('createEnumParam', () => {
+    it('should create an enum schema definition', () => {
+      const params: ParamWithTypeMetadata & BaseParameterObject = {
+        required: true,
+        isArray: false,
+        enumName: 'MyEnum',
+        enum: ['a', 'b', 'c']
+      };
+      const schemas = {};
+      schemaObjectFactory.createEnumParam(params, schemas);
+
+      expect(schemas['MyEnum']).toEqual({
+        enum: ['a', 'b', 'c'],
+        type: 'string'
+      });
+    });
+
+    it('should create an enum schema definition for an array', () => {
+      const params: ParamWithTypeMetadata & BaseParameterObject = {
+        required: true,
+        isArray: true,
+        enumName: 'MyEnum',
+        schema: {
+          type: 'array',
+          items: {
+            type: 'string',
+            enum: ['a', 'b', 'c']
+          }
+        }
+      };
+      const schemas = {};
+      schemaObjectFactory.createEnumParam(params, schemas);
+
+      expect(schemas['MyEnum']).toEqual({
+        enum: ['a', 'b', 'c'],
+        type: 'string'
+      });
+    });
+  });
 });