Support emitting default values in JSON #233

philipithomas · 2016-10-12T22:45:17Z

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.

The text was updated successfully, but these errors were encountered:

philipithomas · 2016-10-12T23:07:14Z

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:

Explicit documentation in the README of this behavior, including documentation for how to override it
Changing the default marshaler to include EmitDefaults.

What is opinion here?

achew22 · 2016-10-13T02:02:51Z

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 · 2016-10-19T06:52:36Z

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

yugui · 2016-10-19T07:23:26Z

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 · 2017-07-12T12:59:03Z

Not Merged?

jinleileiking · 2017-07-12T13:04:32Z

@philipithomas

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

very, very useful!

achew22 · 2017-12-14T08:10:51Z

This is now fixed

dhrp · 2017-12-16T10:08:17Z

@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 · 2017-12-22T10:36:09Z

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 · 2017-12-22T23:04:36Z

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 · 2018-01-07T07:47:38Z

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);

One of the recurring themes of this project has been trouble around the default marshaller. This change modifies it to be more what people expect when they first start the project. 1. It emits the proto3 json style version of field names instead of the field name as it appeared in the .proto file. 2. It emits zero values for fields. This means that if you have a field that is unset it will now have a value unlike before. Fixes: grpc-ecosystem#540, grpc-ecosystem#375, grpc-ecosystem#254, grpc-ecosystem#233

One of the recurring themes of this project has been trouble around the default marshaller. This change modifies it to be more what people expect when they first start the project. 1. It emits the proto3 json style version of field names instead of the field name as it appeared in the .proto file. 2. It emits zero values for fields. This means that if you have a field that is unset it will now have a value unlike before. Upgrade to swagger-codegen 2.4.0 Also fix a regex-o in .travis.yml. + needed to be escaped. Fixes: grpc-ecosystem#540, grpc-ecosystem#375, grpc-ecosystem#254, grpc-ecosystem#233

In order to make APIs more restful, require the marshaler to emit default values. By default, it omits empty types. This causes unexpected REST behavior by omitting all false values or zero-valued integers. Closes grpc-ecosystem#233

In order to make APIs more restful, require the marshaler to emit default values. By default, it omits empty types. This causes unexpected REST behavior by omitting all false values or zero-valued integers. If you like to override its behavior back, use the WithMarshalerOption: ```go gwmux := runtime.NewServeMux(runtime.WithMarshalerOption(runtime.MIMEWildcard, &runtime.JSONPb{OrigName: true, EmitDefaults: false})) ``` Closes grpc-ecosystem#233

johanbrandhorst · 2018-09-09T14:53:05Z

This is part of the v2 work in #546.

stale · 2019-09-09T17:55:58Z

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 · 2020-04-20T08:26:03Z

We want to support this in v2

Fixes #233

johanbrandhorst · 2020-05-23T20:52:15Z

This was merged with #1377

seanlaff · 2024-03-11T14:59:47Z

Would it make sense to mark all fields in the generated swagger required: true when this flag is enabled? As of now the swagger shows every field in a response as optional, which means any code (eg typescript) generated from the swagger requires null-checking fields that can't ever be null.

Seems like it may be challenging to do given people use the same message in both request/response, in which case the swagger generator would need to generate 2 schemas (unless there is a way for an Operation to say its response fields are all required, but I am doubtful that exists).

johanbrandhorst · 2024-03-13T18:44:46Z

In proto3 all fields are required by default. Nullability is explicit using the optional keyword. I don't think the semantics translate cleanly to OpenAPI unfortunately (we've gone back and forth on this) so I don't want to make any behavioural changes at this point.

philipithomas mentioned this issue Oct 12, 2016

Default omitemptycauses unintended behavior golang/protobuf#238

Closed

philipithomas mentioned this issue Oct 20, 2016

Emit default values in marshalled JSON #242

Closed

achew22 closed this as completed Dec 14, 2017

achew22 reopened this Dec 22, 2017

ornithocoder mentioned this issue Dec 23, 2017

Emit default values in marshalled JSON (with tests) #508

Closed

flw-cn mentioned this issue Apr 28, 2018

Emit default values in marshalled JSON (with tests) #628

Closed

johanbrandhorst added breaking change Will require a major version increment v2 labels Sep 9, 2018

henryguillen mentioned this issue Oct 5, 2018

[PCFF-1975] Emit "false" and "0" for bools and ints where appropriate. namely/docker-protoc#94

Merged

cfromknecht mentioned this issue Oct 25, 2018

REST API responses missing falsey values lightningnetwork/lnd#2087

Closed

AlekSi mentioned this issue Sep 2, 2019

Expose jsonpb marshaling options via params #676

Closed

stale bot added the wontfix label Sep 9, 2019

stale bot closed this as completed Sep 16, 2019

wish-master mentioned this issue Mar 25, 2020

Add emitting default values support in JSON utrack/clay#84

Open

johanbrandhorst reopened this Apr 20, 2020

stale bot removed the wontfix label Apr 20, 2020

johanbrandhorst added this to the 2.0 milestone Apr 20, 2020

johanbrandhorst added the help wanted label Apr 20, 2020

johanbrandhorst mentioned this issue May 22, 2020

v2 release planning #1223

Closed

13 tasks

johanbrandhorst added a commit that referenced this issue May 22, 2020

runtime: make default marshaler emit default values

172e2e2

Fixes #233

johanbrandhorst mentioned this issue May 22, 2020

runtime: make default marshaler emit default values #1377

Merged

johanbrandhorst added a commit that referenced this issue May 22, 2020

runtime: make default marshaler emit default values

49942f1

Fixes #233

johanbrandhorst closed this as completed May 23, 2020

GerardSmit mentioned this issue Mar 1, 2021

HttpApi doesn't write empty strings grpc/grpc-dotnet#1213

Closed

lbeckman314 mentioned this issue Nov 2, 2023

TESv1.1 Support ohsu-comp-bio/funnel#716

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support emitting default values in JSON #233

Support emitting default values in JSON #233

philipithomas commented Oct 12, 2016

philipithomas commented Oct 12, 2016

achew22 commented Oct 13, 2016

philipithomas commented Oct 19, 2016

yugui commented Oct 19, 2016

jinleileiking commented Jul 12, 2017

jinleileiking commented Jul 12, 2017

achew22 commented Dec 14, 2017

dhrp commented Dec 16, 2017

ornithocoder commented Dec 22, 2017

achew22 commented Dec 22, 2017

ornithocoder commented Jan 7, 2018

johanbrandhorst commented Sep 9, 2018

stale bot commented Sep 9, 2019

johanbrandhorst commented Apr 20, 2020

johanbrandhorst commented May 23, 2020

seanlaff commented Mar 11, 2024

johanbrandhorst commented Mar 13, 2024

Support emitting default values in JSON #233

Support emitting default values in JSON #233

Comments

philipithomas commented Oct 12, 2016

philipithomas commented Oct 12, 2016

achew22 commented Oct 13, 2016

philipithomas commented Oct 19, 2016

yugui commented Oct 19, 2016

jinleileiking commented Jul 12, 2017

jinleileiking commented Jul 12, 2017

achew22 commented Dec 14, 2017

dhrp commented Dec 16, 2017

ornithocoder commented Dec 22, 2017

achew22 commented Dec 22, 2017

ornithocoder commented Jan 7, 2018

johanbrandhorst commented Sep 9, 2018

stale bot commented Sep 9, 2019

johanbrandhorst commented Apr 20, 2020

johanbrandhorst commented May 23, 2020

seanlaff commented Mar 11, 2024

johanbrandhorst commented Mar 13, 2024