Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions api/docs/adr/000-vertical-slice-architecture.md
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,8 @@ damnit_api/
├── main.py # entrypoint: env/args → Settings → create_app
├── app.py # composition root: create_app(settings), lifespan,
│ # DI wiring, exception handlers, middleware
├── state.py # AppState + factories (no domain classes)
├── settings.py # Settings models only
├── state.py # AppState + factories (no domain classes); see ADR-002
├── settings.py # Settings models only; see ADR-003
├── logging.py # structlog configuration + request-logging middleware
├── core/ # framework-free, imports nothing app-specific:
Expand Down
54 changes: 54 additions & 0 deletions api/docs/adr/002-no-global-mutable-state.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
---
date: 2026-07-07
---

# ADR-002 - No Global Mutable State: `AppState`, Factories, One Composition Root

## Context and Problem Statement

This service has a few long-lived runtime dependencies:

- application database engine and session factory
- (authenticated) clients (MyMdC, OAuth)
- per-proposal DAMNIT database accessors
- OAuth token store
- subscription cursors

These are currently implemented as module-level singletons bootstrapped at startup (`_db.__ENGINE`, `_mymdc.CLIENT`, `auth.__CLIENT`, `TOKEN_STORE`, a registry metaclass).

This is problematic as any function can reach any dependency, initialisation order becomes critical and is quite opaque, tests must mutate or clear process-wide state between cases, multiple app instances with different configurations cannot coexist in one process, in-process state looks shareable but silently breaks under multiple workers, etc...

The aim of this ADR is to consider different options for managing these dependencies.

## Considered Options

- Keep module-level singletons, initialised by startup hooks
- A single typed state container built in the application lifespan, with dependencies injected everywhere else

## Decision Outcome

Chosen option: a single frozen `AppState` dataclass built once in the lifespan. This makes initialisation typed and order-explicit (the constructor is the startup contract), lets tests inject doubles by constructing state rather than patching modules, and makes deliberately process-local state visible instead of hidden in module scope.

### Consequences

- Good: initialisation order and the full dependency set are explicit in one place
- Good: missing dependency results in construction error at startup, not a `None` at request time.
- Good: tests build `AppState` with fakes (a mock MyMdC client, an in-memory token store)
- Good: removes/reduces need to monkeypatch modules and clear cache between tests.
- Bad: (ish?) dependencies must be specified through signatures instead of imported where needed
- This is kind of the whole point, but it does mean there is more code to do the same thing (importing a global is easier/shorter)

## Details

### The rules

1. All runtime dependencies stored in a single frozen `AppState` dataclass (`state.py`) which is constructed once in the application lifespan and attached to `app.state`. This currently contains:
- App DB engine/sessionmaker, MyMdC client, OAuth client (`None` when auth is disabled), DAMNIT DB registry, token store, subscription cursors.
2. Each field is built by a pure factory function (`create_*`) taking `Settings` as explicit arguments. No factory reads module state or has side effects beyond constructing its object.
3. There is exactly one composition/setup root: the app entrypoint and its lifespan (target shape: `create_app(settings)`, see the ADR-000 layout).
- This is the only place that reads settings to select implementations.
- Handlers, resolvers, and services receive dependencies via DI or plain parameters, they must **never** import them.
<!-- TODO: add to import linting -->
4. Caches must be treated as state.
- Any cache must be owned by an object that is itself created by a factory and reachable from `AppState`.
- Module-level and class-level cache decorators on application code are banned.
36 changes: 36 additions & 0 deletions api/docs/adr/003-injected-settings.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
---
date: 2026-07-07
---

# ADR-003 - Settings: Injected Configuration, No Import-Time Singleton

## Context and Problem Statement

Configuration (auth credentials, database paths, MyMdC endpoints, local-mode selection) must be available throughout the application. There are two ways to provide it: a module-level singleton importable from anywhere, or an object constructed once and passed explicitly.

An import-time singleton has structural costs. Importing any module transitively requires a valid environment. Validation errors then fire at import, which breaks tooling, tests, and scripting contexts that never run the app. Configuration access is invisible in signatures, so nothing documents what depends on what. The application factory cannot be called twice with different configurations in one process, which blocks table-driven app tests. Modules dodge import-time failures with function-body imports, which then ossify into circular-import workarounds.

## Considered Options

- Keep the module-level `settings = Settings()` singleton, imported wherever configuration is needed
- Settings models only in `settings.py`; one instance constructed at the entrypoint and threaded explicitly through the composition root

## Decision Outcome

Chosen option: settings models only, constructed once at the entrypoint and injected, because it makes configuration dependencies visible in signatures, keeps imports environment-free, and allows multiple differently-configured app instances in one process.

### Consequences

- Good: tests build `Settings(...)` directly (pydantic-settings accepts init kwargs) and get components wired for that config - no env patching, no module reloads.
- Good: mode-dependent behaviour is forced up to the composition root, because nothing deeper can consult configuration without it showing up in a signature.
- Bad: signatures grow explicit parameters; that visibility is the point, but it is more ceremony than importing a global.

## Details

### The rules

1. `settings.py` defines models only (`Settings` and its nested models). The target state has no module-level instance.
2. `Settings` is constructed exactly once, at the entrypoint, and passed into the composition root. The composition root threads it into factories; everything else receives either the settings object or - preferably - the specific values it needs as plain parameters.
3. Environment handling: `DW_API_` prefix, `__` nested delimiter, `.env` support. Local mode is a derived property read only in the composition root.
4. Defaults must be production-safe: no paths into `tests/`, no writes into the source tree. Development conveniences belong in `.env` files and documentation, not in field defaults.
5. Verification: the `settings` instance is imported only by the composition root and tests; importing any other module with a bare environment succeeds.
4 changes: 2 additions & 2 deletions api/docs/architecture.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ For more information, see [ADR-000](adr/000-vertical-slice-architecture.md).
| `appdb/` | The app's own database (infrastructure) | Models, engine/session plumbing for `dw_api.sqlite` | Planned | `_db/` |
| `mymdc/` | MyMdC client (infrastructure) | Ports, clients, vendored models | Planned | `_mymdc/` |
| `core/` | Cross-cutting, framework-free | Shared error classes (see [ADR-001](adr/001-error-classes.md)), `DamnitType`, value types, converters | Planned | `shared/` + `utils.py` |
| `main.py` / `app.py` / `state.py` | Composition root | `AppState`, `create_*` factories, `create_app()` - the only place that may import everything and read settings | Partial | `main.py` only |
| `main.py` / `app.py` / `state.py` | Composition root | `AppState`, `create_*` factories (see [ADR-002](adr/002-no-global-mutable-state.md)), `create_app()` - the only place that may import everything and read settings | Partial | `main.py` + `state.py` |

Where new code goes:

Expand Down Expand Up @@ -55,7 +55,7 @@ The key rules are:
- Importing another package's `_underscore` name is always wrong.
2. **Composition root is the top:** it may import everything, but nothing is allowed to import it.
- If importing a slice from the composition root forces a function-body import to avoid cycles, the type probably belongs in `core/`.
3. **Composition root reads settings:** everything else receives configuration as parameters.
3. **Composition root reads settings:** everything else receives configuration as parameters (see [ADR-003](adr/003-injected-settings.md)).
4. **Authorisation applied at the edge:** routes and resolvers use dependencies and permission classes.
- This means that services should not apply authorisation rules themselves.
5. **No `if settings.is_local:` outside the composition root:** Local mode is selected by composition, not conditionals throughout the codebase.
Expand Down
16 changes: 0 additions & 16 deletions api/src/damnit_api/_db/__init__.py
Original file line number Diff line number Diff line change
@@ -1,17 +1 @@
"""Database package exports for damnit_api._db."""

from sqlalchemy.ext.asyncio import (
AsyncEngine,
async_sessionmaker,
)
from sqlmodel.ext.asyncio.session import AsyncSession

from .bootstrap import bootstrap

global __ENGINE, __SESSION_LOCAL

__ENGINE: AsyncEngine = None # pyright: ignore[reportAssignmentType]

__SESSION_LOCAL: async_sessionmaker[AsyncSession] = None # pyright: ignore[reportAssignmentType]

__all__ = ["bootstrap"]
65 changes: 0 additions & 65 deletions api/src/damnit_api/_db/bootstrap.py

This file was deleted.

10 changes: 5 additions & 5 deletions api/src/damnit_api/_db/dependencies.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,16 @@
from collections.abc import AsyncIterator
from typing import Annotated

from fastapi import Depends
from fastapi import Depends, Request
from sqlmodel.ext.asyncio.session import AsyncSession

import damnit_api._db
from ..state import get_app_state


async def get_session() -> AsyncIterator[AsyncSession]:
"""Provide a database session for FastAPI dependencies."""
async def get_session(request: Request) -> AsyncIterator[AsyncSession]:
"""Provide a database session from the application state."""

async with damnit_api._db.__SESSION_LOCAL() as session:
async with get_app_state(request).db_sessionmaker() as session:
yield session


Expand Down
15 changes: 0 additions & 15 deletions api/src/damnit_api/_mymdc/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,3 @@
This module should **not** directly expose any routes, it should **only** be used
internally by other modules to interact with the MyMdC API.
"""

from typing import TYPE_CHECKING

from .bootstrap import bootstrap as bootstrap

if TYPE_CHECKING:
from . import clients

global CLIENT

CLIENT: "clients.MyMdCClientAsync" = None # pyright: ignore[reportAssignmentType]
"""Global/singleton MyMdC client instance, configured by [`.bootstrap`]"""


__all__ = ["bootstrap"]
42 changes: 0 additions & 42 deletions api/src/damnit_api/_mymdc/bootstrap.py

This file was deleted.

13 changes: 10 additions & 3 deletions api/src/damnit_api/_mymdc/dependencies.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,15 @@
from typing import Annotated

from fastapi import Depends
from fastapi import Depends, Request

from . import clients, ports
from ..state import get_app_state
from . import clients

MyMdCClient = Annotated[clients.MyMdCClient, Depends(ports.MyMdCPort.from_global)]

def get_mymdc_client(request: Request) -> "clients.MyMdCClient":
"""Provide the MyMdC client from the application state."""
return get_app_state(request).mymdc_client


MyMdCClient = Annotated[clients.MyMdCClient, Depends(get_mymdc_client)]
"""Type alias for the MyMdC client dependency."""
14 changes: 0 additions & 14 deletions api/src/damnit_api/_mymdc/ports.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
"""MyMdC Ports (Interfaces) definitions."""

from abc import ABC, abstractmethod
from typing import TYPE_CHECKING

import async_lru

Expand All @@ -15,9 +14,6 @@
UserProposals,
)

if TYPE_CHECKING:
from . import clients

logger = get_logger()


Expand All @@ -30,16 +26,6 @@ class MyMdCPort(ABC):
added for main metadata module.
"""

@classmethod
def from_global(cls) -> "clients.MyMdCClient":
"""Create a MyMdCPort from the global client."""
from damnit_api import _mymdc

if _mymdc.CLIENT is None:
msg = "MyMdC client has not been initialized. Call bootstrap() first."
raise RuntimeError(msg)
return _mymdc.CLIENT

@abstractmethod
async def _get_proposal_by_number(self, no: ProposalNumber) -> dict: ...

Expand Down
13 changes: 1 addition & 12 deletions api/src/damnit_api/auth/__init__.py
Original file line number Diff line number Diff line change
@@ -1,14 +1,3 @@
from authlib.integrations.starlette_client import ( # type: ignore[import-untyped]
StarletteOAuth2App,
)

from .bootstrap import bootstrap as bootstrap
from .routers import noauth_router, router

global __CLIENT

__CLIENT: StarletteOAuth2App = None # type: ignore[assignment]
"""Global/singleton OAuth client instance."""


__all__ = ["bootstrap", "noauth_router", "router"]
__all__ = ["noauth_router", "router"]
Loading