From 49183f7fb0c34d82de719ef234a4c96fabd0b5e8 Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Sat, 19 Sep 2026 14:10:59 +0300 Subject: [PATCH] docs(adr): compress to 2 records in the domain-modeling format Both records pass the admission test, so nothing is dropped or merged and the numbering is unchanged. Each is now a single paragraph: the section headings, the standalone Decision line and the revisit triggers are gone, with the part of each trigger that is the real boundary of the decision folded into the prose. 841 words to 317. --- docs/adr/0001-keep-dimiddlewarefactory.md | 55 ++++-------------- .../adr/0002-install-middleware-on-startup.md | 56 +++++-------------- 2 files changed, 24 insertions(+), 87 deletions(-) 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.