Skip to content

Commit

Permalink
[ggj][codegen] fix: space out codegen in GrpcServiceStub (#376)
Browse files Browse the repository at this point in the history 
* fix: refactor requestBuilder into separate method in ServiceClientClassComposer

* feat: add EmptyLineStatement

* fix: space out lines in ServiceStubSettings

* fix: space out codegen in GrpcServiceStub

* fix: privately-scoped vars in ServiceClientTest codegen (#377)
  • Loading branch information
@miraleung
miraleung committed Oct 10, 2020
1 parent 533cfc7 commit 46ad436
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 13 deletions.
53 changes: 45 additions & 8 deletions src/main/java/com/google/api/generator/gapic/composer/GrpcServiceStubClassComposer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import com.google.api.generator.engine.ast.AssignmentExpr;
import com.google.api.generator.engine.ast.ClassDefinition;
import com.google.api.generator.engine.ast.ConcreteReference;
import com.google.api.generator.engine.ast.EmptyLineStatement;
import com.google.api.generator.engine.ast.EnumRefExpr;
import com.google.api.generator.engine.ast.Expr;
import com.google.api.generator.engine.ast.ExprStatement;
Expand Down Expand Up @@ -68,6 +69,8 @@
import javax.annotation.Generated;

public class GrpcServiceStubClassComposer implements ClassComposer {
private static final Statement EMPTY_LINE_STATEMENT = EmptyLineStatement.create();

private static final String CLASS_NAME_PATTERN = "Grpc%sStub";
private static final String GRPC_SERVICE_CALLABLE_FACTORY_PATTERN = "Grpc%sCallableFactory";
private static final String METHOD_DESCRIPTOR_NAME_PATTERN = "%sMethodDescriptor";
Expand Down Expand Up @@ -131,11 +134,12 @@ public GapicClass generate(Service service, Map<String, Message> ignore) {
.setType(staticTypes.get("GrpcStubCallableFactory"))
.build()));

List<Statement> classStatements = new ArrayList<>();
classStatements.addAll(
createMethodDescriptorVariableDecls(service, protoMethodNameToDescriptorVarExprs));
classStatements.addAll(createClassMemberFieldDeclarations(callableClassMemberVarExprs));
classStatements.addAll(createClassMemberFieldDeclarations(classMemberVarExprs));
List<Statement> classStatements =
createClassStatements(
service,
protoMethodNameToDescriptorVarExprs,
callableClassMemberVarExprs,
classMemberVarExprs);

ClassDefinition classDef =
ClassDefinition.builder()
Expand All @@ -158,6 +162,25 @@ public GapicClass generate(Service service, Map<String, Message> ignore) {
return GapicClass.create(kind, classDef);
}

private static List<Statement> createClassStatements(
Service service,
Map<String, VariableExpr> protoMethodNameToDescriptorVarExprs,
Map<String, VariableExpr> callableClassMemberVarExprs,
Map<String, VariableExpr> classMemberVarExprs) {
List<Statement> classStatements = new ArrayList<>();
for (Statement statement :
createMethodDescriptorVariableDecls(service, protoMethodNameToDescriptorVarExprs)) {
classStatements.add(statement);
classStatements.add(EMPTY_LINE_STATEMENT);
}

classStatements.addAll(createClassMemberFieldDeclarations(callableClassMemberVarExprs));
classStatements.add(EMPTY_LINE_STATEMENT);

classStatements.addAll(createClassMemberFieldDeclarations(classMemberVarExprs));
return classStatements;
}

private static List<Statement> createMethodDescriptorVariableDecls(
Service service, Map<String, VariableExpr> protoMethodNameToDescriptorVarExprs) {
return service.methods().stream()
Expand Down Expand Up @@ -520,6 +543,7 @@ private static List<MethodDefinition> createConstructorMethods(
Expr thisExpr =
ValueExpr.withValue(ThisObjectValue.withType(types.get(getThisClassName(service.name()))));
// Body of the second constructor method.
List<Statement> secondCtorStatements = new ArrayList<>();
List<Expr> secondCtorExprs = new ArrayList<>();
secondCtorExprs.add(
AssignmentExpr.builder()
Expand All @@ -544,6 +568,10 @@ private static List<MethodDefinition> createConstructorMethods(
.setReturnType(operationsStubClassVarExpr.type())
.build())
.build());
secondCtorStatements.addAll(
secondCtorExprs.stream().map(e -> ExprStatement.withExpr(e)).collect(Collectors.toList()));
secondCtorExprs.clear();
secondCtorStatements.add(EMPTY_LINE_STATEMENT);

// Transport settings local variables.
Map<String, VariableExpr> javaStyleMethodNameToTransportSettingsVarExprs =
Expand Down Expand Up @@ -578,6 +606,10 @@ private static List<MethodDefinition> createConstructorMethods(
JavaStyle.toLowerCamelCase(m.name())),
protoMethodNameToDescriptorVarExprs.get(m.name())))
.collect(Collectors.toList()));
secondCtorStatements.addAll(
secondCtorExprs.stream().map(e -> ExprStatement.withExpr(e)).collect(Collectors.toList()));
secondCtorExprs.clear();
secondCtorStatements.add(EMPTY_LINE_STATEMENT);

// Initialize <method>Callable variables.
secondCtorExprs.addAll(
Expand All @@ -594,6 +626,10 @@ private static List<MethodDefinition> createConstructorMethods(
thisExpr,
javaStyleMethodNameToTransportSettingsVarExprs))
.collect(Collectors.toList()));
secondCtorStatements.addAll(
secondCtorExprs.stream().map(e -> ExprStatement.withExpr(e)).collect(Collectors.toList()));
secondCtorExprs.clear();
secondCtorStatements.add(EMPTY_LINE_STATEMENT);

// Instantiate backgroundResources.
MethodInvocationExpr getBackgroundResourcesMethodExpr =
Expand All @@ -612,14 +648,15 @@ private static List<MethodDefinition> createConstructorMethods(
.setArguments(Arrays.asList(getBackgroundResourcesMethodExpr))
.build())
.build());
secondCtorStatements.addAll(
secondCtorExprs.stream().map(e -> ExprStatement.withExpr(e)).collect(Collectors.toList()));
secondCtorExprs.clear();

// Second constructor method.
MethodDefinition secondCtor =
ctorMakerFn.apply(
Arrays.asList(settingsVarExpr, clientContextVarExpr, callableFactoryVarExpr),
secondCtorExprs.stream()
.map(e -> ExprStatement.withExpr(e))
.collect(Collectors.toList()));
secondCtorStatements);

return Arrays.asList(firstCtor, secondCtor);
}
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/com/google/api/generator/gapic/composer/ServiceClientTestClassComposer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ private static List<Statement> createClassMemberFieldDecls(
ExprStatement.withExpr(
v.toBuilder()
.setIsDecl(true)
.setScope(ScopeNode.PUBLIC)
.setScope(ScopeNode.PRIVATE)
.setIsStatic(v.type().reference().name().startsWith("Mock"))
.build()))
.collect(Collectors.toList());
Expand Down
8 changes: 4 additions & 4 deletions src/test/java/com/google/api/generator/gapic/composer/goldens/EchoClientTest.golden
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,10 @@ import org.junit.Test;

@Generated("by gapic-generator-java")
public class EchoClientTest {
public static MockServiceHelper mockServiceHelper;
public static MockEcho mockEcho;
public EchoClient client;
public LocalChannelProvider channelProvider;
private static MockServiceHelper mockServiceHelper;
private static MockEcho mockEcho;
private EchoClient client;
private LocalChannelProvider channelProvider;

@BeforeClass
public static void startStaticServer() {
Expand Down
12 changes: 12 additions & 0 deletions src/test/java/com/google/api/generator/gapic/composer/goldens/GrpcEchoStub.golden
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,34 +45,39 @@ public class GrpcEchoStub extends EchoStub {
.setRequestMarshaller(ProtoUtils.marshaller(EchoRequest.getDefaultInstance()))
.setResponseMarshaller(ProtoUtils.marshaller(EchoResponse.getDefaultInstance()))
.build();

private static final MethodDescriptor<ExpandRequest, EchoResponse> expandMethodDescriptor =
MethodDescriptor.<ExpandRequest, EchoResponse>newBuilder()
.setType(MethodDescriptor.MethodType.SERVER_STREAMING)
.setFullMethodName("google.showcase.v1beta1.Echo/Expand")
.setRequestMarshaller(ProtoUtils.marshaller(ExpandRequest.getDefaultInstance()))
.setResponseMarshaller(ProtoUtils.marshaller(EchoResponse.getDefaultInstance()))
.build();

private static final MethodDescriptor<EchoRequest, EchoResponse> collectMethodDescriptor =
MethodDescriptor.<EchoRequest, EchoResponse>newBuilder()
.setType(MethodDescriptor.MethodType.CLIENT_STREAMING)
.setFullMethodName("google.showcase.v1beta1.Echo/Collect")
.setRequestMarshaller(ProtoUtils.marshaller(EchoRequest.getDefaultInstance()))
.setResponseMarshaller(ProtoUtils.marshaller(EchoResponse.getDefaultInstance()))
.build();

private static final MethodDescriptor<EchoRequest, EchoResponse> chatMethodDescriptor =
MethodDescriptor.<EchoRequest, EchoResponse>newBuilder()
.setType(MethodDescriptor.MethodType.BIDI_STREAMING)
.setFullMethodName("google.showcase.v1beta1.Echo/Chat")
.setRequestMarshaller(ProtoUtils.marshaller(EchoRequest.getDefaultInstance()))
.setResponseMarshaller(ProtoUtils.marshaller(EchoResponse.getDefaultInstance()))
.build();

private static final MethodDescriptor<EchoRequest, EchoResponse> chatAgainMethodDescriptor =
MethodDescriptor.<EchoRequest, EchoResponse>newBuilder()
.setType(MethodDescriptor.MethodType.BIDI_STREAMING)
.setFullMethodName("google.showcase.v1beta1.Echo/ChatAgain")
.setRequestMarshaller(ProtoUtils.marshaller(EchoRequest.getDefaultInstance()))
.setResponseMarshaller(ProtoUtils.marshaller(EchoResponse.getDefaultInstance()))
.build();

private static final MethodDescriptor<PagedExpandRequest, PagedExpandResponse>
pagedExpandMethodDescriptor =
MethodDescriptor.<PagedExpandRequest, PagedExpandResponse>newBuilder()
Expand All @@ -82,20 +87,23 @@ public class GrpcEchoStub extends EchoStub {
.setResponseMarshaller(
ProtoUtils.marshaller(PagedExpandResponse.getDefaultInstance()))
.build();

private static final MethodDescriptor<WaitRequest, Operation> waitMethodDescriptor =
MethodDescriptor.<WaitRequest, Operation>newBuilder()
.setType(MethodDescriptor.MethodType.UNARY)
.setFullMethodName("google.showcase.v1beta1.Echo/Wait")
.setRequestMarshaller(ProtoUtils.marshaller(WaitRequest.getDefaultInstance()))
.setResponseMarshaller(ProtoUtils.marshaller(Operation.getDefaultInstance()))
.build();

private static final MethodDescriptor<BlockRequest, BlockResponse> blockMethodDescriptor =
MethodDescriptor.<BlockRequest, BlockResponse>newBuilder()
.setType(MethodDescriptor.MethodType.UNARY)
.setFullMethodName("google.showcase.v1beta1.Echo/Block")
.setRequestMarshaller(ProtoUtils.marshaller(BlockRequest.getDefaultInstance()))
.setResponseMarshaller(ProtoUtils.marshaller(BlockResponse.getDefaultInstance()))
.build();

private final UnaryCallable<EchoRequest, EchoResponse> echoCallable;
private final ServerStreamingCallable<ExpandRequest, EchoResponse> expandCallable;
private final ClientStreamingCallable<EchoRequest, EchoResponse> collectCallable;
Expand All @@ -107,6 +115,7 @@ public class GrpcEchoStub extends EchoStub {
private final UnaryCallable<WaitRequest, Operation> waitCallable;
private final OperationCallable<WaitRequest, WaitResponse, WaitMetadata> waitOperationCallable;
private final UnaryCallable<BlockRequest, BlockResponse> blockCallable;

private final BackgroundResource backgroundResources;
private final GrpcOperationsStub operationsStub;
private final GrpcStubCallableFactory callableFactory;
Expand Down Expand Up @@ -136,6 +145,7 @@ public class GrpcEchoStub extends EchoStub {
throws IOException {
this.callableFactory = callableFactory;
this.operationsStub = GrpcOperationsStub.create(clientContext, callableFactory);

GrpcCallSettings<EchoRequest, EchoResponse> echoTransportSettings =
GrpcCallSettings.<EchoRequest, EchoResponse>newBuilder()
.setMethodDescriptor(echoMethodDescriptor)
Expand Down Expand Up @@ -168,6 +178,7 @@ public class GrpcEchoStub extends EchoStub {
GrpcCallSettings.<BlockRequest, BlockResponse>newBuilder()
.setMethodDescriptor(blockMethodDescriptor)
.build();

this.echoCallable =
callableFactory.createUnaryCallable(
echoTransportSettings, settings.echoSettings(), clientContext);
Expand Down Expand Up @@ -198,6 +209,7 @@ public class GrpcEchoStub extends EchoStub {
this.blockCallable =
callableFactory.createUnaryCallable(
blockTransportSettings, settings.blockSettings(), clientContext);

this.backgroundResources =
new BackgroundResourceAggregation(clientContext.getBackgroundResources());
}
Expand Down

0 comments on commit 46ad436

Please sign in to comment.