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

feat: Add callable getters for non-eligible or non-enabled REST methods #1211

Merged
merged 18 commits into from
Jan 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@
import com.google.api.generator.gapic.model.Message;
import com.google.api.generator.gapic.model.Method;
import com.google.api.generator.gapic.model.Service;
import com.google.api.generator.gapic.model.Transport;
import com.google.api.generator.gapic.utils.JavaStyle;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
Expand Down Expand Up @@ -89,7 +90,7 @@ public abstract class AbstractTransportServiceStubClassComposer implements Class
private static final String BACKGROUND_RESOURCES_MEMBER_NAME = "backgroundResources";
private static final String CALLABLE_NAME = "Callable";
private static final String CALLABLE_FACTORY_MEMBER_NAME = "callableFactory";
private static final String CALLABLE_CLASS_MEMBER_PATTERN = "%sCallable";
protected static final String CALLABLE_CLASS_MEMBER_PATTERN = "%sCallable";
lqiu96 marked this conversation as resolved.
Show resolved Hide resolved
private static final String OPERATION_CALLABLE_CLASS_MEMBER_PATTERN = "%sOperationCallable";
private static final String OPERATION_CALLABLE_NAME = "OperationCallable";
// private static final String OPERATIONS_STUB_MEMBER_NAME = "operationsStub";
Expand Down Expand Up @@ -126,7 +127,8 @@ private static TypeStore createStaticTypes() {
RequestParamsExtractor.class,
ServerStreamingCallable.class,
TimeUnit.class,
UnaryCallable.class);
UnaryCallable.class,
UnsupportedOperationException.class);
return new TypeStore(concreteClazzes);
}

Expand Down Expand Up @@ -190,6 +192,19 @@ public GapicClass generate(GapicContext context, Service service) {
messageTypes,
context.restNumericEnumsEnabled());

List<MethodDefinition> methodDefinitions =
createClassMethods(
context,
service,
typeStore,
classMemberVarExprs,
callableClassMemberVarExprs,
protoMethodNameToDescriptorVarExprs,
classStatements);
methodDefinitions.addAll(
createStubOverrideMethods(
classMemberVarExprs.get(BACKGROUND_RESOURCES_MEMBER_NAME), service));

StubCommentComposer commentComposer =
new StubCommentComposer(getTransportContext().transportNames().get(0));

Expand All @@ -204,22 +219,14 @@ public GapicClass generate(GapicContext context, Service service) {
.setName(className)
.setExtendsType(
typeStore.get(getTransportContext().classNames().getServiceStubClassName(service)))
.setMethods(
createClassMethods(
context,
service,
typeStore,
classMemberVarExprs,
callableClassMemberVarExprs,
protoMethodNameToDescriptorVarExprs,
classStatements))
.setMethods(methodDefinitions)
.setStatements(classStatements)
.build();
return GapicClass.create(kind, classDef);
}

protected boolean isSupportedMethod(Method method) {
return true;
protected Transport getTransport() {
return Transport.GRPC;
}

protected abstract Statement createMethodDescriptorVariableDecl(
Expand Down Expand Up @@ -307,7 +314,7 @@ protected List<Statement> createMethodDescriptorVariableDecls(
Map<String, Message> messageTypes,
boolean restNumericEnumsEnabled) {
return service.methods().stream()
.filter(this::isSupportedMethod)
.filter(x -> x.isSupportedByTransport(getTransport()))
.map(
m ->
createMethodDescriptorVariableDecl(
Expand Down Expand Up @@ -336,7 +343,7 @@ private static List<Statement> createClassMemberFieldDeclarations(
protected Map<String, VariableExpr> createProtoMethodNameToDescriptorClassMembers(
Service service, Class<?> descriptorClass) {
return service.methods().stream()
.filter(this::isSupportedMethod)
.filter(x -> x.isSupportedByTransport(getTransport()))
.collect(
Collectors.toMap(
Method::name,
Expand Down Expand Up @@ -368,7 +375,7 @@ private Map<String, VariableExpr> createCallableClassMembers(
Map<String, VariableExpr> callableClassMembers = new LinkedHashMap<>();
// Using a for-loop because the output cardinality is not a 1:1 mapping to the input set.
for (Method protoMethod : service.methods()) {
if (!isSupportedMethod(protoMethod)) {
if (!protoMethod.isSupportedByTransport(getTransport())) {
continue;
}
String javaStyleProtoMethodName = JavaStyle.toLowerCamelCase(protoMethod.name());
Expand Down Expand Up @@ -470,9 +477,6 @@ protected List<MethodDefinition> createClassMethods(
service,
classMemberVarExprs.get(getTransportContext().transportOperationsStubNames().get(0))));
javaMethods.addAll(createCallableGetterMethods(callableClassMemberVarExprs));
javaMethods.addAll(
createStubOverrideMethods(
classMemberVarExprs.get(BACKGROUND_RESOURCES_MEMBER_NAME), service));
return javaMethods;
}

Expand Down Expand Up @@ -660,7 +664,7 @@ protected List<MethodDefinition> createConstructorMethods(
// Transport settings local variables.
Map<String, VariableExpr> javaStyleMethodNameToTransportSettingsVarExprs =
service.methods().stream()
.filter(this::isSupportedMethod)
.filter(x -> x.isSupportedByTransport(getTransport()))
.collect(
Collectors.toMap(
m -> JavaStyle.toLowerCamelCase(m.name()),
Expand All @@ -684,7 +688,7 @@ protected List<MethodDefinition> createConstructorMethods(

secondCtorExprs.addAll(
service.methods().stream()
.filter(this::isSupportedMethod)
.filter(x -> x.isSupportedByTransport(getTransport()))
.map(
m ->
createTransportSettingsInitExpr(
Expand Down Expand Up @@ -1055,7 +1059,7 @@ private List<MethodDefinition> createStubOverrideMethods(

private boolean checkOperationPollingMethod(Service service) {
return service.methods().stream()
.filter(this::isSupportedMethod)
.filter(x -> x.isSupportedByTransport(getTransport()))
.anyMatch(Method::isOperationPollingMethod);
}

Expand Down Expand Up @@ -1094,7 +1098,7 @@ private TypeStore createDynamicTypes(Service service, String stubPakkage) {
typeStore.putAll(
service.pakkage(),
service.methods().stream()
.filter(this::isSupportedMethod)
.filter(x -> x.isSupportedByTransport(getTransport()))
.filter(Method::isPaged)
.map(m -> String.format(PAGED_RESPONSE_TYPE_NAME_PATTERN, m.name()))
.collect(Collectors.toList()),
Expand All @@ -1103,7 +1107,7 @@ private TypeStore createDynamicTypes(Service service, String stubPakkage) {
return typeStore;
}

private static TypeNode getCallableType(Method protoMethod) {
protected static TypeNode getCallableType(Method protoMethod) {
TypeNode callableType = FIXED_TYPESTORE.get("UnaryCallable");
switch (protoMethod.stream()) {
case CLIENT:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,19 +45,21 @@
import com.google.api.generator.engine.ast.Statement;
import com.google.api.generator.engine.ast.StringObjectValue;
import com.google.api.generator.engine.ast.ThisObjectValue;
import com.google.api.generator.engine.ast.ThrowExpr;
import com.google.api.generator.engine.ast.TypeNode;
import com.google.api.generator.engine.ast.ValueExpr;
import com.google.api.generator.engine.ast.VaporReference;
import com.google.api.generator.engine.ast.Variable;
import com.google.api.generator.engine.ast.VariableExpr;
import com.google.api.generator.gapic.composer.common.AbstractTransportServiceStubClassComposer;
import com.google.api.generator.gapic.composer.store.TypeStore;
import com.google.api.generator.gapic.model.GapicContext;
import com.google.api.generator.gapic.model.HttpBindings.HttpBinding;
import com.google.api.generator.gapic.model.Message;
import com.google.api.generator.gapic.model.Method;
import com.google.api.generator.gapic.model.Method.Stream;
import com.google.api.generator.gapic.model.OperationResponse;
import com.google.api.generator.gapic.model.Service;
import com.google.api.generator.gapic.model.Transport;
import com.google.api.generator.gapic.utils.JavaStyle;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.BiMap;
Expand Down Expand Up @@ -115,10 +117,9 @@ private static TypeStore createStaticTypes() {
TypeRegistry.class));
}

protected boolean isSupportedMethod(Method method) {
return method.httpBindings() != null
&& method.stream() != Stream.BIDI
&& method.stream() != Stream.CLIENT;
@Override
protected Transport getTransport() {
return Transport.REST;
}

@Override
Expand Down Expand Up @@ -1248,4 +1249,56 @@ protected List<Statement> createTypeRegistry(Service service) {
.setValueExpr(typeRegistryBuilderExpr)
.build()));
}

@Override
protected List<MethodDefinition> createClassMethods(
GapicContext context,
Service service,
TypeStore typeStore,
Map<String, VariableExpr> classMemberVarExprs,
Map<String, VariableExpr> callableClassMemberVarExprs,
Map<String, VariableExpr> protoMethodNameToDescriptorVarExprs,
List<Statement> classStatements) {
List<MethodDefinition> javaMethods = new ArrayList<>();
javaMethods.addAll(
super.createClassMethods(
context,
service,
typeStore,
classMemberVarExprs,
callableClassMemberVarExprs,
protoMethodNameToDescriptorVarExprs,
classStatements));
javaMethods.addAll(createInvalidClassMethods(service));
return javaMethods;
}

private List<MethodDefinition> createInvalidClassMethods(Service service) {
List<MethodDefinition> methodDefinitions = new ArrayList<>();
for (Method protoMethod : service.methods()) {
if (protoMethod.isSupportedByTransport(getTransport())) {
continue;
}
String javaStyleProtoMethodName = JavaStyle.toLowerCamelCase(protoMethod.name());
String callableName = String.format(CALLABLE_CLASS_MEMBER_PATTERN, javaStyleProtoMethodName);
methodDefinitions.add(
MethodDefinition.builder()
.setIsOverride(true)
.setScope(ScopeNode.PUBLIC)
.setName(callableName)
.setReturnType(getCallableType(protoMethod))
.setBody(
Arrays.asList(
ExprStatement.withExpr(
ThrowExpr.builder()
.setType(FIXED_TYPESTORE.get("UnsupportedOperationException"))
.setMessageExpr(
String.format(
"Not implemented: %s(). %s transport is not implemented for this method yet.",
callableName, getTransport()))
.build())))
.build());
}
return methodDefinitions;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,25 @@ public boolean isOperationPollingMethod() {
return operationPollingMethod();
}

/**
* Determines if method is both eligible and enabled for the Transport. GRPC+REST Transport is not
* supported as each transport's sub composers will invoke this method the specific transport
* (GRPC or REST)
*
* @param transport Expects either GRPC or REST Transport
* @return boolean if method should be generated for the transport
*/
public boolean isSupportedByTransport(Transport transport) {
blakeli0 marked this conversation as resolved.
Show resolved Hide resolved
if (transport == Transport.REST) {
return httpBindings() != null && stream() != Stream.BIDI && stream() != Stream.CLIENT;
} else if (transport == Transport.GRPC) {
return true;
} else {
throw new IllegalArgumentException(
String.format("Invalid Transport: %s. Expecting GRPC or REST", transport.name()));
}
}

public abstract Builder toBuilder();

public static Builder builder() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import com.google.api.gax.httpjson.ProtoMessageResponseParser;
import com.google.api.gax.httpjson.ProtoRestSerializer;
import com.google.api.gax.httpjson.longrunning.stub.HttpJsonOperationsStub;
import com.google.api.gax.longrunning.OperationSnapshot;
import com.google.api.gax.rpc.BidiStreamingCallable;
import com.google.api.gax.rpc.ClientContext;
import com.google.api.gax.rpc.OperationCallable;
import com.google.api.gax.rpc.ServerStreamingCallable;
Expand Down Expand Up @@ -544,6 +545,18 @@ public class HttpJsonEchoStub extends EchoStub {
return nestedBindingCallable;
}

@Override
public BidiStreamingCallable<EchoRequest, EchoResponse> chatCallable() {
throw new UnsupportedOperationException(
"Not implemented: chatCallable(). REST transport is not implemented for this method yet.");
}

@Override
public UnaryCallable<EchoRequest, EchoResponse> noBindingCallable() {
throw new UnsupportedOperationException(
"Not implemented: noBindingCallable(). REST transport is not implemented for this method yet.");
}

@Override
public final void close() {
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package com.google.api.generator.gapic.model;

import static com.google.common.truth.Truth.assertThat;
import static org.junit.Assert.assertThrows;

import com.google.api.generator.engine.ast.TypeNode;
import com.google.api.generator.gapic.model.HttpBindings.HttpBinding;
Expand Down Expand Up @@ -112,4 +113,61 @@ public void shouldSetParamsExtractor_shouldReturnFalseIfHasNoHttpBindingsAndNoRo
Method method = METHOD.toBuilder().setHttpBindings(null).setRoutingHeaderRule(null).build();
assertThat(method.shouldSetParamsExtractor()).isFalse();
}

@Test
public void
isSupportedByTransport_shouldReturnTrueIfHasHttpBindingsAndIsRESTEligibleForRESTTransport() {
Method methodNoStreaming =
METHOD.toBuilder().setHttpBindings(HTTP_BINDINGS).setStream(Method.Stream.NONE).build();
assertThat(methodNoStreaming.isSupportedByTransport(Transport.REST)).isTrue();
Method methodServerSideStreaming =
METHOD.toBuilder().setHttpBindings(HTTP_BINDINGS).setStream(Method.Stream.SERVER).build();
assertThat(methodServerSideStreaming.isSupportedByTransport(Transport.REST)).isTrue();
}

@Test
public void isSupportedByTransport_shouldReturnFalseIfNoHttpBindingsForRESTTransport() {
Method methodNoStreaming =
METHOD.toBuilder().setHttpBindings(null).setStream(Method.Stream.NONE).build();
assertThat(methodNoStreaming.isSupportedByTransport(Transport.REST)).isFalse();
Method methodServerSideStreaming =
METHOD.toBuilder().setHttpBindings(null).setStream(Method.Stream.SERVER).build();
assertThat(methodServerSideStreaming.isSupportedByTransport(Transport.REST)).isFalse();
}

@Test
public void
isSupportedByTransport_shouldReturnFalseIfHasHttpBindingsAndIsNotRESTEligibleForRESTTransport() {
Method methodClientSideStreaming =
METHOD.toBuilder().setHttpBindings(HTTP_BINDINGS).setStream(Method.Stream.CLIENT).build();
assertThat(methodClientSideStreaming.isSupportedByTransport(Transport.REST)).isFalse();
Method methodBiDiStreaming =
METHOD.toBuilder().setHttpBindings(HTTP_BINDINGS).setStream(Method.Stream.BIDI).build();
assertThat(methodBiDiStreaming.isSupportedByTransport(Transport.REST)).isFalse();
}

@Test
public void isSupportedByTransport_shouldReturnTrueForGRPCTransport() {
Method methodNoStreaming =
METHOD.toBuilder().setHttpBindings(HTTP_BINDINGS).setStream(Method.Stream.NONE).build();
assertThat(methodNoStreaming.isSupportedByTransport(Transport.GRPC)).isTrue();
Method methodBiDiStreaming =
METHOD.toBuilder().setHttpBindings(HTTP_BINDINGS).setStream(Method.Stream.BIDI).build();
assertThat(methodBiDiStreaming.isSupportedByTransport(Transport.GRPC)).isTrue();
Method methodNoStreamingNoHttpBindings =
METHOD.toBuilder().setStream(Method.Stream.NONE).build();
assertThat(methodNoStreamingNoHttpBindings.isSupportedByTransport(Transport.GRPC)).isTrue();
Method methodBiDiStreamingNoHttpBindings =
METHOD.toBuilder().setStream(Method.Stream.BIDI).build();
assertThat(methodBiDiStreamingNoHttpBindings.isSupportedByTransport(Transport.GRPC)).isTrue();
}

@Test
public void isSupportedByTransport_shouldThrowExceptionIfPassedGRPCRESTTransport() {
Method methodClientStreaming =
METHOD.toBuilder().setHttpBindings(HTTP_BINDINGS).setStream(Method.Stream.CLIENT).build();
assertThrows(
IllegalArgumentException.class,
() -> methodClientStreaming.isSupportedByTransport(Transport.GRPC_REST));
}
}
Loading