From 0bc0aad0a56a211f13563a9dfa3278acc83db7e4 Mon Sep 17 00:00:00 2001 From: Blake Li Date: Tue, 21 Dec 2021 10:44:03 -0500 Subject: [PATCH] Add support for parsing routing rules annotation. Test out params extractor in composer. #869 --- repositories.bzl | 4 +- .../grpc/GrpcServiceStubClassComposer.java | 52 +++++++ .../api/generator/gapic/model/Method.java | 5 + .../generator/gapic/model/RoutingHeaders.java | 43 ++++++ .../generator/gapic/protoparser/BUILD.bazel | 1 + .../generator/gapic/protoparser/Parser.java | 5 + .../gapic/protoparser/RoutingRuleParser.java | 130 ++++++++++++++++++ .../grpc/goldens/GrpcTestingStub.golden | 1 + .../generator/gapic/protoparser/BUILD.bazel | 2 + .../protoparser/RoutingRuleParserTest.java | 53 +++++++ .../api/generator/gapic/testdata/BUILD.bazel | 1 + .../generator/gapic/testdata/testing.proto | 7 + 12 files changed, 302 insertions(+), 2 deletions(-) create mode 100644 src/main/java/com/google/api/generator/gapic/model/RoutingHeaders.java create mode 100644 src/main/java/com/google/api/generator/gapic/protoparser/RoutingRuleParser.java create mode 100644 src/test/java/com/google/api/generator/gapic/protoparser/RoutingRuleParserTest.java diff --git a/repositories.bzl b/repositories.bzl index d067d4e689..7ad4029164 100644 --- a/repositories.bzl +++ b/repositories.bzl @@ -59,9 +59,9 @@ def gapic_generator_java_repositories(): _maybe( http_archive, name = "com_google_googleapis", - strip_prefix = "googleapis-ba30d8097582039ac4cc4e21b4e4baa426423075", + strip_prefix = "googleapis-987192dfddeb79d3262b9f9f7dbf092827f931ac", urls = [ - "https://github.com/googleapis/googleapis/archive/ba30d8097582039ac4cc4e21b4e4baa426423075.zip", + "https://github.com/googleapis/googleapis/archive/987192dfddeb79d3262b9f9f7dbf092827f931ac.zip", ], ) 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 c82de5552c..49c0aa0c87 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,6 +16,7 @@ import com.google.api.gax.grpc.GrpcCallSettings; import com.google.api.gax.grpc.GrpcStubCallableFactory; +import com.google.api.gax.rpc.RequestParamsExtractor; import com.google.api.generator.engine.ast.AssignmentExpr; import com.google.api.generator.engine.ast.ConcreteReference; import com.google.api.generator.engine.ast.EnumRefExpr; @@ -23,6 +24,7 @@ import com.google.api.generator.engine.ast.ExprStatement; import com.google.api.generator.engine.ast.LambdaExpr; import com.google.api.generator.engine.ast.MethodInvocationExpr; +import com.google.api.generator.engine.ast.NewObjectExpr; import com.google.api.generator.engine.ast.ScopeNode; import com.google.api.generator.engine.ast.Statement; import com.google.api.generator.engine.ast.StringObjectValue; @@ -35,6 +37,7 @@ 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.Service; import com.google.api.generator.gapic.utils.JavaStyle; import com.google.common.base.Preconditions; @@ -209,10 +212,19 @@ protected Expr createTransportSettingsInitExpr( .build(); if (method.hasHttpBindings()) { + NewObjectExpr newExtractorExpr = + NewObjectExpr.builder() + .setType( + TypeNode.withReference( + ConcreteReference.withClazz(ExplicitRoutingHeaderExtractor.class))) + .setArguments() + .build(); callSettingsBuilderExpr = MethodInvocationExpr.builder() .setExprReferenceExpr(callSettingsBuilderExpr) .setMethodName("setParamsExtractor") + // set custom extractor + // .setArguments(newExtractorExpr) .setArguments(createRequestParamsExtractorClassInstance(method)) .build(); } @@ -302,6 +314,8 @@ private LambdaExpr createRequestParamsExtractorClassInstance(Method method) { .setArguments(requestBuilderExpr) .build(); + // TODO: completely remove this part if routing headers is not null? + // Are these params used for anything else other than implicit dynamic routing? Expr paramsPutExpr = MethodInvocationExpr.builder() .setExprReferenceExpr(paramsVarExpr) @@ -313,6 +327,35 @@ private LambdaExpr createRequestParamsExtractorClassInstance(Method method) { bodyExprs.add(paramsPutExpr); } + for (RoutingHeader routingHeader : method.routingHeaders().routingHeadersSet()) { + MethodInvocationExpr.Builder requestFieldGetterExprBuilder = + MethodInvocationExpr.builder().setExprReferenceExpr(requestVarExpr); + // TODO: support nested field + String currFieldName = routingHeader.field(); + String bindingFieldMethodName = + String.format("get%s", JavaStyle.toUpperCamelCase(currFieldName)); + requestFieldGetterExprBuilder = + requestFieldGetterExprBuilder.setMethodName(bindingFieldMethodName); + + MethodInvocationExpr requestBuilderExpr = requestFieldGetterExprBuilder.build(); + 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(routingHeader.name())), + valueOfExpr) + .build(); + bodyExprs.add(paramsPutExpr); + } + TypeNode returnType = TypeNode.withReference( ConcreteReference.builder() @@ -335,4 +378,13 @@ private LambdaExpr createRequestParamsExtractorClassInstance(Method method) { .setReturnExpr(returnExpr) .build(); } + + class ExplicitRoutingHeaderExtractor implements RequestParamsExtractor { + + @Override + public Map extract(Object request) { + // no way to extract the field value since request type is dynamic + return null; + } + } } 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 286bd8c84e..c2961994b5 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 @@ -62,6 +62,9 @@ public boolean isPaged() { @Nullable public abstract HttpBindings httpBindings(); + @Nullable + public abstract RoutingHeaders routingHeaders(); + // Example from Expand in echo.proto: Thet TypeNodes that map to // [["content", "error"], ["content", "error", "info"]]. public abstract ImmutableList> methodSignatures(); @@ -140,6 +143,8 @@ public abstract static class Builder { public abstract Builder setOperationPollingMethod(boolean operationPollingMethod); + public abstract Builder setRoutingHeaders(RoutingHeaders routingHeaders); + 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/RoutingHeaders.java new file mode 100644 index 0000000000..ee84b56cd6 --- /dev/null +++ b/src/main/java/com/google/api/generator/gapic/model/RoutingHeaders.java @@ -0,0 +1,43 @@ +// Copyright 2020 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.model; + +import com.google.auto.value.AutoValue; +import java.util.List; + +@AutoValue +public abstract class RoutingHeaders { + + public abstract List routingHeadersSet(); + + // TODO: change to a builder and expose only add method + public static RoutingHeaders create(List routingHeaderList) { + return new AutoValue_RoutingHeaders(routingHeaderList); + } + + @AutoValue + public abstract static class RoutingHeader { + + public abstract String field(); + + public abstract String name(); + + public abstract String pattern(); + + public static RoutingHeaders.RoutingHeader create(String field, String name, String pattern) { + return new AutoValue_RoutingHeaders_RoutingHeader(field, name, pattern); + } + } +} diff --git a/src/main/java/com/google/api/generator/gapic/protoparser/BUILD.bazel b/src/main/java/com/google/api/generator/gapic/protoparser/BUILD.bazel index 904b7fbf2b..aad6c7f80c 100644 --- a/src/main/java/com/google/api/generator/gapic/protoparser/BUILD.bazel +++ b/src/main/java/com/google/api/generator/gapic/protoparser/BUILD.bazel @@ -27,6 +27,7 @@ java_library( "//src/main/java/com/google/api/generator/gapic/model", "//src/main/java/com/google/api/generator/gapic/utils", "@com_google_api_api_common//jar", + "@com_google_api_grpc_proto_google_common_protos", "@com_google_code_findbugs_jsr305//jar", "@com_google_code_gson//jar", "@com_google_googleapis//google/api:api_java_proto", 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 bbfcbbce05..a4aa2be50b 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,6 +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.Service; import com.google.api.generator.gapic.model.SourceCodeInfoLocation; import com.google.api.generator.gapic.model.Transport; @@ -709,6 +710,9 @@ static List parseMethods( .getOptions() .getExtension(ExtendedOperationsProto.operationPollingMethod) : false; + // may not need to pass in messageTypes? + RoutingHeaders routingHeaders = + RoutingRuleParser.parse(protoMethod, inputMessage, messageTypes); methods.add( methodBuilder .setName(protoMethod.getName()) @@ -726,6 +730,7 @@ static List parseMethods( resourceNames, outputArgResourceNames)) .setHttpBindings(httpBindings) + .setRoutingHeaders(routingHeaders) .setIsBatching(isBatching) .setPageSizeFieldName(parsePageSizeFieldName(protoMethod, messageTypes, transport)) .setIsDeprecated(isDeprecated) 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 new file mode 100644 index 0000000000..8386b8dce2 --- /dev/null +++ b/src/main/java/com/google/api/generator/gapic/protoparser/RoutingRuleParser.java @@ -0,0 +1,130 @@ +// Copyright 2020 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.RoutingParameter; +import com.google.api.RoutingProto; +import com.google.api.RoutingRule; +import com.google.api.generator.gapic.model.Field; +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.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.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.Set; +import java.util.stream.Collectors; + +public class RoutingRuleParser { + + private static final String ASTERISK = "**"; + + public static RoutingHeaders parse( + MethodDescriptor protoMethod, Message inputMessage, Map messageTypes) { + MethodOptions methodOptions = protoMethod.getOptions(); + + if (!methodOptions.hasExtension(RoutingProto.routing)) { + return RoutingHeaders.create(Collections.emptyList()); + } + + RoutingRule routingRule = methodOptions.getExtension(RoutingProto.routing); + + return parseHttpRuleHelper(routingRule, Optional.of(inputMessage), messageTypes); + } + + private static RoutingHeaders parseHttpRuleHelper( + RoutingRule routingRule, + Optional inputMessageOpt, + Map messageTypes) { + + // one field may map to multiple headers, Example 6 + // multiple fields may map to one header as well, Example 8, last wins + // one combination of field/name may have multiple patterns, the one matches win, Example 3c. + List routingHeaderSet = new ArrayList<>(); + for (RoutingParameter routingParameter : routingRule.getRoutingParametersList()) { + String pathTemplate = routingParameter.getPathTemplate(); + String field = routingParameter.getField(); + // TODO: Validate if field exist in Message or nested Messages + // If failed, stop or ignore? stop? + checkHttpFieldIsValid(field, inputMessageOpt.get(), false); + + // TODO: Validate the pattern, if specified and not **, the pattern must contain one and + // only one named segment + // If failed, stop or ignore? stop? + Set params = getPatternBindings(pathTemplate).build(); + String name = field; + // set name to field if empty, Example 1 + if (!params.isEmpty()) { + name = params.iterator().next(); + } + + // set path to ** if empty, Example 1 + if (pathTemplate.isEmpty()) { + pathTemplate = ASTERISK; + } + + RoutingHeader routingHeader = RoutingHeader.create(field, name, pathTemplate); + routingHeaderSet.add(routingHeader); + } + + return RoutingHeaders.create(routingHeaderSet); + } + + 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; + } + + // TODO:Move to Message.java, also need to handle nested fields. The field has to be of type + // String + private static void checkHttpFieldIsValid(String binding, Message inputMessage, boolean isBody) { + Preconditions.checkState( + !Strings.isNullOrEmpty(binding), + String.format("Null or empty binding for " + inputMessage.name())); + Preconditions.checkState( + inputMessage.fieldMap().containsKey(binding), + String.format( + "Expected message %s to contain field %s but none found", + inputMessage.name(), binding)); + Field field = inputMessage.fieldMap().get(binding); + boolean fieldCondition = !field.isRepeated(); + if (!isBody) { + fieldCondition &= field.type().isProtoPrimitiveType() || field.isEnum(); + } + String messageFormat = + "Expected a non-repeated " + + (isBody ? "" : "primitive ") + + "type for field %s in message %s but got type %s"; + Preconditions.checkState( + fieldCondition, + String.format(messageFormat, field.name(), inputMessage.name(), field.type())); + } +} 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 7312b79d37..b2eee7cc0b 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 @@ -222,6 +222,7 @@ public class GrpcTestingStub extends TestingStub { request -> { ImmutableMap.Builder params = ImmutableMap.builder(); params.put("name", String.valueOf(request.getName())); + params.put("rename", String.valueOf(request.getName())); return params.build(); }) .build(); 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 f15a3d1a6e..24eaaf09ad 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,6 +16,7 @@ TESTS = [ "ServiceYamlParserTest", "SourceCodeInfoParserTest", "TypeParserTest", + "RoutingRuleParserTest" ] filegroup( @@ -45,6 +46,7 @@ filegroup( "//src/test/java/com/google/api/generator/gapic/testdata:showcase_java_proto", "//src/test/java/com/google/api/generator/gapic/testdata:testgapic_java_proto", "@com_google_api_api_common//jar", + "@com_google_api_grpc_proto_google_common_protos", "@com_google_googleapis//google/api:api_java_proto", "@com_google_googleapis//google/rpc:rpc_java_proto", "@com_google_protobuf//:protobuf_java", 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 new file mode 100644 index 0000000000..e3039f5e33 --- /dev/null +++ b/src/test/java/com/google/api/generator/gapic/protoparser/RoutingRuleParserTest.java @@ -0,0 +1,53 @@ +// Copyright 2020 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 static org.junit.Assert.assertEquals; + +import com.google.api.generator.gapic.model.Message; +import com.google.api.generator.gapic.model.RoutingHeaders; +import com.google.protobuf.Descriptors.FileDescriptor; +import com.google.protobuf.Descriptors.MethodDescriptor; +import com.google.protobuf.Descriptors.ServiceDescriptor; +import com.google.showcase.v1beta1.TestingOuterClass; +import java.util.Map; +import java.util.stream.Collectors; +import org.junit.Test; + +public class RoutingRuleParserTest { + + // Do we have to test against a real proto file + // Maybe manually create a MethodDescriptor and test against it? more flexibility and precise. + // testing.proto is being used by other tests as well + @Test + public void parseRoutingRuleAnnotation() { + FileDescriptor testingFileDescriptor = TestingOuterClass.getDescriptor(); + ServiceDescriptor testingService = testingFileDescriptor.getServices().get(0); + assertEquals("Testing", testingService.getName()); + + Map messages = Parser.parseMessages(testingFileDescriptor); + + // GetTest method. + MethodDescriptor rpcMethod = testingService.getMethods().get(5); + Message inputMessage = messages.get("com.google.showcase.v1beta1.GetTestRequest"); + RoutingHeaders routingHeaders = RoutingRuleParser.parse(rpcMethod, inputMessage, messages); + assertThat( + routingHeaders.routingHeadersSet().stream() + .map(routingHeader -> routingHeader.field() + " -> " + routingHeader.name()) + .collect(Collectors.toList())) + .containsExactly("name -> rename"); + } +} diff --git a/src/test/java/com/google/api/generator/gapic/testdata/BUILD.bazel b/src/test/java/com/google/api/generator/gapic/testdata/BUILD.bazel index 3991672985..9bc783cdaa 100644 --- a/src/test/java/com/google/api/generator/gapic/testdata/BUILD.bazel +++ b/src/test/java/com/google/api/generator/gapic/testdata/BUILD.bazel @@ -54,6 +54,7 @@ proto_library( ], deps = [ "@com_google_googleapis//google/api:annotations_proto", + "@com_google_googleapis//google/api:routing_proto", "@com_google_googleapis//google/api:client_proto", "@com_google_googleapis//google/api:field_behavior_proto", "@com_google_googleapis//google/api:resource_proto", diff --git a/src/test/java/com/google/api/generator/gapic/testdata/testing.proto b/src/test/java/com/google/api/generator/gapic/testdata/testing.proto index 490e1d17cc..ef4cc2cc1c 100644 --- a/src/test/java/com/google/api/generator/gapic/testdata/testing.proto +++ b/src/test/java/com/google/api/generator/gapic/testdata/testing.proto @@ -15,6 +15,7 @@ syntax = "proto3"; import "google/api/annotations.proto"; +import "google/api/routing.proto"; import "google/api/client.proto"; import "google/api/resource.proto"; import "google/protobuf/empty.proto"; @@ -78,6 +79,12 @@ service Testing { option (google.api.http) = { get: "/v1beta1/{name=tests/*}" }; + option (google.api.routing) = { + routing_parameters { + field: "name" + path_template: "/v1beta1/{rename=tests/*}" + } + }; option (google.api.method_signature) = "name"; }