diff --git a/README.md b/README.md index caca45b..3637a3f 100644 --- a/README.md +++ b/README.md @@ -1,11 +1,13 @@ # modal-template-fastapi -> A production-ready FastAPI template deployed serverlessly on Modal — JWT authentication, full CRUD API, 79-test pytest suite, and automated CI/CD. +> A production-ready FastAPI template deployed serverlessly on Modal — Hexagonal Architecture, JWT authentication with scope-based authorisation, paginated CRUD API, telemetry sidecar pattern, OpenTelemetry observability with Grafana Cloud, 145-test pytest suite, and automated CI/CD. [![Python](https://img.shields.io/badge/Python-3.11-4f46e5?style=flat-square&logo=python&logoColor=white)](https://python.org) [![FastAPI](https://img.shields.io/badge/FastAPI-latest-009688?style=flat-square&logo=fastapi&logoColor=white)](https://fastapi.tiangolo.com) [![Modal](https://img.shields.io/badge/Modal-Serverless-7c3aed?style=flat-square)](https://modal.com) -[![Tests](https://img.shields.io/badge/Tests-79%20passing-15803d?style=flat-square&logo=pytest&logoColor=white)](tests/) +[![Tests](https://img.shields.io/badge/Tests-145%20passing-15803d?style=flat-square&logo=pytest&logoColor=white)](tests/) +[![OpenTelemetry](https://img.shields.io/badge/OpenTelemetry-enabled-f5a800?style=flat-square&logo=opentelemetry&logoColor=white)](src/infrastructure/) +[![Grafana](https://img.shields.io/badge/Grafana-Cloud-f46800?style=flat-square&logo=grafana&logoColor=white)](docs/OBSERVABILITY.md) [![Docs](https://img.shields.io/badge/Docs-GitHub%20Pages-4f46e5?style=flat-square&logo=materialformkdocs&logoColor=white)](https://your-org.github.io/modal-template-fastapi/) [![License](https://img.shields.io/badge/License-MIT-6b7280?style=flat-square)](LICENSE) @@ -15,18 +17,23 @@ ## What is this? -`modal-template-fastapi` is a battle-tested starting point for shipping Python APIs to the cloud. It combines the ergonomics of FastAPI with Modal's serverless platform — giving you autoscaling, secret management, persistent volumes, and environment-aware deployments without managing any infrastructure. +`modal-template-fastapi` is a battle-tested starting point for shipping Python microservices to the cloud. It combines the ergonomics of FastAPI with Modal's serverless platform — giving you autoscaling, secret management, persistent volumes, and environment-aware deployments without managing any infrastructure. -Clone it, swap in your business logic, and ship in minutes. +The template is built on **Hexagonal Architecture (Ports & Adapters)** so every layer — persistence, telemetry, HTTP — can be swapped independently without touching business logic. Clone it, drop in your domain, wire your adapter, and ship. **Key features:** -- 🔒 JWT Bearer authentication with typed error codes (`MISSING_TOKEN`, `TOKEN_EXPIRED`, `INVALID_TOKEN`) -- 🗄️ Full CRUD REST API with Pydantic v2 validation and `session_id` tracing on every response -- ☁️ Modal serverless deployment with `feat` / `dev` / `prod` environments via a declarative `EnvConfig` dataclass -- ✅ 79-test pytest suite — isolated, no external services, runs in < 1 second -- 🚀 GitHub Actions CI/CD — tests gate every deploy (`needs: test`) -- 📖 MkDocs Material documentation hosted on GitHub Pages +- 🏛️ **Hexagonal Architecture** — clean `core/ports/services`, concrete `adapters/`, single wiring point in `deps.py` +- 🔒 **JWT auth + scope-based authorisation** — `require_scope("items:write")` dependency, typed error codes +- 📄 **Paginated CRUD API** — typed `ItemRequest`, `PaginatedItemsResponse` with `total`, `has_more`, `limit`, `offset` +- 🔭 **Telemetry sidecar** — `TracingProxy` wraps any adapter generically; domain and adapters have zero OTel knowledge +- 📡 **OpenTelemetry** — distributed traces, 5 HTTP metrics, log-trace correlation, cold-start tracking +- 📊 **Grafana Cloud** — OTLP/HTTP push, zero idle cost, enabled per environment via Modal Secret +- ☁️ **Modal serverless** — `feat` / `dev` / `prod` environments via `EnvConfig`, memory snapshots for fast cold starts +- 🌐 **CORS per-environment** — `cors_origins` field per `EnvConfig`, propagated through env vars +- 🔌 **Platform-decoupled** — `src/` has zero Modal imports; runs on any ASGI host unchanged +- ✅ **145-test pytest suite** — isolated per-test repositories, no stubs, runs in < 0.3 seconds +- 🚀 **GitHub Actions CI/CD** — branch-based environment routing, tests gate every deploy --- @@ -35,24 +42,90 @@ Clone it, swap in your business logic, and ship in minutes. ``` modal-template-fastapi/ ├── src/ -│ ├── main.py # FastAPI app, CORS, exception handlers -│ └── api/ -│ ├── auth.py # JWT verification + FastAPI dependency -│ ├── handler.py # CRUD logic (swap for your DB here) -│ ├── models.py # Pydantic v2 request/response models -│ └── routes.py # All 5 API endpoints -├── tests/ # pytest suite (79 tests) -├── modal_app.py # Modal app + ASGI wrapper -├── modal_common.py # EnvConfig, container image, environment registry -├── swagger.yaml # OpenAPI 3.0 spec +│ ├── main.py # FastAPI app, CORS, exception handlers +│ ├── deps.py # Wiring: adapter → service, startup validation +│ ├── core/ # Pure business logic — zero I/O imports +│ │ ├── domain/ +│ │ │ └── item.py # Item entity (replace with your domain) +│ │ ├── ports/ +│ │ │ └── item_repository.py # ItemRepository Protocol (the contract) +│ │ └── services/ +│ │ └── item_service.py # Business logic, depends only on ports +│ ├── adapters/ # Concrete implementations — know about core, not HTTP +│ │ ├── http/ +│ │ │ └── middleware.py # TelemetryMiddleware — inbound HTTP sidecar +│ │ ├── persistence/ +│ │ │ └── memory.py # InMemoryItemRepository (replace with Postgres, Dynamo, etc.) +│ │ └── telemetry/ +│ │ └── tracing_proxy.py # TracingProxy — generic outbound port sidecar +│ ├── api/ # HTTP layer — routes, schemas, auth +│ │ ├── auth.py # JWT verification + get_current_user + require_scope +│ │ ├── models.py # Pydantic v2 request/response schemas +│ │ └── routes.py # API endpoints — delegates to ItemService +│ ├── infrastructure/ # Cross-cutting setup — runs once at startup +│ │ ├── setup.py # OTel SDK bootstrap, OTLP exporters, Grafana auth +│ │ └── metrics.py # Domain metric instruments stub (replace per service) +│ └── utils/ +│ └── config.py # App constants read from env vars +├── tests/ # pytest suite (145 tests) +│ ├── conftest.py # Fixtures: fresh repo per test, JWT helpers, scope tokens +│ ├── test_auth.py # JWT authentication tests +│ ├── test_auth_scopes.py # Scope-based authorisation tests +│ ├── test_health.py # Health endpoint tests +│ ├── test_http_middleware.py # TelemetryMiddleware behaviour tests +│ ├── test_infrastructure_setup.py # OTel SDK bootstrap tests +│ ├── test_items.py # CRUD + pagination tests +│ └── test_models.py # Pydantic model validation tests +├── modal_app.py # Modal Cls, @enter telemetry init, ASGI wrapper +├── modal_common.py # EnvConfig, container image, environment registry +├── scripts/ +│ └── jwt_token_generator.py # CLI tool: generate JWT tokens for testing +├── docs/OBSERVABILITY.md # Grafana Cloud setup + PromQL queries +├── swagger.yaml # OpenAPI 3.0 spec └── .github/workflows/ - ├── app-testing.yml # Reusable pytest CI - ├── modal-deploy.yml # Test → deploy pipeline - └── docs.yml # Documentation → GitHub Pages + ├── app-testing.yml # Reusable pytest CI + ├── modal-deploy.yml # Test → deploy pipeline + └── docs.yml # Documentation → GitHub Pages ``` --- +## Architecture + +The template is built on **Hexagonal Architecture**. The dependency rule is strict — everything points inward toward `core/`. Adapters know about `core/`, but `core/` never knows about adapters. + +``` + ┌─────────────────────────────────────┐ + │ src/infrastructure/ │ + │ OTel SDK setup — runs once │ + └────────────────┬────────────────────┘ + │ get_tracer() / get_meter() + ┌─────────────────────────▼─────────────────────────────────┐ + │ src/adapters/ │ + │ │ + │ http/middleware.py ← TelemetryMiddleware (HTTP sidecar)│ + │ telemetry/tracing_proxy.py ← TracingProxy (port sidecar) │ + │ persistence/memory.py ← InMemoryItemRepository │ + └──────────────────────┬────────────────────────────────────┘ + │ ItemRepository (port) + ┌──────────────────────▼─────────────────────────────────────┐ + │ src/core/ │ + │ │ + │ domain/item.py ← pure Item entity │ + │ ports/item_repository.py ← abstract Protocol │ + │ services/item_service.py ← business logic │ + └────────────────────────────────────────────────────────────┘ +``` + +**Swapping persistence** (e.g. Postgres) requires 3 steps: +1. Add `src/adapters/persistence/postgres.py` +2. Add one `elif` branch in `src/deps.py` +3. Set `PERSISTENCE_BACKEND=postgres` + `DATABASE_URL=...` in your Modal secret + +No routes, services, domain, or telemetry files change. + +--- + ## Local Development ```bash @@ -67,7 +140,7 @@ pytest tests/ -v # 3. Authenticate with Modal (first time only) modal setup -# 4. Start the API locally (no cloud deployment needed) +# 4. Start the API locally MODAL_ENV=dev modal run modal_app.py # → http://localhost:8000/api/v1/health ``` @@ -90,33 +163,158 @@ git push origin feat/my-feature --- -## API at a Glance +## API Reference + +| Method | Endpoint | Auth | Scope | Description | +|---|---|---|---|---| +| `GET` | `/api/v1/health` | None | — | Service health check | +| `GET` | `/api/v1/items` | JWT | — | List items (paginated) | +| `GET` | `/api/v1/items/{id}` | JWT | — | Get a single item | +| `POST` | `/api/v1/items` | JWT | `items:write` | Create an item | +| `PUT` | `/api/v1/items/{id}` | JWT | `items:write` | Update an item | +| `DELETE` | `/api/v1/items/{id}` | JWT | `items:write` | Delete an item | + +**Paginated list response:** +```json +{ + "items": [...], + "total": 42, + "limit": 10, + "offset": 0, + "has_more": true +} +``` -| Method | Endpoint | Auth | Description | -|---|---|---|---| -| `GET` | `/api/v1/health` | None | Service health check | -| `GET` | `/api/v1/items` | JWT | List all items | -| `POST` | `/api/v1/items` | JWT | Create an item | -| `GET` | `/api/v1/items/{id}` | JWT | Get a single item | -| `PUT` | `/api/v1/items/{id}` | JWT | Replace an item | -| `DELETE` | `/api/v1/items/{id}` | JWT | Delete an item | +**JWT scopes:** include `"scopes": ["items:read", "items:write"]` in your JWT payload. Read endpoints accept any valid token; write endpoints require `items:write`. -> Full endpoint reference with curl examples, sequence diagrams, and a live Swagger UI: [API docs →](https://your-org.github.io/modal-template-fastapi/api/endpoints/) +Generate a token for testing: +```bash +python scripts/jwt_token_generator.py --secret +``` + +> Full endpoint reference with curl examples and Swagger UI: [API docs →](https://your-org.github.io/modal-template-fastapi/api/endpoints/) --- -## Documentation +## Authentication & Authorisation + +The template uses **JWT Bearer tokens** with scope-based authorisation. + +```python +# Any valid token — read access +current_user: dict = Depends(get_current_user) + +# Requires "items:write" scope in JWT payload +current_user: dict = Depends(require_scope("items:write")) +``` + +**JWT payload shape:** +```json +{ + "sub": "user@example.com", + "exp": 1234567890, + "scopes": ["items:read", "items:write"] +} +``` + +**Error codes:** `MISSING_TOKEN` (401), `TOKEN_EXPIRED` (401), `INVALID_TOKEN` (401), `INSUFFICIENT_SCOPE` (403). + +**Modal secret setup:** +```bash +modal secret create fastapi-auth-secrets \ + JWT_SECRET= \ + JWT_ALGORITHM=HS256 +``` + +--- + +## Observability + +Telemetry is a **sidecar** — domain and adapters have zero OTel knowledge. Two sidecars cover all signals automatically: -The full documentation is hosted on GitHub Pages and covers: +| Sidecar | Where | What it adds | +|---|---|---| +| `TelemetryMiddleware` | HTTP boundary | Request span, duration, status, session ID | +| `TracingProxy` | Port boundary | Child span per repository method call | + +**Signals recorded automatically — zero per-route code:** + +| Signal | Detail | +|---|---| +| Distributed trace | Span per request — method, route template, status, client IP, session ID | +| Request rate | `http.server.request.count` counter | +| Latency p50/p95/p99 | `http.server.request.duration` histogram (ms) | +| Error rate | `http.server.error.count` counter (4xx + 5xx) | +| In-flight requests | `http.server.active_requests` up-down counter | +| Cold starts | `app.container.cold_start.count` — spikes = containers scaling up | +| Log correlation | Every log line carries `otelTraceID` + `otelSpanID` | +| W3C trace propagation | Incoming `traceparent` headers respected — spans become children of upstream traces | + +**Environment strategy:** `feat` and `prod` push telemetry; `dev` is silent (zero cost). Controlled by the `grafana-otlp` Modal Secret — no code changes needed. + +**One-time Grafana Cloud setup:** +```bash +modal secret create grafana-otlp \ + GRAFANA_INSTANCE_ID= \ + GRAFANA_OTLP_TOKEN= \ + GRAFANA_OTLP_ENDPOINT=.grafana.net/otlp> +``` + +> Full setup guide, PromQL queries, and copy-to-new-service checklist: [OBSERVABILITY.md →](docs/OBSERVABILITY.md) + +--- + +## Adapting This Template + +### Replace the domain + +1. Edit `src/core/domain/item.py` — add your typed fields, update `to_dict()` +2. Edit `src/core/services/item_service.py` — adjust business logic +3. Edit `src/api/models.py` — update `ItemRequest` with your request schema +4. Edit `src/api/routes.py` — update route handlers + +### Add a persistence backend + +1. Create `src/adapters/persistence/.py` — implement `get`, `list`, `create`, `update`, `delete` +2. Add the backend to `_REQUIRED_ENV` in `src/deps.py` (with its required env vars) +3. Add an `elif backend == ""` branch in `_get_repository()` +4. Set `PERSISTENCE_BACKEND=` in your Modal secret + +### Add a new service + +1. Add domain entity + port + service under `src/core/` +2. Add adapter under `src/adapters/` +3. Add a `get_()` dependency in `src/deps.py` +4. Wire into routes via `Depends(get_())` + +`TracingProxy` covers telemetry for any new service automatically — no new observability files needed. + +--- + +## CORS Configuration + +CORS origins are configured per environment in `modal_common.py`: + +```python +FEAT = EnvConfig(cors_origins=["*"]) # replace with your Modal domain +PROD = EnvConfig(cors_origins=["*"]) # replace with your Modal domain +``` + +Replace `["*"]` with your actual Modal app URL (e.g. `["https://---prod.modal.run"]`) when your domains are known. The value propagates through `configure_env_vars` → `CORS_ORIGINS` env var → `src/utils/config.py` — no other files need touching. + +--- + +## Documentation | Section | Contents | |---|---| | [Architecture](https://your-org.github.io/modal-template-fastapi/architecture/) | Component diagrams, request lifecycle, module reference | -| [API Reference](https://your-org.github.io/modal-template-fastapi/api/endpoints/) | Endpoints, curl examples, sequence diagrams, Swagger UI | -| [Authentication](https://your-org.github.io/modal-template-fastapi/api/auth/) | JWT flow, error codes, token generation | +| [API Reference](https://your-org.github.io/modal-template-fastapi/api/endpoints/) | Endpoints, curl examples, pagination, Swagger UI | +| [Authentication](https://your-org.github.io/modal-template-fastapi/api/auth/) | JWT flow, scopes, error codes, token generation | | [Testing](https://your-org.github.io/modal-template-fastapi/testing/) | Fixture reference, test matrix, isolation strategy | | [CI/CD](https://your-org.github.io/modal-template-fastapi/cicd/) | Workflow diagrams, trigger matrix, required secrets | | [Deployment](https://your-org.github.io/modal-template-fastapi/deployment/) | EnvConfig reference, environment matrix, operational runbook | +| [Observability](docs/OBSERVABILITY.md) | Grafana Cloud setup, metrics reference, PromQL queries | --- diff --git a/modal_app.py b/modal_app.py index 444102a..52b6ba2 100644 --- a/modal_app.py +++ b/modal_app.py @@ -2,7 +2,7 @@ import modal -from modal_common import build_fastapi_config, get_env_config +from modal_common import build_fastapi_config, configure_env_vars, get_env_config # SETTING MODAL ENVIRONMENT MODAL_ENV = os.environ.get("MODAL_ENV", "dev") @@ -14,11 +14,7 @@ APP_NAME = f"{env_config.app_name}-{env_config.env_name}" app = modal.App(APP_NAME) -_otlp_endpoint = os.environ.get("GRAFANA_OTLP_ENDPOINT") or env_config.otel_endpoint -if _otlp_endpoint: - os.environ.setdefault("OTEL_EXPORTER_OTLP_ENDPOINT", _otlp_endpoint) -os.environ.setdefault("OTEL_SERVICE_NAME", env_config.service_name or env_config.app_name) -os.environ.setdefault("MODAL_ENV", env_config.env_name) +configure_env_vars(env_config) # SETTING MODAL PROJECT @@ -38,8 +34,9 @@ def startup(self) -> None: # Runs once per container after snapshot restore — never on the request hot path. # Network-bound setup (OTLP connections) must live here; they cannot survive # a snapshot because file descriptors and sockets are not portable across restores. - from src.observability import setup_telemetry + from src.infrastructure import record_cold_start, setup_telemetry setup_telemetry() + record_cold_start() # fires against the real MeterProvider — always exported @modal.asgi_app() def fastapi_app(self): @@ -52,7 +49,7 @@ def fastapi_app(self): def main(): # Mirror what @enter does in the Modal container so telemetry works locally too. # The env vars above are already set; setup_telemetry() reads them at call time. - from src.observability import setup_telemetry + from src.infrastructure import setup_telemetry setup_telemetry() from src.main import app as fastapi_app from uvicorn import run diff --git a/modal_common.py b/modal_common.py index 80270fb..bea00b8 100644 --- a/modal_common.py +++ b/modal_common.py @@ -1,3 +1,4 @@ +import os from dataclasses import dataclass, field from typing import Dict, List, Optional @@ -78,6 +79,9 @@ class EnvConfig: secrets: list = field(default_factory=list) volumes: Dict[str, modal.Volume] = field(default_factory=lambda: FASTAPI_VOLUME) + # CORS — restrict origins per environment; ["*"] allows all (dev only) + cors_origins: List[str] = field(default_factory=lambda: ["*"]) + # OBSERVABILITY — set in prod preset only; None = telemetry disabled (feat/dev) otel_endpoint: Optional[str] = None # Grafana Cloud OTLP base URL service_name: Optional[str] = None # defaults to app_name when None @@ -86,6 +90,7 @@ class EnvConfig: FEAT = EnvConfig( env_name="feat", server_domain="feat-app.modal.run", + cors_origins=["*"], otel_endpoint=None, # endpoint comes from GRAFANA_OTLP_ENDPOINT inside the grafana-otlp secret secrets=[ modal.Secret.from_name("fastapi-auth-secrets"), @@ -96,6 +101,7 @@ class EnvConfig: DEV = EnvConfig( env_name="dev", server_domain="dev-app.modal.run", + cors_origins=["*"], # TODO: replace with your actual Modal domain when known otel_endpoint=None, # no telemetry in dev — keeps cost at zero secrets=[ modal.Secret.from_name("fastapi-auth-secrets"), @@ -106,6 +112,7 @@ class EnvConfig: PROD = EnvConfig( env_name="prod", server_domain="prod-app.modal.run", + cors_origins=["*"], # TODO: replace with your actual Modal domain when known # min_containers=1, # Uncomment to keep 1 warm container in production otel_endpoint=None, # endpoint comes from GRAFANA_OTLP_ENDPOINT inside the grafana-otlp secret secrets=[ @@ -131,6 +138,20 @@ def get_env_config(env_name: str) -> EnvConfig: return ENV_CONFIGS[env_name] +def configure_env_vars(env: EnvConfig) -> None: + _otlp_endpoint = os.environ.get("GRAFANA_OTLP_ENDPOINT") or env.otel_endpoint + if _otlp_endpoint: + os.environ.setdefault("OTEL_EXPORTER_OTLP_ENDPOINT", _otlp_endpoint) + os.environ.setdefault("OTEL_SERVICE_NAME", env.service_name or env.app_name) + os.environ.setdefault("MODAL_ENV", env.env_name) + # App metadata — read by src/config.py so src/ never imports modal_common directly + os.environ.setdefault("APP_NAME", env.app_name) + os.environ.setdefault("APP_VERSION", env.app_version) + os.environ.setdefault("APP_DESCRIPTION", env.app_description) + os.environ.setdefault("SERVER_PREFIX", env.server_prefix) + os.environ.setdefault("CORS_ORIGINS", ",".join(env.cors_origins)) + + def build_fastapi_config(env: EnvConfig) -> dict: config = { "image": cpu_image, diff --git a/scripts/jwt_token_generator.py b/scripts/jwt_token_generator.py new file mode 100644 index 0000000..b9495f5 --- /dev/null +++ b/scripts/jwt_token_generator.py @@ -0,0 +1,18 @@ +import jwt +import time +import argparse + +def get_jwt_token(secret: str, scopes: list[str] = None) -> str: + payload = { + "sub": "test-user", + "exp": int(time.time()) + 86400, + "scopes": scopes or ["items:read", "items:write"], + } + return jwt.encode(payload, secret, algorithm="HS256") + +if __name__ == "__main__": + parser = argparse.ArgumentParser() + parser.add_argument("--secret", type=str, required=True) + parser.add_argument("--scopes", type=str, required=False) + args = parser.parse_args() + print(get_jwt_token(args.secret)) \ No newline at end of file diff --git a/src/adapters/__init__.py b/src/adapters/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/src/adapters/http/__init__.py b/src/adapters/http/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/src/observability/middleware.py b/src/adapters/http/middleware.py similarity index 74% rename from src/observability/middleware.py rename to src/adapters/http/middleware.py index 2b30e07..8452180 100644 --- a/src/observability/middleware.py +++ b/src/adapters/http/middleware.py @@ -1,10 +1,11 @@ """ -src/observability/middleware.py +src/adapters/http/middleware.py ================================ -FastAPI/Starlette middleware that auto-instruments every HTTP request. +Inbound HTTP adapter — telemetry sidecar at the HTTP boundary. +Wraps every request in an OTel span, records metrics, and emits structured logs. Add once in main.py — no per-route changes needed. - from src.observability.middleware import TelemetryMiddleware + from src.adapters.http.middleware import TelemetryMiddleware app.add_middleware(TelemetryMiddleware) What is recorded automatically per request @@ -47,53 +48,51 @@ from starlette.responses import Response from starlette.routing import Match -from src.observability.setup import get_meter, get_tracer +from src.infrastructure.setup import get_meter, get_tracer logger = logging.getLogger(__name__) -# ── Instruments ──────────────────────────────────────────────────────────── -# Created at import time against whatever MeterProvider is set. -# If setup_telemetry() hasn't been called yet (e.g. in tests), these -# bind to the global no-op provider and become true no-ops. - -_meter = get_meter() - -_request_count = _meter.create_counter( - "http.server.request.count", - description="Total HTTP requests received", - unit="1", -) -_request_duration = _meter.create_histogram( - "http.server.request.duration", - description="HTTP request wall-clock duration", - unit="ms", -) -_active_requests = _meter.create_up_down_counter( - "http.server.active_requests", - description="HTTP requests currently in flight", - unit="1", -) -_error_count = _meter.create_counter( - "http.server.error.count", - description="HTTP responses with 4xx or 5xx status", - unit="1", -) -_response_size = _meter.create_histogram( - "http.server.response.size", - description="HTTP response body size", - unit="By", -) - -# ── Cold-start counter ───────────────────────────────────────────────────── -# Incremented once at module import, which happens during @enter warmup. -# A spike in this counter in Grafana = containers cold-starting. - -_cold_starts = _meter.create_counter( - "app.container.cold_start.count", - description="Number of container cold starts (module imports)", - unit="1", -) -_cold_starts.add(1, {"service": "modal-fastapi"}) +# ── Lazy instrument initialisation ───────────────────────────────────────── +# Instruments are created on the first request, not at module import time. +# This guarantees they are bound to the real MeterProvider set by +# setup_telemetry() in Modal's startup() hook — not to the no-op provider +# that exists at snapshot / import time. + +_instruments: dict | None = None + + +def _get_instruments() -> dict: + global _instruments + if _instruments is None: + meter = get_meter() + _instruments = { + "request_count": meter.create_counter( + "http.server.request.count", + description="Total HTTP requests received", + unit="1", + ), + "request_duration": meter.create_histogram( + "http.server.request.duration", + description="HTTP request wall-clock duration", + unit="ms", + ), + "active_requests": meter.create_up_down_counter( + "http.server.active_requests", + description="HTTP requests currently in flight", + unit="1", + ), + "error_count": meter.create_counter( + "http.server.error.count", + description="HTTP responses with 4xx or 5xx status", + unit="1", + ), + "response_size": meter.create_histogram( + "http.server.response.size", + description="HTTP response body size", + unit="By", + ), + } + return _instruments # ── Helpers ──────────────────────────────────────────────────────────────── @@ -135,10 +134,11 @@ async def dispatch( request: Request, call_next: RequestResponseEndpoint, ) -> Response: - tracer = get_tracer() - route = _route_template(request) - method = request.method - t_start = time.perf_counter() + tracer = get_tracer() + instr = _get_instruments() + route = _route_template(request) + method = request.method + t_start = time.perf_counter() # Generate once per request so the span always has a non-empty session_id. # setdefault below lets a route override this with its own value if it sets # the x-session-id header explicitly — the middleware value is the fallback. @@ -146,7 +146,7 @@ async def dispatch( # ── In-flight gauge ─────────────────────────────────────────── try: - _active_requests.add(1, {"method": method, "route": route}) + instr["active_requests"].add(1, {"method": method, "route": route}) except Exception: pass @@ -178,8 +178,8 @@ async def dispatch( except Exception as exc: span.record_exception(exc) span.set_status(StatusCode.ERROR, str(exc)) - _record(method, route, 500, (time.perf_counter() - t_start) * 1000, 0) - _active_requests.add(-1, {"method": method, "route": route}) + _record(instr, method, route, 500, (time.perf_counter() - t_start) * 1000, 0) + instr["active_requests"].add(-1, {"method": method, "route": route}) raise # ── Annotate span ───────────────────────────────────────── @@ -201,7 +201,7 @@ async def dispatch( # ── Record metrics ──────────────────────────────────────── duration_ms = (time.perf_counter() - t_start) * 1000 content_length = int(response.headers.get("content-length", 0)) - _record(method, route, status, duration_ms, content_length) + _record(instr, method, route, status, duration_ms, content_length) # ── Structured log ──────────────────────────────────────── ctx = span.get_span_context() @@ -221,11 +221,12 @@ async def dispatch( }, ) - _active_requests.add(-1, {"method": method, "route": route}) + instr["active_requests"].add(-1, {"method": method, "route": route}) return response def _record( + instr: dict, method: str, route: str, status: int, @@ -235,11 +236,11 @@ def _record( """Record all per-request metrics. Swallows exceptions silently.""" try: lb = _labels(method, route, status) - _request_count.add(1, lb) - _request_duration.record(duration_ms, lb) + instr["request_count"].add(1, lb) + instr["request_duration"].record(duration_ms, lb) if status >= 400: - _error_count.add(1, lb) + instr["error_count"].add(1, lb) if response_bytes > 0: - _response_size.record(response_bytes, lb) + instr["response_size"].record(response_bytes, lb) except Exception: - logger.debug("Metric recording failed", exc_info=True) \ No newline at end of file + logger.debug("Metric recording failed", exc_info=True) diff --git a/src/adapters/persistence/__init__.py b/src/adapters/persistence/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/src/adapters/persistence/memory.py b/src/adapters/persistence/memory.py new file mode 100644 index 0000000..bb12e44 --- /dev/null +++ b/src/adapters/persistence/memory.py @@ -0,0 +1,42 @@ +""" +src/adapters/persistence/memory.py +==================================== +In-memory persistence adapter — ships with the template. + +Satisfies ItemRepository Protocol structurally (no inheritance needed). +Replace or supplement with a concrete DB adapter in src/deps.py. + +Thread-safety note: sufficient for Modal's single-threaded async workers. +For multi-threaded environments, protect _store with asyncio.Lock. +""" +from __future__ import annotations + +from typing import Dict, List, Optional, Tuple + +from src.core.domain.item import Item + + +class InMemoryItemRepository: + def __init__(self) -> None: + self._store: Dict[str, Item] = {} + + async def get(self, item_id: str) -> Optional[Item]: + return self._store.get(item_id) + + async def list(self, limit: int = 100, offset: int = 0) -> Tuple[List[Item], int]: + all_items = list(self._store.values()) + total = len(all_items) + return all_items[offset: offset + limit], total + + async def create(self, item: Item) -> Item: + self._store[item.id] = item + return item + + async def update(self, item: Item) -> Optional[Item]: + if item.id not in self._store: + return None + self._store[item.id] = item + return item + + async def delete(self, item_id: str) -> bool: + return self._store.pop(item_id, None) is not None diff --git a/src/adapters/telemetry/__init__.py b/src/adapters/telemetry/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/src/adapters/telemetry/tracing_proxy.py b/src/adapters/telemetry/tracing_proxy.py new file mode 100644 index 0000000..84e1fdb --- /dev/null +++ b/src/adapters/telemetry/tracing_proxy.py @@ -0,0 +1,66 @@ +""" +src/adapters/telemetry/tracing_proxy.py +========================================= +Generic telemetry sidecar for any async adapter. + +Wraps any repository (or service) and intercepts every async method call, +creating a child OTel span around it. Neither the real adapter nor the service +layer need to know telemetry exists. + +Usage in deps.py: + from src.adapters.telemetry.tracing_proxy import TracingProxy + + raw = InMemoryItemRepository() + return TracingProxy(raw, prefix="repository.item") + + # Works unchanged for any backend: + raw = PostgresItemRepository(os.environ["DATABASE_URL"]) + return TracingProxy(raw, prefix="repository.item") + +Span naming: + prefix + "." + method_name + e.g. "repository.item.get", "repository.item.create" + +Span attributes: + Method-level only (name, prefix). For richer attribute sets + (item IDs, result counts, cache hits), graduate to an explicit + typed wrapper for that specific port. + +Performance: + First call per method: __getattr__ lookup + closure allocation (~300ns) + Subsequent calls: direct attribute access on cached closure (~50ns) + Span creation: ~1-3µs (OTel SDK, non-blocking) + Export: zero — BatchSpanProcessor flushes in background thread +""" +from __future__ import annotations + +import asyncio + +from src.infrastructure.setup import get_tracer + + +class TracingProxy: + """ + Generic async telemetry sidecar. Thread-safe and reusable across requests. + Wraps any object whose async methods should be traced as child spans. + """ + + def __init__(self, wrapped: object, prefix: str) -> None: + object.__setattr__(self, "_wrapped", wrapped) + object.__setattr__(self, "_prefix", prefix) + + def __getattr__(self, name: str): + method = getattr(object.__getattribute__(self, "_wrapped"), name) + + if not asyncio.iscoroutinefunction(method): + return method + + prefix = object.__getattribute__(self, "_prefix") + + async def _traced(*args, **kwargs): + with get_tracer().start_as_current_span(f"{prefix}.{name}"): + return await method(*args, **kwargs) + + # Cache on the instance so subsequent calls bypass __getattr__ entirely. + object.__setattr__(self, name, _traced) + return _traced diff --git a/src/api/auth.py b/src/api/auth.py index dd0762d..32f5e0b 100644 --- a/src/api/auth.py +++ b/src/api/auth.py @@ -67,3 +67,35 @@ def get_current_user( headers={"WWW-Authenticate": "Bearer"}, ) return verify_token(credentials.credentials) + + +def require_scope(*scopes: str): + """ + FastAPI dependency factory — enforces JWT scope claims on a route. + + Usage: + @router.post("/items", ...) + async def create_item( + current_user: dict = Depends(require_scope("items:write")), + ): ... + + The JWT must include a "scopes" claim (list of strings) containing all + required scopes. Returns 403 if any scope is missing, 401 if token invalid. + + Scopes are additive — Depends(require_scope("items:read", "items:write")) + requires both scopes to be present. + """ + def _check(current_user: dict = Depends(get_current_user)) -> dict: + token_scopes = set(current_user.get("scopes", [])) + missing = set(scopes) - token_scopes + if missing: + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail=ErrorDetail( + detail=f"Missing required scopes: {', '.join(sorted(missing))}", + session_id=str(uuid.uuid4()), + error_code="INSUFFICIENT_SCOPE", + ).model_dump(), + ) + return current_user + return _check diff --git a/src/api/handler.py b/src/api/handler.py deleted file mode 100644 index b819c3c..0000000 --- a/src/api/handler.py +++ /dev/null @@ -1,42 +0,0 @@ -import uuid -from typing import Any, Dict, List, Optional - -from src.api.models import GenericRequest - -_store: Dict[str, Dict[str, Any]] = {} - - -def create_item(request: GenericRequest) -> Dict[str, Any]: - item_id = str(uuid.uuid4()) - _store[item_id] = { - "id": item_id, - "project_id": request.project_id, - **request.data, - } - return _store[item_id] - - -def get_item(item_id: str) -> Optional[Dict[str, Any]]: - return _store.get(item_id) - - -def list_items() -> List[Dict[str, Any]]: - return list(_store.values()) - - -def update_item(item_id: str, request: GenericRequest) -> Optional[Dict[str, Any]]: - if item_id not in _store: - return None - _store[item_id] = { - "id": item_id, - "project_id": request.project_id, - **request.data, - } - return _store[item_id] - - -def delete_item(item_id: str) -> bool: - if item_id not in _store: - return False - del _store[item_id] - return True diff --git a/src/api/models.py b/src/api/models.py index 146ca4c..3ca3959 100644 --- a/src/api/models.py +++ b/src/api/models.py @@ -33,28 +33,35 @@ class ErrorDetail(BaseModel): # --- Request/Response Models --- -class GenericRequest(BaseModel): - """Generic request model for data operations.""" - data: Dict[str, Any] = Field( - ..., - description="Request data payload", - json_schema_extra={'example': {"name": "example", "value": 123}} - ) +class ItemRequest(BaseModel): + """ + Typed request for item create/update operations. + Replace these fields with your domain-specific schema. + """ + name: str = Field(..., min_length=1, description="Item name") + description: Optional[str] = Field(None, description="Optional description") project_id: Optional[str] = Field( - default=None, + None, min_length=1, - description="The project ID for tracking and organization.", - json_schema_extra={'example': "my-project-123"} + description="Project identifier for grouping and tracking", + json_schema_extra={"example": "my-project-123"}, ) + class ItemResponse(BaseResponse): """Response model for single item operations.""" status: str = Field(..., description="Status of the operation") message: str = Field(..., description="Human-readable message about the operation") - data: Optional[Dict[str, Any]] = Field( - default=None, - description="Item data" - ) + data: Optional[Dict[str, Any]] = Field(default=None, description="Item data") + + +class PaginatedItemsResponse(BaseModel): + """Paginated list response for item collections.""" + items: list[Dict[str, Any]] = Field(..., description="Page of items") + total: int = Field(..., description="Total number of items across all pages") + limit: int = Field(..., description="Maximum items returned in this page") + offset: int = Field(..., description="Number of items skipped") + has_more: bool = Field(..., description="True when more items exist beyond this page") class HealthCheckResponse(BaseResponse): """Response model for health check.""" diff --git a/src/api/routes.py b/src/api/routes.py index bd4334f..d68b083 100644 --- a/src/api/routes.py +++ b/src/api/routes.py @@ -1,20 +1,18 @@ import uuid -import os -from typing import List from fastapi import APIRouter, Depends, HTTPException, status -from src.api import handler -from src.api.auth import get_current_user +from src.api.auth import get_current_user, require_scope from src.api.models import ( - GenericRequest, HealthCheckResponse, HealthStatus, + ItemRequest, ItemResponse, + PaginatedItemsResponse, ) -from modal_common import get_env_config - -env_config = get_env_config(os.environ.get("MODAL_ENV", "dev")) +from src.core.services.item_service import ItemService +from src.deps import get_item_service +from src.utils.config import APP_ENV, APP_NAME, APP_VERSION router = APIRouter() @@ -24,21 +22,37 @@ async def health_check(): return HealthCheckResponse( session_id=str(uuid.uuid4()), status=HealthStatus.HEALTHY, - service_name=f'{env_config.app_name}-{env_config.env_name}', - version=env_config.app_version, + service_name=f"{APP_NAME}-{APP_ENV}", + version=APP_VERSION, services_summary={"total": 1, "healthy": 1, "unhealthy": 0}, ) -@router.get("/items", response_model=List[dict], tags=["Items"]) -async def list_items(current_user: dict = Depends(get_current_user)): - return handler.list_items() +@router.get("/items", response_model=PaginatedItemsResponse, tags=["Items"]) +async def list_items( + limit: int = 100, + offset: int = 0, + service: ItemService = Depends(get_item_service), + current_user: dict = Depends(get_current_user), +): + items, total = await service.list(limit=limit, offset=offset) + return PaginatedItemsResponse( + items=[item.to_dict() for item in items], + total=total, + limit=limit, + offset=offset, + has_more=(offset + limit) < total, + ) @router.get("/items/{item_id}", response_model=ItemResponse, tags=["Items"]) -async def get_item(item_id: str, current_user: dict = Depends(get_current_user)): - item = handler.get_item(item_id) - if not item: +async def get_item( + item_id: str, + service: ItemService = Depends(get_item_service), + current_user: dict = Depends(get_current_user), +): + item = await service.get(item_id) + if item is None: raise HTTPException( status_code=status.HTTP_404_NOT_FOUND, detail=f"Item '{item_id}' not found", @@ -47,7 +61,7 @@ async def get_item(item_id: str, current_user: dict = Depends(get_current_user)) session_id=str(uuid.uuid4()), status="success", message="Item retrieved successfully", - data=item, + data=item.to_dict(), ) @@ -57,20 +71,38 @@ async def get_item(item_id: str, current_user: dict = Depends(get_current_user)) status_code=status.HTTP_201_CREATED, tags=["Items"], ) -async def create_item(request: GenericRequest, current_user: dict = Depends(get_current_user)): - item = handler.create_item(request) +async def create_item( + request: ItemRequest, + service: ItemService = Depends(get_item_service), + current_user: dict = Depends(require_scope("items:write")), +): + item = await service.create( + name=request.name, + description=request.description, + project_id=request.project_id, + ) return ItemResponse( session_id=str(uuid.uuid4()), status="created", message="Item created successfully", - data=item, + data=item.to_dict(), ) @router.put("/items/{item_id}", response_model=ItemResponse, tags=["Items"]) -async def update_item(item_id: str, request: GenericRequest, current_user: dict = Depends(get_current_user)): - item = handler.update_item(item_id, request) - if not item: +async def update_item( + item_id: str, + request: ItemRequest, + service: ItemService = Depends(get_item_service), + current_user: dict = Depends(require_scope("items:write")), +): + item = await service.update( + item_id=item_id, + name=request.name, + description=request.description, + project_id=request.project_id, + ) + if item is None: raise HTTPException( status_code=status.HTTP_404_NOT_FOUND, detail=f"Item '{item_id}' not found", @@ -79,7 +111,7 @@ async def update_item(item_id: str, request: GenericRequest, current_user: dict session_id=str(uuid.uuid4()), status="updated", message="Item updated successfully", - data=item, + data=item.to_dict(), ) @@ -88,8 +120,12 @@ async def update_item(item_id: str, request: GenericRequest, current_user: dict status_code=status.HTTP_204_NO_CONTENT, tags=["Items"], ) -async def delete_item(item_id: str, current_user: dict = Depends(get_current_user)): - if not handler.delete_item(item_id): +async def delete_item( + item_id: str, + service: ItemService = Depends(get_item_service), + current_user: dict = Depends(require_scope("items:write")), +): + if not await service.delete(item_id): raise HTTPException( status_code=status.HTTP_404_NOT_FOUND, detail=f"Item '{item_id}' not found", diff --git a/src/core/__init__.py b/src/core/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/src/core/domain/__init__.py b/src/core/domain/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/src/core/domain/item.py b/src/core/domain/item.py new file mode 100644 index 0000000..6f605a3 --- /dev/null +++ b/src/core/domain/item.py @@ -0,0 +1,29 @@ +""" +src/core/domain/item.py +======================= +Pure domain entity. No I/O imports — no FastAPI, no DB drivers, no HTTP. + +Consumers replace this with their own domain model. Keep to_dict() in sync +with whatever fields you add so the HTTP layer can serialise without knowing +the entity internals. +""" +from __future__ import annotations + +from dataclasses import dataclass +from typing import Any, Dict, Optional + + +@dataclass +class Item: + id: str + name: str + description: Optional[str] = None + project_id: Optional[str] = None + + def to_dict(self) -> Dict[str, Any]: + return { + "id": self.id, + "name": self.name, + "description": self.description, + "project_id": self.project_id, + } diff --git a/src/core/ports/__init__.py b/src/core/ports/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/src/core/ports/item_repository.py b/src/core/ports/item_repository.py new file mode 100644 index 0000000..339a3dd --- /dev/null +++ b/src/core/ports/item_repository.py @@ -0,0 +1,27 @@ +""" +src/core/ports/item_repository.py +================================== +Abstract persistence port — the contract the business layer needs from storage. + +Uses Protocol (structural subtyping) so adapters don't need to inherit from +anything. Any object with the right async method signatures satisfies this port. + +Adding a new persistence backend: + 1. Create src/adapters/persistence/.py + 2. Implement a class with these methods (no import of this file needed) + 3. Wire it in src/deps.py +""" +from __future__ import annotations + +from typing import List, Optional, Protocol, Tuple, runtime_checkable + +from src.core.domain.item import Item + + +@runtime_checkable +class ItemRepository(Protocol): + async def get(self, item_id: str) -> Optional[Item]: ... + async def list(self, limit: int, offset: int) -> Tuple[List[Item], int]: ... + async def create(self, item: Item) -> Item: ... + async def update(self, item: Item) -> Optional[Item]: ... + async def delete(self, item_id: str) -> bool: ... diff --git a/src/core/services/__init__.py b/src/core/services/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/src/core/services/item_service.py b/src/core/services/item_service.py new file mode 100644 index 0000000..a11a776 --- /dev/null +++ b/src/core/services/item_service.py @@ -0,0 +1,58 @@ +""" +src/core/services/item_service.py +================================== +Business logic for items. Depends only on the ItemRepository port — +no knowledge of HTTP, DB drivers, or infrastructure. + +Consumers extend this class (or add new service classes) for their domain. +Telemetry is handled transparently by TelemetryMiddleware at the HTTP layer; +add get_tracer() spans here only for fine-grained sub-request tracing. +""" +from __future__ import annotations + +import uuid +from typing import List, Optional, Tuple + +from src.core.domain.item import Item +from src.core.ports.item_repository import ItemRepository + + +class ItemService: + def __init__(self, repo: ItemRepository) -> None: + self._repo = repo + + async def create( + self, + name: str, + description: Optional[str] = None, + project_id: Optional[str] = None, + ) -> Item: + item = Item( + id=str(uuid.uuid4()), + name=name, + description=description, + project_id=project_id, + ) + return await self._repo.create(item) + + async def get(self, item_id: str) -> Optional[Item]: + return await self._repo.get(item_id) + + async def list(self, limit: int = 100, offset: int = 0) -> Tuple[List[Item], int]: + return await self._repo.list(limit=limit, offset=offset) + + async def update( + self, + item_id: str, + name: str, + description: Optional[str] = None, + project_id: Optional[str] = None, + ) -> Optional[Item]: + existing = await self._repo.get(item_id) + if existing is None: + return None + updated = Item(id=item_id, name=name, description=description, project_id=project_id) + return await self._repo.update(updated) + + async def delete(self, item_id: str) -> bool: + return await self._repo.delete(item_id) diff --git a/src/deps.py b/src/deps.py new file mode 100644 index 0000000..6e95e71 --- /dev/null +++ b/src/deps.py @@ -0,0 +1,88 @@ +""" +src/deps.py +=========== +Dependency wiring — the only file that knows which concrete adapter is active. + +This is the single seam between the application core and infrastructure. +Swapping the persistence backend requires: + 1. Adding a new adapter under src/adapters/persistence/ + 2. Adding one branch here keyed on PERSISTENCE_BACKEND + 3. Setting PERSISTENCE_BACKEND in your Modal secret or local .env + +No routes, services, or domain files need to change. + +Caching layer (optional): + Wrap the repository with a cache adapter before passing it to ItemService. + Example: + repo = PostgresItemRepository(os.environ["DATABASE_URL"]) + cache = RedisCache(os.environ["REDIS_URL"]) + return ItemService(CachedItemRepository(repo, cache)) +""" +from __future__ import annotations + +import os +from functools import lru_cache + +from src.core.services.item_service import ItemService +from src.adapters.telemetry.tracing_proxy import TracingProxy + +# ── Required env vars per backend ────────────────────────────────────────── +# Checked at startup so misconfiguration fails fast with a clear error, +# not silently at the first request that hits the DB. +# Update this dict when adding a new adapter. +_REQUIRED_ENV: dict[str, list[str]] = { + "memory": [], + # "postgres": ["DATABASE_URL"], + # "dynamo": ["DYNAMO_TABLE", "AWS_REGION"], +} + + +def _validate_backend(backend: str) -> None: + if backend not in _REQUIRED_ENV: + raise ValueError( + f"Unknown PERSISTENCE_BACKEND='{backend}'. " + f"Known backends: {', '.join(_REQUIRED_ENV)}. " + f"Add an adapter in src/adapters/persistence/ and register it here." + ) + missing = [v for v in _REQUIRED_ENV[backend] if not os.environ.get(v)] + if missing: + raise EnvironmentError( + f"PERSISTENCE_BACKEND='{backend}' requires env vars that are not set: " + f"{', '.join(missing)}" + ) + + +@lru_cache(maxsize=1) +def _get_repository(): + """ + Instantiated once per process lifetime. For adapters that manage + connection pools (asyncpg, motor, etc.), open the pool in main.py's + lifespan instead, and pass it here via a module-level variable. + """ + backend = os.environ.get("PERSISTENCE_BACKEND", "memory") + _validate_backend(backend) + + if backend == "memory": + from src.adapters.persistence.memory import InMemoryItemRepository + raw = InMemoryItemRepository() + + # ── Add new backends here ─────────────────────────────────────────────── + # elif backend == "postgres": + # from src.adapters.persistence.postgres import PostgresItemRepository + # raw = PostgresItemRepository(os.environ["DATABASE_URL"]) + # + # elif backend == "dynamo": + # from src.adapters.persistence.dynamo import DynamoItemRepository + # raw = DynamoItemRepository(os.environ["DYNAMO_TABLE"]) + # ──────────────────────────────────────────────────────────────────────── + else: + raise ValueError(f"Backend '{backend}' is in _REQUIRED_ENV but has no factory. Add one above.") + + # Wrap with telemetry sidecar — spans become children of the HTTP middleware span. + # Remove this line to disable sub-request tracing with zero other changes. + return TracingProxy(raw, prefix=f"repository.item") + + +def get_item_service() -> ItemService: + """FastAPI dependency — injected into routes via Depends(get_item_service).""" + return ItemService(_get_repository()) diff --git a/src/infrastructure/__init__.py b/src/infrastructure/__init__.py new file mode 100644 index 0000000..b724f5b --- /dev/null +++ b/src/infrastructure/__init__.py @@ -0,0 +1,32 @@ +""" +src/infrastructure +================== +Cross-cutting infrastructure layer — telemetry, observability, SDK wiring. + +Sits outside the hexagon core. Neither domain nor adapters import from here +directly; telemetry is injected as a sidecar via TracingProxy in deps.py. + +Template-owned files (copy verbatim, never edit per-service): + setup.py — OTel SDK init, OTLP/Grafana Cloud exporter, providers + __init__.py — this file + +Service-owned file (replace entirely per service): + metrics.py — domain-specific metric instruments + +Public surface: + setup_telemetry() call once in Modal @enter hook + get_tracer(name?) get a named tracer (used by TracingProxy + adapters) + get_meter(name?) get a named meter (used in metrics.py) + +Note: TelemetryMiddleware lives in src/adapters/http/middleware.py — it is an +inbound HTTP adapter, not infrastructure setup. +""" + +from src.infrastructure.setup import get_meter, get_tracer, record_cold_start, setup_telemetry + +__all__ = [ + "setup_telemetry", + "record_cold_start", + "get_tracer", + "get_meter", +] \ No newline at end of file diff --git a/src/observability/metrics.py b/src/infrastructure/metrics.py similarity index 91% rename from src/observability/metrics.py rename to src/infrastructure/metrics.py index ea7005e..b358207 100644 --- a/src/observability/metrics.py +++ b/src/infrastructure/metrics.py @@ -1,6 +1,6 @@ """ -src/observability/metrics.py -============================= +src/infrastructure/metrics.py +============================== THIS FILE IS A STUB. Replace it entirely in each new service. The template ships this empty on purpose. @@ -17,7 +17,7 @@ EXAMPLE (delete this and replace with your own) ───────────────────────────────────────────────────────────── -from src.observability.setup import get_meter +from src.infrastructure.setup import get_meter _meter = get_meter("modal-fastapi") @@ -37,7 +37,7 @@ ) # Usage in your route handler: -# from src.observability.metrics import items_created, items_fetch_duration +# from src.infrastructure.metrics import items_created, items_fetch_duration # items_created.add(1, {"item_type": item.type}) # items_fetch_duration.record(elapsed_ms, {"cache_hit": "false"}) """ diff --git a/src/observability/setup.py b/src/infrastructure/setup.py similarity index 89% rename from src/observability/setup.py rename to src/infrastructure/setup.py index f9bd315..1b547f6 100644 --- a/src/observability/setup.py +++ b/src/infrastructure/setup.py @@ -148,4 +148,22 @@ def get_tracer(name: str | None = None) -> trace.Tracer: @lru_cache(maxsize=None) def get_meter(name: str | None = None) -> metrics.Meter: svc = os.environ.get("OTEL_SERVICE_NAME", "modal-fastapi") - return metrics.get_meter(name or svc) \ No newline at end of file + return metrics.get_meter(name or svc) + + +def record_cold_start() -> None: + """ + Increment the cold-start counter against the live MeterProvider. + + Call once from Modal's startup() hook after setup_telemetry() so the + increment is exported to Grafana. A spike in this counter = containers + scaling up. Calling at module import time (before setup_telemetry) would + bind to the no-op provider and silently drop the data. + """ + svc = os.environ.get("OTEL_SERVICE_NAME", "modal-fastapi") + counter = get_meter().create_counter( + "app.container.cold_start.count", + description="Number of container cold starts", + unit="1", + ) + counter.add(1, {"service": svc}) \ No newline at end of file diff --git a/src/main.py b/src/main.py index fd29031..3675a0a 100644 --- a/src/main.py +++ b/src/main.py @@ -1,5 +1,4 @@ import logging -import os import uuid from contextlib import asynccontextmanager @@ -10,34 +9,30 @@ from src.api.models import ErrorDetail from src.api.routes import router -from src.observability.middleware import TelemetryMiddleware - -from modal_common import get_env_config - -env_config = get_env_config(os.environ.get("MODAL_ENV", "dev")) +from src.adapters.http.middleware import TelemetryMiddleware +from src.utils.config import APP_DESCRIPTION, APP_VERSION, CORS_ORIGINS, SERVER_PREFIX, SERVICE_TITLE logging.basicConfig(level=logging.INFO) logger = logging.getLogger(__name__) -APP_NAME = f'{env_config.app_name}-{env_config.env_name}' @asynccontextmanager async def lifespan(app: FastAPI): - logger.info(f"Starting {APP_NAME}") + logger.info(f"Starting {SERVICE_TITLE}") yield - logger.info(f"Shutting down {APP_NAME}") + logger.info(f"Shutting down {SERVICE_TITLE}") app = FastAPI( - title=APP_NAME, - description=env_config.app_description, - version=env_config.app_version, + title=SERVICE_TITLE, + description=APP_DESCRIPTION, + version=APP_VERSION, lifespan=lifespan, ) app.add_middleware( CORSMiddleware, - allow_origins=["*"], + allow_origins=CORS_ORIGINS, # set per-env via EnvConfig.cors_origins → CORS_ORIGINS env var allow_credentials=True, allow_methods=["*"], allow_headers=["*"], @@ -58,4 +53,4 @@ async def validation_exception_handler(request: Request, exc: RequestValidationE ) -app.include_router(router, prefix=env_config.server_prefix) +app.include_router(router, prefix=SERVER_PREFIX) diff --git a/src/observability/__init__.py b/src/observability/__init__.py deleted file mode 100644 index 55bb540..0000000 --- a/src/observability/__init__.py +++ /dev/null @@ -1,29 +0,0 @@ -""" -src/observability -================= -OpenTelemetry instrumentation layer for modal-template-fastapi. - -Template-owned files (copy verbatim, never edit per-service): - setup.py — SDK init, OTLP/Grafana Cloud exporter - middleware.py — FastAPI auto-instrumentation - __init__.py — this file - -Service-owned file (replace entirely per service): - metrics.py — domain-specific metric instruments - -Public surface: - setup_telemetry() call once in Modal @enter hook - get_tracer(name?) get a named tracer anywhere - get_meter(name?) get a named meter (used in metrics.py) - TelemetryMiddleware add to FastAPI app in main.py -""" - -from src.observability.setup import get_meter, get_tracer, setup_telemetry -from src.observability.middleware import TelemetryMiddleware - -__all__ = [ - "setup_telemetry", - "get_tracer", - "get_meter", - "TelemetryMiddleware", -] \ No newline at end of file diff --git a/src/utils/config.py b/src/utils/config.py new file mode 100644 index 0000000..d124811 --- /dev/null +++ b/src/utils/config.py @@ -0,0 +1,21 @@ +""" +src/config.py +============= +Application-level config read exclusively from environment variables. + +All values here are set by modal_common.configure_env_vars() before src/ is +ever imported in a Modal container. For local uvicorn runs or tests, sensible +defaults are provided so no Modal dependency is needed. + +This file must never import from modal_common or modal. +""" +import os + +APP_NAME = os.environ.get("APP_NAME", "modal-template-fastapi") +APP_ENV = os.environ.get("MODAL_ENV", "dev") +APP_VERSION = os.environ.get("APP_VERSION", "2.0.0") +APP_DESCRIPTION = os.environ.get("APP_DESCRIPTION", "A FastAPI template deployed on Modal with CRUD endpoints.") +SERVER_PREFIX = os.environ.get("SERVER_PREFIX", "/api/v1") +CORS_ORIGINS = os.environ.get("CORS_ORIGINS", "*").split(",") + +SERVICE_TITLE = f"{APP_NAME}-{APP_ENV}" diff --git a/tests/conftest.py b/tests/conftest.py index 6f40af1..6721154 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,58 +1,24 @@ import os -import sys import time -from types import SimpleNamespace -# Must be set before any src imports so auth._get_secret_key() resolves +# Must be set before any src imports so auth._get_secret_key() resolves. os.environ.setdefault("JWT_SECRET", "test-secret-key-for-pytest-at-least-32-bytes") import jwt import pytest from fastapi.testclient import TestClient -# Provide a lightweight modal stub for local test runs where modal is not installed. -if "modal" not in sys.modules: - class _DummyImage: - def apt_install(self, *args, **kwargs): - return self - - def uv_pip_install(self, *args, **kwargs): - return self - - def add_local_dir(self, *args, **kwargs): - return self - - class _DummyImageFactory: - @staticmethod - def debian_slim(*args, **kwargs): - return _DummyImage() - - class _DummyVolume: - @staticmethod - def from_name(*args, **kwargs): - return _DummyVolume() - - class _DummySecret: - @staticmethod - def from_name(*args, **kwargs): - return {} - - @staticmethod - def from_dict(*args, **kwargs): - return {} - - sys.modules["modal"] = SimpleNamespace( - Image=_DummyImageFactory, - Volume=_DummyVolume, - Secret=_DummySecret, - ) - from src.main import app -from src.api import handler from src.api.auth import get_current_user +from src.deps import get_item_service +from src.adapters.persistence.memory import InMemoryItemRepository +from src.core.services.item_service import ItemService TEST_SECRET = "test-secret-key-for-pytest-at-least-32-bytes" -TEST_USER = {"sub": "test@example.com", "name": "Test User"} +TEST_USER = { + "sub": "test@example.com", + "scopes": ["items:read", "items:write"], +} def make_token( @@ -60,45 +26,60 @@ def make_token( exp_offset: int = 3600, secret: str = TEST_SECRET, algorithm: str = "HS256", + scopes: list[str] | None = None, ) -> str: now = int(time.time()) - return jwt.encode( - {"sub": sub, "iat": now, "exp": now + exp_offset}, - secret, - algorithm=algorithm, - ) + payload: dict = {"sub": sub, "iat": now, "exp": now + exp_offset} + if scopes is not None: + payload["scopes"] = scopes + return jwt.encode(payload, secret, algorithm=algorithm) @pytest.fixture(autouse=True) -def clear_store(): - """Wipe the in-memory item store before and after every test.""" - handler._store.clear() - yield - handler._store.clear() +def fresh_item_service(): + """ + Each test gets an isolated in-memory repository — no state bleed between tests. + Overrides the get_item_service FastAPI dependency for the duration of each test. + """ + service = ItemService(InMemoryItemRepository()) + app.dependency_overrides[get_item_service] = lambda: service + yield service + app.dependency_overrides.pop(get_item_service, None) @pytest.fixture -def client(): +def client(fresh_item_service): """TestClient with auth dependency overridden — use for item/business logic tests.""" app.dependency_overrides[get_current_user] = lambda: TEST_USER with TestClient(app) as c: yield c - app.dependency_overrides.clear() + app.dependency_overrides.pop(get_current_user, None) @pytest.fixture -def auth_client(): +def auth_client(fresh_item_service): """TestClient with real JWT auth — use for auth-specific tests.""" - app.dependency_overrides.clear() + app.dependency_overrides.pop(get_current_user, None) with TestClient(app) as c: yield c @pytest.fixture def valid_token() -> str: + """Valid token with no scopes — passes auth, fails scope checks.""" return make_token() +@pytest.fixture +def read_token() -> str: + return make_token(scopes=["items:read"]) + + +@pytest.fixture +def write_token() -> str: + return make_token(scopes=["items:read", "items:write"]) + + @pytest.fixture def expired_token() -> str: return make_token(exp_offset=-3600) @@ -112,3 +93,8 @@ def wrong_secret_token() -> str: @pytest.fixture def auth_headers(valid_token: str) -> dict: return {"Authorization": f"Bearer {valid_token}"} + + +@pytest.fixture +def write_headers(write_token: str) -> dict: + return {"Authorization": f"Bearer {write_token}"} diff --git a/tests/test_auth.py b/tests/test_auth.py index 35f4143..f760caa 100644 --- a/tests/test_auth.py +++ b/tests/test_auth.py @@ -89,9 +89,10 @@ def test_valid_token_grants_access(auth_client: TestClient, auth_headers: dict): assert response.status_code == 200 -def test_valid_token_on_post(auth_client: TestClient, auth_headers: dict): +def test_valid_token_on_post(auth_client: TestClient, write_headers: dict): + """POST requires items:write scope — a write-scoped token must succeed.""" response = auth_client.post( - "/api/v1/items", json={"data": {"name": "test"}}, headers=auth_headers + "/api/v1/items", json={"name": "test"}, headers=write_headers ) assert response.status_code == 201 diff --git a/tests/test_auth_scopes.py b/tests/test_auth_scopes.py new file mode 100644 index 0000000..6dcb8c7 --- /dev/null +++ b/tests/test_auth_scopes.py @@ -0,0 +1,78 @@ +""" +Tests for scope-based authorisation on write endpoints. + +Read endpoints (GET) require only a valid token. +Write endpoints (POST, PUT, DELETE) require the "items:write" scope. +""" +import pytest +from fastapi.testclient import TestClient + + +# --------------------------------------------------------------------------- +# Read endpoints — any valid token, no scope required +# --------------------------------------------------------------------------- + + +def test_list_items_read_scope_not_required(auth_client: TestClient, auth_headers: dict): + """GET /items succeeds with a token that has no scopes.""" + response = auth_client.get("/api/v1/items", headers=auth_headers) + assert response.status_code == 200 + + +def test_get_item_read_scope_not_required(auth_client: TestClient, auth_headers: dict): + """GET /items/{id} returns 404 (not 403) for a token with no scopes.""" + response = auth_client.get("/api/v1/items/nonexistent", headers=auth_headers) + assert response.status_code == 404 + + +# --------------------------------------------------------------------------- +# Write endpoints — require "items:write" scope +# --------------------------------------------------------------------------- + + +def test_create_requires_write_scope(auth_client: TestClient, auth_headers: dict): + """POST /items returns 403 when token has no scopes.""" + response = auth_client.post("/api/v1/items", json={"name": "x"}, headers=auth_headers) + assert response.status_code == 403 + + +def test_create_requires_write_scope_error_code(auth_client: TestClient, auth_headers: dict): + response = auth_client.post("/api/v1/items", json={"name": "x"}, headers=auth_headers) + assert response.json()["detail"]["error_code"] == "INSUFFICIENT_SCOPE" + + +def test_create_succeeds_with_write_scope(auth_client: TestClient, write_headers: dict): + """POST /items returns 201 when token includes items:write.""" + response = auth_client.post("/api/v1/items", json={"name": "scoped"}, headers=write_headers) + assert response.status_code == 201 + + +def test_update_requires_write_scope(auth_client: TestClient, auth_headers: dict, write_headers: dict): + item_id = auth_client.post("/api/v1/items", json={"name": "x"}, headers=write_headers).json()["data"]["id"] + response = auth_client.put(f"/api/v1/items/{item_id}", json={"name": "y"}, headers=auth_headers) + assert response.status_code == 403 + + +def test_update_succeeds_with_write_scope(auth_client: TestClient, write_headers: dict): + item_id = auth_client.post("/api/v1/items", json={"name": "x"}, headers=write_headers).json()["data"]["id"] + response = auth_client.put(f"/api/v1/items/{item_id}", json={"name": "updated"}, headers=write_headers) + assert response.status_code == 200 + + +def test_delete_requires_write_scope(auth_client: TestClient, auth_headers: dict, write_headers: dict): + item_id = auth_client.post("/api/v1/items", json={"name": "x"}, headers=write_headers).json()["data"]["id"] + response = auth_client.delete(f"/api/v1/items/{item_id}", headers=auth_headers) + assert response.status_code == 403 + + +def test_delete_succeeds_with_write_scope(auth_client: TestClient, write_headers: dict): + item_id = auth_client.post("/api/v1/items", json={"name": "x"}, headers=write_headers).json()["data"]["id"] + response = auth_client.delete(f"/api/v1/items/{item_id}", headers=write_headers) + assert response.status_code == 204 + + +def test_read_scope_insufficient_for_write(auth_client: TestClient, read_token: str): + """A token with only items:read cannot write.""" + headers = {"Authorization": f"Bearer {read_token}"} + response = auth_client.post("/api/v1/items", json={"name": "x"}, headers=headers) + assert response.status_code == 403 diff --git a/tests/test_health.py b/tests/test_health.py index 7f55478..3ad8108 100644 --- a/tests/test_health.py +++ b/tests/test_health.py @@ -19,8 +19,9 @@ def test_health_service_name(client: TestClient): def test_health_version(client: TestClient): + from src.utils.config import APP_VERSION data = client.get("/api/v1/health").json() - assert data["version"] == "1.0.0" + assert data["version"] == APP_VERSION def test_health_has_session_id(client: TestClient): diff --git a/tests/test_observability_middleware.py b/tests/test_http_middleware.py similarity index 97% rename from tests/test_observability_middleware.py rename to tests/test_http_middleware.py index 267d982..d9cbc7c 100644 --- a/tests/test_observability_middleware.py +++ b/tests/test_http_middleware.py @@ -1,5 +1,5 @@ """ -Tests for src/observability/middleware.py +Tests for src/adapters/http/middleware.py Uses the real FastAPI app (with TelemetryMiddleware already mounted) so middleware behavior is tested through actual HTTP requests via TestClient. @@ -82,7 +82,7 @@ def test_middleware_uses_route_template_not_raw_path(client: TestClient): in the structured log. We confirm this by checking the response is not 404 for a known item ID path. """ - created = client.post("/api/v1/items", json={"data": {"name": "test"}}).json() + created = client.post("/api/v1/items", json={"name": "test"}).json() item_id = created["data"]["id"] response = client.get(f"/api/v1/items/{item_id}") assert response.status_code == 200 @@ -105,7 +105,7 @@ def test_middleware_does_not_change_200_status(client: TestClient): def test_middleware_does_not_change_201_status(client: TestClient): - response = client.post("/api/v1/items", json={"data": {"name": "x"}}) + response = client.post("/api/v1/items", json={"name": "x"}) assert response.status_code == 201 diff --git a/tests/test_observability_setup.py b/tests/test_infrastructure_setup.py similarity index 78% rename from tests/test_observability_setup.py rename to tests/test_infrastructure_setup.py index 7f3561a..a35520c 100644 --- a/tests/test_observability_setup.py +++ b/tests/test_infrastructure_setup.py @@ -25,7 +25,7 @@ def _reset_otel_globals(): """Reset OTel global providers and the module-level _INITIALISED flag.""" - import src.observability.setup as setup_mod + import src.infrastructure.setup as setup_mod setup_mod._INITIALISED = False # Clear lru_cache so get_tracer / get_meter return fresh objects setup_mod.get_tracer.cache_clear() @@ -54,13 +54,13 @@ def isolated_otel(monkeypatch): def test_setup_telemetry_no_op_when_endpoint_missing(monkeypatch): """setup_telemetry() runs silently with no endpoint — no exception raised.""" monkeypatch.delenv("OTEL_EXPORTER_OTLP_ENDPOINT", raising=False) - from src.observability.setup import setup_telemetry + from src.infrastructure.setup import setup_telemetry setup_telemetry() # must not raise def test_setup_telemetry_no_op_sets_initialised_flag(monkeypatch): monkeypatch.delenv("OTEL_EXPORTER_OTLP_ENDPOINT", raising=False) - import src.observability.setup as setup_mod + import src.infrastructure.setup as setup_mod setup_mod.setup_telemetry() assert setup_mod._INITIALISED is True @@ -68,7 +68,7 @@ def test_setup_telemetry_no_op_sets_initialised_flag(monkeypatch): def test_setup_telemetry_installs_tracer_provider_on_no_op(monkeypatch): """Even with no endpoint, a TracerProvider is installed so get_tracer() works.""" monkeypatch.delenv("OTEL_EXPORTER_OTLP_ENDPOINT", raising=False) - from src.observability.setup import setup_telemetry, get_tracer + from src.infrastructure.setup import setup_telemetry, get_tracer setup_telemetry() tracer = get_tracer() assert tracer is not None @@ -77,7 +77,7 @@ def test_setup_telemetry_installs_tracer_provider_on_no_op(monkeypatch): def test_setup_telemetry_installs_meter_provider_on_no_op(monkeypatch): """Even with no endpoint, a MeterProvider is installed so get_meter() works.""" monkeypatch.delenv("OTEL_EXPORTER_OTLP_ENDPOINT", raising=False) - from src.observability.setup import setup_telemetry, get_meter + from src.infrastructure.setup import setup_telemetry, get_meter setup_telemetry() meter = get_meter() assert meter is not None @@ -91,7 +91,7 @@ def test_setup_telemetry_installs_meter_provider_on_no_op(monkeypatch): def test_setup_telemetry_is_idempotent(monkeypatch): """Calling setup_telemetry() twice does not raise and only initialises once.""" monkeypatch.delenv("OTEL_EXPORTER_OTLP_ENDPOINT", raising=False) - import src.observability.setup as setup_mod + import src.infrastructure.setup as setup_mod setup_mod.setup_telemetry() setup_mod.setup_telemetry() # second call — must be a no-op assert setup_mod._INITIALISED is True @@ -103,11 +103,11 @@ def test_setup_telemetry_idempotent_with_endpoint(monkeypatch): monkeypatch.setenv("GRAFANA_INSTANCE_ID", "123") monkeypatch.setenv("GRAFANA_OTLP_TOKEN", "test-token") - import src.observability.setup as setup_mod + import src.infrastructure.setup as setup_mod # Patch exporters so no real network call is attempted from unittest.mock import patch, MagicMock - with patch("src.observability.setup.OTLPSpanExporter", return_value=MagicMock()), \ - patch("src.observability.setup.OTLPMetricExporter", return_value=MagicMock()): + with patch("src.infrastructure.setup.OTLPSpanExporter", return_value=MagicMock()), \ + patch("src.infrastructure.setup.OTLPMetricExporter", return_value=MagicMock()): setup_mod.setup_telemetry() setup_mod.setup_telemetry() # second call assert setup_mod._INITIALISED is True @@ -121,28 +121,28 @@ def test_setup_telemetry_idempotent_with_endpoint(monkeypatch): def test_grafana_headers_empty_when_no_credentials(monkeypatch): monkeypatch.delenv("GRAFANA_INSTANCE_ID", raising=False) monkeypatch.delenv("GRAFANA_OTLP_TOKEN", raising=False) - from src.observability.setup import _grafana_headers + from src.infrastructure.setup import _grafana_headers assert _grafana_headers() == {} def test_grafana_headers_empty_when_only_instance_id(monkeypatch): monkeypatch.setenv("GRAFANA_INSTANCE_ID", "123456") monkeypatch.delenv("GRAFANA_OTLP_TOKEN", raising=False) - from src.observability.setup import _grafana_headers + from src.infrastructure.setup import _grafana_headers assert _grafana_headers() == {} def test_grafana_headers_empty_when_only_token(monkeypatch): monkeypatch.delenv("GRAFANA_INSTANCE_ID", raising=False) monkeypatch.setenv("GRAFANA_OTLP_TOKEN", "glc_test_token") - from src.observability.setup import _grafana_headers + from src.infrastructure.setup import _grafana_headers assert _grafana_headers() == {} def test_grafana_headers_returns_authorization_key(monkeypatch): monkeypatch.setenv("GRAFANA_INSTANCE_ID", "123456") monkeypatch.setenv("GRAFANA_OTLP_TOKEN", "glc_test_token") - from src.observability.setup import _grafana_headers + from src.infrastructure.setup import _grafana_headers headers = _grafana_headers() assert "Authorization" in headers @@ -150,7 +150,7 @@ def test_grafana_headers_returns_authorization_key(monkeypatch): def test_grafana_headers_uses_basic_auth_scheme(monkeypatch): monkeypatch.setenv("GRAFANA_INSTANCE_ID", "123456") monkeypatch.setenv("GRAFANA_OTLP_TOKEN", "glc_test_token") - from src.observability.setup import _grafana_headers + from src.infrastructure.setup import _grafana_headers headers = _grafana_headers() assert headers["Authorization"].startswith("Basic ") @@ -158,7 +158,7 @@ def test_grafana_headers_uses_basic_auth_scheme(monkeypatch): def test_grafana_headers_base64_encodes_id_colon_token(monkeypatch): monkeypatch.setenv("GRAFANA_INSTANCE_ID", "123456") monkeypatch.setenv("GRAFANA_OTLP_TOKEN", "glc_test_token") - from src.observability.setup import _grafana_headers + from src.infrastructure.setup import _grafana_headers headers = _grafana_headers() encoded = headers["Authorization"].split(" ", 1)[1] decoded = base64.b64decode(encoded).decode() @@ -168,7 +168,7 @@ def test_grafana_headers_base64_encodes_id_colon_token(monkeypatch): def test_grafana_headers_instance_id_is_username(monkeypatch): monkeypatch.setenv("GRAFANA_INSTANCE_ID", "999888") monkeypatch.setenv("GRAFANA_OTLP_TOKEN", "some-token") - from src.observability.setup import _grafana_headers + from src.infrastructure.setup import _grafana_headers headers = _grafana_headers() encoded = headers["Authorization"].split(" ", 1)[1] decoded = base64.b64decode(encoded).decode() @@ -179,7 +179,7 @@ def test_grafana_headers_instance_id_is_username(monkeypatch): def test_grafana_headers_token_is_password(monkeypatch): monkeypatch.setenv("GRAFANA_INSTANCE_ID", "111") monkeypatch.setenv("GRAFANA_OTLP_TOKEN", "my-secret-token") - from src.observability.setup import _grafana_headers + from src.infrastructure.setup import _grafana_headers headers = _grafana_headers() encoded = headers["Authorization"].split(" ", 1)[1] decoded = base64.b64decode(encoded).decode() @@ -194,42 +194,42 @@ def test_grafana_headers_token_is_password(monkeypatch): def test_build_resource_service_name_from_env(monkeypatch): monkeypatch.setenv("OTEL_SERVICE_NAME", "my-test-service") - from src.observability.setup import _build_resource + from src.infrastructure.setup import _build_resource resource = _build_resource() assert resource.attributes["service.name"] == "my-test-service" def test_build_resource_service_name_default(monkeypatch): monkeypatch.delenv("OTEL_SERVICE_NAME", raising=False) - from src.observability.setup import _build_resource + from src.infrastructure.setup import _build_resource resource = _build_resource() assert resource.attributes["service.name"] == "modal-fastapi" def test_build_resource_environment_from_env(monkeypatch): monkeypatch.setenv("MODAL_ENV", "prod") - from src.observability.setup import _build_resource + from src.infrastructure.setup import _build_resource resource = _build_resource() assert resource.attributes["deployment.environment"] == "prod" def test_build_resource_environment_default(monkeypatch): monkeypatch.delenv("MODAL_ENV", raising=False) - from src.observability.setup import _build_resource + from src.infrastructure.setup import _build_resource resource = _build_resource() assert resource.attributes["deployment.environment"] == "dev" def test_build_resource_version_from_env(monkeypatch): monkeypatch.setenv("OTEL_SERVICE_VERSION", "2.5.0") - from src.observability.setup import _build_resource + from src.infrastructure.setup import _build_resource resource = _build_resource() assert resource.attributes["service.version"] == "2.5.0" def test_build_resource_version_default(monkeypatch): monkeypatch.delenv("OTEL_SERVICE_VERSION", raising=False) - from src.observability.setup import _build_resource + from src.infrastructure.setup import _build_resource resource = _build_resource() assert resource.attributes["service.version"] == "1.0.0" @@ -241,21 +241,21 @@ def test_build_resource_version_default(monkeypatch): def test_setup_telemetry_with_endpoint_sets_initialised(monkeypatch): monkeypatch.setenv("OTEL_EXPORTER_OTLP_ENDPOINT", "http://localhost:4318") - import src.observability.setup as setup_mod + import src.infrastructure.setup as setup_mod from unittest.mock import patch, MagicMock - with patch("src.observability.setup.OTLPSpanExporter", return_value=MagicMock()), \ - patch("src.observability.setup.OTLPMetricExporter", return_value=MagicMock()): + with patch("src.infrastructure.setup.OTLPSpanExporter", return_value=MagicMock()), \ + patch("src.infrastructure.setup.OTLPMetricExporter", return_value=MagicMock()): setup_mod.setup_telemetry() assert setup_mod._INITIALISED is True def test_setup_telemetry_span_exporter_gets_traces_path(monkeypatch): monkeypatch.setenv("OTEL_EXPORTER_OTLP_ENDPOINT", "http://localhost:4318") - import src.observability.setup as setup_mod + import src.infrastructure.setup as setup_mod from unittest.mock import patch, MagicMock, call mock_span_exporter_cls = MagicMock(return_value=MagicMock()) - with patch("src.observability.setup.OTLPSpanExporter", mock_span_exporter_cls), \ - patch("src.observability.setup.OTLPMetricExporter", return_value=MagicMock()): + with patch("src.infrastructure.setup.OTLPSpanExporter", mock_span_exporter_cls), \ + patch("src.infrastructure.setup.OTLPMetricExporter", return_value=MagicMock()): setup_mod.setup_telemetry() call_kwargs = mock_span_exporter_cls.call_args[1] assert call_kwargs["endpoint"] == "http://localhost:4318/v1/traces" @@ -263,11 +263,11 @@ def test_setup_telemetry_span_exporter_gets_traces_path(monkeypatch): def test_setup_telemetry_metric_exporter_gets_metrics_path(monkeypatch): monkeypatch.setenv("OTEL_EXPORTER_OTLP_ENDPOINT", "http://localhost:4318") - import src.observability.setup as setup_mod + import src.infrastructure.setup as setup_mod from unittest.mock import patch, MagicMock mock_metric_exporter_cls = MagicMock(return_value=MagicMock()) - with patch("src.observability.setup.OTLPSpanExporter", return_value=MagicMock()), \ - patch("src.observability.setup.OTLPMetricExporter", mock_metric_exporter_cls): + with patch("src.infrastructure.setup.OTLPSpanExporter", return_value=MagicMock()), \ + patch("src.infrastructure.setup.OTLPMetricExporter", mock_metric_exporter_cls): setup_mod.setup_telemetry() call_kwargs = mock_metric_exporter_cls.call_args[1] assert call_kwargs["endpoint"] == "http://localhost:4318/v1/metrics" @@ -277,12 +277,12 @@ def test_setup_telemetry_passes_auth_headers_to_exporters(monkeypatch): monkeypatch.setenv("OTEL_EXPORTER_OTLP_ENDPOINT", "http://localhost:4318") monkeypatch.setenv("GRAFANA_INSTANCE_ID", "123456") monkeypatch.setenv("GRAFANA_OTLP_TOKEN", "glc_token") - import src.observability.setup as setup_mod + import src.infrastructure.setup as setup_mod from unittest.mock import patch, MagicMock mock_span_cls = MagicMock(return_value=MagicMock()) mock_metric_cls = MagicMock(return_value=MagicMock()) - with patch("src.observability.setup.OTLPSpanExporter", mock_span_cls), \ - patch("src.observability.setup.OTLPMetricExporter", mock_metric_cls): + with patch("src.infrastructure.setup.OTLPSpanExporter", mock_span_cls), \ + patch("src.infrastructure.setup.OTLPMetricExporter", mock_metric_cls): setup_mod.setup_telemetry() span_headers = mock_span_cls.call_args[1]["headers"] metric_headers = mock_metric_cls.call_args[1]["headers"] @@ -293,12 +293,12 @@ def test_setup_telemetry_passes_auth_headers_to_exporters(monkeypatch): def test_setup_telemetry_metric_export_interval_from_env(monkeypatch): monkeypatch.setenv("OTEL_EXPORTER_OTLP_ENDPOINT", "http://localhost:4318") monkeypatch.setenv("OTEL_METRIC_EXPORT_INTERVAL_MS", "5000") - import src.observability.setup as setup_mod + import src.infrastructure.setup as setup_mod from unittest.mock import patch, MagicMock mock_reader_cls = MagicMock(return_value=MagicMock()) - with patch("src.observability.setup.OTLPSpanExporter", return_value=MagicMock()), \ - patch("src.observability.setup.OTLPMetricExporter", return_value=MagicMock()), \ - patch("src.observability.setup.PeriodicExportingMetricReader", mock_reader_cls): + with patch("src.infrastructure.setup.OTLPSpanExporter", return_value=MagicMock()), \ + patch("src.infrastructure.setup.OTLPMetricExporter", return_value=MagicMock()), \ + patch("src.infrastructure.setup.PeriodicExportingMetricReader", mock_reader_cls): setup_mod.setup_telemetry() _, kwargs = mock_reader_cls.call_args assert kwargs.get("export_interval_millis") == 5000 @@ -307,12 +307,12 @@ def test_setup_telemetry_metric_export_interval_from_env(monkeypatch): def test_setup_telemetry_metric_export_interval_default(monkeypatch): monkeypatch.setenv("OTEL_EXPORTER_OTLP_ENDPOINT", "http://localhost:4318") monkeypatch.delenv("OTEL_METRIC_EXPORT_INTERVAL_MS", raising=False) - import src.observability.setup as setup_mod + import src.infrastructure.setup as setup_mod from unittest.mock import patch, MagicMock mock_reader_cls = MagicMock(return_value=MagicMock()) - with patch("src.observability.setup.OTLPSpanExporter", return_value=MagicMock()), \ - patch("src.observability.setup.OTLPMetricExporter", return_value=MagicMock()), \ - patch("src.observability.setup.PeriodicExportingMetricReader", mock_reader_cls): + with patch("src.infrastructure.setup.OTLPSpanExporter", return_value=MagicMock()), \ + patch("src.infrastructure.setup.OTLPMetricExporter", return_value=MagicMock()), \ + patch("src.infrastructure.setup.PeriodicExportingMetricReader", mock_reader_cls): setup_mod.setup_telemetry() _, kwargs = mock_reader_cls.call_args assert kwargs.get("export_interval_millis") == 15000 diff --git a/tests/test_items.py b/tests/test_items.py index d4dfc39..8e6e9ce 100644 --- a/tests/test_items.py +++ b/tests/test_items.py @@ -7,45 +7,83 @@ # --------------------------------------------------------------------------- -def _create_item(client: TestClient, data: dict = None, project_id: str = None) -> dict: - payload: dict = {"data": data or {"name": "widget", "value": 1}} +def _create_item( + client: TestClient, + name: str = "widget", + description: str = None, + project_id: str = None, +) -> dict: + payload: dict = {"name": name} + if description is not None: + payload["description"] = description if project_id: payload["project_id"] = project_id return client.post("/api/v1/items", json=payload).json() +def _list_items(client: TestClient, **params) -> dict: + return client.get("/api/v1/items", params=params).json() + + # --------------------------------------------------------------------------- -# List items +# List items (paginated) # --------------------------------------------------------------------------- def test_list_items_empty(client: TestClient): response = client.get("/api/v1/items") assert response.status_code == 200 - assert response.json() == [] + body = response.json() + assert body["items"] == [] + assert body["total"] == 0 + assert body["has_more"] is False def test_list_items_returns_all_created(client: TestClient): - _create_item(client, {"name": "alpha"}) - _create_item(client, {"name": "beta"}) - items = client.get("/api/v1/items").json() - assert len(items) == 2 + _create_item(client, name="alpha") + _create_item(client, name="beta") + body = _list_items(client) + assert len(body["items"]) == 2 + assert body["total"] == 2 def test_list_items_contains_correct_data(client: TestClient): - _create_item(client, {"name": "gamma"}) - items = client.get("/api/v1/items").json() - names = [i["name"] for i in items] + _create_item(client, name="gamma") + body = _list_items(client) + names = [i["name"] for i in body["items"]] assert "gamma" in names +def test_list_items_pagination_limit(client: TestClient): + for i in range(5): + _create_item(client, name=f"item-{i}") + body = _list_items(client, limit=2, offset=0) + assert len(body["items"]) == 2 + assert body["total"] == 5 + assert body["has_more"] is True + + +def test_list_items_pagination_offset(client: TestClient): + for i in range(3): + _create_item(client, name=f"item-{i}") + body = _list_items(client, limit=10, offset=2) + assert len(body["items"]) == 1 + assert body["has_more"] is False + + +def test_list_items_has_more_false_on_last_page(client: TestClient): + _create_item(client, name="only") + body = _list_items(client, limit=10, offset=0) + assert body["has_more"] is False + + # --------------------------------------------------------------------------- # Create item # --------------------------------------------------------------------------- def test_create_item_returns_201(client: TestClient): - response = client.post("/api/v1/items", json={"data": {"name": "new"}}) + response = client.post("/api/v1/items", json={"name": "new"}) assert response.status_code == 201 @@ -69,23 +107,27 @@ def test_create_item_data_has_id(client: TestClient): assert "id" in data["data"] -def test_create_item_persists_payload(client: TestClient): - data = _create_item(client, {"name": "persisted", "count": 7}) +def test_create_item_persists_name(client: TestClient): + data = _create_item(client, name="persisted") assert data["data"]["name"] == "persisted" - assert data["data"]["count"] == 7 + + +def test_create_item_persists_description(client: TestClient): + data = _create_item(client, name="x", description="a detail") + assert data["data"]["description"] == "a detail" def test_create_item_persists_project_id(client: TestClient): - data = _create_item(client, {"name": "x"}, project_id="proj-42") + data = _create_item(client, name="x", project_id="proj-42") assert data["data"]["project_id"] == "proj-42" def test_create_item_project_id_optional(client: TestClient): - response = client.post("/api/v1/items", json={"data": {"k": "v"}}) + response = client.post("/api/v1/items", json={"name": "no-project"}) assert response.status_code == 201 -def test_create_item_missing_data_returns_422(client: TestClient): +def test_create_item_missing_name_returns_422(client: TestClient): response = client.post("/api/v1/items", json={}) assert response.status_code == 422 @@ -136,46 +178,48 @@ def test_get_item_not_found_detail(client: TestClient): def test_update_item_returns_200(client: TestClient): item_id = _create_item(client)["data"]["id"] - response = client.put(f"/api/v1/items/{item_id}", json={"data": {"name": "updated"}}) + response = client.put(f"/api/v1/items/{item_id}", json={"name": "updated"}) assert response.status_code == 200 def test_update_item_response_status(client: TestClient): item_id = _create_item(client)["data"]["id"] - data = client.put(f"/api/v1/items/{item_id}", json={"data": {"x": 1}}).json() + data = client.put(f"/api/v1/items/{item_id}", json={"name": "updated"}).json() assert data["status"] == "updated" -def test_update_item_reflects_new_data(client: TestClient): - item_id = _create_item(client, {"name": "before"})["data"]["id"] - data = client.put( - f"/api/v1/items/{item_id}", json={"data": {"name": "after"}} - ).json() +def test_update_item_reflects_new_name(client: TestClient): + item_id = _create_item(client, name="before")["data"]["id"] + data = client.put(f"/api/v1/items/{item_id}", json={"name": "after"}).json() assert data["data"]["name"] == "after" +def test_update_item_reflects_new_description(client: TestClient): + item_id = _create_item(client, name="x")["data"]["id"] + data = client.put(f"/api/v1/items/{item_id}", json={"name": "x", "description": "new desc"}).json() + assert data["data"]["description"] == "new desc" + + def test_update_item_preserves_id(client: TestClient): item_id = _create_item(client)["data"]["id"] - data = client.put( - f"/api/v1/items/{item_id}", json={"data": {"name": "new"}} - ).json() + data = client.put(f"/api/v1/items/{item_id}", json={"name": "new"}).json() assert data["data"]["id"] == item_id def test_update_item_not_found_returns_404(client: TestClient): - response = client.put("/api/v1/items/ghost-id", json={"data": {"x": 1}}) + response = client.put("/api/v1/items/ghost-id", json={"name": "x"}) assert response.status_code == 404 -def test_update_item_missing_data_returns_422(client: TestClient): +def test_update_item_missing_name_returns_422(client: TestClient): item_id = _create_item(client)["data"]["id"] response = client.put(f"/api/v1/items/{item_id}", json={}) assert response.status_code == 422 def test_update_item_visible_on_get(client: TestClient): - item_id = _create_item(client, {"name": "old"})["data"]["id"] - client.put(f"/api/v1/items/{item_id}", json={"data": {"name": "new"}}) + item_id = _create_item(client, name="old")["data"]["id"] + client.put(f"/api/v1/items/{item_id}", json={"name": "new"}) fetched = client.get(f"/api/v1/items/{item_id}").json()["data"] assert fetched["name"] == "new" @@ -200,8 +244,7 @@ def test_delete_item_removes_from_store(client: TestClient): def test_delete_item_removed_from_list(client: TestClient): item_id = _create_item(client)["data"]["id"] client.delete(f"/api/v1/items/{item_id}") - items = client.get("/api/v1/items").json() - ids = [i["id"] for i in items] + ids = [i["id"] for i in _list_items(client)["items"]] assert item_id not in ids @@ -211,7 +254,7 @@ def test_delete_item_not_found_returns_404(client: TestClient): def test_delete_only_target_item(client: TestClient): - id1 = _create_item(client, {"name": "keep"})["data"]["id"] - id2 = _create_item(client, {"name": "remove"})["data"]["id"] + id1 = _create_item(client, name="keep")["data"]["id"] + id2 = _create_item(client, name="remove")["data"]["id"] client.delete(f"/api/v1/items/{id2}") assert client.get(f"/api/v1/items/{id1}").status_code == 200 diff --git a/tests/test_models.py b/tests/test_models.py index c526642..7bc3f12 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -7,48 +7,73 @@ ErrorDetail, FileUploadRequest, FileUploadResponse, - GenericRequest, HealthCheckResponse, HealthStatus, + ItemRequest, ItemResponse, + PaginatedItemsResponse, TokenPayload, ) # --------------------------------------------------------------------------- -# GenericRequest +# ItemRequest # --------------------------------------------------------------------------- -def test_generic_request_requires_data(): +def test_item_request_requires_name(): with pytest.raises(ValidationError): - GenericRequest() # type: ignore[call-arg] + ItemRequest() # type: ignore[call-arg] -def test_generic_request_valid(): - req = GenericRequest(data={"key": "value"}) - assert req.data == {"key": "value"} +def test_item_request_valid(): + req = ItemRequest(name="widget") + assert req.name == "widget" + assert req.description is None assert req.project_id is None -def test_generic_request_with_project_id(): - req = GenericRequest(data={}, project_id="proj-123") +def test_item_request_with_all_fields(): + req = ItemRequest(name="widget", description="a detail", project_id="proj-123") + assert req.description == "a detail" assert req.project_id == "proj-123" -def test_generic_request_empty_data_allowed(): - req = GenericRequest(data={}) - assert req.data == {} +def test_item_request_name_min_length(): + with pytest.raises(ValidationError): + ItemRequest(name="") -def test_generic_request_nested_data(): - req = GenericRequest(data={"nested": {"a": 1}}) - assert req.data["nested"]["a"] == 1 +def test_item_request_project_id_min_length(): + with pytest.raises(ValidationError): + ItemRequest(name="x", project_id="") -def test_generic_request_project_id_min_length(): - with pytest.raises(ValidationError): - GenericRequest(data={}, project_id="") +# --------------------------------------------------------------------------- +# PaginatedItemsResponse +# --------------------------------------------------------------------------- + + +def test_paginated_response_valid(): + resp = PaginatedItemsResponse( + items=[{"id": "1", "name": "widget"}], + total=1, + limit=10, + offset=0, + has_more=False, + ) + assert resp.total == 1 + assert resp.has_more is False + + +def test_paginated_response_empty(): + resp = PaginatedItemsResponse(items=[], total=0, limit=10, offset=0, has_more=False) + assert resp.items == [] + + +def test_paginated_response_has_more_true(): + resp = PaginatedItemsResponse(items=[], total=20, limit=10, offset=0, has_more=True) + assert resp.has_more is True # ---------------------------------------------------------------------------