From f6242ccda35882e703f96be56f73998dbca19258 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Thu, 22 Oct 2026 12:58:56 +0200 Subject: [PATCH 01/15] refactor(tenants): split tenants service by responsibility (#522) Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 --- modules/tenants/tenants/_membership_ops.py | 149 +++++++++++++++++++++ modules/tenants/tenants/service.py | 122 +---------------- 2 files changed, 152 insertions(+), 119 deletions(-) create mode 100644 modules/tenants/tenants/_membership_ops.py diff --git a/modules/tenants/tenants/_membership_ops.py b/modules/tenants/tenants/_membership_ops.py new file mode 100644 index 000000000..6f2cf0836 --- /dev/null +++ b/modules/tenants/tenants/_membership_ops.py @@ -0,0 +1,149 @@ +"""Membership operations of :class:`tenants.service.TenantService`. + +Split out of ``service`` by responsibility: tenant lifecycle stays there, the +member roster, seats and role changes live here as a mixin. +""" + +from __future__ import annotations + +from collections.abc import Awaitable, Callable, Sequence +from typing import TYPE_CHECKING + +from simple_module_core.events import Event +from sqlalchemy import func, select +from sqlalchemy.exc import IntegrityError +from sqlalchemy.ext.asyncio import AsyncSession + +from tenants._owner_guard import demote_owner, remove_owner +from tenants.constants import ENTITLEMENT_SEATS, MembershipRole +from tenants.contracts.entitlements import EntitlementProvider, ensure_within_limit +from tenants.contracts.events import MembershipAdded, MembershipRemoved +from tenants.errors import TenantError +from tenants.models import Membership, Tenant + + +class MembershipOpsMixin: + if TYPE_CHECKING: + db: AsyncSession + entitlements: EntitlementProvider + + def _after_commit( + self, + *events: Event, + invalidate: Sequence[str | None] = (), + callback: Callable[[], Awaitable[None]] | None = None, + ) -> None: ... + async def lock(self, tenant_id: str) -> Tenant: ... + async def _require(self, tenant_id: str) -> Tenant: ... + + async def list_for_user(self, user_id: str) -> list[tuple[Tenant, str]]: + stmt = ( + select(Tenant, Membership.role) + .join(Membership, Membership.tenant_id == Tenant.id) + .where(Membership.user_id == user_id) + .order_by(Membership.created_at, Tenant.name) + ) + return [(t, role) for t, role in (await self.db.execute(stmt)).all()] + + async def has_member_email(self, tenant_id: str, email: str) -> bool: + stmt = select(Membership.id).where( + Membership.tenant_id == tenant_id, func.lower(Membership.email) == email.lower() + ) + return await self.db.scalar(stmt) is not None + + async def get_membership(self, tenant_id: str, user_id: str) -> Membership | None: + stmt = select(Membership).where( + Membership.tenant_id == tenant_id, # ty: ignore[invalid-argument-type] + Membership.user_id == user_id, # ty: ignore[invalid-argument-type] + ) + return (await self.db.execute(stmt)).scalar_one_or_none() + + async def list_members(self, tenant_id: str) -> list[Membership]: + stmt = ( + select(Membership) + .where(Membership.tenant_id == tenant_id) # ty: ignore[invalid-argument-type] + .order_by(Membership.created_at) + ) + return list((await self.db.execute(stmt)).scalars().all()) + + async def seats_used(self, tenant_id: str) -> int: + """Members plus pending invitations — an invite reserves a seat.""" + from tenants.invitations import pending_invitation_count + + members = await self.db.scalar( + select(func.count()).select_from(Membership).where(Membership.tenant_id == tenant_id) # ty: ignore[invalid-argument-type] + ) + return int(members or 0) + await pending_invitation_count(self.db, tenant_id) + + async def ensure_seat_available(self, tenant_id: str) -> None: + await self.lock(tenant_id) + used = await self.seats_used(tenant_id) + await ensure_within_limit(self.entitlements, tenant_id, ENTITLEMENT_SEATS, used) + + async def add_member( + self, + tenant_id: str, + user_id: str, + role: MembershipRole, + *, + email: str | None = None, + seat_reserved: bool = False, + ) -> Membership: + """Add ``user_id``. ``seat_reserved``: the seat was held by an invitation.""" + await self._require(tenant_id) + existing = await self.get_membership(tenant_id, user_id) + if existing is not None: + return existing + if not seat_reserved: + await self.ensure_seat_available(tenant_id) + membership = Membership(tenant_id=tenant_id, user_id=user_id, role=role, email=email) + self.db.add(membership) + try: + await self.db.flush() # a concurrent join of the same user loses here + except IntegrityError as exc: + raise TenantError("already_member", status_code=409) from exc + self._after_commit(MembershipAdded(tenant_id, user_id, role), invalidate=[user_id]) + return membership + + async def _owner_count(self, tenant_id: str) -> int: + stmt = ( + select(func.count()) + .select_from(Membership) + .where(Membership.tenant_id == tenant_id, Membership.role == MembershipRole.OWNER) # ty: ignore[invalid-argument-type] + ) + return int(await self.db.scalar(stmt) or 0) + + async def change_role( + self, tenant_id: str, user_id: str, role: MembershipRole, *, actor_role: str + ) -> Membership: + await self.lock(tenant_id) + membership = await self.get_membership(tenant_id, user_id) + if membership is None: + raise TenantError("member_not_found", status_code=404) + touches_owner = MembershipRole.OWNER in (role, membership.role) + if touches_owner and actor_role != MembershipRole.OWNER: + raise TenantError("owner_required", status_code=403) + demotes_owner = membership.role == MembershipRole.OWNER and role != MembershipRole.OWNER + if demotes_owner: + if not await demote_owner(self.db, membership, role): + raise TenantError("last_owner", status_code=409) + else: + membership.role = role + await self.db.flush() + self._after_commit(invalidate=[user_id]) + return membership + + async def remove_member(self, tenant_id: str, user_id: str, *, actor_role: str) -> None: + await self.lock(tenant_id) + membership = await self.get_membership(tenant_id, user_id) + if membership is None: + raise TenantError("member_not_found", status_code=404) + if membership.role == MembershipRole.OWNER: + if actor_role != MembershipRole.OWNER: + raise TenantError("owner_required", status_code=403) + if not await remove_owner(self.db, membership): + raise TenantError("last_owner", status_code=409) + else: + await self.db.delete(membership) + await self.db.flush() + self._after_commit(MembershipRemoved(tenant_id, user_id), invalidate=[user_id]) diff --git a/modules/tenants/tenants/service.py b/modules/tenants/tenants/service.py index 36279869d..0ed35869d 100644 --- a/modules/tenants/tenants/service.py +++ b/modules/tenants/tenants/service.py @@ -22,16 +22,14 @@ from sqlalchemy.ext.asyncio import AsyncSession from tenants._deletion import delete_tenant_rows -from tenants._owner_guard import demote_owner, remove_owner -from tenants.constants import ENTITLEMENT_SEATS, MAX_SLUG_LEN, MembershipRole, TenantStatus +from tenants._membership_ops import MembershipOpsMixin +from tenants.constants import MAX_SLUG_LEN, MembershipRole, TenantStatus from tenants.contracts.entitlements import ( EntitlementProvider, UnlimitedEntitlements, - ensure_within_limit, ) from tenants.contracts.events import ( MembershipAdded, - MembershipRemoved, TenantCreated, TenantStatusChanged, ) @@ -51,7 +49,7 @@ def slugify(name: str) -> str: return slug[:MAX_SLUG_LEN].strip("-") or "org" -class TenantService: +class TenantService(MembershipOpsMixin): def __init__( self, db: AsyncSession, @@ -184,117 +182,3 @@ async def _require(self, tenant_id: str) -> Tenant: if tenant is None: raise TenantError("not_found", status_code=404) return tenant - - # ── memberships ───────────────────────────────────────────── - - async def list_for_user(self, user_id: str) -> list[tuple[Tenant, str]]: - stmt = ( - select(Tenant, Membership.role) - .join(Membership, Membership.tenant_id == Tenant.id) - .where(Membership.user_id == user_id) - .order_by(Membership.created_at, Tenant.name) - ) - return [(t, role) for t, role in (await self.db.execute(stmt)).all()] - - async def has_member_email(self, tenant_id: str, email: str) -> bool: - stmt = select(Membership.id).where( - Membership.tenant_id == tenant_id, func.lower(Membership.email) == email.lower() - ) - return await self.db.scalar(stmt) is not None - - async def get_membership(self, tenant_id: str, user_id: str) -> Membership | None: - stmt = select(Membership).where( - Membership.tenant_id == tenant_id, # ty: ignore[invalid-argument-type] - Membership.user_id == user_id, # ty: ignore[invalid-argument-type] - ) - return (await self.db.execute(stmt)).scalar_one_or_none() - - async def list_members(self, tenant_id: str) -> list[Membership]: - stmt = ( - select(Membership) - .where(Membership.tenant_id == tenant_id) # ty: ignore[invalid-argument-type] - .order_by(Membership.created_at) - ) - return list((await self.db.execute(stmt)).scalars().all()) - - async def seats_used(self, tenant_id: str) -> int: - """Members plus pending invitations — an invite reserves a seat.""" - from tenants.invitations import pending_invitation_count - - members = await self.db.scalar( - select(func.count()).select_from(Membership).where(Membership.tenant_id == tenant_id) # ty: ignore[invalid-argument-type] - ) - return int(members or 0) + await pending_invitation_count(self.db, tenant_id) - - async def ensure_seat_available(self, tenant_id: str) -> None: - await self.lock(tenant_id) - used = await self.seats_used(tenant_id) - await ensure_within_limit(self.entitlements, tenant_id, ENTITLEMENT_SEATS, used) - - async def add_member( - self, - tenant_id: str, - user_id: str, - role: MembershipRole, - *, - email: str | None = None, - seat_reserved: bool = False, - ) -> Membership: - """Add ``user_id``. ``seat_reserved``: the seat was held by an invitation.""" - await self._require(tenant_id) - existing = await self.get_membership(tenant_id, user_id) - if existing is not None: - return existing - if not seat_reserved: - await self.ensure_seat_available(tenant_id) - membership = Membership(tenant_id=tenant_id, user_id=user_id, role=role, email=email) - self.db.add(membership) - try: - await self.db.flush() # a concurrent join of the same user loses here - except IntegrityError as exc: - raise TenantError("already_member", status_code=409) from exc - self._after_commit(MembershipAdded(tenant_id, user_id, role), invalidate=[user_id]) - return membership - - async def _owner_count(self, tenant_id: str) -> int: - stmt = ( - select(func.count()) - .select_from(Membership) - .where(Membership.tenant_id == tenant_id, Membership.role == MembershipRole.OWNER) # ty: ignore[invalid-argument-type] - ) - return int(await self.db.scalar(stmt) or 0) - - async def change_role( - self, tenant_id: str, user_id: str, role: MembershipRole, *, actor_role: str - ) -> Membership: - await self.lock(tenant_id) - membership = await self.get_membership(tenant_id, user_id) - if membership is None: - raise TenantError("member_not_found", status_code=404) - touches_owner = MembershipRole.OWNER in (role, membership.role) - if touches_owner and actor_role != MembershipRole.OWNER: - raise TenantError("owner_required", status_code=403) - demotes_owner = membership.role == MembershipRole.OWNER and role != MembershipRole.OWNER - if demotes_owner: - if not await demote_owner(self.db, membership, role): - raise TenantError("last_owner", status_code=409) - else: - membership.role = role - await self.db.flush() - self._after_commit(invalidate=[user_id]) - return membership - - async def remove_member(self, tenant_id: str, user_id: str, *, actor_role: str) -> None: - await self.lock(tenant_id) - membership = await self.get_membership(tenant_id, user_id) - if membership is None: - raise TenantError("member_not_found", status_code=404) - if membership.role == MembershipRole.OWNER: - if actor_role != MembershipRole.OWNER: - raise TenantError("owner_required", status_code=403) - if not await remove_owner(self.db, membership): - raise TenantError("last_owner", status_code=409) - else: - await self.db.delete(membership) - await self.db.flush() - self._after_commit(MembershipRemoved(tenant_id, user_id), invalidate=[user_id]) From 483d745b8a074a58e5890223c4efeaa842712cc0 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Thu, 22 Oct 2026 12:59:02 +0200 Subject: [PATCH 02/15] refactor(db): split migrations.py by responsibility (#522) Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 --- .../db/simple_module_db/_migration_render.py | 72 +++++++++++++++++++ framework/db/simple_module_db/migrations.py | 67 +---------------- 2 files changed, 73 insertions(+), 66 deletions(-) create mode 100644 framework/db/simple_module_db/_migration_render.py diff --git a/framework/db/simple_module_db/_migration_render.py b/framework/db/simple_module_db/_migration_render.py new file mode 100644 index 000000000..448d3c6ff --- /dev/null +++ b/framework/db/simple_module_db/_migration_render.py @@ -0,0 +1,72 @@ +"""Alembic ``render_item`` callback for SQLModel + extension types.""" + +from __future__ import annotations + +from enum import StrEnum + +import sqlalchemy as sa +from sqlmodel.sql.sqltypes import UTCDateTime + +from simple_module_db.extensions import render_extension_type +from simple_module_db.migration_portability import render_boolean_default + + +def render_item(type_, obj, autogen_context): + """Alembic ``render_item`` callback for SQLModel + extension types. + + * Collapses SQLModel's ``AutoString`` to ``sa.String`` and ``UTCDateTime`` + to ``sa.DateTime(timezone=True)`` so migrations don't need to + ``import sqlmodel`` (GH #440). Any other ``sqlmodel`` type still gets + the import added, so the migration stays importable. + * Renders Python ``StrEnum`` columns with ``values_callable`` so the + Postgres enum labels match the lowercase ``StrEnum`` values rather than + SQLAlchemy's default of using uppercase attribute names. This means raw + SQL like ``WHERE status = 'ready'`` actually works against the live DB. + * Renders ``true()``/``false()`` server defaults as ``sa.true()``/``sa.false()`` + instead of the dialect-specific ``sa.text('1')``/``sa.text('0')`` (GH #342). + * Renders ``LTree``/``TSVector`` through ``simple_module_db.types`` (with + the import) so one revision upgrades on Postgres and SQLite alike. + * Adds the necessary imports for ``fastapi_users_db_sqlalchemy.generics`` + and ``geoalchemy2`` types (rendered by their own classes elsewhere) so + the generated migration is importable. + + Pass to :func:`alembic.context.configure` as ``render_item=render_item``. + """ + if type_ == "server_default": + return render_boolean_default(obj, autogen_context) + if type_ != "type": + return False + cls_name = type(obj).__name__ + cls_module = type(obj).__module__ or "" + imports = autogen_context.imports if autogen_context is not None else None + + if cls_name == "AutoString": + length = getattr(obj, "length", None) + return f"sa.String(length={length})" if length is not None else "sa.String()" + if isinstance(obj, UTCDateTime): # includes our ``AwareDateTime`` subclass + return "sa.DateTime(timezone=True)" + + if (rendered := render_extension_type(obj, imports)) is not None: + return rendered + + if imports is not None and cls_module.startswith("sqlmodel"): + imports.add("import sqlmodel") + + if imports is not None and cls_module.startswith("fastapi_users_db_sqlalchemy"): + imports.add("import fastapi_users_db_sqlalchemy.generics") + if imports is not None and cls_module.startswith("geoalchemy2"): + imports.add("import geoalchemy2") + + # Match by isinstance, not class-name string — a user-defined ``Enum`` + # class elsewhere in the project would otherwise be matched here and + # render with ``values_callable`` against an unrelated ``enum_class``. + if isinstance(obj, sa.Enum): + python_type = getattr(obj, "enum_class", None) + if isinstance(python_type, type) and issubclass(python_type, StrEnum): + name = getattr(obj, "name", None) or python_type.__name__.lower() + members = ", ".join(repr(m.value) for m in python_type) + return ( + f"sa.Enum({members}, name={name!r}, values_callable=lambda e: [m.value for m in e])" + ) + + return False # let alembic use its default rendering diff --git a/framework/db/simple_module_db/migrations.py b/framework/db/simple_module_db/migrations.py index ef1548638..51c3fdfca 100644 --- a/framework/db/simple_module_db/migrations.py +++ b/framework/db/simple_module_db/migrations.py @@ -17,26 +17,22 @@ import importlib import logging from collections.abc import Callable, Sequence -from enum import StrEnum from typing import Literal -import sqlalchemy as sa from alembic.operations.ops import CreateIndexOp, CreateTableOp, DropIndexOp, DropTableOp from simple_module_core import ModuleBase from simple_module_core.diagnostics._expression_index import index_is_expression_based from simple_module_core.discovery import discover_modules, get_module_package_name from sqlalchemy import Index, MetaData from sqlalchemy.schema import SchemaItem -from sqlmodel.sql.sqltypes import UTCDateTime +from simple_module_db._migration_render import render_item # noqa: F401 (re-exported) from simple_module_db.base import all_module_bases from simple_module_db.extensions import ( # noqa: F401 (ensure_db_extensions re-exported) ensure_db_extensions, - render_extension_type, ) from simple_module_db.migration_portability import ( iter_ops_recursive, - render_boolean_default, rewrite_boolean_defaults, warn_unverifiable_expression_indexes, ) @@ -237,64 +233,3 @@ def _inject_drop_index_before_drop_table(downgrade_ops, expression_indexes) -> N existing_drop_names.add(index.name) new_ops.append(op) downgrade_ops.ops = new_ops - - -def render_item(type_, obj, autogen_context): - """Alembic ``render_item`` callback for SQLModel + extension types. - - * Collapses SQLModel's ``AutoString`` to ``sa.String`` and ``UTCDateTime`` - to ``sa.DateTime(timezone=True)`` so migrations don't need to - ``import sqlmodel`` (GH #440). Any other ``sqlmodel`` type still gets - the import added, so the migration stays importable. - * Renders Python ``StrEnum`` columns with ``values_callable`` so the - Postgres enum labels match the lowercase ``StrEnum`` values rather than - SQLAlchemy's default of using uppercase attribute names. This means raw - SQL like ``WHERE status = 'ready'`` actually works against the live DB. - * Renders ``true()``/``false()`` server defaults as ``sa.true()``/``sa.false()`` - instead of the dialect-specific ``sa.text('1')``/``sa.text('0')`` (GH #342). - * Renders ``LTree``/``TSVector`` through ``simple_module_db.types`` (with - the import) so one revision upgrades on Postgres and SQLite alike. - * Adds the necessary imports for ``fastapi_users_db_sqlalchemy.generics`` - and ``geoalchemy2`` types (rendered by their own classes elsewhere) so - the generated migration is importable. - - Pass to :func:`alembic.context.configure` as ``render_item=render_item``. - """ - if type_ == "server_default": - return render_boolean_default(obj, autogen_context) - if type_ != "type": - return False - cls_name = type(obj).__name__ - cls_module = type(obj).__module__ or "" - imports = autogen_context.imports if autogen_context is not None else None - - if cls_name == "AutoString": - length = getattr(obj, "length", None) - return f"sa.String(length={length})" if length is not None else "sa.String()" - if isinstance(obj, UTCDateTime): # includes our ``AwareDateTime`` subclass - return "sa.DateTime(timezone=True)" - - if (rendered := render_extension_type(obj, imports)) is not None: - return rendered - - if imports is not None and cls_module.startswith("sqlmodel"): - imports.add("import sqlmodel") - - if imports is not None and cls_module.startswith("fastapi_users_db_sqlalchemy"): - imports.add("import fastapi_users_db_sqlalchemy.generics") - if imports is not None and cls_module.startswith("geoalchemy2"): - imports.add("import geoalchemy2") - - # Match by isinstance, not class-name string — a user-defined ``Enum`` - # class elsewhere in the project would otherwise be matched here and - # render with ``values_callable`` against an unrelated ``enum_class``. - if isinstance(obj, sa.Enum): - python_type = getattr(obj, "enum_class", None) - if isinstance(python_type, type) and issubclass(python_type, StrEnum): - name = getattr(obj, "name", None) or python_type.__name__.lower() - members = ", ".join(repr(m.value) for m in python_type) - return ( - f"sa.Enum({members}, name={name!r}, values_callable=lambda e: [m.value for m in e])" - ) - - return False # let alembic use its default rendering From 170fb718a6c6ca57eaed07d7a042a2204219f5ce Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Thu, 22 Oct 2026 12:59:05 +0200 Subject: [PATCH 03/15] refactor(permissions): split service by responsibility (#522) Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 --- .../permissions/permissions/_registry_sync.py | 112 ++++++++++++++++++ modules/permissions/permissions/service.py | 86 +------------- 2 files changed, 114 insertions(+), 84 deletions(-) create mode 100644 modules/permissions/permissions/_registry_sync.py diff --git a/modules/permissions/permissions/_registry_sync.py b/modules/permissions/permissions/_registry_sync.py new file mode 100644 index 000000000..079c8bfbb --- /dev/null +++ b/modules/permissions/permissions/_registry_sync.py @@ -0,0 +1,112 @@ +"""Registry resolution and startup sync of :class:`PermissionService`. + +Split out of ``service`` by responsibility: role/user CRUD stays there, the +registry-side resolution and the startup sync to the DB live here as a mixin. +""" + +from __future__ import annotations + +import uuid +from typing import TYPE_CHECKING + +from simple_module_core.permissions import ADMIN_ROLE, WILDCARD +from simple_module_core.user_directory import DirectoryUser +from sqlalchemy import select +from sqlalchemy.ext.asyncio import AsyncSession + +from permissions.models import RolePermission + +if TYPE_CHECKING: + from simple_module_core.permissions import PermissionRegistry + + +def _role_names(user: DirectoryUser) -> list[str]: + # ``None``: the provider keeps roles elsewhere, so nothing is inherited here. + return list(user.roles or ()) + + +class RegistrySyncMixin: + if TYPE_CHECKING: + db: AsyncSession + registry: PermissionRegistry + + async def _get_user(self, user_id: uuid.UUID) -> DirectoryUser | None: ... + async def get_user_direct_keys(self, user_id: uuid.UUID) -> list[str]: ... + async def _get_role_keys(self, role_name: str) -> list[str]: ... + + # ── Effective-permissions resolution ─────────────────────── + + def _resolve_role_permissions(self, role_names: list[str]) -> set[str]: + """Resolve role names to their permission keys via the registry.""" + role_map = self.registry.role_map + resolved: set[str] = set() + for name in role_names: + perms = role_map.get(name, []) + if WILDCARD in perms: + return set(self.registry.all_permissions) + resolved.update(perms) + return resolved + + def _resolve_role_sources(self, role_names: list[str]) -> dict[str, list[str]]: + """Map each inherited permission key to the roles that grant it. + + "Inherited" alone doesn't tell an admin what to change — they need to + know *which* role to edit. Two roles can grant the same key, so the + value is a list. + """ + role_map = self.registry.role_map + sources: dict[str, list[str]] = {} + for name in sorted(role_names): + perms = role_map.get(name, []) + keys = self.registry.all_permissions if WILDCARD in perms else perms + for key in keys: + sources.setdefault(key, []).append(name) + return sources + + async def resolve_effective_permissions(self, user_id: uuid.UUID) -> set[str]: + """Return every permission key the user holds (role-inherited + direct).""" + user = await self._get_user(user_id) + if user is None: + return set() + direct = set(await self.get_user_direct_keys(user_id)) + inherited = self._resolve_role_permissions(_role_names(user)) + return direct | inherited + + # ── Startup sync ─────────────────────────────────────────── + + async def load_all_into_registry(self) -> None: + """Apply every persisted role→permission row onto the registry. + + Runs on app startup so admin-assigned role permissions survive a + restart. Direct user grants are *not* replayed here because the + registry is role-scoped; user grants are resolved on every + request (see :meth:`resolve_effective_permissions`). + """ + result = await self.db.execute( + select(RolePermission.role_name, RolePermission.permission_key) + ) + by_role: dict[str, list[str]] = {} + for name, key in result.all(): + by_role.setdefault(name, []).append(key) + for name, keys in by_role.items(): + self.registry.set_role_overlay(name, keys) + + async def sync_admin_all_permissions(self, assigned_by: str | None = None) -> None: + """Ensure the admin role holds every registered permission key. + + Why: admin is granted ``*`` via the in-memory registry so authorization + always passes, but the role_permission table drives the merged admin + UI — without this sync it would show admin missing newly registered + keys until someone clicked Save. + """ + registered = set(self.registry.all_permissions) + existing = set(await self._get_role_keys(ADMIN_ROLE)) + missing = registered - existing + if not missing: + return + self.db.add_all( + RolePermission(role_name=ADMIN_ROLE, permission_key=key, assigned_by=assigned_by) + for key in missing + ) + await self.db.flush() + self.registry.set_role_overlay(ADMIN_ROLE, registered) diff --git a/modules/permissions/permissions/service.py b/modules/permissions/permissions/service.py index 2c0997284..597a671e7 100644 --- a/modules/permissions/permissions/service.py +++ b/modules/permissions/permissions/service.py @@ -6,11 +6,11 @@ from collections.abc import Callable from typing import TYPE_CHECKING -from simple_module_core.permissions import ADMIN_ROLE, WILDCARD from simple_module_core.user_directory import DirectoryUser, NullUserDirectory, UserDirectory from sqlalchemy import delete, func, select from sqlalchemy.ext.asyncio import AsyncSession +from permissions._registry_sync import RegistrySyncMixin, _role_names from permissions.contracts.schemas import ( PermissionGroupOut, RoleOut, @@ -30,12 +30,7 @@ def _user_out(user: DirectoryUser) -> UserOut: return UserOut(id=uuid.UUID(user.id), email=user.email, full_name=user.full_name) -def _role_names(user: DirectoryUser) -> list[str]: - # ``None``: the provider keeps roles elsewhere, so nothing is inherited here. - return list(user.roles or ()) - - -class PermissionService: +class PermissionService(RegistrySyncMixin): """Bridges the in-memory :class:`PermissionRegistry` and the DB. The registry is the source of truth for *which* permission keys exist @@ -217,80 +212,3 @@ async def set_user_permissions( inherited=sorted(inherited), inherited_by=self._resolve_role_sources(role_names), ) - - # ── Effective-permissions resolution ─────────────────────── - - def _resolve_role_permissions(self, role_names: list[str]) -> set[str]: - """Resolve role names to their permission keys via the registry.""" - role_map = self.registry.role_map - resolved: set[str] = set() - for name in role_names: - perms = role_map.get(name, []) - if WILDCARD in perms: - return set(self.registry.all_permissions) - resolved.update(perms) - return resolved - - def _resolve_role_sources(self, role_names: list[str]) -> dict[str, list[str]]: - """Map each inherited permission key to the roles that grant it. - - "Inherited" alone doesn't tell an admin what to change — they need to - know *which* role to edit. Two roles can grant the same key, so the - value is a list. - """ - role_map = self.registry.role_map - sources: dict[str, list[str]] = {} - for name in sorted(role_names): - perms = role_map.get(name, []) - keys = self.registry.all_permissions if WILDCARD in perms else perms - for key in keys: - sources.setdefault(key, []).append(name) - return sources - - async def resolve_effective_permissions(self, user_id: uuid.UUID) -> set[str]: - """Return every permission key the user holds (role-inherited + direct).""" - user = await self._get_user(user_id) - if user is None: - return set() - direct = set(await self.get_user_direct_keys(user_id)) - inherited = self._resolve_role_permissions(_role_names(user)) - return direct | inherited - - # ── Startup sync ─────────────────────────────────────────── - - async def load_all_into_registry(self) -> None: - """Apply every persisted role→permission row onto the registry. - - Runs on app startup so admin-assigned role permissions survive a - restart. Direct user grants are *not* replayed here because the - registry is role-scoped; user grants are resolved on every - request (see :meth:`resolve_effective_permissions`). - """ - result = await self.db.execute( - select(RolePermission.role_name, RolePermission.permission_key) - ) - by_role: dict[str, list[str]] = {} - for name, key in result.all(): - by_role.setdefault(name, []).append(key) - for name, keys in by_role.items(): - self.registry.set_role_overlay(name, keys) - - async def sync_admin_all_permissions(self, assigned_by: str | None = None) -> None: - """Ensure the admin role holds every registered permission key. - - Why: admin is granted ``*`` via the in-memory registry so authorization - always passes, but the role_permission table drives the merged admin - UI — without this sync it would show admin missing newly registered - keys until someone clicked Save. - """ - registered = set(self.registry.all_permissions) - existing = set(await self._get_role_keys(ADMIN_ROLE)) - missing = registered - existing - if not missing: - return - self.db.add_all( - RolePermission(role_name=ADMIN_ROLE, permission_key=key, assigned_by=assigned_by) - for key in missing - ) - await self.db.flush() - self.registry.set_role_overlay(ADMIN_ROLE, registered) From e8cfd69e3da5f18fe37cb6f31e03fbafbc9f7bb8 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Thu, 22 Oct 2026 12:59:21 +0200 Subject: [PATCH 04/15] refactor(file_storage): split service by responsibility (#522) Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 --- modules/file_storage/file_storage/deletes.py | 126 +++++++++++++++++++ modules/file_storage/file_storage/service.py | 97 +------------- 2 files changed, 130 insertions(+), 93 deletions(-) create mode 100644 modules/file_storage/file_storage/deletes.py diff --git a/modules/file_storage/file_storage/deletes.py b/modules/file_storage/file_storage/deletes.py new file mode 100644 index 000000000..bd400ceb9 --- /dev/null +++ b/modules/file_storage/file_storage/deletes.py @@ -0,0 +1,126 @@ +"""Delete operations of :class:`~file_storage.service.FileStorageService`. + +Mixed into the service so ``service.py`` stays about upload/download. +""" + +from __future__ import annotations + +import contextlib +import logging +import uuid +from collections.abc import Sequence +from datetime import UTC, datetime +from typing import TYPE_CHECKING + +from sqlalchemy import select +from sqlalchemy.ext.asyncio import AsyncSession + +from file_storage import queries, thumbnails +from file_storage.contracts.service import StorageNotFoundError +from file_storage.models import StoredFile +from file_storage.scope import platform_scope + +if TYPE_CHECKING: + from file_storage.contracts.service import StorageBackend + +# Keep the original logger name so existing log filters keep matching. +_logger = logging.getLogger("file_storage.service") + + +class FileStorageDeletes: + """Soft-delete rows and drop their backend objects.""" + + db: AsyncSession + backend: StorageBackend + + if TYPE_CHECKING: + + async def get( + self, file_id: uuid.UUID, *, platform: bool = False, owner: str | None = None + ) -> StoredFile: ... + + async def delete_many( + self, file_ids: Sequence[uuid.UUID], *, owner: str | None = None + ) -> list[StoredFile]: + """Soft-delete every id that still resolves, returning the rows removed. + + Ids that no longer resolve are skipped rather than raising: two admins + clearing the same selection is an ordinary race, and failing the batch + would leave the second one with half the rows gone and an error to + interpret. The caller reports how many rows it actually removed. + """ + if not file_ids: + return [] + stmt = queries.owned_by(select(StoredFile).where(StoredFile.id.in_(list(file_ids))), owner) + rows = list((await self.db.execute(stmt)).scalars().all()) + if not rows: + return [] + + now = datetime.now(UTC) + for row in rows: + row.is_deleted = True + row.deleted_at = now + # One flush for the batch: the DB rows are what the next page load + # reads, so they must all be marked before any object is dropped. + await self.db.flush() + for row in rows: + # Every object is dropped independently. The rows are already + # marked deleted, so letting one unreachable object abort the loop + # would 500 the request *and* leave the caller believing nothing + # happened, while the remaining objects stay behind as orphans with + # no row left pointing at them. A failure here is a janitor's + # problem, not the caller's. + try: + try: + await self.backend.delete(row.key) + finally: + # Variants go even when the original's delete fails. + await thumbnails.delete_variants(self.backend, row.key) + except StorageNotFoundError: + # Acceptably absent — eg. a previous delete partially succeeded. + pass + except Exception: + _logger.exception( + "file_storage.bulk_delete_object_failed key=%s — row is deleted, " + "object orphaned", + row.key, + ) + return rows + + async def delete( + self, + file_id: uuid.UUID, + *, + platform: bool = False, + drop_object: bool = True, + owner: str | None = None, + ) -> StoredFile: + """Soft-delete the row and (by default) drop the backend object. + + ``drop_object=False`` only flushes the soft-delete: a caller that must + commit first (so a failed commit never leaves a live row with no bytes) + then calls :meth:`drop_object` with the returned row. + """ + # One scope around read + write: its opening flush runs before the + # platform row is touched, so the guard never sees that change. + async with platform_scope(self.db, platform): + row = await self.get(file_id, platform=platform, owner=owner) + # Soft-delete in DB first; if the backend delete fails afterwards we + # still have a row marked deleted that can be reaped by a janitor. + row.is_deleted = True + row.deleted_at = datetime.now(UTC) + await self.db.flush() + if drop_object: + await self.drop_object(row) + return row + + async def drop_object(self, row: StoredFile) -> None: + """Delete ``row``'s backend object and its thumbnails; absent ones are fine.""" + try: + # Acceptably absent — eg. a previous delete partially succeeded. + with contextlib.suppress(StorageNotFoundError): + await self.backend.delete(row.key) + finally: + # A failed original delete still raises, but must not orphan the + # variants (``delete_variants`` itself never raises). + await thumbnails.delete_variants(self.backend, row.key) diff --git a/modules/file_storage/file_storage/service.py b/modules/file_storage/file_storage/service.py index 2e9bb9133..3a1d26e41 100644 --- a/modules/file_storage/file_storage/service.py +++ b/modules/file_storage/file_storage/service.py @@ -7,11 +7,10 @@ from __future__ import annotations -import contextlib import hashlib import logging import uuid -from collections.abc import AsyncIterator, Sequence +from collections.abc import AsyncIterator from datetime import UTC, datetime from pathlib import Path from typing import TYPE_CHECKING @@ -20,9 +19,9 @@ from sqlalchemy import select from sqlalchemy.ext.asyncio import AsyncSession -from file_storage import constants, queries, thumbnails +from file_storage import constants, queries from file_storage.contracts.schemas import StoredFileOut -from file_storage.contracts.service import StorageNotFoundError +from file_storage.deletes import FileStorageDeletes from file_storage.downloads import ( Download, RedirectDownload, @@ -55,7 +54,7 @@ class StoredFileNotFoundError(Exception): """Raised when a StoredFile lookup misses; endpoints map this to HTTP 404.""" -class FileStorageService(FileStoragePublic, FileStorageReads): +class FileStorageService(FileStoragePublic, FileStorageReads, FileStorageDeletes): """Orchestrates validation, hashing, backend IO, and DB lifecycle.""" def __init__( @@ -197,94 +196,6 @@ async def download( body = await self.backend.get(row.key) if open_body else None return StreamDownload(file=row, body=body) - # ── Delete ─────────────────────────────────────────────────────── - - async def delete_many( - self, file_ids: Sequence[uuid.UUID], *, owner: str | None = None - ) -> list[StoredFile]: - """Soft-delete every id that still resolves, returning the rows removed. - - Ids that no longer resolve are skipped rather than raising: two admins - clearing the same selection is an ordinary race, and failing the batch - would leave the second one with half the rows gone and an error to - interpret. The caller reports how many rows it actually removed. - """ - if not file_ids: - return [] - stmt = queries.owned_by(select(StoredFile).where(StoredFile.id.in_(list(file_ids))), owner) - rows = list((await self.db.execute(stmt)).scalars().all()) - if not rows: - return [] - - now = datetime.now(UTC) - for row in rows: - row.is_deleted = True - row.deleted_at = now - # One flush for the batch: the DB rows are what the next page load - # reads, so they must all be marked before any object is dropped. - await self.db.flush() - for row in rows: - # Every object is dropped independently. The rows are already - # marked deleted, so letting one unreachable object abort the loop - # would 500 the request *and* leave the caller believing nothing - # happened, while the remaining objects stay behind as orphans with - # no row left pointing at them. A failure here is a janitor's - # problem, not the caller's. - try: - try: - await self.backend.delete(row.key) - finally: - # Variants go even when the original's delete fails. - await thumbnails.delete_variants(self.backend, row.key) - except StorageNotFoundError: - # Acceptably absent — eg. a previous delete partially succeeded. - pass - except Exception: - _logger.exception( - "file_storage.bulk_delete_object_failed key=%s — row is deleted, " - "object orphaned", - row.key, - ) - return rows - - async def delete( - self, - file_id: uuid.UUID, - *, - platform: bool = False, - drop_object: bool = True, - owner: str | None = None, - ) -> StoredFile: - """Soft-delete the row and (by default) drop the backend object. - - ``drop_object=False`` only flushes the soft-delete: a caller that must - commit first (so a failed commit never leaves a live row with no bytes) - then calls :meth:`drop_object` with the returned row. - """ - # One scope around read + write: its opening flush runs before the - # platform row is touched, so the guard never sees that change. - async with platform_scope(self.db, platform): - row = await self.get(file_id, platform=platform, owner=owner) - # Soft-delete in DB first; if the backend delete fails afterwards we - # still have a row marked deleted that can be reaped by a janitor. - row.is_deleted = True - row.deleted_at = datetime.now(UTC) - await self.db.flush() - if drop_object: - await self.drop_object(row) - return row - - async def drop_object(self, row: StoredFile) -> None: - """Delete ``row``'s backend object and its thumbnails; absent ones are fine.""" - try: - # Acceptably absent — eg. a previous delete partially succeeded. - with contextlib.suppress(StorageNotFoundError): - await self.backend.delete(row.key) - finally: - # A failed original delete still raises, but must not orphan the - # variants (``delete_variants`` itself never raises). - await thumbnails.delete_variants(self.backend, row.key) - def generate_key(tenant_id: str, filename: str) -> str: """Build a tenant-prefixed, date-sharded, collision-proof key. From a7965146457cb87444f84a1713c271807d18fc58 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Thu, 22 Oct 2026 12:59:34 +0200 Subject: [PATCH 05/15] refactor(file_storage): split module.py by responsibility (#522) Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 --- .../file_storage/file_storage/audit_labels.py | 38 ++++++++++++ .../file_storage/file_storage/body_limits.py | 35 +++++++++++ modules/file_storage/file_storage/module.py | 60 +------------------ 3 files changed, 76 insertions(+), 57 deletions(-) create mode 100644 modules/file_storage/file_storage/audit_labels.py create mode 100644 modules/file_storage/file_storage/body_limits.py diff --git a/modules/file_storage/file_storage/audit_labels.py b/modules/file_storage/file_storage/audit_labels.py new file mode 100644 index 000000000..a6a3b4d33 --- /dev/null +++ b/modules/file_storage/file_storage/audit_labels.py @@ -0,0 +1,38 @@ +"""Audit-log label resolver for stored files.""" + +from __future__ import annotations + +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from sqlalchemy.ext.asyncio import AsyncSession + + +async def _resolve_file_labels(db: AsyncSession, ids: list[str]) -> dict[str, str]: + """Name a stored file by its filename — "q3-report.pdf", not a uuid. + + Ids that are not uuids belong to some other id space and are left unnamed, + which falls back to showing the id as stored. + """ + import uuid as _uuid + + from sqlalchemy import select + + from file_storage.models import StoredFile + + wanted: dict[_uuid.UUID, str] = {} + for raw in ids: + try: + wanted[_uuid.UUID(raw)] = raw + except (ValueError, AttributeError, TypeError): + continue + if not wanted: + return {} + # Deliberately cross-tenant: the audit log is a platform screen listing + # every tenant's entries, and the entry it labels already records the + # filename among its captured fields, so naming it discloses nothing new. + # Scoped to the bound tenant instead, it would fail closed for a platform + # admin with no organisation and leave every other tenant's file a uuid. + stmt = select(StoredFile.id, StoredFile.filename).where(StoredFile.id.in_(list(wanted))) + rows = (await db.execute(stmt.execution_options(all_tenants=True))).all() + return {wanted.get(file_id, str(file_id)): filename for file_id, filename in rows} diff --git a/modules/file_storage/file_storage/body_limits.py b/modules/file_storage/file_storage/body_limits.py new file mode 100644 index 000000000..175e3118d --- /dev/null +++ b/modules/file_storage/file_storage/body_limits.py @@ -0,0 +1,35 @@ +"""Body-size limits for the upload endpoints.""" + +from __future__ import annotations + +from simple_module_core.body_limits import BodyLimitRegistry + +from file_storage import constants +from file_storage.upload_support import choose_part_size + + +def register_upload_body_limits(registry: BodyLimitRegistry) -> None: + # The upload is multipart and its own ceiling (``max_file_size_bytes``, + # a runtime setting, 100 MB by default) is far above the host's global + # body guard. Track it, plus headroom for the multipart framing. + registry.add_exact( + constants.ROUTE_PREFIX_API + constants.PATH_UPLOAD, + lambda app: ( + app.state.file_storage.settings.max_file_size_bytes + constants.MULTIPART_OVERHEAD_BYTES + ), + methods={"POST"}, + ) + + # Filesystem parts are relayed through the app: one part at most, plus + # slack. (S3 parts never reach us — they go straight to the bucket.) + registry.add_regex( + constants.ROUTE_PREFIX_API + r"/uploads/[^/]+/parts/\d+$", + lambda app: ( + choose_part_size( + app.state.file_storage.settings.max_direct_upload_bytes, + app.state.file_storage.settings.multipart_part_size_bytes, + ) + + constants.PART_BODY_SLACK_BYTES + ), + methods={"PUT"}, + ) diff --git a/modules/file_storage/file_storage/module.py b/modules/file_storage/file_storage/module.py index 880feecd6..0ef50c6c2 100644 --- a/modules/file_storage/file_storage/module.py +++ b/modules/file_storage/file_storage/module.py @@ -21,45 +21,15 @@ from simple_module_core.tenancy import TenantRole, tenant_role from file_storage import constants -from file_storage.upload_support import choose_part_size +from file_storage.audit_labels import _resolve_file_labels +from file_storage.body_limits import register_upload_body_limits if TYPE_CHECKING: from fastapi import FastAPI - from sqlalchemy.ext.asyncio import AsyncSession logger = logging.getLogger(__name__) -async def _resolve_file_labels(db: AsyncSession, ids: list[str]) -> dict[str, str]: - """Name a stored file by its filename — "q3-report.pdf", not a uuid. - - Ids that are not uuids belong to some other id space and are left unnamed, - which falls back to showing the id as stored. - """ - import uuid as _uuid - - from sqlalchemy import select - - from file_storage.models import StoredFile - - wanted: dict[_uuid.UUID, str] = {} - for raw in ids: - try: - wanted[_uuid.UUID(raw)] = raw - except (ValueError, AttributeError, TypeError): - continue - if not wanted: - return {} - # Deliberately cross-tenant: the audit log is a platform screen listing - # every tenant's entries, and the entry it labels already records the - # filename among its captured fields, so naming it discloses nothing new. - # Scoped to the bound tenant instead, it would fail closed for a platform - # admin with no organisation and leave every other tenant's file a uuid. - stmt = select(StoredFile.id, StoredFile.filename).where(StoredFile.id.in_(list(wanted))) - rows = (await db.execute(stmt.execution_options(all_tenants=True))).all() - return {wanted.get(file_id, str(file_id)): filename for file_id, filename in rows} - - class FileStorageModule(ModuleBase): meta = ModuleMeta( name=constants.MODULE_PASCAL, @@ -186,31 +156,7 @@ def register_menu_items(self, registry: MenuRegistry) -> None: ) def register_body_limits(self, registry: BodyLimitRegistry) -> None: - # The upload is multipart and its own ceiling (``max_file_size_bytes``, - # a runtime setting, 100 MB by default) is far above the host's global - # body guard. Track it, plus headroom for the multipart framing. - registry.add_exact( - constants.ROUTE_PREFIX_API + constants.PATH_UPLOAD, - lambda app: ( - app.state.file_storage.settings.max_file_size_bytes - + constants.MULTIPART_OVERHEAD_BYTES - ), - methods={"POST"}, - ) - - # Filesystem parts are relayed through the app: one part at most, plus - # slack. (S3 parts never reach us — they go straight to the bucket.) - registry.add_regex( - constants.ROUTE_PREFIX_API + r"/uploads/[^/]+/parts/\d+$", - lambda app: ( - choose_part_size( - app.state.file_storage.settings.max_direct_upload_bytes, - app.state.file_storage.settings.multipart_part_size_bytes, - ) - + constants.PART_BODY_SLACK_BYTES - ), - methods={"PUT"}, - ) + register_upload_body_limits(registry) def register_csp_sources(self, registry: CspSourceRegistry) -> None: """Let the browser PUT multipart parts straight to the S3 endpoint.""" From 3b285259166f0687b0db48c92ae83171d5d3904d Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Thu, 22 Oct 2026 13:00:13 +0200 Subject: [PATCH 06/15] refactor(background_tasks): split settings by responsibility (#522) Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 --- .../background_tasks/settings.py | 162 +------------- .../background_tasks/settings_celery.py | 201 ++++++++++++++++++ 2 files changed, 205 insertions(+), 158 deletions(-) create mode 100644 modules/background_tasks/background_tasks/settings_celery.py diff --git a/modules/background_tasks/background_tasks/settings.py b/modules/background_tasks/background_tasks/settings.py index d1e73da39..44934f0e3 100644 --- a/modules/background_tasks/background_tasks/settings.py +++ b/modules/background_tasks/background_tasks/settings.py @@ -34,187 +34,33 @@ from __future__ import annotations import logging -import os -from typing import Any, Literal +from typing import Literal from pydantic import Field, ValidationInfo, field_validator, model_validator from pydantic_core import PydanticCustomError -from pydantic_settings import SettingsConfigDict from simple_module_core.settings_hydrate import RELATED_FIELDS_CTX -from background_tasks.config_guards import parse_task_routes from background_tasks.constants import ( - DEFAULT_BROKER_URL, - DEFAULT_INVALIDATION_CHANNEL, - DEFAULT_MAX_RETRIES, DEFAULT_OUTBOX_LEASE_SECONDS, DEFAULT_OUTBOX_MAX_ATTEMPTS, DEFAULT_OUTBOX_SWEEP_INTERVAL_SECONDS, - DEFAULT_PURGE_INTERVAL_SECONDS, - DEFAULT_QUEUE, - DEFAULT_RESULT_BACKEND, - DEFAULT_RETENTION_DAYS, - DEFAULT_STUCK_AFTER_SECONDS, - DEFAULT_STUCK_SWEEP_INTERVAL_SECONDS, ENV_PREFIX, - ENV_REDIS_URL, max_outbox_handler_timeout, ) -from background_tasks.db_settings import WorkerDatabaseSettings +from background_tasks.settings_celery import _CELERY_RESTART, CelerySettingsFields from background_tasks.settings_env_checks import ( forbid_localhost_broker_in_production, warn_on_legacy_redis_vars, ) -from background_tasks.settings_logs import LogSinkFields -_CELERY_RESTART = {"requires_restart": True, "group": "Celery"} +__all__ = ["ENV_PREFIX", "BackgroundTasksSettings"] logger = logging.getLogger(__name__) -def _redis_from_env(default: str) -> str: - """``SM_REDIS_URL`` → the module default. - - ``env_prefix`` means pydantic's env source has already answered - ``SM_BG_TASKS_BROKER_URL`` / ``SM_BG_TASKS_RESULT_BACKEND`` by the time a - default is needed, so this factory is only the ``SM_REDIS_URL`` half — the - legacy names keep working, and keep winning, through that source instead.""" - return os.environ.get(ENV_REDIS_URL) or default - - -class BackgroundTasksSettings(LogSinkFields, WorkerDatabaseSettings): +class BackgroundTasksSettings(CelerySettingsFields): """Configuration for the Celery + Redis task runner.""" - # Without ``env_prefix`` each field resolved from its bare name, so a - # container setting ``broker_url`` or ``retention_days`` for another purpose - # reconfigured Celery and ``SM_BG_TASKS_*`` did nothing (GH #283). - model_config = SettingsConfigDict(extra="ignore", env_prefix=ENV_PREFIX) - - @classmethod - def env_seeded_fields(cls) -> frozenset[str]: - """Fields the environment sets through a default factory, not a source.""" - if os.environ.get(ENV_REDIS_URL): - return frozenset({"broker_url", "result_backend"}) - return frozenset() - - broker_url: str = Field( - default_factory=lambda: _redis_from_env(DEFAULT_BROKER_URL), - json_schema_extra=_CELERY_RESTART, - ) - result_backend: str = Field( - default_factory=lambda: _redis_from_env(DEFAULT_RESULT_BACKEND), - json_schema_extra=_CELERY_RESTART, - ) - task_default_queue: str = Field(default=DEFAULT_QUEUE, json_schema_extra=_CELERY_RESTART) - - # ── Routing (GH #447) ─────────────────────────────────── - # Applied over every module's ``task_config.TASK_ROUTES``. Lets an operator - # re-route a task (``{"reports.*": "heavy"}``) without a code change. - task_routes_override: dict[str, Any] = Field( - default_factory=dict, - description="Task route overrides, {pattern: queue or route dict}.", - json_schema_extra=_CELERY_RESTART, - ) - - # ── Worker tuning (GH #448) ───────────────────────────── - # ``None`` keeps Celery's own default. Workers read these once at start. - visibility_timeout_seconds: int | None = Field( - default=None, - ge=1, - description=( - "Redis visibility timeout. With acks_late, tasks running longer than " - "this (Celery default 1 h) are redelivered and run twice." - ), - json_schema_extra=_CELERY_RESTART, - ) - worker_max_tasks_per_child: int | None = Field( - default=None, ge=1, json_schema_extra=_CELERY_RESTART - ) - worker_max_memory_per_child_kb: int | None = Field( - default=None, ge=1, json_schema_extra=_CELERY_RESTART - ) - task_reject_on_worker_lost: bool | None = Field(default=None, json_schema_extra=_CELERY_RESTART) - task_soft_time_limit: int | None = Field(default=None, ge=1, json_schema_extra=_CELERY_RESTART) - task_time_limit: int | None = Field(default=None, ge=1, json_schema_extra=_CELERY_RESTART) - worker_concurrency: int | None = Field(default=None, ge=1, json_schema_extra=_CELERY_RESTART) - # Raw Celery conf (``SM_BG_TASKS_CELERY_OVERRIDES``, JSON) is deliberately NOT - # a field here: every field is stored in and hydrated from the database, and - # a DB-writable conf dict could switch serializers to pickle. It is read - # from the environment only, in ``config_guards.celery_overrides_from_env``. - - # The per-worker module subset (``SM_BG_TASKS_WORKER_QUEUES`` / - # ``_INCLUDE_MODULES`` / ``_EXCLUDE_MODULES``, GH #450) is likewise not a - # field: it describes one process, so it is read from the environment only - # in ``config_guards.worker_scope_from_env``. - - @field_validator("broker_url", "result_backend") - @classmethod - def _plain_url_scheme(cls, value: str, info: ValidationInfo) -> str: - # Celery/kombu resolve a dotted or ``mod:Class`` scheme (or a bare - # dotted backend name) by importing it, so a DB-stored URL must name a - # transport/backend by plain alias only. - scheme = value.split("://", 1)[0] if "://" in value else value - if any(ch in scheme for ch in ".:/"): - raise ValueError( - f"{info.field_name} must use a plain URL scheme (redis://, amqp://, db+sqlite://); " - "dotted or module:Class names are refused because Celery imports them" - ) - return value - - @field_validator("task_routes_override", mode="before") - @classmethod - def _vet_task_routes(cls, value: object) -> object: - # Admin-editable: the settings UI submits a JSON string. Parse it and - # refuse (422) anything that is not a safe route mapping, rather than - # saving a row that is silently dropped later. ``build_celery`` still - # sanitises, as a defence for values stored before this check. - return parse_task_routes(value) - - # ── Cross-process cache invalidation (GH #318) ────────── - # This module owns the Redis connection, so it installs the transport that - # carries the framework InvalidationBus between workers. Off means every - # per-process cache — ``users``' revocation counter above all — stays stale - # in the *other* workers until its own TTL expires, which is the behaviour - # that predates the transport. Left on by default because a deployment with - # Redis configured is the one that has several workers to keep in step; the - # test suite turns it off, having no Redis to reach. - broadcast_invalidations: bool = Field( - default=True, json_schema_extra={"requires_restart": True, "group": "Celery"} - ) - invalidation_channel: str = Field( - default=DEFAULT_INVALIDATION_CHANNEL, - min_length=1, - json_schema_extra={"requires_restart": True, "group": "Celery"}, - ) - - # Run tasks synchronously inside the calling process. Tests flip it on via - # ``SM_BG_TASKS_TASK_ALWAYS_EAGER`` or by passing it explicitly, either of - # which works without DB-backed hydration (which never fires for suites - # that don't use the FastAPI lifespan). - task_always_eager: bool = False - task_eager_propagates: bool = True - - # A task that has been ``running`` longer than this without a heartbeat is - # flipped to ``stuck`` by the beat sweep. 5 min is long enough to cover - # normal slow jobs while still surfacing wedged workers within one UI - # refresh. - stuck_after_seconds: int = DEFAULT_STUCK_AFTER_SECONDS - stuck_sweep_interval_seconds: int = DEFAULT_STUCK_SWEEP_INTERVAL_SECONDS - purge_interval_seconds: int = DEFAULT_PURGE_INTERVAL_SECONDS - - retention_days: int = Field( - default=DEFAULT_RETENTION_DAYS, - ge=1, - le=3650, - description="Days to keep terminal task execution records (1-3650).", - ) - max_retries: int = Field( - default=DEFAULT_MAX_RETRIES, - ge=0, - le=100, - description="Configured retry ceiling; individual tasks define their own policies (0-100).", - ) - # ── Durable event outbox (GH #459) ────────────────────── outbox_mode: Literal["celery", "inprocess"] = Field( default="celery", json_schema_extra=_CELERY_RESTART diff --git a/modules/background_tasks/background_tasks/settings_celery.py b/modules/background_tasks/background_tasks/settings_celery.py new file mode 100644 index 000000000..980011606 --- /dev/null +++ b/modules/background_tasks/background_tasks/settings_celery.py @@ -0,0 +1,201 @@ +"""Celery, worker-tuning, invalidation and retention fields of the module settings. + +Split out of ``settings`` by responsibility; ``BackgroundTasksSettings`` inherits +them and adds the durable outbox. Field order is unchanged. +""" + +from __future__ import annotations + +import logging +import os +from typing import Any + +from pydantic import Field, ValidationInfo, field_validator +from pydantic_core import PydanticCustomError +from pydantic_settings import SettingsConfigDict +from simple_module_core.settings_hydrate import RELATED_FIELDS_CTX + +from background_tasks.config_guards import parse_task_routes +from background_tasks.constants import ( + DEFAULT_BROKER_URL, + DEFAULT_INVALIDATION_CHANNEL, + DEFAULT_MAX_RETRIES, + DEFAULT_PURGE_INTERVAL_SECONDS, + DEFAULT_QUEUE, + DEFAULT_RESULT_BACKEND, + DEFAULT_RETENTION_DAYS, + DEFAULT_STUCK_AFTER_SECONDS, + DEFAULT_STUCK_SWEEP_INTERVAL_SECONDS, + ENV_PREFIX, + ENV_REDIS_URL, +) +from background_tasks.db_settings import WorkerDatabaseSettings +from background_tasks.settings_logs import LogSinkFields + +_CELERY_RESTART = {"requires_restart": True, "group": "Celery"} + +logger = logging.getLogger(__name__) + + +def _redis_from_env(default: str) -> str: + """``SM_REDIS_URL`` → the module default. + + ``env_prefix`` means pydantic's env source has already answered + ``SM_BG_TASKS_BROKER_URL`` / ``SM_BG_TASKS_RESULT_BACKEND`` by the time a + default is needed, so this factory is only the ``SM_REDIS_URL`` half — the + legacy names keep working, and keep winning, through that source instead.""" + return os.environ.get(ENV_REDIS_URL) or default + + +class CelerySettingsFields(LogSinkFields, WorkerDatabaseSettings): + """Broker, worker tuning, invalidation and retention fields (see ``settings``).""" + + # Without ``env_prefix`` each field resolved from its bare name, so a + # container setting ``broker_url`` or ``retention_days`` for another purpose + # reconfigured Celery and ``SM_BG_TASKS_*`` did nothing (GH #283). + model_config = SettingsConfigDict(extra="ignore", env_prefix=ENV_PREFIX) + + @classmethod + def env_seeded_fields(cls) -> frozenset[str]: + """Fields the environment sets through a default factory, not a source.""" + if os.environ.get(ENV_REDIS_URL): + return frozenset({"broker_url", "result_backend"}) + return frozenset() + + broker_url: str = Field( + default_factory=lambda: _redis_from_env(DEFAULT_BROKER_URL), + json_schema_extra=_CELERY_RESTART, + ) + result_backend: str = Field( + default_factory=lambda: _redis_from_env(DEFAULT_RESULT_BACKEND), + json_schema_extra=_CELERY_RESTART, + ) + task_default_queue: str = Field(default=DEFAULT_QUEUE, json_schema_extra=_CELERY_RESTART) + + # ── Routing (GH #447) ─────────────────────────────────── + # Applied over every module's ``task_config.TASK_ROUTES``. Lets an operator + # re-route a task (``{"reports.*": "heavy"}``) without a code change. + task_routes_override: dict[str, Any] = Field( + default_factory=dict, + description="Task route overrides, {pattern: queue or route dict}.", + json_schema_extra=_CELERY_RESTART, + ) + + # ── Worker tuning (GH #448) ───────────────────────────── + # ``None`` keeps Celery's own default. Workers read these once at start. + visibility_timeout_seconds: int | None = Field( + default=None, + ge=1, + description=( + "Redis visibility timeout. With acks_late, tasks running longer than " + "this (Celery default 1 h) are redelivered and run twice." + ), + json_schema_extra=_CELERY_RESTART, + ) + worker_max_tasks_per_child: int | None = Field( + default=None, ge=1, json_schema_extra=_CELERY_RESTART + ) + worker_max_memory_per_child_kb: int | None = Field( + default=None, ge=1, json_schema_extra=_CELERY_RESTART + ) + task_reject_on_worker_lost: bool | None = Field(default=None, json_schema_extra=_CELERY_RESTART) + task_soft_time_limit: int | None = Field(default=None, ge=1, json_schema_extra=_CELERY_RESTART) + task_time_limit: int | None = Field(default=None, ge=1, json_schema_extra=_CELERY_RESTART) + worker_concurrency: int | None = Field(default=None, ge=1, json_schema_extra=_CELERY_RESTART) + # Raw Celery conf (``SM_BG_TASKS_CELERY_OVERRIDES``, JSON) is deliberately NOT + # a field here: every field is stored in and hydrated from the database, and + # a DB-writable conf dict could switch serializers to pickle. It is read + # from the environment only, in ``config_guards.celery_overrides_from_env``. + + # The per-worker module subset (``SM_BG_TASKS_WORKER_QUEUES`` / + # ``_INCLUDE_MODULES`` / ``_EXCLUDE_MODULES``, GH #450) is likewise not a + # field: it describes one process, so it is read from the environment only + # in ``config_guards.worker_scope_from_env``. + + @field_validator("broker_url", "result_backend") + @classmethod + def _plain_url_scheme(cls, value: str, info: ValidationInfo) -> str: + # Celery/kombu resolve a dotted or ``mod:Class`` scheme (or a bare + # dotted backend name) by importing it, so a DB-stored URL must name a + # transport/backend by plain alias only. + scheme = value.split("://", 1)[0] if "://" in value else value + if any(ch in scheme for ch in ".:/"): + raise ValueError( + f"{info.field_name} must use a plain URL scheme (redis://, amqp://, db+sqlite://); " + "dotted or module:Class names are refused because Celery imports them" + ) + return value + + @field_validator("task_routes_override", mode="before") + @classmethod + def _vet_task_routes(cls, value: object) -> object: + # Admin-editable: the settings UI submits a JSON string. Parse it and + # refuse (422) anything that is not a safe route mapping, rather than + # saving a row that is silently dropped later. ``build_celery`` still + # sanitises, as a defence for values stored before this check. + return parse_task_routes(value) + + # ── Cross-process cache invalidation (GH #318) ────────── + # This module owns the Redis connection, so it installs the transport that + # carries the framework InvalidationBus between workers. Off means every + # per-process cache — ``users``' revocation counter above all — stays stale + # in the *other* workers until its own TTL expires, which is the behaviour + # that predates the transport. Left on by default because a deployment with + # Redis configured is the one that has several workers to keep in step; the + # test suite turns it off, having no Redis to reach. + broadcast_invalidations: bool = Field( + default=True, json_schema_extra={"requires_restart": True, "group": "Celery"} + ) + invalidation_channel: str = Field( + default=DEFAULT_INVALIDATION_CHANNEL, + min_length=1, + json_schema_extra={"requires_restart": True, "group": "Celery"}, + ) + + # Run tasks synchronously inside the calling process. Tests flip it on via + # ``SM_BG_TASKS_TASK_ALWAYS_EAGER`` or by passing it explicitly, either of + # which works without DB-backed hydration (which never fires for suites + # that don't use the FastAPI lifespan). + task_always_eager: bool = False + task_eager_propagates: bool = True + + # A task that has been ``running`` longer than this without a heartbeat is + # flipped to ``stuck`` by the beat sweep. 5 min is long enough to cover + # normal slow jobs while still surfacing wedged workers within one UI + # refresh. + stuck_after_seconds: int = DEFAULT_STUCK_AFTER_SECONDS + stuck_sweep_interval_seconds: int = DEFAULT_STUCK_SWEEP_INTERVAL_SECONDS + purge_interval_seconds: int = DEFAULT_PURGE_INTERVAL_SECONDS + + retention_days: int = Field( + default=DEFAULT_RETENTION_DAYS, + ge=1, + le=3650, + description="Days to keep terminal task execution records (1-3650).", + ) + max_retries: int = Field( + default=DEFAULT_MAX_RETRIES, + ge=0, + le=100, + description="Configured retry ceiling; individual tasks define their own policies (0-100).", + ) + + @field_validator("task_time_limit") + @classmethod + def _soft_limit_below_hard(cls, hard: int | None, info: ValidationInfo) -> int | None: + """A soft limit at or above the hard one can never fire (#548). + + Attached to the later-declared hard limit so the error carries that + field's ``loc`` and the settings form shows it under the field. It names + the soft limit as related so boot hydration can drop a stored soft limit + when the hard one is pinned by env. + """ + soft = info.data.get("task_soft_time_limit") + if hard is not None and soft is not None and soft >= hard: + raise PydanticCustomError( + "time_limit_order", + "task_time_limit ({hard}) must be greater than task_soft_time_limit ({soft}); " + "otherwise the soft limit can never fire", + {"hard": hard, "soft": soft, RELATED_FIELDS_CTX: ["task_soft_time_limit"]}, + ) + return hard From 023ececb3525763d64f94262147c5b50dfad8254 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Thu, 22 Oct 2026 13:01:56 +0200 Subject: [PATCH 07/15] refactor(core): split permissions.py by responsibility (#522) Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 --- .../simple_module_core/_permission_grants.py | 74 +++++++++++++++++ .../core/simple_module_core/permissions.py | 82 +++---------------- 2 files changed, 86 insertions(+), 70 deletions(-) create mode 100644 framework/core/simple_module_core/_permission_grants.py diff --git a/framework/core/simple_module_core/_permission_grants.py b/framework/core/simple_module_core/_permission_grants.py new file mode 100644 index 000000000..1fc137629 --- /dev/null +++ b/framework/core/simple_module_core/_permission_grants.py @@ -0,0 +1,74 @@ +"""Permission primitives: the wildcard grant rule, the admin role and grant-source types.""" + +from __future__ import annotations + +from collections.abc import Awaitable, Callable, Collection, Iterable +from typing import Any + +PermissionSourceProvider = Callable[[], "Iterable[str] | Iterable[tuple[str, str]]"] +"""Sync callable returning permission keys, or ``(key, label)`` pairs.""" + +WILDCARD = "*" + +ADMIN_ROLE = "admin" +"""The one role name the framework itself knows. + +Everything else about roles is module-owned, but the framework needs this to +resolve the wildcard grant below and to decide who can still reach the app +while maintenance mode is on. +""" + +# Default role→permission mapping. Admin gets all permissions via the wildcard. +# Additional mappings are added at registration time via PermissionRegistry.map_role. +DEFAULT_ROLE_PERMISSIONS: dict[str, list[str]] = { + ADMIN_ROLE: [WILDCARD], +} + + +def is_admin(roles: list[str] | None) -> bool: + """Whether ``roles`` carries the framework's admin role.""" + return bool(roles) and ADMIN_ROLE in roles + + +def is_admin_principal(user: Any) -> bool: + """Whether *user* may use the admin role's shortcuts — admin and not a token. + + A principal carrying ``scopes`` (an ``api_tokens`` credential) is capped + at those scopes even when its owner is an admin, so every check that + admits admins on the role alone must use this rather than + :func:`is_admin`; otherwise an admin's ``feature_flags.view`` token would + still open the Doctor page. Duck-typed: ``UserContext`` lives in the auth + module, which framework code must not import. + """ + if user is None or getattr(user, "scopes", None) is not None: + return False + return is_admin(getattr(user, "roles", None)) + + +def grants(held: Collection[str], required: str) -> bool: + """Whether a principal holding *held* satisfies *required*. + + One place for the wildcard rule, so code that gates something *inside* a + response — a column, a label, a card — reads the grant the same way + ``RequiresPermission`` reads it at the door. Two hand-rolled ``in`` checks + are two chances to forget that ``admin`` holds ``*`` and nothing else. + + An empty *required* is satisfied by anything: callers use it to mean "no + permission gates this", which is the default for a declaration that never + named one. + """ + if not required: + return True + return WILDCARD in held or required in held + + +GrantSource = Callable[[Any, Any], Awaitable[Collection[str]]] +"""``async (request, user) -> keys`` — permissions a principal holds beyond its roles. + +How a module that stores grants of its own (``permissions``' per-user grants) +gets them into the one set every check reads, without the framework importing +it (SM009). Called once per authenticated request by +``simple_module_hosting.permissions.resolve_principal_permissions``, so a +source that reads the database must cache: it sits on the hot path of every +page load. +""" diff --git a/framework/core/simple_module_core/permissions.py b/framework/core/simple_module_core/permissions.py index 4e079618b..e48c91c9d 100644 --- a/framework/core/simple_module_core/permissions.py +++ b/framework/core/simple_module_core/permissions.py @@ -4,79 +4,21 @@ import logging import time -from collections.abc import Awaitable, Callable, Collection, Iterable +from collections.abc import Callable, Collection from dataclasses import dataclass, field -from typing import Any -logger = logging.getLogger(__name__) - -PermissionSourceProvider = Callable[[], "Iterable[str] | Iterable[tuple[str, str]]"] -"""Sync callable returning permission keys, or ``(key, label)`` pairs.""" - -WILDCARD = "*" - -ADMIN_ROLE = "admin" -"""The one role name the framework itself knows. - -Everything else about roles is module-owned, but the framework needs this to -resolve the wildcard grant below and to decide who can still reach the app -while maintenance mode is on. -""" - -# Default role→permission mapping. Admin gets all permissions via the wildcard. -# Additional mappings are added at registration time via PermissionRegistry.map_role. -DEFAULT_ROLE_PERMISSIONS: dict[str, list[str]] = { - ADMIN_ROLE: [WILDCARD], -} - - -def is_admin(roles: list[str] | None) -> bool: - """Whether ``roles`` carries the framework's admin role.""" - return bool(roles) and ADMIN_ROLE in roles - - -def is_admin_principal(user: Any) -> bool: - """Whether *user* may use the admin role's shortcuts — admin and not a token. +from simple_module_core._permission_grants import ( # noqa: F401 (re-exported) + ADMIN_ROLE, + DEFAULT_ROLE_PERMISSIONS, + WILDCARD, + GrantSource, + PermissionSourceProvider, + grants, + is_admin, + is_admin_principal, +) - A principal carrying ``scopes`` (an ``api_tokens`` credential) is capped - at those scopes even when its owner is an admin, so every check that - admits admins on the role alone must use this rather than - :func:`is_admin`; otherwise an admin's ``feature_flags.view`` token would - still open the Doctor page. Duck-typed: ``UserContext`` lives in the auth - module, which framework code must not import. - """ - if user is None or getattr(user, "scopes", None) is not None: - return False - return is_admin(getattr(user, "roles", None)) - - -def grants(held: Collection[str], required: str) -> bool: - """Whether a principal holding *held* satisfies *required*. - - One place for the wildcard rule, so code that gates something *inside* a - response — a column, a label, a card — reads the grant the same way - ``RequiresPermission`` reads it at the door. Two hand-rolled ``in`` checks - are two chances to forget that ``admin`` holds ``*`` and nothing else. - - An empty *required* is satisfied by anything: callers use it to mean "no - permission gates this", which is the default for a declaration that never - named one. - """ - if not required: - return True - return WILDCARD in held or required in held - - -GrantSource = Callable[[Any, Any], Awaitable[Collection[str]]] -"""``async (request, user) -> keys`` — permissions a principal holds beyond its roles. - -How a module that stores grants of its own (``permissions``' per-user grants) -gets them into the one set every check reads, without the framework importing -it (SM009). Called once per authenticated request by -``simple_module_hosting.permissions.resolve_principal_permissions``, so a -source that reads the database must cache: it sits on the hot path of every -page load. -""" +logger = logging.getLogger(__name__) @dataclass From a029133c9fb8b8913e09055c3215165a67f9bcef Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Thu, 22 Oct 2026 13:01:56 +0200 Subject: [PATCH 08/15] refactor(core): split i18n.py by responsibility (#522) Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 --- .../simple_module_core/_i18n_translator.py | 107 ++++++++++++++++++ framework/core/simple_module_core/i18n.py | 96 +--------------- 2 files changed, 108 insertions(+), 95 deletions(-) create mode 100644 framework/core/simple_module_core/_i18n_translator.py diff --git a/framework/core/simple_module_core/_i18n_translator.py b/framework/core/simple_module_core/_i18n_translator.py new file mode 100644 index 000000000..95d434f1e --- /dev/null +++ b/framework/core/simple_module_core/_i18n_translator.py @@ -0,0 +1,107 @@ +"""Request-scoped translator and CLDR plural resolution.""" + +from __future__ import annotations + +import logging +from functools import lru_cache +from typing import TYPE_CHECKING, Any + +from babel import Locale + +if TYPE_CHECKING: + from simple_module_core.i18n import I18nRegistry + +# Logged under the historical ``simple_module_core.i18n`` name so existing log +# filters keep matching after the split. +logger = logging.getLogger("simple_module_core.i18n") + + +@lru_cache(maxsize=64) +def _plural_rule(locale: str): + """Cached CLDR plural rule for a locale tag (e.g. 'en', 'ru', 'pt_BR').""" + return Locale.parse(locale).plural_form + + +def _plural_form(locale: str, count: float) -> str: + """Return CLDR plural category ('one', 'few', 'many', 'other', ...). + + Falls back to 'other' if the locale cannot be parsed by Babel. + """ + try: + rule = _plural_rule(locale) + except Exception: + return "other" + return rule(count) + + +class _SafeFormatDict(dict): + """Dict that returns ``{key}`` for missing keys so str.format_map doesn't raise.""" + + def __missing__(self, key: str) -> str: + return "{" + key + "}" + + +class Translator: + """Request-scoped translator bound to a specific locale. + + Construct via:: + + Translator(registry, locale=request.state.locale, default_locale="en") + + Resolution order for :meth:`t`: + + 1. Look up key in ``locale``; if missing, fall back to ``default_locale``. + 2. If still missing, return the key itself (with a debug log). + 3. Interpolate ``{name}``-style placeholders using supplied kwargs. + Missing placeholders are left as ``{name}`` (not raised). + """ + + def __init__( + self, + registry: I18nRegistry, + locale: str, + default_locale: str, + ) -> None: + self._registry = registry + self.locale = locale + self.default_locale = default_locale + + def t(self, key: str, **params: Any) -> str: + """Translate ``key`` with optional interpolation and plural resolution. + + When ``count`` is in params, look up ``_`` using + Babel's CLDR plural rule for the active locale, falling back to + ``_other`` and finally ````. + """ + resolved_key = self._resolve_plural_key(key, params) + template = self._lookup(resolved_key) + if template is None and resolved_key != key: + template = self._lookup(key) + if template is None: + logger.debug("i18n: missing key '%s' in locale '%s'", key, self.locale) + return key + return template.format_map(_SafeFormatDict(params)) + + def _resolve_plural_key(self, key: str, params: dict[str, Any]) -> str: + count = params.get("count") + if count is None: + return key + form = _plural_form(self.locale, count) + # Prefer the exact form; fall back to _other if that form has no entry. + candidate = f"{key}_{form}" + if self._lookup(candidate) is not None: + return candidate + other = f"{key}_other" + if self._lookup(other) is not None: + return other + return key + + def _lookup(self, key: str) -> str | None: + msgs = self._registry.messages(self.locale) + if key in msgs: + return msgs[key] + if self.locale != self.default_locale: + default = self._registry.messages(self.default_locale) + if key in default: + return default[key] + return None diff --git a/framework/core/simple_module_core/i18n.py b/framework/core/simple_module_core/i18n.py index 2495925d2..be406cadc 100644 --- a/framework/core/simple_module_core/i18n.py +++ b/framework/core/simple_module_core/i18n.py @@ -4,14 +4,11 @@ import logging from collections.abc import Mapping -from functools import lru_cache from pathlib import Path from types import MappingProxyType -from typing import Any - -from babel import Locale from simple_module_core._i18n_flatten import flatten_messages, read_catalog +from simple_module_core._i18n_translator import Translator from simple_module_core.i18n_overrides import apply_overrides, load_overrides __all__ = ["PLURAL_CATEGORIES", "I18nRegistry", "Translator", "flatten_messages"] @@ -24,24 +21,6 @@ PLURAL_CATEGORIES: tuple[str, ...] = ("zero", "one", "two", "few", "many", "other") -@lru_cache(maxsize=64) -def _plural_rule(locale: str): - """Cached CLDR plural rule for a locale tag (e.g. 'en', 'ru', 'pt_BR').""" - return Locale.parse(locale).plural_form - - -def _plural_form(locale: str, count: float) -> str: - """Return CLDR plural category ('one', 'few', 'many', 'other', ...). - - Falls back to 'other' if the locale cannot be parsed by Babel. - """ - try: - rule = _plural_rule(locale) - except Exception: - return "other" - return rule(count) - - class I18nRegistry: """Merged view of all module locale JSON files, keyed by locale. @@ -217,76 +196,3 @@ def messages_snapshot(self, locale: str, *, include_admin: bool = True) -> dict[ if raw is None: return self._empty_snapshot return dict(raw) - - -class _SafeFormatDict(dict): - """Dict that returns ``{key}`` for missing keys so str.format_map doesn't raise.""" - - def __missing__(self, key: str) -> str: - return "{" + key + "}" - - -class Translator: - """Request-scoped translator bound to a specific locale. - - Construct via:: - - Translator(registry, locale=request.state.locale, default_locale="en") - - Resolution order for :meth:`t`: - - 1. Look up key in ``locale``; if missing, fall back to ``default_locale``. - 2. If still missing, return the key itself (with a debug log). - 3. Interpolate ``{name}``-style placeholders using supplied kwargs. - Missing placeholders are left as ``{name}`` (not raised). - """ - - def __init__( - self, - registry: I18nRegistry, - locale: str, - default_locale: str, - ) -> None: - self._registry = registry - self.locale = locale - self.default_locale = default_locale - - def t(self, key: str, **params: Any) -> str: - """Translate ``key`` with optional interpolation and plural resolution. - - When ``count`` is in params, look up ``_`` using - Babel's CLDR plural rule for the active locale, falling back to - ``_other`` and finally ````. - """ - resolved_key = self._resolve_plural_key(key, params) - template = self._lookup(resolved_key) - if template is None and resolved_key != key: - template = self._lookup(key) - if template is None: - logger.debug("i18n: missing key '%s' in locale '%s'", key, self.locale) - return key - return template.format_map(_SafeFormatDict(params)) - - def _resolve_plural_key(self, key: str, params: dict[str, Any]) -> str: - count = params.get("count") - if count is None: - return key - form = _plural_form(self.locale, count) - # Prefer the exact form; fall back to _other if that form has no entry. - candidate = f"{key}_{form}" - if self._lookup(candidate) is not None: - return candidate - other = f"{key}_other" - if self._lookup(other) is not None: - return other - return key - - def _lookup(self, key: str) -> str | None: - msgs = self._registry.messages(self.locale) - if key in msgs: - return msgs[key] - if self.locale != self.default_locale: - default = self._registry.messages(self.default_locale) - if key in default: - return default[key] - return None From e4680fbc1cedd8171f03447ec425cdb19ddbfa9b Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Thu, 22 Oct 2026 13:02:25 +0200 Subject: [PATCH 09/15] refactor(cli): split app_project.py by responsibility (#522) Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 --- .../_app_project_manifests.py | 94 ++++++++++++++++++ .../cli/simple_module_cli/app_project.py | 97 ++----------------- 2 files changed, 102 insertions(+), 89 deletions(-) create mode 100644 framework/cli/simple_module_cli/_app_project_manifests.py diff --git a/framework/cli/simple_module_cli/_app_project_manifests.py b/framework/cli/simple_module_cli/_app_project_manifests.py new file mode 100644 index 000000000..7eb2e2a98 --- /dev/null +++ b/framework/cli/simple_module_cli/_app_project_manifests.py @@ -0,0 +1,94 @@ +"""Dependency pins, ``.env`` DB URLs and ``pyproject.toml`` rewriting for ``smpy new``.""" + +from __future__ import annotations + +from collections.abc import Sequence + +from simple_module_cli.scaffolding import resolve_framework_version + +_FRAMEWORK_VERSION = resolve_framework_version() + +# Pin ``simple_module_cli`` as a dev dep so ``uv run smpy`` resolves to the +# project venv. The global ``uv tool`` install runs in its own isolated venv +# that can't see the project's plugin entry points (issue #134). The lint / +# test tooling (ruff, ty, pytest-*) backs the generated `make lint`/`make +# test` targets so a fresh app can run its own quality gates. +_APP_PY_DEV_DEPS = [ + f"simple_module_test=={_FRAMEWORK_VERSION}", + f"simple_module_cli=={_FRAMEWORK_VERSION}", + "pytest>=8.0", + "pytest-asyncio>=0.24", + "pytest-playwright>=0.7.2", + # Floor: unpinned, uv backtracks playwright to 1.18 (GH #445). + "playwright>=1.45", + "ruff>=0.8", + "ty>=0.0.29", +] + +_APP_NPM_DEPS = { + "@simple-module-py/ui": _FRAMEWORK_VERSION, + "@simple-module-py/i18n": _FRAMEWORK_VERSION, + "react": "^19.0.0", + "react-dom": "^19.0.0", + "@inertiajs/react": "^3.7.0", +} +# Flat mode has no npm workspaces, so `client_app/package.json` is never +# installed — the root package.json seeded from these two dicts is the only +# manifest npm reads. It must therefore cover everything client_app needs to +# build, not just the framework pins: `vite.config.ts` imports +# `@tailwindcss/vite`, and `tsc` needs the @types packages. Keep this in step +# with templates/host/client_app/package.json.tpl. +_APP_NPM_DEV_DEPS = { + "@simple-module-py/tsconfig": _FRAMEWORK_VERSION, + "@tailwindcss/vite": "^4.0.0", + "@types/node": "^26.5.0", + "@types/react": "^19.0.0", + "@types/react-dom": "^19.0.0", + "@vitejs/plugin-react": "^6.1.1", + "tailwindcss": "^4.0.0", + "typescript": "^7.0.2", + "vite": "^8.2.2", +} + + +def _db_url(db: str, db_name: str, *, flat: bool, password: str = "") -> str: + if db == "postgres": + return f"postgresql+asyncpg://postgres:{password}@localhost:5432/{db_name}" + # Workspace mode keeps the SQLite file next to host/'s alembic.ini so + # `cd host && uvicorn` and `cd host && alembic` resolve the same path. + return "sqlite+aiosqlite:///./app.db" if flat else "sqlite+aiosqlite:///./host/app.db" + + +def _rewrite_pyproject( + text: str, + deps: list[str], + dev_deps: list[str], + *, + sources: Sequence[str] = (), + project_name: str | None = None, +) -> str: + """Replace deps in a host ``pyproject.toml`` and pin workspace sources. + + ``sources`` lists ``simple_module_*`` packages that should resolve from + the uv workspace (``modules/*``) instead of PyPI. Emits a + ``[tool.uv.sources]`` block per entry. Empty in flat mode. + + ``project_name`` overrides ``[project].name`` — set in workspace mode + so the host's package name differs from the workspace root's. + """ + import tomlkit + + doc = tomlkit.parse(text) + project = doc.setdefault("project", tomlkit.table()) + if project_name is not None: + project["name"] = project_name + project["dependencies"] = list(deps) + groups = doc.setdefault("dependency-groups", tomlkit.table()) + groups["dev"] = list(dev_deps) + if sources: + tool = doc.setdefault("tool", tomlkit.table()) + uv_table = tool.setdefault("uv", tomlkit.table()) + uv_sources = uv_table.setdefault("sources", tomlkit.table()) + for src in sources: + uv_sources[src] = {"workspace": True} + return tomlkit.dumps(doc) diff --git a/framework/cli/simple_module_cli/app_project.py b/framework/cli/simple_module_cli/app_project.py index 301c7fc55..1ac9a1de6 100644 --- a/framework/cli/simple_module_cli/app_project.py +++ b/framework/cli/simple_module_cli/app_project.py @@ -19,6 +19,14 @@ from pathlib import Path from typing import Any +from simple_module_cli._app_project_manifests import ( + _APP_NPM_DEPS, + _APP_NPM_DEV_DEPS, + _APP_PY_DEV_DEPS, + _FRAMEWORK_VERSION, + _db_url, + _rewrite_pyproject, +) from simple_module_cli._env import set_env_key from simple_module_cli._local_env import ( add_postgres_password_placeholder, @@ -35,7 +43,6 @@ create_host, create_module, create_workspace, - resolve_framework_version, ) __all__ = ["create_app_project"] @@ -43,51 +50,6 @@ _SAMPLE_MODULE_NAME = "hello" _SAMPLE_MODULE_PKG = _module_to_pypi_name(_SAMPLE_MODULE_NAME) - -_FRAMEWORK_VERSION = resolve_framework_version() - -# Pin ``simple_module_cli`` as a dev dep so ``uv run smpy`` resolves to the -# project venv. The global ``uv tool`` install runs in its own isolated venv -# that can't see the project's plugin entry points (issue #134). The lint / -# test tooling (ruff, ty, pytest-*) backs the generated `make lint`/`make -# test` targets so a fresh app can run its own quality gates. -_APP_PY_DEV_DEPS = [ - f"simple_module_test=={_FRAMEWORK_VERSION}", - f"simple_module_cli=={_FRAMEWORK_VERSION}", - "pytest>=8.0", - "pytest-asyncio>=0.24", - "pytest-playwright>=0.7.2", - # Floor: unpinned, uv backtracks playwright to 1.18 (GH #445). - "playwright>=1.45", - "ruff>=0.8", - "ty>=0.0.29", -] - -_APP_NPM_DEPS = { - "@simple-module-py/ui": _FRAMEWORK_VERSION, - "@simple-module-py/i18n": _FRAMEWORK_VERSION, - "react": "^19.0.0", - "react-dom": "^19.0.0", - "@inertiajs/react": "^3.7.0", -} -# Flat mode has no npm workspaces, so `client_app/package.json` is never -# installed — the root package.json seeded from these two dicts is the only -# manifest npm reads. It must therefore cover everything client_app needs to -# build, not just the framework pins: `vite.config.ts` imports -# `@tailwindcss/vite`, and `tsc` needs the @types packages. Keep this in step -# with templates/host/client_app/package.json.tpl. -_APP_NPM_DEV_DEPS = { - "@simple-module-py/tsconfig": _FRAMEWORK_VERSION, - "@tailwindcss/vite": "^4.0.0", - "@types/node": "^26.5.0", - "@types/react": "^19.0.0", - "@types/react-dom": "^19.0.0", - "@vitejs/plugin-react": "^6.1.1", - "tailwindcss": "^4.0.0", - "typescript": "^7.0.2", - "vite": "^8.2.2", -} - # Files the host template ships that the workspace template re-emits at # the project root. Host copies are stripped in workspace mode. _HOST_FILES_OWNED_BY_WORKSPACE = ( @@ -255,46 +217,3 @@ def _seed_static_dist_placeholder(static_dist: Path) -> None: # an empty placeholder keeps `uv sync` from failing before vite build runs. static_dist.mkdir(parents=True, exist_ok=True) (static_dist / ".gitkeep").touch() - - -def _db_url(db: str, db_name: str, *, flat: bool, password: str = "") -> str: - if db == "postgres": - return f"postgresql+asyncpg://postgres:{password}@localhost:5432/{db_name}" - # Workspace mode keeps the SQLite file next to host/'s alembic.ini so - # `cd host && uvicorn` and `cd host && alembic` resolve the same path. - return "sqlite+aiosqlite:///./app.db" if flat else "sqlite+aiosqlite:///./host/app.db" - - -def _rewrite_pyproject( - text: str, - deps: list[str], - dev_deps: list[str], - *, - sources: Sequence[str] = (), - project_name: str | None = None, -) -> str: - """Replace deps in a host ``pyproject.toml`` and pin workspace sources. - - ``sources`` lists ``simple_module_*`` packages that should resolve from - the uv workspace (``modules/*``) instead of PyPI. Emits a - ``[tool.uv.sources]`` block per entry. Empty in flat mode. - - ``project_name`` overrides ``[project].name`` — set in workspace mode - so the host's package name differs from the workspace root's. - """ - import tomlkit - - doc = tomlkit.parse(text) - project = doc.setdefault("project", tomlkit.table()) - if project_name is not None: - project["name"] = project_name - project["dependencies"] = list(deps) - groups = doc.setdefault("dependency-groups", tomlkit.table()) - groups["dev"] = list(dev_deps) - if sources: - tool = doc.setdefault("tool", tomlkit.table()) - uv_table = tool.setdefault("uv", tomlkit.table()) - uv_sources = uv_table.setdefault("sources", tomlkit.table()) - for src in sources: - uv_sources[src] = {"workspace": True} - return tomlkit.dumps(doc) From 92af9acf1679c0e9a15667723dbe5c184423185c Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Thu, 22 Oct 2026 13:02:50 +0200 Subject: [PATCH 10/15] refactor(background_tasks): keep DEFAULT_RETENTION_DAYS importable from settings (#522) Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 --- modules/background_tasks/background_tasks/settings.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/modules/background_tasks/background_tasks/settings.py b/modules/background_tasks/background_tasks/settings.py index 44934f0e3..05f10944a 100644 --- a/modules/background_tasks/background_tasks/settings.py +++ b/modules/background_tasks/background_tasks/settings.py @@ -44,6 +44,7 @@ DEFAULT_OUTBOX_LEASE_SECONDS, DEFAULT_OUTBOX_MAX_ATTEMPTS, DEFAULT_OUTBOX_SWEEP_INTERVAL_SECONDS, + DEFAULT_RETENTION_DAYS, ENV_PREFIX, max_outbox_handler_timeout, ) @@ -53,7 +54,7 @@ warn_on_legacy_redis_vars, ) -__all__ = ["ENV_PREFIX", "BackgroundTasksSettings"] +__all__ = ["DEFAULT_RETENTION_DAYS", "ENV_PREFIX", "BackgroundTasksSettings"] logger = logging.getLogger(__name__) From 4a8feb889d29315e43b15e287b4dd10e81078310 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Thu, 22 Oct 2026 13:02:52 +0200 Subject: [PATCH 11/15] refactor(cli): split add_cmd.py by responsibility (#522) Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 --- .../cli/simple_module_cli/_add_install.py | 59 ++++++++ .../cli/simple_module_cli/_add_sources.py | 83 ++++++++++++ framework/cli/simple_module_cli/add_cmd.py | 127 +----------------- 3 files changed, 149 insertions(+), 120 deletions(-) create mode 100644 framework/cli/simple_module_cli/_add_install.py create mode 100644 framework/cli/simple_module_cli/_add_sources.py diff --git a/framework/cli/simple_module_cli/_add_install.py b/framework/cli/simple_module_cli/_add_install.py new file mode 100644 index 000000000..5579ac3ec --- /dev/null +++ b/framework/cli/simple_module_cli/_add_install.py @@ -0,0 +1,59 @@ +"""Post-install steps and shared helpers for ``smpy add``.""" + +from __future__ import annotations + +import subprocess +from collections.abc import Callable +from pathlib import Path + +import typer + +ExecRunner = Callable[[list[str], Path], int] + + +_ENTRYPOINT_CHECK = ( + "import sys\n" + "from importlib.metadata import distributions\n" + "target = sys.argv[1].replace('-', '_').lower()\n" + "for dist in distributions():\n" + " if any(ep.group == 'simple_module' for ep in dist.entry_points):\n" + " name = (dist.metadata['Name'] or '').replace('-', '_').lower()\n" + " if name == target:\n" + " sys.exit(0)\n" + "sys.exit(1)\n" +) + + +def _default_exec_runner(cmd: list[str], cwd: Path) -> int: + return subprocess.run(cmd, cwd=cwd, check=False).returncode + + +def _fail(message: str) -> typer.Exit: + typer.echo(f"ERROR: {message}", err=True) + return typer.Exit(code=1) + + +def post_install( + project_dir: Path, + dists: list[str], + models_dists: list[str], + exec_runner: ExecRunner, +) -> None: + """uv sync → gen-pages → sync-js-deps → entry-point check → reminders.""" + if exec_runner(["uv", "sync"], project_dir) != 0: + raise _fail("`uv sync` failed — pyproject was written; fix and re-run `uv sync`.") + if (project_dir / "client_app").is_dir(): + exec_runner(["uv", "run", "smpy", "host", "gen-pages"], project_dir) + exec_runner(["uv", "run", "smpy", "host", "sync-js-deps"], project_dir) + for dist in dists: + code = exec_runner(["uv", "run", "python", "-c", _ENTRYPOINT_CHECK, dist], project_dir) + if code != 0: + raise _fail( + f"{dist} installed but exposes no [project.entry-points.simple_module] " + "entry point — it is not a SimpleModule module." + ) + for dist in models_dists: + typer.echo( + f"{dist} ships database models — generate and apply the migration:\n" + f' make migration msg="add {dist}"\n make migrate' + ) diff --git a/framework/cli/simple_module_cli/_add_sources.py b/framework/cli/simple_module_cli/_add_sources.py new file mode 100644 index 000000000..81e6163be --- /dev/null +++ b/framework/cli/simple_module_cli/_add_sources.py @@ -0,0 +1,83 @@ +"""PyPI and local-path sources for ``smpy add``.""" + +from __future__ import annotations + +import os +from pathlib import Path + +import typer + +from simple_module_cli._add_install import ExecRunner, _fail, post_install +from simple_module_cli.git_source import derive_range, scan_modules +from simple_module_cli.pyproject_edit import ( + save_pyproject, + write_dependency, + write_path_source, + write_workspace_source, +) +from simple_module_cli.uv_workspace import is_workspace_member + + +def _add_pypi(doc, pyproject: Path, raw: str, no_sync: bool, exec_runner: ExecRunner) -> list[str]: + name = raw + for i, ch in enumerate(raw): + if ch in "<>=!~[; ": + name = raw[:i] + break + constraint = raw[len(name) :] + write_dependency(doc, name, constraint) + save_pyproject(pyproject, doc) + typer.echo(f"Added {name} (PyPI).") + if not no_sync: + post_install(pyproject.parent, [name], [], exec_runner) + return [name] + + +def _resolve_local_path(project_dir: Path, spec_path: Path) -> tuple[Path, str]: + """Resolve a local-path spec to ``(directory, text written to pyproject)``. + + A relative path is resolved against the ``--path`` project (the pyproject + being edited, which is what ``[tool.uv.sources]`` paths are relative to). + If nothing exists there but the path exists relative to the cwd, the cwd + one is used and the written path is re-expressed relative to the project. + """ + if spec_path.is_absolute(): + return spec_path, str(spec_path) + root = project_dir / spec_path + if not root.exists() and (Path.cwd() / spec_path).exists(): + root = Path.cwd() / spec_path + return root, os.path.relpath(root.resolve(), project_dir.resolve()) + return root, str(spec_path) + + +def _add_path( + doc, pyproject: Path, spec_path: Path, no_sync: bool, exec_runner: ExecRunner +) -> list[str]: + root, written = _resolve_local_path(pyproject.parent, spec_path) + found = [m for m in scan_modules(root) if m.subdirectory is None] + if not found: + raise _fail( + f"{root} has no [project.entry-points.simple_module] entry point at its root " + f"(relative paths resolve against the --path project, {pyproject.parent}, " + "falling back to the current directory)" + ) + mod = found[0] + if is_workspace_member(pyproject.parent, root): + # uv refuses a path source for a workspace member; the scaffold's own + # sample module is wired the same way (unpinned + workspace = true). + write_dependency(doc, mod.dist_name, "") + write_workspace_source(doc, mod.dist_name) + typer.echo(f"Added {mod.dist_name} (uv workspace member {written}).") + else: + write_dependency(doc, mod.dist_name, derive_range(mod.version)) + write_path_source(doc, mod.dist_name, path=written) + typer.echo(f"Added {mod.dist_name} (editable path {written}).") + save_pyproject(pyproject, doc) + if not no_sync: + post_install( + pyproject.parent, + [mod.dist_name], + [mod.dist_name] if mod.ships_models else [], + exec_runner, + ) + return [mod.dist_name] diff --git a/framework/cli/simple_module_cli/add_cmd.py b/framework/cli/simple_module_cli/add_cmd.py index a656a2fdb..12c6de847 100644 --- a/framework/cli/simple_module_cli/add_cmd.py +++ b/framework/cli/simple_module_cli/add_cmd.py @@ -11,15 +11,19 @@ from __future__ import annotations -import os -import subprocess import tempfile -from collections.abc import Callable from pathlib import Path from typing import Annotated import typer +from simple_module_cli._add_install import ( + ExecRunner, + _default_exec_runner, + _fail, + post_install, +) +from simple_module_cli._add_sources import _add_path, _add_pypi from simple_module_cli.git_source import ( FoundModule, SpecError, @@ -35,41 +39,15 @@ save_pyproject, write_dependency, write_git_source, - write_path_source, - write_workspace_source, ) -from simple_module_cli.uv_workspace import is_workspace_member __all__ = ["ExecRunner", "add_module", "post_install", "run_add"] -ExecRunner = Callable[[list[str], Path], int] - _SECURITY_NOTICE = ( "note: this installs and runs code from a URL you chose — review the " "repository before booting the host." ) -_ENTRYPOINT_CHECK = ( - "import sys\n" - "from importlib.metadata import distributions\n" - "target = sys.argv[1].replace('-', '_').lower()\n" - "for dist in distributions():\n" - " if any(ep.group == 'simple_module' for ep in dist.entry_points):\n" - " name = (dist.metadata['Name'] or '').replace('-', '_').lower()\n" - " if name == target:\n" - " sys.exit(0)\n" - "sys.exit(1)\n" -) - - -def _default_exec_runner(cmd: list[str], cwd: Path) -> int: - return subprocess.run(cmd, cwd=cwd, check=False).returncode - - -def _fail(message: str) -> typer.Exit: - typer.echo(f"ERROR: {message}", err=True) - return typer.Exit(code=1) - def _choose( found: list[FoundModule], @@ -99,97 +77,6 @@ def _choose( ] -def post_install( - project_dir: Path, - dists: list[str], - models_dists: list[str], - exec_runner: ExecRunner, -) -> None: - """uv sync → gen-pages → sync-js-deps → entry-point check → reminders.""" - if exec_runner(["uv", "sync"], project_dir) != 0: - raise _fail("`uv sync` failed — pyproject was written; fix and re-run `uv sync`.") - if (project_dir / "client_app").is_dir(): - exec_runner(["uv", "run", "smpy", "host", "gen-pages"], project_dir) - exec_runner(["uv", "run", "smpy", "host", "sync-js-deps"], project_dir) - for dist in dists: - code = exec_runner(["uv", "run", "python", "-c", _ENTRYPOINT_CHECK, dist], project_dir) - if code != 0: - raise _fail( - f"{dist} installed but exposes no [project.entry-points.simple_module] " - "entry point — it is not a SimpleModule module." - ) - for dist in models_dists: - typer.echo( - f"{dist} ships database models — generate and apply the migration:\n" - f' make migration msg="add {dist}"\n make migrate' - ) - - -def _add_pypi(doc, pyproject: Path, raw: str, no_sync: bool, exec_runner: ExecRunner) -> list[str]: - name = raw - for i, ch in enumerate(raw): - if ch in "<>=!~[; ": - name = raw[:i] - break - constraint = raw[len(name) :] - write_dependency(doc, name, constraint) - save_pyproject(pyproject, doc) - typer.echo(f"Added {name} (PyPI).") - if not no_sync: - post_install(pyproject.parent, [name], [], exec_runner) - return [name] - - -def _resolve_local_path(project_dir: Path, spec_path: Path) -> tuple[Path, str]: - """Resolve a local-path spec to ``(directory, text written to pyproject)``. - - A relative path is resolved against the ``--path`` project (the pyproject - being edited, which is what ``[tool.uv.sources]`` paths are relative to). - If nothing exists there but the path exists relative to the cwd, the cwd - one is used and the written path is re-expressed relative to the project. - """ - if spec_path.is_absolute(): - return spec_path, str(spec_path) - root = project_dir / spec_path - if not root.exists() and (Path.cwd() / spec_path).exists(): - root = Path.cwd() / spec_path - return root, os.path.relpath(root.resolve(), project_dir.resolve()) - return root, str(spec_path) - - -def _add_path( - doc, pyproject: Path, spec_path: Path, no_sync: bool, exec_runner: ExecRunner -) -> list[str]: - root, written = _resolve_local_path(pyproject.parent, spec_path) - found = [m for m in scan_modules(root) if m.subdirectory is None] - if not found: - raise _fail( - f"{root} has no [project.entry-points.simple_module] entry point at its root " - f"(relative paths resolve against the --path project, {pyproject.parent}, " - "falling back to the current directory)" - ) - mod = found[0] - if is_workspace_member(pyproject.parent, root): - # uv refuses a path source for a workspace member; the scaffold's own - # sample module is wired the same way (unpinned + workspace = true). - write_dependency(doc, mod.dist_name, "") - write_workspace_source(doc, mod.dist_name) - typer.echo(f"Added {mod.dist_name} (uv workspace member {written}).") - else: - write_dependency(doc, mod.dist_name, derive_range(mod.version)) - write_path_source(doc, mod.dist_name, path=written) - typer.echo(f"Added {mod.dist_name} (editable path {written}).") - save_pyproject(pyproject, doc) - if not no_sync: - post_install( - pyproject.parent, - [mod.dist_name], - [mod.dist_name] if mod.ships_models else [], - exec_runner, - ) - return [mod.dist_name] - - def run_add( spec: str, *, From a7f7e601c6114564658a80f15c19dd8d99c1394a Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Thu, 22 Oct 2026 13:04:26 +0200 Subject: [PATCH 12/15] refactor(hosting): split host_settings.py by responsibility (#522) Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 --- .../_host_settings_support.py | 108 ++++++++++++++++++ .../simple_module_hosting/host_settings.py | 94 ++++----------- 2 files changed, 129 insertions(+), 73 deletions(-) create mode 100644 framework/hosting/simple_module_hosting/_host_settings_support.py diff --git a/framework/hosting/simple_module_hosting/_host_settings_support.py b/framework/hosting/simple_module_hosting/_host_settings_support.py new file mode 100644 index 000000000..5144ae446 --- /dev/null +++ b/framework/hosting/simple_module_hosting/_host_settings_support.py @@ -0,0 +1,108 @@ +"""Restart-required markers and validator bodies for :class:`HostSettings`.""" + +from __future__ import annotations + +from typing import TYPE_CHECKING + +from pydantic_core import PydanticCustomError +from simple_module_core.cors import WILDCARD as CORS_WILDCARD +from simple_module_core.discovery import DEFAULT_AUTH_PROVIDER +from simple_module_core.stored_settings import RELATED_FIELDS_CTX + +if TYPE_CHECKING: + from simple_module_hosting.host_settings import HostSettings + +# Marks a field the running process cannot pick up without a restart, so the +# admin UI can say so rather than letting the edit look like it took effect. +_RESTART_DB = {"requires_restart": True, "group": "Database"} +_RESTART_GUARD = {"requires_restart": True, "group": "Request guards"} +_RESTART_CORS = {"requires_restart": True, "group": "CORS"} +_RESTART_API = {"requires_restart": True, "group": "API"} + +# Consumed while create_app builds the tenant middleware, the strict tenant +# filter, the i18n registry and LocaleMiddleware, the auth provider and the +# public-route registry, the proxy middleware and logging — none of which the +# lifespan hydration can rebuild (#499). test_build_time_settings_marked keeps +# this in step with what create_app actually reads. +_RESTART_TENANCY = {"requires_restart": True, "group": "Tenancy"} +_RESTART_I18N = {"requires_restart": True, "group": "Localization"} +_RESTART_AUTH = {"requires_restart": True, "group": "Authentication"} +_RESTART_LOGGING = {"requires_restart": True, "group": "Logging"} +_RESTART_SECURITY = {"requires_restart": True, "group": "Security"} + + +def check_public_base_url(value: str) -> str: + from urllib.parse import urlsplit + + value = value.strip().rstrip("/") + parts = urlsplit(value) + if value and not (parts.scheme in ("http", "https") and parts.netloc): + raise ValueError("public_base_url must be an http(s) origin such as https://example.org") + return value + + +def check_rate(value: str) -> str: + from simple_module_core.rate_limit import parse_rate + + parse_rate(value) # raises RateLimitError (a ValueError) on a typo + return value.strip() + + +def check_native_prefixes(value: list[str]) -> list[str]: + cleaned = [prefix.strip() for prefix in value] + for prefix in cleaned: + if not prefix.startswith("/"): + raise ValueError(f"native_link_prefixes entry {prefix!r} must start with '/'") + return cleaned + + +def check_body_bytes(value: int) -> int: + if value < 0: + raise ValueError("max_request_body_bytes must be >= 0 (0 disables the guard)") + return value + + +def normalize_auth_provider(value: str) -> str: + return value.strip() or DEFAULT_AUTH_PROVIDER + + +def normalize_trusted_proxy(value: str | None) -> str | None: + if value is None: + return None + return value.strip() or None + + +def check_default_tenant(value: str) -> str: + from simple_module_db import PLATFORM_TENANT_ID, is_valid_tenant_id + + value = value.strip() + if value == PLATFORM_TENANT_ID: + raise ValueError(f"default_tenant {value!r} is reserved for platform-owned rows") + if value and not is_valid_tenant_id(value): + raise ValueError(f"default_tenant {value!r} is not a valid tenant id") + return value + + +def check_cors_wildcard(settings: HostSettings) -> None: + if settings.cors_allow_credentials and CORS_WILDCARD in settings.cors_allow_origins: + raise PydanticCustomError( + "cors_wildcard_credentials", + "cors_allow_origins '*' cannot be combined with cors_allow_credentials", + {RELATED_FIELDS_CTX: ["cors_allow_origins", "cors_allow_credentials"]}, + ) + + +def check_default_locale_supported(settings: HostSettings) -> None: + if settings.i18n_default_locale not in settings.i18n_supported_locales: + # Model-level, so the error has no field location: the context + # names both halves, letting lenient boot hydration drop whichever + # one came from the DB instead of refusing to boot (#497). + raise PydanticCustomError( + "default_locale_unsupported", + "i18n_default_locale '{default}' is not in i18n_supported_locales {supported}", + { + "default": settings.i18n_default_locale, + "supported": settings.i18n_supported_locales, + RELATED_FIELDS_CTX: ["i18n_default_locale", "i18n_supported_locales"], + }, + ) diff --git a/framework/hosting/simple_module_hosting/host_settings.py b/framework/hosting/simple_module_hosting/host_settings.py index 3e8c62c4c..e1c83e20d 100644 --- a/framework/hosting/simple_module_hosting/host_settings.py +++ b/framework/hosting/simple_module_hosting/host_settings.py @@ -16,31 +16,22 @@ from typing import Literal from pydantic import Field, field_validator, model_validator -from pydantic_core import PydanticCustomError from pydantic_settings import BaseSettings, SettingsConfigDict -from simple_module_core.cors import WILDCARD as CORS_WILDCARD from simple_module_core.discovery import DEFAULT_AUTH_PROVIDER -from simple_module_core.stored_settings import RELATED_FIELDS_CTX +from simple_module_hosting import _host_settings_support as v from simple_module_hosting._api_version import default_api_version as _default_api_version - -# Marks a field the running process cannot pick up without a restart, so the -# admin UI can say so rather than letting the edit look like it took effect. -_RESTART_DB = {"requires_restart": True, "group": "Database"} -_RESTART_GUARD = {"requires_restart": True, "group": "Request guards"} -_RESTART_CORS = {"requires_restart": True, "group": "CORS"} -_RESTART_API = {"requires_restart": True, "group": "API"} - -# Consumed while create_app builds the tenant middleware, the strict tenant -# filter, the i18n registry and LocaleMiddleware, the auth provider and the -# public-route registry, the proxy middleware and logging — none of which the -# lifespan hydration can rebuild (#499). test_build_time_settings_marked keeps -# this in step with what create_app actually reads. -_RESTART_TENANCY = {"requires_restart": True, "group": "Tenancy"} -_RESTART_I18N = {"requires_restart": True, "group": "Localization"} -_RESTART_AUTH = {"requires_restart": True, "group": "Authentication"} -_RESTART_LOGGING = {"requires_restart": True, "group": "Logging"} -_RESTART_SECURITY = {"requires_restart": True, "group": "Security"} +from simple_module_hosting._host_settings_support import ( + _RESTART_API, + _RESTART_AUTH, + _RESTART_CORS, + _RESTART_DB, + _RESTART_GUARD, + _RESTART_I18N, + _RESTART_LOGGING, + _RESTART_SECURITY, + _RESTART_TENANCY, +) class HostSettings(BaseSettings): @@ -195,39 +186,22 @@ class HostSettings(BaseSettings): @field_validator("public_base_url", mode="after") @classmethod def _check_public_base_url(cls, value: str) -> str: - from urllib.parse import urlsplit - - value = value.strip().rstrip("/") - parts = urlsplit(value) - if value and not (parts.scheme in ("http", "https") and parts.netloc): - raise ValueError( - "public_base_url must be an http(s) origin such as https://example.org" - ) - return value + return v.check_public_base_url(value) @field_validator("rate_limit_public", "rate_limit_authenticated", mode="after") @classmethod def _check_rate(cls, value: str) -> str: - from simple_module_core.rate_limit import parse_rate - - parse_rate(value) # raises RateLimitError (a ValueError) on a typo - return value.strip() + return v.check_rate(value) @field_validator("native_link_prefixes", mode="after") @classmethod def _check_native_prefixes(cls, value: list[str]) -> list[str]: - cleaned = [prefix.strip() for prefix in value] - for prefix in cleaned: - if not prefix.startswith("/"): - raise ValueError(f"native_link_prefixes entry {prefix!r} must start with '/'") - return cleaned + return v.check_native_prefixes(value) @field_validator("max_request_body_bytes", mode="after") @classmethod def _check_body_bytes(cls, value: int) -> int: - if value < 0: - raise ValueError("max_request_body_bytes must be >= 0 (0 disables the guard)") - return value + return v.check_body_bytes(value) @field_validator("auth_provider", mode="after") @classmethod @@ -240,7 +214,7 @@ def _normalize_auth_provider(cls, value: str) -> str: ``resolve_auth_provider``, which already falls back on blank — without this they and the host would disagree about the active provider. """ - return value.strip() or DEFAULT_AUTH_PROVIDER + return v.normalize_auth_provider(value) @field_validator("trusted_proxy", mode="after") @classmethod @@ -253,45 +227,19 @@ def _normalize_trusted_proxy(cls, value: str | None) -> str | None: client — re-introducing GH #223 with no error. Blank → ``None`` so the middleware isn't installed at all. """ - if value is None: - return None - return value.strip() or None + return v.normalize_trusted_proxy(value) @field_validator("default_tenant", mode="after") @classmethod def _check_default_tenant(cls, value: str) -> str: - from simple_module_db import PLATFORM_TENANT_ID, is_valid_tenant_id - - value = value.strip() - if value == PLATFORM_TENANT_ID: - raise ValueError(f"default_tenant {value!r} is reserved for platform-owned rows") - if value and not is_valid_tenant_id(value): - raise ValueError(f"default_tenant {value!r} is not a valid tenant id") - return value + return v.check_default_tenant(value) @model_validator(mode="after") def _check_cors_wildcard(self) -> HostSettings: - if self.cors_allow_credentials and CORS_WILDCARD in self.cors_allow_origins: - raise PydanticCustomError( - "cors_wildcard_credentials", - "cors_allow_origins '*' cannot be combined with cors_allow_credentials", - {RELATED_FIELDS_CTX: ["cors_allow_origins", "cors_allow_credentials"]}, - ) + v.check_cors_wildcard(self) return self @model_validator(mode="after") def _check_default_locale_supported(self) -> HostSettings: - if self.i18n_default_locale not in self.i18n_supported_locales: - # Model-level, so the error has no field location: the context - # names both halves, letting lenient boot hydration drop whichever - # one came from the DB instead of refusing to boot (#497). - raise PydanticCustomError( - "default_locale_unsupported", - "i18n_default_locale '{default}' is not in i18n_supported_locales {supported}", - { - "default": self.i18n_default_locale, - "supported": self.i18n_supported_locales, - RELATED_FIELDS_CTX: ["i18n_default_locale", "i18n_supported_locales"], - }, - ) + v.check_default_locale_supported(self) return self From 281ff6e8146db48928ac5297b6c1e83262f6fa61 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Thu, 22 Oct 2026 13:05:55 +0200 Subject: [PATCH 13/15] refactor(hosting): split app_builder.py by responsibility (#522) Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 --- .../simple_module_hosting/_app_boot.py | 107 ++++++++++++++++++ .../simple_module_hosting/app_builder.py | 84 +++----------- 2 files changed, 124 insertions(+), 67 deletions(-) create mode 100644 framework/hosting/simple_module_hosting/_app_boot.py diff --git a/framework/hosting/simple_module_hosting/_app_boot.py b/framework/hosting/simple_module_hosting/_app_boot.py new file mode 100644 index 000000000..21a168f94 --- /dev/null +++ b/framework/hosting/simple_module_hosting/_app_boot.py @@ -0,0 +1,107 @@ +"""Boot-phase helpers for ``create_app``: app construction, database init, static mount.""" + +from __future__ import annotations + +import logging +from pathlib import Path + +from fastapi import Depends, FastAPI +from simple_module_core.design_packs import DesignPackRegistry +from simple_module_core.feature_flags import FeatureFlagRegistry +from simple_module_core.health import HealthRegistry +from simple_module_core.menu import MenuRegistry +from simple_module_core.permissions import PermissionRegistry +from simple_module_core.public_routes import PublicRouteRegistry +from simple_module_db.listeners import register_listeners +from simple_module_db.session import init_db + +from simple_module_hosting._db_health import register_database_check +from simple_module_hosting._openapi import ( + API_OPENAPI_URL, + install_openapi_filter, + make_unique_id_function, + openapi_served, +) +from simple_module_hosting._process_services import configure_process_services +from simple_module_hosting.db_options import database_options +from simple_module_hosting.settings import Settings +from simple_module_hosting.static_files import PrecompressedStaticFiles +from simple_module_hosting.token_guard import token_route_guard + +logger = logging.getLogger("simple_module_hosting.app_builder") + +_DOCS_URL = "/api/docs" +_REDOC_URL = "/api/redoc" +_STATIC_MOUNT_PATH = "/static" +_STATIC_DIR_NAME = "static" + + +def build_fastapi_app(settings: Settings, lifespan) -> FastAPI: + serve_openapi = openapi_served( + is_development=settings.is_development, openapi_public=settings.openapi_public + ) + app = FastAPI( + title=settings.api_title, + version=settings.api_version, + description=settings.api_description, + openapi_url=API_OPENAPI_URL if serve_openapi else None, + generate_unique_id_function=make_unique_id_function(), + docs_url=_DOCS_URL if settings.is_development else None, + redoc_url=_REDOC_URL if settings.is_development else None, + lifespan=lifespan, + # API tokens reach only permission-gated, non-page routes (or ones + # marked ``TokenAllowed``); see ``token_guard``. + dependencies=[Depends(token_route_guard)], + ) + + install_openapi_filter(app) # AuthMiddleware reads openapi_url: anonymous iff served + app.state.openapi_url = API_OPENAPI_URL if serve_openapi else None + return app + + +def log_registration_summary( + menu_registry: MenuRegistry, + perm_registry: PermissionRegistry, + ff_registry: FeatureFlagRegistry, + health_registry: HealthRegistry, + public_route_registry: PublicRouteRegistry, + design_pack_registry: DesignPackRegistry, +) -> None: + logger.info( + "Registered %d menu items, %d permissions, %d feature flags, " + "%d health checks, %d public routes, %d design packs", + len(menu_registry.all_items), + len(perm_registry.all_permissions), + len(ff_registry.all_flags), + len(health_registry.all_checks), + len(public_route_registry.routes), + len(design_pack_registry.all()), + ) + + +def init_database(settings: Settings, health_registry: HealthRegistry): + db_state = init_db( + settings.database_url, + echo=settings.debug, + **database_options(settings), + ) + # Multi-tenant installs fail closed: a tenant-scoped query with no tenant + # context raises instead of reading every tenant's rows. + db_state.tenant_strict = settings.multi_tenant + # Where unbound / all_tenants() writes land on a single-tenant install: + # its ``default_tenant`` when set, so they stay visible to its requests. + if not settings.multi_tenant and settings.default_tenant: + db_state.default_tenant_id = settings.default_tenant + register_listeners(db_state) + configure_process_services(settings.redis_url, engine=db_state.engine) + # The host's own readiness signal, and the only probe-safe check in a + # default install — module checks reach third parties and are on-demand. + register_database_check(health_registry, db_state) + return db_state + + +def mount_host_static(app: FastAPI, project_root: Path) -> None: + static_dir = project_root / "host" / _STATIC_DIR_NAME + if static_dir.is_dir(): + static = PrecompressedStaticFiles(directory=static_dir) + app.mount(_STATIC_MOUNT_PATH, static, name=_STATIC_DIR_NAME) diff --git a/framework/hosting/simple_module_hosting/app_builder.py b/framework/hosting/simple_module_hosting/app_builder.py index 15cc2f7dc..1dc08ea3f 100644 --- a/framework/hosting/simple_module_hosting/app_builder.py +++ b/framework/hosting/simple_module_hosting/app_builder.py @@ -4,7 +4,7 @@ import logging -from fastapi import Depends, FastAPI +from fastapi import FastAPI from simple_module_core import CspSourceRegistry from simple_module_core.audit_links import AuditLinkRegistry from simple_module_core.design_packs import DesignPackRegistry @@ -20,20 +20,17 @@ from simple_module_core.resources import ResourceTypeRegistry from simple_module_core.services import DiagnosticsState, Services from simple_module_core.setup_steps import SetupRegistry -from simple_module_db.listeners import register_listeners -from simple_module_db.session import init_db +from simple_module_hosting._app_boot import ( + build_fastapi_app, + init_database, + log_registration_summary, + mount_host_static, +) from simple_module_hosting._boot_checks import report_tenant_resolution, report_unguarded_routes -from simple_module_hosting._db_health import register_database_check from simple_module_hosting._dev_boot import run_dev_boot from simple_module_hosting._inertia_setup import setup_inertia from simple_module_hosting._lifespan import build_lifespan -from simple_module_hosting._openapi import ( - API_OPENAPI_URL, - install_openapi_filter, - make_unique_id_function, - openapi_served, -) from simple_module_hosting._phase_helpers import ( attach_public_routes, install_middleware, @@ -42,7 +39,6 @@ wire_module_routes, ) from simple_module_hosting._preapp_config import merge_host_settings -from simple_module_hosting._process_services import configure_process_services from simple_module_hosting._project_root import resolve_project_root as _resolve_project_root from simple_module_hosting._rate_limit_store import build_store from simple_module_hosting._registrations import RequestRegistries, run_module_registrations @@ -51,23 +47,16 @@ check_settings_registration, register_host_settings, ) -from simple_module_hosting.db_options import database_options from simple_module_hosting.health import router as health_router from simple_module_hosting.i18n_manifest import build_i18n_registry from simple_module_hosting.i18n_routes import router as i18n_router from simple_module_hosting.settings import Settings from simple_module_hosting.setup_gate import register_migration_step from simple_module_hosting.setup_wizard import mount_setup_wizard -from simple_module_hosting.static_files import PrecompressedStaticFiles from simple_module_hosting.tenancy import mode_for -from simple_module_hosting.token_guard import token_route_guard logger = logging.getLogger(__name__) -_DOCS_URL = "/api/docs" -_REDOC_URL = "/api/redoc" -_STATIC_MOUNT_PATH = "/static" -_STATIC_DIR_NAME = "static" _PROJECT_ROOT = _resolve_project_root() @@ -150,25 +139,7 @@ def create_app(settings: Settings | None = None) -> FastAPI: lifespan = build_lifespan(modules) - serve_openapi = openapi_served( - is_development=settings.is_development, openapi_public=settings.openapi_public - ) - app = FastAPI( - title=settings.api_title, - version=settings.api_version, - description=settings.api_description, - openapi_url=API_OPENAPI_URL if serve_openapi else None, - generate_unique_id_function=make_unique_id_function(), - docs_url=_DOCS_URL if settings.is_development else None, - redoc_url=_REDOC_URL if settings.is_development else None, - lifespan=lifespan, - # API tokens reach only permission-gated, non-page routes (or ones - # marked ``TokenAllowed``); see ``token_guard``. - dependencies=[Depends(token_route_guard)], - ) - - install_openapi_filter(app) # AuthMiddleware reads openapi_url: anonymous iff served - app.state.openapi_url = API_OPENAPI_URL if serve_openapi else None + app = build_fastapi_app(settings, lifespan) # ── Phase 4: Module settings ─────────────────────────── for mod in modules: @@ -210,35 +181,17 @@ def create_app(settings: Settings | None = None) -> FastAPI: # submitted slug before persisting it, and its view builds the dropdown. app.state.design_packs = design_pack_registry - logger.info( - "Registered %d menu items, %d permissions, %d feature flags, " - "%d health checks, %d public routes, %d design packs", - len(menu_registry.all_items), - len(perm_registry.all_permissions), - len(ff_registry.all_flags), - len(health_registry.all_checks), - len(public_route_registry.routes), - len(design_pack_registry.all()), + log_registration_summary( + menu_registry, + perm_registry, + ff_registry, + health_registry, + public_route_registry, + design_pack_registry, ) # ── Phase 6: Initialize database ─────────────────────── - db_state = init_db( - settings.database_url, - echo=settings.debug, - **database_options(settings), - ) - # Multi-tenant installs fail closed: a tenant-scoped query with no tenant - # context raises instead of reading every tenant's rows. - db_state.tenant_strict = settings.multi_tenant - # Where unbound / all_tenants() writes land on a single-tenant install: - # its ``default_tenant`` when set, so they stay visible to its requests. - if not settings.multi_tenant and settings.default_tenant: - db_state.default_tenant_id = settings.default_tenant - register_listeners(db_state) - configure_process_services(settings.redis_url, engine=db_state.engine) - # The host's own readiness signal, and the only probe-safe check in a - # default install — module checks reach third parties and are on-demand. - register_database_check(health_registry, db_state) + db_state = init_database(settings, health_registry) # ── Phase 7: Inertia + exception handlers ────────────── inertia_config = setup_inertia(app, settings, modules, _PROJECT_ROOT) @@ -263,10 +216,7 @@ def create_app(settings: Settings | None = None) -> FastAPI: app.include_router(i18n_router) # /i18n/set-locale — LocaleSwitcher posts here mount_setup_wizard(app, setup_registry) # /setup — what SetupMiddleware redirects to - static_dir = _PROJECT_ROOT / "host" / _STATIC_DIR_NAME - if static_dir.is_dir(): - static = PrecompressedStaticFiles(directory=static_dir) - app.mount(_STATIC_MOUNT_PATH, static, name=_STATIC_DIR_NAME) + mount_host_static(app, _PROJECT_ROOT) mount_module_static_dirs(app, modules) From f1774b9234456452bb4a6dc3635573ff71434bf8 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Thu, 22 Oct 2026 13:06:37 +0200 Subject: [PATCH 14/15] refactor(hosting): split _phase_helpers.py by responsibility (#522) Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 --- docs/framework/request-guards.md | 2 +- .../_middleware_pipeline.py | 205 +++++++++++++++++ .../simple_module_hosting/_phase_helpers.py | 209 +----------------- 3 files changed, 217 insertions(+), 199 deletions(-) create mode 100644 framework/hosting/simple_module_hosting/_middleware_pipeline.py diff --git a/docs/framework/request-guards.md b/docs/framework/request-guards.md index 78e7b0f25..fca535799 100644 --- a/docs/framework/request-guards.md +++ b/docs/framework/request-guards.md @@ -186,4 +186,4 @@ the shared store instead (see [Sharing the store](#sharing-the-store)). - `simple_module_core.body_limits.BodyLimitRegistry`, `simple_module_core.rate_limit` (rate parsing, in-process store), `PublicRoute.rate` - `simple_module_hosting._body_limit`, `_rate_limit`, `_request_guard_responses` -- Wiring: `_phase_helpers.install_middleware`; registry on `app.state.body_limits` +- Wiring: `_middleware_pipeline.install_middleware`; registry on `app.state.body_limits` diff --git a/framework/hosting/simple_module_hosting/_middleware_pipeline.py b/framework/hosting/simple_module_hosting/_middleware_pipeline.py new file mode 100644 index 000000000..98f254e86 --- /dev/null +++ b/framework/hosting/simple_module_hosting/_middleware_pipeline.py @@ -0,0 +1,205 @@ +"""Middleware pipeline installation, CSP choice and session-cookie security.""" + +from __future__ import annotations + +from typing import TYPE_CHECKING + +from fastapi import FastAPI +from simple_module_core.environments import NON_PROD_ENVIRONMENTS +from simple_module_db import CommitBeforeResponseMiddleware +from starlette.middleware.gzip import GZipMiddleware +from uvicorn.middleware.proxy_headers import ProxyHeadersMiddleware + +from simple_module_hosting import sentry +from simple_module_hosting._body_limit import BodyLimitMiddleware +from simple_module_hosting._cors import CorsMiddleware, host_policy +from simple_module_hosting._forwarded_host import ForwardedHostMiddleware +from simple_module_hosting._inertia_cache import InertiaCacheMiddleware +from simple_module_hosting._rate_limit import RateLimitMiddleware +from simple_module_hosting._route_access import API_PREFIX +from simple_module_hosting.i18n_middleware import LocaleMiddleware +from simple_module_hosting.maintenance import MaintenanceMiddleware +from simple_module_hosting.middleware import ( + CorrelationIdMiddleware, + InertiaLayoutDataMiddleware, + RequestLoggingMiddleware, + SecurityHeadersMiddleware, + TenantMiddleware, +) +from simple_module_hosting.session import SessionMiddleware +from simple_module_hosting.settings import Settings +from simple_module_hosting.setup_gate import SetupMiddleware + +if TYPE_CHECKING: + from simple_module_core.csp import CspSourceRegistry + from simple_module_core.menu import MenuRegistry + from simple_module_core.permissions import PermissionRegistry + from simple_module_core.rate_limit import WindowStore + + +# Below this, gzip framing costs more than it saves. Starlette's own default. +COMPRESSION_MIN_BYTES = 500 + +# Starlette defaults to 9, which buys almost nothing on JSON and HTML: a 76 KB +# first-load page compresses 1.2% smaller at 9 than at 5, for 2.4x the CPU +# (6.4 ms vs 2.7 ms), spent on the event loop that serves every other request. +COMPRESSION_LEVEL = 5 + + +def build_csp(settings: Settings, csp_registry: CspSourceRegistry) -> str: + """Choose the CSP for this boot and fold in module-declared sources. + + Development gets the Vite-widened policy; production the strict default. + ``csp_registry`` carries origins modules declared via + ``register_csp_sources`` (e.g. an external font host) — merged here so + both variants honor them. An empty registry leaves the policy unchanged. + """ + base = ( + SecurityHeadersMiddleware.dev_csp(settings.vite_dev_url) + if settings.is_development + else SecurityHeadersMiddleware.DEFAULT_CSP + ) + return csp_registry.extend_policy(base) + + +def session_cookie_is_secure(settings: Settings) -> bool: + """Whether the session cookie carries ``Secure`` (#495). + + ``session_cookie_secure`` wins when set; unset, the cookie is ``Secure`` + everywhere but the non-prod environments, which run over plain HTTP. + """ + if settings.session_cookie_secure is not None: + return settings.session_cookie_secure + return settings.environment not in NON_PROD_ENVIRONMENTS + + +def install_middleware( + app: FastAPI, + settings: Settings, + modules: list, + menu_registry: MenuRegistry, + perm_registry: PermissionRegistry, + csp_registry: CspSourceRegistry, + rate_limit_store: WindowStore, +) -> None: + """Install the full middleware pipeline. + + Order matters: last added = first executed. Execution order: + (ForwardedHost, if trusted_proxy and forwarded_host_allowlist) → + (ProxyHeaders, if trusted_proxy) → CorrelationId → RequestLogging → CORS + → BodyLimit → GZip → Security → Session → [module] → (Tenant, if + multi_tenant) → Locale → Inertia → InertiaCache → RateLimit → Setup + → Maintenance → CommitBeforeResponse. + """ + # Added first, so it is innermost and its send-wrapper is the first to see + # the response: the request's DB work commits before any byte reaches the + # client, instead of in get_db's post-response exit code (GH #257). + app.add_middleware(CommitBeforeResponseMiddleware) + # Inside InertiaCache, so its short-circuit is still governed by it. The + # maintenance 503 renders through Inertia and carries this user's auth + # block and menus like any other payload; short-circuiting *outside* the + # cache guard would ship exactly the per-user payload GH #272 exists to + # keep out of caches. Added before Inertia so it *executes* after it: the + # page needs the shared props (auth, menus, i18n) to render with a layout, + # and auth + locale — both further out — to know who is asking and in which + # language to answer. + app.add_middleware(MaintenanceMiddleware) + # Added after Maintenance so it *executes* before it: an install that has + # never been set up has nothing meaningful to put into maintenance mode, + # and the setup redirect should win. Inside InertiaCache for the same + # reason Maintenance is — this short-circuits, and the redirect must not + # be stored by any cache. + app.add_middleware(SetupMiddleware) + # Added before InertiaCache so it executes just after it: auth has run (it + # can tell anonymous from signed-in), locale/shared props are in place for + # the error page, and a 429 is still marked non-cacheable. + app.add_middleware( + RateLimitMiddleware, + public_rate=settings.rate_limit_public, + authenticated_rate=settings.rate_limit_authenticated, + store=rate_limit_store, + ) + # Paired with InertiaLayoutDataMiddleware below, which is what puts this + # user's auth, permissions and menus into every Inertia payload: this one + # makes sure the payload that results is never stored where a page request + # can be answered with it. + app.add_middleware(InertiaCacheMiddleware) + app.add_middleware( + InertiaLayoutDataMiddleware, + menu_registry=menu_registry, + permission_registry=perm_registry, + ) + app.add_middleware( + LocaleMiddleware, + supported_locales=settings.i18n_supported_locales, + default_locale=settings.i18n_default_locale, + cookie_name=settings.i18n_cookie_name, + ) + if settings.multi_tenant: + app.add_middleware(TenantMiddleware, header=settings.tenant_header or None) + elif settings.default_tenant: + # Single-tenant host with tenant-scoped tables: every request is the + # one tenant (#359). Never used with multi_tenant — no shared fallback. + app.add_middleware(TenantMiddleware, fixed=settings.default_tenant) + for mod in modules: + mod.register_middleware(app) + app.add_middleware( + SessionMiddleware, + secret_key=settings.secret_key, + https_only=session_cookie_is_secure(settings), + ) + # In dev, relax CSP so the browser can fetch @vite/client, main.tsx, and + # the HMR WebSocket from the Vite origin (build_csp picks the variant). + # HSTS is suppressed in dev because it runs over plain HTTP on loopback. + security_kwargs: dict[str, str | None] = { + "content_security_policy": build_csp(settings, csp_registry) + } + if settings.is_development: + security_kwargs["strict_transport_security"] = None + # Providers registered via ``registry.add_provider`` follow DB-backed + # settings, so the middleware evaluates them per response (#434). + app.add_middleware(SecurityHeadersMiddleware, csp_registry=csp_registry, **security_kwargs) + # Compress response bodies. Added here so it sits inside CorrelationId and + # RequestLogging (which set headers and read request state) but outside + # everything that produces a body — including the /static mount, where it + # matters most: the built CSS is ~139 KB raw and ~21 KB gzipped, and the + # JS bundle compresses about 3x. Uncompressed assets dominated cold page + # load, several times larger than anything on the server request path. + app.add_middleware( + GZipMiddleware, minimum_size=COMPRESSION_MIN_BYTES, compresslevel=COMPRESSION_LEVEL + ) + # Right inside RequestLogging (so a 413 is logged) and outside GZip and the + # whole module tier: an oversized body is refused before anything reads it. + app.add_middleware( + BodyLimitMiddleware, + max_bytes=settings.max_request_body_bytes, + registry=getattr(app.state, "body_limits", None), + ) + # CORS executes between RequestLogging and BodyLimit: preflights are answered + # before auth, rate limit, setup and maintenance can reject them, and a 413 + # still carries the headers. The host's ``/api/`` policy joins the modules'. + cors_registry = app.state.cors_policies + api_policy = host_policy(settings) + if api_policy is not None: + cors_registry.add_prefix(API_PREFIX, api_policy) + app.add_middleware(CorsMiddleware, registry=cors_registry) + app.add_middleware(RequestLoggingMiddleware) + app.add_middleware(CorrelationIdMiddleware) + if sentry.is_enabled(): + app.add_middleware(sentry.SentryContextMiddleware) + # Outermost: rewrite scheme/client from X-Forwarded-* before anything else + # reads them, so request logs see the real client IP rather than the + # proxy's. Inertia no longer needs this: its page url is + # root-relative, so pushState can't see a cross-scheme url either way (it + # used to throw a SecurityError on every page — GH #223). Gated on an + # explicit trust setting — never trust forwarded headers by default. + if settings.trusted_proxy: + app.add_middleware(ProxyHeadersMiddleware, trusted_hosts=settings.trusted_proxy) + # Added after ProxyHeaders so it executes before it, while the TCP peer is + # still the proxy (#528). Needs both a trusted proxy and an allowlist. + if settings.forwarded_host_allowlist and settings.trusted_proxy: + app.add_middleware( + ForwardedHostMiddleware, + trusted_hosts=settings.trusted_proxy, + allowlist=settings.forwarded_host_allowlist, + ) diff --git a/framework/hosting/simple_module_hosting/_phase_helpers.py b/framework/hosting/simple_module_hosting/_phase_helpers.py index bb74c5313..317dac6ae 100644 --- a/framework/hosting/simple_module_hosting/_phase_helpers.py +++ b/framework/hosting/simple_module_hosting/_phase_helpers.py @@ -1,6 +1,7 @@ -"""Helpers extracted from ``app_builder.py`` — exception handlers, middleware -pipeline installation, static mounts, and the SM012 post-registration check. +"""Helpers extracted from ``app_builder.py`` — exception handlers, public-route +seeding, static mounts, and the SM012 post-registration check. +The middleware pipeline lives in ``_middleware_pipeline`` and is re-exported here. Kept private to the hosting package; ``app_builder.create_app`` is the only intended caller. """ @@ -9,67 +10,38 @@ import logging from pathlib import Path -from typing import TYPE_CHECKING from fastapi import FastAPI from fastapi.exceptions import RequestValidationError from fastapi.staticfiles import StaticFiles -from simple_module_core.environments import NON_PROD_ENVIRONMENTS from simple_module_core.exceptions import NotFoundError -from simple_module_db import CommitBeforeResponseMiddleware from simple_module_inertia import ( InertiaVersionConflictException, inertia_version_conflict_exception_handler, ) from starlette.exceptions import HTTPException -from starlette.middleware.gzip import GZipMiddleware -from uvicorn.middleware.proxy_headers import ProxyHeadersMiddleware -from simple_module_hosting import sentry -from simple_module_hosting._body_limit import BodyLimitMiddleware -from simple_module_hosting._cors import CorsMiddleware, host_policy from simple_module_hosting._error_handlers import ( http_exception_handler, not_found_error_handler, request_validation_error_handler, unhandled_exception_handler, ) -from simple_module_hosting._forwarded_host import ForwardedHostMiddleware -from simple_module_hosting._inertia_cache import InertiaCacheMiddleware +from simple_module_hosting._middleware_pipeline import ( # noqa: F401 (re-exported) + COMPRESSION_LEVEL, + COMPRESSION_MIN_BYTES, + build_csp, + install_middleware, + session_cookie_is_secure, +) from simple_module_hosting._module_routes import wire_module_routes -from simple_module_hosting._rate_limit import RateLimitMiddleware -from simple_module_hosting._route_access import API_PREFIX from simple_module_hosting._spa_mount import mount_spa_mounts -from simple_module_hosting.i18n_middleware import LocaleMiddleware -from simple_module_hosting.maintenance import MaintenanceMiddleware -from simple_module_hosting.middleware import ( - CorrelationIdMiddleware, - InertiaLayoutDataMiddleware, - RequestLoggingMiddleware, - SecurityHeadersMiddleware, - TenantMiddleware, -) -from simple_module_hosting.session import SessionMiddleware from simple_module_hosting.settings import Settings -from simple_module_hosting.setup_gate import SETUP_PATH, SetupMiddleware +from simple_module_hosting.setup_gate import SETUP_PATH from simple_module_hosting.static_files import PrecompressedStaticFiles -if TYPE_CHECKING: - from simple_module_core.csp import CspSourceRegistry - from simple_module_core.menu import MenuRegistry - from simple_module_core.permissions import PermissionRegistry - from simple_module_core.rate_limit import WindowStore - logger = logging.getLogger(__name__) -# Below this, gzip framing costs more than it saves. Starlette's own default. -COMPRESSION_MIN_BYTES = 500 - -# Starlette defaults to 9, which buys almost nothing on JSON and HTML: a 76 KB -# first-load page compresses 1.2% smaller at 9 than at 5, for 2.4x the CPU -# (6.4 ms vs 2.7 ms), spent on the event loop that serves every other request. -COMPRESSION_LEVEL = 5 - # Re-exported for back-compat: static-file serving now lives in static_files. ImmutableStaticFiles = PrecompressedStaticFiles @@ -88,165 +60,6 @@ def register_exception_handlers(app: FastAPI, modules: list) -> None: mod.register_exception_handlers(app) -def build_csp(settings: Settings, csp_registry: CspSourceRegistry) -> str: - """Choose the CSP for this boot and fold in module-declared sources. - - Development gets the Vite-widened policy; production the strict default. - ``csp_registry`` carries origins modules declared via - ``register_csp_sources`` (e.g. an external font host) — merged here so - both variants honor them. An empty registry leaves the policy unchanged. - """ - base = ( - SecurityHeadersMiddleware.dev_csp(settings.vite_dev_url) - if settings.is_development - else SecurityHeadersMiddleware.DEFAULT_CSP - ) - return csp_registry.extend_policy(base) - - -def session_cookie_is_secure(settings: Settings) -> bool: - """Whether the session cookie carries ``Secure`` (#495). - - ``session_cookie_secure`` wins when set; unset, the cookie is ``Secure`` - everywhere but the non-prod environments, which run over plain HTTP. - """ - if settings.session_cookie_secure is not None: - return settings.session_cookie_secure - return settings.environment not in NON_PROD_ENVIRONMENTS - - -def install_middleware( - app: FastAPI, - settings: Settings, - modules: list, - menu_registry: MenuRegistry, - perm_registry: PermissionRegistry, - csp_registry: CspSourceRegistry, - rate_limit_store: WindowStore, -) -> None: - """Install the full middleware pipeline. - - Order matters: last added = first executed. Execution order: - (ForwardedHost, if trusted_proxy and forwarded_host_allowlist) → - (ProxyHeaders, if trusted_proxy) → CorrelationId → RequestLogging → CORS - → BodyLimit → GZip → Security → Session → [module] → (Tenant, if - multi_tenant) → Locale → Inertia → InertiaCache → RateLimit → Setup - → Maintenance → CommitBeforeResponse. - """ - # Added first, so it is innermost and its send-wrapper is the first to see - # the response: the request's DB work commits before any byte reaches the - # client, instead of in get_db's post-response exit code (GH #257). - app.add_middleware(CommitBeforeResponseMiddleware) - # Inside InertiaCache, so its short-circuit is still governed by it. The - # maintenance 503 renders through Inertia and carries this user's auth - # block and menus like any other payload; short-circuiting *outside* the - # cache guard would ship exactly the per-user payload GH #272 exists to - # keep out of caches. Added before Inertia so it *executes* after it: the - # page needs the shared props (auth, menus, i18n) to render with a layout, - # and auth + locale — both further out — to know who is asking and in which - # language to answer. - app.add_middleware(MaintenanceMiddleware) - # Added after Maintenance so it *executes* before it: an install that has - # never been set up has nothing meaningful to put into maintenance mode, - # and the setup redirect should win. Inside InertiaCache for the same - # reason Maintenance is — this short-circuits, and the redirect must not - # be stored by any cache. - app.add_middleware(SetupMiddleware) - # Added before InertiaCache so it executes just after it: auth has run (it - # can tell anonymous from signed-in), locale/shared props are in place for - # the error page, and a 429 is still marked non-cacheable. - app.add_middleware( - RateLimitMiddleware, - public_rate=settings.rate_limit_public, - authenticated_rate=settings.rate_limit_authenticated, - store=rate_limit_store, - ) - # Paired with InertiaLayoutDataMiddleware below, which is what puts this - # user's auth, permissions and menus into every Inertia payload: this one - # makes sure the payload that results is never stored where a page request - # can be answered with it. - app.add_middleware(InertiaCacheMiddleware) - app.add_middleware( - InertiaLayoutDataMiddleware, - menu_registry=menu_registry, - permission_registry=perm_registry, - ) - app.add_middleware( - LocaleMiddleware, - supported_locales=settings.i18n_supported_locales, - default_locale=settings.i18n_default_locale, - cookie_name=settings.i18n_cookie_name, - ) - if settings.multi_tenant: - app.add_middleware(TenantMiddleware, header=settings.tenant_header or None) - elif settings.default_tenant: - # Single-tenant host with tenant-scoped tables: every request is the - # one tenant (#359). Never used with multi_tenant — no shared fallback. - app.add_middleware(TenantMiddleware, fixed=settings.default_tenant) - for mod in modules: - mod.register_middleware(app) - app.add_middleware( - SessionMiddleware, - secret_key=settings.secret_key, - https_only=session_cookie_is_secure(settings), - ) - # In dev, relax CSP so the browser can fetch @vite/client, main.tsx, and - # the HMR WebSocket from the Vite origin (build_csp picks the variant). - # HSTS is suppressed in dev because it runs over plain HTTP on loopback. - security_kwargs: dict[str, str | None] = { - "content_security_policy": build_csp(settings, csp_registry) - } - if settings.is_development: - security_kwargs["strict_transport_security"] = None - # Providers registered via ``registry.add_provider`` follow DB-backed - # settings, so the middleware evaluates them per response (#434). - app.add_middleware(SecurityHeadersMiddleware, csp_registry=csp_registry, **security_kwargs) - # Compress response bodies. Added here so it sits inside CorrelationId and - # RequestLogging (which set headers and read request state) but outside - # everything that produces a body — including the /static mount, where it - # matters most: the built CSS is ~139 KB raw and ~21 KB gzipped, and the - # JS bundle compresses about 3x. Uncompressed assets dominated cold page - # load, several times larger than anything on the server request path. - app.add_middleware( - GZipMiddleware, minimum_size=COMPRESSION_MIN_BYTES, compresslevel=COMPRESSION_LEVEL - ) - # Right inside RequestLogging (so a 413 is logged) and outside GZip and the - # whole module tier: an oversized body is refused before anything reads it. - app.add_middleware( - BodyLimitMiddleware, - max_bytes=settings.max_request_body_bytes, - registry=getattr(app.state, "body_limits", None), - ) - # CORS executes between RequestLogging and BodyLimit: preflights are answered - # before auth, rate limit, setup and maintenance can reject them, and a 413 - # still carries the headers. The host's ``/api/`` policy joins the modules'. - cors_registry = app.state.cors_policies - api_policy = host_policy(settings) - if api_policy is not None: - cors_registry.add_prefix(API_PREFIX, api_policy) - app.add_middleware(CorsMiddleware, registry=cors_registry) - app.add_middleware(RequestLoggingMiddleware) - app.add_middleware(CorrelationIdMiddleware) - if sentry.is_enabled(): - app.add_middleware(sentry.SentryContextMiddleware) - # Outermost: rewrite scheme/client from X-Forwarded-* before anything else - # reads them, so request logs see the real client IP rather than the - # proxy's. Inertia no longer needs this: its page url is - # root-relative, so pushState can't see a cross-scheme url either way (it - # used to throw a SecurityError on every page — GH #223). Gated on an - # explicit trust setting — never trust forwarded headers by default. - if settings.trusted_proxy: - app.add_middleware(ProxyHeadersMiddleware, trusted_hosts=settings.trusted_proxy) - # Added after ProxyHeaders so it executes before it, while the TCP peer is - # still the proxy (#528). Needs both a trusted proxy and an allowlist. - if settings.forwarded_host_allowlist and settings.trusted_proxy: - app.add_middleware( - ForwardedHostMiddleware, - trusted_hosts=settings.trusted_proxy, - allowlist=settings.forwarded_host_allowlist, - ) - - def attach_public_routes(app: FastAPI, settings: Settings, registry) -> None: """Seed host-level public paths and publish the registry for AuthMiddleware. From aa1bc0a4d58a3c0f0794c52933fb8760c0195921 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Thu, 22 Oct 2026 13:07:33 +0200 Subject: [PATCH 15/15] refactor(db): split bulk_audit.py by responsibility (#522) Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 --- .../simple_module_db/_bulk_audit_records.py | 129 +++++++++++++++++ framework/db/simple_module_db/bulk_audit.py | 136 +++--------------- 2 files changed, 148 insertions(+), 117 deletions(-) create mode 100644 framework/db/simple_module_db/_bulk_audit_records.py diff --git a/framework/db/simple_module_db/_bulk_audit_records.py b/framework/db/simple_module_db/_bulk_audit_records.py new file mode 100644 index 000000000..7c142378f --- /dev/null +++ b/framework/db/simple_module_db/_bulk_audit_records.py @@ -0,0 +1,129 @@ +"""Audit record building and delivery shared by the bulk-audit hook and its manual entry point.""" + +from __future__ import annotations + +import logging +from collections.abc import Iterable +from typing import Any, Final + +from simple_module_core.correlation import correlation_id +from sqlalchemy import inspect as sa_inspect +from sqlalchemy.orm import ORMExecuteState, Session +from sqlalchemy.sql.elements import BindParameter +from sqlalchemy.sql.visitors import iterate + +from simple_module_db.audit import AuditRecord, _serialize, format_pk +from simple_module_db.query_filter import engine_policy + +logger = logging.getLogger("simple_module_db.bulk_audit") + +ACTION_BULK_INSERTED: Final = "bulk_inserted" +ACTION_BULK_UPDATED: Final = "bulk_updated" +ACTION_BULK_DELETED: Final = "bulk_deleted" +ACTION_BULK_SOFT_DELETED: Final = "bulk_soft_deleted" +ACTION_BULK_RESTORED: Final = "bulk_restored" +SUMMARY_ENTITY_ID: Final = "*" + +_SOFT_DELETE_FLAG: Final = "is_deleted" +_ACTIVE_KEY: Final = "_sm_bulk_audit_active" + + +class _Ctx: + """What one audited statement records, built before it runs.""" + + def __init__(self, entity_type: str, action: str, changes: list[dict[str, Any]]) -> None: + self.entity_type = entity_type + self.action = action + self.row_actions = [action] + self.changes = changes + self.user_id = _current_user() + self.correlation_id = correlation_id.get("") or None + + def record( + self, + entity_id: str, + changes: list[dict[str, Any]] | None = None, + action: str | None = None, + ) -> AuditRecord: + return AuditRecord( + entity_type=self.entity_type, + entity_id=entity_id, + action=action or self.action, + changes=self.changes if changes is None else changes, + user_id=self.user_id, + correlation_id=self.correlation_id, + ) + + +def _current_user() -> str | None: + from simple_module_db.listeners import current_user_id + + return current_user_id.get() + + +def _where_params(stmt: Any, params: Any) -> dict[str, Any] | None: + """The execute params the ``WHERE`` clause binds, so the prefetch can run it.""" + where = getattr(stmt, "whereclause", None) + if where is None or not isinstance(params, dict): + return None + keys = {e.key for e in iterate(where) if isinstance(e, BindParameter)} + return {k: v for k, v in params.items() if k in keys} or None + + +def _bulk_by_pk_ids(execute_state: ORMExecuteState, cls: type) -> list[str] | None: + """``session.execute(update(Model), [{pk..., col...}, ...])``: ids from the rows.""" + params = execute_state.parameters + if not (execute_state.is_update and isinstance(params, list) and params): + return None + pk = [col.key for col in sa_inspect(cls).primary_key] + if not all(all(k in row for k in pk) for row in params): + return None + return [format_pk([row[k] for k in pk]) for row in params] + + +def _summary(ctx: _Ctx, stmt: Any, rowcount: Any, action: str) -> AuditRecord: + where = getattr(stmt, "whereclause", None) + summary = [ + {"field": "where_sql", "new": str(where) if where is not None else None}, + {"field": "rowcount", "new": rowcount}, + {"field": "truncated", "new": True}, + ] + return ctx.record(SUMMARY_ENTITY_ID, [*ctx.changes, *summary], action) + + +def _deliver(session: Session, callback: Any, records: list[AuditRecord]) -> None: + if not records: + return + session.info[_ACTIVE_KEY] = True + try: + callback(session, records) + except Exception: + logger.exception("Audit sink failed for %d bulk record(s)", len(records)) + finally: + session.info.pop(_ACTIVE_KEY, None) + + +def record_bulk_audit( + session: Any, + entity_type: type | str, + action: str, + ids: Iterable[object] | None, + changes: dict[str, Any] | None = None, +) -> None: + """Audit a write the listeners cannot see — a ``text("UPDATE ...")`` statement. + + One record per id (``ids=None`` records one row with ``entity_id`` ``"*"``), + in the session's transaction: a rollback discards them with the write. + ``changes`` maps column name to new value. A no-op on an unaudited engine; + never raises because of the audit sink. Accepts an ``AsyncSession`` or its + sync ``Session``. + """ + sync = getattr(session, "sync_session", session) + callback = getattr(engine_policy(sync), "audit_callback", None) + if callback is None: + return + name = entity_type if isinstance(entity_type, str) else entity_type.__name__ + diff = [{"field": k, "new": _serialize(v)} for k, v in (changes or {}).items()] + ctx = _Ctx(name, action, diff) + keys = [SUMMARY_ENTITY_ID] if ids is None else [str(i) for i in ids] + _deliver(sync, callback, [ctx.record(k) for k in keys]) diff --git a/framework/db/simple_module_db/bulk_audit.py b/framework/db/simple_module_db/bulk_audit.py index 1fe7cf54a..84d957b51 100644 --- a/framework/db/simple_module_db/bulk_audit.py +++ b/framework/db/simple_module_db/bulk_audit.py @@ -25,18 +25,31 @@ from __future__ import annotations import logging -from collections.abc import Iterable from typing import Any, Final -from simple_module_core.correlation import correlation_id from sqlalchemy import inspect as sa_inspect from sqlalchemy import select -from sqlalchemy.orm import ORMExecuteState, Session -from sqlalchemy.sql.elements import BindParameter -from sqlalchemy.sql.visitors import iterate +from sqlalchemy.orm import ORMExecuteState from simple_module_db import model_registry as registry -from simple_module_db.audit import AuditRecord, _excluded_fields, _serialize, format_pk +from simple_module_db._bulk_audit_records import ( # noqa: F401 (re-exported) + _ACTIVE_KEY, + _SOFT_DELETE_FLAG, + ACTION_BULK_DELETED, + ACTION_BULK_INSERTED, + ACTION_BULK_RESTORED, + ACTION_BULK_SOFT_DELETED, + ACTION_BULK_UPDATED, + SUMMARY_ENTITY_ID, + _bulk_by_pk_ids, + _Ctx, + _current_user, + _deliver, + _summary, + _where_params, + record_bulk_audit, +) +from simple_module_db.audit import _excluded_fields, format_pk from simple_module_db.bulk_audit_targets import _is_set, _shown, _targets from simple_module_db.query_filter import engine_policy from simple_module_db.tenancy import ALL_TENANTS_OPTION @@ -49,16 +62,6 @@ BULK_AUDIT_ID_CAP: Final = 500 """Most rows a bulk statement records one by one; past it, one summary row.""" -ACTION_BULK_INSERTED: Final = "bulk_inserted" -ACTION_BULK_UPDATED: Final = "bulk_updated" -ACTION_BULK_DELETED: Final = "bulk_deleted" -ACTION_BULK_SOFT_DELETED: Final = "bulk_soft_deleted" -ACTION_BULK_RESTORED: Final = "bulk_restored" -SUMMARY_ENTITY_ID: Final = "*" - -_SOFT_DELETE_FLAG: Final = "is_deleted" -_ACTIVE_KEY: Final = "_sm_bulk_audit_active" - class UnauditedWriteError(RuntimeError): """An opted-out DML statement against an audited model, in strict mode.""" @@ -76,39 +79,6 @@ def _target_class(execute_state: ORMExecuteState) -> type | None: return registry.classes_by_table.get(table) if table is not None else None -class _Ctx: - """What one audited statement records, built before it runs.""" - - def __init__(self, entity_type: str, action: str, changes: list[dict[str, Any]]) -> None: - self.entity_type = entity_type - self.action = action - self.row_actions = [action] - self.changes = changes - self.user_id = _current_user() - self.correlation_id = correlation_id.get("") or None - - def record( - self, - entity_id: str, - changes: list[dict[str, Any]] | None = None, - action: str | None = None, - ) -> AuditRecord: - return AuditRecord( - entity_type=self.entity_type, - entity_id=entity_id, - action=action or self.action, - changes=self.changes if changes is None else changes, - user_id=self.user_id, - correlation_id=self.correlation_id, - ) - - -def _current_user() -> str | None: - from simple_module_db.listeners import current_user_id - - return current_user_id.get() - - def _plan(execute_state: ORMExecuteState, cls: type) -> _Ctx | None: """The record shape for this statement, or ``None`` when nothing is audited.""" name = cls.__name__ @@ -145,15 +115,6 @@ def _plan(execute_state: ORMExecuteState, cls: type) -> _Ctx | None: return ctx -def _where_params(stmt: Any, params: Any) -> dict[str, Any] | None: - """The execute params the ``WHERE`` clause binds, so the prefetch can run it.""" - where = getattr(stmt, "whereclause", None) - if where is None or not isinstance(params, dict): - return None - keys = {e.key for e in iterate(where) if isinstance(e, BindParameter)} - return {k: v for k, v in params.items() if k in keys} or None - - def _prefetch_ids(execute_state: ORMExecuteState, cls: type, cap: int) -> list[str]: """Primary keys of the rows the statement will touch, at most ``cap + 1``.""" stmt = execute_state.statement @@ -173,27 +134,6 @@ def _prefetch_ids(execute_state: ORMExecuteState, cls: type, cap: int) -> list[s return [format_pk(tuple(row)) for row in rows] -def _bulk_by_pk_ids(execute_state: ORMExecuteState, cls: type) -> list[str] | None: - """``session.execute(update(Model), [{pk..., col...}, ...])``: ids from the rows.""" - params = execute_state.parameters - if not (execute_state.is_update and isinstance(params, list) and params): - return None - pk = [col.key for col in sa_inspect(cls).primary_key] - if not all(all(k in row for k in pk) for row in params): - return None - return [format_pk([row[k] for k in pk]) for row in params] - - -def _summary(ctx: _Ctx, stmt: Any, rowcount: Any, action: str) -> AuditRecord: - where = getattr(stmt, "whereclause", None) - summary = [ - {"field": "where_sql", "new": str(where) if where is not None else None}, - {"field": "rowcount", "new": rowcount}, - {"field": "truncated", "new": True}, - ] - return ctx.record(SUMMARY_ENTITY_ID, [*ctx.changes, *summary], action) - - def _plans_a_record(execute_state: ORMExecuteState, cls: type) -> bool: """Whether the statement would be audited; a planning failure never refuses the write.""" try: @@ -251,41 +191,3 @@ def audit_statement(execute_state: ORMExecuteState) -> Any: records = [_summary(ctx, stmt, rowcount, a) for a in distinct] _deliver(session, callback, records) return result - - -def _deliver(session: Session, callback: Any, records: list[AuditRecord]) -> None: - if not records: - return - session.info[_ACTIVE_KEY] = True - try: - callback(session, records) - except Exception: - logger.exception("Audit sink failed for %d bulk record(s)", len(records)) - finally: - session.info.pop(_ACTIVE_KEY, None) - - -def record_bulk_audit( - session: Any, - entity_type: type | str, - action: str, - ids: Iterable[object] | None, - changes: dict[str, Any] | None = None, -) -> None: - """Audit a write the listeners cannot see — a ``text("UPDATE ...")`` statement. - - One record per id (``ids=None`` records one row with ``entity_id`` ``"*"``), - in the session's transaction: a rollback discards them with the write. - ``changes`` maps column name to new value. A no-op on an unaudited engine; - never raises because of the audit sink. Accepts an ``AsyncSession`` or its - sync ``Session``. - """ - sync = getattr(session, "sync_session", session) - callback = getattr(engine_policy(sync), "audit_callback", None) - if callback is None: - return - name = entity_type if isinstance(entity_type, str) else entity_type.__name__ - diff = [{"field": k, "new": _serialize(v)} for k, v in (changes or {}).items()] - ctx = _Ctx(name, action, diff) - keys = [SUMMARY_ENTITY_ID] if ids is None else [str(i) for i in ids] - _deliver(sync, callback, [ctx.record(k) for k in keys])