Merge pull request #41353 from marko-bekhta/fix/i41315-management-han…

…dler-reload

Make sure that management router is reloaded

extensions/vertx-http/deployment/src/main/java/io/quarkus/vertx/http/deployment/VertxHttpProcessor.java

@@ -265,16 +265,14 @@ VertxWebRouterBuildItem initializeRouter(VertxHttpRecorder recorder,
         List<RouteBuildItem> 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

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;
+    }
+
+}

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);
+    }
+}

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<RoutingContext> {
+
+    @Override
+    public void handle(RoutingContext event) {
+        LiveReloadManagementBean managementBean = Arc.container().instance(LiveReloadManagementBean.class).get();
+        event.response().end(managementBean.string());
+    }
+}

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<RoutingContext> {
+
+    @Override
+    public void handle(RoutingContext event) {
+        event.response().end("I'm a CDI bean handler.");
+    }
+}

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.");
+                });
+    }
+
+}

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<ManagementInterfaceConfiguration> managementConfiguration;
     private static volatile Handler<HttpServerRequest> managementRouter;
+    private static volatile Handler<HttpServerRequest> managementRouterDelegate;
 
     public VertxHttpRecorder(HttpBuildTimeConfig httpBuildTimeConfig,
             ManagementInterfaceBuildTimeConfig managementBuildTimeConfig,
@@ -437,34 +438,14 @@ public void handle(RoutingContext routingContext) {
 
         Handler<HttpServerRequest> 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<RoutingContext>() {
-                            @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<RoutingContext>() {
-                    @Override
-                    public void handle(RoutingContext event) {
-                        Thread.currentThread().setContextClassLoader(currentCl);
-                        hotReplacementHandler.handle(event);
-                    }
-                });
-            }
+            addHotReplacementHandlerIfNeeded(mainRouter);
             root = mainRouter;
         }
 
@@ -545,6 +526,9 @@ 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);
 
             mr.route().last().failureHandler(
                     new QuarkusErrorHandler(launchMode.isDevOrTest(), httpConfiguration.unhandledErrorContentTypeDefault));
@@ -565,9 +549,35 @@ public void handle(RoutingContext event) {
             Handler<HttpServerRequest> handler = HttpServerCommonHandlers.enforceDuplicatedContext(mr);
             handler = HttpServerCommonHandlers.applyProxy(managementConfiguration.getValue().proxy, handler, vertx);
 
-            event.select(ManagementInterface.class).fire(new ManagementInterfaceImpl(managementRouter.getValue()));
+            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<HttpServerRequest>() {
+                        @Override
+                        public void handle(HttpServerRequest event) {
+                            VertxHttpRecorder.managementRouterDelegate.handle(event);
+                        }
+                    };
+                }
+            }
+        }
+    }
 
-            VertxHttpRecorder.managementRouter = handler;
+    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 +1577,18 @@ public boolean getAsBoolean() {
             return true;
         }
     }
+
+    private static class HotReplacementRoutingContextHandler implements Handler<RoutingContext> {
+        private final ClassLoader currentCl;
+
+        public HotReplacementRoutingContextHandler(ClassLoader currentCl) {
+            this.currentCl = currentCl;
+        }
+
+        @Override
+        public void handle(RoutingContext event) {
+            Thread.currentThread().setContextClassLoader(currentCl);
+            hotReplacementHandler.handle(event);
+        }
+    }
 }