-
Notifications
You must be signed in to change notification settings - Fork 184
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
Unpacking nested "Any message" fails because typeRegistry isn't being passed along #568
Conversation
I think this looks good. Could you create a test revealing this also? |
e491f3a
to
fe29333
Compare
Just pushed a test showing this. Only the one in the "encode" group was failing, but for completeness sake I also added a counterpart test in the "encode" group. So the situation occurs when you have a message with an Any field that contains a message with an Any field. When the nested field is being unpacked in order to get a JSON value out of it, the TypeRegistry became missing. Feel free to rename/reshuffle the created proto messages if you like. I think it might already work with only a single |
fe29333
to
a5acf1f
Compare
Is it possible to re-run the wokflow? I think I've fixed all the issues... |
Any idea when this would be merged? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the delay @BenVercammen. I noticed a few redundant lines in the test proto, if you could remove those I'll merge this.
protoc_plugin/test/protos/google/protobuf/unittest_nested_any.proto
Outdated
Show resolved
Hide resolved
a5acf1f
to
c89778d
Compare
c89778d
to
af01378
Compare
Follow official protobuf style guide: 1. License header (if applicable) 2. File overview 3. Syntax 4. Package 5. Imports (sorted) 6. File options 7. Everything else Internally we want a `package` line in every proto, and it's easier to do add `package` lines to protos than to add them as exceptions. License headers added the old files based on creation date. protos in google/protobuf are not updated as they are copied from protoc. The proto file `unittest_nested_any.proto` was added google#568. Since it's not copied from protoc I moved it to test/protos. Fixes google#692
Follow official protobuf style guide: 1. License header (if applicable) 2. File overview 3. Syntax 4. Package 5. Imports (sorted) 6. File options 7. Everything else Internally we want a `package` line in every proto, and it's easier to do add `package` lines to protos than to add them as exceptions. License headers added the old files based on creation date. protos in google/protobuf are not updated as they are copied from protoc. The proto file `unittest_nested_any.proto` was added #568. Since it's not copied from protoc I moved it to test/protos. Fixes #692
While calling
.toProto3Json()
I kept bumping into the error messageThe type of the Any message (...) is not in the given typeRegistry.
even though I had passed along aTypeRegistry
with the relatedGeneratedMessage
types. After some stepping through the code, I found out that thetypeRegistry
parameter was not passed along while unpacking.I fixed the issue for my use case, but still have to write a test for it, though I'm not sure when I can get around to it. Anyhow, I already wanted to open the PR as to be able to receive feedback on any potential harm I'd be doing with this fix. I'll try to write a test as soon as I can, probably around the weekend.
I'm pretty new to protobuf and even dart in general, so if my terminlogy is off (eg: in the commit message) feel free to correct me :-)