OpenAPITools / openapi-generator

OpenAPI Generator allows generation of API client libraries (SDK generation), server stubs, documentation and configuration automatically given an OpenAPI Spec (v2, v3)
https://openapi-generator.tech
Apache License 2.0
20.83k stars 6.33k forks source link

[BUG] The generated javascript client library is not handling "oneOf" properly. #18125

Open davesargrad opened 4 months ago

davesargrad commented 4 months ago
Description

I've been using the openapi generation for years. I tend to generate python and javascript client libraries.

The problem I found occurs when you have a property that uses "oneOf", and the list of types includes "type: object".

I've patched the generated code, with success. The patch is quite simple. Hence I hope that this can be quickly resolved.

The generated javascript code does a check on type. It properly checks numbers, strings, arrays, booleans, however it fails to check on the object type and hence increments match. When I patched this the problem that I saw went away.

Bug image

Patch image

openapi-generator version

7.3.0

OpenAPI declaration file content or url

Just recently we added a tweak to the API spec as follows:

Spec:
      required:
        - name
        - value
      type: object
      properties:
        name:
          type: string
          example: zone
        value:
          oneOf:
            - type: string
            - type: number
            - type: integer
            - type: boolean
            - type: array
              items:
                type: object
            - type: object
          example: Critical
      xml:
        name: spec
Generation Details
npx ./node_modules/\@openapitools/openapi-generator-cli generate -i interface_spec.yaml  -g javascript -o test_lib_javascript
Suggest a fix
    try {
      if (!(typeof instance === "object")) {
        throw new Error(
          "Invalid value. Must be object. Input: " + JSON.stringify(instance)
        );
      }
      this.actualInstance = instance;
      match++;
    } catch (err) {
      // json data failed to deserialize into Object
      errorMessages.push("Failed to construct Object: " + err);
    }
wing328 commented 4 months ago

can you please file a PR to start with?

          oneOf:
            - type: string
            - type: number
            - type: integer
            - type: boolean
            - type: array
              items:
                type: object
            - type: object

looks like that's any type, right?

wing328 commented 4 months ago

I've been using the openapi generation for years. I tend to generate python and javascript client libraries.

happy to hear that you've been using it for years :)

davesargrad commented 4 months ago

can you please file a PR to start with?

          oneOf:
            - type: string
            - type: number
            - type: integer
            - type: boolean
            - type: array
              items:
                type: object
            - type: object

looks like that's any type, right?

Hi Yes. We want to support any type with this one field. I'm not sure what you mean by filing a PR. I think the "Bug" issue that I filed is the problem report. Please clarify

davesargrad commented 4 months ago

I've been using the openapi generation for years. I tend to generate python and javascript client libraries.

happy to hear that you've been using it for years :)

Indeed. It has served us quite well.

davesargrad commented 4 months ago

Though I am not familiar with the template language, at first glance the bug seems to be in this file

wing328 commented 4 months ago

yes, that's the mustache file you will need to update.

davesargrad commented 4 months ago

yes, that's the mustache file you will need to update.

Any idea what the fastest way to resolution is? This is the first such problem that I've ever had with the generator. Thats not so bad given a half decade of usage. I'm not comfortable with Mustache.. nor with the generator development pipeline. If I had some guidance, then perhaps I could introduce a fix. Otherwise, the bug report will go through the standard prioritization applied by the generator team.

Guidance?

wing328 commented 4 months ago

Would you like to sponsor the fix?

If I remember correctly, the oneOf implementation in JS client was also sponsored (and I was the one coming up with the implementation)

davesargrad commented 4 months ago

Would you like to sponsor the fix?

If I remember correctly, the oneOf implementation in JS client was also sponsored (and I was the one coming up with the implementation)

I'd sponsor it if I knew how to do it efficiently.. I dont have a debug env or anything like that in place. What is your estimate for how much time it would take for me to ramp up to a point where i had an env in place.. to test everything before i pushed a fix?

wing328 commented 3 months ago

i've merged a PR to simply any type represented by oneOf/anyOf: https://github.com/OpenAPITools/openapi-generator/pull/18268

can you give it a try with the latest master (snapshot version can be found in readme)?

drej1 commented 2 months ago

Hi @wing328,
I tried it via version 7.5 and it works, however I suggest that it should be modified slightly. The integer type should be optional for this functionality, because: https://swagger.io/docs/specification/data-models/data-types/

image

I mean replacing the anyOf/oneOf with the any type should work also for:

oneOf:
  - type: string
  - type: number
  - type: boolean
  - type: array
    items:
      type: object
  - type: object
drej1 commented 1 month ago

@wing328 please could you look into the above comment? Thanks.

drej1 commented 1 month ago

Hi @wing328 could you please look into the above comment (https://github.com/OpenAPITools/openapi-generator/issues/18125#issuecomment-2119879571)?

drej1 commented 3 weeks ago

Hi @wing328 a kind reminder here :)