From 725dc6784a713c5a9c47d6cac2fb998b64a83e62 Mon Sep 17 00:00:00 2001 From: Martin Kouba Date: Thu, 17 Aug 2023 10:53:39 +0200 Subject: [PATCH] Dev mode: fix several problems when the initial build fails - IsolatedDevModeMain - replace the default exit handler _before_ the app is created - previously, if there was a deployment problem then the default exit handler was used; as a result the dev mode exited after the subsequent build failure - run DevModeListener.afterFirstStart() during restart if the first run did not succeed - fix FixConfigOnErrorTest - add missing Test annotation - fix Qute and ArC optimizations to skip generation of certain classes - fixes #35381 --- .../deployment/dev/DevModeListener.java | 5 ++ .../deployment/dev/IsolatedDevModeMain.java | 43 ++++++++----- .../quarkus/arc/deployment/ArcProcessor.java | 3 +- .../qute/deployment/QuteProcessor.java | 3 +- .../http/devconsole/FixConfigOnErrorTest.java | 2 + .../test/reload/CrashAfterReloadTest.java | 63 +++++++++++++++++++ .../java/io/quarkus/test/reload/SomeBean.java | 19 ++++++ .../quarkus/test/reload/SomeBeanClient.java | 27 ++++++++ 8 files changed, 148 insertions(+), 17 deletions(-) create mode 100644 integration-tests/devmode/src/test/java/io/quarkus/test/reload/CrashAfterReloadTest.java create mode 100644 integration-tests/devmode/src/test/java/io/quarkus/test/reload/SomeBean.java create mode 100644 integration-tests/devmode/src/test/java/io/quarkus/test/reload/SomeBeanClient.java 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 index d373e3a50cdeb..db4d2631ee8bc 100644 --- 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 index 745e6a94317b2..dbd579d226036 100644 --- 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 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 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 index d02186bd837d6..e3b33e3a09c5e 100644 --- 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 index e4effe48efbc7..a1f78c03c65c8 100644 --- 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 @@ -1769,7 +1769,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/extensions/vertx-http/deployment/src/test/java/io/quarkus/vertx/http/devconsole/FixConfigOnErrorTest.java b/extensions/vertx-http/deployment/src/test/java/io/quarkus/vertx/http/devconsole/FixConfigOnErrorTest.java index 10638ed8401d4..a7c990fa49019 100644 --- a/extensions/vertx-http/deployment/src/test/java/io/quarkus/vertx/http/devconsole/FixConfigOnErrorTest.java +++ b/extensions/vertx-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 new file mode 100644 index 0000000000000..ca167af335fdf --- /dev/null +++ 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 new file mode 100644 index 0000000000000..73689d5e2c33f --- /dev/null +++ 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 new file mode 100644 index 0000000000000..2364bd5c99e6c --- /dev/null +++ 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() { + @Override + public void handle(RoutingContext event) { + event.response().end(someBean.ping()); + } + }); + + } + +}