feat: Selective gapic generation phase II #3730

cindy-peng · 2025-04-02T00:16:33Z

Implement Selective GAPIC Generation (Phase II)

This PR implements Phase II of selective GAPIC generation within the gapic-generator-java project. This allows for finer control over the intended usage of generated client methods (public, internal, or hidden) by providing selective gapic generation configuration in service yaml.

Key Changes:

1. Model Updates

Added a isInternalApi attribute to the internal representation of methods to track their intended visibility (e.g., public, internal).

2. Parser Logic

Introduced the getMethodSelectiveGapicType() method responsible for parsing the selective generation configuration for each method.
Modified service filtering logic: Service classes will not be generated if the service definition contains no methods or includes only methods marked as HIDDEN.
Enhanced parseService() to determine and assign the appropriate SelectiveGapicType to each service method and its corresponding generated variants (e.g., overloaded methods).

3. Composer (Code Generation) Updates

Method Annotations: For all method variants designated as INTERNAL, generate an @InternalApi annotation accompanied by a warning message discouraging external use.
Method Header Comments: For methods marked as INTERNAL, generate a specific comment in the method's header indicating its intended internal-only usage.
Sample Generation: Adjusted the logic for generating package-info.java samples to prevent the usage of any methods marked as INTERNAL.

4. Tests

Added unit tests covering the new parser logic and comment generation changes related to selective generation types.
Added/updated golden unit/integration tests to verify the correct code output for various selective generation scenarios, including services with:
- All public methods.
- A mix of public, INTERNAL, and/or HIDDEN methods.
- No public methods (verifying that the service class is not generated).

test/integration/goldens/iam/src/com/google/iam/v1/IAMPolicyClient.java

zhumin8 · 2025-04-10T20:03:16Z

/gcbrun

zhumin8

Looks good to me so far. Left a few small comments and I will take another pass.

gapic-generator-java/src/test/resources/selective_api_generation_generate_omitted_v1beta1.yaml

.../goldens/samples/echoserviceselectivegapicclient/AsyncChatAgainShouldGenerateAsPublic.golden

...va/com/google/api/generator/gapic/composer/samplecode/ServiceClientHeaderSampleComposer.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/composer/ComposerTest.java

.../com/google/api/generator/gapic/composer/grpc/goldens/EchoServiceSelectiveGapicClient.golden

zhumin8 · 2025-04-11T18:01:27Z

...test/java/com/google/api/generator/gapic/composer/grpc/GrpcServiceStubClassComposerTest.java

+    Service service = context.services().get(1);
+    GapicClass clazz = GrpcServiceStubClassComposer.instance().generate(context, service);
+
+    Assert.assertGoldenClass(this.getClass(), clazz, "SelectiveGapicStub.golden");


I don't see any @InternalApi in the golden file for this test. Is this test necessary?

Yeah this should not impact the stubclass. I have removed the test and the golden file.

Am I missing it or there is only test setup to verify -Client changes, but not -Stub, -StubSettings or -ClientSettings ?
For example, in below snippet of golden file, I expect chatShouldGenerateAsInternalCallable() also be marked as internal, and I believe changes in AbstractServiceStubClassComposer.java should do it. Do you have any test coverages for it?

sdk-platform-java/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/grpc/goldens/EchoServiceSelectiveGapicClient.golden

Lines 568 to 571 in a87d3b0

@InternalApi("Internal API. This API is not intended for public consumption.")

public final UnaryCallable<EchoRequest, EchoResponse> chatShouldGenerateAsInternalCallable() {

return stub.chatShouldGenerateAsInternalCallable();

}

No I didn't add those tests earlier. I just updated the test files to include coverage for service settings, stub and stub settings. Please take a look:)

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

...a/src/main/java/com/google/api/generator/gapic/composer/comment/SettingsCommentComposer.java

zhumin8 · 2025-04-11T18:54:09Z

...com/google/api/generator/gapic/composer/common/AbstractServiceStubSettingsClassComposer.java

@@ -140,6 +141,8 @@ public abstract class AbstractServiceStubSettingsClassComposer implements ClassC
  private static final String SETTINGS_LITERAL = "Settings";

  private static final String DOT = ".";
+  private static final String INTERNAL_API_WARNING =


nit: perhaps extract INTERNAL_API_WARNING to a somewhere to be accessible for each of these composes.

Yeah that makes sense. It looks like each abstract composer currently maintains its own string constants currently and there are some duplicates for other strings as well.
For example: this string private static final String PAGED_RESPONSE_TYPE_NAME_PATTERN = "%sPagedResponse"; exists in AbstractServiceClientClassComposer, AbstractServiceStubClassComposer, AbstractServiceSettingsClassComposer, etc.

However I didn't find a central place where I can put the common strings. Do we want to create a new class (e.g. CommonStrings.java) under gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/utils/ to store all the common strings there? Or that's something we want to do in a separate refactoring PR?

composer/utils/CommonStrings.java sounds good to me. I'd rather just include with this PR so we do not forget about it.

I just realized classes under composer/utils/ are in a different package than all the composers under composer/common/. This will require common strings to be exposed as Public in order to be referenced in the composers, which could be a concern.

Should I just create a CommonStrings.java file under common folder? Seems like it is mostly for the common abstract composer classes.

Either way is good with me. I am not super concerned about it being public because this is generator code and not client library code.

Sounds good. Added composer/utils/CommonStrings.java.

...ava/com/google/api/generator/gapic/composer/common/AbstractServiceSettingsClassComposer.java

.../java/com/google/api/generator/gapic/composer/common/AbstractServiceClientClassComposer.java

cindy-peng · 2025-04-15T19:24:46Z

/gcbrun

cindy-peng · 2025-04-15T22:39:49Z

/gcbrun

...le/api/generator/gapic/composer/grpc/goldens/EchoServiceSelectiveGapicServiceSettings.golden

zhumin8 · 2025-04-16T14:28:45Z

...c-generator-java/src/main/java/com/google/api/generator/engine/writer/JavaWriterVisitor.java

@@ -1020,6 +1020,15 @@ public void visit(ClassDefinition classDefinition) {
    if (!classDefinition.isNested()) {
      String formattedClazz = JavaFormatter.format(buffer.toString());

+      // fixing @InternalApi comment after formatting
+      // formatter removes the line break before @InternalApi comment.
+      // See https://github.com/google/google-java-format/issues/1249


Great finding!
I think by convention, block tags should be starting with lowercase, changing it might make formatter recognize it correctly.
One other thing I am not certain about, by using this custom tag, does the tools used in the publishing chain (e.g. https://github.com/googleapis/java-docfx-doclet) all be able to handle it correctly? From this guide, looks like some customizations are needed.

Perhaps it might be easier to include this notice as part of the descriptive text and not use custom tag in Javadoc?

WDYT, @blakeli0 ?

@zhumin8 You are absolutely right! After changing block tag @InternalApi to start with lowercase @internalApi, Java formatter was able to format everything correctly. I will make the change and remove the regex fix here.

@zhumin8 I have changed the block tag into descriptive text. please help take a look. Thanks!

blakeli0 · 2025-04-16T22:16:19Z

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

+    // is in the allow list.
+    // Otherwise, generate this method as INTERNAL or HIDDEN based on GenerateOmittedAsInternal
+    // flag.
+    if (includeMethodsList.isEmpty() && generateOmittedAsInternal == false


This logic is not easy to understand, can we refactor it to make it more readable?
In addition, I don't see test cases in ParserTest that test all scenarios. There are no tests for INTERNAL and HIDDEN either.

I have refactored the logic and added unit tests in ParserTest to cover Internal, Hidden, Public. Please help take a look.

blakeli0 · 2025-04-16T22:26:14Z

gapic-generator-java/src/test/java/com/google/api/generator/engine/ast/JavaDocCommentTest.java

@@ -215,6 +219,7 @@ void createJavaDocComment_throwsAndDeprecatedAndReturn() {
        LineFormatter.lines(
            "@throws java.lang.RuntimeException if the remote call fails.\n",
            "@deprecated Use the {@link ShelfBookName} class instead.\n",
+            "@internalApi This method is internal used only. Please do not use directly.\n",


I don't think this works out of box, we would have to change our doc generation to include this custom tag. See discussions. Can we remove @inernalapi part from the comment for now?

Sounds good. I have also discussed this with @zhumin8 . For now I have removed this tag @internalApi and added a descriptive text to indicate internalApi before block tags. It will be something like this in the comment block:

* * <p><b>Warning: </b>This method is internal used only. Please do not use directly. *

Please take at the EchoServiceSelectiveGapicClient.golden and let me know if that works. Thanks!

.../com/google/api/generator/gapic/composer/grpc/goldens/EchoServiceSelectiveGapicClient.golden

zhumin8

LGTM.

.../com/google/api/generator/gapic/composer/grpc/goldens/EchoServiceSelectiveGapicClient.golden

cindy-peng · 2025-04-17T15:04:50Z

/gcbrun

diegomarquezp · 2025-04-17T15:36:45Z

/gcbrun

## Implement Selective GAPIC Generation (Phase II) This PR implements Phase II of selective GAPIC generation within the `gapic-generator-java` project. This allows for finer control over the intended usage of generated client methods (public, internal, or hidden) by providing selective gapic generation configuration in service yaml. ### Key Changes: #### 1. Model Updates * Added a `isInternalApi` attribute to the internal representation of methods to track their intended visibility (e.g., public, internal). #### 2. Parser Logic * Introduced the `getMethodSelectiveGapicType()` method responsible for parsing the selective generation configuration for each method. * Modified service filtering logic: Service classes will not be generated if the service definition contains no methods or includes only methods marked as **`HIDDEN`**. * Enhanced `parseService()` to determine and assign the appropriate `SelectiveGapicType` to each service method and its corresponding generated variants (e.g., overloaded methods). #### 3. Composer (Code Generation) Updates * **Method Annotations:** For all method variants designated as `INTERNAL`, generate an `@InternalApi` annotation accompanied by a warning message discouraging external use. * **Method Header Comments:** For methods marked as `INTERNAL`, generate a specific comment in the method's header indicating its intended internal-only usage. * **Sample Generation:** Adjusted the logic for generating `package-info.java` samples to prevent the usage of any methods marked as `INTERNAL`. #### 4. Tests * Added **unit tests** covering the new parser logic and comment generation changes related to selective generation types. * Added/updated **golden unit/integration tests** to verify the correct code output for various selective generation scenarios, including services with: * All public methods. * A mix of public, `INTERNAL`, and/or `HIDDEN` methods. * No public methods (verifying that the service class is not generated).

🤖 I have created a release *beep* *boop* --- <details><summary>2.56.0</summary> ## [2.56.0](v2.55.1...v2.56.0) (2025-04-18) ### Features * Selective gapic generation phase II ([#3730](#3730)) ([64ac2c1](64ac2c1)) ### Bug Fixes * **hermetic-build:** use correct image name in templated graalvm jobs ([#3743](#3743)) ([29a78d3](29a78d3)) * plumb mtls endpoint to TransportChannelProvider ([#3673](#3673)) ([a961459](a961459)) ### Dependencies * add opentelemetry gcp-resources to shared deps ([#3722](#3722)) ([b1b075d](b1b075d)) </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>

| Package | Type | Package file | Manager | Update | Change | |---|---|---|---|---|---| | [com.google.api.grpc:proto-google-common-protos](https://github.com/googleapis/sdk-platform-java) | dependencies | misk/gradle/libs.versions.toml | gradle | minor | `2.55.3` -> `2.56.0` | | [com.google.cloud:google-cloud-core-http](https://github.com/googleapis/sdk-platform-java) | dependencies | misk/gradle/libs.versions.toml | gradle | minor | `2.54.3` -> `2.55.0` | | [com.google.cloud:google-cloud-core](https://github.com/googleapis/sdk-platform-java) | dependencies | misk/gradle/libs.versions.toml | gradle | minor | `2.54.3` -> `2.55.0` | | [com.google.api:gax](https://github.com/googleapis/sdk-platform-java) | dependencies | misk/gradle/libs.versions.toml | gradle | minor | `2.64.3` -> `2.65.0` | | [software.amazon.awssdk:sdk-core](https://aws.amazon.com/sdkforjava) | dependencies | misk/gradle/libs.versions.toml | gradle | patch | `2.31.35` -> `2.31.36` | | [software.amazon.awssdk:sqs](https://aws.amazon.com/sdkforjava) | dependencies | misk/gradle/libs.versions.toml | gradle | patch | `2.31.35` -> `2.31.36` | | [software.amazon.awssdk:dynamodb-enhanced](https://aws.amazon.com/sdkforjava) | dependencies | misk/gradle/libs.versions.toml | gradle | patch | `2.31.35` -> `2.31.36` | | [software.amazon.awssdk:dynamodb](https://aws.amazon.com/sdkforjava) | dependencies | misk/gradle/libs.versions.toml | gradle | patch | `2.31.35` -> `2.31.36` | | [software.amazon.awssdk:aws-core](https://aws.amazon.com/sdkforjava) | dependencies | misk/gradle/libs.versions.toml | gradle | patch | `2.31.35` -> `2.31.36` | | [software.amazon.awssdk:bom](https://aws.amazon.com/sdkforjava) | dependencies | misk/gradle/libs.versions.toml | gradle | patch | `2.31.35` -> `2.31.36` | | [software.amazon.awssdk:auth](https://aws.amazon.com/sdkforjava) | dependencies | misk/gradle/libs.versions.toml | gradle | patch | `2.31.35` -> `2.31.36` | --- ### Release Notes <details> <summary>googleapis/sdk-platform-java (com.google.api.grpc:proto-google-common-protos)</summary> ### [`v2.56.0`](https://github.com/googleapis/sdk-platform-java/blob/HEAD/CHANGELOG.md#2560-2025-04-18) ##### Features - Selective gapic generation phase II ([#3730](googleapis/sdk-platform-java#3730)) ([64ac2c1](googleapis/sdk-platform-java@64ac2c1)) ##### Bug Fixes - **hermetic-build:** use correct image name in templated graalvm jobs ([#3743](googleapis/sdk-platform-java#3743)) ([29a78d3](googleapis/sdk-platform-java@29a78d3)) - plumb mtls endpoint to TransportChannelProvider ([#3673](googleapis/sdk-platform-java#3673)) ([a961459](googleapis/sdk-platform-java@a961459)) ##### Dependencies - add opentelemetry gcp-resources to shared deps ([#3722](googleapis/sdk-platform-java#3722)) ([b1b075d](googleapis/sdk-platform-java@b1b075d)) </details> --- ### Configuration 📅 **Schedule**: Branch creation - "after 6pm every weekday,before 2am every weekday" in timezone Australia/Melbourne, Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Never, or you tick the rebase/retry checkbox. 👻 **Immortal**: This PR will be recreated if closed unmerged. Get [config help](https://github.com/renovatebot/renovate/discussions) if that's undesired. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). GitOrigin-RevId: 1a1f0ef96fb9d22ffd1976f1eea8999c165d6c37

feat: Selective gapic generation phase II

595198b

product-auto-label bot added the size: l Pull request size is large. label Apr 2, 2025

Add selective gapic yaml files and golden files

f5c93db

product-auto-label bot added size: xl Pull request size is extra large. and removed size: l Pull request size is large. labels Apr 2, 2025

add golden test files

734efa9

zhumin8 reviewed Apr 2, 2025

View reviewed changes

test/integration/goldens/iam/src/com/google/iam/v1/IAMPolicyClient.java Outdated Show resolved Hide resolved

cindy-peng added 8 commits April 2, 2025 12:51

Revert service client throw comment

fbb19ea

Revert integration golden files

03220b9

Merge branch 'main' into cindy/selective-gapic

c516814

Merge branch 'main' into cindy/selective-gapic

55de64f

Merge branch 'main' into cindy/selective-gapic

27bdc9e

Remove enum from method model

1f77302

Set default method level to be public

eb0c6bf

Fix formatting errors

5a21be7

cindy-peng added 3 commits April 10, 2025 13:23

Change isPublic attribute to isInternalApi in method class

5fad1fe

Merge branch 'main' into cindy/selective-gapic

182a517

Fix linter

221eb8f

zhumin8 reviewed Apr 11, 2025

View reviewed changes

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

zhumin8 reviewed Apr 11, 2025

View reviewed changes

...a/src/main/java/com/google/api/generator/gapic/composer/comment/SettingsCommentComposer.java Outdated Show resolved Hide resolved

zhumin8 reviewed Apr 11, 2025

View reviewed changes

...ava/com/google/api/generator/gapic/composer/common/AbstractServiceSettingsClassComposer.java Outdated Show resolved Hide resolved

zhumin8 reviewed Apr 11, 2025

View reviewed changes

.../java/com/google/api/generator/gapic/composer/common/AbstractServiceClientClassComposer.java Outdated Show resolved Hide resolved

cindy-peng added 3 commits April 15, 2025 11:18

Fix comments from PR review.

ffd66db

Merge branch 'main' into cindy/selective-gapic

602b910

Fix linter.

a87d3b0

zhumin8 mentioned this pull request Apr 15, 2025

(generator) test setup generates samples and produces golden files not used in tests #3740

Open

cindy-peng added 2 commits April 15, 2025 17:50

Add unit tests for stub, stub settings,service settings

cbb7ce5

Fix Javadoc comment line break before @internalapi comment

6c30173

zhumin8 reviewed Apr 16, 2025

View reviewed changes

...le/api/generator/gapic/composer/grpc/goldens/EchoServiceSelectiveGapicServiceSettings.golden Outdated Show resolved Hide resolved

zhumin8 reviewed Apr 16, 2025

View reviewed changes

cindy-peng added 2 commits April 16, 2025 09:37

Change block tag to start with lowercase to resolve formatter issue.

b481cc7

Fix comment to be lowercase @internalapi

edafba1

blakeli0 reviewed Apr 16, 2025

View reviewed changes

Change internalApi javadoc comment to be additional descriptive text.

f4c9cf7

cindy-peng force-pushed the cindy/selective-gapic branch from 838fdff to f4c9cf7 Compare April 17, 2025 00:45

cindy-peng added 3 commits April 16, 2025 18:10

Merge branch 'main' into cindy/selective-gapic

385873d

Add common string and unit tests for parser.

7c3aeaa

Add common string and unit tests for parser.

b0553ac

zhumin8 approved these changes Apr 17, 2025

View reviewed changes

.../com/google/api/generator/gapic/composer/grpc/goldens/EchoServiceSelectiveGapicClient.golden Outdated Show resolved Hide resolved

cindy-peng added 2 commits April 17, 2025 08:01

Change internal use warning message.

01bc7f7

Fix linter formatting.

2ece35b

blakeli0 approved these changes Apr 17, 2025

View reviewed changes

blakeli0 merged commit 64ac2c1 into googleapis:main Apr 17, 2025
34 of 36 checks passed

release-please bot mentioned this pull request Apr 17, 2025

chore(main): release 2.56.0 #3715

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Selective gapic generation phase II #3730

feat: Selective gapic generation phase II #3730

cindy-peng commented Apr 2, 2025 •

edited

Loading

zhumin8 commented Apr 10, 2025

zhumin8 left a comment

zhumin8 Apr 11, 2025

cindy-peng Apr 15, 2025

zhumin8 Apr 15, 2025

cindy-peng Apr 16, 2025

zhumin8 Apr 11, 2025

cindy-peng Apr 15, 2025

zhumin8 Apr 15, 2025

cindy-peng Apr 16, 2025

zhumin8 Apr 16, 2025

cindy-peng Apr 17, 2025

cindy-peng commented Apr 15, 2025

cindy-peng commented Apr 15, 2025

zhumin8 Apr 16, 2025

zhumin8 Apr 16, 2025 •

edited

Loading

cindy-peng Apr 16, 2025

cindy-peng Apr 17, 2025

blakeli0 Apr 16, 2025

cindy-peng Apr 17, 2025

blakeli0 Apr 16, 2025

cindy-peng Apr 17, 2025

zhumin8 left a comment

cindy-peng commented Apr 17, 2025

diegomarquezp commented Apr 17, 2025

	@InternalApi("Internal API. This API is not intended for public consumption.")
	public final UnaryCallable<EchoRequest, EchoResponse> chatShouldGenerateAsInternalCallable() {
	return stub.chatShouldGenerateAsInternalCallable();
	}

feat: Selective gapic generation phase II #3730

feat: Selective gapic generation phase II #3730

Conversation

cindy-peng commented Apr 2, 2025 • edited Loading

Implement Selective GAPIC Generation (Phase II)

Key Changes:

1. Model Updates

2. Parser Logic

3. Composer (Code Generation) Updates

4. Tests

zhumin8 commented Apr 10, 2025

zhumin8 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cindy-peng commented Apr 15, 2025

cindy-peng commented Apr 15, 2025

Choose a reason for hiding this comment

zhumin8 Apr 16, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zhumin8 left a comment

Choose a reason for hiding this comment

cindy-peng commented Apr 17, 2025

diegomarquezp commented Apr 17, 2025

cindy-peng commented Apr 2, 2025 •

edited

Loading

zhumin8 Apr 16, 2025 •

edited

Loading