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

poem-openapi: Replace naive-date with date #753

Closed
wants to merge 1 commit into from

Conversation

sawi97
Copy link

@sawi97 sawi97 commented Feb 19, 2024

When a date type was specified in an openapi Object, it was previously set to the format naive-date, which isn't recognized by doc-generators.

This replaces the openapi text format naive-date with date, aligning with the OAS definition which uses RFC3339 full-date (which is naive by def).

It enables doc-generators (e.g. rapidoc) to generate examples when the schema contains a time::Date or chrono::NaiveDate field.

@attila-lin
Copy link
Collaborator

attila-lin commented Feb 20, 2024

how about date-time? should we change together. ( I'm not familiar with openapi

@sawi97
Copy link
Author

sawi97 commented Feb 20, 2024

I believe openapi uses the same date/time definitions as json schema.

A naive date-time does not exist (yet). And I do not think it would be a good idea to set it to date-time, which is specified to contain timezone. Poem would not parse a date-time with time zone as a naive date-time, so the doc-generated examples would fail. And if there are any external input/output validators, they will most probably cause the request to fail.
The same goes for time (even tough time is not in the OAS specification, but rapidoc uses it).

So in my opinion the other date/time formats are fine, at least until OAS/JSON Schema has a specification for naive formats. :)

@attila-lin
Copy link
Collaborator

Thanks for your reply!

@sunli829
Copy link
Collaborator

If you expect a datetime in RFC3339 format, should use OffsetDateTime instead of PrimitiveDateTime.

@sunli829 sunli829 closed this Mar 24, 2024
@sawi97
Copy link
Author

sawi97 commented Mar 24, 2024

@sunli829 this PR is about date, not datetime.
All date objects are naive in OAS and that is what this PR fixes.

Can you reopen and consider merging?

@GunnarMorrigan
Copy link

GunnarMorrigan commented Sep 4, 2024

@sunli829, @sawi97 is right.

Currently poem openapi translates a date object to string($naive-date) which is nothing in terms of Swagger.
Only a date-time and date object exist.

A time::Date, chrono::NaiveDate should be translated to string($date) for it to be inline with the swagger specifications.

Please consider this PR again as the current implementation is not correct.

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.

4 participants