diff --git a/.env.example b/.env.example index b964f87..4c06d89 100644 --- a/.env.example +++ b/.env.example @@ -56,41 +56,38 @@ IMAGE_VERSION=latest # Logging level (DEBUG, INFO, WARNING, ERROR, CRITICAL) LOG_LEVEL=INFO -# Base directory for runtime data (database, etc.) +# Base directory for runtime data (seed/health/content files, etc.) # Default: ./data (relative to docker-compose.yml location) # Inside containers this is mapped to /data # # Structure: # ${DATA_HOME}/ -# └── collector/ -# └── meshcore.db # SQLite database +# ├── collector/ # collector health/state files +# └── web/ # web tier data DATA_HOME=./data # ----------------------------------------------------------------------------- -# Database +# Database (PostgreSQL — the only backend since v0.19) # ----------------------------------------------------------------------------- -# SQLite is the zero-config default and needs nothing here — it lives at -# ${DATA_HOME}/collector/meshcore.db. +# The bundled postgres container is part of the default `core` profile and +# derives its POSTGRES_USER/PASSWORD/DB from DATABASE_USER/PASSWORD/NAME, so +# a default `docker compose up` works with zero configuration. The default +# password below is dev-only — override DATABASE_PASSWORD in production. # -# To use PostgreSQL instead, set DATABASE_BACKEND=postgres and fill in the -# DATABASE_* values below (the bundled postgres container derives its -# POSTGRES_USER/PASSWORD/DB from DATABASE_USER/PASSWORD/NAME). You must also -# activate the compose 'postgres' profile, e.g. `docker compose --profile postgres up`. -# -# See docs/database.md for the full backend reference: production role/database +# See docs/database.md for the full reference: production role/database # provisioning, managed/external Postgres, and schema-per-instance (search_path) -# isolation for multiple instances sharing one cluster. +# isolation for multiple instances sharing one cluster. Upgrading from an old +# SQLite deployment? See docs/upgrading.md (v0.19 section). # -# DATABASE_BACKEND=postgres # DATABASE_HOST=postgres # DATABASE_PORT=5432 # DATABASE_NAME=meshcorehub # DATABASE_SCHEMA=meshcorehub # override per instance (e.g. prod, stg) on a shared cluster # DATABASE_USER=meshcorehub -# DATABASE_PASSWORD= # required for postgres; e.g. `openssl rand -base64 32` +# DATABASE_PASSWORD= # e.g. `openssl rand -base64 32` (required in production) # # Advanced: set DATABASE_URL to a full SQLAlchemy URL to override all of the above -# (e.g. a managed/external Postgres). Takes precedence over DATABASE_BACKEND. +# (e.g. a managed/external Postgres). # DATABASE_URL=postgresql+psycopg2://user:pass@host:5432/dbname # Directory containing seed data files for import diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d8d53a3..3751539 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -74,6 +74,21 @@ jobs: name: Test runs-on: ubuntu-latest timeout-minutes: 30 + services: + postgres: + # Pinned to match the bundled postgres:17-alpine in docker-compose.yml + image: postgres:17 + env: + POSTGRES_USER: meshcorehub + POSTGRES_PASSWORD: meshcorehub-test + POSTGRES_DB: meshcorehub_test + ports: + - 5432:5432 + options: >- + --health-cmd "pg_isready -U meshcorehub -d meshcorehub_test" + --health-interval 5s + --health-timeout 5s + --health-retries 10 steps: - uses: actions/checkout@v7 @@ -89,6 +104,8 @@ jobs: pip install -e ".[dev]" - name: Run tests with pytest + env: + TEST_POSTGRES_URL: postgresql+psycopg2://meshcorehub:meshcorehub-test@localhost:5432/meshcorehub_test run: | pytest -nauto --cov=meshcore_hub --cov-report=xml --cov-report=term-missing --junitxml=junit.xml -o junit_family=legacy diff --git a/AGENTS.md b/AGENTS.md index c67efdf..101e7cc 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -78,7 +78,7 @@ npm run test:frontend # host: vitest unit + component tests ## Tests & Quality -Coverage is **opt-in**; add `--cov=meshcore_hub` (or `make test-cov`) when you want it. The dev loop defaults to no coverage and parallel across CPU cores. +Coverage is **opt-in**; add `--cov=meshcore_hub` (or `make test-cov`) when you want it. The dev loop defaults to no coverage and parallel across CPU cores. Backend tests require PostgreSQL: `make test` / `make test-cov` / `make test-unit` start and stop the throwaway test Postgres automatically (a `postgres:17-alpine` on `127.0.0.1:55432`; the management is skipped when `TEST_POSTGRES_URL` points at your own instance). For direct `pytest` runs, `make test-db-up` first — pytest fails fast with a hint when the database is unreachable. ```bash # Canonical: run tests in parallel, no coverage, surface the pass/fail summary @@ -138,16 +138,17 @@ Design notes when extending the suite: ## Database & Ops -The default backend is **SQLite** (zero-config, file at `${DATA_HOME}/collector/meshcore.db`). **PostgreSQL** is also supported via `DATABASE_BACKEND=postgres` — see `docs/database.md` for the full backend reference, production provisioning, and schema-per-instance setup. Migrations are backend-agnostic; the commands below work for both. +The database backend is **PostgreSQL** (the only backend since v0.19). The bundled container starts with the `core` compose profile — see `docs/database.md` for the full reference, production provisioning, and schema-per-instance setup. Backend tests also require Postgres: `make test-db-up` starts a throwaway instance on `127.0.0.1:55432` (or point `TEST_POSTGRES_URL` at your own). ```bash -# --- LOCAL (venv): sync the volume DB to ./meshcore.db, then author a migration -# Volume name is ${COMPOSE_PROJECT_NAME:-hub}_data (default: hub_data) -# (SQLite only — for Postgres, point the migration env at the cluster directly) -docker run -it --rm -v hub_data:/data -v "$PWD":/pwd ubuntu cp /data/collector/meshcore.db /pwd/meshcore.db -meshcore-hub db revision --autogenerate -m "description" +# --- LOCAL (venv): author a migration against a local schema (NOT the dev DB) +# Use a scratch schema on the test DB so autogenerate diffs against a clean +# schema; never point revision tooling at the running dev database. +TEST_POSTGRES_URL=postgresql+psycopg2://meshcorehub:meshcorehub-test@localhost:55432/meshcorehub_test \ +DATABASE_URL=postgresql+psycopg2://meshcorehub:meshcorehub-test@localhost:55432/meshcorehub_test \ +DATABASE_SCHEMA=scratch meshcore-hub db revision --autogenerate -m "description" -# --- CONTAINER: apply migrations (the DB lives in the data volume) +# --- CONTAINER: apply migrations (the DB lives in the postgres_data volume) # Migrations auto-apply on `up` via the migrate service. Manual one-off: docker compose -f docker-compose.yml -f docker-compose.dev.yml --profile core run --rm migrate db upgrade @@ -358,16 +359,14 @@ async def test_collector_handles_advertisement(): # Integration test import pytest from httpx import AsyncClient -from sqlalchemy.ext.asyncio import create_async_engine, AsyncSession +from sqlalchemy.ext.asyncio import AsyncSession @pytest.fixture -async def db_session(): - """Create in-memory SQLite database for testing.""" - engine = create_async_engine("sqlite+aiosqlite:///:memory:") - async with engine.begin() as conn: - await conn.run_sync(Base.metadata.create_all) - - async with AsyncSession(engine) as session: +async def db_session(db_url, db_schema): + """Async session on the per-worker Postgres schema (see tests/conftest.py).""" + manager = DatabaseManager(db_url, schema=db_schema) + manager.create_tables() + async with manager.async_session() as session: yield session @pytest.fixture diff --git a/Dockerfile b/Dockerfile index 613e62c..dfcf530 100644 --- a/Dockerfile +++ b/Dockerfile @@ -86,8 +86,6 @@ ENV PYTHONDONTWRITEBYTECODE=1 \ RUN apt-get update && apt-get install -y --no-install-recommends \ # For serial port access udev \ - # For database debugging and maintenance - sqlite3 \ && rm -rf /var/lib/apt/lists/* \ && mkdir -p /data diff --git a/Makefile b/Makefile index afd09c1..bb2cf18 100644 --- a/Makefile +++ b/Makefile @@ -5,7 +5,7 @@ VOLUMES = $(COMPOSE_PROJECT_NAME)_data $(COMPOSE_PROJECT_NAME)_mqtt_data \ $(COMPOSE_PROJECT_NAME)_observer_data .PHONY: build up down logs backup restore test test-cov test-unit test-frontend \ - e2e-build e2e-up e2e-down e2e-seed e2e-test + test-db-up test-db-down e2e-build e2e-up e2e-down e2e-seed e2e-test build: docker compose $(COMPOSE_FILES) --profile all build --no-cache @@ -36,21 +36,53 @@ restore: alpine sh -c "cd / && tar xzf /backup/$$(basename $(FILE))" # --- Tests --------------------------------------------------------------- +# Backend tests run against PostgreSQL only: `test`, `test-cov`, and +# `test-unit` start the throwaway test DB automatically and stop it again +# afterwards — even when pytest fails (EXIT trap). Set TEST_POSTGRES_URL to +# point at your own instance and the automatic up/down is skipped. +# For direct pytest runs, start it manually with `make test-db-up`. # Coverage is opt-in (use test-cov). Dev loop runs in parallel across cores. # `test` runs the backend suite then the frontend (vitest) suite. test: - pytest -nauto --no-cov - $(MAKE) test-frontend + set -e; \ + if [ -z "$$TEST_POSTGRES_URL" ]; then \ + $(MAKE) --no-print-directory test-db-up; \ + trap '$(MAKE) --no-print-directory test-db-down' EXIT; \ + fi; \ + pytest -nauto --no-cov; \ + $(MAKE) --no-print-directory test-frontend test-cov: + set -e; \ + if [ -z "$$TEST_POSTGRES_URL" ]; then \ + $(MAKE) --no-print-directory test-db-up; \ + trap '$(MAKE) --no-print-directory test-db-down' EXIT; \ + fi; \ pytest --cov=meshcore_hub --cov-report=term-missing test-unit: + set -e; \ + if [ -z "$$TEST_POSTGRES_URL" ]; then \ + $(MAKE) --no-print-directory test-db-up; \ + trap '$(MAKE) --no-print-directory test-db-down' EXIT; \ + fi; \ pytest -nauto --no-cov tests/test_common/ tests/test_api/ tests/test_collector/ tests/test_web/ test-frontend: npm run test:frontend +# Throwaway PostgreSQL for the backend suite (127.0.0.1:55432, dev-only creds, +# ephemeral tmpfs storage). pytest fails fast with a hint if it is unreachable. +# The project name is pinned so a COMPOSE_PROJECT_NAME set in .env (the dev +# stack) can never make `down` target the dev services. +TEST_DB_COMPOSE = docker compose -f docker-compose.test-db.yml -p meshcore-hub-test-db + +test-db-up: + $(TEST_DB_COMPOSE) up -d --wait + +test-db-down: + $(TEST_DB_COMPOSE) down -v --remove-orphans + # --- E2E (Playwright) --------------------------------------------------- # Self-contained throwaway stack (own ephemeral Postgres, isolated volumes). # make e2e-build && make e2e-up # start the stack (build first time) diff --git a/README.md b/README.md index e912eb6..a2599fa 100644 --- a/README.md +++ b/README.md @@ -8,7 +8,7 @@ Python 3.13+ platform for managing and orchestrating MeshCore mesh networks. > [!WARNING] -> **DEPRECATION NOTICE** — v0.14 adds PostgreSQL support (`DATABASE_BACKEND=postgres`); SQLite remains the zero-config default. SQLite support will be maintained for at least the next few releases (~3 months), then removed in favour of PostgreSQL-only. See [docs/database.md](docs/database.md) to switch backends and [docs/upgrading.md](docs/upgrading.md) to migrate. +> **PostgreSQL-only since v0.19** — MeshCore Hub runs on PostgreSQL (bundled container by default; `docker compose up` is still zero-effort). SQLite support was removed in v0.19; existing deployments can migrate in place with `meshcore-hub db migrate-to-postgres` — see [docs/upgrading.md](docs/upgrading.md) and [docs/database.md](docs/database.md). ![MeshCore Hub Web Dashboard](docs/images/web.png) @@ -350,7 +350,7 @@ meshcore-hub/ │ ├── hosting/ # Reverse proxy hosting guides │ ├── configuration.md # Single source of truth for environment variables │ ├── content.md # Custom content setup guide -│ ├── database.md # Database backends (SQLite/PostgreSQL) reference +│ ├── database.md # Database (PostgreSQL) reference │ ├── deployment.md # Production setup, scaling, Redis, multi-instance │ ├── i18n.md # Translation reference guide │ ├── letsmesh.md # LetsMesh packet decoding details @@ -372,7 +372,7 @@ meshcore-hub/ - [docs/observer.md](docs/observer.md) - Remote packet-capture observers and `PACKETCAPTURE_*` reference - [docs/routes.md](docs/routes.md) - Route health monitoring and link status - [docs/maintenance.md](docs/maintenance.md) - Backup and restore procedures -- [docs/database.md](docs/database.md) - Database backends (SQLite/PostgreSQL) and migration +- [docs/database.md](docs/database.md) - Database (PostgreSQL) reference and upgrade path - [docs/upgrading.md](docs/upgrading.md) - Upgrade guide for breaking changes - [docs/letsmesh.md](docs/letsmesh.md) - LetsMesh packet decoding details - [docs/seeding.md](docs/seeding.md) - Seed data format and import guide diff --git a/alembic.ini b/alembic.ini index ee03a0b..84641cb 100644 --- a/alembic.ini +++ b/alembic.ini @@ -36,8 +36,9 @@ recursive_version_locations = false # the output encoding used when revision files are written from script.py.mako output_encoding = utf-8 -# Database URL - can be overridden by environment variable -sqlalchemy.url = sqlite:///./meshcore.db +# Database URL is resolved from the environment by alembic/env.py via +# CommonSettings.effective_database_url (explicit DATABASE_URL or the +# DATABASE_* components); no URL is set here. [post_write_hooks] diff --git a/alembic/env.py b/alembic/env.py index 4da8bc3..b64ddee 100644 --- a/alembic/env.py +++ b/alembic/env.py @@ -20,34 +20,25 @@ target_metadata = Base.metadata def get_database_url() -> str: - """Get the database URL using the same resolution as the app. + """Get the PostgreSQL URL using the same resolution as the app. - Delegates to CommonSettings.effective_database_url so DATABASE_BACKEND=postgres - (+ DATABASE_* components) and an explicit DATABASE_URL are honoured identically to - the running services — otherwise migrations would silently target SQLite. + Delegates to CommonSettings.effective_database_url so the DATABASE_* + components and an explicit DATABASE_URL are honoured identically to the + running services. """ - from pathlib import Path - from meshcore_hub.common.config import CommonSettings - url = CommonSettings().effective_database_url - # Ensure the parent directory exists for SQLite file URLs. - if url.startswith("sqlite:///"): - db_path = Path(url.replace("sqlite:///", "")) - db_path.parent.mkdir(parents=True, exist_ok=True) - return url + return CommonSettings().effective_database_url -def get_schema(url: str) -> str | None: +def get_schema(url: str) -> str: """Postgres schema to migrate into, or None for SQLite. Each Hub instance keeps its tables and alembic_version in its own schema so multiple instances (prod, stg, ...) can share one Postgres database with independent migration state. """ - if url.startswith(("postgresql", "postgres")): - return os.environ.get("DATABASE_SCHEMA", "meshcorehub") - return None + return os.environ.get("DATABASE_SCHEMA", "meshcorehub") def run_migrations_offline() -> None: @@ -68,11 +59,8 @@ def run_migrations_offline() -> None: target_metadata=target_metadata, literal_binds=True, dialect_opts={"paramstyle": "named"}, - # Batch mode is a SQLite-only workaround for its limited ALTER TABLE; - # Postgres performs ALTERs directly. - render_as_batch=url.startswith("sqlite"), version_table_schema=schema, - include_schemas=schema is not None, + include_schemas=True, ) with context.begin_transaction(): @@ -97,20 +85,17 @@ def run_migrations_online() -> None: ) with connectable.connect() as connection: - # Ensure the instance's schema exists and scope this connection to it so - # tables (and alembic_version) are created there. No-op for SQLite. - if schema is not None: - connection.execute(text(f'CREATE SCHEMA IF NOT EXISTS "{schema}"')) - connection.execute(text(f'SET search_path TO "{schema}"')) - connection.commit() + # Ensure the instance's schema exists and scope this connection to it + # so tables (and alembic_version) are created there. + connection.execute(text(f'CREATE SCHEMA IF NOT EXISTS "{schema}"')) + connection.execute(text(f'SET search_path TO "{schema}"')) + connection.commit() context.configure( connection=connection, target_metadata=target_metadata, - # Batch mode is a SQLite-only workaround for its limited ALTER TABLE. - render_as_batch=url.startswith("sqlite"), version_table_schema=schema, - include_schemas=schema is not None, + include_schemas=True, ) with context.begin_transaction(): diff --git a/docker-compose.test-db.yml b/docker-compose.test-db.yml new file mode 100644 index 0000000..f8d652a --- /dev/null +++ b/docker-compose.test-db.yml @@ -0,0 +1,38 @@ +# ========================================================================== +# Throwaway PostgreSQL instance for the backend test suite (pytest). +# +# NOT part of the application stack. Start/stop with: +# make test-db-up +# make test-db-down +# +# The Makefile pins -p meshcore-hub-test-db so a COMPOSE_PROJECT_NAME set in +# the repo .env (the dev stack) can never make `down` target dev services. +# +# The meshcorehub role owns the meshcorehub_test database (image entrypoint), +# which is exactly the privilege the per-xdist-worker test schemas need +# (CREATE SCHEMA) — no superuser and no CREATEDB required. The data directory +# is a tmpfs mount, so nothing survives `down`. +# ========================================================================== +name: meshcore-hub-test-db + +services: + test-postgres: + image: postgres:17-alpine + container_name: meshcore-hub-test-postgres + ports: + - "127.0.0.1:55432:5432" + environment: + - POSTGRES_USER=meshcorehub + - POSTGRES_PASSWORD=meshcorehub-test + - POSTGRES_DB=meshcorehub_test + # Every pytest-xdist worker holds its own pooled engine (pool_size=20 + + # max_overflow=30) against this one instance, so raise max_connections + # above the Postgres default of 100. + command: ["postgres", "-c", "max_connections=200"] + tmpfs: + - /var/lib/postgresql/data:size=512m + healthcheck: + test: ["CMD-SHELL", "pg_isready -U meshcorehub -d meshcorehub_test"] + interval: 5s + timeout: 5s + retries: 10 diff --git a/docker-compose.yml b/docker-compose.yml index 89ef84b..993cfd2 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -148,21 +148,25 @@ services: retries: 3 # ========================================================================== - # PostgreSQL - optional database backend (alternative to default SQLite) - # Enable with: DATABASE_BACKEND=postgres (+ DATABASE_* vars) AND the - # 'postgres' profile, e.g. `docker compose --profile core --profile postgres up`. + # PostgreSQL - the database backend (the only one since v0.19) + # Part of the core/all profiles so a default `compose up` is zero-effort; + # the explicit 'postgres' profile also starts it standalone. # POSTGRES_* init vars derive from the same DATABASE_* values (single source # of truth); the schema itself is created by `db upgrade` (alembic). + # The default password is dev-only — override DATABASE_PASSWORD in + # production (the container is not published outside the compose network). # ========================================================================== postgres: image: postgres:17-alpine container_name: ${COMPOSE_PROJECT_NAME:-hub}-postgres profiles: + - all + - core - postgres restart: unless-stopped environment: - POSTGRES_USER=${DATABASE_USER:-meshcorehub} - - POSTGRES_PASSWORD=${DATABASE_PASSWORD:-} + - POSTGRES_PASSWORD=${DATABASE_PASSWORD:-meshcorehub} - POSTGRES_DB=${DATABASE_NAME:-meshcorehub} volumes: - postgres_data:/var/lib/postgresql/data @@ -209,14 +213,13 @@ services: - CHANNEL_REFRESH_INTERVAL_SECONDS=${CHANNEL_REFRESH_INTERVAL_SECONDS:-300} - DATA_HOME=/data - SEED_HOME=/seed - # Database backend (SQLite default; set DATABASE_BACKEND=postgres to switch) - - DATABASE_BACKEND=${DATABASE_BACKEND:-sqlite} + # PostgreSQL connection (bundled postgres service or external) - DATABASE_HOST=${DATABASE_HOST:-postgres} - DATABASE_PORT=${DATABASE_PORT:-5432} - DATABASE_NAME=${DATABASE_NAME:-meshcorehub} - DATABASE_SCHEMA=${DATABASE_SCHEMA:-meshcorehub} - DATABASE_USER=${DATABASE_USER:-meshcorehub} - - DATABASE_PASSWORD=${DATABASE_PASSWORD:-} + - DATABASE_PASSWORD=${DATABASE_PASSWORD:-meshcorehub} # Webhook configuration - WEBHOOK_ADVERTISEMENT_URL=${WEBHOOK_ADVERTISEMENT_URL:-} - WEBHOOK_ADVERTISEMENT_SECRET=${WEBHOOK_ADVERTISEMENT_SECRET:-} @@ -298,14 +301,13 @@ services: - MQTT_TRANSPORT=${MQTT_TRANSPORT:-websockets} - MQTT_WS_PATH=${MQTT_WS_PATH:-/} - DATA_HOME=/data - # Database backend (SQLite default; set DATABASE_BACKEND=postgres to switch) - - DATABASE_BACKEND=${DATABASE_BACKEND:-sqlite} + # PostgreSQL connection (bundled postgres service or external) - DATABASE_HOST=${DATABASE_HOST:-postgres} - DATABASE_PORT=${DATABASE_PORT:-5432} - DATABASE_NAME=${DATABASE_NAME:-meshcorehub} - DATABASE_SCHEMA=${DATABASE_SCHEMA:-meshcorehub} - DATABASE_USER=${DATABASE_USER:-meshcorehub} - - DATABASE_PASSWORD=${DATABASE_PASSWORD:-} + - DATABASE_PASSWORD=${DATABASE_PASSWORD:-meshcorehub} - API_HOST=0.0.0.0 - API_PORT=8000 # Worker processes for multi-core concurrency (1 = single process). @@ -455,23 +457,20 @@ services: - core - migrate restart: "no" - # Wait for postgres only when it's running (postgres profile active); - # required:false means SQLite deployments don't pull it in or block on it. depends_on: postgres: condition: service_healthy - required: false + required: true volumes: - data:/data environment: - DATA_HOME=/data - - DATABASE_BACKEND=${DATABASE_BACKEND:-sqlite} - DATABASE_HOST=${DATABASE_HOST:-postgres} - DATABASE_PORT=${DATABASE_PORT:-5432} - DATABASE_NAME=${DATABASE_NAME:-meshcorehub} - DATABASE_SCHEMA=${DATABASE_SCHEMA:-meshcorehub} - DATABASE_USER=${DATABASE_USER:-meshcorehub} - - DATABASE_PASSWORD=${DATABASE_PASSWORD:-} + - DATABASE_PASSWORD=${DATABASE_PASSWORD:-meshcorehub} command: ["db", "upgrade"] # ========================================================================== diff --git a/docs/configuration.md b/docs/configuration.md index 3a94c78..d9c33d3 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -17,7 +17,7 @@ Process-wide and MQTT-broker settings used by every service. For multi-instance | `COMPOSE_PROJECT_NAME` | `hub` | Docker Compose project name; prefixes container and volume names. Change per instance when running multiple deployments on the same host | | `IMAGE_VERSION` | `latest` | Docker image tag to use (`latest`, `main`, `v1.0.0`, etc.) | | `LOG_LEVEL` | `INFO` | Logging level (`DEBUG`, `INFO`, `WARNING`, `ERROR`, `CRITICAL`) | -| `DATA_HOME` | `./data` | Base directory for runtime data (the SQLite file lives under `${DATA_HOME}/collector/meshcore.db`) | +| `DATA_HOME` | `./data` | Base directory for runtime data (seed/health/content files under `${DATA_HOME}/collector` and `${DATA_HOME}/web`) | | `SEED_HOME` | `./seed` | Directory containing seed data files | | `TZ` | `UTC` | IANA timezone for displaying dates/times (e.g. `America/New_York`, `Europe/London`) | | `MQTT_HOST` | `localhost` (`mqtt` in Docker) | MQTT broker hostname | @@ -34,11 +34,10 @@ Process-wide and MQTT-broker settings used by every service. For multi-instance ## Database -MeshCore Hub defaults to **SQLite** (zero-config, single host). Set `DATABASE_BACKEND=postgres` to switch to **PostgreSQL** for write scaling, multi-host deployments, and multiple instances sharing one cluster via schema-per-instance. Postgres is opt-in — leave the `DATABASE_*` variables unset to keep using SQLite. See [database.md](database.md) for the full backend reference: bundled container, production role/database provisioning, managed/external Postgres, schema-per-instance isolation, and the SQLite → PostgreSQL migration runbook. +MeshCore Hub runs on **PostgreSQL** — the only supported database backend since v0.19. The bundled container is part of the default `core` compose profile (zero-config), and the same `DATABASE_*` variables point the services at a managed/external instance or a shared cluster with schema-per-instance isolation. See [database.md](database.md) for the full reference: bundled container, production role/database provisioning, managed/external Postgres, and schema-per-instance isolation. Upgrading an old SQLite deployment? See [upgrading.md](upgrading.md) (v0.19 section). | Variable | Default | Description | | --- | --- | --- | -| `DATABASE_BACKEND` | `sqlite` | `sqlite` or `postgres`. Explicit switch — Postgres is never selected implicitly | | `DATABASE_HOST` | `postgres` | Postgres hostname (`postgres` = bundled container service name) | | `DATABASE_PORT` | `5432` | Postgres port | | `DATABASE_NAME` | `meshcorehub` | Database name | diff --git a/docs/database.md b/docs/database.md index 5ee8b82..b880a5e 100644 --- a/docs/database.md +++ b/docs/database.md @@ -1,40 +1,24 @@ # Database -MeshCore Hub supports two database backends: **SQLite** (the zero-config default) and **PostgreSQL** (optional, for write scaling and multi-host deployments). Postgres is opt-in — leave the `DATABASE_*` variables unset to keep using SQLite. +MeshCore Hub runs on **PostgreSQL** — the only supported database backend since v0.19. The bundled container is part of the default `core` compose profile, so a standard `docker compose up` works with zero configuration; managed/external Postgres and multiple schema-isolated instances sharing one cluster are equally supported. > [!NOTE] -> As of v0.14, SQLite is **deprecated** in favour of PostgreSQL. SQLite remains the default and continues to work unchanged; support will be removed in a future release (at least 3 months out). New deployments that need to scale across hosts should pick Postgres. Existing SQLite deployments can move to Postgres with a one-command migration — see [Migrating from SQLite to PostgreSQL](#migrating-from-sqlite-to-postgresql) and [upgrading.md](upgrading.md). +> SQLite support (the pre-v0.14 default) was **removed in v0.19**. Existing SQLite deployments can migrate in place with the built-in `meshcore-hub db migrate-to-postgres` command — see [Upgrading from SQLite](#upgrading-from-sqlite) below and the [v0.19 upgrade guide](upgrading.md). -## SQLite (default) +## Docker (bundled container) -SQLite needs no configuration. The database file is created automatically on first run and lives under `DATA_HOME` (see [configuration.md → Common](configuration.md#common)): - -``` -${DATA_HOME}/collector/meshcore.db -``` - -In Docker Compose this is the `hub_data` volume (`${COMPOSE_PROJECT_NAME:-hub}_data`). WAL mode is enabled automatically, allowing concurrent readers alongside the collector's single writer. - -**Limitations:** writes are serialised to one process, and SQLite's file locking does **not** work over network filesystems — it caps you at a single host. To scale writes or run the stack across multiple hosts, switch to PostgreSQL. - -## PostgreSQL - -Set `DATABASE_BACKEND=postgres` and fill in the `DATABASE_*` connection variables. Postgres is never selected implicitly — the explicit switch avoids a silent backend change. For the full variable reference, see [configuration.md → Database](configuration.md#database). - -### Docker (bundled container) - -Postgres is bundled behind the `postgres` compose profile: +Postgres ships with the stack and starts with the `core` profile — no extra flags: ```bash -# Start the stack on Postgres (bundled container) -docker compose -f docker-compose.yml -f docker-compose.dev.yml \ - --profile postgres --profile core up -d - -# Start on SQLite (default — no postgres profile) +# Default stack: bundled Postgres starts alongside the app services docker compose -f docker-compose.yml -f docker-compose.dev.yml --profile core up -d ``` -### Production provisioning (role and database) +The container derives `POSTGRES_USER` / `POSTGRES_PASSWORD` / `POSTGRES_DB` from `DATABASE_USER` / `DATABASE_PASSWORD` / `DATABASE_NAME`, and ships a **dev-only default password** (`meshcorehub`) so a default `up` initializes out of the box. **Production deployments must override `DATABASE_PASSWORD`** — the container is not published outside the compose network, but the default password is public knowledge (it is in the repo). + +The `migrate` service waits for Postgres to be healthy and runs `db upgrade` before `collector` and `api` start. + +## Production provisioning (role and database) The bundled container provisions the role and database for you on first start from the `DATABASE_*` values. For a **managed or external** Postgres, create them once before pointing Hub at it. This mirrors the init script used in the [ipnet-mesh/infrastructure](https://github.com/ipnet-mesh/infrastructure/blob/main/etc/postgres/init/02_meshcorehub_db.sh) cluster: @@ -47,12 +31,11 @@ GRANT ALL PRIVILEGES ON DATABASE meshcorehub TO meshcorehub; The application **schema** and tables are created automatically by `db upgrade` (run by the `migrate` service on startup); the role just needs `CREATE` privilege on the database. Hub only ever connects as `DATABASE_USER` — no admin or bootstrap credentials are needed at runtime. -### Managed or external Postgres +## Managed or external Postgres -To point Hub at an already-running Postgres (e.g. a managed cloud instance), set `DATABASE_HOST` at it and **do not** activate the `postgres` profile: +To point Hub at an already-running Postgres (e.g. a managed cloud instance), set the `DATABASE_*` connection variables. The bundled container still starts with the `core` profile (it simply runs idle if the services point elsewhere); to skip it entirely, start a narrower profile set, e.g. `--profile collector --profile api`: ```bash -DATABASE_BACKEND=postgres DATABASE_HOST=your-managed-postgres.example.com DATABASE_PORT=5432 DATABASE_NAME=meshcorehub @@ -66,6 +49,8 @@ For advanced cases (custom driver, extra query params), set a full SQLAlchemy UR DATABASE_URL=postgresql+psycopg2://meshcorehub:your-password@host:5432/meshcorehub ``` +Missing connection configuration (no `DATABASE_URL` and no `DATABASE_*` components) fails fast at startup with an error naming the missing variables — Hub never silently falls back to a default. + ## Schema-per-instance (`search_path`) Each Hub instance is isolated to its own Postgres **schema** via the connection's `search_path`, rather than its own database. This lets several instances (e.g. `prod`, `stg`) share **one** Postgres cluster without colliding — each gets its own tables and its own `alembic_version`. @@ -75,12 +60,10 @@ Give every instance a distinct `DATABASE_SCHEMA`: ```bash # Production (.env) COMPOSE_PROJECT_NAME=hub -DATABASE_BACKEND=postgres DATABASE_SCHEMA=meshcorehub_prod # Staging (.env, separate directory) COMPOSE_PROJECT_NAME=hub-beta -DATABASE_BACKEND=postgres DATABASE_SCHEMA=meshcorehub_stg ``` @@ -88,8 +71,8 @@ The schema is created automatically on `db upgrade` if it does not exist, so no > **Note:** This is the database-level isolation for instances sharing a Postgres cluster. For running multiple instances on the same Docker host (separate volumes, Traefik routing), see [Multi-Instance Deployments](deployment.md#multi-instance-deployments). -## Migrating from SQLite to PostgreSQL +## Upgrading from SQLite -Existing SQLite deployments can be moved to Postgres with a single built-in command (`meshcore-hub db migrate-to-postgres`), which copies every table in foreign-key order through the ORM and prints a per-table row-count reconciliation. Downtime is required while writers are stopped; the source SQLite file is never modified. +Deployments still on SQLite (the pre-v0.14 default, deprecated in v0.14 and removed in v0.19) can be moved to Postgres in place with a single built-in command (`meshcore-hub db migrate-to-postgres`), which copies every table in foreign-key order through the ORM and prints a per-table row-count reconciliation. Downtime is required while writers are stopped; the source SQLite file is never modified. The command is retained through v0.19 and scheduled for removal in v0.20. -See the **v0.14 upgrade guide** in [upgrading.md](upgrading.md#migrating-an-existing-sqlite-database-to-postgres) for the full step-by-step runbook (backup, stop writers, bring up Postgres, run the migration, restart on Postgres). +See the **v0.19 upgrade guide** in [upgrading.md](upgrading.md) for the full step-by-step runbook (backup, stop writers, bring up Postgres, run the migration, restart). diff --git a/docs/deployment.md b/docs/deployment.md index 1edef94..5147a92 100644 --- a/docs/deployment.md +++ b/docs/deployment.md @@ -80,7 +80,7 @@ TRAEFIK_PRIORITY=20 This ensures `beta.example.com` (priority 20) is matched before the production wildcard `*.example.com` (priority 10). For other services on the same network (e.g., an MQTT broker at `mqtt.example.com`), use an even higher priority (e.g., 30). -> **Shared Postgres cluster:** the setup above runs each instance in its own directory with its own volumes (the default SQLite path). To instead run several instances (e.g. `prod` + `stg`) against **one** PostgreSQL cluster — isolated via a per-instance schema (`search_path`) — see [database.md](database.md#schema-per-instance-search_path). +> **Shared Postgres cluster:** the setup above runs each instance in its own directory with its own volumes (bundled Postgres container per instance). To instead run several instances (e.g. `prod` + `stg`) against **one** PostgreSQL cluster — isolated via a per-instance schema (`search_path`) — see [database.md](database.md#schema-per-instance-search_path). ## Scaling the API @@ -93,9 +93,7 @@ API_WORKERS=4 Each worker is an independent process sharing one listening socket, so the kernel balances connections across them and CPU-bound work (JSON serialisation, validation) spreads over multiple cores. Workers read their configuration from **environment variables** (CLI flags are not propagated to forked workers), which is how Docker Compose already supplies config. Enabling Redis (`REDIS_ENABLED=true`) is recommended so all workers share one cache. -Pick a worker count around the number of CPU cores available to the container; start with `2`–`4` and measure under realistic load. - -**SQLite caveat:** all workers share one SQLite file on the same host (WAL mode lets concurrent readers coexist with the single writer), but writes do not scale and this does not extend across hosts. To scale the API across hosts, switch to PostgreSQL (`DATABASE_BACKEND=postgres`) — the API requires no code changes. See [database.md](database.md) for backend setup and the SQLite → Postgres migration runbook. +Pick a worker count around the number of CPU cores available to the container; start with `2`–`4` and measure under realistic load. Workers all talk to the same PostgreSQL database (the bundled container or an external instance — see [database.md](database.md)), so scaling extends across hosts unchanged. > Prefer `API_WORKERS` over running multiple `api` containers (`--scale api=N`): the `api` service uses a fixed `container_name`, and one process-managed container per stack keeps logs, health checks, and monitoring simple. diff --git a/docs/plans/20260829-2312-remove-sqlite-support/plan.md b/docs/plans/20260829-2312-remove-sqlite-support/plan.md index 2546c5d..150c93f 100644 --- a/docs/plans/20260829-2312-remove-sqlite-support/plan.md +++ b/docs/plans/20260829-2312-remove-sqlite-support/plan.md @@ -213,6 +213,28 @@ ends with the full suite green. - `tests/test_collector/conftest.py::async_db_session` (in-memory `sqlite+aiosqlite:///:memory:` + pragma listener) → async session from the worker-schema `DatabaseManager` (production `async_session()` path). + - `tests/test_collector/test_cli.py` — `TestChannelCommands` has its own + `cli_db_url` fixture (line 128) creating real temp-file SQLite databases + (usages at 287, 406, 439, 699), plus `sqlite:///` mock-settings URL + strings (lines 48, 66, 93, 113) → point `cli_db_url` at the worker-schema + Postgres engine (truncate between tests) and sweep the mock URLs. + - `tests/test_api/test_metrics.py` (lines 89, 381) — builds isolated app + instances from `sqlite:///{test_db_path}`, consuming the `test_db_path` + fixture Phase 1 deletes → create them against the worker-schema Postgres + URL (truncate to isolate from the shared client). + - `tests/test_collector/test_tag_import.py::db_manager` (line 180, in-memory + `DatabaseManager`) → worker-schema `DatabaseManager`. + - `tests/test_collector/test_subscriber.py:1376` — + `database_url="sqlite:///:memory:"` factory arg (MQTT is mocked, the URL + is opaque) → sweep to the Postgres test URL. + - Reword stale SQLite comments/docstrings the Gate 2 grep would flag: + `tests/test_api/test_dashboard.py:30,1366` (the `_date_bucket_key` tests + themselves stay — the helper is dialect-neutral), + `tests/test_collector/test_handlers/test_contacts.py:76`, + `tests/test_api/test_user_profiles.py:103`, + `tests/test_api/test_routes.py:521` ("the shared SQLite file" — stale once + fixtures convert), `tests/test_collector/test_spam.py:4`, + `tests/test_collector/test_cli.py:126`. - `tests/test_api/conftest.py` and `tests/test_collector/conftest.py` proper: delete the `db_backend == "postgres"` branches and the SQLite pragma listeners; `api_db_engine` / `db_manager` use the schema-scoped @@ -254,7 +276,8 @@ ends with the full suite green. - `api/routes/dashboard.py`: update the SQLite-contrast comments (lines 104, 433, 451); the `func.date()` str-vs-date coercion stays (it is the dialect-neutral fix from plan `20260616-2023-fix-postgres-charts-flatline`). - `collector/spam.py` has no SQLite references (verified) — no change there. + `collector/spam.py` needs only a docstring reword — its module docstring + says "Postgres/SQLite" (line 7); no code changes there. - `common/db_migrate.py` + the `db migrate-to-postgres` command in `__main__.py`: **kept** in v0.19 (decided); annotate the module docstring with its v0.20 removal. @@ -395,9 +418,10 @@ ends with the full suite green. ## Review -**Status**: Approved with Changes +**Status**: Approved -**Reviewed**: 2026-08-29 +**Reviewed**: 2026-08-30 (second pass; first review 2026-08-29 resolved the +original open questions — those resolutions are retained below) ### Resolutions @@ -436,13 +460,25 @@ ends with the full suite green. `tests/test_api/test_app_factory.py:52-79` (asserts SQLite default URL resolution) and cosmetic mock-URL sweep in `tests/test_api/test_cache.py`. - **Factual correction:** Phase 2 originally cited SQLite comments in - `collector/spam.py`; verified no SQLite references exist there. Removed; - `api/routes/dashboard.py:104,433,451` confirmed as the real sites. + `collector/spam.py`; the first review found no *code* references there and + `api/routes/dashboard.py:104,433,451` were confirmed as the real sites. + Second pass correction: `collector/spam.py:7` (module docstring) does say + "Postgres/SQLite" — a wording-only fix is now listed in Phase 2 (Gate 2's + case-insensitive grep would otherwise fail). - **Connection-pool pressure (new, found in review):** added Risk + mitigation (`max_connections=200` on the test container). - **mesh-link-monitoring coordination (new, found in review):** approved but unimplemented plan designs dual-backend code; added Risk with ordering rule (this plan lands first; link-monitoring implementation drops SQLite arms). +- **Missed real-SQLite test files (second pass, 2026-08-30):** Phase 1 now + also enumerates `tests/test_collector/test_cli.py` (`TestChannelCommands` + real temp-file SQLite DBs + mock URLs), `tests/test_api/test_metrics.py:89,381` + (isolated app instances via the `test_db_path` fixture being deleted), + `tests/test_collector/test_tag_import.py:180` (in-memory `DatabaseManager`), + and `tests/test_collector/test_subscriber.py:1376` (`:memory:` factory arg), + plus wording sweeps in six comment-only test locations. All previously would + have surfaced only as Gate 2 grep failures or broken tests after fixture + deletion. ### Remaining Action Items diff --git a/docs/plans/20260829-2312-remove-sqlite-support/tasks.md b/docs/plans/20260829-2312-remove-sqlite-support/tasks.md new file mode 100644 index 0000000..abe7897 --- /dev/null +++ b/docs/plans/20260829-2312-remove-sqlite-support/tasks.md @@ -0,0 +1,117 @@ +# Tasks: Remove SQLite Support — PostgreSQL-Only from v0.19 + +> Generated from `plan.md` on 2026-08-30 + +## Phase 1: Postgres-only test harness (convert suite first) + +- [x] Add throwaway test-database stack + - [x] Create `docker-compose.test-db.yml`: `postgres:17-alpine` published on `127.0.0.1:55432`, `POSTGRES_USER=meshcorehub`, `POSTGRES_PASSWORD=meshcorehub-test`, `POSTGRES_DB=meshcorehub_test`, ephemeral volume, `command: ["postgres", "-c", "max_connections=200"]` (role owns the DB — no superuser/CREATEDB) +- [x] Add Makefile targets for the test DB + - [x] Add `test-db-up` / `test-db-down` + - [x] Reference them in the `test` / `test-unit` target comments (reachability enforced by pytest, not shell probing) +- [x] Rewrite `tests/conftest.py` core fixtures + - [x] Delete `db_engine` (in-memory SQLite), `test_db_path` (temp file), `db_backend`, and the SQLite branch of `db_url` + - [x] Make `db_url` Postgres-only via `TEST_POSTGRES_URL` (default `postgresql+psycopg2://meshcorehub:meshcorehub-test@localhost:55432/meshcorehub_test`); missing/unreachable DB → `pytest.exit()` with an actionable "run `make test-db-up`" message + - [x] Worker isolation via schema: derive `schema = f"hub_test_{worker_id}"`, create it with a plain `CREATE SCHEMA IF NOT EXISTS` connection, build the engine via production `create_database_engine(db_url, schema=...)` (search_path + UTC wiring exercised); teardown `DROP SCHEMA "" CASCADE` + - [x] Provide Postgres-backed `db_engine`/`db_session` equivalents bound to the worker schema, with the truncate-between-tests pattern +- [x] Clean the component conftests + - [x] `tests/test_api/conftest.py`: delete `db_backend == "postgres"` branches and SQLite pragma listeners; `api_db_engine` uses the schema-scoped engine unconditionally + - [x] `tests/test_collector/conftest.py`: same cleanup; rebuild `async_db_session` (in-memory aiosqlite + pragma listener) as an async session from the worker-schema `DatabaseManager` (production `async_session()` path) +- [x] Convert local SQLite fixtures not reached by the shared rewrite + - [x] `tests/test_api/test_channel_visibility.py:23` — own in-memory `db_session` → shared Postgres fixture + - [x] `tests/test_common/test_models.py:26` and `tests/test_common/test_channel_model.py:19` — local `db_session` → shared fixture + - [x] `tests/test_collector/test_cli.py` — `TestChannelCommands::cli_db_url` (line 128, real temp-file SQLite; usages 287, 406, 439, 699) → worker-schema Postgres engine with truncate between tests + - [x] `tests/test_api/test_metrics.py:89,381` — isolated app instances built from `sqlite:///{test_db_path}` (fixture being deleted) → worker-schema Postgres URL with truncate isolation + - [x] `tests/test_collector/test_tag_import.py:180` — in-memory `DatabaseManager` → worker-schema `DatabaseManager` + - [x] `tests/test_collector/test_subscriber.py:1376` — `database_url="sqlite:///:memory:"` factory arg → Postgres test URL +- [x] Reword stale SQLite comments/docstrings in tests (wording only, tests stay) + - [x] `tests/test_api/test_dashboard.py:30,1366` (`_date_bucket_key` tests stay — helper is dialect-neutral) + - [x] `tests/test_collector/test_handlers/test_contacts.py:76` + - [x] `tests/test_api/test_user_profiles.py:103` + - [x] `tests/test_api/test_routes.py:521` ("the shared SQLite file") + - [x] `tests/test_collector/test_spam.py:4` + - [x] `tests/test_collector/test_cli.py:126` (class docstring) +- [x] Rewrite SQLite-specific test files + - [x] `tests/test_common/test_database.py` — drop `test_sqlite_never_has_schema`, `test_async_engine_skips_pool_args_for_memory_sqlite`, pragma assertions; keep/add Postgres `search_path` + timezone assertions + - [x] `tests/test_common/test_config.py` — replace SQLite-default tests with the new resolution contract (components → URL, missing-var error, `DATABASE_URL` override, `DATABASE_BACKEND=sqlite` rejection, `DATABASE_BACKEND=postgres` no-op) + - [x] Verify kept tests `tests/test_common/test_db_migrate.py` + `tests/test_main.py` still pass (stdlib sqlite3 only — no aiosqlite import creeps in) +- [x] CI (`.github/workflows/ci.yml`) + - [x] Add a `postgres:17` service container (pinned to match bundled `postgres:17-alpine`) to the test job + - [x] Export `TEST_POSTGRES_URL` pointing at it; remove SQLite assumptions from the test step +- [x] **Gate 1**: full backend suite green on Postgres with SQLite still present in the code + +## Phase 2: Remove SQLite from application code + +- [x] `src/meshcore_hub/common/config.py` + - [x] Keep `database_backend` as validated-rejection field: `sqlite` raises targeted error ("SQLite support was removed in v0.19 — migrate with `db migrate-to-postgres`, see docs/upgrading.md"); `postgres` accepted as no-op + - [x] Collapse `effective_database_url` to "explicit `DATABASE_URL` or assembled Postgres URL", failing fast on missing host/name/user/password + - [x] `effective_database_schema` always returns `database_schema` +- [x] `src/meshcore_hub/common/database.py` + - [x] Delete SQLite branches: `check_same_thread`, PRAGMA event listeners (sync + async), in-memory pool special-casing, sqlite arm of `_to_async_url`, parent-dir mkdir + - [x] Make `search_path`/timezone wiring unconditional; reword SQLite-contrast comments (timezone rationale stays) +- [x] `src/meshcore_hub/common/models/event_observer.py` — `add_event_observer()` becomes a plain `sqlalchemy.dialects.postgresql.insert` upsert +- [x] Alembic + - [x] `alembic/env.py`: drop `render_as_batch` conditional and `None`-schema branch (`version_table_schema`/`include_schemas` always set) + - [x] `alembic.ini`: remove the `sqlite:///./meshcore.db` placeholder URL (line 40) +- [x] Remove SQLite defaults/help text from CLI/app entry points + - [x] `src/meshcore_hub/api/app.py:34,75` — app factory requires explicit URL or settings resolution (no file-DB default) + - [x] `src/meshcore_hub/api/cli.py:33` — default arg + help text + - [x] `src/meshcore_hub/collector/cli.py:90` — default arg + help text + - [x] `src/meshcore_hub/collector/subscriber.py:919,1005` — `database_url` defaults + - [x] `src/meshcore_hub/__main__.py` — keep only the migrate-to-postgres source-URL default (retained command) +- [x] Comment/docstring sweeps in app code + - [x] `src/meshcore_hub/api/routes/dashboard.py:104,433,451` — update SQLite-contrast comments (keep `func.date()` coercion) + - [x] `src/meshcore_hub/collector/spam.py:7` — module docstring "Postgres/SQLite" reword + - [x] `src/meshcore_hub/common/db_migrate.py` — annotate module docstring with v0.20 removal +- [x] Update SQLite-asserting tests + - [x] Rewrite `tests/test_api/test_app_factory.py:52-79` — Postgres resolution contract instead of `sqlite:////srv/hubdata/...` defaults + - [x] Sweep `tests/test_api/test_cache.py:1011-1271` mock `sqlite:///` URLs → Postgres URLs +- [x] **Gate 2**: full suite green; grep audit — no `sqlite` (case-insensitive) in `src/` except `db_migrate.py`/`__main__.py` tooling, none in `tests/` except that tooling's tests + +## Phase 3: Infra & packaging + +- [x] `docker-compose.yml` + - [x] Move `postgres` into the `core` (and `all`) profile + - [x] `migrate.depends_on.postgres`: `condition: service_healthy`, `required: true` + - [x] Drop `DATABASE_BACKEND=${DATABASE_BACKEND:-sqlite}` env lines from `collector`/`api`/`migrate` + - [x] `POSTGRES_PASSWORD=${DATABASE_PASSWORD:-meshcorehub}` and mirror the same default in app services' `DATABASE_PASSWORD` (zero-config boot); update service header comments +- [x] `Dockerfile` — remove the `sqlite3` apt package (line 90; stdlib driver suffices for the retained migration command) +- [x] Verify `docker-compose.dev.yml` / `.prod.yml` / `.traefik.yml` have no SQLite/backend assumptions +- [x] `e2e/docker-compose.test.yml` — drop the three `DATABASE_BACKEND=postgres` lines (53, 129, 172) +- [ ] **Gate 3** (user-run per AGENTS.md): user builds and brings up the `core` profile — bundled Postgres healthy, `migrate` completes, collector ingests, API/web serve + +## Phase 4: Dependencies + +- [x] `pyproject.toml` + - [x] Remove `aiosqlite` from `dependencies` + - [x] Move `asyncpg>=0.28.0` and `psycopg2-binary>=2.9.0` into `dependencies` + - [x] Keep `[postgres]` as an empty alias extra with deprecation comment ("no-op — drivers are core since v0.19; removed in v0.20") + - [x] Update the `[dev]` comment about the "dual-backend test suite" +- [x] Refresh `.venv` (`pip install -e ".[dev]"`) and confirm nothing imports `aiosqlite` +- [x] **Gate 4**: full suite + `pre-commit run --all-files` green + +## Phase 5: Documentation & upgrade story + +- [x] `docs/upgrading.md` — new v0.19 section: breaking-change notice, in-place runbook (backup → stop writers → bring up Postgres → `db migrate-to-postgres` → restart), `DATABASE_BACKEND=sqlite` rejection behaviour, bundled default password (production must override), v0.20 removal schedule (migration command, `database_backend` field, `[postgres]` extra) +- [x] `docs/database.md` — Postgres-only reference: remove "SQLite (default)" and backend-choice sections; keep bundled-container setup, managed/external Postgres, schema-per-instance provisioning, migration section repositioned as upgrade guidance +- [x] `docs/configuration.md` — `DATA_HOME` description, `DATABASE_BACKEND` row +- [x] `docs/deployment.md` — remove SQLite caveats +- [x] `docs/seeding.md` — directory tree without `meshcore.db` +- [x] `.env.example` — rewrite Database section: `DATABASE_*` defaults, no SQLite commentary, `DATA_HOME` comment without the db file +- [x] `README.md` — replace deprecation notice with "PostgreSQL-only" statement; update doc-tree descriptions +- [x] `AGENTS.md` — rewrite "Database & Ops" (bundled Postgres default; migration authoring against a local PG schema instead of `./meshcore.db`) and test commands (`make test-db-up` / `TEST_POSTGRES_URL` required) +- [x] **Gate 5**: docs sweep — grep `sqlite`, `DATA_HOME` + `meshcore.db`, `DATABASE_BACKEND` returns only intentional mentions (historical plans, upgrading.md history, retained tooling) + +## Phase 6: Final Verification + +- [x] Backend: `pytest -nauto --no-cov` (full suite on Postgres) +- [x] Frontend: `npm run test:frontend`, `npx tsc --noEmit` +- [x] `pre-commit run --all-files` +- [x] E2E toolchain: `npm run typecheck:e2e` + `npx playwright test --config=e2e/playwright.config.ts --list` (collection only) +- [x] Fresh-install proof: against a clean test database, `meshcore-hub db upgrade` from revision zero completes on Postgres +- [ ] User-run: `make build && make up` on the `core` profile; e2e via `make e2e-build && make e2e-up && make e2e-test` + +## Follow-ups & Coordination (not part of this implementation) + +- [ ] Track for v0.20: remove `db migrate-to-postgres` + `common/db_migrate.py` + its tests, the `database_backend` settings field, and the `[postgres]` extra alias (announced in the v0.19 `docs/upgrading.md`) +- [ ] When taskifying `docs/plans/20260705-2306-mesh-link-monitoring` (future session): note its SQLite branches (`sqlite_insert` arm, batch-mode DDL, `SQLITE_MAX_VARIABLE_NUMBER` avoidance) are superseded by this plan — implementation targets Postgres-only diff --git a/docs/seeding.md b/docs/seeding.md index 7b306b1..8e3a5a7 100644 --- a/docs/seeding.md +++ b/docs/seeding.md @@ -25,8 +25,8 @@ seed/ # SEED_HOME (seed data files) └── routes.yaml # Route health definitions for import data/ # DATA_HOME (runtime data) -└── collector/ - └── meshcore.db # SQLite database +├── collector/ # collector health/state files +└── web/ # web tier data ``` Example seed files are provided in `example/seed/`. diff --git a/docs/upgrading.md b/docs/upgrading.md index 4dff2bd..9e1b160 100644 --- a/docs/upgrading.md +++ b/docs/upgrading.md @@ -2,6 +2,56 @@ This guide covers upgrading from a previous MeshCore Hub release to the current version. Check the relevant version section below before upgrading. +## v0.19.0 + +### PostgreSQL-only (SQLite support removed) + +**⚠️ Breaking:** SQLite is no longer a supported database backend. PostgreSQL is the only backend — announced in v0.14 and removed here. Existing SQLite deployments must migrate their data (runbook below) before upgrading. + +What changed: + +- **The bundled `postgres` container is part of the default `core` compose profile.** A plain `docker compose up` now starts PostgreSQL alongside the app services — still zero-config, including a working default password (`meshcorehub`, overridable via `DATABASE_PASSWORD`). **Production deployments must override `DATABASE_PASSWORD`** — the default is dev-only (the container is not published outside the compose network, but don't rely on that). +- **`DATABASE_BACKEND` is rejected, not ignored.** A leftover `DATABASE_BACKEND=sqlite` in your environment now fails at startup with a targeted error pointing at the migration command; `DATABASE_BACKEND=postgres` is accepted as a no-op. Remove the variable — the field itself is removed in v0.20. +- **Missing database configuration fails fast.** With neither `DATABASE_URL` nor the `DATABASE_*` components (`DATABASE_HOST`, `DATABASE_NAME`, `DATABASE_USER`, `DATABASE_PASSWORD`) set, services refuse to start with an error naming the missing variables — no silent fallback. +- **Tests run against PostgreSQL.** The backend suite requires a Postgres instance (`make test-db-up` starts a throwaway one; `TEST_POSTGRES_URL` points at your own). CI uses a `postgres:17` service container. +- The `sqlite3` CLI was removed from the Docker image, and `aiosqlite` is no longer a dependency (the retained migration command reads SQLite via Python's stdlib driver only). + +#### Migrating an existing SQLite deployment in place + +Downtime is required while writers are stopped; the source SQLite file is never modified. + +1. **Back up first.** Copy your `meshcore.db` (or back up the `hub_data` volume — see [Backup & Restore](maintenance.md)). +2. **Stop the writers** (collector and api): + ```bash + docker compose -f docker-compose.yml -f docker-compose.dev.yml stop collector api + ``` +3. **Bring up Postgres and create the schema** (on the v0.19 image; the bundled container starts with the `core` profile): + ```bash + docker compose -f docker-compose.yml -f docker-compose.dev.yml up -d postgres + docker compose -f docker-compose.yml -f docker-compose.dev.yml run --rm migrate + ``` + `migrate` runs `db upgrade`, creating the schema, all tables (with correct native types — `boolean`, `json`, `timestamptz`), and stamping `alembic_version`. +4. **Copy the data** with the built-in command: + ```bash + docker compose -f docker-compose.yml -f docker-compose.dev.yml \ + run --rm migrate meshcore-hub db migrate-to-postgres + ``` + It defaults the source to `sqlite:///{DATA_HOME}/collector/meshcore.db` and the target to your configured `DATABASE_*` connection. It copies every table in foreign-key order through the ORM (so SQLite's dynamically typed values are converted correctly — `0/1` → `boolean`, JSON text → `json`, naive datetimes → UTC `timestamptz`), then prints a per-table source-vs-target row-count reconciliation and fails on any mismatch. Use `--dry-run` to preview counts first, and `--truncate` to overwrite a non-empty target. +5. **Remove `DATABASE_BACKEND` from your `.env`** (any value) and bring the stack back up: + ```bash + docker compose -f docker-compose.yml -f docker-compose.dev.yml --profile core up -d + ``` + +> **Managed Postgres / non-superuser roles:** the migration disables foreign-key triggers during the copy via `session_replication_role = replica`, which requires a superuser. When the target role is not a superuser (typical for managed Postgres), the command automatically falls back to copying in parent-first order instead. Pass `--no-replication-role` to force the fallback explicitly. + +#### Scheduled for removal in v0.20 + +The following are retained through v0.19 for upgrade compatibility and removed in v0.20: + +- the `meshcore-hub db migrate-to-postgres` command and its `common/db_migrate.py` module, +- the `database_backend` settings field (the `DATABASE_BACKEND` rejection), +- the `[postgres]` pip extra alias (the drivers are core dependencies since v0.19). + ## v0.18.0 ### Security hardening (fail-closed defaults) diff --git a/e2e/docker-compose.test.yml b/e2e/docker-compose.test.yml index fe0e13f..c1e4e22 100644 --- a/e2e/docker-compose.test.yml +++ b/e2e/docker-compose.test.yml @@ -50,7 +50,6 @@ services: - test_data:/data environment: - DATA_HOME=/data - - DATABASE_BACKEND=postgres - DATABASE_HOST=postgres - DATABASE_PORT=5432 - DATABASE_NAME=meshcorehub @@ -126,7 +125,6 @@ services: - MQTT_USERNAME=test-admin - MQTT_PASSWORD=test-password - DATA_HOME=/data - - DATABASE_BACKEND=postgres - DATABASE_HOST=postgres - DATABASE_PORT=5432 - DATABASE_NAME=meshcorehub @@ -169,7 +167,6 @@ services: - MQTT_USERNAME=test-admin - MQTT_PASSWORD=test-password - DATA_HOME=/data - - DATABASE_BACKEND=postgres - DATABASE_HOST=postgres - DATABASE_PORT=5432 - DATABASE_NAME=meshcorehub diff --git a/pyproject.toml b/pyproject.toml index dbb6877..2bdb9ab 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -39,7 +39,8 @@ dependencies = [ "httpx>=0.25.0", "authlib>=1.3.0", "itsdangerous>=2.0.0", - "aiosqlite>=0.19.0", + "asyncpg>=0.28.0", + "psycopg2-binary>=2.9.0", "pyyaml>=6.0.0", "python-frontmatter>=1.0.0", "prometheus-client>=0.20.0", @@ -60,15 +61,11 @@ dev = [ "beautifulsoup4>=4.12.0", "types-paho-mqtt>=1.6.0", "types-PyYAML>=6.0.0", - # Postgres drivers needed to run the dual-backend test suite - # (TEST_DATABASE_BACKEND=postgres) locally. - "asyncpg>=0.28.0", - "psycopg2-binary>=2.9.0", -] -postgres = [ - "asyncpg>=0.28.0", - "psycopg2-binary>=2.9.0", ] +# Deprecated no-op alias: the Postgres drivers are core dependencies since +# v0.19. Kept one release for external install instructions that reference +# `.[postgres]`; removed in v0.20. +postgres = [] [project.scripts] meshcore-hub = "meshcore_hub.__main__:main" diff --git a/src/meshcore_hub/api/app.py b/src/meshcore_hub/api/app.py index 4b3d1be..e54c35d 100644 --- a/src/meshcore_hub/api/app.py +++ b/src/meshcore_hub/api/app.py @@ -31,7 +31,12 @@ async def lifespan(app: FastAPI) -> AsyncGenerator[None, None]: global _db_manager # Get database URL from app state - database_url = getattr(app.state, "database_url", "sqlite:///./meshcore.db") + database_url = getattr(app.state, "database_url", None) + if not database_url: + raise RuntimeError( + "No database URL configured — pass database_url to create_app " + "or configure DATABASE_URL / DATABASE_* for the app factory" + ) # Initialize database (schema managed by Alembic migrations) logger.info(f"Initializing database: {database_url}") @@ -72,7 +77,7 @@ async def lifespan(app: FastAPI) -> AsyncGenerator[None, None]: def create_app( - database_url: str = "sqlite:///./meshcore.db", + database_url: str | None = None, read_key: str | None = None, admin_key: str | None = None, mqtt_host: str = "localhost", diff --git a/src/meshcore_hub/api/cli.py b/src/meshcore_hub/api/cli.py index 728cab8..22b94de 100644 --- a/src/meshcore_hub/api/cli.py +++ b/src/meshcore_hub/api/cli.py @@ -30,7 +30,7 @@ import click type=str, default=None, envvar="DATABASE_URL", - help="Database connection URL (default: sqlite:///{data_home}/collector/meshcore.db)", + help="Database connection URL (default: the configured PostgreSQL connection from DATABASE_URL / DATABASE_* vars)", ) @click.option( "--read-key", diff --git a/src/meshcore_hub/api/routes/dashboard.py b/src/meshcore_hub/api/routes/dashboard.py index d26772b..1b32bf9 100644 --- a/src/meshcore_hub/api/routes/dashboard.py +++ b/src/meshcore_hub/api/routes/dashboard.py @@ -101,10 +101,10 @@ def _flood_only_filter( def _date_bucket_key(value: str | date | None) -> str | None: """Coerce a DB-returned date bucket key to a canonical ``%Y-%m-%d`` string. - SQLite's ``func.date()`` returns a ``str``; Postgres returns a - ``datetime.date``. This normalizes both to the same string key so dict - lookups by ``"%Y-%m-%d"`` succeed on either backend. ``None`` passes - through unchanged. + Postgres's ``func.date()`` returns a ``datetime.date``; this normalizes it + to the same string key the frontend/serializers expect so dict lookups by + ``"%Y-%m-%d"`` succeed. String values pass through unchanged; ``None`` + passes through unchanged. """ if isinstance(value, str): return value @@ -430,7 +430,8 @@ def get_activity( start_date = end_date - timedelta(days=days) # Query advertisement counts grouped by date - # Use SQLite's date() function for grouping (returns string 'YYYY-MM-DD') + # (func.date() truncates to the UTC day boundary; the session timezone + # is pinned to UTC by the engine layer) date_expr = func.date(Advertisement.received_at) query = ( @@ -447,8 +448,8 @@ def get_activity( results = session.execute(query).all() - # Build a dict of date -> count, normalizing the key to a string so it - # works on both SQLite (func.date() returns str) and Postgres (returns date). + # Build a dict of date -> count, normalizing the key to a string + # (func.date() returns date objects on Postgres). counts_by_date = {_date_bucket_key(row.date): row.count for row in results} # Generate all dates in the range, filling in zeros for missing days diff --git a/src/meshcore_hub/collector/cli.py b/src/meshcore_hub/collector/cli.py index e5157b1..9d0e20e 100644 --- a/src/meshcore_hub/collector/cli.py +++ b/src/meshcore_hub/collector/cli.py @@ -87,7 +87,7 @@ if TYPE_CHECKING: type=str, default=None, envvar="DATABASE_URL", - help="Database connection URL (default: sqlite:///{data_home}/collector/meshcore.db)", + help="Database connection URL (default: the configured PostgreSQL connection from DATABASE_URL / DATABASE_* vars)", ) @click.option( "--log-level", diff --git a/src/meshcore_hub/collector/spam.py b/src/meshcore_hub/collector/spam.py index fd99f01..43c6f58 100644 --- a/src/meshcore_hub/collector/spam.py +++ b/src/meshcore_hub/collector/spam.py @@ -4,10 +4,10 @@ Pure helpers (`normalize_sender`, `compute_path_prefix`) plus two DB-querying scorers (`score_message`, `rescore_recent`) and a config object (`SpamConfig`). The design is described in ``docs/plans/20260622-2243-spam-detection/plan.md``. -Counts are computed directly from Postgres/SQLite over the +Counts are computed directly from PostgreSQL over the ``(path_prefix, received_at)`` and ``(sender_normalized, received_at)`` indexes; the window cutoff is always a Python-computed datetime passed as a bound -parameter, so identical code runs on both backends (no SQL ``NOW()``/``INTERVAL``). +parameter (no SQL ``NOW()``/``INTERVAL``). """ from __future__ import annotations diff --git a/src/meshcore_hub/collector/subscriber.py b/src/meshcore_hub/collector/subscriber.py index 282ae4f..9c18c4a 100644 --- a/src/meshcore_hub/collector/subscriber.py +++ b/src/meshcore_hub/collector/subscriber.py @@ -916,7 +916,7 @@ def create_subscriber( mqtt_tls: bool = False, mqtt_transport: str = "websockets", mqtt_ws_path: str = "/", - database_url: str = "sqlite:///./meshcore.db", + database_url: Optional[str] = None, webhook_dispatcher: Optional["WebhookDispatcher"] = None, cleanup_enabled: bool = False, cleanup_retention_days: int = 30, @@ -939,7 +939,7 @@ def create_subscriber( mqtt_tls: Enable TLS/SSL for MQTT connection mqtt_transport: MQTT transport protocol (tcp or websockets) mqtt_ws_path: WebSocket path (used when transport=websockets) - database_url: Database connection URL + database_url: PostgreSQL connection URL (required) webhook_dispatcher: Optional webhook dispatcher for event forwarding cleanup_enabled: Enable automatic event data cleanup cleanup_retention_days: Number of days to retain event data @@ -951,6 +951,11 @@ def create_subscriber( Returns: Configured Subscriber instance """ + if not database_url: + raise ValueError( + "database_url is required (PostgreSQL) — configure DATABASE_URL " + "or the DATABASE_* component vars" + ) # Create MQTT client with unique client ID to allow multiple collectors unique_id = uuid.uuid4().hex[:8] mqtt_config = MQTTConfig( @@ -1002,7 +1007,7 @@ def run_collector( mqtt_tls: bool = False, mqtt_transport: str = "websockets", mqtt_ws_path: str = "/", - database_url: str = "sqlite:///./meshcore.db", + database_url: Optional[str] = None, webhook_dispatcher: Optional["WebhookDispatcher"] = None, cleanup_enabled: bool = False, cleanup_retention_days: int = 30, diff --git a/src/meshcore_hub/common/config.py b/src/meshcore_hub/common/config.py index 0155976..c2a9f07 100644 --- a/src/meshcore_hub/common/config.py +++ b/src/meshcore_hub/common/config.py @@ -3,7 +3,7 @@ from enum import Enum from typing import Optional -from pydantic import Field +from pydantic import Field, field_validator from pydantic_settings import BaseSettings, SettingsConfigDict @@ -24,13 +24,6 @@ class MQTTTransport(str, Enum): WEBSOCKETS = "websockets" -class DatabaseBackend(str, Enum): - """Database backend selector.""" - - SQLITE = "sqlite" - POSTGRES = "postgres" - - class CommonSettings(BaseSettings): """Common settings shared by all components.""" @@ -46,87 +39,101 @@ class CommonSettings(BaseSettings): description="Base directory for service data (e.g., ./data or /data)", ) - # Database backend selection and connection components. - # SQLite is the zero-config default; set DATABASE_BACKEND=postgres (plus the - # DATABASE_* component vars) to use Postgres. An explicit DATABASE_URL overrides - # everything (managed/external Postgres, tests). - database_backend: DatabaseBackend = Field( - default=DatabaseBackend.SQLITE, - description="Database backend: 'sqlite' (default) or 'postgres'", + # Database connection. PostgreSQL is the only backend since v0.19: either + # an explicit DATABASE_URL or the DATABASE_* components below. The legacy + # DATABASE_BACKEND selector is kept for one release purely so a leftover + # `sqlite` setting fails with a targeted upgrade error instead of being + # silently ignored; the field is removed in v0.20. + database_backend: str = Field( + default="postgres", + description=( + "Legacy backend selector retained for one release; 'postgres' is " + "accepted (no-op) and 'sqlite' is rejected (support removed in v0.19)" + ), ) database_url: Optional[str] = Field( default=None, description=( - "Explicit SQLAlchemy database URL; overrides DATABASE_BACKEND/component vars. " - "Default: sqlite:///{data_home}/collector/meshcore.db" + "Explicit SQLAlchemy PostgreSQL URL; overrides the DATABASE_* " + "component vars (managed/external Postgres, tests)" ), ) database_host: Optional[str] = Field( default=None, - description="Postgres host (required when DATABASE_BACKEND=postgres)", + description="PostgreSQL host (required unless DATABASE_URL is set)", ) - database_port: int = Field(default=5432, description="Postgres port") + database_port: int = Field(default=5432, description="PostgreSQL port") database_name: str = Field( - default="meshcorehub", description="Postgres database name" + default="meshcorehub", description="PostgreSQL database name" ) database_schema: str = Field( default="meshcorehub", - description="Postgres schema (namespace); override per instance on a shared cluster", + description="PostgreSQL schema (namespace); override per instance on a shared cluster", + ) + database_user: str = Field( + default="meshcorehub", description="PostgreSQL role/user" ) - database_user: str = Field(default="meshcorehub", description="Postgres role/user") database_password: Optional[str] = Field( default=None, - description="Postgres password (required when DATABASE_BACKEND=postgres)", + description="PostgreSQL password (required unless DATABASE_URL is set)", ) + @field_validator("database_backend", mode="before") + @classmethod + def _validate_legacy_backend(cls, value: object) -> str: + """Reject the removed SQLite backend with a targeted upgrade error.""" + raw = "postgres" if value is None else str(value).strip().lower() + if raw == "sqlite": + raise ValueError( + "SQLite support was removed in v0.19 — migrate with " + "`meshcore-hub db migrate-to-postgres`, see docs/upgrading.md" + ) + if raw != "postgres": + raise ValueError( + f"DATABASE_BACKEND must be 'postgres' (the only supported " + f"backend), got {value!r}" + ) + return raw + @property def effective_database_url(self) -> str: - """Resolve the SQLAlchemy database URL. + """Resolve the SQLAlchemy PostgreSQL URL. - Precedence: explicit DATABASE_URL > postgres (assembled from components) > - SQLite default under DATA_HOME. Fails fast for a misconfigured postgres backend - rather than silently falling back to SQLite. + Precedence: explicit DATABASE_URL > URL assembled from the DATABASE_* + components. Fails fast when components are missing rather than + silently falling back to anything. """ if self.database_url: return self.database_url - if self.database_backend == DatabaseBackend.POSTGRES: - missing = [ - name - for name, value in ( - ("DATABASE_HOST", self.database_host), - ("DATABASE_NAME", self.database_name), - ("DATABASE_USER", self.database_user), - ("DATABASE_PASSWORD", self.database_password), - ) - if not value - ] - if missing: - raise ValueError( - "DATABASE_BACKEND=postgres requires: " + ", ".join(missing) - ) - from urllib.parse import quote_plus - - user = quote_plus(self.database_user) - password = quote_plus(self.database_password or "") - return ( - f"postgresql+psycopg2://{user}:{password}" - f"@{self.database_host}:{self.database_port}/{self.database_name}" + missing = [ + name + for name, value in ( + ("DATABASE_HOST", self.database_host), + ("DATABASE_NAME", self.database_name), + ("DATABASE_USER", self.database_user), + ("DATABASE_PASSWORD", self.database_password), ) - from pathlib import Path + if not value + ] + if missing: + raise ValueError( + "PostgreSQL connection is not configured: missing " + + ", ".join(missing) + + " (or set DATABASE_URL)" + ) + from urllib.parse import quote_plus - db_path = Path(self.data_home) / "collector" / "meshcore.db" - return f"sqlite:///{db_path}" + user = quote_plus(self.database_user) + password = quote_plus(self.database_password or "") + return ( + f"postgresql+psycopg2://{user}:{password}" + f"@{self.database_host}:{self.database_port}/{self.database_name}" + ) @property - def effective_database_schema(self) -> Optional[str]: - """Postgres schema to scope connections to, or None for SQLite. - - Returns the schema only when the effective URL is Postgres; SQLite has no - schema concept, so callers leave search_path untouched. - """ - if self.effective_database_url.startswith(("postgresql", "postgres")): - return self.database_schema - return None + def effective_database_schema(self) -> str: + """PostgreSQL schema to scope connections to (search_path).""" + return self.database_schema # Logging log_level: LogLevel = Field(default=LogLevel.INFO, description="Logging level") diff --git a/src/meshcore_hub/common/database.py b/src/meshcore_hub/common/database.py index f875daf..c39cad3 100644 --- a/src/meshcore_hub/common/database.py +++ b/src/meshcore_hub/common/database.py @@ -6,7 +6,7 @@ import os from contextlib import asynccontextmanager, contextmanager from typing import Any, AsyncGenerator, Generator -from sqlalchemy import create_engine, event +from sqlalchemy import create_engine from sqlalchemy.engine import Engine from sqlalchemy.ext.asyncio import AsyncEngine, AsyncSession, create_async_engine from sqlalchemy.orm import Session, sessionmaker @@ -16,38 +16,27 @@ from meshcore_hub.common.models.base import Base logger = logging.getLogger(__name__) -def _resolve_pg_schema(database_url: str, schema: str | None) -> str | None: +def _resolve_pg_schema(schema: str | None) -> str | None: """Resolve the Postgres schema to scope a connection to (search_path). - Returns None for SQLite (no schema concept). For Postgres, an explicit ``schema`` - wins; otherwise it falls back to the ``DATABASE_SCHEMA`` env var. The CLI's - ``load_dotenv()`` and Docker both populate that var, so runtime entrypoints don't - need to thread the schema through every constructor. + An explicit ``schema`` wins; otherwise it falls back to the + ``DATABASE_SCHEMA`` env var. The CLI's ``load_dotenv()`` and Docker both + populate that var, so runtime entrypoints don't need to thread the schema + through every constructor. """ - if not database_url.startswith(("postgresql", "postgres")): - return None - if schema: - return schema - return os.environ.get("DATABASE_SCHEMA") + return schema or os.environ.get("DATABASE_SCHEMA") def _to_async_url(database_url: str) -> str: - """Map a sync database URL to its async-driver equivalent. + """Map a sync database URL to its asyncpg equivalent. - Postgres always maps to asyncpg for the async engine, even when the sync URL - names a sync driver (e.g. ``postgresql+psycopg2://``, which is what the config - assembler produces) — otherwise async sessions would try to use the sync driver. - SQLite maps to aiosqlite unless a driver is already specified. + The async engine always uses asyncpg, even when the sync URL names a sync + driver (e.g. ``postgresql+psycopg2://``, which is what the config + assembler produces) — otherwise async sessions would try to use the sync + driver. """ scheme = database_url.split("://", 1)[0] - dialect = scheme.split("+", 1)[0] - if dialect in ("postgresql", "postgres"): - return database_url.replace(f"{scheme}://", "postgresql+asyncpg://", 1) - if dialect == "sqlite": - if "+" in scheme: - return database_url - return database_url.replace("sqlite://", "sqlite+aiosqlite://", 1) - return database_url + return database_url.replace(f"{scheme}://", "postgresql+asyncpg://", 1) def create_database_engine( @@ -60,43 +49,35 @@ def create_database_engine( Args: database_url: SQLAlchemy database URL echo: Enable SQL query logging - schema: Postgres schema to scope connections to via search_path. Defaults to - the DATABASE_SCHEMA env var when not given. Ignored for SQLite. + schema: Postgres schema to scope connections to via search_path. + Defaults to the DATABASE_SCHEMA env var when not given. Returns: SQLAlchemy Engine instance """ connect_args: dict[str, Any] = {} - engine_kwargs: dict[str, Any] = {} - # SQLite-specific configuration - if database_url.startswith("sqlite"): - connect_args["check_same_thread"] = False - - # Scope Postgres connections to the configured schema via search_path and pin - # the session timezone to UTC so func.date() truncates on the UTC - # day boundary — matching SQLite, which stores UTC text. This keeps the models - # schema-agnostic (no hardcoded schema=) so the same code serves SQLite, - # single-instance Postgres, and multiple schema-isolated instances on one cluster. - resolved_schema = _resolve_pg_schema(database_url, schema) - is_postgres = database_url.startswith(("postgresql", "postgres")) - if is_postgres: - options_parts: list[str] = [] - if resolved_schema: - options_parts.append(f"-csearch_path={resolved_schema}") - options_parts.append("-ctimezone=UTC") - connect_args["options"] = " ".join(options_parts) + # Scope connections to the configured schema via search_path and pin the + # session timezone to UTC so func.date() truncates on the UTC + # day boundary — the collector writes UTC, so day buckets must line up on + # it. This keeps the models schema-agnostic (no hardcoded schema=) so the + # same code serves single-instance Postgres and multiple schema-isolated + # instances on one cluster. + resolved_schema = _resolve_pg_schema(schema) + options_parts: list[str] = [] + if resolved_schema: + options_parts.append(f"-csearch_path={resolved_schema}") + options_parts.append("-ctimezone=UTC") + connect_args["options"] = " ".join(options_parts) # Size the pool above the default Starlette threadpool (~40 threads) so - # concurrent request handlers don't block waiting for a connection. Applies - # to file-based SQLite and networked backends (e.g. a future Postgres). - # In-memory SQLite uses a non-overflow pool, so skip these args there. - is_memory_sqlite = database_url in ("sqlite://", "sqlite:///:memory:") - if not is_memory_sqlite: - engine_kwargs["pool_size"] = 20 - engine_kwargs["max_overflow"] = 30 + # concurrent request handlers don't block waiting for a connection. + engine_kwargs: dict[str, Any] = { + "pool_size": 20, + "max_overflow": 30, + } - engine = create_engine( + return create_engine( database_url, echo=echo, connect_args=connect_args, @@ -104,24 +85,6 @@ def create_database_engine( **engine_kwargs, ) - # Apply SQLite pragmas on every new connection - if database_url.startswith("sqlite"): - - @event.listens_for(engine, "connect") - def set_sqlite_pragma(dbapi_connection, connection_record): # type: ignore - cursor = dbapi_connection.cursor() - # WAL lets readers run concurrently with a single writer (the - # collector), and busy_timeout waits instead of immediately raising - # "database is locked" under contention. synchronous=NORMAL is safe - # under WAL and faster. - cursor.execute("PRAGMA journal_mode=WAL") - cursor.execute("PRAGMA busy_timeout=5000") - cursor.execute("PRAGMA synchronous=NORMAL") - cursor.execute("PRAGMA foreign_keys=ON") - cursor.close() - - return engine - def create_session_factory(engine: Engine) -> sessionmaker[Session]: """Create a session factory for the given engine. @@ -175,19 +138,11 @@ class DatabaseManager: database_url: SQLAlchemy database URL echo: Enable SQL query logging schema: Postgres schema to scope connections to (search_path). Defaults to - the DATABASE_SCHEMA env var when not given; ignored for SQLite. + the DATABASE_SCHEMA env var when not given. """ self.database_url = database_url self._echo = echo - self._schema = _resolve_pg_schema(database_url, schema) - - # Ensure parent directory exists for SQLite databases - if database_url.startswith("sqlite:///"): - from pathlib import Path - - # Extract path from sqlite:///path/to/db.sqlite - db_path = Path(database_url.replace("sqlite:///", "")) - db_path.parent.mkdir(parents=True, exist_ok=True) + self._schema = _resolve_pg_schema(schema) self.engine = create_database_engine(database_url, echo=echo, schema=schema) self.session_factory = create_session_factory(self.engine) @@ -204,31 +159,24 @@ class DatabaseManager: from sqlalchemy.ext.asyncio import async_sessionmaker async_url = _to_async_url(self.database_url) - async_connect_args: dict[str, Any] = {} - # asyncpg sets search_path and timezone via server_settings (not the libpq - # -c options string the sync psycopg2 engine uses). self._schema is already - # resolved (explicit arg or DATABASE_SCHEMA env) and None for SQLite. - # Timezone is pinned to UTC unconditionally for Postgres so day boundaries - # match SQLite's UTC-text truncation. - is_postgres = self.database_url.startswith(("postgresql", "postgres")) - if is_postgres: - server_settings: dict[str, str] = {"timezone": "UTC"} - if self._schema: - server_settings["search_path"] = self._schema - async_connect_args["server_settings"] = server_settings + # asyncpg sets search_path and timezone via server_settings (not the + # libpq -c options string the sync psycopg2 engine uses). self._schema + # is already resolved (explicit arg or DATABASE_SCHEMA env). Timezone + # is pinned to UTC so day boundaries line up with the collector's + # UTC writes. + server_settings: dict[str, str] = {"timezone": "UTC"} + if self._schema: + server_settings["search_path"] = self._schema + async_connect_args: dict[str, Any] = {"server_settings": server_settings} # Mirror the sync engine's pool configuration (see # create_database_engine): without pool_pre_ping a Postgres restart - # leaves stale asyncpg connections in the pool that error on next - # use. In-memory SQLite uses a non-overflow pool, so skip there. - is_memory_sqlite = self.database_url in ( - "sqlite+aiosqlite://", - "sqlite+aiosqlite:///:memory:", - ) - async_engine_kwargs: dict[str, Any] = {"pool_pre_ping": True} - if not is_memory_sqlite: - async_engine_kwargs["pool_size"] = 20 - async_engine_kwargs["max_overflow"] = 30 + # leaves stale asyncpg connections in the pool that error on next use. + async_engine_kwargs: dict[str, Any] = { + "pool_pre_ping": True, + "pool_size": 20, + "max_overflow": 30, + } self._async_engine = create_async_engine( async_url, @@ -237,21 +185,6 @@ class DatabaseManager: **async_engine_kwargs, ) - # Apply the same SQLite pragmas as the sync engine (see - # create_database_engine) for the async engine's connections. - if self.database_url.startswith("sqlite"): - - @event.listens_for(self._async_engine.sync_engine, "connect") - def set_sqlite_pragma_async( - dbapi_connection: object, connection_record: object - ) -> None: - cursor = dbapi_connection.cursor() # type: ignore[attr-defined] - cursor.execute("PRAGMA journal_mode=WAL") - cursor.execute("PRAGMA busy_timeout=5000") - cursor.execute("PRAGMA synchronous=NORMAL") - cursor.execute("PRAGMA foreign_keys=ON") - cursor.close() - self._async_session_factory = async_sessionmaker( self._async_engine, class_=AsyncSession, diff --git a/src/meshcore_hub/common/db_migrate.py b/src/meshcore_hub/common/db_migrate.py index 0b6e5b3..a0f8e60 100644 --- a/src/meshcore_hub/common/db_migrate.py +++ b/src/meshcore_hub/common/db_migrate.py @@ -1,10 +1,13 @@ """Copy all data from a source (SQLite) database into a target (Postgres) database. -This powers ``meshcore-hub db migrate-to-postgres``. It operates at the SQLAlchemy -Core level, iterating ``Base.metadata.sorted_tables`` and copying each table through -the ORM's typed columns. The round-trip ``SQLite value -> Python object -> Postgres -value`` is what makes the conversion correct (e.g. integer ``0/1`` -> ``bool``, JSON -``TEXT`` -> ``dict``, datetime string -> ``timestamptz``) without any per-model code. +This powers ``meshcore-hub db migrate-to-postgres`` — the v0.19 upgrade path for +existing SQLite deployments. It is scheduled for removal in v0.20. It operates at +the SQLAlchemy Core level, iterating ``Base.metadata.sorted_tables`` and copying +each table through the ORM's typed columns. The round-trip ``SQLite value -> +Python object -> Postgres value`` is what makes the conversion correct (e.g. +integer ``0/1`` -> ``bool``, JSON ``TEXT`` -> ``dict``, datetime string -> +``timestamptz``) without any per-model code. SQLite is read via Python's stdlib +``sqlite3`` driver — it is not a supported runtime backend. The schema must already exist in the target (created by ``db upgrade``); this module only moves data and never mutates the source. diff --git a/src/meshcore_hub/common/models/event_observer.py b/src/meshcore_hub/common/models/event_observer.py index 40166d3..b64e8f4 100644 --- a/src/meshcore_hub/common/models/event_observer.py +++ b/src/meshcore_hub/common/models/event_observer.py @@ -8,13 +8,13 @@ from sqlalchemy import ( DateTime, Float, ForeignKey, - Insert, Integer, Index, String, UniqueConstraint, update, ) +from sqlalchemy.dialects.postgresql import insert as pg_insert from sqlalchemy.orm import Mapped, Session, mapped_column, relationship from meshcore_hub.common.models.base import Base, TimestampMixin, UUIDMixin, utc_now @@ -104,7 +104,8 @@ def add_event_observer( ) -> bool: """Add an observer to an event, handling duplicates gracefully. - Uses INSERT OR IGNORE to handle the unique constraint on (event_hash, observer_node_id). + Uses a Postgres ``INSERT ... ON CONFLICT DO NOTHING`` against the unique + constraint on (event_hash, observer_node_id). Args: session: SQLAlchemy session @@ -122,10 +123,6 @@ def add_event_observer( now = observed_at or datetime.now(timezone.utc) - # Both SQLite and Postgres expose on_conflict_do_nothing() with the same signature, - # but the INSERT construct must come from the matching dialect or it emits SQL for - # the wrong backend. Build the statement in each branch (rather than aliasing the - # insert() function) so the two dialect-specific Insert types stay distinct. values = { "id": str(uuid4()), "event_type": event_type, @@ -137,25 +134,11 @@ def add_event_observer( "created_at": now, "updated_at": now, } - conflict_cols = ["event_hash", "observer_node_id"] - stmt: Insert - - if session.get_bind().dialect.name == "postgresql": - from sqlalchemy.dialects.postgresql import insert as pg_insert - - stmt = ( - pg_insert(EventObserver) - .values(**values) - .on_conflict_do_nothing(index_elements=conflict_cols) - ) - else: - from sqlalchemy.dialects.sqlite import insert as sqlite_insert - - stmt = ( - sqlite_insert(EventObserver) - .values(**values) - .on_conflict_do_nothing(index_elements=conflict_cols) - ) + stmt = ( + pg_insert(EventObserver) + .values(**values) + .on_conflict_do_nothing(index_elements=["event_hash", "observer_node_id"]) + ) result = session.execute(stmt) rowcount = getattr(result, "rowcount", 0) diff --git a/tests/conftest.py b/tests/conftest.py index 011d1be..6230ebf 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,16 +1,15 @@ """Shared pytest fixtures for all tests.""" import os -import tempfile -from typing import Generator import dotenv import pytest from sqlalchemy import create_engine, text -from sqlalchemy.engine.url import make_url from sqlalchemy.orm import sessionmaker +from sqlalchemy.pool import NullPool from meshcore_hub.common import config as config_module +from meshcore_hub.common.database import create_database_engine from meshcore_hub.common.models import Base # The CLI entrypoint (meshcore_hub.__main__) calls load_dotenv() at import time so @@ -105,16 +104,78 @@ def _ignore_dotenv(monkeypatch): monkeypatch.delenv(ev, raising=False) +def _truncate_all(engine) -> None: + """Delete rows from every table in child-first order (FK-safe).""" + with engine.begin() as conn: + for table in reversed(Base.metadata.sorted_tables): + conn.execute(table.delete()) + + +# Default points at the throwaway stack from `make test-db-up`. Override with +# TEST_POSTGRES_URL to use any other Postgres instance. +DEFAULT_TEST_POSTGRES_URL = ( + "postgresql+psycopg2://meshcorehub:meshcorehub-test" + "@localhost:55432/meshcorehub_test" +) + + +@pytest.fixture(scope="session") +def db_url(worker_id: str) -> str: + """PostgreSQL URL for this pytest session (Postgres is the only backend). + + Missing or unreachable database is a hard exit — Postgres is mandatory, + not skippable — with an actionable message pointing at ``make test-db-up``. + """ + url = os.environ.get("TEST_POSTGRES_URL") or DEFAULT_TEST_POSTGRES_URL + probe = create_engine(url, poolclass=NullPool) + try: + with probe.connect() as conn: + conn.execute(text("SELECT 1")) + except Exception as exc: + pytest.exit( + f"PostgreSQL test database unreachable at {url!r}: {exc}\n" + "Backend tests require PostgreSQL. Start the throwaway stack " + "with: make test-db-up\n" + "(or point TEST_POSTGRES_URL at your own instance)", + returncode=4, + ) + finally: + probe.dispose() + return url + + +@pytest.fixture(scope="session") +def db_schema(worker_id: str, db_url: str): + """Per-xdist-worker schema isolating parallel test runs. + + ``hub_test_master`` / ``hub_test_gw0`` / ... The test role owns the + database (no superuser or CREATEDB needed), so the schema is created with + a plain connection — no admin connection to a maintenance database. + Engines built through the production ``create_database_engine`` factory + scope themselves to it via ``search_path``. Torn down with DROP SCHEMA + CASCADE. + """ + schema = f"hub_test_{worker_id}" + admin = create_engine(db_url, poolclass=NullPool) + with admin.begin() as conn: + conn.execute(text(f'CREATE SCHEMA IF NOT EXISTS "{schema}"')) + yield schema + with admin.begin() as conn: + conn.execute(text(f'DROP SCHEMA IF EXISTS "{schema}" CASCADE')) + admin.dispose() + + @pytest.fixture -def db_engine(): - """Create an in-memory SQLite database engine for testing.""" - engine = create_engine( - "sqlite:///:memory:", - connect_args={"check_same_thread": False}, - ) +def db_engine(db_url: str, db_schema: str): + """Worker-schema Postgres engine built by the production factory. + + Tables are created idempotently; rows are truncated after each test so + sibling tests start clean without paying schema-build costs. + """ + engine = create_database_engine(db_url, schema=db_schema) Base.metadata.create_all(engine) yield engine - Base.metadata.drop_all(engine) + _truncate_all(engine) engine.dispose() @@ -125,97 +186,3 @@ def db_session(db_engine): session = Session() yield session session.close() - - -@pytest.fixture(scope="session") -def test_db_path(): - """Session-scoped temporary SQLite database file path. - - One file per pytest session; engines below build schema on it once. - """ - fd, path = tempfile.mkstemp(suffix=".db") - os.close(fd) - yield path - if os.path.exists(path): - os.unlink(path) - - -@pytest.fixture(scope="session") -def db_backend() -> str: - """Active test database backend (``sqlite`` or ``postgres``). - - Controlled by ``TEST_DATABASE_BACKEND`` env var (default: ``sqlite``). - When ``postgres``, ``TEST_POSTGRES_URL`` must also be set. - """ - backend = os.environ.get("TEST_DATABASE_BACKEND", "sqlite").lower() - if backend not in ("sqlite", "postgres"): - raise ValueError( - f"TEST_DATABASE_BACKEND must be 'sqlite' or 'postgres', got: {backend}" - ) - return backend - - -@pytest.fixture(scope="session") -def db_url(db_backend: str, test_db_path: str, request) -> Generator[str, None, None]: - """Database URL for the active backend. - - For Postgres, each pytest-xdist worker gets its own database (e.g. - ``test_gw0``) to avoid truncation races between parallel workers. Shared by - the API and collector suites so both exercise the same backend. - """ - if db_backend == "postgres": - env_url = os.environ.get("TEST_POSTGRES_URL") - if not env_url: - pytest.skip( - "TEST_DATABASE_BACKEND=postgres but TEST_POSTGRES_URL is not set; " - "e.g. TEST_POSTGRES_URL=postgresql+psycopg2://postgres:postgres@localhost:55432/test" - ) - assert env_url is not None - - worker_id = "master" - if hasattr(request.config, "workerinput"): - worker_id = request.config.workerinput["workerid"] - - base_url = make_url(env_url) - worker_db = f"{base_url.database}_{worker_id}" - worker_url = base_url.set(database=worker_db).render_as_string( - hide_password=False - ) - - admin_url = base_url.set(database="postgres") - admin_engine = create_engine( - admin_url.render_as_string(hide_password=False), - isolation_level="AUTOCOMMIT", - ) - try: - with admin_engine.connect() as conn: - exists = conn.execute( - text("SELECT 1 FROM pg_database WHERE datname = :name"), - {"name": worker_db}, - ).scalar() - if not exists: - conn.execute(text(f'CREATE DATABASE "{worker_db}"')) - finally: - admin_engine.dispose() - - yield worker_url - - admin_engine = create_engine( - admin_url.render_as_string(hide_password=False), - isolation_level="AUTOCOMMIT", - ) - try: - with admin_engine.connect() as conn: - conn.execute( - text( - "SELECT pg_terminate_backend(pid) " - "FROM pg_stat_activity " - "WHERE datname = :name AND pid <> pg_backend_pid()" - ), - {"name": worker_db}, - ) - conn.execute(text(f'DROP DATABASE IF EXISTS "{worker_db}"')) - finally: - admin_engine.dispose() - else: - yield f"sqlite:///{test_db_path}" diff --git a/tests/test_api/conftest.py b/tests/test_api/conftest.py index e63a368..029872f 100644 --- a/tests/test_api/conftest.py +++ b/tests/test_api/conftest.py @@ -1,8 +1,8 @@ """API test fixtures. -The ``db_backend`` / ``db_url`` / ``test_db_path`` fixtures that drive the -SQLite-vs-Postgres switch live in the shared ``tests/conftest.py`` so the -collector suite can reuse the same backend; they are inherited here. +The ``db_url`` / ``db_schema`` fixtures that provide the per-xdist-worker +Postgres schema live in the shared ``tests/conftest.py`` so the collector +suite can reuse the same database; they are inherited here. """ from contextlib import contextmanager @@ -10,7 +10,6 @@ from datetime import datetime, timezone import pytest from fastapi.testclient import TestClient -from sqlalchemy import create_engine, event as sa_event from sqlalchemy.orm import sessionmaker from meshcore_hub.api.app import create_app @@ -36,31 +35,16 @@ from meshcore_hub.common.models import ( @pytest.fixture(scope="session") -def api_db_engine(db_url: str, db_backend: str): - """Session-scoped database engine. Schema is built once per pytest session. +def api_db_engine(db_url: str, db_schema: str): + """Session-scoped worker-schema Postgres engine. Schema built once. - For Postgres, uses the production ``create_database_engine`` factory so the - test exercises the same ``connect_args`` (including ``-ctimezone=UTC``). - Each xdist worker has its own database (see ``db_url``), so no locking - is needed. + Uses the production ``create_database_engine`` factory so tests exercise + the same ``connect_args`` (``search_path`` + ``-ctimezone=UTC``) as the + running services. Each xdist worker has its own schema (see + ``db_schema``), so no locking is needed. """ - if db_backend == "postgres": - engine = create_database_engine(db_url) - Base.metadata.drop_all(engine) - else: - engine = create_engine( - db_url, - connect_args={"check_same_thread": False}, - ) - - @sa_event.listens_for(engine, "connect") - def set_sqlite_pragma( - dbapi_connection: object, connection_record: object - ) -> None: - cursor = dbapi_connection.cursor() # type: ignore[attr-defined] - cursor.execute("PRAGMA foreign_keys=ON") - cursor.close() - + engine = create_database_engine(db_url, schema=db_schema) + Base.metadata.drop_all(engine) Base.metadata.create_all(engine) yield engine Base.metadata.drop_all(engine) diff --git a/tests/test_api/test_app_factory.py b/tests/test_api/test_app_factory.py index dd72e03..d717d13 100644 --- a/tests/test_api/test_app_factory.py +++ b/tests/test_api/test_app_factory.py @@ -10,6 +10,12 @@ from meshcore_hub.api.app import create_app_from_env _FACTORY_ENV = [ "DATABASE_URL", "DATA_HOME", + "DATABASE_HOST", + "DATABASE_PORT", + "DATABASE_NAME", + "DATABASE_SCHEMA", + "DATABASE_USER", + "DATABASE_PASSWORD", "REDIS_ENABLED", "REDIS_HOST", "REDIS_PORT", @@ -31,6 +37,11 @@ _FACTORY_ENV = [ def clean_env(monkeypatch): for var in _FACTORY_ENV: monkeypatch.delenv(var, raising=False) + # The factory requires a database connection (PostgreSQL-only); default + # to an explicit URL so tests not about DB resolution still build an app. + monkeypatch.setenv( + "DATABASE_URL", "postgresql+psycopg2://u:p@factory-test:5432/hub" + ) return monkeypatch @@ -49,7 +60,7 @@ def _served_paths(app): def test_factory_reads_database_and_redis_from_env(clean_env): """Workers must pick up the real DB/Redis config from env, not the hardcoded create_app defaults.""" - clean_env.setenv("DATABASE_URL", "sqlite:////tmp/workers-test.db") + clean_env.setenv("DATABASE_URL", "postgresql+psycopg2://u:p@workers-test:5432/hub") clean_env.setenv("REDIS_ENABLED", "true") clean_env.setenv("REDIS_HOST", "redis-test") clean_env.setenv("REDIS_PORT", "6390") @@ -58,7 +69,7 @@ def test_factory_reads_database_and_redis_from_env(clean_env): app = create_app_from_env() - assert app.state.database_url == "sqlite:////tmp/workers-test.db" + assert app.state.database_url == "postgresql+psycopg2://u:p@workers-test:5432/hub" assert app.state.redis_enabled is True assert app.state.redis_host == "redis-test" assert app.state.redis_port == 6390 @@ -66,17 +77,27 @@ def test_factory_reads_database_and_redis_from_env(clean_env): assert app.state.metrics_cache_ttl == 99 -def test_factory_honours_explicit_disable_and_data_home(clean_env): - """Env values override anything else, and a data-home (no explicit - DATABASE_URL) resolves to the collector DB path — never the bare - create_app default that would point workers at ./meshcore.db.""" +def test_factory_assembles_url_from_component_vars(clean_env): + """Without an explicit DATABASE_URL, the DATABASE_* components assemble + the connection — the app factory never falls back to a default file DB.""" + clean_env.delenv("DATABASE_URL") clean_env.setenv("REDIS_ENABLED", "false") - clean_env.setenv("DATA_HOME", "/srv/hubdata") + clean_env.setenv("DATABASE_HOST", "pg-test") + clean_env.setenv("DATABASE_PASSWORD", "pw") app = create_app_from_env() assert app.state.redis_enabled is False - assert app.state.database_url == "sqlite:////srv/hubdata/collector/meshcore.db" + assert app.state.database_url == ( + "postgresql+psycopg2://meshcorehub:pw@pg-test:5432/meshcorehub" + ) + + +def test_factory_without_database_config_raises(clean_env): + """Missing connection config fails fast instead of guessing a default.""" + clean_env.delenv("DATABASE_URL") + with pytest.raises(ValueError, match="PostgreSQL connection is not configured"): + _ = create_app_from_env() def test_factory_redis_enabled_accepts_truthy_values(clean_env): diff --git a/tests/test_api/test_cache.py b/tests/test_api/test_cache.py index d3d9d99..0e156d4 100644 --- a/tests/test_api/test_cache.py +++ b/tests/test_api/test_cache.py @@ -1008,7 +1008,7 @@ class TestLifespanRedis: from meshcore_hub.api.app import lifespan app = FastAPI() - app.state.database_url = "sqlite:///./test.db" + app.state.database_url = "postgresql+psycopg2://u:p@testhost:5432/testdb" app.state.redis_enabled = False app_module._db_manager = None @@ -1023,7 +1023,7 @@ class TestLifespanRedis: from meshcore_hub.api.app import lifespan app = FastAPI() - app.state.database_url = "sqlite:///./test.db" + app.state.database_url = "postgresql+psycopg2://u:p@testhost:5432/testdb" app.state.redis_enabled = True app.state.redis_host = "redis-host" app.state.redis_port = 6380 @@ -1053,7 +1053,7 @@ class TestLifespanRedis: from meshcore_hub.api.app import lifespan app = FastAPI() - app.state.database_url = "sqlite:///./test.db" + app.state.database_url = "postgresql+psycopg2://u:p@testhost:5432/testdb" app.state.redis_enabled = True mock_cache = MagicMock() @@ -1157,7 +1157,7 @@ class TestCliRedis: with patch("meshcore_hub.common.config.get_api_settings") as mock_settings: mock_settings.return_value = MagicMock( data_home="/tmp/test", - effective_database_url="sqlite:///test.db", + effective_database_url="postgresql+psycopg2://u:p@testhost:5432/testdb", ) result = runner.invoke( api, @@ -1178,7 +1178,7 @@ class TestCliRedis: with patch("meshcore_hub.common.config.get_api_settings") as mock_settings: mock_settings.return_value = MagicMock( data_home="/tmp/test", - effective_database_url="sqlite:///test.db", + effective_database_url="postgresql+psycopg2://u:p@testhost:5432/testdb", ) result = runner.invoke(api, catch_exceptions=False) assert "Redis enabled: False" in result.output @@ -1194,7 +1194,7 @@ class TestCliRedis: with patch("meshcore_hub.common.config.get_api_settings") as mock_settings: mock_settings.return_value = MagicMock( data_home="/tmp/test", - effective_database_url="sqlite:///test.db", + effective_database_url="postgresql+psycopg2://u:p@testhost:5432/testdb", ) with patch("meshcore_hub.api.app.create_app") as mock_create_app: mock_create_app.return_value = MagicMock() @@ -1242,7 +1242,7 @@ class TestCliRedis: with patch("meshcore_hub.common.config.get_api_settings") as mock_settings: mock_settings.return_value = MagicMock( data_home="/tmp/test", - effective_database_url="sqlite:///test.db", + effective_database_url="postgresql+psycopg2://u:p@testhost:5432/testdb", ) with patch("meshcore_hub.api.app.create_app") as mock_create_app: mock_create_app.return_value = MagicMock() @@ -1268,7 +1268,7 @@ class TestCliRedis: with patch("meshcore_hub.common.config.get_api_settings") as mock_settings: mock_settings.return_value = MagicMock( data_home="/tmp/test", - effective_database_url="sqlite:///test.db", + effective_database_url="postgresql+psycopg2://u:p@testhost:5432/testdb", ) with patch("meshcore_hub.api.app.create_app") as mock_create_app: mock_create_app.return_value = MagicMock() diff --git a/tests/test_api/test_channel_visibility.py b/tests/test_api/test_channel_visibility.py index 66eb20c..fc4a1a3 100644 --- a/tests/test_api/test_channel_visibility.py +++ b/tests/test_api/test_channel_visibility.py @@ -2,36 +2,15 @@ from unittest.mock import MagicMock -import pytest -from sqlalchemy import create_engine -from sqlalchemy.orm import sessionmaker - from meshcore_hub.api.channel_visibility import ( get_all_known_channel_indices, get_max_visibility_level, get_visible_channel_indices, resolve_user_role, ) -from meshcore_hub.common.models import Base from meshcore_hub.common.models.channel import Channel -@pytest.fixture -def db_session(): - """Create an in-memory SQLite database session.""" - engine = create_engine( - "sqlite:///:memory:", - connect_args={"check_same_thread": False}, - ) - Base.metadata.create_all(engine) - Session = sessionmaker(bind=engine) - session = Session() - yield session - session.close() - Base.metadata.drop_all(engine) - engine.dispose() - - def _make_request( headers: dict | None = None, app_state: dict | None = None ) -> MagicMock: diff --git a/tests/test_api/test_cli.py b/tests/test_api/test_cli.py index d924c5f..d2119d0 100644 --- a/tests/test_api/test_cli.py +++ b/tests/test_api/test_cli.py @@ -6,13 +6,19 @@ from click.testing import CliRunner from meshcore_hub.api.cli import api +# Opaque URL: satisfies the fail-fast database check; nothing connects +# (uvicorn.run is mocked, so the lifespan never starts). +_TEST_DB_URL = "postgresql+psycopg2://meshcorehub:pw@localhost:5432/meshcorehub" + def test_api_default_runs_single_process(): """With the default worker count, the app object is passed directly and no worker/factory options are used.""" runner = CliRunner() with patch("uvicorn.run") as mock_run: - result = runner.invoke(api, [], catch_exceptions=False) + result = runner.invoke( + api, ["--database-url", _TEST_DB_URL], catch_exceptions=False + ) assert result.exit_code == 0 assert mock_run.call_count == 1 @@ -27,7 +33,11 @@ def test_api_workers_uses_env_factory_import_string(): string with the requested worker count.""" runner = CliRunner() with patch("uvicorn.run") as mock_run: - result = runner.invoke(api, ["--workers", "3"], catch_exceptions=False) + result = runner.invoke( + api, + ["--database-url", _TEST_DB_URL, "--workers", "3"], + catch_exceptions=False, + ) assert result.exit_code == 0 args, kwargs = mock_run.call_args @@ -41,7 +51,10 @@ def test_api_workers_from_env_var(): runner = CliRunner() with patch("uvicorn.run") as mock_run: result = runner.invoke( - api, [], env={"API_WORKERS": "2"}, catch_exceptions=False + api, + ["--database-url", _TEST_DB_URL], + env={"API_WORKERS": "2"}, + catch_exceptions=False, ) assert result.exit_code == 0 @@ -57,7 +70,7 @@ def test_api_single_process_passes_spam_and_role_settings(): with patch("uvicorn.run") as mock_run: result = runner.invoke( api, - [], + ["--database-url", _TEST_DB_URL], env={ "SPAM_DETECTION_ENABLED": "true", "SPAM_SCORE_THRESHOLD": "0.8", diff --git a/tests/test_api/test_dashboard.py b/tests/test_api/test_dashboard.py index 04fc575..19cf6a3 100644 --- a/tests/test_api/test_dashboard.py +++ b/tests/test_api/test_dashboard.py @@ -27,7 +27,7 @@ class TestDateBucketKey: """Unit tests for the _date_bucket_key dialect-neutral normalization helper.""" def test_str_passthrough(self) -> None: - """SQLite returns str — pass through unchanged.""" + """String keys (as stored/serialized) pass through unchanged.""" assert _date_bucket_key("2026-06-15") == "2026-06-15" def test_date_object_normalized(self) -> None: @@ -1363,16 +1363,13 @@ class TestDashboardDateBucketRegression: """Regression tests for the Postgres date-bucket flatline bug. These tests seed data at deterministic UTC timestamps and assert the - specific date buckets have non-zero counts. On SQLite (where - func.date() returns str) these always passed. On Postgres (where - func.date() returns date) these would have failed before the - _date_bucket_key fix, because the dict lookup by string key would - miss the date-object key. + specific date buckets have non-zero counts. Postgres's ``func.date()`` + returns ``date`` objects, so without the ``_date_bucket_key`` fix the + dict lookup by string key would miss the date-object key. Run against Postgres with:: - TEST_DATABASE_BACKEND=postgres \\ - TEST_POSTGRES_URL=postgresql+psycopg2://postgres:postgres@localhost:55432/test \\ + make test-db-up pytest tests/test_api/test_dashboard.py -k Regression """ diff --git a/tests/test_api/test_metrics.py b/tests/test_api/test_metrics.py index 81872da..f076822 100644 --- a/tests/test_api/test_metrics.py +++ b/tests/test_api/test_metrics.py @@ -83,11 +83,9 @@ class TestMetricsAuth: assert response.status_code == 200 def test_deny_by_default_when_no_read_key( - self, test_db_path, api_db_engine, mock_mqtt, mock_db_manager + self, db_url, api_db_engine, mock_mqtt, mock_db_manager ): """Unauthenticated /metrics is denied when no key is configured.""" - db_url = f"sqlite:///{test_db_path}" - with patch("meshcore_hub.api.app._db_manager", mock_db_manager): app = create_app( database_url=db_url, @@ -375,11 +373,9 @@ class TestMetricsDisabled: """Tests for when metrics are disabled.""" def test_metrics_404_when_disabled( - self, test_db_path, api_db_engine, mock_mqtt, mock_db_manager + self, db_url, api_db_engine, mock_mqtt, mock_db_manager ): """Test that /metrics returns 404 when disabled.""" - db_url = f"sqlite:///{test_db_path}" - with patch("meshcore_hub.api.app._db_manager", mock_db_manager): app = create_app( database_url=db_url, diff --git a/tests/test_api/test_routes.py b/tests/test_api/test_routes.py index 9a2ff69..991df6e 100644 --- a/tests/test_api/test_routes.py +++ b/tests/test_api/test_routes.py @@ -518,7 +518,7 @@ class TestRouteQualityAvg: # Use uuid-derived public_keys so concurrent tests using _sample_nodes # (which always inserts "a"*64 / "b"*64) can't trip the unique # constraint on Node.public_key during parallel xdist runs against - # the shared SQLite file. + # the shared worker schema. suffix = uuid4().hex keys = [(suffix[:32]).rjust(64, "0"), (suffix[32:64]).rjust(64, "0")] nodes = [_make_node(session, k) for k in keys] diff --git a/tests/test_api/test_user_profiles.py b/tests/test_api/test_user_profiles.py index 6beca7c..616d53a 100644 --- a/tests/test_api/test_user_profiles.py +++ b/tests/test_api/test_user_profiles.py @@ -100,22 +100,14 @@ class TestListProfiles: api_db_session.commit() # Temporarily disable FK enforcement to simulate an orphaned adoption - # (node deleted while the adoption record persists). SQLite uses - # PRAGMA; Postgres uses session_replication_role = replica. - dialect = api_db_session.bind.dialect.name # type: ignore[union-attr] - if dialect == "postgresql": - api_db_session.execute(text("SET session_replication_role = replica")) - else: - api_db_session.execute(text("PRAGMA foreign_keys=OFF")) + # (node deleted while the adoption record persists). + api_db_session.execute(text("SET session_replication_role = replica")) api_db_session.execute( text("DELETE FROM nodes WHERE id = :id"), {"id": sample_node.id}, ) api_db_session.commit() - if dialect == "postgresql": - api_db_session.execute(text("SET session_replication_role = DEFAULT")) - else: - api_db_session.execute(text("PRAGMA foreign_keys=ON")) + api_db_session.execute(text("SET session_replication_role = DEFAULT")) response = client_no_auth.get( "/api/v1/user/profiles", diff --git a/tests/test_collector/conftest.py b/tests/test_collector/conftest.py index 99968de..e49282b 100644 --- a/tests/test_collector/conftest.py +++ b/tests/test_collector/conftest.py @@ -1,18 +1,14 @@ """Fixtures for collector component tests. -The ``db_backend`` / ``db_url`` switch lives in the shared ``tests/conftest.py``. -The synchronous ``db_manager`` / ``db_session`` fixtures honour it so the spam -scorer, the re-scoring sweep, and the message handler are exercised on both -SQLite (default) and Postgres (``TEST_DATABASE_BACKEND=postgres``). On Postgres -they share the per-xdist-worker database with the API suite, so tables are -created idempotently and *truncated* (never dropped) at teardown to coexist with -the API's session-scoped engine. +All fixtures run against the shared per-xdist-worker Postgres schema from +``tests/conftest.py`` (``db_url`` / ``db_schema``). The synchronous +``db_manager`` / ``db_session`` fixtures and the async ``async_db_session`` +fixture use the production ``DatabaseManager`` paths (including +``async_session()``) and truncate between tests to coexist with the API +suite's session-scoped engine. """ import pytest -from sqlalchemy import event as sa_event -from sqlalchemy.ext.asyncio import AsyncSession, create_async_engine -from sqlalchemy.ext.asyncio import async_sessionmaker from meshcore_hub.common.database import DatabaseManager from meshcore_hub.common.models.base import Base @@ -26,24 +22,13 @@ def _truncate_all(engine) -> None: @pytest.fixture -def db_manager(db_backend, db_url): - """Create a database manager for testing, honouring the active backend. - - Default (SQLite) uses an isolated in-memory database per test, exactly as - before. Postgres reuses the shared worker database with idempotent schema + - truncate-on-teardown. - """ - if db_backend == "postgres": - manager = DatabaseManager(db_url) - manager.create_tables() - yield manager - _truncate_all(manager.engine) - manager.dispose() - else: - manager = DatabaseManager("sqlite:///:memory:") - manager.create_tables() - yield manager - manager.dispose() +def db_manager(db_url, db_schema): + """Database manager bound to the worker schema (production engine path).""" + manager = DatabaseManager(db_url, schema=db_schema) + manager.create_tables() + yield manager + _truncate_all(manager.engine) + manager.dispose() @pytest.fixture @@ -55,34 +40,11 @@ def db_session(db_manager): @pytest.fixture -async def async_db_session(): - """Create an async database session for testing. - - Uses a separate in-memory database with tables created inline. - """ - # Create async engine with in-memory database - engine = create_async_engine("sqlite+aiosqlite:///:memory:") - - @sa_event.listens_for(engine.sync_engine, "connect") - def set_sqlite_pragma_async( - dbapi_connection: object, connection_record: object - ) -> None: - cursor = dbapi_connection.cursor() # type: ignore[attr-defined] - cursor.execute("PRAGMA foreign_keys=ON") - cursor.close() - - # Create tables - async with engine.begin() as conn: - await conn.run_sync(Base.metadata.create_all) - - # Create session factory - async_session_maker = async_sessionmaker( - engine, class_=AsyncSession, expire_on_commit=False - ) - - # Provide session - async with async_session_maker() as session: +async def async_db_session(db_url, db_schema): + """Async session from the production ``DatabaseManager.async_session()``.""" + manager = DatabaseManager(db_url, schema=db_schema) + manager.create_tables() + async with manager.async_session() as session: yield session - - # Cleanup - await engine.dispose() + _truncate_all(manager.engine) + await manager.adispose() diff --git a/tests/test_collector/test_cleanup.py b/tests/test_collector/test_cleanup.py index a2de490..9798e7c 100644 --- a/tests/test_collector/test_cleanup.py +++ b/tests/test_collector/test_cleanup.py @@ -118,7 +118,7 @@ async def test_cleanup_old_data_live(async_db_session: AsyncSession) -> None: old_trace = TracePath( observer_node_id=node.id, - initiator_tag="test", + initiator_tag=12345, created_at=old_date, updated_at=old_date, ) @@ -354,12 +354,12 @@ async def test_cleanup_orphaned_node_relations( await async_db_session.commit() - await async_db_session.execute(text("PRAGMA foreign_keys=OFF")) + await async_db_session.execute(text("SET session_replication_role = replica")) await async_db_session.execute( text("DELETE FROM nodes WHERE id = :id"), {"id": node.id} ) await async_db_session.commit() - await async_db_session.execute(text("PRAGMA foreign_keys=ON")) + await async_db_session.execute(text("SET session_replication_role = DEFAULT")) counts = await cleanup_orphaned_node_relations(async_db_session, dry_run=False) @@ -403,12 +403,12 @@ async def test_cleanup_orphaned_node_relations_dry_run( async_db_session.add(adoption) await async_db_session.commit() - await async_db_session.execute(text("PRAGMA foreign_keys=OFF")) + await async_db_session.execute(text("SET session_replication_role = replica")) await async_db_session.execute( text("DELETE FROM nodes WHERE id = :id"), {"id": node.id} ) await async_db_session.commit() - await async_db_session.execute(text("PRAGMA foreign_keys=ON")) + await async_db_session.execute(text("SET session_replication_role = DEFAULT")) counts = await cleanup_orphaned_node_relations(async_db_session, dry_run=True) diff --git a/tests/test_collector/test_cli.py b/tests/test_collector/test_cli.py index 0712626..24c70f7 100644 --- a/tests/test_collector/test_cli.py +++ b/tests/test_collector/test_cli.py @@ -7,13 +7,24 @@ from click.testing import CliRunner from meshcore_hub.collector.cli import _import_channels, _import_routes, collector from meshcore_hub.collector.routes import derive_expected_hash -from meshcore_hub.common.database import DatabaseManager +from meshcore_hub.common.database import DatabaseManager, create_database_engine +from meshcore_hub.common.models import Base from meshcore_hub.common.models.channel import Channel from meshcore_hub.common.models.node import Node from meshcore_hub.common.models.route import Route from meshcore_hub.common.models.route_node import RouteNode from meshcore_hub.common.models.route_observer import RouteObserver +# Opaque URL handed to mocked settings (nothing connects to it). +_MOCK_PG_URL = "postgresql+psycopg2://meshcorehub:pw@localhost:5432/meshcorehub" + + +def _truncate_all(engine) -> None: + """Delete rows from every table in child-first order (FK-safe).""" + with engine.begin() as conn: + for table in reversed(Base.metadata.sorted_tables): + conn.execute(table.delete()) + def _make_mock_settings(db_url: str, seed_home: str = "/tmp/seed") -> MagicMock: mock_settings = MagicMock( @@ -45,7 +56,7 @@ class TestCollectorGroup: def test_collector_without_subcommand_calls_run_service(self): runner = CliRunner() - mock_settings = _make_mock_settings("sqlite:///tmp/test.db") + mock_settings = _make_mock_settings(_MOCK_PG_URL) with ( patch( @@ -63,7 +74,7 @@ class TestCollectorGroup: def test_collector_with_data_home_override(self): runner = CliRunner() - mock_settings = _make_mock_settings("sqlite:///default/db") + mock_settings = _make_mock_settings(_MOCK_PG_URL) with ( patch( @@ -90,7 +101,7 @@ class TestCollectorRunSubcommand: def test_run_subcommand_calls_run_service(self): runner = CliRunner() - mock_settings = _make_mock_settings("sqlite:///tmp/test.db") + mock_settings = _make_mock_settings(_MOCK_PG_URL) with ( patch( @@ -110,7 +121,7 @@ class TestCollectorSeedSubcommand: def test_seed_command_help(self): runner = CliRunner() - mock_settings = _make_mock_settings("sqlite:///tmp/test.db") + mock_settings = _make_mock_settings(_MOCK_PG_URL) with patch( "meshcore_hub.common.config.get_collector_settings", @@ -123,16 +134,23 @@ class TestCollectorSeedSubcommand: class TestChannelCommands: - """Integration tests for channel CLI commands using real SQLite.""" + """Integration tests for channel CLI commands against worker-schema Postgres.""" @pytest.fixture - def cli_db_url(self, tmp_path): - db_path = tmp_path / "test.db" - db_url = f"sqlite:///{db_path}" - db = DatabaseManager(db_url) - db.create_tables() - db.dispose() - return db_url + def cli_db_url(self, db_url, db_schema, monkeypatch): + """Worker-schema Postgres URL for the CLI. + + ``DATABASE_SCHEMA`` is exported so the CLI's own + ``DatabaseManager(ctx.obj["database_url"])`` resolves the same schema + via the production env fallback. + """ + monkeypatch.setenv("DATABASE_SCHEMA", db_schema) + engine = create_database_engine(db_url, schema=db_schema) + Base.metadata.create_all(engine) + _truncate_all(engine) + yield db_url + _truncate_all(engine) + engine.dispose() def test_channel_list_empty(self, cli_db_url): runner = CliRunner() @@ -282,11 +300,12 @@ class TestImportChannels: """Unit tests for _import_channels YAML import function.""" @pytest.fixture - def import_db(self, tmp_path): - db_path = tmp_path / "import.db" - db = DatabaseManager(f"sqlite:///{db_path}") + def import_db(self, db_url, db_schema): + db = DatabaseManager(db_url, schema=db_schema) db.create_tables() + _truncate_all(db.engine) yield db + _truncate_all(db.engine) db.dispose() def _write_yaml(self, tmp_path, content: str) -> str: @@ -394,7 +413,7 @@ class TestImportChannels: class TestChannelSeedImport: """Integration tests for seed command with channels.yaml.""" - def test_seed_imports_channels_yaml(self, tmp_path): + def test_seed_imports_channels_yaml(self, tmp_path, db_url, db_schema, monkeypatch): runner = CliRunner() seed_dir = tmp_path / "seed" seed_dir.mkdir() @@ -402,10 +421,11 @@ class TestChannelSeedImport: "SeededCh: AABBCCDDEEFF00112233445566778899\n" ) - db_path = tmp_path / "seed_test.db" - db_url = f"sqlite:///{db_path}" - db = DatabaseManager(db_url) + # DATABASE_SCHEMA routes the CLI's own DatabaseManager to the worker schema. + monkeypatch.setenv("DATABASE_SCHEMA", db_schema) + db = DatabaseManager(db_url, schema=db_schema) db.create_tables() + _truncate_all(db.engine) db.dispose() mock_settings = _make_mock_settings(db_url, seed_home=str(seed_dir)) @@ -423,22 +443,22 @@ class TestChannelSeedImport: assert result.exit_code == 0 assert "Channels: 1 created" in result.output - db = DatabaseManager(db_url) + db = DatabaseManager(db_url, schema=db_schema) with db.session_scope() as session: ch = session.query(Channel).filter(Channel.name == "SeededCh").first() assert ch is not None assert ch.visibility == "community" db.dispose() - def test_seed_no_seed_files(self, tmp_path): + def test_seed_no_seed_files(self, tmp_path, db_url, db_schema, monkeypatch): runner = CliRunner() empty_seed = tmp_path / "empty_seed" empty_seed.mkdir() - db_path = tmp_path / "noseed.db" - db_url = f"sqlite:///{db_path}" - db = DatabaseManager(db_url) + monkeypatch.setenv("DATABASE_SCHEMA", db_schema) + db = DatabaseManager(db_url, schema=db_schema) db.create_tables() + _truncate_all(db.engine) db.dispose() mock_settings = _make_mock_settings(db_url, seed_home=str(empty_seed)) @@ -681,7 +701,7 @@ class TestImportRoutes: class TestRouteSeedImport: """Integration tests for the ``seed`` command with routes.yaml.""" - def test_seed_imports_routes_yaml(self, tmp_path): + def test_seed_imports_routes_yaml(self, tmp_path, db_url, db_schema, monkeypatch): runner = CliRunner() seed_dir = tmp_path / "seed" seed_dir.mkdir() @@ -695,10 +715,10 @@ class TestRouteSeedImport: f" path:\n - '{pk_a}'\n - '{pk_b}'\n" ) - db_path = tmp_path / "seed_route.db" - db_url = f"sqlite:///{db_path}" - db = DatabaseManager(db_url) + monkeypatch.setenv("DATABASE_SCHEMA", db_schema) + db = DatabaseManager(db_url, schema=db_schema) db.create_tables() + _truncate_all(db.engine) with db.session_scope() as session: session.add(Node(public_key=pk_a, name="NodeA")) session.add(Node(public_key=pk_b, name="NodeB")) @@ -718,7 +738,7 @@ class TestRouteSeedImport: assert result.exit_code == 0 assert "Routes: 1 created" in result.output - db = DatabaseManager(db_url) + db = DatabaseManager(db_url, schema=db_schema) with db.session_scope() as session: route = session.query(Route).filter(Route.from_label == "NodeA").first() assert route is not None diff --git a/tests/test_collector/test_handlers/test_contacts.py b/tests/test_collector/test_handlers/test_contacts.py index d7300b5..9006de5 100644 --- a/tests/test_collector/test_handlers/test_contacts.py +++ b/tests/test_collector/test_handlers/test_contacts.py @@ -73,7 +73,7 @@ def test_handle_contact_updates_existing_node_name(db_session, mock_db_manager): db_session.expire_all() node = db_session.query(Node).filter_by(public_key="b" * 64).first() assert node.name == "NewName" - # Compare timestamps without timezone (SQLite strips timezone info) + # Compare timestamps without timezone (naive datetimes from the handler) assert node.last_seen is not None assert node.last_seen.replace(tzinfo=None) == last_seen_time.replace(tzinfo=None) diff --git a/tests/test_collector/test_spam.py b/tests/test_collector/test_spam.py index ce5edf1..2f7fd7e 100644 --- a/tests/test_collector/test_spam.py +++ b/tests/test_collector/test_spam.py @@ -1,8 +1,7 @@ """Tests for the spam scoring module (collector/spam.py). -The DB-touching tests run against whichever backend the shared ``db_manager`` -fixture is configured for (SQLite by default, Postgres when -``TEST_DATABASE_BACKEND=postgres``). +The DB-touching tests run against the per-worker Postgres schema via the +shared ``db_manager`` fixture. """ from dataclasses import replace diff --git a/tests/test_collector/test_subscriber.py b/tests/test_collector/test_subscriber.py index 01ec733..8e1f2b6 100644 --- a/tests/test_collector/test_subscriber.py +++ b/tests/test_collector/test_subscriber.py @@ -1367,13 +1367,13 @@ class TestSubscriberDispatch: class TestCreateSubscriber: """Tests for create_subscriber factory function.""" - def test_creates_subscriber(self): + def test_creates_subscriber(self, db_url): """Test creating a subscriber.""" with patch("meshcore_hub.collector.subscriber.MQTTClient") as MockMQTT: subscriber = create_subscriber( mqtt_host="localhost", mqtt_port=1883, - database_url="sqlite:///:memory:", + database_url=db_url, ) assert subscriber is not None diff --git a/tests/test_collector/test_tag_import.py b/tests/test_collector/test_tag_import.py index e7f80e7..90e1fac 100644 --- a/tests/test_collector/test_tag_import.py +++ b/tests/test_collector/test_tag_import.py @@ -12,7 +12,6 @@ from meshcore_hub.collector.tag_import import ( load_tags_file, validate_public_key, ) -from meshcore_hub.common.database import DatabaseManager from meshcore_hub.common.models import Node, NodeTag @@ -174,14 +173,6 @@ class TestLoadTagsFile: class TestImportTags: """Tests for import_tags function.""" - @pytest.fixture - def db_manager(self): - """Create an in-memory database manager for testing.""" - manager = DatabaseManager("sqlite:///:memory:") - manager.create_tables() - yield manager - manager.dispose() - @pytest.fixture def sample_tags_file(self): """Create a sample tags file with new format.""" diff --git a/tests/test_common/test_channel_model.py b/tests/test_common/test_channel_model.py index afae536..ef64446 100644 --- a/tests/test_common/test_channel_model.py +++ b/tests/test_common/test_channel_model.py @@ -4,30 +4,12 @@ import hashlib import pytest from pydantic import ValidationError -from sqlalchemy import create_engine from sqlalchemy.exc import IntegrityError -from sqlalchemy.orm import sessionmaker -from meshcore_hub.common.models import Base, Channel, ChannelVisibility +from meshcore_hub.common.models import Channel, ChannelVisibility from meshcore_hub.common.schemas.channels import ChannelCreate, ChannelUpdate -@pytest.fixture -def db_session(): - """Create an in-memory SQLite database session.""" - engine = create_engine( - "sqlite:///:memory:", - connect_args={"check_same_thread": False}, - ) - Base.metadata.create_all(engine) - Session = sessionmaker(bind=engine) - session = Session() - yield session - session.close() - Base.metadata.drop_all(engine) - engine.dispose() - - class TestChannelModel: """Tests for Channel SQLAlchemy model.""" diff --git a/tests/test_common/test_config.py b/tests/test_common/test_config.py index 5750c32..b0035b3 100644 --- a/tests/test_common/test_config.py +++ b/tests/test_common/test_config.py @@ -1,6 +1,7 @@ """Tests for configuration settings.""" import pytest +from pydantic import ValidationError from meshcore_hub.common.config import ( CommonSettings, @@ -68,14 +69,10 @@ class TestCollectorSettings: """Test that custom data_home affects effective paths.""" settings = CollectorSettings(_env_file=None, data_home="/custom/data") - assert ( - settings.effective_database_url - == "sqlite:////custom/data/collector/meshcore.db" - ) assert settings.collector_data_dir == "/custom/data/collector" def test_explicit_database_url_overrides(self) -> None: - """Test that explicit database_url overrides the default.""" + """Test that explicit database_url overrides the components.""" settings = CollectorSettings( _env_file=None, database_url="postgresql://user@host/db" ) @@ -181,16 +178,13 @@ class TestAPISettings: """Tests for APISettings.""" def test_custom_data_home(self) -> None: - """Test that custom data_home affects effective database path.""" + """Test that custom data_home is stored (seed/health/content paths).""" settings = APISettings(_env_file=None, data_home="/custom/data") - assert ( - settings.effective_database_url - == "sqlite:////custom/data/collector/meshcore.db" - ) + assert settings.data_home == "/custom/data" def test_explicit_database_url_overrides(self) -> None: - """Test that explicit database_url overrides the default.""" + """Test that explicit database_url overrides the components.""" settings = APISettings(_env_file=None, database_url="postgresql://user@host/db") assert settings.database_url == "postgresql://user@host/db" @@ -328,24 +322,13 @@ class TestWebSettings: assert settings.features["radio_config"] is False -class TestDatabaseBackendResolution: - """Tests for DATABASE_BACKEND selection and URL/schema resolution.""" +class TestDatabaseUrlResolution: + """Tests for the PostgreSQL-only URL/schema resolution contract.""" - def test_default_backend_is_sqlite_unchanged(self) -> None: - """No DB env vars -> the same SQLite path and no schema as before.""" - settings = CollectorSettings(_env_file=None, data_home="/data") - - assert settings.database_backend.value == "sqlite" - assert ( - settings.effective_database_url == "sqlite:////data/collector/meshcore.db" - ) - assert settings.effective_database_schema is None - - def test_postgres_backend_assembles_url_and_schema(self) -> None: - """Postgres backend assembles a URL from components and exposes the schema.""" + def test_components_assemble_url_and_schema(self) -> None: + """DATABASE_* components assemble a URL; the schema is always exposed.""" settings = APISettings( _env_file=None, - database_backend="postgres", database_host="pg", database_password="pw", ) @@ -355,22 +338,20 @@ class TestDatabaseBackendResolution: ) assert settings.effective_database_schema == "meshcorehub" - def test_postgres_password_is_url_encoded(self) -> None: + def test_password_is_url_encoded(self) -> None: """Special characters in the password are percent-encoded.""" settings = APISettings( _env_file=None, - database_backend="postgres", database_host="pg", database_password="s3cr3t/p@ss", ) assert "s3cr3t%2Fp%40ss" in settings.effective_database_url - def test_postgres_schema_override_per_instance(self) -> None: + def test_schema_override_per_instance(self) -> None: """DATABASE_SCHEMA isolates instances sharing one database.""" settings = APISettings( _env_file=None, - database_backend="postgres", database_host="pg", database_password="pw", database_schema="stg", @@ -378,19 +359,44 @@ class TestDatabaseBackendResolution: assert settings.effective_database_schema == "stg" - def test_postgres_missing_required_vars_fails_fast(self) -> None: - """Misconfigured postgres backend raises rather than silently using SQLite.""" - settings = APISettings(_env_file=None, database_backend="postgres") + def test_missing_required_vars_fails_fast(self) -> None: + """No DATABASE_URL and no components raises rather than guessing.""" + settings = APISettings(_env_file=None) - with pytest.raises(ValueError, match="DATABASE_BACKEND=postgres requires"): + with pytest.raises(ValueError, match="PostgreSQL connection is not configured"): _ = settings.effective_database_url - def test_explicit_url_overrides_backend(self) -> None: - """An explicit DATABASE_URL wins even when a backend is selected.""" + def test_explicit_url_overrides_components(self) -> None: + """An explicit DATABASE_URL wins over the component vars.""" settings = CollectorSettings( _env_file=None, - database_backend="postgres", + database_host="pg", + database_password="pw", database_url="postgresql+psycopg2://u:p@h/db", ) assert settings.effective_database_url == "postgresql+psycopg2://u:p@h/db" + + def test_backend_defaults_to_postgres_noop(self) -> None: + """DATABASE_BACKEND=postgres (the default) is accepted as a no-op.""" + settings = CommonSettings( + _env_file=None, + database_backend="postgres", + database_host="pg", + database_password="pw", + ) + + assert settings.database_backend == "postgres" + assert settings.effective_database_url.startswith("postgresql+psycopg2://") + + def test_backend_sqlite_is_rejected_with_upgrade_hint(self) -> None: + """DATABASE_BACKEND=sqlite fails with the targeted v0.19 upgrade error.""" + with pytest.raises( + ValidationError, match="SQLite support was removed in v0.19" + ): + _ = CommonSettings(_env_file=None, database_backend="sqlite") + + def test_backend_unknown_value_is_rejected(self) -> None: + """Anything other than 'postgres' is rejected.""" + with pytest.raises(ValidationError, match="must be 'postgres'"): + _ = CommonSettings(_env_file=None, database_backend="mysql") diff --git a/tests/test_common/test_database.py b/tests/test_common/test_database.py index 38b8ae2..88f0909 100644 --- a/tests/test_common/test_database.py +++ b/tests/test_common/test_database.py @@ -1,10 +1,8 @@ """Tests for database engine configuration.""" -from pathlib import Path from unittest.mock import AsyncMock, MagicMock, patch import pytest -from sqlalchemy import text from meshcore_hub.common.database import ( DatabaseManager, @@ -14,43 +12,12 @@ from meshcore_hub.common.database import ( ) -class TestSqlitePragmas: - """Verify concurrency-related SQLite pragmas are applied on connect.""" - - def test_wal_and_busy_timeout_enabled(self, tmp_path: Path) -> None: - """File-based SQLite engines should run in WAL mode with a busy timeout.""" - db_path = tmp_path / "pragma.db" - engine = create_database_engine(f"sqlite:///{db_path}") - try: - with engine.connect() as conn: - journal_mode = conn.execute(text("PRAGMA journal_mode")).scalar() - busy_timeout = conn.execute(text("PRAGMA busy_timeout")).scalar() - foreign_keys = conn.execute(text("PRAGMA foreign_keys")).scalar() - - assert str(journal_mode).lower() == "wal" - assert busy_timeout is not None and int(busy_timeout) >= 5000 - assert foreign_keys is not None and int(foreign_keys) == 1 - finally: - engine.dispose() - - def test_in_memory_engine_builds(self) -> None: - """In-memory SQLite must still build (no overflow-pool kwargs).""" - engine = create_database_engine("sqlite:///:memory:") - try: - with engine.connect() as conn: - assert conn.execute(text("SELECT 1")).scalar() == 1 - finally: - engine.dispose() - - class TestAsyncUrlMapping: - """Map sync URLs to their async-driver equivalents for the async engine.""" + """Map sync URLs to their asyncpg equivalents for the async engine.""" @pytest.mark.parametrize( "sync_url,expected", [ - ("sqlite:///x.db", "sqlite+aiosqlite:///x.db"), - ("sqlite+aiosqlite:///x.db", "sqlite+aiosqlite:///x.db"), ("postgresql://u:p@h/db", "postgresql+asyncpg://u:p@h/db"), ("postgres://u:p@h/db", "postgresql+asyncpg://u:p@h/db"), # config assembles +psycopg2; the async engine must still use asyncpg @@ -65,21 +32,16 @@ class TestAsyncUrlMapping: class TestSchemaResolution: """search_path schema resolution (explicit arg vs DATABASE_SCHEMA env).""" - def test_sqlite_never_has_schema(self, monkeypatch: pytest.MonkeyPatch) -> None: - monkeypatch.setenv("DATABASE_SCHEMA", "ignored") - assert _resolve_pg_schema("sqlite:///x.db", None) is None - assert _resolve_pg_schema("sqlite:///x.db", "explicit") is None - def test_explicit_schema_wins(self) -> None: - assert _resolve_pg_schema("postgresql://u@h/db", "prod") == "prod" + assert _resolve_pg_schema("prod") == "prod" def test_falls_back_to_env(self, monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setenv("DATABASE_SCHEMA", "stg") - assert _resolve_pg_schema("postgresql://u@h/db", None) == "stg" + assert _resolve_pg_schema(None) == "stg" def test_none_when_no_env(self, monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.delenv("DATABASE_SCHEMA", raising=False) - assert _resolve_pg_schema("postgresql://u@h/db", None) is None + assert _resolve_pg_schema(None) is None class TestPostgresSessionTimezone: @@ -87,7 +49,7 @@ class TestPostgresSessionTimezone: func.date() truncates on the session timezone's day boundary. The collector writes UTC, so the session must be UTC for day buckets to - match SQLite's UTC-text truncation. + line up with those writes. """ def test_sync_engine_sets_timezone_utc_without_schema( @@ -109,13 +71,14 @@ class TestPostgresSessionTimezone: assert "-csearch_path=meshcorehub" in options assert "-ctimezone=UTC" in options - def test_sqlite_engine_has_no_timezone_options(self, tmp_path: Path) -> None: - """SQLite engines must not set timezone options.""" - engine = create_database_engine(f"sqlite:///{tmp_path / 'x.db'}") - try: - assert "options" not in engine.url.query - finally: - engine.dispose() + def test_sync_engine_always_sizes_the_pool(self) -> None: + """Pool sizing is unconditional (sized above the Starlette threadpool).""" + with patch("meshcore_hub.common.database.create_engine") as mock_create: + create_database_engine("postgresql://u:p@h/db") + _, kwargs = mock_create.call_args + assert kwargs["pool_size"] == 20 + assert kwargs["max_overflow"] == 30 + assert kwargs["pool_pre_ping"] is True def test_async_engine_sets_server_settings_timezone_utc( self, monkeypatch: pytest.MonkeyPatch @@ -166,28 +129,10 @@ class TestPostgresSessionTimezone: assert kwargs["pool_size"] == 20 assert kwargs["max_overflow"] == 30 - def test_async_engine_skips_pool_args_for_memory_sqlite(self) -> None: - """In-memory SQLite uses a non-overflow pool; sizing args must be omitted.""" - manager = DatabaseManager.__new__(DatabaseManager) - manager.database_url = "sqlite+aiosqlite:///:memory:" - manager._echo = False - manager._schema = None - manager._async_engine = None - manager._async_session_factory = None - - with ( - patch("meshcore_hub.common.database.create_async_engine") as mock_async, - patch("meshcore_hub.common.database.event.listens_for"), - ): - manager._ensure_async_engine() - _, kwargs = mock_async.call_args - assert "pool_size" not in kwargs - assert "max_overflow" not in kwargs - async def test_adispose_closes_async_engine(self) -> None: """adispose awaits the async engine dispose and clears it.""" manager = DatabaseManager.__new__(DatabaseManager) - manager.database_url = "sqlite+aiosqlite:///:memory:" + manager.database_url = "postgresql://u:p@h/db" manager._echo = False manager._schema = None mock_async_engine = AsyncMock() @@ -205,7 +150,7 @@ class TestPostgresSessionTimezone: def test_dispose_sync_context_closes_async_engine_via_temp_loop(self) -> None: """Outside a running loop, dispose() drains the async engine too.""" manager = DatabaseManager.__new__(DatabaseManager) - manager.database_url = "sqlite+aiosqlite:///:memory:" + manager.database_url = "postgresql://u:p@h/db" manager._echo = False manager._schema = None disposed = [] diff --git a/tests/test_common/test_models.py b/tests/test_common/test_models.py index 382fa21..0472491 100644 --- a/tests/test_common/test_models.py +++ b/tests/test_common/test_models.py @@ -1,11 +1,8 @@ """Tests for database models.""" -import pytest -from sqlalchemy import create_engine, select -from sqlalchemy.orm import sessionmaker +from sqlalchemy import select from meshcore_hub.common.models import ( - Base, Node, NodeTag, Message, @@ -19,22 +16,6 @@ from meshcore_hub.common.models import ( ) -@pytest.fixture -def db_session(): - """Create an in-memory SQLite database session.""" - engine = create_engine( - "sqlite:///:memory:", - connect_args={"check_same_thread": False}, - ) - Base.metadata.create_all(engine) - Session = sessionmaker(bind=engine) - session = Session() - yield session - session.close() - Base.metadata.drop_all(engine) - engine.dispose() - - class TestNodeModel: """Tests for Node model."""