feat: Parser to consume the api-versioning value from proto #2630

zhumin8 · 2024-04-10T16:36:21Z

This is part one to resolve b/330980553, this pr allows read and parse api versioning from proto to service model.

Changes included in this pr:

rename current apiVersion to packageVersion. This packageVersion is parsed from package name and exposed in refactor: expose parsed api short name and version as fields in Service #1075. It was intended to be used in Spring codegen, but not actually used. It is used in generating sample region tags (see details 9ec3531)
exposing apiVersion from Service model.
Parser.java to parse apiVersion from ClientProto.apiVersion to service.

…ion.

lqiu96 · 2024-04-11T20:15:41Z

rename current apiVersion to apiMajorVersion. This apiMajorVersion is parsed from package name and exposed in #1075. It was intended to be used in Spring codegen, but not actually used. It is used in generating sample region tags (see details 9ec3531)

I know you wrote that it's not actually used inside Spring codegen, but for sample region tags. Would this break any functionality for the generated samples in Spring (and/or any other generated samples)? It sounds like it probably won't, but can double check this to confirm?

gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/Service.java

gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java

gapic-generator-java/src/test/java/com/google/api/generator/gapic/model/ServiceTest.java

gapic-generator-java/src/test/proto/echo.proto

lqiu96 · 2024-04-11T20:30:39Z

Thanks @zhumin8! I think it look good! I left a few nits and a few open questions that popped in while I reviewing. I think we can just double check with the team to confirm.

zhumin8 · 2024-04-15T14:50:37Z

rename current apiVersion to apiMajorVersion. This apiMajorVersion is parsed from package name and exposed in #1075. It was intended to be used in Spring codegen, but not actually used. It is used in generating sample region tags (see details 9ec3531)

I know you wrote that it's not actually used inside Spring codegen, but for sample region tags. Would this break any functionality for the generated samples in Spring (and/or any other generated samples)? It sounds like it probably won't, but can double check this to confirm?

For non-Spring samples, it's not breaking anything after the renaming.
For Spring generator, code search confirms apiVersion() is not used, only apiShortName() is used here.

I should also clarify that the change is only on the method names, which are internal, not actual behaviors.

…s correctly parse per service.

lqiu96 · 2024-04-19T18:33:46Z

gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java

+              if (serviceOptions.hasExtension(ClientProto.apiVersion)) {
+                String apiVersion = serviceOptions.getExtension(ClientProto.apiVersion);
+                serviceBuilder.setApiVersion(apiVersion);
+              }


nit:

String apiVersion = null; if (serviceOptions.hasExtension(ClientProto.apiVersion)) { apiVersion = serviceOptions.getExtension(ClientProto.apiVersion); } ... return serviceBuilder .setName(serviceName) .... .setApiVersion(apiVersion)

I believe the Service class can take a null or empty ApiVersion

I had similar concerns at first. But I believe both null and empty are covered. (I'll create additional test cases in ParserTest for it).
Note that If value is not set for serviceBuilder, it is null by default.

gapic-generator-java/src/test/proto/echo.proto

lqiu96

LGTM. I added a few nits if you could resolve before merging.

lqiu96 · 2024-04-19T18:42:05Z

Can you also update the description: rename current apiVersion to apiMajorVersion. I believe it should be rename current apiVersion to packageVersion.

…t this feat.

…geVersion.

sonarcloud · 2024-04-23T19:25:35Z

Quality Gate passed for 'gapic-generator-java-root'

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

sonarcloud · 2024-04-23T19:28:39Z

Quality Gate passed for 'java_showcase_integration_tests'

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

zhumin8 · 2024-04-23T21:39:51Z

@lqiu96 Moved testing to a dedicated new proto as you suggested. Thisapi_version_testing.proto may contain some redundant definitions at this point. I plan to address either as part of the next pr, or in a followup pr.

lqiu96 · 2024-04-23T22:07:49Z

@lqiu96 Moved testing to a dedicated new proto as you suggested. Thisapi_version_testing.proto may contain some redundant definitions at this point. I plan to address either as part of the next pr, or in a followup pr.

Thanks, LGTM.

#2671) This is part 2, following changes in #2630 Changes in this pr: - ApiClientHeaderProvider: add key and token setter for api version. - AbstractServiceStubSettingsClassComposer: add logic to set this header when api-version is present (not null or empty) - Unit and golden tests to reflect above changes. (This includeds a few new golden files created for tests based on the dedicated proto created in #2630) This pr does not include gapic-showcased testing for now. --- manual tested with [gapic-showcase](https://github.com/googleapis/gapic-showcase) v0.33.0 steps: - Use gapic-showcase v0.33.0 (which includes update in 1484) - use `gapic-showcase run -v` which prints the request header - From showcase folder, run `mvn verify -P enable-integration-tests -Dit.test=ITAutoPopulatedFields.java -pl=gapic-showcase` Printed header in manual testing Grpc ``` 2024/04/16 10:26:41 Received Unary Request for Method: /google.showcase.v1beta1.Echo/Echo 2024/04/16 10:26:41 Request headers: 2024/04/16 10:26:41 grpc-accept-encoding: gzip 2024/04/16 10:26:41 content-type: application/grpc 2024/04/16 10:26:41 x-goog-api-version: v1_20240408 2024/04/16 10:26:41 :authority: localhost:7469 2024/04/16 10:26:41 user-agent: grpc-java-netty/1.62.2 2024/04/16 10:26:41 x-goog-api-client: gl-java/17.0.7__Eclipse-Adoptium__Temurin-17.0.7-7 gapic/0.0.1-SNAPSHOT gax/2.46.2-SNAPSHOT grpc/1.62.2 2024/04/16 10:26:41 Request: error:{code:2} request_id:"1e26ee16-ed07-4eeb-be0e-49b769678779" other_request_id:"40883e5d-3b20-4add-854c-9808bd009260" 2024/04/16 10:26:41 Returning Error: rpc error: code = Unknown desc = ``` Httpjson ``` 2024/04/16 10:26:41 Received POST request matching '/v1beta1/echo:echo': "/v1beta1/echo:echo" 2024/04/16 10:26:41 urlPathParams (expect 0, have 0): map[] 2024/04/16 10:26:41 urlRequestHeaders: User-Agent: "Google-HTTP-Java-Client/1.44.1 (gzip)" Content-Type: "application/json; charset=utf-8" Connection: "keep-alive" Accept-Encoding: "gzip" X-Goog-Api-Client: "gl-java/17.0.7__Eclipse-Adoptium__Temurin-17.0.7-7 gapic/0.0.1-SNAPSHOT gax/2.46.2-SNAPSHOT rest/" X-Goog-Api-Version: "v1_20240408" Accept: "text/html, image/gif, image/jpeg, *; q=.2, */*; q=.2" Content-Length: "129" 2024/04/16 10:26:41 request: { "error": { "code": 500, "message": "", "details": [] }, "severity": "UNNECESSARY", "header": "", "otherHeader": "", "requestId": "e690a84b-176d-421e-9e5d-ba752f68fec8", "otherRequestId": "47e0fbde-056a-43ae-bba6-8b6770d861f7" } ```

🤖 I have created a release *beep* *boop* --- <details><summary>2.40.0</summary> ## [2.40.0](v2.39.0...v2.40.0) (2024-05-02) ### Features * [common-protos] add `Weight` to common types for Shopping APIs to be used for accounts bundle ([#2699](#2699)) ([5bb9770](5bb9770)) * add a CLI tool to validate generation configuration ([#2691](#2691)) ([f2ce524](f2ce524)) * Parser to consume the api-versioning value from proto ([#2630](#2630)) ([40711fd](40711fd)) * Update Gapic generator and Gax to emit api-versioning via header ([#2671](#2671)) ([e63d1b4](e63d1b4)) ### Bug Fixes * change folder prefix for adding headers ([#2688](#2688)) ([4e92be8](4e92be8)) * Log HttpJson's async thread pool core size ([#2697](#2697)) ([34b4bc3](34b4bc3)) * replace `cfg = "host"` with `cfg = "exec"` ([#2637](#2637)) ([6d673f3](6d673f3)) * Return resolved endpoint from StubSettings' Builder ([#2715](#2715)) ([32c9995](32c9995)) ### Dependencies * Make opentelemetry-api an optional dependency. ([#2681](#2681)) ([3967a19](3967a19)) * update dependency absl-py to v2.1.0 ([#2659](#2659)) ([cae6d79](cae6d79)) * update dependency gitpython to v3.1.43 ([#2656](#2656)) ([208bef4](208bef4)) * update dependency lxml to v5.2.1 ([#2661](#2661)) ([b95ad49](b95ad49)) * update dependency net.bytebuddy:byte-buddy to v1.14.14 ([#2703](#2703)) ([87069bc](87069bc)) * update dependency typing to v3.10.0.0 ([#2663](#2663)) ([7fb5653](7fb5653)) * update gapic-showcase to v0.33.0 ([#2653](#2653)) ([0a71cbf](0a71cbf)) ### Documentation * Add contributing guidelines to PR and issue templates ([#2682](#2682)) ([42526dc](42526dc)) </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>

This pr updates gapic-showcase version to 0.35.0 and adds showcase tests to verify behavior of api version headers being emitted (changes in #2630 and #2671).

This is part one to resolve b/330980553, this pr allows read and parse api versioning from proto to service model. Changes included in this pr: - rename current apiVersion to packageVersion. This packageVersion is parsed from package name and exposed in #1075. It was intended to be used in Spring codegen, but not actually used. It is used in generating sample region tags (see details [9ec3531](9ec3531)) - exposing apiVersion from Service model. - Parser.java to parse apiVersion from `ClientProto.apiVersion` to service.

#2671) This is part 2, following changes in #2630 Changes in this pr: - ApiClientHeaderProvider: add key and token setter for api version. - AbstractServiceStubSettingsClassComposer: add logic to set this header when api-version is present (not null or empty) - Unit and golden tests to reflect above changes. (This includeds a few new golden files created for tests based on the dedicated proto created in #2630) This pr does not include gapic-showcased testing for now. --- manual tested with [gapic-showcase](https://github.com/googleapis/gapic-showcase) v0.33.0 steps: - Use gapic-showcase v0.33.0 (which includes update in 1484) - use `gapic-showcase run -v` which prints the request header - From showcase folder, run `mvn verify -P enable-integration-tests -Dit.test=ITAutoPopulatedFields.java -pl=gapic-showcase` Printed header in manual testing Grpc ``` 2024/04/16 10:26:41 Received Unary Request for Method: /google.showcase.v1beta1.Echo/Echo 2024/04/16 10:26:41 Request headers: 2024/04/16 10:26:41 grpc-accept-encoding: gzip 2024/04/16 10:26:41 content-type: application/grpc 2024/04/16 10:26:41 x-goog-api-version: v1_20240408 2024/04/16 10:26:41 :authority: localhost:7469 2024/04/16 10:26:41 user-agent: grpc-java-netty/1.62.2 2024/04/16 10:26:41 x-goog-api-client: gl-java/17.0.7__Eclipse-Adoptium__Temurin-17.0.7-7 gapic/0.0.1-SNAPSHOT gax/2.46.2-SNAPSHOT grpc/1.62.2 2024/04/16 10:26:41 Request: error:{code:2} request_id:"1e26ee16-ed07-4eeb-be0e-49b769678779" other_request_id:"40883e5d-3b20-4add-854c-9808bd009260" 2024/04/16 10:26:41 Returning Error: rpc error: code = Unknown desc = ``` Httpjson ``` 2024/04/16 10:26:41 Received POST request matching '/v1beta1/echo:echo': "/v1beta1/echo:echo" 2024/04/16 10:26:41 urlPathParams (expect 0, have 0): map[] 2024/04/16 10:26:41 urlRequestHeaders: User-Agent: "Google-HTTP-Java-Client/1.44.1 (gzip)" Content-Type: "application/json; charset=utf-8" Connection: "keep-alive" Accept-Encoding: "gzip" X-Goog-Api-Client: "gl-java/17.0.7__Eclipse-Adoptium__Temurin-17.0.7-7 gapic/0.0.1-SNAPSHOT gax/2.46.2-SNAPSHOT rest/" X-Goog-Api-Version: "v1_20240408" Accept: "text/html, image/gif, image/jpeg, *; q=.2, */*; q=.2" Content-Length: "129" 2024/04/16 10:26:41 request: { "error": { "code": 500, "message": "", "details": [] }, "severity": "UNNECESSARY", "header": "", "otherHeader": "", "requestId": "e690a84b-176d-421e-9e5d-ba752f68fec8", "otherRequestId": "47e0fbde-056a-43ae-bba6-8b6770d861f7" } ```

🤖 I have created a release *beep* *boop* --- <details><summary>2.40.0</summary> ## [2.40.0](v2.39.0...v2.40.0) (2024-05-02) ### Features * [common-protos] add `Weight` to common types for Shopping APIs to be used for accounts bundle ([#2699](#2699)) ([5bb9770](5bb9770)) * add a CLI tool to validate generation configuration ([#2691](#2691)) ([f2ce524](f2ce524)) * Parser to consume the api-versioning value from proto ([#2630](#2630)) ([40711fd](40711fd)) * Update Gapic generator and Gax to emit api-versioning via header ([#2671](#2671)) ([e63d1b4](e63d1b4)) ### Bug Fixes * change folder prefix for adding headers ([#2688](#2688)) ([4e92be8](4e92be8)) * Log HttpJson's async thread pool core size ([#2697](#2697)) ([34b4bc3](34b4bc3)) * replace `cfg = "host"` with `cfg = "exec"` ([#2637](#2637)) ([6d673f3](6d673f3)) * Return resolved endpoint from StubSettings' Builder ([#2715](#2715)) ([32c9995](32c9995)) ### Dependencies * Make opentelemetry-api an optional dependency. ([#2681](#2681)) ([3967a19](3967a19)) * update dependency absl-py to v2.1.0 ([#2659](#2659)) ([cae6d79](cae6d79)) * update dependency gitpython to v3.1.43 ([#2656](#2656)) ([208bef4](208bef4)) * update dependency lxml to v5.2.1 ([#2661](#2661)) ([b95ad49](b95ad49)) * update dependency net.bytebuddy:byte-buddy to v1.14.14 ([#2703](#2703)) ([87069bc](87069bc)) * update dependency typing to v3.10.0.0 ([#2663](#2663)) ([7fb5653](7fb5653)) * update gapic-showcase to v0.33.0 ([#2653](#2653)) ([0a71cbf](0a71cbf)) ### Documentation * Add contributing guidelines to PR and issue templates ([#2682](#2682)) ([42526dc](42526dc)) </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>

This pr updates gapic-showcase version to 0.35.0 and adds showcase tests to verify behavior of api version headers being emitted (changes in #2630 and #2671).

zhumin8 added 4 commits April 8, 2024 16:39

chore: rename current apiVersion to apiMajorVersion.

8772c51

continue: rename apiVersion used in sample regiontags to apiMajorVers…

9ec3531

…ion.

add new apiVersion field to Service model.

3b35cd0

add logic so Parser can handle optional api version.:

23f2ce7

product-auto-label bot added the size: m Pull request size is medium. label Apr 10, 2024

zhumin8 added the owlbot:run Add this label to trigger the Owlbot post processor. label Apr 10, 2024

gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Apr 10, 2024

zhumin8 requested a review from lqiu96 April 11, 2024 19:00

lqiu96 reviewed Apr 11, 2024

View reviewed changes

gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/Service.java Show resolved Hide resolved

lqiu96 reviewed Apr 11, 2024

View reviewed changes

gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java Outdated Show resolved Hide resolved

lqiu96 reviewed Apr 11, 2024

View reviewed changes

gapic-generator-java/src/test/java/com/google/api/generator/gapic/model/ServiceTest.java Show resolved Hide resolved

lqiu96 reviewed Apr 11, 2024

View reviewed changes

gapic-generator-java/src/test/proto/echo.proto Outdated Show resolved Hide resolved

fix: simplify parser logic.

255c6f3

zhumin8 added 4 commits April 17, 2024 16:30

rename old apiVersion to packageVersion to disambiguous.

1ddb284

test: add additional service in echo.proto to test that api-version i…

0e8705d

…s correctly parse per service.

Merge branch 'main' into api-version

8f1a131

fix: checkstyle

f025a3f

zhumin8 added the owlbot:run Add this label to trigger the Owlbot post processor. label Apr 19, 2024

gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Apr 19, 2024

zhumin8 marked this pull request as ready for review April 19, 2024 16:59

zhumin8 requested a review from a team as a code owner April 19, 2024 16:59

zhumin8 requested a review from lqiu96 April 19, 2024 16:59

zhumin8 mentioned this pull request Apr 19, 2024

feat: Update Gapic generator and Gax to emit api-versioning via header #2671

Merged

lqiu96 reviewed Apr 19, 2024

View reviewed changes

gapic-generator-java/src/test/proto/echo.proto Outdated Show resolved Hide resolved

lqiu96 approved these changes Apr 19, 2024

View reviewed changes

zhumin8 added 3 commits April 23, 2024 14:57

test: revert changes to echo.proto, and create dedicated proto to tes…

1bc4d64

…t this feat.

chore: rename test name and var references of old apiVersion to packa…

0b57b3e

…geVersion.

test: add test case for service when apiVersion is empty.

a28a8e8

product-auto-label bot added size: l Pull request size is large. and removed size: m Pull request size is medium. labels Apr 23, 2024

zhumin8 merged commit 40711fd into main Apr 24, 2024
24 checks passed

zhumin8 deleted the api-version branch April 24, 2024 14:06

release-please bot mentioned this pull request Apr 23, 2024

chore(main): release 2.40.0 #2679

Merged

This was referenced May 6, 2024

feat: add comment to service client to disclose api version. #2687

Open

test: add showcase test for api-version #2737

Merged

zhumin8 added a commit that referenced this pull request May 10, 2024

test: add showcase test for api-version (#2737)

e1e1fb6

This pr updates gapic-showcase version to 0.35.0 and adds showcase tests to verify behavior of api version headers being emitted (changes in #2630 and #2671).

lqiu96 pushed a commit that referenced this pull request May 22, 2024

test: add showcase test for api-version (#2737)

9ddf5f2

This pr updates gapic-showcase version to 0.35.0 and adds showcase tests to verify behavior of api version headers being emitted (changes in #2630 and #2671).

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Parser to consume the api-versioning value from proto #2630

feat: Parser to consume the api-versioning value from proto #2630

zhumin8 commented Apr 10, 2024 •

edited

lqiu96 commented Apr 11, 2024 •

edited

lqiu96 commented Apr 11, 2024

zhumin8 commented Apr 15, 2024 •

edited

lqiu96 Apr 19, 2024

zhumin8 Apr 23, 2024

lqiu96 left a comment

lqiu96 commented Apr 19, 2024

sonarcloud bot commented Apr 23, 2024

sonarcloud bot commented Apr 23, 2024

zhumin8 commented Apr 23, 2024

lqiu96 commented Apr 23, 2024

feat: Parser to consume the api-versioning value from proto #2630

feat: Parser to consume the api-versioning value from proto #2630

Conversation

zhumin8 commented Apr 10, 2024 • edited

lqiu96 commented Apr 11, 2024 • edited

lqiu96 commented Apr 11, 2024

zhumin8 commented Apr 15, 2024 • edited

lqiu96 Apr 19, 2024

Choose a reason for hiding this comment

zhumin8 Apr 23, 2024

Choose a reason for hiding this comment

lqiu96 left a comment

Choose a reason for hiding this comment

lqiu96 commented Apr 19, 2024

sonarcloud bot commented Apr 23, 2024

Quality Gate passed for 'gapic-generator-java-root'

sonarcloud bot commented Apr 23, 2024

Quality Gate passed for 'java_showcase_integration_tests'

zhumin8 commented Apr 23, 2024

lqiu96 commented Apr 23, 2024

zhumin8 commented Apr 10, 2024 •

edited

lqiu96 commented Apr 11, 2024 •

edited

zhumin8 commented Apr 15, 2024 •

edited