refactor: expose parsed api short name and version as fields in Service #1075

emmileaf · 2022-10-31T14:11:36Z

Follow-up to #1066.

This PR exposes parsed apiShortName and apiVersion as fields in Service.java, since the source fields (defaultHost and protoPakkage) are both defined per-service. It also:

Adds these two fields correspondingly to GapicClass so that it can be composed from Service given these two upstream fields, then replaces withDefaultHost (for building GapicClass) added in fix: update sample region tag to parse host instead of proto package #1040 with withApiShortName and withApiVersion. (Composers and tests where this is currently used for sample generation are updated)

This change will enable Spring Codegen (when eventually split out from this repo) to reuse this parsing logic in descriptive comments and metadata. It also moves the parsing logic to earlier in the parse-compose process.

… generation

blakeli0

LGTM. I actually don't know refactor is a conventional commit as most of time we just use chore for things that are not fix or feat, but refactor is definitely a better fit in this case.

alicejli

Thank you for your hard work! Looks good with a minor note on a test (from an error I made, so thank you in advance for addressing!)

To Blake's point - I think refactor: should be okay as a prefix per https://www.conventionalcommits.org/en/v1.0.0/.

alicejli · 2022-10-31T19:52:09Z

src/test/java/com/google/api/generator/gapic/composer/ComposerTest.java

+      assertEquals(
+          "ApiShortName should be Localhost7469",
+          "Localhost7469",
+          sample.regionTag().apiShortName());
      assertEquals("ApiVersion should be V1beta1", "V1Beta1", sample.regionTag().apiVersion());


I realize that this wasn't from your change (it was mine, sorry!) but since you're updating this section, do you mind updating to assertEquals("ApiVersion should be V1Beta1")? thanks!

sonarqubecloud · 2022-10-31T20:41:17Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
1 Code Smell

86.1% Coverage
0.0% Duplication

emmileaf · 2022-10-31T20:48:00Z

Thanks @alicejli and @blakeli0 for the review - I didn’t realize refactor was a valid prefix until checking the docs as well (to look for something other than fix or feat), but glad it fits the purpose and passes the check!

* fix: fixes regionTag breakage * update credentials integration test * removing getPureServiceName function * linter * refactor test * chore(deps): update dependency org.apache.maven.plugins:maven-shade-plugin to v3.4.1 (#1072) [![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [org.apache.maven.plugins:maven-shade-plugin](https://maven.apache.org/plugins/) | `3.4.0` -> `3.4.1` | [![age](https://badges.renovateapi.com/packages/maven/org.apache.maven.plugins:maven-shade-plugin/3.4.1/age-slim)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://badges.renovateapi.com/packages/maven/org.apache.maven.plugins:maven-shade-plugin/3.4.1/adoption-slim)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://badges.renovateapi.com/packages/maven/org.apache.maven.plugins:maven-shade-plugin/3.4.1/compatibility-slim/3.4.0)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://badges.renovateapi.com/packages/maven/org.apache.maven.plugins:maven-shade-plugin/3.4.1/confidence-slim/3.4.0)](https://docs.renovatebot.com/merge-confidence/) | --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, click this checkbox. --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://app.renovatebot.com/dashboard#github/googleapis/gapic-generator-java).  * refactor: expose parsed api short name and version as fields in Service (#1075) * Exposes parsed apiShortName and apiVersion as fields in Service.java, since the source fields (defaultHost and protoPakkage) are both defined per-service. * Adds these two fields correspondingly to GapicClass so that it can be composed from Service given these two upstream fields, then replaces withDefaultHost (for building GapicClass) withApiShortName and withApiVersion. (Composers and tests where this is currently used for sample generation are updated) * This change will enable Spring Codegen (when eventually split out from this repo) to reuse this parsing logic in descriptive comments and metadata. It also moves the parsing logic to earlier in the parse-compose process. * fix: fix REST transport client creation generated javadoc sample (#1077) * fix: fixes regionTag breakage * remove errant comment Co-authored-by: WhiteSource Renovate <bot@renovateapp.com> Co-authored-by: Emily Wang <emmwang@google.com> Co-authored-by: Vadym Matsishevskyi <25311427+vam-google@users.noreply.github.com>

…ce (#1075) * Exposes parsed apiShortName and apiVersion as fields in Service.java, since the source fields (defaultHost and protoPakkage) are both defined per-service. * Adds these two fields correspondingly to GapicClass so that it can be composed from Service given these two upstream fields, then replaces withDefaultHost (for building GapicClass) withApiShortName and withApiVersion. (Composers and tests where this is currently used for sample generation are updated) * This change will enable Spring Codegen (when eventually split out from this repo) to reuse this parsing logic in descriptive comments and metadata. It also moves the parsing logic to earlier in the parse-compose process.

* fix: fixes regionTag breakage * update credentials integration test * removing getPureServiceName function * linter * refactor test * chore(deps): update dependency org.apache.maven.plugins:maven-shade-plugin to v3.4.1 (#1072) [![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [org.apache.maven.plugins:maven-shade-plugin](https://maven.apache.org/plugins/) | `3.4.0` -> `3.4.1` | [![age](https://badges.renovateapi.com/packages/maven/org.apache.maven.plugins:maven-shade-plugin/3.4.1/age-slim)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://badges.renovateapi.com/packages/maven/org.apache.maven.plugins:maven-shade-plugin/3.4.1/adoption-slim)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://badges.renovateapi.com/packages/maven/org.apache.maven.plugins:maven-shade-plugin/3.4.1/compatibility-slim/3.4.0)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://badges.renovateapi.com/packages/maven/org.apache.maven.plugins:maven-shade-plugin/3.4.1/confidence-slim/3.4.0)](https://docs.renovatebot.com/merge-confidence/) | --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, click this checkbox. --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://app.renovatebot.com/dashboard#github/googleapis/gapic-generator-java).  * refactor: expose parsed api short name and version as fields in Service (#1075) * Exposes parsed apiShortName and apiVersion as fields in Service.java, since the source fields (defaultHost and protoPakkage) are both defined per-service. * Adds these two fields correspondingly to GapicClass so that it can be composed from Service given these two upstream fields, then replaces withDefaultHost (for building GapicClass) withApiShortName and withApiVersion. (Composers and tests where this is currently used for sample generation are updated) * This change will enable Spring Codegen (when eventually split out from this repo) to reuse this parsing logic in descriptive comments and metadata. It also moves the parsing logic to earlier in the parse-compose process. * fix: fix REST transport client creation generated javadoc sample (#1077) * fix: fixes regionTag breakage * remove errant comment Co-authored-by: WhiteSource Renovate <bot@renovateapp.com> Co-authored-by: Emily Wang <emmwang@google.com> Co-authored-by: Vadym Matsishevskyi <25311427+vam-google@users.noreply.github.com>

…in Service (#1075)" This reverts commit 2ebe948.

* Revert "test: update golden files after annotations import fix" This reverts commit 0bbfc59. * Revert "fix(ast): add import generation for annotations on VariableExpr (#1076)" This reverts commit 9e4721f. * Revert "chore(main): release 2.10.3 (#1079)" This reverts commit 63b9bce. * Revert "fix: fixes regionTag breakage (#1068)" This reverts commit cd35be8. * Revert "fix: fix REST transport client creation generated javadoc sample (#1077)" This reverts commit 5346eef. * Revert "refactor: expose parsed api short name and version as fields in Service (#1075)" This reverts commit 2ebe948. * Revert "chore(deps): update dependency org.apache.maven.plugins:maven-shade-plugin to v3.4.1 (#1072)" This reverts commit 6774245. * Revert "chore(main): release 2.10.2 (#1067)" This reverts commit e8ee82f. * Revert "fix(deps): update dependency com.google.cloud:google-cloud-shared-dependencies to v3.0.5 (#1063)" This reverts commit aa1782d. * Revert "fix: update regionTag to use service name (#1047)" This reverts commit 38010fd. * Revert "chore(main): release 2.10.1 (#1036)" This reverts commit 3282e0f. * Revert "deps: Upgrade protobuf to 3.21.7 (#1048)" This reverts commit c768691. * Revert "fix: update sample region tag to parse host instead of proto package (#1040)" This reverts commit eb94f30. * Revert "chore(deps): update dependency com.google.auto.value:auto-value to v1.10 (#1058)" This reverts commit bd5599f. * Revert "fix: Get numeric value for Enum fields if it is configured as query param or path param (#1042)" This reverts commit 4b5eadb. * Revert "fix(deps): update dependency com.google.cloud:google-cloud-shared-dependencies to v3.0.4 (#1050)" This reverts commit e58bf9d. * Revert "fix(deps): update dependency org.yaml:snakeyaml to v1.33 (#1043)" This reverts commit d615305. * Revert "chore(deps): update dependency org.apache.maven.plugins:maven-shade-plugin to v3.4.0 (#1038)" This reverts commit ce93705. * Revert "fix(deps): update dependency com.google.cloud:google-cloud-shared-dependencies to v3.0.3 (#1039)" This reverts commit dba69f4. * Revert "fix(deps): update dependency org.yaml:snakeyaml to v1.32 (#1037)" This reverts commit e754a7c.

…ce (#1075) * Exposes parsed apiShortName and apiVersion as fields in Service.java, since the source fields (defaultHost and protoPakkage) are both defined per-service. * Adds these two fields correspondingly to GapicClass so that it can be composed from Service given these two upstream fields, then replaces withDefaultHost (for building GapicClass) withApiShortName and withApiVersion. (Composers and tests where this is currently used for sample generation are updated) * This change will enable Spring Codegen (when eventually split out from this repo) to reuse this parsing logic in descriptive comments and metadata. It also moves the parsing logic to earlier in the parse-compose process.

* fix: fixes regionTag breakage * update credentials integration test * removing getPureServiceName function * linter * refactor test * chore(deps): update dependency org.apache.maven.plugins:maven-shade-plugin to v3.4.1 (#1072) [![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [org.apache.maven.plugins:maven-shade-plugin](https://maven.apache.org/plugins/) | `3.4.0` -> `3.4.1` | [![age](https://badges.renovateapi.com/packages/maven/org.apache.maven.plugins:maven-shade-plugin/3.4.1/age-slim)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://badges.renovateapi.com/packages/maven/org.apache.maven.plugins:maven-shade-plugin/3.4.1/adoption-slim)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://badges.renovateapi.com/packages/maven/org.apache.maven.plugins:maven-shade-plugin/3.4.1/compatibility-slim/3.4.0)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://badges.renovateapi.com/packages/maven/org.apache.maven.plugins:maven-shade-plugin/3.4.1/confidence-slim/3.4.0)](https://docs.renovatebot.com/merge-confidence/) | --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, click this checkbox. --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://app.renovatebot.com/dashboard#github/googleapis/gapic-generator-java).  * refactor: expose parsed api short name and version as fields in Service (#1075) * Exposes parsed apiShortName and apiVersion as fields in Service.java, since the source fields (defaultHost and protoPakkage) are both defined per-service. * Adds these two fields correspondingly to GapicClass so that it can be composed from Service given these two upstream fields, then replaces withDefaultHost (for building GapicClass) withApiShortName and withApiVersion. (Composers and tests where this is currently used for sample generation are updated) * This change will enable Spring Codegen (when eventually split out from this repo) to reuse this parsing logic in descriptive comments and metadata. It also moves the parsing logic to earlier in the parse-compose process. * fix: fix REST transport client creation generated javadoc sample (#1077) * fix: fixes regionTag breakage * remove errant comment Co-authored-by: WhiteSource Renovate <bot@renovateapp.com> Co-authored-by: Emily Wang <emmwang@google.com> Co-authored-by: Vadym Matsishevskyi <25311427+vam-google@users.noreply.github.com>

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.

emmileaf added 3 commits October 19, 2022 14:58

WIP: add parsed apiShortName and apiVersion as fields in Service class

79b0928

Refactor unit tests

e98b405

Update composer tests that included checks for defaultHost and sample…

5e7f9b3

… generation

emmileaf marked this pull request as ready for review October 31, 2022 16:32

emmileaf requested review from a team as code owners October 31, 2022 16:32

emmileaf requested review from alicejli, blakeli0 and zhumin8 October 31, 2022 16:33

blakeli0 approved these changes Oct 31, 2022

View reviewed changes

alicejli approved these changes Oct 31, 2022

View reviewed changes

emmileaf added 2 commits October 31, 2022 16:31

Fix assertion message

d0c02ac

Remove extra semicolons

2a10960

emmileaf merged commit da4b130 into main Oct 31, 2022

emmileaf deleted the refactor-parse-defaulthost-2 branch October 31, 2022 21:04

emmileaf added a commit that referenced this pull request Nov 3, 2022

Revert "refactor: expose parsed api short name and version as fields …

3fdf7d0

…in Service (#1075)" This reverts commit 2ebe948.

emmileaf added a commit that referenced this pull request Nov 3, 2022

Revert "refactor: expose parsed api short name and version as fields …

4302757

…in Service (#1075)" This reverts commit 2ebe948.

zhumin8 mentioned this pull request Nov 7, 2022

fix(spring): use parsed api shortname as libname #1084

Merged

zhumin8 mentioned this pull request Apr 10, 2024

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

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: expose parsed api short name and version as fields in Service #1075

refactor: expose parsed api short name and version as fields in Service #1075

emmileaf commented Oct 31, 2022

blakeli0 left a comment

alicejli left a comment

alicejli Oct 31, 2022

emmileaf Oct 31, 2022

sonarqubecloud bot commented Oct 31, 2022

emmileaf commented Oct 31, 2022

refactor: expose parsed api short name and version as fields in Service #1075

refactor: expose parsed api short name and version as fields in Service #1075

Conversation

emmileaf commented Oct 31, 2022

blakeli0 left a comment

Choose a reason for hiding this comment

alicejli left a comment

Choose a reason for hiding this comment

alicejli Oct 31, 2022

Choose a reason for hiding this comment

emmileaf Oct 31, 2022

Choose a reason for hiding this comment

sonarqubecloud bot commented Oct 31, 2022

emmileaf commented Oct 31, 2022