From 4a85a2764873a33213a857c21e4f58659dd7a24f Mon Sep 17 00:00:00 2001 From: Zakaria El Orche Date: Wed, 9 Sep 2026 10:22:23 +0000 Subject: [PATCH] feat(core): expose bound ports, service override, FlashApplication and close hooks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four small seams, each useful on its own, that together make a Flash app testable without hand-rolled scaffolding. - ServerHandle/ServerLifecycle/FlashApp gain port()/ports(). Listeners already bind in the FlashApp constructor, so port(0) resolved to a real port that nothing could read back; every integration test worked around this by opening a ServerSocket(0), closing it and reusing the number, which races anything else on the machine. - FlashContext.override() replaces a binding instead of rejecting it. The duplicate-is-an-error rule stays everywhere else; this is the single deliberate exception, for swapping a service out in tests. A replacement is logged at INFO so misuse in production is visible. - FlashApplication + FlashApp.apply() name an application independently of the port it runs on, so the same one can be booted twice. It takes FlashApp rather than FlashRegistrar because ws() and mount() live there. Being a functional interface, a lambda and a named class are the same thing. - FlashContext.onClose() runs cleanup at stop(), children first and then in reverse registration order. stop() previously closed sockets and the executor and never touched the service graph, so a pooled DataSource was only ever released by JVM exit — invisible with one app per process, a leak per test class once a suite boots and stops many. Co-Authored-By: Claude Opus 5 --- .../java/dev/relism/flash/ServerHandle.java | 11 ++ .../dev/relism/flash/extension/FlashApp.java | 30 ++- .../flash/extension/FlashApplication.java | 41 +++++ .../relism/flash/extension/FlashContext.java | 51 ++++++ .../flash/transport/ServerLifecycle.java | 8 + .../extension/FlashAppLifecycleTest.java | 172 ++++++++++++++++++ 6 files changed, 312 insertions(+), 1 deletion(-) create mode 100644 flash/src/main/java/dev/relism/flash/extension/FlashApplication.java create mode 100644 flash/src/test/java/dev/relism/flash/extension/FlashAppLifecycleTest.java diff --git a/flash/src/main/java/dev/relism/flash/ServerHandle.java b/flash/src/main/java/dev/relism/flash/ServerHandle.java index e03e8ce..f02a5b0 100644 --- a/flash/src/main/java/dev/relism/flash/ServerHandle.java +++ b/flash/src/main/java/dev/relism/flash/ServerHandle.java @@ -7,6 +7,7 @@ import dev.relism.flash.routing.AbstractWsRouter; import dev.relism.flash.transport.TransportFactory; import java.io.IOException; +import java.util.List; import java.util.concurrent.CompletableFuture; /** @@ -28,6 +29,16 @@ public interface ServerHandle { /** Gracefully stops the server, draining active connections. */ CompletableFuture stop(); + /** + * Port of the first bound listener. Valid as soon as the handle exists — listeners are + * bound at construction, not at {@link #start()} — so configuring with port {@code 0} + * and reading the OS-assigned port back is a supported pattern. + */ + int port(); + + /** Ports of every bound listener, in configuration order. */ + List ports(); + static ServerHandle create(FlashConfiguration config, AbstractRouter httpRouter, AbstractWsRouter wsRouter) throws IOException { diff --git a/flash/src/main/java/dev/relism/flash/extension/FlashApp.java b/flash/src/main/java/dev/relism/flash/extension/FlashApp.java index 61a83a8..896be15 100644 --- a/flash/src/main/java/dev/relism/flash/extension/FlashApp.java +++ b/flash/src/main/java/dev/relism/flash/extension/FlashApp.java @@ -22,6 +22,7 @@ import lombok.extern.slf4j.Slf4j; import java.io.IOException; import java.util.ArrayList; import java.util.List; +import java.util.Objects; import java.util.concurrent.CompletableFuture; import java.util.function.Consumer; @@ -138,6 +139,16 @@ public final class FlashApp extends FlashRegistrar { return this; } + /** + * Applies a {@link FlashApplication} to this app. Runs immediately, so {@code configure} + * sees a context that is still open for declarations and a listener that is already + * bound ({@link #port()}). + */ + public FlashApp apply(FlashApplication application) { + Objects.requireNonNull(application, "application").configure(this); + return this; + } + // ── Lifecycle ───────────────────────────────────────────────────────────── /** @@ -166,7 +177,24 @@ public final class FlashApp extends FlashRegistrar { server.startAndBlock(); } - public CompletableFuture stop() { return server.stop(); } + /** + * Gracefully stops the server, then runs every {@link FlashContext#onClose} callback so + * services release what they hold. Draining first means in-flight requests still see a + * live service graph. + */ + public CompletableFuture stop() { + return server.stop().whenComplete((ignored, failure) -> ctx.runCloseCallbacks()); + } + + /** + * Port of the first bound listener. Usable before {@link #start()}: listeners bind when + * the app is created, so {@code FlashApp.create(cfg with port 0).port()} yields the + * OS-assigned port, and an app can even be configured against its own address. + */ + public int port() { return server.port(); } + + /** Ports of every bound listener, in configuration order. */ + public List ports() { return server.ports(); } // ── FlashRegistrar impl ─────────────────────────────────────────────────── diff --git a/flash/src/main/java/dev/relism/flash/extension/FlashApplication.java b/flash/src/main/java/dev/relism/flash/extension/FlashApplication.java new file mode 100644 index 0000000..4d992b9 --- /dev/null +++ b/flash/src/main/java/dev/relism/flash/extension/FlashApplication.java @@ -0,0 +1,41 @@ +package dev.relism.flash.extension; + +/** + * One application's complete contribution to a {@link FlashApp} — its routes, extensions and + * services — expressed independently of which port or configuration it runs on. + * + *

Optional. The fluent form keeps working exactly as before; this interface exists so the + * same application can be created more than once, on more than one port: + * + * 

{@code
+ * public final class BlogApp implements FlashApplication {
+ *     @Override public void configure(FlashApp app) {
+ *         app.install(new JacksonExtension());
+ *         app.mount("/api", scope -> scope.scan("dev.blog.api"));
+ *         app.ws("/live", new FeedSocket());
+ *     }
+ * }
+ *
+ * // production
+ * FlashApp.create(8080).apply(new BlogApp()).startAndBlock();
+ * }
+ * + *

It takes {@link FlashApp} rather than {@link FlashRegistrar} deliberately: {@code ws} and + * {@code mount} live on {@code FlashApp}, and an application that could not register a + * WebSocket route or a mounted namespace would be a half-application. + * + *

Being a {@code @FunctionalInterface}, a lambda and a named class are the same thing here — + * {@code app -> app.get("/ping", ...)} is as valid as {@code new BlogApp()}. + * + * @see FlashApp#apply(FlashApplication) + */ +@FunctionalInterface +public interface FlashApplication { + + /** + * Registers this application onto {@code app}. Called immediately by + * {@link FlashApp#apply}, so the service graph is still open for declarations + * ({@code app.ctx().supply(...)}) and the listener is already bound ({@code app.port()}). + */ + void configure(FlashApp app); +} diff --git a/flash/src/main/java/dev/relism/flash/extension/FlashContext.java b/flash/src/main/java/dev/relism/flash/extension/FlashContext.java index 36bed75..1bf3134 100644 --- a/flash/src/main/java/dev/relism/flash/extension/FlashContext.java +++ b/flash/src/main/java/dev/relism/flash/extension/FlashContext.java @@ -1,10 +1,13 @@ package dev.relism.flash.extension; +import lombok.extern.slf4j.Slf4j; + import java.util.*; import java.util.function.Function; import java.util.function.Supplier; /** Deterministic boot-time service graph, frozen before handlers are initialised. */ +@Slf4j public final class FlashContext { private enum State { DECLARING, RESOLVING, READY } @@ -13,6 +16,7 @@ public final class FlashContext { private final List processors = new ArrayList<>(); private final List routeListeners = new ArrayList<>(); private final List readyCallbacks = new ArrayList<>(); + private final List closeCallbacks = new ArrayList<>(); private final List children = new ArrayList<>(); private final Deque> resolutionPath = new ArrayDeque<>(); private State state = State.DECLARING; @@ -34,6 +38,23 @@ public final class FlashContext { declare(type, new Binding<>(type, List.of(), ignored -> Objects.requireNonNull(instance, "instance"))); } + /** + * Replaces the binding for {@code type}, whether or not one already exists. + * + *

The declaration rule everywhere else is that a duplicate is an error + * ({@link #provide}, {@link #supply}); this is the single deliberate exception, and it + * exists for tests — {@code flash-testing} installs overrides as the last extension so a + * fake wins over whatever the application or an extension declared. Using it in + * production code is legal but almost always a mistake, so a replacement is logged. + */ + public void override(Class type, T instance) { + requireDeclaring(); + Objects.requireNonNull(type, "type"); + Objects.requireNonNull(instance, "instance"); + if (bindings.put(type, new Binding<>(type, List.of(), ignored -> instance)) != null) + log.info("Service binding overridden: {}", type.getName()); + } + /** Declares a no-dependency boot factory. */ public void supply(Class type, Supplier factory) { declare(type, new Binding<>(type, List.of(), ignored -> factory.get())); @@ -53,6 +74,21 @@ public final class FlashContext { /** Registers work materialised after all services are resolved. */ public void onReady(Runnable callback) { requireDeclaring(); readyCallbacks.add(Objects.requireNonNull(callback)); } + /** + * Registers cleanup to run when the app stops, after in-flight requests have drained. + * + *

Deliberately callable outside {@code DECLARING} so a factory can register its own + * teardown while it is being resolved: + * 

{@code
+     * ctx.supply(DataSource.class, c -> {
+     *     HikariDataSource ds = new HikariDataSource(cfg);
+     *     c.onClose(ds::close);
+     *     return ds;
+     * });
+     * }
+ */ + public void onClose(Runnable callback) { closeCallbacks.add(Objects.requireNonNull(callback)); } + @SuppressWarnings("unchecked") public T require(Class type) { if (state == State.DECLARING) @@ -106,6 +142,21 @@ public final class FlashContext { for (FlashContext child : children) child.runReadyCallbacks(); } + /** + * Runs every {@link #onClose} callback once: children first, then this context's own in + * reverse registration order, so a service always closes before whatever it depends on. + * A throwing callback is logged and does not stop the rest. Clearing makes a second + * {@code stop()} a no-op. + */ + void runCloseCallbacks() { + for (FlashContext child : children) child.runCloseCallbacks(); + for (int i = closeCallbacks.size() - 1; i >= 0; i--) { + try { closeCallbacks.get(i).run(); } + catch (RuntimeException e) { log.error("Close callback failed", e); } + } + closeCallbacks.clear(); + } + /** Completes graph resolution and runs all deferred materialisation callbacks once. */ public void complete() { resolveAll(); diff --git a/flash/src/main/java/dev/relism/flash/transport/ServerLifecycle.java b/flash/src/main/java/dev/relism/flash/transport/ServerLifecycle.java index d0536dc..c422587 100644 --- a/flash/src/main/java/dev/relism/flash/transport/ServerLifecycle.java +++ b/flash/src/main/java/dev/relism/flash/transport/ServerLifecycle.java @@ -46,6 +46,14 @@ public final class ServerLifecycle implements ServerHandle { this.acceptLatch = new CountDownLatch(TransportTuning.ACCEPT_THREADS * listeners.size()); } + @Override + public int port() { return listeners.get(0).socket().getLocalPort(); } + + @Override + public List ports() { + return listeners.stream().map(bound -> bound.socket().getLocalPort()).toList(); + } + /** Whether the server has begun shutting down. Passed down to every connection as a * {@link java.util.function.BooleanSupplier} so in-flight request loops can drain * promptly instead of waiting for their next keep-alive request. */ diff --git a/flash/src/test/java/dev/relism/flash/extension/FlashAppLifecycleTest.java b/flash/src/test/java/dev/relism/flash/extension/FlashAppLifecycleTest.java new file mode 100644 index 0000000..099f406 --- /dev/null +++ b/flash/src/test/java/dev/relism/flash/extension/FlashAppLifecycleTest.java @@ -0,0 +1,172 @@ +package dev.relism.flash.extension; + +import org.junit.jupiter.api.Test; + +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.atomic.AtomicInteger; + +import static org.junit.jupiter.api.Assertions.*; + +/** + * The four seams {@code flash-testing} is built on: reading back an ephemeral port, + * replacing a binding, applying an application, and closing services at stop. + */ +class FlashAppLifecycleTest { + + private static FlashConfiguration ephemeral() { + return FlashConfiguration.builder().port(0).host("127.0.0.1") + .shutdownDrainTimeoutMs(250).build(); + } + + // ── port() ─────────────────────────────────────────────────────────────── + + @Test + void exposesTheOsAssignedPortBeforeStart() { + FlashApp app = FlashApp.create(ephemeral()); + try { + assertTrue(app.port() > 0, "port 0 should resolve to a real bound port"); + assertEquals(List.of(app.port()), app.ports()); + } finally { + app.stop().join(); + } + } + + @Test + void keepsTheSamePortAcrossStart() { + FlashApp app = FlashApp.create(ephemeral()).get("/ping", (req, res) -> "pong"); + try { + int beforeStart = app.port(); + app.start(); + assertEquals(beforeStart, app.port()); + } finally { + app.stop().join(); + } + } + + // ── override() ─────────────────────────────────────────────────────────── + + @Test + void overrideReplacesAnExistingBinding() { + FlashContext ctx = new FlashContext(); + ctx.provide(Greeter.class, () -> "real"); + ctx.override(Greeter.class, () -> "fake"); + + ctx.resolveAll(); + + assertEquals("fake", ctx.require(Greeter.class).greet()); + } + + @Test + void overrideAlsoDeclaresWhenNothingWasBound() { + FlashContext ctx = new FlashContext(); + ctx.override(Greeter.class, () -> "fake"); + + ctx.resolveAll(); + + assertEquals("fake", ctx.require(Greeter.class).greet()); + } + + @Test + void overrideIsRejectedOnceDeclarationsAreClosed() { + FlashContext ctx = new FlashContext(); + ctx.resolveAll(); + + assertThrows(IllegalStateException.class, () -> ctx.override(Greeter.class, () -> "fake")); + } + + @Test + void lastInstalledExtensionWinsOverAnEarlierProvider() { + FlashApp app = FlashApp.create(ephemeral()) + .install((registrar, ctx) -> ctx.provide(Greeter.class, () -> "real")) + .install((registrar, ctx) -> ctx.override(Greeter.class, () -> "fake")); + try { + app.start(); + assertEquals("fake", app.ctx().require(Greeter.class).greet()); + } finally { + app.stop().join(); + } + } + + // ── apply() ────────────────────────────────────────────────────────────── + + @Test + void applyRunsImmediatelyWithAnOpenContextAndABoundPort() { + List portSeenInsideConfigure = new ArrayList<>(); + + FlashApp app = FlashApp.create(ephemeral()).apply(configured -> { + portSeenInsideConfigure.add(configured.port()); + configured.ctx().provide(Greeter.class, () -> "from-application"); + configured.get("/hello", (req, res) -> "hi"); + }); + try { + assertEquals(List.of(app.port()), portSeenInsideConfigure); + app.start(); + assertEquals("from-application", app.ctx().require(Greeter.class).greet()); + } finally { + app.stop().join(); + } + } + + // ── onClose() ──────────────────────────────────────────────────────────── + + @Test + void stopRunsCloseCallbacksInReverseOrder() { + List closed = new ArrayList<>(); + FlashApp app = FlashApp.create(ephemeral()).apply(configured -> { + configured.ctx().onClose(() -> closed.add("first")); + configured.ctx().onClose(() -> closed.add("second")); + }); + + app.start(); + app.stop().join(); + + assertEquals(List.of("second", "first"), closed); + } + + @Test + void aFactoryCanRegisterItsOwnTeardownWhileResolving() { + AtomicInteger closes = new AtomicInteger(); + FlashApp app = FlashApp.create(ephemeral()).apply(configured -> + configured.ctx().supply(Greeter.class, services -> { + services.onClose(closes::incrementAndGet); + return () -> "resolved"; + })); + + app.start(); + assertEquals(0, closes.get(), "must not close while the app is live"); + + app.stop().join(); + assertEquals(1, closes.get()); + } + + @Test + void aThrowingCloseCallbackDoesNotStopTheRest() { + List closed = new ArrayList<>(); + FlashApp app = FlashApp.create(ephemeral()).apply(configured -> { + configured.ctx().onClose(() -> closed.add("ran")); + configured.ctx().onClose(() -> { throw new IllegalStateException("boom"); }); + }); + + app.start(); + assertDoesNotThrow(() -> app.stop().join()); + + assertEquals(List.of("ran"), closed); + } + + @Test + void closeCallbacksRunAtMostOnce() { + AtomicInteger closes = new AtomicInteger(); + FlashApp app = FlashApp.create(ephemeral()) + .apply(configured -> configured.ctx().onClose(closes::incrementAndGet)); + + app.start(); + app.stop().join(); + app.stop().join(); + + assertEquals(1, closes.get()); + } + + @FunctionalInterface + interface Greeter { String greet(); } +}