#869. Add field type validation for String type. Add more unit tests …

…for edge cases. Resolve review comments and some small refactoring.

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<String, Message> 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<String, Message> messageTypes, TypeNode type) {
     List<String> 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<String, Message> 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()));
       }
     }
   }

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

src/main/java/com/google/api/generator/gapic/model/RoutingHeaders.java → 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<RoutingHeader> routingHeadersList();
+  public abstract ImmutableList<RoutingHeaderParam> 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<String> getDescendantFieldNames() {
@@ -48,15 +53,16 @@ public List<String> getDescendantFieldNames() {
 
   @AutoValue.Builder
   public abstract static class Builder {
-    abstract ImmutableList.Builder<RoutingHeader> routingHeadersListBuilder();
+    abstract ImmutableList.Builder<RoutingHeaderParam> 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<RoutingHeader> routingHeadersList);
+    public abstract Builder setRoutingHeaderParams(
+        ImmutableList<RoutingHeaderParam> routingHeaderParams);
 
-    public abstract RoutingHeaders build();
+    public abstract RoutingHeaderRule build();
   }
 }

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<Message> inputMessageOpt, Map<String, Message> messageTypes) {
     // Get pattern.
     String pattern = getHttpVerbPattern(httpRule);
-    ImmutableSet.Builder<String> bindingsBuilder = getPatternBindings(pattern);
+    ImmutableSet.Builder<String> 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<String> getPatternBindings(String pattern) {
-    ImmutableSortedSet.Builder<String> 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),

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<Method> 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<Method> parseMethods(
                       resourceNames,
                       outputArgResourceNames))
               .setHttpBindings(httpBindings)
-              .setRoutingHeaders(routingHeaders)
+              .setRoutingHeaderRule(routingHeaderRule)
               .setIsBatching(isBatching)
               .setPageSizeFieldName(parsePageSizeFieldName(protoMethod, messageTypes, transport))
               .setIsDeprecated(isDeprecated)

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<String> getPattenBindings(String pattern) {
+    ImmutableSortedSet.Builder<String> 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();
+  }
+}

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<String, Message> 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<String> params = getPatternBindings(pathTemplate).build();
+        Set<String> 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<String> getPatternBindings(String pattern) {
-    ImmutableSortedSet.Builder<String> 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();
   }
 }