From fcbabd068f53584e0070318bc2fbc0abec8962c1 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Wed, 6 May 2020 00:17:46 +0000 Subject: [PATCH 1/3] chore: support complex resource identifiers --- .../google/api/pathtemplate/PathTemplate.java | 331 ++++++++++++------ .../api/pathtemplate/PathTemplateTest.java | 251 ++++++++++++- 2 files changed, 466 insertions(+), 116 deletions(-) diff --git a/src/main/java/com/google/api/pathtemplate/PathTemplate.java b/src/main/java/com/google/api/pathtemplate/PathTemplate.java index e21becd23..6b249ef06 100644 --- a/src/main/java/com/google/api/pathtemplate/PathTemplate.java +++ b/src/main/java/com/google/api/pathtemplate/PathTemplate.java @@ -40,6 +40,7 @@ import java.io.UnsupportedEncodingException; import java.net.URLDecoder; import java.net.URLEncoder; +import java.util.ArrayList; import java.util.List; import java.util.ListIterator; import java.util.Map; @@ -139,12 +140,13 @@ public class PathTemplate { // A splitter on slash. private static final Splitter SLASH_SPLITTER = Splitter.on('/').trimResults(); + // A regex to match the valid complex resource ID delimiters. + private static final Pattern DELIMITER_PATTERN = Pattern.compile("(_|\\-|\\.|~)"); + // Helper Types // ============ - /** - * Specifies a path segment kind. - */ + /** Specifies a path segment kind. */ enum SegmentKind { /** A literal path segment. */ LITERAL, @@ -165,37 +167,34 @@ enum SegmentKind { END_BINDING, } - /** - * Specifies a path segment. - */ + /** Specifies a path segment. */ @AutoValue abstract static class Segment { - /** - * A constant for the WILDCARD segment. - */ + /** A constant for the WILDCARD segment. */ private static final Segment WILDCARD = create(SegmentKind.WILDCARD, "*"); - /** - * A constant for the PATH_WILDCARD segment. - */ + /** A constant for the PATH_WILDCARD segment. */ private static final Segment PATH_WILDCARD = create(SegmentKind.PATH_WILDCARD, "**"); - /** - * A constant for the END_BINDING segment. - */ + /** A constant for the END_BINDING segment. */ private static final Segment END_BINDING = create(SegmentKind.END_BINDING, ""); - /** - * Creates a segment of given kind and value. - */ + /** Creates a segment of given kind and value. */ private static Segment create(SegmentKind kind, String value) { - return new AutoValue_PathTemplate_Segment(kind, value); + return new AutoValue_PathTemplate_Segment(kind, value, ""); } - /** - * The path segment kind. - */ + private static Segment wildcardCreate(String complexSeparator) { + return new AutoValue_PathTemplate_Segment( + SegmentKind.WILDCARD, + "*", + !complexSeparator.isEmpty() && DELIMITER_PATTERN.matcher(complexSeparator).find() + ? complexSeparator + : ""); + } + + /** The path segment kind. */ abstract SegmentKind kind(); /** @@ -204,9 +203,9 @@ private static Segment create(SegmentKind kind, String value) { */ abstract String value(); - /** - * Returns true of this segment is one of the wildcards, - */ + abstract String complexSeparator(); + + /** Returns true of this segment is one of the wildcards, */ boolean isAnyWildcard() { return kind() == SegmentKind.WILDCARD || kind() == SegmentKind.PATH_WILDCARD; } @@ -277,9 +276,7 @@ private PathTemplate(Iterable segments, boolean urlEncoding) { this.urlEncoding = urlEncoding; } - /** - * Returns the set of variable names used in the template. - */ + /** Returns the set of variable names used in the template. */ public Set vars() { return bindings.keySet(); } @@ -363,16 +360,12 @@ public PathTemplate subTemplate(String varName) { String.format("Variable '%s' is undefined in template '%s'", varName, this.toRawString())); } - /** - * Returns true of this template ends with a literal. - */ + /** Returns true of this template ends with a literal. */ public boolean endsWithLiteral() { return segments.get(segments.size() - 1).kind() == SegmentKind.LITERAL; } - /** - * Returns true of this template ends with a custom verb. - */ + /** Returns true of this template ends with a custom verb. */ public boolean endsWithCustomVerb() { return segments.get(segments.size() - 1).kind() == SegmentKind.CUSTOM_VERB; } @@ -464,9 +457,7 @@ public Map validatedMatch(String path, String exceptionMessagePr return matchMap; } - /** - * Returns true if the template matches the path. - */ + /** Returns true if the template matches the path. */ public boolean matches(String path) { return match(path) != null; } @@ -565,7 +556,8 @@ private Map match(String path, boolean forceHostName) { return ImmutableMap.copyOf(values); } - // Aligns input to start of literal value of literal or binding segment if input contains hostname. + // Aligns input to start of literal value of literal or binding segment if input contains + // hostname. private int alignInputToAlignableSegment(List input, int inPos, Segment segment) { switch (segment.kind()) { case BINDING: @@ -597,6 +589,7 @@ private boolean match( int segPos, Map values) { String currentVar = null; + List modifableInput = new ArrayList<>(input); while (segPos < segments.size()) { Segment seg = segments.get(segPos++); switch (seg.kind()) { @@ -614,18 +607,30 @@ private boolean match( break; case LITERAL: case WILDCARD: - if (inPos >= input.size()) { + if (inPos >= modifableInput.size()) { // End of input return false; } // Check literal match. - String next = decodeUrl(input.get(inPos++)); + String next = decodeUrl(modifableInput.get(inPos++)); if (seg.kind() == SegmentKind.LITERAL) { if (!seg.value().equals(next)) { // Literal does not match. return false; } } + if (seg.kind() == SegmentKind.WILDCARD && !seg.complexSeparator().isEmpty()) { + // Parse the complex resource separators one by one. + int complexSeparatorIndex = next.indexOf(seg.complexSeparator()); + if (complexSeparatorIndex >= 0) { + modifableInput.add(inPos, next.substring(complexSeparatorIndex + 1)); + next = next.substring(0, complexSeparatorIndex); + modifableInput.set(inPos - 1, next); + } else { + // No coplex resource ID separator found in the literal when we expected one. + return false; + } + } if (currentVar != null) { // Create or extend current match values.put(currentVar, concatCaptures(values.get(currentVar), next)); @@ -645,18 +650,19 @@ private boolean match( segsToMatch++; } } - int available = (input.size() - inPos) - segsToMatch; + int available = (modifableInput.size() - inPos) - segsToMatch; // If this segment is empty, make sure it is still captured. if (available == 0 && !values.containsKey(currentVar)) { values.put(currentVar, ""); } while (available-- > 0) { values.put( - currentVar, concatCaptures(values.get(currentVar), decodeUrl(input.get(inPos++)))); + currentVar, + concatCaptures(values.get(currentVar), decodeUrl(modifableInput.get(inPos++)))); } } } - return inPos == input.size(); + return inPos == modifableInput.size(); } private static String concatCaptures(@Nullable String cur, String next) { @@ -681,9 +687,7 @@ public String instantiate(Map values) { return instantiate(values, false); } - /** - * Shortcut for {@link #instantiate(Map)} with a vararg parameter for keys and values. - */ + /** Shortcut for {@link #instantiate(Map)} with a vararg parameter for keys and values. */ public String instantiate(String... keysAndValues) { ImmutableMap.Builder builder = ImmutableMap.builder(); for (int i = 0; i < keysAndValues.length; i += 2) { @@ -831,6 +835,7 @@ public List decode(String path) { // ================ private static ImmutableList parseTemplate(String template) { + System.out.println("DEL: PARSING TEMPLATE"); // Skip useless leading slash. if (template.startsWith("/")) { template = template.substring(1); @@ -850,89 +855,120 @@ private static ImmutableList parseTemplate(String template) { int pathWildCardBound = 0; for (String seg : Splitter.on('/').trimResults().split(template)) { + if (DELIMITER_PATTERN.matcher(seg.substring(0, 1)).find() + || DELIMITER_PATTERN.matcher(seg.substring(seg.length() - 1)).find()) { + throw new ValidationException("parse error: invalid begin or end character in '%s'", seg); + } + // Disallow zero or multiple delimiters between variable names. + if (Pattern.compile("\\}(_|\\-|\\.|~){2,}\\{").matcher(seg).find()) { + throw new ValidationException( + "parse error: two consecutive delimiter characters in '%s'", seg); + } // If segment starts with '{', a binding group starts. boolean bindingStarts = seg.startsWith("{"); boolean implicitWildcard = false; + boolean complexDelimiterFound = false; if (bindingStarts) { if (varName != null) { throw new ValidationException("parse error: nested binding in '%s'", template); } seg = seg.substring(1); - int i = seg.indexOf('='); - if (i <= 0) { - // Possibly looking at something like "{name}" with implicit wildcard. - if (seg.endsWith("}")) { - // Remember to add an implicit wildcard later. - implicitWildcard = true; - varName = seg.substring(0, seg.length() - 1).trim(); - seg = seg.substring(seg.length() - 1).trim(); - } else { - throw new ValidationException("parse error: invalid binding syntax in '%s'", template); - } - } else { - // Looking at something like "{name=wildcard}". - varName = seg.substring(0, i).trim(); - seg = seg.substring(i + 1).trim(); + // Check for invalid complex resource ID delimiters. + if (Pattern.compile("\\}[^_\\-\\.~]\\{").matcher(seg).find() + || Pattern.compile("\\}\\{").matcher(seg).find()) { + throw new ValidationException( + "parse error: missing or invalid complex resource ID delimiter character in '%s'", + seg); } - builder.add(Segment.create(SegmentKind.BINDING, varName)); - } - // If segment ends with '}', a binding group ends. Remove the brace and remember. - boolean bindingEnds = seg.endsWith("}"); - if (bindingEnds) { - seg = seg.substring(0, seg.length() - 1).trim(); - } + Matcher complexPatternDelimiterMatcher = + Pattern.compile("\\}(_|\\-|\\.|~){1}").matcher(seg); + complexDelimiterFound = complexPatternDelimiterMatcher.find(); - // Process the segment, after stripping off "{name=.." and "..}". - switch (seg) { - case "**": - case "*": - if ("**".equals(seg)) { - pathWildCardBound++; - } - Segment wildcard = seg.length() == 2 ? Segment.PATH_WILDCARD : Segment.WILDCARD; - if (varName == null) { - // Not in a binding, turn wildcard into implicit binding. - // "*" => "{$n=*}" - builder.add(Segment.create(SegmentKind.BINDING, "$" + freeWildcardCounter)); - freeWildcardCounter++; - builder.add(wildcard); - builder.add(Segment.END_BINDING); + // Look for complex resource names. + // Need to handles something like "{user_a}~{user_b}". + if (complexDelimiterFound) { + builder.addAll(parseComplexResourceId(seg)); + } else { + int i = seg.indexOf('='); + if (i <= 0) { + // Possibly looking at something like "{name}" with implicit wildcard. + if (seg.endsWith("}")) { + // Remember to add an implicit wildcard later. + implicitWildcard = true; + varName = seg.substring(0, seg.length() - 1).trim(); + seg = seg.substring(seg.length() - 1).trim(); + } else { + throw new ValidationException( + "parse error: invalid binding syntax in '%s'", template); + } } else { - builder.add(wildcard); - } - break; - case "": - if (!bindingEnds) { - throw new ValidationException( - "parse error: empty segment not allowed in '%s'", template); + // Looking at something like "{name=wildcard}". + varName = seg.substring(0, i).trim(); + seg = seg.substring(i + 1).trim(); } - // If the wildcard is implicit, seg will be empty. Just continue. - break; - default: - builder.add(Segment.create(SegmentKind.LITERAL, seg)); + builder.add(Segment.create(SegmentKind.BINDING, varName)); + } } - // End a binding. - if (bindingEnds) { - // Reset varName to null for next binding. - varName = null; + if (!complexDelimiterFound) { + // If segment ends with '}', a binding group ends. Remove the brace and remember. + boolean bindingEnds = seg.endsWith("}"); + if (bindingEnds) { + seg = seg.substring(0, seg.length() - 1).trim(); + } - if (implicitWildcard) { - // Looking at something like "{var}". Insert an implicit wildcard, as it is the same - // as "{var=*}". - builder.add(Segment.WILDCARD); + // Process the segment, after stripping off "{name=.." and "..}". + switch (seg) { + case "**": + case "*": + if ("**".equals(seg)) { + pathWildCardBound++; + } + Segment wildcard = seg.length() == 2 ? Segment.PATH_WILDCARD : Segment.WILDCARD; + if (varName == null) { + // Not in a binding, turn wildcard into implicit binding. + // "*" => "{$n=*}" + builder.add(Segment.create(SegmentKind.BINDING, "$" + freeWildcardCounter)); + freeWildcardCounter++; + builder.add(wildcard); + builder.add(Segment.END_BINDING); + } else { + builder.add(wildcard); + } + break; + case "": + if (!bindingEnds) { + throw new ValidationException( + "parse error: empty segment not allowed in '%s'", template); + } + // If the wildcard is implicit, seg will be empty. Just continue. + break; + default: + builder.add(Segment.create(SegmentKind.LITERAL, seg)); } - builder.add(Segment.END_BINDING); - } - if (pathWildCardBound > 1) { - // Report restriction on number of '**' in the pattern. There can be only one, which - // enables non-backtracking based matching. - throw new ValidationException( - "parse error: pattern must not contain more than one path wildcard ('**') in '%s'", - template); + // End a binding. + if (bindingEnds && !complexDelimiterFound) { + // Reset varName to null for next binding. + varName = null; + + if (implicitWildcard) { + // Looking at something like "{var}". Insert an implicit wildcard, as it is the same + // as "{var=*}". + builder.add(Segment.WILDCARD); + } + builder.add(Segment.END_BINDING); + } + + if (pathWildCardBound > 1) { + // Report restriction on number of '**' in the pattern. There can be only one, which + // enables non-backtracking based matching. + throw new ValidationException( + "parse error: pattern must not contain more than one path wildcard ('**') in '%s'", + template); + } } } @@ -942,6 +978,77 @@ private static ImmutableList parseTemplate(String template) { return builder.build(); } + private static List parseComplexResourceId(String seg) { + List segments = new ArrayList<>(); + List separatorIndices = new ArrayList<>(); + + Matcher complexPatternDelimiterMatcher = Pattern.compile("\\}(_|\\-|\\.|~){1}").matcher(seg); + boolean delimiterFound = complexPatternDelimiterMatcher.find(); + + while (delimiterFound) { + int delimiterIndex = complexPatternDelimiterMatcher.start(); + if (seg.substring(delimiterIndex).startsWith("}")) { + delimiterIndex += 1; + } + String currDelimiter = seg.substring(delimiterIndex, delimiterIndex + 1); + if (!DELIMITER_PATTERN.matcher(currDelimiter).find()) { + throw new ValidationException( + "parse error: invalid complex ID delimiter '%s' in '%s'", currDelimiter, seg); + } + separatorIndices.add(currDelimiter); + delimiterFound = complexPatternDelimiterMatcher.find(delimiterIndex + 1); + } + // The last entry does not have a delimiter. + separatorIndices.add(""); + + String subVarName = null; + Iterable complexSubsegments = + Splitter.onPattern("\\}[_\\-\\.~]").trimResults().split(seg); + boolean complexSegImplicitWildcard = false; + int currIteratorIndex = 0; + for (String complexSeg : complexSubsegments) { + boolean subsegmentBindingStarts = complexSeg.startsWith("{"); + if (subsegmentBindingStarts) { + if (subVarName != null) { + throw new ValidationException("parse error: nested binding in '%s'", complexSeg); + } + complexSeg = complexSeg.substring(1); + } + subVarName = complexSeg.trim(); + + boolean subBindingEnds = complexSeg.endsWith("}"); + int i = complexSeg.indexOf('='); + if (i <= 0) { + // Possibly looking at something like "{name}" with implicit wildcard. + if (subBindingEnds) { + // Remember to add an implicit wildcard later. + complexSegImplicitWildcard = true; + subVarName = complexSeg.substring(0, complexSeg.length() - 1).trim(); + complexSeg = complexSeg.substring(complexSeg.length() - 1).trim(); + } + } else { + // Looking at something like "{name=wildcard}". + subVarName = complexSeg.substring(0, i).trim(); + complexSeg = complexSeg.substring(i + 1).trim(); + if (complexSeg.equals("**")) { + throw new ValidationException( + "parse error: wildcard path not allowed in complex ID resource '%s'", subVarName); + } + } + String complexDelimiter = + currIteratorIndex < separatorIndices.size() + ? separatorIndices.get(currIteratorIndex) + : ""; + segments.add(Segment.create(SegmentKind.BINDING, subVarName)); + segments.add(Segment.wildcardCreate(complexDelimiter)); + segments.add(Segment.END_BINDING); + subVarName = null; + + currIteratorIndex++; + } + return segments; + } + // Helpers // ======= @@ -1003,9 +1110,7 @@ private static void restore(ListIterator segments, int index) { // Equality and String Conversion // ============================== - /** - * Returns a pretty version of the template as a string. - */ + /** Returns a pretty version of the template as a string. */ @Override public String toString() { return toSyntax(segments, true); diff --git a/src/test/java/com/google/api/pathtemplate/PathTemplateTest.java b/src/test/java/com/google/api/pathtemplate/PathTemplateTest.java index 45be25f3b..338ed3643 100644 --- a/src/test/java/com/google/api/pathtemplate/PathTemplateTest.java +++ b/src/test/java/com/google/api/pathtemplate/PathTemplateTest.java @@ -33,6 +33,9 @@ import com.google.common.collect.ImmutableMap; import com.google.common.truth.Truth; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; import java.util.Map; import org.junit.Rule; import org.junit.Test; @@ -40,9 +43,7 @@ import org.junit.runner.RunWith; import org.junit.runners.JUnit4; -/** - * Tests for {@link PathTemplate}. - */ +/** Tests for {@link PathTemplate}. */ @RunWith(JUnit4.class) public class PathTemplateTest { @@ -171,6 +172,250 @@ public void matchWithUnboundInMiddle() { assertPositionalMatch(template.match("bar/foo/foo/foo/bar"), "foo/foo", "bar"); } + // Complex Resource ID Segments. + // ======== + + @Test + public void complexResourceIdBasicCases() { + // Separate by "~". + PathTemplate template = PathTemplate.create("projects/{project}/zones/{zone_a}~{zone_b}"); + Map match = + template.match( + "https://www.googleapis.com/compute/v1/projects/project-123/zones/europe-west3-c~us-east3-a"); + 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}"); + match = template.match("projects/project-123/zones/europe-west3-c~us-east3-a"); + 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}"); + match = template.match("projects/project-123/zones/europe-west3-c.us-east3-a"); + 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}"); + match = template.match("projects/project-123/zones/europe-west3-c_us-east3-a"); + 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=*}"); + Map match = + template.match("projects/project-123/zones/europe-west3-c~us-east3-a"); + Truth.assertThat(match).isNotNull(); + 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"); + } + + @Test + public void complexResourceIdEqualsPathWildcard() { + thrown.expect(ValidationException.class); + PathTemplate template = PathTemplate.create("projects/{project=*}/zones/{zone_a=**}~{zone_b}"); + thrown.expectMessage( + String.format( + "parse error: wildcard path not allowed in complex ID resource '%s'", "zone_a")); + + template = PathTemplate.create("projects/{project=*}/zones/{zone_a}.{zone_b=**}"); + thrown.expectMessage( + String.format( + "parse error: wildcard path not allowed in complex ID resource '%s'", "zone_b")); + } + + @Test + public void complexResourceIdMissingMatches() { + PathTemplate template = PathTemplate.create("projects/{project}/zones/{zone_a}~{zone_b}"); + Truth.assertThat(template.match("projects/project-123/zones/europe-west3-c")).isNull(); + + template = PathTemplate.create("projects/{project}/zones/{zone_a}~{zone_b}.{zone_c}"); + Map match = + template.match("projects/project-123/zones/europe-west3-c~.us-east3-a"); + Truth.assertThat(match).isNotNull(); + 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")).isEmpty(); + Truth.assertThat(match.get("zone_c")).isEqualTo("us-east3-a"); + } + + @Test + public void complexResourceIdNoSeparator() { + thrown.expect(ValidationException.class); + PathTemplate.create("projects/{project}/zones/{zone_a}{zone_b}"); + thrown.expectMessage( + String.format( + "parse error: missing or invalid complex resource ID delimiter character in '%s'", + "{zone_a}{zone_b}")); + + PathTemplate.create("projects/{project}/zones/{zone_a}_{zone_b}{zone_c}"); + thrown.expectMessage( + String.format( + "parse error: missing or invalid complex resource ID delimiter character in '%s'", + "{zone_a}_{zone_b}{zone_c}")); + } + + @Test + public void complexResourceIdInvalidDelimiter() { + thrown.expect(ValidationException.class); + // Not a comprehensive set of invalid delimiters, please check the class's defined pattern. + List someInvalidDelimiters = + new ArrayList<>(Arrays.asList("|", "!", "@", "a", "1", ",", "{", ")")); + for (String invalidDelimiter : someInvalidDelimiters) { + PathTemplate.create( + String.format("projects/{project=*}/zones/{zone_a}%s{zone_b}", invalidDelimiter)); + thrown.expectMessage( + String.format( + "parse error: missing or invalid complex resource ID delimiter character in '%s'", + String.format("{zone_a}%s{zone_b}", invalidDelimiter))); + } + } + + @Test + public void complexResourceIdMixedSeparators() { + // Separate by a mix of delimiters. + PathTemplate template = + PathTemplate.create("projects/{project}/zones/{zone_a}~{zone_b}.{zone_c}-{zone_d}"); + Map match = + template.match( + "https://www.googleapis.com/compute/v1/projects/project-123/zones/europe-west3-c~us-east3-a.us-west2-b-europe-west2-b"); + 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")).isEqualTo("europe-west3-c"); + Truth.assertThat(match.get("zone_b")).isEqualTo("us-east3-a"); + Truth.assertThat(match.get("zone_c")).isEqualTo("us"); + Truth.assertThat(match.get("zone_d")).isEqualTo("west2-b-europe-west2-b"); + + template = PathTemplate.create("projects/{project}/zones/{zone_a}.{zone_b}.{zone_c}~{zone_d}"); + match = + template.match( + "https://www.googleapis.com/compute/v1/projects/project-123/zones/europe-west3-c.us-east3-a.us-west2-b~europe-west2-b"); + 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")).isEqualTo("europe-west3-c"); + Truth.assertThat(match.get("zone_b")).isEqualTo("us-east3-a"); + Truth.assertThat(match.get("zone_c")).isEqualTo("us-west2-b"); + Truth.assertThat(match.get("zone_d")).isEqualTo("europe-west2-b"); + } + + @Test + public void complexResourceIdInParent() { + // One parent has a complex resource ID. + PathTemplate template = + PathTemplate.create( + "projects/{project}/zones/{zone_a}-{zone_b}_{zone_c}/machines/{machine}"); + Map match = + template.match( + "https://www.googleapis.com/compute/v1/projects/project-123/zones/europe-west3-c-us-east3-a_us-west2-b/machines/roomba"); + 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")).isEqualTo("europe"); + Truth.assertThat(match.get("zone_b")).isEqualTo("west3-c-us-east3-a"); + Truth.assertThat(match.get("zone_c")).isEqualTo("us-west2-b"); + Truth.assertThat(match.get("machine")).isEqualTo("roomba"); + + // All parents and resource IDs have complex resource IDs. + template = + PathTemplate.create( + "projects/{foo}_{bar}/zones/{zone_a}-{zone_b}_{zone_c}/machines/{cell1}.{cell2}"); + match = + template.match( + "https://www.googleapis.com/compute/v1/projects/project_123/zones/europe-west3-c-us-east3-a_us-west2-b/machines/roomba.broomba"); + Truth.assertThat(match).isNotNull(); + Truth.assertThat(match.get(PathTemplate.HOSTNAME_VAR)).isEqualTo("https://www.googleapis.com"); + Truth.assertThat(match.get("foo")).isEqualTo("project"); + Truth.assertThat(match.get("bar")).isEqualTo("123"); + Truth.assertThat(match.get("zone_a")).isEqualTo("europe"); + Truth.assertThat(match.get("zone_b")).isEqualTo("west3-c-us-east3-a"); + Truth.assertThat(match.get("zone_c")).isEqualTo("us-west2-b"); + Truth.assertThat(match.get("cell1")).isEqualTo("roomba"); + Truth.assertThat(match.get("cell2")).isEqualTo("broomba"); + } + + @Test + public void complexResourceBasicInvalidIds() { + thrown.expect(ValidationException.class); + PathTemplate.create("projects/*/zones/~{zone_a}"); + thrown.expectMessage( + String.format("parse error: invalid begin or end character in '%s'", "~{zone_a}")); + + PathTemplate.create("projects/*/zones/{zone_a}~"); + thrown.expectMessage( + String.format("parse error: invalid begin or end character in '%s'", "{zone_a}~")); + + PathTemplate.create("projects/*/zones/.{zone_a}"); + thrown.expectMessage( + String.format("parse error: invalid begin or end character in '%s'", ".{zone_a}")); + + PathTemplate.create("projects/*/zones/{zone_a}."); + thrown.expectMessage( + String.format("parse error: invalid begin or end character in '%s'", "{zone_a}.")); + + PathTemplate.create("projects/*/zones/-{zone_a}"); + thrown.expectMessage( + String.format("parse error: invalid begin or end character in '%s'", "-{zone_a}")); + + PathTemplate.create("projects/*/zones/{zone_a}-"); + thrown.expectMessage( + String.format("parse error: invalid begin or end character in '%s'", "{zone_a}-")); + + PathTemplate.create("projects/*/zones/_{zone_a}"); + thrown.expectMessage( + String.format("parse error: invalid begin or end character in '%s'", "{zone_a}_")); + + PathTemplate.create("projects/*/zones/{zone_a}_"); + thrown.expectMessage( + String.format("parse error: invalid begin or end character in '%s'", "{zone_a}_")); + } + + @Test + public void complexResourceMultipleDelimiters() { + thrown.expect(ValidationException.class); + PathTemplate.create("projects/*/zones/.-~{zone_a}"); + thrown.expectMessage( + String.format("parse error: invalid begin or end character in '%s'", ".-~{zone_a}")); + + PathTemplate.create("projects/*/zones/{zone_a}~.{zone_b}"); + thrown.expectMessage( + String.format( + "parse error: two consecutive delimiter characters in '%s'", "{zone_a}~.{zone_b}")); + + PathTemplate.create("projects/*/zones/{zone_a}~{zone_b}..{zone_c}"); + thrown.expectMessage( + String.format( + "parse error: two consecutive delimiter characters in '%s'", + "{zone_a}~{zone_b}..{zone_c}")); + + String pathString = "projects/project_123/zones/lorum~ipsum"; + PathTemplate template = PathTemplate.create("projects/*/zones/{zone_.~-a}~{zone_b}"); + template.validate(pathString, ""); + // No assertion - success is no exception thrown from template.validate(). + Map match = template.match(pathString); + Truth.assertThat(match).isNotNull(); + Truth.assertThat(match.get("zone_.~-a")).isEqualTo("lorum"); + Truth.assertThat(match.get("zone_b")).isEqualTo("ipsum"); + } + // Validate // ======== From e2181f6c4ae868b2fe994aa032dd5c59b24943d1 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Wed, 6 May 2020 00:38:24 +0000 Subject: [PATCH 2/3] remove debug printf --- .../google/api/pathtemplate/PathTemplate.java | 71 +++++++------------ 1 file changed, 27 insertions(+), 44 deletions(-) diff --git a/src/main/java/com/google/api/pathtemplate/PathTemplate.java b/src/main/java/com/google/api/pathtemplate/PathTemplate.java index 6b249ef06..d18ae118f 100644 --- a/src/main/java/com/google/api/pathtemplate/PathTemplate.java +++ b/src/main/java/com/google/api/pathtemplate/PathTemplate.java @@ -53,21 +53,18 @@ /** * Represents a path template. * - *

- * Templates use the syntax of the API platform; see the protobuf of HttpRule for details. A + *

Templates use the syntax of the API platform; see the protobuf of HttpRule for details. A * template consists of a sequence of literals, wildcards, and variable bindings, where each binding - * can have a sub-path. A string representation can be parsed into an instance of - * {@link PathTemplate}, which can then be used to perform matching and instantiation. + * can have a sub-path. A string representation can be parsed into an instance of {@link + * PathTemplate}, which can then be used to perform matching and instantiation. * - *

- * Matching and instantiation deals with unescaping and escaping using URL encoding rules. For - * example, if a template variable for a single segment is instantiated with a string like - * {@code "a/b"}, the slash will be escaped to {@code "%2f"}. (Note that slash will not be escaped - * for a multiple-segment variable, but other characters will). The literals in the template itself - * are not escaped automatically, and must be already URL encoded. + *

Matching and instantiation deals with unescaping and escaping using URL encoding rules. For + * example, if a template variable for a single segment is instantiated with a string like {@code + * "a/b"}, the slash will be escaped to {@code "%2f"}. (Note that slash will not be escaped for a + * multiple-segment variable, but other characters will). The literals in the template itself are + * not escaped automatically, and must be already URL encoded. * - *

- * Here is an example for a template using simple variables: + *

Here is an example for a template using simple variables: * * 

  *   PathTemplate template = PathTemplate.create("v1/shelves/{shelf}/books/{book}");
@@ -125,9 +122,8 @@
 public class PathTemplate {
 
   /**
-   * A constant identifying the special variable used for endpoint bindings in the result of
-   * {@link #matchFromFullName(String)}. It may also contain protocol string, if its provided in the
-   * input.
+   * A constant identifying the special variable used for endpoint bindings in the result of {@link
+   * #matchFromFullName(String)}. It may also contain protocol string, if its provided in the input.
    */
   public static final String HOSTNAME_VAR = "$hostname";
 
@@ -333,11 +329,10 @@ public PathTemplate withoutVars() {
    *
* * The returned template will never have named variables, but only wildcards, which are dealt with - * in matching and instantiation using '$n'-variables. See the documentation of - * {@link #match(String)} and {@link #instantiate(Map)}, respectively. + * in matching and instantiation using '$n'-variables. See the documentation of {@link + * #match(String)} and {@link #instantiate(Map)}, respectively. * - *

- * For a variable which has no sub-path, this returns a path template with a single wildcard + *

For a variable which has no sub-path, this returns a path template with a single wildcard * ('*'). * * @throws ValidationException if the variable does not exist in the template. @@ -403,9 +398,7 @@ public void validate(String path, String exceptionMessagePrefix) { throw new ValidationException( String.format( "%s: Parameter \"%s\" must be in the form \"%s\"", - exceptionMessagePrefix, - path, - this.toString())); + exceptionMessagePrefix, path, this.toString())); } } @@ -415,15 +408,12 @@ public void validate(String path, String exceptionMessagePrefix) { * throws a ValidationException. The exceptionMessagePrefix parameter will be prepended to the * ValidationException message. * - *

- * If the path starts with '//', the first segment will be interpreted as a host name and stored - * in the variable {@link #HOSTNAME_VAR}. + *

If the path starts with '//', the first segment will be interpreted as a host name and + * stored in the variable {@link #HOSTNAME_VAR}. * - *

- * See the {@link PathTemplate} class documentation for examples. + *

See the {@link PathTemplate} class documentation for examples. * - *

- * For free wildcards in the template, the matching process creates variables named '$n', where + *

For free wildcards in the template, the matching process creates variables named '$n', where * 'n' is the wildcard's position in the template (starting at n=0). For example: * * 

@@ -450,9 +440,7 @@ public Map validatedMatch(String path, String exceptionMessagePr
       throw new ValidationException(
           String.format(
               "%s: Parameter \"%s\" must be in the form \"%s\"",
-              exceptionMessagePrefix,
-              path,
-              this.toString()));
+              exceptionMessagePrefix, path, this.toString()));
     }
     return matchMap;
   }
@@ -467,15 +455,12 @@ public boolean matches(String path) {
    * will be properly unescaped using URL encoding rules. If the path does not match the template,
    * null is returned.
    *
-   * 

-   * If the path starts with '//', the first segment will be interpreted as a host name and stored
-   * in the variable {@link #HOSTNAME_VAR}.
+   * 
If the path starts with '//', the first segment will be interpreted as a host name and
+   * stored in the variable {@link #HOSTNAME_VAR}.
    *
-   * 

-   * See the {@link PathTemplate} class documentation for examples.
+   * 
See the {@link PathTemplate} class documentation for examples.
    *
-   * 

-   * For free wildcards in the template, the matching process creates variables named '$n', where
+   * 
For free wildcards in the template, the matching process creates variables named '$n', where
    * 'n' is the wildcard's position in the template (starting at n=0). For example:
    *
    * 
@@ -676,10 +661,9 @@ private static String concatCaptures(@Nullable String cur, String next) {
    * Instantiate the template based on the given variable assignment. Performs proper URL escaping
    * of variable assignments.
    *
-   * 

-   * Note that free wildcards in the template must have bindings of '$n' variables, where 'n' is the
-   * position of the wildcard (starting at 0). See the documentation of {@link #match(String)} for
-   * details.
+   * 
Note that free wildcards in the template must have bindings of '$n' variables, where 'n' is
+   * the position of the wildcard (starting at 0). See the documentation of {@link #match(String)}
+   * for details.
    *
    * @throws ValidationException if a variable occurs in the template without a binding.
    */
@@ -835,7 +819,6 @@ public List decode(String path) {
   // ================
 
   private static ImmutableList parseTemplate(String template) {
-    System.out.println("DEL: PARSING TEMPLATE");
     // Skip useless leading slash.
     if (template.startsWith("/")) {
       template = template.substring(1);

From 12b22f3738326016dc0d82e42c2671d8ebf68400 Mon Sep 17 00:00:00 2001
From: Mira Leung 
Date: Thu, 7 May 2020 19:12:26 +0000
Subject: [PATCH 3/3] fix: clean up PathTemplate.java and tests

---
 .../google/api/pathtemplate/PathTemplate.java | 131 +++++++++++-------
 .../api/pathtemplate/PathTemplateTest.java    |  11 +-
 2 files changed, 86 insertions(+), 56 deletions(-)

diff --git a/src/main/java/com/google/api/pathtemplate/PathTemplate.java b/src/main/java/com/google/api/pathtemplate/PathTemplate.java
index d18ae118f..970fa510e 100644
--- a/src/main/java/com/google/api/pathtemplate/PathTemplate.java
+++ b/src/main/java/com/google/api/pathtemplate/PathTemplate.java
@@ -53,18 +53,21 @@
 /**
  * Represents a path template.
  *
- * 
Templates use the syntax of the API platform; see the protobuf of HttpRule for details. A
+ * 

+ * Templates use the syntax of the API platform; see the protobuf of HttpRule for details. A
  * template consists of a sequence of literals, wildcards, and variable bindings, where each binding
- * can have a sub-path. A string representation can be parsed into an instance of {@link
- * PathTemplate}, which can then be used to perform matching and instantiation.
+ * can have a sub-path. A string representation can be parsed into an instance of
+ * {@link PathTemplate}, which can then be used to perform matching and instantiation.
  *
- * 
Matching and instantiation deals with unescaping and escaping using URL encoding rules. For
- * example, if a template variable for a single segment is instantiated with a string like {@code
- * "a/b"}, the slash will be escaped to {@code "%2f"}. (Note that slash will not be escaped for a
- * multiple-segment variable, but other characters will). The literals in the template itself are
- * not escaped automatically, and must be already URL encoded.
+ * 

+ * Matching and instantiation deals with unescaping and escaping using URL encoding rules. For
+ * example, if a template variable for a single segment is instantiated with a string like
+ * {@code "a/b"}, the slash will be escaped to {@code "%2f"}. (Note that slash will not be escaped
+ * for a multiple-segment variable, but other characters will). The literals in the template itself
+ * are not escaped automatically, and must be already URL encoded.
  *
- * 
Here is an example for a template using simple variables:
+ * 

+ * Here is an example for a template using simple variables:
  *
  * 
  *   PathTemplate template = PathTemplate.create("v1/shelves/{shelf}/books/{book}");
@@ -122,8 +125,9 @@
 public class PathTemplate {
 
   /**
-   * A constant identifying the special variable used for endpoint bindings in the result of {@link
-   * #matchFromFullName(String)}. It may also contain protocol string, if its provided in the input.
+   * A constant identifying the special variable used for endpoint bindings in the result of
+   * {@link #matchFromFullName(String)}. It may also contain protocol string, if its provided in the
+   * input.
    */
   public static final String HOSTNAME_VAR = "$hostname";
 
@@ -137,7 +141,22 @@ public class PathTemplate {
   private static final Splitter SLASH_SPLITTER = Splitter.on('/').trimResults();
 
   // A regex to match the valid complex resource ID delimiters.
-  private static final Pattern DELIMITER_PATTERN = Pattern.compile("(_|\\-|\\.|~)");
+  private static final Pattern COMPLEX_DELIMITER_PATTERN = Pattern.compile("[_\\-\\.~]");
+
+  // A regex to match multiple complex resource ID delimiters.
+  private static final Pattern MULTIPLE_COMPLEX_DELIMITER_PATTERN =
+      Pattern.compile("\\}[_\\-\\.~]{2,}\\{");
+
+  // A regex to match a missing complex resource ID delimiter.
+  private static final Pattern MISSING_COMPLEX_DELIMITER_PATTERN = Pattern.compile("\\}\\{");
+
+  // A regex to match invalid complex resource ID delimiters.
+  private static final Pattern INVALID_COMPLEX_DELIMITER_PATTERN =
+      Pattern.compile("\\}[^_\\-\\.~]\\{");
+
+  // A regex to match a closing segment (end brace) followed by one complex resource ID delimiter.
+  private static final Pattern END_SEGMENT_COMPLEX_DELIMITER_PATTERN =
+      Pattern.compile("\\}[_\\-\\.~]{1}");
 
   // Helper Types
   // ============
@@ -185,7 +204,7 @@ private static Segment wildcardCreate(String complexSeparator) {
       return new AutoValue_PathTemplate_Segment(
           SegmentKind.WILDCARD,
           "*",
-          !complexSeparator.isEmpty() && DELIMITER_PATTERN.matcher(complexSeparator).find()
+          !complexSeparator.isEmpty() && COMPLEX_DELIMITER_PATTERN.matcher(complexSeparator).find()
               ? complexSeparator
               : "");
     }
@@ -329,10 +348,11 @@ public PathTemplate withoutVars() {
    * 

    *
    * The returned template will never have named variables, but only wildcards, which are dealt with
-   * in matching and instantiation using '$n'-variables. See the documentation of {@link
-   * #match(String)} and {@link #instantiate(Map)}, respectively.
+   * in matching and instantiation using '$n'-variables. See the documentation of
+   * {@link #match(String)} and {@link #instantiate(Map)}, respectively.
    *
-   * 
For a variable which has no sub-path, this returns a path template with a single wildcard
+   * 

+   * For a variable which has no sub-path, this returns a path template with a single wildcard
    * ('*').
    *
    * @throws ValidationException if the variable does not exist in the template.
@@ -398,7 +418,9 @@ public void validate(String path, String exceptionMessagePrefix) {
       throw new ValidationException(
           String.format(
               "%s: Parameter \"%s\" must be in the form \"%s\"",
-              exceptionMessagePrefix, path, this.toString()));
+              exceptionMessagePrefix,
+              path,
+              this.toString()));
     }
   }
 
@@ -408,12 +430,15 @@ public void validate(String path, String exceptionMessagePrefix) {
    * throws a ValidationException. The exceptionMessagePrefix parameter will be prepended to the
    * ValidationException message.
    *
-   * 
If the path starts with '//', the first segment will be interpreted as a host name and
-   * stored in the variable {@link #HOSTNAME_VAR}.
+   * 

+   * If the path starts with '//', the first segment will be interpreted as a host name and stored
+   * in the variable {@link #HOSTNAME_VAR}.
    *
-   * 
See the {@link PathTemplate} class documentation for examples.
+   * 

+   * See the {@link PathTemplate} class documentation for examples.
    *
-   * 
For free wildcards in the template, the matching process creates variables named '$n', where
+   * 

+   * For free wildcards in the template, the matching process creates variables named '$n', where
    * 'n' is the wildcard's position in the template (starting at n=0). For example:
    *
    * 
@@ -440,7 +465,9 @@ public Map validatedMatch(String path, String exceptionMessagePr
       throw new ValidationException(
           String.format(
               "%s: Parameter \"%s\" must be in the form \"%s\"",
-              exceptionMessagePrefix, path, this.toString()));
+              exceptionMessagePrefix,
+              path,
+              this.toString()));
     }
     return matchMap;
   }
@@ -455,12 +482,15 @@ public boolean matches(String path) {
    * will be properly unescaped using URL encoding rules. If the path does not match the template,
    * null is returned.
    *
-   * 
If the path starts with '//', the first segment will be interpreted as a host name and
-   * stored in the variable {@link #HOSTNAME_VAR}.
+   * 

+   * If the path starts with '//', the first segment will be interpreted as a host name and stored
+   * in the variable {@link #HOSTNAME_VAR}.
    *
-   * 
See the {@link PathTemplate} class documentation for examples.
+   * 

+   * See the {@link PathTemplate} class documentation for examples.
    *
-   * 
For free wildcards in the template, the matching process creates variables named '$n', where
+   * 

+   * For free wildcards in the template, the matching process creates variables named '$n', where
    * 'n' is the wildcard's position in the template (starting at n=0). For example:
    *
    * 
@@ -574,7 +604,7 @@ private boolean match(
       int segPos,
       Map values) {
     String currentVar = null;
-    List modifableInput = new ArrayList<>(input);
+    List modifiableInput = new ArrayList<>(input);
     while (segPos < segments.size()) {
       Segment seg = segments.get(segPos++);
       switch (seg.kind()) {
@@ -592,12 +622,12 @@ private boolean match(
           break;
         case LITERAL:
         case WILDCARD:
-          if (inPos >= modifableInput.size()) {
+          if (inPos >= modifiableInput.size()) {
             // End of input
             return false;
           }
           // Check literal match.
-          String next = decodeUrl(modifableInput.get(inPos++));
+          String next = decodeUrl(modifiableInput.get(inPos++));
           if (seg.kind() == SegmentKind.LITERAL) {
             if (!seg.value().equals(next)) {
               // Literal does not match.
@@ -608,11 +638,11 @@ private boolean match(
             // Parse the complex resource separators one by one.
             int complexSeparatorIndex = next.indexOf(seg.complexSeparator());
             if (complexSeparatorIndex >= 0) {
-              modifableInput.add(inPos, next.substring(complexSeparatorIndex + 1));
+              modifiableInput.add(inPos, next.substring(complexSeparatorIndex + 1));
               next = next.substring(0, complexSeparatorIndex);
-              modifableInput.set(inPos - 1, next);
+              modifiableInput.set(inPos - 1, next);
             } else {
-              // No coplex resource ID separator found in the literal when we expected one.
+              // No complex resource ID separator found in the literal when we expected one.
               return false;
             }
           }
@@ -635,7 +665,7 @@ private boolean match(
                 segsToMatch++;
             }
           }
-          int available = (modifableInput.size() - inPos) - segsToMatch;
+          int available = (modifiableInput.size() - inPos) - segsToMatch;
           // If this segment is empty, make sure it is still captured.
           if (available == 0 && !values.containsKey(currentVar)) {
             values.put(currentVar, "");
@@ -643,11 +673,11 @@ private boolean match(
           while (available-- > 0) {
             values.put(
                 currentVar,
-                concatCaptures(values.get(currentVar), decodeUrl(modifableInput.get(inPos++))));
+                concatCaptures(values.get(currentVar), decodeUrl(modifiableInput.get(inPos++))));
           }
       }
     }
-    return inPos == modifableInput.size();
+    return inPos == modifiableInput.size();
   }
 
   private static String concatCaptures(@Nullable String cur, String next) {
@@ -661,9 +691,10 @@ private static String concatCaptures(@Nullable String cur, String next) {
    * Instantiate the template based on the given variable assignment. Performs proper URL escaping
    * of variable assignments.
    *
-   * 
Note that free wildcards in the template must have bindings of '$n' variables, where 'n' is
-   * the position of the wildcard (starting at 0). See the documentation of {@link #match(String)}
-   * for details.
+   * 

+   * Note that free wildcards in the template must have bindings of '$n' variables, where 'n' is the
+   * position of the wildcard (starting at 0). See the documentation of {@link #match(String)} for
+   * details.
    *
    * @throws ValidationException if a variable occurs in the template without a binding.
    */
@@ -838,14 +869,15 @@ private static ImmutableList parseTemplate(String template) {
     int pathWildCardBound = 0;
 
     for (String seg : Splitter.on('/').trimResults().split(template)) {
-      if (DELIMITER_PATTERN.matcher(seg.substring(0, 1)).find()
-          || DELIMITER_PATTERN.matcher(seg.substring(seg.length() - 1)).find()) {
+      if (COMPLEX_DELIMITER_PATTERN.matcher(seg.substring(0, 1)).find()
+          || COMPLEX_DELIMITER_PATTERN.matcher(seg.substring(seg.length() - 1)).find()) {
         throw new ValidationException("parse error: invalid begin or end character in '%s'", seg);
       }
       // Disallow zero or multiple delimiters between variable names.
-      if (Pattern.compile("\\}(_|\\-|\\.|~){2,}\\{").matcher(seg).find()) {
+      if (MULTIPLE_COMPLEX_DELIMITER_PATTERN.matcher(seg).find()
+          || MISSING_COMPLEX_DELIMITER_PATTERN.matcher(seg).find()) {
         throw new ValidationException(
-            "parse error: two consecutive delimiter characters in '%s'", seg);
+            "parse error: missing or 2+ consecutive delimiter characters in '%s'", seg);
       }
       // If segment starts with '{', a binding group starts.
       boolean bindingStarts = seg.startsWith("{");
@@ -858,19 +890,16 @@ private static ImmutableList parseTemplate(String template) {
         seg = seg.substring(1);
 
         // Check for invalid complex resource ID delimiters.
-        if (Pattern.compile("\\}[^_\\-\\.~]\\{").matcher(seg).find()
-            || Pattern.compile("\\}\\{").matcher(seg).find()) {
+        if (INVALID_COMPLEX_DELIMITER_PATTERN.matcher(seg).find()) {
           throw new ValidationException(
-              "parse error: missing or invalid complex resource ID delimiter character in '%s'",
-              seg);
+              "parse error: invalid complex resource ID delimiter character in '%s'", seg);
         }
 
-        Matcher complexPatternDelimiterMatcher =
-            Pattern.compile("\\}(_|\\-|\\.|~){1}").matcher(seg);
+        Matcher complexPatternDelimiterMatcher = END_SEGMENT_COMPLEX_DELIMITER_PATTERN.matcher(seg);
         complexDelimiterFound = complexPatternDelimiterMatcher.find();
 
         // Look for complex resource names.
-        // Need to handles something like "{user_a}~{user_b}".
+        // Need to handle something like "{user_a}~{user_b}".
         if (complexDelimiterFound) {
           builder.addAll(parseComplexResourceId(seg));
         } else {
@@ -965,7 +994,7 @@ private static List parseComplexResourceId(String seg) {
     List segments = new ArrayList<>();
     List separatorIndices = new ArrayList<>();
 
-    Matcher complexPatternDelimiterMatcher = Pattern.compile("\\}(_|\\-|\\.|~){1}").matcher(seg);
+    Matcher complexPatternDelimiterMatcher = END_SEGMENT_COMPLEX_DELIMITER_PATTERN.matcher(seg);
     boolean delimiterFound = complexPatternDelimiterMatcher.find();
 
     while (delimiterFound) {
@@ -974,7 +1003,7 @@ private static List parseComplexResourceId(String seg) {
         delimiterIndex += 1;
       }
       String currDelimiter = seg.substring(delimiterIndex, delimiterIndex + 1);
-      if (!DELIMITER_PATTERN.matcher(currDelimiter).find()) {
+      if (!COMPLEX_DELIMITER_PATTERN.matcher(currDelimiter).find()) {
         throw new ValidationException(
             "parse error: invalid complex ID delimiter '%s' in '%s'", currDelimiter, seg);
       }
diff --git a/src/test/java/com/google/api/pathtemplate/PathTemplateTest.java b/src/test/java/com/google/api/pathtemplate/PathTemplateTest.java
index 338ed3643..33b9033b8 100644
--- a/src/test/java/com/google/api/pathtemplate/PathTemplateTest.java
+++ b/src/test/java/com/google/api/pathtemplate/PathTemplateTest.java
@@ -262,13 +262,13 @@ public void complexResourceIdNoSeparator() {
     PathTemplate.create("projects/{project}/zones/{zone_a}{zone_b}");
     thrown.expectMessage(
         String.format(
-            "parse error: missing or invalid complex resource ID delimiter character in '%s'",
+            "parse error: missing or 2+ consecutive delimiter characters in '%s'",
             "{zone_a}{zone_b}"));
 
     PathTemplate.create("projects/{project}/zones/{zone_a}_{zone_b}{zone_c}");
     thrown.expectMessage(
         String.format(
-            "parse error: missing or invalid complex resource ID delimiter character in '%s'",
+            "parse error: missing or 2+ consecutive delimiter characters in '%s'",
             "{zone_a}_{zone_b}{zone_c}"));
   }
 
@@ -283,7 +283,7 @@ public void complexResourceIdInvalidDelimiter() {
           String.format("projects/{project=*}/zones/{zone_a}%s{zone_b}", invalidDelimiter));
       thrown.expectMessage(
           String.format(
-              "parse error: missing or invalid complex resource ID delimiter character in '%s'",
+              "parse error: invalid complex resource ID delimiter character in '%s'",
               String.format("{zone_a}%s{zone_b}", invalidDelimiter)));
     }
   }
@@ -398,12 +398,13 @@ public void complexResourceMultipleDelimiters() {
     PathTemplate.create("projects/*/zones/{zone_a}~.{zone_b}");
     thrown.expectMessage(
         String.format(
-            "parse error: two consecutive delimiter characters in '%s'", "{zone_a}~.{zone_b}"));
+            "parse error: missing or 2+ consecutive delimiter characters in '%s'",
+            "{zone_a}~.{zone_b}"));
 
     PathTemplate.create("projects/*/zones/{zone_a}~{zone_b}..{zone_c}");
     thrown.expectMessage(
         String.format(
-            "parse error: two consecutive delimiter characters in '%s'",
+            "parse error: missing or 2+ consecutive delimiter characters in '%s'",
             "{zone_a}~{zone_b}..{zone_c}"));
 
     String pathString = "projects/project_123/zones/lorum~ipsum";