Skip to content

Commit

Permalink
fix: refactor requestBuilder into separate method in ServiceClientCla…
Browse files Browse the repository at this point in the history 
…ssComposer (#374)
  • Loading branch information
@miraleung
miraleung authored Oct 8, 2020
1 parent 150d784 commit 8040bfc
Show file tree
Hide file tree
Showing 5 changed files with 703 additions and 74 deletions.
138 changes: 75 additions & 63 deletions src/main/java/com/google/api/generator/gapic/composer/ServiceClientClassComposer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@
import com.google.api.generator.gapic.model.MethodArgument;
import com.google.api.generator.gapic.model.Service;
import com.google.api.generator.gapic.utils.JavaStyle;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
import com.google.common.base.Strings;
import com.google.common.util.concurrent.MoreExecutors;
Expand All @@ -90,6 +91,9 @@ public class ServiceClientClassComposer implements ClassComposer {
private static final String PAGED_CALLABLE_NAME_PATTERN = "%sPagedCallable";
private static final String OPERATION_CALLABLE_NAME_PATTERN = "%sOperationCallable";

private static final Reference LIST_REFERENCE = ConcreteReference.withClazz(List.class);
private static final Reference MAP_REFERENCE = ConcreteReference.withClazz(Map.class);

private enum CallableMethodKind {
REGULAR,
LRO,
Expand Down Expand Up @@ -499,8 +503,6 @@ private static List<MethodDefinition> createMethodVariants(
}

String methodInputTypeName = methodInputType.reference().name();
Reference listRef = ConcreteReference.withClazz(List.class);
Reference mapRef = ConcreteReference.withClazz(Map.class);

// Make the method signature order deterministic, which helps with unit testing and per-version
// diffs.
Expand Down Expand Up @@ -544,71 +546,12 @@ private static List<MethodDefinition> createMethodVariants(
.setIsDecl(true)
.build();

MethodInvocationExpr newBuilderExpr =
MethodInvocationExpr.builder()
.setMethodName("newBuilder")
.setStaticReferenceType(methodInputType)
.build();
// TODO(miraleung): Handle nested arguments and descending setters here.
for (MethodArgument argument : signature) {
String argumentName = JavaStyle.toLowerCamelCase(argument.name());
TypeNode argumentType = argument.type();
String setterMethodVariantPattern = "set%s";
if (TypeNode.isReferenceType(argumentType)) {
if (listRef.isSupertypeOrEquals(argumentType.reference())) {
setterMethodVariantPattern = "addAll%s";
} else if (mapRef.isSupertypeOrEquals(argumentType.reference())) {
setterMethodVariantPattern = "putAll%s";
}
}
String setterMethodName =
String.format(setterMethodVariantPattern, JavaStyle.toUpperCamelCase(argumentName));

Expr argVarExpr =
VariableExpr.withVariable(
Variable.builder().setName(argumentName).setType(argumentType).build());

if (argument.isResourceNameHelper()) {
MethodInvocationExpr isNullCheckExpr =
MethodInvocationExpr.builder()
.setStaticReferenceType(types.get("Objects"))
.setMethodName("isNull")
.setArguments(Arrays.asList(argVarExpr))
.setReturnType(TypeNode.BOOLEAN)
.build();
Expr nullExpr = ValueExpr.withValue(NullObjectValue.create());
MethodInvocationExpr toStringExpr =
MethodInvocationExpr.builder()
.setExprReferenceExpr(argVarExpr)
.setMethodName("toString")
.setReturnType(TypeNode.STRING)
.build();
argVarExpr =
TernaryExpr.builder()
.setConditionExpr(isNullCheckExpr)
.setThenExpr(nullExpr)
.setElseExpr(toStringExpr)
.build();
}
Expr requestBuilderExpr = createRequestBuilderExpr(method, signature, types);

newBuilderExpr =
MethodInvocationExpr.builder()
.setMethodName(setterMethodName)
.setArguments(Arrays.asList(argVarExpr))
.setExprReferenceExpr(newBuilderExpr)
.build();
}

MethodInvocationExpr builderExpr =
MethodInvocationExpr.builder()
.setMethodName("build")
.setExprReferenceExpr(newBuilderExpr)
.setReturnType(methodInputType)
.build();
AssignmentExpr requestAssignmentExpr =
AssignmentExpr.builder()
.setVariableExpr(requestVarExpr)
.setValueExpr(builderExpr)
.setValueExpr(requestBuilderExpr)
.build();
List<Statement> statements = Arrays.asList(ExprStatement.withExpr(requestAssignmentExpr));

Expand Down Expand Up @@ -1373,6 +1316,75 @@ private static ClassDefinition createNestedRpcFixedSizeCollectionClass(
.build();
}

@VisibleForTesting
static Expr createRequestBuilderExpr(
Method method, List<MethodArgument> signature, Map<String, TypeNode> types) {
TypeNode methodInputType = method.inputType();
MethodInvocationExpr newBuilderExpr =
MethodInvocationExpr.builder()
.setMethodName("newBuilder")
.setStaticReferenceType(methodInputType)
.build();
// TODO(miraleung): Handle nested arguments and descending setters here.
for (MethodArgument argument : signature) {
String argumentName = JavaStyle.toLowerCamelCase(argument.name());
TypeNode argumentType = argument.type();
String setterMethodVariantPattern = "set%s";
if (TypeNode.isReferenceType(argumentType)) {
if (LIST_REFERENCE.isSupertypeOrEquals(argumentType.reference())) {
setterMethodVariantPattern = "addAll%s";
} else if (MAP_REFERENCE.isSupertypeOrEquals(argumentType.reference())) {
setterMethodVariantPattern = "putAll%s";
}
}
String setterMethodName =
String.format(setterMethodVariantPattern, JavaStyle.toUpperCamelCase(argumentName));

Expr argVarExpr =
VariableExpr.withVariable(
Variable.builder().setName(argumentName).setType(argumentType).build());

if (argument.isResourceNameHelper()) {
MethodInvocationExpr isNullCheckExpr =
MethodInvocationExpr.builder()
.setStaticReferenceType(types.get("Objects"))
.setMethodName("isNull")
.setArguments(Arrays.asList(argVarExpr))
.setReturnType(TypeNode.BOOLEAN)
.build();
Expr nullExpr = ValueExpr.withValue(NullObjectValue.create());
MethodInvocationExpr toStringExpr =
MethodInvocationExpr.builder()
.setExprReferenceExpr(argVarExpr)
.setMethodName("toString")
.setReturnType(TypeNode.STRING)
.build();
argVarExpr =
TernaryExpr.builder()
.setConditionExpr(isNullCheckExpr)
.setThenExpr(nullExpr)
.setElseExpr(toStringExpr)
.build();
}

newBuilderExpr =
MethodInvocationExpr.builder()
.setMethodName(setterMethodName)
.setArguments(Arrays.asList(argVarExpr))
.setExprReferenceExpr(newBuilderExpr)
.build();
}

MethodInvocationExpr builderExpr =
MethodInvocationExpr.builder()
.setMethodName("build")
.setExprReferenceExpr(newBuilderExpr)
.setReturnType(methodInputType)
.build();

return builderExpr;
}

private static Map<String, TypeNode> createTypes(
Service service, Map<String, Message> messageTypes) {
Map<String, TypeNode> types = new HashMap<>();
Expand Down
39 changes: 28 additions & 11 deletions src/test/java/com/google/api/generator/gapic/composer/ServiceClientClassComposerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,28 +27,22 @@
import com.google.protobuf.Descriptors.FileDescriptor;
import com.google.protobuf.Descriptors.ServiceDescriptor;
import com.google.showcase.v1beta1.EchoOuterClass;
import com.google.showcase.v1beta1.IdentityOuterClass;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import org.junit.Before;
import org.junit.Test;

public class ServiceClientClassComposerTest {
private ServiceDescriptor echoService;
private FileDescriptor echoFileDescriptor;

@Before
public void setUp() {
echoFileDescriptor = EchoOuterClass.getDescriptor();
echoService = echoFileDescriptor.getServices().get(0);
assertEquals(echoService.getName(), "Echo");
}

@Test
public void generateServiceClasses() {
FileDescriptor echoFileDescriptor = EchoOuterClass.getDescriptor();
ServiceDescriptor echoService = echoFileDescriptor.getServices().get(0);
assertEquals(echoService.getName(), "Echo");

Map<String, Message> messageTypes = Parser.parseMessages(echoFileDescriptor);
Map<String, ResourceName> resourceNames = Parser.parseResourceNames(echoFileDescriptor);
Set<ResourceName> outputResourceNames = new HashSet<>();
Expand All @@ -65,4 +59,27 @@ public void generateServiceClasses() {
Path goldenFilePath = Paths.get(ComposerConstants.GOLDENFILES_DIRECTORY, "EchoClient.golden");
assertCodeEquals(goldenFilePath, visitor.write());
}

@Test
public void generateServiceClasses_methodSignatureHasNestedFields() {
FileDescriptor fileDescriptor = IdentityOuterClass.getDescriptor();
ServiceDescriptor identityService = fileDescriptor.getServices().get(0);
assertEquals(identityService.getName(), "Identity");

Map<String, Message> messageTypes = Parser.parseMessages(fileDescriptor);
Map<String, ResourceName> resourceNames = Parser.parseResourceNames(fileDescriptor);
Set<ResourceName> outputResourceNames = new HashSet<>();
List<Service> services =
Parser.parseService(fileDescriptor, messageTypes, resourceNames, outputResourceNames);

Service protoService = services.get(0);
GapicClass clazz = ServiceClientClassComposer.instance().generate(protoService, messageTypes);

JavaWriterVisitor visitor = new JavaWriterVisitor();
clazz.classDefinition().accept(visitor);
Utils.saveCodegenToFile(this.getClass(), "IdentityClient.golden", visitor.write());
Path goldenFilePath =
Paths.get(ComposerConstants.GOLDENFILES_DIRECTORY, "IdentityClient.golden");
assertCodeEquals(goldenFilePath, visitor.write());
}
}
Loading

0 comments on commit 8040bfc

Please sign in to comment.