Skip to content

Commit

Permalink
fix: Fix BetaApi annotaiton usage for REST transport and clean `Bet…
Browse files Browse the repository at this point in the history
…aApi` 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.
  • Loading branch information
vam-google committed May 9, 2022
1 parent a5bfed2 commit d22b966
Show file tree
Hide file tree
Showing 65 changed files with 493 additions and 184 deletions.
1 change: 1 addition & 0 deletions BUILD.bazel
Expand Up @@ -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",
Expand Down
Expand Up @@ -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);
Expand Down
Expand Up @@ -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)
Expand Down
Expand Up @@ -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)
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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(
Expand All @@ -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"))
Expand Down
Expand Up @@ -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;

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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(
Expand All @@ -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)
Expand Down
Expand Up @@ -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 =
Expand All @@ -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);
}

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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)
Expand Down
Expand Up @@ -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()
Expand Down
Expand Up @@ -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();
Expand Down Expand Up @@ -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);

Expand Down
Expand Up @@ -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))
Expand Down
Expand Up @@ -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)
Expand Down

0 comments on commit d22b966

Please sign in to comment.