router: Add option to detect or prevent duplicate route registrations

- RouterOptions.failOnDuplicateRoutes
- fix #3741

Author: jknack

jooby/src/main/java/io/jooby/Route.java

@@ -15,6 +15,7 @@
 import java.util.concurrent.Executor;
 import java.util.function.Consumer;
 import java.util.function.Predicate;
+import java.util.stream.Stream;
 
 import edu.umd.cs.findbugs.annotations.NonNull;
 import edu.umd.cs.findbugs.annotations.Nullable;
@@ -24,6 +25,7 @@
 import io.jooby.exception.NotFoundException;
 import io.jooby.exception.StatusCodeException;
 import io.jooby.exception.UnsupportedMediaType;
+import io.jooby.internal.RouterImpl;
 
 /**
  * Route contains information about the HTTP method, path pattern, which content types consumes and
@@ -317,6 +319,16 @@ public interface Handler extends Serializable, Aware {
     }
   }
 
+  /**
+   * Route location.
+   *
+   * @param filename File name.
+   * @param line Line.
+   */
+  public record Location(String filename, int line) {}
+
+  private static final Location NO_LOCATION = new Location("<<Unknown>>", -1);
+
   /** Handler for {@link StatusCode#NOT_FOUND} responses. */
   public static final Handler NOT_FOUND =
       ctx -> ctx.sendError(new NotFoundException(ctx.getRequestPath()));
@@ -466,6 +478,8 @@ public MethodHandle toMethodHandle() {
 
   private boolean httpHead;
 
+  private final Location location;
+
   /**
    * Creates a new route.
    *
@@ -477,6 +491,33 @@ public Route(@NonNull String method, @NonNull String pattern, @NonNull Handler h
     this.method = method.toUpperCase();
     this.pattern = pattern;
     this.handler = handler;
+    this.location =
+        StackWalker.getInstance(StackWalker.Option.RETAIN_CLASS_REFERENCE)
+            .walk(
+                frame ->
+                    frame
+                        .dropWhile(
+                            f ->
+                                Stream.of(
+                                        Route.class.getName(),
+                                        RouterImpl.class.getName(),
+                                        Router.class.getName(),
+                                        Jooby.class.getName(),
+                                        "io.jooby.kt.Kooby",
+                                        "io.jooby.kt.CoroutineRouter")
+                                    .anyMatch(it -> it.equals(f.getDeclaringClass().getName())))
+                        .findFirst()
+                        .map(it -> new Location(it.getFileName(), it.getLineNumber()))
+                        .orElse(NO_LOCATION));
+  }
+
+  /**
+   * Where the route was defined.
+   *
+   * @return Where the route was defined.
+   */
+  public Location getLocation() {
+    return location;
   }
 
   /**

jooby/src/main/java/io/jooby/RouterOptions.java

@@ -25,6 +25,12 @@
  * @since 2.4.0
  */
 public class RouterOptions {
+  /**
+   * Detect or prevent duplicate route registrations. This option must be set before creating
+   * routes.
+   */
+  private boolean failOnDuplicateRoutes = false;
+
   /**
    * Indicates whenever routing algorithm does case-sensitive matching on an incoming request path.
    * Default is <code>case sensitive</code>.
@@ -81,6 +87,27 @@ public RouterOptions setIgnoreCase(boolean ignoreCase) {
     return this;
   }
 
+  /**
+   * Detect or prevent duplicate route registrations. This option must be set before creating
+   * routes.
+   *
+   * @return Detect or prevent duplicate route registrations.
+   */
+  public boolean isFailOnDuplicateRoutes() {
+    return failOnDuplicateRoutes;
+  }
+
+  /**
+   * Detect or prevent duplicate route registrations.
+   *
+   * @param failOnDuplicateRoutes True for detect or prevent duplicate route registrations.
+   * @return This options.
+   */
+  public RouterOptions setFailOnDuplicateRoutes(boolean failOnDuplicateRoutes) {
+    this.failOnDuplicateRoutes = failOnDuplicateRoutes;
+    return this;
+  }
+
   /**
    * Indicates whenever routing algorithm does case-sensitive matching on an incoming request path.
    * Default is <code>false (case sensitive)</code>.

jooby/src/main/java/io/jooby/internal/Chi.java

@@ -674,7 +674,7 @@ public String toString() {
       return node.toString();
     }
 
-    Node insertRoute(String method, String pattern, Route route) {
+    Node insertRoute(String method, String pattern, Route route, boolean failOnDuplicateRoutes) {
       Node n = this;
       Node parent;
       String search = pattern;
@@ -683,7 +683,7 @@ Node insertRoute(String method, String pattern, Route route) {
         // Handle key exhaustion
         if (search.isEmpty()) {
           // Insert or update the node's leaf handler
-          n.setEndpoint(method, route);
+          n.setEndpoint(method, route, failOnDuplicateRoutes);
           return n;
         }
 
@@ -716,7 +716,7 @@ Node insertRoute(String method, String pattern, Route route) {
         if (n == null) {
           Node child = new Node().label(label).tail(seg.tail).prefix(search);
           Node hn = parent.addChild(child, search);
-          hn.setEndpoint(method, route);
+          hn.setEndpoint(method, route, failOnDuplicateRoutes);
           return hn;
         }
 
@@ -752,14 +752,14 @@ Node insertRoute(String method, String pattern, Route route) {
         // If the new key is a subset, set the method/handler on this node and finish.
         search = search.substring(commonPrefix);
         if (search.isEmpty()) {
-          child.setEndpoint(method, route);
+          child.setEndpoint(method, route, failOnDuplicateRoutes);
           return child;
         }
 
         // Create a new edge for the node
         Node subchild = new Node().typ(ntStatic).label(search.charAt(0)).prefix(search);
         Node hn = child.addChild(subchild, search);
-        hn.setEndpoint(method, route);
+        hn.setEndpoint(method, route, failOnDuplicateRoutes);
         return hn;
       }
     }
@@ -871,7 +871,7 @@ Node getEdge(int ntyp, char label, char tail, String prefix) {
       return null;
     }
 
-    void setEndpoint(String method, Route route) {
+    void setEndpoint(String method, Route route, boolean failOnDuplicateRoutes) {
       Node n = this;
       // Set the handler for the method type on the node
       if (n.endpoints == null) {
@@ -893,6 +893,20 @@ void setEndpoint(String method, Route route) {
       //          h.paramKeys = paramKeys;
       //        }
       //      } else {
+      if (failOnDuplicateRoutes) {
+        var existing = n.endpoints.get(method);
+        if (existing != null) {
+          throw new IllegalArgumentException(
+              "Route already exists: "
+                  + method
+                  + " "
+                  + existing.getPattern()
+                  + " at "
+                  + existing.getLocation().filename()
+                  + ":"
+                  + existing.getLocation().line());
+        }
+      }
       n.endpoints.put(method, route);
       //        Endpoint h = n.endpoints.computeIfAbsent(method, k -> new Endpoint(handler));
       //        h.handler = handler;
@@ -1193,6 +1207,12 @@ public void destroy() {
 
   private StaticMap staticPaths = StaticMap.INIT;
 
+  boolean failOnDuplicateRoutes;
+
+  public Chi(boolean failOnDuplicateRoutes) {
+    this.failOnDuplicateRoutes = failOnDuplicateRoutes;
+  }
+
   public void insert(String method, String pattern, Route route) {
     String baseCatchAll = baseCatchAll(pattern);
     if (baseCatchAll.length() > 1) {
@@ -1209,7 +1229,7 @@ public void insert(String method, String pattern, Route route) {
       staticPaths = staticPaths.put(pattern, staticRoute);
       staticRoute.put(method, route);
     }
-    root.insertRoute(method, pattern, route);
+    root.insertRoute(method, pattern, route, failOnDuplicateRoutes);
   }
 
   private String baseCatchAll(String pattern) {

jooby/src/main/java/io/jooby/internal/RouterImpl.java

@@ -126,7 +126,7 @@ public Stack executor(Executor executor) {
 
   private Map<String, StatusCode> errorCodes;
 
-  private RouteTree chi = new Chi();
+  private RouteTree chi = new Chi(false);
 
   private LinkedList<Stack> stack = new LinkedList<>();
 
@@ -200,6 +200,7 @@ public RouterOptions getRouterOptions() {
   @NonNull @Override
   public Router setRouterOptions(@NonNull RouterOptions options) {
     this.routerOptions = options;
+    ((Chi) chi).failOnDuplicateRoutes = options.isFailOnDuplicateRoutes();
     return this;
   }
 
@@ -257,7 +258,7 @@ public Route.Set domain(@NonNull String domain, @NonNull Router subrouter) {
 
   @NonNull @Override
   public Route.Set mount(@NonNull Predicate<Context> predicate, @NonNull Runnable body) {
-    var tree = new Chi();
+    var tree = new Chi(routerOptions.isFailOnDuplicateRoutes());
     putPredicate(predicate, tree);
     int start = this.routes.size();
     newStack(tree, "/", body);
@@ -270,7 +271,7 @@ public Router install(
       @NonNull SneakyThrows.Supplier<Jooby> factory) {
     var existingRouter = this.chi;
     try {
-      var tree = new Chi();
+      var tree = new Chi(routerOptions.isFailOnDuplicateRoutes());
       this.chi = tree;
       putPredicate(predicate, tree);
       path(path, factory::get);
@@ -693,7 +694,7 @@ public Match match(@NonNull Context ctx) {
 
   @Override
   public boolean match(@NonNull String pattern, @NonNull String path) {
-    Chi chi = new Chi();
+    Chi chi = new Chi(false);
     chi.insert(Router.GET, pattern, ROUTE_MARK);
     return chi.exists(Router.GET, path);
   }

jooby/src/test/java/io/jooby/Issue3741.java

@@ -0,0 +1,53 @@
+/*
+ * Jooby https://jooby.io
+ * Apache License Version 2.0 https://jooby.io/LICENSE.txt
+ * Copyright 2014 Edgar Espina
+ */
+package io.jooby;
+
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import org.junit.jupiter.api.Test;
+
+public class Issue3741 {
+  public static class RouterBase extends Jooby {
+    {
+      get("/3741/{id}", ctx -> "Path Var");
+
+      get("/3741/static", ctx -> "Static");
+    }
+  }
+
+  @Test
+  public void shouldDetectDuplicatedRoutes() {
+    var main = new Jooby();
+    main.setRouterOptions(new RouterOptions().setFailOnDuplicateRoutes(true));
+    main.mount(new RouterBase());
+    var cause =
+        assertThrows(
+            IllegalArgumentException.class, () -> main.get("/3741/{id}", ctx -> "Something"));
+    assertTrue(cause.getMessage().startsWith("Route already exists"));
+
+    cause =
+        assertThrows(
+            IllegalArgumentException.class, () -> main.get("/3741/static", ctx -> "Static"));
+    assertTrue(cause.getMessage().startsWith("Route already exists"));
+  }
+
+  @Test
+  public void shouldDetectDuplicatedRoutesOnPaths() {
+    var main = new Jooby();
+    main.setRouterOptions(new RouterOptions().setFailOnDuplicateRoutes(true));
+    main.mount("/path", new RouterBase());
+    var cause =
+        assertThrows(
+            IllegalArgumentException.class, () -> main.get("/path/3741/{id}", ctx -> "Something"));
+    assertTrue(cause.getMessage().startsWith("Route already exists"));
+
+    cause =
+        assertThrows(
+            IllegalArgumentException.class, () -> main.get("/path/3741/static", ctx -> "Static"));
+    assertTrue(cause.getMessage().startsWith("Route already exists"));
+  }
+}

jooby/src/test/java/io/jooby/internal/ChiTest.java

@@ -22,7 +22,7 @@
 public class ChiTest {
   @Test
   public void routeOverride() {
-    Chi router = new Chi();
+    Chi router = new Chi(false);
     Route foo = route("GET", "/abcd", stringHandler("foo"));
     Route bar = route("GET", "/abcd", stringHandler("bar"));
     router.insert(foo);
@@ -35,7 +35,7 @@ public void routeOverride() {
 
   @Test
   public void staticMap6() {
-    Chi router = new Chi();
+    Chi router = new Chi(false);
     router.insert(route("GET", "/1", stringHandler("1")));
     router.insert(route("GET", "/2", stringHandler("2")));
     router.insert(route("GET", "/3", stringHandler("3")));
@@ -49,7 +49,7 @@ public void staticMap6() {
 
   @Test
   public void routeCase() {
-    Chi router = new Chi();
+    Chi router = new Chi(false);
     Route foo = route("GET", "/abcd", stringHandler("foo"));
     Route foos = route("GET", "/abcd/", stringHandler("foo/"));
     router.insert(foo);
@@ -62,7 +62,7 @@ public void routeCase() {
 
   @Test
   public void wildOnRoot() throws Exception {
-    Chi router = new Chi();
+    Chi router = new Chi(false);
 
     router.insert(route("GET", "/foo/?*", stringHandler("foo")));
     router.insert(route("GET", "/bar/*", stringHandler("bar")));
@@ -127,7 +127,7 @@ public void wildOnRoot() throws Exception {
 
   @Test
   public void searchString() throws Exception {
-    Chi router = new Chi();
+    Chi router = new Chi(false);
 
     router.insert(route("GET", "/regex/{nid:[0-9]+}", stringHandler("nid")));
     router.insert(route("GET", "/regex/{zid:[0-9]+}/edit", stringHandler("zid")));
@@ -161,7 +161,7 @@ public void searchString() throws Exception {
 
   @Test
   public void searchParam() throws Exception {
-    Chi router = new Chi();
+    Chi router = new Chi(false);
 
     router.insert(route("GET", "/catchallWithVarPrefix/{id}/*path", stringHandler("path")));
 
@@ -195,7 +195,7 @@ public void searchParam() throws Exception {
 
   @Test
   public void multipleRegex() {
-    Chi router = new Chi();
+    Chi router = new Chi(false);
 
     router.insert(route("GET", "/{lang:[a-z][a-z]}/{page:[^.]+}/", stringHandler("1515")));
 
@@ -226,7 +226,7 @@ public void multipleRegex() {
 
   @Test
   public void regexWithQuantity() {
-    Chi router = new Chi();
+    Chi router = new Chi(false);
 
     router.insert(route("GET", "/{lang:[a-z]{2}}/", stringHandler("qx")));
 

jooby/src/test/java/io/jooby/internal/Issue1822.java

@@ -19,7 +19,7 @@ public class Issue1822 {
   public void routerExistsShouldNotReturnsFalsePositives() {
     String pattern = "/api/*";
     String path = "/";
-    Chi router = new Chi();
+    Chi router = new Chi(false);
     router.insert(route("GET", pattern, stringHandler("api")));
     assertFalse(router.exists("GET", path));
     assertTrue(router.exists("GET", "/api/v1"));