Iss22 suggestions (#36)

* Add a countdown latch as a startup barrier for use by the RunOnFxThreadInterceptor to avoid race conditions between background threads interacting with javafx.application.Platform thread during startup. fixes #22 Signed-off-by: Scott M Stark <[email protected]> * Updates per the comments fixes #22 Signed-off-by: Scott M Stark <[email protected]> * Updates per the comments fixes #22 Signed-off-by: Scott M Stark <[email protected]> * ClockEvents should be using synchronous events from RunOnFxThread Signed-off-by: Scott M Stark <[email protected]> * Reorganize the code so that FxApplication remains a non-CDI bean Signed-off-by: Scott M Stark <[email protected]> * wip * tests * wip * Add suggestions * Align RunOnFxThread test * 0.2.0 --------- Signed-off-by: Scott M Stark <[email protected]> Co-authored-by: Scott M Stark <[email protected]>
quarkiverse · Feb 9, 2024 · cefd200 · cefd200
1 parent feb8ab6
commit cefd200
Show file tree

Hide file tree

Showing 19 changed files with 244 additions and 135 deletions.
diff --git a/.github/project.yml b/.github/project.yml
@@ -1,3 +1,3 @@
 release:
-  current-version: "0.1.0"
+  current-version: "0.2.0"
   next-version: "999-SNAPSHOT"
diff --git a/deployment/src/main/java/io/quarkiverse/fx/deployment/QuarkusFxExtensionProcessor.java b/deployment/src/main/java/io/quarkiverse/fx/deployment/QuarkusFxExtensionProcessor.java
@@ -10,7 +10,12 @@
 import org.jboss.jandex.VoidType;
 import org.jboss.logging.Logger;
 
-import io.quarkiverse.fx.*;
+import io.quarkiverse.fx.FXMLLoaderProducer;
+import io.quarkiverse.fx.FxStartupLatch;
+import io.quarkiverse.fx.PrimaryStage;
+import io.quarkiverse.fx.QuarkusFxApplication;
+import io.quarkiverse.fx.RunOnFxThread;
+import io.quarkiverse.fx.RunOnFxThreadInterceptor;
 import io.quarkus.arc.deployment.AdditionalBeanBuildItem;
 import io.quarkus.deployment.annotations.BuildProducer;
 import io.quarkus.deployment.annotations.BuildStep;
@@ -52,7 +57,7 @@ AdditionalBeanBuildItem primaryStage() {
      */
     @BuildStep
     AdditionalBeanBuildItem startupLatch() {
-        return new AdditionalBeanBuildItem(StartupLatch.class);
+        return new AdditionalBeanBuildItem(FxStartupLatch.class);
     }
 
     @BuildStep

diff --git a/deployment/src/test/java/io/quarkiverse/fx/deployment/FxStartupTest.java b/deployment/src/test/java/io/quarkiverse/fx/deployment/FxStartupTest.java
@@ -0,0 +1,41 @@
+package io.quarkiverse.fx.deployment;
+
+import java.util.concurrent.CompletableFuture;
+
+import jakarta.inject.Inject;
+
+import org.jboss.shrinkwrap.api.ShrinkWrap;
+import org.jboss.shrinkwrap.api.spec.JavaArchive;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.Timeout;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+import io.quarkiverse.fx.FxStartupLatch;
+import io.quarkiverse.fx.QuarkusFxApplication;
+import io.quarkus.runtime.Quarkus;
+import io.quarkus.test.QuarkusUnitTest;
+
+public class FxStartupTest {
+
+    @RegisterExtension
+    static final QuarkusUnitTest unitTest = new QuarkusUnitTest()
+            .setArchiveProducer(() -> ShrinkWrap.create(JavaArchive.class));
+
+    @Inject
+    FxStartupLatch latch;
+
+    @Test
+    @Timeout(value = 5)
+    void test() {
+
+        Assertions.assertNotNull(this.latch);
+        CompletableFuture.runAsync(() -> Quarkus.run(QuarkusFxApplication.class));
+
+        try {
+            this.latch.await();
+        } catch (InterruptedException e) {
+            Assertions.fail(e);
+        }
+    }
+}
diff --git a/deployment/src/test/java/io/quarkiverse/fx/deployment/FxTestConstants.java b/deployment/src/test/java/io/quarkiverse/fx/deployment/FxTestConstants.java
@@ -0,0 +1,7 @@
+package io.quarkiverse.fx.deployment;
+
+public final class FxTestConstants {
+
+    // Allow some time between launch and FX readiness
+    static final int LAUNCH_TIMEOUT_MS = 3_000;
+}
diff --git a/deployment/src/test/java/io/quarkiverse/fx/deployment/QuarkusFxTest.java b/deployment/src/test/java/io/quarkiverse/fx/deployment/QuarkusFxTest.java
@@ -1,36 +1,37 @@
 package io.quarkiverse.fx.deployment;
 
-import io.quarkiverse.fx.PrimaryStage;
-import io.quarkiverse.fx.QuarkusFxApplication;
-import io.quarkus.runtime.Quarkus;
-import io.quarkus.test.QuarkusUnitTest;
+import static org.awaitility.Awaitility.await;
+
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicInteger;
+
 import jakarta.enterprise.context.Dependent;
 import jakarta.enterprise.event.Observes;
 import jakarta.inject.Inject;
 import jakarta.inject.Singleton;
-import javafx.fxml.FXML;
-import javafx.fxml.FXMLLoader;
-import javafx.scene.layout.Pane;
-import javafx.stage.Stage;
+
 import org.jboss.shrinkwrap.api.ShrinkWrap;
 import org.jboss.shrinkwrap.api.spec.JavaArchive;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.RegisterExtension;
 
-import java.io.ByteArrayInputStream;
-import java.io.IOException;
-import java.io.InputStream;
-import java.util.concurrent.CompletableFuture;
-import java.util.concurrent.TimeUnit;
-import java.util.concurrent.atomic.AtomicBoolean;
-import java.util.concurrent.atomic.AtomicInteger;
-
-import static org.awaitility.Awaitility.await;
+import io.quarkiverse.fx.PrimaryStage;
+import io.quarkiverse.fx.QuarkusFxApplication;
+import io.quarkus.runtime.Quarkus;
+import io.quarkus.test.QuarkusUnitTest;
+import javafx.fxml.FXML;
+import javafx.fxml.FXMLLoader;
+import javafx.scene.layout.Pane;
+import javafx.stage.Stage;
 
 public class QuarkusFxTest {
 
-    private static final int LAUNCH_TIMEOUT_MS = 3_000;
     private static final int A_FANCY_TEST_VALUE = 42;
 
     private static final String FXML_CONTENT = """
@@ -63,7 +64,6 @@ void initialize() {
         }
     }
 
-    // Start unit test with your extension loaded
     @RegisterExtension
     static final QuarkusUnitTest unitTest = new QuarkusUnitTest()
             .setArchiveProducer(() -> ShrinkWrap.create(JavaArchive.class));
@@ -82,7 +82,7 @@ void testFXMLLaunchAndLoad() {
         CompletableFuture.runAsync(() -> Quarkus.run(QuarkusFxApplication.class));
 
         await()
-                .atMost(LAUNCH_TIMEOUT_MS, TimeUnit.MILLISECONDS)
+                .atMost(FxTestConstants.LAUNCH_TIMEOUT_MS, TimeUnit.MILLISECONDS)
                 .until(primaryStageObserved::get);
 
         try {

diff --git a/deployment/src/test/java/io/quarkiverse/fx/deployment/RunOnFxThreadTest.java b/deployment/src/test/java/io/quarkiverse/fx/deployment/RunOnFxThreadTest.java
@@ -1,83 +1,72 @@
 package io.quarkiverse.fx.deployment;
 
+import static org.awaitility.Awaitility.await;
+
 import java.util.concurrent.CompletableFuture;
-import java.util.concurrent.ExecutionException;
-import java.util.concurrent.ExecutorService;
-import java.util.concurrent.Executors;
 import java.util.concurrent.TimeUnit;
-import java.util.concurrent.atomic.AtomicReference;
 
-import jakarta.enterprise.context.Dependent;
 import jakarta.enterprise.event.Observes;
 
-import org.awaitility.Awaitility;
+import org.jboss.shrinkwrap.api.ShrinkWrap;
+import org.jboss.shrinkwrap.api.spec.JavaArchive;
 import org.junit.jupiter.api.Assertions;
-import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.RegisterExtension;
 
-import io.quarkiverse.fx.FxApplication;
 import io.quarkiverse.fx.PrimaryStage;
+import io.quarkiverse.fx.QuarkusFxApplication;
 import io.quarkiverse.fx.RunOnFxThread;
+import io.quarkus.runtime.Quarkus;
 import io.quarkus.test.QuarkusUnitTest;
-import javafx.application.Application;
 import javafx.application.Platform;
 import javafx.stage.Stage;
 
-@Disabled // Disabled until #9 is fixed
 public class RunOnFxThreadTest {
 
-    private static final int LAUNCH_TIMEOUT_MS = 1_000;
+    private static final int ASYNC_RUN_TIMEOUT_MS = 500;
 
     @RegisterExtension
-    static final QuarkusUnitTest config = new QuarkusUnitTest()
-            .withApplicationRoot((jar) -> jar.addClasses(Observer.class));
+    static final QuarkusUnitTest unitTest = new QuarkusUnitTest()
+            .setArchiveProducer(() -> ShrinkWrap.create(JavaArchive.class));
+
+    private static boolean primaryStageObserved = false;
+    private static Boolean regularThread;
+    private static Boolean fxThread;
 
     @Test
-    void annotatedMethodsExecuteOnFxThread() {
-        // launch
-        CompletableFuture.runAsync(() -> Application.launch(FxApplication.class));
-        // wait for stage to be observed
-        Awaitility.await().atMost(LAUNCH_TIMEOUT_MS, TimeUnit.MILLISECONDS).until(Observer::stageObserved);
-        // stage should not be null
-        Assertions.assertNotNull(Observer.OBSERVED_STAGE.get());
-        //
-        Assertions.assertTrue(Observer.STAGE_OBSERVER_METHOD_THREAD_IS_FX_THREAD);
-        Assertions.assertFalse(Observer.DO_ON_CURRENT_METHOD_THREAD_IS_FX_THREAD);
-        Assertions.assertTrue(Observer.DO_ON_FX_METHOD_THREAD_IS_FX_THREAD);
+    void test() {
+        // Non-blocking JavaFX launch
+        CompletableFuture.runAsync(() -> Quarkus.run(QuarkusFxApplication.class));
+
+        await()
+                .atMost(FxTestConstants.LAUNCH_TIMEOUT_MS, TimeUnit.MILLISECONDS)
+                .until(() -> primaryStageObserved);
+
+        // Synchronous regular thread run
+        this.runOnRegularThread();
+        Assertions.assertTrue(regularThread);
+
+        // Asynchronous FX thread run
+        this.runOnFxThread();
+        Assertions.assertNull(fxThread);
+
+        await().atMost(ASYNC_RUN_TIMEOUT_MS, TimeUnit.MILLISECONDS)
+                .until(() -> fxThread != null);
+
+        Assertions.assertTrue(fxThread);
     }
 
-    @Dependent
-    static class Observer {
-        public static final AtomicReference<Stage> OBSERVED_STAGE = new AtomicReference<>();
-        public static boolean STAGE_OBSERVER_METHOD_THREAD_IS_FX_THREAD;
-        public static boolean DO_ON_CURRENT_METHOD_THREAD_IS_FX_THREAD;
-        public static boolean DO_ON_FX_METHOD_THREAD_IS_FX_THREAD;
-
-        public void observeStage(@Observes @PrimaryStage final Stage stage) throws ExecutionException, InterruptedException {
-            // Observe current thread
-            STAGE_OBSERVER_METHOD_THREAD_IS_FX_THREAD = Platform.isFxApplicationThread();
-            // launch methods and observe threads
-            ExecutorService executor = Executors.newSingleThreadExecutor();
-            executor.submit(this::doOnCurrentThread).get();
-            executor.submit(this::doOnFxThread).get();
-            executor.shutdownNow();
-            // observe stage
-            Observer.OBSERVED_STAGE.set(stage);
-        }
-
-        void doOnCurrentThread() {
-            DO_ON_CURRENT_METHOD_THREAD_IS_FX_THREAD = Platform.isFxApplicationThread();
-        }
-
-        @RunOnFxThread
-        void doOnFxThread() {
-            DO_ON_FX_METHOD_THREAD_IS_FX_THREAD = Platform.isFxApplicationThread();
-        }
-
-        public static boolean stageObserved() {
-            return OBSERVED_STAGE.get() != null;
-        }
+    void observePrimaryStage(@Observes @PrimaryStage final Stage stage) {
+        Assertions.assertNotNull(stage);
+        primaryStageObserved = true;
     }
 
+    void runOnRegularThread() {
+        regularThread = !Platform.isFxApplicationThread();
+    }
+
+    @RunOnFxThread
+    void runOnFxThread() {
+        fxThread = Platform.isFxApplicationThread();
+    }
 }
diff --git a/docs/modules/ROOT/nav.adoc b/docs/modules/ROOT/nav.adoc
@@ -1,2 +1,3 @@
-* xref:index.adoc[Quarkus FX]
-* xref:run-on-fx-thread.adoc[]
+* xref:index.adoc[]
+* xref:run-on-fx-thread.adoc[]
+* xref:startup-synchronization.adoc[]
diff --git a/docs/modules/ROOT/pages/includes/attributes.adoc b/docs/modules/ROOT/pages/includes/attributes.adoc
@@ -1 +1 @@
-:project-version: 0.1.0
+:project-version: 0.2.0
diff --git a/docs/modules/ROOT/pages/index.adoc b/docs/modules/ROOT/pages/index.adoc
@@ -35,7 +35,7 @@ dependencies {
 The extension allows using cdi features in JavaFX controller classes. +
 The FXMLLoader is made a CDI bean and can be injected in your application.
 
-[source,java,subs=attributes+]
+[source,java]
 ----
 @Inject
 FXMLLoader fxmlLoader;
@@ -48,7 +48,7 @@ public void load() {
 }
 ----
 
-[source,java,subs=attributes+]
+[source,java]
 ----
 public class MyFxmlController {
 
@@ -68,7 +68,7 @@ The application will automatically be launched (thanks to a call to `javafx.appl
 
 If you need to customize the launch, you can provide a custom `@QuarkusMain`, such as :
 
-[source,java,subs=attributes+]
+[source,java]
 ----
 package io.quarkiverse.fx.fxapp;
 
@@ -89,7 +89,7 @@ public class QuarkusFxApplication implements QuarkusApplication {
 
 When the application is started, you can get obtain access to the primary `Stage` instance by observing the CDI event :
 
-[source,java,subs=attributes+]
+[source,java]
 ----
 void onStart(@Observes @PrimaryStage final Stage stage) {
   // Do something with the stage

diff --git a/docs/modules/ROOT/pages/startup-synchronization.adoc b/docs/modules/ROOT/pages/startup-synchronization.adoc
@@ -0,0 +1,40 @@
+= Startup Latch
+
+To synchronize the application with JavaFX readiness, one can use the `FxStartupLatch` bean.
+
+It provides an `await` method that is released when JavaFX app is ready (primary `Stage` instance is available).
+
+That is used in the inner `@RunOnFxThread` to ensure app is ready.
+
+[source, java]
+----
+@Inject
+FxStartupLatch startupLatch;
+
+// Blocking until FX env is ready
+startupLatch.await();
+
+// FX is ready
+----
+
+Alternatively, the `FxStartupEvent` can be observed.
+
+Example of a `SkipPredicated` that can be used in conjunction with a `@Scheduled`
+
+[source,java]
+----
+@Singleton
+public class FxApplicationNotStarted implements SkipPredicate {
+
+    private volatile boolean started;
+
+    void onFxStartup(@Observes final FxStartupEvent event) {
+        this.started = true;
+    }
+
+    @Override
+    public boolean test(final ScheduledExecution execution) {
+        return !this.started;
+    }
+}
+----