From d1fe9e138a4958735cffcb398797e5af764b15bc Mon Sep 17 00:00:00 2001 From: Blake Li Date: Wed, 26 Jan 2022 21:21:31 -0500 Subject: [PATCH] #869. Add field type validation for String type. Add more unit tests for edge cases. Resolve review comments and some small refactoring. --- .../grpc/GrpcServiceStubClassComposer.java | 263 ++++++++--------- .../api/generator/gapic/model/Message.java | 17 +- .../api/generator/gapic/model/Method.java | 12 +- ...ingHeaders.java => RoutingHeaderRule.java} | 30 +- .../gapic/protoparser/HttpRuleParser.java | 19 +- .../generator/gapic/protoparser/Parser.java | 7 +- .../gapic/protoparser/PatternParser.java | 34 +++ .../gapic/protoparser/RoutingRuleParser.java | 56 ++-- .../goldens/GrpcRoutingHeadersStub.golden | 270 +++++++----------- .../grpc/goldens/GrpcTestingStub.golden | 19 +- .../api/generator/gapic/model/BUILD.bazel | 2 +- .../generator/gapic/model/MessageTest.java | 117 +++++--- .../api/generator/gapic/model/MethodTest.java | 83 ++++++ ...sTest.java => RoutingHeaderParamTest.java} | 9 +- .../generator/gapic/protoparser/BUILD.bazel | 3 +- .../gapic/protoparser/PatternParserTest.java | 40 +++ .../protoparser/RoutingRuleParserTest.java | 51 ++-- ...licit_dynamic_routing_header_testing.proto | 21 ++ 18 files changed, 599 insertions(+), 454 deletions(-) rename src/main/java/com/google/api/generator/gapic/model/{RoutingHeaders.java => RoutingHeaderRule.java} (50%) create mode 100644 src/main/java/com/google/api/generator/gapic/protoparser/PatternParser.java rename src/test/java/com/google/api/generator/gapic/model/{RoutingHeadersTest.java => RoutingHeaderParamTest.java} (73%) create mode 100644 src/test/java/com/google/api/generator/gapic/protoparser/PatternParserTest.java diff --git a/src/main/java/com/google/api/generator/gapic/composer/grpc/GrpcServiceStubClassComposer.java b/src/main/java/com/google/api/generator/gapic/composer/grpc/GrpcServiceStubClassComposer.java index 517112073f..a33056dad4 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/grpc/GrpcServiceStubClassComposer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/grpc/GrpcServiceStubClassComposer.java @@ -16,7 +16,7 @@ import com.google.api.gax.grpc.GrpcCallSettings; import com.google.api.gax.grpc.GrpcStubCallableFactory; -import com.google.api.gax.grpc.RoutingHeaderParamsBuilder; +import com.google.api.gax.rpc.RequestParamsBuilder; import com.google.api.generator.engine.ast.AssignmentExpr; import com.google.api.generator.engine.ast.ConcreteReference; import com.google.api.generator.engine.ast.EnumRefExpr; @@ -26,7 +26,6 @@ import com.google.api.generator.engine.ast.LambdaExpr; import com.google.api.generator.engine.ast.LogicalOperationExpr; import com.google.api.generator.engine.ast.MethodInvocationExpr; -import com.google.api.generator.engine.ast.NewObjectExpr; import com.google.api.generator.engine.ast.RelationalOperationExpr; import com.google.api.generator.engine.ast.ScopeNode; import com.google.api.generator.engine.ast.Statement; @@ -40,10 +39,11 @@ import com.google.api.generator.gapic.model.HttpBindings.HttpBinding; import com.google.api.generator.gapic.model.Message; import com.google.api.generator.gapic.model.Method; -import com.google.api.generator.gapic.model.RoutingHeaders.RoutingHeader; +import com.google.api.generator.gapic.model.RoutingHeaderRule.RoutingHeaderParam; import com.google.api.generator.gapic.model.Service; import com.google.api.generator.gapic.utils.JavaStyle; import com.google.api.pathtemplate.PathTemplate; +import com.google.common.base.Splitter; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.longrunning.stub.GrpcOperationsStub; @@ -218,7 +218,7 @@ protected Expr createTransportSettingsInitExpr( .setArguments(Arrays.asList(methodDescriptorVarExpr)) .build(); - if (method.hasHttpBindings() || method.hasRoutingHeaders()) { + if (method.shouldSetParamsExtractor()) { callSettingsBuilderExpr = MethodInvocationExpr.builder() .setExprReferenceExpr(callSettingsBuilderExpr) @@ -267,16 +267,16 @@ private LambdaExpr createRequestParamsExtractorClassInstance( .setClazz(Map.class) .setGenerics(TypeNode.STRING.reference(), TypeNode.STRING.reference()) .build()); - Expr returnExpr; + MethodInvocationExpr.Builder returnExpr = + MethodInvocationExpr.builder().setReturnType(returnType); // If the google.api.routing annotation is present(even with empty routing parameters), // the implicit routing headers specified in the google.api.http annotation should not be sent - if (method.routingHeaders() == null) { - returnExpr = - addRequestParamsForHttpBindings(method, bodyStatements, requestVarExpr, returnType); + if (method.routingHeaderRule() == null) { + createRequestParamsExtractorBodyForHttpBindings( + method, requestVarExpr, bodyStatements, returnExpr); } else { - returnExpr = - addRequestParamsForRoutingHeaders( - method, classStatements, bodyStatements, requestVarExpr, returnType); + createRequestParamsExtractorBodyForRoutingHeaders( + method, requestVarExpr, classStatements, bodyStatements, returnExpr); } // Overrides extract(). @@ -284,103 +284,161 @@ private LambdaExpr createRequestParamsExtractorClassInstance( return LambdaExpr.builder() .setArguments(requestVarExpr.toBuilder().setIsDecl(true).build()) .setBody(bodyStatements) - .setReturnExpr(returnExpr) + .setReturnExpr(returnExpr.build()) .build(); } - private Expr addRequestParamsForRoutingHeaders( + private void createRequestParamsExtractorBodyForHttpBindings( Method method, - List classStatements, + VariableExpr requestVarExpr, List bodyStatements, + MethodInvocationExpr.Builder returnExprBuilder) { + TypeNode paramsVarType = + TypeNode.withReference( + ConcreteReference.builder() + .setClazz(ImmutableMap.Builder.class) + .setGenerics(TypeNode.STRING.reference(), TypeNode.STRING.reference()) + .build()); + VariableExpr paramsVarExpr = + VariableExpr.withVariable( + Variable.builder().setName("params").setType(paramsVarType).build()); + + Expr paramsAssignExpr = + AssignmentExpr.builder() + .setVariableExpr(paramsVarExpr.toBuilder().setIsDecl(true).build()) + .setValueExpr( + MethodInvocationExpr.builder() + .setStaticReferenceType(FIXED_TYPESTORE.get("ImmutableMap")) + .setMethodName("builder") + .setReturnType(paramsVarType) + .build()) + .build(); + bodyStatements.add(ExprStatement.withExpr(paramsAssignExpr)); + + for (HttpBinding httpBindingFieldBinding : method.httpBindings().pathParameters()) { + MethodInvocationExpr requestBuilderExpr = + createRequestFieldGetterExpr(requestVarExpr, httpBindingFieldBinding.name()); + Expr valueOfExpr = + MethodInvocationExpr.builder() + .setStaticReferenceType(TypeNode.STRING) + .setMethodName("valueOf") + .setArguments(requestBuilderExpr) + .build(); + + Expr paramsPutExpr = + MethodInvocationExpr.builder() + .setExprReferenceExpr(paramsVarExpr) + .setMethodName("put") + .setArguments( + ValueExpr.withValue(StringObjectValue.withValue(httpBindingFieldBinding.name())), + valueOfExpr) + .build(); + bodyStatements.add(ExprStatement.withExpr(paramsPutExpr)); + } + + returnExprBuilder.setExprReferenceExpr(paramsVarExpr).setMethodName("build"); + } + + private void createRequestParamsExtractorBodyForRoutingHeaders( + Method method, VariableExpr requestVarExpr, - TypeNode returnType) { + List classStatements, + List bodyStatements, + MethodInvocationExpr.Builder returnExprBuilder) { TypeNode routingHeadersBuilderType = TypeNode.withReference( - ConcreteReference.builder().setClazz(RoutingHeaderParamsBuilder.class).build()); + ConcreteReference.builder().setClazz(RequestParamsBuilder.class).build()); VariableExpr routingHeadersBuilderVarExpr = VariableExpr.builder() .setVariable( Variable.builder().setName("builder").setType(routingHeadersBuilderType).build()) .setIsDecl(true) .build(); - Expr newBuilderExpr = NewObjectExpr.builder().setType(routingHeadersBuilderType).build(); - Expr newRoutingHeadersAssignExpr = + MethodInvocationExpr routingHeaderBuilderInvokeExpr = + MethodInvocationExpr.builder() + .setStaticReferenceType(routingHeadersBuilderType) + .setMethodName("create") + .setReturnType(routingHeadersBuilderType) + .build(); + Expr routingHeadersBuilderInitExpr = AssignmentExpr.builder() .setVariableExpr(routingHeadersBuilderVarExpr) - .setValueExpr(newBuilderExpr) + .setValueExpr(routingHeaderBuilderInvokeExpr) .build(); - bodyStatements.add(ExprStatement.withExpr(newRoutingHeadersAssignExpr)); - List routingHeaders = method.routingHeaders().routingHeadersList(); + bodyStatements.add(ExprStatement.withExpr(routingHeadersBuilderInitExpr)); + List routingHeaderParams = method.routingHeaderRule().routingHeaderParams(); VariableExpr routingHeadersBuilderVarNonDeclExpr = VariableExpr.builder() .setVariable( Variable.builder().setName("builder").setType(routingHeadersBuilderType).build()) .build(); - for (int i = 0; i < routingHeaders.size(); i++) { - RoutingHeader routingHeader = routingHeaders.get(i); + for (int i = 0; i < routingHeaderParams.size(); i++) { + RoutingHeaderParam routingHeaderParam = routingHeaderParams.get(i); MethodInvocationExpr requestFieldGetterExpr = - createRequestFieldGetterExpr(requestVarExpr, routingHeader.fieldName()); + createRequestFieldGetterExpr(requestVarExpr, routingHeaderParam.fieldName()); Expr routingHeaderKeyExpr = - ValueExpr.withValue(StringObjectValue.withValue(routingHeader.name())); - // TODO: Create proper snake style upper case name + ValueExpr.withValue(StringObjectValue.withValue(routingHeaderParam.key())); String pathTemplateName = - String.format("PATH_TEMPLATE_%s_%s", method.name().toUpperCase(), i); + String.format("%s_%s_PATH_TEMPLATE", JavaStyle.toUpperSnakeCase(method.name()), i); TypeNode pathTemplateType = TypeNode.withReference(ConcreteReference.withClazz(PathTemplate.class)); Variable pathTemplateVar = Variable.builder().setType(pathTemplateType).setName(pathTemplateName).build(); Expr routingHeaderPatternExpr = VariableExpr.withVariable(pathTemplateVar); - VariableExpr pathTemplateVarExpr = - VariableExpr.builder() - .setVariable(pathTemplateVar) - .setIsDecl(true) - .setIsStatic(true) - .setIsFinal(true) - .setScope(ScopeNode.PRIVATE) - .build(); - ValueExpr valueExpr = - ValueExpr.withValue(StringObjectValue.withValue(routingHeader.pattern())); - Expr pathTemplateExpr = - AssignmentExpr.builder() - .setVariableExpr(pathTemplateVarExpr) - .setValueExpr( - MethodInvocationExpr.builder() - .setStaticReferenceType(pathTemplateType) - .setMethodName("create") - .setArguments(valueExpr) - .setReturnType(pathTemplateType) - .build()) - .build(); - Statement pathTemplateClassVar = ExprStatement.withExpr(pathTemplateExpr); + Statement pathTemplateClassVar = + createPathTemplateClassStatement(routingHeaderParam, pathTemplateType, pathTemplateVar); classStatements.add(pathTemplateClassVar); - Expr valueOfExpr = - MethodInvocationExpr.builder() - .setStaticReferenceType(TypeNode.STRING) - .setMethodName("valueOf") - .setArguments(requestFieldGetterExpr) - .build(); - MethodInvocationExpr addParamsMethodExpr = + MethodInvocationExpr addParamMethodExpr = MethodInvocationExpr.builder() .setExprReferenceExpr(routingHeadersBuilderVarNonDeclExpr) .setMethodName("add") - .setArguments(valueOfExpr, routingHeaderKeyExpr, routingHeaderPatternExpr) + .setArguments(requestFieldGetterExpr, routingHeaderKeyExpr, routingHeaderPatternExpr) .build(); - IfStatement ifStatement = - IfStatement.builder() - .setConditionExpr( - fieldValuesNotNullConditionExpr( - requestVarExpr, routingHeader.getDescendantFieldNames())) - .setBody(ImmutableList.of(ExprStatement.withExpr(addParamsMethodExpr))) - .build(); - bodyStatements.add(ifStatement); + ExprStatement addParamStatement = ExprStatement.withExpr(addParamMethodExpr); + // No need to add null check if there is no nested fields + if (routingHeaderParam.getDescendantFieldNames().size() == 1) { + bodyStatements.add(addParamStatement); + } else { + IfStatement ifStatement = + IfStatement.builder() + .setConditionExpr( + fieldValuesNotNullConditionExpr( + requestVarExpr, routingHeaderParam.getDescendantFieldNames())) + .setBody(ImmutableList.of(addParamStatement)) + .build(); + bodyStatements.add(ifStatement); + } } - - return MethodInvocationExpr.builder() + returnExprBuilder .setExprReferenceExpr(routingHeadersBuilderVarNonDeclExpr) - .setMethodName("build") - .setReturnType(returnType) - .build(); + .setMethodName("build"); + } + + private Statement createPathTemplateClassStatement( + RoutingHeaderParam routingHeaderParam, TypeNode pathTemplateType, Variable pathTemplateVar) { + VariableExpr pathTemplateVarExpr = + VariableExpr.builder() + .setVariable(pathTemplateVar) + .setIsDecl(true) + .setIsStatic(true) + .setIsFinal(true) + .setScope(ScopeNode.PRIVATE) + .build(); + ValueExpr valueExpr = + ValueExpr.withValue(StringObjectValue.withValue(routingHeaderParam.pattern())); + Expr pathTemplateExpr = + AssignmentExpr.builder() + .setVariableExpr(pathTemplateVarExpr) + .setValueExpr( + MethodInvocationExpr.builder() + .setStaticReferenceType(pathTemplateType) + .setMethodName("create") + .setArguments(valueExpr) + .setReturnType(pathTemplateType) + .build()) + .build(); + return ExprStatement.withExpr(pathTemplateExpr); } private Expr fieldValuesNotNullConditionExpr( @@ -388,7 +446,7 @@ private Expr fieldValuesNotNullConditionExpr( MethodInvocationExpr.Builder requestFieldGetterExprBuilder = MethodInvocationExpr.builder().setExprReferenceExpr(requestVarExpr); Expr fieldValuesNotNullExpr = null; - for (int i = 0; i < fieldNames.size(); i++) { + for (int i = 0; i < fieldNames.size() - 1; i++) { String currFieldName = fieldNames.get(i); String bindingFieldMethodName = String.format("get%s", JavaStyle.toUpperCamelCase(currFieldName)); @@ -415,75 +473,20 @@ private Expr fieldValuesNotNullConditionExpr( return fieldValuesNotNullExpr; } - private Expr addRequestParamsForHttpBindings( - Method method, - List bodyStatements, - VariableExpr requestVarExpr, - TypeNode returnType) { - TypeNode paramsVarType = - TypeNode.withReference( - ConcreteReference.builder() - .setClazz(ImmutableMap.Builder.class) - .setGenerics(TypeNode.STRING.reference(), TypeNode.STRING.reference()) - .build()); - VariableExpr paramsVarExpr = - VariableExpr.withVariable( - Variable.builder().setName("params").setType(paramsVarType).build()); - - Expr paramsAssignExpr = - AssignmentExpr.builder() - .setVariableExpr(paramsVarExpr.toBuilder().setIsDecl(true).build()) - .setValueExpr( - MethodInvocationExpr.builder() - .setStaticReferenceType(FIXED_TYPESTORE.get("ImmutableMap")) - .setMethodName("builder") - .setReturnType(paramsVarType) - .build()) - .build(); - bodyStatements.add(ExprStatement.withExpr(paramsAssignExpr)); - - for (HttpBinding httpBindingFieldBinding : method.httpBindings().pathParameters()) { - MethodInvocationExpr requestBuilderExpr = - createRequestFieldGetterExpr(requestVarExpr, httpBindingFieldBinding.name()); - Expr valueOfExpr = - MethodInvocationExpr.builder() - .setStaticReferenceType(TypeNode.STRING) - .setMethodName("valueOf") - .setArguments(requestBuilderExpr) - .build(); - - Expr paramsPutExpr = - MethodInvocationExpr.builder() - .setExprReferenceExpr(paramsVarExpr) - .setMethodName("put") - .setArguments( - ValueExpr.withValue(StringObjectValue.withValue(httpBindingFieldBinding.name())), - valueOfExpr) - .build(); - bodyStatements.add(ExprStatement.withExpr(paramsPutExpr)); - } - - return MethodInvocationExpr.builder() - .setExprReferenceExpr(paramsVarExpr) - .setMethodName("build") - .setReturnType(returnType) - .build(); - } - private MethodInvocationExpr createRequestFieldGetterExpr( VariableExpr requestVarExpr, String fieldName) { MethodInvocationExpr.Builder requestFieldGetterExprBuilder = MethodInvocationExpr.builder().setExprReferenceExpr(requestVarExpr); - String[] descendantFields = fieldName.split("\\."); + List descendantFields = Splitter.on(".").splitToList(fieldName); // Handle foo.bar cases by descending into the subfields. // e.g. foo.bar -> request.getFoo().getBar() - for (int i = 0; i < descendantFields.length; i++) { - String currFieldName = descendantFields[i]; + for (int i = 0; i < descendantFields.size(); i++) { + String currFieldName = descendantFields.get(i); String bindingFieldMethodName = String.format("get%s", JavaStyle.toUpperCamelCase(currFieldName)); requestFieldGetterExprBuilder = requestFieldGetterExprBuilder.setMethodName(bindingFieldMethodName); - if (i < descendantFields.length - 1) { + if (i < descendantFields.size() - 1) { requestFieldGetterExprBuilder = MethodInvocationExpr.builder() .setExprReferenceExpr(requestFieldGetterExprBuilder.build()); diff --git a/src/main/java/com/google/api/generator/gapic/model/Message.java b/src/main/java/com/google/api/generator/gapic/model/Message.java index 98972a9a5d..ecfdd484df 100644 --- a/src/main/java/com/google/api/generator/gapic/model/Message.java +++ b/src/main/java/com/google/api/generator/gapic/model/Message.java @@ -84,7 +84,15 @@ public boolean hasResource() { return resource() != null; } - public void validateField(String fieldName, Map messageTypes) { + /** + * Validates if the field or fields exist in the message and the type of the leaf level field. + * + * @param fieldName The field name. For nested field, concatenate each field name with dot. For + * example: abc.def.ghi + * @param messageTypes All messages configured in a rpc service. + * @param type {@link TypeNode} The expected type for the leaf level field + */ + public void validateField(String fieldName, Map messageTypes, TypeNode type) { List subFields = Splitter.on(".").splitToList(fieldName); Message nestedMessage = this; for (int i = 0; i < subFields.size(); i++) { @@ -106,12 +114,9 @@ public void validateField(String fieldName, Map messageTypes) { "No containing message found for field %s with type %s", field.name(), field.type().reference().simpleName())); } else { - // TODO: Type check for String or primitive? Preconditions.checkState( - !field.isRepeated() && field.type().isProtoPrimitiveType(), - String.format( - "The type of field %s must be primitive and not repeated.", - field.name())); + !field.isRepeated() && field.type().equals(type), + String.format("The type of field %s must be String and not repeated.", field.name())); } } } diff --git a/src/main/java/com/google/api/generator/gapic/model/Method.java b/src/main/java/com/google/api/generator/gapic/model/Method.java index f29f0f0c66..dc2452f2d5 100644 --- a/src/main/java/com/google/api/generator/gapic/model/Method.java +++ b/src/main/java/com/google/api/generator/gapic/model/Method.java @@ -63,7 +63,7 @@ public boolean isPaged() { public abstract HttpBindings httpBindings(); @Nullable - public abstract RoutingHeaders routingHeaders(); + public abstract RoutingHeaderRule routingHeaderRule(); // Example from Expand in echo.proto: Thet TypeNodes that map to // [["content", "error"], ["content", "error", "info"]]. @@ -83,8 +83,12 @@ public boolean hasHttpBindings() { return httpBindings() != null && !httpBindings().pathParameters().isEmpty(); } - public boolean hasRoutingHeaders() { - return routingHeaders() != null && !routingHeaders().routingHeadersList().isEmpty(); + public boolean hasRoutingHeaderParams() { + return routingHeaderRule() != null && !routingHeaderRule().routingHeaderParams().isEmpty(); + } + + public boolean shouldSetParamsExtractor() { + return (hasHttpBindings() && routingHeaderRule() == null) || hasRoutingHeaderParams(); } public boolean isMixin() { @@ -147,7 +151,7 @@ public abstract static class Builder { public abstract Builder setOperationPollingMethod(boolean operationPollingMethod); - public abstract Builder setRoutingHeaders(RoutingHeaders routingHeaders); + public abstract Builder setRoutingHeaderRule(RoutingHeaderRule routingHeaderRule); public abstract Method build(); } diff --git a/src/main/java/com/google/api/generator/gapic/model/RoutingHeaders.java b/src/main/java/com/google/api/generator/gapic/model/RoutingHeaderRule.java similarity index 50% rename from src/main/java/com/google/api/generator/gapic/model/RoutingHeaders.java rename to src/main/java/com/google/api/generator/gapic/model/RoutingHeaderRule.java index 82a6354ea1..ff6cf4de28 100644 --- a/src/main/java/com/google/api/generator/gapic/model/RoutingHeaders.java +++ b/src/main/java/com/google/api/generator/gapic/model/RoutingHeaderRule.java @@ -19,26 +19,31 @@ import com.google.common.collect.ImmutableList; import java.util.List; +/** + * This model represents routing rules configured in rpc services. It will be used for generating + * the logic to match-and-extract field values from request, the extracted values will be + * concatenated to a request header that is used for routing purposes. + */ @AutoValue -public abstract class RoutingHeaders { +public abstract class RoutingHeaderRule { - public abstract ImmutableList routingHeadersList(); + public abstract ImmutableList routingHeaderParams(); public static Builder builder() { - return new AutoValue_RoutingHeaders.Builder().setRoutingHeadersList(ImmutableList.of()); + return new AutoValue_RoutingHeaderRule.Builder().setRoutingHeaderParams(ImmutableList.of()); } @AutoValue - public abstract static class RoutingHeader { + public abstract static class RoutingHeaderParam { public abstract String fieldName(); - public abstract String name(); + public abstract String key(); public abstract String pattern(); - public static RoutingHeaders.RoutingHeader create(String field, String name, String pattern) { - return new AutoValue_RoutingHeaders_RoutingHeader(field, name, pattern); + public static RoutingHeaderParam create(String field, String key, String pattern) { + return new AutoValue_RoutingHeaderRule_RoutingHeaderParam(field, key, pattern); } public List getDescendantFieldNames() { @@ -48,15 +53,16 @@ public List getDescendantFieldNames() { @AutoValue.Builder public abstract static class Builder { - abstract ImmutableList.Builder routingHeadersListBuilder(); + abstract ImmutableList.Builder routingHeaderParamsBuilder(); - public final Builder addRoutingHeader(RoutingHeader routingHeader) { - routingHeadersListBuilder().add(routingHeader); + public final Builder addParam(RoutingHeaderParam routingHeaderParam) { + routingHeaderParamsBuilder().add(routingHeaderParam); return this; } - public abstract Builder setRoutingHeadersList(ImmutableList routingHeadersList); + public abstract Builder setRoutingHeaderParams( + ImmutableList routingHeaderParams); - public abstract RoutingHeaders build(); + public abstract RoutingHeaderRule build(); } } diff --git a/src/main/java/com/google/api/generator/gapic/protoparser/HttpRuleParser.java b/src/main/java/com/google/api/generator/gapic/protoparser/HttpRuleParser.java index 5a37a38074..db8f74292a 100644 --- a/src/main/java/com/google/api/generator/gapic/protoparser/HttpRuleParser.java +++ b/src/main/java/com/google/api/generator/gapic/protoparser/HttpRuleParser.java @@ -36,7 +36,6 @@ import java.util.Set; import java.util.regex.Matcher; import java.util.regex.Pattern; -import java.util.stream.Collectors; public class HttpRuleParser { private static final String ASTERISK = "*"; @@ -68,11 +67,12 @@ private static HttpBindings parseHttpRuleHelper( HttpRule httpRule, Optional inputMessageOpt, Map messageTypes) { // Get pattern. String pattern = getHttpVerbPattern(httpRule); - ImmutableSet.Builder bindingsBuilder = getPatternBindings(pattern); + ImmutableSet.Builder bindingsBuilder = ImmutableSet.builder(); + bindingsBuilder.addAll(PatternParser.getPattenBindings(pattern)); if (httpRule.getAdditionalBindingsCount() > 0) { for (HttpRule additionalRule : httpRule.getAdditionalBindingsList()) { // TODO: save additional bindings path in HttpRuleBindings - bindingsBuilder.addAll(getPatternBindings(getHttpVerbPattern(additionalRule)).build()); + bindingsBuilder.addAll(PatternParser.getPattenBindings(getHttpVerbPattern(additionalRule))); } } @@ -177,19 +177,6 @@ private static String getHttpVerbPattern(HttpRule httpRule) { } } - private static ImmutableSortedSet.Builder getPatternBindings(String pattern) { - ImmutableSortedSet.Builder bindings = ImmutableSortedSet.naturalOrder(); - if (pattern.isEmpty()) { - return bindings; - } - - PathTemplate template = PathTemplate.create(pattern); - // Filter out any unbound variable like "$0, $1, etc. - bindings.addAll( - template.vars().stream().filter(s -> !s.contains("$")).collect(Collectors.toSet())); - return bindings; - } - private static void checkHttpFieldIsValid(String binding, Message inputMessage, boolean isBody) { Preconditions.checkState( !Strings.isNullOrEmpty(binding), diff --git a/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java b/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java index a4aa2be50b..df45446229 100644 --- a/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java +++ b/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java @@ -34,7 +34,7 @@ import com.google.api.generator.gapic.model.OperationResponse; import com.google.api.generator.gapic.model.ResourceName; import com.google.api.generator.gapic.model.ResourceReference; -import com.google.api.generator.gapic.model.RoutingHeaders; +import com.google.api.generator.gapic.model.RoutingHeaderRule; import com.google.api.generator.gapic.model.Service; import com.google.api.generator.gapic.model.SourceCodeInfoLocation; import com.google.api.generator.gapic.model.Transport; @@ -710,8 +710,7 @@ static List parseMethods( .getOptions() .getExtension(ExtendedOperationsProto.operationPollingMethod) : false; - // may not need to pass in messageTypes? - RoutingHeaders routingHeaders = + RoutingHeaderRule routingHeaderRule = RoutingRuleParser.parse(protoMethod, inputMessage, messageTypes); methods.add( methodBuilder @@ -730,7 +729,7 @@ static List parseMethods( resourceNames, outputArgResourceNames)) .setHttpBindings(httpBindings) - .setRoutingHeaders(routingHeaders) + .setRoutingHeaderRule(routingHeaderRule) .setIsBatching(isBatching) .setPageSizeFieldName(parsePageSizeFieldName(protoMethod, messageTypes, transport)) .setIsDeprecated(isDeprecated) diff --git a/src/main/java/com/google/api/generator/gapic/protoparser/PatternParser.java b/src/main/java/com/google/api/generator/gapic/protoparser/PatternParser.java new file mode 100644 index 0000000000..47ab9e5411 --- /dev/null +++ b/src/main/java/com/google/api/generator/gapic/protoparser/PatternParser.java @@ -0,0 +1,34 @@ +// Copyright 2022 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.api.generator.gapic.protoparser; + +import com.google.api.pathtemplate.PathTemplate; +import com.google.common.collect.ImmutableSortedSet; +import java.util.Set; + +public class PatternParser { + + public static Set getPattenBindings(String pattern) { + ImmutableSortedSet.Builder bindings = ImmutableSortedSet.naturalOrder(); + if (pattern.isEmpty()) { + return bindings.build(); + } + + PathTemplate template = PathTemplate.create(pattern); + // Filter out any unbound variable like "$0, $1, etc. + template.vars().stream().filter(s -> !s.contains("$")).forEach(bindings::add); + return bindings.build(); + } +} diff --git a/src/main/java/com/google/api/generator/gapic/protoparser/RoutingRuleParser.java b/src/main/java/com/google/api/generator/gapic/protoparser/RoutingRuleParser.java index 3ac54f51a1..23f8ed0741 100644 --- a/src/main/java/com/google/api/generator/gapic/protoparser/RoutingRuleParser.java +++ b/src/main/java/com/google/api/generator/gapic/protoparser/RoutingRuleParser.java @@ -17,70 +17,50 @@ import com.google.api.RoutingParameter; import com.google.api.RoutingProto; import com.google.api.RoutingRule; +import com.google.api.generator.engine.ast.TypeNode; import com.google.api.generator.gapic.model.Message; -import com.google.api.generator.gapic.model.RoutingHeaders; -import com.google.api.generator.gapic.model.RoutingHeaders.RoutingHeader; -import com.google.api.pathtemplate.PathTemplate; +import com.google.api.generator.gapic.model.RoutingHeaderRule; +import com.google.api.generator.gapic.model.RoutingHeaderRule.RoutingHeaderParam; import com.google.common.base.Preconditions; import com.google.common.base.Strings; -import com.google.common.collect.ImmutableSortedSet; import com.google.protobuf.DescriptorProtos.MethodOptions; import com.google.protobuf.Descriptors.MethodDescriptor; import java.util.Map; import java.util.Set; -import java.util.stream.Collectors; public class RoutingRuleParser { - public static RoutingHeaders parse( + public static RoutingHeaderRule parse( MethodDescriptor protoMethod, Message inputMessage, Map messageTypes) { MethodOptions methodOptions = protoMethod.getOptions(); - - RoutingHeaders.Builder builder = RoutingHeaders.builder(); if (!methodOptions.hasExtension(RoutingProto.routing)) { return null; } + RoutingHeaderRule.Builder routingHeaderRuleBuilder = RoutingHeaderRule.builder(); RoutingRule routingRule = methodOptions.getExtension(RoutingProto.routing); - for (RoutingParameter routingParameter : routingRule.getRoutingParametersList()) { String pathTemplate = routingParameter.getPathTemplate(); - String field = routingParameter.getField(); - // validate if field exist in Message or nested Messages - inputMessage.validateField(field, messageTypes); - String name; - // if specified, the pattern must contain one and only one named segment + String fieldName = routingParameter.getField(); + // validate if field exist in Message or nested Messages and the type of leaf level field + inputMessage.validateField(fieldName, messageTypes, TypeNode.STRING); + String key; if (Strings.isNullOrEmpty(pathTemplate)) { - name = field; - pathTemplate = String.format("{%s=**}", name); + key = fieldName; + pathTemplate = String.format("{%s=**}", key); } else { - Set params = getPatternBindings(pathTemplate).build(); + Set namedSegments = PatternParser.getPattenBindings(pathTemplate); Preconditions.checkArgument( - params.size() == 1, + namedSegments.size() == 1, String.format( "There needs to be one and only one named segment in path template %s", pathTemplate)); - name = params.iterator().next(); + key = namedSegments.iterator().next(); } - - RoutingHeader routingHeader = RoutingHeader.create(field, name, pathTemplate); - builder.addRoutingHeader(routingHeader); - } - - return builder.build(); - } - - // TODO: duplicate of HttpRuleParser.getPatternBindings, move to a helper class - private static ImmutableSortedSet.Builder getPatternBindings(String pattern) { - ImmutableSortedSet.Builder bindings = ImmutableSortedSet.naturalOrder(); - if (pattern.isEmpty()) { - return bindings; + RoutingHeaderParam routingHeaderParam = + RoutingHeaderParam.create(fieldName, key, pathTemplate); + routingHeaderRuleBuilder.addParam(routingHeaderParam); } - - PathTemplate template = PathTemplate.create(pattern); - // Filter out any unbound variable like "$0, $1, etc. - bindings.addAll( - template.vars().stream().filter(s -> !s.contains("$")).collect(Collectors.toSet())); - return bindings; + return routingHeaderRuleBuilder.build(); } } diff --git a/src/test/java/com/google/api/generator/gapic/composer/grpc/goldens/GrpcRoutingHeadersStub.golden b/src/test/java/com/google/api/generator/gapic/composer/grpc/goldens/GrpcRoutingHeadersStub.golden index 399456316c..84489dd164 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/grpc/goldens/GrpcRoutingHeadersStub.golden +++ b/src/test/java/com/google/api/generator/gapic/composer/grpc/goldens/GrpcRoutingHeadersStub.golden @@ -4,12 +4,13 @@ import com.google.api.gax.core.BackgroundResource; import com.google.api.gax.core.BackgroundResourceAggregation; import com.google.api.gax.grpc.GrpcCallSettings; import com.google.api.gax.grpc.GrpcStubCallableFactory; -import com.google.api.gax.grpc.RoutingHeaderParamsBuilder; import com.google.api.gax.rpc.ClientContext; +import com.google.api.gax.rpc.RequestParamsBuilder; import com.google.api.gax.rpc.UnaryCallable; import com.google.api.pathtemplate.PathTemplate; import com.google.common.collect.ImmutableMap; import com.google.explicit.dynamic.routing.header.Request; +import com.google.explicit.dynamic.routing.header.RequestWithNestedField; import com.google.longrunning.stub.GrpcOperationsStub; import com.google.protobuf.Empty; import io.grpc.MethodDescriptor; @@ -145,6 +146,17 @@ public class GrpcExplicitDynamicRoutingHeaderTestingStub .setResponseMarshaller(ProtoUtils.marshaller(Empty.getDefaultInstance())) .build(); + private static final MethodDescriptor + nestedFieldTestMethodDescriptor = + MethodDescriptor.newBuilder() + .setType(MethodDescriptor.MethodType.UNARY) + .setFullMethodName( + "google.explicit.dynamic.routing.header.ExplicitDynamicRoutingHeaderTesting/NestedFieldTest") + .setRequestMarshaller( + ProtoUtils.marshaller(RequestWithNestedField.getDefaultInstance())) + .setResponseMarshaller(ProtoUtils.marshaller(Empty.getDefaultInstance())) + .build(); + private final UnaryCallable example1TestCallable; private final UnaryCallable example2TestCallable; private final UnaryCallable example3TestCallable; @@ -158,52 +170,55 @@ public class GrpcExplicitDynamicRoutingHeaderTestingStub private final UnaryCallable backwardsCompatible1TestCallable; private final UnaryCallable backwardsCompatible2TestCallable; private final UnaryCallable backwardsCompatible3TestCallable; + private final UnaryCallable nestedFieldTestCallable; private final BackgroundResource backgroundResources; private final GrpcOperationsStub operationsStub; private final GrpcStubCallableFactory callableFactory; - private static final PathTemplate PATH_TEMPLATE_EXAMPLE1TEST_0 = + private static final PathTemplate EXAMPLE1_TEST_0_PATH_TEMPLATE = PathTemplate.create("{app_profile_id=**}"); - private static final PathTemplate PATH_TEMPLATE_EXAMPLE2TEST_0 = + private static final PathTemplate EXAMPLE2_TEST_0_PATH_TEMPLATE = PathTemplate.create("{routing_id=**}"); - private static final PathTemplate PATH_TEMPLATE_EXAMPLE3TEST_0 = + private static final PathTemplate EXAMPLE3_TEST_0_PATH_TEMPLATE = PathTemplate.create("{table_name=projects/*/instances/*/**}"); - private static final PathTemplate PATH_TEMPLATE_EXAMPLE3CTEST_0 = + private static final PathTemplate EXAMPLE3_C_TEST_0_PATH_TEMPLATE = PathTemplate.create("{table_name=regions/*/zones/*/**}"); - private static final PathTemplate PATH_TEMPLATE_EXAMPLE3CTEST_1 = + private static final PathTemplate EXAMPLE3_C_TEST_1_PATH_TEMPLATE = PathTemplate.create("{table_name=projects/*/instances/*/**}"); - private static final PathTemplate PATH_TEMPLATE_EXAMPLE4TEST_0 = + private static final PathTemplate EXAMPLE4_TEST_0_PATH_TEMPLATE = PathTemplate.create("{routing_id=projects/*}/**"); - private static final PathTemplate PATH_TEMPLATE_EXAMPLE5TEST_0 = + private static final PathTemplate EXAMPLE5_TEST_0_PATH_TEMPLATE = PathTemplate.create("{routing_id=projects/*}/**"); - private static final PathTemplate PATH_TEMPLATE_EXAMPLE5TEST_1 = + private static final PathTemplate EXAMPLE5_TEST_1_PATH_TEMPLATE = PathTemplate.create("{routing_id=projects/*/instances/*}/**"); - private static final PathTemplate PATH_TEMPLATE_EXAMPLE6TEST_0 = + private static final PathTemplate EXAMPLE6_TEST_0_PATH_TEMPLATE = PathTemplate.create("{project_id=projects/*}/instances/*/**"); - private static final PathTemplate PATH_TEMPLATE_EXAMPLE6TEST_1 = + private static final PathTemplate EXAMPLE6_TEST_1_PATH_TEMPLATE = PathTemplate.create("projects/*/{instance_id=instances/*}/**"); - private static final PathTemplate PATH_TEMPLATE_EXAMPLE7TEST_0 = + private static final PathTemplate EXAMPLE7_TEST_0_PATH_TEMPLATE = PathTemplate.create("{project_id=projects/*}/**"); - private static final PathTemplate PATH_TEMPLATE_EXAMPLE7TEST_1 = + private static final PathTemplate EXAMPLE7_TEST_1_PATH_TEMPLATE = PathTemplate.create("{routing_id=**}"); - private static final PathTemplate PATH_TEMPLATE_EXAMPLE8TEST_0 = + private static final PathTemplate EXAMPLE8_TEST_0_PATH_TEMPLATE = PathTemplate.create("{routing_id=projects/*}/**"); - private static final PathTemplate PATH_TEMPLATE_EXAMPLE8TEST_1 = + private static final PathTemplate EXAMPLE8_TEST_1_PATH_TEMPLATE = PathTemplate.create("{routing_id=regions/*}/**"); - private static final PathTemplate PATH_TEMPLATE_EXAMPLE8TEST_2 = + private static final PathTemplate EXAMPLE8_TEST_2_PATH_TEMPLATE = PathTemplate.create("{routing_id=**}"); - private static final PathTemplate PATH_TEMPLATE_EXAMPLE9TEST_0 = + private static final PathTemplate EXAMPLE9_TEST_0_PATH_TEMPLATE = PathTemplate.create("projects/*/{table_location=instances/*}/tables/*"); - private static final PathTemplate PATH_TEMPLATE_EXAMPLE9TEST_1 = + private static final PathTemplate EXAMPLE9_TEST_1_PATH_TEMPLATE = PathTemplate.create("{table_location=regions/*/zones/*}/tables/*"); - private static final PathTemplate PATH_TEMPLATE_EXAMPLE9TEST_2 = + private static final PathTemplate EXAMPLE9_TEST_2_PATH_TEMPLATE = PathTemplate.create("{routing_id=projects/*}/**"); - private static final PathTemplate PATH_TEMPLATE_EXAMPLE9TEST_3 = + private static final PathTemplate EXAMPLE9_TEST_3_PATH_TEMPLATE = PathTemplate.create("{routing_id=**}"); - private static final PathTemplate PATH_TEMPLATE_EXAMPLE9TEST_4 = + private static final PathTemplate EXAMPLE9_TEST_4_PATH_TEMPLATE = PathTemplate.create("profiles/{routing_id=*}"); - private static final PathTemplate PATH_TEMPLATE_BACKWARDSCOMPATIBLE1TEST_0 = + private static final PathTemplate BACKWARDS_COMPATIBLE1_TEST_0_PATH_TEMPLATE = + PathTemplate.create("{routing_id=projects/*}/**"); + private static final PathTemplate NESTED_FIELD_TEST_0_PATH_TEMPLATE = PathTemplate.create("{routing_id=projects/*}/**"); public static final GrpcExplicitDynamicRoutingHeaderTestingStub create( @@ -255,13 +270,9 @@ public class GrpcExplicitDynamicRoutingHeaderTestingStub .setMethodDescriptor(example1TestMethodDescriptor) .setParamsExtractor( request -> { - RoutingHeaderParamsBuilder builder = new RoutingHeaderParamsBuilder(); - if (request.getAppProfileId() != null) { - builder.add( - String.valueOf(request.getAppProfileId()), - "app_profile_id", - PATH_TEMPLATE_EXAMPLE1TEST_0); - } + RequestParamsBuilder builder = RequestParamsBuilder.create(); + builder.add( + request.getAppProfileId(), "app_profile_id", EXAMPLE1_TEST_0_PATH_TEMPLATE); return builder.build(); }) .build(); @@ -270,13 +281,9 @@ public class GrpcExplicitDynamicRoutingHeaderTestingStub .setMethodDescriptor(example2TestMethodDescriptor) .setParamsExtractor( request -> { - RoutingHeaderParamsBuilder builder = new RoutingHeaderParamsBuilder(); - if (request.getAppProfileId() != null) { - builder.add( - String.valueOf(request.getAppProfileId()), - "routing_id", - PATH_TEMPLATE_EXAMPLE2TEST_0); - } + RequestParamsBuilder builder = RequestParamsBuilder.create(); + builder.add( + request.getAppProfileId(), "routing_id", EXAMPLE2_TEST_0_PATH_TEMPLATE); return builder.build(); }) .build(); @@ -285,13 +292,8 @@ public class GrpcExplicitDynamicRoutingHeaderTestingStub .setMethodDescriptor(example3TestMethodDescriptor) .setParamsExtractor( request -> { - RoutingHeaderParamsBuilder builder = new RoutingHeaderParamsBuilder(); - if (request.getTableName() != null) { - builder.add( - String.valueOf(request.getTableName()), - "table_name", - PATH_TEMPLATE_EXAMPLE3TEST_0); - } + RequestParamsBuilder builder = RequestParamsBuilder.create(); + builder.add(request.getTableName(), "table_name", EXAMPLE3_TEST_0_PATH_TEMPLATE); return builder.build(); }) .build(); @@ -300,19 +302,11 @@ public class GrpcExplicitDynamicRoutingHeaderTestingStub .setMethodDescriptor(example3CTestMethodDescriptor) .setParamsExtractor( request -> { - RoutingHeaderParamsBuilder builder = new RoutingHeaderParamsBuilder(); - if (request.getTableName() != null) { - builder.add( - String.valueOf(request.getTableName()), - "table_name", - PATH_TEMPLATE_EXAMPLE3CTEST_0); - } - if (request.getTableName() != null) { - builder.add( - String.valueOf(request.getTableName()), - "table_name", - PATH_TEMPLATE_EXAMPLE3CTEST_1); - } + RequestParamsBuilder builder = RequestParamsBuilder.create(); + builder.add( + request.getTableName(), "table_name", EXAMPLE3_C_TEST_0_PATH_TEMPLATE); + builder.add( + request.getTableName(), "table_name", EXAMPLE3_C_TEST_1_PATH_TEMPLATE); return builder.build(); }) .build(); @@ -321,13 +315,8 @@ public class GrpcExplicitDynamicRoutingHeaderTestingStub .setMethodDescriptor(example4TestMethodDescriptor) .setParamsExtractor( request -> { - RoutingHeaderParamsBuilder builder = new RoutingHeaderParamsBuilder(); - if (request.getTableName() != null) { - builder.add( - String.valueOf(request.getTableName()), - "routing_id", - PATH_TEMPLATE_EXAMPLE4TEST_0); - } + RequestParamsBuilder builder = RequestParamsBuilder.create(); + builder.add(request.getTableName(), "routing_id", EXAMPLE4_TEST_0_PATH_TEMPLATE); return builder.build(); }) .build(); @@ -336,19 +325,9 @@ public class GrpcExplicitDynamicRoutingHeaderTestingStub .setMethodDescriptor(example5TestMethodDescriptor) .setParamsExtractor( request -> { - RoutingHeaderParamsBuilder builder = new RoutingHeaderParamsBuilder(); - if (request.getTableName() != null) { - builder.add( - String.valueOf(request.getTableName()), - "routing_id", - PATH_TEMPLATE_EXAMPLE5TEST_0); - } - if (request.getTableName() != null) { - builder.add( - String.valueOf(request.getTableName()), - "routing_id", - PATH_TEMPLATE_EXAMPLE5TEST_1); - } + RequestParamsBuilder builder = RequestParamsBuilder.create(); + builder.add(request.getTableName(), "routing_id", EXAMPLE5_TEST_0_PATH_TEMPLATE); + builder.add(request.getTableName(), "routing_id", EXAMPLE5_TEST_1_PATH_TEMPLATE); return builder.build(); }) .build(); @@ -357,19 +336,9 @@ public class GrpcExplicitDynamicRoutingHeaderTestingStub .setMethodDescriptor(example6TestMethodDescriptor) .setParamsExtractor( request -> { - RoutingHeaderParamsBuilder builder = new RoutingHeaderParamsBuilder(); - if (request.getTableName() != null) { - builder.add( - String.valueOf(request.getTableName()), - "project_id", - PATH_TEMPLATE_EXAMPLE6TEST_0); - } - if (request.getTableName() != null) { - builder.add( - String.valueOf(request.getTableName()), - "instance_id", - PATH_TEMPLATE_EXAMPLE6TEST_1); - } + RequestParamsBuilder builder = RequestParamsBuilder.create(); + builder.add(request.getTableName(), "project_id", EXAMPLE6_TEST_0_PATH_TEMPLATE); + builder.add(request.getTableName(), "instance_id", EXAMPLE6_TEST_1_PATH_TEMPLATE); return builder.build(); }) .build(); @@ -378,19 +347,10 @@ public class GrpcExplicitDynamicRoutingHeaderTestingStub .setMethodDescriptor(example7TestMethodDescriptor) .setParamsExtractor( request -> { - RoutingHeaderParamsBuilder builder = new RoutingHeaderParamsBuilder(); - if (request.getTableName() != null) { - builder.add( - String.valueOf(request.getTableName()), - "project_id", - PATH_TEMPLATE_EXAMPLE7TEST_0); - } - if (request.getAppProfileId() != null) { - builder.add( - String.valueOf(request.getAppProfileId()), - "routing_id", - PATH_TEMPLATE_EXAMPLE7TEST_1); - } + RequestParamsBuilder builder = RequestParamsBuilder.create(); + builder.add(request.getTableName(), "project_id", EXAMPLE7_TEST_0_PATH_TEMPLATE); + builder.add( + request.getAppProfileId(), "routing_id", EXAMPLE7_TEST_1_PATH_TEMPLATE); return builder.build(); }) .build(); @@ -399,25 +359,11 @@ public class GrpcExplicitDynamicRoutingHeaderTestingStub .setMethodDescriptor(example8TestMethodDescriptor) .setParamsExtractor( request -> { - RoutingHeaderParamsBuilder builder = new RoutingHeaderParamsBuilder(); - if (request.getTableName() != null) { - builder.add( - String.valueOf(request.getTableName()), - "routing_id", - PATH_TEMPLATE_EXAMPLE8TEST_0); - } - if (request.getTableName() != null) { - builder.add( - String.valueOf(request.getTableName()), - "routing_id", - PATH_TEMPLATE_EXAMPLE8TEST_1); - } - if (request.getAppProfileId() != null) { - builder.add( - String.valueOf(request.getAppProfileId()), - "routing_id", - PATH_TEMPLATE_EXAMPLE8TEST_2); - } + RequestParamsBuilder builder = RequestParamsBuilder.create(); + builder.add(request.getTableName(), "routing_id", EXAMPLE8_TEST_0_PATH_TEMPLATE); + builder.add(request.getTableName(), "routing_id", EXAMPLE8_TEST_1_PATH_TEMPLATE); + builder.add( + request.getAppProfileId(), "routing_id", EXAMPLE8_TEST_2_PATH_TEMPLATE); return builder.build(); }) .build(); @@ -426,37 +372,16 @@ public class GrpcExplicitDynamicRoutingHeaderTestingStub .setMethodDescriptor(example9TestMethodDescriptor) .setParamsExtractor( request -> { - RoutingHeaderParamsBuilder builder = new RoutingHeaderParamsBuilder(); - if (request.getTableName() != null) { - builder.add( - String.valueOf(request.getTableName()), - "table_location", - PATH_TEMPLATE_EXAMPLE9TEST_0); - } - if (request.getTableName() != null) { - builder.add( - String.valueOf(request.getTableName()), - "table_location", - PATH_TEMPLATE_EXAMPLE9TEST_1); - } - if (request.getTableName() != null) { - builder.add( - String.valueOf(request.getTableName()), - "routing_id", - PATH_TEMPLATE_EXAMPLE9TEST_2); - } - if (request.getAppProfileId() != null) { - builder.add( - String.valueOf(request.getAppProfileId()), - "routing_id", - PATH_TEMPLATE_EXAMPLE9TEST_3); - } - if (request.getAppProfileId() != null) { - builder.add( - String.valueOf(request.getAppProfileId()), - "routing_id", - PATH_TEMPLATE_EXAMPLE9TEST_4); - } + RequestParamsBuilder builder = RequestParamsBuilder.create(); + builder.add( + request.getTableName(), "table_location", EXAMPLE9_TEST_0_PATH_TEMPLATE); + builder.add( + request.getTableName(), "table_location", EXAMPLE9_TEST_1_PATH_TEMPLATE); + builder.add(request.getTableName(), "routing_id", EXAMPLE9_TEST_2_PATH_TEMPLATE); + builder.add( + request.getAppProfileId(), "routing_id", EXAMPLE9_TEST_3_PATH_TEMPLATE); + builder.add( + request.getAppProfileId(), "routing_id", EXAMPLE9_TEST_4_PATH_TEMPLATE); return builder.build(); }) .build(); @@ -465,24 +390,17 @@ public class GrpcExplicitDynamicRoutingHeaderTestingStub .setMethodDescriptor(backwardsCompatible1TestMethodDescriptor) .setParamsExtractor( request -> { - RoutingHeaderParamsBuilder builder = new RoutingHeaderParamsBuilder(); - if (request.getTableName() != null) { - builder.add( - String.valueOf(request.getTableName()), - "routing_id", - PATH_TEMPLATE_BACKWARDSCOMPATIBLE1TEST_0); - } + RequestParamsBuilder builder = RequestParamsBuilder.create(); + builder.add( + request.getTableName(), + "routing_id", + BACKWARDS_COMPATIBLE1_TEST_0_PATH_TEMPLATE); return builder.build(); }) .build(); GrpcCallSettings backwardsCompatible2TestTransportSettings = GrpcCallSettings.newBuilder() .setMethodDescriptor(backwardsCompatible2TestMethodDescriptor) - .setParamsExtractor( - request -> { - RoutingHeaderParamsBuilder builder = new RoutingHeaderParamsBuilder(); - return builder.build(); - }) .build(); GrpcCallSettings backwardsCompatible3TestTransportSettings = GrpcCallSettings.newBuilder() @@ -494,6 +412,22 @@ public class GrpcExplicitDynamicRoutingHeaderTestingStub return params.build(); }) .build(); + GrpcCallSettings nestedFieldTestTransportSettings = + GrpcCallSettings.newBuilder() + .setMethodDescriptor(nestedFieldTestMethodDescriptor) + .setParamsExtractor( + request -> { + RequestParamsBuilder builder = RequestParamsBuilder.create(); + if (request.getNestedField() != null + && request.getNestedField().getAnotherNestedField() != null) { + builder.add( + request.getNestedField().getAnotherNestedField().getName(), + "routing_id", + NESTED_FIELD_TEST_0_PATH_TEMPLATE); + } + return builder.build(); + }) + .build(); this.example1TestCallable = callableFactory.createUnaryCallable( @@ -540,6 +474,9 @@ public class GrpcExplicitDynamicRoutingHeaderTestingStub backwardsCompatible3TestTransportSettings, settings.backwardsCompatible3TestSettings(), clientContext); + this.nestedFieldTestCallable = + callableFactory.createUnaryCallable( + nestedFieldTestTransportSettings, settings.nestedFieldTestSettings(), clientContext); this.backgroundResources = new BackgroundResourceAggregation(clientContext.getBackgroundResources()); @@ -614,6 +551,11 @@ public class GrpcExplicitDynamicRoutingHeaderTestingStub return backwardsCompatible3TestCallable; } + @Override + public UnaryCallable nestedFieldTestCallable() { + return nestedFieldTestCallable; + } + @Override public final void close() { try { diff --git a/src/test/java/com/google/api/generator/gapic/composer/grpc/goldens/GrpcTestingStub.golden b/src/test/java/com/google/api/generator/gapic/composer/grpc/goldens/GrpcTestingStub.golden index 73146650b8..6c0a667291 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/grpc/goldens/GrpcTestingStub.golden +++ b/src/test/java/com/google/api/generator/gapic/composer/grpc/goldens/GrpcTestingStub.golden @@ -8,8 +8,8 @@ import com.google.api.gax.core.BackgroundResource; import com.google.api.gax.core.BackgroundResourceAggregation; import com.google.api.gax.grpc.GrpcCallSettings; import com.google.api.gax.grpc.GrpcStubCallableFactory; -import com.google.api.gax.grpc.RoutingHeaderParamsBuilder; import com.google.api.gax.rpc.ClientContext; +import com.google.api.gax.rpc.RequestParamsBuilder; import com.google.api.gax.rpc.UnaryCallable; import com.google.api.pathtemplate.PathTemplate; import com.google.common.collect.ImmutableMap; @@ -144,9 +144,9 @@ public class GrpcTestingStub extends TestingStub { private final GrpcOperationsStub operationsStub; private final GrpcStubCallableFactory callableFactory; - private static final PathTemplate PATH_TEMPLATE_GETTEST_0 = + private static final PathTemplate GET_TEST_0_PATH_TEMPLATE = PathTemplate.create("/v1beta1/{rename=tests/*}"); - private static final PathTemplate PATH_TEMPLATE_GETTEST_1 = + private static final PathTemplate GET_TEST_1_PATH_TEMPLATE = PathTemplate.create("/v1beta1/{routing_id=tests/*}"); public static final GrpcTestingStub create(TestingStubSettings settings) throws IOException { @@ -227,16 +227,11 @@ public class GrpcTestingStub extends TestingStub { .setMethodDescriptor(getTestMethodDescriptor) .setParamsExtractor( request -> { - RoutingHeaderParamsBuilder builder = new RoutingHeaderParamsBuilder(); - if (request.getName() != null) { + RequestParamsBuilder builder = RequestParamsBuilder.create(); + builder.add(request.getName(), "rename", GET_TEST_0_PATH_TEMPLATE); + if (request.getRouting() != null) { builder.add( - String.valueOf(request.getName()), "rename", PATH_TEMPLATE_GETTEST_0); - } - if (request.getRouting() != null && request.getRouting().getName() != null) { - builder.add( - String.valueOf(request.getRouting().getName()), - "routing_id", - PATH_TEMPLATE_GETTEST_1); + request.getRouting().getName(), "routing_id", GET_TEST_1_PATH_TEMPLATE); } return builder.build(); }) diff --git a/src/test/java/com/google/api/generator/gapic/model/BUILD.bazel b/src/test/java/com/google/api/generator/gapic/model/BUILD.bazel index 7c1ad5181f..bc62109ad9 100644 --- a/src/test/java/com/google/api/generator/gapic/model/BUILD.bazel +++ b/src/test/java/com/google/api/generator/gapic/model/BUILD.bazel @@ -7,7 +7,7 @@ TESTS = [ "MethodArgumentTest", "MethodTest", "MessageTest", - "RoutingHeadersTest", + "RoutingHeaderParamTest", ] filegroup( diff --git a/src/test/java/com/google/api/generator/gapic/model/MessageTest.java b/src/test/java/com/google/api/generator/gapic/model/MessageTest.java index 76f19c2b87..469c86c212 100644 --- a/src/test/java/com/google/api/generator/gapic/model/MessageTest.java +++ b/src/test/java/com/google/api/generator/gapic/model/MessageTest.java @@ -25,8 +25,13 @@ public class MessageTest { - public static final String MESSAGE_NAME = "TestMessage"; - public static final Message.Builder TEST_MESSAGE_BUILDER = + private static final String SUB_FIELD_NAME = "table"; + private static final String LEAF_FIELD_NAME = "size"; + private static final String SUB_FIELD_TYPE = "TableFieldType"; + public static final VaporReference FIELD_TYPE = + VaporReference.builder().setPakkage("com.google").setName(SUB_FIELD_TYPE).build(); + private static final String MESSAGE_NAME = "TestMessage"; + private static final Message.Builder testMessageBuilder = Message.builder() .setName(MESSAGE_NAME) .setFullProtoName("com.google.test.TestMessage") @@ -34,22 +39,26 @@ public class MessageTest { @Test public void validateField_shouldThrowExceptionIfFieldNameIsEmpty() { - Message message = TEST_MESSAGE_BUILDER.build(); + Message message = testMessageBuilder.build(); IllegalStateException illegalStateException = assertThrows( - IllegalStateException.class, () -> message.validateField("", ImmutableMap.of())); - assertThat(illegalStateException.getMessage()) + IllegalStateException.class, + () -> message.validateField("", ImmutableMap.of(), TypeNode.STRING)); + assertThat(illegalStateException) + .hasMessageThat() .isEqualTo(String.format("Null or empty field name found for message %s", MESSAGE_NAME)); } @Test public void validateField_shouldThrowExceptionIfFieldDoesNotExist() { - Message message = TEST_MESSAGE_BUILDER.build(); + Message message = testMessageBuilder.build(); String fieldName = "doesNotExist"; NullPointerException nullPointerException = assertThrows( - NullPointerException.class, () -> message.validateField(fieldName, ImmutableMap.of())); - assertThat(nullPointerException.getMessage()) + NullPointerException.class, + () -> message.validateField(fieldName, ImmutableMap.of(), TypeNode.STRING)); + assertThat(nullPointerException) + .hasMessageThat() .isEqualTo( String.format( "Expected message %s to contain field %s but none found", MESSAGE_NAME, fieldName)); @@ -57,54 +66,86 @@ public void validateField_shouldThrowExceptionIfFieldDoesNotExist() { @Test public void validateField_shouldThrowExceptionIfMessageDoesNotExist() { - String subFieldName = "table"; - String fieldTypeName = "doesNotMatter"; Field subField = Field.builder() - .setName(subFieldName) + .setName(SUB_FIELD_NAME) .setType( TypeNode.withReference( VaporReference.builder() .setPakkage("com.google") - .setName(fieldTypeName) + .setName(SUB_FIELD_TYPE) .build())) .build(); Message message = - TEST_MESSAGE_BUILDER.setFieldMap(ImmutableMap.of(subFieldName, subField)).build(); - String fieldName = subFieldName + "." + "size"; + testMessageBuilder.setFieldMap(ImmutableMap.of(SUB_FIELD_NAME, subField)).build(); + String fieldName = SUB_FIELD_NAME + "." + LEAF_FIELD_NAME; NullPointerException nullPointerException = assertThrows( - NullPointerException.class, () -> message.validateField(fieldName, ImmutableMap.of())); - assertThat(nullPointerException.getMessage()) + NullPointerException.class, + () -> message.validateField(fieldName, ImmutableMap.of(), TypeNode.STRING)); + assertThat(nullPointerException) + .hasMessageThat() .isEqualTo( String.format( "No containing message found for field %s with type %s", - subFieldName, fieldTypeName)); + SUB_FIELD_NAME, SUB_FIELD_TYPE)); + } + + @Test + public void validateField_shouldThrowExceptionIfFieldIsRepeated() { + Field leafField = + Field.builder() + .setType(TypeNode.STRING) + .setIsRepeated(true) + .setName(LEAF_FIELD_NAME) + .build(); + testLeafField(leafField); + } + + @Test + public void validateField_shouldThrowExceptionIfFieldIsOfWrongType() { + Field leafField = Field.builder().setType(TypeNode.BOOLEAN).setName(LEAF_FIELD_NAME).build(); + testLeafField(leafField); + } + + private void testLeafField(Field leafField) { + Message subMessage = createSubMessage(leafField); + Map messageTypes = ImmutableMap.of(FIELD_TYPE.fullName(), subMessage); + IllegalStateException illegalStateException = + assertThrows( + IllegalStateException.class, + () -> + createdMessage() + .validateField( + SUB_FIELD_NAME + "." + LEAF_FIELD_NAME, messageTypes, TypeNode.STRING)); + assertThat(illegalStateException) + .hasMessageThat() + .isEqualTo( + String.format( + "The type of field %s must be String and not repeated.", LEAF_FIELD_NAME)); } @Test public void validateField_shouldNotThrowExceptionIfFieldExist() { - String subFieldName = "table"; - String fieldTypeName = "TableFieldType"; - VaporReference fieldType = - VaporReference.builder().setPakkage("com.google").setName(fieldTypeName).build(); + Field leafField = Field.builder().setType(TypeNode.STRING).setName(LEAF_FIELD_NAME).build(); + Message subMessage = createSubMessage(leafField); + Map messageTypes = ImmutableMap.of(FIELD_TYPE.fullName(), subMessage); + createdMessage() + .validateField(SUB_FIELD_NAME + "." + LEAF_FIELD_NAME, messageTypes, TypeNode.STRING); + } + + private Message createdMessage() { Field subField = - Field.builder().setName(subFieldName).setType(TypeNode.withReference(fieldType)).build(); - String subFieldName2 = "size"; - String fieldName = subFieldName + "." + subFieldName2; - Message subMessage = - Message.builder() - .setName(fieldTypeName) - .setFullProtoName("com.google." + fieldTypeName) - .setType(TypeNode.OBJECT) - .setFieldMap( - ImmutableMap.of( - subFieldName2, - Field.builder().setType(TypeNode.STRING).setName(subFieldName2).build())) - .build(); - Map messageTypes = ImmutableMap.of(fieldType.fullName(), subMessage); - Message message = - TEST_MESSAGE_BUILDER.setFieldMap(ImmutableMap.of(subFieldName, subField)).build(); - message.validateField(fieldName, messageTypes); + Field.builder().setName(SUB_FIELD_NAME).setType(TypeNode.withReference(FIELD_TYPE)).build(); + return testMessageBuilder.setFieldMap(ImmutableMap.of(SUB_FIELD_NAME, subField)).build(); + } + + private Message createSubMessage(Field leafField) { + return Message.builder() + .setName(SUB_FIELD_TYPE) + .setFullProtoName("com.google." + SUB_FIELD_TYPE) + .setType(TypeNode.OBJECT) + .setFieldMap(ImmutableMap.of(LEAF_FIELD_NAME, leafField)) + .build(); } } diff --git a/src/test/java/com/google/api/generator/gapic/model/MethodTest.java b/src/test/java/com/google/api/generator/gapic/model/MethodTest.java index 2c245151f4..fd5bba4b49 100644 --- a/src/test/java/com/google/api/generator/gapic/model/MethodTest.java +++ b/src/test/java/com/google/api/generator/gapic/model/MethodTest.java @@ -16,9 +16,29 @@ import static com.google.common.truth.Truth.assertThat; +import com.google.api.generator.engine.ast.TypeNode; +import com.google.api.generator.gapic.model.HttpBindings.HttpBinding; +import com.google.api.generator.gapic.model.HttpBindings.HttpVerb; +import com.google.api.generator.gapic.model.RoutingHeaderRule.RoutingHeaderParam; +import com.google.common.collect.ImmutableSet; import org.junit.Test; public class MethodTest { + + private static final Method METHOD = + Method.builder() + .setName("My method") + .setInputType(TypeNode.STRING) + .setOutputType(TypeNode.STRING) + .build(); + private static final HttpBindings HTTP_BINDINGS = + HttpBindings.builder() + .setPathParameters(ImmutableSet.of(HttpBinding.create("table", true, ""))) + .setPattern("/pattern/test") + .setIsAsteriskBody(false) + .setHttpVerb(HttpVerb.GET) + .build(); + @Test public void toStream() { // Argument order: isClientStreaming, isServerStreaming. @@ -27,4 +47,67 @@ public void toStream() { assertThat(Method.toStream(false, true)).isEqualTo(Method.Stream.SERVER); assertThat(Method.toStream(true, true)).isEqualTo(Method.Stream.BIDI); } + + @Test + public void hasRoutingHeaders_shouldReturnFalseIfRoutingHeadersIsNull() { + assertThat(METHOD.hasRoutingHeaderParams()).isFalse(); + } + + @Test + public void hasRoutingHeaders_shouldReturnFalseIfRoutingHeadersIsEmpty() { + Method method = + METHOD.toBuilder().setRoutingHeaderRule(RoutingHeaderRule.builder().build()).build(); + assertThat(method.hasRoutingHeaderParams()).isFalse(); + } + + @Test + public void hasRoutingHeaders_shouldReturnTrueIfRoutingHeadersIsNotEmpty() { + Method method = + METHOD + .toBuilder() + .setRoutingHeaderRule( + RoutingHeaderRule.builder() + .addParam(RoutingHeaderParam.create("table", "routing_id", "")) + .build()) + .build(); + assertThat(method.hasRoutingHeaderParams()).isTrue(); + } + + @Test + public void shouldSetParamsExtractor_shouldReturnTrueIfHasRoutingHeaders() { + Method method = + METHOD + .toBuilder() + .setRoutingHeaderRule( + RoutingHeaderRule.builder() + .addParam(RoutingHeaderParam.create("table", "routing_id", "")) + .build()) + .build(); + assertThat(method.shouldSetParamsExtractor()).isTrue(); + } + + @Test + public void shouldSetParamsExtractor_shouldReturnTrueIfHasHttpBindingsAndRoutingHeadersIsNull() { + Method method = + METHOD.toBuilder().setHttpBindings(HTTP_BINDINGS).setRoutingHeaderRule(null).build(); + assertThat(method.shouldSetParamsExtractor()).isTrue(); + } + + @Test + public void + shouldSetParamsExtractor_shouldReturnFalseIfHasHttpBindingsAndRoutingHeadersIsEmpty() { + Method method = + METHOD + .toBuilder() + .setHttpBindings(HTTP_BINDINGS) + .setRoutingHeaderRule(RoutingHeaderRule.builder().build()) + .build(); + assertThat(method.shouldSetParamsExtractor()).isFalse(); + } + + @Test + public void shouldSetParamsExtractor_shouldReturnFalseIfHasNoHttpBindingsAndNoRoutingHeaders() { + Method method = METHOD.toBuilder().setHttpBindings(null).setRoutingHeaderRule(null).build(); + assertThat(method.shouldSetParamsExtractor()).isFalse(); + } } diff --git a/src/test/java/com/google/api/generator/gapic/model/RoutingHeadersTest.java b/src/test/java/com/google/api/generator/gapic/model/RoutingHeaderParamTest.java similarity index 73% rename from src/test/java/com/google/api/generator/gapic/model/RoutingHeadersTest.java rename to src/test/java/com/google/api/generator/gapic/model/RoutingHeaderParamTest.java index 3ae092f456..8c2d1d361e 100644 --- a/src/test/java/com/google/api/generator/gapic/model/RoutingHeadersTest.java +++ b/src/test/java/com/google/api/generator/gapic/model/RoutingHeaderParamTest.java @@ -16,16 +16,17 @@ import static com.google.common.truth.Truth.assertThat; -import com.google.api.generator.gapic.model.RoutingHeaders.RoutingHeader; +import com.google.api.generator.gapic.model.RoutingHeaderRule.RoutingHeaderParam; import java.util.List; import org.junit.Test; -public class RoutingHeadersTest { +public class RoutingHeaderParamTest { @Test public void getDescendantFieldNames_shouldSplitFieldNameByDot() { - RoutingHeader routingHeader = RoutingHeader.create("table.name", "name", "/abc/dec"); - List descendantFieldNames = routingHeader.getDescendantFieldNames(); + RoutingHeaderParam routingHeaderParam = + RoutingHeaderParam.create("table.name", "name", "/abc/dec"); + List descendantFieldNames = routingHeaderParam.getDescendantFieldNames(); assertThat(descendantFieldNames).containsExactly("table", "name"); } } diff --git a/src/test/java/com/google/api/generator/gapic/protoparser/BUILD.bazel b/src/test/java/com/google/api/generator/gapic/protoparser/BUILD.bazel index 09fa41b0ef..ead36d5fb0 100644 --- a/src/test/java/com/google/api/generator/gapic/protoparser/BUILD.bazel +++ b/src/test/java/com/google/api/generator/gapic/protoparser/BUILD.bazel @@ -16,7 +16,8 @@ TESTS = [ "ServiceYamlParserTest", "SourceCodeInfoParserTest", "TypeParserTest", - "RoutingRuleParserTest" + "RoutingRuleParserTest", + "PatternParserTest", ] filegroup( diff --git a/src/test/java/com/google/api/generator/gapic/protoparser/PatternParserTest.java b/src/test/java/com/google/api/generator/gapic/protoparser/PatternParserTest.java new file mode 100644 index 0000000000..8415ca2b21 --- /dev/null +++ b/src/test/java/com/google/api/generator/gapic/protoparser/PatternParserTest.java @@ -0,0 +1,40 @@ +// Copyright 2022 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.api.generator.gapic.protoparser; + +import static com.google.common.truth.Truth.assertThat; + +import java.util.Set; +import org.junit.Test; + +public class PatternParserTest { + @Test + public void getPattenBindings_shouldReturnEmptySetIfPatternIsEmpty() { + assertThat(PatternParser.getPattenBindings("")).isEmpty(); + } + + @Test + public void getPattenBindings_shouldFilterOutUnboundVariables() { + Set actual = PatternParser.getPattenBindings("{routing_id=projects/*}/**"); + assertThat(actual).hasSize(1); + } + + @Test + public void getPattenBindings_shouldReturnBindingsInNatualOrder() { + Set actual = + PatternParser.getPattenBindings("{routing_id=projects/*}/{name=instance/*}"); + assertThat(actual).containsExactly("name", "routing_id").inOrder(); + } +} diff --git a/src/test/java/com/google/api/generator/gapic/protoparser/RoutingRuleParserTest.java b/src/test/java/com/google/api/generator/gapic/protoparser/RoutingRuleParserTest.java index 25d82232bb..0908d79080 100644 --- a/src/test/java/com/google/api/generator/gapic/protoparser/RoutingRuleParserTest.java +++ b/src/test/java/com/google/api/generator/gapic/protoparser/RoutingRuleParserTest.java @@ -18,8 +18,8 @@ import static org.junit.Assert.assertThrows; import com.google.api.generator.gapic.model.Message; -import com.google.api.generator.gapic.model.RoutingHeaders; -import com.google.api.generator.gapic.model.RoutingHeaders.RoutingHeader; +import com.google.api.generator.gapic.model.RoutingHeaderRule; +import com.google.api.generator.gapic.model.RoutingHeaderRule.RoutingHeaderParam; import com.google.explicit.dynamic.routing.header.RoutingRuleParserTestingOuterClass; import com.google.protobuf.Descriptors.FileDescriptor; import com.google.protobuf.Descriptors.MethodDescriptor; @@ -31,22 +31,21 @@ public class RoutingRuleParserTest { private static final FileDescriptor TESTING_FILE_DESCRIPTOR = RoutingRuleParserTestingOuterClass.getDescriptor(); - private static final Map MESSAGES = - Parser.parseMessages(TESTING_FILE_DESCRIPTOR); private static final ServiceDescriptor TESTING_SERVICE = TESTING_FILE_DESCRIPTOR.getServices().get(0); @Test public void parse_shouldReturnNullRoutingHeadersIfMethodHasNoRoutingRules() { - RoutingHeaders actual = getRoutingHeaders(0); + RoutingHeaderRule actual = getRoutingHeaders(0); assertThat(actual).isNull(); } @Test public void parse_shouldSetPathTemplateToWildcardIfNotDefined() { - RoutingHeaders actual = getRoutingHeaders(1); - RoutingHeader expected = RoutingHeader.create("name", "name", String.format("{%s=**}", "name")); - assertThat(actual.routingHeadersList()).containsExactly(expected); + RoutingHeaderRule actual = getRoutingHeaders(1); + RoutingHeaderParam expected = + RoutingHeaderParam.create("name", "name", String.format("{%s=**}", "name")); + assertThat(actual.routingHeaderParams()).containsExactly(expected); } @Test @@ -73,27 +72,30 @@ public void parse_shouldThrowExceptionIfPathTemplateHasMoreThanOneNamedSegment() @Test public void parse_shouldParseRoutingRulesWithOneParameter() { - RoutingHeaders actual = getRoutingHeaders(4); - RoutingHeader expected = RoutingHeader.create("name", "rename", "/v1beta1/{rename=tests/*}"); - assertThat(actual.routingHeadersList()).containsExactly(expected); + RoutingHeaderRule actual = getRoutingHeaders(4); + RoutingHeaderParam expected = + RoutingHeaderParam.create("name", "rename", "/v1beta1/{rename=tests/*}"); + assertThat(actual.routingHeaderParams()).containsExactly(expected); } @Test public void parse_shouldParseRoutingRulesWithMultipleParameter() { - RoutingHeaders actual = getRoutingHeaders(5); - RoutingHeader expectedHeader1 = - RoutingHeader.create("name", "rename", "/v1beta1/{rename=tests/*}"); - RoutingHeader expectedHeader2 = - RoutingHeader.create("routing_id", "id", "/v1beta1/{id=projects/*}/tables/*"); - assertThat(actual.routingHeadersList()).containsExactly(expectedHeader1, expectedHeader2); + RoutingHeaderRule actual = getRoutingHeaders(5); + RoutingHeaderParam expectedHeader1 = + RoutingHeaderParam.create("name", "rename", "/v1beta1/{rename=tests/*}"); + RoutingHeaderParam expectedHeader2 = + RoutingHeaderParam.create("routing_id", "id", "/v1beta1/{id=projects/*}/tables/*"); + assertThat(actual.routingHeaderParams()) + .containsExactly(expectedHeader1, expectedHeader2) + .inOrder(); } @Test public void parse_shouldParseRoutingRulesWithNestedFields() { - RoutingHeaders actual = getRoutingHeaders(6); - RoutingHeader expectedHeader1 = - RoutingHeader.create("account.name", "rename", "/v1beta1/{rename=tests/*}"); - assertThat(actual.routingHeadersList()).containsExactly(expectedHeader1); + RoutingHeaderRule actual = getRoutingHeaders(6); + RoutingHeaderParam expectedHeader1 = + RoutingHeaderParam.create("account.name", "rename", "/v1beta1/{rename=tests/*}"); + assertThat(actual.routingHeaderParams()).containsExactly(expectedHeader1); } @Test @@ -101,9 +103,10 @@ public void parse_shouldThrowExceptionIfFieldValidationFailed() { assertThrows(Exception.class, () -> getRoutingHeaders(7)); } - private RoutingHeaders getRoutingHeaders(int testingIndex) { + private RoutingHeaderRule getRoutingHeaders(int testingIndex) { MethodDescriptor rpcMethod = TESTING_SERVICE.getMethods().get(testingIndex); - Message inputMessage = MESSAGES.get("com." + rpcMethod.getInputType().getFullName()); - return RoutingRuleParser.parse(rpcMethod, inputMessage, MESSAGES); + Map messages = Parser.parseMessages(TESTING_FILE_DESCRIPTOR); + Message inputMessage = messages.get("com." + rpcMethod.getInputType().getFullName()); + return RoutingRuleParser.parse(rpcMethod, inputMessage, messages); } } diff --git a/src/test/java/com/google/api/generator/gapic/testdata/explicit_dynamic_routing_header_testing.proto b/src/test/java/com/google/api/generator/gapic/testdata/explicit_dynamic_routing_header_testing.proto index f2add97a27..cbe9526a7b 100644 --- a/src/test/java/com/google/api/generator/gapic/testdata/explicit_dynamic_routing_header_testing.proto +++ b/src/test/java/com/google/api/generator/gapic/testdata/explicit_dynamic_routing_header_testing.proto @@ -256,6 +256,15 @@ service ExplicitDynamicRoutingHeaderTesting { get: "/v1beta1/{table_name=tests/*}" }; } + + rpc NestedFieldTest(RequestWithNestedField) returns (google.protobuf.Empty) { + option (google.api.routing) = { + routing_parameters { + field: "nested_field.another_nested_field.name" + path_template: "{routing_id=projects/*}/**" + } + }; + } } @@ -281,3 +290,15 @@ message Request { string app_profile_id = 2; } +message RequestWithNestedField { + NestedField nested_field = 1; +} + +message NestedField { + AnotherNestedField another_nested_field = 1; +} + +message AnotherNestedField { + string name = 1; +} +