iddm / serde-aux

An auxiliary serde library providing helpful functions for serialisation and deserialisation for containers, struct fields and others.
MIT License
152 stars 26 forks source link

deserialize_default_from_empty_object works differently when default tag is set on inner struct #6

Open georgemp opened 4 years ago

georgemp commented 4 years ago

Hi,

With the example here, if we tag InnerStruct with #[serde(default)], then the test with empty object fails. A default InnerStruct is created on the struct MyStruct, instead of None. I believe the default for an Option is None. So, not sure why a default object is being constructed in place of it. I have added comments to the relevant sections of the code.

extern crate serde_json;
extern crate serde_aux;
extern crate serde;

use serde_aux::prelude::*;

#[derive(Serialize, Deserialize, Debug)]
struct MyStruct {
    #[serde(deserialize_with = "deserialize_default_from_empty_object")]
    empty_as_default: Option<MyInnerStruct>,
}

#[derive(Serialize, Deserialize, Debug)]
#[serde(default)] //Added this, so that mandatory will be set to 0, if the field is missing from json.
struct MyInnerStruct {
    mandatory: u64,
}

fn main() {
    let s = r#" { "empty_as_default": { "mandatory": 42 } } "#;
    let a: MyStruct = serde_json::from_str(s).unwrap();
    assert_eq!(a.empty_as_default.unwrap().mandatory, 42);

    let s = r#" { "empty_as_default": null } "#;
    let a: MyStruct = serde_json::from_str(s).unwrap();
    assert!(a.empty_as_default.is_none());

    let s = r#" { "empty_as_default": {} } "#;
    let a: MyStruct = serde_json::from_str(s).unwrap(); 
    assert!(a.empty_as_default.is_none()); //This assert fails

    let s = r#" { "empty_as_default": { "unknown": 42 } } "#;
    assert!(serde_json::from_str::<MyStruct>(s).is_err());
}

Thanks

iddm commented 4 years ago

Hi!

Sorry for the late response. If this is still an issue, I can't reproduce it currently using this code:

use serde_aux::prelude::*;

#[derive(serde::Serialize, serde::Deserialize, Debug)]
struct MyStruct {
    #[serde(deserialize_with = "deserialize_default_from_empty_object")]
    empty_as_default: Option<MyInnerStruct>,
}

#[derive(serde::Serialize, serde::Deserialize, Debug)]
#[serde(default)] //Added this, so that mandatory will be set to 0, if the field is missing from json.
struct MyInnerStruct {
    mandatory: u64,
}

fn main() {
    let s = r#" { "empty_as_default": { "mandatory": 42 } } "#;
    let a: MyStruct = serde_json::from_str(s).unwrap();
    assert_eq!(a.empty_as_default.unwrap().mandatory, 42);

    let s = r#" { "empty_as_default": null } "#;
    let a: MyStruct = serde_json::from_str(s).unwrap();
    assert!(a.empty_as_default.is_none());

    let s = r#" { "empty_as_default": {} } "#;
    let a: MyStruct = serde_json::from_str(s).unwrap();
    assert!(a.empty_as_default.is_none()); //This assert fails

    let s = r#" { "empty_as_default": { "unknown": 42 } } "#;
    assert!(serde_json::from_str::<MyStruct>(s).is_err());
}
   Compiling t v0.1.0 (/tmp/t)
error[E0277]: the trait bound `MyInnerStruct: std::default::Default` is not satisfied
 --> src/main.rs:9:28
  |
9 | #[derive(serde::Serialize, serde::Deserialize, Debug)]
  |                            ^^^^^^^^^^^^^^^^^^ the trait `std::default::Default` is not implemented for `MyInnerStruct`
  |
  = help: see issue #48214

error: aborting due to previous error

For more information about this error, try `rustc --explain E0277`.
error: could not compile `t`.

Also, you don't need the [serde(default)], you need a #[derive(Default)] instead. However, in tyour case, you don't need even the #[derive(Default)] as we are not using your MyInnerStruct type directly but through an Option which has a Default implementation to None.

iddm commented 4 years ago

Hopefully, I helped. If you have any more questions, do not hesitate to ask.

georgemp commented 3 years ago

Hi,

I apologize for the extremely late response. I totally lost track of this issue. I do believe you are right - I had misunderstood the behavior around defaults. Thanks for your response :)

georgemp commented 3 years ago
extern crate serde;
extern crate serde_aux;
extern crate serde_json;

use serde::{Deserialize, Serialize};
use serde_aux::prelude::*;

#[derive(Serialize, Deserialize, Debug, Default)]
struct MyStruct {
    #[serde(deserialize_with = "deserialize_default_from_empty_object")]
    empty_as_default: Option<MyInnerStruct>,
}

#[derive(Serialize, Deserialize, Debug, Default, PartialEq)]
#[serde(default)]
struct MyInnerStruct {
    mandatory: u64,
    optional: u64,
}

fn main() {
    let s = r#" { "empty_as_default": {} } "#;
    let a: MyStruct = serde_json::from_str(s).unwrap();
    assert_eq!(a.empty_as_default, None);
}

apologies again :) Just looked at my code again, and, understood the source of my original confusion. Hope you can help me understand why this is happening. In this example, the json has field empty_as_default set to an empty object {}. So, I'm expecting it to be deserialized to None (which I believe to be the default for an Option - in this case Option<MyInnerStruct>). However, it is deserialized to Some((MyInnerStruct { mandatory: 0, optional: 0 }).

The reason I have #serde[default] on MyInnerStruct is that I could have json such as "empty_as_default": {"mandatory": 42}, and, would like the remaining fields to be defaulted.

Thanks

georgemp commented 3 years ago
#[derive(Serialize, Deserialize, Debug, Default, PartialEq)]
struct MyInnerStruct {
    mandatory: u64,
    #[serde(default)]
    optional: u64,
}

Setting #[serde(default)] on the field level, instead of container level, deserializes to None. Not sure if the container level behavior is to be expected, but, this seems to work fine for my use case (outside of having to put in the attribute at each field) :)

iddm commented 3 years ago

I guess this is different because of the #[serde(default)], as the example from the documentation works fine:

use serde_aux::prelude::*;

#[derive(serde::Serialize, serde::Deserialize, Debug)]
struct MyStruct {
    #[serde(deserialize_with = "deserialize_default_from_empty_object")]
    empty_as_default: Option<MyInnerStruct>,
}

#[derive(serde::Serialize, serde::Deserialize, Debug)]
struct MyInnerStruct {
    mandatory: u64,
}

fn main() {
    let s = r#" { "empty_as_default": { "mandatory": 42 } } "#;
    let a: MyStruct = serde_json::from_str(s).unwrap();
    assert_eq!(a.empty_as_default.unwrap().mandatory, 42);

    let s = r#" { "empty_as_default": null } "#;
    let a: MyStruct = serde_json::from_str(s).unwrap();
    assert!(a.empty_as_default.is_none());

    let s = r#" { "empty_as_default": {} } "#;
    let a: MyStruct = serde_json::from_str(s).unwrap();
    assert!(a.empty_as_default.is_none());

    let s = r#" { "empty_as_default": { "unknown": 42 } } "#;
    assert!(serde_json::from_str::<MyStruct>(s).is_err());
}

Perhaps, this #[serde(default)] does some magic stuff, because if I look at the code which does this deserialisation, it should call Option::default() constructor here, as the type T is deduced to be Option<MyInnerStruct>:

pub fn deserialize_default_from_empty_object<'de, D, T>(deserializer: D) -> Result<T, D::Error>
where
    D: Deserializer<'de>,
    T: Deserialize<'de> + Default,
{
    #[derive(Debug, Deserialize)]
    #[serde(deny_unknown_fields)]
    struct EmptyObject {}

    #[derive(Debug, Deserialize)]
    #[serde(untagged)]
    enum EmptyOrNot<Y> {
        NonEmpty(Y),
        Empty(EmptyObject),
    }

    let empty_or_not: EmptyOrNot<T> = EmptyOrNot::deserialize(deserializer)?;

    match empty_or_not {
        EmptyOrNot::NonEmpty(e) => Ok(e),
        _ => Ok(T::default()),
    }
}

It could be that #[serde(default)] alters this behaviour but I don't know how exactly, the types are the types unless it is changed. UPD: Yes, according to the docs and my observation, the #[serde(default)] works first and simply doesn't invoke the deserialise_with implementation it seems. I don't think this is intended though, I would expect the deserialise_with to always work in any single case. Something that's worth clarification from the serde guys, perhaps, even a mention of this behaviour (if it is indeed intended) in the documentation.

georgemp commented 3 years ago

Unfortunately, settings field level attribute #[serde(default)] on all fields on the struct, goes back to container level behavior.

#[derive(Serialize, Deserialize, Debug, Default, PartialEq)]
struct MyInnerStruct {
    #[serde(default)]
    mandatory: u64,
    #[serde(default)]
    optional: u64,
}

For now, i'm just picking a field that I think will never be absent, and, removing the default attribute on that field :)

iddm commented 3 years ago

Thanks for noticing this odd behaviour. I don't think this is how my library should work as I clearly stated the behaviour in the documentation and this deviates from it. But this doesn't seem like something to be fixed in the serde-aux crate, rather than the serde, unfortunately.

iddm commented 2 years ago

It takes too long for serde guys to respond. I guess the best way is to somehow avoid having the #[serde(default)] specified. I can't do anything more or less universal to work this around.

iddm commented 2 years ago
extern crate serde;
extern crate serde_aux;
extern crate serde_json;

use serde::{Deserialize, Serialize};
use serde_aux::prelude::*;

#[derive(Serialize, Deserialize, Debug, Default)]
struct MyStruct {
    #[serde(deserialize_with = "deserialize_default_from_empty_object")]
    empty_as_default: Option<MyInnerStruct>,
}

#[derive(Serialize, Deserialize, Debug, Default, PartialEq)]
#[serde(default)]
struct MyInnerStruct {
    mandatory: u64,
    optional: u64,
}

fn main() {
    let s = r#" { "empty_as_default": {} } "#;
    let a: MyStruct = serde_json::from_str(s).unwrap();
    assert_eq!(a.empty_as_default, None);
}

apologies again :) Just looked at my code again, and, understood the source of my original confusion. Hope you can help me understand why this is happening. In this example, the json has field empty_as_default set to an empty object {}. So, I'm expecting it to be deserialized to None (which I believe to be the default for an Option - in this case Option<MyInnerStruct>). However, it is deserialized to Some((MyInnerStruct { mandatory: 0, optional: 0 }).

The reason I have #serde[default] on MyInnerStruct is that I could have json such as "empty_as_default": {"mandatory": 42}, and, would like the remaining fields to be defaulted.

Thanks

I am revisiting this issue since one of the maintainers helped me understand what is going on. I do not remember the details of the last attempt of mine of trying to understand all of this, but I was sure that the #[serde(default)] didn't invoke the deserialize_with. I have just tried this now and it works. The same was pointed out to me by the maintainer. Now, having tried this out and revisited the code I am quoting here, the reason we have Some((MyInnerStruct { mandatory: 0, optional: 0 }) is because the #[serde(default)] is set right on the struct, providing it with a deserializer which allows it to be constructed with default values from {}, so it becomes a perfectly valid object. Since it is a perfectly-valid and fully-deserialized object in this case, the Option has Some inside and so eventually in the assertion we have Some((MyInnerStruct { mandatory: 0, optional: 0 }). If we remove #[serde(default)], there will be no way to deserialize the input provided into a meaningful object of this type, and so None will be returned. Does it help or I am still missing some point?