Support proto3 field presence (#281)

* Update protobuf pregenerated files

* Update grpcio-tools to latest version

* Implement proto3 field presence

* Fix to_dict with None optional fields.

* Add test with optional enum

* Properly support optional enums

* Add tests for 64-bit ints and floats

* Support field presence for int64 types

* Fix oneof serialization with proto3 field presence (#292)

= Description

The serialization of a oneof message that contains a message with fields
with explicit presence was buggy.

For example:

```
message A {
    oneof kind {
        B b = 1;
        C c = 2;
    }
}

message B {}
message C {
    optional bool z = 1;
}
```

Serializing `A(b=B())` would lead to this payload:

```
0A # tag1, length delimited
00 # length: 0
12 # tag2, length delimited
00 # length: 0
```

Which when deserialized, leads to the message `A(c=C())`.

= Explanation

The issue lies in the post_init method. All fields are introspected, and
if different from PLACEHOLDER, the message is marked as having been
"serialized_on_wire".
Then, when serializing `A(b=B())`, we go through each field of the
oneof:

- field 'b': this is the selected field from the group, so it is
  serialized
- field 'c': marked as 'serialized_on_wire', so it is added as well.

= Fix

The issue is that support for explicit presence changed the default
value from PLACEHOLDER to None. This breaks the post_init method in that
case, which is relatively easy to fix: if a field is optional, and set
to None, this is considered as the default value (which it is).

This fix however has a side-effect: the group_current for this field (the
oneof trick for explicit presence) is no longer set. This changes the
behavior when serializing the message in JSON: as the value is the
default one (None), and the group is not set (which would force the
serialization of the field), so None fields are no longer serialized in
JSON. This break one test, and will be fixed in the next commit.

* fix: do not serialize None fields in JSON format

This is linked to the fix from the previous commit: after it, scalar
None fields were not included in the JSON format, but some were still
included.

This is all cleaned up: None fields are not added in JSON by default,
as they indicate the default value of fields with explicit presence.
However, if `include_default_values is set, they are included.

* Fix: use builtin annotation prefix

* Remove comment

Co-authored-by: roblabla <unfiltered@roblab.la>
Co-authored-by: Vincent Thiberville <vthib@pm.me>
This commit is contained in:
Kalan
2021-12-29 13:38:32 -08:00
committed by GitHub
parent 671c0ff4ac
commit d77f44ebb7
17 changed files with 809 additions and 573 deletions

View File

@@ -0,0 +1,12 @@
{
"test1": 128,
"test2": true,
"test3": "A value",
"test4": "aGVsbG8=",
"test5": {
"test": "Hello"
},
"test6": "B",
"test7": "8589934592",
"test8": 2.5
}

View File

@@ -0,0 +1,21 @@
syntax = "proto3";
message InnerTest {
string test = 1;
}
message Test {
optional uint32 test1 = 1;
optional bool test2 = 2;
optional string test3 = 3;
optional bytes test4 = 4;
optional InnerTest test5 = 5;
optional TestEnum test6 = 6;
optional uint64 test7 = 7;
optional float test8 = 8;
}
enum TestEnum {
A = 0;
B = 1;
}

View File

@@ -0,0 +1,9 @@
{
"test1": 0,
"test2": false,
"test3": "",
"test4": "",
"test6": "A",
"test7": "0",
"test8": 0
}

View File

@@ -0,0 +1,38 @@
import json
from tests.output_betterproto.proto3_field_presence import Test, InnerTest, TestEnum
def test_null_fields_json():
"""Ensure that using "null" in JSON is equivalent to not specifying a
field, for fields with explicit presence"""
def test_json(ref_json: str, obj_json: str) -> None:
"""`ref_json` and `obj_json` are JSON strings describing a `Test` object.
Test that deserializing both leads to the same object, and that
`ref_json` is the normalized format."""
ref_obj = Test().from_json(ref_json)
obj = Test().from_json(obj_json)
assert obj == ref_obj
assert json.loads(obj.to_json(0)) == json.loads(ref_json)
test_json("{}", '{ "test1": null, "test2": null, "test3": null }')
test_json("{}", '{ "test4": null, "test5": null, "test6": null }')
test_json("{}", '{ "test7": null, "test8": null }')
test_json('{ "test5": {} }', '{ "test3": null, "test5": {} }')
# Make sure that if include_default_values is set, None values are
# exported.
obj = Test()
assert obj.to_dict() == {}
assert obj.to_dict(include_default_values=True) == {
"test1": None,
"test2": None,
"test3": None,
"test4": None,
"test5": None,
"test6": None,
"test7": None,
"test8": None,
}

View File

@@ -0,0 +1,3 @@
{
"nested": {}
}

View File

@@ -0,0 +1,20 @@
syntax = "proto3";
message Test {
oneof kind {
Nested nested = 1;
WithOptional with_optional = 2;
}
}
message InnerNested {
optional bool a = 1;
}
message Nested {
InnerNested inner = 1;
}
message WithOptional {
optional bool b = 2;
}

View File

@@ -0,0 +1,29 @@
from tests.output_betterproto.proto3_field_presence_oneof import (
Test,
InnerNested,
Nested,
WithOptional,
)
def test_serialization():
"""Ensure that serialization of fields unset but with explicit field
presence do not bloat the serialized payload with length-delimited fields
with length 0"""
def test_empty_nested(message: Test) -> None:
# '0a' => tag 1, length delimited
# '00' => length: 0
assert bytes(message) == bytearray.fromhex("0a 00")
test_empty_nested(Test(nested=Nested()))
test_empty_nested(Test(nested=Nested(inner=None)))
test_empty_nested(Test(nested=Nested(inner=InnerNested(a=None))))
def test_empty_with_optional(message: Test) -> None:
# '12' => tag 2, length delimited
# '00' => length: 0
assert bytes(message) == bytearray.fromhex("12 00")
test_empty_with_optional(Test(with_optional=WithOptional()))
test_empty_with_optional(Test(with_optional=WithOptional(b=None)))