diff --git a/.github/scripts/public-api-commit-on-default-branch.sh b/.github/scripts/public-api-commit-on-default-branch.sh new file mode 100755 index 000000000..6b13911e4 --- /dev/null +++ b/.github/scripts/public-api-commit-on-default-branch.sh @@ -0,0 +1,18 @@ +#!/usr/bin/env bash +# Require the checked-out Public API commit to be on the Public API's default +# branch, so sim's table models only ever describe a migration chain the +# Public API has merged (rule sim.persistence.models-match-public-api). +# +# Usage: public-api-commit-on-default-branch.sh +set -euo pipefail + +checkout="${1:?usage: public-api-commit-on-default-branch.sh }" +branch="${2:?usage: public-api-commit-on-default-branch.sh }" +commit="$(git -C "${checkout}" rev-parse HEAD)" +git -C "${checkout}" fetch --quiet origin "${branch}" +if ! git -C "${checkout}" merge-base --is-ancestor "${commit}" FETCH_HEAD; then + printf '%s is not on the Public API'"'"'s %s branch yet; pin a merged commit\n' \ + "${commit}" "${branch}" >&2 + exit 1 +fi +printf '%s is on the Public API'"'"'s %s branch\n' "${commit}" "${branch}" diff --git a/.github/scripts/public-api-schema-commit.sh b/.github/scripts/public-api-schema-commit.sh new file mode 100755 index 000000000..02a592bc3 --- /dev/null +++ b/.github/scripts/public-api-schema-commit.sh @@ -0,0 +1,21 @@ +#!/usr/bin/env bash +# Read the pinned Public API commit whose v2 migration chain sim's table +# models describe (rule sim.persistence.models-match-public-api). +# +# Usage: public-api-schema-commit.sh +# +# The pin file holds one full commit hash. The script prints it and, on +# GitHub Actions, passes it to the later steps as the step output `commit`. +set -euo pipefail + +pin_file="${1:?usage: public-api-schema-commit.sh }" +commit="$(tr -d '[:space:]' < "${pin_file}")" +if ! [[ "${commit}" =~ ^[0-9a-f]{40}$ ]]; then + printf '%s must hold one Public API commit as 40 lowercase hexadecimal characters\n' \ + "${pin_file}" >&2 + exit 1 +fi +printf 'Pinned Public API commit: %s\n' "${commit}" +if [ -n "${GITHUB_OUTPUT:-}" ]; then + printf 'commit=%s\n' "${commit}" >> "${GITHUB_OUTPUT}" +fi diff --git a/.github/scripts/public-api-schema-drift-issue.sh b/.github/scripts/public-api-schema-drift-issue.sh new file mode 100755 index 000000000..25fd48909 --- /dev/null +++ b/.github/scripts/public-api-schema-drift-issue.sh @@ -0,0 +1,49 @@ +#!/usr/bin/env bash +# Report a failed weekly comparison of sim's table models with the Public +# API's default branch: comment on the open issue that carries the drift +# label, or open one. Runs with the workflow's own token (GH_TOKEN, with +# issues: write) and GitHub's GITHUB_SERVER_URL, GITHUB_REPOSITORY and +# GITHUB_RUN_ID, which name the failed run. +set -euo pipefail + +label="public-api-schema-drift" +run_url="${GITHUB_SERVER_URL:?}/${GITHUB_REPOSITORY:?}/actions/runs/${GITHUB_RUN_ID:?}" + +# Creates the label, or leaves it as it is apart from its color and text. +gh label create "${label}" --force --color B60205 \ + --description "The weekly comparison with the Public API's tables failed" + +open_issue="$( + gh issue list --label "${label}" --state open --limit 1 \ + --json number --jq '.[0].number // empty' +)" + +if [ -n "${open_issue}" ]; then + printf 'The comparison failed again: %s\n' "${run_url}" | + gh issue comment "${open_issue}" --body-file - + exit 0 +fi + +gh issue create \ + --title "The weekly comparison with the Public API's tables failed" \ + --label "${label}" \ + --body-file - < +# +# PUBLIC_API_SCHEMA_DATABASE_URL names the empty database to build, as +# postgresql://user:password@host:port/database. The Public API's disposable +# migration mode accepts only the local policyengine_v2_alembic_test database, +# with explicit credentials, and creates the roles its manifest manages +# without a login. The script prints the Public API commit it built and, on +# GitHub Actions, passes it to the later steps as PUBLIC_API_SCHEMA_COMMIT, so +# a failed comparison names it. +set -euo pipefail + +checkout="${1:?usage: public-api-schema-reference.sh }" +database_url="${PUBLIC_API_SCHEMA_DATABASE_URL:?PUBLIC_API_SCHEMA_DATABASE_URL must name the database to build}" + +cd "${checkout}" +commit="$(git rev-parse HEAD)" +printf 'Public API commit: %s\n' "${commit}" +if [ -n "${GITHUB_ENV:-}" ]; then + printf 'PUBLIC_API_SCHEMA_COMMIT=%s\n' "${commit}" >> "${GITHUB_ENV}" +fi +# The Public API installs its lock on Python 3.12, as its own CI does. +export UV_PYTHON=3.12 +export V2_MIGRATION_DATABASE_URL="postgresql+psycopg://${database_url#*://}" +export V2_ALEMBIC_DISPOSABLE_TEST=1 +export DEPLOYMENT_ENVIRONMENT=ci +uv sync --frozen --extra dev +uv run --frozen --extra dev alembic -c alembic-v2.ini upgrade head +uv run --frozen --extra dev python scripts/reconcile_v2_privileges.py apply --create-missing-roles diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 58d2df29c..068690334 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -18,7 +18,7 @@ jobs: # workflows on every pull request. See docs/engineering/skills/github-prs.md. name: CI complete if: ${{ always() }} - needs: [test, simulation-database, lint, pyright, quality-guards, docker-build, test-integration] + needs: [test, simulation-database, public-api-schema, lint, pyright, quality-guards, docker-build, test-integration] runs-on: ubuntu-latest permissions: contents: read @@ -81,9 +81,10 @@ jobs: simulation-database: name: Simulation database - # Runs the persistence tests that need PostgreSQL, which the Test job - # skips, on a fresh server. Full CI runs only on the lowest unmerged pull - # request in a stack. + # Runs the persistence tests marked postgres, which the Test job skips, + # on a fresh server. Tests that need the Public API's tables are marked + # public_api_schema instead and run in the Public API schema job. Full CI + # runs only on the lowest unmerged pull request in a stack. if: ${{ !github.event.pull_request.stack || github.event.pull_request.stack.base.ref == github.event.pull_request.base.ref }} runs-on: ubuntu-latest permissions: @@ -129,6 +130,92 @@ jobs: working-directory: libs/policyengine-simulation-persistence run: uv run --frozen pytest tests/ -v -m postgres + public-api-schema: + name: Public API schema + # Builds the Public API's tables with its own v2 migration chain, at the + # commit pinned in libs/policyengine-simulation-persistence/public-api-commit, + # grants the privileges its manifest declares, and runs the persistence + # tests marked public_api_schema against them: sim's table models must + # match those tables, and the simulation service's role must write them + # (rule sim.persistence.models-match-public-api). The pinned commit must + # also be on the Public API's default branch. The Public API's repository + # is public, so the job reads no secret. Full CI runs only on the lowest + # unmerged pull request in a stack. + if: ${{ !github.event.pull_request.stack || github.event.pull_request.stack.base.ref == github.event.pull_request.base.ref }} + runs-on: ubuntu-latest + permissions: + contents: read + services: + postgres: + image: postgres:17 + env: + POSTGRES_USER: postgres + POSTGRES_PASSWORD: simulation_ci + # The Public API's disposable migration mode accepts only this + # database; the server's postgres database holds the models' tables. + POSTGRES_DB: policyengine_v2_alembic_test + ports: + - 5432:5432 + options: >- + --health-cmd="pg_isready -U postgres" + --health-interval=5s + --health-timeout=5s + --health-retries=20 + env: + SIMULATION_TEST_DATABASE_URL: postgresql://postgres:simulation_ci@127.0.0.1:5432/postgres + PUBLIC_API_SCHEMA_DATABASE_URL: postgresql://postgres:simulation_ci@127.0.0.1:5432/policyengine_v2_alembic_test + # A missing server or Public API database fails the tests instead of + # skipping them. + SIMULATION_REQUIRE_DATABASE_TESTS: "1" + SIMULATION_REQUIRE_PUBLIC_API_SCHEMA: "1" + + steps: + # Neither checkout keeps the job's token, since the job runs the Public + # API's code from a pinned commit. + - uses: actions/checkout@v6 + with: + persist-credentials: false + + - name: Read the pinned Public API commit + id: public-api + run: .github/scripts/public-api-schema-commit.sh libs/policyengine-simulation-persistence/public-api-commit + + - name: Check out the pinned Public API commit + uses: actions/checkout@v6 + with: + repository: PolicyEngine/policyengine-api + ref: ${{ steps.public-api.outputs.commit }} + path: public-api + persist-credentials: false + + - name: Set up Python + uses: actions/setup-python@v6 + with: + # 3.12 for the Public API's lock, as its own CI uses; 3.13 for sim's. + python-version: | + 3.12 + 3.13 + + - name: Install uv + uses: astral-sh/setup-uv@v8.1.0 + with: + version: "0.13.6" + enable-cache: true + + - name: Build the Public API's tables at the pinned commit + run: .github/scripts/public-api-schema-reference.sh public-api + + - name: Install dependencies + working-directory: libs/policyengine-simulation-persistence + run: uv sync --extra test --frozen + + - name: Compare sim's table models with the Public API's tables + working-directory: libs/policyengine-simulation-persistence + run: uv run --frozen pytest tests/ -v -m public_api_schema + + - name: Require the pinned commit on the Public API's default branch + run: .github/scripts/public-api-commit-on-default-branch.sh public-api master + lint: name: Lint # Full CI runs only on the lowest unmerged pull request in a stack. diff --git a/.github/workflows/public-api-schema-drift.yml b/.github/workflows/public-api-schema-drift.yml new file mode 100644 index 000000000..62880ce8e --- /dev/null +++ b/.github/workflows/public-api-schema-drift.yml @@ -0,0 +1,111 @@ +name: Public API schema drift + +# Compares sim's table models with the tables the Public API's v2 migration +# chain builds at the head of its default branch, every Monday and on demand. +# Pull requests compare them at the pinned commit instead (the PR workflow's +# public-api-schema job). A failure here means the Public API has merged a +# change to a table the simulation service writes, or to its role's grants, +# that the models and the pin need to follow, or that its tables can't be +# built at that head; the log says which (rule +# sim.persistence.models-match-public-api). The Public API's repository is +# public, so the workflow reads no secret. When the comparison fails, a second +# job opens an issue labelled public-api-schema-drift, or comments on the open +# one, with the workflow's own token; only that job may write issues. + +on: + schedule: + - cron: "17 6 * * 1" + workflow_dispatch: + +permissions: + contents: read + +jobs: + public-api-schema-drift: + name: Compare the models with the Public API's default branch + runs-on: ubuntu-latest + services: + postgres: + image: postgres:17 + env: + POSTGRES_USER: postgres + POSTGRES_PASSWORD: simulation_ci + # The Public API's disposable migration mode accepts only this + # database; the server's postgres database holds the models' tables. + POSTGRES_DB: policyengine_v2_alembic_test + ports: + - 5432:5432 + options: >- + --health-cmd="pg_isready -U postgres" + --health-interval=5s + --health-timeout=5s + --health-retries=20 + env: + SIMULATION_TEST_DATABASE_URL: postgresql://postgres:simulation_ci@127.0.0.1:5432/postgres + PUBLIC_API_SCHEMA_DATABASE_URL: postgresql://postgres:simulation_ci@127.0.0.1:5432/policyengine_v2_alembic_test + # A missing server or Public API database fails the tests instead of + # skipping them. + SIMULATION_REQUIRE_DATABASE_TESTS: "1" + SIMULATION_REQUIRE_PUBLIC_API_SCHEMA: "1" + + steps: + # Neither checkout keeps the job's token, since the job runs the Public + # API's code. + - uses: actions/checkout@v6 + with: + persist-credentials: false + + - name: Check out the Public API's default branch + uses: actions/checkout@v6 + with: + repository: PolicyEngine/policyengine-api + ref: master + path: public-api + persist-credentials: false + + - name: Set up Python + uses: actions/setup-python@v6 + with: + # 3.12 for the Public API's lock, as its own CI uses; 3.13 for sim's. + python-version: | + 3.12 + 3.13 + + - name: Install uv + uses: astral-sh/setup-uv@v8.1.0 + with: + version: "0.13.6" + enable-cache: true + + - name: Build the Public API's tables at its default branch + run: .github/scripts/public-api-schema-reference.sh public-api + + - name: Install dependencies + working-directory: libs/policyengine-simulation-persistence + run: uv sync --extra test --frozen + + - name: Compare sim's table models with the Public API's tables + working-directory: libs/policyengine-simulation-persistence + run: uv run --frozen pytest tests/ -v -m public_api_schema + + report-public-api-schema-drift: + name: Open or update the drift issue + needs: public-api-schema-drift + # Only a failed comparison: a passing or cancelled run reports nothing. + if: ${{ failure() }} + runs-on: ubuntu-latest + # It checks out sim alone, to run its script; it never checks out or runs + # the Public API's code. + permissions: + contents: read + issues: write + steps: + - uses: actions/checkout@v6 + with: + persist-credentials: false + + - name: Open or comment on the drift issue + env: + GH_TOKEN: ${{ github.token }} + GH_REPO: ${{ github.repository }} + run: .github/scripts/public-api-schema-drift-issue.sh diff --git a/AGENTS.md b/AGENTS.md index 4263ba81e..1079699f6 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -92,13 +92,14 @@ same-repository draft PR. - The simulation service lives in `projects/policyengine-simulation-executor`. - API integration tests live in `projects/policyengine-apis-integ`. - PR CI runs simulation unit tests, the persistence tests that need - PostgreSQL, Ruff format and lint checks, Pyright for every package, quality - guards (including the rules registry), Docker build, and local integration - tests. Two jobs run separately, each from its own workflow, on every PR, - stacked ones included: `PR description` checks that every open PR's - description starts with `Fixes #ISSUE_NUMBER` for an open issue in this - repository, and `Changelog fragment` checks that the PR adds a changelog - fragment relative to its own base branch. + PostgreSQL, the comparison of the persistence models with the Public API's + tables at a pinned commit, Ruff format and lint checks, Pyright for every + package, quality guards (including the rules registry), Docker build, and + local integration tests. Two jobs run separately, each from its own + workflow, on every PR, stacked ones included: `PR description` checks that + every open PR's description starts with `Fixes #ISSUE_NUMBER` for an open + issue in this repository, and `Changelog fragment` checks that the PR adds a + changelog fragment relative to its own base branch. - Every PR adds a changelog fragment, `changelog.d/..md`, and never edits `CHANGELOG.md` to describe a change (rule `sim.process.changelog-fragment`). A merge to `main` doesn't deploy itself: @@ -135,7 +136,7 @@ To add or change a rule, read `docs/engineering/skills/rules-registry.md`. ### Simulation service database -- Checked: `sim.persistence.missing-privileges-reported` +- Checked: `sim.persistence.missing-privileges-reported`, `sim.persistence.models-match-public-api` ### Deployment pipeline diff --git a/changelog.d/b2-public-api-schema-check.added.md b/changelog.d/b2-public-api-schema-check.added.md new file mode 100644 index 000000000..900ddca26 --- /dev/null +++ b/changelog.d/b2-public-api-schema-check.added.md @@ -0,0 +1 @@ +CI checks sim's models of the tables the simulation service writes against the tables the Public API's migration chain builds at a pinned Public API commit, and that the service's role can write them; a weekly run compares them with the Public API's default branch, and opens an issue, or comments on the open one, when that comparison fails. diff --git a/docs/engineering/skills/database-access.md b/docs/engineering/skills/database-access.md index 4aa8ea829..f35e9728e 100644 --- a/docs/engineering/skills/database-access.md +++ b/docs/engineering/skills/database-access.md @@ -73,9 +73,11 @@ but still connect through SQLAlchemy. The guard reads source, not types. It misses imports through `importlib` or `__import__`, a table reached through some other local variable, and SQL text -assembled from separate literals. It doesn't read shell scripts or SQL files; -today no shell script calls a database client, and the repository has no SQL -files. +assembled from separate literals. It doesn't read shell scripts or SQL files. +The one shell script that reaches a database, +`.github/scripts/public-api-schema-reference.sh`, runs the Public API's own +migration and privilege commands against a disposable CI database for +`sim.persistence.models-match-public-api`; the repository has no SQL files. ## Out of scope diff --git a/docs/generated/rules.md b/docs/generated/rules.md index 09ea6cae6..e8b7d007f 100644 --- a/docs/generated/rules.md +++ b/docs/generated/rules.md @@ -8,11 +8,11 @@ Every rule that governs `policyengine-sim-api` is listed here with how it is enf | Tier | enforced | ratcheted | advisory | doc-only | Total | | --- | ---: | ---: | ---: | ---: | ---: | -| mechanical | 60 | 0 | 0 | 14 | 74 | +| mechanical | 61 | 0 | 0 | 14 | 75 | | metric | 1 | 0 | 0 | 0 | 1 | | judgment | 0 | 0 | 0 | 0 | 0 | -**Blocking:** 61 of 75 rules. +**Blocking:** 62 of 76 rules. ## Gaps @@ -140,14 +140,22 @@ flowchart LR | Rule | Statement | Tier | Status | Blocking | Enforced by | CI | Proof | | --- | --- | --- | --- | --- | --- | --- | --- | | `sim.persistence.missing-privileges-reported` | The simulation service's privilege check, which its readiness check uses, names every table privilege and every column UPDATE privilege its role needs on the Public API's tables and lacks, as PostgreSQL reports it, counts a table missing from the public schema as lacking all of them and a missing column as lacking its own, and names a missing USAGE privilege on that schema. | mechanical | enforced | yes | pytest `libs/policyengine-simulation-persistence/tests/test_access.py` | `pr.yml:simulation-database` | yes | +| `sim.persistence.models-match-public-api` | Sim's models of the Public API's tables match the tables the Public API's v2 migration chain builds at the pinned Public API commit, which is on the Public API's default branch: every column's type, nullability and default, every constraint and index by name and definition, and every enum's values; and the simulation service's role, with the privileges the Public API's manifest declares, writes every modelled table. | mechanical | enforced | yes | pytest `libs/policyengine-simulation-persistence/tests/test_public_api_schema.py`, `libs/policyengine-simulation-persistence/tests/test_service_writes.py` | `pr.yml:public-api-schema`, `public-api-schema-drift.yml:public-api-schema-drift` | yes | ```mermaid flowchart LR r_sim_persistence_missing_privileges_reported("sim.persistence.missing-privileges-reported (enforced)") e_pytest_libs_policyengine_simulation_persistence_tests_test_access_py["pytest libs/policyengine-simulation-persistence/tests/test_access.py"] j_CI_pr_yml_simulation_database["CI pr.yml:simulation-database"] + r_sim_persistence_models_match_public_api("sim.persistence.models-match-public-api (enforced)") + e_pytest_libs_policyengine_simulation_persistence_tests_test_public_api_schema_py__libs_policyengine_simulation_persistence_tests_test_service_writes_py["pytest libs/policyengine-simulation-persistence/tests/test_public_api_schema.py, libs/policyengine-simulation-persistence/tests/test_service_writes.py"] + j_CI_pr_yml_public_api_schema["CI pr.yml:public-api-schema"] + j_CI_public_api_schema_drift_yml_public_api_schema_drift["CI public-api-schema-drift.yml:public-api-schema-drift"] r_sim_persistence_missing_privileges_reported --> e_pytest_libs_policyengine_simulation_persistence_tests_test_access_py e_pytest_libs_policyengine_simulation_persistence_tests_test_access_py --> j_CI_pr_yml_simulation_database + r_sim_persistence_models_match_public_api --> e_pytest_libs_policyengine_simulation_persistence_tests_test_public_api_schema_py__libs_policyengine_simulation_persistence_tests_test_service_writes_py + e_pytest_libs_policyengine_simulation_persistence_tests_test_public_api_schema_py__libs_policyengine_simulation_persistence_tests_test_service_writes_py --> j_CI_pr_yml_public_api_schema + e_pytest_libs_policyengine_simulation_persistence_tests_test_public_api_schema_py__libs_policyengine_simulation_persistence_tests_test_service_writes_py --> j_CI_public_api_schema_drift_yml_public_api_schema_drift ``` ## Deployment pipeline diff --git a/libs/policyengine-simulation-persistence/README.md b/libs/policyengine-simulation-persistence/README.md index e5efef9b8..2520772cf 100644 --- a/libs/policyengine-simulation-persistence/README.md +++ b/libs/policyengine-simulation-persistence/README.md @@ -47,6 +47,58 @@ household report with only a baseline). A report with a `budget_window_size` is calculated as a `budget_window_runs` row with one report run per year; each year's report run names that row and its year, and belongs to the same report. +## Checking the models against the Public API + +Rule `sim.persistence.models-match-public-api`. The PR workflow's +`Public API schema` job checks out the Public API at the commit pinned in +`public-api-commit`, builds its tables with its own v2 migration chain and +grants the privileges its manifest declares +(`.github/scripts/public-api-schema-reference.sh`), then runs the tests +marked `public_api_schema`. `test_public_api_schema` builds the models' tables +from `service_metadata` in a disposable database, after the Public API's +tables their keys name (`referenced_metadata`, by `id` alone), and compares +the models' tables with the Public API's through PostgreSQL's catalog +(`tests/schema_comparison.py`): every column's type, nullability and default, +every constraint and index by name and definition, and every enum's values +must match. `test_service_writes` creates a row in every modelled table as +the service's role. Another test, outside the rule, requires the modelled +tables' foreign keys to the Public API's users, policies, dynamics, +households, datasets, regions and model catalog tables to be exactly the 20 +in `tests/public_api_keys.py`, each by its own table, its name and the table +it names, so a key dropped, added, renamed, moved or pointed at another table +shows even if the models follow it. Last, the job requires the +pinned commit to be on the Public API's `master` branch, so the models only +ever describe a chain the Public API has merged. + +**Choosing the pin.** Pin the Public API commit whose migration chain the +models describe: the merge commit of the Public API change that last +altered a modelled table, or, while that change is still a pull request, its +head commit. Until that pull request merges, the job's last step fails and +says so. A merge commit keeps the pull request's head on `master`; if the +Public API squashes or rebases the pull request instead, its head never +reaches `master`, so move the pin to the commit that landed. + +**Keeping it current.** A sim pull request that changes the models moves the +pin in the same change. The `Public API schema drift` workflow runs the same +tests every Monday, and on demand, against the head of the Public API's +`master`. It fails when the Public API has merged a change to a modelled +table, or to the service role's grants, that the models and the pin haven't +followed yet, and also when the Public API's tables can't be built at that +head; its log says which. When it fails, a second job opens an issue labelled +`public-api-schema-drift`, or comments on the one already open, with the +workflow's own token; the comparison job's token stays read-only. The pin +still moves only in a pull request. + +To run the comparison locally, build the reference with the script (it +needs an empty `policyengine_v2_alembic_test` database and a checkout of the +Public API at the pinned commit), then: + +```bash +SIMULATION_TEST_DATABASE_URL=postgresql://postgres:password@127.0.0.1:5432/postgres \ +PUBLIC_API_SCHEMA_DATABASE_URL=postgresql://postgres:password@127.0.0.1:5432/policyengine_v2_alembic_test \ +uv run pytest tests/ -m public_api_schema +``` + ## The role's privileges `SERVICE_TABLE_PRIVILEGES` lists the table privileges the service's role @@ -85,11 +137,15 @@ uv sync --extra test uv run pytest tests/ ``` -Tests marked `public_api_schema` run as the service's role in a database -the Public API's own v2 migration chain built, with the privileges its -manifest declares granted: set `PUBLIC_API_SCHEMA_DATABASE_URL` to that -database's superuser URL. They create a row in every modelled table and roll -it back. Without the variable they skip. +Tests marked `public_api_schema` run in the `Public API schema` jobs; the +rule's proof among them needs no database, and the others need one the +Public API's own v2 migration chain built, with the privileges its manifest +declares granted: +set `PUBLIC_API_SCHEMA_DATABASE_URL` to that database's superuser URL. +`test_service_writes` creates a row in every modelled table as the service's +role and rolls it back; the comparison in `test_public_api_schema` also needs +`SIMULATION_TEST_DATABASE_URL`, to build the models' own tables. Without the +variables they skip. Tests marked `postgres` need a PostgreSQL server: set `SIMULATION_TEST_DATABASE_URL` to its superuser URL, such as diff --git a/libs/policyengine-simulation-persistence/public-api-commit b/libs/policyengine-simulation-persistence/public-api-commit new file mode 100644 index 000000000..6063afc40 --- /dev/null +++ b/libs/policyengine-simulation-persistence/public-api-commit @@ -0,0 +1 @@ +772ca6b14d06f1b1dfdaf19f7353cd6d8ee356a7 diff --git a/libs/policyengine-simulation-persistence/pyproject.toml b/libs/policyengine-simulation-persistence/pyproject.toml index 1e3db3310..2118ea4f5 100644 --- a/libs/policyengine-simulation-persistence/pyproject.toml +++ b/libs/policyengine-simulation-persistence/pyproject.toml @@ -37,8 +37,8 @@ packages = ["src/policyengine_simulation_persistence"] pythonpath = ["src"] testpaths = ["tests"] markers = [ - "postgres: needs a disposable PostgreSQL server named by SIMULATION_TEST_DATABASE_URL", - "public_api_schema: needs a database the Public API's migration chain built, named by PUBLIC_API_SCHEMA_DATABASE_URL", + "postgres: run by the Simulation database job; needs a disposable PostgreSQL server named by SIMULATION_TEST_DATABASE_URL", + "public_api_schema: run by the Public API schema jobs; most need a database the Public API's migration chain built (PUBLIC_API_SCHEMA_DATABASE_URL), and the comparison also a disposable PostgreSQL server (SIMULATION_TEST_DATABASE_URL)", ] [tool.pyright] diff --git a/libs/policyengine-simulation-persistence/tests/conftest.py b/libs/policyengine-simulation-persistence/tests/conftest.py index 8176a3c28..328c731b3 100644 --- a/libs/policyengine-simulation-persistence/tests/conftest.py +++ b/libs/policyengine-simulation-persistence/tests/conftest.py @@ -6,7 +6,8 @@ the variable those tests skip, unless ``SIMULATION_REQUIRE_DATABASE_TESTS=1`` (set in CI), where they fail instead. -Tests marked ``public_api_schema`` need a database the Public API's own v2 +Tests marked ``public_api_schema`` run in the Public API schema jobs; those +that use ``public_api_database_url`` need a database the Public API's own v2 migration chain built, with the privileges its manifest declares granted (run its ``alembic -c alembic-v2.ini upgrade head`` and ``scripts/reconcile_v2_privileges.py apply --create-missing-roles`` against diff --git a/libs/policyengine-simulation-persistence/tests/schema_comparison.py b/libs/policyengine-simulation-persistence/tests/schema_comparison.py new file mode 100644 index 000000000..b393297a2 --- /dev/null +++ b/libs/policyengine-simulation-persistence/tests/schema_comparison.py @@ -0,0 +1,335 @@ +"""Describe tables from PostgreSQL's catalog, and list how two descriptions differ. + +Sim's table models describe tables the Public API's migration chain creates. +The comparison builds the models' tables in one database and reads both that +database and one the chain built through the same catalog queries, so every +type, default and definition is written the way PostgreSQL itself writes it. + +``describe_tables`` reads, for each table: its columns (type, with an enum +named as ``enum ``; whether they allow null; their default), its +constraints (primary key, unique, foreign key, check and exclusion) by name +with PostgreSQL's definition, and its other indexes by name with their +definition; and the values of each enum its columns use. +``schema_differences`` lists every way the models' tables differ from the +real ones. Tables the models leave out are not compared. +``foreign_key_targets`` reads which table each foreign key names, and +``foreign_keys_to`` keeps the keys that name the given tables. +""" + +from __future__ import annotations + +import json +import re +from dataclasses import asdict, dataclass +from typing import TYPE_CHECKING + +from sqlalchemy import text + +if TYPE_CHECKING: + from collections.abc import Collection + from pathlib import Path + + from sqlalchemy import Connection + +ENUM_PREFIX = "enum " +# An identifier as PostgreSQL writes it: quoted, with "" for a quote, or bare. +_IDENTIFIER = r'"(?:[^"]|"")+"|[^\s".(]+' +# The table a foreign key's definition names. PostgreSQL qualifies it by its +# schema whenever the bare name wouldn't find that table on the search path. +_FOREIGN_KEY_TARGET = re.compile( + rf"^FOREIGN KEY \(.+?\) REFERENCES (?:(?P{_IDENTIFIER})\.)?" + rf"(?P{_IDENTIFIER})\(" +) + +_COLUMNS = text( + "SELECT c.relname AS table_name, a.attname AS column_name, " + "CASE WHEN t.typtype = 'e' THEN 'enum ' || t.typname " + "ELSE format_type(a.atttypid, a.atttypmod) END AS column_type, " + "NOT a.attnotnull AS nullable, " + "pg_get_expr(d.adbin, d.adrelid) AS column_default " + "FROM pg_attribute a " + "JOIN pg_class c ON c.oid = a.attrelid " + "JOIN pg_namespace n ON n.oid = c.relnamespace " + "JOIN pg_type t ON t.oid = a.atttypid " + "LEFT JOIN pg_attrdef d ON d.adrelid = a.attrelid AND d.adnum = a.attnum " + "WHERE n.nspname = 'public' AND c.relkind IN ('r', 'p') " + "AND a.attnum > 0 AND NOT a.attisdropped " + "AND c.relname = ANY(:table_names) " + "ORDER BY c.relname, a.attnum" +) +_CONSTRAINTS = text( + "SELECT c.relname AS table_name, con.conname AS constraint_name, " + "pg_get_constraintdef(con.oid) AS definition " + "FROM pg_constraint con " + "JOIN pg_class c ON c.oid = con.conrelid " + "JOIN pg_namespace n ON n.oid = c.relnamespace " + "WHERE n.nspname = 'public' AND con.contype IN ('p', 'u', 'f', 'c', 'x') " + "AND c.relname = ANY(:table_names) " + "ORDER BY c.relname, con.conname" +) +# Indexes that back a primary key, unique or exclusion constraint are compared +# as that constraint. +_INDEXES = text( + "SELECT t.relname AS table_name, i.relname AS index_name, " + "pg_get_indexdef(i.oid) AS definition " + "FROM pg_index x " + "JOIN pg_class i ON i.oid = x.indexrelid " + "JOIN pg_class t ON t.oid = x.indrelid " + "JOIN pg_namespace n ON n.oid = t.relnamespace " + "WHERE n.nspname = 'public' AND t.relname = ANY(:table_names) " + "AND NOT EXISTS (SELECT 1 FROM pg_constraint con " + "WHERE con.conindid = x.indexrelid AND con.contype IN ('p', 'u', 'x')) " + "ORDER BY t.relname, i.relname" +) +_ENUM_VALUES = text( + "SELECT t.typname AS enum_name, " + "array_agg(e.enumlabel::text ORDER BY e.enumsortorder) AS enum_values " + "FROM pg_enum e JOIN pg_type t ON t.oid = e.enumtypid " + "JOIN pg_namespace n ON n.oid = t.typnamespace " + "WHERE n.nspname = 'public' GROUP BY t.typname" +) + + +@dataclass(frozen=True) +class ColumnShape: + """A column's type, nullability and default, as PostgreSQL writes them.""" + + column_type: str + nullable: bool + default: str | None + + +@dataclass(frozen=True) +class TableShape: + """A table's columns, its constraints and its other indexes, by name.""" + + columns: dict[str, ColumnShape] + constraints: dict[str, str] + indexes: dict[str, str] + + +@dataclass(frozen=True) +class SchemaShape: + """Tables by name, and the values of each enum their columns use.""" + + tables: dict[str, TableShape] + enums: dict[str, tuple[str, ...]] + + +def describe_tables( + connection: Connection, table_names: Collection[str] +) -> SchemaShape: + """Describe these tables of the ``public`` schema from the database's catalog.""" + names = sorted(table_names) + parameters = {"table_names": names} + columns: dict[str, dict[str, ColumnShape]] = {} + for row in connection.execute(_COLUMNS, parameters).mappings(): + columns.setdefault(row["table_name"], {})[row["column_name"]] = ColumnShape( + column_type=row["column_type"], + nullable=row["nullable"], + default=row["column_default"], + ) + constraints: dict[str, dict[str, str]] = {} + for row in connection.execute(_CONSTRAINTS, parameters).mappings(): + constraints.setdefault(row["table_name"], {})[row["constraint_name"]] = row[ + "definition" + ] + indexes: dict[str, dict[str, str]] = {} + for row in connection.execute(_INDEXES, parameters).mappings(): + indexes.setdefault(row["table_name"], {})[row["index_name"]] = row["definition"] + used_enums = { + column.column_type.removeprefix(ENUM_PREFIX) + for table_columns in columns.values() + for column in table_columns.values() + if column.column_type.startswith(ENUM_PREFIX) + } + enums = { + row["enum_name"]: tuple(row["enum_values"]) + for row in connection.execute(_ENUM_VALUES).mappings() + if row["enum_name"] in used_enums + } + return SchemaShape( + tables={ + name: TableShape( + columns=columns[name], + constraints=constraints.get(name, {}), + indexes=indexes.get(name, {}), + ) + for name in names + if name in columns + }, + enums=enums, + ) + + +def _default_difference(model: str | None, real: str | None) -> str: + model_default = "has no default" if model is None else f"defaults to {model}" + real_default = "has no default" if real is None else f"defaults to {real}" + return f"the model {model_default}, the Public API's {real_default}" + + +def _column_differences( + table_name: str, models: TableShape, real: TableShape +) -> list[str]: + found = [] + for column_name, modelled in models.columns.items(): + actual = real.columns.get(column_name) + where = f"{table_name}.{column_name}" + if actual is None: + found.append(f"{where}: the Public API's table has no such column") + continue + if modelled.column_type != actual.column_type: + found.append( + f"{where}: the model is {modelled.column_type}, the Public API's " + f"is {actual.column_type}" + ) + if modelled.nullable != actual.nullable: + found.append( + f"{where}: the model " + f"{'allows' if modelled.nullable else 'refuses'} null, the Public " + f"API's {'allows' if actual.nullable else 'refuses'} it" + ) + if modelled.default != actual.default: + found.append( + f"{where}: {_default_difference(modelled.default, actual.default)}" + ) + found.extend( + f"{table_name}.{column_name}: the model has no such column" + for column_name in real.columns + if column_name not in models.columns + ) + return found + + +def _definition_differences( + table_name: str, + kind: str, + models: dict[str, str], + real: dict[str, str], +) -> list[str]: + found = [] + for name, modelled in models.items(): + actual = real.get(name) + if actual is None: + found.append( + f"{table_name}: the Public API's table has no {kind} {name} " + f"({modelled})" + ) + elif actual != modelled: + found.append( + f"{table_name}: {name} is {modelled} in the model, {actual} in " + "the Public API" + ) + found.extend( + f"{table_name}: the model has no {kind} {name} ({actual})" + for name, actual in real.items() + if name not in models + ) + return found + + +def schema_differences(models: SchemaShape, real: SchemaShape) -> list[str]: + """Return every way the models' tables differ from the real ones, or nothing.""" + found = [] + for table_name, modelled in models.tables.items(): + actual = real.tables.get(table_name) + if actual is None: + found.append(f"{table_name}: the Public API has no such table") + continue + found.extend(_column_differences(table_name, modelled, actual)) + found.extend( + _definition_differences( + table_name, "constraint", modelled.constraints, actual.constraints + ) + ) + found.extend( + _definition_differences( + table_name, "index", modelled.indexes, actual.indexes + ) + ) + for enum_name, modelled_values in models.enums.items(): + actual_values = real.enums.get(enum_name) + if actual_values is None: + # Enum columns are named without their schema, so an enum moved out + # of the public schema would otherwise pass unchecked. + found.append( + f"enum {enum_name}: the Public API has no such enum in its public " + "schema" + ) + elif actual_values != modelled_values: + found.append( + f"enum {enum_name}: the model has ({', '.join(modelled_values)}), " + f"the Public API has ({', '.join(actual_values)})" + ) + return found + + +def schema_to_json(schema: SchemaShape) -> str: + """Write a description as JSON, as the rule's proof fixtures store it.""" + return json.dumps(asdict(schema), indent=2, sort_keys=True) + "\n" + + +def schema_from_json(path: Path) -> SchemaShape: + """Read a description that ``schema_to_json`` wrote.""" + payload = json.loads(path.read_text(encoding="utf-8")) + return SchemaShape( + tables={ + name: TableShape( + columns={ + column_name: ColumnShape(**column) + for column_name, column in table["columns"].items() + }, + constraints=dict(table["constraints"]), + indexes=dict(table["indexes"]), + ) + for name, table in payload["tables"].items() + }, + enums={name: tuple(values) for name, values in payload["enums"].items()}, + ) + + +def _unquoted(identifier: str) -> str: + if identifier.startswith('"'): + return identifier[1:-1].replace('""', '"') + return identifier + + +def foreign_key_targets(schema: SchemaShape) -> dict[tuple[str, str], str]: + """Return the table each foreign key names, by its table and constraint name. + + The table is unquoted, and keeps its schema when PostgreSQL wrote one, as + ``auth.users``. Raise ValueError for a foreign key whose definition names + no table this can read, so no key goes uncounted. + """ + targets: dict[tuple[str, str], str] = {} + for table_name, table in schema.tables.items(): + for constraint_name, definition in table.constraints.items(): + if not definition.startswith("FOREIGN KEY "): + continue + target = _FOREIGN_KEY_TARGET.match(definition) + if target is None: + message = ( + f"{table_name}.{constraint_name}: no table read from {definition!r}" + ) + raise ValueError(message) + named_table = _unquoted(target["table"]) + if target["schema"] is not None: + named_table = f"{_unquoted(target['schema'])}.{named_table}" + targets[(table_name, constraint_name)] = named_table + return targets + + +def foreign_keys_to( + schema: SchemaShape, table_names: Collection[str] +) -> dict[tuple[str, str], str]: + """Return each foreign key that names one of these tables, and the table. + + Keys are told apart by their own table and their name, which is unique + only within that table. A table PostgreSQL wrote with its schema, as + ``auth.users``, isn't the bare ``users``. + """ + return { + table_and_name: named_table + for table_and_name, named_table in foreign_key_targets(schema).items() + if named_table in table_names + } diff --git a/libs/policyengine-simulation-persistence/tests/test_database_job.py b/libs/policyengine-simulation-persistence/tests/test_database_job.py index e1436f30f..ab7ab03d8 100644 --- a/libs/policyengine-simulation-persistence/tests/test_database_job.py +++ b/libs/policyengine-simulation-persistence/tests/test_database_job.py @@ -1,7 +1,9 @@ -"""The PR job that runs the persistence tests needing PostgreSQL. +"""The PR job that runs the persistence tests marked ``postgres``. Those tests skip without a server, so the ``Test`` job never runs them; the ``Simulation database`` job gives them one and makes a missing server fail. +The tests that also need the Public API's tables are marked +``public_api_schema`` instead, and run in the Public API schema jobs. """ from __future__ import annotations diff --git a/libs/policyengine-simulation-persistence/tests/test_public_api_schema.py b/libs/policyengine-simulation-persistence/tests/test_public_api_schema.py new file mode 100644 index 000000000..70215091c --- /dev/null +++ b/libs/policyengine-simulation-persistence/tests/test_public_api_schema.py @@ -0,0 +1,123 @@ +"""Sim's table models match the tables the Public API's migration chain builds. + +Rule sim.persistence.models-match-public-api. ``PUBLIC_API_SCHEMA_DATABASE_URL`` +names a database the Public API's v2 migration chain built at the pinned +commit (the ``public-api-schema`` job builds it). The models' own tables are +built from ``service_metadata`` in a disposable database, after the Public +API's tables their keys name, which ``referenced_metadata`` models by their +``id`` alone, and both databases are read through the same catalog queries. +Only the models' own tables are compared. Without the Public API's database the +comparison skips, unless ``SIMULATION_REQUIRE_PUBLIC_API_SCHEMA=1`` (set in +that job), where it fails instead. +""" + +from __future__ import annotations + +import os +from pathlib import Path + +import pytest +from policyengine_simulation_persistence import referenced_metadata, service_metadata +from public_api_keys import KEYS_TO_PUBLIC_API_TABLES +from schema_comparison import ( + describe_tables, + foreign_keys_to, + schema_differences, + schema_from_json, +) +from sqlalchemy import create_engine +from sqlalchemy.engine import make_url + +# The reference script sets it on GitHub Actions: the Public API commit built. +COMMIT_VARIABLE = "PUBLIC_API_SCHEMA_COMMIT" +PROOF_FIXTURES = ( + Path(__file__).resolve().parents[3] + / "rules" + / "fixtures" + / "sim.persistence.models-match-public-api" +) + + +def _engine(database_url: str): + return create_engine(make_url(database_url).set(drivername="postgresql+psycopg")) + + +# Not marked postgres: the Simulation database job, which selects that +# marker, has no Public API database, so the test could only skip there. +@pytest.mark.public_api_schema +def test_the_models_match_the_public_api_tables( + disposable_database, public_api_database_url +) -> None: + table_names = [table.name for table in service_metadata.tables.values()] + models_engine = _engine(disposable_database.superuser_url) + real_engine = _engine(public_api_database_url) + try: + # Tests may build the models' tables; runtime code never does. The + # tables their keys name come first. + referenced_metadata.create_all(models_engine) + service_metadata.create_all(models_engine) + with models_engine.connect() as connection: + models = describe_tables(connection, table_names) + with real_engine.connect() as connection: + real = describe_tables(connection, table_names) + finally: + models_engine.dispose() + real_engine.dispose() + + assert set(models.tables) == set(table_names) + compared_with = os.environ.get(COMMIT_VARIABLE) or "an unrecorded commit" + assert schema_differences(models, real) == [], ( + f"compared with the Public API at {compared_with}" + ) + + +# Marked so the rule's own CI jobs, which select public_api_schema, collect it. +@pytest.mark.public_api_schema +def test_rule_proof_models_match_public_api() -> None: + """Proof for rule sim.persistence.models-match-public-api. + + ``public_api_schema.json`` describes ``simulation_runs`` as an earlier + commit of the Public API's chain built it, before the table gained + ``year`` and its keys to the Public API's own tables; the proof needs only + a fixed description. The bad models copy ``status`` as + ``character varying(32)``, where the Public API's column is the enum + ``v2_simulation_status``, and leave out ``modal_call_id``, which a later + revision added. + """ + real = schema_from_json(PROOF_FIXTURES / "public_api_schema.json") + bad = schema_from_json(PROOF_FIXTURES / "bad_models.json") + good = schema_from_json(PROOF_FIXTURES / "good_models.json") + + assert schema_differences(bad, real) == [ + "simulation_runs.status: the model is character varying(32), the Public " + "API's is enum v2_simulation_status", + "simulation_runs.modal_call_id: the model has no such column", + ] + assert schema_differences(good, real) == [] + + +@pytest.mark.public_api_schema +def test_the_public_api_keeps_its_keys_from_the_services_tables( + public_api_database_url, +) -> None: + """The keys from the service's tables to the Public API's own are the 20 expected. + + Each is compared by its table, its name and the table it names; the + comparison with the models checks the rest of its definition. + + The Public API's own tables are those ``referenced_metadata`` models: users, + policies, dynamics, households, datasets, regions and the model catalog. + """ + table_names = [table.name for table in service_metadata.tables.values()] + public_api_tables = {table.name for table in referenced_metadata.tables.values()} + real_engine = _engine(public_api_database_url) + try: + with real_engine.connect() as connection: + real = describe_tables(connection, table_names) + finally: + real_engine.dispose() + + assert foreign_keys_to(real, public_api_tables) == { + table_and_name: named_columns.partition("(")[0] + for table_and_name, (_, named_columns, _) in KEYS_TO_PUBLIC_API_TABLES.items() + } diff --git a/libs/policyengine-simulation-persistence/tests/test_public_api_schema_drift_issue.py b/libs/policyengine-simulation-persistence/tests/test_public_api_schema_drift_issue.py new file mode 100644 index 000000000..3c75b6137 --- /dev/null +++ b/libs/policyengine-simulation-persistence/tests/test_public_api_schema_drift_issue.py @@ -0,0 +1,154 @@ +"""The weekly comparison's failure report: an issue, opened or commented on. + +When the ``Public API schema drift`` workflow's comparison fails, a second job +runs ``public-api-schema-drift-issue.sh`` with the workflow's own token. The +script comments on the open issue that carries the drift label, or opens one. +The comparison job keeps its read-only token. +""" + +from __future__ import annotations + +from pathlib import Path +from typing import TYPE_CHECKING + +import yaml +from test_public_api_schema_scripts import _fake_command, _run + +if TYPE_CHECKING: + import subprocess + +REPOSITORY_ROOT = Path(__file__).resolve().parents[3] +DRIFT_WORKFLOW = ( + REPOSITORY_ROOT / ".github" / "workflows" / "public-api-schema-drift.yml" +) +ISSUE_SCRIPT = ( + REPOSITORY_ROOT / ".github" / "scripts" / "public-api-schema-drift-issue.sh" +) +COMPARISON_JOB_ID = "public-api-schema-drift" +REPORT_JOB_ID = "report-public-api-schema-drift" +DRIFT_LABEL = "public-api-schema-drift" +RUN_ENVIRONMENT = { + "GITHUB_SERVER_URL": "https://github.com", + "GITHUB_REPOSITORY": "PolicyEngine/policyengine-sim-api", + "GITHUB_RUN_ID": "123456", +} +RUN_URL = "https://github.com/PolicyEngine/policyengine-sim-api/actions/runs/123456" +# Logs each call; for `issue list`, applies the script's own --jq filter to +# the open issues as GitHub returns them, as gh does; saves the body an issue +# or comment reads from standard input; fails the named call. +FAKE_GH = """\ +if [ "${1} ${2}" = "${FAKE_GH_FAILING_CALL:-}" ]; then exit 1; fi +case "${1} ${2}" in + "issue list") + while [ "${1}" != "--jq" ]; do shift; done + jq -r "${2}" <<<"${FAKE_OPEN_ISSUES_JSON}" + ;; + "issue create" | "issue comment") cat > "${FAKE_COMMAND_LOG}.body" ;; +esac +""" + + +def _jobs() -> dict: + return yaml.safe_load(DRIFT_WORKFLOW.read_text(encoding="utf-8"))["jobs"] + + +def test_a_second_job_reports_only_a_failed_comparison() -> None: + job = _jobs()[REPORT_JOB_ID] + + assert job["needs"] == COMPARISON_JOB_ID + assert job["if"] == "${{ failure() }}" + + +def test_only_the_reporting_job_may_write_issues() -> None: + workflow = yaml.safe_load(DRIFT_WORKFLOW.read_text(encoding="utf-8")) + + assert workflow["permissions"] == {"contents": "read"} + assert "permissions" not in workflow["jobs"][COMPARISON_JOB_ID] + assert workflow["jobs"][REPORT_JOB_ID]["permissions"] == { + "contents": "read", + "issues": "write", + } + + +def test_the_reporting_job_runs_only_sims_script_with_the_workflow_token() -> None: + """It never checks out or runs the Public API's code, and reads no secret.""" + job = _jobs()[REPORT_JOB_ID] + checkout, report = job["steps"] + + assert checkout == { + "uses": "actions/checkout@v6", + "with": {"persist-credentials": False}, + } + assert report["run"] == ".github/scripts/public-api-schema-drift-issue.sh" + assert report["env"] == { + "GH_TOKEN": "${{ github.token }}", + "GH_REPO": "${{ github.repository }}", + } + job_text = yaml.safe_dump(job) + assert "secrets." not in job_text + assert "policyengine-api" not in job_text + + +def _report( + tmp_path: Path, **environment: str +) -> tuple[subprocess.CompletedProcess[str], list[str], str | None]: + fake_bin = tmp_path / "bin" + fake_bin.mkdir() + _fake_command(fake_bin, "gh", FAKE_GH) + result, calls = _run(ISSUE_SCRIPT, [], tmp_path, **(RUN_ENVIRONMENT | environment)) + body_file = tmp_path / "commands.log.body" + body = body_file.read_text(encoding="utf-8") if body_file.exists() else None + return result, calls, body + + +def _calls_starting(calls: list[str], prefix: str) -> list[str]: + return [call for call in calls if call.startswith(f"gh|{prefix}")] + + +def test_a_first_failure_opens_an_issue_with_the_drift_label(tmp_path): + result, calls, body = _report(tmp_path, FAKE_OPEN_ISSUES_JSON="[]") + + assert result.returncode == 0, result.stderr + [label] = _calls_starting(calls, "label create") + assert label.startswith(f"gh|label create {DRIFT_LABEL} --force") + [created] = _calls_starting(calls, "issue create") + assert f"--label {DRIFT_LABEL}" in created + assert "--body-file -" in created + assert _calls_starting(calls, "issue comment") == [] + assert RUN_URL in body + assert "libs/policyengine-simulation-persistence/public-api-commit" in body + # Any failure of the comparison job reports, setup steps included. + assert "re-run" in body + + +def test_a_later_failure_comments_on_the_open_issue(tmp_path): + result, calls, body = _report( + tmp_path, FAKE_OPEN_ISSUES_JSON='[{"number": 42}, {"number": 7}]' + ) + + assert result.returncode == 0, result.stderr + [listed] = _calls_starting(calls, "issue list") + assert f"--label {DRIFT_LABEL}" in listed + assert "--state open" in listed + assert _calls_starting(calls, "issue comment") == [ + "gh|issue comment 42 --body-file -" + ] + assert _calls_starting(calls, "issue create") == [] + assert RUN_URL in body + + +def test_a_failed_github_request_fails_the_job(tmp_path): + result, calls, body = _report( + tmp_path, FAKE_OPEN_ISSUES_JSON="[]", FAKE_GH_FAILING_CALL="issue list" + ) + + assert result.returncode != 0 + assert _calls_starting(calls, "issue create") == [] + assert body is None + + +def test_the_report_needs_the_runs_address(tmp_path): + result, calls, _ = _report(tmp_path, GITHUB_RUN_ID="") + + assert result.returncode != 0 + assert calls == [] diff --git a/libs/policyengine-simulation-persistence/tests/test_public_api_schema_job.py b/libs/policyengine-simulation-persistence/tests/test_public_api_schema_job.py new file mode 100644 index 000000000..17153f30b --- /dev/null +++ b/libs/policyengine-simulation-persistence/tests/test_public_api_schema_job.py @@ -0,0 +1,242 @@ +"""The CI jobs that run the persistence library's database rules. + +Rule sim.persistence.models-match-public-api: the PR job and the weekly run +build the Public API's tables with its own v2 migration chain (at the pinned +commit, or at the head of its default branch), run the tests marked +``public_api_schema`` against them, and the PR job requires the pinned +commit to be on the Public API's default branch. A last test checks, for +every persistence rule, that its jobs run each test it names. +""" + +from __future__ import annotations + +import importlib +import shlex +from pathlib import Path + +import pytest +import tomllib +import yaml + +REPOSITORY_ROOT = Path(__file__).resolve().parents[3] +PR_WORKFLOW = REPOSITORY_ROOT / ".github" / "workflows" / "pr.yml" +REGISTRY = REPOSITORY_ROOT / "rules" / "registry.toml" +JOB_ID = "public-api-schema" +PIN_FILE = "libs/policyengine-simulation-persistence/public-api-commit" +DRIFT_WORKFLOW = ( + REPOSITORY_ROOT / ".github" / "workflows" / "public-api-schema-drift.yml" +) +DRIFT_JOB_ID = "public-api-schema-drift" +STACK_CONDITION = ( + "${{ !github.event.pull_request.stack || " + "github.event.pull_request.stack.base.ref == github.event.pull_request.base.ref }}" +) + + +def _jobs() -> dict: + return yaml.safe_load(PR_WORKFLOW.read_text(encoding="utf-8"))["jobs"] + + +def _step(job: dict, name: str) -> dict: + return next(step for step in job["steps"] if step.get("name") == name) + + +def test_the_job_checks_out_the_pinned_public_api_commit() -> None: + job = _jobs()[JOB_ID] + pin = _step(job, "Read the pinned Public API commit") + checkout = _step(job, "Check out the pinned Public API commit") + + assert pin["run"] == f".github/scripts/public-api-schema-commit.sh {PIN_FILE}" + assert checkout["uses"] == "actions/checkout@v6" + assert checkout["with"] == { + "repository": "PolicyEngine/policyengine-api", + "ref": f"${{{{ steps.{pin['id']}.outputs.commit }}}}", + "path": "public-api", + "persist-credentials": False, + } + + +def test_the_job_builds_the_reference_and_compares_without_skipping() -> None: + job = _jobs()[JOB_ID] + server = job["services"]["postgres"] + password = server["env"]["POSTGRES_PASSWORD"] + + assert server["image"] == "postgres:17" + # The Public API's disposable migration mode accepts only this database. + assert server["env"]["POSTGRES_DB"] == "policyengine_v2_alembic_test" + assert job["env"] == { + "SIMULATION_TEST_DATABASE_URL": ( + f"postgresql://postgres:{password}@127.0.0.1:5432/postgres" + ), + "PUBLIC_API_SCHEMA_DATABASE_URL": ( + f"postgresql://postgres:{password}@127.0.0.1:5432/" + "policyengine_v2_alembic_test" + ), + "SIMULATION_REQUIRE_DATABASE_TESTS": "1", + "SIMULATION_REQUIRE_PUBLIC_API_SCHEMA": "1", + } + names = [step.get("name") for step in job["steps"]] + build = _step(job, "Build the Public API's tables at the pinned commit") + compare = _step(job, "Compare sim's table models with the Public API's tables") + on_default_branch = _step( + job, "Require the pinned commit on the Public API's default branch" + ) + + assert build["run"] == ".github/scripts/public-api-schema-reference.sh public-api" + assert compare["working-directory"] == "libs/policyengine-simulation-persistence" + assert compare["run"] == "uv run --frozen pytest tests/ -v -m public_api_schema" + assert on_default_branch["run"] == ( + ".github/scripts/public-api-commit-on-default-branch.sh public-api master" + ) + assert ( + names.index(build["name"]) + < names.index(compare["name"]) + < names.index(on_default_branch["name"]) + ) + + +def test_the_job_reads_no_secret() -> None: + job_text = yaml.safe_dump(_jobs()[JOB_ID]) + + assert "secrets." not in job_text + assert _jobs()[JOB_ID]["permissions"] == {"contents": "read"} + + +def test_ci_complete_needs_the_job_at_the_bottom_of_a_stack() -> None: + jobs = _jobs() + + assert JOB_ID in jobs["ci-complete"]["needs"] + assert jobs[JOB_ID]["if"] == STACK_CONDITION + + +def _drift_workflow() -> dict: + return yaml.safe_load(DRIFT_WORKFLOW.read_text(encoding="utf-8")) + + +def test_a_weekly_run_compares_the_models_with_the_default_branch() -> None: + workflow = _drift_workflow() + # PyYAML reads the bare key `on` as True. + triggers = workflow[True] + job = workflow["jobs"][DRIFT_JOB_ID] + checkout = _step(job, "Check out the Public API's default branch") + compare = _step(job, "Compare sim's table models with the Public API's tables") + + assert [entry["cron"] for entry in triggers["schedule"]] == ["17 6 * * 1"] + assert "workflow_dispatch" in triggers + assert checkout["with"] == { + "repository": "PolicyEngine/policyengine-api", + "ref": "master", + "path": "public-api", + "persist-credentials": False, + } + assert compare["run"] == "uv run --frozen pytest tests/ -v -m public_api_schema" + assert job["env"] == _jobs()[JOB_ID]["env"] + assert job["services"] == _jobs()[JOB_ID]["services"] + + +def test_the_weekly_run_reads_no_secret() -> None: + workflow = _drift_workflow() + + assert "secrets." not in DRIFT_WORKFLOW.read_text(encoding="utf-8") + assert workflow["permissions"] == {"contents": "read"} + + +SKIPPING_MARKS = frozenset({"skip", "skipif", "xfail"}) +# The Simulation database job selects postgres but has no Public API +# database, so a schema test marked postgres could only skip there. +EXCLUDED_MARKERS = {"sim.persistence.models-match-public-api": {"postgres"}} + + +def _persistence_rules() -> list[dict]: + return [ + entry + for entry in tomllib.loads(REGISTRY.read_text(encoding="utf-8"))["rule"] + if entry["enforcer"].get("project") + == "libs/policyengine-simulation-persistence" + and entry["enforcer"]["kind"] == "pytest" + ] + + +def _selected_marker(invocation: str) -> str: + """Return the one marker a pytest invocation selects with ``-m``. + + Fail for an invocation that selects none, or an expression rather than + one marker: this test couldn't tell which tests such a job runs. + """ + arguments = shlex.split(invocation) + assert "-m" in arguments, f"{invocation!r} selects no marker" + marker = arguments[arguments.index("-m") + 1] + assert marker.isidentifier(), f"{invocation!r} selects {marker!r}, not one marker" + return marker + + +def _marks(node_id: str) -> set[str]: + """Return the names of a test's marks, its module's included.""" + module_path, test_name = node_id.split("::") + module = importlib.import_module(Path(module_path).stem) + module_marks = getattr(module, "pytestmark", []) + if not isinstance(module_marks, list): + module_marks = [module_marks] + test_marks = getattr(getattr(module, test_name), "pytestmark", []) + return {mark.name for mark in [*module_marks, *test_marks]} + + +def test_every_persistence_rule_test_runs_in_the_jobs_that_enforce_it() -> None: + """Each rule's jobs select tests by marker, which the rules guard can't read. + + So each test a persistence rule names, its proof included, must carry + every marker its jobs select, none that would send it to a job without + its database, and no mark that would let it skip or fail unnoticed. + """ + rules = _persistence_rules() + assert {rule["id"] for rule in rules} >= set(EXCLUDED_MARKERS) + for rule in rules: + # Every job of the rule selects its tests by one marker, so each + # job's tests are known. + selected = {_selected_marker(ci["invocation"]) for ci in rule["ci"]} + for node_id in [*rule["enforcer"]["nodes"], rule["proof"]["test"]]: + marks = _marks(node_id) + assert selected <= marks, (rule["id"], node_id) + assert not marks & EXCLUDED_MARKERS.get(rule["id"], set()), node_id + assert not marks & SKIPPING_MARKS, node_id + + +def test_no_checkout_leaves_a_token_while_the_public_api_code_runs() -> None: + """The jobs run the Public API's code from a pinned, perhaps unmerged commit.""" + jobs = [_jobs()[JOB_ID], _drift_workflow()["jobs"][DRIFT_JOB_ID]] + + for job in jobs: + checkouts = [ + step + for step in job["steps"] + if step.get("uses", "").startswith("actions/checkout@") + ] + assert len(checkouts) == 2 + for checkout in checkouts: + assert checkout["with"]["persist-credentials"] is False, checkout + + +def test_the_weekly_run_builds_the_reference_and_installs_like_the_pr_job() -> None: + pr_job = _jobs()[JOB_ID] + drift_job = _drift_workflow()["jobs"][DRIFT_JOB_ID] + build = _step(drift_job, "Build the Public API's tables at its default branch") + + assert build["run"] == ".github/scripts/public-api-schema-reference.sh public-api" + assert _step(drift_job, "Install dependencies") == _step( + pr_job, "Install dependencies" + ) + assert _step(drift_job, "Install uv") == _step(pr_job, "Install uv") + assert _step(drift_job, "Set up Python") == _step(pr_job, "Set up Python") + + +def test_a_rule_job_must_select_its_tests_by_one_marker() -> None: + """An invocation without a marker can't be checked, so it is refused.""" + assert _selected_marker("uv run --frozen pytest tests/ -v -m postgres") == ( + "postgres" + ) + for invocation in ( + "uv run --frozen pytest tests/", + 'uv run --frozen pytest tests/ -m "postgres and not slow"', + ): + with pytest.raises(AssertionError): + _selected_marker(invocation) diff --git a/libs/policyengine-simulation-persistence/tests/test_public_api_schema_scripts.py b/libs/policyengine-simulation-persistence/tests/test_public_api_schema_scripts.py new file mode 100644 index 000000000..d973f4464 --- /dev/null +++ b/libs/policyengine-simulation-persistence/tests/test_public_api_schema_scripts.py @@ -0,0 +1,239 @@ +"""The scripts the Public API schema job runs, with fake commands on PATH. + +``public-api-schema-commit.sh`` reads the pinned Public API commit, +``public-api-commit-on-default-branch.sh`` requires it to be on the Public +API's default branch, and ``public-api-schema-reference.sh`` builds the +Public API's tables at that commit and grants its declared privileges. +""" + +from __future__ import annotations + +import os +import subprocess +import textwrap +from pathlib import Path + +import pytest + +REPOSITORY_ROOT = Path(__file__).resolve().parents[3] +SCRIPTS = REPOSITORY_ROOT / ".github" / "scripts" +COMMIT_SCRIPT = SCRIPTS / "public-api-schema-commit.sh" +DEFAULT_BRANCH_SCRIPT = SCRIPTS / "public-api-commit-on-default-branch.sh" +REFERENCE_SCRIPT = SCRIPTS / "public-api-schema-reference.sh" +PINNED_COMMIT = "0123456789abcdef0123456789abcdef01234567" +DATABASE_URL = ( + "postgresql://postgres:simulation_ci@127.0.0.1:5432/policyengine_v2_alembic_test" +) + + +def _fake_command(directory: Path, name: str, body: str) -> None: + """Write an executable that logs its arguments and runs ``body``.""" + command = directory / name + command.write_text( + textwrap.dedent( + f"""\ + #!/usr/bin/env bash + set -euo pipefail + printf '%s|%s\\n' "{name}" "$*" >> "${{FAKE_COMMAND_LOG}}" + {body} + """ + ), + encoding="utf-8", + ) + command.chmod(0o755) + + +def _run(script: Path, arguments: list[str], tmp_path: Path, **environment): + fake_bin = tmp_path / "bin" + fake_bin.mkdir(exist_ok=True) + log = tmp_path / "commands.log" + log.touch() + command_environment = { + "PATH": f"{fake_bin}{os.pathsep}{os.environ['PATH']}", + "FAKE_COMMAND_LOG": str(log), + "HOME": str(tmp_path), + } | environment + result = subprocess.run( + ["bash", str(script), *arguments], + check=False, + capture_output=True, + text=True, + env=command_environment, + ) + return result, log.read_text(encoding="utf-8").splitlines() + + +# --- The pinned commit ---------------------------------------------------- + + +def test_the_pinned_commit_is_read_and_passed_to_later_steps(tmp_path): + pin = tmp_path / "public-api-commit" + pin.write_text(f"{PINNED_COMMIT}\n", encoding="utf-8") + github_output = tmp_path / "github-output" + + result, _ = _run( + COMMIT_SCRIPT, [str(pin)], tmp_path, GITHUB_OUTPUT=str(github_output) + ) + + assert result.returncode == 0, result.stderr + assert f"Pinned Public API commit: {PINNED_COMMIT}" in result.stdout + assert github_output.read_text(encoding="utf-8") == f"commit={PINNED_COMMIT}\n" + + +@pytest.mark.parametrize( + "content", + ["", "master\n", f"{PINNED_COMMIT[:12]}\n", f"{PINNED_COMMIT.upper()}\n"], + ids=["empty", "branch", "short", "upper-case"], +) +def test_a_pin_that_is_not_a_full_commit_fails(tmp_path, content): + pin = tmp_path / "public-api-commit" + pin.write_text(content, encoding="utf-8") + + result, _ = _run(COMMIT_SCRIPT, [str(pin)], tmp_path) + + assert result.returncode != 0 + assert "40 lowercase hexadecimal characters" in result.stderr + + +def test_the_repository_pins_a_full_commit(tmp_path): + result, _ = _run( + COMMIT_SCRIPT, + [ + str( + REPOSITORY_ROOT + / "libs" + / "policyengine-simulation-persistence" + / "public-api-commit" + ) + ], + tmp_path, + ) + + assert result.returncode == 0, result.stderr + + +# --- The default branch --------------------------------------------------- + + +def _fake_git(tmp_path, *, ancestor: bool) -> None: + fake_bin = tmp_path / "bin" + fake_bin.mkdir(exist_ok=True) + _fake_command( + fake_bin, + "git", + textwrap.dedent( + f"""\ + case "$*" in + *"rev-parse HEAD"*) printf '%s\\n' "{PINNED_COMMIT}" ;; + *"merge-base --is-ancestor"*) exit {0 if ancestor else 1} ;; + esac + """ + ), + ) + + +def test_a_commit_on_the_default_branch_passes(tmp_path): + _fake_git(tmp_path, ancestor=True) + + result, commands = _run(DEFAULT_BRANCH_SCRIPT, ["public-api", "master"], tmp_path) + + assert result.returncode == 0, result.stderr + assert "git|-C public-api fetch --quiet origin master" in commands + assert ( + f"git|-C public-api merge-base --is-ancestor {PINNED_COMMIT} FETCH_HEAD" + in commands + ) + + +def test_a_commit_not_yet_on_the_default_branch_fails_and_says_so(tmp_path): + _fake_git(tmp_path, ancestor=False) + + result, _ = _run(DEFAULT_BRANCH_SCRIPT, ["public-api", "master"], tmp_path) + + assert result.returncode == 1 + assert f"{PINNED_COMMIT} is not on the Public API's master branch" in ( + result.stderr + ) + + +# --- The reference database ----------------------------------------------- + + +def _fake_reference_commands(tmp_path, *, failing_step: str = "") -> None: + fake_bin = tmp_path / "bin" + fake_bin.mkdir(exist_ok=True) + _fake_command(fake_bin, "git", f"printf '%s\\n' '{PINNED_COMMIT}'") + _fake_command( + fake_bin, + "uv", + textwrap.dedent( + f"""\ + printf 'uv-env|%s|%s|%s|%s\\n' "${{V2_MIGRATION_DATABASE_URL:-}}" \\ + "${{V2_ALEMBIC_DISPOSABLE_TEST:-}}" "${{DEPLOYMENT_ENVIRONMENT:-}}" \\ + "${{UV_PYTHON:-}}" >> "${{FAKE_COMMAND_LOG}}" + case "$*" in + *"{failing_step}"*) [ -n "{failing_step}" ] && exit 3 ;; + esac + exit 0 + """ + ), + ) + + +def test_the_reference_migrates_the_public_api_and_grants_its_privileges(tmp_path): + checkout = tmp_path / "public-api" + checkout.mkdir() + github_env = tmp_path / "github-env" + _fake_reference_commands(tmp_path) + + result, commands = _run( + REFERENCE_SCRIPT, + [str(checkout)], + tmp_path, + PUBLIC_API_SCHEMA_DATABASE_URL=DATABASE_URL, + GITHUB_ENV=str(github_env), + ) + + assert result.returncode == 0, result.stderr + uv_calls = [line for line in commands if line.startswith("uv|")] + assert uv_calls == [ + "uv|sync --frozen --extra dev", + "uv|run --frozen --extra dev alembic -c alembic-v2.ini upgrade head", + "uv|run --frozen --extra dev python scripts/reconcile_v2_privileges.py " + "apply --create-missing-roles", + ] + psycopg_url = "postgresql+psycopg://" + DATABASE_URL.split("://", 1)[1] + # The Public API's lock installs on Python 3.12, as its own CI does. + assert f"uv-env|{psycopg_url}|1|ci|3.12" in commands + assert f"Public API commit: {PINNED_COMMIT}" in result.stdout + assert github_env.read_text(encoding="utf-8") == ( + f"PUBLIC_API_SCHEMA_COMMIT={PINNED_COMMIT}\n" + ) + + +@pytest.mark.parametrize("failing_step", ["upgrade head", "reconcile_v2_privileges"]) +def test_a_failed_reference_step_fails_the_job(tmp_path, failing_step): + checkout = tmp_path / "public-api" + checkout.mkdir() + _fake_reference_commands(tmp_path, failing_step=failing_step) + + result, _ = _run( + REFERENCE_SCRIPT, + [str(checkout)], + tmp_path, + PUBLIC_API_SCHEMA_DATABASE_URL=DATABASE_URL, + ) + + assert result.returncode == 3 + + +def test_the_reference_needs_a_database_to_migrate(tmp_path): + checkout = tmp_path / "public-api" + checkout.mkdir() + _fake_reference_commands(tmp_path) + + result, commands = _run(REFERENCE_SCRIPT, [str(checkout)], tmp_path) + + assert result.returncode != 0 + assert "PUBLIC_API_SCHEMA_DATABASE_URL" in result.stderr + assert not [line for line in commands if line.startswith("uv|")] diff --git a/libs/policyengine-simulation-persistence/tests/test_schema_comparison.py b/libs/policyengine-simulation-persistence/tests/test_schema_comparison.py new file mode 100644 index 000000000..d70b081e4 --- /dev/null +++ b/libs/policyengine-simulation-persistence/tests/test_schema_comparison.py @@ -0,0 +1,427 @@ +"""How sim's table models are compared with the Public API's tables. + +``schema_differences`` lists every way the models' tables differ from the +tables the Public API's migration chain builds. These tests need no database: +they compare descriptions built here, except the one marked ``postgres``, +which describes a table it creates. +""" + +from __future__ import annotations + +import pytest +from schema_comparison import ( + ColumnShape, + SchemaShape, + TableShape, + describe_tables, + foreign_key_targets, + foreign_keys_to, + schema_differences, + schema_from_json, + schema_to_json, +) +from sqlalchemy import create_engine, text +from sqlalchemy.engine import make_url + +STATUS_ENUM = "v2_simulation_status" + + +def _simulation_runs(**changes) -> SchemaShape: + """Return a simulation_runs table and its status enum, changed as given.""" + columns = { + "id": ColumnShape("uuid", nullable=False, default=None), + "simulation_id": ColumnShape("uuid", nullable=False, default=None), + "status": ColumnShape( + f"enum {STATUS_ENUM}", + nullable=False, + default=f"'pending'::{STATUS_ENUM}", + ), + "created_at": ColumnShape( + "timestamp with time zone", nullable=False, default="now()" + ), + "modal_call_id": ColumnShape( + "character varying(255)", nullable=True, default=None + ), + } + columns.update(changes.pop("columns", {})) + for removed in changes.pop("without_columns", ()): + del columns[removed] + constraints = { + "pk_simulation_runs": "PRIMARY KEY (id)", + "fk_simulation_runs_simulation_id_simulations": ( + "FOREIGN KEY (simulation_id) REFERENCES simulations(id) ON DELETE CASCADE" + ), + } + constraints.update(changes.pop("constraints", {})) + for removed in changes.pop("without_constraints", ()): + del constraints[removed] + indexes = { + "ix_simulation_runs_status_started_at": ( + "CREATE INDEX ix_simulation_runs_status_started_at ON " + "public.simulation_runs USING btree (status, created_at)" + ), + } + indexes.update(changes.pop("indexes", {})) + for removed in changes.pop("without_indexes", ()): + del indexes[removed] + enums = changes.pop( + "enums", {STATUS_ENUM: ("pending", "running", "succeeded", "failed")} + ) + assert not changes, changes + return SchemaShape( + tables={ + "simulation_runs": TableShape( + columns=columns, constraints=constraints, indexes=indexes + ) + }, + enums=enums, + ) + + +def test_each_foreign_key_is_read_with_the_table_it_names() -> None: + """PostgreSQL qualifies a table outside the search path and quotes odd names.""" + schema = _simulation_runs( + constraints={ + "fk_simulation_runs_dataset_id_datasets": ( + "FOREIGN KEY (dataset_id) REFERENCES datasets(id) ON DELETE RESTRICT" + ), + "fk_simulation_runs_user_id_users": ( + "FOREIGN KEY (user_id) REFERENCES auth.users(id)" + ), + "fk_simulation_runs_owner_id_owners": ( + 'FOREIGN KEY (owner_id) REFERENCES "Owners"(id) ON DELETE SET NULL' + ), + "fk_simulation_runs_window": ( + "FOREIGN KEY (window_id, simulation_id) REFERENCES " + "budget_window_runs(id, report_id) ON DELETE CASCADE" + ), + "ck_simulation_runs_year": "CHECK (((year >= 1900) AND (year <= 2200)))", + } + ) + + assert foreign_key_targets(schema) == { + ("simulation_runs", "fk_simulation_runs_simulation_id_simulations"): ( + "simulations" + ), + ("simulation_runs", "fk_simulation_runs_dataset_id_datasets"): "datasets", + ("simulation_runs", "fk_simulation_runs_user_id_users"): "auth.users", + ("simulation_runs", "fk_simulation_runs_owner_id_owners"): "Owners", + ("simulation_runs", "fk_simulation_runs_window"): "budget_window_runs", + } + + +def test_quoted_names_are_read_whole() -> None: + """A quoted name may hold a dot, a parenthesis or a doubled quote.""" + schema = _simulation_runs( + constraints={ + "fk_simulation_runs_owner": ( + 'FOREIGN KEY (owner_id) REFERENCES "Auth"."Users"(id)' + ), + "fk_simulation_runs_odd": ( + 'FOREIGN KEY (odd_id) REFERENCES "odd.name(1)"(id)' + ), + "fk_simulation_runs_quote": ( + 'FOREIGN KEY ("quote id") REFERENCES "say ""hi"""(id)' + ), + } + ) + + targets = foreign_key_targets(schema) + + assert targets[("simulation_runs", "fk_simulation_runs_owner")] == "Auth.Users" + assert targets[("simulation_runs", "fk_simulation_runs_odd")] == "odd.name(1)" + assert targets[("simulation_runs", "fk_simulation_runs_quote")] == 'say "hi"' + + +def test_keys_to_the_given_tables_are_found_on_every_table() -> None: + """A key's name is unique only within its table, so each is kept by both.""" + simulation_runs = _simulation_runs( + constraints={ + "fk_policy": "FOREIGN KEY (policy_id) REFERENCES policies(id)", + "fk_user": "FOREIGN KEY (user_id) REFERENCES auth.users(id)", + } + ) + household_jobs = TableShape( + columns={"id": ColumnShape("uuid", nullable=False, default=None)}, + constraints={ + "pk_household_jobs": "PRIMARY KEY (id)", + "fk_policy": ( + "FOREIGN KEY (policy_id) REFERENCES policies(id) ON DELETE SET NULL" + ), + }, + indexes={}, + ) + schema = SchemaShape( + tables={**simulation_runs.tables, "household_jobs": household_jobs}, + enums=simulation_runs.enums, + ) + + # auth.users is another schema's table, and simulations isn't asked for. + assert foreign_keys_to(schema, {"policies", "users"}) == { + ("simulation_runs", "fk_policy"): "policies", + ("household_jobs", "fk_policy"): "policies", + } + + +def test_a_foreign_key_whose_table_cannot_be_read_is_refused() -> None: + """A key left uncounted would hide from a check of which keys exist.""" + schema = _simulation_runs( + constraints={"fk_simulation_runs_unreadable": "FOREIGN KEY (x) REFERENCES"} + ) + + with pytest.raises(ValueError, match="fk_simulation_runs_unreadable"): + foreign_key_targets(schema) + + +def test_identical_tables_have_no_differences() -> None: + assert schema_differences(_simulation_runs(), _simulation_runs()) == [] + + +@pytest.mark.parametrize( + ("models", "real", "difference"), + [ + ( + _simulation_runs( + columns={ + "status": ColumnShape( + "character varying(32)", + nullable=False, + default=f"'pending'::{STATUS_ENUM}", + ) + } + ), + _simulation_runs(), + "simulation_runs.status: the model is character varying(32), the " + "Public API's is enum v2_simulation_status", + ), + ( + _simulation_runs( + columns={ + "modal_call_id": ColumnShape( + "character varying(255)", nullable=False, default=None + ) + } + ), + _simulation_runs(), + "simulation_runs.modal_call_id: the model refuses null, the Public " + "API's allows it", + ), + ( + _simulation_runs( + columns={ + "created_at": ColumnShape( + "timestamp with time zone", nullable=False, default=None + ) + } + ), + _simulation_runs(), + "simulation_runs.created_at: the model has no default, the Public " + "API's defaults to now()", + ), + ( + _simulation_runs(without_columns=["modal_call_id"]), + _simulation_runs(), + "simulation_runs.modal_call_id: the model has no such column", + ), + ( + _simulation_runs(), + _simulation_runs(without_columns=["modal_call_id"]), + "simulation_runs.modal_call_id: the Public API's table has no such column", + ), + ( + _simulation_runs( + constraints={ + "fk_simulation_runs_simulation_id_simulations": ( + "FOREIGN KEY (simulation_id) REFERENCES simulations(id)" + ) + } + ), + _simulation_runs(), + "simulation_runs: fk_simulation_runs_simulation_id_simulations is " + "FOREIGN KEY (simulation_id) REFERENCES simulations(id) in the " + "model, FOREIGN KEY (simulation_id) REFERENCES simulations(id) ON " + "DELETE CASCADE in the Public API", + ), + ( + _simulation_runs( + without_constraints=["fk_simulation_runs_simulation_id_simulations"] + ), + _simulation_runs(), + "simulation_runs: the model has no constraint " + "fk_simulation_runs_simulation_id_simulations (FOREIGN KEY " + "(simulation_id) REFERENCES simulations(id) ON DELETE CASCADE)", + ), + ( + _simulation_runs(), + _simulation_runs( + without_constraints=["fk_simulation_runs_simulation_id_simulations"] + ), + "simulation_runs: the Public API's table has no constraint " + "fk_simulation_runs_simulation_id_simulations (FOREIGN KEY " + "(simulation_id) REFERENCES simulations(id) ON DELETE CASCADE)", + ), + ( + _simulation_runs(without_indexes=["ix_simulation_runs_status_started_at"]), + _simulation_runs(), + "simulation_runs: the model has no index " + "ix_simulation_runs_status_started_at (CREATE INDEX " + "ix_simulation_runs_status_started_at ON public.simulation_runs USING " + "btree (status, created_at))", + ), + ( + _simulation_runs(), + _simulation_runs(enums={STATUS_ENUM: ("pending", "running", "failed")}), + "enum v2_simulation_status: the model has (pending, running, " + "succeeded, failed), the Public API has (pending, running, failed)", + ), + ( + _simulation_runs(), + _simulation_runs(enums={}), + "enum v2_simulation_status: the Public API has no such enum in its " + "public schema", + ), + ( + _simulation_runs(), + SchemaShape(tables={}, enums=_simulation_runs().enums), + "simulation_runs: the Public API has no such table", + ), + ], + ids=[ + "type", + "nullability", + "default", + "column-only-in-the-public-api", + "column-only-in-the-model", + "constraint-definition", + "constraint-only-in-the-public-api", + "constraint-only-in-the-model", + "index-only-in-the-public-api", + "enum-values", + "missing-enum", + "missing-table", + ], +) +def test_each_difference_is_reported(models, real, difference) -> None: + assert schema_differences(models, real) == [difference] + + +def test_an_index_with_another_definition_is_reported() -> None: + reordered = ( + "CREATE INDEX ix_simulation_runs_status_started_at ON " + "public.simulation_runs USING btree (created_at, status)" + ) + models = _simulation_runs( + indexes={"ix_simulation_runs_status_started_at": reordered} + ) + + assert schema_differences(models, _simulation_runs()) == [ + f"simulation_runs: ix_simulation_runs_status_started_at is {reordered} in " + "the model, CREATE INDEX ix_simulation_runs_status_started_at ON " + "public.simulation_runs USING btree (status, created_at) in the Public API", + ] + + +def test_an_index_only_the_model_has_is_reported() -> None: + real = _simulation_runs(without_indexes=["ix_simulation_runs_status_started_at"]) + + assert schema_differences(_simulation_runs(), real) == [ + "simulation_runs: the Public API's table has no index " + "ix_simulation_runs_status_started_at (CREATE INDEX " + "ix_simulation_runs_status_started_at ON public.simulation_runs USING " + "btree (status, created_at))", + ] + + +def test_tables_the_models_leave_out_are_not_compared() -> None: + real = _simulation_runs() + real.tables["users"] = TableShape( + columns={"id": ColumnShape("uuid", nullable=False, default=None)}, + constraints={}, + indexes={}, + ) + + assert schema_differences(_simulation_runs(), real) == [] + + +@pytest.mark.postgres +def test_the_catalog_describes_each_part_of_a_table(disposable_database) -> None: + engine = create_engine( + make_url(disposable_database.superuser_url).set( + drivername="postgresql+psycopg" + ), + isolation_level="AUTOCOMMIT", + ) + statements = [ + "CREATE TYPE v2_simulation_status AS ENUM ('pending', 'running')", + "CREATE TABLE simulations (id uuid CONSTRAINT pk_simulations PRIMARY KEY)", + "CREATE TABLE simulation_runs (" + "id uuid CONSTRAINT pk_simulation_runs PRIMARY KEY, " + "simulation_id uuid NOT NULL CONSTRAINT " + "fk_simulation_runs_simulation_id_simulations REFERENCES simulations (id) " + "ON DELETE CASCADE, " + "status v2_simulation_status NOT NULL DEFAULT 'pending', " + "attempt integer NOT NULL CONSTRAINT ck_simulation_runs_attempt " + "CHECK (attempt > 0), " + "created_at timestamp with time zone NOT NULL DEFAULT now(), " + "modal_call_id varchar(255), " + "CONSTRAINT uq_simulation_runs_simulation_attempt " + "UNIQUE (simulation_id, attempt))", + "CREATE INDEX ix_simulation_runs_status_created_at " + "ON simulation_runs (status, created_at)", + ] + try: + with engine.connect() as connection: + for statement in statements: + connection.execute(text(statement)) + described = describe_tables(connection, ["simulation_runs", "absent"]) + finally: + engine.dispose() + + assert described == SchemaShape( + tables={ + "simulation_runs": TableShape( + columns={ + "id": ColumnShape("uuid", nullable=False, default=None), + "simulation_id": ColumnShape("uuid", nullable=False, default=None), + "status": ColumnShape( + "enum v2_simulation_status", + nullable=False, + default="'pending'::v2_simulation_status", + ), + "attempt": ColumnShape("integer", nullable=False, default=None), + "created_at": ColumnShape( + "timestamp with time zone", nullable=False, default="now()" + ), + "modal_call_id": ColumnShape( + "character varying(255)", nullable=True, default=None + ), + }, + constraints={ + "ck_simulation_runs_attempt": "CHECK ((attempt > 0))", + "fk_simulation_runs_simulation_id_simulations": ( + "FOREIGN KEY (simulation_id) REFERENCES simulations(id) " + "ON DELETE CASCADE" + ), + "pk_simulation_runs": "PRIMARY KEY (id)", + "uq_simulation_runs_simulation_attempt": ( + "UNIQUE (simulation_id, attempt)" + ), + }, + indexes={ + "ix_simulation_runs_status_created_at": ( + "CREATE INDEX ix_simulation_runs_status_created_at ON " + "public.simulation_runs USING btree (status, created_at)" + ), + }, + ) + }, + enums={"v2_simulation_status": ("pending", "running")}, + ) + + +def test_a_description_round_trips_through_json(tmp_path) -> None: + described = _simulation_runs() + path = tmp_path / "schema.json" + path.write_text(schema_to_json(described), encoding="utf-8") + + assert schema_from_json(path) == described diff --git a/rules/fixtures/sim.persistence.models-match-public-api/bad_models.json b/rules/fixtures/sim.persistence.models-match-public-api/bad_models.json new file mode 100644 index 000000000..eafd6da7b --- /dev/null +++ b/rules/fixtures/sim.persistence.models-match-public-api/bad_models.json @@ -0,0 +1,100 @@ +{ + "enums": {}, + "tables": { + "simulation_runs": { + "columns": { + "completed_at": { + "column_type": "timestamp with time zone", + "default": null, + "nullable": true + }, + "country_package_version": { + "column_type": "character varying(128)", + "default": null, + "nullable": false + }, + "created_at": { + "column_type": "timestamp with time zone", + "default": "now()", + "nullable": false + }, + "dataset_id": { + "column_type": "uuid", + "default": null, + "nullable": true + }, + "error_message": { + "column_type": "character varying", + "default": null, + "nullable": true + }, + "household_result": { + "column_type": "json", + "default": null, + "nullable": true + }, + "id": { + "column_type": "uuid", + "default": null, + "nullable": false + }, + "output_sha256": { + "column_type": "character varying(64)", + "default": null, + "nullable": true + }, + "output_uri": { + "column_type": "character varying(2048)", + "default": null, + "nullable": true + }, + "policyengine_version": { + "column_type": "character varying(128)", + "default": null, + "nullable": false + }, + "region_id": { + "column_type": "uuid", + "default": null, + "nullable": true + }, + "simulation_id": { + "column_type": "uuid", + "default": null, + "nullable": false + }, + "started_at": { + "column_type": "timestamp with time zone", + "default": null, + "nullable": true + }, + "status": { + "column_type": "character varying(32)", + "default": null, + "nullable": false + }, + "tax_benefit_model_version_id": { + "column_type": "uuid", + "default": null, + "nullable": false + }, + "updated_at": { + "column_type": "timestamp with time zone", + "default": "now()", + "nullable": false + } + }, + "constraints": { + "ck_simulation_runs_output_pair": "CHECK (((output_uri IS NULL) = (output_sha256 IS NULL)))", + "ck_simulation_runs_output_sha256_length": "CHECK (((output_sha256 IS NULL) OR (length((output_sha256)::text) = 64)))", + "ck_simulation_runs_terminal_completion": "CHECK (((status <> ALL (ARRAY['succeeded'::v2_simulation_status, 'failed'::v2_simulation_status])) OR (completed_at IS NOT NULL)))", + "fk_simulation_runs_simulation_id_simulations": "FOREIGN KEY (simulation_id) REFERENCES simulations(id) ON DELETE CASCADE", + "pk_simulation_runs": "PRIMARY KEY (id)" + }, + "indexes": { + "ix_simulation_runs_current_output": "CREATE INDEX ix_simulation_runs_current_output ON public.simulation_runs USING btree (simulation_id, status, country_package_version, policyengine_version, completed_at, id)", + "ix_simulation_runs_status_started_at": "CREATE INDEX ix_simulation_runs_status_started_at ON public.simulation_runs USING btree (status, started_at)" + } + } + } +} diff --git a/rules/fixtures/sim.persistence.models-match-public-api/good_models.json b/rules/fixtures/sim.persistence.models-match-public-api/good_models.json new file mode 100644 index 000000000..4cfc0fd9e --- /dev/null +++ b/rules/fixtures/sim.persistence.models-match-public-api/good_models.json @@ -0,0 +1,112 @@ +{ + "enums": { + "v2_simulation_status": [ + "pending", + "running", + "succeeded", + "failed" + ] + }, + "tables": { + "simulation_runs": { + "columns": { + "completed_at": { + "column_type": "timestamp with time zone", + "default": null, + "nullable": true + }, + "country_package_version": { + "column_type": "character varying(128)", + "default": null, + "nullable": false + }, + "created_at": { + "column_type": "timestamp with time zone", + "default": "now()", + "nullable": false + }, + "dataset_id": { + "column_type": "uuid", + "default": null, + "nullable": true + }, + "error_message": { + "column_type": "character varying", + "default": null, + "nullable": true + }, + "household_result": { + "column_type": "json", + "default": null, + "nullable": true + }, + "id": { + "column_type": "uuid", + "default": null, + "nullable": false + }, + "modal_call_id": { + "column_type": "character varying(255)", + "default": null, + "nullable": true + }, + "output_sha256": { + "column_type": "character varying(64)", + "default": null, + "nullable": true + }, + "output_uri": { + "column_type": "character varying(2048)", + "default": null, + "nullable": true + }, + "policyengine_version": { + "column_type": "character varying(128)", + "default": null, + "nullable": false + }, + "region_id": { + "column_type": "uuid", + "default": null, + "nullable": true + }, + "simulation_id": { + "column_type": "uuid", + "default": null, + "nullable": false + }, + "started_at": { + "column_type": "timestamp with time zone", + "default": null, + "nullable": true + }, + "status": { + "column_type": "enum v2_simulation_status", + "default": null, + "nullable": false + }, + "tax_benefit_model_version_id": { + "column_type": "uuid", + "default": null, + "nullable": false + }, + "updated_at": { + "column_type": "timestamp with time zone", + "default": "now()", + "nullable": false + } + }, + "constraints": { + "ck_simulation_runs_output_pair": "CHECK (((output_uri IS NULL) = (output_sha256 IS NULL)))", + "ck_simulation_runs_output_sha256_length": "CHECK (((output_sha256 IS NULL) OR (length((output_sha256)::text) = 64)))", + "ck_simulation_runs_terminal_completion": "CHECK (((status <> ALL (ARRAY['succeeded'::v2_simulation_status, 'failed'::v2_simulation_status])) OR (completed_at IS NOT NULL)))", + "fk_simulation_runs_simulation_id_simulations": "FOREIGN KEY (simulation_id) REFERENCES simulations(id) ON DELETE CASCADE", + "pk_simulation_runs": "PRIMARY KEY (id)" + }, + "indexes": { + "ix_simulation_runs_current_output": "CREATE INDEX ix_simulation_runs_current_output ON public.simulation_runs USING btree (simulation_id, status, country_package_version, policyengine_version, completed_at, id)", + "ix_simulation_runs_status_started_at": "CREATE INDEX ix_simulation_runs_status_started_at ON public.simulation_runs USING btree (status, started_at)" + } + } + } +} diff --git a/rules/fixtures/sim.persistence.models-match-public-api/public_api_schema.json b/rules/fixtures/sim.persistence.models-match-public-api/public_api_schema.json new file mode 100644 index 000000000..4cfc0fd9e --- /dev/null +++ b/rules/fixtures/sim.persistence.models-match-public-api/public_api_schema.json @@ -0,0 +1,112 @@ +{ + "enums": { + "v2_simulation_status": [ + "pending", + "running", + "succeeded", + "failed" + ] + }, + "tables": { + "simulation_runs": { + "columns": { + "completed_at": { + "column_type": "timestamp with time zone", + "default": null, + "nullable": true + }, + "country_package_version": { + "column_type": "character varying(128)", + "default": null, + "nullable": false + }, + "created_at": { + "column_type": "timestamp with time zone", + "default": "now()", + "nullable": false + }, + "dataset_id": { + "column_type": "uuid", + "default": null, + "nullable": true + }, + "error_message": { + "column_type": "character varying", + "default": null, + "nullable": true + }, + "household_result": { + "column_type": "json", + "default": null, + "nullable": true + }, + "id": { + "column_type": "uuid", + "default": null, + "nullable": false + }, + "modal_call_id": { + "column_type": "character varying(255)", + "default": null, + "nullable": true + }, + "output_sha256": { + "column_type": "character varying(64)", + "default": null, + "nullable": true + }, + "output_uri": { + "column_type": "character varying(2048)", + "default": null, + "nullable": true + }, + "policyengine_version": { + "column_type": "character varying(128)", + "default": null, + "nullable": false + }, + "region_id": { + "column_type": "uuid", + "default": null, + "nullable": true + }, + "simulation_id": { + "column_type": "uuid", + "default": null, + "nullable": false + }, + "started_at": { + "column_type": "timestamp with time zone", + "default": null, + "nullable": true + }, + "status": { + "column_type": "enum v2_simulation_status", + "default": null, + "nullable": false + }, + "tax_benefit_model_version_id": { + "column_type": "uuid", + "default": null, + "nullable": false + }, + "updated_at": { + "column_type": "timestamp with time zone", + "default": "now()", + "nullable": false + } + }, + "constraints": { + "ck_simulation_runs_output_pair": "CHECK (((output_uri IS NULL) = (output_sha256 IS NULL)))", + "ck_simulation_runs_output_sha256_length": "CHECK (((output_sha256 IS NULL) OR (length((output_sha256)::text) = 64)))", + "ck_simulation_runs_terminal_completion": "CHECK (((status <> ALL (ARRAY['succeeded'::v2_simulation_status, 'failed'::v2_simulation_status])) OR (completed_at IS NOT NULL)))", + "fk_simulation_runs_simulation_id_simulations": "FOREIGN KEY (simulation_id) REFERENCES simulations(id) ON DELETE CASCADE", + "pk_simulation_runs": "PRIMARY KEY (id)" + }, + "indexes": { + "ix_simulation_runs_current_output": "CREATE INDEX ix_simulation_runs_current_output ON public.simulation_runs USING btree (simulation_id, status, country_package_version, policyengine_version, completed_at, id)", + "ix_simulation_runs_status_started_at": "CREATE INDEX ix_simulation_runs_status_started_at ON public.simulation_runs USING btree (status, started_at)" + } + } + } +} diff --git a/rules/registry.toml b/rules/registry.toml index b6f5f8cb4..f2aa3725f 100644 --- a/rules/registry.toml +++ b/rules/registry.toml @@ -170,6 +170,43 @@ good_fixture = "rules/fixtures/sim.persistence.missing-privileges-reported/good_ project = "libs/policyengine-simulation-persistence" test = "tests/test_access.py::test_rule_proof_missing_privileges_reported" +[[rule]] +id = "sim.persistence.models-match-public-api" +category = "persistence" +statement = "Sim's models of the Public API's tables match the tables the Public API's v2 migration chain builds at the pinned Public API commit, which is on the Public API's default branch: every column's type, nullability and default, every constraint and index by name and definition, and every enum's values; and the simulation service's role, with the privileges the Public API's manifest declares, writes every modelled table." +rationale = "libs/policyengine-simulation-persistence/README.md#checking-the-models-against-the-public-api" +scope = [ + "libs/policyengine-simulation-persistence/src/policyengine_simulation_persistence/tables.py", + "libs/policyengine-simulation-persistence/public-api-commit", + ".github/scripts/public-api-schema-commit.sh", + ".github/scripts/public-api-schema-reference.sh", + ".github/scripts/public-api-commit-on-default-branch.sh", +] +tier = "mechanical" +status = "enforced" +autofix = false +[rule.enforcer] +kind = "pytest" +project = "libs/policyengine-simulation-persistence" +nodes = [ + "tests/test_public_api_schema.py::test_the_models_match_the_public_api_tables", + "tests/test_service_writes.py::test_the_service_role_holds_every_privilege_the_service_needs", + "tests/test_service_writes.py::test_the_service_role_creates_and_reads_a_row_in_every_table_it_writes", +] +[[rule.ci]] +workflow = ".github/workflows/pr.yml" +job = "public-api-schema" +invocation = "uv run --frozen pytest tests/ -v -m public_api_schema" +[[rule.ci]] +workflow = ".github/workflows/public-api-schema-drift.yml" +job = "public-api-schema-drift" +invocation = "uv run --frozen pytest tests/ -v -m public_api_schema" +[rule.proof] +bad_fixture = "rules/fixtures/sim.persistence.models-match-public-api/bad_models.json" +good_fixture = "rules/fixtures/sim.persistence.models-match-public-api/good_models.json" +project = "libs/policyengine-simulation-persistence" +test = "tests/test_public_api_schema.py::test_rule_proof_models_match_public_api" + # --- Stage 12 persistence -------------------------------------------------- [[rule]]