From aa3ded356078d59f9ec93deabd612b96349f4ab1 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Fri, 29 May 2026 12:50:24 +0200 Subject: [PATCH] =?UTF-8?q?refactor(db):=20drop=20schema-per-module=20?= =?UTF-8?q?=E2=80=94=20single=20shared=20schema=20on=20Postgres=20+=20SQLi?= =?UTF-8?q?te?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All module tables now live in the host's single schema on every backend. Per-module `MetaData` is retained so Alembic autogenerate can attribute tables to a module via `branch_labels`, but the schema-per-module provider/auto-detect machinery is gone. - framework/db: `create_module_base()` no longer takes `provider=`; always builds a flat `MetaData`. `psycopg2-binary` added as a sync driver dep for Alembic. - Removed `test_postgres_schema_per_module.py` and provider-related test cases; updated `_models.py` / `test_base.py` / `test_mixins.py`. - Docs, skills, CLAUDE.md, and the new-module scaffold template updated to describe a single shared schema (no Postgres/SQLite divergence). - modules/{background_tasks,file_storage,permissions}: removed provider-specific comments from `models.py`. Catch-up migrations for two pre-existing drifts: - 873ca2015033_users_refresh_token_table — adds `users_refresh_token`. - 168a2882f443_keycloak_initial_schema — adds `keycloak_user_cache` with `branch_labels=("keycloak",)`. Verified: `alembic check` clean, full base↔head roundtrip clean on Postgres 16 and SQLite, `make doctor` clean, 1273 pytest tests pass. --- CLAUDE.md | 4 +- docs/database/models.md | 10 +- docs/database/per-module-base.md | 29 ++--- docs/framework-conventions.md | 9 +- docs/guide/first-module.md | 4 +- docs/guide/quickstart.md | 4 +- framework/db/README.md | 2 +- framework/db/pyproject.toml | 4 + framework/db/simple_module_db/__init__.py | 2 +- framework/db/simple_module_db/base.py | 95 ++++------------ framework/db/simple_module_db/migrations.py | 2 +- framework/db/tests/_audit_models.py | 3 +- framework/db/tests/_models.py | 3 +- framework/db/tests/test_base.py | 35 ++---- framework/db/tests/test_mixins.py | 3 +- .../tests/test_postgres_schema_per_module.py | 106 ------------------ .../168a2882f443_keycloak_initial_schema.py | 44 ++++++++ .../873ca2015033_users_refresh_token_table.py | 44 ++++++++ .../background_tasks/models.py | 4 - modules/file_storage/file_storage/models.py | 1 - modules/permissions/permissions/models.py | 2 +- scripts/_templates_py.py | 5 +- skills/simple-module-creating/SKILL.md | 2 +- skills/simple-module-database/SKILL.md | 18 +-- 24 files changed, 158 insertions(+), 277 deletions(-) delete mode 100644 framework/db/tests/test_postgres_schema_per_module.py create mode 100644 host/migrations/versions/168a2882f443_keycloak_initial_schema.py create mode 100644 host/migrations/versions/873ca2015033_users_refresh_token_table.py diff --git a/CLAUDE.md b/CLAUDE.md index e0b3524f..9a3fd180 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -54,9 +54,7 @@ modules/// **Middleware pipeline** (Starlette `add_middleware` is LIFO — last added runs first). Execution order on a request: `CorrelationId → RequestLogging → SecurityHeaders → Session → → Tenant (opt-in) → Locale → InertiaLayoutData → app`. When two modules add middleware at the same dependency tier, the module that sorts **later** wraps outermost. Use `depends_on` to express relative order — don't rely on names. -**Database**: per-module `Base` via `create_module_base("")`. Provider auto-detected from `SM_DATABASE_URL`: -- **Postgres** → one schema per module (`orders.`). -- **SQLite** → single schema; `__tablename__` must be prefixed with the module name (`orders_order`). +**Database**: per-module `Base` via `create_module_base("")`. Every module owns its own `MetaData` (so Alembic autogenerate can attribute tables to a module), but all tables live in the host's single schema. `__tablename__` must be prefixed with the module name to avoid collisions (`orders_order`). Postgres and SQLite share the same layout. Standard mixins in `simple_module_db.mixins`: `AuditMixin`, `SoftDeleteMixin` (bypass with `stmt.execution_options(include_deleted=True)`), `MultiTenantMixin`, `VersionedMixin`. The per-request session (`get_db`) auto-commits **only if** there are pending writes (via `after_flush` listener); otherwise rollback. Service code should **not** call `session.commit()` — flush if you need DB-assigned values. diff --git a/docs/database/models.md b/docs/database/models.md index cebd0708..8a71b24f 100644 --- a/docs/database/models.md +++ b/docs/database/models.md @@ -23,10 +23,7 @@ class Order(Base, AuditMixin, table=True): ### Table naming -- **Postgres.** `create_module_base("orders")` gives the class a per-module schema. The `__tablename__` can be just `order`, and the fully-qualified name is `orders.order`. The prefix `orders_` is redundant but harmless. -- **SQLite.** One schema, so `__tablename__` must be prefixed with the module name to avoid collisions: `orders_order`. - -**Convention: always prefix the table name with the module name.** This makes migrations and DB dumps readable in both providers and avoids a "works on my machine" footgun when swapping between them. +All modules share the host's single schema, so `__tablename__` must be prefixed with the module name to avoid collisions: `orders_order`, `users_user`, etc. `create_module_base` does not enforce the prefix — it's a convention the framework relies on. ### Primary keys @@ -50,8 +47,7 @@ class OrderLine(Base, table=True): __tablename__ = "orders_order_line" id: int | None = Field(default=None, primary_key=True) - order_id: int = Field(foreign_key="orders.order.id") # Postgres - # order_id: int = Field(foreign_key="orders_order.id") # SQLite + order_id: int = Field(foreign_key="orders_order.id") quantity: int order: "Order" = Relationship(back_populates="lines") @@ -165,7 +161,7 @@ Do not re-enable these rules in module-local configs. Real bugs caused by wrong ## Next steps -- [Per-module Base](/database/per-module-base) — how provider detection and schema isolation works. +- [Per-module Base](/database/per-module-base) — how `create_module_base` and `build_module_metadata` work. - [Mixins](/database/mixins) — `AuditMixin`, `SoftDeleteMixin`, `MultiTenantMixin`, `VersionedMixin`. - [Session lifecycle](/database/sessions) — the `get_db` dependency and why you don't call `commit()`. - [Migrations](/database/migrations) — Alembic autogenerate and per-module branch labels. diff --git a/docs/database/per-module-base.md b/docs/database/per-module-base.md index 6bb9b652..87dfae2c 100644 --- a/docs/database/per-module-base.md +++ b/docs/database/per-module-base.md @@ -13,25 +13,13 @@ class Order(Base, table=True): ... ``` -`create_module_base` returns a SQLModel base that's bound to a private `MetaData` object. This isolation is what lets modules live side-by-side without their tables trampling each other's Alembic autogenerate output. +`create_module_base` returns a SQLModel base bound to a private `MetaData` object. This isolation is what lets Alembic autogenerate attribute each table to a specific module and what makes `build_module_metadata()` able to assemble the combined target metadata. -## Provider auto-detection +## Single shared schema -The function inspects `SM_DATABASE_URL` at import time and picks the right strategy: +All modules — whether on Postgres or SQLite — live in the host's single schema. There is no per-module schema policy. `__tablename__` must be prefixed with the module name to avoid collisions (`orders_order`, `users_user`). `create_module_base` doesn't enforce the prefix; it's a convention the framework relies on. -- **PostgreSQL** → the Base sets `__table_args__ = {"schema": ""}`. Tables live at `orders.
`. Each module effectively owns a namespace; one `DROP SCHEMA orders CASCADE` can cleanly uninstall a module. -- **SQLite** → no schema (SQLite has one). The `__tablename__` must still be prefixed with the module name to avoid collisions. `create_module_base` does not enforce the prefix — it's a convention. - -You can override detection explicitly for tests or special cases: - -```python -from simple_module_db.base import create_module_base -from simple_module_db.types import DatabaseProvider - -Base = create_module_base("orders", provider=DatabaseProvider.SQLITE) -``` - -Production code should let auto-detection do its thing. +The same migrations apply to Postgres and SQLite. There is no provider branching in model metadata. ## The `build_module_metadata()` function @@ -59,9 +47,9 @@ If you write a one-off host-level table that autogenerate shouldn't track, exten ## Naming rules -- Module name must match `ModuleMeta.name.lower()`. The framework caches the Base by module name; a mismatch causes silent schema drift. +- Module name must match `ModuleMeta.name.lower()`. The framework caches the Base by module name; a mismatch causes silent metadata drift. - Module names should be identifiers: `[a-z][a-z0-9_]*`. Hyphens break SQL identifier parsing on some providers. -- Don't rename a module after it ships without migrating data. Both the schema name (Postgres) and the table prefix (SQLite) are durable. +- Don't rename a module after it ships without migrating data. The table prefix is durable across deployments. ## Tables across modules @@ -74,13 +62,12 @@ from orders.models import Order class Invoice(Base, table=True): __tablename__ = "invoices_invoice" id: int | None = Field(default=None, primary_key=True) - order_id: int = Field(foreign_key="orders.order.id") + order_id: int = Field(foreign_key="orders_order.id") ``` Caveats: - Add `depends_on=["Orders"]` in `InvoicesModule.meta` — modules are loaded in topological order; without `depends_on`, `orders.models` might not be imported when `invoices.models` runs. -- Autogenerate handles cross-schema FKs on Postgres natively. - Uninstalling `Orders` while `Invoices` still references it produces a DB error — cross-module FKs are a commitment. If you can avoid a hard FK (store `order_id: int` without the constraint), module lifecycles stay more independent. Prefer application-level validation for loose coupling. @@ -94,7 +81,7 @@ from simple_module_db.base import build_module_metadata meta = build_module_metadata() for t in meta.sorted_tables: - print(t.schema or "(no schema)", t.name) + print(t.name) ``` This is also what the boot-time `SM011` check uses — it compares this set against the Alembic history to detect tables that exist in code but not in any migration. diff --git a/docs/framework-conventions.md b/docs/framework-conventions.md index 42a517f8..58c2ec25 100644 --- a/docs/framework-conventions.md +++ b/docs/framework-conventions.md @@ -37,7 +37,7 @@ class OrdersModule(ModuleBase): ) ``` -- `name` is unique across the app — it's the schema name for PostgreSQL, the SQLite table prefix, the Inertia component namespace, and the diagnostic reporter name. +- `name` is unique across the app — it's the table-name prefix, the Inertia component namespace, and the diagnostic reporter name. - `depends_on` expresses a hard ordering requirement; the framework topo-sorts modules and invokes lifecycle hooks in that order. - Missing or invalid `meta` fails the boot in production (strict discovery) and logs a `SM001` warning in development. @@ -159,13 +159,10 @@ class OrderCreate(SQLModel): ```python from simple_module_db.base import create_module_base -Base = create_module_base("orders") # provider auto-detected from SM_DATABASE_URL +Base = create_module_base("orders") ``` -- **PostgreSQL**: the module gets its own schema (`orders`). Tables live at `orders.
`. -- **SQLite**: single schema. Prefix `__tablename__` with the module name to avoid collisions (`orders_order`). - -You can pin the provider for tests (`provider=DatabaseProvider.SQLITE`), but code that ships should let auto-detection handle it. +`create_module_base` returns a SQLModel base bound to its own `MetaData`, but all modules share the host's single schema (same layout on Postgres and SQLite). Prefix `__tablename__` with the module name to avoid collisions (`orders_order`). ### Mixins diff --git a/docs/guide/first-module.md b/docs/guide/first-module.md index 61c02216..f06a19da 100644 --- a/docs/guide/first-module.md +++ b/docs/guide/first-module.md @@ -36,7 +36,7 @@ class Order(Base, AuditMixin, SoftDeleteMixin, table=True): - `AuditMixin` adds `created_at`, `updated_at`, `created_by`, `updated_by` — populated automatically from the request user. - `SoftDeleteMixin` replaces `DELETE` with `is_deleted=true`. `SELECT` filters them out by default; pass `include_deleted=True` to bypass. -- `__tablename__` must be prefixed with the module name under SQLite. On Postgres, the per-module Base puts tables in an `orders` schema automatically, so the prefix is redundant but harmless. +- `__tablename__` must be prefixed with the module name (`orders_order`) so it doesn't collide with other modules' tables — every module shares the host's single schema on both Postgres and SQLite. ## 3. Update the DTOs @@ -75,7 +75,7 @@ uv run alembic revision --autogenerate -m "add orders tables" Open `migrations/versions/XXXX_add_orders_tables.py` and eyeball it: -- It should create the `orders` schema (Postgres) or the `orders_order` table (SQLite). +- It should create the `orders_order` table. - Add `branch_labels = ("orders",)` to the revision so you can later `alembic downgrade orders@base` to roll the module back to empty without touching other modules. Apply: diff --git a/docs/guide/quickstart.md b/docs/guide/quickstart.md index 04eb12bb..9deae01f 100644 --- a/docs/guide/quickstart.md +++ b/docs/guide/quickstart.md @@ -76,7 +76,7 @@ uv run alembic revision --autogenerate -m "add orders tables" make migrate ``` -Alembic's autogenerate picks up the new `orders` schema (Postgres) or the `orders_*` tables (SQLite) and writes `migrations/versions/XXXX_add_orders_tables.py`. Add `branch_labels = ("orders",)` to that revision so you can later `alembic downgrade orders@base` to roll the module back in isolation. +Alembic's autogenerate picks up the new `orders_*` tables and writes `migrations/versions/XXXX_add_orders_tables.py`. Add `branch_labels = ("orders",)` to that revision so you can later `alembic downgrade orders@base` to roll the module back in isolation. ## 7. Hit the module @@ -107,7 +107,7 @@ uv run pytest modules/orders/tests/test_orders.py -v - **Routes** — `register_routes(api_router, view_router)` attached the `orders` routers at `/api/orders` and `/orders`. - **Menu** — `register_menu_items` pushed an entry onto `MenuRegistry`; the Inertia shared-props middleware serialized it into `menus.sidebar` for every authenticated request. - **Frontend** — `modules.generated.ts` (rebuilt by `make gen-pages`) maps `"Orders/Browse"` to `modules/orders/orders/pages/Browse.tsx`. Vite resolves and HMR-watches that file. -- **Database** — `create_module_base("orders")` namespaced the `Order` table under a Postgres `orders` schema (or the `orders_order` table name under SQLite). +- **Database** — `create_module_base("orders")` gave the module its own SQLModel `MetaData` so Alembic can attribute the `orders_order` table to it. ## Next steps diff --git a/framework/db/README.md b/framework/db/README.md index 606e7d21..c1ea174d 100644 --- a/framework/db/README.md +++ b/framework/db/README.md @@ -10,7 +10,7 @@ pip install simple_module_db ## What it provides -- `create_module_base("")` — a module-scoped declarative `Base`. PostgreSQL maps it to its own schema; SQLite namespaces via table-name prefix. +- `create_module_base("")` — a module-scoped declarative `Base` with its own `MetaData`. All modules share the host's single schema, so `__tablename__` should be prefixed with the module name (e.g. `users_user`) to avoid collisions. - Per-request async session (`get_db`) with an auto-commit-on-flush hook — `after_flush` commits if there are pending writes, rolls back otherwise. - Mixins in `simple_module_db.mixins`: `AuditMixin` (created_at/updated_at), `SoftDeleteMixin` (auto-filtered unless `stmt.execution_options(include_deleted=True)`), `MultiTenantMixin`, `VersionedMixin`. - `DatabaseState` container used by the framework to avoid global mutable state. diff --git a/framework/db/pyproject.toml b/framework/db/pyproject.toml index 60a85886..f42c5a8d 100644 --- a/framework/db/pyproject.toml +++ b/framework/db/pyproject.toml @@ -24,6 +24,10 @@ dependencies = [ "aiosqlite>=0.20", "alembic>=1.14", "asyncpg>=0.30", + # psycopg2 is the *sync* Postgres driver Alembic uses for migrations. + # The async app code goes through asyncpg; migration env.py rewrites the + # URL to +psycopg2 because Alembic's online migrations are sync-only. + "psycopg2-binary>=2.9", "simple_module_core==0.0.17", "sqlalchemy[asyncio]>=2.0", "sqlmodel>=0.0.22", diff --git a/framework/db/simple_module_db/__init__.py b/framework/db/simple_module_db/__init__.py index f209110b..7890558c 100644 --- a/framework/db/simple_module_db/__init__.py +++ b/framework/db/simple_module_db/__init__.py @@ -1,4 +1,4 @@ -"""SimpleModule DB - SQLAlchemy async support with per-module schema isolation.""" +"""SimpleModule DB — async SQLAlchemy/SQLModel runtime shared by every module.""" from simple_module_db.audit import AuditRecord from simple_module_db.base import create_module_base diff --git a/framework/db/simple_module_db/base.py b/framework/db/simple_module_db/base.py index cd53ea56..e825e9d9 100644 --- a/framework/db/simple_module_db/base.py +++ b/framework/db/simple_module_db/base.py @@ -1,15 +1,16 @@ -"""Per-module SQLModel base with schema isolation.""" +"""Per-module SQLModel base. -from __future__ import annotations +Every module owns its own :class:`sqlalchemy.MetaData` so Alembic autogenerate +can attribute tables to a module, but all tables live in the host's single +``public`` schema. Modules prefix ``__tablename__`` with the module name to +avoid collisions (e.g. ``users_user``). +""" -import os +from __future__ import annotations -from simple_module_core.dotenv import env_bool from sqlalchemy import MetaData from sqlmodel import SQLModel -from simple_module_db.provider import DatabaseProvider, detect_provider - # Convention-based naming for constraints (helps Alembic) _naming_convention = { "ix": "ix_%(column_0_label)s", @@ -19,7 +20,6 @@ "pk": "pk_%(table_name)s", } -# Cache created bases to avoid recreating for the same module _base_cache: dict[str, type[SQLModel]] = {} # Track all module bases for Alembic discovery. Module-level *mutable* list @@ -30,77 +30,27 @@ def _register_base(base: type[SQLModel]) -> None: - """Append ``base`` to ``all_module_bases`` iff not already present. - - Guards against the list growing under repeated imports (test suites, - reloaders, plugin discovery) without changing the public type. - """ + """Append ``base`` to ``all_module_bases`` iff not already present.""" if base not in all_module_bases: all_module_bases.append(base) -def _default_schema_policy() -> DatabaseProvider: - """Resolve the schema layout to register module tables under. +def create_module_base(module_name: str) -> type[SQLModel]: + """Create a SQLModel abstract base with its own ``MetaData`` for a module. - The :class:`DatabaseProvider` enum doubles as a *schema-layout* - selector here — ``POSTGRESQL`` means "give every module its own - schema (``orders.
``)", ``SQLITE`` means "shared public schema, - name-prefixed tables (``orders_
``)". The conflation is - deliberate so existing call sites keep working, but conceptually this - is "schema policy", not "what DB are we connecting to": you can run - Postgres with ``SM_SCHEMA_PER_MODULE=false`` to keep a flat layout. + All modules share the host's single schema. Concrete tables should prefix + ``__tablename__`` with ``module_name`` (e.g. ``users_user``) so names don't + collide. The per-module ``MetaData`` is what lets Alembic autogenerate + attribute each table to its module and what makes ``build_module_metadata`` + able to assemble the combined target metadata. - Resolution order: - 1. ``SM_SCHEMA_PER_MODULE`` (authoritative when set, decoupled from URL). - 2. ``SM_DATABASE_URL`` (legacy fallback so deployments that haven't - migrated to the explicit knob keep working). - 3. ``SQLITE`` (shared schema, the safe default). - """ - explicit = os.environ.get("SM_SCHEMA_PER_MODULE") - if explicit is not None: - return ( - DatabaseProvider.POSTGRESQL - if env_bool("SM_SCHEMA_PER_MODULE") - else DatabaseProvider.SQLITE - ) - - url = os.environ.get("SM_DATABASE_URL", "") - if url: - return detect_provider(url) - return DatabaseProvider.SQLITE - - -def create_module_base( - module_name: str, - provider: DatabaseProvider | None = None, -) -> type[SQLModel]: - """Create a SQLModel abstract base with schema isolation for a module. - - - PostgreSQL: uses a dedicated schema (e.g., ``products``) - - SQLite: single schema; modules are expected to prefix ``__tablename__`` - with the module name to avoid collisions (e.g., ``products_product``) - - The provider defaults to whatever ``SM_DATABASE_URL`` indicates, so - module models work in both dev (SQLite) and prod (PostgreSQL) without - code changes. Pass ``provider=`` explicitly in tests that need to pin it. - - Returns a cached base if already created for this module+provider. The - returned class is a ``SQLModel`` subclass with a per-module ``MetaData``; - concrete table classes declare ``table=True`` and inherit from it. + Returns a cached base on repeat calls for the same module name. """ - if provider is None: - provider = _default_schema_policy() - - cache_key = f"{module_name}:{provider}" - if cache_key in _base_cache: - return _base_cache[cache_key] - - schema_name = module_name.lower() + module_name = module_name.lower() + if module_name in _base_cache: + return _base_cache[module_name] - if provider == DatabaseProvider.POSTGRESQL: - mod_metadata = MetaData(schema=schema_name, naming_convention=_naming_convention) - else: - mod_metadata = MetaData(naming_convention=_naming_convention) + mod_metadata = MetaData(naming_convention=_naming_convention) # Use type() to create the class, avoiding class body scoping issues ModuleBase = type( # noqa: N806 @@ -112,9 +62,8 @@ def create_module_base( }, ) - # Store module name for reference - ModuleBase.__module_name__ = schema_name # type: ignore[attr-defined] + ModuleBase.__module_name__ = module_name # type: ignore[attr-defined] - _base_cache[cache_key] = ModuleBase + _base_cache[module_name] = ModuleBase _register_base(ModuleBase) return ModuleBase diff --git a/framework/db/simple_module_db/migrations.py b/framework/db/simple_module_db/migrations.py index e32c0feb..02879bb4 100644 --- a/framework/db/simple_module_db/migrations.py +++ b/framework/db/simple_module_db/migrations.py @@ -1,4 +1,4 @@ -"""Helpers for Alembic integration with module-based schemas. +"""Helpers for Alembic integration with module-based metadata. A host's ``migrations/env.py`` should call :func:`build_module_metadata` to obtain the combined ``target_metadata`` for autogenerate, and diff --git a/framework/db/tests/_audit_models.py b/framework/db/tests/_audit_models.py index 2106851a..0a7345a3 100644 --- a/framework/db/tests/_audit_models.py +++ b/framework/db/tests/_audit_models.py @@ -10,10 +10,9 @@ from simple_module_db.base import create_module_base from simple_module_db.mixins import AuditMixin, SoftDeleteMixin -from simple_module_db.provider import DatabaseProvider from sqlmodel import Field -AuditBase = create_module_base("test_audit", provider=DatabaseProvider.SQLITE) +AuditBase = create_module_base("test_audit") class AuditTestItem(AuditBase, AuditMixin, table=True): # type: ignore[call-arg] # ty: ignore[unsupported-base] diff --git a/framework/db/tests/_models.py b/framework/db/tests/_models.py index 7cbb228f..d153884b 100644 --- a/framework/db/tests/_models.py +++ b/framework/db/tests/_models.py @@ -10,10 +10,9 @@ from simple_module_db.base import create_module_base from simple_module_db.mixins import MultiTenantMixin, SoftDeleteMixin -from simple_module_db.provider import DatabaseProvider from sqlmodel import Field -_TenantBase = create_module_base("mt_test", provider=DatabaseProvider.SQLITE) +_TenantBase = create_module_base("mt_test") class _TenantItem(_TenantBase, MultiTenantMixin, table=True): # ty: ignore[unsupported-base] diff --git a/framework/db/tests/test_base.py b/framework/db/tests/test_base.py index 403476e7..f375a296 100644 --- a/framework/db/tests/test_base.py +++ b/framework/db/tests/test_base.py @@ -9,46 +9,35 @@ class TestCreateModuleBase: async def test_returns_declarative_base(self): - base = create_module_base("test_mod_base", provider=DatabaseProvider.SQLITE) + base = create_module_base("test_mod_base") assert hasattr(base, "metadata") assert base.__abstract__ is True - async def test_caching_same_args(self): - base1 = create_module_base("cache_test", provider=DatabaseProvider.SQLITE) - base2 = create_module_base("cache_test", provider=DatabaseProvider.SQLITE) - assert base1 is base2 - - async def test_different_providers_different_bases(self): - base_sqlite = create_module_base("multi_prov", provider=DatabaseProvider.SQLITE) - base_pg = create_module_base("multi_prov", provider=DatabaseProvider.POSTGRESQL) - assert base_sqlite is not base_pg - - async def test_postgresql_uses_schema(self): - base = create_module_base("schemamod", provider=DatabaseProvider.POSTGRESQL) - assert base.metadata.schema == "schemamod" - - async def test_sqlite_no_schema(self): - base = create_module_base("noschemod", provider=DatabaseProvider.SQLITE) + async def test_metadata_has_no_schema(self): + base = create_module_base("noschemod") assert base.metadata.schema is None + async def test_caching_same_name(self): + base1 = create_module_base("cache_test") + base2 = create_module_base("cache_test") + assert base1 is base2 + async def test_module_name_stored(self): - base = create_module_base("named_mod", provider=DatabaseProvider.SQLITE) + base = create_module_base("named_mod") assert base.__module_name__ == "named_mod" # type: ignore[attr-defined] async def test_all_module_bases_is_deduped(self): """Re-creating the same module must not grow ``all_module_bases``.""" from simple_module_db import base as base_mod - create_module_base("dedupe_test", provider=DatabaseProvider.SQLITE) + create_module_base("dedupe_test") before = len(base_mod.all_module_bases) - create_module_base("dedupe_test", provider=DatabaseProvider.SQLITE) + create_module_base("dedupe_test") after = len(base_mod.all_module_bases) assert after == before - assert create_module_base("dedupe_test", provider=DatabaseProvider.SQLITE) in ( - base_mod.all_module_bases - ) + assert create_module_base("dedupe_test") in base_mod.all_module_bases class TestDetectProvider: diff --git a/framework/db/tests/test_mixins.py b/framework/db/tests/test_mixins.py index 7ed290f1..773743e7 100644 --- a/framework/db/tests/test_mixins.py +++ b/framework/db/tests/test_mixins.py @@ -27,12 +27,11 @@ SoftDeleteMixin, VersionedMixin, ) -from simple_module_db.provider import DatabaseProvider from simple_module_db.session import init_db from sqlalchemy.ext.asyncio import AsyncSession from sqlmodel import Field, select -_MixinsBase = create_module_base("mixins_test", provider=DatabaseProvider.SQLITE) +_MixinsBase = create_module_base("mixins_test") class _AuditRow(_MixinsBase, AuditMixin, table=True): # type: ignore[call-arg] # ty: ignore[unsupported-base] diff --git a/framework/db/tests/test_postgres_schema_per_module.py b/framework/db/tests/test_postgres_schema_per_module.py deleted file mode 100644 index 5ad6eb32..00000000 --- a/framework/db/tests/test_postgres_schema_per_module.py +++ /dev/null @@ -1,106 +0,0 @@ -"""End-to-end Postgres schema-per-module test (skipped without a PG URL). - -CLAUDE.md promises that on Postgres each module's tables live in their own -schema (``orders.
``). All of the existing DB test suites run against -SQLite where the convention is to prefix the table name instead, so the -Postgres branch of ``create_module_base`` is exercised only by the -diagnostics tests indirectly. - -This test runs only if ``SM_POSTGRES_TEST_URL`` is set, so CI on machines -without a Postgres available is a clean skip rather than a failure. -""" - -from __future__ import annotations - -import os -import uuid -from typing import TYPE_CHECKING - -import pytest -from simple_module_db.base import create_module_base -from simple_module_db.provider import DatabaseProvider -from simple_module_db.session import init_db -from sqlalchemy import text -from sqlmodel import Field - -if TYPE_CHECKING: - pass - - -_PG_URL = os.environ.get("SM_POSTGRES_TEST_URL") -_PG_SKIP_REASON = "Set SM_POSTGRES_TEST_URL=postgresql+asyncpg://... to run Postgres tests" - - -@pytest.mark.anyio -@pytest.mark.skipif(not _PG_URL, reason=_PG_SKIP_REASON) -async def test_module_tables_isolated_per_schema(): - """Two modules' tables must live in separate schemas with the same suffix. - - Without per-schema isolation, ``orders.product`` and ``billing.product`` - would collide. We create two ad-hoc module bases with the same suffix, - insert rows in each, and confirm the rows are not visible cross-schema. - """ - schema_a = f"sm_test_a_{uuid.uuid4().hex[:8]}" - schema_b = f"sm_test_b_{uuid.uuid4().hex[:8]}" - - base_a = create_module_base(schema_a, provider=DatabaseProvider.POSTGRESQL) - base_b = create_module_base(schema_b, provider=DatabaseProvider.POSTGRESQL) - - class _ProductA(base_a, table=True): # type: ignore[call-arg,misc] # ty: ignore[unsupported-base] - __tablename__ = "product" - id: int | None = Field(default=None, primary_key=True) - name: str = Field(max_length=100) - - class _ProductB(base_b, table=True): # type: ignore[call-arg,misc] # ty: ignore[unsupported-base] - __tablename__ = "product" - id: int | None = Field(default=None, primary_key=True) - name: str = Field(max_length=100) - - db_state = init_db(_PG_URL) # type: ignore[arg-type] - try: - async with db_state.engine.begin() as conn: - await conn.execute(text(f'CREATE SCHEMA IF NOT EXISTS "{schema_a}"')) - await conn.execute(text(f'CREATE SCHEMA IF NOT EXISTS "{schema_b}"')) - await conn.run_sync(base_a.metadata.create_all) - await conn.run_sync(base_b.metadata.create_all) - - async with db_state.session_factory() as session: - session.add(_ProductA(name="a-thing")) - session.add(_ProductB(name="b-thing")) - await session.commit() - - # Raw SQL to bypass ORM tenant filters and confirm schema isolation. - in_a = ( - (await session.execute(text(f'SELECT name FROM "{schema_a}"."product"'))) - .scalars() - .all() - ) - in_b = ( - (await session.execute(text(f'SELECT name FROM "{schema_b}"."product"'))) - .scalars() - .all() - ) - assert in_a == ["a-thing"] - assert in_b == ["b-thing"] - - # Teardown - async with db_state.engine.begin() as conn: - await conn.execute(text(f'DROP SCHEMA "{schema_a}" CASCADE')) - await conn.execute(text(f'DROP SCHEMA "{schema_b}" CASCADE')) - finally: - await db_state.engine.dispose() - - -def test_module_metadata_has_schema_set(): - """Static check: a base created with provider=POSTGRESQL stamps a schema. - - Runs without a live Postgres so we still get coverage of the metadata - branch on every test run. - """ - base = create_module_base("isolated_smoke", provider=DatabaseProvider.POSTGRESQL) - assert base.metadata.schema == "isolated_smoke" - - -def test_module_metadata_has_no_schema_for_sqlite(): - base = create_module_base("flat_smoke", provider=DatabaseProvider.SQLITE) - assert base.metadata.schema is None diff --git a/host/migrations/versions/168a2882f443_keycloak_initial_schema.py b/host/migrations/versions/168a2882f443_keycloak_initial_schema.py new file mode 100644 index 00000000..6bfedff4 --- /dev/null +++ b/host/migrations/versions/168a2882f443_keycloak_initial_schema.py @@ -0,0 +1,44 @@ +"""keycloak initial schema + +Revision ID: 168a2882f443 +Revises: 873ca2015033 +Create Date: 2026-05-28 19:44:07.782353 +""" + +from collections.abc import Sequence + +import sqlalchemy as sa +from alembic import op + +# revision identifiers, used by Alembic. +revision: str = "168a2882f443" +down_revision: str | None = "873ca2015033" +branch_labels: str | Sequence[str] | None = ("keycloak",) +depends_on: str | Sequence[str] | None = None + + +def upgrade() -> None: + # ### commands auto generated by Alembic - please adjust! ### + op.create_table( + "keycloak_user_cache", + sa.Column("id", sa.Uuid(), nullable=False), + sa.Column("keycloak_sub", sa.String(), nullable=False), + sa.Column("email", sa.String(), nullable=False), + sa.Column("full_name", sa.String(), nullable=True), + sa.Column("last_login_at", sa.DateTime(), nullable=True), + sa.PrimaryKeyConstraint("id", name=op.f("pk_keycloak_user_cache")), + ) + op.create_index( + op.f("ix_keycloak_user_cache_keycloak_sub"), + "keycloak_user_cache", + ["keycloak_sub"], + unique=True, + ) + # ### end Alembic commands ### + + +def downgrade() -> None: + # ### commands auto generated by Alembic - please adjust! ### + op.drop_index(op.f("ix_keycloak_user_cache_keycloak_sub"), table_name="keycloak_user_cache") + op.drop_table("keycloak_user_cache") + # ### end Alembic commands ### diff --git a/host/migrations/versions/873ca2015033_users_refresh_token_table.py b/host/migrations/versions/873ca2015033_users_refresh_token_table.py new file mode 100644 index 00000000..ac9bc132 --- /dev/null +++ b/host/migrations/versions/873ca2015033_users_refresh_token_table.py @@ -0,0 +1,44 @@ +"""users refresh_token table + +Revision ID: 873ca2015033 +Revises: 41cf2c53660e +Create Date: 2026-05-28 19:43:48.671727 +""" + +from collections.abc import Sequence + +import sqlalchemy as sa +from alembic import op + +# revision identifiers, used by Alembic. +revision: str = "873ca2015033" +down_revision: str | None = "41cf2c53660e" +branch_labels: str | Sequence[str] | None = None +depends_on: str | Sequence[str] | None = None + + +def upgrade() -> None: + # ### commands auto generated by Alembic - please adjust! ### + op.create_table( + "users_refresh_token", + sa.Column("token", sa.Uuid(), nullable=False), + sa.Column("user_id", sa.Uuid(), nullable=False), + sa.Column("created_at", sa.DateTime(), nullable=False), + sa.Column("expires_at", sa.DateTime(), nullable=False), + sa.Column("revoked_at", sa.DateTime(), nullable=True), + sa.ForeignKeyConstraint( + ["user_id"], ["users_user.id"], name=op.f("fk_users_refresh_token_user_id_users_user") + ), + sa.PrimaryKeyConstraint("token", name=op.f("pk_users_refresh_token")), + ) + op.create_index( + op.f("ix_users_refresh_token_user_id"), "users_refresh_token", ["user_id"], unique=False + ) + # ### end Alembic commands ### + + +def downgrade() -> None: + # ### commands auto generated by Alembic - please adjust! ### + op.drop_index(op.f("ix_users_refresh_token_user_id"), table_name="users_refresh_token") + op.drop_table("users_refresh_token") + # ### end Alembic commands ### diff --git a/modules/background_tasks/background_tasks/models.py b/modules/background_tasks/background_tasks/models.py index 259a68fb..295fa1ce 100644 --- a/modules/background_tasks/background_tasks/models.py +++ b/modules/background_tasks/background_tasks/models.py @@ -18,10 +18,6 @@ TaskStatus, ) -# Provider is auto-detected from SM_DATABASE_URL (falls back to SQLite). -# On PostgreSQL this gives the module its own `background_tasks` schema; on -# SQLite all modules share one schema, so __tablename__ is prefixed for -# isolation. Base = create_module_base(MODULE_NAME) diff --git a/modules/file_storage/file_storage/models.py b/modules/file_storage/file_storage/models.py index 4e15eaeb..543a81fd 100644 --- a/modules/file_storage/file_storage/models.py +++ b/modules/file_storage/file_storage/models.py @@ -12,7 +12,6 @@ from file_storage import constants -# PostgreSQL → ``file_storage`` schema; SQLite → table name is prefixed below. Base = create_module_base(constants.MODULE_NAME) diff --git a/modules/permissions/permissions/models.py b/modules/permissions/permissions/models.py index 864ebe01..105303c8 100644 --- a/modules/permissions/permissions/models.py +++ b/modules/permissions/permissions/models.py @@ -11,7 +11,7 @@ top of (or independent of) any role they happen to hold. Both junction rows are keyed on plain strings rather than FKs into the -``users`` schema — per-module :class:`MetaData` cannot express a +``users`` tables — per-module :class:`MetaData` cannot express a cross-module foreign key, and this keeps the permissions module independent of ``users``' table layout. """ diff --git a/scripts/_templates_py.py b/scripts/_templates_py.py index e3b5748d..af1ba6dd 100644 --- a/scripts/_templates_py.py +++ b/scripts/_templates_py.py @@ -193,9 +193,8 @@ def models_py(ctx: ScaffoldContext) -> str: from simple_module_db.mixins import AuditMixin from sqlmodel import Field - # Provider is auto-detected from SM_DATABASE_URL (falls back to SQLite). - # On PostgreSQL this gives the module its own `{ctx.name}` schema; on SQLite - # all modules share one schema, so __tablename__ is prefixed for isolation. + # All modules share the host's single schema, so __tablename__ is + # prefixed with the module name to avoid collisions. Base = create_module_base("{ctx.name}") diff --git a/skills/simple-module-creating/SKILL.md b/skills/simple-module-creating/SKILL.md index b1ee30ab..99a3d62f 100644 --- a/skills/simple-module-creating/SKILL.md +++ b/skills/simple-module-creating/SKILL.md @@ -68,7 +68,7 @@ class OrdersModule(ModuleBase): ) ``` -`ModuleMeta.name` is load-bearing in three places: the Postgres schema name, the SQLite `__tablename__` prefix you author, and the PascalCase Inertia component namespace. So directory `blog_posts` → `name="BlogPosts"` → `inertia.render("BlogPosts/Index", ...)` → `pages/Index.tsx`. Mismatches fire diagnostic codes `SM003` (orphan page) / `SM004` (phantom render). +`ModuleMeta.name` is load-bearing in two places: the `__tablename__` prefix you author and the PascalCase Inertia component namespace. So directory `blog_posts` → `name="BlogPosts"` → `inertia.render("BlogPosts/Index", ...)` → `pages/Index.tsx`. Mismatches fire diagnostic codes `SM003` (orphan page) / `SM004` (phantom render). For modules you intend to publish, also add `version=` (your module's semver) and `requires_framework=` (a PEP 440 spec for the framework API range, e.g. `">=1.0,<2.0"`) so the host can reject incompatible installs at boot. diff --git a/skills/simple-module-database/SKILL.md b/skills/simple-module-database/SKILL.md index 6a473ce4..f7105c6a 100644 --- a/skills/simple-module-database/SKILL.md +++ b/skills/simple-module-database/SKILL.md @@ -18,26 +18,14 @@ from simple_module_db.mixins import AuditMixin Base = create_module_base("orders") class Order(Base, AuditMixin, table=True): - __tablename__ = "orders_order" # SQLite-safe prefix; required (see below) + __tablename__ = "orders_order" # module-name prefix; required id: int | None = Field(default=None, primary_key=True) name: str = Field(max_length=200) ``` -The provider is auto-detected from `SM_DATABASE_URL`. Pin it explicitly only in tests: +## Table naming -```python -from simple_module_db.base import create_module_base, DatabaseProvider -Base = create_module_base("orders", provider=DatabaseProvider.SQLITE) -``` - -## Postgres vs SQLite — the same code, different physical layout - -| Provider | Physical placement | Required convention | -|---|---|---| -| **PostgreSQL** | One **schema** per module (`orders.order`, `users.user`). Created automatically. | Set `__tablename__` to a name unique within the module. | -| **SQLite** | Single schema. Cross-module name collisions break things. | Prefix `__tablename__` with the module name (`orders_order`). | - -Always include the module-name prefix in `__tablename__`. It's redundant on Postgres (the schema already namespaces it) and **required** on SQLite. Code that runs in CI on SQLite and prod on Postgres needs both — pick the prefix. +All modules — on Postgres and SQLite alike — share the host's single schema. Cross-module name collisions break things, so `__tablename__` must be prefixed with the module name (e.g. `orders_order`, `users_user`). The framework does not enforce the prefix; it's a convention the migrations and diagnostics rely on. ## Mixins