Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Protobuf oneof not decoding correctly #629

Closed
dzanot opened this issue Jul 16, 2019 · 4 comments
Closed

Protobuf oneof not decoding correctly #629

dzanot opened this issue Jul 16, 2019 · 4 comments

Comments

@dzanot
Copy link
Contributor

dzanot commented Jul 16, 2019

While doing some investigations into #606 I noticed in many cases no matter what you sent to oneof fields mu will interpret it as the first field. For example:

message CoproductRequest {
  oneof coproduct {
    bool boolt = 1;
    int32 intt = 2;
    string stringt = 3;
  }
}

Sending from a mu client intt or an empty stringt, mu server will parse a boolean. A non empty stringt will be decoded properly.

Sending anything but boolt from the grpcurl client (Mu client and grpcurl are encoding the payload differently – another issue) causes the mu server to choke with java.lang.UnsupportedOperationException: Can't read CNil

I have a simple server/client project here https://github.com/dzanot/mu-test/tree/58c628bde0f254e6a6d0f949e004e625d4000335

First you must start the server with sbt server/run then the tests from another sbt session sbt client/test

I intend to keep investigating this as part of resolving #606 is going to be sorting out defaults for oneof fields as well as enums.

@dzanot
Copy link
Contributor Author

dzanot commented Jul 17, 2019

Mu client and grpcurl are encoding the payload differently – another issue

This is due to the way the pbdirect dependency handles serializing coproducts – no matter which value of the coproduct it will always have the same field index. Protobuf oneof has a unique field index for each member. I am not sure the best way to approach a fix here

@dzanot
Copy link
Contributor Author

dzanot commented Jul 18, 2019

So PBDirect explicitly doesn't support oneof per the authors initial blog post on the subject.

I have seen mentioned in other mu issues that there is a shift away from "scala as the source of source of truth". Based on my understanding of what gRPC/IDLs in general are trying to accomplish this makes a lot of sense for mu to me.

As the goal of pbdirect is protobufs without the .proto it is my suggestion we remove the dependency here in favor of a a library that treats a .proto as the source of truth, and maps the messages to scala objects accordingly.

@fedefernandez
Copy link
Contributor

@dzanot I agree with you and it seems pbdirect doesn't perfectly fit in the new scenario of using the idls as a source of truth from the user's perspective. I emphasize the user's perspective because internally, the source of truth for mu are the scala models and protocols.

I guess we need to find something like avrohugger but for proto.

@cb372
Copy link
Member

cb372 commented Jan 17, 2020

This issue should be fixed in the latest version (v0.20.1) of mu-scala. We've largely rewritten the protobuf serialization library and fixed a lot of bugs, as well as adding support for oneof fields, and we've added a suite of tests for compliance with the proto3 spec.

If you have any more problems with protobuf oneof fields, please open a new issue.

@cb372 cb372 closed this as completed Jan 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants