[ggj][codegen] fix: space out codegen in GrpcServiceStub (#376)

* 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)

Author: miraleung

src/main/java/com/google/api/generator/gapic/composer/GrpcServiceStubClassComposer.java

@@ -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;
@@ -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";
@@ -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()
@@ -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()
@@ -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()
@@ -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 =
@@ -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(
@@ -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 =
@@ -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);
  }

src/main/java/com/google/api/generator/gapic/composer/ServiceClientTestClassComposer.java

@@ -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());

src/test/java/com/google/api/generator/gapic/composer/goldens/EchoClientTest.golden

@@ -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() {

src/test/java/com/google/api/generator/gapic/composer/goldens/GrpcEchoStub.golden

@@ -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()
@@ -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;
@@ -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;
@@ -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)
@@ -168,6 +178,7 @@ public class GrpcEchoStub extends EchoStub {
  GrpcCallSettings.<BlockRequest, BlockResponse>newBuilder()
  .setMethodDescriptor(blockMethodDescriptor)
  .build();
+
  this.echoCallable =
  callableFactory.createUnaryCallable(
  echoTransportSettings, settings.echoSettings(), clientContext);
@@ -198,6 +209,7 @@ public class GrpcEchoStub extends EchoStub {
  this.blockCallable =
  callableFactory.createUnaryCallable(
  blockTransportSettings, settings.blockSettings(), clientContext);
+
  this.backgroundResources =
  new BackgroundResourceAggregation(clientContext.getBackgroundResources());
  }