fix: Fix BetaApi annotaiton usage for REST transport and clean BetaApi for default stubs in all transports (#987)

This PR essentially does the following:
1) Clean `@BetaApi("A restructuring of stub classes is planned, so this may break in the future")` annotaiton for Stub-related methods, because those methods and classes have been in "beta" state like that for several  years already and are de-facto GA.
2) Make sure that all HttpJson (REST) related classes and methods on the surface of the client are marked as beta for `grpc+rest` (mixed transport) clients. This is necessary in the context of the upcoming REGAPIC rollout to indicate that soon to be released REST transport functionality is released at Beta quality level.

Author: vam-google

BUILD.bazel

@@ -200,6 +200,7 @@ GOLDEN_UPDATING_UNIT_TESTS = [
     "com.google.api.generator.gapic.composer.grpcrest.ServiceClientClassComposerTest",
     "com.google.api.generator.gapic.composer.grpcrest.ServiceClientTestClassComposerTest",
     "com.google.api.generator.gapic.composer.grpcrest.ServiceSettingsClassComposerTest",
+    "com.google.api.generator.gapic.composer.grpcrest.ServiceStubSettingsClassComposerTest",
     "com.google.api.generator.gapic.composer.resourcename.ResourceNameHelperClassComposerTest",
     "com.google.api.generator.gapic.composer.rest.HttpJsonServiceCallableFactoryClassComposerTest",
     "com.google.api.generator.gapic.composer.rest.HttpJsonServiceStubClassComposerTest",

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

@@ -89,6 +89,27 @@ public class SettingsCommentComposer {
           .map(c -> CommentStatement.withComment(c))
           .collect(Collectors.toList());
 
+  private final CommentStatement newTransportBuilderMethodComment;
+  private final CommentStatement transportProviderBuilderMethodComment;
+
+  public SettingsCommentComposer(String transportPrefix) {
+    this.newTransportBuilderMethodComment =
+        toSimpleComment(String.format("Returns a new %s builder for this class.", transportPrefix));
+    this.transportProviderBuilderMethodComment =
+        toSimpleComment(
+            String.format(
+                "Returns a builder for the default %s ChannelProvider for this service.",
+                transportPrefix));
+  }
+
+  public CommentStatement getNewTransportBuilderMethodComment() {
+    return newTransportBuilderMethodComment;
+  }
+
+  public CommentStatement getTransportProviderBuilderMethodComment() {
+    return transportProviderBuilderMethodComment;
+  }
+
   public static CommentStatement createCallSettingsGetterComment(
       String javaMethodName, boolean isMethodDeprecated) {
     String methodComment = String.format(CALL_SETTINGS_METHOD_DOC_PATTERN, javaMethodName);

src/main/java/com/google/api/generator/gapic/composer/common/AbstractServiceCallableFactoryClassComposer.java

@@ -70,7 +70,7 @@ public GapicClass generate(GapicContext context, Service service) {
     String pakkage = String.format("%s.stub", service.pakkage());
 
     StubCommentComposer commentComposer =
-        new StubCommentComposer(getTransportContext().transportName());
+        new StubCommentComposer(getTransportContext().transportNames().get(0));
     ClassDefinition classDef =
         ClassDefinition.builder()
             .setPackageString(pakkage)

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

@@ -340,18 +340,11 @@ private static List<MethodDefinition> createStaticCreatorMethods(
                 .setType(typeStore.get(ClassNames.getServiceStubClassName(service)))
                 .setName("stub")
                 .build());
-    AnnotationNode betaAnnotation =
-        AnnotationNode.builder()
-            .setType(typeStore.get("BetaApi"))
-            .setDescription(
-                "A restructuring of stub classes is planned, so this may break in the future")
-            .build();
     methods.add(
         MethodDefinition.builder()
             .setHeaderCommentStatements(
                 ServiceClientCommentComposer.createCreateMethodStubArgComment(
                     ClassNames.getServiceClientClassName(service), settingsVarExpr.type()))
-            .setAnnotations(Arrays.asList(betaAnnotation))
             .setScope(ScopeNode.PUBLIC)
             .setIsStatic(true)
             .setIsFinal(true)
@@ -448,15 +441,8 @@ private List<MethodDefinition> createConstructorMethods(
     if (hasLroClient) {
       ctorAssignmentExprs.addAll(operationsClientAssignExprs);
     }
-    AnnotationNode betaAnnotation =
-        AnnotationNode.builder()
-            .setType(typeStore.get("BetaApi"))
-            .setDescription(
-                "A restructuring of stub classes is planned, so this may break in the future")
-            .build();
     methods.add(
         MethodDefinition.constructorBuilder()
-            .setAnnotations(Arrays.asList(betaAnnotation))
             .setScope(ScopeNode.PROTECTED)
             .setReturnType(thisClassType)
             .setArguments(stubVarExpr.toBuilder().setIsDecl(true).build())
@@ -534,12 +520,6 @@ private List<MethodDefinition> createGetterMethods(
         methodNameToTypes.put(opClientMethodName, opClientTypesIt.next());
       }
     }
-    AnnotationNode betaStubAnnotation =
-        AnnotationNode.builder()
-            .setType(typeStore.get("BetaApi"))
-            .setDescription(
-                "A restructuring of stub classes is planned, so this may break in the future")
-            .build();
 
     return methodNameToTypes.entrySet().stream()
         .map(
@@ -554,10 +534,6 @@ private List<MethodDefinition> createGetterMethods(
                         ServiceClientCommentComposer.GET_OPERATIONS_CLIENT_METHOD_COMMENT);
               }
               return methodBuilder
-                  .setAnnotations(
-                      methodName.equals("getStub")
-                          ? Arrays.asList(betaStubAnnotation)
-                          : Collections.emptyList())
                   .setScope(ScopeNode.PUBLIC)
                   .setName(methodName)
                   .setIsFinal(!methodName.equals("getStub"))

src/main/java/com/google/api/generator/gapic/composer/common/AbstractServiceSettingsClassComposer.java

@@ -85,7 +85,7 @@ public abstract class AbstractServiceSettingsClassComposer implements ClassCompo
 
   private static final String OPERATION_SETTINGS_LITERAL = "OperationSettings";
   private static final String SETTINGS_LITERAL = "Settings";
-  private static final TypeStore FIXED_TYPESTORE = createStaticTypes();
+  protected static final TypeStore FIXED_TYPESTORE = createStaticTypes();
 
   private final TransportContext transportContext;
 
@@ -185,7 +185,14 @@ private List<MethodDefinition> createClassMethods(Service service, TypeStore typ
     javaMethods.addAll(createSettingsGetterMethods(service, typeStore));
     javaMethods.add(createCreatorMethod(service, typeStore));
     javaMethods.addAll(createDefaultGetterMethods(service, typeStore));
-    javaMethods.addAll(createNewBuilderMethods(service, typeStore, "newBuilder", "createDefault"));
+    javaMethods.addAll(
+        createNewBuilderMethods(
+            service,
+            typeStore,
+            "newBuilder",
+            "createDefault",
+            ImmutableList.of(),
+            SettingsCommentComposer.NEW_BUILDER_METHOD_COMMENT));
     javaMethods.addAll(createBuilderHelperMethods(service, typeStore));
     javaMethods.add(createConstructorMethod(service, typeStore));
     return javaMethods;
@@ -370,13 +377,37 @@ private List<MethodDefinition> createDefaultGetterMethods(Service service, TypeS
         getTransportContext().defaultTransportProviderBuilderNames().iterator();
     Iterator<Class<?>> channelProviderClassesIt =
         getTransportContext().instantiatingChannelProviderBuilderClasses().iterator();
-    while (providerBuilderNamesIt.hasNext() && channelProviderClassesIt.hasNext()) {
+    Iterator<String> transportNamesIt = getTransportContext().transportNames().iterator();
+
+    boolean secondaryTransportProviderBuilder = false;
+    while (providerBuilderNamesIt.hasNext()
+        && channelProviderClassesIt.hasNext()
+        && transportNamesIt.hasNext()) {
+      List<AnnotationNode> annotations = ImmutableList.of();
+
+      // For clients supporting multiple transports (mainly grpc+rest case) make secondary transport
+      // declared as @BetaApi for now.
+      if (secondaryTransportProviderBuilder) {
+        annotations =
+            Arrays.asList(AnnotationNode.builder().setType(FIXED_TYPESTORE.get("BetaApi")).build());
+      }
+      CommentStatement commentStatement =
+          SettingsCommentComposer.DEFAULT_TRANSPORT_PROVIDER_BUILDER_METHOD_COMMENT;
+      if (getTransportContext().transportNames().size() > 1) {
+        commentStatement =
+            new SettingsCommentComposer(transportNamesIt.next())
+                .getTransportProviderBuilderMethodComment();
+      }
+
       javaMethods.add(
           methodMakerFn.apply(
-              methodStarterFn.apply(
-                  providerBuilderNamesIt.next(),
-                  typeMakerFn.apply(channelProviderClassesIt.next())),
-              SettingsCommentComposer.DEFAULT_TRANSPORT_PROVIDER_BUILDER_METHOD_COMMENT));
+              methodStarterFn
+                  .apply(
+                      providerBuilderNamesIt.next(),
+                      typeMakerFn.apply(channelProviderClassesIt.next()))
+                  .setAnnotations(annotations),
+              commentStatement));
+      secondaryTransportProviderBuilder = true;
     }
 
     javaMethods.add(
@@ -408,11 +439,14 @@ protected List<MethodDefinition> createNewBuilderMethods(
       Service service,
       TypeStore typeStore,
       String newBuilderMethodName,
-      String createDefaultMethodName) {
+      String createDefaultMethodName,
+      List<AnnotationNode> annotations,
+      CommentStatement comment) {
     TypeNode builderType = typeStore.get(BUILDER_CLASS_NAME);
     return ImmutableList.of(
         MethodDefinition.builder()
-            .setHeaderCommentStatements(SettingsCommentComposer.NEW_BUILDER_METHOD_COMMENT)
+            .setHeaderCommentStatements(comment)
+            .setAnnotations(annotations)
             .setScope(ScopeNode.PUBLIC)
             .setIsStatic(true)
             .setReturnType(builderType)
@@ -500,7 +534,8 @@ private List<MethodDefinition> createNestedBuilderClassMethods(
     List<MethodDefinition> javaMethods = new ArrayList<>();
     javaMethods.addAll(createNestedBuilderConstructorMethods(service, typeStore));
     javaMethods.addAll(
-        createNestedBuilderCreatorMethods(service, typeStore, "newBuilder", "createDefault"));
+        createNestedBuilderCreatorMethods(
+            service, typeStore, "newBuilder", "createDefault", ImmutableList.of()));
     javaMethods.add(createNestedBuilderStubSettingsBuilderMethod(service, typeStore));
     javaMethods.add(createNestedBuilderApplyToAllUnaryMethod(service, typeStore));
     javaMethods.addAll(createNestedBuilderSettingsGetterMethods(service, typeStore));
@@ -591,7 +626,8 @@ protected List<MethodDefinition> createNestedBuilderCreatorMethods(
       Service service,
       TypeStore typeStore,
       String newBuilderMethodName,
-      String createDefaultMethodName) {
+      String createDefaultMethodName,
+      List<AnnotationNode> annotations) {
     MethodInvocationExpr ctorArg =
         MethodInvocationExpr.builder()
             .setStaticReferenceType(
@@ -602,6 +638,7 @@ protected List<MethodDefinition> createNestedBuilderCreatorMethods(
     TypeNode builderType = typeStore.get(BUILDER_CLASS_NAME);
     return ImmutableList.of(
         MethodDefinition.builder()
+            .setAnnotations(annotations)
             .setScope(ScopeNode.PRIVATE)
             .setIsStatic(true)
             .setReturnType(builderType)

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

@@ -249,12 +249,14 @@ protected List<MethodDefinition> createDefaultTransportTransportProviderBuilderM
         getTransportContext().instantiatingChannelProviderBuilderClasses().iterator();
     Iterator<String> builderNamesIt =
         getTransportContext().defaultTransportProviderBuilderNames().iterator();
+    Iterator<String> transportNamesIt = getTransportContext().transportNames().iterator();
 
     List<MethodDefinition> methods = new ArrayList<>();
 
     while (providerClassIt.hasNext()
         && providerBuilderClassIt.hasNext()
-        && builderNamesIt.hasNext()) {
+        && builderNamesIt.hasNext()
+        && transportNamesIt.hasNext()) {
       TypeNode returnType =
           TypeNode.withReference(ConcreteReference.withClazz(providerBuilderClassIt.next()));
       TypeNode channelProviderType =
@@ -269,16 +271,29 @@ protected List<MethodDefinition> createDefaultTransportTransportProviderBuilderM
       Expr returnExpr =
           initializeTransportProviderBuilder(transportChannelProviderBuilderExpr, returnType);
 
+      List<AnnotationNode> annotations = new ArrayList<>();
+      if (!methods.isEmpty()) {
+        annotations.add(AnnotationNode.builder().setType(FIXED_TYPESTORE.get("BetaApi")).build());
+      }
+      CommentStatement commentStatement =
+          SettingsCommentComposer.DEFAULT_TRANSPORT_PROVIDER_BUILDER_METHOD_COMMENT;
+      if (getTransportContext().transportNames().size() > 1) {
+        commentStatement =
+            new SettingsCommentComposer(transportNamesIt.next())
+                .getTransportProviderBuilderMethodComment();
+      }
+
       MethodDefinition method =
           MethodDefinition.builder()
-              .setHeaderCommentStatements(
-                  SettingsCommentComposer.DEFAULT_TRANSPORT_PROVIDER_BUILDER_METHOD_COMMENT)
+              .setHeaderCommentStatements(commentStatement)
+              .setAnnotations(annotations)
               .setScope(ScopeNode.PUBLIC)
               .setIsStatic(true)
               .setReturnType(returnType)
               .setName(builderNamesIt.next())
               .setReturnExpr(returnExpr)
               .build();
+
       methods.add(method);
     }
 
@@ -981,7 +996,13 @@ private List<MethodDefinition> createClassMethods(
         createMethodSettingsGetterMethods(methodSettingsMemberVarExprs, deprecatedSettingVarNames));
     javaMethods.add(createCreateStubMethod(service, typeStore));
     javaMethods.addAll(createDefaultHelperAndGetterMethods(service, typeStore));
-    javaMethods.addAll(createNewBuilderMethods(service, typeStore, "newBuilder", "createDefault"));
+    javaMethods.addAll(
+        createNewBuilderMethods(
+            service,
+            typeStore,
+            "newBuilder",
+            "createDefault",
+            SettingsCommentComposer.NEW_BUILDER_METHOD_COMMENT));
     javaMethods.addAll(createBuilderHelperMethods(service, typeStore));
     javaMethods.add(createClassConstructor(service, methodSettingsMemberVarExprs, typeStore));
     return javaMethods;
@@ -1086,15 +1107,8 @@ private MethodDefinition createCreateStubMethod(Service service, TypeStore typeS
 
     // Put the method together.
     TypeNode returnType = typeStore.get(ClassNames.getServiceStubClassName(service));
-    AnnotationNode annotation =
-        AnnotationNode.builder()
-            .setType(FIXED_TYPESTORE.get("BetaApi"))
-            .setDescription(
-                "A restructuring of stub classes is planned, so this may break in the future")
-            .build();
 
     return MethodDefinition.builder()
-        .setAnnotations(Arrays.asList(annotation))
         .setScope(ScopeNode.PUBLIC)
         .setReturnType(returnType)
         .setName("createStub")
@@ -1186,12 +1200,13 @@ protected List<MethodDefinition> createNewBuilderMethods(
       Service service,
       TypeStore typeStore,
       String newBuilderMethodName,
-      String createDefaultMethodName) {
+      String createDefaultMethodName,
+      CommentStatement methodComment) {
     // Create the newBuilder() method.
     final TypeNode builderReturnType = typeStore.get(NESTED_BUILDER_CLASS_NAME);
     return ImmutableList.of(
         MethodDefinition.builder()
-            .setHeaderCommentStatements(SettingsCommentComposer.NEW_BUILDER_METHOD_COMMENT)
+            .setHeaderCommentStatements(methodComment)
             .setScope(ScopeNode.PUBLIC)
             .setIsStatic(true)
             .setReturnType(builderReturnType)

src/main/java/com/google/api/generator/gapic/composer/common/AbstractTransportServiceStubClassComposer.java

@@ -190,7 +190,7 @@ public GapicClass generate(GapicContext context, Service service) {
             messageTypes);
 
     StubCommentComposer commentComposer =
-        new StubCommentComposer(getTransportContext().transportName());
+        new StubCommentComposer(getTransportContext().transportNames().get(0));
 
     ClassDefinition classDef =
         ClassDefinition.builder()

src/main/java/com/google/api/generator/gapic/composer/common/TransportContext.java

@@ -30,8 +30,7 @@ public abstract class TransportContext {
   // For AbstractServiceStubClassComposer
   public abstract Transport transport();
 
-  @Nullable
-  public abstract String transportName();
+  public abstract List<String> transportNames();
 
   @Nullable
   public abstract Class<?> callSettingsClass();
@@ -98,7 +97,7 @@ public abstract static class Builder {
 
     public abstract Builder setTransport(Transport transport);
 
-    public abstract Builder setTransportName(String value);
+    public abstract Builder setTransportNames(List<String> values);
 
     public abstract Builder setCallSettingsClass(Class<?> callSettingsClass);
 

src/main/java/com/google/api/generator/gapic/composer/grpc/GrpcContext.java

@@ -34,7 +34,7 @@ public abstract class GrpcContext extends TransportContext {
       GrpcContext.builder()
           .setClassNames(new ClassNames("Grpc"))
           .setTransport(Transport.GRPC)
-          .setTransportName("gRPC")
+          .setTransportNames(ImmutableList.of("gRPC"))
           // For grpc.GrpcServiceStubClassComposer
           .setCallSettingsClass(GrpcCallSettings.class)
           .setStubCallableFactoryType(classToType(GrpcStubCallableFactory.class))

src/main/java/com/google/api/generator/gapic/composer/grpcrest/GrpcRestContext.java

@@ -33,7 +33,7 @@ public abstract class GrpcRestContext extends TransportContext {
       GrpcRestContext.builder()
           .setClassNames(new ClassNames("Grpc", "HttpJson"))
           .setTransport(Transport.GRPC_REST)
-          .setTransportName(null)
+          .setTransportNames(ImmutableList.of("gRPC", "REST"))
           // For grpcrest.GrpcServiceStubClassComposer
           .setCallSettingsClass(null)
           .setStubCallableFactoryType(null)