Serde #[serde(skip_serializing_if="Option::is_none")] fails

[chridou]

When using #[serde(skip_serializing_if="Option::is_none")] on a field that has the value None and if it is not the last field deserialization fails(maybe it is already serialized in a wrong way)

Here is some test code:

#[cfg(test)]
mod test_rmp_serde {
    use serde::{Serialize, Deserialize};
    use serde_msgpack::{Serializer, Deserializer};

    #[derive(Debug, PartialEq, Serialize, Deserialize)]
    struct Example1 {
        test_field: i64,
        name: Option<String>,
        items: Vec<i64>,
    }

    #[derive(Debug, PartialEq, Serialize, Deserialize)]
    struct Example2 {
        #[serde(rename = "test_field_renamed")]
        test_field: i64,
        // If you remove this annotation, it will pass
        #[serde(skip_serializing_if="Option::is_none")]
        name: Option<String>,
        #[serde(rename = "items_renamed")]
        items: Vec<i64>,
    }

    #[test]
    fn without_serde_attributes() {
        let original = Example1 {
            test_field: 0,
            items: vec![],
            name: None,
        };

        let mut buf = Vec::new();
        {
            let mut ser = Serializer::new(&mut buf);
            original.serialize(&mut ser).unwrap();
        }

        let mut deser = Deserializer::new(buf.as_slice());
        let deserialized = Example1::deserialize(&mut deser).unwrap();
        assert_eq!(original, deserialized);
    }

    #[test]
    fn with_serde_attributes() {
        let original = Example2 {
            test_field: 0,
            items: vec![],
            name: None,
        };

        let mut buf = Vec::new();
        {
            let mut ser = Serializer::new(&mut buf);
            original.serialize(&mut ser).unwrap();
        }

        let mut deser = Deserializer::new(buf.as_slice());
        let deserialized = Example2::deserialize(&mut deser).unwrap();
        assert_eq!(original, deserialized);
    }
}

[SuperFluffy]

I was just wondering about how rmp would treat this. I think that since the Deserializer would need to know how many bytes it needs to consume, skipping values makes it much more difficult than doing it with json.

Maybe there is a way to have rmp ignore serde's skip_serializing_if?

[spamwax]

I think I am also running into this issue with #[serde(skip)]. Serializing and deserializing a struct fails with errors such as:

value: Syntax("invalid length 6, expected tuple of 7 elements")

[oysterpack]

I also ran into the issue.

   #[test]
    fn rmp_serde_deserialize_fails_when_skipping_none_option() {
        #[derive(Debug, Serialize, Deserialize)]
        struct Foo {
            #[serde(skip_serializing_if = "Option::is_none")]
            bar: Option<bool>
        }

        let foo = Foo { bar: None } ;
        let foo_bytes = rmp_serde::to_vec(&foo).unwrap();
        // deserializing panics because rmp is expecting 1 element
        let foo : Foo = rmp_serde::from_slice(&foo_bytes).unwrap();
    }

---- message::test::rmp_serde_deserialize_fails_when_skipping_none_option stdout ---- thread 'message::test::rmp_serde_deserialize_fails_when_skipping_none_option' panicked at 'called Result::unwrap() on an Err value: Syntax("invalid length 0, expected struct Foo with 1 element")', libcore/result.rs:1009:5

... this is a tough one to fix on the deserialization side because of the MessagePack format. I propose that rmp_serde simply drop support for #[serde(skip_serializing_if = "Option::is_none")]. If it serializes, then it should deserialize.

[Mubelotix]

But since rmp is supposed to be lightweight it should support optional fields. In my case, it is worth switching to serde_json.

[kornelski]

It's probably yet another side-effect of magic handling of enums in rmp: #245

[jnicholls]

I am in agreement with this:

I propose that rmp_serde simply drop support for #[serde(skip_serializing_if = "Option::is_none")]. If it serializes, then it should deserialize.

If there is no other solution that allows for field skipping and is equally deterministic, I think skips should be ignored by rmp-serde. Unfortunately it's not that simple since that is a serde construct, and that's outside of the purview of rmp-serde to control.

What to do, what to do...