-
Notifications
You must be signed in to change notification settings - Fork 219
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
Add support for chrono::NaiveTime #641
Conversation
Since the OpenAPI spec does not explicitly support times without attached dates, I have implemented this in the same way that support for `duration` was done, but formatting them as strings.
I am getting some errors when I try to run the tests, but I'm not sure if it's because I have the wrong feature flags set up or because I'm on an M1 mac and that's doing funky things. Errors
The
Do you have GitHub actions setup to run the tests? Or alternatively, could you point me in the right direction for how I could resolve this issue? Thanks for the great crate! |
Yes there is Github Actions, also you can use the shell script |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, looks good 🎉 Thanks for the patch and great that you like the crate.
Okay thanks, it's still failing I think because I haven't implemented Any chance you know what needs to be added? |
After short debuggin the issue in You can also test this behavior by runnig this test with different flags:
|
utoipa-gen/src/schema_type.rs
Outdated
known_format = matches!(name, "DateTime" | "Date" | "NaiveDate" | "NaiveDateTime"); | ||
known_format = matches!( | ||
name, | ||
"DateTime" | "Date" | "NaiveDate" | "NaiveTime" | "NaiveDateTime" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now I know what is going on here. The issue is that you have defined NaiveTime
as a known format, but it does not have a format, but only a type which should be string. Now with that known format the naive_time will generate the spec as:
.property(
"naive_time",
utoipa::openapi::ObjectBuilder::new()
.schema_type(utoipa::openapi::SchemaType::String)
.format(Some()), // <-- note missing format!
)
Note the missing format, it renders empty format because you haven't defined what should be the format of the NaiveTime
.
utoipa/utoipa-gen/src/schema_type.rs
Lines 295 to 340 in f8c6d07
impl ToTokens for Type<'_> { | |
fn to_tokens(&self, tokens: &mut proc_macro2::TokenStream) { | |
let last_segment = self.0.segments.last().unwrap_or_else(|| { | |
abort_call_site!("expected there to be at least one segment in the path") | |
}); | |
let name = &*last_segment.ident.to_string(); | |
match name { | |
#[cfg(feature="non_strict_integers")] | |
"i8" => tokens.extend(quote! { utoipa::openapi::SchemaFormat::KnownFormat(utoipa::openapi::KnownFormat::Int8) }), | |
#[cfg(feature="non_strict_integers")] | |
"u8" => tokens.extend(quote! { utoipa::openapi::SchemaFormat::KnownFormat(utoipa::openapi::KnownFormat::UInt8) }), | |
#[cfg(feature="non_strict_integers")] | |
"i16" => tokens.extend(quote! { utoipa::openapi::SchemaFormat::KnownFormat(utoipa::openapi::KnownFormat::Int16) }), | |
#[cfg(feature="non_strict_integers")] | |
"u16" => tokens.extend(quote! { utoipa::openapi::SchemaFormat::KnownFormat(utoipa::openapi::KnownFormat::UInt16) }), | |
#[cfg(feature="non_strict_integers")] | |
#[cfg(feature="non_strict_integers")] | |
"u32" => tokens.extend(quote! { utoipa::openapi::SchemaFormat::KnownFormat(utoipa::openapi::KnownFormat::UInt32) }), | |
#[cfg(feature="non_strict_integers")] | |
"u64" => tokens.extend(quote! { utoipa::openapi::SchemaFormat::KnownFormat(utoipa::openapi::KnownFormat::UInt64) }), | |
#[cfg(not(feature="non_strict_integers"))] | |
"i8" | "i16" | "u8" | "u16" | "u32" => { | |
tokens.extend(quote! { utoipa::openapi::SchemaFormat::KnownFormat(utoipa::openapi::KnownFormat::Int32) }) | |
} | |
#[cfg(not(feature="non_strict_integers"))] | |
"u64" => tokens.extend(quote! { utoipa::openapi::SchemaFormat::KnownFormat(utoipa::openapi::KnownFormat::Int64) }), | |
"i32" => tokens.extend(quote! { utoipa::openapi::SchemaFormat::KnownFormat(utoipa::openapi::KnownFormat::Int32) }), | |
"i64" => tokens.extend(quote! { utoipa::openapi::SchemaFormat::KnownFormat(utoipa::openapi::KnownFormat::Int64) }), | |
"f32" => tokens.extend(quote! { utoipa::openapi::SchemaFormat::KnownFormat(utoipa::openapi::KnownFormat::Float) }), | |
"f64" => tokens.extend(quote! { utoipa::openapi::SchemaFormat::KnownFormat(utoipa::openapi::KnownFormat::Double) }), | |
#[cfg(feature = "chrono")] | |
"NaiveDate" => tokens.extend(quote! { utoipa::openapi::SchemaFormat::KnownFormat(utoipa::openapi::KnownFormat::Date) }), | |
#[cfg(feature = "chrono")] | |
"DateTime" => tokens.extend(quote! { utoipa::openapi::SchemaFormat::KnownFormat(utoipa::openapi::KnownFormat::DateTime) }), | |
#[cfg(feature = "chrono")] | |
"NaiveDateTime" => tokens.extend(quote! { utoipa::openapi::SchemaFormat::KnownFormat(utoipa::openapi::KnownFormat::DateTime) }), | |
#[cfg(any(feature = "chrono", feature = "time"))] | |
"Date" => tokens.extend(quote! { utoipa::openapi::SchemaFormat::KnownFormat(utoipa::openapi::KnownFormat::Date) }), | |
#[cfg(feature = "uuid")] | |
"Uuid" => tokens.extend(quote! { utoipa::openapi::SchemaFormat::KnownFormat(utoipa::openapi::KnownFormat::Uuid) }), | |
#[cfg(feature = "time")] | |
"PrimitiveDateTime" | "OffsetDateTime" => { | |
tokens.extend(quote! { utoipa::openapi::SchemaFormat::KnownFormat(utoipa::openapi::KnownFormat::DateTime) }) | |
} | |
_ => (), |
Though in this case I believe the fix would be to remove the NaiveTime
from known format function since there is no format specifier for the time like the duration does not have.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And then the format in the test should be null
as in below:
"properties.naive_time.format" = r#"null"#, "Post time format"
Remove `NaiveTime` from `known_format` since it does not have a additional format specifier and fix the chrono test. Additionally fix uuid dependency when running tests and update dev dependencies.
Thanks so much! |
Since the OpenAPI spec does not explicitly support times without attached dates, I have implemented this in the same way that support for
duration
was done, but formatting them as strings.