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

marshal_jsonpb: add check for slice sub types implementing proto.Message #856

Merged
merged 2 commits into from
Jan 21, 2019

Conversation

abice
Copy link
Contributor

@abice abice commented Jan 20, 2019

I found that the slice of response bodies, even if they implement proto.Message are still being marshalled using the default json marshaller and ignoring the EnumsAsInts flag. I've updated the implementation to handle this case, and pass off each individual object in the slice to the jsonpb marshaller.

Let me know if there's a better way of handling these slice types that I'm missing.

Thanks!

@codecov-io
Copy link

codecov-io commented Jan 20, 2019

Codecov Report

Merging #856 into master will increase coverage by 0.03%.
The diff coverage is 61.9%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #856      +/-   ##
==========================================
+ Coverage   51.63%   51.66%   +0.03%     
==========================================
  Files          39       39              
  Lines        3788     3807      +19     
==========================================
+ Hits         1956     1967      +11     
- Misses       1647     1651       +4     
- Partials      185      189       +4
Impacted Files Coverage Δ
runtime/marshal_jsonpb.go 69.1% <61.9%> (-2.05%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ff4dc68...0a364dd. Read the comment docs.

Copy link
Collaborator

@johanbrandhorst johanbrandhorst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a small comment, looks good overall!

runtime/marshal_jsonpb.go Show resolved Hide resolved
runtime/marshal_jsonpb_test.go Show resolved Hide resolved
Copy link
Collaborator

@johanbrandhorst johanbrandhorst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for correcting me :).

@johanbrandhorst johanbrandhorst merged commit 0353029 into grpc-ecosystem:master Jan 21, 2019
@abice abice deleted the emit_proto_slice branch January 21, 2019 13:21
adasari pushed a commit to adasari/grpc-gateway that referenced this pull request Apr 9, 2020
…age (grpc-ecosystem#856)

* marshal_jsonpb: add check for slice sub types implementing proto.Message

* Added more tests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants