From 2aa3b48f51a2ab93df1e1ba078cd20128ba316c3 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Wed, 21 Oct 2026 07:03:25 +0200 Subject: [PATCH 1/9] feat(diagnostics): SM009 flags dynamic imports of plugin packages (#500) Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 --- CHANGELOG.md | 1 + docs/reference/diagnostic-codes.md | 2 +- .../diagnostics/_coupling.py | 60 ++++++++++++- .../core/tests/test_sm009_dynamic_coupling.py | 84 +++++++++++++++++++ 4 files changed, 145 insertions(+), 2 deletions(-) create mode 100644 framework/core/tests/test_sm009_dynamic_coupling.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 04062513b..8f4b8987a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ All notable changes to this project are documented in this file. The format is b - `captured_mail` pytest fixture in `simple_module_test` records sent mail. ### Changed +- SM009 now detects dynamic framework imports of plugin packages, including importlib aliases and fixed f-string package prefixes (#500). - **Behaviour change:** the `users` mailer settings (`mailer`, `smtp_*`) moved to the `mail` module as `backend` and `smtp_*`; a data migration (`d7b3a9e5c1f2`) moves stored values (#496). `simple_module_users` now depends on `simple_module_mail`, and diff --git a/docs/reference/diagnostic-codes.md b/docs/reference/diagnostic-codes.md index e04d911e3..add653111 100644 --- a/docs/reference/diagnostic-codes.md +++ b/docs/reference/diagnostic-codes.md @@ -17,7 +17,7 @@ The framework runs a set of static checks over installed modules at app boot. Th | `SM004` | WARNING | An `inertia.render("/")` call exists but no matching `.tsx` file is shipped. | Create `pages/.tsx`, or correct the render key. | | `SM007` | INFO | Module overrides zero `register_*` hooks and has no `on_startup`/`on_shutdown`. | Delete the module if it's vestigial; otherwise ignore. | | `SM008` | ERROR | Two modules declare the same `ModuleMeta.name`. Table-prefix collision. | Rename one. Remember: the name is the table-name prefix (all modules share one DB schema) and the Inertia namespace. | -| `SM009` | ERROR | A file under `framework/*` imports from any package under `modules/*`. | Invert the dependency: have the module register a callback into the framework (see `principal_serializer` pattern). | +| `SM009` | ERROR | A file under `framework/*` imports from any package under `modules/*`, including dynamic imports with literal names or fixed f-string package prefixes. | Invert the dependency: have the module register a callback into the framework (see `principal_serializer` pattern). | | `SM010` | ERROR | DB `alembic_version` is behind the migration head. | Run `make migrate`. The lifespan migration check raises and aborts boot in every environment before serving. | | `SM011` | WARNING | A module's model declares a table that doesn't appear in any Alembic migration. | Run `uv run alembic revision --autogenerate -m "..."`, review, `make migrate`. | | `SM012` | WARNING | Module overrides `register_settings` but does not assign to `app.state.`. Dev-only. | Either remove the override or move your state assignment into it. | diff --git a/framework/core/simple_module_core/diagnostics/_coupling.py b/framework/core/simple_module_core/diagnostics/_coupling.py index 4bb671639..4f2f305f3 100644 --- a/framework/core/simple_module_core/diagnostics/_coupling.py +++ b/framework/core/simple_module_core/diagnostics/_coupling.py @@ -37,6 +37,61 @@ def _imported_plugin_pkg(node: ast.AST, module_packages: dict[str, str]) -> str return None +def _importlib_aliases(tree: ast.AST) -> dict[str, str]: + aliases: dict[str, str] = {} + for node in ast.walk(tree): + if isinstance(node, ast.Import): + for alias in node.names: + if alias.name == "importlib" or alias.name.startswith("importlib."): + aliases[alias.asname or "importlib"] = ( + alias.name if alias.asname else "importlib" + ) + elif ( + isinstance(node, ast.ImportFrom) + and node.level == 0 + and node.module in {"importlib", "importlib.util"} + ): + for alias in node.names: + aliases[alias.asname or alias.name] = f"{node.module}.{alias.name}" + return aliases + + +def _dynamic_plugin_pkg( + node: ast.Call, module_packages: dict[str, str], aliases: dict[str, str] +) -> str | None: + func = node.func + attributes: list[str] = [] + while isinstance(func, ast.Attribute): + attributes.append(func.attr) + func = func.value + if not isinstance(func, ast.Name): + return None + qualified = ".".join([aliases.get(func.id, func.id), *reversed(attributes)]) + if qualified not in { + "importlib.import_module", + "importlib.util.find_spec", + "__import__", + }: + return None + name = ( + node.args[0] + if node.args + else next((kw.value for kw in node.keywords if kw.arg == "name"), None) + ) + if isinstance(name, ast.JoinedStr): + # A dot must terminate the top segment before any computed value. + name = name.values[0] if name.values else None + if not isinstance(name, ast.Constant) or not isinstance(name.value, str): + return None + if "." not in name.value: + return None + if isinstance(name, ast.Constant) and isinstance(name.value, str): + top = name.value.split(".")[0] + if top in module_packages: + return top + return None + + def check_framework_module_coupling(modules: list[ModuleBase]) -> list[Diagnostic]: """The framework (core, hosting, db) must never import from a plugin module.""" module_packages: dict[str, str] = { @@ -63,8 +118,11 @@ def check_framework_module_coupling(modules: list[ModuleBase]) -> list[Diagnosti tree = ast.parse(py_file.read_text(), filename=str(py_file)) except SyntaxError: continue + aliases = _importlib_aliases(tree) for node in ast.walk(tree): imported_pkg = _imported_plugin_pkg(node, module_packages) + if isinstance(node, ast.Call): + imported_pkg = _dynamic_plugin_pkg(node, module_packages, aliases) if imported_pkg: diags.append( Diagnostic( @@ -75,7 +133,7 @@ def check_framework_module_coupling(modules: list[ModuleBase]) -> list[Diagnosti f"from module package '{imported_pkg}'" ), module_name=module_packages[imported_pkg], - file=str(py_file), + file=f"{py_file}:{node.lineno}", suggestion=( "Use a ModuleBase lifecycle hook " "(register_middleware, register_routes, etc.) " diff --git a/framework/core/tests/test_sm009_dynamic_coupling.py b/framework/core/tests/test_sm009_dynamic_coupling.py new file mode 100644 index 000000000..7bcc78e77 --- /dev/null +++ b/framework/core/tests/test_sm009_dynamic_coupling.py @@ -0,0 +1,84 @@ +"""SM009 detects literal dynamic imports without executing framework code.""" + +from pathlib import Path + +import pytest +from simple_module_core.diagnostics import _coupling +from simple_module_core.diagnostics._types import DiagnosticLevel +from simple_module_core.module import ModuleBase, ModuleMeta + + +class _Plugin(ModuleBase): + __module__ = "sample_plugin.module" + meta = ModuleMeta(name="SamplePlugin") + + +@pytest.fixture +def framework_file(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> Path: + monkeypatch.setattr( + _coupling, + "_find_package_dir", + lambda package: tmp_path if package == "simple_module_core" else None, + ) + return tmp_path / "loader.py" + + +@pytest.mark.parametrize( + "source", + [ + "import importlib\nimportlib.import_module('sample_plugin.service')", + "import importlib as il\nil.import_module('sample_plugin')", + "from importlib import import_module\nimport_module('sample_plugin.service')", + "from importlib import import_module as im\nim('sample_plugin.service')", + "# builtin\n__import__('sample_plugin.service')", + "import importlib.util\nimportlib.util.find_spec('sample_plugin.service')", + "import importlib as il\nil.util.find_spec('sample_plugin.service')", + "import importlib\nimportlib.import_module(name='sample_plugin.service')", + "# builtin keyword\n__import__(name='sample_plugin')", + "import importlib.util\nimportlib.util.find_spec(name='sample_plugin')", + 'import importlib\nimportlib.import_module(f"sample_plugin.{suffix}")', + 'from importlib import import_module as im\nim(name=f"sample_plugin.api.{suffix}")', + "# static regression\nimport sample_plugin.service", + "# static regression\nfrom sample_plugin import service", + ], +) +def test_sm009_flags_plugin_import(framework_file: Path, source: str) -> None: + framework_file.write_text(source) + results = _coupling.check_framework_module_coupling([_Plugin()]) + assert len(results) == 1 + diagnostic = results[0] + assert diagnostic.code == "SM009" + assert diagnostic.level == DiagnosticLevel.ERROR + assert diagnostic.module_name == "SamplePlugin" + assert diagnostic.file == f"{framework_file}:2" + assert "sample_plugin" in diagnostic.message + + +@pytest.mark.parametrize( + "call", + [ + "importlib.import_module('simple_module_core.module')", + "importlib.import_module('json')", + "importlib.util.find_spec('pathlib')", + "__import__('json')", + "importlib.import_module('sample_plugin_extra.service')", + "importlib.import_module(module_name)", + "importlib.import_module('sample_plugin.' + suffix)", + 'importlib.import_module(f"{package}.service")', + 'importlib.import_module(f"sample_plugin{suffix}.service")', + "importlib.import_module()", + "importlib.import_module(123)", + "other.import_module('sample_plugin.service')", + "import_module('sample_plugin.service')", + ], +) +def test_sm009_ignores_harmless_or_computed_imports(framework_file: Path, call: str) -> None: + framework_file.write_text(f"import importlib.util\n{call}\n") + assert _coupling.check_framework_module_coupling([_Plugin()]) == [] + + +def test_sm009_skips_templates(framework_file: Path) -> None: + templates = framework_file.parent / "templates" + templates.mkdir() + (templates / "loader.py").write_text("__import__('sample_plugin')") + assert _coupling.check_framework_module_coupling([_Plugin()]) == [] From 17292b810261946aa3bed9e9606bb560b32ed926 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Wed, 21 Oct 2026 07:20:06 +0200 Subject: [PATCH 2/9] docs(spec): settings mechanism moves into the framework (#500) Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 --- ...-settings-mechanism-in-framework-design.md | 65 +++++++++++++++++++ 1 file changed, 65 insertions(+) create mode 100644 docs/superpowers/specs/2026-10-21-settings-mechanism-in-framework-design.md diff --git a/docs/superpowers/specs/2026-10-21-settings-mechanism-in-framework-design.md b/docs/superpowers/specs/2026-10-21-settings-mechanism-in-framework-design.md new file mode 100644 index 000000000..16c307bdb --- /dev/null +++ b/docs/superpowers/specs/2026-10-21-settings-mechanism-in-framework-design.md @@ -0,0 +1,65 @@ +# Settings mechanism moves into the framework (#500) + +Approved 2026-10-21. + +## Problem + +`simple_module_hosting` cannot boot without the `settings` plugin, yet reaches it +only through `importlib.import_module("settings.…")` strings and raw SQL with +hard-coded column names (`_hydrate_step.py`, `_lifespan.py`, +`_settings_registration.py`, `_preapp_config.py`, `_secret_key.py`). SM009 sees +none of it. Option 2 (import `settings` statically) is impossible: the +`settings` package already depends on `simple_module_hosting`. + +## Design + +Move the mechanism down; keep the admin UI and the write path in `settings`. + +| What | From `modules/settings/settings/` | To | +|---|---|---| +| `register_module_settings` + `ModuleSettingsRegistry` | `registration.py`, `module_registry.py` | `simple_module_core/module_settings.py` | +| Secret-field masking (pure pydantic) | `_secrets.py` | `simple_module_core/secret_fields.py` | +| Hydration (env → DB → default), `RELATED_FIELDS_CTX`, `build_settings`, field helpers | `hydrate.py` | `simple_module_core/settings_hydrate.py`; DB reads through `OverrideSource` Protocol (`async get_overrides(package) -> dict[str, tuple[str, str]]`) | +| `Setting` model, table/column/scope constants, read-only override queries (`store.py:31-63`), `get_overrides_sync`, `hydrate_settings_sync` | `models.py`, `store.py`, `constants.py` | `simple_module_db/stored_settings.py` (split if >300 lines) | + +- The module-settings registry is created lazily on `app.state` by the framework, + so registration no longer requires the `settings` module to have registered + first, and works with `settings` disabled. +- Stays in `settings`: `SettingService`, the write half of `SettingsStore` + (still satisfies `OverrideSource`), `reload`/`SettingsReloaded`, `contracts`, + `known_keys`, admin endpoints/pages/CLI. `SettingsServices.module_registry` is + the framework registry object. +- Framework call sites (`_lifespan`, `_hydrate_step`, `_settings_registration`) + import statically. `_preapp_config` and `_secret_key` take table/column names + from `simple_module_db.stored_settings` and build SQL with + `sqlalchemy.table()/column()`, not f-strings. +- Table: `settings_setting` unchanged (same name, columns, constraints, + `create_module_base("settings")` metadata name). No migration. + `build_module_metadata` and `simple_module_test/_schema.py` import + `simple_module_db.stored_settings` explicitly so the table is collected + without the plugin. `alembic check` (Postgres round trip) proves no drift. +- Back-compat: `settings.registration`, `settings.module_registry`, + `settings.hydrate`, `settings.models`, `settings._secrets`, and the moved names + in `settings.store`/`settings.constants` re-export the same objects (never a + second `Setting` class). `registration`, `module_registry`, `hydrate` emit a + module-level `DeprecationWarning`; removal no earlier than 0.1.0. In-repo code + uses the new paths. +- SM009 (`diagnostics/_coupling.py`): also flag `importlib.import_module`, + `import_module`, `__import__`, `importlib.util.find_spec` calls whose literal + first argument (or `name=`) has a plugin package as its top segment; ERROR. +- Same PR: the 7 modules using `importlib` to reach `register_module_settings` + import it statically from core; the `make new-module` templates use + `DbBackedSettings` + `register_module_settings`. + +## Not in scope + +Missing `depends_on` targets still only warn (#501). Write path, contracts and +reload stay in the plugin (no new package, no new PyPI name). + +## Risks + +- `_secret_key` treats a missing table as a fresh install; safe only because the + framework now always owns the table metadata. +- `app_builder.py` is near the 300-line cap: no new lines there. +- Out-of-tree hosts without `settings` will see autogenerate emit + `settings_setting` (CHANGELOG). From 0a02996ce462e245f0b15b6e67902d195d94117b Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Wed, 21 Oct 2026 07:20:07 +0200 Subject: [PATCH 3/9] refactor(settings): move the module-settings mechanism into the framework (#500) register_module_settings and ModuleSettingsRegistry move to simple_module_core.module_settings (registry created lazily on app.state), hydration to simple_module_core.settings_hydrate behind an OverrideSource protocol, secret-field masking to simple_module_core.secret_fields, and the Setting model, its constants, the read-only override queries and hydrate_settings_sync to simple_module_db.stored_settings. The table and its metadata owner are unchanged; no migration. The hosting layer imports these statically: the lifespan hydrates through a session-backed source, host settings always register, and the pre-app read and secret-key bootstrap build their SQL from the shared column constants. Boot works with the settings plugin disabled. build_module_metadata and the test schema collect settings_setting without the plugin. The plugin keeps its write path, admin UI and reload; its old modules re-export the moved objects (registration, module_registry and hydrate with a DeprecationWarning). Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 --- .../simple_module_core/module_settings.py | 108 ++++++++++ .../core/simple_module_core/secret_fields.py | 137 +++++++++++++ .../simple_module_core/settings_hydrate.py | 190 +++++++++++++++++ .../simple_module_core/stored_settings.py | 7 +- .../core/tests/test_core_module_settings.py | 66 ++++++ framework/db/simple_module_db/migrations.py | 4 + .../db/simple_module_db/stored_settings.py | 188 +++++++++++++++++ framework/db/tests/test_db_stored_settings.py | 85 ++++++++ .../simple_module_hosting/_hydrate_step.py | 21 +- .../simple_module_hosting/_lifespan.py | 23 +-- .../simple_module_hosting/_preapp_config.py | 90 +++------ .../simple_module_hosting/_secret_key.py | 76 +++---- .../_settings_registration.py | 25 +-- .../tests/test_boot_config_hardening.py | 34 +--- .../tests/test_preapp_read_failures.py | 7 - .../testing/simple_module_test/_schema.py | 2 + modules/settings/README.md | 4 +- modules/settings/settings/_announce.py | 3 +- modules/settings/settings/_listing.py | 9 +- modules/settings/settings/_module_settings.py | 18 +- modules/settings/settings/_row_masking.py | 5 +- modules/settings/settings/_secrets.py | 146 ++----------- modules/settings/settings/_unique_write.py | 3 +- modules/settings/settings/cli.py | 6 +- modules/settings/settings/constants.py | 58 +++--- .../settings/settings/contracts/accessor.py | 3 +- .../settings/settings/contracts/schemas.py | 10 +- modules/settings/settings/endpoints/api.py | 2 +- .../settings/settings/endpoints/module_api.py | 9 +- modules/settings/settings/hydrate.py | 191 ++---------------- modules/settings/settings/known_keys.py | 2 +- modules/settings/settings/models.py | 54 +---- modules/settings/settings/module.py | 14 +- modules/settings/settings/module_registry.py | 56 +---- modules/settings/settings/registration.py | 43 +--- modules/settings/settings/reload.py | 6 +- modules/settings/settings/service.py | 6 +- modules/settings/settings/services.py | 10 +- modules/settings/settings/store.py | 54 ++--- modules/settings/settings/tenant_view.py | 4 +- .../tests/test_clear_route_per_scope.py | 2 +- .../settings/tests/test_duplicate_and_race.py | 2 +- modules/settings/tests/test_env_pinned.py | 2 +- modules/settings/tests/test_hydrate.py | 8 +- .../tests/test_hydrate_related_fields.py | 2 +- .../test_lenient_related_fields_validators.py | 2 +- .../tests/test_module_api_json_fields.py | 2 +- .../settings/tests/test_module_registry.py | 2 +- modules/settings/tests/test_registration.py | 5 +- modules/settings/tests/test_reload.py | 2 +- .../tests/test_settings_env_unprefixed.py | 2 +- .../test_settings_framework_reexports.py | 48 +++++ .../tests/test_settings_group_metadata.py | 2 +- .../settings/tests/test_settings_schemas.py | 2 +- .../test_settings_secret_json_roundtrip.py | 4 +- .../tests/test_settings_secret_masking.py | 3 +- .../tests/test_settings_secret_urls.py | 8 +- .../settings/tests/test_settings_service.py | 2 +- .../test_settings_typed_secret_masking.py | 4 +- modules/settings/tests/test_store_filters.py | 6 +- .../tests/test_store_secret_masking.py | 6 +- .../settings/tests/test_upload_key_delete.py | 2 +- tests/test_boot_without_settings_plugin.py | 35 ++++ 63 files changed, 1154 insertions(+), 778 deletions(-) create mode 100644 framework/core/simple_module_core/module_settings.py create mode 100644 framework/core/simple_module_core/secret_fields.py create mode 100644 framework/core/simple_module_core/settings_hydrate.py create mode 100644 framework/core/tests/test_core_module_settings.py create mode 100644 framework/db/simple_module_db/stored_settings.py create mode 100644 framework/db/tests/test_db_stored_settings.py create mode 100644 modules/settings/tests/test_settings_framework_reexports.py create mode 100644 tests/test_boot_without_settings_plugin.py diff --git a/framework/core/simple_module_core/module_settings.py b/framework/core/simple_module_core/module_settings.py new file mode 100644 index 000000000..f3cd0d101 --- /dev/null +++ b/framework/core/simple_module_core/module_settings.py @@ -0,0 +1,108 @@ +"""Per-module settings classes: the registry and ``register_module_settings``. + +A module calls :func:`register_module_settings` from its ``register_settings`` +hook. Two things happen: + +1. The class is recorded in the app's :class:`ModuleSettingsRegistry`, so the + hosting lifespan can hydrate it from the settings table (env → DB → + default) before any module ``on_startup`` hook runs. +2. A fresh instance (defaults only) is passed to ``services_factory`` and the + result is mounted at ``app.state.``. + +The registry is created lazily on ``app.state`` by whoever asks for it first, +so registration does not depend on the ``settings`` plugin having run — or on +it being enabled at all (#500). The ``settings`` plugin's admin screens read +the same registry object through ``SettingsServices.module_registry``. +""" + +from __future__ import annotations + +from collections.abc import Callable +from dataclasses import dataclass, field +from typing import Any, Final + +from fastapi import FastAPI +from pydantic_settings import BaseSettings + +from simple_module_core.secret_fields import register_typed_secrets + +__all__ = [ + "MODULE_SETTINGS_STATE_ATTR", + "ModuleSettingsRegistry", + "get_module_settings_registry", + "module_settings_registry", + "register_module_settings", +] + +MODULE_SETTINGS_STATE_ATTR: Final = "module_settings_registry" +"""``app.state`` attribute holding the app's :class:`ModuleSettingsRegistry`.""" + + +@dataclass(slots=True) +class ModuleSettingsRegistry: + """In-memory map of ``package`` → ``BaseSettings`` subclass.""" + + _classes: dict[str, type[BaseSettings]] = field(default_factory=dict) + _manage_urls: dict[str, str] = field(default_factory=dict) + + def register( + self, + package: str, + cls: type[BaseSettings], + manage_url: str | None = None, + ) -> None: + if package in self._classes: + raise ValueError(f"{package!r} already registered") + self._classes[package] = cls + register_typed_secrets(package, cls) + if manage_url: + self._manage_urls[package] = manage_url + + def manage_url(self, package: str) -> str | None: + """URL of the module's own management page, if it declared one. + + Modules with a purpose-built settings screen (e.g. Branding) declare it + so the generic module-settings editor links there instead of offering a + second, raw editor for the same fields. + """ + return self._manage_urls.get(package) + + def get(self, package: str) -> type[BaseSettings] | None: + return self._classes.get(package) + + def all_packages(self) -> list[str]: + return sorted(self._classes) + + def items(self) -> list[tuple[str, type[BaseSettings]]]: + return sorted(self._classes.items()) + + +def get_module_settings_registry(app: FastAPI) -> ModuleSettingsRegistry | None: + """The app's registry, or ``None`` when nothing has registered yet.""" + return getattr(app.state, MODULE_SETTINGS_STATE_ATTR, None) + + +def module_settings_registry(app: FastAPI) -> ModuleSettingsRegistry: + """The app's registry, created on first use.""" + registry = get_module_settings_registry(app) + if registry is None: + registry = ModuleSettingsRegistry() + setattr(app.state, MODULE_SETTINGS_STATE_ATTR, registry) + return registry + + +def register_module_settings[T: BaseSettings]( + app: FastAPI, + package: str, + settings_cls: type[T], + services_factory: Callable[[T], Any], + manage_url: str | None = None, +) -> None: + """Register a module's settings class and mount its services on ``app.state``. + + ``manage_url`` points at the module's own management page when it has one; + the generic module-settings editor then links there instead of rendering a + second editor for the same fields. + """ + module_settings_registry(app).register(package, settings_cls, manage_url=manage_url) + setattr(app.state, package, services_factory(settings_cls())) diff --git a/framework/core/simple_module_core/secret_fields.py b/framework/core/simple_module_core/secret_fields.py new file mode 100644 index 000000000..b635d3d1b --- /dev/null +++ b/framework/core/simple_module_core/secret_fields.py @@ -0,0 +1,137 @@ +"""What counts as a secret on a settings field, and how one is hidden. + +The "should this value be shown?" question: a name rule, a type rule, a value +rule and one mask. Lives in the framework (#500) because the module-settings +registry records typed secrets at registration time, and that registry no +longer depends on the ``settings`` plugin being installed or enabled. +""" + +from __future__ import annotations + +import re +from typing import Any, get_args, get_origin +from urllib.parse import urlsplit + +from pydantic import BaseModel, SecretBytes, SecretStr + +# We intentionally DON'T match the bare word "token" (would mask +# `verification_token_lifetime_seconds` — just an int) or "key" alone (would +# mask `s3_bucket_key_prefix`). Only fragments that actually indicate material. +_SECRET_PATTERNS = re.compile( + r"(password|secret|api[_-]?key|private[_-]?key|token[_-]?secret)", re.I +) +SECRET_MASK = "••••••••" + +# Value types that cannot carry credential material, so a secret-ish *name* +# on one of them is a false positive rather than something to hide. +_NEVER_SECRET_TYPES = frozenset({"int", "float", "bool"}) + + +# ``.`` keys whose declared type holds a ``SecretStr`` / +# ``SecretBytes``. Filled by ``ModuleSettingsRegistry.register`` so the row +# browser, tenant view and known-keys list — which see only a key and a stored +# string — mask them too. Only ever grows; keys are package-qualified. +_TYPED_SECRET_KEYS: set[str] = set() + + +def annotation_holds_secret(annotation: Any, _seen: frozenset[type] = frozenset()) -> bool: + """True if a ``SecretStr``/``SecretBytes`` sits anywhere in ``annotation``. + + Walks unions (``SecretStr | None``), containers (``dict[str, SecretStr]``) + and nested models. Since json fields store the real secret (GH #487 + review), a field typed this way must be masked whatever its name. + """ + if get_origin(annotation) is None and isinstance(annotation, type): + if issubclass(annotation, SecretStr | SecretBytes): + return True + if issubclass(annotation, BaseModel) and annotation not in _seen: + seen = _seen | {annotation} + return any( + annotation_holds_secret(f.annotation, seen) + for f in annotation.model_fields.values() + ) + return any(annotation_holds_secret(arg, _seen) for arg in get_args(annotation)) + + +def register_typed_secrets(package: str, cls: type[BaseModel]) -> None: + """Record which of ``cls``'s fields are secrets by type.""" + _TYPED_SECRET_KEYS.update( + f"{package}.{name}" + for name, info in cls.model_fields.items() + if annotation_holds_secret(info.annotation) + ) + + +def is_secret_field(name: str) -> bool: + """True if a field name suggests it holds credential material.""" + return bool(_SECRET_PATTERNS.search(name)) + + +def is_named_secret(name: str, value_type: str | None = None) -> bool: + """True if the *name* marks this field as credential material. + + ``value_type`` exempts the types that cannot hold one, so a numeric field + whose name merely contains a secret-ish word is not masked and made + uneditable — ``reset_password_token_lifetime_seconds`` is an int, but it + matches on "password" exactly as the real secrets do. Phrased as "exempt + the types that cannot hold a credential" rather than "mask only strings" so + the failure direction is safe: an unexpected type (``str | None`` resolves + to "json") stays masked instead of silently exposing a secret. + """ + if value_type in _NEVER_SECRET_TYPES: + return False + return is_secret_field(name) + + +def conceals_secret(name: str, value: Any, value_type: str | None = None) -> bool: + """True if this name/value pair must be masked on the way out. + + The one rule every read path shares: mask per *type* (a registered + ``.`` declared as a secret), per *value* and per name, + so a DSN carrying a password is hidden even on a field called + ``broker_url``, while that field's password-free default is still shown. + """ + return ( + name in _TYPED_SECRET_KEYS or is_named_secret(name, value_type) or embeds_credential(value) + ) + + +def embeds_credential(value: Any) -> bool: + """True if ``value`` carries a password in a URL authority. + + The name rule alone cannot see these: ``broker_url``, ``result_backend``, + ``redis_url`` and ``database_url`` match nothing in + :data:`_SECRET_PATTERNS`, yet a production DSN is precisely where the + Redis and Postgres passwords live — and those fields are ``env_readable``, + so the value reached both the module editor and the ``known_keys`` + suggestion list in clear text. + + Judged on the value rather than the name because the name is the thing + that was wrong. A DSN without a password stays visible: hiding + ``redis://localhost:6379/0`` helps nobody debug why the queue is idle. + + Containers are walked rather than dismissed: a ``list[str]`` of broker + URLs or a ``dict`` of per-tenant DSNs holds exactly the same material as + the bare string, and returning False for anything non-``str`` meant one + such field would have been shown in full. + """ + if isinstance(value, str): + if "://" not in value: + return False + try: + return bool(urlsplit(value).password) + except ValueError: + # A malformed authority (an unclosed IPv6 literal, say) is not a + # credential, but it must not take the settings screen down either. + return False + if isinstance(value, (list, tuple, set, frozenset)): + return any(embeds_credential(item) for item in value) + if isinstance(value, dict): + return any(embeds_credential(item) for item in value.values()) + return False + + +def mask(value: Any) -> Any: + if value in (None, "", [], {}): + return value + return SECRET_MASK diff --git a/framework/core/simple_module_core/settings_hydrate.py b/framework/core/simple_module_core/settings_hydrate.py new file mode 100644 index 000000000..87d4e6682 --- /dev/null +++ b/framework/core/simple_module_core/settings_hydrate.py @@ -0,0 +1,190 @@ +"""Resolve a module's settings class: env → DB overrides → pydantic defaults. + +A field's declared Python type maps to one of the five ``value_type`` labels +the settings table stores (``string | bool | int | float | json``). The +hydrator reads overrides from an :class:`OverrideSource`, parses each according +to its stored ``value_type`` and constructs the class, so pydantic enforces +field validators and any ``@model_validator`` hooks. + +Lives in the framework (#500): the hosting lifespan hydrates every registered +class at boot whether or not the ``settings`` plugin is enabled. The DB read +itself is behind :class:`OverrideSource`, which ``simple_module_db`` (direct +session reads) and the ``settings`` plugin (``SettingsStore``) both satisfy; +the sync worker twin, ``hydrate_settings_sync``, lives in +``simple_module_db.stored_settings`` because it needs a SQLAlchemy session. +""" + +from __future__ import annotations + +import logging +import types +from typing import Any, Literal, Protocol, Union, get_args, get_origin + +from pydantic import BaseModel +from pydantic_settings import ( + BaseSettings, + DotEnvSettingsSource, + EnvSettingsSource, + InitSettingsSource, + SecretsSettingsSource, +) + +from simple_module_core.stored_settings import ( + RELATED_FIELDS_CTX, + construct_leniently, + parse_stored_value, +) + +logger = logging.getLogger(__name__) + +_SCALAR_TYPES = {bool: "bool", int: "int", float: "float"} + +# Re-exported: modules raise cross-field errors with it (``background_tasks``). +__all__ = [ + "RELATED_FIELDS_CTX", + "OverrideSource", + "build_settings", + "env_supplied_fields", + "hydrate_settings", + "input_type_for_field", + "value_type_for_field", +] + + +class OverrideSource(Protocol): + """Where stored overrides come from: ``{field_name: (raw, value_type)}``.""" + + async def get_overrides(self, package: str) -> dict[str, tuple[str, str]]: ... + + +def value_type_for_field(cls: type[BaseSettings], field_name: str) -> str: + """Return the ``value_type`` label for a field based on its annotation. + + - ``bool`` → ``"bool"`` + - ``int`` → ``"int"`` + - ``float`` → ``"float"`` + - ``str``, enums and ``Literal`` of strings → ``"string"`` + - ``list``, ``dict``, and other container types → ``"json"`` + """ + info = cls.model_fields[field_name] + ann = info.annotation + origin = get_origin(ann) + if origin is Literal and all(isinstance(a, str) for a in get_args(ann)): + return "string" + if origin is not None: + return "json" + return _SCALAR_TYPES.get(ann, "string") + + +def _holds_structure(ann: object) -> bool: + """A bare ``dict``/``list``/model: not editable as a single-line string.""" + return isinstance(ann, type) and issubclass( + ann, dict | list | set | frozenset | tuple | BaseModel + ) + + +def input_type_for_field(cls: type[BaseSettings], field_name: str) -> tuple[str, bool]: + """``(input_type, nullable)`` — which editor control a field needs. + + Storage stays on :func:`value_type_for_field`, which files every union + under ``json``. That is right for the store and wrong for the form: + ``int | None`` wants a number box where empty means ``None``, not a JSON + textarea that shows the literal text ``null`` (GH #487). + """ + ann = cls.model_fields[field_name].annotation + if get_origin(ann) not in (Union, types.UnionType): + return value_type_for_field(cls, field_name), False + args = get_args(ann) + rest = [a for a in args if a is not type(None)] + nullable = len(rest) < len(args) + if len(rest) == 1 and get_origin(rest[0]) is None: + only = rest[0] + if _holds_structure(only): + return "json", nullable + return _SCALAR_TYPES.get(only, "string"), nullable + return "json", nullable + + +def env_supplied_fields(cls: type[BaseSettings]) -> frozenset[str]: + """Fields the class's own non-init sources (env, ``.env``, secrets) answer. + + Asks the sources ``cls`` actually installs, so a ``DbBackedSettings`` + (init-only) reports nothing and an ``env_prefix`` class reports exactly the + ```` names that are set. Deliberately not ``cls()``: a + model validator can reject the env-only view (a production broker check) + that the DB-merged one would pass. + + A class with no ``env_prefix`` reports nothing from its sources: a plain + ``BaseSettings`` reads bare, case-insensitive names, so a field called + ``user``, ``home`` or ``path`` would be pinned by the process environment + and every admin edit refused. Only its ``env_seeded_fields`` count. + """ + found: set[str] = set() + if cls.model_config.get("env_prefix"): + found.update(_source_supplied_fields(cls)) + # A class may also read env through a ``default_factory`` (one shared + # variable seeding several fields), which no source can see. It names + # those fields itself, so env still beats the DB for them. + seeded = getattr(cls, "env_seeded_fields", None) + if callable(seeded): + found.update(seeded()) + return frozenset(found) & cls.model_fields.keys() + + +def _source_supplied_fields(cls: type[BaseSettings]) -> set[str]: + sources = cls.settings_customise_sources( + cls, + init_settings=InitSettingsSource(cls, init_kwargs={}), + env_settings=EnvSettingsSource(cls), + dotenv_settings=DotEnvSettingsSource(cls), + file_secret_settings=SecretsSettingsSource(cls), + ) + by_key = {(info.alias or name): name for name, info in cls.model_fields.items()} + found: set[str] = set() + for source in sources: + if isinstance(source, InitSettingsSource): + continue + found.update(by_key.get(key, key) for key in source()) + return found + + +def build_settings[T: BaseSettings]( + cls: type[T], raw_overrides: dict[str, tuple[str, str]], *, lenient: bool = False +) -> T: + """Construct ``cls`` from ``{field: (raw, value_type)}``, skipping unknown fields. + + Shared by the async and sync hydrators so parsing cannot drift. + + Strict by default: an invalid stored value raises, which is what a settings + write relies on to answer 422. ``lenient=True`` is for *boot* paths (web + lifespan, workers): a row that was stored before a validator tightened must + not take the app down, nor discard the module's other, valid overrides. Each + offending field is logged and ignored (it falls back to env/default). + + Precedence is env → DB → default (GH #486). pydantic-settings ranks init + kwargs above its env source, so a stored override for a field the + environment sets is dropped here rather than passed in. + """ + from_env = env_supplied_fields(cls) + parsed: dict[str, Any] = {} + for field_name, (raw, vtype) in raw_overrides.items(): + if field_name not in cls.model_fields or field_name in from_env: + continue + try: + parsed[field_name] = parse_stored_value(raw, vtype) + except ValueError: + if not lenient: + raise + logger.error( + "Ignoring stored %s.%s: cannot parse as %s", cls.__name__, field_name, vtype + ) + if not lenient: + return cls(**parsed) + return construct_leniently(cls, parsed, level=logging.ERROR) + + +async def hydrate_settings[T: BaseSettings]( + cls: type[T], source: OverrideSource, package: str, *, lenient: bool = False +) -> T: + """Construct ``cls`` with ``source``'s overrides merged over pydantic defaults.""" + return build_settings(cls, await source.get_overrides(package), lenient=lenient) diff --git a/framework/core/simple_module_core/stored_settings.py b/framework/core/simple_module_core/stored_settings.py index 93a80bad2..eda078517 100644 --- a/framework/core/simple_module_core/stored_settings.py +++ b/framework/core/simple_module_core/stored_settings.py @@ -2,10 +2,9 @@ Shared by the two places that turn ``settings_setting`` rows into a settings object: the hosting layer's pre-app read (``_preapp_config``), which runs -before any module is importable as a plugin, and the settings module's -hydrator (``settings.hydrate``). It lives in the framework because the -framework may not import a plugin (SM009), while a plugin may import the -framework — so one copy here keeps the two parsers from drifting. +before any module is importable as a plugin, and the module-settings +hydrator (``simple_module_core.settings_hydrate``). One copy keeps the two +parsers from drifting. """ from __future__ import annotations diff --git a/framework/core/tests/test_core_module_settings.py b/framework/core/tests/test_core_module_settings.py new file mode 100644 index 000000000..06998592c --- /dev/null +++ b/framework/core/tests/test_core_module_settings.py @@ -0,0 +1,66 @@ +"""The module-settings registry lives in the framework (#500). + +Registration must not depend on the ``settings`` plugin having run first, or +being enabled at all: the registry is created on ``app.state`` on first use. +""" + +from __future__ import annotations + +import pytest +from fastapi import FastAPI +from pydantic import SecretStr +from simple_module_core.module_settings import ( + MODULE_SETTINGS_STATE_ATTR, + ModuleSettingsRegistry, + get_module_settings_registry, + module_settings_registry, + register_module_settings, +) +from simple_module_core.secret_fields import conceals_secret +from simple_module_core.settings_base import DbBackedSettings + + +class _OrdersSettings(DbBackedSettings): + page_size: int = 20 + webhook_signing: SecretStr | None = None + + +class _Services: + def __init__(self, settings: _OrdersSettings) -> None: + self.settings = settings + + +def test_registry_is_created_lazily_on_app_state() -> None: + app = FastAPI() + assert get_module_settings_registry(app) is None + + registry = module_settings_registry(app) + + assert isinstance(registry, ModuleSettingsRegistry) + assert getattr(app.state, MODULE_SETTINGS_STATE_ATTR) is registry + assert module_settings_registry(app) is registry + + +def test_register_without_the_settings_plugin_mounts_services() -> None: + app = FastAPI() + + register_module_settings(app, "orders", _OrdersSettings, _Services, manage_url="/orders/cfg") + + registry = get_module_settings_registry(app) + assert registry is not None + assert registry.get("orders") is _OrdersSettings + assert registry.manage_url("orders") == "/orders/cfg" + assert app.state.orders.settings.page_size == 20 + + +def test_duplicate_registration_is_refused() -> None: + app = FastAPI() + register_module_settings(app, "orders", _OrdersSettings, _Services) + with pytest.raises(ValueError, match="orders"): + register_module_settings(app, "orders", _OrdersSettings, _Services) + + +def test_typed_secret_fields_are_recorded_for_masking() -> None: + ModuleSettingsRegistry().register("orders_typed", _OrdersSettings) + assert conceals_secret("orders_typed.webhook_signing", "plain") + assert not conceals_secret("orders_typed.page_size", 20) diff --git a/framework/db/simple_module_db/migrations.py b/framework/db/simple_module_db/migrations.py index ee790c234..9b83a3778 100644 --- a/framework/db/simple_module_db/migrations.py +++ b/framework/db/simple_module_db/migrations.py @@ -68,6 +68,10 @@ def build_module_metadata(modules: Sequence[ModuleBase] | None = None) -> MetaDa """ if modules is None: modules = discover_modules() + # The framework owns ``settings_setting`` (#500): collect it even when the + # ``settings`` plugin is not installed, since the host reads it at boot. + import simple_module_db.stored_settings # noqa: F401 + for mod in modules: pkg = get_module_package_name(mod) try: diff --git a/framework/db/simple_module_db/stored_settings.py b/framework/db/simple_module_db/stored_settings.py new file mode 100644 index 000000000..8aa8da956 --- /dev/null +++ b/framework/db/simple_module_db/stored_settings.py @@ -0,0 +1,188 @@ +"""The ``settings_setting`` table and its read-only override queries. + +Owned by the framework (#500) because the hosting layer reads this table on +every boot — host overrides before the app is built, the session-signing key, +and each registered module's settings in the lifespan — whether or not the +``settings`` plugin is enabled. The plugin keeps the write path and the admin +UI; ``settings.models.Setting`` is this same class, re-exported. + +The table keeps ``create_module_base("settings")`` as its metadata owner, so +its name, columns and constraints are exactly what the migrations created. +""" + +from __future__ import annotations + +from collections.abc import Iterable +from typing import Final + +from pydantic_settings import BaseSettings +from simple_module_core.settings_hydrate import build_settings +from sqlalchemy import Select, UniqueConstraint, inspect, select +from sqlalchemy.ext.asyncio import AsyncSession +from sqlalchemy.orm import Session +from sqlalchemy.sql.expression import TableClause, column, table +from sqlmodel import Field + +from simple_module_db.base import create_module_base +from simple_module_db.mixins import AuditMixin + +# ── Table identity ─────────────────────────────────────────────────── +SETTINGS_METADATA_NAME: Final = "settings" +TABLE_SETTING: Final = "settings_setting" +UQ_SCOPE_KEY: Final = "uq_settings_setting_scope_scope_id_key" +COL_SCOPE: Final = "scope" +COL_SCOPE_ID: Final = "scope_id" +COL_KEY: Final = "key" +COL_VALUE: Final = "value" +COL_VALUE_TYPE: Final = "value_type" + +# ── Scopes ─────────────────────────────────────────────────────────── +# Precedence high → low when resolving a key: USER > TENANT > SYSTEM. +SCOPE_SYSTEM: Final = "system" +SCOPE_TENANT: Final = "tenant" +SCOPE_USER: Final = "user" +# Empty string (not NULL) for SYSTEM so the composite unique key works the +# same on SQLite and PostgreSQL (NULL breaks uniqueness on PG). +SYSTEM_SCOPE_ID: Final = "" + +# ── Value types ────────────────────────────────────────────────────── +# Values are always stored as strings; ``value_type`` says how to read them. +VALUE_TYPE_STRING: Final = "string" +VALUE_TYPE_BOOL: Final = "bool" +VALUE_TYPE_INT: Final = "int" +VALUE_TYPE_FLOAT: Final = "float" +VALUE_TYPE_JSON: Final = "json" +ALL_VALUE_TYPES: Final = ( + VALUE_TYPE_STRING, + VALUE_TYPE_BOOL, + VALUE_TYPE_INT, + VALUE_TYPE_FLOAT, + VALUE_TYPE_JSON, +) + +# ── Field limits ───────────────────────────────────────────────────── +KEY_MAX_LENGTH: Final = 200 +VALUE_MAX_LENGTH: Final = 4000 +DESCRIPTION_MAX_LENGTH: Final = 2000 +SCOPE_MAX_LENGTH: Final = 10 +SCOPE_ID_MAX_LENGTH: Final = 255 +VALUE_TYPE_MAX_LENGTH: Final = 10 + +Base = create_module_base(SETTINGS_METADATA_NAME) + + +class Setting(Base, AuditMixin, table=True): # ty: ignore[unsupported-base] + """A typed key/value configuration entry scoped to system, tenant, or user. + + Resolution precedence when a consumer asks for a key: USER > TENANT > + SYSTEM. Uniqueness is enforced across the (scope, scope_id, key) tuple + so the same key can live at different scopes without collision. + + ``value`` is always stored as a string. ``value_type`` advertises how + the bytes should be interpreted ("string", "bool", "int", "float", + "json") so UIs pick the right input control and pydantic can reject + inputs that don't parse. + """ + + __tablename__ = TABLE_SETTING + __table_args__ = (UniqueConstraint(COL_SCOPE, COL_SCOPE_ID, COL_KEY, name=UQ_SCOPE_KEY),) + + id: int | None = Field(default=None, primary_key=True) + scope: str = Field(default=SCOPE_SYSTEM, max_length=SCOPE_MAX_LENGTH, index=True) + scope_id: str = Field(default=SYSTEM_SCOPE_ID, max_length=SCOPE_ID_MAX_LENGTH, index=True) + key: str = Field(max_length=KEY_MAX_LENGTH, index=True) + value: str = Field(max_length=VALUE_MAX_LENGTH) + value_type: str = Field(default=VALUE_TYPE_STRING, max_length=VALUE_TYPE_MAX_LENGTH) + description: str | None = Field(default=None, max_length=DESCRIPTION_MAX_LENGTH) + + +def settings_table_clause() -> TableClause: + """A bare ``settings_setting`` clause for code that runs before any ORM setup. + + The pre-app host read and the secret-key bootstrap use their own + short-lived engines; a lightweight clause keeps their SQL to exactly the + columns they touch, with the names taken from the constants above. + """ + return table( + TABLE_SETTING, + column(COL_SCOPE), + column(COL_SCOPE_ID), + column(COL_KEY), + column(COL_VALUE), + column(COL_VALUE_TYPE), + ) + + +def package_overrides( + rows: Iterable[tuple[str, str, str]], package: str +) -> dict[str, tuple[str, str]]: + """Map ``(key, value, value_type)`` rows to ``{field_name: (raw, value_type)}``. + + The one place the ``.`` key format is interpreted. Nested + keys (``pkg.a.b``) belong to a sub-object, not a top-level field. + """ + prefix = f"{package}." + out: dict[str, tuple[str, str]] = {} + for key, value, value_type in rows: + if not key.startswith(prefix): + continue + field_name = key[len(prefix) :] + if "." in field_name: + continue + out[field_name] = (value, value_type) + return out + + +def _overrides_stmt(package: str) -> Select: + return select(Setting.key, Setting.value, Setting.value_type).where( + Setting.scope == SCOPE_SYSTEM, + Setting.scope_id == SYSTEM_SCOPE_ID, + Setting.key.startswith(f"{package}.", autoescape=True), + ) + + +def get_overrides_sync(session: Session, package: str) -> dict[str, tuple[str, str]]: + """SYSTEM-scope overrides for ``package`` through a plain sync ``Session``. + + Unmasked: this feeds live settings objects, not a screen. + """ + return package_overrides(session.execute(_overrides_stmt(package)).tuples(), package) + + +async def get_overrides(session: AsyncSession, package: str) -> dict[str, tuple[str, str]]: + """Async twin of :func:`get_overrides_sync`.""" + return package_overrides((await session.execute(_overrides_stmt(package))).tuples(), package) + + +async def settings_table_exists(session: AsyncSession) -> bool: + """Whether the settings table has been created (false on a fresh install).""" + conn = await session.connection() + return await conn.run_sync(lambda sync: inspect(sync).has_table(TABLE_SETTING)) + + +class SessionOverrideSource: + """An ``OverrideSource`` reading straight from the settings table. + + What the hosting lifespan hydrates through, so boot-time hydration needs + no plugin. The ``settings`` plugin's ``SettingsStore`` satisfies the same + protocol for its own reload path. + """ + + def __init__(self, session: AsyncSession) -> None: + self._session = session + + async def get_overrides(self, package: str) -> dict[str, tuple[str, str]]: + return await get_overrides(self._session, package) + + +def hydrate_settings_sync[T: BaseSettings]( + cls: type[T], session: Session, package: str, *, lenient: bool = False +) -> T: + """Sync ``hydrate_settings`` for Celery workers and other non-async code. + + Workers never run the hosting lifespan, so a bare ``cls()`` there returns + pydantic defaults (``DbBackedSettings`` ignores the environment on purpose). + Takes a plain sync ``Session`` and applies the same SYSTEM-scope overrides + with the same ``value_type`` parsing as the web process. + """ + return build_settings(cls, get_overrides_sync(session, package), lenient=lenient) diff --git a/framework/db/tests/test_db_stored_settings.py b/framework/db/tests/test_db_stored_settings.py new file mode 100644 index 000000000..17d8d74f1 --- /dev/null +++ b/framework/db/tests/test_db_stored_settings.py @@ -0,0 +1,85 @@ +"""The framework-owned settings table and its read-only override queries (#500).""" + +from __future__ import annotations + +from collections.abc import AsyncGenerator + +import pytest +from simple_module_core.settings_base import DbBackedSettings +from simple_module_core.settings_hydrate import hydrate_settings +from simple_module_db.stored_settings import ( + SCOPE_SYSTEM, + SCOPE_TENANT, + TABLE_SETTING, + UQ_SCOPE_KEY, + SessionOverrideSource, + Setting, + get_overrides, + hydrate_settings_sync, + settings_table_exists, +) +from sqlalchemy import create_engine +from sqlalchemy.ext.asyncio import AsyncSession, create_async_engine +from sqlalchemy.orm import Session + + +class _Cfg(DbBackedSettings): + limit: int = 1 + label: str = "x" + + +def _rows() -> list[Setting]: + return [ + Setting(key="orders.limit", value="5", value_type="int"), + Setting(key="orders.nested.field", value="1", value_type="int"), + Setting(key="other.limit", value="9", value_type="int"), + Setting( + scope=SCOPE_TENANT, scope_id="t1", key="orders.label", value="y", value_type="string" + ), + ] + + +@pytest.fixture +async def session() -> AsyncGenerator[AsyncSession, None]: + engine = create_async_engine("sqlite+aiosqlite:///:memory:") + async with engine.begin() as conn: + await conn.run_sync(Setting.metadata.create_all, tables=[Setting.__table__]) + async with AsyncSession(engine) as s: + s.add_all(_rows()) + await s.commit() + yield s + await engine.dispose() + + +def test_table_identity_is_unchanged() -> None: + table = Setting.__table__ + assert table.name == TABLE_SETTING + assert {c.name for c in table.constraints if c.name} >= {UQ_SCOPE_KEY} + assert table.metadata is not None + + +async def test_get_overrides_reads_only_the_packages_system_rows(session: AsyncSession) -> None: + assert await get_overrides(session, "orders") == {"limit": ("5", "int")} + + +async def test_session_source_hydrates_without_the_plugin(session: AsyncSession) -> None: + cfg = await hydrate_settings(_Cfg, SessionOverrideSource(session), "orders") + assert (cfg.limit, cfg.label) == (5, "x") + assert await settings_table_exists(session) + + +async def test_missing_table_is_reported() -> None: + engine = create_async_engine("sqlite+aiosqlite:///:memory:") + async with AsyncSession(engine) as s: + assert not await settings_table_exists(s) + await engine.dispose() + + +def test_sync_hydration_for_workers() -> None: + engine = create_engine("sqlite://") + Setting.metadata.create_all(engine, tables=[Setting.__table__]) + with Session(engine) as s: + s.add(Setting(scope=SCOPE_SYSTEM, key="orders.limit", value="7", value_type="int")) + s.commit() + assert hydrate_settings_sync(_Cfg, s, "orders").limit == 7 + engine.dispose() diff --git a/framework/hosting/simple_module_hosting/_hydrate_step.py b/framework/hosting/simple_module_hosting/_hydrate_step.py index e07f9527d..134d7e056 100644 --- a/framework/hosting/simple_module_hosting/_hydrate_step.py +++ b/framework/hosting/simple_module_hosting/_hydrate_step.py @@ -1,24 +1,25 @@ """Hydrate every registered module's settings from the DB at lifespan start. Runs before any module ``on_startup`` hook so startup code sees DB-backed -values. ``importlib`` is used to resolve plugin names lazily so the -framework→plugin AST check (SM009) stays clean. +values. Everything it needs lives in the framework (#500): the registry in +``simple_module_core.module_settings``, the hydrator in +``simple_module_core.settings_hydrate``; the caller supplies the override +source, so this works with the ``settings`` plugin disabled. """ from __future__ import annotations -import importlib import logging from typing import Any from fastapi import FastAPI +from simple_module_core.module_settings import get_module_settings_registry +from simple_module_core.settings_hydrate import OverrideSource, hydrate_settings logger = logging.getLogger(__name__) -_MODULE_PACKAGE = "settings" - -async def hydrate_all(app: FastAPI, store: Any, session: Any = None) -> None: +async def hydrate_all(app: FastAPI, store: OverrideSource, session: Any = None) -> None: """Resolve every registered module's settings from the DB. ``hydrate_settings`` is already lenient per field: a stored value that no @@ -31,13 +32,11 @@ async def hydrate_all(app: FastAPI, store: Any, session: Any = None) -> None: Postgres transaction aborted, and every later module's read on the same session would then fail too, so it is rolled back before moving on. """ - settings_services = getattr(app.state, _MODULE_PACKAGE, None) - if settings_services is None: + registry = get_module_settings_registry(app) + if registry is None: return - hydrate_settings = importlib.import_module("settings.hydrate").hydrate_settings - - for package, cls in settings_services.module_registry.items(): + for package, cls in registry.items(): try: hydrated = await hydrate_settings(cls, store, package, lenient=True) except Exception: diff --git a/framework/hosting/simple_module_hosting/_lifespan.py b/framework/hosting/simple_module_hosting/_lifespan.py index 53abeb88d..d50a4aaa7 100644 --- a/framework/hosting/simple_module_hosting/_lifespan.py +++ b/framework/hosting/simple_module_hosting/_lifespan.py @@ -19,7 +19,10 @@ from contextlib import asynccontextmanager from fastapi import FastAPI +from simple_module_core.module_settings import get_module_settings_registry +from simple_module_db.stored_settings import SessionOverrideSource, settings_table_exists +from simple_module_hosting._hydrate_step import hydrate_all from simple_module_hosting._route_guard import guard_included_routers from simple_module_hosting.migrations import migration_status from simple_module_hosting.setup_gate import STEP_MIGRATIONS @@ -30,27 +33,23 @@ async def hydrate_settings_from_db(app: FastAPI) -> None: """Merge DB-stored overrides into every registered settings object. - Resolved through ``importlib`` rather than a static import because the - SM009 coupling check is AST-based and forbids ``framework/*`` from naming - a plugin package. No-ops when the settings module isn't installed. + Reads the settings table directly (``simple_module_db.stored_settings``), + so it works whether or not the ``settings`` plugin is enabled (#500). A + missing table is a fresh install: nothing is stored yet, so there is + nothing to merge. Note this is the *second* settings read of a boot. The first happens in ``_preapp_config`` before ``create_app`` builds anything, because the module list, i18n registry and middleware stack are all constructed from settings and cannot be rebuilt from here. """ - if not hasattr(app.state, "settings"): + if get_module_settings_registry(app) is None: return - import importlib - - from simple_module_hosting._hydrate_step import hydrate_all - - service_cls = importlib.import_module("settings.service").SettingService - store_cls = importlib.import_module("settings.store").SettingsStore - async with app.state.sm.db.session_factory() as session: - await hydrate_all(app, store_cls(service_cls(session)), session) + if not await settings_table_exists(session): + return + await hydrate_all(app, SessionOverrideSource(session), session) async def _is_first_run(app: FastAPI) -> bool: diff --git a/framework/hosting/simple_module_hosting/_preapp_config.py b/framework/hosting/simple_module_hosting/_preapp_config.py index 7eb5dfc80..71191359f 100644 --- a/framework/hosting/simple_module_hosting/_preapp_config.py +++ b/framework/hosting/simple_module_hosting/_preapp_config.py @@ -31,7 +31,19 @@ from simple_module_core.dotenv import parse_dotenv from simple_module_core.environments import NON_PROD_ENVIRONMENTS from simple_module_core.stored_settings import construct_leniently, parse_stored_value -from sqlalchemy import inspect, text +from simple_module_db.stored_settings import ( + COL_KEY, + COL_SCOPE, + COL_SCOPE_ID, + COL_VALUE, + COL_VALUE_TYPE, + SCOPE_SYSTEM, + SYSTEM_SCOPE_ID, + TABLE_SETTING, + package_overrides, + settings_table_clause, +) +from sqlalchemy import inspect, select from sqlalchemy.ext.asyncio import create_async_engine from sqlalchemy.pool import NullPool @@ -48,60 +60,26 @@ class HostSettingsUnavailableError(RuntimeError): """The stored host settings exist but could not be read before boot. - Raised only outside development, and only when the settings module is - installed. Building with defaults instead would silently drop every + Raised only outside development. Building with defaults instead would silently drop every build-time override — ``multi_tenant`` above all, so one worker that booted during a DB blip would serve requests single-tenant, with no tenant middleware and no strict filter, while its siblings stay isolated (#494). """ -def _settings_table() -> tuple[str, str, str] | None: - """Resolve the settings module's table name and system-scope markers. - - Resolved through ``importlib`` rather than a static import for the same - reason as ``_settings_registration.register_host_settings``: the SM009 coupling - check is AST-based and forbids ``framework/*`` from naming a plugin - package. Returns ``None`` when the settings module isn't installed, which - makes the whole pre-app read a no-op rather than an error. - - Only a *missing* package means "not installed". Any other failure — the - package is there but raises on import, or lacks a constant — propagates: - swallowing it would let a broken install boot on defaults as though it - had never stored anything (#494). - """ - import importlib - - try: - constants = importlib.import_module("settings.constants") - except ModuleNotFoundError as exc: - if exc.name in ("settings", "settings.constants"): - return None - raise - return ( - constants.TABLE_SETTING, - constants.SCOPE_SYSTEM, - constants.SYSTEM_SCOPE_ID, - ) - - async def _read(database_url: str) -> dict[str, tuple[str, str]]: """Select the ``host.*`` system-scoped overrides. - Returns ``{}`` when the settings module isn't installed or its table does - not exist yet (a fresh install). Anything else — the connect, the - existence check, the select — raises, so the caller can tell an expected - empty result from a failure. - """ - info = _settings_table() - if info is None: - return {} - table, scope, scope_id = info + Returns ``{}`` when the settings table does not exist yet (a fresh + install). Anything else — the connect, the existence check, the select — + raises, so the caller can tell an expected empty result from a failure. - # The table name comes from a module constant, never from user input, so - # interpolating it is safe — SQL identifiers cannot be bound parameters. - stmt = text( - f"SELECT key, value, value_type FROM {table} WHERE scope = :scope AND scope_id = :scope_id" + The table belongs to the framework (``simple_module_db.stored_settings``, + #500), so this read no longer depends on the ``settings`` plugin. + """ + t = settings_table_clause() + stmt = select(t.c[COL_KEY], t.c[COL_VALUE], t.c[COL_VALUE_TYPE]).where( + t.c[COL_SCOPE] == SCOPE_SYSTEM, t.c[COL_SCOPE_ID] == SYSTEM_SCOPE_ID ) engine = create_async_engine(database_url, poolclass=NullPool) @@ -111,31 +89,19 @@ async def _read(database_url: str) -> dict[str, tuple[str, str]]: # table" is a fresh install and must stay quiet, while a refused # connection or a failing query is not, and the driver errors for # the two are not reliably distinguishable across backends. - if not await conn.run_sync(lambda sync: inspect(sync).has_table(table)): + if not await conn.run_sync(lambda sync: inspect(sync).has_table(TABLE_SETTING)): return {} - rows = (await conn.execute(stmt, {"scope": scope, "scope_id": scope_id})).all() + rows = (await conn.execute(stmt)).tuples().all() finally: await engine.dispose() - - prefix = f"{HOST_PACKAGE}." - out: dict[str, tuple[str, str]] = {} - for key, value, value_type in rows: - if not key.startswith(prefix): - continue - field = key[len(prefix) :] - # Nested keys belong to a sub-object, not a top-level field. Same - # rule the settings store applies in get_overrides. - if "." in field: - continue - out[field] = (value, value_type) - return out + return package_overrides(rows, HOST_PACKAGE) def load_host_overrides(database_url: str, *, strict: bool = False) -> dict[str, tuple[str, str]]: """Return ``{field: (raw_value, value_type)}`` for host settings in the DB. - Returns ``{}`` quietly when the settings module isn't installed, or when - the database is reachable but not yet migrated or holds no rows. Those are + Returns ``{}`` quietly when the database is reachable but not yet + migrated or holds no rows. Those are the ordinary states of a fresh install, and what the setup wizard repairs. A failure to connect or to query is different: the stored overrides may diff --git a/framework/hosting/simple_module_hosting/_secret_key.py b/framework/hosting/simple_module_hosting/_secret_key.py index 363af9c64..ce7aea072 100644 --- a/framework/hosting/simple_module_hosting/_secret_key.py +++ b/framework/hosting/simple_module_hosting/_secret_key.py @@ -19,12 +19,24 @@ import secrets from concurrent.futures import ThreadPoolExecutor -from sqlalchemy import inspect, text +from simple_module_db.stored_settings import ( + COL_KEY, + COL_SCOPE, + COL_SCOPE_ID, + COL_VALUE, + COL_VALUE_TYPE, + SCOPE_SYSTEM, + SYSTEM_SCOPE_ID, + TABLE_SETTING, + VALUE_TYPE_STRING, + settings_table_clause, +) +from sqlalchemy import and_, insert, inspect, select, update from sqlalchemy.exc import IntegrityError from sqlalchemy.ext.asyncio import create_async_engine from sqlalchemy.pool import NullPool -from simple_module_hosting._preapp_config import HOST_PACKAGE, _settings_table +from simple_module_hosting._preapp_config import HOST_PACKAGE logger = logging.getLogger(__name__) @@ -61,16 +73,17 @@ def assert_not_placeholder(settings) -> None: ) -async def _fetch(conn, table: str, scope: str, scope_id: str) -> str | None: - row = ( - await conn.execute( - text( - f"SELECT value FROM {table} WHERE scope = :scope " - "AND scope_id = :scope_id AND key = :key" - ), - {"scope": scope, "scope_id": scope_id, "key": _KEY}, - ) - ).first() +_T = settings_table_clause() +# The one row this module reads and writes: SYSTEM scope, key ``host.secret_key``. +_ROW = and_( + _T.c[COL_SCOPE] == SCOPE_SYSTEM, + _T.c[COL_SCOPE_ID] == SYSTEM_SCOPE_ID, + _T.c[COL_KEY] == _KEY, +) + + +async def _fetch(conn) -> str | None: + row = (await conn.execute(select(_T.c[COL_VALUE]).where(_ROW))).first() return row[0] if row else None @@ -88,17 +101,12 @@ async def _ensure(database_url: str) -> str | None: ``None`` means one thing only: the settings table does not exist yet, i.e. a confirmed fresh install. Every other failure raises. """ - info = _settings_table() - if info is None: - return None - table, scope, scope_id = info - engine = create_async_engine(database_url, poolclass=NullPool) try: async with engine.connect() as conn: - if not await conn.run_sync(lambda sync: inspect(sync).has_table(table)): + if not await conn.run_sync(lambda sync: inspect(sync).has_table(TABLE_SETTING)): return None - existing = await _fetch(conn, table, scope, scope_id) + existing = await _fetch(conn) if existing: return existing @@ -116,31 +124,31 @@ async def _ensure(database_url: str) -> str | None: # contextlib.suppress is a sync context manager and does not implement # the async protocol, so `async with engine.begin(), suppress(...)` # raises TypeError at runtime. - params = {"scope": scope, "scope_id": scope_id, "key": _KEY, "value": _generate()} + value = _generate() async with engine.begin() as conn: if existing == "": # A blank row would otherwise defeat the insert forever and # leave every boot on an ephemeral key. Repair it in place; the # ``value = ''`` guard lets exactly one concurrent worker win. await conn.execute( - text( # table name is a module constant, never user input - f"UPDATE {table} SET value = :value WHERE scope = :scope " - "AND scope_id = :scope_id AND key = :key AND value = ''" - ), - params, + update(_T).where(_ROW, _T.c[COL_VALUE] == "").values({COL_VALUE: value}) ) else: with contextlib.suppress(IntegrityError): await conn.execute( - text( # table name is a module constant, never user input - f"INSERT INTO {table} (scope, scope_id, key, value, value_type) " - "VALUES (:scope, :scope_id, :key, :value, 'string')" - ), - params, + insert(_T).values( + { + COL_SCOPE: SCOPE_SYSTEM, + COL_SCOPE_ID: SYSTEM_SCOPE_ID, + COL_KEY: _KEY, + COL_VALUE: value, + COL_VALUE_TYPE: VALUE_TYPE_STRING, + } + ) ) async with engine.connect() as conn: - stored = await _fetch(conn, table, scope, scope_id) + stored = await _fetch(conn) finally: await engine.dispose() if not stored: @@ -165,7 +173,7 @@ def ensure_secret_key( Anywhere else an ephemeral key means each worker signs with its own and rejects its siblings' sessions. So with ``strict`` (outside development) a - failed lookup or save, or no settings module to store the key in, raises + failed lookup or save raises :class:`SecretKeyUnavailableError`; without it, it warns and falls back. ``stored`` is the value the caller already read from the settings table, @@ -180,10 +188,6 @@ def ensure_secret_key( return stored try: - if _settings_table() is None: - raise RuntimeError( - "the settings module is not installed, so there is nowhere to store it" - ) with ThreadPoolExecutor(max_workers=1) as pool: resolved = pool.submit(lambda: asyncio.run(_ensure(database_url))).result() except Exception as exc: diff --git a/framework/hosting/simple_module_hosting/_settings_registration.py b/framework/hosting/simple_module_hosting/_settings_registration.py index 57a3094b4..9e505f602 100644 --- a/framework/hosting/simple_module_hosting/_settings_registration.py +++ b/framework/hosting/simple_module_hosting/_settings_registration.py @@ -4,35 +4,20 @@ from fastapi import FastAPI from simple_module_core.diagnostics import Diagnostic, DiagnosticLevel +from simple_module_core.module_settings import register_module_settings from simple_module_hosting._host_services import _HostServices +from simple_module_hosting._preapp_config import HOST_PACKAGE from simple_module_hosting.host_settings import HostSettings def register_host_settings(app: FastAPI) -> None: """Register host-level settings under ``package="host"`` (DB-backed). - The Settings module must already have run ``register_settings`` — topo - order puts it early, since its ``meta.depends_on`` is empty. When the - Settings module isn't enabled there's no registry to register against, so - this skips quietly. - - ``settings.registration`` is resolved via importlib rather than a plain - ``from settings.registration import ...``: the SM009 coupling check is - AST-based and forbids any static import of a plugin package name from - within ``framework/*``. Dynamic resolution keeps the framework AST - plugin-free while still hitting the real helper at runtime. + The registry belongs to the framework (#500), so this no longer depends on + the ``settings`` plugin having registered first, or being enabled at all. """ - if not hasattr(app.state, "settings"): - return - - import importlib - - register_module_settings = importlib.import_module( - "settings.registration" - ).register_module_settings - - register_module_settings(app, "host", HostSettings, lambda s: _HostServices(settings=s)) + register_module_settings(app, HOST_PACKAGE, HostSettings, lambda s: _HostServices(settings=s)) def check_settings_registration(app: FastAPI, modules: list) -> list[Diagnostic]: diff --git a/framework/hosting/tests/test_boot_config_hardening.py b/framework/hosting/tests/test_boot_config_hardening.py index 2d2ce829b..f36a9d06d 100644 --- a/framework/hosting/tests/test_boot_config_hardening.py +++ b/framework/hosting/tests/test_boot_config_hardening.py @@ -1,15 +1,13 @@ """Boot-config paths that used to fail open or crash (follow-up to #494/#497). Each case here once turned an error into a silent default on an established -install: a typo'd boolean read as ``False``, a broken settings import read as -"not installed", a failed key lookup minted a per-worker key, a failed module -hydrate poisoned the session for every later module, and a raising setup -check reopened the setup wizard on a live system. +install: a typo'd boolean read as ``False``, a failed key lookup minted a +per-worker key, a failed module hydrate poisoned the session for every later +module, and a raising setup check reopened the setup wizard on a live system. """ from __future__ import annotations -import importlib import logging import sqlite3 from pathlib import Path @@ -25,9 +23,7 @@ from simple_module_hosting._hydrate_step import hydrate_all from simple_module_hosting._lifespan import _is_first_run from simple_module_hosting._preapp_config import ( - HostSettingsUnavailableError, apply_host_overrides, - load_host_overrides, ) from simple_module_hosting._secret_key import SecretKeyUnavailableError, ensure_secret_key from simple_module_hosting.setup_gate import STEP_MIGRATIONS @@ -53,22 +49,6 @@ def test_typo_bool_is_dropped_not_coerced_to_false(caplog: pytest.LogCaptureFixt assert "multi_tenant" in caplog.text -# ── 2. A broken settings import is a failure, not "not installed" ── - - -def test_settings_import_error_is_a_read_failure(monkeypatch: pytest.MonkeyPatch) -> None: - real = importlib.import_module - - def broken(name: str, *args, **kwargs): - if name == "settings.constants": - raise ImportError("circular import in settings") - return real(name, *args, **kwargs) - - monkeypatch.setattr(importlib, "import_module", broken) - with pytest.raises(HostSettingsUnavailableError): - load_host_overrides("sqlite+aiosqlite:///:memory:", strict=True) - - # ── 3. Ephemeral secret keys only on a confirmed fresh install ── @@ -92,12 +72,6 @@ def test_fresh_install_may_use_an_ephemeral_key_when_strict(tmp_path: Path) -> N assert ensure_secret_key(url, strict=True) -def test_no_settings_module_refuses_when_strict(monkeypatch: pytest.MonkeyPatch) -> None: - monkeypatch.setattr(_secret_key, "_settings_table", lambda: None) - with pytest.raises(SecretKeyUnavailableError, match="SM_SECRET_KEY"): - ensure_secret_key("sqlite+aiosqlite:///:memory:", strict=True) - - def test_blank_stored_key_is_repaired_once(tmp_path: Path) -> None: """A blank row used to defeat the insert forever: an ephemeral key per boot.""" db = tmp_path / "blank.db" @@ -134,7 +108,7 @@ async def get_overrides(self, package: str) -> dict[str, tuple[str, str]]: registry = SimpleNamespace(items=lambda: [("a", _A), ("b", _B)]) a_before = _A() state = SimpleNamespace( - settings=SimpleNamespace(module_registry=registry), + module_settings_registry=registry, a=SimpleNamespace(settings=a_before), b=SimpleNamespace(settings=_B()), ) diff --git a/framework/hosting/tests/test_preapp_read_failures.py b/framework/hosting/tests/test_preapp_read_failures.py index db880c6b5..68be3c7e4 100644 --- a/framework/hosting/tests/test_preapp_read_failures.py +++ b/framework/hosting/tests/test_preapp_read_failures.py @@ -78,10 +78,3 @@ def test_unreachable_postgres_refuses_boot_outside_development() -> None: url = "postgresql+asyncpg://nobody:nobody@127.0.0.1:1/nothing" with pytest.raises(HostSettingsUnavailableError): merge_host_settings(_bootstrap(url, "production")) - - -def test_no_settings_module_never_refuses(monkeypatch: pytest.MonkeyPatch) -> None: - """Without the settings module there is nothing stored to lose.""" - monkeypatch.setattr(_preapp_config, "_settings_table", lambda: None) - url = "postgresql+asyncpg://nobody:nobody@127.0.0.1:1/nothing" - assert load_host_overrides(url, strict=True) == {} diff --git a/framework/testing/simple_module_test/_schema.py b/framework/testing/simple_module_test/_schema.py index f420c7f40..5408508bc 100644 --- a/framework/testing/simple_module_test/_schema.py +++ b/framework/testing/simple_module_test/_schema.py @@ -19,6 +19,8 @@ @lru_cache(maxsize=1) def _ensure_models_imported() -> list: """Import all module models so all_module_bases is populated (cached).""" + import simple_module_db.stored_settings # noqa: F401 framework-owned table (#500) + for mod in discover_modules(): pkg = type(mod).__module__.split(".")[0] with contextlib.suppress(ModuleNotFoundError): diff --git a/modules/settings/README.md b/modules/settings/README.md index 09a1e2050..f04634afa 100644 --- a/modules/settings/README.md +++ b/modules/settings/README.md @@ -26,7 +26,7 @@ class OrdersModule(ModuleBase): meta = ModuleMeta(name="orders") def register_settings(self, app: FastAPI) -> None: - from settings.registration import register_module_settings + from simple_module_core.module_settings import register_module_settings from orders.settings import OrdersSettings from orders.services import OrdersServices @@ -35,7 +35,7 @@ class OrdersModule(ModuleBase): ) ``` -That surfaces the module's settings fields under `/settings/modules`, hydrated from the DB store at startup. The settings class is a `pydantic_settings.BaseSettings` with the module's `SM__*` env-var defaults. +That surfaces the module's settings fields under `/admin/settings/`, hydrated from the DB store at startup. The settings class is normally a `simple_module_core.settings_base.DbBackedSettings` (DB is the only source). Registration and hydration live in the framework (#500); this module adds the admin editor, hot reload and the key/value API. `settings.registration`, `settings.module_registry` and `settings.hydrate` are deprecated re-exports. ## Depends on diff --git a/modules/settings/settings/_announce.py b/modules/settings/settings/_announce.py index 8db64c1f9..f2b5eabb9 100644 --- a/modules/settings/settings/_announce.py +++ b/modules/settings/settings/_announce.py @@ -9,8 +9,9 @@ from typing import TYPE_CHECKING from simple_module_db.callbacks import register_on_commit +from simple_module_db.stored_settings import SCOPE_SYSTEM, SCOPE_TENANT -from settings.constants import INVALIDATION_CHANNEL, SCOPE_SYSTEM, SCOPE_TENANT +from settings.constants import INVALIDATION_CHANNEL from settings.contracts.invalidation import invalidation_key if TYPE_CHECKING: diff --git a/modules/settings/settings/_listing.py b/modules/settings/settings/_listing.py index b28bac760..908a224f9 100644 --- a/modules/settings/settings/_listing.py +++ b/modules/settings/settings/_listing.py @@ -3,18 +3,13 @@ from __future__ import annotations from simple_module_db import LIKE_ESCAPE_CHAR, like_contains_pattern +from simple_module_db.stored_settings import SYSTEM_SCOPE_ID, Setting from sqlalchemy import func, select from sqlalchemy.ext.asyncio import AsyncSession from settings._row_masking import out -from settings.constants import ( - ALL_SCOPES, - DEFAULT_PER_PAGE, - SCOPE_ALL, - SYSTEM_SCOPE_ID, -) +from settings.constants import ALL_SCOPES, DEFAULT_PER_PAGE, SCOPE_ALL from settings.contracts.schemas import SettingOut, SettingScope -from settings.models import Setting class SettingListing: diff --git a/modules/settings/settings/_module_settings.py b/modules/settings/settings/_module_settings.py index 0dc509ddf..c40700fdc 100644 --- a/modules/settings/settings/_module_settings.py +++ b/modules/settings/settings/_module_settings.py @@ -12,11 +12,10 @@ from fastapi import FastAPI from pydantic_settings import BaseSettings - -from settings._field_meta import choices_for, env_readable_var, resolve_default +from simple_module_core.module_settings import get_module_settings_registry # Re-exported: callers reach for ``SECRET_MASK`` / ``is_secret_field`` here. -from settings._secrets import ( # noqa: F401 +from simple_module_core.secret_fields import ( # noqa: F401 _NEVER_SECRET_TYPES, SECRET_MASK, annotation_holds_secret, @@ -26,8 +25,14 @@ is_secret_field, mask, ) +from simple_module_core.settings_hydrate import ( + env_supplied_fields, + input_type_for_field, + value_type_for_field, +) + +from settings._field_meta import choices_for, env_readable_var, resolve_default from settings.env_vars import env_prefix_for -from settings.hydrate import env_supplied_fields, input_type_for_field, value_type_for_field from settings.service import SettingService @@ -199,7 +204,7 @@ def collect_module_settings( """Return a sorted, serializable view of every module's BaseSettings. Folds in both ``app.state.sm.modules`` (plugin modules) and additional - packages registered via ``app.state.settings.module_registry`` (e.g. + packages registered in the framework's module-settings registry (e.g. ``"host"``) that aren't backed by a ``ModuleBase`` instance. ``overrides`` maps package -> field names carrying a stored override. It @@ -210,8 +215,7 @@ def collect_module_settings( views: list[ModuleSettingsView] = [] seen: set[str] = set() - settings_services = getattr(app.state, "settings", None) - registry = getattr(settings_services, "module_registry", None) + registry = get_module_settings_registry(app) def _manage_url(package: str) -> str | None: return registry.manage_url(package) if registry is not None else None diff --git a/modules/settings/settings/_row_masking.py b/modules/settings/settings/_row_masking.py index b33f73ff5..e2190e993 100644 --- a/modules/settings/settings/_row_masking.py +++ b/modules/settings/settings/_row_masking.py @@ -9,10 +9,11 @@ from __future__ import annotations -from settings._secrets import conceals_secret +from simple_module_core.secret_fields import conceals_secret +from simple_module_db.stored_settings import Setting + from settings.constants import SENSITIVE_KEYS, SENSITIVE_PLACEHOLDER from settings.contracts.schemas import SettingOut -from settings.models import Setting def is_masked(entity: Setting) -> bool: diff --git a/modules/settings/settings/_secrets.py b/modules/settings/settings/_secrets.py index a29fb967a..8e1724d9b 100644 --- a/modules/settings/settings/_secrets.py +++ b/modules/settings/settings/_secrets.py @@ -1,137 +1,17 @@ -"""What counts as a secret on a settings field, and how one is hidden. - -Split out of ``_module_settings`` so the "should this value be shown?" -question — a name rule, a value rule and one mask — lives in one file and can -be reasoned about without a FastAPI app in scope. Re-exported from -``_module_settings``, which is where the rest of the codebase imports it from. -""" +"""Re-export of :mod:`simple_module_core.secret_fields` (moved there in #500).""" from __future__ import annotations -import re -from typing import Any, get_args, get_origin -from urllib.parse import urlsplit - -from pydantic import BaseModel, SecretBytes, SecretStr - -# We intentionally DON'T match the bare word "token" (would mask -# `verification_token_lifetime_seconds` — just an int) or "key" alone (would -# mask `s3_bucket_key_prefix`). Only fragments that actually indicate material. -_SECRET_PATTERNS = re.compile( - r"(password|secret|api[_-]?key|private[_-]?key|token[_-]?secret)", re.I +from simple_module_core.secret_fields import ( # noqa: F401 + _NEVER_SECRET_TYPES, + _SECRET_PATTERNS, + _TYPED_SECRET_KEYS, + SECRET_MASK, + annotation_holds_secret, + conceals_secret, + embeds_credential, + is_named_secret, + is_secret_field, + mask, + register_typed_secrets, ) -SECRET_MASK = "••••••••" - -# Value types that cannot carry credential material, so a secret-ish *name* -# on one of them is a false positive rather than something to hide. -_NEVER_SECRET_TYPES = frozenset({"int", "float", "bool"}) - - -# ``.`` keys whose declared type holds a ``SecretStr`` / -# ``SecretBytes``. Filled by ``ModuleSettingsRegistry.register`` so the row -# browser, tenant view and known-keys list — which see only a key and a stored -# string — mask them too. Only ever grows; keys are package-qualified. -_TYPED_SECRET_KEYS: set[str] = set() - - -def annotation_holds_secret(annotation: Any, _seen: frozenset[type] = frozenset()) -> bool: - """True if a ``SecretStr``/``SecretBytes`` sits anywhere in ``annotation``. - - Walks unions (``SecretStr | None``), containers (``dict[str, SecretStr]``) - and nested models. Since json fields store the real secret (GH #487 - review), a field typed this way must be masked whatever its name. - """ - if get_origin(annotation) is None and isinstance(annotation, type): - if issubclass(annotation, SecretStr | SecretBytes): - return True - if issubclass(annotation, BaseModel) and annotation not in _seen: - seen = _seen | {annotation} - return any( - annotation_holds_secret(f.annotation, seen) - for f in annotation.model_fields.values() - ) - return any(annotation_holds_secret(arg, _seen) for arg in get_args(annotation)) - - -def register_typed_secrets(package: str, cls: type[BaseModel]) -> None: - """Record which of ``cls``'s fields are secrets by type.""" - _TYPED_SECRET_KEYS.update( - f"{package}.{name}" - for name, info in cls.model_fields.items() - if annotation_holds_secret(info.annotation) - ) - - -def is_secret_field(name: str) -> bool: - """True if a field name suggests it holds credential material.""" - return bool(_SECRET_PATTERNS.search(name)) - - -def is_named_secret(name: str, value_type: str | None = None) -> bool: - """True if the *name* marks this field as credential material. - - ``value_type`` exempts the types that cannot hold one, so a numeric field - whose name merely contains a secret-ish word is not masked and made - uneditable — ``reset_password_token_lifetime_seconds`` is an int, but it - matches on "password" exactly as the real secrets do. Phrased as "exempt - the types that cannot hold a credential" rather than "mask only strings" so - the failure direction is safe: an unexpected type (``str | None`` resolves - to "json") stays masked instead of silently exposing a secret. - """ - if value_type in _NEVER_SECRET_TYPES: - return False - return is_secret_field(name) - - -def conceals_secret(name: str, value: Any, value_type: str | None = None) -> bool: - """True if this name/value pair must be masked on the way out. - - The one rule every read path shares: mask per *type* (a registered - ``.`` declared as a secret), per *value* and per name, - so a DSN carrying a password is hidden even on a field called - ``broker_url``, while that field's password-free default is still shown. - """ - return ( - name in _TYPED_SECRET_KEYS or is_named_secret(name, value_type) or embeds_credential(value) - ) - - -def embeds_credential(value: Any) -> bool: - """True if ``value`` carries a password in a URL authority. - - The name rule alone cannot see these: ``broker_url``, ``result_backend``, - ``redis_url`` and ``database_url`` match nothing in - :data:`_SECRET_PATTERNS`, yet a production DSN is precisely where the - Redis and Postgres passwords live — and those fields are ``env_readable``, - so the value reached both the module editor and the ``known_keys`` - suggestion list in clear text. - - Judged on the value rather than the name because the name is the thing - that was wrong. A DSN without a password stays visible: hiding - ``redis://localhost:6379/0`` helps nobody debug why the queue is idle. - - Containers are walked rather than dismissed: a ``list[str]`` of broker - URLs or a ``dict`` of per-tenant DSNs holds exactly the same material as - the bare string, and returning False for anything non-``str`` meant one - such field would have been shown in full. - """ - if isinstance(value, str): - if "://" not in value: - return False - try: - return bool(urlsplit(value).password) - except ValueError: - # A malformed authority (an unclosed IPv6 literal, say) is not a - # credential, but it must not take the settings screen down either. - return False - if isinstance(value, (list, tuple, set, frozenset)): - return any(embeds_credential(item) for item in value) - if isinstance(value, dict): - return any(embeds_credential(item) for item in value.values()) - return False - - -def mask(value: Any) -> Any: - if value in (None, "", [], {}): - return value - return SECRET_MASK diff --git a/modules/settings/settings/_unique_write.py b/modules/settings/settings/_unique_write.py index 8dc3c0b72..65e6cb3a6 100644 --- a/modules/settings/settings/_unique_write.py +++ b/modules/settings/settings/_unique_write.py @@ -8,11 +8,10 @@ from __future__ import annotations +from simple_module_db.stored_settings import Setting from sqlalchemy.exc import IntegrityError from sqlalchemy.ext.asyncio import AsyncSession -from settings.models import Setting - class DuplicateSettingError(Exception): """A row with this (scope, scope_id, key) already exists.""" diff --git a/modules/settings/settings/cli.py b/modules/settings/settings/cli.py index 1ee675eaa..80fcfb397 100644 --- a/modules/settings/settings/cli.py +++ b/modules/settings/settings/cli.py @@ -14,10 +14,10 @@ import typer from fastapi import FastAPI from simple_module_core.dotenv import find_env_file, parse_dotenv +from simple_module_core.module_settings import module_settings_registry +from simple_module_core.settings_hydrate import value_type_for_field -from settings.constants import MODULE_PACKAGE from settings.env_vars import env_prefix_for -from settings.hydrate import value_type_for_field from settings.store import SettingsStore app = typer.Typer(help="Settings module administration.", no_args_is_help=True) @@ -35,7 +35,7 @@ async def import_from_env_impl( """ if environ is None: environ = {**parse_dotenv(find_env_file()), **os.environ} - registry = getattr(app_inst.state, MODULE_PACKAGE).module_registry + registry = module_settings_registry(app_inst) count = 0 for package, cls in registry.items(): prefix = env_prefix_for(package) diff --git a/modules/settings/settings/constants.py b/modules/settings/settings/constants.py index ded7d293a..7f7c1777d 100644 --- a/modules/settings/settings/constants.py +++ b/modules/settings/settings/constants.py @@ -10,6 +10,28 @@ from typing import Final +# Re-exported (#500): these now live with the table in the framework. +from simple_module_db.stored_settings import ( # noqa: F401 + ALL_VALUE_TYPES, + DESCRIPTION_MAX_LENGTH, + KEY_MAX_LENGTH, + SCOPE_ID_MAX_LENGTH, + SCOPE_MAX_LENGTH, + SCOPE_SYSTEM, + SCOPE_TENANT, + SCOPE_USER, + SYSTEM_SCOPE_ID, + TABLE_SETTING, + UQ_SCOPE_KEY, + VALUE_MAX_LENGTH, + VALUE_TYPE_BOOL, + VALUE_TYPE_FLOAT, + VALUE_TYPE_INT, + VALUE_TYPE_JSON, + VALUE_TYPE_MAX_LENGTH, + VALUE_TYPE_STRING, +) + # ── Module identity ────────────────────────────────────────────────── MODULE_NAME: Final = "Settings" MODULE_PACKAGE: Final = "settings" @@ -17,30 +39,10 @@ LOCALE_NAMESPACE: Final = MODULE_PACKAGE # ── Scopes ─────────────────────────────────────────────────────────── -# Precedence high → low when resolving a key: USER > TENANT > SYSTEM. -SCOPE_SYSTEM: Final = "system" -SCOPE_TENANT: Final = "tenant" -SCOPE_USER: Final = "user" +# Scope, value-type, table and field-limit constants moved to +# ``simple_module_db.stored_settings`` with the table itself (#500); they are +# re-exported from the import block at the top of this file. ALL_SCOPES: Final = (SCOPE_SYSTEM, SCOPE_TENANT, SCOPE_USER) -# scope_id is empty for SYSTEM; empty string (not NULL) so composite unique -# works uniformly on SQLite and PostgreSQL (NULL breaks uniqueness on PG). -SYSTEM_SCOPE_ID: Final = "" - -# ── Value types ────────────────────────────────────────────────────── -# Values are always stored as strings; ``value_type`` tells consumers how -# to interpret the bytes and lets the UI pick the right input control. -VALUE_TYPE_STRING: Final = "string" -VALUE_TYPE_BOOL: Final = "bool" -VALUE_TYPE_INT: Final = "int" -VALUE_TYPE_FLOAT: Final = "float" -VALUE_TYPE_JSON: Final = "json" -ALL_VALUE_TYPES: Final = ( - VALUE_TYPE_STRING, - VALUE_TYPE_BOOL, - VALUE_TYPE_INT, - VALUE_TYPE_FLOAT, - VALUE_TYPE_JSON, -) # ── Secrets ────────────────────────────────────────────────────────── # Keys whose stored value must never leave the settings API or the raw @@ -104,16 +106,6 @@ # ── Database ───────────────────────────────────────────────────────── DB_SCHEMA: Final = MODULE_PACKAGE -TABLE_SETTING: Final = "settings_setting" -UQ_SCOPE_KEY: Final = "uq_settings_setting_scope_scope_id_key" - -# ── Field limits ───────────────────────────────────────────────────── -KEY_MAX_LENGTH: Final = 200 -VALUE_MAX_LENGTH: Final = 4000 -DESCRIPTION_MAX_LENGTH: Final = 2000 -SCOPE_MAX_LENGTH: Final = 10 -SCOPE_ID_MAX_LENGTH: Final = 255 -VALUE_TYPE_MAX_LENGTH: Final = 10 # ── Inertia page component names ───────────────────────────────────── PAGE_BROWSE: Final = f"{MODULE_NAME}/Browse" diff --git a/modules/settings/settings/contracts/accessor.py b/modules/settings/settings/contracts/accessor.py index c0df3e26d..3e9ca5aa4 100644 --- a/modules/settings/settings/contracts/accessor.py +++ b/modules/settings/settings/contracts/accessor.py @@ -23,7 +23,8 @@ async def handler(settings: SettingsDep): from collections.abc import Callable from typing import TYPE_CHECKING, Any, Final -from settings.constants import SYSTEM_SCOPE_ID +from simple_module_db.stored_settings import SYSTEM_SCOPE_ID + from settings.contracts.registry import SettingsRegistry from settings.contracts.schemas import ( BOOL_LITERALS_FALSE, diff --git a/modules/settings/settings/contracts/schemas.py b/modules/settings/settings/contracts/schemas.py index 7460d0743..4251661f0 100644 --- a/modules/settings/settings/contracts/schemas.py +++ b/modules/settings/settings/contracts/schemas.py @@ -7,13 +7,8 @@ from enum import StrEnum from pydantic import ConfigDict, model_validator -from sqlmodel import Field, SQLModel - -from settings.constants import ( +from simple_module_db.stored_settings import ( DESCRIPTION_MAX_LENGTH, - ERR_SCOPED_REQUIRES_ID, - ERR_SYSTEM_SCOPE_NO_ID, - ERR_VALUE_MISMATCH, KEY_MAX_LENGTH, SCOPE_ID_MAX_LENGTH, SCOPE_SYSTEM, @@ -27,6 +22,9 @@ VALUE_TYPE_JSON, VALUE_TYPE_STRING, ) +from sqlmodel import Field, SQLModel + +from settings.constants import ERR_SCOPED_REQUIRES_ID, ERR_SYSTEM_SCOPE_NO_ID, ERR_VALUE_MISMATCH _BOOL_LITERALS = frozenset( {"true", "false", "1", "0", "t", "f", "yes", "no", "y", "n", "on", "off"} diff --git a/modules/settings/settings/endpoints/api.py b/modules/settings/settings/endpoints/api.py index 7b2e71b8f..4a4e0ada3 100644 --- a/modules/settings/settings/endpoints/api.py +++ b/modules/settings/settings/endpoints/api.py @@ -3,6 +3,7 @@ from __future__ import annotations from fastapi import APIRouter, Depends, HTTPException, Query, Request +from simple_module_db.stored_settings import SYSTEM_SCOPE_ID from simple_module_hosting.permissions import RequiresPermission from settings._unique_write import DuplicateSettingError @@ -28,7 +29,6 @@ STATUS_NO_CONTENT, STATUS_NOT_FOUND, STATUS_UNPROCESSABLE, - SYSTEM_SCOPE_ID, ) from settings.contracts.schemas import ( SettingCreate, diff --git a/modules/settings/settings/endpoints/module_api.py b/modules/settings/settings/endpoints/module_api.py index 8e5f700b7..2cde6e7e8 100644 --- a/modules/settings/settings/endpoints/module_api.py +++ b/modules/settings/settings/endpoints/module_api.py @@ -11,6 +11,8 @@ from fastapi import APIRouter, Depends, FastAPI, HTTPException, Request, Response, status from pydantic import ValidationError +from simple_module_core.module_settings import module_settings_registry +from simple_module_core.settings_hydrate import hydrate_settings from simple_module_hosting.i18n_deps import TranslatorDep from simple_module_hosting.permissions import RequiresPermission @@ -20,10 +22,9 @@ overrides_by_package, ) from settings._module_settings_props import serialize -from settings.constants import MODULE_PACKAGE, PERM_DELETE, PERM_EDIT, PERM_VIEW +from settings.constants import PERM_DELETE, PERM_EDIT, PERM_VIEW from settings.contracts.events import SettingsReloaded from settings.deps import get_setting_service -from settings.hydrate import hydrate_settings from settings.reload import EnvPinnedError, apply_changes_and_reload from settings.service import SettingService from settings.store import SettingsStore @@ -94,7 +95,7 @@ async def update_module( t: TranslatorDep, service: SettingService = Depends(get_setting_service), ) -> dict[str, Any]: - registry = getattr(request.app.state, MODULE_PACKAGE).module_registry + registry = module_settings_registry(request.app) if registry.get(package) is None: raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Unknown module package") if registry.manage_url(package) is not None: @@ -146,7 +147,7 @@ async def clear_module_field( request: Request, service: SettingService = Depends(get_setting_service), ) -> Response: - registry = getattr(request.app.state, MODULE_PACKAGE).module_registry + registry = module_settings_registry(request.app) cls = registry.get(package) if cls is None: raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Unknown module package") diff --git a/modules/settings/settings/hydrate.py b/modules/settings/settings/hydrate.py index 91a149819..e55fc97e5 100644 --- a/modules/settings/settings/hydrate.py +++ b/modules/settings/settings/hydrate.py @@ -1,40 +1,24 @@ -"""Resolve a module's BaseSettings from DB overrides + pydantic defaults. +"""Deprecated: hydration moved to the framework (#500). -A field's declared Python type maps to one of the five ``value_type`` labels -understood by SettingsStore (``string | bool | int | float | json``). The -hydrator reads overrides, parses each according to its stored ``value_type``, -and constructs the BaseSettings — pydantic enforces field validators and any -``@model_validator`` hooks. +``simple_module_core.settings_hydrate`` holds the hydrator and field helpers; +the sync worker twin is ``simple_module_db.stored_settings.hydrate_settings_sync``. +Kept as a re-export until 0.1.0 at the earliest. """ from __future__ import annotations -import logging -import types -from typing import Any, Literal, Union, get_args, get_origin +import warnings -from pydantic import BaseModel -from pydantic_settings import ( - BaseSettings, - DotEnvSettingsSource, - EnvSettingsSource, - InitSettingsSource, - SecretsSettingsSource, -) -from simple_module_core.stored_settings import ( +from simple_module_core.settings_hydrate import ( RELATED_FIELDS_CTX, - construct_leniently, - parse_stored_value, + build_settings, + env_supplied_fields, + hydrate_settings, + input_type_for_field, + value_type_for_field, ) -from sqlalchemy.orm import Session - -from settings.store import SettingsStore, get_overrides_sync - -logger = logging.getLogger(__name__) - -_SCALAR_TYPES = {bool: "bool", int: "int", float: "float"} +from simple_module_db.stored_settings import hydrate_settings_sync -# Re-exported: modules raise cross-field errors with it (``background_tasks``). __all__ = [ "RELATED_FIELDS_CTX", "build_settings", @@ -45,148 +29,9 @@ "value_type_for_field", ] - -def value_type_for_field(cls: type[BaseSettings], field_name: str) -> str: - """Return the ``value_type`` label for a field based on its annotation. - - - ``bool`` → ``"bool"`` - - ``int`` → ``"int"`` - - ``float`` → ``"float"`` - - ``str``, enums and ``Literal`` of strings → ``"string"`` - - ``list``, ``dict``, and other container types → ``"json"`` - """ - info = cls.model_fields[field_name] - ann = info.annotation - origin = get_origin(ann) - if origin is Literal and all(isinstance(a, str) for a in get_args(ann)): - return "string" - if origin is not None: - return "json" - return _SCALAR_TYPES.get(ann, "string") - - -def _holds_structure(ann: object) -> bool: - """A bare ``dict``/``list``/model: not editable as a single-line string.""" - return isinstance(ann, type) and issubclass( - ann, dict | list | set | frozenset | tuple | BaseModel - ) - - -def input_type_for_field(cls: type[BaseSettings], field_name: str) -> tuple[str, bool]: - """``(input_type, nullable)`` — which editor control a field needs. - - Storage stays on :func:`value_type_for_field`, which files every union - under ``json``. That is right for the store and wrong for the form: - ``int | None`` wants a number box where empty means ``None``, not a JSON - textarea that shows the literal text ``null`` (GH #487). - """ - ann = cls.model_fields[field_name].annotation - if get_origin(ann) not in (Union, types.UnionType): - return value_type_for_field(cls, field_name), False - args = get_args(ann) - rest = [a for a in args if a is not type(None)] - nullable = len(rest) < len(args) - if len(rest) == 1 and get_origin(rest[0]) is None: - only = rest[0] - if _holds_structure(only): - return "json", nullable - return _SCALAR_TYPES.get(only, "string"), nullable - return "json", nullable - - -def env_supplied_fields(cls: type[BaseSettings]) -> frozenset[str]: - """Fields the class's own non-init sources (env, ``.env``, secrets) answer. - - Asks the sources ``cls`` actually installs, so a ``DbBackedSettings`` - (init-only) reports nothing and an ``env_prefix`` class reports exactly the - ```` names that are set. Deliberately not ``cls()``: a - model validator can reject the env-only view (a production broker check) - that the DB-merged one would pass. - - A class with no ``env_prefix`` reports nothing from its sources: a plain - ``BaseSettings`` reads bare, case-insensitive names, so a field called - ``user``, ``home`` or ``path`` would be pinned by the process environment - and every admin edit refused. Only its ``env_seeded_fields`` count. - """ - found: set[str] = set() - if cls.model_config.get("env_prefix"): - found.update(_source_supplied_fields(cls)) - # A class may also read env through a ``default_factory`` (one shared - # variable seeding several fields), which no source can see. It names - # those fields itself, so env still beats the DB for them. - seeded = getattr(cls, "env_seeded_fields", None) - if callable(seeded): - found.update(seeded()) - return frozenset(found) & cls.model_fields.keys() - - -def _source_supplied_fields(cls: type[BaseSettings]) -> set[str]: - sources = cls.settings_customise_sources( - cls, - init_settings=InitSettingsSource(cls, init_kwargs={}), - env_settings=EnvSettingsSource(cls), - dotenv_settings=DotEnvSettingsSource(cls), - file_secret_settings=SecretsSettingsSource(cls), - ) - by_key = {(info.alias or name): name for name, info in cls.model_fields.items()} - found: set[str] = set() - for source in sources: - if isinstance(source, InitSettingsSource): - continue - found.update(by_key.get(key, key) for key in source()) - return found - - -def build_settings[T: BaseSettings]( - cls: type[T], raw_overrides: dict[str, tuple[str, str]], *, lenient: bool = False -) -> T: - """Construct ``cls`` from ``{field: (raw, value_type)}``, skipping unknown fields. - - Shared by the async and sync hydrators so parsing cannot drift. - - Strict by default: an invalid stored value raises, which is what a settings - write relies on to answer 422. ``lenient=True`` is for *boot* paths (web - lifespan, workers): a row that was stored before a validator tightened must - not take the app down, nor discard the module's other, valid overrides. Each - offending field is logged and ignored (it falls back to env/default). - - Precedence is env → DB → default (GH #486). pydantic-settings ranks init - kwargs above its env source, so a stored override for a field the - environment sets is dropped here rather than passed in. - """ - from_env = env_supplied_fields(cls) - parsed: dict[str, Any] = {} - for field_name, (raw, vtype) in raw_overrides.items(): - if field_name not in cls.model_fields or field_name in from_env: - continue - try: - parsed[field_name] = parse_stored_value(raw, vtype) - except ValueError: - if not lenient: - raise - logger.error( - "Ignoring stored %s.%s: cannot parse as %s", cls.__name__, field_name, vtype - ) - if not lenient: - return cls(**parsed) - return construct_leniently(cls, parsed, level=logging.ERROR) - - -async def hydrate_settings[T: BaseSettings]( - cls: type[T], store: SettingsStore, package: str, *, lenient: bool = False -) -> T: - """Construct ``cls`` with DB overrides merged over pydantic defaults.""" - return build_settings(cls, await store.get_overrides(package), lenient=lenient) - - -def hydrate_settings_sync[T: BaseSettings]( - cls: type[T], session: Session, package: str, *, lenient: bool = False -) -> T: - """Sync :func:`hydrate_settings` for Celery workers and other non-async code. - - Workers never run the hosting lifespan, so a bare ``cls()`` there returns - pydantic defaults (``DbBackedSettings`` ignores the environment on purpose). - Takes a plain sync ``Session`` and applies the same SYSTEM-scope overrides - with the same ``value_type`` parsing as the web process. - """ - return build_settings(cls, get_overrides_sync(session, package), lenient=lenient) +warnings.warn( + "settings.hydrate is deprecated; import from simple_module_core.settings_hydrate " + "(hydrate_settings_sync: simple_module_db.stored_settings)", + DeprecationWarning, + stacklevel=2, +) diff --git a/modules/settings/settings/known_keys.py b/modules/settings/settings/known_keys.py index 0e1665d3a..2fa88727d 100644 --- a/modules/settings/settings/known_keys.py +++ b/modules/settings/settings/known_keys.py @@ -17,9 +17,9 @@ from fastapi import FastAPI from fastapi.encoders import jsonable_encoder +from simple_module_core.secret_fields import conceals_secret, mask from settings._module_settings import collect_module_settings -from settings._secrets import conceals_secret, mask def _from_field(package: str, field) -> dict[str, Any]: diff --git a/modules/settings/settings/models.py b/modules/settings/settings/models.py index 7a515d820..9aca9c0be 100644 --- a/modules/settings/settings/models.py +++ b/modules/settings/settings/models.py @@ -1,50 +1,12 @@ -"""SQLModel tables for the Settings module.""" +"""SQLModel tables for the Settings module. -from __future__ import annotations - -from simple_module_db.base import create_module_base -from simple_module_db.mixins import AuditMixin -from sqlalchemy import UniqueConstraint -from sqlmodel import Field - -from settings.constants import ( - DESCRIPTION_MAX_LENGTH, - KEY_MAX_LENGTH, - MODULE_PACKAGE, - SCOPE_ID_MAX_LENGTH, - SCOPE_MAX_LENGTH, - SCOPE_SYSTEM, - SYSTEM_SCOPE_ID, - TABLE_SETTING, - UQ_SCOPE_KEY, - VALUE_MAX_LENGTH, - VALUE_TYPE_MAX_LENGTH, - VALUE_TYPE_STRING, -) - -Base = create_module_base(MODULE_PACKAGE) +``Setting`` moved to ``simple_module_db.stored_settings`` (#500) so the +framework owns the table it reads at boot. Re-exported here: it is the same +class object, never a second mapping of ``settings_setting``. +""" +from __future__ import annotations -class Setting(Base, AuditMixin, table=True): # ty: ignore[unsupported-base] - """A typed key/value configuration entry scoped to system, tenant, or user. - - Resolution precedence when a consumer asks for a key: USER > TENANT > - SYSTEM. Uniqueness is enforced across the (scope, scope_id, key) tuple - so the same key can live at different scopes without collision. - - ``value`` is always stored as a string. ``value_type`` advertises how - the bytes should be interpreted ("string", "bool", "int", "float", - "json") so UIs pick the right input control and pydantic can reject - inputs that don't parse. - """ - - __tablename__ = TABLE_SETTING - __table_args__ = (UniqueConstraint("scope", "scope_id", "key", name=UQ_SCOPE_KEY),) +from simple_module_db.stored_settings import Base, Setting - id: int | None = Field(default=None, primary_key=True) - scope: str = Field(default=SCOPE_SYSTEM, max_length=SCOPE_MAX_LENGTH, index=True) - scope_id: str = Field(default=SYSTEM_SCOPE_ID, max_length=SCOPE_ID_MAX_LENGTH, index=True) - key: str = Field(max_length=KEY_MAX_LENGTH, index=True) - value: str = Field(max_length=VALUE_MAX_LENGTH) - value_type: str = Field(default=VALUE_TYPE_STRING, max_length=VALUE_TYPE_MAX_LENGTH) - description: str | None = Field(default=None, max_length=DESCRIPTION_MAX_LENGTH) +__all__ = ["Base", "Setting"] diff --git a/modules/settings/settings/module.py b/modules/settings/settings/module.py index 814318186..abbda9876 100644 --- a/modules/settings/settings/module.py +++ b/modules/settings/settings/module.py @@ -40,10 +40,9 @@ async def _resolve_setting_labels(db: AsyncSession, ids: list[str]) -> dict[str, screen calls the row. Ids that are not integers belong to some other id space and are left unnamed, which falls back to showing the id. """ + from simple_module_db.stored_settings import Setting from sqlalchemy import select - from settings.models import Setting - numeric = {int(raw): raw for raw in ids if raw.lstrip("-").isdigit()} if not numeric: return {} @@ -62,20 +61,23 @@ class SettingsModule(ModuleBase): ) def register_settings(self, app: FastAPI) -> None: + from simple_module_core.module_settings import module_settings_registry + from settings.contracts.registry import SettingsRegistry - from settings.module_registry import ModuleSettingsRegistry from settings.services import SettingsServices from settings.settings import SettingsSettings + # The framework owns the module-settings registry (#500); the admin + # screens read the same object through ``services.module_registry``. services = SettingsServices( settings=SettingsSettings(), registry=SettingsRegistry(), - module_registry=ModuleSettingsRegistry(), + module_registry=module_settings_registry(app), ) setattr(app.state, MODULE_PACKAGE, services) # Self-register so the UI lists our own settings alongside other modules. - services.module_registry.register("settings", SettingsSettings) + services.module_registry.register(MODULE_PACKAGE, SettingsSettings) def register_exception_handlers(self, app: FastAPI) -> None: from settings.errors import install_exception_handlers @@ -123,7 +125,7 @@ def register_permissions(self, registry: PermissionRegistry) -> None: registry.map_role(tenant_role(role), [PERM_TENANT_EDIT]) def register_audit_links(self, registry: AuditLinkRegistry) -> None: - from settings.models import Setting + from simple_module_db.stored_settings import Setting registry.register( AuditLink( diff --git a/modules/settings/settings/module_registry.py b/modules/settings/settings/module_registry.py index 1084fefd5..e1d7765fd 100644 --- a/modules/settings/settings/module_registry.py +++ b/modules/settings/settings/module_registry.py @@ -1,53 +1,19 @@ -"""Registry of per-module pydantic BaseSettings classes. +"""Deprecated: import from :mod:`simple_module_core.module_settings` (#500). -Populated during each module's ``register_settings`` via -``register_module_settings``. The hosting lifespan reads this at startup -to hydrate every module's effective settings from the DB. +Kept as a re-export until 0.1.0 at the earliest. """ from __future__ import annotations -from dataclasses import dataclass, field +import warnings -from pydantic_settings import BaseSettings +from simple_module_core.module_settings import ModuleSettingsRegistry -from settings._secrets import register_typed_secrets +__all__ = ["ModuleSettingsRegistry"] - -@dataclass(slots=True) -class ModuleSettingsRegistry: - """In-memory map of ``package`` → ``BaseSettings`` subclass.""" - - _classes: dict[str, type[BaseSettings]] = field(default_factory=dict) - _manage_urls: dict[str, str] = field(default_factory=dict) - - def register( - self, - package: str, - cls: type[BaseSettings], - manage_url: str | None = None, - ) -> None: - if package in self._classes: - raise ValueError(f"{package!r} already registered") - self._classes[package] = cls - register_typed_secrets(package, cls) - if manage_url: - self._manage_urls[package] = manage_url - - def manage_url(self, package: str) -> str | None: - """URL of the module's own management page, if it declared one. - - Modules with a purpose-built settings screen (e.g. Branding) declare it - so the generic module-settings editor links there instead of offering a - second, raw editor for the same fields. - """ - return self._manage_urls.get(package) - - def get(self, package: str) -> type[BaseSettings] | None: - return self._classes.get(package) - - def all_packages(self) -> list[str]: - return sorted(self._classes) - - def items(self) -> list[tuple[str, type[BaseSettings]]]: - return sorted(self._classes.items()) +warnings.warn( + "settings.module_registry is deprecated; import ModuleSettingsRegistry from " + "simple_module_core.module_settings", + DeprecationWarning, + stacklevel=2, +) diff --git a/modules/settings/settings/registration.py b/modules/settings/settings/registration.py index cb37e186b..99c20e031 100644 --- a/modules/settings/settings/registration.py +++ b/modules/settings/settings/registration.py @@ -1,40 +1,19 @@ -"""Helper modules call from ``register_settings`` to install their BaseSettings. +"""Deprecated: import from :mod:`simple_module_core.module_settings` (#500). -Two things happen: -1. A fresh ``BaseSettings`` (pydantic defaults only) is constructed. -2. The class is recorded in ``app.state.settings.module_registry`` so the - hosting lifespan can hydrate it from the DB before module ``on_startup`` - hooks run. - -The module's services dataclass is built from the default settings object and -attached at ``app.state.``. +Kept as a re-export until 0.1.0 at the earliest. """ from __future__ import annotations -from collections.abc import Callable -from typing import Any - -from fastapi import FastAPI -from pydantic_settings import BaseSettings - -from settings.constants import MODULE_PACKAGE +import warnings +from simple_module_core.module_settings import register_module_settings -def register_module_settings[T: BaseSettings]( - app: FastAPI, - package: str, - settings_cls: type[T], - services_factory: Callable[[T], Any], - manage_url: str | None = None, -) -> None: - """Register a module's BaseSettings class and mount its services on app.state. +__all__ = ["register_module_settings"] - ``manage_url`` points at the module's own management page when it has one; - the generic module-settings editor then links there instead of rendering a - second editor for the same fields. - """ - registry = getattr(app.state, MODULE_PACKAGE).module_registry - registry.register(package, settings_cls, manage_url=manage_url) - defaults = settings_cls() - setattr(app.state, package, services_factory(defaults)) +warnings.warn( + "settings.registration is deprecated; import register_module_settings from " + "simple_module_core.module_settings", + DeprecationWarning, + stacklevel=2, +) diff --git a/modules/settings/settings/reload.py b/modules/settings/settings/reload.py index d361dba41..7fda67e99 100644 --- a/modules/settings/settings/reload.py +++ b/modules/settings/settings/reload.py @@ -10,10 +10,10 @@ from pydantic_core import to_jsonable_python from pydantic_settings import BaseSettings from simple_module_core.events import EventBus +from simple_module_core.module_settings import module_settings_registry +from simple_module_core.settings_hydrate import env_supplied_fields, value_type_for_field -from settings.constants import MODULE_PACKAGE from settings.contracts.events import SettingsReloaded -from settings.hydrate import env_supplied_fields, value_type_for_field from settings.store import SettingsStore @@ -88,7 +88,7 @@ async def apply_changes_and_reload( changes: dict[str, Any], ) -> BaseSettings: """Validate, persist, hot-swap, and publish ``SettingsReloaded``.""" - registry = getattr(app.state, MODULE_PACKAGE).module_registry + registry = module_settings_registry(app) cls = registry.get(package) if cls is None: raise KeyError(f"Unknown settings package: {package!r}") diff --git a/modules/settings/settings/service.py b/modules/settings/settings/service.py index 137d43738..8eb80df95 100644 --- a/modules/settings/settings/service.py +++ b/modules/settings/settings/service.py @@ -4,6 +4,7 @@ from typing import TYPE_CHECKING +from simple_module_db.stored_settings import SYSTEM_SCOPE_ID, VALUE_TYPE_STRING, Setting from sqlalchemy import select from sqlalchemy.ext.asyncio import AsyncSession @@ -12,10 +13,6 @@ from settings._managed_keys import TenantIsLive, ensure_deletable from settings._row_masking import drop_placeholder_write, is_placeholder_write, out from settings._unique_write import DuplicateSettingError, insert_if_free -from settings.constants import ( - SYSTEM_SCOPE_ID, - VALUE_TYPE_STRING, -) from settings.contracts.schemas import ( SettingCreate, SettingOut, @@ -23,7 +20,6 @@ SettingUpdate, SettingUpsert, ) -from settings.models import Setting if TYPE_CHECKING: from simple_module_core.invalidation import InvalidationBus diff --git a/modules/settings/settings/services.py b/modules/settings/settings/services.py index ea079afa8..97b91522d 100644 --- a/modules/settings/settings/services.py +++ b/modules/settings/settings/services.py @@ -12,8 +12,9 @@ from dataclasses import dataclass, field +from simple_module_core.module_settings import ModuleSettingsRegistry + from settings.contracts.registry import SettingsRegistry -from settings.module_registry import ModuleSettingsRegistry from settings.settings import SettingsSettings @@ -26,9 +27,10 @@ class SettingsServices: so admins can discover every knob and the accessor can fall back to declared defaults. - ``module_registry`` tracks per-module ``BaseSettings`` classes registered - via ``register_module_settings``; the hosting lifespan uses it to hydrate - each module's effective settings from the DB before ``on_startup`` runs. + ``module_registry`` is the framework's registry of per-module + ``BaseSettings`` classes (``simple_module_core.module_settings``), the + same object the hosting lifespan hydrates from the DB before + ``on_startup`` runs. """ settings: SettingsSettings diff --git a/modules/settings/settings/store.py b/modules/settings/settings/store.py index 619e5adb3..51520cb10 100644 --- a/modules/settings/settings/store.py +++ b/modules/settings/settings/store.py @@ -1,5 +1,10 @@ """DB-backed key/value store for module settings (SYSTEM scope). +The read-only queries (``package_overrides``, ``get_overrides_sync``) moved to +``simple_module_db.stored_settings`` with the table (#500) and are re-exported +here. ``SettingsStore`` satisfies ``simple_module_core.settings_hydrate. +OverrideSource``. + Wraps the existing SettingService. Keys are namespaced ``.`` to avoid collision with free-form user-defined setting keys. @@ -13,14 +18,13 @@ from __future__ import annotations -from collections.abc import Iterable - -from sqlalchemy import select -from sqlalchemy.orm import Session +from simple_module_db.stored_settings import ( # noqa: F401 (re-exported, #500) + SYSTEM_SCOPE_ID, + get_overrides_sync, + package_overrides, +) -from settings.constants import SYSTEM_SCOPE_ID from settings.contracts.schemas import SettingScope, SettingUpsert, SettingValueType -from settings.models import Setting from settings.service import SettingService @@ -28,43 +32,11 @@ def _key(package: str, field: str) -> str: return f"{package}.{field}" -def package_overrides( - rows: Iterable[tuple[str, str, str]], package: str -) -> dict[str, tuple[str, str]]: - """Map ``(key, value, value_type)`` rows to ``{field_name: (raw, value_type)}``. +class SettingsStore: + """SYSTEM-scoped key/value store keyed by ``(package, field)``. - The one place the ``.`` key format is interpreted, shared by - the async :meth:`SettingsStore.get_overrides` and the sync - :func:`get_overrides_sync` so the two cannot drift. - """ - prefix = f"{package}." - out: dict[str, tuple[str, str]] = {} - for key, value, value_type in rows: - if not key.startswith(prefix): - continue - field_name = key[len(prefix) :] - if "." in field_name: - continue - out[field_name] = (value, value_type) - return out - - -def get_overrides_sync(session: Session, package: str) -> dict[str, tuple[str, str]]: - """Sync twin of :meth:`SettingsStore.get_overrides` for worker processes. - - Reads SYSTEM-scope rows through a plain sync ``Session`` (unmasked, like the - async path — this feeds live settings objects, not a screen). + Satisfies ``OverrideSource`` for :func:`hydrate_settings`. """ - stmt = select(Setting.key, Setting.value, Setting.value_type).where( - Setting.scope == SettingScope.SYSTEM.value, - Setting.scope_id == SYSTEM_SCOPE_ID, - Setting.key.startswith(f"{package}.", autoescape=True), - ) - return package_overrides(session.execute(stmt).tuples(), package) - - -class SettingsStore: - """SYSTEM-scoped key/value store keyed by ``(package, field)``.""" def __init__(self, service: SettingService) -> None: self._service = service diff --git a/modules/settings/settings/tenant_view.py b/modules/settings/settings/tenant_view.py index 4e1f1410a..22db08ca0 100644 --- a/modules/settings/settings/tenant_view.py +++ b/modules/settings/settings/tenant_view.py @@ -8,10 +8,10 @@ from __future__ import annotations +from simple_module_core.secret_fields import conceals_secret, mask +from simple_module_db.stored_settings import SYSTEM_SCOPE_ID from sqlmodel import SQLModel -from settings._secrets import conceals_secret, mask -from settings.constants import SYSTEM_SCOPE_ID from settings.contracts.registry import SettingsRegistry from settings.contracts.schemas import SettingScope, SettingValueType from settings.service import SettingService diff --git a/modules/settings/tests/test_clear_route_per_scope.py b/modules/settings/tests/test_clear_route_per_scope.py index 0b1de18a5..02ac3c030 100644 --- a/modules/settings/tests/test_clear_route_per_scope.py +++ b/modules/settings/tests/test_clear_route_per_scope.py @@ -4,10 +4,10 @@ import pytest from settings._managed_keys import ManagedKeyError -from settings.constants import SYSTEM_SCOPE_ID from settings.contracts.registry import SettingDefinition, clear_route from settings.contracts.schemas import SettingScope, SettingUpsert from settings.service import SettingService +from simple_module_db.stored_settings import SYSTEM_SCOPE_ID KEY = "demo.scoped_logo" SYSTEM_ROUTE = "/api/demo/logo" diff --git a/modules/settings/tests/test_duplicate_and_race.py b/modules/settings/tests/test_duplicate_and_race.py index dbe40f612..8349eabe9 100644 --- a/modules/settings/tests/test_duplicate_and_race.py +++ b/modules/settings/tests/test_duplicate_and_race.py @@ -11,10 +11,10 @@ import pytest from settings.contracts.schemas import SettingScope, SettingUpsert -from settings.models import Base, Setting from settings.service import SettingService from simple_module_db.listeners import register_listeners from simple_module_db.session import init_db +from simple_module_db.stored_settings import Base, Setting from simple_module_test.database import ( database_url_for_tests, init_db_kwargs, diff --git a/modules/settings/tests/test_env_pinned.py b/modules/settings/tests/test_env_pinned.py index f9a39b078..d10c15e66 100644 --- a/modules/settings/tests/test_env_pinned.py +++ b/modules/settings/tests/test_env_pinned.py @@ -13,13 +13,13 @@ from fastapi import FastAPI from pydantic import SecretStr from pydantic_settings import BaseSettings, SettingsConfigDict -from settings.registration import register_module_settings from settings.reload import EnvPinnedError, apply_changes_and_reload from settings.service import SettingService from settings.services import SettingsServices from settings.settings import SettingsSettings from settings.store import SettingsStore from simple_module_core.events import EventBus +from simple_module_core.module_settings import register_module_settings class _Cfg(BaseSettings): diff --git a/modules/settings/tests/test_hydrate.py b/modules/settings/tests/test_hydrate.py index 50f9850dc..b8b05f2da 100644 --- a/modules/settings/tests/test_hydrate.py +++ b/modules/settings/tests/test_hydrate.py @@ -5,9 +5,9 @@ import pytest from pydantic import ValidationError, field_validator, model_validator from pydantic_settings import BaseSettings, SettingsConfigDict -from settings.hydrate import hydrate_settings, value_type_for_field from settings.service import SettingService from settings.store import SettingsStore +from simple_module_core.settings_hydrate import hydrate_settings, value_type_for_field class _Cfg(BaseSettings): @@ -69,8 +69,7 @@ def test_value_type_for_bool_int_float_str_list() -> None: def test_hydrate_sync_matches_async_parsing(tmp_path) -> None: - from settings.hydrate import hydrate_settings_sync - from settings.models import Setting + from simple_module_db.stored_settings import Setting, hydrate_settings_sync from sqlalchemy import create_engine from sqlalchemy.orm import Session @@ -175,8 +174,7 @@ async def test_env_var_beats_a_stored_override(db_session, monkeypatch) -> None: def test_env_beats_db_for_sync_hydration(tmp_path, monkeypatch) -> None: - from settings.hydrate import hydrate_settings_sync - from settings.models import Setting + from simple_module_db.stored_settings import Setting, hydrate_settings_sync from sqlalchemy import create_engine from sqlalchemy.orm import Session diff --git a/modules/settings/tests/test_hydrate_related_fields.py b/modules/settings/tests/test_hydrate_related_fields.py index 4c769e679..0824f62e1 100644 --- a/modules/settings/tests/test_hydrate_related_fields.py +++ b/modules/settings/tests/test_hydrate_related_fields.py @@ -6,7 +6,7 @@ from pydantic import ValidationError, ValidationInfo, field_validator from pydantic_core import PydanticCustomError from pydantic_settings import BaseSettings, SettingsConfigDict -from settings.hydrate import RELATED_FIELDS_CTX, build_settings +from simple_module_core.settings_hydrate import RELATED_FIELDS_CTX, build_settings class _Pair(BaseSettings): diff --git a/modules/settings/tests/test_lenient_related_fields_validators.py b/modules/settings/tests/test_lenient_related_fields_validators.py index f447e5af5..d6c3a57f5 100644 --- a/modules/settings/tests/test_lenient_related_fields_validators.py +++ b/modules/settings/tests/test_lenient_related_fields_validators.py @@ -11,7 +11,7 @@ from file_storage.settings import FileStorageSettings from keycloak.settings import KeycloakSettings from pydantic import ValidationError -from settings.hydrate import RELATED_FIELDS_CTX, build_settings +from simple_module_core.settings_hydrate import RELATED_FIELDS_CTX, build_settings from simple_module_core.stored_settings import construct_leniently from simple_module_hosting.host_settings import HostSettings from site_lock.settings import SiteLockSettings diff --git a/modules/settings/tests/test_module_api_json_fields.py b/modules/settings/tests/test_module_api_json_fields.py index 61a5bfa74..103b2e8ab 100644 --- a/modules/settings/tests/test_module_api_json_fields.py +++ b/modules/settings/tests/test_module_api_json_fields.py @@ -8,9 +8,9 @@ from __future__ import annotations from background_tasks.settings import BackgroundTasksSettings -from settings.hydrate import hydrate_settings, input_type_for_field from settings.service import SettingService from settings.store import SettingsStore +from simple_module_core.settings_hydrate import hydrate_settings, input_type_for_field _PKG = "background_tasks" diff --git a/modules/settings/tests/test_module_registry.py b/modules/settings/tests/test_module_registry.py index 580dfc9db..aa9dd680e 100644 --- a/modules/settings/tests/test_module_registry.py +++ b/modules/settings/tests/test_module_registry.py @@ -4,7 +4,7 @@ import pytest from pydantic_settings import BaseSettings -from settings.module_registry import ModuleSettingsRegistry +from simple_module_core.module_settings import ModuleSettingsRegistry class _Foo(BaseSettings): diff --git a/modules/settings/tests/test_registration.py b/modules/settings/tests/test_registration.py index 0a68238d7..070799471 100644 --- a/modules/settings/tests/test_registration.py +++ b/modules/settings/tests/test_registration.py @@ -5,10 +5,9 @@ import pytest from fastapi import FastAPI from pydantic_settings import BaseSettings -from settings.module_registry import ModuleSettingsRegistry -from settings.registration import register_module_settings from settings.services import SettingsServices from settings.settings import SettingsSettings +from simple_module_core.module_settings import module_settings_registry, register_module_settings class _FakeModuleServices: @@ -24,7 +23,7 @@ def app() -> FastAPI: registry=__import__( "settings.contracts.registry", fromlist=["SettingsRegistry"] ).SettingsRegistry(), - module_registry=ModuleSettingsRegistry(), + module_registry=module_settings_registry(app), ) return app diff --git a/modules/settings/tests/test_reload.py b/modules/settings/tests/test_reload.py index 29f2bb746..99a1a1f5f 100644 --- a/modules/settings/tests/test_reload.py +++ b/modules/settings/tests/test_reload.py @@ -7,13 +7,13 @@ from pydantic import ValidationError from pydantic_settings import BaseSettings from settings.contracts.events import SettingsReloaded -from settings.registration import register_module_settings from settings.reload import apply_changes_and_reload from settings.service import SettingService from settings.services import SettingsServices from settings.settings import SettingsSettings from settings.store import SettingsStore from simple_module_core.events import EventBus +from simple_module_core.module_settings import register_module_settings class _UsersCfg(BaseSettings): diff --git a/modules/settings/tests/test_settings_env_unprefixed.py b/modules/settings/tests/test_settings_env_unprefixed.py index 1d54b6a28..4e90a4a52 100644 --- a/modules/settings/tests/test_settings_env_unprefixed.py +++ b/modules/settings/tests/test_settings_env_unprefixed.py @@ -9,7 +9,7 @@ from __future__ import annotations from pydantic_settings import BaseSettings, SettingsConfigDict -from settings.hydrate import build_settings, env_supplied_fields +from simple_module_core.settings_hydrate import build_settings, env_supplied_fields class _Unprefixed(BaseSettings): diff --git a/modules/settings/tests/test_settings_framework_reexports.py b/modules/settings/tests/test_settings_framework_reexports.py new file mode 100644 index 000000000..f395edb1a --- /dev/null +++ b/modules/settings/tests/test_settings_framework_reexports.py @@ -0,0 +1,48 @@ +"""The plugin's old import paths re-export the framework objects (#500).""" + +from __future__ import annotations + +import importlib +import sys + +import pytest +import simple_module_core.module_settings as core_registry +import simple_module_core.secret_fields as core_secrets +import simple_module_core.settings_hydrate as core_hydrate +import simple_module_db.stored_settings as db_settings + + +def test_setting_model_is_one_class() -> None: + from settings.models import Base, Setting + + assert Setting is db_settings.Setting + assert Base is db_settings.Base + + +def test_moved_constants_and_helpers_are_the_same_objects() -> None: + from settings import _secrets, constants, store + + assert constants.SYSTEM_SCOPE_ID == db_settings.SYSTEM_SCOPE_ID + assert constants.TABLE_SETTING == db_settings.TABLE_SETTING + assert store.get_overrides_sync is db_settings.get_overrides_sync + assert _secrets._TYPED_SECRET_KEYS is core_secrets._TYPED_SECRET_KEYS + + +@pytest.mark.parametrize( + ("module", "name", "target"), + [ + ("settings.registration", "register_module_settings", core_registry), + ("settings.module_registry", "ModuleSettingsRegistry", core_registry), + ("settings.hydrate", "hydrate_settings", core_hydrate), + ("settings.hydrate", "hydrate_settings_sync", db_settings), + ], +) +def test_deprecated_modules_warn_and_reexport(module: str, name: str, target: object) -> None: + sys.modules.pop(module, None) + with pytest.warns(DeprecationWarning, match=module): + mod = importlib.import_module(module) + assert getattr(mod, name) is getattr(target, name) + + +def test_plugin_services_share_the_framework_registry(app) -> None: + assert app.state.settings.module_registry is core_registry.get_module_settings_registry(app) diff --git a/modules/settings/tests/test_settings_group_metadata.py b/modules/settings/tests/test_settings_group_metadata.py index b56e611a9..5bf50ee5f 100644 --- a/modules/settings/tests/test_settings_group_metadata.py +++ b/modules/settings/tests/test_settings_group_metadata.py @@ -20,7 +20,7 @@ def test_string_literal_field_is_a_string_select(): from typing import Literal from settings._field_meta import choices_for - from settings.hydrate import input_type_for_field, value_type_for_field + from simple_module_core.settings_hydrate import input_type_for_field, value_type_for_field class Config(BaseSettings): mode: Literal["lax", "strict"] = "lax" diff --git a/modules/settings/tests/test_settings_schemas.py b/modules/settings/tests/test_settings_schemas.py index 33f323ac0..9609d47c2 100644 --- a/modules/settings/tests/test_settings_schemas.py +++ b/modules/settings/tests/test_settings_schemas.py @@ -4,7 +4,6 @@ import pytest from pydantic import ValidationError -from settings.constants import SYSTEM_SCOPE_ID from settings.contracts.schemas import ( SettingCreate, SettingScope, @@ -12,6 +11,7 @@ SettingUpsert, SettingValueType, ) +from simple_module_db.stored_settings import SYSTEM_SCOPE_ID class TestScopeValidation: diff --git a/modules/settings/tests/test_settings_secret_json_roundtrip.py b/modules/settings/tests/test_settings_secret_json_roundtrip.py index c09952e6e..0b8e3e8f7 100644 --- a/modules/settings/tests/test_settings_secret_json_roundtrip.py +++ b/modules/settings/tests/test_settings_secret_json_roundtrip.py @@ -11,8 +11,8 @@ from pydantic import SecretStr from pydantic_settings import BaseSettings, SettingsConfigDict -from settings.hydrate import build_settings from settings.reload import apply_changes_and_reload +from simple_module_core.settings_hydrate import build_settings class _SecretCfg(BaseSettings): @@ -39,7 +39,7 @@ def _app(): registry = SimpleNamespace(get=lambda package: _SecretCfg) return SimpleNamespace( state=SimpleNamespace( - settings=SimpleNamespace(module_registry=registry), + module_settings_registry=registry, secretjson=SimpleNamespace(settings=_SecretCfg()), ) ) diff --git a/modules/settings/tests/test_settings_secret_masking.py b/modules/settings/tests/test_settings_secret_masking.py index d29142595..ec175cf7a 100644 --- a/modules/settings/tests/test_settings_secret_masking.py +++ b/modules/settings/tests/test_settings_secret_masking.py @@ -12,9 +12,10 @@ from __future__ import annotations import pytest -from settings.constants import SENSITIVE_PLACEHOLDER, SYSTEM_SCOPE_ID +from settings.constants import SENSITIVE_PLACEHOLDER from settings.contracts.schemas import SettingCreate, SettingScope from settings.service import SettingService +from simple_module_db.stored_settings import SYSTEM_SCOPE_ID pytestmark = pytest.mark.anyio diff --git a/modules/settings/tests/test_settings_secret_urls.py b/modules/settings/tests/test_settings_secret_urls.py index a7cd78479..9cc13df7f 100644 --- a/modules/settings/tests/test_settings_secret_urls.py +++ b/modules/settings/tests/test_settings_secret_urls.py @@ -140,22 +140,22 @@ class TestContainersAreWalked: """ def test_a_dsn_in_a_list_is_a_credential(self): - from settings._secrets import embeds_credential + from simple_module_core.secret_fields import embeds_credential assert embeds_credential(["redis://localhost:6379/0", _WITH_PASSWORD]) is True def test_a_dsn_in_a_dict_value_is_a_credential(self): - from settings._secrets import embeds_credential + from simple_module_core.secret_fields import embeds_credential assert embeds_credential({"primary": _WITH_PASSWORD}) is True def test_a_nested_container_is_walked(self): - from settings._secrets import embeds_credential + from simple_module_core.secret_fields import embeds_credential assert embeds_credential({"shards": [{"dsn": _WITH_PASSWORD}]}) is True def test_password_free_containers_stay_visible(self): - from settings._secrets import embeds_credential + from simple_module_core.secret_fields import embeds_credential assert embeds_credential(["redis://localhost:6379/0"]) is False assert embeds_credential({"n": 5, "flag": True, "url": "https://example.com"}) is False diff --git a/modules/settings/tests/test_settings_service.py b/modules/settings/tests/test_settings_service.py index ca9bbef0b..31c7903a2 100644 --- a/modules/settings/tests/test_settings_service.py +++ b/modules/settings/tests/test_settings_service.py @@ -2,7 +2,6 @@ from __future__ import annotations -from settings.constants import SYSTEM_SCOPE_ID from settings.contracts.schemas import ( SettingCreate, SettingScope, @@ -10,6 +9,7 @@ SettingUpsert, ) from settings.service import SettingService +from simple_module_db.stored_settings import SYSTEM_SCOPE_ID from sqlalchemy.ext.asyncio import AsyncSession diff --git a/modules/settings/tests/test_settings_typed_secret_masking.py b/modules/settings/tests/test_settings_typed_secret_masking.py index c8eee0ecf..0b2d0dc22 100644 --- a/modules/settings/tests/test_settings_typed_secret_masking.py +++ b/modules/settings/tests/test_settings_typed_secret_masking.py @@ -14,10 +14,10 @@ from pydantic import SecretStr from pydantic_settings import BaseSettings, SettingsConfigDict from settings._module_settings import SECRET_MASK -from settings._secrets import annotation_holds_secret -from settings.registration import register_module_settings from settings.service import SettingService from settings.store import SettingsStore +from simple_module_core.module_settings import register_module_settings +from simple_module_core.secret_fields import annotation_holds_secret PACKAGE = "typedsecret" URL = f"/api/settings/modules/{PACKAGE}" diff --git a/modules/settings/tests/test_store_filters.py b/modules/settings/tests/test_store_filters.py index 7125ac4a0..3128091ed 100644 --- a/modules/settings/tests/test_store_filters.py +++ b/modules/settings/tests/test_store_filters.py @@ -20,7 +20,7 @@ async def _seed(app: FastAPI) -> None: """25 system, 3 tenant and 2 user rows — enough to need a second page.""" - from settings.models import Setting + from simple_module_db.stored_settings import Setting async with app.state.sm.db.session_factory() as session: for i in range(25): @@ -175,8 +175,8 @@ class TestServiceQuery: """The service owns the query; the view only clamps and shapes it.""" async def test_search_and_page_return_rows_and_the_unpaged_total(self, db_session) -> None: - from settings.models import Setting from settings.service import SettingService + from simple_module_db.stored_settings import Setting for i in range(5): db_session.add(Setting(scope="system", scope_id="", key=f"a.key_{i}", value="v")) @@ -190,8 +190,8 @@ async def test_search_and_page_return_rows_and_the_unpaged_total(self, db_sessio assert [r.key for r in rows] == ["a.key_2", "a.key_3"] async def test_counts_by_scope_names_every_scope_even_at_zero(self, db_session) -> None: - from settings.models import Setting from settings.service import SettingService + from simple_module_db.stored_settings import Setting db_session.add(Setting(scope="system", scope_id="", key="a.b", value="v")) await db_session.flush() diff --git a/modules/settings/tests/test_store_secret_masking.py b/modules/settings/tests/test_store_secret_masking.py index 3ab5c4c55..a2ba58210 100644 --- a/modules/settings/tests/test_store_secret_masking.py +++ b/modules/settings/tests/test_store_secret_masking.py @@ -14,10 +14,10 @@ import pytest from fastapi import FastAPI from settings._row_masking import is_masked -from settings.constants import SENSITIVE_PLACEHOLDER, VALUE_TYPE_INT, VALUE_TYPE_STRING +from settings.constants import SENSITIVE_PLACEHOLDER from settings.contracts.schemas import SettingCreate, SettingScope -from settings.models import Setting from settings.service import SettingService +from simple_module_db.stored_settings import VALUE_TYPE_INT, VALUE_TYPE_STRING, Setting _DSN = "postgresql://svc:hunter2@db.internal/app" @@ -182,8 +182,8 @@ class TestTheMaskStopsAtTheScreen: """ async def test_the_store_reads_through_the_mask(self, db_session) -> None: - from settings.constants import SYSTEM_SCOPE_ID from settings.store import SettingsStore + from simple_module_db.stored_settings import SYSTEM_SCOPE_ID service = SettingService(db_session) await service.create( diff --git a/modules/settings/tests/test_upload_key_delete.py b/modules/settings/tests/test_upload_key_delete.py index e0f4e702a..9e6a6dbee 100644 --- a/modules/settings/tests/test_upload_key_delete.py +++ b/modules/settings/tests/test_upload_key_delete.py @@ -11,10 +11,10 @@ import pytest from settings._managed_keys import ManagedKeyError -from settings.constants import SYSTEM_SCOPE_ID from settings.contracts.registry import SettingDefinition from settings.contracts.schemas import SettingScope, SettingUpsert from settings.service import SettingService +from simple_module_db.stored_settings import SYSTEM_SCOPE_ID KEY = "demo.logo" UPLOAD = "/api/demo/tenant/logo" diff --git a/tests/test_boot_without_settings_plugin.py b/tests/test_boot_without_settings_plugin.py new file mode 100644 index 000000000..f603a9f07 --- /dev/null +++ b/tests/test_boot_without_settings_plugin.py @@ -0,0 +1,35 @@ +"""The app boots and hydrates settings with the ``settings`` plugin disabled (#500). + +The framework owns the module-settings registry, the hydrator and the table, +so host and module settings still come from the DB without the admin UI. +""" + +from __future__ import annotations + +from simple_module_core.discovery import discover_modules +from simple_module_core.module_settings import get_module_settings_registry +from simple_module_db.stored_settings import Setting + +# Settings itself, plus the modules that hard-depend on it. +_EXCLUDED = {"Settings", "Branding", "Mail"} + + +async def test_boot_without_settings_plugin_hydrates_from_db(settings) -> None: + from simple_module_hosting.app_builder import create_app + from simple_module_test._schema import _create_all_tables + + enabled = [m.meta.name for m in discover_modules() if m.meta.name not in _EXCLUDED] + app = create_app(settings.model_copy(update={"modules_enabled": enabled})) + await _create_all_tables(app.state.sm.db.engine) + async with app.state.sm.db.session_factory() as session: + session.add(Setting(key="host.api_title", value="From the DB", value_type="string")) + session.add(Setting(key="users.allow_signup", value="true", value_type="bool")) + await session.commit() + + async with app.router.lifespan_context(app): + assert not hasattr(app.state, "settings"), "the plugin is disabled" + registry = get_module_settings_registry(app) + assert registry is not None + assert {"host", "users"} <= set(registry.all_packages()) + assert app.state.host.settings.api_title == "From the DB" + assert app.state.users.settings.allow_signup is True From 7d701e6d59a45e984f5ab083832c02a72e32d56f Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Wed, 21 Oct 2026 07:20:12 +0200 Subject: [PATCH 4/9] refactor(modules): import settings registration and hydration from the framework (#500) The seven modules that reached settings.registration through importlib (users, file_storage, site_lock, api_tokens, tenants, background_tasks, keycloak) now import register_module_settings statically from simple_module_core.module_settings; branding, mail, background_tasks' worker and every test move off the deprecated plugin paths. Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 --- modules/api_tokens/api_tokens/module.py | 6 +----- .../tests/test_entity_label_permission.py | 2 +- modules/audit_log/tests/test_entity_labels.py | 2 +- .../background_tasks/background_tasks/constants.py | 4 ++-- .../background_tasks/background_tasks/module.py | 10 +--------- .../background_tasks/background_tasks/settings.py | 2 +- .../background_tasks/worker_settings.py | 2 +- .../tests/test_background_tasks_settings.py | 6 +++--- .../background_tasks/tests/test_bg_redis_url.py | 6 +++--- .../tests/test_bg_runner_settings.py | 2 +- .../tests/test_bg_worker_settings.py | 2 +- modules/branding/branding/module.py | 2 +- .../tests/test_branding_clear_route_scope.py | 2 +- modules/file_storage/file_storage/module.py | 14 +++----------- modules/keycloak/keycloak/module.py | 6 +----- modules/mail/mail/module.py | 4 ++-- modules/mail/mail/runtime.py | 2 +- modules/mail/mail/settings.py | 2 +- modules/site_lock/site_lock/module.py | 12 +++--------- modules/tenants/tenants/module.py | 4 +--- modules/users/tests/test_edit_recent_activity.py | 2 +- modules/users/tests/test_settings.py | 4 ++-- modules/users/users/module.py | 14 +++----------- 23 files changed, 36 insertions(+), 76 deletions(-) diff --git a/modules/api_tokens/api_tokens/module.py b/modules/api_tokens/api_tokens/module.py index 8bdb76c88..227b981f7 100644 --- a/modules/api_tokens/api_tokens/module.py +++ b/modules/api_tokens/api_tokens/module.py @@ -9,6 +9,7 @@ from simple_module_core.menu import MenuItem, MenuRegistry, MenuSection from simple_module_core.module import ModuleBase, ModuleMeta +from simple_module_core.module_settings import register_module_settings from simple_module_core.tenancy import TenantRole, tenant_role from api_tokens import constants as c @@ -34,11 +35,6 @@ def register_settings(self, app: FastAPI) -> None: from api_tokens.services import ApiTokensServices from api_tokens.settings import ApiTokensSettings - # SM009 is AST-based: resolve via importlib like the other - # settings-backed modules. - register_module_settings = importlib.import_module( - "settings.registration" - ).register_module_settings register_module_settings( app, c.MODULE_PACKAGE, diff --git a/modules/audit_log/tests/test_entity_label_permission.py b/modules/audit_log/tests/test_entity_label_permission.py index 79cbf7d18..03750abe3 100644 --- a/modules/audit_log/tests/test_entity_label_permission.py +++ b/modules/audit_log/tests/test_entity_label_permission.py @@ -28,8 +28,8 @@ from _entity_label_support import seed_entry as _seed_entry from audit_log.constants import PERM_VIEW from audit_log.resolve import resolve_entity_labels -from settings.models import Setting from simple_module_core.audit_links import AuditLink, AuditLinkRegistry +from simple_module_db.stored_settings import Setting from simple_module_test import forge_session_cookie from sqlalchemy.ext.asyncio import AsyncSession from users.constants import PERM_USERS_MANAGE diff --git a/modules/audit_log/tests/test_entity_labels.py b/modules/audit_log/tests/test_entity_labels.py index 36f9ee885..e39142c58 100644 --- a/modules/audit_log/tests/test_entity_labels.py +++ b/modules/audit_log/tests/test_entity_labels.py @@ -18,7 +18,7 @@ from _entity_label_support import seed_entry as _seed_entry from background_tasks.models import TaskExecution from feature_flags.models import FeatureFlagOverride -from settings.models import Setting +from simple_module_db.stored_settings import Setting from users.models import User diff --git a/modules/background_tasks/background_tasks/constants.py b/modules/background_tasks/background_tasks/constants.py index c047c19b4..3de27853f 100644 --- a/modules/background_tasks/background_tasks/constants.py +++ b/modules/background_tasks/background_tasks/constants.py @@ -19,8 +19,8 @@ # ── Module dependencies ───────────────────────────────────────── _MODULE_USERS = "Users" -# register_settings() calls settings.registration.register_module_settings, -# which reads app.state.settings — so Settings must register first. +# Sorted after Settings for its admin editor and reload path; registering +# settings itself no longer needs it (the framework owns the registry, #500). _MODULE_SETTINGS = "Settings" # ── Env / settings ────────────────────────────────────────────── diff --git a/modules/background_tasks/background_tasks/module.py b/modules/background_tasks/background_tasks/module.py index 087621f11..5d3b0a532 100644 --- a/modules/background_tasks/background_tasks/module.py +++ b/modules/background_tasks/background_tasks/module.py @@ -12,6 +12,7 @@ from simple_module_core.audit_links import AuditLinkRegistry from simple_module_core.menu import MenuItem, MenuRegistry, MenuSection from simple_module_core.module import ModuleBase, ModuleMeta +from simple_module_core.module_settings import register_module_settings from simple_module_core.permissions import PermissionRegistry from background_tasks.constants import ( @@ -78,19 +79,10 @@ class BackgroundTasksModule(ModuleBase): ) def register_settings(self, app: FastAPI) -> None: - import importlib from background_tasks.services import BackgroundTasksServices from background_tasks.settings import BackgroundTasksSettings - # SM009 is AST-based: a static `from settings.registration import ...` - # from a module helper is fine (plugin→plugin), but we resolve via - # importlib here to match the convention used framework-side and to - # keep the dependency direction one-way explicit. - register_module_settings = importlib.import_module( - "settings.registration" - ).register_module_settings - register_module_settings( app, "background_tasks", diff --git a/modules/background_tasks/background_tasks/settings.py b/modules/background_tasks/background_tasks/settings.py index a015e7aae..94cba1027 100644 --- a/modules/background_tasks/background_tasks/settings.py +++ b/modules/background_tasks/background_tasks/settings.py @@ -43,7 +43,7 @@ from pydantic import Field, ValidationInfo, field_validator, model_validator from pydantic_core import PydanticCustomError from pydantic_settings import BaseSettings, SettingsConfigDict -from settings.hydrate import RELATED_FIELDS_CTX +from simple_module_core.settings_hydrate import RELATED_FIELDS_CTX from background_tasks.config_guards import parse_task_routes from background_tasks.constants import ( diff --git a/modules/background_tasks/background_tasks/worker_settings.py b/modules/background_tasks/background_tasks/worker_settings.py index 9c36f45f1..edff112fe 100644 --- a/modules/background_tasks/background_tasks/worker_settings.py +++ b/modules/background_tasks/background_tasks/worker_settings.py @@ -20,7 +20,7 @@ from typing import cast from pydantic_settings import BaseSettings -from settings.hydrate import hydrate_settings_sync +from simple_module_db.stored_settings import hydrate_settings_sync from background_tasks.constants import MODULE_NAME from background_tasks.settings import BackgroundTasksSettings diff --git a/modules/background_tasks/tests/test_background_tasks_settings.py b/modules/background_tasks/tests/test_background_tasks_settings.py index 9c882fbfa..33c8bf3ae 100644 --- a/modules/background_tasks/tests/test_background_tasks_settings.py +++ b/modules/background_tasks/tests/test_background_tasks_settings.py @@ -45,9 +45,9 @@ def test_soft_time_limit_must_be_below_hard_limit() -> None: @pytest.mark.asyncio async def test_stored_invalid_limit_pair_does_not_break_lenient_hydration(db_session) -> None: - from settings.hydrate import hydrate_settings from settings.service import SettingService from settings.store import SettingsStore + from simple_module_core.settings_hydrate import hydrate_settings store = SettingsStore(SettingService(db_session)) await store.set_override("background_tasks", "task_soft_time_limit", "100", "int") @@ -64,16 +64,16 @@ async def test_hydration_drops_the_stored_soft_limit(db_session, caplog, monkeyp """With the hard limit pinned by env, the pair rule reported on ``task_time_limit`` cannot be satisfied by dropping it; the stored soft limit is the half to drop (#548).""" - from settings.hydrate import hydrate_settings from settings.service import SettingService from settings.store import SettingsStore + from simple_module_core.settings_hydrate import hydrate_settings monkeypatch.setenv("SM_BG_TASKS_TASK_TIME_LIMIT", "50") store = SettingsStore(SettingService(db_session)) await store.set_override("background_tasks", "task_soft_time_limit", "100", "int") await store.set_override("background_tasks", "max_retries", "7", "int") - with caplog.at_level(logging.ERROR, logger="settings.hydrate"): + with caplog.at_level(logging.ERROR, logger="simple_module_core.settings_hydrate"): cfg = await hydrate_settings( BackgroundTasksSettings, store, "background_tasks", lenient=True ) diff --git a/modules/background_tasks/tests/test_bg_redis_url.py b/modules/background_tasks/tests/test_bg_redis_url.py index 289584ec6..96ea6e89e 100644 --- a/modules/background_tasks/tests/test_bg_redis_url.py +++ b/modules/background_tasks/tests/test_bg_redis_url.py @@ -116,7 +116,7 @@ def test_redis_url_satisfies_the_production_validator( def test_redis_url_beats_a_stored_value(monkeypatch: pytest.MonkeyPatch) -> None: """env → DB → default holds for the shared variable too, though only a default factory reads it.""" - from settings.hydrate import build_settings + from simple_module_core.settings_hydrate import build_settings monkeypatch.setenv("SM_REDIS_URL", "redis://cache:6379/2") @@ -127,7 +127,7 @@ def test_redis_url_beats_a_stored_value(monkeypatch: pytest.MonkeyPatch) -> None def test_stored_values_apply_without_redis_url() -> None: - from settings.hydrate import build_settings + from simple_module_core.settings_hydrate import build_settings settings = build_settings(BackgroundTasksSettings, _STORED) @@ -136,7 +136,7 @@ def test_stored_values_apply_without_redis_url() -> None: def test_legacy_var_still_beats_redis_url_and_the_db(monkeypatch: pytest.MonkeyPatch) -> None: - from settings.hydrate import build_settings + from simple_module_core.settings_hydrate import build_settings monkeypatch.setenv("SM_REDIS_URL", "redis://cache:6379/2") monkeypatch.setenv("SM_BG_TASKS_BROKER_URL", "redis://legacy:6379/0") diff --git a/modules/background_tasks/tests/test_bg_runner_settings.py b/modules/background_tasks/tests/test_bg_runner_settings.py index 58b43050f..6c060b9fc 100644 --- a/modules/background_tasks/tests/test_bg_runner_settings.py +++ b/modules/background_tasks/tests/test_bg_runner_settings.py @@ -9,7 +9,7 @@ from background_tasks import sync_db from background_tasks.settings import BackgroundTasksSettings from background_tasks.worker_settings import clear_settings_cache, load_runner_settings -from settings.models import Setting +from simple_module_db.stored_settings import Setting from sqlalchemy import create_engine from sqlalchemy.orm import Session diff --git a/modules/background_tasks/tests/test_bg_worker_settings.py b/modules/background_tasks/tests/test_bg_worker_settings.py index 1c41bf89e..2d30411cd 100644 --- a/modules/background_tasks/tests/test_bg_worker_settings.py +++ b/modules/background_tasks/tests/test_bg_worker_settings.py @@ -8,8 +8,8 @@ from background_tasks import sync_db from background_tasks.worker_settings import clear_settings_cache, settings_for from pydantic import Field -from settings.models import Setting from simple_module_core.settings_base import DbBackedSettings +from simple_module_db.stored_settings import Setting from sqlalchemy import create_engine from sqlalchemy.orm import Session diff --git a/modules/branding/branding/module.py b/modules/branding/branding/module.py index 03ab1906e..b891209b9 100644 --- a/modules/branding/branding/module.py +++ b/modules/branding/branding/module.py @@ -36,7 +36,7 @@ class BrandingModule(ModuleBase): ) def register_settings(self, app: FastAPI) -> None: - from settings.registration import register_module_settings + from simple_module_core.module_settings import register_module_settings from branding.services import BrandingServices from branding.settings import BrandingSettings diff --git a/modules/branding/tests/test_branding_clear_route_scope.py b/modules/branding/tests/test_branding_clear_route_scope.py index ae8115807..43076f3b3 100644 --- a/modules/branding/tests/test_branding_clear_route_scope.py +++ b/modules/branding/tests/test_branding_clear_route_scope.py @@ -6,9 +6,9 @@ from __future__ import annotations -from settings.constants import SYSTEM_SCOPE_ID from settings.contracts.schemas import SettingScope, SettingUpsert from settings.service import SettingService +from simple_module_db.stored_settings import SYSTEM_SCOPE_ID KEY = "branding.logo_file_id" SYSTEM_ROUTE = "/api/branding/logo" diff --git a/modules/file_storage/file_storage/module.py b/modules/file_storage/file_storage/module.py index 91b491ca2..5e92f4b81 100644 --- a/modules/file_storage/file_storage/module.py +++ b/modules/file_storage/file_storage/module.py @@ -14,6 +14,7 @@ from simple_module_core.feature_flags import FeatureFlagDefinition, FeatureFlagRegistry from simple_module_core.menu import MenuItem, MenuRegistry, MenuSection from simple_module_core.module import ModuleBase, ModuleMeta +from simple_module_core.module_settings import register_module_settings from simple_module_core.permissions import PermissionRegistry from simple_module_core.public_routes import PublicRouteRegistry from simple_module_core.tenancy import TenantRole, tenant_role @@ -63,26 +64,17 @@ class FileStorageModule(ModuleBase): name=constants.MODULE_PASCAL, route_prefix=constants.ROUTE_PREFIX_API, view_prefix=constants.ROUTE_PREFIX_VIEW, - # Needs Settings to run first so register_module_settings can reach - # app.state.settings.module_registry during register_settings. + # Kept after Settings so the module order is unchanged; registering + # settings no longer needs it (the framework owns the registry, #500). depends_on=[constants._MODULE_SETTINGS], i18n_audience="admin", ) def register_settings(self, app: FastAPI) -> None: - import importlib from file_storage.services import FileStorageServices from file_storage.settings import FileStorageSettings - # SM009 is AST-based: a static `from settings.registration import ...` - # from a module helper is fine (plugin→plugin), but we resolve via - # importlib here to match the convention used framework-side and to - # keep the dependency direction one-way explicit. - register_module_settings = importlib.import_module( - "settings.registration" - ).register_module_settings - register_module_settings( app, "file_storage", diff --git a/modules/keycloak/keycloak/module.py b/modules/keycloak/keycloak/module.py index 253dd58b5..e421eb768 100644 --- a/modules/keycloak/keycloak/module.py +++ b/modules/keycloak/keycloak/module.py @@ -8,6 +8,7 @@ from simple_module_core.menu import MenuItem, MenuRegistry, MenuSection from simple_module_core.module import ModuleBase, ModuleMeta +from simple_module_core.module_settings import register_module_settings if TYPE_CHECKING: from fastapi import APIRouter, FastAPI @@ -26,16 +27,11 @@ class KeycloakModule(ModuleBase): _is_auth_provider = True def register_settings(self, app: FastAPI) -> None: - import importlib from keycloak.provider import KeycloakAuthProvider from keycloak.settings import KeycloakSettings from keycloak.state import KeycloakState - register_module_settings = importlib.import_module( - "settings.registration" - ).register_module_settings - register_module_settings( app, "keycloak", diff --git a/modules/mail/mail/module.py b/modules/mail/mail/module.py index c094117a3..4e707beeb 100644 --- a/modules/mail/mail/module.py +++ b/modules/mail/mail/module.py @@ -18,12 +18,12 @@ class MailModule(ModuleBase): meta = ModuleMeta( name=c.MODULE_NAME, - # ``Settings`` so register_module_settings can reach the registry. + # ``Settings``: the runtime re-reads overrides through its SettingsStore. depends_on=[c.MODULE_SETTINGS], ) def register_settings(self, app: FastAPI) -> None: - from settings.registration import register_module_settings + from simple_module_core.module_settings import register_module_settings from mail.settings import MailSettings from mail.state import MailState diff --git a/modules/mail/mail/runtime.py b/modules/mail/mail/runtime.py index 8cc89bb8f..be700e4db 100644 --- a/modules/mail/mail/runtime.py +++ b/modules/mail/mail/runtime.py @@ -5,12 +5,12 @@ import os from typing import Any -from settings.hydrate import hydrate_settings from settings.service import SettingService from settings.store import SettingsStore from simple_module_core.environments import NON_PROD_ENVIRONMENTS from simple_module_core.health import HealthCheckResult, HealthStatus from simple_module_core.mail import MailSender, current_mail_sender, install_mail_sender +from simple_module_core.settings_hydrate import hydrate_settings from mail import constants as c from mail.sender import build_sender diff --git a/modules/mail/mail/settings.py b/modules/mail/mail/settings.py index e80acf716..bdde2b3e7 100644 --- a/modules/mail/mail/settings.py +++ b/modules/mail/mail/settings.py @@ -1,7 +1,7 @@ """Mail module settings — DB-backed via ``register_module_settings``. ``smtp_password`` is masked by the settings UI because its name matches the -secret-field pattern (``settings._secrets.is_secret_field``). +secret-field pattern (``simple_module_core.secret_fields.is_secret_field``). """ from __future__ import annotations diff --git a/modules/site_lock/site_lock/module.py b/modules/site_lock/site_lock/module.py index 60b8d3a72..a30738839 100644 --- a/modules/site_lock/site_lock/module.py +++ b/modules/site_lock/site_lock/module.py @@ -5,6 +5,7 @@ from typing import TYPE_CHECKING from simple_module_core.module import ModuleBase, ModuleMeta +from simple_module_core.module_settings import register_module_settings from site_lock import constants as c @@ -15,25 +16,18 @@ class SiteLockModule(ModuleBase): meta = ModuleMeta( name=c.MODULE_NAME, - # ``Settings`` so register_module_settings can reach the module - # registry; ``Auth`` so this module sorts *after* auth and its + # ``Settings`` keeps the module order unchanged (registering settings + # no longer needs it, #500); ``Auth`` so this module sorts *after* auth and its # middleware therefore wraps outermost, executing before # AuthMiddleware. Both are load-bearing — see the module README. depends_on=[c.MODULE_SETTINGS, c.MODULE_AUTH], ) def register_settings(self, app: FastAPI) -> None: - import importlib from site_lock.settings import SiteLockSettings from site_lock.state import SiteLockState - # SM009 is AST-based: resolving via importlib matches the convention - # used by the other settings-backed modules. - register_module_settings = importlib.import_module( - "settings.registration" - ).register_module_settings - register_module_settings( app, c.MODULE_PACKAGE, diff --git a/modules/tenants/tenants/module.py b/modules/tenants/tenants/module.py index 8f82c7918..fe17ac00f 100644 --- a/modules/tenants/tenants/module.py +++ b/modules/tenants/tenants/module.py @@ -16,6 +16,7 @@ from simple_module_core.menu import MenuItem, MenuRegistry, MenuSection from simple_module_core.module import ModuleBase, ModuleMeta +from simple_module_core.module_settings import register_module_settings from tenants import constants as c @@ -67,9 +68,6 @@ def register_settings(self, app: FastAPI) -> None: from tenants.settings import TenantsSettings from tenants.shared_props import tenant_shared_props - register_module_settings = importlib.import_module( - "settings.registration" - ).register_module_settings register_module_settings( app, c.MODULE_PACKAGE, diff --git a/modules/users/tests/test_edit_recent_activity.py b/modules/users/tests/test_edit_recent_activity.py index c2b8a9db3..59dfc7ff7 100644 --- a/modules/users/tests/test_edit_recent_activity.py +++ b/modules/users/tests/test_edit_recent_activity.py @@ -102,7 +102,7 @@ async def test_a_non_user_entity_is_named_by_its_owning_module(self, admin_clien settings. """ from audit_log.models import AuditEntry - from settings.models import Setting + from simple_module_db.stored_settings import Setting user_id = await _admin_id(users_app) async with users_app.state.sm.db.session_factory() as session: diff --git a/modules/users/tests/test_settings.py b/modules/users/tests/test_settings.py index ee51ff349..ddc3d2aab 100644 --- a/modules/users/tests/test_settings.py +++ b/modules/users/tests/test_settings.py @@ -62,9 +62,9 @@ class TestUsersSettingsDbOverrides: @pytest.mark.asyncio async def test_allow_signup_override_from_db(self, db_session): - from settings.hydrate import hydrate_settings from settings.service import SettingService from settings.store import SettingsStore + from simple_module_core.settings_hydrate import hydrate_settings from users.settings import UsersSettings store = SettingsStore(SettingService(db_session)) @@ -75,9 +75,9 @@ async def test_allow_signup_override_from_db(self, db_session): @pytest.mark.asyncio async def test_base_url_override_from_db(self, db_session): - from settings.hydrate import hydrate_settings from settings.service import SettingService from settings.store import SettingsStore + from simple_module_core.settings_hydrate import hydrate_settings from users.settings import UsersSettings store = SettingsStore(SettingService(db_session)) diff --git a/modules/users/users/module.py b/modules/users/users/module.py index 8669935ae..bfd8cb3b8 100644 --- a/modules/users/users/module.py +++ b/modules/users/users/module.py @@ -10,6 +10,7 @@ from simple_module_core.audit_links import AuditLinkRegistry from simple_module_core.menu import MenuItem, MenuRegistry, MenuSection from simple_module_core.module import ModuleBase, ModuleMeta +from simple_module_core.module_settings import register_module_settings from simple_module_core.permissions import PermissionRegistry from simple_module_core.setup_steps import SetupRegistry @@ -26,8 +27,8 @@ from simple_module_core.invalidation import InvalidationBus _MODULE_DEPENDENCY_AUTH = "Auth" -# register_settings() goes through settings.registration.register_module_settings, -# which reads app.state.settings — so Settings must register first. +# Sorted after Settings for its SettingsReloaded event. Registering this +# module's settings no longer needs it: the framework owns that registry (#500). _MODULE_DEPENDENCY_SETTINGS = "Settings" # Menu URLs @@ -55,19 +56,10 @@ class UsersModule(ModuleBase): _is_auth_provider = True def register_settings(self, app: FastAPI) -> None: - import importlib from users.settings import UsersSettings from users.state import UsersState - # SM009 is AST-based: a static `from settings.registration import ...` - # from a module helper is fine (plugin→plugin), but we resolve via - # importlib here to match the convention used framework-side and to - # keep the dependency direction one-way explicit. - register_module_settings = importlib.import_module( - "settings.registration" - ).register_module_settings - register_module_settings(app, "users", UsersSettings, lambda s: UsersState(settings=s)) from users.provider import UsersAuthProvider From 03c6ffe0560eec69b89c9c8e73884c4f63a6adab Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Wed, 21 Oct 2026 07:20:12 +0200 Subject: [PATCH 5/9] fix(scaffold): new modules use DbBackedSettings + register_module_settings (#500) Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 --- .../module/__PACKAGE__/module.py.tpl | 10 ++++++++- .../module/__PACKAGE__/settings.py.tpl | 17 ++++++++------- scripts/_templates_module.py | 21 +++++++++++++------ scripts/tests/test_scaffold.py | 10 ++++++++- 4 files changed, 43 insertions(+), 15 deletions(-) diff --git a/framework/cli/simple_module_cli/templates/module/__PACKAGE__/module.py.tpl b/framework/cli/simple_module_cli/templates/module/__PACKAGE__/module.py.tpl index 044b9d888..b049cedb4 100644 --- a/framework/cli/simple_module_cli/templates/module/__PACKAGE__/module.py.tpl +++ b/framework/cli/simple_module_cli/templates/module/__PACKAGE__/module.py.tpl @@ -8,6 +8,7 @@ from pathlib import Path from fastapi import APIRouter, FastAPI from simple_module_core import ModuleBase, ModuleMeta from simple_module_core.menu import MenuItem, MenuRegistry, MenuSection +from simple_module_core.module_settings import register_module_settings class {{MODULE_NAME}}Module(ModuleBase): @@ -41,7 +42,14 @@ class {{MODULE_NAME}}Module(ModuleBase): from {{PACKAGE_NAME}}.services import {{MODULE_NAME}}Services from {{PACKAGE_NAME}}.settings import {{MODULE_NAME}}Settings - app.state.{{PACKAGE_NAME}} = {{MODULE_NAME}}Services(settings={{MODULE_NAME}}Settings()) + # Records the class so the host hydrates it from the settings table + # (env → DB → default) and mounts the services on app.state.{{PACKAGE_NAME}}. + register_module_settings( + app, + "{{PACKAGE_NAME}}", + {{MODULE_NAME}}Settings, + lambda s: {{MODULE_NAME}}Services(settings=s), + ) def static_mounts(self) -> dict[str, Path]: """Expose pre-built frontend assets when they exist. diff --git a/framework/cli/simple_module_cli/templates/module/__PACKAGE__/settings.py.tpl b/framework/cli/simple_module_cli/templates/module/__PACKAGE__/settings.py.tpl index e83724a98..07e13152f 100644 --- a/framework/cli/simple_module_cli/templates/module/__PACKAGE__/settings.py.tpl +++ b/framework/cli/simple_module_cli/templates/module/__PACKAGE__/settings.py.tpl @@ -1,14 +1,17 @@ -"""{{MODULE_NAME}} module settings. +"""{{MODULE_NAME}} module settings (DB-backed). -Per-module env-var prefix is ``SM_{{PACKAGE_NAME_UPPER}}_*``. Add fields here -as the module grows; the framework wires them onto -``app.state.{{PACKAGE_NAME}}`` via ``register_settings``. +Values come from the pydantic defaults below at boot, then get hydrated from +the settings table by the hosting lifespan before module ``on_startup`` runs +(``register_module_settings`` in ``module.py`` records the class). +``DbBackedSettings`` reads no environment variables: the DB is the only +source. Add fields here as the module grows. """ from __future__ import annotations -from pydantic_settings import BaseSettings, SettingsConfigDict +from pydantic_settings import SettingsConfigDict +from simple_module_core.settings_base import DbBackedSettings -class {{MODULE_NAME}}Settings(BaseSettings): - model_config = SettingsConfigDict(env_prefix="SM_{{PACKAGE_NAME_UPPER}}_", extra="ignore") +class {{MODULE_NAME}}Settings(DbBackedSettings): + model_config = SettingsConfigDict(extra="ignore") diff --git a/scripts/_templates_module.py b/scripts/_templates_module.py index 4cf723cd0..45abf4a43 100644 --- a/scripts/_templates_module.py +++ b/scripts/_templates_module.py @@ -23,6 +23,7 @@ def module_py(ctx: ScaffoldContext) -> str: from fastapi import APIRouter, FastAPI from simple_module_core.menu import MenuItem, MenuRegistry, MenuSection from simple_module_core.module import ModuleBase, ModuleMeta + from simple_module_core.module_settings import register_module_settings from simple_module_core.permissions import PermissionRegistry @@ -37,7 +38,12 @@ def register_settings(self, app: FastAPI) -> None: from {ctx.pkg}.services import {ctx.class_name}Services from {ctx.pkg}.settings import {ctx.class_name}Settings - app.state.{ctx.pkg} = {ctx.class_name}Services(settings={ctx.class_name}Settings()) + register_module_settings( + app, + "{ctx.pkg}", + {ctx.class_name}Settings, + lambda s: {ctx.class_name}Services(settings=s), + ) def register_routes(self, api_router: APIRouter, view_router: APIRouter) -> None: from {ctx.pkg}.endpoints.api import router as api @@ -80,9 +86,11 @@ def settings_py(ctx: ScaffoldContext) -> str: """{ctx.class_name} module settings (DB-backed). Values come from the pydantic defaults below at boot, then get hydrated - from the settings store by the hosting lifespan before module - ``on_startup`` runs. Runtime changes go through - ``settings.reload.apply_changes_and_reload``. + from the settings table by the hosting lifespan before module + ``on_startup`` runs (``register_module_settings`` in ``module.py`` + records the class). ``DbBackedSettings`` reads no environment + variables: the DB is the only source. Runtime changes go through the + Settings admin screen (``settings.reload.apply_changes_and_reload``). Add fields as the module needs them; an empty settings class is fine until then. ``extra="ignore"`` keeps an unknown stored key from @@ -91,10 +99,11 @@ def settings_py(ctx: ScaffoldContext) -> str: from __future__ import annotations - from pydantic_settings import BaseSettings, SettingsConfigDict + from pydantic_settings import SettingsConfigDict + from simple_module_core.settings_base import DbBackedSettings - class {ctx.class_name}Settings(BaseSettings): + class {ctx.class_name}Settings(DbBackedSettings): """Configuration for the {ctx.pkg} module.""" model_config = SettingsConfigDict(extra="ignore") diff --git a/scripts/tests/test_scaffold.py b/scripts/tests/test_scaffold.py index 313c4fe44..7d0397451 100644 --- a/scripts/tests/test_scaffold.py +++ b/scripts/tests/test_scaffold.py @@ -61,7 +61,15 @@ def test_scaffold_pyproject_declares_the_wheel_package(self, scaffolded_orders: def test_scaffold_settings_class_matches_the_module_import(self, scaffolded_orders: Path): """module.py and services.py both import OrdersSettings by name.""" content = (scaffolded_orders / "settings.py").read_text() - assert "class OrdersSettings(BaseSettings):" in content + assert "class OrdersSettings(DbBackedSettings):" in content + + def test_scaffold_registers_settings_through_the_framework(self, scaffolded_orders: Path): + """register_settings goes through register_module_settings (#500), so the + host hydrates the class from the DB; a bare app.state assignment would not.""" + content = (scaffolded_orders / "module.py").read_text() + assert "from simple_module_core.module_settings import register_module_settings" in content + assert "register_module_settings(" in content + assert "app.state.orders =" not in content def test_scaffold_module_class_name(self, scaffolded_orders: Path): content = (scaffolded_orders / "module.py").read_text() From 4035450be35796ee69c7907618be1263732cc965 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Wed, 21 Oct 2026 07:20:13 +0200 Subject: [PATCH 6/9] docs: settings mechanism lives in the framework; changelog (#500) Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 --- CHANGELOG.md | 34 +++++++++++ CLAUDE.md | 15 +++-- docs/framework/invalidation.md | 2 +- docs/framework/settings.md | 73 +++++++++++++++-------- docs/guide/configuration.md | 16 +++-- docs/modules/background_tasks.md | 2 +- docs/modules/settings.md | 6 +- skills/simple-module-conventions/SKILL.md | 13 ++-- skills/simple-module-creating/SKILL.md | 2 +- 9 files changed, 116 insertions(+), 47 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 04062513b..90ed05cdf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,32 @@ All notable changes to this project are documented in this file. The format is b - `captured_mail` pytest fixture in `simple_module_test` records sent mail. ### Changed +- The module-settings mechanism moved from the `settings` plugin into the + framework (#500): `register_module_settings` / `ModuleSettingsRegistry` to + `simple_module_core.module_settings`, hydration (`hydrate_settings`, + `build_settings`, `OverrideSource`, field helpers) to + `simple_module_core.settings_hydrate`, secret-field masking to + `simple_module_core.secret_fields`, and the `Setting` model, its constants, + the read-only override queries and `hydrate_settings_sync` to + `simple_module_db.stored_settings`. The table (`settings_setting`) is + unchanged; no migration. The hosting layer imports these statically instead + of reaching the plugin through `importlib` strings, and builds its pre-app + and secret-key SQL from the shared column constants. +- **Behaviour change (#500):** registration and boot-time hydration no longer + need the `settings` plugin. The registry is created on `app.state` on first + use, `host` settings are always registered, and the lifespan hydrates from the + table directly, so module and host settings still come from the DB with the + plugin disabled (it keeps the admin UI, hot reload and the key/value API). + The pre-app host read and the generated `SM_SECRET_KEY` are likewise stored + and read without it; outside development an unreachable database now refuses + the boot even when the plugin is not installed. +- **Out-of-tree hosts without the `settings` plugin (#500):** the framework now + always contributes `settings_setting` to the migration metadata, so the next + `alembic revision --autogenerate` will emit a `create_table` for it. Keep that + operation (the framework reads the table at boot). +- `make new-module` / `smpy create-module` scaffolds subclass `DbBackedSettings` + and register through `register_module_settings` instead of assigning + `app.state.` directly (#500). - **Behaviour change:** the `users` mailer settings (`mailer`, `smtp_*`) moved to the `mail` module as `backend` and `smtp_*`; a data migration (`d7b3a9e5c1f2`) moves stored values (#496). `simple_module_users` now depends on `simple_module_mail`, and @@ -140,6 +166,14 @@ All notable changes to this project are documented in this file. The format is b `openapi_public` (`SM_OPENAPI_PUBLIC`) is on. The bare `/openapi.json` was previously served, and anonymous, in production. +### Deprecated +- `settings.registration`, `settings.module_registry` and `settings.hydrate` + re-export the framework objects above and emit a `DeprecationWarning`; + `settings.models.Setting`, `settings._secrets` and the moved names in + `settings.store` / `settings.constants` re-export them silently (the same + objects, never a second `Setting` mapping). Removal no earlier than 0.1.0 + (#500). + ### Added - `TenantDeleted` event and `TenantService.delete_tenant` (#491). No endpoint yet: what deleting a tenant means for other modules' rows is still open. diff --git a/CLAUDE.md b/CLAUDE.md index 61fea8f47..fabd4e888 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -333,9 +333,13 @@ development and testing (`session_cookie_secure`). is marked restart-required (`test_build_time_settings_marked.py`). Module settings are a `DbBackedSettings` subclass (constructor + DB, no env source) registered in `register_settings` through - `settings.registration.register_module_settings(app, package, SettingsCls, - services_factory)`, which mounts the services object on `app.state.` - (`SM012`). The exceptions that read env are secrets and process wiring + `simple_module_core.module_settings.register_module_settings(app, package, + SettingsCls, services_factory)`, which mounts the services object on + `app.state.` (`SM012`). Registration, hydration and the + `settings_setting` table (`simple_module_db.stored_settings`) belong to the + framework and work with the `settings` plugin disabled; the plugin's + `settings.registration`/`module_registry`/`hydrate` are deprecated + re-exports. The exceptions that read env are secrets and process wiring (`SM_BG_TASKS_*`, `SM_KEYCLOAK_*`, `SM_USERS_*_TOKEN_SECRET`). `docs/framework/settings.md`, `docs/reference/env-vars.md`. - **Framework must not import plugins** (`SM009`): nothing under `framework/` @@ -389,9 +393,8 @@ development and testing (`session_cookie_secure`). `make new-module name=` scaffolds the package, wires it into `host/pyproject.toml` and the root `testpaths`/`extra-paths`, and runs `uv sync`. Expect `make lint` to fail right after: the scaffold still emits a -literal `inertia.render("/Create")`, permission and menu strings -without constants or `label_key`, and sets `app.state.` directly instead -of calling `register_module_settings`. Fix those first. It also does **not**: +literal `inertia.render("/Create")` and permission and menu strings +without constants or `label_key`. Fix those first. It also does **not**: 1. Run `npm install`, so regenerate and commit `package-lock.json` (CI uses `npm ci`). diff --git a/docs/framework/invalidation.md b/docs/framework/invalidation.md index 50a4a42d6..cf28a3dd2 100644 --- a/docs/framework/invalidation.md +++ b/docs/framework/invalidation.md @@ -80,7 +80,7 @@ A transport failure never propagates. The caller has already committed; turning Every framework channel shares that one Redis channel; `Invalidation.channel` routes inside the receiving process. -> **A stored override beats the environment.** `settings.hydrate.hydrate_settings` passes DB values to the settings class as *init args*, and pydantic-settings ranks init args above its env source — so once `background_tasks.broadcast_invalidations` has a row in the settings store (written from the admin UI, or by `smpy settings import-from-env`), the env var above stops having any effect after boot hydration. This inverts the precedence CLAUDE.md states (`env → DB → default`) and is not specific to these two fields: it applies to every env-readable module setting, `broker_url` included. Check the settings screen, not just the container's environment, when a knob appears not to work. +> **A stored override beats the environment.** `simple_module_core.settings_hydrate.hydrate_settings` passes DB values to the settings class as *init args*, and pydantic-settings ranks init args above its env source — so once `background_tasks.broadcast_invalidations` has a row in the settings store (written from the admin UI, or by `smpy settings import-from-env`), the env var above stops having any effect after boot hydration. This inverts the precedence CLAUDE.md states (`env → DB → default`) and is not specific to these two fields: it applies to every env-readable module setting, `broker_url` included. Check the settings screen, not just the container's environment, when a knob appears not to work. > **Redis pub/sub ignores the database index.** A publisher on `redis://host/0` and a subscriber on `redis://host//1` with the same channel name *do* reach each other — pub/sub is server-global, not per-database. So the `dev-services` convention of giving each repo its own logical DB (this one owns 4 and 5) isolates keys but **not** invalidations: two installs left on the default channel name will clear each other's caches, and each will see the other's user ids on the wire. Verified by experiment. Rename the channel per app; do not rely on the DB number. diff --git a/docs/framework/settings.md b/docs/framework/settings.md index e897543dd..23a601293 100644 --- a/docs/framework/settings.md +++ b/docs/framework/settings.md @@ -37,45 +37,68 @@ Outside the non-prod environments, boot fails if `secret_key == "change-me-in-pr ## Module settings -Each module that needs configuration declares a **`Env`** pydantic-settings class with its own `env_prefix` and a **`State`** dataclass to bundle settings plus any runtime-computed singletons. +Each module that needs configuration declares a `DbBackedSettings` subclass +(constructor + DB, no env source) and a services dataclass that bundles the +settings with any runtime singletons, then registers both from +`register_settings`: ```python -# modules/users/users/settings.py -from pydantic_settings import BaseSettings +# modules/orders/orders/settings.py +from pydantic_settings import SettingsConfigDict +from simple_module_core.settings_base import DbBackedSettings -class UsersEnv(BaseSettings): - allow_signup: bool = False - base_url: str | None = None +class OrdersSettings(DbBackedSettings): + model_config = SettingsConfigDict(extra="ignore") - class Config: - env_prefix = "SM_USERS_" + max_items_per_order: int = 100 ``` ```python -# modules/users/users/state.py -from dataclasses import dataclass +# modules/orders/orders/module.py +from simple_module_core.module_settings import register_module_settings -@dataclass -class UsersState: - settings: UsersEnv - mailer: Mailer # sends through the mail module's sender - principal_serializer: Callable[[User], dict] +class OrdersModule(ModuleBase): + def register_settings(self, app: FastAPI) -> None: + from orders.services import OrdersServices + from orders.settings import OrdersSettings + + register_module_settings( + app, "orders", OrdersSettings, lambda s: OrdersServices(settings=s) + ) ``` -Attach in `register_settings`: +`register_module_settings` records the class in the app's +`ModuleSettingsRegistry` and mounts `services_factory(OrdersSettings())` on +`app.state.orders`. The hosting lifespan then replaces +`app.state.orders.settings` with an instance hydrated from the +`settings_setting` table (env → DB → default) before any `on_startup` runs. + +The whole mechanism lives in the framework (#500), so it works whether or not +the `settings` plugin is enabled; the plugin adds the admin editor, hot +reload and the generic key/value API on top. + +| What | Import from | +|---|---| +| `register_module_settings`, `ModuleSettingsRegistry`, `module_settings_registry(app)` | `simple_module_core.module_settings` | +| `hydrate_settings`, `build_settings`, `OverrideSource`, `RELATED_FIELDS_CTX`, field helpers | `simple_module_core.settings_hydrate` | +| Secret-field masking (`conceals_secret`, `mask`, ...) | `simple_module_core.secret_fields` | +| `Setting` model, scope/table constants, `get_overrides_sync`, `hydrate_settings_sync` | `simple_module_db.stored_settings` | + +The old plugin paths (`settings.registration`, `settings.module_registry`, +`settings.hydrate`) still re-export these and emit a `DeprecationWarning`; +`settings.models`, `settings._secrets` and the moved names in +`settings.store` / `settings.constants` re-export them silently. They go no +earlier than 0.1.0. + +A Celery worker or CLI script never runs the lifespan, so it hydrates +synchronously: ```python -# modules/users/users/module.py -class UsersModule(ModuleBase): - def register_settings(self, app: FastAPI) -> None: - settings = UsersEnv() - app.state.users = UsersState( - settings=settings, - mailer=build_mailer(settings), - principal_serializer=serialize_user, - ) +from simple_module_db.stored_settings import hydrate_settings_sync + +cfg = hydrate_settings_sync(OrdersSettings, session, "orders") ``` ### Why `app.state.`? diff --git a/docs/guide/configuration.md b/docs/guide/configuration.md index 81da87c46..f7600f959 100644 --- a/docs/guide/configuration.md +++ b/docs/guide/configuration.md @@ -83,15 +83,16 @@ From then on, edit at `/admin/settings/` (requires the `settings.manage` permiss ## Per-module settings convention -When you write your own module, declare settings as a `pydantic_settings.BaseSettings` subclass and store them on `app.state.` (the lowercase package name) inside `register_settings(app)`. Env-var prefix is `SM__*`. +When you write your own module, declare settings as a `DbBackedSettings` subclass (`simple_module_core.settings_base`; constructor + DB, no env source) and register it from `register_settings(app)` with `register_module_settings`, which mounts your services object on `app.state.` (the lowercase package name) and lets the host hydrate it from the database before `on_startup`. ```python # modules/orders/orders/settings.py -from pydantic_settings import BaseSettings, SettingsConfigDict +from pydantic_settings import SettingsConfigDict +from simple_module_core.settings_base import DbBackedSettings -class OrdersSettings(BaseSettings): - model_config = SettingsConfigDict(env_prefix="SM_ORDERS_", extra="ignore") +class OrdersSettings(DbBackedSettings): + model_config = SettingsConfigDict(extra="ignore") max_items_per_order: int = 100 @@ -107,12 +108,17 @@ class OrdersServices: # modules/orders/orders/module.py +from simple_module_core.module_settings import register_module_settings + + class OrdersModule(ModuleBase): def register_settings(self, app: FastAPI) -> None: from orders.services import OrdersServices from orders.settings import OrdersSettings - app.state.orders = OrdersServices(settings=OrdersSettings()) + register_module_settings( + app, "orders", OrdersSettings, lambda s: OrdersServices(settings=s) + ) ``` If you override `register_settings` but don't write to `app.state.`, diagnostic `SM012` warns in dev. See [Settings & app.state](/framework/settings) for details. diff --git a/docs/modules/background_tasks.md b/docs/modules/background_tasks.md index 3ca65c051..38fc21d5d 100644 --- a/docs/modules/background_tasks.md +++ b/docs/modules/background_tasks.md @@ -526,4 +526,4 @@ def convert(file_id: int) -> None: cfg = settings_for(FileStorageSettings, "file_storage") ``` -It reads SYSTEM-scope overrides through the worker's sync session and caches per process for 30 seconds (`ttl=0` always re-reads; `clear_settings_cache()` drops the cache). The underlying sync helper is `settings.hydrate.hydrate_settings_sync`. +It reads SYSTEM-scope overrides through the worker's sync session and caches per process for 30 seconds (`ttl=0` always re-reads; `clear_settings_cache()` drops the cache). The underlying sync helper is `simple_module_db.stored_settings.hydrate_settings_sync`. diff --git a/docs/modules/settings.md b/docs/modules/settings.md index 2759fb52f..e8ff2eca0 100644 --- a/docs/modules/settings.md +++ b/docs/modules/settings.md @@ -20,9 +20,7 @@ Two distinct surfaces: ### Register module settings -The pattern (pydantic `BaseSettings` subclass + `register_module_settings` in `register_settings`) is documented in [Per-module settings convention](/guide/configuration#per-module-settings-convention). What the settings module adds on top: - -- **DB hydration**: `app.state..settings` is replaced with a fresh instance built from DB overrides + pydantic defaults during the host's hydrate phase, before `on_startup` runs. +The pattern (a `DbBackedSettings` subclass + `register_module_settings` from `simple_module_core.module_settings` in `register_settings`) is documented in [Settings & app.state](/framework/settings#module-settings). Registration and boot-time DB hydration belong to the framework since #500 and work with this module disabled. What the settings module adds on top: - **Env-var migration**: `smpy settings import-from-env` scans every registered class's `env_prefix` and seeds matching `SM_*` env vars as SYSTEM-scoped rows. Idempotent. - **Admin editing**: registered fields appear under that package at `/admin/settings/` with type-aware inputs. - **Hot reload**: saving via the admin UI calls `apply_changes_and_reload`, which validates the diff against the pydantic class, persists deltas, swaps the live `app.state..settings`, and publishes [`SettingsReloaded`](#events) so dependents (SMTP clients, Celery configs, …) can rebuild. @@ -32,7 +30,7 @@ The pattern (pydantic `BaseSettings` subclass + `register_module_settings` in `r `DbBackedSettings` subclasses ignore the environment, and only the hosting lifespan hydrates `app.state..settings`. A Celery worker (or CLI script) never runs that lifespan, so a bare `FileStorageSettings()` there silently returns pydantic defaults. Hydrate synchronously instead: ```python -from settings.hydrate import hydrate_settings_sync +from simple_module_db.stored_settings import hydrate_settings_sync cfg = hydrate_settings_sync(FileStorageSettings, session, "file_storage") ``` diff --git a/skills/simple-module-conventions/SKILL.md b/skills/simple-module-conventions/SKILL.md index 47eeeebcd..8b0f028a2 100644 --- a/skills/simple-module-conventions/SKILL.md +++ b/skills/simple-module-conventions/SKILL.md @@ -36,18 +36,23 @@ Forbidden in module code: `pydantic.BaseModel` for DTOs, and SQLAlchemy `Declara Treat 300 lines as the design pressure on every `.py`, `.ts`, and `.tsx` file (the reference codebase enforces this in CI). When you approach it, **split by responsibility** — don't golf comments and blank lines to squeeze under. -### 3. Per-module settings: `SM__*` on `app.state.` +### 3. Per-module settings: `register_module_settings` → `app.state.` ```python +from simple_module_core.module_settings import register_module_settings + + class UsersModule(ModuleBase): def register_settings(self, app: FastAPI) -> None: - from users.settings import UsersSettings # reads SM_USERS_* env + from users.settings import UsersSettings # a DbBackedSettings subclass from users.state import UsersState - app.state.users = UsersState(settings=UsersSettings()) + register_module_settings(app, "users", UsersSettings, lambda s: UsersState(settings=s)) ``` -- Env-var prefix is **always** `SM__*`. Cross-module reads are discouraged by convention (no enforcement) — go through the registry or event bus instead. +- Settings are DB-backed: subclass `simple_module_core.settings_base.DbBackedSettings` (no env source). The host hydrates every registered class from the settings table (env → DB → default) before `on_startup`; this works even with the `settings` admin module disabled. Only secrets and process wiring read env. +- Import from the framework, not the plugin: `simple_module_core.module_settings`, `simple_module_core.settings_hydrate`, `simple_module_db.stored_settings` (`Setting`, `hydrate_settings_sync` for workers). `settings.registration` / `settings.module_registry` / `settings.hydrate` are deprecated re-exports. +- Cross-module reads are discouraged by convention (no enforcement) — go through the registry or event bus instead. - The state attribute key is the lowercase package name or the lowercased `meta.name` — the diagnostics check accepts either. For module package `auth` with `meta.name="Auth"`, `app.state.auth` works either way. - If you override `register_settings` and don't add anything to `app.state.`, `SM012` fires at dev boot. diff --git a/skills/simple-module-creating/SKILL.md b/skills/simple-module-creating/SKILL.md index 779bbccc7..ef1a82e37 100644 --- a/skills/simple-module-creating/SKILL.md +++ b/skills/simple-module-creating/SKILL.md @@ -92,7 +92,7 @@ In execution order — all no-op by default: | Hook | Use it for | |---|---| -| `register_settings(app)` | Read `SM__*` env into a dataclass on `app.state.` | +| `register_settings(app)` | `register_module_settings(app, "", SettingsCls, factory)` (from `simple_module_core.module_settings`): DB-hydrated settings on `app.state.` | | `register_menu_items(registry)` | Sidebar / navbar entries | | `register_permissions(registry)` | `.` strings, grouped | | `register_feature_flags(registry)` | `FeatureFlagDefinition` constants | From b3a839db7e27c67058a9db81b8fe393d0c1cb703 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Wed, 21 Oct 2026 07:36:21 +0200 Subject: [PATCH 7/9] refactor(optimize): dedupe settings-table existence check, fix stale docstrings Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 --- framework/db/simple_module_db/stored_settings.py | 8 ++++++-- framework/hosting/simple_module_hosting/_preapp_config.py | 6 +++--- framework/hosting/simple_module_hosting/_secret_key.py | 6 +++--- modules/settings/settings/_row_masking.py | 2 +- modules/settings/settings/store.py | 4 ++-- 5 files changed, 15 insertions(+), 11 deletions(-) diff --git a/framework/db/simple_module_db/stored_settings.py b/framework/db/simple_module_db/stored_settings.py index 8aa8da956..bb314e2d1 100644 --- a/framework/db/simple_module_db/stored_settings.py +++ b/framework/db/simple_module_db/stored_settings.py @@ -18,7 +18,7 @@ from pydantic_settings import BaseSettings from simple_module_core.settings_hydrate import build_settings from sqlalchemy import Select, UniqueConstraint, inspect, select -from sqlalchemy.ext.asyncio import AsyncSession +from sqlalchemy.ext.asyncio import AsyncConnection, AsyncSession from sqlalchemy.orm import Session from sqlalchemy.sql.expression import TableClause, column, table from sqlmodel import Field @@ -156,7 +156,11 @@ async def get_overrides(session: AsyncSession, package: str) -> dict[str, tuple[ async def settings_table_exists(session: AsyncSession) -> bool: """Whether the settings table has been created (false on a fresh install).""" - conn = await session.connection() + return await settings_table_exists_on(await session.connection()) + + +async def settings_table_exists_on(conn: AsyncConnection) -> bool: + """:func:`settings_table_exists` for a caller that already holds a connection.""" return await conn.run_sync(lambda sync: inspect(sync).has_table(TABLE_SETTING)) diff --git a/framework/hosting/simple_module_hosting/_preapp_config.py b/framework/hosting/simple_module_hosting/_preapp_config.py index 71191359f..021c3f317 100644 --- a/framework/hosting/simple_module_hosting/_preapp_config.py +++ b/framework/hosting/simple_module_hosting/_preapp_config.py @@ -39,11 +39,11 @@ COL_VALUE_TYPE, SCOPE_SYSTEM, SYSTEM_SCOPE_ID, - TABLE_SETTING, package_overrides, settings_table_clause, + settings_table_exists_on, ) -from sqlalchemy import inspect, select +from sqlalchemy import select from sqlalchemy.ext.asyncio import create_async_engine from sqlalchemy.pool import NullPool @@ -89,7 +89,7 @@ async def _read(database_url: str) -> dict[str, tuple[str, str]]: # table" is a fresh install and must stay quiet, while a refused # connection or a failing query is not, and the driver errors for # the two are not reliably distinguishable across backends. - if not await conn.run_sync(lambda sync: inspect(sync).has_table(TABLE_SETTING)): + if not await settings_table_exists_on(conn): return {} rows = (await conn.execute(stmt)).tuples().all() finally: diff --git a/framework/hosting/simple_module_hosting/_secret_key.py b/framework/hosting/simple_module_hosting/_secret_key.py index ce7aea072..41ca9a335 100644 --- a/framework/hosting/simple_module_hosting/_secret_key.py +++ b/framework/hosting/simple_module_hosting/_secret_key.py @@ -27,11 +27,11 @@ COL_VALUE_TYPE, SCOPE_SYSTEM, SYSTEM_SCOPE_ID, - TABLE_SETTING, VALUE_TYPE_STRING, settings_table_clause, + settings_table_exists_on, ) -from sqlalchemy import and_, insert, inspect, select, update +from sqlalchemy import and_, insert, select, update from sqlalchemy.exc import IntegrityError from sqlalchemy.ext.asyncio import create_async_engine from sqlalchemy.pool import NullPool @@ -104,7 +104,7 @@ async def _ensure(database_url: str) -> str | None: engine = create_async_engine(database_url, poolclass=NullPool) try: async with engine.connect() as conn: - if not await conn.run_sync(lambda sync: inspect(sync).has_table(TABLE_SETTING)): + if not await settings_table_exists_on(conn): return None existing = await _fetch(conn) if existing: diff --git a/modules/settings/settings/_row_masking.py b/modules/settings/settings/_row_masking.py index e2190e993..9ea41c241 100644 --- a/modules/settings/settings/_row_masking.py +++ b/modules/settings/settings/_row_masking.py @@ -2,7 +2,7 @@ Split out of ``service`` so the "is this a secret, and is this write the mask being echoed back?" question lives in one file, next to the field-level rules in -``_secrets`` that it delegates to. ``service`` imports these; nothing else +``simple_module_core.secret_fields`` that it delegates to. ``service`` imports these; nothing else should need to, because the point of funnelling every read through ``out`` is that there is no second way to serialize a row. """ diff --git a/modules/settings/settings/store.py b/modules/settings/settings/store.py index 51520cb10..4a7cbbba4 100644 --- a/modules/settings/settings/store.py +++ b/modules/settings/settings/store.py @@ -2,8 +2,8 @@ The read-only queries (``package_overrides``, ``get_overrides_sync``) moved to ``simple_module_db.stored_settings`` with the table (#500) and are re-exported -here. ``SettingsStore`` satisfies ``simple_module_core.settings_hydrate. -OverrideSource``. +here. ``SettingsStore`` satisfies +``simple_module_core.settings_hydrate.OverrideSource``. Wraps the existing SettingService. Keys are namespaced ``.`` to avoid collision with free-form user-defined setting keys. From 0c1675e256213e660cac838fc9e3b488f1f19306 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Wed, 21 Oct 2026 07:43:20 +0200 Subject: [PATCH 8/9] chore(review): tidy register_settings blanks, scaffold comment, docstring wrap Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 --- .../templates/module/__PACKAGE__/module.py.tpl | 2 +- .../hosting/simple_module_hosting/_preapp_config.py | 13 +++++++------ modules/background_tasks/background_tasks/module.py | 1 - modules/file_storage/file_storage/module.py | 1 - modules/keycloak/keycloak/module.py | 1 - modules/site_lock/site_lock/module.py | 1 - modules/users/users/module.py | 1 - 7 files changed, 8 insertions(+), 12 deletions(-) diff --git a/framework/cli/simple_module_cli/templates/module/__PACKAGE__/module.py.tpl b/framework/cli/simple_module_cli/templates/module/__PACKAGE__/module.py.tpl index b049cedb4..f2c68b46c 100644 --- a/framework/cli/simple_module_cli/templates/module/__PACKAGE__/module.py.tpl +++ b/framework/cli/simple_module_cli/templates/module/__PACKAGE__/module.py.tpl @@ -43,7 +43,7 @@ class {{MODULE_NAME}}Module(ModuleBase): from {{PACKAGE_NAME}}.settings import {{MODULE_NAME}}Settings # Records the class so the host hydrates it from the settings table - # (env → DB → default) and mounts the services on app.state.{{PACKAGE_NAME}}. + # (DB → default) and mounts the services on app.state.{{PACKAGE_NAME}}. register_module_settings( app, "{{PACKAGE_NAME}}", diff --git a/framework/hosting/simple_module_hosting/_preapp_config.py b/framework/hosting/simple_module_hosting/_preapp_config.py index 021c3f317..8133d0594 100644 --- a/framework/hosting/simple_module_hosting/_preapp_config.py +++ b/framework/hosting/simple_module_hosting/_preapp_config.py @@ -60,10 +60,11 @@ class HostSettingsUnavailableError(RuntimeError): """The stored host settings exist but could not be read before boot. - Raised only outside development. Building with defaults instead would silently drop every - build-time override — ``multi_tenant`` above all, so one worker that - booted during a DB blip would serve requests single-tenant, with no tenant - middleware and no strict filter, while its siblings stay isolated (#494). + Raised only outside development. Building with defaults instead would + silently drop every build-time override — ``multi_tenant`` above all, so + one worker that booted during a DB blip would serve requests single-tenant, + with no tenant middleware and no strict filter, while its siblings stay + isolated (#494). """ @@ -101,8 +102,8 @@ def load_host_overrides(database_url: str, *, strict: bool = False) -> dict[str, """Return ``{field: (raw_value, value_type)}`` for host settings in the DB. Returns ``{}`` quietly when the database is reachable but not yet - migrated or holds no rows. Those are - the ordinary states of a fresh install, and what the setup wizard repairs. + migrated or holds no rows. Those are the ordinary states of a fresh + install, and what the setup wizard repairs. A failure to connect or to query is different: the stored overrides may exist and simply be unreadable right now. That logs a WARNING and, with diff --git a/modules/background_tasks/background_tasks/module.py b/modules/background_tasks/background_tasks/module.py index 5d3b0a532..088c83b12 100644 --- a/modules/background_tasks/background_tasks/module.py +++ b/modules/background_tasks/background_tasks/module.py @@ -79,7 +79,6 @@ class BackgroundTasksModule(ModuleBase): ) def register_settings(self, app: FastAPI) -> None: - from background_tasks.services import BackgroundTasksServices from background_tasks.settings import BackgroundTasksSettings diff --git a/modules/file_storage/file_storage/module.py b/modules/file_storage/file_storage/module.py index 5e92f4b81..14e7900da 100644 --- a/modules/file_storage/file_storage/module.py +++ b/modules/file_storage/file_storage/module.py @@ -71,7 +71,6 @@ class FileStorageModule(ModuleBase): ) def register_settings(self, app: FastAPI) -> None: - from file_storage.services import FileStorageServices from file_storage.settings import FileStorageSettings diff --git a/modules/keycloak/keycloak/module.py b/modules/keycloak/keycloak/module.py index e421eb768..9a9df10f2 100644 --- a/modules/keycloak/keycloak/module.py +++ b/modules/keycloak/keycloak/module.py @@ -27,7 +27,6 @@ class KeycloakModule(ModuleBase): _is_auth_provider = True def register_settings(self, app: FastAPI) -> None: - from keycloak.provider import KeycloakAuthProvider from keycloak.settings import KeycloakSettings from keycloak.state import KeycloakState diff --git a/modules/site_lock/site_lock/module.py b/modules/site_lock/site_lock/module.py index a30738839..01d27cca6 100644 --- a/modules/site_lock/site_lock/module.py +++ b/modules/site_lock/site_lock/module.py @@ -24,7 +24,6 @@ class SiteLockModule(ModuleBase): ) def register_settings(self, app: FastAPI) -> None: - from site_lock.settings import SiteLockSettings from site_lock.state import SiteLockState diff --git a/modules/users/users/module.py b/modules/users/users/module.py index bfd8cb3b8..d8d6a1b9a 100644 --- a/modules/users/users/module.py +++ b/modules/users/users/module.py @@ -56,7 +56,6 @@ class UsersModule(ModuleBase): _is_auth_provider = True def register_settings(self, app: FastAPI) -> None: - from users.settings import UsersSettings from users.state import UsersState From 5426955f3136ef014dcd4867be00bb586601adb7 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Wed, 21 Oct 2026 07:46:34 +0200 Subject: [PATCH 9/9] fix: address code review findings (round 1, pass 1) Keep hydrate_settings' pre-#500 keyword name 'store' so keyword callers of the settings.hydrate shim keep working, and attribute the shims' DeprecationWarning to the importer instead of importlib's frozen frames. Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 --- framework/core/simple_module_core/settings_hydrate.py | 10 +++++++--- modules/settings/settings/hydrate.py | 3 ++- modules/settings/settings/module_registry.py | 3 ++- modules/settings/settings/registration.py | 3 ++- 4 files changed, 13 insertions(+), 6 deletions(-) diff --git a/framework/core/simple_module_core/settings_hydrate.py b/framework/core/simple_module_core/settings_hydrate.py index 87d4e6682..2e76ec527 100644 --- a/framework/core/simple_module_core/settings_hydrate.py +++ b/framework/core/simple_module_core/settings_hydrate.py @@ -184,7 +184,11 @@ def build_settings[T: BaseSettings]( async def hydrate_settings[T: BaseSettings]( - cls: type[T], source: OverrideSource, package: str, *, lenient: bool = False + cls: type[T], store: OverrideSource, package: str, *, lenient: bool = False ) -> T: - """Construct ``cls`` with ``source``'s overrides merged over pydantic defaults.""" - return build_settings(cls, await source.get_overrides(package), lenient=lenient) + """Construct ``cls`` with ``store``'s overrides merged over pydantic defaults. + + The parameter keeps its pre-#500 name ``store`` so keyword callers of the + old ``settings.hydrate.hydrate_settings`` keep working. + """ + return build_settings(cls, await store.get_overrides(package), lenient=lenient) diff --git a/modules/settings/settings/hydrate.py b/modules/settings/settings/hydrate.py index e55fc97e5..060ece33a 100644 --- a/modules/settings/settings/hydrate.py +++ b/modules/settings/settings/hydrate.py @@ -33,5 +33,6 @@ "settings.hydrate is deprecated; import from simple_module_core.settings_hydrate " "(hydrate_settings_sync: simple_module_db.stored_settings)", DeprecationWarning, - stacklevel=2, + # Attribute the warning to the importer, not importlib's frozen frames. + skip_file_prefixes=("