diff --git a/docs/adr/0001-keep-dimiddlewarefactory.md b/docs/adr/0001-keep-dimiddlewarefactory.md index 7a6500f..80f26ef 100644 --- a/docs/adr/0001-keep-dimiddlewarefactory.md +++ b/docs/adr/0001-keep-dimiddlewarefactory.md @@ -1,46 +1,13 @@ # Keep the `_DIMiddlewareFactory` two-class split -**Decision:** Keep `_DIMiddlewareFactory` as a distinct class that binds the container and -constructs `_DiMiddleware`, rather than collapsing the two into one class, a closure, or a -`functools.partial`. - -## Why it keeps coming up - -`_DIMiddlewareFactory` reads as a shallow module: a one-method class whose `__call__` binds -`di_container` and forwards to `_DiMiddleware`. On the surface it is a pass-through worth folding -away. The deletion test appears to agree — delete it and the `setattr`-like binding simply inlines. - -The original decision (2026-06-25) kept it on a *complexity moves, not concentrates* argument: the -container must be bound ahead of FastStream's deferred middleware construction, so some carrier is -unavoidable, and a named `__slots__`-ed class is the most legible carrier. That argument leaned on -a prediction — that `functools.partial` would still need the same -`# ty: ignore[invalid-argument-type]` on its `ParamSpec` forwarding. - -## Why it survives the collapse being possible - -That prediction is now false. Typing the factory to FastStream's real -`__call__(msg, /, *, context: ContextRepo) -> _DiMiddleware` contract (#39) made the forwarding -type-clean and dropped both `ty` suppressions, so `functools.partial(_DiMiddleware, container)` was -built and measured against the named factory. Both pass `ty`, `ruff`, and the suite; `partial` is -about ten lines shorter. The original argument no longer decides. - -What decides instead is **type-checkability at the registration seam**. `functools.partial` types -as `(*args: Any, **kwargs: Any)`, which is assignable to *any* protocol. Under `partial`, renaming -`_DiMiddleware.__init__`'s `context` keyword leaves `ty` reporting `All checks passed!` and the -mismatch surfaces at runtime on the first message. With the explicit factory, the same break is a -type error at `broker.add_middleware(...)`: `_DIMiddlewareFactory` is not assignable to -`BrokerMiddleware[Any, Any]`, parameter `context` is missing. - -So the factory is not a pass-through whose complexity merely moves. It is the site where this -package's adaptation to FastStream's construction contract is asserted and checked; deleting it -deletes the check. For a package whose entire job is that adaptation, and which spent two release -cycles with the mismatch masked by `ty` suppressions, ten lines buy a real guard against silent -upstream drift. - -A closure has the same blindness as `partial`. A classmethod constructor still needs an instance to -hold `di_container`, reintroducing the state the factory already names. - -**Revisit trigger:** `functools.partial` (or the call site) gains precise enough signature typing -that a contract break is caught at `add_middleware`, **or** FastStream starts accepting a pre-bound -middleware instance so no deferred factory is needed. Either removes the factory's remaining -justification. +`_DIMiddlewareFactory` reads as a pass-through worth folding away: a one-method class whose +`__call__` binds the root container and forwards to `_DiMiddleware`. Typing it to FastStream's real +`__call__(msg, /, *, context: ContextRepo)` contract (#39) dropped the last `ty` suppressions and +made `functools.partial(_DiMiddleware, container)` viable and about ten lines shorter, so the +original "some carrier is unavoidable" argument stopped deciding. What decides instead is +type-checkability at the registration seam: `partial` types as `(*args: Any, **kwargs: Any)` and is +assignable to any protocol, so renaming `_DiMiddleware.__init__`'s `context` keyword would still +pass `ty` and fail at runtime on the first message, while the named factory turns the same break +into a type error at `broker.add_middleware(...)`. A closure is blind the same way, and a +classmethod constructor still needs an instance to hold the container. Only FastStream accepting a +pre-bound middleware instance would remove the factory's job. diff --git a/docs/adr/0002-install-middleware-on-startup.md b/docs/adr/0002-install-middleware-on-startup.md index 1291acc..034a9bf 100644 --- a/docs/adr/0002-install-middleware-on-startup.md +++ b/docs/adr/0002-install-middleware-on-startup.md @@ -1,45 +1,15 @@ # Install the DI middleware on startup, on every broker -**Decision:** `setup_di` does not call `add_middleware` itself. It registers an `on_startup` hook -that walks `app.brokers` and adds the middleware factory to each broker that does not already -carry it. - -## Why - -FastStream 0.7 apps hold a list of brokers. `FastStream(*brokers)` accepts many, `app.add_broker` -appends more after construction, and `app.broker` is only `brokers[0]`. Installing on `app.broker` -at `setup_di` time therefore left every other broker without DI, and the gap was silent: the app -started, and the first message to a subscriber on another broker failed inside `FromDI` with a -missing request container and nothing pointing at the cause -([#42](https://github.com/modern-python/modern-di-faststream/issues/42)). - -Iterating `app.brokers` inside `setup_di` fixes the construction-time case but still misses a -broker added afterwards, and the only remedy would be a documented ordering rule the user has to -remember. Startup is the one moment when the broker list is complete and no message has been -consumed yet, so installing there needs no rule. It is safe because FastStream builds a -subscriber's middleware stack per message from the broker config, so a middleware added in an -`on_startup` hook applies to subscribers registered before it. - -The membership check exists because `on_startup` runs on every start. Without it a stopped and -restarted app would carry two copies and build two request containers per message. The check -reads `broker.config.broker_middlewares`, the same sequence FastStream itself builds the stack -from, rather than a private record of installed brokers that could drift from it. - -## What changes for a reader - -Between `setup_di` and startup the middleware is not yet on any broker. Nothing in this package, -its tests, or its documentation inspects a broker in that window; `TestApp` runs the startup hooks. - -`setup_di` no longer requires a broker at call time. The original version of this decision kept -the `if not app.broker` guard; [#56](https://github.com/modern-python/modern-di-faststream/issues/56) -dropped it so that a broker created inside the user's own `on_startup` hook is covered, because the -broker list is read at startup anyway. Hooks run in registration order, so that hook must be -registered before `setup_di`; the install hook raises when the list is still empty when it runs, -naming both remedies, and the message-time error from `FromDI` names the other order. A broker -that a later hook adds while another broker already exists is the one case that still surfaces at -message time. - -**Revisit trigger:** FastStream exposes a hook for a broker being added to an app, so the -middleware can be installed at that moment instead of on startup, **or** FastStream freezes a -subscriber's middleware stack before `on_startup` runs, which would make a startup-time install -too late. +`setup_di` does not call `add_middleware`; it registers an `on_startup` hook that walks +`app.brokers` and adds the middleware factory to every broker not already carrying it. A FastStream +0.7 app holds a list of brokers and `app.broker` is only `brokers[0]`, so installing at `setup_di` +time left every other broker without DI, and the gap stayed silent until the first message failed +inside `FromDI` ([#42](https://github.com/modern-python/modern-di-faststream/issues/42)). Iterating +`app.brokers` inside `setup_di` fixes only the construction-time case and leaves a documented +ordering rule for the rest; startup is the one moment when the list is complete and no message has +been consumed, and a late install still applies because FastStream builds a subscriber's middleware +stack per message from the broker config. The membership check reads `broker.config.broker_middlewares`, the same +sequence FastStream builds from, so a restarted app does not install a second copy. +[#56](https://github.com/modern-python/modern-di-faststream/issues/56) then dropped the +`if not app.broker` guard, so a broker created in a user hook registered before `setup_di` is +covered too.