Proposal to add default values in deserialisation

[theoturner]

Protobuf does not serialise default values, however the betterproto objects could retain them.

Consider:

enum MyType {
    A = 0;
    B = 1;
}

message MyMessage {
    string name = 1;
    MyType type = 2;
}

I create a new message with the default enum:

m = api.MyMessage(name='foo', type=api.MyType(0))

If I send this message, and the receiver creates a betterproto message, they get:

api.MyMessage(name='foo')

Fortunately betterproto has equality between these messages, even though in the latter type is not specified.

However, this is not particularly easy to find without a bit of digging. I believe it would be simpler to just add the default values when deserialising, which is actually pretty simple:

def _fill_defaults(message: api.betterproto.Message) -> api.betterproto.Message:
    message_keys = {}
    for k in message.__dict__.keys():
        if k not in ["_serialized_on_wire", "_unknown_fields", "_group_current"]:
            current_value = getattr(message, k)
            if not current_value:
                message_keys[k] = message._get_field_default(k)
            else:
                message_keys[k] = current_value
    return message.__class__(**message_keys)

I'm sure there is a neater way of handling the internal attributes e.g. _serialise_on_wire.

With the above, the receiver would have the same object as the sender, which is more intuitive.

[uschi2000]

Could you clarify, @theoturner , what the current round-trip behavior is with a simple "echo" server that just returns the received message:

def echo(request: api.MyMessage) -> api.MyMessage):
  return request

def test_roundtrip():
  sent = api.MyMessage(name='foo', type=api.MyType(0))
  received = client.echo(sent)

  assert sent == received # ?
  assert sent.name == received.name # ?
  assert send.type == received.type # ?
  assert received.type == api.MyType.A # ?

[theoturner]

I get:
True
True
False
False

Edit: correction on first line

[uschi2000]

Thanks for checking. Indeed, that behavior seems "surprising" in best case. @danielgtaylor , what's your appetite for getting this fixed? We're happy to provide the code.

[uschi2000]

@theoturner I wonder if this got fixed in the meantime? On today's master, the following test case passes: https://github.com/uschi2000/python-betterproto/pull/1/files

@pytest.mark.asyncio
async def test_messages_roundtrip_with_default_fields():
    async with ChannelFor([ThingService()]) as channel:
        client = ThingServiceClient(channel)
        request = DoThingRequest("name", ["comment"], ThingType.UNKNOWN, 0) # UNKNOWN and 0 are protobuf default values
        response = await client.do_thing(request)

        # Assert we get back the request
        assert request == response.request
        assert response.names == ["name"]
        assert response.request.number == 0
        assert response.request.type == ThingType.UNKNOWN
        assert id(request) != id(response.request) # prove that objects went through serde

        # Try with a newly construction request object
        expected_request = DoThingRequest("name", ["comment"], ThingType.UNKNOWN, 0)
        assert expected_request == response.request

[Gobot1234]

I'm pretty sure the issue you are describing here is fixed by #293