Skip to content
New issue

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

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

Already on GitHub? # to your account

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

Open
davesargrad opened this issue Mar 16, 2024 · 14 comments

Comments

@davesargrad
Copy link

davesargrad commented Mar 16, 2024

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

wing328 commented Mar 16, 2024

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

wing328 commented Mar 16, 2024

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

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

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

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

@wing328
Copy link
Member

wing328 commented Mar 17, 2024

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

@davesargrad
Copy link
Author

davesargrad commented Mar 18, 2024

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

wing328 commented Mar 18, 2024

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

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

wing328 commented Apr 3, 2024

i've merged a PR to simply any type represented by oneOf/anyOf: #18268

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

@drej1
Copy link

drej1 commented May 20, 2024

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

drej1 commented May 29, 2024

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

@drej1
Copy link

drej1 commented Jun 3, 2024

Hi @wing328 could you please look into the above comment (#18125 (comment))?

@drej1
Copy link

drej1 commented Jul 3, 2024

Hi @wing328 a kind reminder here :)

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

No branches or pull requests

3 participants