fdeantoni / prost-wkt

Prost Well-Known-Types serialization and deserialization.
Apache License 2.0
76 stars 35 forks source link

Missing fields are not handled correctly. #5

Open gorzell opened 2 years ago

gorzell commented 2 years ago

Since every thing is "optional" in proto3, it can be the case the not all fields exist in the JSON object. This is especially the case if one side of the exchange has a different version of the proto than the other and there are new fields or removed fields. This leads to a bunch of default values that should be used when something doesn't exist. I believe there is also some way to check if a value was unset or set to the same as the default, although maybe that is not consistent across languages.

fdeantoni commented 2 years ago

To be honest I have not had to deal with proto2 that much so did not really consider these scenarios. I suspect the marking of whether a field was set or not is (or should be) a feature provided by Prost. Does the protobuf spec describe how to handle an upgrade from proto2 to proto3? A cursory search did not show anything but I did find this stackoverflow post describing a similar scenario...

gorzell commented 2 years ago

Did you mean that you haven't had to deal with proto3?

At least in Go, if you serialize an "empty" struct to JSON the result is {}. I looked at the prost code a bit, but couldn't find where they were handling missing fields. It looks like on the serde side the easiest option would be to add the #[serde(default)] to every message/struct that prost generates. I checked and the Rust defaults look like they align with the proto3 spec.


fn main() {

println!("{}", bool::default());
println!("{}", u32::default());
println!("{}", i32::default());
println!("{}", f32::default());
println!("{:?}", Vec::<u32>::default());
println!("{:?}", Vec::<u8>::default());
println!("\"{}\"", String::default());
println!("{:?}", Option::<i32>::default());

}
gorzell commented 2 years ago

I just realized that the serde annotations are actually part of the example build.rs, rather than the library itself, so maybe this can be closed, or the example should just be updated. I commented on https://github.com/tokio-rs/prost/issues/150#issuecomment-1000279092 to try and make the configuration a bit easier.