Skip to content
This repository was archived by the owner on Sep 27, 2023. It is now read-only.

Commit d4913d3

Browse files
vam-googlevam-google
authored
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.
1 parent 2ca4359 commit d4913d3

File tree

3 files changed

+78
-2
lines changed

3 files changed

+78
-2
lines changed

‎build.gradle‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,10 @@ project.version = "1.10.5-SNAPSHOT" // {x-version-update:api-common:current}
3232
sourceCompatibility = 1.7
3333
targetCompatibility = 1.7
3434

35+
jacoco {
36+
toolVersion = "0.8.7"
37+
}
38+
3539
// Dependencies
3640
// ------------
3741

‎src/main/java/com/google/api/pathtemplate/PathTemplate.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -664,7 +664,8 @@ private boolean match(
664664
switch (segments.get(i).kind()) {
665665
case BINDING:
666666
case END_BINDING:
667-
// skip
667+
case CUSTOM_VERB:
668+
// These segments do not actually consume any input.
668669
continue;
669670
default:
670671
segsToMatch++;
@@ -746,7 +747,8 @@ private String instantiate(Map<String, String> values, boolean allowPartial) {
746747
while (iterator.hasNext()) {
747748
Segment seg = iterator.next();
748749
if (!skip && !continueLast) {
749-
String separator = prevSeparator.isEmpty() ? seg.separator() : prevSeparator;
750+
String separator =
751+
prevSeparator.isEmpty() || !iterator.hasNext() ? seg.separator() : prevSeparator;
750752
result.append(separator);
751753
prevSeparator = seg.complexSeparator().isEmpty() ? seg.separator() : seg.complexSeparator();
752754
}

‎src/test/java/com/google/api/pathtemplate/PathTemplateTest.java‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,7 @@ public void pathWildcards_matchZeroOrMoreSegments() {
114114
PathTemplate start = PathTemplate.create("{glob=**}/b");
115115
PathTemplate middle = PathTemplate.create("a/{glob=**}/b");
116116
PathTemplate end = PathTemplate.create("a/{glob=**}");
117+
PathTemplate endWithCustomVerb = PathTemplate.create("a/{glob=**}:foo");
117118

118119
Truth.assertThat(start.match("b").get("glob")).isEmpty();
119120
Truth.assertThat(start.match("/b").get("glob")).isEmpty();
@@ -129,6 +130,10 @@ public void pathWildcards_matchZeroOrMoreSegments() {
129130
Truth.assertThat(end.match("a/").get("glob")).isEmpty();
130131
Truth.assertThat(end.match("a/b").get("glob")).isEqualTo("b");
131132
Truth.assertThat(end.match("a/b/b/b").get("glob")).isEqualTo("b/b/b");
133+
134+
Truth.assertThat(endWithCustomVerb.match("a/:foo").get("glob")).isEmpty();
135+
Truth.assertThat(endWithCustomVerb.match("a/b:foo").get("glob")).isEqualTo("b");
136+
Truth.assertThat(endWithCustomVerb.match("a/b/b:foo").get("glob")).isEqualTo("b/b");
132137
}
133138

134139
@Test
@@ -173,6 +178,12 @@ public void matchWithUnboundInMiddle() {
173178
assertPositionalMatch(template.match("bar/foo/foo/foo/bar"), "foo/foo", "bar");
174179
}
175180

181+
@Test
182+
public void matchWithCustomVerbs() {
183+
PathTemplate template = PathTemplate.create("**:foo");
184+
assertPositionalMatch(template.match("a/b/c:foo"), "a/b/c");
185+
}
186+
176187
// Complex Resource ID Segments.
177188
// ========
178189

@@ -215,6 +226,45 @@ public void complexResourceIdBasicCases() {
215226
Truth.assertThat(match.get("zone_b")).isEqualTo("us-east3-a");
216227
}
217228

229+
@Test
230+
public void complexResourceIdCustomVerb() {
231+
// Separate by "~".
232+
PathTemplate template = PathTemplate.create("projects/{project}/zones/{zone_a}~{zone_b}:hello");
233+
Map<String, String> match =
234+
template.match(
235+
"https://www.googleapis.com/compute/v1/projects/project-123/zones/europe-west3-c~us-east3-a:hello");
236+
Truth.assertThat(match).isNotNull();
237+
Truth.assertThat(match.get(PathTemplate.HOSTNAME_VAR)).isEqualTo("https://www.googleapis.com");
238+
Truth.assertThat(match.get("project")).isEqualTo("project-123");
239+
Truth.assertThat(match.get("zone_a}~{zone_b")).isNull();
240+
Truth.assertThat(match.get("zone_a")).isEqualTo("europe-west3-c");
241+
Truth.assertThat(match.get("zone_b")).isEqualTo("us-east3-a");
242+
243+
// Separate by "-".
244+
template = PathTemplate.create("projects/{project}/zones/{zone_a}-{zone_b}:hello");
245+
match = template.match("projects/project-123/zones/europe-west3-c~us-east3-a:hello");
246+
Truth.assertThat(match).isNotNull();
247+
Truth.assertThat(match.get("project")).isEqualTo("project-123");
248+
Truth.assertThat(match.get("zone_a")).isEqualTo("europe");
249+
Truth.assertThat(match.get("zone_b")).isEqualTo("west3-c~us-east3-a");
250+
251+
// Separate by ".".
252+
template = PathTemplate.create("projects/{project}/zones/{zone_a}.{zone_b}:hello");
253+
match = template.match("projects/project-123/zones/europe-west3-c.us-east3-a:hello");
254+
Truth.assertThat(match).isNotNull();
255+
Truth.assertThat(match.get("project")).isEqualTo("project-123");
256+
Truth.assertThat(match.get("zone_a")).isEqualTo("europe-west3-c");
257+
Truth.assertThat(match.get("zone_b")).isEqualTo("us-east3-a");
258+
259+
// Separate by "_".
260+
template = PathTemplate.create("projects/{project}/zones/{zone_a}_{zone_b}:hello");
261+
match = template.match("projects/project-123/zones/europe-west3-c_us-east3-a:hello");
262+
Truth.assertThat(match).isNotNull();
263+
Truth.assertThat(match.get("project")).isEqualTo("project-123");
264+
Truth.assertThat(match.get("zone_a")).isEqualTo("europe-west3-c");
265+
Truth.assertThat(match.get("zone_b")).isEqualTo("us-east3-a");
266+
}
267+
218268
@Test
219269
public void complexResourceIdEqualsWildcard() {
220270
PathTemplate template = PathTemplate.create("projects/{project=*}/zones/{zone_a=*}~{zone_b=*}");
@@ -604,6 +654,18 @@ public void instantiateWithComplexResourceId_basic() {
604654
Truth.assertThat(instance).isEqualTo("projects/a%2Fb%2Fc/zones/apple~baseball");
605655
}
606656

657+
@Test
658+
public void instantiateWithComplexResourceId_customVerb() {
659+
PathTemplate template = PathTemplate.create("projects/{project}/zones/{zone_a}~{zone_b}:hello");
660+
String instance =
661+
template.instantiate("project", "a/b/c", "zone_a", "apple", "zone_b", "baseball");
662+
Truth.assertThat(instance).isEqualTo("projects/a%2Fb%2Fc/zones/apple~baseball:hello");
663+
664+
template = PathTemplate.create("projects/{project}/zones/{zone_a}~{zone_b}/stuff:hello");
665+
instance = template.instantiate("project", "a/b/c", "zone_a", "apple", "zone_b", "baseball");
666+
Truth.assertThat(instance).isEqualTo("projects/a%2Fb%2Fc/zones/apple~baseball/stuff:hello");
667+
}
668+
607669
@Test
608670
public void instantiateWithComplexResourceId_mixedSeparators() {
609671
PathTemplate template =
@@ -647,6 +709,14 @@ public void instantiateWithComplexResourceId_mixedSeparatorsInParent() {
647709
Truth.assertThat(instance).isEqualTo("projects/a%2Fb%2Fc~foo.bar/zones/apple~baseball");
648710
}
649711

712+
@Test
713+
public void instantiateWithCustomVerbs() {
714+
PathTemplate template = PathTemplate.create("/v1/{name=operations/**}:cancel");
715+
String templateInstance = template.instantiate("name", "operations/3373707");
716+
Truth.assertThat(templateInstance).isEqualTo("v1/operations/3373707:cancel");
717+
Truth.assertThat(template.matches(templateInstance));
718+
}
719+
650720
// Other
651721
// =====
652722

0 commit comments

Comments
 (0)