danielgtaylor / danielgtaylor/python-betterproto

Default values are not serialized in oneofs when calling .to_dict() or .to_json() (which is different behaviour from official protobuf impl)

Open
#109 4 comments 0 reactions 0 assignees View on GitHub
bug medium priority
Dominant language
Python
Stars
1.8k
Forks
234
PR merge metrics
No merged PRs in 30d

Description

# Consider this protobuf message:

```
message OneOfDemo{
oneof value_type {
bool bool_value = 1;
int64 int64_value = 2;
}
}
```

# Using the official protobuf codegen and library (libprotoc 3.12.3):

```
from google.protobuf.json_format import MessageToDict

# Official protobuf:
print("True: ", json.dumps(MessageToDict(OneOfDemo(bool_value=True))))
print("False:", json.dumps(MessageToDict(OneOfDemo(bool_value=False))))
print("1: ", json.dumps(MessageToDict(OneOfDemo(int64_value=1))))
print("0: ", json.dumps(MessageToDict(OneOfDemo(int64_value=0))))

Output:
True: {"boolValue": true}
False: {"boolValue": false}
1: {"int64Value": "1"}
0: {"int64Value": "0"}
```

# Using betterproto (1.2.5)

```
# Generated code:
@dataclass
class OneOfDemoBetter(betterproto.Message):
bool_value: bool = betterproto.bool_field(1, group="value_type")
int64_value: int = betterproto.int64_field(2, group="value_type")

print("True: ", OneOfDemoBetter(bool_value=True).to_json())
print("False:", OneOfDemoBetter(bool_value=False).to_json())
print("1: ", OneOfDemoBetter(int64_value=1).to_json())
print("0: ", OneOfDemoBetter(int64_value=0).to_json())

Output:
True: {"boolValue": true}
False: {}
1: {"int64Value": "1"}
0: {}
```

This obviously leads to the following inconsistency:

```
od = OneOfDemoBetter(bool_value=False)
print(betterproto.which_one_of(od, "value_type"))

od2 = OneOfDemoBetter().from_json(od.to_json())
print(betterproto.which_one_of(od2, "value_type"))

Output:
('bool_value', False)
('', None)
```

# Misc

* Python 3.8.1
* I love what you are trying to do with this project btw - the official generated protobuf code is AWEFUL and this is a HUGE step forwards. This one difference in behaviour is blocking wholesale adoption of your project unfortunately
* Tangentially related ideas and observations
* Would it make sense to have values in the oneof be declared `Optional`? (ie: `bool_value: Optional[bool]`) since only one is expected to be set? Personally I would prefer this but I can understand if its getting too far away from the idea of "there are no nulls in proto3".
* `.to_json()` is missing the `casing` arg (and `include_default_values`) from `.to_dict()`
* Stretch/wishlist: personally I would find it super valuable if there were some way to default to `Casing.SNAKE` for all `.to_json()` operations automatically. Perhaps something like a default option you could specify on a per message basis?

Great work on this project! Keep it up!

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.