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

fix: correct OpenAPI schema for enum constraints (#2812) #2818

Merged
merged 2 commits into from Dec 2, 2023

Conversation

hzhou0
Copy link
Contributor

@hzhou0 hzhou0 commented Dec 2, 2023

Pull Request Checklist

  • New code has 100% test coverage
  • (If applicable) The prose documentation has been updated to reflect the changes introduced by this PR
  • (If applicable) The reference documentation has been updated to reflect the changes introduced by this PR
  • Pre-Commit Checks were ran and passed
  • Tests were ran and passed

Description

  • Fixes OpenAPI schema generation when enum constraints are present, such as in python Enums or Literal types.
    • The JSONSchema "type" constraint is now a list of all types present in the enum constraints, which is the specified schema expectation.
  • Fixes tests to expect corrected behaviour.

Close Issue(s)

@hzhou0 hzhou0 requested review from a team as code owners December 2, 2023 01:09
@peterschutt
Copy link
Contributor

Thanks for this! I'm out of time to review it for now, but will get to it ASAP, or someone else will. Are those type check failures issues with the way we have the properties typed on the spec models?

@peterschutt
Copy link
Contributor

@all-contributors add @hzhou0 for bug and code

Copy link
Contributor

@peterschutt

I've put up a pull request to add @hzhou0! 🎉

@hzhou0
Copy link
Contributor Author

hzhou0 commented Dec 2, 2023

Thanks for this! I'm out of time to review it for now, but will get to it ASAP, or someone else will. Are those type check failures issues with the way we have the properties typed on the spec models?

I fixed most of it, the problem is TYPE_MAP in schema.py. It has Schema values, which have type: OpenAPIType | Sequence[OpenAPIType] | None, however in that map it could be narrowed to type: OpenAPIType. I tried to typeguard, but not sure how to do that against Sequence types.

This PR fixes OpenAPI schema generation when enum constraints are present, such as in python Enums or Literal types. The JSONSchema "type" constraint is now a list of all types present in the enum constraints, which is the specified schema expectation. Fixes tests to expect this behaviour.
Copy link
Member

@cofin cofin left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks @hzhou0

@cofin cofin enabled auto-merge (squash) December 2, 2023 18:17
@cofin cofin disabled auto-merge December 2, 2023 18:17
@cofin cofin merged commit a281ce7 into litestar-org:main Dec 2, 2023
12 checks passed
Copy link

github-actions bot commented Dec 2, 2023

Documentation preview will be available shortly at https://litestar-org.github.io/litestar-docs-preview/2818

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

Successfully merging this pull request may close these issues.

Bug: Incorrect OpenAPI Generated For Optional Literal Types
3 participants