From 30bedeb7ee63aa7b4198cfe1af8884ea199879d0 Mon Sep 17 00:00:00 2001 From: marko-bekhta Date: Fri, 21 Jun 2024 12:52:54 +0200 Subject: [PATCH 1/2] Make sure that management router is reloaded --- .../management/LiveReloadManagementBean.java | 16 +++ .../LiveReloadManagementEndpoint.java | 35 ++++++ .../LiveReloadManagementHandler.java | 14 +++ .../LiveReloadManagementHandlerAsCDIBean.java | 17 +++ .../management/LiveReloadManagementTest.java | 107 ++++++++++++++++++ .../vertx/http/runtime/VertxHttpRecorder.java | 60 ++++++---- 6 files changed, 226 insertions(+), 23 deletions(-) create mode 100644 extensions/vertx-http/deployment/src/test/java/io/quarkus/vertx/http/devmode/management/LiveReloadManagementBean.java create mode 100644 extensions/vertx-http/deployment/src/test/java/io/quarkus/vertx/http/devmode/management/LiveReloadManagementEndpoint.java create mode 100644 extensions/vertx-http/deployment/src/test/java/io/quarkus/vertx/http/devmode/management/LiveReloadManagementHandler.java create mode 100644 extensions/vertx-http/deployment/src/test/java/io/quarkus/vertx/http/devmode/management/LiveReloadManagementHandlerAsCDIBean.java create mode 100644 extensions/vertx-http/deployment/src/test/java/io/quarkus/vertx/http/devmode/management/LiveReloadManagementTest.java diff --git a/extensions/vertx-http/deployment/src/test/java/io/quarkus/vertx/http/devmode/management/LiveReloadManagementBean.java b/extensions/vertx-http/deployment/src/test/java/io/quarkus/vertx/http/devmode/management/LiveReloadManagementBean.java new file mode 100644 index 0000000000000..e525c5ba566f1 --- /dev/null +++ b/extensions/vertx-http/deployment/src/test/java/io/quarkus/vertx/http/devmode/management/LiveReloadManagementBean.java @@ -0,0 +1,16 @@ +package io.quarkus.vertx.http.devmode.management; + +import jakarta.enterprise.context.ApplicationScoped; +import jakarta.inject.Named; + +@Named +@ApplicationScoped +public class LiveReloadManagementBean { + + private final String value = "string1"; + + String string() { + return value; + } + +} diff --git a/extensions/vertx-http/deployment/src/test/java/io/quarkus/vertx/http/devmode/management/LiveReloadManagementEndpoint.java b/extensions/vertx-http/deployment/src/test/java/io/quarkus/vertx/http/devmode/management/LiveReloadManagementEndpoint.java new file mode 100644 index 0000000000000..177b867abcbec --- /dev/null +++ b/extensions/vertx-http/deployment/src/test/java/io/quarkus/vertx/http/devmode/management/LiveReloadManagementEndpoint.java @@ -0,0 +1,35 @@ +package io.quarkus.vertx.http.devmode.management; + +import jakarta.enterprise.context.ApplicationScoped; +import jakarta.enterprise.event.Observes; + +import io.quarkus.vertx.http.ManagementInterface; +import io.vertx.ext.web.Router; +import io.vertx.ext.web.client.HttpRequest; +import io.vertx.ext.web.client.WebClient; + +// Case 3: import jakarta.inject.Inject; + +@ApplicationScoped +public class LiveReloadManagementEndpoint { + + // Case 3: @Inject + // Case 3: LiveReloadManagementHandlerAsCDIBean handler; + + void managementRoutes(@Observes ManagementInterface mi) { + Router router = mi.router(); + // Case 1: testing name change: + router.route("/manage-1") + .produces("text/plain") + .handler(rc -> rc.response().end(WebClient.class.hashCode() + "-" + HttpRequest.class.hashCode())); + + // Test that a new handler using some CDI bean can be loaded: + // Case 2: router.route("/manage-cdi") + // Case 2: .produces("text/plain") + // Case 2: .handler(new LiveReloadManagementHandler()); + + // Case 3: router.route("/manage-bean-handler") + // Case 3: .produces("text/plain") + // Case 3: .handler(handler); + } +} diff --git a/extensions/vertx-http/deployment/src/test/java/io/quarkus/vertx/http/devmode/management/LiveReloadManagementHandler.java b/extensions/vertx-http/deployment/src/test/java/io/quarkus/vertx/http/devmode/management/LiveReloadManagementHandler.java new file mode 100644 index 0000000000000..6716c606d3d10 --- /dev/null +++ b/extensions/vertx-http/deployment/src/test/java/io/quarkus/vertx/http/devmode/management/LiveReloadManagementHandler.java @@ -0,0 +1,14 @@ +package io.quarkus.vertx.http.devmode.management; + +import io.quarkus.arc.Arc; +import io.vertx.core.Handler; +import io.vertx.ext.web.RoutingContext; + +public class LiveReloadManagementHandler implements Handler { + + @Override + public void handle(RoutingContext event) { + LiveReloadManagementBean managementBean = Arc.container().instance(LiveReloadManagementBean.class).get(); + event.response().end(managementBean.string()); + } +} diff --git a/extensions/vertx-http/deployment/src/test/java/io/quarkus/vertx/http/devmode/management/LiveReloadManagementHandlerAsCDIBean.java b/extensions/vertx-http/deployment/src/test/java/io/quarkus/vertx/http/devmode/management/LiveReloadManagementHandlerAsCDIBean.java new file mode 100644 index 0000000000000..269ac6488045c --- /dev/null +++ b/extensions/vertx-http/deployment/src/test/java/io/quarkus/vertx/http/devmode/management/LiveReloadManagementHandlerAsCDIBean.java @@ -0,0 +1,17 @@ +package io.quarkus.vertx.http.devmode.management; + +import jakarta.enterprise.context.ApplicationScoped; +import jakarta.inject.Named; + +import io.vertx.core.Handler; +import io.vertx.ext.web.RoutingContext; + +@Named +@ApplicationScoped +public class LiveReloadManagementHandlerAsCDIBean implements Handler { + + @Override + public void handle(RoutingContext event) { + event.response().end("I'm a CDI bean handler."); + } +} diff --git a/extensions/vertx-http/deployment/src/test/java/io/quarkus/vertx/http/devmode/management/LiveReloadManagementTest.java b/extensions/vertx-http/deployment/src/test/java/io/quarkus/vertx/http/devmode/management/LiveReloadManagementTest.java new file mode 100644 index 0000000000000..472b8f9471a2a --- /dev/null +++ b/extensions/vertx-http/deployment/src/test/java/io/quarkus/vertx/http/devmode/management/LiveReloadManagementTest.java @@ -0,0 +1,107 @@ +package io.quarkus.vertx.http.devmode.management; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.util.concurrent.TimeUnit; + +import org.awaitility.Awaitility; +import org.jboss.shrinkwrap.api.asset.StringAsset; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.quarkus.test.QuarkusDevModeTest; +import io.restassured.RestAssured; + +public class LiveReloadManagementTest { + + private static final String APP_PROPS = """ + quarkus.class-loading.reloadable-artifacts=io.vertx:vertx-web-client + quarkus.management.enabled=true + """; + + @RegisterExtension + static final QuarkusDevModeTest test = new QuarkusDevModeTest() + .withApplicationRoot((jar) -> jar + .addAsResource(new StringAsset(APP_PROPS), "application.properties") + .addClasses(LiveReloadManagementEndpoint.class)); + + private final String managementPath = "http://localhost:9000"; + + @Test + public void test() { + // 1. We start with a single management endpoint /manage-1 + String firstClassToString = RestAssured.get(managementPath + "/manage-1") + .then() + .statusCode(200) + .extract().body().asString(); + // 2. We change the path of that endpoint, esentailly testing adding new and removing old one: + test.modifySourceFile(LiveReloadManagementEndpoint.class, s -> s.replace("\"/manage-1\"", "\"/manage-2\"")); + + // 2.1 Wait for the reload to be done, and first make sure that the "new" endpoint is there: + Awaitility.await() + .atMost(5, TimeUnit.SECONDS) + .untilAsserted(() -> { + String secondClassToString = RestAssured.get(managementPath + "/manage-2") + .then() + .statusCode(200) + .extract().body().asString(); + assertThat(firstClassToString).isNotEqualTo(secondClassToString); + }); + + // 2.2 Since the above check was a success, it means that the app was reloaded and we are sure that + // the /manage-1 is no longer available: + RestAssured.get(managementPath + "/manage-1") + .then() + .statusCode(404); + + // 3. Now we want to add another endpoint that attempts to access a CDI bean + // 4. First let's make sure that it is not there already: + RestAssured.get(managementPath + "/manage-cdi").then().statusCode(404); + + // 5. Add/update all necessary source files: + test.addSourceFile(LiveReloadManagementBean.class); + test.addSourceFile(LiveReloadManagementHandler.class); + test.modifySourceFile(LiveReloadManagementEndpoint.class, s -> s.replace("// Case 2: ", "")); + + // 6. Now wait for the app to get reloaded: + Awaitility.await() + .atMost(5, TimeUnit.SECONDS) + .untilAsserted(() -> { + String response = RestAssured.get(managementPath + "/manage-cdi") + .then() + .statusCode(200) + .extract().body().asString(); + assertThat(response).isEqualTo("string1"); + }); + + // 7. Update the bean used in the handler: + test.modifySourceFile(LiveReloadManagementBean.class, s -> s.replace("string1", "string2")); + + // 8. Wait for the app to get reloaded and management endpoint should be using the reloaded CDI bean: + Awaitility.await() + .atMost(5, TimeUnit.SECONDS) + .untilAsserted(() -> { + String response = RestAssured.get(managementPath + "/manage-cdi") + .then() + .statusCode(200) + .extract().body().asString(); + assertThat(response).isEqualTo("string2"); + }); + + // 9. Now let's add yet another handler, this time the handler is injected as a bean. (case 3) + test.addSourceFile(LiveReloadManagementHandlerAsCDIBean.class); + test.modifySourceFile(LiveReloadManagementEndpoint.class, s -> s.replace("// Case 3: ", "")); + + // 10. Now wait for the app to get reloaded: + Awaitility.await() + .atMost(5, TimeUnit.SECONDS) + .untilAsserted(() -> { + String response = RestAssured.get(managementPath + "/manage-bean-handler") + .then() + .statusCode(200) + .extract().body().asString(); + assertThat(response).isEqualTo("I'm a CDI bean handler."); + }); + } + +} diff --git a/extensions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/VertxHttpRecorder.java b/extensions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/VertxHttpRecorder.java index 5fb19dd2d92e2..fa466c4610f96 100644 --- a/extensions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/VertxHttpRecorder.java +++ b/extensions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/VertxHttpRecorder.java @@ -204,6 +204,7 @@ private boolean uriValid(HttpServerRequest httpServerRequest) { final RuntimeValue managementConfiguration; private static volatile Handler managementRouter; + private static volatile Handler managementRouterDelegate; public VertxHttpRecorder(HttpBuildTimeConfig httpBuildTimeConfig, ManagementInterfaceBuildTimeConfig managementBuildTimeConfig, @@ -437,34 +438,14 @@ public void handle(RoutingContext routingContext) { Handler root; if (rootPath.equals("/")) { - if (hotReplacementHandler != null) { - //recorders are always executed in the current CL - ClassLoader currentCl = Thread.currentThread().getContextClassLoader(); - httpRouteRouter.route().order(RouteConstants.ROUTE_ORDER_HOT_REPLACEMENT) - .handler(new Handler() { - @Override - public void handle(RoutingContext event) { - Thread.currentThread().setContextClassLoader(currentCl); - hotReplacementHandler.handle(event); - } - }); - } + addHotReplacementHandlerIfNeeded(httpRouteRouter); root = httpRouteRouter; } else { Router mainRouter = mainRouterRuntimeValue.isPresent() ? mainRouterRuntimeValue.get().getValue() : Router.router(vertx.get()); mainRouter.mountSubRouter(rootPath, httpRouteRouter); - if (hotReplacementHandler != null) { - ClassLoader currentCl = Thread.currentThread().getContextClassLoader(); - mainRouter.route().order(RouteConstants.ROUTE_ORDER_HOT_REPLACEMENT).handler(new Handler() { - @Override - public void handle(RoutingContext event) { - Thread.currentThread().setContextClassLoader(currentCl); - hotReplacementHandler.handle(event); - } - }); - } + addHotReplacementHandlerIfNeeded(mainRouter); root = mainRouter; } @@ -546,6 +527,8 @@ public void handle(RoutingContext event) { // Add body handler and cors handler var mr = managementRouter.getValue(); + addHotReplacementHandlerIfNeeded(mr); + mr.route().last().failureHandler( new QuarkusErrorHandler(launchMode.isDevOrTest(), httpConfiguration.unhandledErrorContentTypeDefault)); @@ -567,7 +550,24 @@ public void handle(RoutingContext event) { event.select(ManagementInterface.class).fire(new ManagementInterfaceImpl(managementRouter.getValue())); - VertxHttpRecorder.managementRouter = handler; + VertxHttpRecorder.managementRouterDelegate = handler; + if (VertxHttpRecorder.managementRouter == null) { + VertxHttpRecorder.managementRouter = new Handler() { + @Override + public void handle(HttpServerRequest event) { + VertxHttpRecorder.managementRouterDelegate.handle(event); + } + }; + } + } + } + + private void addHotReplacementHandlerIfNeeded(Router router) { + if (hotReplacementHandler != null) { + //recorders are always executed in the current CL + ClassLoader currentCl = Thread.currentThread().getContextClassLoader(); + router.route().order(RouteConstants.ROUTE_ORDER_HOT_REPLACEMENT) + .handler(new HotReplacementRoutingContextHandler(currentCl)); } } @@ -1567,4 +1567,18 @@ public boolean getAsBoolean() { return true; } } + + private static class HotReplacementRoutingContextHandler implements Handler { + private final ClassLoader currentCl; + + public HotReplacementRoutingContextHandler(ClassLoader currentCl) { + this.currentCl = currentCl; + } + + @Override + public void handle(RoutingContext event) { + Thread.currentThread().setContextClassLoader( currentCl ); + hotReplacementHandler.handle(event); + } + } } From a9f61352979af790a94020f9b4d2dd905cf43e35 Mon Sep 17 00:00:00 2001 From: marko-bekhta Date: Mon, 24 Jun 2024 19:20:49 +0200 Subject: [PATCH 2/2] Always produce management router ... and decide in the recorder whether we will actually keep that management router or not. --- .../http/deployment/VertxHttpProcessor.java | 8 ++--- .../vertx/http/runtime/VertxHttpRecorder.java | 32 ++++++++++++------- 2 files changed, 24 insertions(+), 16 deletions(-) diff --git a/extensions/vertx-http/deployment/src/main/java/io/quarkus/vertx/http/deployment/VertxHttpProcessor.java b/extensions/vertx-http/deployment/src/main/java/io/quarkus/vertx/http/deployment/VertxHttpProcessor.java index e8e11f917310a..335899e3fb120 100644 --- a/extensions/vertx-http/deployment/src/main/java/io/quarkus/vertx/http/deployment/VertxHttpProcessor.java +++ b/extensions/vertx-http/deployment/src/main/java/io/quarkus/vertx/http/deployment/VertxHttpProcessor.java @@ -265,16 +265,14 @@ VertxWebRouterBuildItem initializeRouter(VertxHttpRecorder recorder, List redirectRoutes = new ArrayList<>(); boolean frameworkRouterCreated = false; boolean mainRouterCreated = false; - boolean managementRouterCreated = false; boolean isManagementInterfaceEnabled = managementBuildTimeConfig.enabled; + if (isManagementInterfaceEnabled) { + managementRouter = recorder.initializeRouter(vertx.getVertx()); + } for (RouteBuildItem route : routes) { if (route.isManagement() && isManagementInterfaceEnabled) { - if (!managementRouterCreated) { - managementRouter = recorder.initializeRouter(vertx.getVertx()); - managementRouterCreated = true; - } recorder.addRoute(managementRouter, route.getRouteFunction(), route.getHandler(), route.getType()); } else if (nonApplicationRootPath.isDedicatedRouterRequired() && route.isRouterFramework()) { // Non-application endpoints on a separate path diff --git a/extensions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/VertxHttpRecorder.java b/extensions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/VertxHttpRecorder.java index fa466c4610f96..5529a6e3b95ee 100644 --- a/extensions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/VertxHttpRecorder.java +++ b/extensions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/VertxHttpRecorder.java @@ -526,6 +526,7 @@ public void handle(RoutingContext event) { if (managementRouter != null && managementRouter.getValue() != null) { // Add body handler and cors handler var mr = managementRouter.getValue(); + boolean hasManagementRoutes = !mr.getRoutes().isEmpty(); addHotReplacementHandlerIfNeeded(mr); @@ -548,16 +549,25 @@ public void handle(RoutingContext event) { Handler handler = HttpServerCommonHandlers.enforceDuplicatedContext(mr); handler = HttpServerCommonHandlers.applyProxy(managementConfiguration.getValue().proxy, handler, vertx); - event.select(ManagementInterface.class).fire(new ManagementInterfaceImpl(managementRouter.getValue())); - - VertxHttpRecorder.managementRouterDelegate = handler; - if (VertxHttpRecorder.managementRouter == null) { - VertxHttpRecorder.managementRouter = new Handler() { - @Override - public void handle(HttpServerRequest event) { - VertxHttpRecorder.managementRouterDelegate.handle(event); - } - }; + int routesBeforeMiEvent = mr.getRoutes().size(); + event.select(ManagementInterface.class).fire(new ManagementInterfaceImpl(mr)); + + // It may be that no build steps produced any management routes. + // But we still want to give a chance to the "ManagementInterface event" to collect any + // routes that users may have provided through observing this event. + // + // Hence, we only initialize the `managementRouter` router when we either had some routes from extensions (`hasManagementRoutes`) + // or if the event collected some routes (`routesBeforeMiEvent < routesAfterMiEvent`) + if (hasManagementRoutes || routesBeforeMiEvent < mr.getRoutes().size()) { + VertxHttpRecorder.managementRouterDelegate = handler; + if (VertxHttpRecorder.managementRouter == null) { + VertxHttpRecorder.managementRouter = new Handler() { + @Override + public void handle(HttpServerRequest event) { + VertxHttpRecorder.managementRouterDelegate.handle(event); + } + }; + } } } } @@ -1577,7 +1587,7 @@ public HotReplacementRoutingContextHandler(ClassLoader currentCl) { @Override public void handle(RoutingContext event) { - Thread.currentThread().setContextClassLoader( currentCl ); + Thread.currentThread().setContextClassLoader(currentCl); hotReplacementHandler.handle(event); } }