[grid] flatten combined routes to improve routing (#13856)

Co-authored-by: Diego Molina <[email protected]>
SeleniumHQ · May 3, 2024 · 5fe3362 · 5fe3362
1 parent 17d927b
commit 5fe3362
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 5 deletions.
diff --git a/java/src/org/openqa/selenium/remote/http/Route.java b/java/src/org/openqa/selenium/remote/http/Route.java
@@ -27,8 +27,8 @@
 import static org.openqa.selenium.remote.http.HttpMethod.POST;
 import static org.openqa.selenium.remote.http.UrlPath.ROUTE_PREFIX_KEY;
 
+import java.util.ArrayList;
 import java.util.Collections;
-import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 import java.util.function.Function;
@@ -292,7 +292,7 @@ private HttpRequest transform(HttpRequest request) {
       // Don't forget to register our prefix
       Object rawPrefixes = request.getAttribute(ROUTE_PREFIX_KEY);
       if (!(rawPrefixes instanceof List)) {
-        rawPrefixes = new LinkedList<>();
+        rawPrefixes = Collections.emptyList();
       }
       List<String> prefixes =
           Stream.concat(((List<?>) rawPrefixes).stream(), Stream.of(prefix))
@@ -321,7 +321,21 @@ private static class CombinedRoute extends Route {
     private CombinedRoute(Stream<Routable> routes) {
       // We want later routes to have a greater chance of being called so that we can override
       // routes as necessary.
-      List<Routable> routables = routes.collect(Collectors.toList());
+      List<Routable> routables =
+          routes
+              .flatMap(
+                  route -> {
+                    // flatten a nested CombinedRoute
+                    if (route instanceof CombinedRoute) {
+                      List<Routable> nestedRoutes =
+                          new ArrayList<>(((CombinedRoute) route).allRoutes);
+                      // reverse to have the identical behaviour like not flattened
+                      Collections.reverse(nestedRoutes);
+                      return nestedRoutes.stream();
+                    }
+                    return Stream.of(route);
+                  })
+              .collect(Collectors.toList());
       Collections.reverse(routables);
       allRoutes = List.copyOf(routables);
       Require.stateCondition(!allRoutes.isEmpty(), "At least one route must be specified.");

diff --git a/java/test/org/openqa/selenium/remote/http/RouteTest.java b/java/test/org/openqa/selenium/remote/http/RouteTest.java
@@ -152,8 +152,11 @@ void laterRoutesOverrideEarlierRoutesToFacilitateOverridingRoutes() {
     HttpHandler handler =
         Route.combine(
             Route.get("/hello").to(() -> req -> new HttpResponse().setContent(utf8String("world"))),
-            Route.get("/hello")
-                .to(() -> req -> new HttpResponse().setContent(utf8String("buddy"))));
+            Route.combine(
+                Route.get("/hello")
+                    .to(() -> req -> new HttpResponse().setContent(utf8String("world"))),
+                Route.get("/hello")
+                    .to(() -> req -> new HttpResponse().setContent(utf8String("buddy")))));
 
     HttpResponse response = handler.execute(new HttpRequest(GET, "/hello"));
     assertThat(string(response)).isEqualTo("buddy");