From 6a1da305ff109cdd31d147e7e10352fa07b06db3 Mon Sep 17 00:00:00 2001 From: francesco Date: Tue, 27 Jun 2017 00:34:39 +0200 Subject: [PATCH 1/2] Added multiple handlers support inside route object. --- .../java/io/vertx/ext/web/impl/RouteImpl.java | 49 ++++++---- .../ext/web/impl/RoutingContextImpl.java | 1 + .../ext/web/impl/RoutingContextImplBase.java | 25 ++++- .../java/io/vertx/ext/web/RouterTest.java | 95 ++++++++++++++++--- 4 files changed, 140 insertions(+), 30 deletions(-) diff --git a/vertx-web/src/main/java/io/vertx/ext/web/impl/RouteImpl.java b/vertx-web/src/main/java/io/vertx/ext/web/impl/RouteImpl.java index 0101b87997..2b4d43f8ad 100644 --- a/vertx-web/src/main/java/io/vertx/ext/web/impl/RouteImpl.java +++ b/vertx-web/src/main/java/io/vertx/ext/web/impl/RouteImpl.java @@ -49,8 +49,10 @@ public class RouteImpl implements Route { private String path; private int order; private boolean enabled = true; - private Handler contextHandler; - private Handler failureHandler; + private List> contextHandlers; + private int actualHandlerIndex; + private List> failureHandlers; + private int actualFailureHandlerIndex; private boolean added; private Pattern pattern; private List groups; @@ -59,6 +61,9 @@ public class RouteImpl implements Route { RouteImpl(RouterImpl router, int order) { this.router = router; this.order = order; + this.contextHandlers = new ArrayList<>(); + this.failureHandlers = new ArrayList<>(); + resetIndexes(); } RouteImpl(RouterImpl router, int order, HttpMethod method, String path) { @@ -134,10 +139,7 @@ public synchronized Route last() { @Override public synchronized Route handler(Handler contextHandler) { - if (this.contextHandler != null) { - throw new IllegalStateException("Setting handler for a route more than once!"); - } - this.contextHandler = contextHandler; + this.contextHandlers.add(contextHandler); checkAdd(); return this; } @@ -154,10 +156,7 @@ public synchronized Route blockingHandler(Handler contextHandler @Override public synchronized Route failureHandler(Handler exceptionHandler) { - if (this.failureHandler != null) { - throw new IllegalStateException("Setting failureHandler for a route more than once!"); - } - this.failureHandler = exceptionHandler; + this.failureHandlers.add(exceptionHandler); checkAdd(); return this; } @@ -196,8 +195,8 @@ public String toString() { StringBuilder sb = new StringBuilder("Route[ "); sb.append("path:").append(path); sb.append(" pattern:").append(pattern); - sb.append(" handler:").append(contextHandler); - sb.append(" failureHandler:").append(failureHandler); + sb.append(" handlers:").append(contextHandlers); + sb.append(" failureHandlers:").append(failureHandlers); sb.append(" order:").append(order); sb.append(" methods:["); int cnt = 0; @@ -213,20 +212,22 @@ public String toString() { } synchronized void handleContext(RoutingContext context) { - if (contextHandler != null) { - contextHandler.handle(context); + if (this.hasNextContextHandler()) { + actualHandlerIndex++; + contextHandlers.get(actualHandlerIndex - 1).handle(context); } } synchronized void handleFailure(RoutingContext context) { - if (failureHandler != null) { - failureHandler.handle(context); + if (this.hasNextFailureHandler()) { + actualFailureHandlerIndex++; + failureHandlers.get(actualFailureHandlerIndex - 1).handle(context); } } synchronized boolean matches(RoutingContext context, String mountPoint, boolean failure) { - if (failure && failureHandler == null || !failure && contextHandler == null) { + if (failure && !hasNextFailureHandler() || !failure && !hasNextContextHandler()) { return false; } if (!enabled) { @@ -416,4 +417,18 @@ private void checkAdd() { } } + synchronized protected boolean hasNextContextHandler() { + if (actualHandlerIndex < contextHandlers.size()) return true; + else return false; + } + + synchronized protected boolean hasNextFailureHandler() { + if (actualFailureHandlerIndex < failureHandlers.size()) return true; + else return false; + } + + synchronized protected void resetIndexes() { + actualFailureHandlerIndex = 0; + actualHandlerIndex = 0; + } } diff --git a/vertx-web/src/main/java/io/vertx/ext/web/impl/RoutingContextImpl.java b/vertx-web/src/main/java/io/vertx/ext/web/impl/RoutingContextImpl.java index 1bf239f88e..8f5926b8e5 100644 --- a/vertx-web/src/main/java/io/vertx/ext/web/impl/RoutingContextImpl.java +++ b/vertx-web/src/main/java/io/vertx/ext/web/impl/RoutingContextImpl.java @@ -417,6 +417,7 @@ private Set getFileUploads() { private void doFail() { this.iter = router.iterator(); + currentRoute = null; next(); } diff --git a/vertx-web/src/main/java/io/vertx/ext/web/impl/RoutingContextImplBase.java b/vertx-web/src/main/java/io/vertx/ext/web/impl/RoutingContextImplBase.java index 9adcf03160..9da232193f 100644 --- a/vertx-web/src/main/java/io/vertx/ext/web/impl/RoutingContextImplBase.java +++ b/vertx-web/src/main/java/io/vertx/ext/web/impl/RoutingContextImplBase.java @@ -65,8 +65,31 @@ protected void restart() { protected boolean iterateNext() { boolean failed = failed(); - while (iter.hasNext()) { + if (currentRoute != null) { // Handle multiple handlers inside route object + try { + if (!failed && currentRoute.hasNextContextHandler()) { + currentRoute.handleContext(this); + return true; + } else if (failed && currentRoute.hasNextFailureHandler()) { + currentRoute.handleFailure(this); + return true; + } + } catch (Throwable t) { + if (log.isTraceEnabled()) log.trace("Throwable thrown from handler", t); + if (!failed) { + if (log.isTraceEnabled()) log.trace("Failing the routing"); + fail(t); + } else { + // Failure in handling failure! + if (log.isTraceEnabled()) log.trace("Failure in handling failure"); + unhandledFailure(-1, t, currentRoute.router()); + } + return true; + } + } + while (iter.hasNext()) { // Search for more handlers RouteImpl route = iter.next(); + route.resetIndexes(); if (route.matches(this, mountPoint(), failed)) { if (log.isTraceEnabled()) log.trace("Route matches: " + route); try { diff --git a/vertx-web/src/test/java/io/vertx/ext/web/RouterTest.java b/vertx-web/src/test/java/io/vertx/ext/web/RouterTest.java index 573da418b2..8186e83673 100644 --- a/vertx-web/src/test/java/io/vertx/ext/web/RouterTest.java +++ b/vertx-web/src/test/java/io/vertx/ext/web/RouterTest.java @@ -19,6 +19,7 @@ import io.vertx.core.Handler; import io.vertx.core.MultiMap; import io.vertx.core.http.HttpMethod; +import io.vertx.core.http.HttpServerResponse; import org.junit.Test; import java.util.List; @@ -2002,21 +2003,91 @@ public void testGetWithPlusPath2() throws Exception { @Test public void testMultipleSetHandler() throws Exception { - try { - router.route().handler(context -> {}).handler(context -> {}); - fail(); - } catch (IllegalStateException e) { - // OK - } + router.get("/path").handler(routingContext -> { + routingContext.put("response", "handler1"); + routingContext.next(); + }).handler(routingContext -> { + routingContext.put("response", routingContext.get("response") + "handler2"); + routingContext.next(); + }).handler(routingContext -> { + HttpServerResponse response = routingContext.response(); + response.setChunked(true); + response.end(routingContext.get("response") + "handler3"); + }); + testRequest(HttpMethod.GET, "/path", 200, "OK", "handler1handler2handler3"); } @Test public void testMultipleSetFailureHandler() throws Exception { - try { - router.route().failureHandler(context -> {}).failureHandler(context -> {}); - fail(); - } catch (IllegalStateException e) { - // OK - } + router.get("/path").handler(routingContext -> { + routingContext.fail(500); + }).failureHandler(routingContext -> { + routingContext.put("response", "handler1"); + routingContext.next(); + }).failureHandler(routingContext -> { + routingContext.put("response", routingContext.get("response") + "handler2"); + routingContext.next(); + }).failureHandler(routingContext -> { + HttpServerResponse response = routingContext.response(); + response.setChunked(true); + response.setStatusMessage("ERROR"); + response.setStatusCode(500); + response.end(routingContext.get("response") + "handler3"); + }); + testRequest(HttpMethod.GET, "/path", 500, "ERROR", "handler1handler2handler3"); + } + + @Test + public void testMultipleSetFailureHandlerCorrectOrder() throws Exception { + router.route().failureHandler(routingContext -> { + routingContext.put("response", "handler1"); + routingContext.next(); + }); + + router.get("/path").handler(routingContext -> { + routingContext.fail(500); + }).failureHandler(routingContext -> { + routingContext.put("response", routingContext.get("response") + "handler2"); + routingContext.next(); + }).failureHandler(routingContext -> { + HttpServerResponse response = routingContext.response(); + response.setChunked(true); + response.setStatusMessage("ERROR"); + response.setStatusCode(500); + response.end(routingContext.get("response") + "handler3"); + }); + testRequest(HttpMethod.GET, "/path", 500, "ERROR", "handler1handler2handler3"); + } + + @Test + public void testMultipleHandlersMixed() throws Exception { + router.route().failureHandler(routingContext -> { + routingContext.put("response", "fhandler1"); + routingContext.next(); + }); + + router.get("/:param").handler(routingContext -> { + if (routingContext.pathParam("param").equals("fail")) routingContext.fail(500); + routingContext.put("response", "handler1"); + routingContext.next(); + }).handler(routingContext -> { + routingContext.put("response", routingContext.get("response") + "handler2"); + routingContext.next(); + }).handler(routingContext -> { + HttpServerResponse response = routingContext.response(); + response.setChunked(true); + response.end(routingContext.get("response") + "handler3"); + }).failureHandler(routingContext -> { + routingContext.put("response", routingContext.get("response") + "fhandler2"); + routingContext.next(); + }).failureHandler(routingContext -> { + HttpServerResponse response = routingContext.response(); + response.setChunked(true); + response.setStatusMessage("ERROR"); + response.setStatusCode(500); + response.end(routingContext.get("response") + "fhandler3"); + }); + testRequest(HttpMethod.GET, "/path", 200, "OK", "handler1handler2handler3"); + testRequest(HttpMethod.GET, "/fail", 500, "ERROR", "fhandler1fhandler2fhandler3"); } } From f45ca972a4a6ae2f81c4d5366c9570e4ba1022c4 Mon Sep 17 00:00:00 2001 From: francesco Date: Tue, 27 Jun 2017 00:44:30 +0200 Subject: [PATCH 2/2] Added another test for multiple handlers --- .../test/java/io/vertx/ext/web/RouterTest.java | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/vertx-web/src/test/java/io/vertx/ext/web/RouterTest.java b/vertx-web/src/test/java/io/vertx/ext/web/RouterTest.java index 8186e83673..a84f39aa95 100644 --- a/vertx-web/src/test/java/io/vertx/ext/web/RouterTest.java +++ b/vertx-web/src/test/java/io/vertx/ext/web/RouterTest.java @@ -2090,4 +2090,21 @@ public void testMultipleHandlersMixed() throws Exception { testRequest(HttpMethod.GET, "/path", 200, "OK", "handler1handler2handler3"); testRequest(HttpMethod.GET, "/fail", 500, "ERROR", "fhandler1fhandler2fhandler3"); } + + @Test + public void testMultipleSetHandlerMultipleRouteObject() throws Exception { + router.get("/path").handler(routingContext -> { + routingContext.put("response", "handler1"); + routingContext.next(); + }); + router.get("/path").handler(routingContext -> { + routingContext.put("response", routingContext.get("response") + "handler2"); + routingContext.next(); + }).handler(routingContext -> { + HttpServerResponse response = routingContext.response(); + response.setChunked(true); + response.end(routingContext.get("response") + "handler3"); + }); + testRequest(HttpMethod.GET, "/path", 200, "OK", "handler1handler2handler3"); + } }