nestjs / terminus

Terminus module for Nest framework (node.js) :robot:
https://nestjs.com/
MIT License
678 stars 99 forks source link

Schema does not match health-indicator-result interface #2516

Closed pauliusg closed 9 months ago

pauliusg commented 9 months ago

Is there an existing issue for this?

Current behavior

Schema returns additionalProperties of type: 'string': https://github.com/nestjs/terminus/blob/master/lib/health-check/health-check.schema.ts#L26

const healthIndicatorSchema = (example: HealthIndicatorResult) => ({
  type: 'object',
  example,
  additionalProperties: {
    type: 'object',
    properties: {
      status: {
        type: 'string',
      },
    },
    additionalProperties: {
      type: 'string',
    },
  },
});

When interface has [optionalKeys: string]: any;:

export type HealthIndicatorResult = {
  /**
   * The key of the health indicator which should be unique
   */
  [key: string]: {
    /**
     * The status if the given health indicator was successful or not
     */
    status: HealthIndicatorStatus;
    /**
     * Optional settings of the health indicator result
     */
    [optionalKeys: string]: any;
  };
};

https://github.com/nestjs/terminus/blob/master/lib/health-indicator/health-indicator-result.interface.ts#L22

Minimum reproduction code

Swagger: image

Steps to reproduce

No response

Expected behavior

additionalProperties should be of type: 'any':

Package version

9.2.2

NestJS version

9.4.2

Node.js version

18.19.0

In which operating systems have you tested?

Other

Would be nice to have a fix not only in the latest version, but also in 9.X.X!

BrunnerLivio commented 9 months ago

Makes sense! I set it to the object type simply because that is the same behaviour for @nestjs/swagger when an any type is used

BrunnerLivio commented 9 months ago

This has now been released with 10.2.2 šŸŽ‰

pauliusg commented 9 months ago

@BrunnerLivio, sadly but this does not solve my issue. I am using OpenAPI Generator to generate typescript-axios client. Additionaly I have turned on strict typescript mode in tsconfig.json:

{
  "compilerOptions": {
    "strict": true,
    ...
  }
}

This way typescript check fails for generated file: image

So, with 10.2.2 I am getting [key: string]: object;. But typescript check does not fail only with [key: string]: any;

I have similar case in my own data model where generated code has [key: string]: any;. I am using decorator with such properties:

export class WorkerTaskSuspendedOutputDto {
  @ApiProperty({ type: String, description: 'Message will be shown in link "View alert" modal.' })
  message: string;

  /**
   * This data will be merged with task input in case user wants to continue a task.
   */
  @ApiProperty({
    type: Object,
    description: 'This data will be merged with task input in case user wants to continue a task.',
    additionalProperties: true,
  })
  jobContinueData: { [key: string]: any };
}

and this is translated to openAPI spec:

...
         "WorkerTaskSuspendedOutputDto":{
            "type":"object",
            "properties":{
               "message":{
                  "type":"string",
                  "description":"Message will be shown in link \"View alert\" modal."
               },
               "jobContinueData":{
                  "type":"object",
                  "description":"This data will be merged with task input in case user wants to continue a task.",
                  "additionalProperties":true
               }
            },
            "required":[
               "message",
               "jobContinueData"
            ]
         },
...

Looks like the missing part here is additionalProperties: true.

OpenAPI specification https://swagger.io/docs/specification/data-models/data-types/ regarding Free-Form Object says: A free-form object (arbitrary property/value pairs) is defined as: type: object

This is equivalent to type: object additionalProperties: true

and type: object additionalProperties: {}

So, your change was right, however OpenAPI Generator (typescript-axios) behaves differently:

I would be very grateful, if you could add additionalProperties: true. It would not break OpenAPI specification, but it would fix issue for me, because now every time I have to fix my file manually after generation of new client.

BrunnerLivio commented 9 months ago

@pauliusg I see. Would it be possible you create a PR yourself so you can verify right away whether it will work or not with your setup? :)

You can just fork & clone this repo, change the needed lines, run npm run build and then npm link. Then go to your project and run npm link @nestjs/terminus and if everything goes right you should then be able to temporarily use your local version of NestJS terminus and see whether your changes fixed this issue.

pauliusg commented 9 months ago

@BrunnerLivio, agreed. I will try. I am using npx link (a least for me this package is way easier to use than built-in npm link feature).

pauliusg commented 9 months ago

@BrunnerLivio, good that I tried myself, required change https://github.com/nestjs/terminus/pull/2523 was a bit different. After my changes: generated spec part:

...
            "responses":{
               "200":{
                  "description":"The Health Check is successful",
                  "content":{
                     "application/json":{
                        "schema":{
                           "type":"object",
                           "properties":{
                              "status":{
                                 "type":"string",
                                 "example":"ok"
                              },
                              "info":{
                                 "type":"object",
                                 "example":{
                                    "database":{
                                       "status":"up"
                                    }
                                 },
                                 "additionalProperties":{
                                    "type":"object",
                                    "required":[
                                       "status"
                                    ],
                                    "properties":{
                                       "status":{
                                          "type":"string"
                                       }
                                    },
                                    "additionalProperties":true
                                 },
                                 "nullable":true
                              },
                              "error":{
                                 "type":"object",
                                 "example":{

                                 },
                                 "additionalProperties":{
                                    "type":"object",
                                    "required":[
                                       "status"
                                    ],
                                    "properties":{
                                       "status":{
                                          "type":"string"
                                       }
                                    },
                                    "additionalProperties":true
                                 },
                                 "nullable":true
                              },
                              "details":{
                                 "type":"object",
                                 "example":{
                                    "database":{
                                       "status":"up"
                                    }
                                 },
                                 "additionalProperties":{
                                    "type":"object",
                                    "required":[
                                       "status"
                                    ],
                                    "properties":{
                                       "status":{
                                          "type":"string"
                                       }
                                    },
                                    "additionalProperties":true
                                 }
                              }
                           }
                        }
                     }
                  }
               },
...

Generated OpenAPI Generator file is without issues:

/* tslint:disable */
/* eslint-disable */
/**
 * Core API
 * No description provided (generated by Openapi Generator https://github.com/openapitools/openapi-generator)
 *
 * The version of the OpenAPI document: 1.16.0
 * 
 *
 * NOTE: This class is auto generated by OpenAPI Generator (https://openapi-generator.tech).
 * https://openapi-generator.tech
 * Do not edit the class manually.
 */

/**
 * 
 * @export
 * @interface HealthControllerHealthCheck200ResponseInfoValue
 */
export interface HealthControllerHealthCheck200ResponseInfoValue {
    [key: string]: any;

    /**
     * 
     * @type {string}
     * @memberof HealthControllerHealthCheck200ResponseInfoValue
     */
    'status': string;
}
BrunnerLivio commented 9 months ago

Released with v10.2.3 šŸŽ‰