Merge pull request #35399 from mkouba/issue-35381

Dev mode: fix several problems when the initial build fails
quarkusio · Aug 19, 2023 · 01902ac · 01902ac
2 parents bb28c1a + 725dc67
commit 01902ac
Show file tree

Hide file tree

Showing 8 changed files with 148 additions and 17 deletions.
diff --git a/core/deployment/src/main/java/io/quarkus/deployment/dev/DevModeListener.java b/core/deployment/src/main/java/io/quarkus/deployment/dev/DevModeListener.java
@@ -9,6 +9,11 @@ public interface DevModeListener {
 
     int DEFAULT_ORDER = 0;
 
+    /**
+     * Executed after the first successfull application start.
+     *
+     * @param application
+     */
     void afterFirstStart(RunningQuarkusApplication application);
 
     void beforeShutdown();

diff --git a/core/deployment/src/main/java/io/quarkus/deployment/dev/IsolatedDevModeMain.java b/core/deployment/src/main/java/io/quarkus/deployment/dev/IsolatedDevModeMain.java
@@ -43,6 +43,7 @@
 import io.quarkus.deployment.dev.testing.TestSupport;
 import io.quarkus.deployment.steps.ClassTransformingBuildStep;
 import io.quarkus.dev.appstate.ApplicationStartException;
+import io.quarkus.dev.appstate.ApplicationStateNotification;
 import io.quarkus.dev.console.DevConsoleManager;
 import io.quarkus.dev.spi.DeploymentFailedStartHandler;
 import io.quarkus.dev.spi.DevModeType;
@@ -79,7 +80,6 @@ private synchronized void firstStart() {
 
             //ok, we have resolved all the deps
             try {
-                StartupAction start = augmentAction.createInitialRuntimeApplication();
                 //this is a bit yuck, but we need replace the default
                 //exit handler in the runtime class loader
                 //TODO: look at implementing a common core classloader, that removes the need for this sort of crappy hack
@@ -110,23 +110,13 @@ public void accept(Integer integer) {
                             }
                         });
 
+                StartupAction start = augmentAction.createInitialRuntimeApplication();
+
                 runner = start.runMainClass(context.getArgs());
                 RuntimeUpdatesProcessor.INSTANCE.setConfiguredInstrumentationEnabled(
                         runner.getConfigValue("quarkus.live-reload.instrumentation", Boolean.class).orElse(false));
                 firstStartCompleted = true;
-
-                for (DevModeListener listener : ServiceLoader.load(DevModeListener.class)) {
-                    listeners.add(listener);
-                }
-                listeners.sort(Comparator.comparingInt(DevModeListener::order));
-
-                for (DevModeListener listener : listeners) {
-                    try {
-                        listener.afterFirstStart(runner);
-                    } catch (Exception e) {
-                        log.warn("Unable to invoke 'afterFirstStart' of " + listener.getClass(), e);
-                    }
-                }
+                notifyListenersAfterStart();
 
             } catch (Throwable t) {
                 Throwable rootCause = t;
@@ -140,6 +130,11 @@ public void accept(Integer integer) {
                         //this is so the config is still valid, and we can read HTTP config from application.properties
                         log.info(
                                 "Attempting to start live reload endpoint to recover from previous Quarkus startup failure");
+
+                        // Make sure to change the application state so that QuarkusDevModeTest does not hang when
+                        // allowFailedStart=true and the build fails when the dev mode starts initially
+                        ApplicationStateNotification.notifyStartupFailed(t);
+
                         if (RuntimeUpdatesProcessor.INSTANCE != null) {
                             Thread.currentThread().setContextClassLoader(curatedApplication.getBaseRuntimeClassLoader());
                             try {
@@ -196,7 +191,10 @@ public synchronized void restartApp(Set<String> changedResources, ClassChangeInf
                 StartupAction start = augmentAction.reloadExistingApplication(firstStartCompleted, changedResources,
                         classChangeInformation);
                 runner = start.runMainClass(context.getArgs());
-                firstStartCompleted = true;
+                if (!firstStartCompleted) {
+                    notifyListenersAfterStart();
+                    firstStartCompleted = true;
+                }
             } catch (Throwable t) {
                 deploymentProblem = t;
                 Throwable rootCause = t;
@@ -215,6 +213,21 @@ public synchronized void restartApp(Set<String> changedResources, ClassChangeInf
         }
     }
 
+    private void notifyListenersAfterStart() {
+        for (DevModeListener listener : ServiceLoader.load(DevModeListener.class)) {
+            listeners.add(listener);
+        }
+        listeners.sort(Comparator.comparingInt(DevModeListener::order));
+
+        for (DevModeListener listener : listeners) {
+            try {
+                listener.afterFirstStart(runner);
+            } catch (Exception e) {
+                log.warn("Unable to invoke 'afterFirstStart' of " + listener.getClass(), e);
+            }
+        }
+    }
+
     private RuntimeUpdatesProcessor setupRuntimeCompilation(DevModeContext context, Path appRoot, DevModeType devModeType)
             throws Exception {
         if (!context.getAllModules().isEmpty()) {

diff --git a/extensions/arc/deployment/src/main/java/io/quarkus/arc/deployment/ArcProcessor.java b/extensions/arc/deployment/src/main/java/io/quarkus/arc/deployment/ArcProcessor.java
@@ -501,7 +501,8 @@ public PreBeanContainerBuildItem generateResources(ArcConfig config, ArcRecorder
         BeanProcessor beanProcessor = validationPhase.getBeanProcessor();
         beanProcessor.processValidationErrors(validationPhase.getContext());
         ExistingClasses existingClasses = liveReloadBuildItem.getContextObject(ExistingClasses.class);
-        if (existingClasses == null) {
+        if (existingClasses == null || !liveReloadBuildItem.isLiveReload()) {
+            // Reset the data if there is no context object or if the first start was unsuccessful
             existingClasses = new ExistingClasses();
             liveReloadBuildItem.setContextObject(ExistingClasses.class, existingClasses);
         }

diff --git a/extensions/qute/deployment/src/main/java/io/quarkus/qute/deployment/QuteProcessor.java b/extensions/qute/deployment/src/main/java/io/quarkus/qute/deployment/QuteProcessor.java
@@ -1889,7 +1889,8 @@ public String apply(String name) {
         // NOTE: We can't use this optimization for classes generated by ValueResolverGenerator because we cannot easily
         // map a target class to a specific set of generated classes
         ExistingValueResolvers existingValueResolvers = liveReloadBuildItem.getContextObject(ExistingValueResolvers.class);
-        if (existingValueResolvers == null) {
+        if (existingValueResolvers == null || !liveReloadBuildItem.isLiveReload()) {
+            // Reset the data if there is no context object or if the first start was unsuccessful
             existingValueResolvers = new ExistingValueResolvers();
             liveReloadBuildItem.setContextObject(ExistingValueResolvers.class, existingValueResolvers);
         }

diff --git a/...-http/deployment/src/test/java/io/quarkus/vertx/http/devconsole/FixConfigOnErrorTest.java b/...-http/deployment/src/test/java/io/quarkus/vertx/http/devconsole/FixConfigOnErrorTest.java
@@ -2,6 +2,7 @@
 
 import static org.hamcrest.Matchers.containsString;
 
+import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.RegisterExtension;
 
 import io.quarkus.test.QuarkusDevModeTest;
@@ -14,6 +15,7 @@ public class FixConfigOnErrorTest {
             .setAllowFailedStart(true)
             .withApplicationRoot((jar) -> jar.addClass(ConfigBean.class));
 
+    @Test
     public void testFailedStartup() {
         RestAssured.with()
                 .get("/msg")

diff --git a/integration-tests/devmode/src/test/java/io/quarkus/test/reload/CrashAfterReloadTest.java b/integration-tests/devmode/src/test/java/io/quarkus/test/reload/CrashAfterReloadTest.java
@@ -0,0 +1,63 @@
+package io.quarkus.test.reload;
+
+import static org.hamcrest.Matchers.containsString;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+import io.quarkus.test.QuarkusDevModeTest;
+import io.restassured.RestAssured;
+
+// https://github.com/quarkusio/quarkus/issues/35381
+public class CrashAfterReloadTest {
+
+    @RegisterExtension
+    static final QuarkusDevModeTest config = new QuarkusDevModeTest()
+            .setAllowFailedStart(true)
+            .withApplicationRoot(root -> root.addClasses(SomeBean.class, SomeBeanClient.class));
+
+    @Test
+    public void testRestarts() {
+        // The build should fail initially
+        RestAssured.with()
+                .get("/test")
+                .then()
+                .statusCode(500);
+
+        String someBeanWithoutScope = "public class SomeBean {";
+        String someBeanWithScope = "@ApplicationScoped public class SomeBean {";
+        String originalImport = "import jakarta.annotation.PostConstruct;";
+        String newImport = "import jakarta.annotation.PostConstruct;\nimport jakarta.enterprise.context.ApplicationScoped;";
+
+        // Add the scope annotation
+        config.modifySourceFile(SomeBean.class,
+                s -> s.replace(someBeanWithoutScope, someBeanWithScope).replace(originalImport, newImport));
+
+        RestAssured.with()
+                .get("/test")
+                .then()
+                .statusCode(200)
+                .body(containsString("pong"));
+
+        // Remove the scope annotation - the build should fail but the dev mode should not exit
+        config.modifySourceFile(SomeBean.class,
+                s -> s.replace(someBeanWithScope, someBeanWithoutScope));
+
+        RestAssured.with()
+                .get("/test")
+                .then()
+                .statusCode(500);
+
+        // Add the scope annotation
+        config.modifySourceFile(SomeBean.class,
+                s -> s.replace(someBeanWithoutScope, someBeanWithScope));
+
+        RestAssured.with()
+                .get("/test")
+                .then()
+                .statusCode(200)
+                .body(containsString("pong"));
+
+    }
+
+}
diff --git a/integration-tests/devmode/src/test/java/io/quarkus/test/reload/SomeBean.java b/integration-tests/devmode/src/test/java/io/quarkus/test/reload/SomeBean.java
@@ -0,0 +1,19 @@
+package io.quarkus.test.reload;
+
+import jakarta.annotation.PostConstruct;
+
+//missing scope - the build fails during the first dev mode start
+public class SomeBean {
+
+    private String msg;
+
+    public String ping() {
+        return msg;
+    }
+
+    @PostConstruct
+    void init() {
+        msg = "pong";
+    }
+
+}
diff --git a/integration-tests/devmode/src/test/java/io/quarkus/test/reload/SomeBeanClient.java b/integration-tests/devmode/src/test/java/io/quarkus/test/reload/SomeBeanClient.java
@@ -0,0 +1,27 @@
+package io.quarkus.test.reload;
+
+import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.enterprise.event.Observes;
+import jakarta.inject.Inject;
+
+import io.vertx.core.Handler;
+import io.vertx.ext.web.Router;
+import io.vertx.ext.web.RoutingContext;
+
+@ApplicationScoped
+public class SomeBeanClient {
+
+    @Inject
+    SomeBean someBean;
+
+    void route(@Observes Router router) {
+        router.route("/test").handler(new Handler<RoutingContext>() {
+            @Override
+            public void handle(RoutingContext event) {
+                event.response().end(someBean.ping());
+            }
+        });
+
+    }
+
+}