From fb252dc70e40b09c9e4eed761c5365640e87c149 Mon Sep 17 00:00:00 2001 From: Max Schmitt Date: Thu, 19 Dec 2024 16:24:31 +0100 Subject: [PATCH 1/4] chore: refactor UrlMatcher --- .../microsoft/playwright/impl/UrlMatcher.java | 80 ++++++++++++------- .../com/microsoft/playwright/impl/Utils.java | 12 ++- 2 files changed, 56 insertions(+), 36 deletions(-) diff --git a/playwright/src/main/java/com/microsoft/playwright/impl/UrlMatcher.java b/playwright/src/main/java/com/microsoft/playwright/impl/UrlMatcher.java index 69153ce71..1e0998bf0 100644 --- a/playwright/src/main/java/com/microsoft/playwright/impl/UrlMatcher.java +++ b/playwright/src/main/java/com/microsoft/playwright/impl/UrlMatcher.java @@ -22,6 +22,8 @@ import java.net.MalformedURLException; import java.net.URL; +import java.util.ArrayList; +import java.util.List; import java.util.Objects; import java.util.function.Predicate; import java.util.regex.Pattern; @@ -30,20 +32,14 @@ import static com.microsoft.playwright.impl.Utils.toJsRegexFlags; class UrlMatcher { - final Object rawSource; - private final Predicate predicate; - - private static Predicate toPredicate(Pattern pattern) { - return s -> pattern.matcher(s).find(); - } - - static UrlMatcher any() { - return new UrlMatcher((Object) null, null); - } + private final URL baseURL; + public final String glob; + public final Pattern pattern; + public final Predicate predicate; static UrlMatcher forOneOf(URL baseUrl, Object object) { if (object == null) { - return UrlMatcher.any(); + return new UrlMatcher(null, null, null, null); } if (object instanceof String) { return new UrlMatcher(baseUrl, (String) object); @@ -68,24 +64,40 @@ static String resolveUrl(URL baseUrl, String spec) { } } - UrlMatcher(URL base, String url) { - this(url, toPredicate(Pattern.compile(globToRegex(resolveUrl(base, url)))).or(s -> url == null || url.equals(s))); + UrlMatcher(URL baseURL, String glob) { + this(baseURL, glob, null, null); } UrlMatcher(Pattern pattern) { - this(pattern, toPredicate(pattern)); + this(null, null, pattern, null); } + UrlMatcher(Predicate predicate) { - this(predicate, predicate); + this(null, null, null, predicate); } - private UrlMatcher(Object rawSource, Predicate predicate) { - this.rawSource = rawSource; + private UrlMatcher(URL baseURL, String glob, Pattern pattern, Predicate predicate) { + this.baseURL = baseURL; + this.glob = glob; + this.pattern = pattern; this.predicate = predicate; } boolean test(String value) { - return predicate == null || predicate.test(value); + return testImpl(baseURL, pattern, predicate, glob, value); + } + + private static boolean testImpl(URL baseURL, Pattern pattern, Predicate predicate, String glob, String value) { + if (pattern != null) { + return pattern.matcher(value).find(); + } + if (predicate != null) { + return predicate.test(value); + } + if (glob != null) { + return Pattern.compile(globToRegex(resolveUrl(baseURL, glob))).matcher(value).find(); + } + return true; } @Override @@ -93,25 +105,35 @@ public boolean equals(Object o) { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; UrlMatcher that = (UrlMatcher) o; - if (rawSource instanceof Pattern && that.rawSource instanceof Pattern) { - Pattern a = (Pattern) rawSource; - Pattern b = (Pattern) that.rawSource; - return a.pattern().equals(b.pattern()) && a.flags() == b.flags(); + if (pattern != null && !pattern.pattern().equals(that.pattern.pattern()) && pattern.flags() == that.pattern.flags()) { + return false; + } + if (predicate != null && !predicate.equals(that.predicate)) { + return false; } - return Objects.equals(rawSource, that.rawSource); + if (glob != null && !glob.equals(that.glob)) { + return false; + } + return true; } @Override public int hashCode() { - return Objects.hash(rawSource); + if (pattern != null) { + return pattern.hashCode(); + } + if (predicate != null) { + return predicate.hashCode(); + } + return glob.hashCode(); } @Override public String toString() { - if (rawSource == null) - return ""; - if (rawSource instanceof Predicate) - return "matching predicate"; - return rawSource.toString(); + if (pattern != null) + return String.format("", pattern.pattern(), toJsRegexFlags(pattern)); + if (this.predicate != null) + return ""; + return String.format("", glob); } } diff --git a/playwright/src/main/java/com/microsoft/playwright/impl/Utils.java b/playwright/src/main/java/com/microsoft/playwright/impl/Utils.java index 61df059ac..42ab44d3c 100644 --- a/playwright/src/main/java/com/microsoft/playwright/impl/Utils.java +++ b/playwright/src/main/java/com/microsoft/playwright/impl/Utils.java @@ -462,13 +462,11 @@ static JsonObject interceptionPatterns(List matchers) { JsonArray jsonPatterns = new JsonArray(); for (UrlMatcher matcher: matchers) { JsonObject jsonPattern = new JsonObject(); - Object urlFilter = matcher.rawSource; - if (urlFilter instanceof String) { - jsonPattern.addProperty("glob", (String) urlFilter); - } else if (urlFilter instanceof Pattern) { - Pattern pattern = (Pattern) urlFilter; - jsonPattern.addProperty("regexSource", pattern.pattern()); - jsonPattern.addProperty("regexFlags", toJsRegexFlags(pattern)); + if (matcher.glob != null) { + jsonPattern.addProperty("glob", matcher.glob); + } else if (matcher.pattern != null) { + jsonPattern.addProperty("regexSource", matcher.pattern.pattern()); + jsonPattern.addProperty("regexFlags", toJsRegexFlags(matcher.pattern)); } else { // Match all requests. jsonPattern.addProperty("glob", "**/*"); From 97def24da5c443fd8638607d973e27eea67ba81f Mon Sep 17 00:00:00 2001 From: Max Schmitt Date: Thu, 19 Dec 2024 23:13:54 +0100 Subject: [PATCH 2/4] fix NPE --- .../microsoft/playwright/impl/UrlMatcher.java | 21 +++++++++---------- .../microsoft/playwright/TestPageRoute.java | 12 +++++++++++ 2 files changed, 22 insertions(+), 11 deletions(-) diff --git a/playwright/src/main/java/com/microsoft/playwright/impl/UrlMatcher.java b/playwright/src/main/java/com/microsoft/playwright/impl/UrlMatcher.java index 1e0998bf0..3e823bea2 100644 --- a/playwright/src/main/java/com/microsoft/playwright/impl/UrlMatcher.java +++ b/playwright/src/main/java/com/microsoft/playwright/impl/UrlMatcher.java @@ -84,10 +84,6 @@ private UrlMatcher(URL baseURL, String glob, Pattern pattern, Predicate } boolean test(String value) { - return testImpl(baseURL, pattern, predicate, glob, value); - } - - private static boolean testImpl(URL baseURL, Pattern pattern, Predicate predicate, String glob, String value) { if (pattern != null) { return pattern.matcher(value).find(); } @@ -105,14 +101,14 @@ public boolean equals(Object o) { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; UrlMatcher that = (UrlMatcher) o; - if (pattern != null && !pattern.pattern().equals(that.pattern.pattern()) && pattern.flags() == that.pattern.flags()) { - return false; + if (pattern != null) { + return that.pattern != null && pattern.pattern().equals(that.pattern.pattern()) && pattern.flags() == that.pattern.flags(); } - if (predicate != null && !predicate.equals(that.predicate)) { - return false; + if (predicate != null) { + return predicate.equals(that.predicate); } - if (glob != null && !glob.equals(that.glob)) { - return false; + if (glob != null) { + return glob.equals(that.glob); } return true; } @@ -125,7 +121,10 @@ public int hashCode() { if (predicate != null) { return predicate.hashCode(); } - return glob.hashCode(); + if (glob != null) { + return glob.hashCode(); + } + return super.hashCode(); } @Override diff --git a/playwright/src/test/java/com/microsoft/playwright/TestPageRoute.java b/playwright/src/test/java/com/microsoft/playwright/TestPageRoute.java index adbdbd8d8..6812c7d4d 100644 --- a/playwright/src/test/java/com/microsoft/playwright/TestPageRoute.java +++ b/playwright/src/test/java/com/microsoft/playwright/TestPageRoute.java @@ -97,6 +97,18 @@ void shouldUnroute() { assertEquals(asList(1), intercepted); } + @Test + void shouldUnrouteNonExistentPatternHandler() { + List intercepted = new ArrayList<>(); + page.route(Pattern.compile("empty.html"), route -> { + intercepted.add(1); + route.fallback(); + }); + page.unroute("**/*"); + page.navigate(server.EMPTY_PAGE); + assertEquals(asList( 1), intercepted); + } + @Test void shouldSupportQuestionMarkInGlobPattern() { server.setRoute("/index", exchange -> { From 06b133663041d379db0221d49a1e14d7192afce4 Mon Sep 17 00:00:00 2001 From: Max Schmitt Date: Thu, 19 Dec 2024 23:35:02 +0100 Subject: [PATCH 3/4] nit --- .../src/main/java/com/microsoft/playwright/impl/UrlMatcher.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/playwright/src/main/java/com/microsoft/playwright/impl/UrlMatcher.java b/playwright/src/main/java/com/microsoft/playwright/impl/UrlMatcher.java index 3e823bea2..fe870187e 100644 --- a/playwright/src/main/java/com/microsoft/playwright/impl/UrlMatcher.java +++ b/playwright/src/main/java/com/microsoft/playwright/impl/UrlMatcher.java @@ -124,7 +124,7 @@ public int hashCode() { if (glob != null) { return glob.hashCode(); } - return super.hashCode(); + return 0; } @Override From 16c60f6d55261db6318c87d6df9aa2deae4934f5 Mon Sep 17 00:00:00 2001 From: Max Schmitt Date: Fri, 27 Dec 2024 18:21:21 +0100 Subject: [PATCH 4/4] apply suggestion --- .../src/main/java/com/microsoft/playwright/impl/UrlMatcher.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/playwright/src/main/java/com/microsoft/playwright/impl/UrlMatcher.java b/playwright/src/main/java/com/microsoft/playwright/impl/UrlMatcher.java index fe870187e..968699012 100644 --- a/playwright/src/main/java/com/microsoft/playwright/impl/UrlMatcher.java +++ b/playwright/src/main/java/com/microsoft/playwright/impl/UrlMatcher.java @@ -110,7 +110,7 @@ public boolean equals(Object o) { if (glob != null) { return glob.equals(that.glob); } - return true; + return that.pattern == null && that.predicate == null && that.glob == null; } @Override