[ggj][codegen][resnames] fix: prevent resname tokens from matching su…

…bcomponents (#422) * fix: support non-name fields with res-refs in resname def parsing * fix: add workaround for missing default_host and oauth_scopes annotation * [ggj][infra][3/5]feat: add goldens update bazel rules for Redis API (#396) * goldens update bazel rules * clean up * [ggj][infra][4/5]feat: add goldens update bazel rules for Asset API (#397) * goldens update bazel rules * clean up * goldens update rule for asset API * update goldens * [ggj][infra][3/5]feat: add goldens update bazel rules for Redis API (#396) * goldens update bazel rules * clean up * [ggj][infra][4/5]feat: add goldens update bazel rules for Asset API (#397) * goldens update bazel rules * clean up * goldens update rule for asset API * update goldens * [ggj][infra][3/5]feat: add goldens update bazel rules for Redis API (#396) * goldens update bazel rules * clean up * [ggj][infra][4/5]feat: add goldens update bazel rules for Asset API (#397) * goldens update bazel rules * clean up * goldens update rule for asset API * update goldens * fix: clarify LRO parsing error messages * feat: support deeply-nested types in AST and proto message parsing * fix: prevent resname tokens from matching subcomponents Co-authored-by: Xiaozhen Liu <[email protected]>
googleapis · Oct 30, 2020 · 2812d54 · 2812d54
1 parent 0807672
commit 2812d54
Show file tree

Hide file tree

Showing 2 changed files with 38 additions and 7 deletions.
diff --git a/src/main/java/com/google/api/generator/gapic/composer/ResourceNameTokenizer.java b/src/main/java/com/google/api/generator/gapic/composer/ResourceNameTokenizer.java
@@ -15,16 +15,21 @@
 package com.google.api.generator.gapic.composer;
 
 import com.google.api.pathtemplate.PathTemplate;
+import com.google.common.base.Preconditions;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
 import java.util.Set;
 import java.util.stream.Collectors;
 
 public class ResourceNameTokenizer {
-  private static final String SLASH = "/";
   private static final String LEFT_BRACE = "{";
   private static final String RIGHT_BRACE = "}";
+  private static final String SLASH = "/";
+  private static final String EMPTY = "";
+
+  private static final String EQUALS_WILDCARD = "=*";
+  private static final String EQUALS_PATH_WILDCARD = "=**";
 
   static List<List<String>> parseTokenHierarchy(List<String> patterns) {
     List<String> nonSlashSepStrings = Arrays.asList("}_{", "}-{", "}.{", "}~{");
@@ -36,22 +41,38 @@ static List<List<String>> parseTokenHierarchy(List<String> patterns) {
       String[] patternTokens = pattern.split(SLASH);
       for (String patternToken : patternTokens) {
         if (patternToken.startsWith(LEFT_BRACE) && patternToken.endsWith(RIGHT_BRACE)) {
-          String processedPatternToken = patternToken;
+          String processedPatternToken =
+              // Replacement order matters - ensure the first is not a subcomponent of the second.
+              patternToken.replace(EQUALS_PATH_WILDCARD, EMPTY).replace(EQUALS_WILDCARD, EMPTY);
 
           // Handle non-slash separators.
           if (nonSlashSepStrings.stream().anyMatch(s -> patternToken.contains(s))) {
             for (String str : nonSlashSepStrings) {
               processedPatternToken = processedPatternToken.replace(str, "_");
             }
           } else {
+            final int processedPatternTokenLength = processedPatternToken.length();
             // Handles wildcards.
-            processedPatternToken =
+            List<String> candidateVars =
                 vars.stream()
-                    .filter(v -> patternToken.contains(v))
-                    .collect(Collectors.toList())
-                    .get(0);
+                    // Check that the token size is within ~3 of the var, to avoid mismatching on
+                    // variables with same-named subcomponents.
+                    // Otherwise, "customer_client_link" will match with "customer".
+                    .filter(
+                        v ->
+                            patternToken.contains(v)
+                                // Accounting for braces.
+                                && processedPatternTokenLength - v.length() < 3)
+                    .collect(Collectors.toList());
+            Preconditions.checkState(
+                !candidateVars.isEmpty(),
+                String.format(
+                    "No variable candidates found for token %s in pattern %s",
+                    processedPatternToken, pattern));
+            processedPatternToken = candidateVars.get(0);
           }
-          hierarchy.add(processedPatternToken.replace("{", "").replace("}", ""));
+          hierarchy.add(
+              processedPatternToken.replace(LEFT_BRACE, EMPTY).replace(RIGHT_BRACE, EMPTY));
         }
       }
       tokenHierachies.add(hierarchy);

diff --git a/src/test/java/com/google/api/generator/gapic/composer/ResourceNameTokenizerTest.java b/src/test/java/com/google/api/generator/gapic/composer/ResourceNameTokenizerTest.java
@@ -51,6 +51,16 @@ public void parseTokenHierarchy_basic() {
     assertThat(tokenHierarchies.get(2)).containsExactly("project", "autoscaling_policy");
   }
 
+  @Test
+  public void parseTokenHierarchy_substringsInPattern() {
+    List<String> patterns =
+        Arrays.asList(
+            "customers/{customer}/customerExtensionSettings/{customer_extension_setting}");
+    List<List<String>> tokenHierarchies = ResourceNameTokenizer.parseTokenHierarchy(patterns);
+    assertEquals(1, tokenHierarchies.size());
+    assertThat(tokenHierarchies.get(0)).containsExactly("customer", "customer_extension_setting");
+  }
+
   @Test
   public void parseTokenHierarchy_wildcards() {
     List<String> patterns =