feat: Add numeric enum support. #1743

blakeli0 · 2022-07-23T03:24:46Z

Add numeric enum support based on go/actools-numeric-enum-impl.
Add a new overloaded toBody method that support serializing request object to Json with numeric enums.

blakeli0 · 2022-07-23T03:28:34Z

gax-httpjson/src/main/java/com/google/api/gax/httpjson/ProtoMessageResponseParser.java

@@ -79,7 +79,7 @@ public ResponseT parse(Reader httpContent, TypeRegistry registry) {
  /* {@inheritDoc} */
  @Override
  public String serialize(ResponseT response) {
-    return ProtoRestSerializer.create(defaultRegistry).toJson(response);
+    return ProtoRestSerializer.create(defaultRegistry).toJson(response, false);


@vam-google I couldn't find any usage of this method, is this method only used by tests? It also feels strange to me that a response parser need to serialize an object to String.

Yes, I believe it is used only by tests, but that also counts as "usage", right? I think this "serialize" interface thing predates my time on rest transport, so I just had to keep it there to not break things.

Cool, I think passing false is appropriate then.

However, we should probably add some comments to indicate this public method is only used in tests to set up things? Or we can make it better by using a different way to serialize object here, so that we can remove this test only method?

gax-httpjson/src/main/java/com/google/api/gax/httpjson/ProtoRestSerializer.java

vam-google

Please make it non-breaking. BetaApi is mainly to convey a message that API is not stable yet and may change only if there is no better way. Here it seems making it non-breaking is simple and cheap, so it seems like abetter approach than taking a breaking change.

gax-httpjson/src/main/java/com/google/api/gax/httpjson/ProtoRestSerializer.java

vam-google

LGTM, but lets resolve the field name question before submitting

vam-google · 2022-07-29T16:28:12Z

gax-httpjson/src/main/java/com/google/api/gax/httpjson/ProtoMessageResponseParser.java

@@ -79,7 +79,7 @@ public ResponseT parse(Reader httpContent, TypeRegistry registry) {
  /* {@inheritDoc} */
  @Override
  public String serialize(ResponseT response) {
-    return ProtoRestSerializer.create(defaultRegistry).toJson(response);
+    return ProtoRestSerializer.create(defaultRegistry).toJson(response, false);


Yes, I believe it is used only by tests, but that also counts as "usage", right? I think this "serialize" interface thing predates my time on rest transport, so I just had to keep it there to not break things.

gax-httpjson/src/main/java/com/google/api/gax/httpjson/ProtoRestSerializer.java

vam-google

LGTM sinc i'll be OOO, but please resolve the field name quesiton first.

sonarcloud · 2022-08-22T22:25:48Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

88.9% Coverage
0.0% Duplication

blakeli0 · 2022-08-22T22:50:12Z

LGTM sinc i'll be OOO, but please resolve the field name quesiton first.

Added the fieldName argument to the overloaded method.

🤖 I have created a release *beep* *boop* --- ## [2.19.0](v2.18.7...v2.19.0) (2022-08-22) ### Features * Add numeric enum support. ([#1743](#1743)) ([3f7628e](3f7628e)) ### Bug Fixes * **deps:** update dependency com.google.auth:google-auth-library-credentials to v1.10.0 ([#1768](#1768)) ([3f2188d](3f2188d)) * **deps:** update dependency com.google.auth:google-auth-library-credentials to v1.9.0 ([#1765](#1765)) ([103db3c](103db3c)) * **deps:** update dependency com.google.auth:google-auth-library-oauth2-http to v1.10.0 ([#1769](#1769)) ([0b1eb92](0b1eb92)) * **deps:** update dependency com.google.auth:google-auth-library-oauth2-http to v1.9.0 ([#1766](#1766)) ([2677f07](2677f07)) * **deps:** update dependency com.google.code.gson:gson to v2.9.1 ([#1757](#1757)) ([ea2a075](ea2a075)) * **deps:** update dependency com.google.protobuf:protobuf-bom to v3.21.5 ([#1772](#1772)) ([d7a48d1](d7a48d1)) * **deps:** update dependency io.grpc:grpc-bom to v1.48.1 ([#1763](#1763)) ([e5e4232](e5e4232)) * **deps:** update dependency org.graalvm.sdk:graal-sdk to v22.2.0 ([#1740](#1740)) ([ded44a6](ded44a6)) * **deps:** update dependency org.mockito:mockito-core to v4.7.0 ([#1774](#1774)) ([29678c8](29678c8)) * **deps:** update dependency org.threeten:threetenbp to v1.6.1 ([#1773](#1773)) ([d2c84e6](d2c84e6)) * **test:** testThrottlingBlocking flakyness fix ([#1775](#1775)) ([e69393c](e69393c)) ### Documentation * explaining UNIX environment is required ([#1760](#1760)) ([1d31e90](1d31e90)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

🤖 I have created a release *beep* *boop* --- ## [2.19.0](googleapis/gax-java@v2.18.7...v2.19.0) (2022-08-22) ### Features * Add numeric enum support. ([#1743](googleapis/gax-java#1743)) ([fa87970](googleapis/gax-java@fa87970)) ### Bug Fixes * **deps:** update dependency com.google.auth:google-auth-library-credentials to v1.10.0 ([#1768](googleapis/gax-java#1768)) ([6008b0d](googleapis/gax-java@6008b0d)) * **deps:** update dependency com.google.auth:google-auth-library-credentials to v1.9.0 ([#1765](googleapis/gax-java#1765)) ([ba597b4](googleapis/gax-java@ba597b4)) * **deps:** update dependency com.google.auth:google-auth-library-oauth2-http to v1.10.0 ([#1769](googleapis/gax-java#1769)) ([db37c42](googleapis/gax-java@db37c42)) * **deps:** update dependency com.google.auth:google-auth-library-oauth2-http to v1.9.0 ([#1766](googleapis/gax-java#1766)) ([fe181b0](googleapis/gax-java@fe181b0)) * **deps:** update dependency com.google.code.gson:gson to v2.9.1 ([#1757](googleapis/gax-java#1757)) ([36a0136](googleapis/gax-java@36a0136)) * **deps:** update dependency com.google.protobuf:protobuf-bom to v3.21.5 ([#1772](googleapis/gax-java#1772)) ([c9a2a56](googleapis/gax-java@c9a2a56)) * **deps:** update dependency io.grpc:grpc-bom to v1.48.1 ([#1763](googleapis/gax-java#1763)) ([5535101](googleapis/gax-java@5535101)) * **deps:** update dependency org.graalvm.sdk:graal-sdk to v22.2.0 ([#1740](googleapis/gax-java#1740)) ([77f66d3](googleapis/gax-java@77f66d3)) * **deps:** update dependency org.mockito:mockito-core to v4.7.0 ([#1774](googleapis/gax-java#1774)) ([e192e0b](googleapis/gax-java@e192e0b)) * **deps:** update dependency org.threeten:threetenbp to v1.6.1 ([#1773](googleapis/gax-java#1773)) ([b9e4767](googleapis/gax-java@b9e4767)) * **test:** testThrottlingBlocking flakyness fix ([#1775](googleapis/gax-java#1775)) ([9aef46a](googleapis/gax-java@9aef46a)) ### Documentation * explaining UNIX environment is required ([#1760](googleapis/gax-java#1760)) ([1555924](googleapis/gax-java@1555924)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

feat: Add numeric enum support.

f2ad039

blakeli0 requested review from a team as code owners July 23, 2022 03:24

blakeli0 commented Jul 23, 2022

View reviewed changes

blakeli0 requested review from vam-google and meltsufin July 23, 2022 03:29

blakeli0 commented Jul 25, 2022

View reviewed changes

gax-httpjson/src/main/java/com/google/api/gax/httpjson/ProtoRestSerializer.java Show resolved Hide resolved

meltsufin reviewed Jul 25, 2022

View reviewed changes

gax-httpjson/src/main/java/com/google/api/gax/httpjson/ProtoRestSerializer.java Outdated Show resolved Hide resolved

blakeli0 requested a review from vchudnov-g July 27, 2022 16:03

meltsufin approved these changes Jul 28, 2022

View reviewed changes

vam-google suggested changes Jul 29, 2022

View reviewed changes

gax-httpjson/src/main/java/com/google/api/gax/httpjson/ProtoRestSerializer.java Outdated Show resolved Hide resolved

fix: Add back the original method to prevent breaking changes.

59974c8

blakeli0 requested a review from vam-google July 29, 2022 15:08

vam-google reviewed Jul 29, 2022

View reviewed changes

vam-google approved these changes Aug 5, 2022

View reviewed changes

blakeli0 added 2 commits August 22, 2022 16:54

Merge branch 'main' into support_numeric_enum

69aa8bb

feat: Add fieldName argument back.

a4724cd

blakeli0 merged commit 3f7628e into main Aug 22, 2022

blakeli0 deleted the support_numeric_enum branch August 22, 2022 22:50

release-please bot mentioned this pull request Aug 22, 2022

chore(main): release 2.19.0 #1776

Merged

blakeli0 mentioned this pull request Aug 23, 2022

feat: Add support for rest numeric enums. googleapis/sdk-platform-java#1020

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add numeric enum support. #1743

feat: Add numeric enum support. #1743

blakeli0 commented Jul 23, 2022 •

edited

Loading

blakeli0 Jul 23, 2022

vam-google Jul 29, 2022

blakeli0 Jul 29, 2022

blakeli0 Jul 29, 2022

vam-google left a comment

vam-google left a comment

vam-google Jul 29, 2022

vam-google left a comment

sonarcloud bot commented Aug 22, 2022

blakeli0 commented Aug 22, 2022

feat: Add numeric enum support. #1743

feat: Add numeric enum support. #1743

Conversation

blakeli0 commented Jul 23, 2022 • edited Loading

blakeli0 Jul 23, 2022

Choose a reason for hiding this comment

vam-google Jul 29, 2022

Choose a reason for hiding this comment

blakeli0 Jul 29, 2022

Choose a reason for hiding this comment

blakeli0 Jul 29, 2022

Choose a reason for hiding this comment

vam-google left a comment

Choose a reason for hiding this comment

vam-google left a comment

Choose a reason for hiding this comment

vam-google Jul 29, 2022

Choose a reason for hiding this comment

vam-google left a comment

Choose a reason for hiding this comment

sonarcloud bot commented Aug 22, 2022

blakeli0 commented Aug 22, 2022

blakeli0 commented Jul 23, 2022 •

edited

Loading