diff --git a/backend/alembic/env.py b/backend/alembic/env.py index eff622e..1b83272 100644 --- a/backend/alembic/env.py +++ b/backend/alembic/env.py @@ -3,17 +3,19 @@ from __future__ import annotations import importlib import os from logging.config import fileConfig +from typing import Any from alembic import context -from sqlalchemy import create_engine, pool +from sqlalchemy import create_engine, event, pool from sqlalchemy.engine import URL, make_url -Base = importlib.import_module("backup_tool.db.models").Base +models = importlib.import_module("backup_tool.db.models") +sqlite_runtime = importlib.import_module("backup_tool.db.sqlite") config = context.config if config.config_file_name is not None: fileConfig(config.config_file_name) -target_metadata = Base.metadata +target_metadata = models.Base.metadata def configured_url() -> str: @@ -23,6 +25,19 @@ def configured_url() -> str: return raw_url +def busy_timeout_ms() -> int: + raw_value = os.environ.get( + "BACKUP_TOOL_SQLITE_BUSY_TIMEOUT_MS", str(sqlite_runtime.DEFAULT_BUSY_TIMEOUT_MS) + ) + try: + value = int(raw_value) + except ValueError as error: + raise RuntimeError("SQLite busy timeout must be an integer") from error + if value < 1_000 or value > 120_000: + raise RuntimeError("SQLite busy timeout must be between 1000 and 120000 ms") + return value + + def synchronous_url(raw_url: str) -> URL: url = make_url(raw_url) if url.drivername != "sqlite+aiosqlite": @@ -44,10 +59,17 @@ def run_migrations_offline() -> None: def run_migrations_online() -> None: + timeout = busy_timeout_ms() engine = create_engine( synchronous_url(configured_url()), poolclass=pool.NullPool, + connect_args=sqlite_runtime.sqlite_connect_args(timeout), ) + + @event.listens_for(engine, "connect") + def configure_sqlite(dbapi_connection: Any, _connection_record: Any) -> None: + sqlite_runtime.configure_sqlite_connection(dbapi_connection) + with engine.connect() as connection: context.configure(connection=connection, target_metadata=target_metadata, compare_type=True) with context.begin_transaction(): diff --git a/backend/src/backup_tool/cli.py b/backend/src/backup_tool/cli.py index d923937..840f03d 100644 --- a/backend/src/backup_tool/cli.py +++ b/backend/src/backup_tool/cli.py @@ -3,26 +3,81 @@ from __future__ import annotations import argparse -from collections.abc import Sequence +import asyncio +from collections.abc import Callable, Sequence +from pathlib import Path + +from alembic import command +from alembic.config import Config from backup_tool import __version__ +from backup_tool.config import Settings -ROLES = ("web", "scheduler", "worker", "migrate", "admin") +from .db.engine import assert_schema_current, create_engine + +RoleHandler = Callable[[Settings], int] +DATABASE_ROLES = ("web", "scheduler", "worker", "admin") + + +def _placeholder_role(_settings: Settings) -> int: + return 0 + + +ROLE_HANDLERS: dict[str, RoleHandler] = {role: _placeholder_role for role in DATABASE_ROLES} def build_parser() -> argparse.ArgumentParser: parser = argparse.ArgumentParser(prog="backup-tool") parser.add_argument("--version", action="version", version=__version__) subparsers = parser.add_subparsers(dest="role", required=True) - for role in ROLES: + for role in DATABASE_ROLES: subparsers.add_parser(role, help=f"run the {role} role") + migrate = subparsers.add_parser("migrate", help="manage the metadata schema") + migrate.add_argument("action", choices=("upgrade", "downgrade", "current")) return parser -def main(argv: Sequence[str] | None = None) -> int: - build_parser().parse_args(argv) +def build_alembic_config(settings: Settings) -> Config: + backend_root = Path(__file__).resolve().parents[2] + migration = Config(backend_root / "alembic.ini") + migration.set_main_option("sqlalchemy.url", settings.database_url) + return migration + + +def require_current_schema(settings: Settings) -> None: + async def check() -> None: + engine = create_engine(settings) + try: + await assert_schema_current(engine, build_alembic_config(settings)) + finally: + await engine.dispose() + + asyncio.run(check()) + + +def run_migration(action: str, settings: Settings) -> int: + migration = build_alembic_config(settings) + if action == "upgrade": + command.upgrade(migration, "head") + elif action == "downgrade": + command.downgrade(migration, "base") + else: + command.current(migration) return 0 +def load_settings() -> Settings: + return Settings() # type: ignore[call-arg] + + +def main(argv: Sequence[str] | None = None, *, settings: Settings | None = None) -> int: + args = build_parser().parse_args(argv) + runtime_settings = settings or load_settings() + if args.role == "migrate": + return run_migration(args.action, runtime_settings) + require_current_schema(runtime_settings) + return ROLE_HANDLERS[args.role](runtime_settings) + + if __name__ == "__main__": raise SystemExit(main()) diff --git a/backend/src/backup_tool/db/engine.py b/backend/src/backup_tool/db/engine.py index 25402df..f0bd260 100644 --- a/backend/src/backup_tool/db/engine.py +++ b/backend/src/backup_tool/db/engine.py @@ -1,6 +1,5 @@ from __future__ import annotations -from collections.abc import Callable from typing import Any from alembic.config import Config @@ -12,23 +11,23 @@ from sqlalchemy.ext.asyncio import AsyncEngine, create_async_engine from backup_tool.config import Settings +from . import sqlite as sqlite_runtime + class SchemaNotCurrentError(RuntimeError): pass def create_engine(settings: Settings) -> AsyncEngine: - engine = create_async_engine(settings.database_url, pool_pre_ping=True) + engine = create_async_engine( + settings.database_url, + pool_pre_ping=True, + connect_args=sqlite_runtime.sqlite_connect_args(settings.sqlite_busy_timeout_ms), + ) @event.listens_for(engine.sync_engine, "connect") def configure_sqlite(dbapi_connection: Any, _connection_record: Any) -> None: - cursor = dbapi_connection.cursor() - try: - cursor.execute("PRAGMA foreign_keys=ON") - cursor.execute(f"PRAGMA busy_timeout={settings.sqlite_busy_timeout_ms}") - cursor.execute("PRAGMA journal_mode=WAL") - finally: - cursor.close() + sqlite_runtime.configure_sqlite_connection(dbapi_connection) return engine @@ -45,6 +44,3 @@ async def assert_schema_current(engine: AsyncEngine, alembic_config: Config) -> raise SchemaNotCurrentError( f"database schema is not current: expected {expected!r}, found {current!r}" ) - - -SchemaCheck = Callable[[AsyncEngine, Config], Any] diff --git a/backend/src/backup_tool/db/models.py b/backend/src/backup_tool/db/models.py index 73ce006..a9c0ff7 100644 --- a/backend/src/backup_tool/db/models.py +++ b/backend/src/backup_tool/db/models.py @@ -7,7 +7,6 @@ from sqlalchemy import ( JSON, Boolean, CheckConstraint, - DateTime, ForeignKey, Index, Integer, @@ -21,6 +20,10 @@ from sqlalchemy import ( ) from sqlalchemy.orm import DeclarativeBase, Mapped, mapped_column +from backup_tool.ids import new_uuid7 + +from .types import UTCDateTime + NAMING_CONVENTION = { "ix": "ix_%(column_0_label)s", "uq": "uq_%(table_name)s_%(column_0_name)s", @@ -35,15 +38,15 @@ class Base(DeclarativeBase): class IdentityMixin: - id: Mapped[str] = mapped_column(String(36), primary_key=True) + id: Mapped[str] = mapped_column(String(36), primary_key=True, default=lambda: str(new_uuid7())) class TimestampMixin: created_at: Mapped[datetime] = mapped_column( - DateTime(timezone=True), nullable=False, server_default=func.current_timestamp() + UTCDateTime(), nullable=False, server_default=func.current_timestamp() ) updated_at: Mapped[datetime] = mapped_column( - DateTime(timezone=True), + UTCDateTime(), nullable=False, server_default=func.current_timestamp(), onupdate=func.current_timestamp(), @@ -63,8 +66,8 @@ class ApiToken(IdentityMixin, TimestampMixin, Base): owner_id: Mapped[str] = mapped_column(ForeignKey("users.id", ondelete="RESTRICT")) token_hash: Mapped[str] = mapped_column(Text, nullable=False, unique=True) scopes: Mapped[list[str]] = mapped_column(JSON, nullable=False) - expires_at: Mapped[datetime | None] = mapped_column(DateTime(timezone=True)) - revoked_at: Mapped[datetime | None] = mapped_column(DateTime(timezone=True)) + expires_at: Mapped[datetime | None] = mapped_column(UTCDateTime()) + revoked_at: Mapped[datetime | None] = mapped_column(UTCDateTime()) __table_args__ = (Index("ix_api_tokens_owner_id", "owner_id"),) @@ -134,7 +137,7 @@ class Schedule(IdentityMixin, TimestampMixin, Base): misfire_grace_seconds: Mapped[int] = mapped_column(Integer, nullable=False, default=900) overlap_policy: Mapped[str] = mapped_column(String(32), nullable=False, default="prohibit") enabled: Mapped[bool] = mapped_column(Boolean, nullable=False, default=True) - next_nominal_at: Mapped[datetime | None] = mapped_column(DateTime(timezone=True)) + next_nominal_at: Mapped[datetime | None] = mapped_column(UTCDateTime()) last_enqueue_outcome: Mapped[str | None] = mapped_column(String(64)) __table_args__ = ( CheckConstraint("misfire_grace_seconds >= 0", name="misfire_nonnegative"), @@ -146,18 +149,18 @@ class Execution(IdentityMixin, TimestampMixin, Base): __tablename__ = "executions" job_id: Mapped[str] = mapped_column(ForeignKey("jobs.id", ondelete="RESTRICT")) schedule_id: Mapped[str | None] = mapped_column(ForeignKey("schedules.id", ondelete="RESTRICT")) - nominal_run_at: Mapped[datetime | None] = mapped_column(DateTime(timezone=True)) + nominal_run_at: Mapped[datetime | None] = mapped_column(UTCDateTime()) trigger: Mapped[str] = mapped_column(String(32), nullable=False) state: Mapped[str] = mapped_column(String(32), nullable=False, default="queued") attempt: Mapped[int] = mapped_column(Integer, nullable=False, default=1) lease_owner: Mapped[str | None] = mapped_column(String(255)) - lease_expires_at: Mapped[datetime | None] = mapped_column(DateTime(timezone=True)) - heartbeat_at: Mapped[datetime | None] = mapped_column(DateTime(timezone=True)) + lease_expires_at: Mapped[datetime | None] = mapped_column(UTCDateTime()) + heartbeat_at: Mapped[datetime | None] = mapped_column(UTCDateTime()) progress: Mapped[dict[str, Any]] = mapped_column(JSON, nullable=False) reason_code: Mapped[str | None] = mapped_column(String(64)) operator_message: Mapped[str | None] = mapped_column(Text) - started_at: Mapped[datetime | None] = mapped_column(DateTime(timezone=True)) - completed_at: Mapped[datetime | None] = mapped_column(DateTime(timezone=True)) + started_at: Mapped[datetime | None] = mapped_column(UTCDateTime()) + completed_at: Mapped[datetime | None] = mapped_column(UTCDateTime()) __table_args__ = ( CheckConstraint("attempt > 0", name="attempt_positive"), CheckConstraint( @@ -192,9 +195,9 @@ class Backup(IdentityMixin, Base): stored_bytes: Mapped[int] = mapped_column(Integer, nullable=False) integrity: Mapped[str] = mapped_column(String(32), nullable=False, default="unverified") pinned: Mapped[bool] = mapped_column(Boolean, nullable=False, default=False) - tombstoned_at: Mapped[datetime | None] = mapped_column(DateTime(timezone=True)) + tombstoned_at: Mapped[datetime | None] = mapped_column(UTCDateTime()) created_at: Mapped[datetime] = mapped_column( - DateTime(timezone=True), nullable=False, server_default=func.current_timestamp() + UTCDateTime(), nullable=False, server_default=func.current_timestamp() ) __table_args__ = ( CheckConstraint("logical_bytes >= 0", name="logical_bytes_nonnegative"), @@ -233,7 +236,7 @@ class AuditEvent(IdentityMixin, Base): request_id: Mapped[str] = mapped_column(String(36), nullable=False) details: Mapped[dict[str, Any]] = mapped_column(JSON, nullable=False) created_at: Mapped[datetime] = mapped_column( - DateTime(timezone=True), nullable=False, server_default=func.current_timestamp() + UTCDateTime(), nullable=False, server_default=func.current_timestamp() ) __table_args__ = ( CheckConstraint("outcome IN ('success','failure','denied')", name="outcome"), @@ -264,7 +267,7 @@ class NotificationDelivery(IdentityMixin, TimestampMixin, Base): attempt: Mapped[int] = mapped_column(Integer, nullable=False, default=1) state: Mapped[str] = mapped_column(String(32), nullable=False) response_class: Mapped[str | None] = mapped_column(String(64)) - next_attempt_at: Mapped[datetime | None] = mapped_column(DateTime(timezone=True)) + next_attempt_at: Mapped[datetime | None] = mapped_column(UTCDateTime()) __table_args__ = ( CheckConstraint("attempt > 0", name="attempt_positive"), CheckConstraint("state IN ('pending','delivered','retry','failed')", name="state"), @@ -282,7 +285,7 @@ class IdempotencyRecord(IdentityMixin, Base): response_resource_type: Mapped[str] = mapped_column(String(64), nullable=False) response_resource_id: Mapped[str] = mapped_column(String(36), nullable=False) created_at: Mapped[datetime] = mapped_column( - DateTime(timezone=True), nullable=False, server_default=func.current_timestamp() + UTCDateTime(), nullable=False, server_default=func.current_timestamp() ) __table_args__ = ( UniqueConstraint("actor_id", "key", "operation", name="uq_idempotency_actor_key_operation"), diff --git a/backend/src/backup_tool/db/sqlite.py b/backend/src/backup_tool/db/sqlite.py new file mode 100644 index 0000000..4ef8b86 --- /dev/null +++ b/backend/src/backup_tool/db/sqlite.py @@ -0,0 +1,20 @@ +from __future__ import annotations + +from typing import Any + +DEFAULT_BUSY_TIMEOUT_MS = 5_000 + + +def sqlite_connect_args(busy_timeout_ms: int) -> dict[str, float]: + """Configure SQLite lock waiting without dynamic PRAGMA SQL.""" + return {"timeout": busy_timeout_ms / 1_000} + + +def configure_sqlite_connection(dbapi_connection: Any) -> None: + """Apply connection-local safety and persistent WAL mode.""" + cursor = dbapi_connection.cursor() + try: + cursor.execute("PRAGMA foreign_keys=ON") + cursor.execute("PRAGMA journal_mode=WAL") + finally: + cursor.close() diff --git a/backend/src/backup_tool/db/types.py b/backend/src/backup_tool/db/types.py new file mode 100644 index 0000000..8b86510 --- /dev/null +++ b/backend/src/backup_tool/db/types.py @@ -0,0 +1,34 @@ +from __future__ import annotations + +from datetime import UTC, datetime +from typing import Any + +from sqlalchemy import DateTime +from sqlalchemy.engine import Dialect +from sqlalchemy.types import TypeDecorator + + +class UTCDateTime(TypeDecorator[datetime]): + """Store instants and always return timezone-aware UTC datetimes.""" + + impl = DateTime + cache_ok = True + + def load_dialect_impl(self, dialect: Dialect) -> Any: + return dialect.type_descriptor(DateTime(timezone=True)) + + def process_bind_param(self, value: datetime | None, dialect: Dialect) -> datetime | None: + del dialect + if value is None: + return None + if value.tzinfo is None: + raise ValueError("datetime values must be timezone-aware") + return value.astimezone(UTC) + + def process_result_value(self, value: datetime | None, dialect: Dialect) -> datetime | None: + del dialect + if value is None: + return None + if value.tzinfo is None: + return value.replace(tzinfo=UTC) + return value.astimezone(UTC) diff --git a/docs/release/m1-evidence.md b/docs/release/m1-evidence.md index 7960979..b704596 100644 --- a/docs/release/m1-evidence.md +++ b/docs/release/m1-evidence.md @@ -35,9 +35,28 @@ Observed results: - Frontend typecheck/build: passed. - Disposable database removed after verification. +## Review-Fix RED/GREEN + +RED commit: `45a526a test(v2): prove runtime dispatch and persistence invariants` + +The focused M1 command exited 1 with five intended failures: role dispatch was absent, migrate did not dispatch Alembic, database roles accepted an unmigrated DB, persisted ORM IDs had no default, and migration connections had no shared SQLite configurator. + +GREEN verification after implementing the missing behavior: + +- CLI dispatch and `migrate upgrade` tests passed. +- Unmigrated worker startup was rejected before its handler ran. +- Persisted User and Repository IDs were UUIDv7. +- Generated and non-UTC supplied timestamps reloaded as aware UTC. +- Alembic connection observation reported `foreign_keys=1`, `journal_mode=wal`, and `busy_timeout=7000`. +- Baseline active-execution index, schedule uniqueness, and execution checks were present. +- Focused M1 suite: `19 passed`. +- Fast unit/contract suite: `30 passed`. +- Alembic upgrade/downgrade/upgrade, Ruff, strict mypy, frontend typecheck/build, forbidden-v1 scan, and `git diff --check`: passed. + ## Hand Review - Baseline revision contains explicit `op.create_table`, constraints, foreign keys, and indexes for all 14 v2 entities; it does not call `create_all`. -- SQLite connections enable foreign keys, WAL, and a bounded busy timeout. +- Runtime and Alembic share one SQLite connection configurator; lock waiting uses the driver timeout rather than interpolated PRAGMA SQL. - Startup compares the database Alembic revision with the current script head and rejects unmigrated databases. -- Runtime roles are explicit CLI subcommands; no role starts another role in M1. +- Every ORM identity has a UUIDv7 default; every persisted datetime uses the aware-UTC normalizing type. +- Runtime roles are explicit CLI subcommands and dispatch only after the schema gate; `migrate` invokes Alembic directly.