google-cloud-rust
google-cloud-rust copied to clipboard
Detect if two variants of a `oneof` field are set in JSON inputs
When deserializing a oneof from JSON we need to detect if there are multiple variants set. Ignoring this could lead to silent data loss, or at least confusing behavior.
- [ ]
Required.Proto3.JsonInput.OneofFieldDuplicate# Should have failed to parse, but didn't. - [ ]
Required.Proto3.JsonInput.OneofFieldNullFirst.JsonOutput# JSON output we received from test was unparseable.
ERROR, test=Required.Proto3.JsonInput.OneofFieldDuplicate: Should have failed to parse, but didn't., request=goo.gle/debugonly json_payload: "{\"oneofUint32\": 1, \"oneofString\": \"test\"}" requested_output_format: JSON message_type: "protobuf_test_messages.proto3.TestAllTypesProto3" test_category: JSON_TEST, response=goo.gle/debugonly json_payload: "{\"oneofUint32\":1,\"oneofString\":\"test\"}"
ERROR, test=Required.Proto3.JsonInput.OneofFieldNullFirst.JsonOutput: JSON output we received from test was unparseable., request=goo.gle/debugonly json_payload: "{\"oneofUint32\": null, \"oneofString\": \"test\"}" requested_output_format: JSON message_type: "protobuf_test_messages.proto3.TestAllTypesProto3" test_category: JSON_TEST, response=goo.gle/debugonly json_payload: "{\"oneofUint32\":0,\"oneofString\":\"test\"}"
The fixes for this are going to take multiple steps, which I think deserve at least an outline.
- I have improved testing to include messages with all the primitive types as required, optional, repeated, and map fields.
- I have improved testing to include messages with enum types as required, optional, repeated, and map fields.
- I have improved testing to include message types as optional (by default and explicitly), repeated, and map fields.
- I have improved testing to include a well-known message type as optional (by default and explicitly), repeated, and map fields that serializes to strings.
- I have improved testing to include the
google.protobuf.Valuetype as optional (by default and explicitly), repeated, and map fields. - For all these messages, I will change the generator to emit a helper type with hand-crafted serialization
- Initially only the body (no fields) will be serialized.
- I will slowly add different field types to this serialization, including unit tests.
- I will start with fields that do not require
serde_withandserde_as()mapping. - Once all the fields are working, I will switch the generator and tests to use the new serialization in the normal messages.
- We should be able to remove a lot of serde annotations.
- We should also be able to cleanup generator code and tests after these changes.