Implement Deserialize and Serialize for plist::Dictionary and plist::Value

[MLKrisJohnson]

Implements serialization and deserialization of the Dictionary and Value types.

This fixes the nested-dictionary deserialization issue discussed in #54, and also allows plist files to be read and written as general Dictionary structs rather than as custom structs.

All the existing tests passed, and I don't think I've made any changes that would affect any existing code that uses this crate.

There are two limitations to this implementation:

• Date elements in a plist are deserialized as String elements in the dictionary.
• Uid elements are deserialized as Integer elements in the dictionary.

I tried to figure out how to fix these issues, but gave up. I think plist::value::Value::deserialize() or de:Deserializer::deserialize_any() need a change to properly handle these two newstruct types. However, even with these limitations, this code is useful. Maybe someone else can figure out how to complete the implementation for Date and Uid

[ebarnard]

Hi, thanks for the PR. Sorry, I completely forgot about it. I'll have a look at it in the next couple of days.

[gibfahn]

Tried this out and it's working for me in a local project, @ebarnard if you get time to take a look it would be great to get this merged.

[ebarnard]

Looks good and I'm happy to merge this. I'd like to get Date and Uid round-tripping properly when a Value is deserialised from a plist file before releasing it though, and it might take me a while to get round to it myself. If you want to give it a shot, some guidance is below.

Serialisation is already working thanks to: https://github.com/ebarnard/rust-plist/blob/e06e4366a4246be04ac9f7bbd7a96fd2cb239be8/src/date.rs#L111

Deserialisation just needs the visitor to implement visit_newtype_struct and switch on the newtype struct name, as is done in the serialiser: https://github.com/ebarnard/rust-plist/blob/e06e4366a4246be04ac9f7bbd7a96fd2cb239be8/src/ser.rs#L247

[ebarnard]

I'm not sure why tests aren't being run but a push usually fixes that.

[MLKrisJohnson]

I'll give this another try, with your hint to implement visit_newtype_struct

[MLKrisJohnson]

This is probably very simple, but I'm not familiar with the internals of serde and I get confused when I try to understand the relationship between visitors and deserializers. It seems the deserializer calls the visitor which calls the deserializer which calls the visitor which calls the deserializer....

If I understand your suggestion, I should add a method like this to the ValueVisitor implementation at https://github.com/MLKrisJohnson/rust-plist/blob/67caaa83110afa51d84f4d927ff03d9de72f392e/src/value.rs#L370

            fn visit_newtype_struct<D>(self, deserializer: D) -> Result<Value, D::Error>
            where
                D: Deserializer<'de>,
            {
                todo!();
            }

Then I should "switch on the newtype name", but this method doesn't have access to the newtype name. The plist::Deserializer type does have a deserialize_newtype_struct() method that would have access to the name, but that method currently calls visit_newtype_struct:

https://github.com/MLKrisJohnson/rust-plist/blob/67caaa83110afa51d84f4d927ff03d9de72f392e/src/de.rs#L182

However, it's not clear to me whether I should

• change deserialize_newtype_struct so that it switches on the name, calls self.events.next() to read a string for Date or a u64 for Uid and then construct that kind of value. (This seems straightforward, but every deserialize_newtype_struct example I find says it should just call visit_newtype_struct so it feels like I'm missing something.)
• change deserialize_any (at https://github.com/MLKrisJohnson/rust-plist/blob/67caaa83110afa51d84f4d927ff03d9de72f392e/src/de.rs#L91) so that it calls visitor.visit_newtype_struct for the Event::Date and Event::Uid cases, providing either self or some kind of type-specific deserializer.
• somehow call DateNewtypeVisitor::visit_newtype_struct and UidNewtypeVisitor::visit_newtype_struct someplace
• somehow call Date::deserialize() or Uid::deserialize() someplace

Can someone please point me in the right direction? Thanks!

[ebarnard]

I thought there was a way for a Deserializer to hint to a Deserialize/Visitor the name of an expected newtype struct but you're right, there isn't, it's the other way round.

I can think of a few other ways of doing this, but none are particularly nice so happy to merge as is, although you might have to wait a little bit for a release.

[MLKrisJohnson]

I'd like to get this right, but I don't really have a strategy at the moment. I'd say go ahead and merge it if you're happy with it.

Do you need me to make another push or something to get the tests to run? (It looks like tests are failing because they consider serde to be undeclared, but that doesn't make sense. Tests run fine on my machine.)

[ebarnard]

Serde is only enabled with #[cfg(feature = “serde”)] which is why the tests are failing. I suggest putting all the serde stuff in a module like is done in https://github.com/ebarnard/rust-plist/blob/9ed64da1cc52ba766ae9ba84eefa6edffc31c90a/src/uid.rs#L28

[MLKrisJohnson]

I've moved code around so that the tests run. I think this is ready for merging.

[ebarnard]

Looks great, thanks for doing all this work