feat(engine): add RouterFactorySpi for pluggable stream factory composition by jfallows · Pull Request #1757 · aklivity/zilla

jfallows · 2026-05-08T18:54:28Z

Summary

Adds an SPI surface (RouterFactorySpi) that lets engine extensions contribute to the composition of EngineContext.streamFactory(). Mirrors the factory + context shape already used by Store:

RouterFactorySpi.create(Configuration) → Router
Router.supply(RouteableContext) → RouterContext
RouterContext.attach(RouterConfig) → BindingHandler (paired with detach(long routerId))

RouteableContext is the under-engine surface available to the router during setup: it exposes the engine Configuration, the engine's current default BindingHandler stream factory (which the router may wrap), and attachComposite(...) / detachComposite(...) for injecting synthesized namespaces alongside operator-authored ones.

When zilla.engine.router selects a router by name, the engine instantiates it via ServiceLoader, supplies a RouteableContext, attaches a synthesized RouterConfig, and installs the returned BindingHandler as the worker's stream factory. With no router configured, runtime behavior is unchanged.

The change is engine-only:

New runtime.engine.router package: RouterFactorySpi, RouterFactory, Router, RouterContext, RouteableContext
New runtime.engine.config.RouterConfig (with id) + RouterConfigBuilder
EngineConfiguration adds zilla.engine.router property and routerName() accessor
EngineBuilder / Engine / EngineWorker thread the router through; EngineWorker.streamFactory() returns a field initialised to this::newStream and replaced by the wrapped handler when a router attaches; onClose invokes RouterContext.detach(routerId)
moditect module-info exports the new package and declares uses RouterFactorySpi
Test impl under test/internal/router/ (TestRouterFactorySpi, TestRouter, TestRouterContext) returns the engine's default handler unchanged; wired via META-INF/services
RouterFactoryTest covers discovery, creation, and the unrecognized-name error citing RouterFactory.names()
EngineIT.shouldHandshakeWithRouter exercises the full startup path with a router attached

Closes #1754

Test plan

./mvnw -pl runtime/engine clean verify — 319 unit tests, 203 ITs pass; JaCoCo coverage check passes
RouterFactoryTest (3 tests): discovery, creation, unrecognized-name error
EngineIT#shouldHandshakeWithRouter: full handshake with the test router attached, default behavior preserved
EngineIT#shouldHandshake still passes with no router configured (no-router default unchanged)
EngineConfigurationTest#shouldVerifyConstants updated to verify the new zilla.engine.router property name

Generated by Claude Code

…sition Introduce a service-provider interface that allows extensions to contribute to the composition of the engine's BindingHandler stream factory. When a router is selected by name from engine Configuration, it is instantiated via ServiceLoader, supplied with a RouteableContext (config, current default streamFactory, attachComposite/detachComposite), and attached with a synthesized RouterConfig; the resulting BindingHandler is installed as the worker's stream factory. With no router configured, default behavior is preserved. Mirrors the factory + context shape of Store: RouterFactorySpi.create → Router.supply(RouteableContext) → RouterContext.attach(RouterConfig) → BindingHandler. Engine shutdown invokes RouterContext.detach(routerId). Closes #1754

jfallows · 2026-05-08T19:10:48Z

+    EngineRouteableContext(
+        Configuration config,
+        BindingHandler streamFactory,
+        EngineContext engineContext)


This seems cyclic, since EngineRouteableContext is contributing to the creation of EngineContext.

Perhaps we should pass in these attachComposite and detachComposite methods as lambdas instead and avoid passing EngineContext.

jfallows · 2026-05-08T19:15:27Z

+        Router router,
+        RouterConfig routerConfig,


This feels incorrectly externalized, as it logically belongs to the Engine, should be handled internally instead.

jfallows · 2026-05-08T19:16:19Z

+        final RouterFactory routerFactory = RouterFactory.instantiate();
+        final String routerName = config.routerName();
+        Router router = null;
+        RouterConfig routerConfig = null;
+        if (routerName != null)
+        {
+            router = routerFactory.create(routerName, config);
+            routerConfig = RouterConfig.builder()
+                .id(0L)
+                .name(routerName)
+                .build();
+        }
+


Move to Engine, use to create EngineRouter, pass to EngineWorker.

jfallows · 2026-05-08T19:16:59Z

        return ENGINE_HOST_RESOLVER.get(this)::resolve;
    }

+    public String routerName()


Method name should align with property name, use router() here.

- rename EngineConfiguration.routerName() to router() to align with property - move RouterFactory loading from EngineBuilder into Engine - EngineRouteableContext takes Consumer<NamespaceConfig> lambdas for attachComposite/detachComposite instead of the full EngineContext (the previous shape was cyclic — EngineRouteableContext is contributing to the creation of EngineContext) - encapsulate router lifecycle in a new EngineRouter class that itself implements BindingHandler with a mutable internal delegate; start() at EngineWorker.onStart, close() at onClose - EngineWorker.streamFactory() returns the EngineRouter so binding factories cache a stable reference at supply time; this fixes the NPE seen in binding-http (HttpServerFactory cached context.streamFactory() during Binding.supply, but the field was previously initialized at the end of the EngineWorker constructor — after supply)

…eRouter - Engine resolves the Router and synthesises RouterConfig from EngineConfiguration.router(), passes both to each EngineWorker - EngineWorker constructs an EngineRouter early (before binding.supply) so the BindingHandler reference cached by bindings is the EngineRouter; this fixes the binding-http NPE - EngineConfiguration.routerName() renamed to router()

Aligns the accessor name with the property name `zilla.engine.router`.

Constructs EngineRouter early (before Binding.supply) so the BindingHandler reference cached by binding factories is the EngineRouter. Calls start() at onStart and close() at onClose to attach/detach the router context.

…nfig Per review: Engine creates EngineRouter and passes to EngineWorker. The worker provides its per-worker default stream factory and routeable context via a new attach(...) method called early in its constructor (before binding.supply).

Engine creates one EngineRouter per worker (in the worker creation loop) using the resolved Router and synthesised RouterConfig, and passes the EngineRouter to EngineWorker.

Constructor parameter changed from (Router, RouterConfig) to EngineRouter. Worker calls engineRouter.attach(defaultStreamFactory, routeable) early in the constructor (before binding.supply) so binding factories cache the EngineRouter as their stable BindingHandler reference.

jfallows · 2026-05-08T23:18:30Z

    private long budgetId;
    private long authorizedId;

+    private final EngineRouter engineRouter;


Suggested change

private final EngineRouter engineRouter;

private final EngineRouter router;

jfallows · 2026-05-08T23:21:15Z

+        BindingHandler defaultStreamFactory = this::newStream;
+        RouteableContext routeable = new EngineRouteableContext(config, defaultStreamFactory,
+            this::attachComposite, this::detachComposite);
+        this.engineRouter = engineRouter;
+        this.engineRouter.attach(defaultStreamFactory, routeable);
+


Suggested change

BindingHandler defaultStreamFactory = this::newStream;

RouteableContext routeable = new EngineRouteableContext(config, defaultStreamFactory,

this::attachComposite, this::detachComposite);

this.engineRouter = engineRouter;

this.engineRouter.attach(defaultStreamFactory, routeable);

EngineRouteable routeable = new EngineRouteable(config, this::newStream,

this::attachComposite, this::detachComposite);

this.router = router.supplyContext(routeable);

jfallows · 2026-05-08T23:44:12Z

+        final String routerName = config.router();
+        final Router router = routerName != null
+            ? RouterFactory.instantiate().create(routerName, config)
+            : null;
+        final RouterConfig routerConfig = routerName != null
+            ? RouterConfig.builder()
+                .id(0L)
+                .name(routerName)
+                .build()
+            : null;
+


Let's do all this inside EngineRouter constructor.

jfallows · 2026-05-08T23:44:45Z

        List<EngineWorker> workers = new ArrayList<>(workerCount);
        for (int workerIndex = 0; workerIndex < workerCount; workerIndex++)
        {
+            EngineRouter engineRouter = new EngineRouter(router, routerConfig);


Suggested change

EngineRouter engineRouter = new EngineRouter(router, routerConfig);

EngineRouter router = new EngineRouter(config);

jfallows · 2026-05-09T00:02:38Z

+    @Override
+    public MessageConsumer newStream(
+        int msgTypeId,
+        DirectBuffer buffer,
+        int index,
+        int length,
+        MessageConsumer sender)
+    {
+        return delegate.newStream(msgTypeId, buffer, index, length, sender);
+    }


Remove this.

jfallows · 2026-05-09T00:04:21Z

+    void attach(
+        BindingHandler defaultStreamFactory,
+        RouteableContext routeable)
+    {
+        this.defaultDelegate = defaultStreamFactory;
+        this.delegate = defaultStreamFactory;
+        this.routeable = routeable;
+    }


Suggested change

void attach(

BindingHandler defaultStreamFactory,

RouteableContext routeable)

{

this.defaultDelegate = defaultStreamFactory;

this.delegate = defaultStreamFactory;

this.routeable = routeable;

}

BindingHandler attach(

RouterConfig config)

{

return context != null ? context.attach(config) : streamFactory;

}

jfallows · 2026-05-09T00:05:31Z

+        {
+            context.detach(config.id);
+            context = null;
+            delegate = defaultDelegate;


Suggested change

delegate = defaultDelegate;

jfallows · 2026-05-09T00:06:05Z

+        }
+    }
+
+    void close()


Suggested change

void close()

void detach(RouterConfig config)

jfallows · 2026-05-09T00:06:16Z

+
+    void start()
+    {
+        if (router != null && context == null)
+        {
+            context = router.supply(routeable);
+            delegate = context.attach(config);
+        }
+    }


Remove this.

…/attach/detach EngineRouter no longer implements BindingHandler. Two constructors: engine-wide (EngineConfiguration) and per-worker (RouterContext, BindingHandler). Engine-wide exposes supplyContext(routeable) returning per-worker. Per-worker exposes attach(RouterConfig) returning the wrapped BindingHandler (or default) and detach(RouterConfig) for cleanup. Rename EngineRouteableContext to EngineRouteable. Engine constructs one EngineRouter per worker via new EngineRouter(config).

Field renamed engineRouter -> router. Worker captures routerConfig from engine-wide router via router.config(), constructs EngineRouteable, calls router.supplyContext(routeable) to get the per-worker EngineRouter, and router.attach(routerConfig) to obtain the BindingHandler used as streamFactory. Detaches in onClose.

…eRouteable)

…ntext EngineRouter implements Router as the default no-op pass-through. The new EngineRouterContext returns RouteableContext.streamFactory() from attach() and is a no-op on detach(routerId).

…se routerConfig() Engine resolves the Router (operator-configured via RouterFactory or the default EngineRouter), synthesises a RouterConfig, and exposes routerConfig(). EngineWorker now takes (Router, RouterConfig).

Component supply loops (binding/exporter/guard/vault/catalog/store/model/ metric-group) and EngineRegistry construction move from the constructor to onStart, so the constructor finishes before components start calling back into EngineContext. streamFactory is set in onStart from router.attach (routerConfig); router.detach(routerConfig.id) is called in onClose. EngineWorker now takes (Router, RouterConfig); the per-worker RouterContext is obtained via router.supply(routeable).

jfallows · 2026-05-09T01:39:46Z

+    private final Collection<Store> stores;
+    private final Collector collector;
+    private final Consumer<NamespaceConfig> process;
+    private Map<String, MetricGroup> metricGroupsByName;


Move newly non-final fields down after all the final fields into a block of their own for readability.

jfallows · 2026-05-09T01:42:40Z

+        Map<String, BindingContext> bindingsByType = new LinkedHashMap<>();
+        for (Binding binding : bindings)
+        {
+            bindingsByType.put(binding.name(), binding.supply(this));
+        }


Can we simplify each of these using bindings.stream().collect(toMap(Binding::name, b -> b.supply(this))) for brevity?

jfallows · 2026-05-09T01:44:46Z

+        final String routerName = config.router();
+        final Router router = routerName != null
+            ? RouterFactory.instantiate().create(routerName, config)
+            : new EngineRouter();


Does it make sense to register an SPI for EngineRouter, then default the engine router name to "engine" and not need the special case for null?

…"engine" EngineRouterFactorySpi exposes the default EngineRouter through ServiceLoader and the moditect module-info `provides` clause. zilla.engine.router defaults to "engine" so Engine no longer needs a null special case to instantiate the default Router. RouterFactoryTest expects both "engine" and "test" routers to be discovered.

Engine resolves Router via RouterFactory using config.router() — the default "engine" property value selects the EngineRouter SPI.

- Move metricGroupsByName + usageMetric back to the constructor as final fields (they don't escape `this`) - Group remaining non-final fields (modelsByType, registry, streamFactory) into a dedicated block at the end of the field declarations for readability - Simplify supply loops in onStart with stream().collect(toMap(...)) for bindings, exporters, guards, stores, and models (vault/catalog kept as for-loops because they fan out aliases)

jfallows

LGTM

claude and others added 2 commits May 8, 2026 18:39

docs(engine): document router as a test concept type

8441343

jfallows commented May 8, 2026

View reviewed changes

jfallows added 7 commits May 8, 2026 12:34

refactor(engine): rename EngineConfiguration.routerName() to router()

5b6d29c

Aligns the accessor name with the property name `zilla.engine.router`.

refactor(engine): Engine constructs EngineRouter per worker

2dab56f

Engine creates one EngineRouter per worker (in the worker creation loop) using the resolved Router and synthesised RouterConfig, and passes the EngineRouter to EngineWorker.

jfallows commented May 9, 2026

View reviewed changes

jfallows added 6 commits May 8, 2026 17:40

refactor(engine): remove old EngineRouteableContext (renamed to Engin…

5be25d9

…eRouteable)

refactor(engine): EngineRouter is the default Router; pass-through co…

4a9f0ed

…ntext EngineRouter implements Router as the default no-op pass-through. The new EngineRouterContext returns RouteableContext.streamFactory() from attach() and is a no-op on detach(routerId).

jfallows commented May 9, 2026

View reviewed changes

jfallows added 4 commits May 8, 2026 18:58

refactor(engine): remove null special case for router resolution

b89a4e7

Engine resolves Router via RouterFactory using config.router() — the default "engine" property value selects the EngineRouter SPI.

refactor(engine): default zilla.engine.router property to "engine"

3afdb7f

jfallows commented May 9, 2026

View reviewed changes

jfallows merged commit ff57764 into develop May 9, 2026
109 of 110 checks passed

jfallows mentioned this pull request May 10, 2026

fix(engine): await worker/boss onStart before EngineManager registration #1760

Merged

4 tasks

	private final EngineRouter engineRouter;
	private final EngineRouter router;

	EngineRouter engineRouter = new EngineRouter(router, routerConfig);
	EngineRouter router = new EngineRouter(config);

Conversation

jfallows commented May 8, 2026

Summary

Test plan

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jfallows left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants