[java] fixed partial matches for UrlTemplate

java/src/org/openqa/selenium/remote/http/UrlTemplate.java

@@ -24,37 +24,48 @@
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
-/** An incredibly bad implementation of URL Templates, but enough for our needs. */
+/** A bad implementation of URL Templates, but enough for our needs. */
 public class UrlTemplate {
 
-  private static final Pattern GROUP_NAME = Pattern.compile("\\(\\?<([a-zA-Z][a-zA-Z0-9]*)>");
-  private final List<Matches> template;
+  private static final Pattern GROUP_NAME = Pattern.compile("(\\{\\p{Alnum}+\\})");
+  private final Pattern pattern;
+  private final List<String> groups;
 
   public UrlTemplate(String template) {
     if (template == null || template.isEmpty()) {
       throw new IllegalArgumentException("Template must not be 0 length");
     }
 
-    ImmutableList.Builder<Matches> fragments = ImmutableList.builder();
-    for (String fragment : template.split("/")) {
-      // Convert the fragment to a pattern by replacing "{...}" with a capturing group. We capture
-      // from the opening '{' and do a non-greedy match of letters until the closing '}'.
-      Matcher matcher = Pattern.compile("\\{(\\p{Alnum}+?)\\}").matcher(fragment);
-      String toCompile = matcher.replaceAll("(?<$1>[^/]+)");
+    // ^ start of string
+    StringBuilder regex = new StringBuilder("^");
+    Matcher groupNameMatcher = GROUP_NAME.matcher(template);
 
-      // There's no API for getting the names of capturing groups from a pattern in java. So we're
-      // going to use a regex to find them. ffs.
-      Matcher groupNameMatcher = GROUP_NAME.matcher(toCompile);
+    ImmutableList.Builder<String> groups = ImmutableList.builder();
+    int lastGroup = 0;
 
-      ImmutableList.Builder<String> names = ImmutableList.builder();
-      while (groupNameMatcher.find()) {
-        names.add(groupNameMatcher.group(1));
-      }
+    while (groupNameMatcher.find()) {
+      int start = groupNameMatcher.start(1);
+      int end = groupNameMatcher.end(1);
 
-      fragments.add(
-          new Matches(Pattern.compile(Matcher.quoteReplacement(toCompile)), names.build()));
+      // everything before the current group
+      regex.append(Pattern.quote(template.substring(lastGroup, start)));
+      // replace the group name with a capturing group
+      regex.append("([^/]+)");
+      // register the group name, to resolve into parameters
+      groups.add(template.substring(start + 1, end - 1));
+      lastGroup = end;
     }
-    this.template = fragments.build();
+
+    if (template.length() > lastGroup) {
+      // everything behind the last group
+      regex.append(Pattern.quote(template.substring(lastGroup)));
+    }
+
+    // $ end of string
+    regex.append('$');
+
+    this.pattern = Pattern.compile(regex.toString());
+    this.groups = groups.build();
   }
 
   /**
@@ -65,21 +76,14 @@ public UrlTemplate.Match match(String matchAgainst) {
       return null;
     }
 
-    String[] fragments = matchAgainst.split("/");
-    if (fragments.length != template.size()) {
+    Matcher matcher = pattern.matcher(matchAgainst);
+    if (!matcher.matches()) {
       return null;
     }
 
     ImmutableMap.Builder<String, String> params = ImmutableMap.builder();
-    for (int i = 0; i < fragments.length; i++) {
-      Matcher matcher = template.get(i).matcher(fragments[i]);
-      if (!matcher.find()) {
-        return null;
-      }
-
-      for (String name : template.get(i).groupNames) {
-        params.put(name, matcher.group(name));
-      }
+    for (int i = 0; i < groups.size(); i++) {
+      params.put(groups.get(i), matcher.group(i + 1));
     }
 
     return new Match(matchAgainst, params.build());
@@ -103,19 +107,4 @@ public Map<String, String> getParameters() {
       return parameters;
     }
   }
-
-  private static class Matches {
-
-    private final Pattern pattern;
-    private final List<String> groupNames;
-
-    private Matches(Pattern pattern, List<String> groupNames) {
-      this.pattern = pattern;
-      this.groupNames = groupNames;
-    }
-
-    public Matcher matcher(String fragment) {
-      return pattern.matcher(fragment);
-    }
-  }
 }

java/test/org/openqa/selenium/grid/node/CustomLocatorHandlerTest.java

@@ -241,7 +241,7 @@ void shouldBeAbleToRootASearchWithinAnElement() {
 
     Node node = Mockito.mock(Node.class);
     when(node.executeWebDriverCommand(
-            argThat(matchesUri("/session/{sessionId}/element/{elementId}/element"))))
+            argThat(matchesUri("/session/{sessionId}/element/{elementId}/elements"))))
         .thenReturn(
             new HttpResponse()
                 .addHeader("Content-Type", Json.JSON_UTF_8)

java/test/org/openqa/selenium/remote/http/UrlTemplateTest.java

@@ -49,6 +49,14 @@ void shouldExpandParameters() {
     assertThat(match.getParameters()).isEqualTo(ImmutableMap.of("veggie", "cake"));
   }
 
+  @Test
+  void shouldExpandTwoParameters() {
+    UrlTemplate.Match match = new UrlTemplate("/i/like/{flavor}/{veggie}").match("/i/like/sweet/cake");
+
+    assertThat(match.getUrl()).isEqualTo("/i/like/sweet/cake");
+    assertThat(match.getParameters()).isEqualTo(ImmutableMap.of("flavor", "sweet", "veggie", "cake"));
+  }
+
   @Test
   void itIsFineForTheFirstCharacterToBeAPattern() {
     UrlTemplate.Match match = new UrlTemplate("{cake}/type").match("cheese/type");
@@ -61,4 +69,11 @@ void itIsFineForTheFirstCharacterToBeAPattern() {
   void aNullMatchDoesNotCauseANullPointerExceptionToBeThrown() {
     assertThat(new UrlTemplate("/").match(null)).isNull();
   }
+
+  @Test
+  void noPartialMatches() {
+    assertThat(new UrlTemplate("/session").match("/no-session")).isNull();
+    assertThat(new UrlTemplate("/session").match("/session-no")).isNull();
+    assertThat(new UrlTemplate("/session").match("/no-session-no")).isNull();
+  }
 }