allow use of jsonpb for marshaling about grpc-gateway HOT 13 CLOSED

A lot of the overhead comes from checking to ensure the result is actually compliant with protobuf's defined JSON mapping. It's slower, but it's correct :)

from grpc-gateway.

Using it would be nice, but note that in my benchmarks, jsonpb is
approximately 6x slower at marshaling then the standard library package

On Tuesday, December 29, 2015, Eric Chiang [email protected] wrote:

github.com/golang/protobuf/jsonpb
https://godoc.org/github.com/golang/protobuf/jsonpb is a more compliant
protobuf -> JSON package than encoding/json, and is the only way
github.com/golang/protobuf plans to support JSON encoding in the future.

See this comment
golang/protobuf#44 (comment)

The newly added jsonpb package (see 67cbcad) is the official vehicle for
JSON support...

It would be great to be able to using this package for marshaling. It
might even be wise to switch to it entirely.

—
Reply to this email directly or view it on GitHub
#79.

from grpc-gateway.

Ya, I know :) Ugh. Programming is no fun :)

On Tuesday, December 29, 2015, Eric Chiang [email protected] wrote:

A lot of the overhead comes from checking to ensure the result is actually
compliant with protobuf's defined JSON mapping
https://developers.google.com/protocol-buffers/docs/proto3#json. It's
slower, but it's correct :)

—
Reply to this email directly or view it on GitHub
#79 (comment).

from grpc-gateway.

So, someone needs to build a codegen ala ffjson

On Tue, Dec 29, 2015 at 11:24 PM Peter Edge [email protected]
wrote:

Ya, I know :) Ugh. Programming is no fun :)

On Tuesday, December 29, 2015, Eric Chiang [email protected]
wrote:

A lot of the overhead comes from checking to ensure the result is
actually
compliant with protobuf's defined JSON mapping
https://developers.google.com/protocol-buffers/docs/proto3#json. It's
slower, but it's correct :)

—
Reply to this email directly or view it on GitHub
<#79 (comment)
.

—
Reply to this email directly or view it on GitHub
#79 (comment).

from grpc-gateway.

@peter-edge Thank you for your information. I didn't know that it is that slower.
Then, it sounds better to have an option to keep using encoding/json, although jsonpb is mandatory for users who wants Any, Timestamp types, oneof fields and others.

I am thinking of a mechanism which allows users to register/overwrite marshaler per MIME type.
With this mechanism, jsonpb would be a good default marshaler for application/json but you could keep using encoding/json if you don't need new proto3 types of fields.

from grpc-gateway.

@yugui can you sketch out what sort of approach you were thinking? I may hack on this soon.

from grpc-gateway.

The jsonpb.Marshaler recently changed its default behavior to output camelCase, so not using jsonpb within grpc-gateway is now a compatibility issue. jsonpb users will have to set OrigName: true when creating the Marshaler to work around this, which took me a while to figure out and is less than ideal. I would like to see support land soon.

from grpc-gateway.

@yugui @tmc @hbchai don't want to repeat work, but I'd propose what looks like (based on an overly-simple grep) a very small PR:

For code generation:

--grpc-gateway_out=jsonlib={std,jsonpb}:/path/to/etc/etc...

For library code:

var (
  // DefaultJSONAdapter would wrap stdlib JSON functions
  DefaultJSONAdapter = &defaultJSONAdapter{}
  // JSONPBAdapter would wrap jsonpb
  JSONPBAdapter = &jsonpbAdapter{}
  globalJSONAdapter = DefaultJSONAdapter
)

// not trying too hard here with naming or interface definitions, just trying to show the concept
type JSONAdapter interface {
  Marshal(proto.Message) ([]byte, error)
  Unmarshal([]byte) (proto.Message, error)
}

func SetJSONAdapter(jsonAdapter JSONAdapter) {
  globalJSONAdapter = jsonAdapter
}

func jsonMarshal(message proto.Message) ([]byte, error) {
  return globalJSONAdapter.Marshal(message)
}

func jsonUnmarshal(data []byte) (proto.Message, error) {
  return globalJSONAdapter.Unmarshal(data)
}

from grpc-gateway.

@peter-edge It might make more sense for the choice of library to be a new option to the generated RegisterXYZHandler function.

from grpc-gateway.

Ya actually that might be better :)

from grpc-gateway.

this has implications for swagger specs doesn't it? Should this be described via a custom annotation?

from grpc-gateway.

Is this completed? Can it be closed?

from grpc-gateway.

Yep, see #144.

from grpc-gateway.