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

[BUG][aspnetcore] optional query parameters with enum types not nullable #17518

Closed
5 of 6 tasks
fujieda opened this issue Jan 3, 2024 · 10 comments · Fixed by #18137
Closed
5 of 6 tasks

[BUG][aspnetcore] optional query parameters with enum types not nullable #17518

fujieda opened this issue Jan 3, 2024 · 10 comments · Fixed by #18137

Comments

@fujieda
Copy link
Contributor

fujieda commented Jan 3, 2024

Bug Report Checklist

  • Have you provided a full/minimal spec to reproduce the issue?
  • Have you validated the input using an OpenAPI validator (example)?
  • Have you tested with the latest master to confirm the issue still exists?
  • Have you searched for related issues/PRs?
  • What's the actual output vs expected output?
  • [Optional] Sponsorship to speed up the bug fix or feature request (example)
Description

The latest aspnetcore generator doesn't set nullable for an optional query parameter in the following definition.

openapi: 3.0.0
info:
  title: optional query test
  version: 1.0.0
paths:
  /test:
    get:
      summary: Test API
      parameters:
        - in: query
          name: testQuery
          schema:
            $ref: '#/components/schemas/TestEnum'
          required: false
      responses:
        '200':
          description: OK

components:

  schemas:
    TestEnum:
      type: string
      enum:
        - A
        - B

Expected output:

public virtual IActionResult TestGet([FromQuery (Name = "testQuery")]TestEnum? testQuery)

Actual output:

public virtual IActionResult TestGet([FromQuery (Name = "testQuery")]TestEnum testQuery)
openapi-generator version

From the commit 6299af1 to 7.2.0 and the latest master

Steps to reproduce

Just run the following command. `simple.yml' contains the above declarations.

 java -jar openapi-generator-cli-7.2.0.jar generate -g aspnetcore -i simple.yaml
Suggest a fix

This issue can be fixed by reverting the commit 6299af1.

@kipusoep
Copy link

We ran into this issue as well, pretty annoying as we have to revert all the removed question marks manually every time we generate a new client.

@wing328
Copy link
Member

wing328 commented Feb 21, 2024

cc @devhl-labs who's the author of the commit 6299af1

@devhl-labs
Copy link
Contributor

devhl-labs commented Feb 25, 2024

I don't think that commit is the problem. In the DefaultCodegen, property.isModel should be returning true. You can see both of my debugging variables b and c are false. If I set isModel to true here, then it works.

image

@troutkilroy
Copy link

I'm observing same issue with csharp generator in any 7.X

@fujieda
Copy link
Contributor Author

fujieda commented Mar 17, 2024

I don't think that commit is the problem. In the DefaultCodegen, property.isModel should be returning true. You can see both of my debugging variables b and c are false. If I set isModel to true here, then it works.

The commit introduced a fatal incompatibility in the generated code. Please check out commit 598c27d, the parent of the commit. Build it, and then generate the code for the above YAML using the aspnetcore generator. You will see the code we expect.

@devhl-labs
Copy link
Contributor

The commit is correct, but it highlights another bug yet to be addressed. I explained exactly what the bug is here

@wing328
Copy link
Member

wing328 commented Mar 17, 2024

In the DefaultCodegen, property.isModel should be returning true

If I remember correctly, isModel means its properties is non empty.

What about using isEnumRef instead?

@devhl-labs
Copy link
Contributor

Should I also use isEnum?

@wing328
Copy link
Member

wing328 commented Mar 18, 2024

isEnum is true when the schema is an inline enum.

isEnumOrRef is true when both isEnum and isEnumRef are true.

using which one really on the use cases.

@wing328
Copy link
Member

wing328 commented Mar 18, 2024

thanks for the fix by @devhl-labs which has been merged.

please pull the latest master to give it a try

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

Successfully merging a pull request may close this issue.

5 participants