Add support to optional fields (oneOf)

[richard-ramos]

The import_proto3 fails when attempting to parse a protobuffer definition that contains oneof:

message Test {
  oneof test_oneof {
     string foo = 1;
     bytes bar = 2;
  }
}

The error happens in this line https://github.com/status-im/nim-protobuf-serialization/blob/master/protobuf_serialization/files/type_generator.nim#L54, which seems to indicate that logic for handling optional fields hasn't been implemented yet (oneof ProtoNodes contain oneofName instead of name)

[arnetheduck]

If you don't need to know which field was set, you can simply remove oneof from the definition - there's no actual difference in the wire encoding - when you receive data, one of the fields will simply have a non-default value - you just have to be careful to clear "the other" fields when setting a particular one or you'll end up sending the wrong data.

If you do need to know which field was set, you can fall back on manual parsing for this message - not sure if this repo exposes utilities for that, but if it doesn't there's always minprotobuf that handles oneof as well.

To implement support for this, one would need to generate an extra discriminator field that says which field was actually set - in theory one would use a case object in Nim but the semantics surrounding changing the case kind are quite broken in the language (the semantics surrounding switching case are.. unusual) - there are two ways to work around the language issues:

• create a separate type for the oneof group that contains a discriminator and the values - this breaks the one-message-one-type mapping that presently occurs but is probably the more clean solution - it somewhat mimics C++ that creates a union which is a separate type.
• not use case and clear set field when setting another, in setters - this will use more memory per message because now all the fields take up space instead of just one - this is a problem mainly if there are many cases in the oneof

One difficulty is that the discriminator must also handle the "none of the fields set" case: https://developers.google.com/protocol-buffers/docs/reference/cpp-generated#oneof

[BurnySc2]

I'm running into a similar issue when using oneof. Here is an example where both encode to an empty array:

syntax = "proto3";

message ExampleRequest {
  oneof request {
    RequestOne request_a = 1;
    RequestTwo request_b = 2;
  }
}

message RequestOne {
}

message RequestTwo {
}

import protobuf_serialization
import protobuf_serialization/proto_parser
import_proto3 "my_protocol.proto3"

# Needs to encode to something because `RequestOne` was chosen
let request1: ExampleRequest = ExampleRequest(request_a: RequestOne())
echo request1 # (request_b: (), request_a: ())
echo Protobuf.encode(request1) # @[]

# Needs to encode to something because `RequestTwo` was chosen
let request2: ExampleRequest = ExampleRequest(request_b: RequestTwo())
echo request2 # (request_b: (), request_a: ())
echo Protobuf.encode(request2) # @[]

# Shouldn't be allowed due to `oneof` - only one may be set
let request3: ExampleRequest = ExampleRequest(request_a: RequestOne(), request_b: RequestTwo())
echo request3
echo Protobuf.encode(request3)

Sadly this is a real case in the StarCraft 2 proto https://github.com/Blizzard/s2client-proto/blob/bb587ce9acb37b776b516cdc1529934341426580/s2clientprotocol/sc2api.proto#L84-L96 https://github.com/Blizzard/s2client-proto/blob/bb587ce9acb37b776b516cdc1529934341426580/s2clientprotocol/sc2api.proto#L334-L335

For the minprotobuf you mentioned, I believe you mean https://github.com/status-im/nim-libp2p/blob/c6aa085e98e7526cb8d4415cb9a7f886e6dcab30/libp2p/protobuf/minprotobuf.nim or https://github.com/PMunch/protobuf-nim