From 8a47c0533f8b8e648f0b9322c4df860a05f75b46 Mon Sep 17 00:00:00 2001 From: Nate-Soul Date: Mon, 13 Jul 2026 14:53:34 +0100 Subject: [PATCH] refactor: extract DB mixins, middleware handlers, and lifecycle hooks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three structural splits to keep the boilerplate readable as it grows — no behavior change (all endpoints, headers, and error envelopes are byte-identical). - database/mixins.py: split TimestampedBase into composable UUIDPrimaryKeyMixin, TimestampMixin, SoftDeleteMixin; TimestampedBase now composes all three. base.py keeps only Base + GUID + UTC helpers. Models can pick mixins a la carte. - middleware/cors.py + middleware/error_handler.py: move the inline CORS setup and exception handlers out of main.py behind configure_cors(app) and register_exception_handlers(app). - core/events.py: extract the ASGI lifespan into named _startup/_shutdown hooks, now wrapped in try/finally so shutdown runs on teardown-after-error. Shaped for Redis/queues/warm-up to slot in as fail-open steps later. main.py is now a thin assembly file. README updated to match. Co-Authored-By: Claude Opus 4.8 --- README.md | 9 +-- app/core/events.py | 45 +++++++++++++ app/infrastructure/database/base.py | 24 ++----- app/infrastructure/database/mixins.py | 56 ++++++++++++++++ app/infrastructure/middleware/cors.py | 17 +++++ .../middleware/error_handler.py | 58 ++++++++++++++++ app/main.py | 66 ++----------------- app/modules/auth/models.py | 3 +- app/modules/users/models.py | 2 +- 9 files changed, 195 insertions(+), 85 deletions(-) create mode 100644 app/core/events.py create mode 100644 app/infrastructure/database/mixins.py create mode 100644 app/infrastructure/middleware/cors.py create mode 100644 app/infrastructure/middleware/error_handler.py diff --git a/README.md b/README.md index 10e18f3..d5d6dde 100644 --- a/README.md +++ b/README.md @@ -41,11 +41,11 @@ app/ ├── api/ │ ├── health.py # /health + /health/ready │ └── v1/router.py # central v1 aggregator — includes each module's router -├── core/ # config, security (hashing/JWT), exceptions, logging +├── core/ # config, security (hashing/JWT), exceptions, logging, events (lifespan) ├── common/ # shared schemas (error envelope, Page) + deps (DbSession) ├── infrastructure/ -│ ├── database/ # Base + GUID + TimestampedBase, engine/session -│ └── middleware/ # security_headers, request_id, rate_limit +│ ├── database/ # base (Base + GUID), mixins (Timestamp/SoftDelete/UUID PK), engine/session +│ └── middleware/ # security_headers, request_id, rate_limit, cors, error_handler └── modules/ ├── users/ # models, schemas, service, routes └── auth/ # models, schemas, service, routes, dependencies @@ -98,7 +98,8 @@ a Docker build against a real Postgres service on every push/PR. 1. Create `app/modules//` with `models.py`, `schemas.py`, `service.py`, `routes.py` (and `dependencies.py` if it has its own auth needs). 2. Models extend `TimestampedBase` (UUID PK + timestamps + soft delete for free) from - `app.infrastructure.database.base`. + `app.infrastructure.database.mixins` — or compose the individual `UUIDPrimaryKeyMixin`, + `TimestampMixin`, and `SoftDeleteMixin` when a model needs a different combination. 3. Register the module's `router` in `app/api/v1/router.py`. 4. Import the module's models in `alembic/env.py` so autogenerate sees them. 5. `alembic revision --autogenerate -m "add "` — **check the generated file**: diff --git a/app/core/events.py b/app/core/events.py new file mode 100644 index 0000000..56f31f0 --- /dev/null +++ b/app/core/events.py @@ -0,0 +1,45 @@ +"""Application lifecycle: the ASGI ``lifespan`` context manager plus the startup and +shutdown hooks it runs. + +Deliberately minimal today — startup just logs, shutdown disposes the DB engine's +connection pool. It is shaped to grow: add a resource (Redis, a task queue, a model +warm-up) as its own step inside ``_startup`` / ``_shutdown``. Follow the reference +services' convention and make optional resources *fail open* — wrap the step in +try/except and log, so one unavailable dependency degrades a feature instead of +blocking the whole app from booting. Anything the app genuinely cannot run without +should raise and abort startup instead. +""" + +from contextlib import asynccontextmanager + +from fastapi import FastAPI + +from app.core.config import get_settings +from app.core.logging import get_logger +from app.infrastructure.database.session import engine + +logger = get_logger(__name__) +settings = get_settings() + + +async def _startup() -> None: + logger.info("app.started", env=settings.environment) + # Add startup steps here (e.g. cache connect, scheduler start, model warm-up). + + +async def _shutdown() -> None: + # Add teardown steps here (mirror the startup steps, in reverse) before the engine + # is disposed. Dispose the pool last so nothing tries to use it after it's gone. + await engine.dispose() + logger.info("app.stopped") + + +@asynccontextmanager +async def lifespan(app: FastAPI): + """Bind to FastAPI via ``FastAPI(lifespan=lifespan)``. The try/finally guarantees + shutdown runs even if the server is torn down after an error while serving.""" + await _startup() + try: + yield + finally: + await _shutdown() diff --git a/app/infrastructure/database/base.py b/app/infrastructure/database/base.py index edd960d..d4b2401 100644 --- a/app/infrastructure/database/base.py +++ b/app/infrastructure/database/base.py @@ -1,9 +1,12 @@ +"""ORM foundations: the declarative ``Base``, the cross-dialect ``GUID`` type, and +UTC datetime helpers. Reusable column groups live in ``mixins.py``, which builds on +these primitives.""" + import uuid from datetime import datetime, timezone -from sqlalchemy import DateTime from sqlalchemy.dialects.postgresql import UUID as PG_UUID -from sqlalchemy.orm import DeclarativeBase, Mapped, mapped_column +from sqlalchemy.orm import DeclarativeBase from sqlalchemy.types import CHAR, TypeDecorator @@ -45,20 +48,3 @@ def as_aware_utc(dt: datetime) -> datetime: Normalize here so comparisons against datetime.now(timezone.utc) work identically on both backends instead of raising TypeError on SQLite only.""" return dt if dt.tzinfo is not None else dt.replace(tzinfo=timezone.utc) - - -class TimestampedBase(Base): - """Abstract base: UUID PK (non-enumerable), created_at/updated_at, soft delete.""" - - __abstract__ = True - - id: Mapped[uuid.UUID] = mapped_column(GUID(), primary_key=True, default=uuid.uuid4) - created_at: Mapped[datetime] = mapped_column(DateTime(timezone=True), default=utcnow, nullable=False) - updated_at: Mapped[datetime] = mapped_column( - DateTime(timezone=True), default=utcnow, onupdate=utcnow, nullable=False - ) - deleted_at: Mapped[datetime | None] = mapped_column(DateTime(timezone=True), nullable=True, default=None) - - @property - def is_deleted(self) -> bool: - return self.deleted_at is not None diff --git a/app/infrastructure/database/mixins.py b/app/infrastructure/database/mixins.py new file mode 100644 index 0000000..43c9766 --- /dev/null +++ b/app/infrastructure/database/mixins.py @@ -0,0 +1,56 @@ +"""Composable ORM mixins. + +Each mixin contributes one concern (identity, timestamps, soft delete) and nothing +else, so a model can pick exactly the columns it needs: + + class Widget(UUIDPrimaryKeyMixin, TimestampMixin, Base): # no soft delete + __tablename__ = "widgets" + +``TimestampedBase`` bundles all three for the common case. Mixins are plain classes +(not ``DeclarativeBase`` subclasses) — SQLAlchemy 2.0 resolves their ``mapped_column`` +annotations when they appear in a model's MRO alongside a declarative ``Base``. +""" + +import uuid +from datetime import datetime + +from sqlalchemy import DateTime +from sqlalchemy.orm import Mapped, mapped_column + +from app.infrastructure.database.base import GUID, Base, utcnow + + +class UUIDPrimaryKeyMixin: + """Non-enumerable UUID primary key (safe to expose in URLs, unlike serial ints).""" + + id: Mapped[uuid.UUID] = mapped_column(GUID(), primary_key=True, default=uuid.uuid4) + + +class TimestampMixin: + """``created_at`` / ``updated_at``, both stored as UTC-aware timestamps.""" + + created_at: Mapped[datetime] = mapped_column(DateTime(timezone=True), default=utcnow, nullable=False) + updated_at: Mapped[datetime] = mapped_column( + DateTime(timezone=True), default=utcnow, onupdate=utcnow, nullable=False + ) + + +class SoftDeleteMixin: + """``deleted_at`` marker for soft deletes. Rows are hidden by filtering on it + rather than physically removed, so history is recoverable.""" + + deleted_at: Mapped[datetime | None] = mapped_column(DateTime(timezone=True), nullable=True, default=None) + + @property + def is_deleted(self) -> bool: + return self.deleted_at is not None + + +class TimestampedBase(UUIDPrimaryKeyMixin, TimestampMixin, SoftDeleteMixin, Base): + """Batteries-included base: UUID PK + created_at/updated_at + soft delete. + + Use this for most models; drop down to the individual mixins when a model needs + a different combination. + """ + + __abstract__ = True diff --git a/app/infrastructure/middleware/cors.py b/app/infrastructure/middleware/cors.py new file mode 100644 index 0000000..9b4671a --- /dev/null +++ b/app/infrastructure/middleware/cors.py @@ -0,0 +1,17 @@ +"""CORS configuration.""" + +from fastapi import FastAPI +from fastapi.middleware.cors import CORSMiddleware + +from app.core.config import get_settings + + +def configure_cors(app: FastAPI) -> None: + settings = get_settings() + app.add_middleware( + CORSMiddleware, + allow_origins=settings.cors_origins, # explicit allowlist, never "*" + allow_credentials=True, + allow_methods=["*"], + allow_headers=["*"], + ) diff --git a/app/infrastructure/middleware/error_handler.py b/app/infrastructure/middleware/error_handler.py new file mode 100644 index 0000000..e66960b --- /dev/null +++ b/app/infrastructure/middleware/error_handler.py @@ -0,0 +1,58 @@ +"""Exception handlers — every failure leaves the app as one JSON error envelope: + + {"error": {"code": "...", "message": "...", "details": {...}}} +""" + +from fastapi import FastAPI, Request, status +from fastapi.exceptions import RequestValidationError +from fastapi.responses import JSONResponse +from slowapi import _rate_limit_exceeded_handler +from slowapi.errors import RateLimitExceeded +from starlette.exceptions import HTTPException as StarletteHTTPException + +from app.core.exceptions import AppError +from app.core.logging import get_logger + +logger = get_logger(__name__) + + +def _envelope(code: str, message: str, details: dict | None = None) -> dict: + return {"error": {"code": code, "message": message, "details": details or {}}} + + +async def app_error_handler(request: Request, exc: AppError) -> JSONResponse: + return JSONResponse(status_code=exc.status_code, content=_envelope(exc.code, exc.message, exc.details)) + + +async def validation_error_handler(request: Request, exc: RequestValidationError) -> JSONResponse: + return JSONResponse( + status_code=status.HTTP_422_UNPROCESSABLE_CONTENT, + content=_envelope("VALIDATION_ERROR", "Request failed validation", {"errors": exc.errors()}), + ) + + +async def http_exception_handler(request: Request, exc: StarletteHTTPException) -> JSONResponse: + return JSONResponse(status_code=exc.status_code, content=_envelope("HTTP_ERROR", str(exc.detail))) + + +async def unhandled_exception_handler(request: Request, exc: Exception) -> JSONResponse: + # Never leak internals to the client, even in debug — debug=True only affects + # uvicorn/FastAPI's own tracebacks in logs, not what's sent over the wire here. + logger.error("unhandled_exception", exc_info=exc) + return JSONResponse( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + content=_envelope("INTERNAL_ERROR", "An unexpected error occurred"), + ) + + +def register_exception_handlers(app: FastAPI) -> None: + """Wire every exception type to its handler. Order is irrelevant — Starlette + dispatches on the most specific exception class.""" + # Handlers are typed to the concrete exception they handle; Starlette's signature + # expects Exception, hence the arg-type ignores (same pattern as the rate limiter). + app.add_exception_handler(AppError, app_error_handler) # type: ignore[arg-type] + app.add_exception_handler(RequestValidationError, validation_error_handler) # type: ignore[arg-type] + app.add_exception_handler(StarletteHTTPException, http_exception_handler) # type: ignore[arg-type] + # slowapi ships its own handler; reuse it so 429s keep the RateLimit-* headers. + app.add_exception_handler(RateLimitExceeded, _rate_limit_exceeded_handler) # type: ignore[arg-type] + app.add_exception_handler(Exception, unhandled_exception_handler) diff --git a/app/main.py b/app/main.py index 9650bc5..ed4cb6a 100644 --- a/app/main.py +++ b/app/main.py @@ -1,19 +1,12 @@ -from contextlib import asynccontextmanager - -from fastapi import FastAPI, Request, status -from fastapi.exceptions import RequestValidationError -from fastapi.middleware.cors import CORSMiddleware -from fastapi.responses import JSONResponse -from slowapi import _rate_limit_exceeded_handler -from slowapi.errors import RateLimitExceeded -from starlette.exceptions import HTTPException as StarletteHTTPException +from fastapi import FastAPI from app.api import health from app.api.v1.router import api_router from app.core.config import get_settings -from app.core.exceptions import AppError +from app.core.events import lifespan from app.core.logging import configure_logging, get_logger -from app.infrastructure.database.session import engine +from app.infrastructure.middleware.cors import configure_cors +from app.infrastructure.middleware.error_handler import register_exception_handlers from app.infrastructure.middleware.rate_limit import limiter from app.infrastructure.middleware.request_id import RequestIDMiddleware from app.infrastructure.middleware.security_headers import SecurityHeadersMiddleware @@ -22,15 +15,6 @@ logger = get_logger(__name__) settings = get_settings() - -@asynccontextmanager -async def lifespan(app: FastAPI): - logger.info("app.started", env=settings.environment) - yield - await engine.dispose() - logger.info("app.stopped") - - _is_prod = settings.environment == "production" app = FastAPI( @@ -42,53 +26,15 @@ async def lifespan(app: FastAPI): ) app.state.limiter = limiter -app.add_exception_handler(RateLimitExceeded, _rate_limit_exceeded_handler) # type: ignore[arg-type] # Middleware wrap requests outermost → innermost in reverse registration order. # We want: CORS (outer) → SecurityHeaders → RequestID (inner), so CORS headers are # present even on error responses. add_middleware prepends, so register inner first. app.add_middleware(RequestIDMiddleware) app.add_middleware(SecurityHeadersMiddleware) -app.add_middleware( - CORSMiddleware, - allow_origins=settings.cors_origins, # explicit allowlist, never "*" - allow_credentials=True, - allow_methods=["*"], - allow_headers=["*"], -) - - -def _envelope(code: str, message: str, details: dict | None = None) -> dict: - return {"error": {"code": code, "message": message, "details": details or {}}} - - -@app.exception_handler(AppError) -async def app_error_handler(request: Request, exc: AppError): - return JSONResponse(status_code=exc.status_code, content=_envelope(exc.code, exc.message, exc.details)) - - -@app.exception_handler(RequestValidationError) -async def validation_error_handler(request: Request, exc: RequestValidationError): - return JSONResponse( - status_code=status.HTTP_422_UNPROCESSABLE_CONTENT, - content=_envelope("VALIDATION_ERROR", "Request failed validation", {"errors": exc.errors()}), - ) - - -@app.exception_handler(StarletteHTTPException) -async def http_exception_handler(request: Request, exc: StarletteHTTPException): - return JSONResponse(status_code=exc.status_code, content=_envelope("HTTP_ERROR", str(exc.detail))) - +configure_cors(app) # added last → outermost -@app.exception_handler(Exception) -async def unhandled_exception_handler(request: Request, exc: Exception): - # Never leak internals to the client, even in debug — debug=True only affects - # uvicorn/FastAPI's own tracebacks in logs, not what's sent over the wire here. - logger.error("unhandled_exception", exc_info=exc) - return JSONResponse( - status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, - content=_envelope("INTERNAL_ERROR", "An unexpected error occurred"), - ) +register_exception_handlers(app) @app.get("/") diff --git a/app/modules/auth/models.py b/app/modules/auth/models.py index b6c3303..69d9453 100644 --- a/app/modules/auth/models.py +++ b/app/modules/auth/models.py @@ -4,7 +4,8 @@ from sqlalchemy import Boolean, DateTime, ForeignKey, String from sqlalchemy.orm import Mapped, mapped_column, relationship -from app.infrastructure.database.base import GUID, TimestampedBase +from app.infrastructure.database.base import GUID +from app.infrastructure.database.mixins import TimestampedBase class RefreshToken(TimestampedBase): diff --git a/app/modules/users/models.py b/app/modules/users/models.py index ef9ab7f..088a8aa 100644 --- a/app/modules/users/models.py +++ b/app/modules/users/models.py @@ -3,7 +3,7 @@ from sqlalchemy import Boolean, Enum, String from sqlalchemy.orm import Mapped, mapped_column, relationship -from app.infrastructure.database.base import TimestampedBase +from app.infrastructure.database.mixins import TimestampedBase class Role(str, enum.Enum):