Skip to content

Commit 30adcb4

Browse files
joerg1985joerg1985
committed
[java] fixed partial matches for UrlTemplate
1 parent a918642 commit 30adcb4

3 files changed

Lines changed: 50 additions & 46 deletions

File tree

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

Lines changed: 34 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -24,37 +24,48 @@
2424
import java.util.regex.Matcher;
2525
import java.util.regex.Pattern;
2626

27-
/** An incredibly bad implementation of URL Templates, but enough for our needs. */
27+
/** A bad implementation of URL Templates, but enough for our needs. */
2828
public class UrlTemplate {
2929

30-
private static final Pattern GROUP_NAME = Pattern.compile("\\(\\?<([a-zA-Z][a-zA-Z0-9]*)>");
31-
private final List<Matches> template;
30+
private static final Pattern GROUP_NAME = Pattern.compile("(\\{\\p{Alnum}+\\})");
31+
private final Pattern pattern;
32+
private final List<String> groups;
3233

3334
public UrlTemplate(String template) {
3435
if (template == null || template.isEmpty()) {
3536
throw new IllegalArgumentException("Template must not be 0 length");
3637
}
3738

38-
ImmutableList.Builder<Matches> fragments = ImmutableList.builder();
39-
for (String fragment : template.split("/")) {
40-
// Convert the fragment to a pattern by replacing "{...}" with a capturing group. We capture
41-
// from the opening '{' and do a non-greedy match of letters until the closing '}'.
42-
Matcher matcher = Pattern.compile("\\{(\\p{Alnum}+?)\\}").matcher(fragment);
43-
String toCompile = matcher.replaceAll("(?<$1>[^/]+)");
39+
// ^ start of string
40+
StringBuilder regex = new StringBuilder("^");
41+
Matcher groupNameMatcher = GROUP_NAME.matcher(template);
4442

45-
// There's no API for getting the names of capturing groups from a pattern in java. So we're
46-
// going to use a regex to find them. ffs.
47-
Matcher groupNameMatcher = GROUP_NAME.matcher(toCompile);
43+
ImmutableList.Builder<String> groups = ImmutableList.builder();
44+
int lastGroup = 0;
4845

49-
ImmutableList.Builder<String> names = ImmutableList.builder();
50-
while (groupNameMatcher.find()) {
51-
names.add(groupNameMatcher.group(1));
52-
}
46+
while (groupNameMatcher.find()) {
47+
int start = groupNameMatcher.start(1);
48+
int end = groupNameMatcher.end(1);
5349

54-
fragments.add(
55-
new Matches(Pattern.compile(Matcher.quoteReplacement(toCompile)), names.build()));
50+
// everything before the current group
51+
regex.append(Pattern.quote(template.substring(lastGroup, start)));
52+
// replace the group name with a capturing group
53+
regex.append("([^/]+)");
54+
// register the group name, to resolve into parameters
55+
groups.add(template.substring(start + 1, end - 1));
56+
lastGroup = end;
5657
}
57-
this.template = fragments.build();
58+
59+
if (template.length() > lastGroup) {
60+
// everything behind the last group
61+
regex.append(Pattern.quote(template.substring(lastGroup)));
62+
}
63+
64+
// $ end of string
65+
regex.append('$');
66+
67+
this.pattern = Pattern.compile(regex.toString());
68+
this.groups = groups.build();
5869
}
5970

6071
/**
@@ -65,21 +76,14 @@ public UrlTemplate.Match match(String matchAgainst) {
6576
return null;
6677
}
6778

68-
String[] fragments = matchAgainst.split("/");
69-
if (fragments.length != template.size()) {
79+
Matcher matcher = pattern.matcher(matchAgainst);
80+
if (!matcher.matches()) {
7081
return null;
7182
}
7283

7384
ImmutableMap.Builder<String, String> params = ImmutableMap.builder();
74-
for (int i = 0; i < fragments.length; i++) {
75-
Matcher matcher = template.get(i).matcher(fragments[i]);
76-
if (!matcher.find()) {
77-
return null;
78-
}
79-
80-
for (String name : template.get(i).groupNames) {
81-
params.put(name, matcher.group(name));
82-
}
85+
for (int i = 0; i < groups.size(); i++) {
86+
params.put(groups.get(i), matcher.group(i + 1));
8387
}
8488

8589
return new Match(matchAgainst, params.build());
@@ -103,19 +107,4 @@ public Map<String, String> getParameters() {
103107
return parameters;
104108
}
105109
}
106-
107-
private static class Matches {
108-
109-
private final Pattern pattern;
110-
private final List<String> groupNames;
111-
112-
private Matches(Pattern pattern, List<String> groupNames) {
113-
this.pattern = pattern;
114-
this.groupNames = groupNames;
115-
}
116-
117-
public Matcher matcher(String fragment) {
118-
return pattern.matcher(fragment);
119-
}
120-
}
121110
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,7 @@ void shouldBeAbleToRootASearchWithinAnElement() {
241241

242242
Node node = Mockito.mock(Node.class);
243243
when(node.executeWebDriverCommand(
244-
argThat(matchesUri("/session/{sessionId}/element/{elementId}/element"))))
244+
argThat(matchesUri("/session/{sessionId}/element/{elementId}/elements"))))
245245
.thenReturn(
246246
new HttpResponse()
247247
.addHeader("Content-Type", Json.JSON_UTF_8)

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,14 @@ void shouldExpandParameters() {
4949
assertThat(match.getParameters()).isEqualTo(ImmutableMap.of("veggie", "cake"));
5050
}
5151

52+
@Test
53+
void shouldExpandTwoParameters() {
54+
UrlTemplate.Match match = new UrlTemplate("/i/like/{flavor}/{veggie}").match("/i/like/sweet/cake");
55+
56+
assertThat(match.getUrl()).isEqualTo("/i/like/sweet/cake");
57+
assertThat(match.getParameters()).isEqualTo(ImmutableMap.of("flavor", "sweet", "veggie", "cake"));
58+
}
59+
5260
@Test
5361
void itIsFineForTheFirstCharacterToBeAPattern() {
5462
UrlTemplate.Match match = new UrlTemplate("{cake}/type").match("cheese/type");
@@ -61,4 +69,11 @@ void itIsFineForTheFirstCharacterToBeAPattern() {
6169
void aNullMatchDoesNotCauseANullPointerExceptionToBeThrown() {
6270
assertThat(new UrlTemplate("/").match(null)).isNull();
6371
}
72+
73+
@Test
74+
void noPartialMatches() {
75+
assertThat(new UrlTemplate("/session").match("/no-session")).isNull();
76+
assertThat(new UrlTemplate("/session").match("/session-no")).isNull();
77+
assertThat(new UrlTemplate("/session").match("/no-session-no")).isNull();
78+
}
6479
}

0 commit comments

Comments
 (0)