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

jsonpb: bring back old behaviour of handling nulls and JSONPBUnmarshaler #1301

Closed
krzysztofdrys opened this issue Mar 22, 2021 · 4 comments
Closed

Comments

@krzysztofdrys
Copy link
Contributor

What version of protobuf and what language are you using?
go version go1.16.2 darwin/amd64
v1.5.1

What did you do?
We are using a field with a message, which implements jsonpb. JSONPBUnmarshaler to implement NullValue like behaviour for our own messages. This is very similar to the old behaviour of TestUnmarshalNullWithJSONPBUnmarshaler (pre: cc376d7)

What did you expect to see?
When I receive null json value in a nested field, which implements jsonpb. JSONPBUnmarshaler, I expect UnmarshalJSONPB to be called.

I other words, I expect test TestUnmarshalNullWithJSONPBUnmarshaler in form pre cc376d7 to pass.

What did you see instead?

nil is assigned to this field.

I am aware that some breaking changes in similar area have been announced: https://github.com/golang/protobuf/releases#v1.4-nil-values But from what I understand, these release notes are about changes in handling nil go structures, whereas my change is about handling null json elements.

I created a pull request which resolves this issue #1300

@dsnet
Copy link
Member

dsnet commented Mar 24, 2021

The github.com/golang/protobuf module is deprecated. We only accept changes to documentation, bugs fixes with justifiable impact, or fixes for a regression. Since this is fixing a regression, it seem appropriate to restore this functionality.

@dsnet
Copy link
Member

dsnet commented Mar 24, 2021

#1300 has been merged. Thanks for the report and fix.

@dsnet dsnet closed this as completed Mar 24, 2021
@krzysztofdrys
Copy link
Contributor Author

Thanks @dsnet for guiding me trough the process of fixing this.

Is there any chance that this will be released soon?

@dsnet
Copy link
Member

dsnet commented Mar 29, 2021

v1.5.2 is tagged.

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

2 participants