dxos / codec-protobuf

GNU Affero General Public License v3.0
2 stars 2 forks source link

Errors with nested types #11

Closed telackey closed 4 years ago

telackey commented 4 years ago

In trying to reorganize some of the Greeting protobufs, I ran into several issues.

The first is that with a definition like this:

message GreetCommand {
  oneof command {
    ...
    SubmitCommand submit = 3;
  }
}

message SubmitCommand {
  bytes secret = 1;
  repeated google.protobuf.Any messages = 2;
}

The codec will fail with the error 'Invalid field: messages'. Debugging a bit, at that point internally type is set to 'GreetCommand' not 'SubmitCommand', and GreetCommand has no field 'messages'.

Next problem, if I strongly type the field like this, and try to encode it directly (not as part of a GreetCommand) I get an error about __typeurl on a non-Any type. (I think switching this away from an error is already in the queue)

message SubmitCommand {
  bytes secret = 1;
  repeated Message messages = 2;
}

But if I comment out that error, after decoding I get an array of empty objects ({}) rather than messages, while if it is an Any it works as expected.

I added tests cases for all three.

To reproduce use:

telackey commented 4 years ago

Quick summary version:

  1. As-is, it does not properly set the nested type.
  2. If I try to encode just a SubmitCommand (no nesting), it gives the __typeurl on non-Any error.
  3. If I take out that error, then it still fails. The array comes through, but the ‘Messages’ are empty objects.
  4. If I switch it back to ‘Any’ then encoding just the SubmitCommand works, but still not when nested in the GreetCommand (see 1).
tinchoz49 commented 4 years ago

@telackey can you try the version beta.8 ?

tinchoz49 commented 4 years ago

and remember doing something like

codec.encode({ __type_url: theType, ...message })

would not work, type_url is only for any types.

If you want to encode a message without the container root message you have to do:

codec.encodeByType({ ... }, theType)
telackey commented 4 years ago

Fixed by beta.9