fix: Fix PathTemplate custom verb logic matching and instantiating (#244

) The custom verb (":literal" at the end of a url path) was not handled properly. On instantiation `:` was incorrectly replaced with `/`. On matching `:literal` was not properly recognized as a valid input.
googleapis · Jul 20, 2021 · d4913d3 · d4913d3
1 parent 2ca4359
commit d4913d3
Show file tree

Hide file tree

Showing 3 changed files with 78 additions and 2 deletions.
diff --git a/build.gradle b/build.gradle
@@ -32,6 +32,10 @@ project.version = "1.10.5-SNAPSHOT" // {x-version-update:api-common:current}
 sourceCompatibility = 1.7
 targetCompatibility = 1.7
 
+jacoco {
+    toolVersion = "0.8.7"
+}
+
 // Dependencies
 // ------------
 

diff --git a/src/main/java/com/google/api/pathtemplate/PathTemplate.java b/src/main/java/com/google/api/pathtemplate/PathTemplate.java
@@ -664,7 +664,8 @@ private boolean match(
             switch (segments.get(i).kind()) {
               case BINDING:
               case END_BINDING:
-                // skip
+              case CUSTOM_VERB:
+                // These segments do not actually consume any input.
                 continue;
               default:
                 segsToMatch++;
@@ -746,7 +747,8 @@ private String instantiate(Map<String, String> values, boolean allowPartial) {
     while (iterator.hasNext()) {
       Segment seg = iterator.next();
       if (!skip && !continueLast) {
-        String separator = prevSeparator.isEmpty() ? seg.separator() : prevSeparator;
+        String separator =
+            prevSeparator.isEmpty() || !iterator.hasNext() ? seg.separator() : prevSeparator;
         result.append(separator);
         prevSeparator = seg.complexSeparator().isEmpty() ? seg.separator() : seg.complexSeparator();
       }

diff --git a/src/test/java/com/google/api/pathtemplate/PathTemplateTest.java b/src/test/java/com/google/api/pathtemplate/PathTemplateTest.java
@@ -114,6 +114,7 @@ public void pathWildcards_matchZeroOrMoreSegments() {
     PathTemplate start = PathTemplate.create("{glob=**}/b");
     PathTemplate middle = PathTemplate.create("a/{glob=**}/b");
     PathTemplate end = PathTemplate.create("a/{glob=**}");
+    PathTemplate endWithCustomVerb = PathTemplate.create("a/{glob=**}:foo");
 
     Truth.assertThat(start.match("b").get("glob")).isEmpty();
     Truth.assertThat(start.match("/b").get("glob")).isEmpty();
@@ -129,6 +130,10 @@ public void pathWildcards_matchZeroOrMoreSegments() {
     Truth.assertThat(end.match("a/").get("glob")).isEmpty();
     Truth.assertThat(end.match("a/b").get("glob")).isEqualTo("b");
     Truth.assertThat(end.match("a/b/b/b").get("glob")).isEqualTo("b/b/b");
+
+    Truth.assertThat(endWithCustomVerb.match("a/:foo").get("glob")).isEmpty();
+    Truth.assertThat(endWithCustomVerb.match("a/b:foo").get("glob")).isEqualTo("b");
+    Truth.assertThat(endWithCustomVerb.match("a/b/b:foo").get("glob")).isEqualTo("b/b");
   }
 
   @Test
@@ -173,6 +178,12 @@ public void matchWithUnboundInMiddle() {
     assertPositionalMatch(template.match("bar/foo/foo/foo/bar"), "foo/foo", "bar");
   }
 
+  @Test
+  public void matchWithCustomVerbs() {
+    PathTemplate template = PathTemplate.create("**:foo");
+    assertPositionalMatch(template.match("a/b/c:foo"), "a/b/c");
+  }
+
   // Complex Resource ID Segments.
   // ========
 
@@ -215,6 +226,45 @@ public void complexResourceIdBasicCases() {
     Truth.assertThat(match.get("zone_b")).isEqualTo("us-east3-a");
   }
 
+  @Test
+  public void complexResourceIdCustomVerb() {
+    // Separate by "~".
+    PathTemplate template = PathTemplate.create("projects/{project}/zones/{zone_a}~{zone_b}:hello");
+    Map<String, String> match =
+        template.match(
+            "https://www.googleapis.com/compute/v1/projects/project-123/zones/europe-west3-c~us-east3-a:hello");
+    Truth.assertThat(match).isNotNull();
+    Truth.assertThat(match.get(PathTemplate.HOSTNAME_VAR)).isEqualTo("https://www.googleapis.com");
+    Truth.assertThat(match.get("project")).isEqualTo("project-123");
+    Truth.assertThat(match.get("zone_a}~{zone_b")).isNull();
+    Truth.assertThat(match.get("zone_a")).isEqualTo("europe-west3-c");
+    Truth.assertThat(match.get("zone_b")).isEqualTo("us-east3-a");
+
+    // Separate by "-".
+    template = PathTemplate.create("projects/{project}/zones/{zone_a}-{zone_b}:hello");
+    match = template.match("projects/project-123/zones/europe-west3-c~us-east3-a:hello");
+    Truth.assertThat(match).isNotNull();
+    Truth.assertThat(match.get("project")).isEqualTo("project-123");
+    Truth.assertThat(match.get("zone_a")).isEqualTo("europe");
+    Truth.assertThat(match.get("zone_b")).isEqualTo("west3-c~us-east3-a");
+
+    // Separate by ".".
+    template = PathTemplate.create("projects/{project}/zones/{zone_a}.{zone_b}:hello");
+    match = template.match("projects/project-123/zones/europe-west3-c.us-east3-a:hello");
+    Truth.assertThat(match).isNotNull();
+    Truth.assertThat(match.get("project")).isEqualTo("project-123");
+    Truth.assertThat(match.get("zone_a")).isEqualTo("europe-west3-c");
+    Truth.assertThat(match.get("zone_b")).isEqualTo("us-east3-a");
+
+    // Separate by "_".
+    template = PathTemplate.create("projects/{project}/zones/{zone_a}_{zone_b}:hello");
+    match = template.match("projects/project-123/zones/europe-west3-c_us-east3-a:hello");
+    Truth.assertThat(match).isNotNull();
+    Truth.assertThat(match.get("project")).isEqualTo("project-123");
+    Truth.assertThat(match.get("zone_a")).isEqualTo("europe-west3-c");
+    Truth.assertThat(match.get("zone_b")).isEqualTo("us-east3-a");
+  }
+
   @Test
   public void complexResourceIdEqualsWildcard() {
     PathTemplate template = PathTemplate.create("projects/{project=*}/zones/{zone_a=*}~{zone_b=*}");
@@ -604,6 +654,18 @@ public void instantiateWithComplexResourceId_basic() {
     Truth.assertThat(instance).isEqualTo("projects/a%2Fb%2Fc/zones/apple~baseball");
   }
 
+  @Test
+  public void instantiateWithComplexResourceId_customVerb() {
+    PathTemplate template = PathTemplate.create("projects/{project}/zones/{zone_a}~{zone_b}:hello");
+    String instance =
+        template.instantiate("project", "a/b/c", "zone_a", "apple", "zone_b", "baseball");
+    Truth.assertThat(instance).isEqualTo("projects/a%2Fb%2Fc/zones/apple~baseball:hello");
+
+    template = PathTemplate.create("projects/{project}/zones/{zone_a}~{zone_b}/stuff:hello");
+    instance = template.instantiate("project", "a/b/c", "zone_a", "apple", "zone_b", "baseball");
+    Truth.assertThat(instance).isEqualTo("projects/a%2Fb%2Fc/zones/apple~baseball/stuff:hello");
+  }
+
   @Test
   public void instantiateWithComplexResourceId_mixedSeparators() {
     PathTemplate template =
@@ -647,6 +709,14 @@ public void instantiateWithComplexResourceId_mixedSeparatorsInParent() {
     Truth.assertThat(instance).isEqualTo("projects/a%2Fb%2Fc~foo.bar/zones/apple~baseball");
   }
 
+  @Test
+  public void instantiateWithCustomVerbs() {
+    PathTemplate template = PathTemplate.create("/v1/{name=operations/**}:cancel");
+    String templateInstance = template.instantiate("name", "operations/3373707");
+    Truth.assertThat(templateInstance).isEqualTo("v1/operations/3373707:cancel");
+    Truth.assertThat(template.matches(templateInstance));
+  }
+
   // Other
   // =====