Grpc-gateway: Support emitting default values in JSON

[philipithomas]

Right now, github.com/golang/protobuf hardcodes omitempty, and the gateway does not support a way to enable the EmitDefaults option in

This produces unintended behavior - like all false booleans or zero integers being omitted from JSON apis. I don't think it's wise to assume that API consumers know that the absence of a response parameter means that it is a specific empty value.

I'm opening this issue for tracking - I'm not sure what the best solution is at this time though.

[philipithomas]

I was able to implement this using the WithMarshalerOption like this:

gwmux := runtime.NewServeMux(runtime.WithMarshalerOption(runtime.MIMEWildcard, &runtime.JSONPb{OrigName: true, EmitDefaults: true}))

I think this behavior is a hurdle and unexpected behavior for new users of this system.

I propose one of two changes:

1) Explicit documentation in the README of this behavior, including documentation for how to override it

2) Changing the default marshaler to include EmitDefaults.

What is opinion here?

[achew22]

I think I'm comfortable with your first suggested solution. Let's not do anything without getting thoughts from @yugui. What do you think?

[philipithomas]

@yugui Wanted to follow up on this - what are your thoughts?

[yugui]

Sorry for my late reply.
Although it breaks compatibility, I still prefer the option (2) because it is more commonly used in REST APIs and we still keep a way to disable EmitDefaults.
I'm happy to review such a PR.

[jinleileiking]

Not Merged?

[jinleileiking]

@philipithomas

gwmux := runtime.NewServeMux(runtime.WithMarshalerOption(runtime.MIMEWildcard, &runtime.JSONPb{OrigName: true, EmitDefaults: true}))

very, very useful!

[achew22]

This is now fixed

[dhrp]

@achew22, @philipithomas I believe somehow @achew22 's commit for EmitEmpty has dropped out.

I was trying it and it doesn't work. I you look at the code changes in https://github.com/grpc-ecosystem/grpc-gateway/pull/145/files you'll notice there is no mention of 'Emit' anywhere. So perhaps it got lost rebasing or something?

[ornithocoder]

Hi there! Any progress on this one? I see the issue closed but #242 isn't merged yet. My team would love to see this one merged to simplify our interfaces and reduce boilerplate code on clients. Thank you!

[achew22]

Ah, I see what happened here. I was mistaken to close this -- my bad. Unfortunately there hasn't been progress.

@ornithocoder, if you were to extend the work and fix the tests I would be more than happy to merge it as soon as it goes green. It's just a little bit of test cleanup required.

[ornithocoder]

Hi @philipithomas / @achew22, here's part of the patch for the tests. It doesn't fix all the tests, tho. client_test.go and integration_test.go still have to be fixed.

diff --git a/examples/browser/a_bit_of_everything_service.spec.js b/examples/browser/a_bit_of_everything_service.spec.js
index edcbebe..f99c867 100644
--- a/examples/browser/a_bit_of_everything_service.spec.js
+++ b/examples/browser/a_bit_of_everything_service.spec.js
@@ -34,6 +34,15 @@ describe('ABitOfEverythingService', function() {
       sint32_value: 2147483647,
       sint64_value: "4611686018427387903",
       nonConventionalNameValue: "camelCase",
+      single_nested: null,
+      nested: [  ],
+      enum_value: "ZERO",
+      repeated_string_value: [  ],
+      map_value: Object({  }),
+      mapped_string_value: Object({  }),
+      mapped_nested_value: Object({  }),
+      timestamp_value: null,
+      repeated_enum_value: [  ],
     };

     beforeEach(function(done) {
@@ -72,10 +81,9 @@ describe('ABitOfEverythingService', function() {
       sint32_value: 2147483647,
       sint64_value: "4611686018427387903",
       nonConventionalNameValue: "camelCase",
-
       nested: [
-       { name: "bar", amount: 10 },
-       { name: "baz", amount: 20 },
+       { name: "bar", amount: 10, ok: 'FALSE' },
+       { name: "baz", amount: 20, ok: 'FALSE' },
       ],
       repeated_string_value: ["a", "b", "c"],
       oneof_string: "x",
@@ -83,9 +91,13 @@ describe('ABitOfEverythingService', function() {
       map_value: { a: 1, b: 2 },
       mapped_string_value: { a: "x", b: "y" },
       mapped_nested_value: {
-        a: { name: "x", amount: 1 },
-        b: { name: "y", amount: 2 },
+        a: { name: "x", amount: 1, ok: 'FALSE' },
+        b: { name: "y", amount: 2, ok: 'FALSE' },
       },
+      single_nested: null,
+      enum_value: "ZERO",
+      timestamp_value: null,
+      repeated_enum_value: [  ],
     };

     beforeEach(function(done) {
diff --git a/examples/browser/echo_service.spec.js b/examples/browser/echo_service.spec.js
index 97888c3..eca49f9 100644
--- a/examples/browser/echo_service.spec.js
+++ b/examples/browser/echo_service.spec.js
@@ -21,7 +21,7 @@ describe('EchoService', function() {
           {id: "foo"},
           {responseContentType: "application/json"}
       ).then(function(resp) {
-        expect(resp.obj).toEqual({id: "foo"});
+        expect(resp.obj).toEqual({id: "foo", num: '0'});
       }).catch(function(err) {
         done.fail(err);
       }).then(done);
@@ -34,7 +34,7 @@ describe('EchoService', function() {
           {body: {id: "foo"}},
           {responseContentType: "application/json"}
       ).then(function(resp) {
-        expect(resp.obj).toEqual({id: "foo"});
+        expect(resp.obj).toEqual({id: "foo", num: '0'});
       }).catch(function(err) {
         done.fail(err);
       }).then(done);

[johanbrandhorst]

This is part of the v2 work in #546.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[johanbrandhorst]

We want to support this in v2

[johanbrandhorst]

This was merged with #1377