add runtime.WithUnmarshalerOption

[AyushG3112]

This adds runtime.WithUnmarshalOption to allow use of a custom Unmarshaller.

• Adds runtime.WithUnmarshalOption
• Split runtime.Marshaler into runtime.Marshaler and runtime.Unmarshaler
• Make methods of runtime.Marshaler and runtime.Unmarshaler accept context.Context

Related: #1226

[johanbrandhorst]

This looks fantastic, thank you Ayush for starting this. Just a few grammatical thoughts, but this is basically exactly what I had in mind. Very impressed!

[johanbrandhorst]

Please rebase on v2

[googlebot]

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

[AyushG3112]

@johanbrandhorst Done.

[codecov-io]

Codecov Report

Merging #1228 into v2 will increase coverage by 0.13%.
The diff coverage is 74.07%.

@@            Coverage Diff             @@
##               v2    #1228      +/-   ##
==========================================
+ Coverage   53.99%   54.13%   +0.13%     
==========================================
  Files          42       42              
  Lines        4369     4391      +22     
==========================================
+ Hits         2359     2377      +18     
- Misses       1752     1754       +2     
- Partials      258      260       +2     

Impacted Files	Coverage Δ
...c-gen-grpc-gateway/internal/gengateway/template.go	80.58% <ø> (ø)
runtime/marshaler.go	100.00% <ø> (ø)
runtime/proto_errors.go	34.88% <0.00%> (ø)
runtime/mux.go	55.90% <28.57%> (+0.34%)	⬆️
runtime/handler.go	47.50% <60.00%> (ø)
runtime/marshal_jsonpb.go	68.99% <81.81%> (ø)
runtime/marshaler_registry.go	83.67% <85.18%> (-2.05%)	⬇️
runtime/errors.go	50.00% <100.00%> (ø)
runtime/marshal_httpbodyproto.go	76.92% <100.00%> (ø)

---

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 a13dd49...2250922. Read the comment docs.

[AyushG3112]

@johanbrandhorst this will need your consent because you're the co-author of fef9392 and 35d0c34

[johanbrandhorst]

I've had another think about this, and I wonder if we need the two separate interfaces? What use case are we trying to support by allowing the user to configure the marshaler and unmarshaler separately? Would it be possible to keep it as a single interface and accomplish the goals we want?

[AyushG3112]

If someone only wants to implement a custom unmarshaler, a single interface will force them to implement stubs for the Encode and Marshal methods too. Multiple interfaces prevent that, and the user can pass the same implementation for both interfaces if they want.

[johanbrandhorst]

If someone only wants to implement a custom unmarshaler, a single interface will force them to implement stubs for the Encode and Marshal methods too. Multiple interfaces prevent that, and the user can pass the same implementation for both interfaces if they want.

That is true, but in the grand scheme of things, I think this are simpler with a single interface, and in the use case where a user only wants to implement the unmarshaling part, they can simply embed the default marshaler. I think most cases will be about changing the marshaler, and the user will modify embed the default marshaller to avoid modifying the unmarshalling.

It also makes me wonder if we already had this capability, why did we add DisallowUnknownFields? I'm looking at https://github.com/grpc-ecosystem/grpc-gateway/blob/master/runtime/marshaler_registry.go and it looks to me like all one has to do to get custom unmarshaling behaviour is to define your own marshaler and embed the default marshaler. Am I missing something?

[johanbrandhorst]

I believe we concluded that this change was not necessary in the end, so I'm closing it. Looking forward to future contributions from you Ayush :).