Skip to content

Commit

Permalink
feat: Do not generate Service REST code if there are no matching RPC …
Browse files Browse the repository at this point in the history
…in a Service (#1236)

* feat: Add callable getter methods for REST

* chore: Update showcase tests

* chore: Update error message

* feat: Move httpjson specific logic to sub composer

* feat: Move method supported logic to Method

* feat: Move method supported logic to Method

* chore: Format the files

* chore: Cleanup Abstract composer

* chore: Move code to httpjson composer

* chore: Resolve code smell

* feat: Use generic error message

* chore: Fix format issues

* feat: Add tests for Method.isSupportedByTransport()

* feat: Resolve PR comments

* feat: Update back to private

* feat: Update error message

* feat: Update javadoc comment

* feat: Do not generate Service REST code if there are no matching RPC in a Service

* chore: Pull in latest part 1 changes

* fix: Update Kind to NON_GENERATED

* feat: Do not generate secondary Transport sample if no rest code is generated

* feat: Do not generate httpjson tests if no matching RPCs

* chore: Run mvn test -DupdateUnitGoldens

* chore: Update formatting

* chore: Add apigeeconnect to test REGAPIC

* chore: Create directory for new modules

* chore: Update googleapis commit to a later version for grpc+rest enabled APIs

* chore Add in goldens for apigeeconnect

* chore: Add comments for funcs

* chore: Refactor ServiceClientClassComposer for GRPC_REST

* chore: Remove unused import

* chore: Remove unnecessary if/else check

* chore: Fix transport sample name

* chore: Update apigeeconnect IT goldens

* chore: Update asset IT goldens

* chore: Update asset goldens

* chore: Update credentials IT goldens

* chore: Update library IT goldens

* chore: Update redis IT goldens

* chore: Fix linting issues

* chore: Add showcase extended proto framework

* chore: Use seperate Bazel rules for showcase extended

* chore: Seperate GRPC jar into separate jobs

* chore: Update bazel build file

* Apply suggestions from code review

Co-authored-by: Burke Davison <40617934+burkedavison@users.noreply.github.com>

* chore: Update showcase tests

* chore: Resolve sonar comments

* chore: Update unit tests for wicked proto

* chore: Resolve format

* chore: Update sonar comments

* chore: Update PR comments

* chore: Update golden test cases

* chore: Revert back to original sample name

* chore: Update showcase and goldens

* chore: Leave framework but remove wicked proto from showcase extended

* chore: Update showcase project with extended info

* chore: Remove comment

* chore: Use TransportContext instead of duplicate transportName()

* chore: Fix formatting issues

* chore: Remove the changed spacing in the file

* chore: Use transportContext where possible

* chore: Fix format issues

* chore: Fix compile issue

---------

Co-authored-by: Burke Davison <40617934+burkedavison@users.noreply.github.com>
  • Loading branch information
lqiu96 and burkedavison committed Jan 27, 2023
1 parent bdf12b0 commit 9c06bc9
Show file tree
Hide file tree
Showing 82 changed files with 6,445 additions and 144 deletions.
Expand Up @@ -62,6 +62,11 @@ protected TransportContext getTransportContext() {

@Override
public GapicClass generate(GapicContext context, Service service) {
// Do not generate the Callable Factory if there are no RPCs enabled for the Transport
if (!service.hasAnyEnabledMethodsForTransport(getTransportContext().transport())) {
return GapicClass.createNonGeneratedGapicClass();
}

TypeStore typeStore = createTypes(service);

String className =
Expand Down
Expand Up @@ -109,6 +109,11 @@ public GapicClass generate(GapicContext context, Service service) {
}

protected GapicClass generate(String className, GapicContext context, Service service) {
// Do not generate Client Test code for Transport if there are no matching RPCs for a Transport
if (!service.hasAnyEnabledMethodsForTransport(getTransportContext().transport())) {
return GapicClass.createNonGeneratedGapicClass();
}

Map<String, ResourceName> resourceNames = context.helperResourceNames();
String pakkage = service.pakkage();
TypeStore typeStore = new TypeStore();
Expand All @@ -131,10 +136,6 @@ protected GapicClass generate(String className, GapicContext context, Service se
return GapicClass.create(kind, classDef);
}

protected boolean isSupportedMethod(Method method) {
return true;
}

private List<AnnotationNode> createClassAnnotations() {
return Arrays.asList(
AnnotationNode.builder()
Expand Down Expand Up @@ -230,7 +231,7 @@ private List<MethodDefinition> createTestMethods(
Map<String, Message> messageTypes = context.messages();
List<MethodDefinition> javaMethods = new ArrayList<>();
for (Method method : service.methods()) {
if (!isSupportedMethod(method)) {
if (!method.isSupportedByTransport(getTransportContext().transport())) {
javaMethods.add(createUnsupportedTestMethod(method));
continue;
}
Expand Down
Expand Up @@ -385,6 +385,14 @@ private List<MethodDefinition> createDefaultGetterMethods(Service service, TypeS
while (providerBuilderNamesIt.hasNext()
&& channelProviderClassesIt.hasNext()
&& transportNamesIt.hasNext()) {
String providerBuilderName = providerBuilderNamesIt.next();
Class<?> channelProviderClass = channelProviderClassesIt.next();
String transportName = transportNamesIt.next();

if (!service.hasAnyEnabledMethodsForTransport(transportName)) {
continue;
}

List<AnnotationNode> annotations = ImmutableList.of();

// For clients supporting multiple transports (mainly grpc+rest case) make secondary transport
Expand All @@ -397,16 +405,13 @@ private List<MethodDefinition> createDefaultGetterMethods(Service service, TypeS
SettingsCommentComposer.DEFAULT_TRANSPORT_PROVIDER_BUILDER_METHOD_COMMENT;
if (getTransportContext().transportNames().size() > 1) {
commentStatement =
new SettingsCommentComposer(transportNamesIt.next())
.getTransportProviderBuilderMethodComment();
new SettingsCommentComposer(transportName).getTransportProviderBuilderMethodComment();
}

javaMethods.add(
methodMakerFn.apply(
methodStarterFn
.apply(
providerBuilderNamesIt.next(),
typeMakerFn.apply(channelProviderClassesIt.next()))
.apply(providerBuilderName, typeMakerFn.apply(channelProviderClass))
.setAnnotations(annotations),
commentStatement));
secondaryTransportProviderBuilder = true;
Expand Down
Expand Up @@ -243,7 +243,8 @@ protected MethodDefinition createDefaultCredentialsProviderBuilderMethod() {
.build();
}

protected List<MethodDefinition> createDefaultTransportTransportProviderBuilderMethods() {
protected List<MethodDefinition> createDefaultTransportTransportProviderBuilderMethods(
Service service) {
// Create the defaultGrpcTransportProviderBuilder method.
Iterator<Class<?>> providerClassIt =
getTransportContext().instantiatingChannelProviderClasses().iterator();
Expand All @@ -259,10 +260,19 @@ protected List<MethodDefinition> createDefaultTransportTransportProviderBuilderM
&& providerBuilderClassIt.hasNext()
&& builderNamesIt.hasNext()
&& transportNamesIt.hasNext()) {
Class<?> providerClass = providerClassIt.next();
Class<?> providerBuilderClass = providerBuilderClassIt.next();
String builderName = builderNamesIt.next();
String transportName = transportNamesIt.next();

if (!service.hasAnyEnabledMethodsForTransport(transportName)) {
continue;
}

TypeNode returnType =
TypeNode.withReference(ConcreteReference.withClazz(providerBuilderClassIt.next()));
TypeNode.withReference(ConcreteReference.withClazz(providerBuilderClass));
TypeNode channelProviderType =
TypeNode.withReference(ConcreteReference.withClazz(providerClassIt.next()));
TypeNode.withReference(ConcreteReference.withClazz(providerClass));

MethodInvocationExpr transportChannelProviderBuilderExpr =
MethodInvocationExpr.builder()
Expand All @@ -281,8 +291,7 @@ protected List<MethodDefinition> createDefaultTransportTransportProviderBuilderM
SettingsCommentComposer.DEFAULT_TRANSPORT_PROVIDER_BUILDER_METHOD_COMMENT;
if (getTransportContext().transportNames().size() > 1) {
commentStatement =
new SettingsCommentComposer(transportNamesIt.next())
.getTransportProviderBuilderMethodComment();
new SettingsCommentComposer(transportName).getTransportProviderBuilderMethodComment();
}

MethodDefinition method =
Expand All @@ -292,7 +301,7 @@ protected List<MethodDefinition> createDefaultTransportTransportProviderBuilderM
.setScope(ScopeNode.PUBLIC)
.setIsStatic(true)
.setReturnType(returnType)
.setName(builderNamesIt.next())
.setName(builderName)
.setReturnExpr(returnExpr)
.build();

Expand Down Expand Up @@ -1047,17 +1056,23 @@ private MethodDefinition createCreateStubMethod(Service service, TypeStore typeS
.setMethodName("getTransportName")
.build();

Iterator<String> transportNamesIt = getTransportContext().transportNames().iterator();
Iterator<TypeNode> channelTypesIt = getTransportContext().transportChannelTypes().iterator();
Iterator<String> getterNameIt = getTransportContext().transportGetterNames().iterator();
Iterator<String> serivceStubClassNameIt =
getTransportContext().classNames().getTransportServiceStubClassNames(service).iterator();

while (channelTypesIt.hasNext() && getterNameIt.hasNext()) {
String transportName = transportNamesIt.next();
TypeNode channelType = channelTypesIt.next();
String getterName = getterNameIt.next();
String serivceStubClassName = serivceStubClassNameIt.next();

Expr tRansportNameExpr =
if (!service.hasAnyEnabledMethodsForTransport(transportName)) {
continue;
}

Expr transportNameExpr =
MethodInvocationExpr.builder()
.setStaticReferenceType(channelType)
.setMethodName(getterName)
Expand All @@ -1067,7 +1082,7 @@ private MethodDefinition createCreateStubMethod(Service service, TypeStore typeS
MethodInvocationExpr.builder()
.setExprReferenceExpr(getTransportNameExpr)
.setMethodName("equals")
.setArguments(tRansportNameExpr)
.setArguments(transportNameExpr)
.setReturnType(TypeNode.BOOLEAN)
.build();

Expand Down Expand Up @@ -1191,7 +1206,7 @@ private List<MethodDefinition> createDefaultHelperAndGetterMethods(
.build());

javaMethods.add(createDefaultCredentialsProviderBuilderMethod());
javaMethods.addAll(createDefaultTransportTransportProviderBuilderMethods());
javaMethods.addAll(createDefaultTransportTransportProviderBuilderMethods(service));
javaMethods.add(createDefaultTransportChannelProviderMethod());
javaMethods.addAll(createApiClientHeaderProviderBuilderMethods(service, typeStore));

Expand Down Expand Up @@ -1424,7 +1439,7 @@ private List<MethodDefinition> createNestedClassMethods(
nestedClassMethods.addAll(
createNestedClassConstructorMethods(
service, serviceConfig, nestedMethodSettingsMemberVarExprs, typeStore));
nestedClassMethods.addAll(createNestedClassCreateDefaultMethods(typeStore));
nestedClassMethods.addAll(createNestedClassCreateDefaultMethods(service, typeStore));
nestedClassMethods.add(createNestedClassInitDefaultsMethod(service, serviceConfig, typeStore));
nestedClassMethods.add(createNestedClassApplyToAllUnaryMethodsMethod(superType, typeStore));
nestedClassMethods.add(createNestedClassUnaryMethodSettingsBuilderGetterMethod());
Expand Down Expand Up @@ -1762,14 +1777,19 @@ private static List<MethodDefinition> createNestedClassConstructorMethods(
return ctorMethods;
}

protected List<MethodDefinition> createNestedClassCreateDefaultMethods(TypeStore typeStore) {
return Collections.singletonList(
createNestedClassCreateDefaultMethod(
typeStore,
"createDefault",
"defaultTransportChannelProvider",
null,
"defaultApiClientHeaderProviderBuilder"));
protected List<MethodDefinition> createNestedClassCreateDefaultMethods(
Service service, TypeStore typeStore) {
if (service.hasAnyEnabledMethodsForTransport(getTransportContext().transport())) {
return Arrays.asList(
createNestedClassCreateDefaultMethod(
typeStore,
"createDefault",
"defaultTransportChannelProvider",
null,
"defaultApiClientHeaderProviderBuilder"));
} else {
return Collections.emptyList();
}
}

protected MethodDefinition createNestedClassCreateDefaultMethod(
Expand Down
Expand Up @@ -59,7 +59,6 @@
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 @@ -94,7 +93,7 @@ public abstract class AbstractTransportServiceStubClassComposer implements Class
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";
private static final String PAGED_CALLABLE_NAME = "PagedCallable";
protected static final String PAGED_CALLABLE_NAME = "PagedCallable";

protected static final TypeStore FIXED_TYPESTORE = createStaticTypes();

Expand Down Expand Up @@ -134,6 +133,10 @@ private static TypeStore createStaticTypes() {

@Override
public GapicClass generate(GapicContext context, Service service) {
if (!service.hasAnyEnabledMethodsForTransport(getTransportContext().transport())) {
return GapicClass.createNonGeneratedGapicClass();
}

String pakkage = service.pakkage() + ".stub";
TypeStore typeStore = createDynamicTypes(service, pakkage);
String className = getTransportContext().classNames().getTransportServiceStubClassName(service);
Expand Down Expand Up @@ -225,10 +228,6 @@ public GapicClass generate(GapicContext context, Service service) {
return GapicClass.create(kind, classDef);
}

protected Transport getTransport() {
return Transport.GRPC;
}

protected abstract Statement createMethodDescriptorVariableDecl(
Service service,
Method protoMethod,
Expand Down Expand Up @@ -314,7 +313,7 @@ protected List<Statement> createMethodDescriptorVariableDecls(
Map<String, Message> messageTypes,
boolean restNumericEnumsEnabled) {
return service.methods().stream()
.filter(x -> x.isSupportedByTransport(getTransport()))
.filter(x -> x.isSupportedByTransport(getTransportContext().transport()))
.map(
m ->
createMethodDescriptorVariableDecl(
Expand Down Expand Up @@ -343,7 +342,7 @@ private static List<Statement> createClassMemberFieldDeclarations(
protected Map<String, VariableExpr> createProtoMethodNameToDescriptorClassMembers(
Service service, Class<?> descriptorClass) {
return service.methods().stream()
.filter(x -> x.isSupportedByTransport(getTransport()))
.filter(x -> x.isSupportedByTransport(getTransportContext().transport()))
.collect(
Collectors.toMap(
Method::name,
Expand Down Expand Up @@ -375,7 +374,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 (!protoMethod.isSupportedByTransport(getTransport())) {
if (!protoMethod.isSupportedByTransport(getTransportContext().transport())) {
continue;
}
String javaStyleProtoMethodName = JavaStyle.toLowerCamelCase(protoMethod.name());
Expand Down Expand Up @@ -664,7 +663,7 @@ protected List<MethodDefinition> createConstructorMethods(
// Transport settings local variables.
Map<String, VariableExpr> javaStyleMethodNameToTransportSettingsVarExprs =
service.methods().stream()
.filter(x -> x.isSupportedByTransport(getTransport()))
.filter(x -> x.isSupportedByTransport(getTransportContext().transport()))
.collect(
Collectors.toMap(
m -> JavaStyle.toLowerCamelCase(m.name()),
Expand All @@ -688,7 +687,7 @@ protected List<MethodDefinition> createConstructorMethods(

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

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

Expand Down Expand Up @@ -1098,7 +1097,7 @@ private TypeStore createDynamicTypes(Service service, String stubPakkage) {
typeStore.putAll(
service.pakkage(),
service.methods().stream()
.filter(x -> x.isSupportedByTransport(getTransport()))
.filter(x -> x.isSupportedByTransport(getTransportContext().transport()))
.filter(Method::isPaged)
.map(m -> String.format(PAGED_RESPONSE_TYPE_NAME_PATTERN, m.name()))
.collect(Collectors.toList()),
Expand Down
Expand Up @@ -21,8 +21,6 @@
import com.google.api.generator.gapic.composer.store.TypeStore;
import com.google.api.generator.gapic.model.GapicClass;
import com.google.api.generator.gapic.model.GapicContext;
import com.google.api.generator.gapic.model.Method;
import com.google.api.generator.gapic.model.Method.Stream;
import com.google.api.generator.gapic.model.Service;
import java.util.Map;

Expand All @@ -46,12 +44,6 @@ protected GapicClass generate(String className, GapicContext context, Service se
service);
}

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

@Override
protected MethodDefinition createStartStaticServerMethod(
Service service,
Expand Down
Expand Up @@ -35,6 +35,8 @@ public static HttpJsonServiceStubClassComposer instance() {
@Override
protected List<MethodDefinition> createStaticCreatorMethods(
Service service, TypeStore typeStore, String newBuilderMethod) {
// No need to check if REST Transport is enabled
// AbstractTransportServiceStubClassComposer won't generate a file if REST isn't enabled
return super.createStaticCreatorMethods(service, typeStore, "newHttpJsonBuilder");
}
}
Expand Up @@ -26,6 +26,7 @@
import com.google.api.generator.gapic.model.ResourceName;
import com.google.api.generator.gapic.model.Sample;
import com.google.api.generator.gapic.model.Service;
import com.google.api.generator.gapic.model.Transport;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
Expand All @@ -48,6 +49,10 @@ protected List<CommentStatement> createClassHeaderComments(
Map<String, ResourceName> resourceNames,
Map<String, Message> messageTypes,
List<Sample> samples) {
if (!service.hasAnyEnabledMethodsForTransport(Transport.REST)) {
return super.createClassHeaderComments(
service, typeStore, resourceNames, messageTypes, samples);
}
TypeNode clientType = typeStore.get(ClassNames.getServiceClientClassName(service));
TypeNode settingsType = typeStore.get(ClassNames.getServiceSettingsClassName(service));
Sample classMethodSampleCode =
Expand Down

0 comments on commit 9c06bc9

Please sign in to comment.