@ApiQuery, Default schema attribute should not be generated when content is set

[edvinv]

I'm submitting a...

[ ] Regression 
[x] Bug report
[ ] Feature request
[ ] Documentation issue or request
[ ] Support request => Please do not submit support request here, instead post your question on Stack Overflow.

Current behavior

I want to define query JSON serialized parameter using content as:

@Get()
@ApiOkResponse({ type: UserResponseDto, description: 'Get users.' })
@ApiOperation({ summary: 'Search users.' })
@ApiQuery({
  name: 'filter',
  required: false,
  content: {
    'application/json': {
      schema: {
        $ref: getSchemaPath(UserSearchFilterDto),
      },
    },
  },
})
searchUsers(@Query('filter') filter: string): UserResponseDto {
  const user = new UserResponseDto();
  return user;
}

The generated open api document looks like:

/api/users:
get:
  operationId: searchUsers
  summary: Search users.
  parameters:
    - name: filter
      required: false
      in: query
      content:
        application/json:
          schema:
            $ref: '#/components/schemas/UserSearchFilterDto'
      schema:
        type: string

As you can see, there is added schema with type string.

Expected behavior

Following document would be expected:

/api/users:
get:
  operationId: searchUsers
  summary: Search users.
  parameters:
    - name: filter
      required: false
      in: query
      content:
        application/json:
          schema:
            $ref: '#/components/schemas/UserSearchFilterDto'

Because of this client generators are creating wrong code.

So, is there a way to prevent generating schema attribute, because it looks like that it is generated as type string when not defined.

Environment

[System Information] OS Version : Linux 4.15 NodeJS Version : v14.17.2 NPM Version : 6.14.13

[Nest CLI] Nest CLI Version : 7.6.0

[Nest Platform Information] platform-express version : 7.6.18 passport version : 7.1.6 swagger version : 4.8.1 common version : 7.6.18 config version : 0.6.3 core version : 7.6.18 jwt version : 7.2.0

[kamilmysliwiec]

Would you like to create a PR for this issue?

[edvinv]

First , how should this work not to be a breaking change?

1. if schema is defined use schema
2. if type is defined use type
3. if content is defined use content
4. if none is defined, then set to schema.type='string' as it is now

Regarding PR, currently I have no knowledge abot this project at source level, but I can look when I'm back from vacation.

[kamilmysliwiec]

^ sounds good to me

[MatteoMaso]

any update here? I'm also facing this issue

[KrisKnez]

Using the 'content' option of the ApiQuery decorator results in a schema with a structural error.

As per OpenAPI v3 specification, each parameter must have either schema or content property defined, but not both.

[KrisKnez]

I came up with a temporary workaround for this issue, a hack.

Basically, it is possible to edit the document generated from the SwaggerModule before we serve it to the endpoint.

By doing this, we can check if any of the query operations have both a content and a schema field and if they have, we delete the schema field.

async function bootstrap() {
  const app = await NestFactory.create(AppModule);

  const options = new DocumentBuilder().setTitle('API').setDescription('API description').setVersion('1.0').build();
  let document = SwaggerModule.createDocument(app, options);
  
  // Modify the document to fix the @ApiQuery issue
  for (const path of Object.values(document.paths)) {
    for (const operation of Object.values(path)) {
      if (operation.parameters) {
        for (const parameter of operation.parameters) {
          if (parameter.content) {
            delete parameter.schema;
          }
        }
      }
    }
  }

  SwaggerModule.setup('api', app, document);

  await app.listen(3000);
}

Also, from my experience having the endpoint method argument type makes the factory ignore the content field but you can fix this by setting the ApiQuery option type to an empty string

  @ApiQuery({
    name: 'filter',
    required: false,
    type: '',
    content: {
      'application/json': {
        schema: {
          $ref: getSchemaPath(FilterNoteDto),
        },
      },
    },
  })

[KrisKnez]

First , how should this work not to be a breaking change?

1. if schema is defined use schema
2. if type is defined use type
3. if content is defined use content
4. if none is defined, then set to schema.type='string' as it is now

Regarding PR, currently I have no knowledge abot this project at source level, but I can look when I'm back from vacation.

If there is no type, the generated schema field is an empty object (valid according to specification) and the Swagger UI renders a string field.

Default behavior is for Nest.js to inherit the name and type of the method parameter (query param) for the generated schema accordingly. (If it's a DTO it will be 'object', if it's a number it will be 'number', e.g.)

There are fields inside ApiQuery options that allow you to overwrite this default behavior:

interface ApiQueryMetadata extends ParameterOptions {
  name?: string;
  type?: Type<unknown> | Function | [Function] | string;
  isArray?: boolean;
  enum?: SwaggerEnumType;
  enumName?: string;
}

My proposal is that ApiQuery decorator has three possible types of options

type ParameterOptions = Omit<ParameterObject, 'in' | 'schema' | 'name' | 'content'>;

interface ApiQueryMetadata extends ParameterOptions {
  name?: string;
  type?: Type<unknown> | Function | [Function] | string;
  isArray?: boolean;
  enum?: SwaggerEnumType;
  enumName?: string;
}

interface ApiQuerySchemaHost extends ParameterOptions {
  name?: string;
  schema: SchemaObject | ReferenceObject;
}

// This is a new interface, content was previously part of ParameterOptions
interface ApiQueryContentHost extends ParameterOptions {
  name?: string;
  content: ContentObject;

  // We don't need type, isArray, and enum because it can be
  // defined separately inside SchemaObject or ContentObject,
}

With this you have three options, either you let Nest.JS generate based on method parameter and overwrite some options (if you need to) or you define it as scheme or as content.

If this seems alright I can make a PR for this.