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.
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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");
}
}
Original file line number Diff line number Diff line change
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
Loading

0 comments on commit 9c06bc9

Please sign in to comment.