From 7d1b8336e560497838b60287f6b98c039b1c92e4 Mon Sep 17 00:00:00 2001 From: vam-google Date: Thu, 8 Jul 2021 18:34:54 -0700 Subject: [PATCH 1/2] fix: (rest transport) Add `@BetaApi` to the generated `TransportServiceFactory` class and lro-specific method This is to make implementing LRO post-GA safer. Note, `TransportServiceFactory` class is public for technical reasons, and is not supposed to be used by users directly. --- .../generator/engine/ast/AnnotationNode.java | 9 ++++--- ...ctServiceCallableFactoryClassComposer.java | 14 +++++++--- ...pcServiceCallableFactoryClassComposer.java | 19 +++++++++----- ...onServiceCallableFactoryClassComposer.java | 26 ++++++++++++++++++- .../HttpJsonComplianceCallableFactory.golden | 2 ++ .../HttpJsonAddressesCallableFactory.java | 4 +++ ...tpJsonRegionOperationsCallableFactory.java | 4 +++ 7 files changed, 63 insertions(+), 15 deletions(-) diff --git a/src/main/java/com/google/api/generator/engine/ast/AnnotationNode.java b/src/main/java/com/google/api/generator/engine/ast/AnnotationNode.java index a7c69d8729..888262cbf4 100644 --- a/src/main/java/com/google/api/generator/engine/ast/AnnotationNode.java +++ b/src/main/java/com/google/api/generator/engine/ast/AnnotationNode.java @@ -41,11 +41,12 @@ public void accept(AstNodeVisitor visitor) { visitor.visit(this); } + public static AnnotationNode withTypeAndDescription(TypeNode type, String description) { + return AnnotationNode.builder().setType(type).setDescription(description).build(); + } + public static AnnotationNode withSuppressWarnings(String description) { - return AnnotationNode.builder() - .setType(annotationType(SuppressWarnings.class)) - .setDescription(description) - .build(); + return withTypeAndDescription(annotationType(SuppressWarnings.class), description); } public static AnnotationNode withType(TypeNode type) { diff --git a/src/main/java/com/google/api/generator/gapic/composer/common/AbstractServiceCallableFactoryClassComposer.java b/src/main/java/com/google/api/generator/gapic/composer/common/AbstractServiceCallableFactoryClassComposer.java index efe6daf3d6..a76131b31f 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/common/AbstractServiceCallableFactoryClassComposer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/common/AbstractServiceCallableFactoryClassComposer.java @@ -44,6 +44,7 @@ import com.google.api.generator.gapic.model.Service; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.List; import java.util.stream.Collectors; import javax.annotation.Generated; @@ -136,7 +137,8 @@ protected MethodDefinition createUnaryCallableMethod(TypeStore typeStore) { /*callSettingsVariantName=*/ methodVariantName, /*callSettingsTemplateObjects=*/ methodTemplateNames.stream() .map(n -> (Object) n) - .collect(Collectors.toList())); + .collect(Collectors.toList()), + Collections.emptyList()); } protected MethodDefinition createPagedCallableMethod(TypeStore typeStore) { @@ -158,7 +160,8 @@ protected MethodDefinition createPagedCallableMethod(TypeStore typeStore) { /*callSettingsVariantName=*/ methodVariantName, /*callSettingsTemplateObjects=*/ methodTemplateNames.stream() .map(n -> (Object) n) - .collect(Collectors.toList())); + .collect(Collectors.toList()), + Collections.emptyList()); } protected MethodDefinition createBatchingCallableMethod(TypeStore typeStore) { @@ -178,7 +181,8 @@ protected MethodDefinition createBatchingCallableMethod(TypeStore typeStore) { /*callSettingsVariantName=*/ methodVariantName, /*callSettingsTemplateObjects=*/ methodTemplateNames.stream() .map(n -> (Object) n) - .collect(Collectors.toList())); + .collect(Collectors.toList()), + Collections.emptyList()); } protected abstract MethodDefinition createOperationCallableMethod(TypeStore typeStore); @@ -191,7 +195,8 @@ protected MethodDefinition createGenericCallableMethod( String methodVariantName, List transportCallSettingsTemplateObjects, String callSettingsVariantName, - List callSettingsTemplateObjects) { + List callSettingsTemplateObjects, + List annotations) { String methodName = String.format("create%sCallable", methodVariantName); String callSettingsTypeName = String.format("%sCallSettings", callSettingsVariantName); @@ -261,6 +266,7 @@ protected MethodDefinition createGenericCallableMethod( .setName(methodName) .setArguments(arguments) .setReturnExpr(returnExpr) + .setAnnotations(annotations) .build(); } diff --git a/src/main/java/com/google/api/generator/gapic/composer/grpc/GrpcServiceCallableFactoryClassComposer.java b/src/main/java/com/google/api/generator/gapic/composer/grpc/GrpcServiceCallableFactoryClassComposer.java index 5698f5a443..b5aad75cb9 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/grpc/GrpcServiceCallableFactoryClassComposer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/grpc/GrpcServiceCallableFactoryClassComposer.java @@ -22,6 +22,7 @@ import com.google.longrunning.Operation; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.List; import java.util.stream.Collectors; @@ -72,7 +73,8 @@ protected MethodDefinition createUnaryCallableMethod(TypeStore typeStore) { /*callSettingsVariantName=*/ methodVariantName, /*callSettingsTemplateObjects=*/ methodTemplateNames.stream() .map(n -> (Object) n) - .collect(Collectors.toList())); + .collect(Collectors.toList()), + Collections.emptyList()); } protected MethodDefinition createPagedCallableMethod(TypeStore typeStore) { @@ -94,7 +96,8 @@ protected MethodDefinition createPagedCallableMethod(TypeStore typeStore) { /*callSettingsVariantName=*/ methodVariantName, /*callSettingsTemplateObjects=*/ methodTemplateNames.stream() .map(n -> (Object) n) - .collect(Collectors.toList())); + .collect(Collectors.toList()), + Collections.emptyList()); } @Override @@ -114,7 +117,8 @@ protected MethodDefinition createOperationCallableMethod(TypeStore typeStore) { /*callSettingsVariantName=*/ methodVariantName, /*callSettingsTemplateObjects=*/ methodTemplateNames.stream() .map(n -> (Object) n) - .collect(Collectors.toList())); + .collect(Collectors.toList()), + Collections.emptyList()); } private MethodDefinition createBidiStreamingCallableMethod(TypeStore typeStore) { @@ -134,7 +138,8 @@ private MethodDefinition createBidiStreamingCallableMethod(TypeStore typeStore) /*callSettingsVariantName=*/ "Streaming", /*callSettingsTemplateObjects=*/ methodTemplateNames.stream() .map(n -> (Object) n) - .collect(Collectors.toList())); + .collect(Collectors.toList()), + Collections.emptyList()); } private MethodDefinition createServerStreamingCallableMethod(TypeStore typeStore) { @@ -154,7 +159,8 @@ private MethodDefinition createServerStreamingCallableMethod(TypeStore typeStore /*callSettingsVariantName=*/ methodVariantName, /*callSettingsTemplateObjects=*/ methodTemplateNames.stream() .map(n -> (Object) n) - .collect(Collectors.toList())); + .collect(Collectors.toList()), + Collections.emptyList()); } private MethodDefinition createClientStreamingCallableMethod(TypeStore typeStore) { @@ -174,6 +180,7 @@ private MethodDefinition createClientStreamingCallableMethod(TypeStore typeStore /*callSettingsVariantName=*/ "Streaming", /*callSettingsTemplateObjects=*/ methodTemplateNames.stream() .map(n -> (Object) n) - .collect(Collectors.toList())); + .collect(Collectors.toList()), + Collections.emptyList()); } } diff --git a/src/main/java/com/google/api/generator/gapic/composer/rest/HttpJsonServiceCallableFactoryClassComposer.java b/src/main/java/com/google/api/generator/gapic/composer/rest/HttpJsonServiceCallableFactoryClassComposer.java index 435f9e63b3..65b9fb368f 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/rest/HttpJsonServiceCallableFactoryClassComposer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/rest/HttpJsonServiceCallableFactoryClassComposer.java @@ -16,12 +16,14 @@ import com.google.api.gax.core.BackgroundResource; import com.google.api.gax.httpjson.ApiMessage; +import com.google.api.generator.engine.ast.AnnotationNode; import com.google.api.generator.engine.ast.ConcreteReference; import com.google.api.generator.engine.ast.MethodDefinition; import com.google.api.generator.engine.ast.TypeNode; import com.google.api.generator.engine.ast.ValueExpr; import com.google.api.generator.gapic.composer.common.AbstractServiceCallableFactoryClassComposer; import com.google.api.generator.gapic.composer.store.TypeStore; +import com.google.api.generator.gapic.model.Service; import java.util.Arrays; import java.util.List; import java.util.stream.Collectors; @@ -44,6 +46,18 @@ public static HttpJsonServiceCallableFactoryClassComposer instance() { return INSTANCE; } + @Override + protected List createClassAnnotations(Service service, TypeStore typeStore) { + List annotations = super.createClassAnnotations(service, typeStore); + // Always add @BetaApi annotation to the generated CallableFactory for now. It is a public class + // for technical reasons, end users are not expected to interact with it, but it may change + // when we add LRO support, that is why making it @BetaApi for now. + if (annotations.stream().noneMatch(a -> a.type().equals(typeStore.get("BetaApi")))) { + annotations.add(AnnotationNode.withType(typeStore.get("BetaApi"))); + } + return annotations; + } + @Override protected List createClassImplements(TypeStore typeStore) { return Arrays.asList( @@ -63,6 +77,15 @@ protected MethodDefinition createOperationCallableMethod(TypeStore typeStore) { String responseTemplateName = "ResponseT"; List methodTemplateNames = Arrays.asList(requestTemplateName, responseTemplateName, "MetadataT"); + + // Always add @BetaApi annotation to the generated createOperationCallable()method for now, + // until LRO is fully implemented. + AnnotationNode betaAnnotation = + AnnotationNode.withTypeAndDescription( + typeStore.get("BetaApi"), + "The surface for long-running operations is not stable yet and may change in the" + + " future."); + MethodDefinition method = createGenericCallableMethod( typeStore, @@ -75,7 +98,8 @@ protected MethodDefinition createOperationCallableMethod(TypeStore typeStore) { /*callSettingsVariantName=*/ methodVariantName, /*callSettingsTemplateObjects=*/ methodTemplateNames.stream() .map(n -> (Object) n) - .collect(Collectors.toList())); + .collect(Collectors.toList()), + Arrays.asList(betaAnnotation)); return method.toBuilder().setReturnExpr(ValueExpr.createNullExpr()).build(); } } diff --git a/src/test/java/com/google/api/generator/gapic/composer/rest/goldens/HttpJsonComplianceCallableFactory.golden b/src/test/java/com/google/api/generator/gapic/composer/rest/goldens/HttpJsonComplianceCallableFactory.golden index 2ddcef8135..7d6032778d 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/rest/goldens/HttpJsonComplianceCallableFactory.golden +++ b/src/test/java/com/google/api/generator/gapic/composer/rest/goldens/HttpJsonComplianceCallableFactory.golden @@ -54,6 +54,8 @@ public class HttpJsonComplianceCallableFactory httpJsonCallSettings, callSettings, clientContext); } + @BetaApi( + "The surface for long-running operations is not stable yet and may change in the future.") @Override public OperationCallable createOperationCallable( diff --git a/test/integration/goldens/compute/com/google/cloud/compute/v1/stub/HttpJsonAddressesCallableFactory.java b/test/integration/goldens/compute/com/google/cloud/compute/v1/stub/HttpJsonAddressesCallableFactory.java index 98f81444ec..afa275a6f5 100644 --- a/test/integration/goldens/compute/com/google/cloud/compute/v1/stub/HttpJsonAddressesCallableFactory.java +++ b/test/integration/goldens/compute/com/google/cloud/compute/v1/stub/HttpJsonAddressesCallableFactory.java @@ -16,6 +16,7 @@ package com.google.cloud.compute.v1.stub; +import com.google.api.core.BetaApi; import com.google.api.gax.core.BackgroundResource; import com.google.api.gax.httpjson.ApiMessage; import com.google.api.gax.httpjson.HttpJsonCallSettings; @@ -37,6 +38,7 @@ *

This class is for advanced usage. */ @Generated("by gapic-generator-java") +@BetaApi public class HttpJsonAddressesCallableFactory implements HttpJsonStubCallableFactory { @@ -68,6 +70,8 @@ public UnaryCallable createBatchingCa httpJsonCallSettings, callSettings, clientContext); } + @BetaApi( + "The surface for long-running operations is not stable yet and may change in the future.") @Override public OperationCallable createOperationCallable( diff --git a/test/integration/goldens/compute/com/google/cloud/compute/v1/stub/HttpJsonRegionOperationsCallableFactory.java b/test/integration/goldens/compute/com/google/cloud/compute/v1/stub/HttpJsonRegionOperationsCallableFactory.java index 2694d1aa62..6dac14a84e 100644 --- a/test/integration/goldens/compute/com/google/cloud/compute/v1/stub/HttpJsonRegionOperationsCallableFactory.java +++ b/test/integration/goldens/compute/com/google/cloud/compute/v1/stub/HttpJsonRegionOperationsCallableFactory.java @@ -16,6 +16,7 @@ package com.google.cloud.compute.v1.stub; +import com.google.api.core.BetaApi; import com.google.api.gax.core.BackgroundResource; import com.google.api.gax.httpjson.ApiMessage; import com.google.api.gax.httpjson.HttpJsonCallSettings; @@ -37,6 +38,7 @@ *

This class is for advanced usage. */ @Generated("by gapic-generator-java") +@BetaApi public class HttpJsonRegionOperationsCallableFactory implements HttpJsonStubCallableFactory { @@ -68,6 +70,8 @@ public UnaryCallable createBatchingCa httpJsonCallSettings, callSettings, clientContext); } + @BetaApi( + "The surface for long-running operations is not stable yet and may change in the future.") @Override public OperationCallable createOperationCallable( From 5db2998a2e67ed34ccf29a1ef1d5c0dcf6f5c0de Mon Sep 17 00:00:00 2001 From: vam-google Date: Mon, 12 Jul 2021 14:51:18 -0700 Subject: [PATCH 2/2] address PR feedback --- ...ctServiceCallableFactoryClassComposer.java | 30 +++++++++++++++---- ...pcServiceCallableFactoryClassComposer.java | 19 ++++-------- ...onServiceCallableFactoryClassComposer.java | 6 +++- 3 files changed, 35 insertions(+), 20 deletions(-) diff --git a/src/main/java/com/google/api/generator/gapic/composer/common/AbstractServiceCallableFactoryClassComposer.java b/src/main/java/com/google/api/generator/gapic/composer/common/AbstractServiceCallableFactoryClassComposer.java index a76131b31f..b3f4c7ee74 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/common/AbstractServiceCallableFactoryClassComposer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/common/AbstractServiceCallableFactoryClassComposer.java @@ -137,8 +137,7 @@ protected MethodDefinition createUnaryCallableMethod(TypeStore typeStore) { /*callSettingsVariantName=*/ methodVariantName, /*callSettingsTemplateObjects=*/ methodTemplateNames.stream() .map(n -> (Object) n) - .collect(Collectors.toList()), - Collections.emptyList()); + .collect(Collectors.toList())); } protected MethodDefinition createPagedCallableMethod(TypeStore typeStore) { @@ -160,8 +159,7 @@ protected MethodDefinition createPagedCallableMethod(TypeStore typeStore) { /*callSettingsVariantName=*/ methodVariantName, /*callSettingsTemplateObjects=*/ methodTemplateNames.stream() .map(n -> (Object) n) - .collect(Collectors.toList()), - Collections.emptyList()); + .collect(Collectors.toList())); } protected MethodDefinition createBatchingCallableMethod(TypeStore typeStore) { @@ -181,12 +179,32 @@ protected MethodDefinition createBatchingCallableMethod(TypeStore typeStore) { /*callSettingsVariantName=*/ methodVariantName, /*callSettingsTemplateObjects=*/ methodTemplateNames.stream() .map(n -> (Object) n) - .collect(Collectors.toList()), - Collections.emptyList()); + .collect(Collectors.toList())); } protected abstract MethodDefinition createOperationCallableMethod(TypeStore typeStore); + protected MethodDefinition createGenericCallableMethod( + TypeStore typeStore, + List methodTemplateNames, + String returnCallableKindName, + List returnCallableTemplateNames, + String methodVariantName, + List transportCallSettingsTemplateObjects, + String callSettingsVariantName, + List callSettingsTemplateObjects) { + return createGenericCallableMethod( + typeStore, + methodTemplateNames, + returnCallableKindName, + returnCallableTemplateNames, + methodVariantName, + transportCallSettingsTemplateObjects, + callSettingsVariantName, + callSettingsTemplateObjects, + Collections.emptyList()); + } + protected MethodDefinition createGenericCallableMethod( TypeStore typeStore, List methodTemplateNames, diff --git a/src/main/java/com/google/api/generator/gapic/composer/grpc/GrpcServiceCallableFactoryClassComposer.java b/src/main/java/com/google/api/generator/gapic/composer/grpc/GrpcServiceCallableFactoryClassComposer.java index b5aad75cb9..5698f5a443 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/grpc/GrpcServiceCallableFactoryClassComposer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/grpc/GrpcServiceCallableFactoryClassComposer.java @@ -22,7 +22,6 @@ import com.google.longrunning.Operation; import java.util.ArrayList; import java.util.Arrays; -import java.util.Collections; import java.util.List; import java.util.stream.Collectors; @@ -73,8 +72,7 @@ protected MethodDefinition createUnaryCallableMethod(TypeStore typeStore) { /*callSettingsVariantName=*/ methodVariantName, /*callSettingsTemplateObjects=*/ methodTemplateNames.stream() .map(n -> (Object) n) - .collect(Collectors.toList()), - Collections.emptyList()); + .collect(Collectors.toList())); } protected MethodDefinition createPagedCallableMethod(TypeStore typeStore) { @@ -96,8 +94,7 @@ protected MethodDefinition createPagedCallableMethod(TypeStore typeStore) { /*callSettingsVariantName=*/ methodVariantName, /*callSettingsTemplateObjects=*/ methodTemplateNames.stream() .map(n -> (Object) n) - .collect(Collectors.toList()), - Collections.emptyList()); + .collect(Collectors.toList())); } @Override @@ -117,8 +114,7 @@ protected MethodDefinition createOperationCallableMethod(TypeStore typeStore) { /*callSettingsVariantName=*/ methodVariantName, /*callSettingsTemplateObjects=*/ methodTemplateNames.stream() .map(n -> (Object) n) - .collect(Collectors.toList()), - Collections.emptyList()); + .collect(Collectors.toList())); } private MethodDefinition createBidiStreamingCallableMethod(TypeStore typeStore) { @@ -138,8 +134,7 @@ private MethodDefinition createBidiStreamingCallableMethod(TypeStore typeStore) /*callSettingsVariantName=*/ "Streaming", /*callSettingsTemplateObjects=*/ methodTemplateNames.stream() .map(n -> (Object) n) - .collect(Collectors.toList()), - Collections.emptyList()); + .collect(Collectors.toList())); } private MethodDefinition createServerStreamingCallableMethod(TypeStore typeStore) { @@ -159,8 +154,7 @@ private MethodDefinition createServerStreamingCallableMethod(TypeStore typeStore /*callSettingsVariantName=*/ methodVariantName, /*callSettingsTemplateObjects=*/ methodTemplateNames.stream() .map(n -> (Object) n) - .collect(Collectors.toList()), - Collections.emptyList()); + .collect(Collectors.toList())); } private MethodDefinition createClientStreamingCallableMethod(TypeStore typeStore) { @@ -180,7 +174,6 @@ private MethodDefinition createClientStreamingCallableMethod(TypeStore typeStore /*callSettingsVariantName=*/ "Streaming", /*callSettingsTemplateObjects=*/ methodTemplateNames.stream() .map(n -> (Object) n) - .collect(Collectors.toList()), - Collections.emptyList()); + .collect(Collectors.toList())); } } diff --git a/src/main/java/com/google/api/generator/gapic/composer/rest/HttpJsonServiceCallableFactoryClassComposer.java b/src/main/java/com/google/api/generator/gapic/composer/rest/HttpJsonServiceCallableFactoryClassComposer.java index 65b9fb368f..f9c4045cb9 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/rest/HttpJsonServiceCallableFactoryClassComposer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/rest/HttpJsonServiceCallableFactoryClassComposer.java @@ -52,6 +52,8 @@ protected List createClassAnnotations(Service service, TypeStore // Always add @BetaApi annotation to the generated CallableFactory for now. It is a public class // for technical reasons, end users are not expected to interact with it, but it may change // when we add LRO support, that is why making it @BetaApi for now. + // + // Remove the @BetaApi annotation once the LRO feature is fully implemented and stabilized. if (annotations.stream().noneMatch(a -> a.type().equals(typeStore.get("BetaApi")))) { annotations.add(AnnotationNode.withType(typeStore.get("BetaApi"))); } @@ -78,8 +80,10 @@ protected MethodDefinition createOperationCallableMethod(TypeStore typeStore) { List methodTemplateNames = Arrays.asList(requestTemplateName, responseTemplateName, "MetadataT"); - // Always add @BetaApi annotation to the generated createOperationCallable()method for now, + // Always add @BetaApi annotation to the generated createOperationCallable() method for now, // until LRO is fully implemented. + // + // Remove the @BetaApi annotation once the LRO feature is fully implemented and stabilized. AnnotationNode betaAnnotation = AnnotationNode.withTypeAndDescription( typeStore.get("BetaApi"),