time-rs / time

The most used Rust library for date and time handling.
https://time-rs.github.io
Apache License 2.0
1.09k stars 273 forks source link

`time::serde::format_description!` fails on `null` when struct is part of an enum #562

Closed Kazy closed 1 year ago

Kazy commented 1 year ago

Hello !

I've encountered a bug when trying to deserialize to an Option<PrimitiveDateTime> using the time::serde::format_description! macro. When the struct containing the optional datetime is part of an enum, it fails when receiving a null value. When deserializing to the inner structure directly or when the value isn't null everything works as expected.

Here is a reproduction of the issue. This has been reproduced on 0.3.9, 0.3.20, and the current Git master (c644bd2847c0a111c595b6d9a2935f990a267525)

use serde::*;
use time::PrimitiveDateTime;

time::serde::format_description!(
    my_time,
    PrimitiveDateTime,
    "[day] [month repr:short] [year] [hour]:[minute]:[second] GMT"
);

#[derive(Deserialize, Debug)]
struct Inner {
    #[serde(with = "my_time::option")]
    date: Option<PrimitiveDateTime>,
}

#[derive(Deserialize, Debug)]
#[serde(tag = "type")]
enum Outer {
    #[serde(rename = "inner")]
    Inner(Inner),
}

fn main() {
    let value: Result<Inner, _> = serde_json::from_str(
        r#"
        {"date": null}
    "#,
    );
    println!("{:#?}", value);

    // Issue is with this one. It should give an Ok with a None value for the date but it doesn't.
    let value: Result<Outer, _> = serde_json::from_str(
        r#"
        {"type": "inner", "date": null}
    "#,
    );
    println!("{:#?}", value);

    let value: Result<Outer, _> = serde_json::from_str(
        r#"
        {"type": "inner", "date": "23 Mar 2023 08:40:50 GMT"}
    "#,
    );
    println!("{:#?}", value);
}

Result:

Ok(
    Inner {
        date: None,
    },
)
Err(
    Error("invalid type: null, expected an `Option<PrimitiveDateTime>` in the format \"[day] [month repr:short] [year] [hour]:[minute]:[second] GMT\"", line: 0, column: 0),
)
Ok(
    Inner(
        Inner {
            date: Some(
                2023-03-23 8:40:50.0,
            ),
        },
    ),
)

Thank you for your help !

jhpratt commented 1 year ago

This behavior is documented in the docs for time::serde::format_description! here:

The returned Option will contain a deserialized value if present and None if the field is present but the value is null (or the equivalent in other formats). To return None when the field is not present, you should use #[serde(default)] on the field.

Kazy commented 1 year ago

@jhpratt This isn't the issue here, unless I'm deeply misunderstanding something. If you look at the second example, it returns an error in presence of a null value, even though in the first example the same struct but not this time not part of an enum correctly deserializes to None.

I expect the second example to be Ok(Inner(Inner { date: None } )), just like the third one but with a None since the value of date is null.

huuff commented 5 months ago

@Kazy did you find a workaround? We're currently blocked at the same issue. Adding #[serde(default)] didn't help.

dbaynard commented 4 months ago

I think this should be re-opened (though if you re-open it now, it won't incentivize me to produce a minimal repro…)

I've hit this: specifically, at the top (nesting) level of the json object, null values are deserialized to None. At levels below, the parser fails. I implemented the code that the macro creates, and made further modifications. I was able to operate on the top and the next level, but not the one below that. I have not taken the time to grasp serde's recursive structure: I think someone with such experience will be able to spot what is missing, in short order.