diff --git a/docs/adr/035-config-subclass-without-dataclass.md b/docs/adr/035-config-subclass-without-dataclass.md new file mode 100644 index 00000000..881dc48a --- /dev/null +++ b/docs/adr/035-config-subclass-without-dataclass.md @@ -0,0 +1,50 @@ +# 035. Config subclasses work without `@dataclass` + +Date: 2026-10-23 + +## Problem + +`env()` returns a `dataclasses.Field` with a `default_factory`, so it only becomes a value when the class is processed by `@dataclass`. `Config` relies on every subclass being decorated. A plain subclass is never processed: + +```python +class DatabaseConfig(Config): + default: str = env("DB_CONNECTION", "sqlite") + +DatabaseConfig().default +``` + +`Config` has no `__init__` of its own, so the instance has no attribute and the lookup falls back to the class attribute, which is the raw `Field`. The value is never read from the environment or cast. + +## Alternatives + +1. Make `env()` return a descriptor instead of a `Field`. This breaks dataclass configs, which need a `Field` to get a per-instance `default_factory`, and changes `value()` and the `_ConfigMeta` lookups. +2. Apply `dataclass` eagerly to every subclass in `__init_subclass__`. A subclass that is also decorated is then processed twice. The first pass deletes the `default_factory` class attributes, so the second pass reads them with `getattr`, which reaches `_ConfigMeta.__getattr__` and gets the resolved value. Every `env()` field is frozen to its value at import time, and any `field(default_factory=...)` returning a dict or list fails with `ValueError: mutable default ... use default_factory`. This was checked on Python 3.15. +3. Apply `dataclass` lazily, the first time an unprocessed class is instantiated or subclassed. By then the class's own decorator, if any, has already run, so no class is processed twice. + +## Decision + +Option 3, in `support/config.py`: + +- `_ConfigMeta.__call__` processes every unprocessed class in the instantiated class's MRO, base first, before building the instance. +- `Config.__init_subclass__` processes the new class's unprocessed ancestors before its own decorator runs. A `@dataclass` child of an undecorated parent therefore inherits the parent's fields; otherwise the parent would be processed after the child and its `Field` class attributes deleted from under the child. +- A class is processed only when it is a `Config` subclass, has no `__dataclass_fields__` in its own `__dict__`, and its own `__dict__` holds a `dataclasses.Field` (an `env()` or `field()` default). Every other class is left exactly as before, so plain mutable defaults (`opts: dict = {}`), bare annotations (`name: str`) and undecorated children of frozen dataclasses keep working, and `Config` itself, which holds no `Field`, is never a dataclass. +- A skipped class in the middle of the hierarchy does not block inheritance. Its processed ancestor's `__init__` and `__dataclass_fields__` are inherited, so instantiating it resolves the ancestor's `env()` fields, and a processed descendant collects the ancestor's fields through it. The skipped class's own annotations are not dataclass fields; its plain class attributes are still read through the class. +- Automatic processing uses `dataclass(eq=False)`, so undecorated subclasses keep the identity equality and hashing they had before, and `frozen=True` when the nearest dataclass ancestor is frozen, because dataclasses forbid a non-frozen child of a frozen parent. Classes the user decorates follow normal `@dataclass` rules. +- Processing runs under a module lock with a re-check inside it, so concurrent first instantiations process a class once. + +Values are cast by the existing `env()` rules: the default's type decides the cast, so `env("DB_PORT", 5432)` returns an `int`. Decorated classes, `Config.get` dotted access and `Configuration.set` behave as before. + +A single class body that mixes `env()` fields with a plain mutable default (`opts: dict = {}`) is processed and raises the usual dataclass `ValueError`; before this change such a class returned raw `Field`s, so it never worked. Use `field(default_factory=dict)` there. + +A `@dataclass(frozen=True)` child of an undecorated `env()` parent raises `TypeError: cannot inherit frozen dataclass from a non-frozen one` at class definition, because `__init_subclass__` processes the parent as non-frozen before the child's decorator runs and cannot see it. Put `@dataclass(frozen=True)` on the parent instead. + +Class-level access (`DatabaseConfig.default`) on an undecorated leaf class that has been neither instantiated nor subclassed still returns the `Field`. No hook runs between class creation and a following decorator, so processing the leaf earlier would bring back the double processing of option 2. + +## Validation + +- Tests for an undecorated subclass: the fallback value without the variable, the variable's value when set, `int` and `bool` casting, explicit keyword arguments, and the environment being read again on every instantiation. +- Inheritance: an undecorated parent with an undecorated child, and a `@dataclass` child of an undecorated parent that keeps the parent's fields. +- `Config` is never a dataclass, undecorated subclass instances stay hashable, a decorated class is processed exactly once, and eight threads instantiating a new class at the same time process it once. +- Classes without a `Field` are untouched: an undecorated child of a frozen dataclass, a plain `dict` default and a bare annotation behave as before. An `env()` child of a frozen parent is processed as frozen. In a mixed hierarchy (`env()` base, plain middle class with a bare annotation and a dict default, `env()` leaf) only the base and leaf are processed, and the middle class still resolves the base's fields. +- The reported repro prints `'sqlite'`. +- Full suite without Postgres passes with the coverage threshold. diff --git a/docs/adr/AGENTS.md b/docs/adr/AGENTS.md index c13f7f1b..2783f14d 100644 --- a/docs/adr/AGENTS.md +++ b/docs/adr/AGENTS.md @@ -35,3 +35,4 @@ Read this index first, then open the records relevant to the change. Before impl | [029](029-cache-module.md) | 2026-10-21 | Cache module with remember, memo, locks and tags | New `fastapi_startkit.cache`: async `Cache.get/put/has/forget/flush`, `get` with a callable default, and `remember(key, ttl, factory)` that runs a sync or async factory only on a miss; `remember(key, ttl)` without a factory is a decorator keyed by `key` plus a hash of the call arguments. Stores implement an async contract with a `MISSING` sentinel (cached `None` is a hit): array, file (pickled per-key files), database (masoniteorm table) and redis (named connection from the existing `RedisManager`, `SET PX` TTL, flush by `SCAN` of prefixed keys, never `FLUSHDB`). TTL in seconds or `timedelta`, `None` forever, <=0 forgets. `CacheConfig` via `resolve_config`; `CacheProvider` binds `cache`; facade `remember` resolves lazily. `Cache.memo()` memoizes reads per asyncio task through a `ContextVar` owned by the task, so requests never share memo state. `Cache.lock(name, seconds, owner)` gives atomic, owner-checked, non-re-entrant locks with `get`, `block` (250 ms polling, `LockTimeoutError`), `release`, `force_release` and `async with`, per store: event-loop dict, `fcntl.flock` file, `lock:` row with update-if-expired, Redis `SET NX PX` plus a Lua owner-checked delete. `Cache.tags([...])` uses Laravel's namespaced keys: random tag ids at `:tag::key`, entries at `:sha1(ids):`, flush rewrites the ids; tags are sorted and deduplicated. Shipped in slices into `feature/caches`; the former memo, lock and tags ADRs (030, 031, 034) are folded in. | | [032](032-migrate-seed-option.md) | 2026-10-21 | db:migrate:fresh and db:migrate:refresh --seed run seeders in the same event loop | `--seed` called the unregistered `seed:run` through Cleo `call` (which also drops the first argument) inside a running event loop, where `db:seed` would call `asyncio.run` again. The seeder selection moves into async `DBSeedCommand.run_seeders`, which `db:seed`, `db:migrate:fresh` and `db:migrate:refresh` all await. `db:migrate:refresh` is now registered. | | [033](033-async-seeder-stub.md) | 2026-10-21 | Generated seeders have an async run | `create_seed.stub` generated a synchronous `run` with docstrings, but `Seeder` awaits `run()`, so every generated table seeder raised `TypeError`. The stub now generates `async def run(self) -> None` with no docstrings; `Seeder` is unchanged. | +| [035](035-config-subclass-without-dataclass.md) | 2026-10-23 | Config subclasses work without `@dataclass` | A `Config` subclass without `@dataclass` kept `env()`'s raw `Field` as a class attribute, so `DatabaseConfig().default` returned the `Field`. Undecorated `Config` classes whose own body holds a `Field` (an `env()` or `field()` default) now get `dataclass(eq=False)` lazily, frozen when the dataclass parent is frozen; all other classes are untouched: `_ConfigMeta.__call__` processes the MRO at first instantiation, and `Config.__init_subclass__` processes undecorated ancestors so `@dataclass` children inherit their fields. `Config` itself is never a dataclass, undecorated instances stay hashable, and a lock keeps concurrent first instantiations from processing a class twice. Eager processing was rejected because a second `@dataclass` pass freezes `env()` values and fails on mutable defaults. Class-level access on an undecorated leaf that was never instantiated or subclassed still returns the `Field`. | diff --git a/fastapi_startkit/src/fastapi_startkit/support/config.py b/fastapi_startkit/src/fastapi_startkit/support/config.py index 5bb03380..43c2fc58 100644 --- a/fastapi_startkit/src/fastapi_startkit/support/config.py +++ b/fastapi_startkit/src/fastapi_startkit/support/config.py @@ -1,17 +1,44 @@ -from dataclasses import Field, MISSING +from dataclasses import Field, MISSING, dataclass +from threading import Lock from typing import TypeVar, cast from fastapi_startkit.environment import value T = TypeVar("T") +_dataclass_lock = Lock() + class _ConfigMeta(type): def __call__(cls: type[T], *args: object, **kwargs: object) -> T: + _ConfigMeta.ensure_dataclass(cls.__mro__) resolved_args = [value(argument) for argument in args] resolved_kwargs = {name: value(argument) for name, argument in kwargs.items()} return cast(T, type.__call__(cls, *resolved_args, **resolved_kwargs)) + @staticmethod + def ensure_dataclass(classes: tuple[type, ...]) -> None: + pending = [klass for klass in reversed(classes) if _ConfigMeta.needs_dataclass(klass)] + if not pending: + return + with _dataclass_lock: + for klass in pending: + if _ConfigMeta.needs_dataclass(klass): + dataclass(eq=False, frozen=_ConfigMeta.has_frozen_base(klass))(klass) + + @staticmethod + def needs_dataclass(klass: type) -> bool: + return ( + isinstance(klass, _ConfigMeta) + and "__dataclass_fields__" not in klass.__dict__ + and any(isinstance(attribute, Field) for attribute in klass.__dict__.values()) + ) + + @staticmethod + def has_frozen_base(klass: type) -> bool: + params = getattr(klass, "__dataclass_params__", None) + return params is not None and params.frozen + def __getattr__(cls, name: str) -> object: try: fields = cast(dict[str, Field[object]], type.__getattribute__(cls, "__dataclass_fields__")) @@ -24,4 +51,6 @@ def __getattr__(cls, name: str) -> object: class Config(metaclass=_ConfigMeta): - pass + def __init_subclass__(cls, **kwargs: object) -> None: + super().__init_subclass__(**kwargs) + _ConfigMeta.ensure_dataclass(cls.__mro__[1:]) diff --git a/fastapi_startkit/tests/support/test_config_without_dataclass.py b/fastapi_startkit/tests/support/test_config_without_dataclass.py new file mode 100644 index 00000000..c00b5aa2 --- /dev/null +++ b/fastapi_startkit/tests/support/test_config_without_dataclass.py @@ -0,0 +1,216 @@ +import time +from concurrent.futures import ThreadPoolExecutor +from dataclasses import FrozenInstanceError, dataclass, field, fields +from threading import Barrier + +import pytest + +import fastapi_startkit.support.config as config_module +from fastapi_startkit.configuration import Config +from fastapi_startkit.environment import env + + +class PlainDatabaseConfig(Config): + default: str = env("PLAIN_DB_CONNECTION", "sqlite") + port: int = env("PLAIN_DB_PORT", 5432) + debug: bool = env("PLAIN_DB_DEBUG", False) + + +def test_fallback_is_returned_when_variable_is_unset(monkeypatch): + monkeypatch.delenv("PLAIN_DB_CONNECTION", raising=False) + assert PlainDatabaseConfig().default == "sqlite" + + +def test_variable_value_is_returned_when_set(monkeypatch): + monkeypatch.setenv("PLAIN_DB_CONNECTION", "pgsql") + assert PlainDatabaseConfig().default == "pgsql" + + +def test_values_are_cast_to_default_type(monkeypatch): + monkeypatch.setenv("PLAIN_DB_PORT", "6543") + monkeypatch.setenv("PLAIN_DB_DEBUG", "true") + config = PlainDatabaseConfig() + assert config.port == 6543 + assert config.debug is True + + +def test_keyword_arguments_override_environment(monkeypatch): + monkeypatch.setenv("PLAIN_DB_CONNECTION", "pgsql") + assert PlainDatabaseConfig(default="mysql").default == "mysql" + + +def test_undecorated_parent_fields_are_inherited(monkeypatch): + class Parent(Config): + host: str = env("PLAIN_PARENT_HOST", "localhost") + + class Child(Parent): + port: int = env("PLAIN_CHILD_PORT", 80) + + monkeypatch.setenv("PLAIN_PARENT_HOST", "example.test") + child = Child() + assert (child.host, child.port) == ("example.test", 80) + assert Parent().host == "example.test" + + +def test_decorated_class_is_not_processed_again(monkeypatch): + @dataclass + class Decorated(Config): + host: str = env("PLAIN_DECORATED_HOST", "localhost") + + original = Decorated.__dataclass_fields__ + monkeypatch.setenv("PLAIN_DECORATED_HOST", "example.test") + assert Decorated().host == "example.test" + assert Decorated.__dataclass_fields__ is original + assert [field.name for field in fields(Decorated)] == ["host"] + + +def test_decorated_child_inherits_undecorated_parent_fields(monkeypatch): + class Parent(Config): + host: str = env("PLAIN_PARENT_HOST", "localhost") + + @dataclass + class Child(Parent): + port: int = env("PLAIN_CHILD_PORT", 80) + + monkeypatch.setenv("PLAIN_PARENT_HOST", "example.test") + assert [field.name for field in fields(Child)] == ["host", "port"] + assert (Child().host, Child().port) == ("example.test", 80) + assert Child.host == "example.test" + + +def test_config_base_is_never_a_dataclass(): + class Plain(Config): + host: str = env("PLAIN_BASE_HOST", "localhost") + + Plain() + Config() + assert "__dataclass_fields__" not in Config.__dict__ + assert Config.__hash__ is object.__hash__ + first, second = Config(), Config() + assert first != second + assert hash(first) != hash(second) + + +def test_undecorated_subclass_instances_stay_hashable(): + class Hashable(Config): + host: str = env("PLAIN_HASH_HOST", "localhost") + + first, second = Hashable(), Hashable() + assert len({first, second, Config()}) == 3 + assert first != second + + +def test_environment_is_read_on_every_instantiation(monkeypatch): + class Live(Config): + connections: dict[str, str] = field(default_factory=lambda: {"default": "sqlite"}) + host: str = env("PLAIN_LIVE_HOST", "localhost") + + monkeypatch.setenv("PLAIN_LIVE_HOST", "first") + first = Live() + monkeypatch.setenv("PLAIN_LIVE_HOST", "second") + second = Live() + assert (first.host, second.host) == ("first", "second") + assert first.connections is not second.connections + + +def test_concurrent_first_instantiation_decorates_once(monkeypatch): + class Concurrent(Config): + connections: dict[str, str] = field(default_factory=lambda: {"default": "sqlite"}) + host: str = env("PLAIN_CONCURRENT_HOST", "localhost") + + calls: list[type] = [] + original = config_module.dataclass + + def counting_dataclass(**options: bool): + def decorate(klass: type) -> type: + calls.append(klass) + time.sleep(0.01) + return original(**options)(klass) + + return decorate + + monkeypatch.setattr(config_module, "dataclass", counting_dataclass) + barrier = Barrier(8) + + def build(_: int) -> Concurrent: + barrier.wait() + return Concurrent() + + with ThreadPoolExecutor(max_workers=8) as executor: + results = list(executor.map(build, range(8))) + + assert calls == [Concurrent] + assert all(result.host == "localhost" for result in results) + + +def test_undecorated_child_of_frozen_parent_is_left_alone(): + @dataclass(frozen=True) + class Frozen(Config): + host: str = "localhost" + + class Child(Frozen): + pass + + assert "__dataclass_fields__" not in Child.__dict__ + assert Child().host == "localhost" + + +def test_env_child_of_frozen_parent_is_frozen(monkeypatch): + @dataclass(frozen=True) + class Frozen(Config): + host: str = "localhost" + + class Child(Frozen): + port: int = env("PLAIN_FROZEN_PORT", 80) + + monkeypatch.setenv("PLAIN_FROZEN_PORT", "8080") + child = Child() + assert (child.host, child.port) == ("localhost", 8080) + with pytest.raises(FrozenInstanceError): + child.port = 1 + + +def test_plain_mutable_default_is_left_alone(): + class Options(Config): + opts: dict[str, str] = {} + + assert "__dataclass_fields__" not in Options.__dict__ + assert Options().opts == {} + + +def test_bare_annotation_is_left_alone(): + class Named(Config): + name: str + + assert "__dataclass_fields__" not in Named.__dict__ + assert not hasattr(Named(), "name") + + +def test_mixed_inheritance_processes_only_env_classes(monkeypatch): + class Base(Config): + host: str = env("PLAIN_MIXED_HOST", "localhost") + + class Middle(Base): + name: str + opts: dict[str, str] = {} + + class Leaf(Middle): + port: int = env("PLAIN_MIXED_PORT", 80) + + monkeypatch.setenv("PLAIN_MIXED_HOST", "example.test") + leaf = Leaf() + assert "__dataclass_fields__" not in Middle.__dict__ + assert [field.name for field in fields(Leaf)] == ["host", "port"] + assert (leaf.host, leaf.port, leaf.opts) == ("example.test", 80, {}) + assert Middle().host == "example.test" + + +def test_undecorated_child_without_env_inherits_resolved_parent(monkeypatch): + class Parent(Config): + host: str = env("PLAIN_INHERIT_HOST", "localhost") + + class Child(Parent): + timeout: int = 5 + + monkeypatch.setenv("PLAIN_INHERIT_HOST", "example.test") + assert (Child().host, Child().timeout) == ("example.test", 5)