Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing fields are not handled correctly. #5

Open
gorzell opened this issue Dec 20, 2021 · 3 comments
Open

Missing fields are not handled correctly. #5

gorzell opened this issue Dec 20, 2021 · 3 comments

Comments

@gorzell
Copy link
Contributor

gorzell commented Dec 20, 2021

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
Copy link
Owner

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
Copy link
Contributor Author

gorzell commented Dec 23, 2021

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
Copy link
Contributor Author

gorzell commented Dec 23, 2021

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 tokio-rs/prost#150 (comment) to try and make the configuration a bit easier.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants