diff --git a/README.md b/README.md index 83052fc..aef4c3e 100644 --- a/README.md +++ b/README.md @@ -92,10 +92,10 @@ def my_command(ctx: typer.Context) -> None: ## API - `setup_di(app, container)` — register the container with a Typer app -- `inject` — decorator that resolves `FromDI`-annotated parameters before the command runs; also exposes `typer.Context` with `ctx.obj["di_container"]` for manual use. Raises `RuntimeError` naming `setup_di` when a command reaches it without `setup_di` called +- `inject` — decorator that resolves `FromDI`-annotated parameters before the command runs; also exposes `typer.Context`, from which `fetch_di_container(ctx)` returns the app container. Raises `RuntimeError` naming `setup_di` when a command reaches it without `setup_di` called - `FromDI(provider)` — marker used in `Annotated[T, FromDI(...)]`; accepts a provider instance or a type - `action_scope(ctx)` — context manager yielding a fresh `Scope.ACTION` container (a child of the command container); open one per action. Raises `RuntimeError` naming `@inject` when called from a command without it -- `fetch_di_container(ctx)` — returns the app-scoped container from `ctx.obj` +- `fetch_di_container(ctx)` — returns the app container registered by `setup_di`, from any command of the app, including those of nested `add_typer` sub-apps; it does not read `ctx.obj`, so the container stays reachable after a callback assigns `ctx.obj` ## Used by diff --git a/docs/adr/0001-action-scope-stays-caller-driven.md b/docs/adr/0001-action-scope-stays-caller-driven.md index a4dcb58..9c3eee7 100644 --- a/docs/adr/0001-action-scope-stays-caller-driven.md +++ b/docs/adr/0001-action-scope-stays-caller-driven.md @@ -1,32 +1,27 @@ # Action scope stays caller-driven, multiple per command -**Decision:** Reach `Scope.ACTION` dependencies through a caller-driven context manager -`action_scope(ctx)`, not by auto-resolving ACTION-scoped `FromDI` parameters into the command -signature. +`Scope.ACTION` dependencies are reached through a caller-driven context manager, `action_scope(ctx)`, +not by auto-resolving ACTION-scoped `FromDI` parameters into the command signature. -Deepening ACTION scope behind `@inject` had two shapes on the table. **Auto-injection** would have +Deepening ACTION scope behind `@inject` had two shapes on the table. Auto-injection would have `@inject` build one ACTION container per command and inject resolved ACTION parameters alongside -the REQUEST ones — symmetric with what already happens, and simple. **Caller-driven** gives the -command a named context manager that opens an ACTION container on demand. +the REQUEST ones, symmetric with what already happens. Caller-driven gives the command a named +context manager that opens an ACTION container on demand. -`Scope.ACTION` is *finer* than `Scope.REQUEST`, and its whole purpose is to allow several -action-scoped lifetimes within one command — a loop over a batch, one action per item. -Auto-injection builds a single ACTION container per command, so the injected value is one fixed -instance for the whole command: it removes the multiplicity that is the only reason ACTION sits -below REQUEST. It would silently destroy the loop case and surprise anyone using ACTION as -intended. +`Scope.ACTION` is finer than `Scope.REQUEST`, and its whole purpose is to allow several +action-scoped lifetimes within one command, one action per item of a batch. Auto-injection builds a +single ACTION container per command, so the injected value is one fixed instance for the whole +command: it removes the multiplicity that is the only reason ACTION sits below REQUEST. -Caller-driven achieves the same deepening goal — `modern-di`'s `build_child_container()` stops -appearing at every call site — without changing what ACTION means. Each `with action_scope(ctx)` -block is a fresh container. +Caller-driven achieves the same deepening goal, `build_child_container()` stops appearing at every +call site, without changing what ACTION means. Each `with action_scope(ctx)` block is a fresh +container. -Three sub-decisions were taken with it. The manager is **ctx-based**, promoting the existing -private command-container concept rather than introducing a new injection marker for an opener. It -yields the **raw `Container`**: `resolve_provider` / `resolve` is `modern-di`'s normal interface -and not a leak worth hiding — only `build_child_container` was. And the command container is -stashed on **`ctx.meta`**, which is per-invocation and isolated, rather than `ctx.obj`, which is -shared app state. +Three sub-decisions were taken with it. The manager is ctx-based, promoting the existing private +command-container concept rather than introducing a new injection marker for an opener. It yields +the raw `Container`: `resolve_provider` and `resolve` are `modern-di`'s normal interface and not a +leak worth hiding, only `build_child_container` was. And the command container is stashed on +`ctx.meta`, which is per-invocation and isolated, rather than `ctx.obj`, which is shared app state. -**Revisit trigger:** a concrete use case appears where "one action = one command" is the natural -model and the loop case is absent. At that moment auto-injection becomes worth reconsidering as an -additive option. +Revisit when a concrete use case appears where "one action = one command" is the natural model and +the loop case is absent. Auto-injection then becomes worth reconsidering as an additive option. diff --git a/docs/adr/0002-no-connection-binding-for-a-cli.md b/docs/adr/0002-no-connection-binding-for-a-cli.md index a742c12..70e84bf 100644 --- a/docs/adr/0002-no-connection-binding-for-a-cli.md +++ b/docs/adr/0002-no-connection-binding-for-a-cli.md @@ -1,27 +1,27 @@ # No connection binding: this integration uses only the resolution half of the kit -**Decision:** `modern_di_typer` composes `modern_di.integrations` for markers and resolution -(`from_di`, `parse_markers`, `resolve_markers`) and deliberately uses none of the connection-binding -half (`bind`, `classify_connection`). +`modern_di_typer` composes `modern_di.integrations` for markers and resolution (`from_di`, +`parse_markers`, `resolve_markers`) and deliberately uses none of the connection-binding half +(`bind`, `classify_connection`). The kit was designed against a survey of adapters that all share a shape: a framework hands the -adapter a per-unit-of-work object — a web `Request`, a broker `StreamMessage` — which `bind()` +adapter a per-unit-of-work object, a web `Request` or a broker `StreamMessage`, which `bind()` turns into the DI context a child container is built with. Every sibling integration uses both halves, so the standing question about this one is why it does not. A CLI invocation has no such object. The only per-invocation thing Typer produces is Click's -`typer.Context`, and `@inject` already threads that through natively — it is the parameter the +`typer.Context`, and `@inject` already threads that through natively: it is the parameter the command itself may declare, the carrier for `action_scope`, and the handle on the app container. Binding it into DI context would make the same object reachable two ways, one of them stringly -typed. Accordingly the command container is built with no `context=` argument at all, and there is -no dictionary key `bind()` could derive. +typed. So the command container is built with no `context=` argument at all, and there is no +dictionary key `bind()` could derive. That leaves the marker/parse/resolve triad, which maps one-to-one onto what this package used to hand-roll, and one wrinkle the kit has no equivalent for: finding an already-declared `typer.Context` parameter so `@inject` does not insert a redundant one. That scan stays local because it is a Typer concern, not a DI one. -**Revisit trigger:** a per-invocation object worth putting into DI context appears — a CLI session -or transaction handle that providers should resolve against, rather than something the command -reads off `ctx` itself. At that point `bind`/`classify_connection` have something to attach to and +Revisit when a per-invocation object worth putting into DI context appears, a CLI session or +transaction handle that providers should resolve against rather than something the command reads +off `ctx` itself. At that point `bind` and `classify_connection` have something to attach to and this integration should look like its siblings. diff --git a/docs/adr/0003-app-container-is-read-from-the-root-command-not-ctx-obj.md b/docs/adr/0003-app-container-is-read-from-the-root-command-not-ctx-obj.md new file mode 100644 index 0000000..3ac6fc0 --- /dev/null +++ b/docs/adr/0003-app-container-is-read-from-the-root-command-not-ctx-obj.md @@ -0,0 +1,21 @@ +# The app container is read from the root command, not from `ctx.obj` + +`setup_di` stores the app container in the Typer app's context settings, and `fetch_di_container` +reads it back from the root command those settings become, however deep in the context tree the +calling command sits. Nothing reads `ctx.obj`. + +The context settings are the only channel from a Typer app into a Click context, so the write side +has no alternative. The read side did. `ctx.obj` was the original choice, and it is Typer's +user-facing slot: assigning it in the app callback is Typer's documented way to carry user state, +and a sub-app with its own settings shadows it for every command beneath. In both cases the +container was gone by the time a command looked for it, and the error blamed a `setup_di` call +that had been made. Namespacing the key inside `ctx.obj` fixes neither, because the whole object is +replaced. + +The key keeps its old name. It is off the read path now, so renaming it would break anyone who read +it directly and buy nothing. + +Revisit when Typer grows a per-app slot that reaches the Click context without going through the +user's object, or when a single-command app whose one command declares its own context settings is +reported: Typer then builds the root command from the command's settings rather than the app's, and +this read finds nothing. diff --git a/modern_di_typer/main.py b/modern_di_typer/main.py index bb2f980..91a0228 100644 --- a/modern_di_typer/main.py +++ b/modern_di_typer/main.py @@ -9,6 +9,7 @@ T = typing.TypeVar("T") +_APP_CONTAINER_KEY: typing.Final = "di_container" _COMMAND_CONTAINER_KEY: typing.Final = "modern_di_typer.command_container" @@ -19,14 +20,14 @@ def setup_di(app: typer.Typer, container: Container) -> Container: if not app.info.context_settings: app.info.context_settings = {} obj = app.info.context_settings.get("obj") or {} - obj["di_container"] = container + obj[_APP_CONTAINER_KEY] = container app.info.context_settings["obj"] = obj return container def fetch_di_container(ctx: typer.Context) -> Container: try: - return typing.cast(Container, ctx.obj["di_container"]) + return typing.cast(Container, ctx.find_root().command.context_settings["obj"][_APP_CONTAINER_KEY]) except (TypeError, KeyError): msg = ( "No modern-di container found on the app. " diff --git a/tests/test_commands.py b/tests/test_commands.py index 6941586..672753d 100644 --- a/tests/test_commands.py +++ b/tests/test_commands.py @@ -296,3 +296,99 @@ def cmd(ctx: typer.Context) -> None: with pytest.raises(RuntimeError, match="@inject"): runner.invoke(app, catch_exceptions=False) + + +def test_nested_sub_app_resolves_from_root_setup_di() -> None: + runner = CliRunner() + root = typer.Typer() + sub = typer.Typer() + received: dict[str, typing.Any] = {} + + @sub.command() + @inject + def cmd(instance: typing.Annotated[SimpleCreator, FromDI(SimpleCreator)]) -> None: + received["instance"] = instance + + root.add_typer(sub, name="sub") + with modern_di.Container(groups=[Dependencies]) as container: + modern_di_typer.setup_di(root, container=container) + result = runner.invoke(root, ["sub", "cmd"]) + assert result.exit_code == 0, result.output + assert isinstance(received["instance"], SimpleCreator) + + +def test_nested_sub_app_action_scope_resolves_action_provider() -> None: + runner = CliRunner() + root = typer.Typer() + sub = typer.Typer() + received: dict[str, typing.Any] = {} + + @sub.command() + @inject + def cmd(ctx: typer.Context) -> None: + with action_scope(ctx) as action: + received["instance"] = action.resolve_provider(Dependencies.action_factory) + + root.add_typer(sub, name="sub") + with modern_di.Container(groups=[Dependencies]) as container: + modern_di_typer.setup_di(root, container=container) + result = runner.invoke(root, ["sub", "cmd"]) + assert result.exit_code == 0, result.output + assert isinstance(received["instance"], DependentCreator) + + +def test_nested_sub_app_with_own_obj_resolves_from_root_setup_di() -> None: + """INVARIANT: a sub-app's own ``context_settings["obj"]`` does not hide the app container. + + Broken by reading the container from ``ctx.obj``: Click gives a sub-app that declares its own + ``obj`` a context that no longer inherits the parent's, so every command under it would report + a missing ``setup_di`` that was in fact called on the root + (``docs/adr/0003-app-container-is-read-from-the-root-command-not-ctx-obj.md``). + """ + runner = CliRunner() + root = typer.Typer() + sub = typer.Typer(context_settings={"obj": {"mine": 1}}) + received: dict[str, typing.Any] = {} + + @sub.command() + @inject + def cmd(ctx: typer.Context, instance: typing.Annotated[SimpleCreator, FromDI(SimpleCreator)]) -> None: + received["obj"] = ctx.obj + received["instance"] = instance + + root.add_typer(sub, name="sub") + with modern_di.Container(groups=[Dependencies]) as container: + modern_di_typer.setup_di(root, container=container) + result = runner.invoke(root, ["sub", "cmd"]) + assert result.exit_code == 0, result.output + assert received["obj"] == {"mine": 1} + assert isinstance(received["instance"], SimpleCreator) + + +def test_callback_replacing_ctx_obj_keeps_app_container_reachable(app: typer.Typer) -> None: + """INVARIANT: ``ctx.obj`` belongs to the user; assigning it in a callback cannot lose the container. + + Broken by reading the container from ``ctx.obj``. Assigning ``ctx.obj`` in the app callback is + Typer's documented way to carry user state, and it replaces the whole object, so no key inside + it survives (``docs/adr/0003-app-container-is-read-from-the-root-command-not-ctx-obj.md``). + """ + runner = CliRunner() + received: dict[str, typing.Any] = {} + user_state = object() + + @app.callback() + def callback(ctx: typer.Context) -> None: + ctx.obj = user_state + + @app.command() + @inject + def cmd(ctx: typer.Context, instance: typing.Annotated[SimpleCreator, FromDI(SimpleCreator)]) -> None: + received["obj"] = ctx.obj + received["instance"] = instance + received["fetched"] = modern_di_typer.fetch_di_container(ctx) + + result = runner.invoke(app, ["cmd"]) + assert result.exit_code == 0, result.output + assert received["obj"] is user_state + assert isinstance(received["instance"], SimpleCreator) + assert isinstance(received["fetched"], modern_di.Container)