Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
2 of 4 tasks
pauliusg opened this issue Feb 1, 2024 · 7 comments
Closed
2 of 4 tasks

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

pauliusg opened this issue Feb 1, 2024 · 7 comments

Comments

@pauliusg
Copy link

pauliusg commented Feb 1, 2024

Is there an existing issue for this?

  • I have searched the existing issues

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?

  • macOS
  • Windows
  • Linux

Other

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

@BrunnerLivio
Copy link
Member

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
Copy link
Member

This has now been released with 10.2.2 🎉

@pauliusg
Copy link
Author

pauliusg commented Feb 15, 2024

@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:

  • when only type: object is specified it generates [key: string]: object;
  • when type: object and additionalProperties: true is specified it generates [key: string]: any;

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 BrunnerLivio reopened this Feb 15, 2024
@BrunnerLivio
Copy link
Member

@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
Copy link
Author

pauliusg commented Feb 15, 2024

@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 pushed a commit to pauliusg/terminus that referenced this issue Feb 15, 2024
pauliusg pushed a commit to pauliusg/terminus that referenced this issue Feb 15, 2024
@pauliusg
Copy link
Author

@BrunnerLivio, good that I tried myself, required change #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 added a commit that referenced this issue Feb 18, 2024
#2516 Fix additional props in openapi health schema
@BrunnerLivio
Copy link
Member

BrunnerLivio commented Feb 18, 2024

Released with v10.2.3 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants