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

[aspnetcore] Add TypeConverter for enum string conversion #3557

Merged
merged 2 commits into from
Aug 11, 2019

Conversation

yilinjuang
Copy link
Contributor

@yilinjuang yilinjuang commented Aug 5, 2019

PR checklist

  • Read the contribution guidelines.
  • Ran the shell script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run ./bin/{LANG}-petstore.sh, ./bin/openapi3/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in .\bin\windows\. If contributing template-only or documentation-only changes which will change sample output, be sure to build the project first.
  • Filed the PR against the correct branch: master, 4.1.x, 5.0.x. Default: master.
  • Copied the technical committee to review the pull request if your PR is targeting a particular programming language.

Description of the PR

If enum is used in query string, it can't be parsed with EnumMember. Add TypeConverter to utilize json.net. #3513

@wing328
@jimschubert
@mandrean

@auto-labeler
Copy link

auto-labeler bot commented Aug 5, 2019

👍 Thanks for opening this issue!
🏷 I have applied any labels matching special text in your issue.

The team will review the labels and make any necessary changes.

@jimschubert
Copy link
Member

Thanks for the PR. I think we'll need to also consider using a TypeConverter to fix this issue, because this rename will:

  1. Be a breaking change for those who regenerate (with the library option)
  2. The enum suffix exists because of the order of suboptimal course out there. This rename will cause issue, for instance, with code written as Pet.PetEnum.

@yilinjuang yilinjuang changed the title [C#] Remove enum name suffix [aspnetcore] Add TypeConverter for enum string conversion Aug 6, 2019
@yilinjuang
Copy link
Contributor Author

@jimschubert changed to TypeConverter. Thanks!

Copy link
Member

@jimschubert jimschubert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regenerated samples appear to be changes pulled in from master.

@jimschubert
Copy link
Member

Tested with the steps in the issue linked in the description and it appear to work as expected now. Valid enum returns a 200 (after modifying the generated controller to return 200 with empty body). Sending a request for an invalid enum value result in a 400 with body:

{
  "errors": {
    "param": [
      "The input was not valid."
    ]
  },
  "title": "One or more validation errors occurred.",
  "status": 400,
  "traceId": "0HLOU0ROUFHK9:00000003"
}

Looks good! Thanks for fixing this issue!

@jimschubert jimschubert merged commit 22d022b into OpenAPITools:master Aug 11, 2019
@jimschubert jimschubert added this to the 4.1.1 milestone Aug 11, 2019
@yilinjuang yilinjuang deleted the fix-enum branch August 12, 2019 09:28
@jimschubert
Copy link
Member

@frankyjuang Thanks again for the bug fix! Just wanted to let you know this is part of the recent 4.1.1 release.

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

Successfully merging this pull request may close these issues.

2 participants