diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 073f5036..0b1d60cb 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -159,6 +159,37 @@ jobs: - name: Module asset build guards run: uv run pytest framework/cli/tests/test_module_css_build.py + # Migrations are autogenerated on SQLite, which hides Postgres-only defects + # (integer defaults on boolean columns, undetected expression indexes, enum + # types surviving a downgrade). Only a real Postgres run catches them: apply + # everything, take it all back down, re-apply, then require zero drift. GH #342. + migrations-roundtrip-pg: + name: Migrations round trip (Postgres) + runs-on: ubuntu-latest + services: + postgres: + image: postgres:16-alpine + env: + POSTGRES_USER: postgres + POSTGRES_PASSWORD: postgres + POSTGRES_DB: sm_migrations_check + ports: ["5432:5432"] + options: >- + --health-cmd "pg_isready -U postgres" + --health-interval 5s + --health-timeout 3s + --health-retries 10 + env: + SM_MIGRATIONS_PG_URL: postgresql+asyncpg://postgres:postgres@localhost:5432/sm_migrations_check + steps: + - uses: actions/checkout@v7 + - uses: astral-sh/setup-uv@v9.0.0 + with: + enable-cache: true + cache-dependency-glob: ${{ env.UV_CACHE_GLOB }} + - run: make install-py + - run: make migrations-roundtrip-pg + e2e-smoke: name: E2E smoke (Playwright) runs-on: ubuntu-latest @@ -379,6 +410,7 @@ jobs: - js-typecheck - js-tests - js-build + - migrations-roundtrip-pg - e2e-smoke - perf-guards - file-size-check diff --git a/CLAUDE.md b/CLAUDE.md index de71a50a..faf5327b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -39,7 +39,8 @@ All day-to-day tasks go through `make`: | `make test-e2e` | Playwright smoke tests (requires `make dev` running + `uv run playwright install chromium`) | | `make lint` | Ruff format-check + Ruff + `ty` + Biome + per-workspace `tsc` + 300-line file cap | | `make doctor` | Module diagnostics (orphan pages, coupling violations, migration drift, locale checks) — same checks run at prod boot | -| `make migrate` / `make migration msg="..."` | Apply / autogenerate Alembic migrations | +| `make migrate` / `make migration msg="..."` | Apply / autogenerate Alembic migrations (autogenerated on SQLite? review per `docs/module-authoring.md` § Migrations) | +| `make migrations-roundtrip-pg` | Postgres portability gate: `upgrade heads` → `downgrade base` → `upgrade heads` → `alembic check` on `SM_MIGRATIONS_PG_URL` (a disposable DB; CI runs it) | | `make new-module name=` | Scaffold a new module package end-to-end | | `make gen-pages` | Regenerate `host/client_app/modules.{manifest.json,generated.ts,generated.css}` from installed modules | @@ -112,7 +113,7 @@ Standard mixins in `simple_module_db.mixins`: `AuditMixin`, `SoftDeleteMixin` (b ## Diagnostic codes -Meaningful codes when reading `make doctor` output: `SM001` missing meta (error), `SM003` orphan page / `SM004` phantom render (warn), `SM007` module overrides no hooks (info), `SM008` duplicate name (error), `SM009` framework→plugin import (error), `SM010` DB revision behind head (error), `SM011` module table not in migration history (warn), `SM012` `register_settings` overridden but nothing on `app.state.` (warn, fires at dev boot only), `SM013`–`SM016` locale issues, `SM017` module ships `.tsx` pages but is missing `package.json`/`tsconfig.json` (warn), `SM018` Inertia `router.{post,patch,put,delete}()` in a page targets a JSON `/api/*` endpoint (warn — Inertia rejects non-Inertia responses), `SM019` module registers view routes (non-empty `view_prefix` + overrides `register_routes`) but overrides neither `register_menu_items` nor `register_permissions` (warn — pages exist with no sidebar entry and no role-editor visibility; admins can't reach them through the UI). Modules whose views are sub-pages of another module typically register permissions to stay discoverable in the role editor without needing their own sidebar entry. `SM020` multiple auth provider modules installed (error), `SM021` no auth provider module installed (warn), `SM022` `@theme`/`@custom-variant`/`@utility` in a module's `styles.css`, where `layer(components)` makes them inert (warn), `SM023` an unlayered rule in a module's `theme.css`, which outranks every Tailwind utility (warn). `SM024` a unique key on a `MultiTenantMixin` table that omits `tenant_id` (warn). `SM025` `multi_tenant` is on but no module registered `app.state.tenant_resolver` (warn — checked at boot after module registration, in every environment, not by the `make doctor` CLI). In production, errors fail boot. +Meaningful codes when reading `make doctor` output: `SM001` missing meta (error), `SM003` orphan page / `SM004` phantom render (warn), `SM007` module overrides no hooks (info), `SM008` duplicate name (error), `SM009` framework→plugin import (error), `SM010` DB revision behind head (error), `SM011` module table not in migration history (warn), `SM012` `register_settings` overridden but nothing on `app.state.` (warn, fires at dev boot only), `SM013`–`SM016` locale issues, `SM017` module ships `.tsx` pages but is missing `package.json`/`tsconfig.json` (warn), `SM018` Inertia `router.{post,patch,put,delete}()` in a page targets a JSON `/api/*` endpoint (warn — Inertia rejects non-Inertia responses), `SM019` module registers view routes (non-empty `view_prefix` + overrides `register_routes`) but overrides neither `register_menu_items` nor `register_permissions` (warn — pages exist with no sidebar entry and no role-editor visibility; admins can't reach them through the UI). Modules whose views are sub-pages of another module typically register permissions to stay discoverable in the role editor without needing their own sidebar entry. `SM020` multiple auth provider modules installed (error), `SM021` no auth provider module installed (warn), `SM022` `@theme`/`@custom-variant`/`@utility` in a module's `styles.css`, where `layer(components)` makes them inert (warn), `SM023` an unlayered rule in a module's `theme.css`, which outranks every Tailwind utility (warn). `SM024` a unique key on a `MultiTenantMixin` table that omits `tenant_id` (warn). `SM025` `multi_tenant` is on but no module registered `app.state.tenant_resolver` (warn — checked at boot after module registration, in every environment, not by the `make doctor` CLI). `SM026` the database is SQLite and a model declares an expression index it cannot verify (info — autogenerate and `alembic check` skip those on SQLite; add them by hand and run `make migrations-roundtrip-pg`). In production, errors fail boot. ## Tests & fixtures diff --git a/Makefile b/Makefile index 2f1b4a00..22501c43 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,4 @@ -.PHONY: install install-py install-js dev dev-api dev-ui build test test-py test-py-pg test-js test-e2e bench memray-run memray-flamegraph loadtest loadtest-seed loadtest-memray bench-nav lint doctor migrate migration downgrade migration-history docker-up docker-down kill new-module gen-pages gen-i18n docker-build docker-app docker-compose-app sync-module-deps ci-python-lint ci-python-typecheck ci-js-lint ci-js-typecheck ci-check-file-size ci-check-hardcoded-strings ci-check-untranslated ci-build-packages worker beat worker-docker +.PHONY: install install-py install-js dev dev-api dev-ui build test test-py test-py-pg test-js test-e2e bench memray-run memray-flamegraph loadtest loadtest-seed loadtest-memray bench-nav lint doctor migrate migration downgrade migration-history migrations-roundtrip-pg docker-up docker-down kill new-module gen-pages gen-i18n docker-build docker-app docker-compose-app sync-module-deps ci-python-lint ci-python-typecheck ci-js-lint ci-js-typecheck ci-check-file-size ci-check-hardcoded-strings ci-check-untranslated ci-build-packages worker beat worker-docker # Install install: @@ -183,6 +183,18 @@ migration: ## Create new migration (usage: make migration msg=" downgrade: ## Downgrade one revision uv run --project host alembic -c host/alembic.ini downgrade -1 +# Portability gate for GH #342: revisions are autogenerated on SQLite, which hides +# Boolean-default, expression-index and enum-downgrade defects until Postgres runs +# them. Runs on SM_MIGRATIONS_PG_URL, a DISPOSABLE, already-created database +# (`createdb sm_migrations_check` on the shared dev Postgres); it ends empty. +SM_MIGRATIONS_PG_URL ?= postgresql+asyncpg://postgres:postgres@localhost:5432/sm_migrations_check +ALEMBIC_PG = SM_DATABASE_URL=$(SM_MIGRATIONS_PG_URL) uv run --project host alembic -c host/alembic.ini +migrations-roundtrip-pg: ## upgrade heads -> downgrade base -> upgrade heads -> alembic check, on Postgres + $(ALEMBIC_PG) upgrade heads + $(ALEMBIC_PG) downgrade base + $(ALEMBIC_PG) upgrade heads + $(ALEMBIC_PG) check + migration-history: ## Show migration history uv run --project host alembic -c host/alembic.ini history --verbose diff --git a/docs/module-authoring.md b/docs/module-authoring.md index 0972feec..3ac3dbcf 100644 --- a/docs/module-authoring.md +++ b/docs/module-authoring.md @@ -302,6 +302,59 @@ that allowlists only tables owned by installed modules. Any host-defined table (e.g. a user table the host dev added directly) is preserved untouched by autogenerate. +### Reviewing a revision autogenerated on SQLite + +`make migration` runs against whatever `SM_DATABASE_URL` points at, which is +SQLite by default. SQLite is forgiving, so a revision generated there can fail +on PostgreSQL while `alembic check` and the boot check report clean. Review each +generated file against this checklist before treating it as portable +([GH #342](https://github.com/antosubash/simple_module_python/issues/342)): + +1. **Boolean defaults.** A Boolean `server_default` must be `sa.false()` / + `sa.true()`, never `sa.text('0')` / `sa.text('1')` — PostgreSQL rejects an + integer default on a boolean column (`DatatypeMismatch`). The framework's + `render_item` and `process_revision_directives` hooks (already wired in the + scaffolded `env.py`) rewrite these for you; check any revision written + without them, and always write `server_default=sa.false()` on the model. +2. **Expression indexes.** SQLite cannot reflect `Index(..., text("lower(email)"))`, + so autogenerate skips it and `alembic check` cannot see it. Autogenerate on + SQLite logs a banner naming every expression index in your models, and + `make doctor` reports `SM026`. Add each one to a revision by hand + (`op.create_index(..., [sa.text("lower(email)")], if_not_exists=True)`) and + confirm with the PostgreSQL round trip below. The warning is deliberately not + an error: it fires on every autogenerate, including unrelated revisions, and + the Postgres round trip is the actual gate. +3. **Enum types on downgrade.** `op.drop_table` leaves the PostgreSQL + `CREATE TYPE` behind, so `downgrade` then `upgrade` fails with + `DuplicateObject`. Autogenerate never emits the drop. Finish `downgrade()` + with the helper, which is a no-op off PostgreSQL: + + ```python + from simple_module_db import drop_enums_if_postgres + + + def downgrade() -> None: + op.drop_table("orders_order") + drop_enums_if_postgres(op, "orders_status") + ``` + +4. **`SAEnum` server defaults use the member name.** For + `sa.Enum(Status)` the stored label is the member *name* + (`server_default="PUBLISH"`), not its value (`"publish"`). SQLite accepts + either; PostgreSQL rejects the value with `invalid input value for enum`. + (A `StrEnum` rendered with `values_callable`, which is what `render_item` + emits, stores the *value* — match the default to whichever the column uses.) + +Prove it on PostgreSQL before merging. Create a disposable database once +(`createdb sm_migrations_check` on the shared dev Postgres), then: + +```bash +make migrations-roundtrip-pg # upgrade heads, downgrade base, upgrade heads, alembic check +``` + +CI runs the same target on a Postgres service (`migrations-roundtrip-pg` job in +`pr.yml`), so a revision that only works on SQLite fails the PR. + ### Multi-module branches Each module's first revision should set a `branch_labels` tuple matching diff --git a/docs/reference/diagnostic-codes.md b/docs/reference/diagnostic-codes.md index 7d60ffe3..ff44a3cf 100644 --- a/docs/reference/diagnostic-codes.md +++ b/docs/reference/diagnostic-codes.md @@ -34,6 +34,7 @@ The framework runs a set of static checks over installed modules at app boot. Th | `SM023` | WARNING | A module's `theme.css` contains an unlayered plain rule (anything but an at-rule or a `:root`-style selector). Unlayered CSS outranks every Tailwind utility. | Move the rule to the module's `styles.css`, which is imported into `layer(components)` so utilities still win. | | `SM024` | WARNING | A unique column, constraint or index on a `MultiTenantMixin` table does not include `tenant_id`, so the first tenant to claim a value locks every other tenant out of it. | Make the key per tenant: add `tenant_id` to it (`Index(..., "tenant_id", "slug", unique=True)`). | | `SM025` | WARNING | `multi_tenant` is on but no module registered `app.state.tenant_resolver`, so only the principal's `tenant_id` claim can bind a tenant — with most auth providers every tenant-scoped query then fails closed. Checked at boot (it needs the built app), in every environment, not by `make doctor`. | Install the `tenants` module, or register your own `async (Request) -> str \| None` resolver on `app.state.tenant_resolver`. | +| `SM026` | INFO | The configured database is SQLite and a module model declares an expression index (e.g. `lower(email)`). SQLite cannot reflect those, so autogenerate and `alembic check` cannot verify them and a clean diff says nothing about them. | Add the index to a revision by hand and verify with `make migrations-roundtrip-pg` (PostgreSQL). See [Migrations](/module-authoring#reviewing-a-revision-autogenerated-on-sqlite). | `SM022`/`SM023` are the two halves of the same invariant: a module's optional [`theme.css` is imported unlayered and `styles.css` into `layer(components)`](/module-authoring#styling), and CSS put in the wrong one silently does nothing (or silently wins everything). diff --git a/framework/cli/simple_module_cli/templates/host/migrations/env.py b/framework/cli/simple_module_cli/templates/host/migrations/env.py index 4c5a19f8..2b7b0f46 100644 --- a/framework/cli/simple_module_cli/templates/host/migrations/env.py +++ b/framework/cli/simple_module_cli/templates/host/migrations/env.py @@ -31,8 +31,9 @@ target_metadata = build_module_metadata() include_object = make_include_object(target_metadata) -# Re-emit expression-based indexes (e.g. ``lower(email)``) that autogenerate -# silently drops under SQLite. See ``make_process_revision_directives`` docstring. +# Re-emit expression-based indexes that autogenerate drops under SQLite, rewrite +# Boolean 0/1 server defaults to sa.false()/sa.true(), and warn that SQLite cannot +# verify expression indexes (GH #342) — see make_process_revision_directives. process_revision_directives = make_process_revision_directives(target_metadata) diff --git a/framework/core/simple_module_core/__main__.py b/framework/core/simple_module_core/__main__.py index ffec6b6d..6ccbdd50 100644 --- a/framework/core/simple_module_core/__main__.py +++ b/framework/core/simple_module_core/__main__.py @@ -73,6 +73,16 @@ def _load_i18n_settings_from_env() -> tuple[list[str] | None, str]: return supported, default +def _database_dialect() -> str: + """Dialect name of ``SM_DATABASE_URL`` (``.env`` merged by the i18n loader first). + + Defaults to ``sqlite`` like ``Settings.database_url``. Reads the env var + directly for the same reason the i18n settings do: no hosting import. + """ + url = os.environ.get("SM_DATABASE_URL", "sqlite+aiosqlite:///./app.db") + return url.split(":", 1)[0].split("+", 1)[0] + + def _discover_extra_locale_sources() -> list[tuple[str, str, Path]]: """Return ``[(reporter, namespace, path), ...]`` for host + ui locale dirs.""" # Anchor on the same project root the `.env` was loaded from @@ -135,6 +145,7 @@ def main() -> int: i18n_supported_locales=supported, i18n_default_locale=default, i18n_extra_sources=extra, + database_dialect=_database_dialect(), ) print_diagnostics(diagnostics) diff --git a/framework/core/simple_module_core/diagnostics/_expression_index.py b/framework/core/simple_module_core/diagnostics/_expression_index.py new file mode 100644 index 00000000..dbee41b7 --- /dev/null +++ b/framework/core/simple_module_core/diagnostics/_expression_index.py @@ -0,0 +1,106 @@ +"""Expression-index diagnostics (SM026). + +SQLAlchemy cannot reflect expression indexes (``lower(email)``) on SQLite, so +autogenerate and ``alembic check`` silently skip them there and report a clean +diff. That is "unverified", not "clean": on Postgres the same index is either +missing from the history or drifted. SM026 says so whenever the configured +database is SQLite and any module model declares such an index. + +Duck-typed on SQLAlchemy ``Table``/``Index`` objects (core does not depend on +SQLAlchemy). +""" + +from __future__ import annotations + +import importlib +import logging +from collections.abc import Iterable +from typing import TYPE_CHECKING, Any + +from simple_module_core.diagnostics._types import Diagnostic, DiagnosticLevel + +if TYPE_CHECKING: + from simple_module_core.module import ModuleBase + +logger = logging.getLogger(__name__) + + +def index_is_expression_based(index: Any) -> bool: + """True when any element of ``index`` is an expression rather than a plain column. + + A plain ``Column`` is attached to its table; a ``text()`` or function + element is not. + """ + return any(getattr(expr, "table", None) is None for expr in index.expressions) + + +def find_expression_indexes(tables: Iterable[Any]) -> list[tuple[str, str]]: + """Return sorted ``(table name, index name)`` pairs for expression indexes.""" + found = { + (table.name, index.name or "") + for table in tables + for index in table.indexes + if index_is_expression_based(index) + } + return sorted(found) + + +def module_all_tables(mod: ModuleBase) -> list[Any]: + """Every table declared in the module's ``models`` submodule.""" + pkg = type(mod).__module__.rsplit(".", 1)[0] + try: + models = importlib.import_module(f"{pkg}.models") + except ModuleNotFoundError: + return [] + except Exception: # pragma: no cover - a broken models module fails elsewhere, loudly + logger.debug("Could not import %s.models for SM026", pkg, exc_info=True) + return [] + seen: dict[str, Any] = {} + for value in vars(models).values(): + table = getattr(value, "__table__", None) + if ( + isinstance(value, type) + and table is not None + and hasattr(table, "indexes") + # Skip models re-imported from another module, or each importer would + # be told about an index it does not own. + and getattr(value, "__module__", "").startswith(pkg) + ): + seen.setdefault(table.name, table) + return list(seen.values()) + + +def check_expression_indexes_unverifiable( + tables: Iterable[Any], dialect: str | None, module_name: str +) -> list[Diagnostic]: + """SM026: INFO on SQLite when the models declare indexes it cannot verify.""" + if dialect != "sqlite": + return [] + found = find_expression_indexes(tables) + if not found: + return [] + names = ", ".join(f"{idx} ({tbl})" for tbl, idx in found) + return [ + Diagnostic( + level=DiagnosticLevel.INFO, + code="SM026", + message=( + f"SQLite cannot reflect expression indexes, so autogenerate and " + f"`alembic check` cannot verify {names}; a clean diff here says nothing " + f"about them" + ), + module_name=module_name, + suggestion=( + "Add them to a revision by hand and verify on Postgres: " + "`make migrations-roundtrip-pg`" + ), + ) + ] + + +__all__ = [ + "check_expression_indexes_unverifiable", + "find_expression_indexes", + "index_is_expression_based", + "module_all_tables", +] diff --git a/framework/core/simple_module_core/diagnostics/_runner.py b/framework/core/simple_module_core/diagnostics/_runner.py index fb1d6f6d..93f020bd 100644 --- a/framework/core/simple_module_core/diagnostics/_runner.py +++ b/framework/core/simple_module_core/diagnostics/_runner.py @@ -7,6 +7,10 @@ from pathlib import Path from typing import TYPE_CHECKING +from simple_module_core.diagnostics._expression_index import ( + check_expression_indexes_unverifiable, + module_all_tables, +) from simple_module_core.diagnostics._migration import MigrationDiagnostics from simple_module_core.diagnostics._module import ModuleDiagnostics from simple_module_core.diagnostics._types import Diagnostic, DiagnosticLevel @@ -26,6 +30,7 @@ def run_diagnostics( i18n_supported_locales: list[str] | None = None, i18n_default_locale: str | None = None, i18n_extra_sources: list[tuple[str, str, Path]] | None = None, + database_dialect: str | None = None, ) -> list[Diagnostic]: """Convenience function to run all diagnostics. @@ -35,9 +40,19 @@ def run_diagnostics( whatever locale files are on disk instead of skipping them entirely (see :class:`I18nDiagnostics`). ``i18n_extra_sources`` lets callers include host/ui locale dirs that aren't owned by a ``ModuleBase``. + ``database_dialect`` (``"sqlite"``, ``"postgresql"``...) enables SM026, which + flags expression indexes a SQLite database cannot verify. """ diagnostics = ModuleDiagnostics().run(modules) + if database_dialect is not None: + for mod in modules: + diagnostics.extend( + check_expression_indexes_unverifiable( + module_all_tables(mod), database_dialect, mod.meta.name + ) + ) + if i18n_default_locale: from simple_module_core.diagnostics._i18n import I18nDiagnostics diff --git a/framework/core/tests/test_expression_index_diagnostics.py b/framework/core/tests/test_expression_index_diagnostics.py new file mode 100644 index 00000000..2b56ae29 --- /dev/null +++ b/framework/core/tests/test_expression_index_diagnostics.py @@ -0,0 +1,86 @@ +"""SM026: expression indexes SQLite cannot verify (GH #342).""" + +from __future__ import annotations + +import sqlalchemy as sa +from simple_module_core import ModuleBase, ModuleMeta +from simple_module_core.diagnostics import DiagnosticLevel, run_diagnostics +from simple_module_core.diagnostics._expression_index import ( + check_expression_indexes_unverifiable, + find_expression_indexes, +) + + +def _tables() -> list[sa.Table]: + md = sa.MetaData() + return [ + sa.Table( + "users_user", + md, + sa.Column("id", sa.Integer, primary_key=True), + sa.Column("email", sa.String), + sa.Index("ix_users_user_email_lower", sa.text("lower(email)")), + sa.Index("ix_users_user_email", "email"), + ), + sa.Table("plain", md, sa.Column("id", sa.Integer, primary_key=True)), + ] + + +def test_find_expression_indexes_ignores_plain_column_indexes(): + assert find_expression_indexes(_tables()) == [("users_user", "ix_users_user_email_lower")] + + +def test_function_expression_counts_as_expression(): + md = sa.MetaData() + table = sa.Table( + "t", + md, + sa.Column("id", sa.Integer, primary_key=True), + sa.Column("name", sa.String), + ) + sa.Index("ix_t_name_lower", sa.func.lower(table.c.name)) + assert find_expression_indexes([table]) == [("t", "ix_t_name_lower")] + + +def test_sqlite_reports_info_naming_the_index(): + (diag,) = check_expression_indexes_unverifiable(_tables(), "sqlite", "users") + + assert diag.code == "SM026" + assert diag.level == DiagnosticLevel.INFO + assert "ix_users_user_email_lower" in diag.message + assert "cannot verify" in diag.message + assert "migrations-roundtrip-pg" in (diag.suggestion or "") + + +def test_postgres_and_unknown_dialect_are_silent(): + assert check_expression_indexes_unverifiable(_tables(), "postgresql", "users") == [] + assert check_expression_indexes_unverifiable(_tables(), None, "users") == [] + + +def test_no_expression_indexes_is_silent(): + assert check_expression_indexes_unverifiable(_tables()[1:], "sqlite", "users") == [] + + +def test_run_diagnostics_emits_sm026_for_installed_modules_on_sqlite(): + from simple_module_core.discovery import discover_modules + + modules = discover_modules() + sqlite = [d for d in run_diagnostics(modules, database_dialect="sqlite") if d.code == "SM026"] + postgres = [ + d for d in run_diagnostics(modules, database_dialect="postgresql") if d.code == "SM026" + ] + unset = [d for d in run_diagnostics(modules) if d.code == "SM026"] + + # The users module declares ix_users_user_email_lower. + assert any("ix_users_user_email_lower" in d.message for d in sqlite) + assert postgres == [] + assert unset == [] + + +def test_module_without_models_is_skipped(): + from simple_module_core.diagnostics._expression_index import module_all_tables + + class NoModels(ModuleBase): + meta = ModuleMeta(name="NoModels") + + assert module_all_tables(NoModels()) == [] diff --git a/framework/db/simple_module_db/__init__.py b/framework/db/simple_module_db/__init__.py index 3148900f..61e64b22 100644 --- a/framework/db/simple_module_db/__init__.py +++ b/framework/db/simple_module_db/__init__.py @@ -4,6 +4,7 @@ from simple_module_db.base import create_module_base from simple_module_db.callbacks import OnCommitCallback from simple_module_db.deps import get_db +from simple_module_db.migration_portability import drop_enums_if_postgres from simple_module_db.migrations import ( build_module_metadata, make_include_object, @@ -54,6 +55,7 @@ "create_module_base", "current_tenant_id", "detect_provider", + "drop_enums_if_postgres", "finalize_session", "get_db", "hard_delete", diff --git a/framework/db/simple_module_db/migration_portability.py b/framework/db/simple_module_db/migration_portability.py new file mode 100644 index 00000000..b7e157f3 --- /dev/null +++ b/framework/db/simple_module_db/migration_portability.py @@ -0,0 +1,164 @@ +"""Keep revisions autogenerated on SQLite portable to PostgreSQL (GH #342). + +Autogenerate against the default SQLite database emits things Postgres rejects +or never sees, and neither ``alembic check`` nor the boot check notices: + +* Boolean ``server_default`` rendered as ``sa.text('0')`` — Postgres refuses an + integer default on a boolean column. :func:`rewrite_boolean_defaults` and + :func:`render_boolean_default` emit ``sa.false()`` / ``sa.true()`` instead. +* Expression indexes (``lower(email)``) are skipped on SQLite because it cannot + reflect them. :func:`warn_unverifiable_expression_indexes` says so loudly. +* ``op.drop_table`` leaves ``CREATE TYPE`` behind on Postgres, so a downgrade + followed by an upgrade fails with ``DuplicateObject``. Use + :func:`drop_enums_if_postgres` in the revision's ``downgrade``. + +``simple_module_db.migrations`` wires the first two into the hooks every +``env.py`` already passes to Alembic. +""" + +from __future__ import annotations + +import logging +from collections.abc import Iterator +from typing import Any + +import sqlalchemy as sa +from alembic.operations.ops import AddColumnOp, AlterColumnOp, CreateTableOp +from simple_module_core.diagnostics._expression_index import find_expression_indexes +from sqlalchemy.dialects import postgresql +from sqlalchemy.schema import DefaultClause +from sqlalchemy.sql.elements import ColumnElement, False_, TextClause, True_ + +logger = logging.getLogger("alembic.env") + +_FALSE_LITERALS = frozenset({"0", "false"}) +_TRUE_LITERALS = frozenset({"1", "true"}) + + +def boolean_default_for(default: Any) -> ColumnElement[bool] | None: + """Map a Boolean ``server_default`` spelled as 0/1/true/false to ``sa.false()``/``sa.true()``. + + Accepts a ``DefaultClause``, ``text()``, bare string, or SQLAlchemy + ``true()``/``false()``; returns ``None`` when it is none of those literals. + """ + arg = default.arg if isinstance(default, DefaultClause) else default + if isinstance(arg, False_): + return sa.false() + if isinstance(arg, True_): + return sa.true() + text = arg.text if isinstance(arg, TextClause) else arg + if not isinstance(text, str): + return None + literal = text.strip().strip("()").strip("'\"").lower() + if literal in _FALSE_LITERALS: + return sa.false() + if literal in _TRUE_LITERALS: + return sa.true() + return None + + +def render_boolean_default(obj: Any, autogen_context: Any) -> str | bool: + """``render_item`` branch: render ``true()``/``false()`` defaults as such. + + SQLAlchemy compiles ``false()`` under SQLite to ``0``, which Alembic then + renders as ``sa.text('0')``. Intercepting the ``server_default`` item keeps + the portable expression the model declared. + """ + arg = obj.arg if isinstance(obj, DefaultClause) else obj + if not isinstance(arg, (True_, False_)): + return False + prefix = autogen_context.opts.get("sqlalchemy_module_prefix", "sa.") + return f"{prefix}{'true' if isinstance(arg, True_) else 'false'}()" + + +def iter_ops_recursive(container: Any) -> Iterator[Any]: + """Yield every op under ``container``, descending into nested op groups. + + Autogenerate does not emit a flat op list: index operations for a table are + grouped inside a ``ModifyTableOps`` container alongside the top-level + ``CreateTableOp``/``DropTableOp``. A dedup check that only looks at + ``container.ops`` therefore sees no ``CreateIndexOp`` at all and re-injects + an index the dialect already emitted — which is exactly how a dialect that + *can* reflect expression-based indexes (PostgreSQL) ended up with a + duplicate ``CREATE INDEX`` in its initial migration. + """ + for op in container.ops: + yield op + if hasattr(op, "ops"): + yield from iter_ops_recursive(op) + + +def rewrite_boolean_defaults(upgrade_ops: Any) -> None: + """Rewrite integer-looking defaults on Boolean columns in a revision's op tree. + + Covers columns of ``create_table``, ``add_column`` and ``alter_column`` + (the latter via ``existing_type``). + """ + for op in iter_ops_recursive(upgrade_ops): + if isinstance(op, CreateTableOp): + columns = [c for c in op.columns if isinstance(c, sa.Column)] + elif isinstance(op, AddColumnOp): + columns = [op.column] + elif isinstance(op, AlterColumnOp): + replacement = None + if isinstance(op.existing_type or op.modify_type, sa.Boolean): + replacement = boolean_default_for(op.modify_server_default) + if replacement is not None: + op.modify_server_default = replacement + continue + else: + continue + for column in columns: + if not isinstance(column.type, sa.Boolean) or column.server_default is None: + continue + replacement = boolean_default_for(column.server_default) + if replacement is not None: + column.server_default = DefaultClause(replacement) + + +def warn_unverifiable_expression_indexes(dialect_name: str | None, metadata: sa.MetaData) -> list: + """Warn when autogenerating on SQLite while models declare expression indexes. + + A warning rather than an error: autogenerate also runs for ordinary, + unrelated revisions, and failing every one of them for an index that is + already in history would block normal work. The warning is emitted on every + run, names each index, and ``alembic check`` on Postgres + (``make migrations-roundtrip-pg``) is the real gate. Returns the + ``(table, index)`` pairs it warned about. + """ + if dialect_name != "sqlite": + return [] + found = find_expression_indexes(metadata.tables.values()) + if not found: + return [] + listing = "\n".join(f" - {index} on {table}" for table, index in found) + logger.warning( + "\n%s\n" + "WARNING: autogenerating on SQLite. SQLite cannot reflect expression indexes,\n" + "so this revision does NOT account for the following (they are neither\n" + "created nor verified here) and `alembic check` will not notice drift:\n" + "%s\n" + "Add them to the revision by hand and verify on PostgreSQL before merging\n" + "(`make migrations-roundtrip-pg`). See docs/module-authoring.md, Migrations.\n" + "%s", + "=" * 78, + listing, + "=" * 78, + ) + return found + + +def drop_enums_if_postgres(op: Any, *names: str) -> None: + """Drop the named enum types on PostgreSQL; a no-op elsewhere. + + Call it at the end of ``downgrade()``, after the tables that use the types + are dropped. ``op.drop_table`` leaves the ``CREATE TYPE`` behind, and + autogenerate never emits the ``Enum.drop``, so without this a + ``downgrade`` followed by an ``upgrade`` fails with ``DuplicateObject``. + SQLite has no such object, which is why the round trip passes there. + """ + bind = op.get_bind() + if bind.dialect.name != "postgresql": + return + for name in names: + postgresql.ENUM(name=name).drop(bind, checkfirst=True) diff --git a/framework/db/simple_module_db/migrations.py b/framework/db/simple_module_db/migrations.py index b3bdc7eb..dec46f6b 100644 --- a/framework/db/simple_module_db/migrations.py +++ b/framework/db/simple_module_db/migrations.py @@ -23,11 +23,18 @@ 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 Column, Index, MetaData +from sqlalchemy import Index, MetaData from sqlalchemy.schema import SchemaItem from simple_module_db.base import all_module_bases +from simple_module_db.migration_portability import ( + iter_ops_recursive, + render_boolean_default, + rewrite_boolean_defaults, + warn_unverifiable_expression_indexes, +) logger = logging.getLogger(__name__) @@ -139,6 +146,11 @@ def make_process_revision_directives( Any index already named in the op tree — at any nesting depth — is left alone, which keeps this dialect-agnostic rather than special-casing SQLite. + It also rewrites ``0``/``1``/``false``/``true`` server defaults on Boolean + columns to ``sa.false()``/``sa.true()`` and, when running on SQLite, warns + that expression indexes cannot be verified (see + :mod:`simple_module_db.migration_portability`). + Call as:: context.configure( @@ -149,50 +161,35 @@ def make_process_revision_directives( expression_indexes: dict[str, list[Index]] = {} for table in metadata.tables.values(): for index in table.indexes: - if _index_is_expression_based(index): + if index_is_expression_based(index): expression_indexes.setdefault(table.name, []).append(index) def process_revision_directives(context, revision, directives): - if not expression_indexes: - return + warn_unverifiable_expression_indexes( + getattr(getattr(context, "dialect", None), "name", None), metadata + ) for script in directives: upgrade_ops = getattr(script, "upgrade_ops", None) if upgrade_ops is not None: - _inject_create_index_after_create_table(upgrade_ops, expression_indexes) + rewrite_boolean_defaults(upgrade_ops) + if expression_indexes: + _inject_create_index_after_create_table(upgrade_ops, expression_indexes) downgrade_ops = getattr(script, "downgrade_ops", None) if downgrade_ops is not None: - _inject_drop_index_before_drop_table(downgrade_ops, expression_indexes) + # A downgrade re-creates dropped tables/columns from reflected + # state, so it carries the same ``sa.text('0')`` Boolean defaults. + rewrite_boolean_defaults(downgrade_ops) + if expression_indexes: + _inject_drop_index_before_drop_table(downgrade_ops, expression_indexes) return process_revision_directives -def _index_is_expression_based(index: Index) -> bool: - """An index is expression-based when any of its expressions is not a plain ``Column``.""" - return any(not isinstance(expr, Column) for expr in index.expressions) - - -def _iter_ops_recursive(container): - """Yield every op under ``container``, descending into nested op groups. - - Autogenerate does not emit a flat op list: index operations for a table are - grouped inside a ``ModifyTableOps`` container alongside the top-level - ``CreateTableOp``/``DropTableOp``. A dedup check that only looks at - ``container.ops`` therefore sees no ``CreateIndexOp`` at all and re-injects - an index the dialect already emitted — which is exactly how a dialect that - *can* reflect expression-based indexes (PostgreSQL) ended up with a - duplicate ``CREATE INDEX`` in its initial migration. - """ - for op in container.ops: - yield op - if hasattr(op, "ops"): - yield from _iter_ops_recursive(op) - - def _existing_index_names(container, op_type) -> set[str | None]: """Names of every ``op_type`` index op already present anywhere under ``container``.""" return { getattr(op, "index_name", None) - for op in _iter_ops_recursive(container) + for op in iter_ops_recursive(container) if isinstance(op, op_type) } @@ -241,12 +238,16 @@ def render_item(type_, obj, autogen_context): 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). * 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__ diff --git a/framework/db/tests/test_migration_portability.py b/framework/db/tests/test_migration_portability.py new file mode 100644 index 00000000..0b3878b8 --- /dev/null +++ b/framework/db/tests/test_migration_portability.py @@ -0,0 +1,164 @@ +"""Portability of revisions autogenerated on SQLite (GH #342).""" + +from __future__ import annotations + +import logging +from types import SimpleNamespace + +import pytest +import sqlalchemy as sa +from alembic.autogenerate import produce_migrations, render_python_code +from alembic.runtime.migration import MigrationContext +from simple_module_db.migration_portability import ( + boolean_default_for, + drop_enums_if_postgres, + warn_unverifiable_expression_indexes, +) +from simple_module_db.migrations import make_process_revision_directives, render_item +from sqlalchemy.dialects import postgresql + + +def _metadata(*, with_index: bool = False) -> sa.MetaData: + md = sa.MetaData() + extra = [sa.Index("ix_flags_email_lower", sa.text("lower(email)"))] if with_index else [] + sa.Table( + "flags", + md, + sa.Column("id", sa.Integer, primary_key=True), + sa.Column("email", sa.String), + sa.Column("a", sa.Boolean, server_default=sa.false(), nullable=False), + sa.Column("b", sa.Boolean, server_default=sa.true(), nullable=False), + sa.Column("c", sa.Boolean, server_default=sa.text("0"), nullable=False), + sa.Column("d", sa.Boolean, server_default="1", nullable=False), + sa.Column("n", sa.Integer, server_default=sa.text("0"), nullable=False), + *extra, + ) + return md + + +def _autogenerate(md: sa.MetaData) -> tuple[str, MigrationContext, object]: + engine = sa.create_engine("sqlite://") + with engine.connect() as conn: + ctx = MigrationContext.configure(conn, opts={"render_item": render_item}) + script = produce_migrations(ctx, md) + hook = make_process_revision_directives(md) + hook(ctx, None, [script]) + code = render_python_code( + script.upgrade_ops, render_item=render_item, migration_context=ctx + ) + return code, ctx, script + + +def test_boolean_defaults_render_portably(): + code, _, _ = _autogenerate(_metadata()) + + assert code.count("sa.false()") == 2 # a (declared false()) and c (text('0')) + assert code.count("sa.true()") == 2 # b (declared true()) and d ('1') + # The Integer column keeps its integer default: only Boolean columns change. + assert "sa.Column('n', sa.Integer(), server_default=sa.text('0')" in code + + +@pytest.mark.parametrize( + ("raw", "expected"), + [ + (sa.text("0"), "false"), + (sa.text("1"), "true"), + ("0", "false"), + ("1", "true"), + ("false", "false"), + ("true", "true"), + ("'false'", "false"), + (sa.text("(1)"), "true"), + (sa.false(), "false"), + (sa.DefaultClause(sa.text("0")), "false"), + ("2", None), + (sa.text("now()"), None), + (None, None), + ], +) +def test_boolean_default_for(raw, expected): + result = boolean_default_for(raw) + if expected is None: + assert result is None + else: + assert str(result.compile(dialect=postgresql.dialect())) == expected + + +def test_expression_index_warning_on_sqlite(caplog): + md = _metadata(with_index=True) + with caplog.at_level(logging.WARNING, logger="alembic.env"): + found = warn_unverifiable_expression_indexes("sqlite", md) + + assert found == [("flags", "ix_flags_email_lower")] + assert "ix_flags_email_lower" in caplog.text + assert "by hand" in caplog.text + assert "PostgreSQL" in caplog.text + + +def test_expression_index_warning_silent_elsewhere(caplog): + md = _metadata(with_index=True) + with caplog.at_level(logging.WARNING, logger="alembic.env"): + assert warn_unverifiable_expression_indexes("postgresql", md) == [] + assert warn_unverifiable_expression_indexes("sqlite", _metadata()) == [] + assert caplog.text == "" + + +def test_hook_warns_during_autogenerate_on_sqlite(caplog): + with caplog.at_level(logging.WARNING, logger="alembic.env"): + _autogenerate(_metadata(with_index=True)) + assert "ix_flags_email_lower" in caplog.text + + +def test_plain_column_index_is_not_flagged(): + md = sa.MetaData() + sa.Table( + "t", + md, + sa.Column("id", sa.Integer, primary_key=True), + sa.Column("x", sa.Integer), + sa.Index("ix_t_x", "x"), + ) + assert warn_unverifiable_expression_indexes("sqlite", md) == [] + + +class _FakeOp: + def __init__(self, dialect: str) -> None: + self.bind = SimpleNamespace(dialect=SimpleNamespace(name=dialect)) + + def get_bind(self): + return self.bind + + +def test_drop_enums_noop_off_postgres(monkeypatch): + calls: list = [] + monkeypatch.setattr(postgresql.ENUM, "drop", lambda self, *a, **k: calls.append(self.name)) + drop_enums_if_postgres(_FakeOp("sqlite"), "status") + assert calls == [] + + +def test_drop_enums_on_postgres(monkeypatch): + calls: list = [] + + def fake_drop(self, bind, checkfirst=True): + calls.append((self.name, checkfirst)) + + monkeypatch.setattr(postgresql.ENUM, "drop", fake_drop) + drop_enums_if_postgres(_FakeOp("postgresql"), "status", "kind") + assert calls == [("status", True), ("kind", True)] + + +def test_downgrade_recreated_boolean_defaults_render_portably(): + """A dropped table's downgrade re-creates it from reflected ``sa.text('0')`` defaults.""" + engine = sa.create_engine("sqlite://") + _metadata().create_all(engine) + empty = sa.MetaData() + with engine.connect() as conn: + ctx = MigrationContext.configure(conn, opts={"render_item": render_item}) + script = produce_migrations(ctx, empty) + make_process_revision_directives(empty)(ctx, None, [script]) + code = render_python_code( + script.downgrade_ops, render_item=render_item, migration_context=ctx + ) + + assert code.count("sa.false()") == 2 + assert code.count("sa.true()") == 2 diff --git a/framework/hosting/simple_module_hosting/_dev_boot.py b/framework/hosting/simple_module_hosting/_dev_boot.py index 6ebe0815..76d7adc5 100644 --- a/framework/hosting/simple_module_hosting/_dev_boot.py +++ b/framework/hosting/simple_module_hosting/_dev_boot.py @@ -54,6 +54,7 @@ def run_dev_boot( i18n_supported_locales=settings.i18n_supported_locales, i18n_default_locale=settings.i18n_default_locale, i18n_extra_sources=i18n_extra, + database_dialect=settings.database_url.split(":", 1)[0].split("+", 1)[0], ) diagnostics = diagnostics_state.rerun() errors = [d for d in diagnostics if d.level == DiagnosticLevel.ERROR] diff --git a/framework/hosting/simple_module_hosting/migrations.py b/framework/hosting/simple_module_hosting/migrations.py index 6545c5eb..0be51fb9 100644 --- a/framework/hosting/simple_module_hosting/migrations.py +++ b/framework/hosting/simple_module_hosting/migrations.py @@ -198,6 +198,33 @@ def _pending_on_branch(head: str) -> int: } +def _note_unverifiable_expression_indexes(engine) -> None: + """Say so when the dialect cannot verify the models' expression indexes. + + "At head" on SQLite does not cover ``lower(email)``-style indexes: the + dialect cannot reflect them, so neither this check nor ``alembic check`` + can tell whether the schema has them (GH #342, diagnostic SM026). + """ + if engine.dialect.name != "sqlite": + return + try: + from simple_module_core.diagnostics._expression_index import find_expression_indexes + from simple_module_db.base import all_module_bases + + found = find_expression_indexes( + table for base in all_module_bases for table in base.metadata.tables.values() + ) + except Exception: # pragma: no cover - purely advisory + logger.debug("Could not inspect expression indexes", exc_info=True) + return + if found: + logger.info( + "Migrations are at head, but SQLite cannot verify expression indexes (%s); " + "check them on PostgreSQL (make migrations-roundtrip-pg). See SM026.", + ", ".join(f"{index} ({table})" for table, index in found), + ) + + async def check_migrations(engine, alembic_ini_path: str | None = None) -> dict: """Return migration state, raising if the database is behind head. @@ -214,4 +241,5 @@ async def check_migrations(engine, alembic_ini_path: str | None = None) -> dict: f"(at {status['current_revision']!r}, head is " f"{status['head_revision']!r}). Run: make migrate" ) + _note_unverifiable_expression_indexes(engine) return status diff --git a/framework/hosting/tests/test_check_migrations_expression_note.py b/framework/hosting/tests/test_check_migrations_expression_note.py new file mode 100644 index 00000000..bc3e321f --- /dev/null +++ b/framework/hosting/tests/test_check_migrations_expression_note.py @@ -0,0 +1,30 @@ +"""check_migrations must not imply "clean" for what SQLite cannot verify (GH #342).""" + +from __future__ import annotations + +import logging +from types import SimpleNamespace + +import pytest +from simple_module_hosting.migrations import _note_unverifiable_expression_indexes + + +def _engine(dialect: str): + return SimpleNamespace(dialect=SimpleNamespace(name=dialect)) + + +def test_sqlite_logs_that_expression_indexes_are_unverified(caplog): + pytest.importorskip("simple_module_users") + with caplog.at_level(logging.INFO, logger="simple_module_hosting.migrations"): + _note_unverifiable_expression_indexes(_engine("sqlite")) + + assert "cannot verify expression indexes" in caplog.text + assert "ix_users_user_email_lower" in caplog.text + assert "SM026" in caplog.text + + +def test_postgres_stays_quiet(caplog): + with caplog.at_level(logging.INFO, logger="simple_module_hosting.migrations"): + _note_unverifiable_expression_indexes(_engine("postgresql")) + + assert caplog.text == "" diff --git a/host/migrations/env.py b/host/migrations/env.py index 0617f470..0d15e402 100644 --- a/host/migrations/env.py +++ b/host/migrations/env.py @@ -38,8 +38,9 @@ # host's user-added tables or framework internals. include_object = make_include_object(target_metadata) -# Re-emit expression-based indexes (e.g. ``lower(email)``) that autogenerate -# silently drops under SQLite — see make_process_revision_directives docstring. +# Re-emit expression-based indexes that autogenerate drops under SQLite, rewrite +# Boolean 0/1 server defaults to sa.false()/sa.true(), and warn that SQLite cannot +# verify expression indexes (GH #342) — see make_process_revision_directives. process_revision_directives = make_process_revision_directives(target_metadata) diff --git a/host/migrations/versions/e5f2a8c1d7b3_timestamps_timezone_aware.py b/host/migrations/versions/e5f2a8c1d7b3_timestamps_timezone_aware.py new file mode 100644 index 00000000..561526f5 --- /dev/null +++ b/host/migrations/versions/e5f2a8c1d7b3_timestamps_timezone_aware.py @@ -0,0 +1,58 @@ +"""users_refresh_token / background_tasks_task_execution: timestamps -> timestamptz + +SQLModel's ``datetime`` fields map to ``UTCDateTime`` (``DateTime(timezone=True)``), +but these columns were created by autogenerate on SQLite as naive ``DateTime()``. +SQLite cannot tell the difference; on PostgreSQL they are ``timestamp without +time zone`` and ``alembic check`` reports ``modify_type`` drift on each of them +(found by the PostgreSQL round-trip CI job, GH #342). + +Existing values are UTC (the application always wrote UTC), so they are +reinterpreted ``AT TIME ZONE 'UTC'`` rather than shifted. A no-op on SQLite. + + +Revision ID: e5f2a8c1d7b3 +Revises: c7f2d9a41e83 +Create Date: 2026-10-05 10:00:00.000000 +""" + +from collections.abc import Sequence + +import sqlalchemy as sa +from alembic import op + +revision: str = "e5f2a8c1d7b3" +down_revision: str | None = "c7f2d9a41e83" +branch_labels: str | Sequence[str] | None = None +depends_on: str | Sequence[str] | None = None + +_COLUMNS = ( + ("background_tasks_task_execution", "queued_at", True), + ("background_tasks_task_execution", "started_at", True), + ("background_tasks_task_execution", "finished_at", True), + ("background_tasks_task_execution", "heartbeat_at", True), + ("users_refresh_token", "created_at", False), + ("users_refresh_token", "expires_at", False), + ("users_refresh_token", "revoked_at", True), +) + + +def _retype(*, aware: bool) -> None: + if op.get_bind().dialect.name != "postgresql": + return + for table, column, nullable in _COLUMNS: + op.alter_column( + table, + column, + type_=sa.DateTime(timezone=aware), + existing_type=sa.DateTime(timezone=not aware), + existing_nullable=nullable, + postgresql_using=f"{column} AT TIME ZONE 'UTC'", + ) + + +def upgrade() -> None: + _retype(aware=True) + + +def downgrade() -> None: + _retype(aware=False) diff --git a/host/migrations/versions/f6a3b9d2e8c4_keycloak_timestamp_timezone_aware.py b/host/migrations/versions/f6a3b9d2e8c4_keycloak_timestamp_timezone_aware.py new file mode 100644 index 00000000..8603a8dd --- /dev/null +++ b/host/migrations/versions/f6a3b9d2e8c4_keycloak_timestamp_timezone_aware.py @@ -0,0 +1,41 @@ +"""keycloak_user_cache.last_login_at: timestamp -> timestamptz + +Same drift as ``e5f2a8c1d7b3`` (naive ``DateTime()`` created on SQLite, model +declares a timezone-aware column), on the keycloak branch. Values are UTC and +are reinterpreted ``AT TIME ZONE 'UTC'``. A no-op on SQLite. + +Revision ID: f6a3b9d2e8c4 +Revises: 168a2882f443 +Create Date: 2026-10-05 10:00:01.000000 +""" + +from collections.abc import Sequence + +import sqlalchemy as sa +from alembic import op + +revision: str = "f6a3b9d2e8c4" +down_revision: str | None = "168a2882f443" +branch_labels: str | Sequence[str] | None = None +depends_on: str | Sequence[str] | None = None + + +def _retype(*, aware: bool) -> None: + if op.get_bind().dialect.name != "postgresql": + return + op.alter_column( + "keycloak_user_cache", + "last_login_at", + type_=sa.DateTime(timezone=aware), + existing_type=sa.DateTime(timezone=not aware), + existing_nullable=True, + postgresql_using="last_login_at AT TIME ZONE 'UTC'", + ) + + +def upgrade() -> None: + _retype(aware=True) + + +def downgrade() -> None: + _retype(aware=False)