Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DRAFT] - Code to generate spring autoconfig modules. (not intended to merge) #1030

Closed
wants to merge 107 commits into from

Conversation

zhumin8
Copy link
Contributor

@zhumin8 zhumin8 commented Aug 25, 2022

This is a WIP pr for discussion.

Code is incomplete but with major skeleton done.

  • Code for composing the Spring classes are moved to src/spring/composer
  • Addition in Writer.java is to generate spring.factories file (as in sample here). Will consider extending Write.java instead. Create a copy as it contains mostly static methods.
  • Main.java is the entry point to use gapic-gen under current infra.
  • java_gapic.bzl and java_gapic_pkg.bzl changes mimics how sample is generated, and adds a spring top level folder to final output tar.

To do items:

  • Complete SpringAutoConfigClassComposer.java.
  • Add expected to test for SpringPropertiesClassComposer.
  • Complete other TODO items as marked in draft.
  • Move out code addition to RetrySettingsComposer.java
  • golden test setup.
  • Move Spring Composers to new module.
  • Add Spring dependency to gapic-generator-java
  • Consider not directly adding to Writer.java.
  • Re-consider java_gapic.bzl and java_gapic_pkg.bzl additions.

A manual sample repo of intended output is here.

From in-person review takeaway points:

  • Add Spring dependencies in. One benefit being safe in case of future package name changes for imported classes.
  • Setup golden test to compare results instead of expected string.
  • Try creating separate bazel targets for Spring code gen, and separate out Main.class changes.

zhumin8 added 29 commits June 24, 2022 13:58
…trySettingsComposer to help generate declare,getter,setters for retrysettings in properties. Other WIP changes in AutoConfig.
zhumin8 and others added 3 commits November 21, 2022 10:50
…tatic types instead of vapor references (#1046)

Adding Spring related dependencies to build files and use static types instead of vapor references. 
Benefit of this change:
- easier to test
- less code change needed for future changes (e.g. package change of classes).

One caveat is that because `gapic-generator-java` is brought into `googleapis` as `http_archive` code addition to `googleapis` is also needed to include these libraries. Add below code snippet to [WORKSPACE](https://github.com/googleapis/googleapis/blob/38e8b447d173909d3b2fe8fdc3e6cbb3c85442fd/WORKSPACE#L239-L245):
```
SPRING_MAVEN_ARTIFACTS = [
    "org.springframework.boot:spring-boot-starter:2.7.4",
    "com.google.cloud:spring-cloud-gcp-core:3.3.0",
]

maven_install(
    artifacts = PROTOBUF_MAVEN_ARTIFACTS + SPRING_MAVEN_ARTIFACTS,
    generate_compat_repositories = True,
    repositories = [
        "https://repo.maven.apache.org/maven2/",
    ],
)

```

------------
Updates:
- Updated pr with #1065, 
- Moved annotation classed added in #1045 to static types.
- Fixed conflicts from merged changes in #1093
- Moved logging classes added in #1053 to static types.
- `{{starter-version}}` are not needed anymore as we inherit version from parent pom.
- add parent version placeholder. Changes on spring-cloud-gcp side: GoogleCloudPlatform/spring-cloud-gcp#1353.
- remove redundant groupid.
- remove versions for `spring-cloud-gcp-core` and `spring-boot-starter`, specified in `genereated-parent` pom
* fix(spring codegen): use client-library-artifact-id as pom artifactId prefix. (#1092)

* rename to global properties

* fix(test): update goldens

* fix: format

* fix: typo in GLobalProperties

* fix: typo in goldens

* fix: remove unnecessary comments

* fix: reference to com.google.cloud.spring.global

* fix: typo and static ref fix

Co-authored-by: Min Zhu <[email protected]>
@sonarqubecloud
Copy link

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 93 Code Smells

89.7% 89.7% Coverage
3.3% 3.3% Duplication

zhumin8 and others added 21 commits November 30, 2022 19:19
* feat: degrade logging level to trace

* feat: wrap logging with `isTraceEnabled()`

* feat: credential usage logging

* fix: use Statement for logging variables

* fix: use Statement return type in createLoggerStatement()

* fix: unnecessary variable creation
change `@Generated` description to spring specific and separate from "gapic-generator-java" for client libraries.
Also moved relevant annotation from `SpringPackageInfoComposer` to `SpringComposer`.
This change intend to set the auto-configuration to be enabled by default.
* feat: spring service settings as its own bean

* chore: update goldens

* fix: rename settings creation method

* fix: remove name parameter from conditional bean annotation

* fix: common service settings method name and unnecessary comment
…1123)

As discussed off-line, instead of depending on a new `global-properties` module, use `CredentialsProvider` bean from `spring-cloud-gcp-autoconfigure` instead.
- Replace dependency from `core` to `autoconfigure` module.
- Revert #1071 and implement changes shown in [poc](zhumin8/language_poc1#7)
- Minor code cleanups and util method extracted from `SpringPropertiesClassComposer.java`. 

Note: #1071 and GoogleCloudPlatform/spring-cloud-gcp#1308 will be obsolete after this change.
Also, conflicts needs to be resolved between this pr and #1110 depending on merging order.
This PR updates spring branch with CI fix from #1163 - with commit 71f0df1 cherry-picked from main branch.
…ed properties (#1143)

* This PR refactors the current setup (method-level and individual settings) to use a nested Retry object provided through properties at both service-level and method-level.
This is part 2 of implementation for repo structural changes in [PoC](GoogleCloudPlatform/spring-cloud-gcp#1383).
Changes in Pom string:
- changed parent to `spring-cloud-gcp-starters` and version with "-preview" suffix
- removed client library version (should inherit from Libraries BOM).

Corresponding changes in spring-cloud-gcp: GoogleCloudPlatform/spring-cloud-gcp#1394
Add name to `@ConditionalOnMissingBean` annotation when creating `TransportChannelProvider` bean so that it can be picked up with Qualifier in serviceSettings bean creation that follows.
This was missed in original [commit](0643085) adding this bean and should fix error seen in GoogleCloudPlatform/spring-cloud-gcp#1407 (comment)
This adds AutoConfigureAfter(GcpContextAutoConfiguration.class) annotation to enforce configuration order. It also makes two related changes to enable this annotation:
* Switches dependency from spring-cloud-gcp-core to spring-cloud-gcp-autoconfigure, and bumps version to latest
* Fixes ImportWriter to account for types introduced by annotation parameters
* This PR is a patch for the RetrySettings configuration issue discovered in GoogleCloudPlatform/spring-cloud-gcp#1407. It excludes streaming and LRO methods from retry configuration through spring properties for now.
Adds `@param` and `@return` to method javadoc comments in generated autoconfiguration classes. 
Also adding a change to `JavaDocComment` to enable `@return` in comment. (this change in 537e33c can be copied to main branch of `gapic-generator-java` if useful.)
…rt option (#1227)

* Updated behaviour for when Transport.REST_GRPC and useRest = true (through properties):
  * The ServiceSettings bean uses newHttpJsonBuilder
  * The TransportChannelProvider bean definition returns defaultHttpJsonTransportProviderBuilder
* This PR switches camelcase handling to use align with logic used in generating client libraries.
This enables release-please setup in spring-cloud-gcp to update version as needed.
* use ClassNames util to generate client and settings class names in comments
* update settings bean comment, and remove other usages of service.name()
@product-auto-label product-auto-label bot added the size: xl Pull request size is extra large. label Feb 9, 2023
…golden tests (#1348)

This PR is a follow-up on #1343:
 - Extends fix to SpringPropertiesClassComposer, so that for services without REST-enabled rpcs, the unused useRest property is also not generated
- Adds golden tests for the updated hasRestOption scenario using the wicked proto
- Updates SpringAutoconfigCommentComposer for javadoc comments alluding to the useRest option
@suztomo suztomo closed this Mar 10, 2023
@suztomo
Copy link
Member

suztomo commented Mar 10, 2023

Please reopen if you need this pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size: xl Pull request size is extra large. spring pr that's related to spring code gen, intend to merge into autoconfig-gen-draft2 branch.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants