diff --git a/.github/workflows/v2-integration-check.yml b/.github/workflows/v2-integration-check.yml index 3f402ae80..7959a83c2 100644 --- a/.github/workflows/v2-integration-check.yml +++ b/.github/workflows/v2-integration-check.yml @@ -82,7 +82,7 @@ jobs: - name: Test v2 policy persistence and immediate v1 mirroring run: uv run coverage run -a --branch -m pytest -q tests/integration/test_v2_policy_persistence.py tests/integration/test_v1_policy_dual_write.py tests/integration/test_mysql_policy_dual_write.py tests/integration/test_v2_user_policy_mirroring.py tests/integration/test_v1_user_policy_dual_write.py - name: Test signed-in identity provisioning and user associations - run: uv run coverage run -a --branch -m pytest -q tests/integration/test_v2_user_identity_persistence.py + run: uv run coverage run -a --branch -m pytest -q tests/integration/test_v2_user_identity_persistence.py tests/integration/test_v2_saved_policy_view.py - name: Test comparison-run persistence and privilege reconciliation as the managed roles run: uv run coverage run -a --branch -m pytest -q tests/integration/test_v2_stage12_comparison_persistence.py tests/integration/test_v2_privilege_reconciliation.py - name: Test v2 household persistence and immediate v1 copying diff --git a/changelog.d/v2plan-1f-122-rename-user-policies.changed.md b/changelog.d/v2plan-1f-122-rename-user-policies.changed.md new file mode 100644 index 000000000..9c9a77ad8 --- /dev/null +++ b/changelog.d/v2plan-1f-122-rename-user-policies.changed.md @@ -0,0 +1 @@ +The v2 table user_policies is renamed to user_policy_associations, matching the other user-association tables; its constraints and convention-named indexes take the new name. Saved policies, their v1 mappings and the reports that name them are kept. A temporary view named user_policies, with the runtime role's privileges, lets the release from before the rename keep reading and writing saved policies during the deploy and after a rollback; a later change removes it. The privilege reconciliation now finds, grants and verifies privileges on views. The route /v2/user-policies, the user_policy_id columns, legacy_user_policy_mappings and the v1 Cloud SQL table user_policies are unchanged. diff --git a/docs/engineering/skills/alembic-migrations.md b/docs/engineering/skills/alembic-migrations.md index d4a043d91..093358e22 100644 --- a/docs/engineering/skills/alembic-migrations.md +++ b/docs/engineering/skills/alembic-migrations.md @@ -155,8 +155,10 @@ check the declarations: `apply` grants each declared privilege a role lacks. It never revokes: it reports privileges the file doesn't declare, and the projects' default -privileges, in the job summary. It grants nothing when a declared role or -table is missing. The roles themselves are provisioned outside the +privileges, in the job summary. It grants nothing when a declared role, +table or view is missing; a declared table must exist as a table and a +temporary view (`TEMPORARY_VIEW_TABLES`) as a view, so a relation of the other +kind counts as missing. The roles themselves are provisioned outside the repository; `--create-missing-roles` is accepted only for the disposable test database. @@ -179,6 +181,7 @@ When a pull request adds or changes a v2 table, or a connector's use of one: uv run python scripts/reconcile_v2_privileges.py apply --create-missing-roles uv run python scripts/reconcile_v2_privileges.py verify uv run pytest -q tests/integration/test_v2_user_identity_persistence.py \ + tests/integration/test_v2_saved_policy_view.py \ tests/integration/test_v2_stage12_comparison_persistence.py \ tests/integration/test_v2_privilege_reconciliation.py ``` @@ -294,3 +297,48 @@ last dependent table is dropped. Its only post-generation correction drops the nine generated `v2_*` enum types at the end of the baseline downgrade. The v2 Postgres lifecycle test covers empty upgrade, downgrade to base, and re-upgrade so stale enum types cannot make the generated baseline non-reversible. + +### API v2 saved-policy table rename + +Revision `fa7fa1035444` was autogenerated after renaming the `UserPolicy` +model's table from `user_policies` to `user_policy_associations`, with its +constraints and convention-named indexes. Autogeneration cannot detect a +rename: it created an empty `user_policy_associations` table and dropped +`user_policies`, which would delete every saved policy, and its copies of the +three composite indexes collided with the old table's indexes of the same +names. Its only post-generation correction replaces that pair with +`op.rename_table` and, for each constraint the generated table named after +`user_policy_associations` (primary key, unique, three checks and two foreign +keys), drops it under its old name and creates it under the generated name +with the generated definition. The generated drops and re-creations of the +two convention-named indexes and of the three foreign keys that refer to the +table are kept, ordered around the rename. The downgrade is the exact reverse. +The v2 Postgres lifecycle test stores a saved policy with a v1 mapping and a +report that names it, and checks that the rows, the references, the +cascades, a table grant and every constraint and index name survive the +upgrade and the downgrade. + +The revision also holds the only approved hand-written exception in the v2 +chain, approved by a maintainer on 2026-10-22: after the rename it creates a +view named `user_policies` with every column of `user_policy_associations`, +and the downgrade drops the view before renaming the table back. The Public +API release still serving during a deploy, or after a rollback to it, names +the table `user_policies`; a single-table view is automatically updatable, so +that release keeps reading and writing saved policies through it. The +privilege manifest declares the view with the table's privileges +(`TEMPORARY_VIEW_TABLES` in `policyengine_api/data/v2/privileges.py`), and the +privilege reconciliation grants and verifies privileges on views as on tables. +Autogeneration does not reflect views, so `alembic check` and the head schema +validation ignore it. A later revision drops the view once no deployable +release names `user_policies`; until then, a revision cannot drop or change +the type of a `user_policy_associations` column the view shows without +dropping the view first. + +Tests: `test_alembic_v2.py` allows exactly these two statements and no other +`op.execute` in any v2 revision. The lifecycle test checks the view's columns +after the upgrade and its absence after the downgrade, and runs the previous +revision's inserts, reads, updates and deletes through it. +`test_v2_saved_policy_view.py` runs today's saved-policy services, the +sign-in merge, report creation and the Stage 10 saved-policy copy as the +runtime role with the table named `user_policies`, so every statement goes +through the view. diff --git a/docs/migration/stage-12a-user-login.md b/docs/migration/stage-12a-user-login.md index 3fc391392..ed952e189 100644 --- a/docs/migration/stage-12a-user-login.md +++ b/docs/migration/stage-12a-user-login.md @@ -126,7 +126,7 @@ unauthenticated: a standard user's row is never locked. | State | Outcome | What happens | | --- | --- | --- | | The identity has no user yet; the browser's unauthenticated user exists | `converted` | The `user_identities` row (issuer, subject, verified email) is inserted pointing at the browser's `users.id`; the user becomes `standard` with `primary_country = country_id`. The ID is unchanged, so every association stays owned. | -| The identity already belongs to standard user S; the unauthenticated user exists | `merged` | Every row of `user_policies`, `user_household_associations`, `user_simulation_associations`, and `user_report_associations` owned by the unauthenticated user moves to S with its ID, contents, and timestamps unchanged; then the unauthenticated user is deleted. | +| The identity already belongs to standard user S; the unauthenticated user exists | `merged` | Every row of `user_policy_associations`, `user_household_associations`, `user_simulation_associations`, and `user_report_associations` owned by the unauthenticated user moves to S with its ID, contents, and timestamps unchanged; then the unauthenticated user is deleted. | | No unauthenticated user with that ID | `none` | The identity's user is resolved, or provisioned, as on any request. | | The ID names a standard user (already converted, or anyone else's) | `none` | That user is never touched; the identity's user is resolved as on any request. | @@ -194,8 +194,8 @@ Concurrency on Postgres: `legacy_household_id`; with `legacy_household_id`, both are stored, and it must be the copy of that v1 household when one exists. - `user-simulations` and `user-reports` accept only legacy IDs in 12A. -- **Stage 10 mirror rows.** A `user_policies` row with a - `legacy_user_policy_mappings` entry returns 409 on PATCH and DELETE. The +- **Stage 10 mirror rows.** A saved-policy row (`user_policy_associations`) + with a `legacy_user_policy_mappings` entry returns 409 on PATCH and DELETE. The mirror projection now also records `legacy_policy_id`. - **Request bounds.** Every `/v2/me*` and `/v2/user-*` write rejects a body over 256 KiB with 413. FastAPI reads and parses the whole JSON body before @@ -324,7 +324,16 @@ from the final SQLModel metadata. It has no post-generation corrections. "Rollback"). The models drop the `user_associations` relationships from v2 simulations and -reports. `V2_ALEMBIC_HEAD_REVISION` is `fbb7488ebe8a`. +reports. `V2_ALEMBIC_HEAD_REVISION` was `fbb7488ebe8a` when Stage 12A shipped. + +Revision `fa7fa1035444` later renamed `user_policies` to +`user_policy_associations`, with its constraints and its convention-named +indexes, matching the other association tables; the composite listing +indexes keep their `ix_user_policies_*` names. The route `/v2/user-policies`, +the `user_policy_id` columns and `legacy_user_policy_mappings` are unchanged. +The bullet points above describe revision `fbb7488ebe8a`, when the table was +still named `user_policies`. "Schema compatibility" below says what the +rename means for earlier Public API releases. ### Reshape safety @@ -347,15 +356,19 @@ and production and confirm both dormant tables are empty. ### Rollback -Roll forward whenever possible. The schema lets the previous Public API -revision run (see "Schema compatibility" below), but that revision serves -`/v2/user-policies` and `/v2/user-households` without authentication: any -caller can list a user's saved items by `user_id`, read, rename or delete one -by record ID, and create rows for any user. Rolling the application back -would therefore hand users' saved work to anyone. +Roll forward whenever possible. Since revision `fa7fa1035444` (see +"Schema compatibility" below), releases from before that revision use saved +policies through the temporary view `user_policies` until a later revision +drops it; after that, rolling the application back to such a release also +needs revision `fa7fa1035444` downgraded, which keeps every row (see below). +The release before Stage 12A also serves `/v2/user-policies` and `/v2/user-households` +without authentication: any caller can list a user's saved items by +`user_id`, read, rename or delete one by record ID, and create rows for any +user. Rolling the application back would therefore hand users' saved work to +anyone. - **Before any user-owned record exists, an application rollback is safe - without a database downgrade.** Signed-out saving needs no configuration, so + without a Stage 12A database downgrade.** Signed-out saving needs no configuration, so treat user-owned records as existing from the moment this revision serves traffic in an environment. - **Once any user-owned record exists, signed in or not, an application @@ -389,17 +402,36 @@ would therefore hand users' saved work to anyone. `/v2/user-policies` and `/v2/user-households` responses require a non-null `policy_id` / `household_id`, so any response that includes a legacy-only row (a v1 reference with no v2 copy) fails with 500. -- **A database downgrade is lossy.** It restores NOT NULL on `policy_id`, - `household_id`, `simulation_id`, `report_id`, `country`, and - `users.primary_country`, so it fails while legacy-only rows, any simulation - or report associations, or any unauthenticated user exist. Removing them - first deletes users' saved work. +- **Downgrading the rename alone is not lossy.** Downgrading revision + `fa7fa1035444` drops the temporary view and renames + `user_policy_associations` back to `user_policies` with every row, + reference and table grant. +- **A database downgrade of Stage 12A is lossy.** Downgrading revision + `fbb7488ebe8a` restores NOT NULL on `policy_id`, `household_id`, + `simulation_id`, `report_id`, `country`, and `users.primary_country`, so it + fails while legacy-only rows, any simulation or report associations, or any + unauthenticated user exist. Removing them first deletes users' saved work. ### Schema compatibility -The revision is additive (expand-only) for everything the previous Public API -revision maps: it drops no table or column, and it adds no required column -without a server default to any table that revision writes. The only new +**Since revision `fa7fa1035444`, earlier releases use saved policies through a +temporary view.** That revision renames `user_policies` to +`user_policy_associations` and creates a view named `user_policies` with every +column of the renamed table, the approved hand-written exception described in +`docs/engineering/skills/alembic-migrations.md` ("API v2 saved-policy table +rename"). A single-table view is automatically updatable, and the runtime role +holds on it what it holds on the table, so a release from before the rename +keeps reading and writing saved policies, during a deploy and after a +rollback: its `/v2/user-policies` routes, report creation that checks the saved +policies a report names, the sign-in merge, and the Stage 10 saved-policy +copy. `test_v2_saved_policy_view.py` runs those operations as the runtime role +through the view. Once a later revision drops the view, such a release fails +every statement on saved policies. + +The rest of this section describes revision `fbb7488ebe8a` alone. It was +additive (expand-only) for everything the previous Public API revision maps: +it drops no table or column, and it adds no required column without a server +default to any table that revision writes. The only new required columns are `country_id` on the simulation and report associations, which that revision maps but never inserts into, and `users.user_type`, whose server default makes every user the previous revision creates (its Stage 10 @@ -412,7 +444,9 @@ row with a NULL `primary_country` does not fail there. The lifecycle test `test_stage_12a_schema_serves_the_previous_revision` exercises this rather than only comparing columns. It reflects the schema at -the parent revision with SQLAlchemy and upgrades to head. Then, through the +the parent revision with SQLAlchemy and upgrades to head. Since the rename, +the parent's `user_policies` is head's temporary view of that name, so the +parent's statements on saved policies run through the view. Then, through the reflected parent tables, it does the following for every table the parent revision maps: diff --git a/migrations/v2/versions/fa7fa1035444_rename_user_policies_to_user_policy_.py b/migrations/v2/versions/fa7fa1035444_rename_user_policies_to_user_policy_.py new file mode 100644 index 000000000..d9dea42da --- /dev/null +++ b/migrations/v2/versions/fa7fa1035444_rename_user_policies_to_user_policy_.py @@ -0,0 +1,306 @@ +"""rename user_policies to user_policy_associations + +Revision ID: fa7fa1035444 +Revises: fbb7488ebe8a +Create Date: 2026-10-22 17:07:03.720031 +Generation: uv run alembic -c alembic-v2.ini revision --autogenerate +""" + +from typing import Sequence, Union + +from alembic import op +import sqlalchemy as sa +import sqlmodel +from sqlalchemy.dialects import postgresql + +revision: str = "fa7fa1035444" +down_revision: Union[str, None] = "fbb7488ebe8a" +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + # ### commands auto generated by Alembic - please adjust! ### + op.drop_constraint( + op.f("fk_legacy_user_policy_mappings_association_country"), + "legacy_user_policy_mappings", + type_="foreignkey", + ) + op.drop_constraint( + op.f("fk_user_report_associations_baseline_user_policy_id_use_7740"), + "user_report_associations", + type_="foreignkey", + ) + op.drop_constraint( + op.f("fk_user_report_associations_user_policy_id_user_policies"), + "user_report_associations", + type_="foreignkey", + ) + op.drop_index(op.f("ix_user_policies_policy_id"), table_name="user_policies") + op.drop_index(op.f("ix_user_policies_user_id"), table_name="user_policies") + # Post-generation correction: autogeneration cannot detect a rename. It + # created an empty user_policy_associations table and dropped user_policies, + # which would delete every saved policy (its three composite indexes would + # also have collided with the old table's indexes of the same names). The + # table is renamed in place instead, keeping its rows, grants and composite + # indexes. Each constraint the generated table named after + # user_policy_associations is dropped under its old name and created under + # the generated name with the generated definition. The foreign keys that + # refer to this table were dropped above, before its primary key and unique + # constraint. + op.rename_table("user_policies", "user_policy_associations") + op.drop_constraint("pk_user_policies", "user_policy_associations", type_="primary") + op.create_primary_key( + op.f("pk_user_policy_associations"), "user_policy_associations", ["id"] + ) + op.drop_constraint( + "uq_user_policies_id_country", "user_policy_associations", type_="unique" + ) + op.create_unique_constraint( + "uq_user_policy_associations_id_country", + "user_policy_associations", + ["id", "country_id"], + ) + op.drop_constraint( + "ck_user_policies_baseline", "user_policy_associations", type_="check" + ) + op.create_check_constraint( + op.f("ck_user_policy_associations_baseline"), + "user_policy_associations", + "baseline IS NULL OR baseline IN ('current-law', 'current-policy')", + ) + op.drop_constraint( + "ck_user_policies_country", "user_policy_associations", type_="check" + ) + op.create_check_constraint( + op.f("ck_user_policy_associations_country"), + "user_policy_associations", + "country_id IN ('us', 'uk')", + ) + op.drop_constraint( + "ck_user_policies_policy_ref", "user_policy_associations", type_="check" + ) + op.create_check_constraint( + op.f("ck_user_policy_associations_policy_ref"), + "user_policy_associations", + "policy_id IS NOT NULL OR legacy_policy_id IS NOT NULL", + ) + op.drop_constraint( + "fk_user_policies_policy_country", + "user_policy_associations", + type_="foreignkey", + ) + op.create_foreign_key( + "fk_user_policy_associations_policy_country", + "user_policy_associations", + "policies", + ["policy_id", "country_id"], + ["id", "country_id"], + ondelete="RESTRICT", + ) + op.drop_constraint( + "fk_user_policies_user_id_users", + "user_policy_associations", + type_="foreignkey", + ) + op.create_foreign_key( + op.f("fk_user_policy_associations_user_id_users"), + "user_policy_associations", + "users", + ["user_id"], + ["id"], + ondelete="CASCADE", + ) + op.create_index( + op.f("ix_user_policy_associations_policy_id"), + "user_policy_associations", + ["policy_id"], + unique=False, + ) + op.create_index( + op.f("ix_user_policy_associations_user_id"), + "user_policy_associations", + ["user_id"], + unique=False, + ) + op.create_foreign_key( + "fk_legacy_user_policy_mappings_association_country", + "legacy_user_policy_mappings", + "user_policy_associations", + ["user_policy_id", "country_id"], + ["id", "country_id"], + ondelete="CASCADE", + ) + op.create_foreign_key( + op.f("fk_user_report_associations_user_policy_id_user_policy_associations"), + "user_report_associations", + "user_policy_associations", + ["user_policy_id"], + ["id"], + ondelete="SET NULL", + ) + op.create_foreign_key( + op.f( + "fk_user_report_associations_baseline_user_policy_id_user_policy_associations" + ), + "user_report_associations", + "user_policy_associations", + ["baseline_user_policy_id"], + ["id"], + ondelete="SET NULL", + ) + # Approved hand-written exception: a temporary view under the table's + # earlier name, so the Public API release still serving during the deploy, + # or after a rollback to it, keeps reading and writing saved policies. A + # maintainer approved this hand-written statement on 2026-10-22, and a later + # revision drops the view. A single-table view is automatically updatable; + # the privilege manifest grants on it what it grants on the table. + op.execute( + "CREATE VIEW user_policies AS SELECT id, created_at, updated_at, user_id, " + "policy_id, country_id, name, description, legacy_policy_id, baseline, " + "provenance FROM user_policy_associations" + ) + # ### end Alembic commands ### + + +def downgrade() -> None: + # ### commands auto generated by Alembic - please adjust! ### + # Approved hand-written exception: the upgrade's temporary view is dropped + # first, so the table can take its earlier name back. + op.execute("DROP VIEW user_policies") + op.drop_constraint( + op.f( + "fk_user_report_associations_baseline_user_policy_id_user_policy_associations" + ), + "user_report_associations", + type_="foreignkey", + ) + op.drop_constraint( + op.f("fk_user_report_associations_user_policy_id_user_policy_associations"), + "user_report_associations", + type_="foreignkey", + ) + op.drop_constraint( + "fk_legacy_user_policy_mappings_association_country", + "legacy_user_policy_mappings", + type_="foreignkey", + ) + op.drop_index( + op.f("ix_user_policy_associations_user_id"), + table_name="user_policy_associations", + ) + op.drop_index( + op.f("ix_user_policy_associations_policy_id"), + table_name="user_policy_associations", + ) + # Post-generation correction: the reverse of the upgrade's rename. The + # generated downgrade created an empty user_policies table and dropped + # user_policy_associations, which would delete every saved policy. The + # table is renamed back in place, and each constraint is dropped under its + # new name and created under the name and definition it has at revision + # fbb7488ebe8a. + op.drop_constraint( + op.f("fk_user_policy_associations_user_id_users"), + "user_policy_associations", + type_="foreignkey", + ) + op.drop_constraint( + "fk_user_policy_associations_policy_country", + "user_policy_associations", + type_="foreignkey", + ) + op.drop_constraint( + op.f("ck_user_policy_associations_policy_ref"), + "user_policy_associations", + type_="check", + ) + op.drop_constraint( + op.f("ck_user_policy_associations_country"), + "user_policy_associations", + type_="check", + ) + op.drop_constraint( + op.f("ck_user_policy_associations_baseline"), + "user_policy_associations", + type_="check", + ) + op.drop_constraint( + "uq_user_policy_associations_id_country", + "user_policy_associations", + type_="unique", + ) + op.drop_constraint( + op.f("pk_user_policy_associations"), + "user_policy_associations", + type_="primary", + ) + op.rename_table("user_policy_associations", "user_policies") + op.create_primary_key(op.f("pk_user_policies"), "user_policies", ["id"]) + op.create_unique_constraint( + op.f("uq_user_policies_id_country"), "user_policies", ["id", "country_id"] + ) + op.create_check_constraint( + op.f("ck_user_policies_baseline"), + "user_policies", + "baseline IS NULL OR baseline IN ('current-law', 'current-policy')", + ) + op.create_check_constraint( + op.f("ck_user_policies_country"), + "user_policies", + "country_id IN ('us', 'uk')", + ) + op.create_check_constraint( + op.f("ck_user_policies_policy_ref"), + "user_policies", + "policy_id IS NOT NULL OR legacy_policy_id IS NOT NULL", + ) + op.create_foreign_key( + op.f("fk_user_policies_policy_country"), + "user_policies", + "policies", + ["policy_id", "country_id"], + ["id", "country_id"], + ondelete="RESTRICT", + ) + op.create_foreign_key( + op.f("fk_user_policies_user_id_users"), + "user_policies", + "users", + ["user_id"], + ["id"], + ondelete="CASCADE", + ) + op.create_index( + op.f("ix_user_policies_user_id"), "user_policies", ["user_id"], unique=False + ) + op.create_index( + op.f("ix_user_policies_policy_id"), + "user_policies", + ["policy_id"], + unique=False, + ) + op.create_foreign_key( + op.f("fk_user_report_associations_user_policy_id_user_policies"), + "user_report_associations", + "user_policies", + ["user_policy_id"], + ["id"], + ondelete="SET NULL", + ) + op.create_foreign_key( + op.f("fk_user_report_associations_baseline_user_policy_id_use_7740"), + "user_report_associations", + "user_policies", + ["baseline_user_policy_id"], + ["id"], + ondelete="SET NULL", + ) + op.create_foreign_key( + op.f("fk_legacy_user_policy_mappings_association_country"), + "legacy_user_policy_mappings", + "user_policies", + ["user_policy_id", "country_id"], + ["id", "country_id"], + ondelete="CASCADE", + ) + # ### end Alembic commands ### diff --git a/policyengine_api/data/v2/metadata_validation.py b/policyengine_api/data/v2/metadata_validation.py index 7445269f1..ab60ab93a 100644 --- a/policyengine_api/data/v2/metadata_validation.py +++ b/policyengine_api/data/v2/metadata_validation.py @@ -3,8 +3,8 @@ from collections.abc import Iterable # These names belong only to the existing v1 database domain. Names that are -# intentionally shared between v1 and v2, such as `simulations` and -# `user_policies`, are excluded from this targeted rejection set. +# intentionally shared between v1 and v2, such as `simulations`, are excluded +# from this targeted rejection set. V1_ONLY_TABLES = frozenset( { "analysis", diff --git a/policyengine_api/data/v2/migration_target.py b/policyengine_api/data/v2/migration_target.py index fd4c05fdf..a57c2419c 100644 --- a/policyengine_api/data/v2/migration_target.py +++ b/policyengine_api/data/v2/migration_target.py @@ -24,7 +24,7 @@ from collections.abc import Mapping V2_ALEMBIC_DISPOSABLE_TEST = "V2_ALEMBIC_DISPOSABLE_TEST" -V2_ALEMBIC_HEAD_REVISION = "fbb7488ebe8a" +V2_ALEMBIC_HEAD_REVISION = "fa7fa1035444" DISPOSABLE_DATABASE_NAME = "policyengine_v2_alembic_test" DISPOSABLE_HOSTS = frozenset({"localhost", "127.0.0.1", "::1", "postgres"}) MIGRATION_ROLE = "policyengine_v2_migrator" diff --git a/policyengine_api/data/v2/models/associations.py b/policyengine_api/data/v2/models/associations.py index 2c5cd21b1..4823e87fb 100644 --- a/policyengine_api/data/v2/models/associations.py +++ b/policyengine_api/data/v2/models/associations.py @@ -106,29 +106,29 @@ class UserPolicy(TimestampedModel, table=True): policy that users have saved cannot be deleted. """ - __tablename__ = "user_policies" + __tablename__ = "user_policy_associations" __table_args__ = ( sa.UniqueConstraint( "id", "country_id", - name="uq_user_policies_id_country", + name="uq_user_policy_associations_id_country", ), sa.CheckConstraint( "country_id IN ('us', 'uk')", - name="ck_user_policies_country", + name="ck_user_policy_associations_country", ), sa.CheckConstraint( "policy_id IS NOT NULL OR legacy_policy_id IS NOT NULL", - name="ck_user_policies_policy_ref", + name="ck_user_policy_associations_policy_ref", ), sa.CheckConstraint( "baseline IS NULL OR baseline IN ('current-law', 'current-policy')", - name="ck_user_policies_baseline", + name="ck_user_policy_associations_baseline", ), sa.ForeignKeyConstraint( ["policy_id", "country_id"], ["policies.id", "policies.country_id"], - name="fk_user_policies_policy_country", + name="fk_user_policy_associations_policy_country", ondelete="RESTRICT", ), sa.Index( @@ -278,7 +278,7 @@ class UserReportAssociation(TimestampedModel, table=True): description: str | None = Field(default=None, sa_type=sa.Text) user_policy_id: UUID | None = Field( default=None, - foreign_key="user_policies.id", + foreign_key="user_policy_associations.id", ondelete="SET NULL", index=True, ) @@ -286,7 +286,7 @@ class UserReportAssociation(TimestampedModel, table=True): # comparison was not current law; its name is the baseline's label. baseline_user_policy_id: UUID | None = Field( default=None, - foreign_key="user_policies.id", + foreign_key="user_policy_associations.id", ondelete="SET NULL", index=True, ) diff --git a/policyengine_api/data/v2/models/policy_mappings.py b/policyengine_api/data/v2/models/policy_mappings.py index 7cdbc91b9..69aef5642 100644 --- a/policyengine_api/data/v2/models/policy_mappings.py +++ b/policyengine_api/data/v2/models/policy_mappings.py @@ -104,7 +104,7 @@ class LegacyUserPolicyMapping(TimestampedModel, table=True): ), sa.ForeignKeyConstraint( ["user_policy_id", "country_id"], - ["user_policies.id", "user_policies.country_id"], + ["user_policy_associations.id", "user_policy_associations.country_id"], name="fk_legacy_user_policy_mappings_association_country", ondelete="CASCADE", ), diff --git a/policyengine_api/data/v2/policy_migration_qualification.py b/policyengine_api/data/v2/policy_migration_qualification.py index beb03db24..d353226b1 100644 --- a/policyengine_api/data/v2/policy_migration_qualification.py +++ b/policyengine_api/data/v2/policy_migration_qualification.py @@ -10,12 +10,12 @@ from alembic.config import Config from alembic.migration import MigrationContext from alembic.script import ScriptDirectory -from sqlalchemy import Connection, Engine, func, inspect, select +from sqlalchemy import Connection, Engine, func, inspect, select, table from sqlalchemy.exc import NoInspectionAvailable from sqlalchemy.pool import NullPool from sqlmodel import create_engine -from policyengine_api.data.v2.models import ParameterValue, Policy, UserPolicy +from policyengine_api.data.v2.models import ParameterValue, Policy from policyengine_api.data.v2.settings import ( V2ConfigurationError, V2DatabaseSettings, @@ -26,6 +26,10 @@ from collections.abc import Callable, Collection, Mapping POLICY_MIGRATION_REVISION = "711ec2f0a5a5" +# The saved-policy table's name while the policy revision is pending. A later +# revision renames it to ``user_policy_associations``, so counting through +# the current model would query a table such a database does not have yet. +SAVED_POLICY_TABLE_AT_POLICY_REVISION = "user_policies" V2_ALEMBIC_CONFIG = "alembic-v2.ini" @@ -123,8 +127,15 @@ def table_exists(name: str) -> bool: if table_exists("parameter_values") else 0, user_policies=( - int(executor.scalar(select(func.count()).select_from(UserPolicy)) or 0) - if table_exists("user_policies") + int( + executor.scalar( + select(func.count()).select_from( + table(SAVED_POLICY_TABLE_AT_POLICY_REVISION) + ) + ) + or 0 + ) + if table_exists(SAVED_POLICY_TABLE_AT_POLICY_REVISION) else 0 ), ) diff --git a/policyengine_api/data/v2/privilege_reconciliation.py b/policyengine_api/data/v2/privilege_reconciliation.py index d9d0cacec..33af3896b 100644 --- a/policyengine_api/data/v2/privilege_reconciliation.py +++ b/policyengine_api/data/v2/privilege_reconciliation.py @@ -6,8 +6,10 @@ - ``apply`` reads the managed roles' current grants, grants every declared privilege a role lacks, and reports privileges the manifest doesn't declare. - It never revokes, and it changes nothing when a declared role or table is - missing. Running it again grants nothing more. + It never revokes, and it changes nothing when a declared role, table or view + is missing. A declared table must exist as a table and a declared temporary + view as a view; a relation of the other kind counts as missing. Running it + again grants nothing more. - ``verify`` checks every declared privilege with ``has_table_privilege`` (rule ``api.v2.deployed-privileges-match``) and fails on any that is missing. @@ -42,6 +44,7 @@ ) from policyengine_api.data.v2.privileges import ( PERSISTENT_ENVIRONMENTS, + TEMPORARY_VIEW_TABLES, TablePrivilege, declared_privileges, ) @@ -56,6 +59,8 @@ GITHUB_STEP_SUMMARY = "GITHUB_STEP_SUMMARY" SCHEMA = "public" SAFE_IDENTIFIER = re.compile(r"^[a-z_][a-z0-9_]{0,62}$") +# The manifest's temporary views; every other declared name is a table. +DECLARED_VIEWS: frozenset[str] = frozenset(TEMPORARY_VIEW_TABLES) class PrivilegeReconciliationError(RuntimeError): @@ -94,11 +99,20 @@ class ReconciliationPlan: undeclared: tuple[PrivilegeEntry, ...] missing_roles: tuple[str, ...] missing_tables: tuple[str, ...] + missing_views: tuple[str, ...] = () @property def can_apply(self) -> bool: - """Whether every declared role and table exists, so grants may run.""" - return not self.missing_roles and not self.missing_tables + """Whether every declared role, table and view exists, so grants may run.""" + return not (self.missing_roles or self.missing_tables or self.missing_views) + + +@dataclass(frozen=True) +class PublicRelations: + """The tables and views in the public schema, by name.""" + + tables: frozenset[str] + views: frozenset[str] def plan_reconciliation( @@ -106,21 +120,30 @@ def plan_reconciliation( granted: Mapping[str, Mapping[str, Collection[str]]], *, existing_roles: Collection[str], - existing_tables: Collection[str], + existing_relations: PublicRelations, + declared_views: Collection[str] = (), ) -> ReconciliationPlan: """Compare declared privileges with what the database grants. ``granted`` holds the managed roles' direct grants, by role then table. + Names in ``declared_views`` must exist as views, and every other declared + name as a table. """ missing_roles = tuple(sorted(set(declared) - set(existing_roles))) - declared_tables = {table for tables in declared.values() for table in tables} - missing_tables = tuple(sorted(declared_tables - set(existing_tables))) + declared_names = {table for tables in declared.values() for table in tables} + missing_views = tuple( + sorted((declared_names & set(declared_views)) - existing_relations.views) + ) + missing_tables = tuple( + sorted(declared_names - set(declared_views) - existing_relations.tables) + ) + missing_relations = {*missing_tables, *missing_views} grants = sorted( PrivilegeEntry(role, table, privilege.value) for role, tables in declared.items() if role not in missing_roles for table, privileges in tables.items() - if table not in missing_tables + if table not in missing_relations for privilege in privileges if privilege.value not in granted.get(role, {}).get(table, ()) ) @@ -140,6 +163,7 @@ def plan_reconciliation( undeclared=tuple(undeclared), missing_roles=missing_roles, missing_tables=missing_tables, + missing_views=missing_views, ) @@ -214,6 +238,12 @@ def render_summary( + ", ".join(f"`{table}`" for table in plan.missing_tables), "", ] + if plan.missing_views: + lines += [ + "Declared views missing from the database (nothing was granted): " + + ", ".join(f"`{view}`" for view in plan.missing_views), + "", + ] grant_heading = "Granted" if applied else "Would grant" lines += [f"### {grant_heading}", ""] if plan.grants: @@ -270,15 +300,17 @@ def render_summary( "SELECT rolname FROM pg_roles WHERE rolname IN :roles" ).bindparams(bindparam("roles", expanding=True)) -_PUBLIC_TABLES = text( +_PUBLIC_RELATIONS = text( """ - SELECT table_class.relname - FROM pg_class AS table_class - JOIN pg_namespace AS table_namespace - ON table_namespace.oid = table_class.relnamespace - WHERE table_namespace.nspname = :schema AND table_class.relkind IN ('r', 'p') + SELECT relation_class.relname, relation_class.relkind + FROM pg_class AS relation_class + JOIN pg_namespace AS relation_namespace + ON relation_namespace.oid = relation_class.relnamespace + WHERE relation_namespace.nspname = :schema + AND relation_class.relkind IN ('r', 'p', 'v') """ ) +_TABLE_KINDS = frozenset({"r", "p"}) _DEFAULT_PRIVILEGES = text( """ @@ -311,9 +343,17 @@ def read_existing_roles(connection: Connection, roles: Collection[str]) -> set[s return set(connection.execute(_EXISTING_ROLES, {"roles": list(roles)}).scalars()) -def read_public_tables(connection: Connection) -> set[str]: - """Return every table in the public schema.""" - return set(connection.execute(_PUBLIC_TABLES, {"schema": SCHEMA}).scalars()) +def read_public_relations(connection: Connection) -> PublicRelations: + """Return the tables and the views in the public schema. + + The manifest declares privileges on temporary views as it does on tables, + and ``GRANT ... ON TABLE`` grants on a view too. + """ + rows = connection.execute(_PUBLIC_RELATIONS, {"schema": SCHEMA}).all() + return PublicRelations( + tables=frozenset(name for name, kind in rows if kind in _TABLE_KINDS), + views=frozenset(name for name, kind in rows if kind == "v"), + ) def read_granted_privileges( @@ -363,17 +403,22 @@ def create_missing_roles(connection: Connection, roles: Iterable[str]) -> list[s def apply_declared_privileges( connection: Connection, declared: Mapping[str, Mapping[str, frozenset[TablePrivilege]]], + *, + declared_views: Collection[str] = DECLARED_VIEWS, ) -> tuple[ReconciliationPlan, list[DefaultPrivilege]]: """Grant every declared privilege a role lacks; never revoke. - Grants nothing when a declared role or table is missing. + Grants nothing when a declared role, table or view is missing, including a + declared view that exists as a table or a declared table that exists as a + view. """ roles = list(declared) plan = plan_reconciliation( declared, read_granted_privileges(connection, roles), existing_roles=read_existing_roles(connection, roles), - existing_tables=read_public_tables(connection), + existing_relations=read_public_relations(connection), + declared_views=declared_views, ) if plan.can_apply: for statement in grant_statements( @@ -386,16 +431,24 @@ def apply_declared_privileges( def check_declared_privileges( connection: Connection, declared: Mapping[str, Mapping[str, frozenset[TablePrivilege]]], + *, + declared_views: Collection[str] = DECLARED_VIEWS, ) -> list[PrivilegeEntry]: """Return every declared privilege a role doesn't effectively hold. - A missing role or table counts as missing every privilege declared for it. + A missing role, table or view counts as missing every privilege declared + for it; so does a declared view that exists as a table, or the reverse. """ existing_roles = read_existing_roles(connection, list(declared)) - existing_tables = read_public_tables(connection) + relations = read_public_relations(connection) + + def exists_as_declared(name: str) -> bool: + if name in declared_views: + return name in relations.views + return name in relations.tables def has_privilege(role: str, table: str, privilege: str) -> bool: - if role not in existing_roles or table not in existing_tables: + if role not in existing_roles or not exists_as_declared(table): return False return bool( connection.execute( diff --git a/policyengine_api/data/v2/privileges.py b/policyengine_api/data/v2/privileges.py index 8f8c34515..5014230f0 100644 --- a/policyengine_api/data/v2/privileges.py +++ b/policyengine_api/data/v2/privileges.py @@ -10,7 +10,9 @@ ``api.v2.runtime-privileges-tested-as-runtime-role``). Every table in the v2 SQLModel metadata, plus ``alembic_version``, has an entry -for every managed role, so "no access" is a decision, not an omission. +for every managed role, so "no access" is a decision, not an omission. A +temporary view under a renamed table's earlier name (``TEMPORARY_VIEW_TABLES``) +is declared with its table's privileges. Roles and what decides their privileges: @@ -135,7 +137,7 @@ def _access( # Users and their saved work (Stage 12A). "user_household_associations": _access(runtime=READ_WRITE), "user_identities": _access(runtime=READ_WRITE), - "user_policies": _access(runtime=READ_WRITE), + "user_policy_associations": _access(runtime=READ_WRITE), "user_report_associations": _access(runtime=READ_WRITE), "user_simulation_associations": _access(runtime=READ_WRITE), "users": _access(runtime=READ_WRITE), @@ -145,6 +147,17 @@ def _access( } +# Temporary views under a renamed table's earlier name, each mapped to the +# table it shows. The Public API release that still serves during a deploy, or +# after a rollback to it, reads and writes the table through the view, so every +# role holds on the view what it holds on the table. A later revision drops +# each view, and its entry here goes with it. +TEMPORARY_VIEW_TABLES: Final[dict[str, str]] = { + # Revision fa7fa1035444 renamed user_policies to user_policy_associations. + "user_policies": "user_policy_associations", +} + + class InvalidDeploymentEnvironmentError(ValueError): """Raised for a deployment environment that can't name a Stage 12 role.""" @@ -167,12 +180,23 @@ def stage12_role(deployment_environment: str) -> str: def declared_privileges( deployment_environment: str, ) -> dict[str, dict[str, frozenset[TablePrivilege]]]: - """Return each managed role's declared privileges, keyed by role then table.""" + """Return each managed role's declared privileges, keyed by role then table. + + Each temporary view is declared with its table's privileges. + """ stage12 = stage12_role(deployment_environment) + access_by_relation = TABLE_ACCESS | { + view: TABLE_ACCESS[table] for view, table in TEMPORARY_VIEW_TABLES.items() + } return { - RUNTIME_ROLE: {table: access.runtime for table, access in TABLE_ACCESS.items()}, + RUNTIME_ROLE: { + relation: access.runtime for relation, access in access_by_relation.items() + }, CATALOG_WRITER_ROLE: { - table: access.catalog_writer for table, access in TABLE_ACCESS.items() + relation: access.catalog_writer + for relation, access in access_by_relation.items() + }, + stage12: { + relation: access.stage12 for relation, access in access_by_relation.items() }, - stage12: {table: access.stage12 for table, access in TABLE_ACCESS.items()}, } diff --git a/policyengine_api/data/v2/user_association_migration_qualification.py b/policyengine_api/data/v2/user_association_migration_qualification.py index 9d7aad2a5..fac2b0d00 100644 --- a/policyengine_api/data/v2/user_association_migration_qualification.py +++ b/policyengine_api/data/v2/user_association_migration_qualification.py @@ -24,14 +24,13 @@ from alembic.config import Config from alembic.migration import MigrationContext from alembic.script import ScriptDirectory -from sqlalchemy import Connection, Engine, func, inspect, select +from sqlalchemy import Connection, Engine, func, inspect, select, table from sqlalchemy.exc import NoInspectionAvailable from sqlalchemy.pool import NullPool from sqlmodel import create_engine from policyengine_api.data.v2.models import ( UserHouseholdAssociation, - UserPolicy, UserReportAssociation, UserSimulationAssociation, ) @@ -44,7 +43,14 @@ if TYPE_CHECKING: from collections.abc import Callable, Collection, Mapping + from sqlalchemy.sql.expression import TableClause + USER_ASSOCIATION_MIGRATION_REVISION = "fbb7488ebe8a" +# The saved-policy table's name at this revision, and the name a later +# revision gives it. The command runs before every upgrade, so it counts the +# table under whichever name the database has. +SAVED_POLICY_TABLE_AT_RESHAPE = "user_policies" +SAVED_POLICY_TABLE_AFTER_RENAME = "user_policy_associations" V2_ALEMBIC_CONFIG = "alembic-v2.ini" @@ -122,13 +128,20 @@ def read_user_association_counts( ) -> UserAssociationCounts: """Count every row in the four user-association tables.""" - def count(model: type, table_name: str) -> int: + def count(source: type | TableClause, table_name: str) -> int: if existing_tables is not None and table_name not in existing_tables: return 0 - return int(executor.scalar(select(func.count()).select_from(model)) or 0) + return int(executor.scalar(select(func.count()).select_from(source)) or 0) + + saved_policy_table = ( + SAVED_POLICY_TABLE_AFTER_RENAME + if existing_tables is not None + and SAVED_POLICY_TABLE_AFTER_RENAME in existing_tables + else SAVED_POLICY_TABLE_AT_RESHAPE + ) return UserAssociationCounts( - user_policies=count(UserPolicy, "user_policies"), + user_policies=count(table(saved_policy_table), saved_policy_table), user_household_associations=count( UserHouseholdAssociation, "user_household_associations" ), diff --git a/rules/fixtures/api.v2.runtime-privileges-declared/reads_good.py b/rules/fixtures/api.v2.runtime-privileges-declared/reads_good.py index 5437a95c0..0f7567704 100644 --- a/rules/fixtures/api.v2.runtime-privileges-declared/reads_good.py +++ b/rules/fixtures/api.v2.runtime-privileges-declared/reads_good.py @@ -1,7 +1,7 @@ """Good fixture: a runtime read of only the tables the runtime role is granted. -The user-policy lookup reads ``user_policies``, which the runtime role holds -SELECT on, and nothing else. +The user-policy lookup reads ``user_policy_associations``, which the runtime +role holds SELECT on, and nothing else. """ from __future__ import annotations diff --git a/rules/registry.toml b/rules/registry.toml index 66b86bb8d..24faf8334 100644 --- a/rules/registry.toml +++ b/rules/registry.toml @@ -671,7 +671,7 @@ id = "api.v2.runtime-privileges-tested-as-runtime-role" category = "migration" statement = "The cross-database integration tests run the v2 user services as policyengine_v2_runtime and the comparison-run services as policyengine_stage12_, holding only their declared privileges, so a statement those privileges don't allow fails CI." rationale = "docs/engineering/skills/alembic-migrations.md#table-privileges" -scope = ["tests/integration/conftest.py", "tests/integration/test_v2_user_identity_persistence.py", "tests/integration/test_v2_stage12_comparison_persistence.py"] +scope = ["tests/integration/conftest.py", "tests/integration/test_v2_user_identity_persistence.py", "tests/integration/test_v2_saved_policy_view.py", "tests/integration/test_v2_stage12_comparison_persistence.py"] tier = "mechanical" status = "enforced" autofix = false diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index b90efebd2..eb855ebb2 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -105,8 +105,8 @@ def managed_v2_roles(disposable_v2_database_url: str) -> ManagedV2Roles: engine.dispose() if not plan.can_apply: pytest.fail( - "the disposable v2 database lacks declared roles or tables: " - f"{plan.missing_roles} {plan.missing_tables}" + "the disposable v2 database lacks declared roles, tables or views: " + f"{plan.missing_roles} {plan.missing_tables} {plan.missing_views}" ) return ManagedV2Roles( deployment_environment=deployment_environment, diff --git a/tests/integration/test_alembic_v2_lifecycle.py b/tests/integration/test_alembic_v2_lifecycle.py index 814971236..5ef7c6169 100644 --- a/tests/integration/test_alembic_v2_lifecycle.py +++ b/tests/integration/test_alembic_v2_lifecycle.py @@ -21,6 +21,7 @@ V2_ALEMBIC_HEAD_REVISION, V2MigrationTargetError, load_v2_alembic_settings, + validate_v2_head_schema, ) from policyengine_api.data.v2.models import V2_METADATA from policyengine_api.data.v2.settings import V2_MIGRATION_DATABASE_URL @@ -30,6 +31,7 @@ STAGE_12_PREVIOUS_REVISION = "724b1b11a33e" STAGE_12_COMPARISON_PREVIOUS_REVISION = "439303be14fe" STAGE_12A_PREVIOUS_REVISION = "91af4fc8d8c1" +SAVED_POLICY_RENAME_PARENT_REVISION = "fbb7488ebe8a" V2_TABLE_NAMES = frozenset(table.name for table in V2_METADATA.tables.values()) @@ -147,7 +149,7 @@ def _assert_stage_12a_association_shape(engine) -> None: "subject", ] for table_name, reference in ( - ("user_policies", "policy"), + ("user_policy_associations", "policy"), ("user_household_associations", "household"), ("user_simulation_associations", "simulation"), ("user_report_associations", "report"), @@ -171,7 +173,7 @@ def _assert_stage_12a_association_shape(engine) -> None: report_policy_key = next( foreign_key for foreign_key in inspect(engine).get_foreign_keys("user_report_associations") - if foreign_key["referred_table"] == "user_policies" + if foreign_key["referred_table"] == "user_policy_associations" ) assert report_policy_key["options"]["ondelete"] == "SET NULL" for table_name in ("user_simulation_associations", "user_report_associations"): @@ -604,7 +606,9 @@ def test_stage_12a_schema_serves_the_previous_revision() -> None: requires), read the row back with every mapped column, rewrite those columns, delete it, and delete a user who owns Stage 12A rows so the cascades run. Head may not give any table the parent revision writes a - new required column without a server default. + new required column without a server default. Since the saved-policy + rename, the parent's ``user_policies`` is head's temporary view of that + name, so the parent's statements on it run through the view. """ database_url = _disposable_url() config = _config() @@ -616,7 +620,7 @@ def test_stage_12a_schema_serves_the_previous_revision() -> None: parent.reflect(engine) command.upgrade(config, "head") head = sa.MetaData() - head.reflect(engine) + head.reflect(engine, views=True) parent_tables = [ table for table in parent.sorted_tables if table.name != "alembic_version" ] @@ -957,3 +961,456 @@ def reflected(name: str) -> sa.Table: connection.execute(sa.delete(table).where(table.c.id == row_id)) command.upgrade(config, "head") engine.dispose() + + +def _constraint_names(engine, table_name: str) -> set[str]: + with engine.connect() as connection: + return set( + connection.execute( + text( + "SELECT conname FROM pg_constraint " + "WHERE conrelid = to_regclass(:table_name)" + ), + {"table_name": f"public.{table_name}"}, + ).scalars() + ) + + +def _index_names(engine, table_name: str) -> set[str]: + with engine.connect() as connection: + return set( + connection.execute( + text( + "SELECT indexname FROM pg_indexes " + "WHERE schemaname = 'public' AND tablename = :table_name" + ), + {"table_name": table_name}, + ).scalars() + ) + + +def _referring_foreign_keys(engine, table_name: str) -> set[tuple[str, str]]: + with engine.connect() as connection: + return { + (row.on_table, row.conname) + for row in connection.execute( + text( + "SELECT conrelid::regclass::text AS on_table, conname " + "FROM pg_constraint WHERE contype = 'f' " + "AND confrelid = to_regclass(:table_name)" + ), + {"table_name": f"public.{table_name}"}, + ) + } + + +def _closure_of_referred_tables( + metadata: sa.MetaData, table_names: set[str] +) -> set[str]: + closure: set[str] = set() + pending = list(table_names) + while pending: + table_name = pending.pop() + if table_name in closure: + continue + closure.add(table_name) + pending.extend( + foreign_key.referred_table.name + for foreign_key in metadata.tables[table_name].foreign_key_constraints + ) + return closure + + +def _saved_policy_with_references(connection, parent: sa.MetaData) -> dict: + """Store a user, a policy, a saved policy, its v1 mapping and a report.""" + sequence = itertools.count() + stored_rows: dict[str, dict] = {} + owner_tables = _closure_of_referred_tables(parent, {"users", "policies"}) + for table in parent.sorted_tables: + if table.name in owner_tables: + values = _row_values(table, stored_rows, sequence) + if table.name == "users": + # A standard user needs a country (ck_users_standard_primary_country). + values["primary_country"] = "us" + stored_rows[table.name] = _insert_and_read_back(connection, table, values) + saved_policy_id = uuid4() + connection.execute( + parent.tables["user_policies"] + .insert() + .values( + id=saved_policy_id, + user_id=stored_rows["users"]["id"], + policy_id=stored_rows["policies"]["id"], + country_id=stored_rows["policies"]["country_id"], + name="Saved reform", + baseline="current-law", + ) + ) + connection.execute( + parent.tables["legacy_user_policy_mappings"] + .insert() + .values( + id=uuid4(), + country_id=stored_rows["policies"]["country_id"], + legacy_user_policy_id=41, + user_policy_id=saved_policy_id, + fingerprint_version=1, + fingerprint_sha256="a" * 64, + ) + ) + report_id = uuid4() + connection.execute( + parent.tables["user_report_associations"] + .insert() + .values( + id=report_id, + user_id=stored_rows["users"]["id"], + country_id=stored_rows["policies"]["country_id"], + legacy_report_id=42, + user_policy_id=saved_policy_id, + baseline_user_policy_id=saved_policy_id, + ) + ) + return { + "stored_rows": stored_rows, + "saved_policy_id": saved_policy_id, + "report_id": report_id, + } + + +def _saved_policy_row(connection, table_name: str, saved_policy_id: UUID) -> dict: + return dict( + connection.execute( + sa.select(sa.text("*")) + .select_from(sa.table(table_name)) + .where(sa.column("id") == saved_policy_id) + ) + .mappings() + .one() + ) + + +def _delete_saved_policy_rows(engine, parent: sa.MetaData, saved: dict) -> None: + """Remove the stored rows at head, children first, so later tests start clean.""" + with engine.begin() as connection: + connection.execute( + text("DELETE FROM user_report_associations WHERE id = :id"), + {"id": saved["report_id"]}, + ) + # Deleting the saved policy cascades to its v1 mapping. + connection.execute( + text("DELETE FROM user_policy_associations WHERE id = :id"), + {"id": saved["saved_policy_id"]}, + ) + for table in reversed(parent.sorted_tables): + stored_row = saved["stored_rows"].get(table.name) + if stored_row is not None: + connection.execute( + table.delete().where(_primary_key_clause(table, stored_row)) + ) + + +# The names head gives the renamed table's constraints, indexes and the +# foreign keys that refer to it. Names longer than PostgreSQL's 63 characters +# end in a hash, as SQLAlchemy shortens the model's convention names. +SAVED_POLICY_SCHEMA_NAMES_AT_HEAD = { + "constraints": { + "pk_user_policy_associations", + "uq_user_policy_associations_id_country", + "ck_user_policy_associations_country", + "ck_user_policy_associations_policy_ref", + "ck_user_policy_associations_baseline", + "fk_user_policy_associations_policy_country", + "fk_user_policy_associations_user_id_users", + }, + "indexes": { + "pk_user_policy_associations", + "uq_user_policy_associations_id_country", + "ix_user_policy_associations_user_id", + "ix_user_policy_associations_policy_id", + "ix_user_policies_country_user_created_id", + "ix_user_policies_country_policy", + "ix_user_policies_country_legacy_policy", + }, + "referring_foreign_keys": { + ( + "legacy_user_policy_mappings", + "fk_legacy_user_policy_mappings_association_country", + ), + ( + "user_report_associations", + "fk_user_report_associations_user_policy_id_user_policy__0820", + ), + ( + "user_report_associations", + "fk_user_report_associations_baseline_user_policy_id_use_0a3d", + ), + }, +} +RENAME_PRIVILEGE_PROBE_ROLE = "rename_privilege_probe" + + +def _schema_names(engine, table_name: str) -> dict[str, set]: + return { + "constraints": _constraint_names(engine, table_name), + "indexes": _index_names(engine, table_name), + "referring_foreign_keys": _referring_foreign_keys(engine, table_name), + } + + +def _saved_policy_references(connection, saved: dict) -> tuple: + """Return the saved policy a v1 mapping and a report's two keys point at.""" + mapped = connection.execute( + text( + "SELECT user_policy_id FROM legacy_user_policy_mappings " + "WHERE legacy_user_policy_id = 41" + ) + ).scalar_one_or_none() + report_references = connection.execute( + text( + "SELECT user_policy_id, baseline_user_policy_id " + "FROM user_report_associations WHERE id = :id" + ), + {"id": saved["report_id"]}, + ).one() + return (mapped, *report_references) + + +def _references_after_deleting_saved_policy( + engine, table_name: str, saved: dict +) -> tuple: + """Delete the saved policy in a rolled-back transaction; return the references. + + The v1 mapping cascades away and the report's keys are set to null. + """ + with engine.connect() as connection: + transaction = connection.begin() + try: + connection.execute( + sa.delete(sa.table(table_name, sa.column("id"))).where( + sa.column("id") == saved["saved_policy_id"] + ) + ) + return _saved_policy_references(connection, saved) + finally: + transaction.rollback() + + +def _probe_privileges(connection, table_name: str) -> dict[str, bool]: + """Return which of three privileges the probe role holds, one at a time. + + ``has_table_privilege`` with a list of privileges is true when any one is + held, so each is checked separately. + """ + return { + privilege: connection.execute( + text("SELECT has_table_privilege(:role, :table, :privilege)"), + { + "role": RENAME_PRIVILEGE_PROBE_ROLE, + "table": f"public.{table_name}", + "privilege": privilege, + }, + ).scalar_one() + for privilege in ("SELECT", "UPDATE", "INSERT") + } + + +GRANTED_TO_PROBE = {"SELECT": True, "UPDATE": True, "INSERT": False} +CLEARED_REFERENCES = (None, None, None) + + +def _view_names(engine) -> set[str]: + return set(inspect(engine).get_view_names(schema="public")) + + +def _column_names(engine, relation_name: str) -> list[str]: + return [ + column["name"] + for column in inspect(engine).get_columns(relation_name, schema="public") + ] + + +def _assert_view_under_old_name_at_head(engine, config) -> None: + """Head keeps a view named ``user_policies`` with the table's columns. + + Neither the head schema validation nor ``alembic check`` reports it. + """ + assert _view_names(engine) == {"user_policies"} + assert _column_names(engine, "user_policies") == _column_names( + engine, "user_policy_associations" + ) + with engine.connect() as connection: + validate_v2_head_schema(connection, V2_METADATA) + command.check(config) + + +def _drop_rename_privilege_probe(engine) -> None: + with engine.begin() as connection: + connection.execute( + text( + "DO $$ BEGIN IF EXISTS (SELECT FROM pg_roles WHERE rolname = " + f"'{RENAME_PRIVILEGE_PROBE_ROLE}') THEN " + f"DROP OWNED BY {RENAME_PRIVILEGE_PROBE_ROLE}; " + f"DROP ROLE {RENAME_PRIVILEGE_PROBE_ROLE}; END IF; END $$" + ) + ) + + +def test_saved_policy_rename_keeps_rows_references_and_privileges() -> None: + """Upgrading renames the table in place, and downgrading renames it back. + + Saved policies, their v1 mappings and the reports that name them survive + both directions, the references and a table grant follow the table, and + the constraints and indexes named after the table take its name. Head has + a view named ``user_policies`` with the table's columns, which neither the + schema validation nor ``alembic check`` reports. The downgrade drops the + view and restores every name the parent revision had. + """ + config = _config() + engine = create_engine(_disposable_url()) + parent = sa.MetaData() + saved = None + try: + command.downgrade(config, SAVED_POLICY_RENAME_PARENT_REVISION) + parent.reflect(engine) + names_at_parent = _schema_names(engine, "user_policies") + _drop_rename_privilege_probe(engine) + with engine.begin() as connection: + saved = _saved_policy_with_references(connection, parent) + connection.execute(text(f"CREATE ROLE {RENAME_PRIVILEGE_PROBE_ROLE}")) + connection.execute( + text( + "GRANT SELECT, UPDATE ON user_policies TO " + f"{RENAME_PRIVILEGE_PROBE_ROLE}" + ) + ) + before = _saved_policy_row( + connection, "user_policies", saved["saved_policy_id"] + ) + saved_policy_id = saved["saved_policy_id"] + + command.upgrade(config, "head") + + table_names = set(inspect(engine).get_table_names(schema="public")) + assert "user_policies" not in table_names + _assert_view_under_old_name_at_head(engine, config) + with engine.connect() as connection: + after = _saved_policy_row( + connection, "user_policy_associations", saved_policy_id + ) + references = _saved_policy_references(connection, saved) + privileges = _probe_privileges(connection, "user_policy_associations") + assert after == before + assert references == (saved_policy_id, saved_policy_id, saved_policy_id) + assert privileges == GRANTED_TO_PROBE + assert ( + _schema_names(engine, "user_policy_associations") + == SAVED_POLICY_SCHEMA_NAMES_AT_HEAD + ) + assert ( + _references_after_deleting_saved_policy( + engine, "user_policy_associations", saved + ) + == CLEARED_REFERENCES + ) + + command.downgrade(config, SAVED_POLICY_RENAME_PARENT_REVISION) + + table_names = set(inspect(engine).get_table_names(schema="public")) + assert "user_policy_associations" not in table_names + assert "user_policies" in table_names + assert _view_names(engine) == set() + with engine.connect() as connection: + restored = _saved_policy_row(connection, "user_policies", saved_policy_id) + restored_references = _saved_policy_references(connection, saved) + restored_privileges = _probe_privileges(connection, "user_policies") + assert restored == before + assert restored_references == references + assert restored_privileges == GRANTED_TO_PROBE + assert _schema_names(engine, "user_policies") == names_at_parent + assert ( + _references_after_deleting_saved_policy(engine, "user_policies", saved) + == CLEARED_REFERENCES + ) + finally: + command.upgrade(config, "head") + if saved is not None: + _delete_saved_policy_rows(engine, parent, saved) + _drop_rename_privilege_probe(engine) + engine.dispose() + + +def test_previous_release_still_finds_user_policies() -> None: + """The release running during the deploy uses saved policies by the old name. + + A saved policy stored before the rename is found under ``user_policies`` + at head, and a row written, changed and deleted through the view is + written, changed and deleted in ``user_policy_associations``. The runtime + role's own use of the view is in ``test_v2_saved_policy_view``. + """ + config = _config() + engine = create_engine(_disposable_url()) + parent = sa.MetaData() + saved = None + try: + command.downgrade(config, SAVED_POLICY_RENAME_PARENT_REVISION) + parent.reflect(engine) + with engine.begin() as connection: + saved = _saved_policy_with_references(connection, parent) + command.upgrade(config, "head") + saved_policy_id = saved["saved_policy_id"] + with engine.connect() as connection: + stored = _saved_policy_row( + connection, "user_policy_associations", saved_policy_id + ) + found = _saved_policy_row(connection, "user_policies", saved_policy_id) + assert found == stored + + written_id = uuid4() + with engine.connect() as connection: + transaction = connection.begin() + try: + connection.execute( + text( + "INSERT INTO user_policies " + "(id, user_id, policy_id, country_id, name) " + "VALUES (:id, :user_id, :policy_id, :country_id, 'Written')" + ), + { + "id": written_id, + "user_id": stored["user_id"], + "policy_id": stored["policy_id"], + "country_id": stored["country_id"], + }, + ) + written = _saved_policy_row( + connection, "user_policy_associations", written_id + ) + connection.execute( + text("UPDATE user_policies SET name = 'Changed' WHERE id = :id"), + {"id": written_id}, + ) + changed_name = _saved_policy_row( + connection, "user_policy_associations", written_id + )["name"] + connection.execute( + text("DELETE FROM user_policies WHERE id = :id"), + {"id": written_id}, + ) + remaining = connection.execute( + text( + "SELECT count(*) FROM user_policy_associations WHERE id = :id" + ), + {"id": written_id}, + ).scalar_one() + finally: + transaction.rollback() + assert written["name"] == "Written" + assert written["created_at"] is not None + assert changed_name == "Changed" + assert remaining == 0 + finally: + command.upgrade(config, "head") + if saved is not None: + _delete_saved_policy_rows(engine, parent, saved) + engine.dispose() diff --git a/tests/integration/test_v2_privilege_reconciliation.py b/tests/integration/test_v2_privilege_reconciliation.py index f720c3fb9..9596a5966 100644 --- a/tests/integration/test_v2_privilege_reconciliation.py +++ b/tests/integration/test_v2_privilege_reconciliation.py @@ -23,7 +23,7 @@ check_declared_privileges, run, ) -from policyengine_api.data.v2.privileges import READ, TablePrivilege +from policyengine_api.data.v2.privileges import READ, READ_WRITE, TablePrivilege from policyengine_api.data.v2.settings import V2_MIGRATION_DATABASE_URL if TYPE_CHECKING: @@ -62,6 +62,21 @@ def create_role() -> str: connection.execute(text(f'DROP ROLE "{role}"')) +@pytest.fixture +def temporary_view(superuser_engine) -> Iterator[str]: + """Create a view over ``regions`` for one test, then drop it.""" + view = f"policyengine_privilege_test_view_{uuid4().hex[:12]}" + with superuser_engine.begin() as connection: + connection.execute( + text(f'CREATE VIEW public."{view}" AS SELECT * FROM public.regions') + ) + try: + yield view + finally: + with superuser_engine.begin() as connection: + connection.execute(text(f'DROP VIEW public."{view}"')) + + def _has_privilege(engine: Engine, role: str, table: str, privilege: str) -> bool: with engine.connect() as connection: return bool( @@ -137,3 +152,56 @@ def test_a_missing_role_blocks_every_grant_and_fails_verification( PrivilegeEntry(role, "regions", "SELECT"), ] ) + + +def test_apply_grants_and_verify_checks_privileges_on_a_declared_view( + superuser_engine, temporary_role, temporary_view +) -> None: + """A view declared like a table is found, granted and verified.""" + role = temporary_role() + declared = {role: {temporary_view: READ_WRITE}} + with superuser_engine.begin() as connection: + plan, _defaults = apply_declared_privileges( + connection, declared, declared_views={temporary_view} + ) + missing = check_declared_privileges( + connection, declared, declared_views={temporary_view} + ) + + assert plan.missing_tables == plan.missing_views == () + assert {entry.privilege for entry in plan.grants} == { + privilege.value for privilege in READ_WRITE + } + assert missing == [] + for privilege in READ_WRITE: + assert _has_privilege(superuser_engine, role, temporary_view, privilege.value) + + +def test_a_view_declared_as_a_table_or_a_table_declared_as_a_view_blocks_grants( + superuser_engine, temporary_role, temporary_view +) -> None: + """Each declared name must exist with its declared kind. + + ``regions`` is a table, declared here as a view, and the temporary view is + declared as a table: neither is granted, and both fail verification. + """ + role = temporary_role() + declared = {role: {"regions": READ, temporary_view: READ}} + with superuser_engine.begin() as connection: + plan, _defaults = apply_declared_privileges( + connection, declared, declared_views={"regions"} + ) + missing = check_declared_privileges( + connection, declared, declared_views={"regions"} + ) + + assert plan.missing_views == ("regions",) + assert plan.missing_tables == (temporary_view,) + assert not plan.can_apply + assert not _has_privilege(superuser_engine, role, "regions", "SELECT") + assert missing == sorted( + [ + PrivilegeEntry(role, "regions", "SELECT"), + PrivilegeEntry(role, temporary_view, "SELECT"), + ] + ) diff --git a/tests/integration/test_v2_saved_policy_view.py b/tests/integration/test_v2_saved_policy_view.py new file mode 100644 index 000000000..7ffb666d5 --- /dev/null +++ b/tests/integration/test_v2_saved_policy_view.py @@ -0,0 +1,382 @@ +"""The release before the saved-policy rename keeps working through the view. + +Revision ``fa7fa1035444`` renames ``user_policies`` to +``user_policy_associations`` and keeps a temporary view under the old name. The +Public API release that still serves during the deploy, or after a rollback to +it, runs the same statements as today's services with the old table name. +These tests run today's services as the runtime role, rewrite each statement's +table name to ``user_policies`` before it is sent, as that release would +compile it, and check the result in the renamed table. +""" + +from __future__ import annotations + +import re +from typing import TYPE_CHECKING +from uuid import UUID, uuid4 + +import pytest +from sqlalchemy import create_engine, delete, event, select, text +from sqlalchemy.orm import sessionmaker +from sqlmodel import Session + +from policyengine_api.constants import COUNTRY_PACKAGE_VERSIONS, POLICYENGINE_VERSION +from policyengine_api.data.v2.models import ( + LegacyPolicyMapping, + LegacyUserMapping, + Parameter, + ParameterValue, + Policy, + TaxBenefitModel, + TaxBenefitModelVersion, + User, + UserIdentity, +) +from policyengine_api.services.v2.identity.types import ( + UnauthenticatedPrincipal, + UserPrincipal, +) +from policyengine_api.services.v2.policies.types import LegacyPolicySnapshot +from policyengine_api.services.v2.user_accounts.database_session import ( + AccountDatabaseSession, +) +from policyengine_api.services.v2.user_accounts.services import V2AccountService +from policyengine_api.services.v2.user_policies.database_session import ( + UserPolicyDatabaseSession, +) +from policyengine_api.services.v2.user_policies.services import ( + V2UserPolicyService, + mirror_legacy_user_policy_in_session, +) +from policyengine_api.services.v2.user_policies.types import ( + LegacyUserPolicySnapshot, + UserPolicyCreateFields, + UserPolicyUpdateInput, +) +from policyengine_api.services.v2.user_reports.database_session import ( + UserReportDatabaseSession, +) +from policyengine_api.services.v2.user_reports.services import V2UserReportService +from policyengine_api.services.v2.user_reports.types import UserReportCreateFields +from tests.unit.v2.user_service_support import default_legacy_references + +if TYPE_CHECKING: + from collections.abc import Iterator + + from sqlalchemy import Engine + +TABLE_BEFORE_THE_RENAME = "user_policies" +TABLE_AFTER_THE_RENAME = "user_policy_associations" +# The renamed table where a statement names it as a table, not inside a label +# such as ``user_policy_associations_id``. +RENAMED_TABLE_REFERENCE = re.compile(rf"\b{TABLE_AFTER_THE_RENAME}\b") +MIRRORED_LEGACY_USER_ID = "auth0|saved-policy-view" + + +@pytest.fixture +def cleanup_engine(disposable_v2_database_url) -> Iterator[Engine]: + engine = create_engine(disposable_v2_database_url) + try: + yield engine + finally: + engine.dispose() + + +@pytest.fixture +def previous_release_statements() -> list[str]: + """Every statement on saved policies, as sent with the earlier table name.""" + return [] + + +@pytest.fixture +def sessions( + managed_v2_roles, v2_engine_as_role, cleanup_engine, previous_release_statements +) -> Iterator[sessionmaker[Session]]: + """Open sessions as the runtime role that name saved policies as before. + + Each statement that names ``user_policy_associations`` is sent naming + ``user_policies`` instead. Results are matched to columns by position, so + the rewritten labels do not matter. + """ + runtime_engine = v2_engine_as_role(managed_v2_roles.runtime) + + @event.listens_for(runtime_engine, "before_cursor_execute", retval=True) + def name_saved_policies_as_before_the_rename( + _connection, _cursor, statement, parameters, _context, _executemany + ): + if RENAMED_TABLE_REFERENCE.search(statement): + statement = RENAMED_TABLE_REFERENCE.sub(TABLE_BEFORE_THE_RENAME, statement) + previous_release_statements.append(statement) + return statement, parameters + + factory = sessionmaker(runtime_engine, class_=Session, expire_on_commit=False) + issuer = f"https://saved-policy-view-{uuid4().hex[:12]}.example.test/" + factory.kw["info"] = {"issuer": issuer, "browser_user_ids": []} + try: + yield factory + finally: + with cleanup_engine.begin() as connection: + owned = select(UserIdentity.user_id).where(UserIdentity.issuer == issuer) + user_ids = [ + *connection.execute(owned).scalars(), + *factory.kw["info"]["browser_user_ids"], + ] + if user_ids: + connection.execute(delete(User).where(User.id.in_(user_ids))) + runtime_engine.dispose() + + +def _browser(sessions) -> UnauthenticatedPrincipal: + user_id = uuid4() + sessions.kw["info"]["browser_user_ids"].append(user_id) + return UnauthenticatedPrincipal(user_id=user_id) + + +def _saved_policy_row(cleanup_engine, association_id: UUID) -> dict | None: + with cleanup_engine.connect() as connection: + row = ( + connection.execute( + text(f"SELECT * FROM {TABLE_AFTER_THE_RENAME} WHERE id = :id"), + {"id": association_id}, + ) + .mappings() + .one_or_none() + ) + return None if row is None else dict(row) + + +def _statement_kinds(statements: list[str]) -> set[str]: + """Return the kind of each statement: its first word.""" + return {statement.split(maxsplit=1)[0].upper() for statement in statements} + + +def test_runtime_role_holds_its_saved_policy_privileges_on_the_view( + sessions, managed_v2_roles +) -> None: + with sessions() as session: + held = { + privilege: session.execute( + text("SELECT has_table_privilege(:role, :view, :privilege)"), + { + "role": managed_v2_roles.runtime, + "view": f"public.{TABLE_BEFORE_THE_RENAME}", + "privilege": privilege, + }, + ).scalar_one() + for privilege in ("SELECT", "INSERT", "UPDATE", "DELETE") + } + + assert held == dict.fromkeys(("SELECT", "INSERT", "UPDATE", "DELETE"), True) + + +def test_previous_release_saves_reads_renames_and_deletes_through_the_view( + sessions, cleanup_engine, previous_release_statements +) -> None: + browser = _browser(sessions) + policies = V2UserPolicyService( + UserPolicyDatabaseSession(sessions), default_legacy_references() + ) + + created = policies.create_user_policy( + principal=browser, + association_input=UserPolicyCreateFields( + country_id="us", legacy_policy_id=1, name="Saved" + ), + ) + stored_after_create = _saved_policy_row(cleanup_engine, created.id) + read = policies.get_user_policy( + principal=browser, country_id="us", association_id=created.id + ) + page = policies.list_user_policies(principal=browser, country_id="us") + renamed = policies.patch_user_policy( + principal=browser, + country_id="us", + association_id=created.id, + association_input=UserPolicyUpdateInput(name="Renamed"), + ) + stored_after_rename = _saved_policy_row(cleanup_engine, created.id) + policies.delete_user_policy( + principal=browser, country_id="us", association_id=created.id + ) + + assert stored_after_create is not None + assert stored_after_create["user_id"] == browser.user_id + assert stored_after_create["created_at"] == created.created_at + assert read.id == created.id + assert [association.id for association in page.items] == [created.id] + assert renamed.name == "Renamed" + assert stored_after_rename is not None + assert stored_after_rename["name"] == "Renamed" + assert _saved_policy_row(cleanup_engine, created.id) is None + assert _statement_kinds(previous_release_statements) == { + "INSERT", + "SELECT", + "UPDATE", + "DELETE", + } + + +def test_previous_release_merges_saved_policies_and_checks_reports_through_the_view( + sessions, cleanup_engine, previous_release_statements +) -> None: + """Sign-in moves a browser's saved policies, and reports name them.""" + browser = _browser(sessions) + signed_in = UserPrincipal( + issuer=sessions.kw["info"]["issuer"], subject="google-oauth2|view-merge" + ) + references = default_legacy_references() + policies = V2UserPolicyService(UserPolicyDatabaseSession(sessions), references) + reports = V2UserReportService(UserReportDatabaseSession(sessions), references) + accounts = V2AccountService(AccountDatabaseSession(sessions)) + + account_id = accounts.get_account(principal=signed_in, country_id="us").id + saved = policies.create_user_policy( + principal=browser, + association_input=UserPolicyCreateFields(country_id="us", legacy_policy_id=1), + ) + report = reports.create_user_report( + principal=browser, + association_input=UserReportCreateFields( + country_id="us", + legacy_report_id=615, + user_policy_id=saved.id, + baseline_user_policy_id=saved.id, + ), + ) + accounts.upgrade_account( + principal=signed_in, + unauthenticated_user_id=browser.user_id, + country_id="us", + ) + exported = accounts.export_account(principal=signed_in, country_id="us") + + stored = _saved_policy_row(cleanup_engine, saved.id) + assert stored is not None + assert stored["user_id"] == account_id + assert report.user_policy_id == saved.id + assert [policy.id for policy in exported.policies] == [saved.id] + # The sign-in merge moves every saved policy of the browser's user. + assert any( + re.match( + rf"UPDATE {TABLE_BEFORE_THE_RENAME} SET .*\buser_id=.* WHERE " + rf"{TABLE_BEFORE_THE_RENAME}\.user_id = ", + statement, + ) + for statement in previous_release_statements + ) + + +def _seed_catalog(cleanup_engine) -> tuple[UUID, str]: + parameter_name = f"gov.saved_policy_view.rate_{uuid4().hex[:8]}" + with Session(cleanup_engine) as session, session.begin(): + # The policy copy selects the catalog by this model name. + model = TaxBenefitModel(name="policyengine-us") + version = TaxBenefitModelVersion( + model=model, + version=POLICYENGINE_VERSION, + current_law_id=1, + metadata_time_periods=[2026], + ) + session.add(Parameter(name=parameter_name, tax_benefit_model_version=version)) + session.flush() + return model.id, parameter_name + + +def _delete_mirrored_rows(cleanup_engine, model_id: UUID) -> None: + with cleanup_engine.begin() as connection: + user_ids = select(LegacyUserMapping.user_id).where( + LegacyUserMapping.legacy_user_id == MIRRORED_LEGACY_USER_ID + ) + mirrored_user_ids = connection.execute(user_ids).scalars().all() + connection.execute( + delete(LegacyUserMapping).where( + LegacyUserMapping.legacy_user_id == MIRRORED_LEGACY_USER_ID + ) + ) + # Deleting the user deletes its saved policies and their v1 mappings. + connection.execute(delete(User).where(User.id.in_(mirrored_user_ids))) + policy_ids = select(Policy.id).where(Policy.tax_benefit_model_id == model_id) + for model in (LegacyPolicyMapping, ParameterValue): + connection.execute(delete(model).where(model.policy_id.in_(policy_ids))) + connection.execute( + delete(Policy).where(Policy.tax_benefit_model_id == model_id) + ) + version_ids = select(TaxBenefitModelVersion.id).where( + TaxBenefitModelVersion.model_id == model_id + ) + connection.execute( + delete(Parameter).where( + Parameter.tax_benefit_model_version_id.in_(version_ids) + ) + ) + connection.execute( + delete(TaxBenefitModelVersion).where( + TaxBenefitModelVersion.model_id == model_id + ) + ) + connection.execute( + delete(TaxBenefitModel).where(TaxBenefitModel.id == model_id) + ) + + +def _legacy_saved_policy(reform_label: str) -> LegacyUserPolicySnapshot: + return LegacyUserPolicySnapshot.model_validate( + { + "country_id": "us", + "legacy_user_policy_id": 901, + "reform_id": 801, + "reform_label": reform_label, + "baseline_id": 1, + "baseline_label": "Current law", + "user_id": MIRRORED_LEGACY_USER_ID, + "year": "2026", + "geography": "us", + "dataset": "enhanced_cps_2024", + "number_of_provisions": 1, + "api_version": "1.0.0", + "added_date": 1, + "updated_date": 2, + "budgetary_impact": None, + "type": None, + } + ) + + +def test_previous_release_copies_v1_saved_policies_through_the_view( + sessions, cleanup_engine, previous_release_statements +) -> None: + """The Stage 10 saved-policy copy inserts and then updates through the view.""" + model_id, parameter_name = _seed_catalog(cleanup_engine) + reform = LegacyPolicySnapshot( + country_id="us", + legacy_policy_id=801, + label="Ignored legacy label", + api_version=COUNTRY_PACKAGE_VERSIONS["us"], + policy_json={parameter_name: {"2026": 0.2}}, + source_policy_hash=f"view-{uuid4().hex}", + ) + try: + with sessions.begin() as session: + first = mirror_legacy_user_policy_in_session( + session, _legacy_saved_policy("Copied"), reform, source_revision=1 + ) + with sessions.begin() as session: + second = mirror_legacy_user_policy_in_session( + session, + _legacy_saved_policy("Copied again"), + reform, + source_revision=2, + changed_fields=frozenset({"reform_label"}), + ) + stored = _saved_policy_row(cleanup_engine, first.association_id) + finally: + _delete_mirrored_rows(cleanup_engine, model_id) + + assert first.association_created is True + assert second.association_id == first.association_id + assert second.association_created is False + assert stored is not None + assert stored["name"] == "Copied again" + assert {"INSERT", "SELECT", "UPDATE"} <= _statement_kinds( + previous_release_statements + ) diff --git a/tests/unit/test_alembic_workflows.py b/tests/unit/test_alembic_workflows.py index 797c82848..a0e85c6c5 100644 --- a/tests/unit/test_alembic_workflows.py +++ b/tests/unit/test_alembic_workflows.py @@ -3,6 +3,7 @@ import os import re import subprocess +import tomllib from pathlib import Path import pytest @@ -257,6 +258,41 @@ def test_v2_integration_check_grants_and_verifies_privileges_before_tests(): assert schema_upgrade < grant < verify < first_test +def _integration_modules_using(pattern: str) -> list[str]: + return sorted( + path.relative_to(REPO).as_posix() + for path in (REPO / "tests" / "integration").glob("test_*.py") + if re.search(pattern, path.read_text(encoding="utf-8")) + ) + + +def test_every_managed_role_integration_module_runs_in_ci_and_is_in_its_rule(): + """Integration modules that use the managed v2 roles run in the workflow. + + Those that run services as a managed role (``v2_engine_as_role``) are also + in the scope of rule ``api.v2.runtime-privileges-tested-as-runtime-role``. + """ + workflow = _workflow("v2-integration-check.yml") + rules = tomllib.loads((REPO / "rules" / "registry.toml").read_text()) + (runtime_role_rule,) = ( + rule + for rule in rules["rule"] + if rule["id"] == "api.v2.runtime-privileges-tested-as-runtime-role" + ) + managed_role_modules = _integration_modules_using( + r"\b(managed_v2_roles|v2_engine_as_role)\b" + ) + role_session_modules = _integration_modules_using(r"\bv2_engine_as_role\b") + + assert "tests/integration/test_v2_saved_policy_view.py" in role_session_modules + assert [module for module in managed_role_modules if module not in workflow] == [] + assert [ + module + for module in role_session_modules + if module not in runtime_role_rule["scope"] + ] == [] + + def test_release_migration_fails_closed_before_tests_and_cloud_run_deploy(): workflow = _workflow("push.yml") orchestration_script = ( diff --git a/tests/unit/v2/test_alembic_v2.py b/tests/unit/v2/test_alembic_v2.py index 4c0e6ee09..94d79f577 100644 --- a/tests/unit/v2/test_alembic_v2.py +++ b/tests/unit/v2/test_alembic_v2.py @@ -54,6 +54,18 @@ f"{PROJECT_REF}:test-password@aws-0-us-east-2.pooler.supabase.com:5432/" "postgres?sslmode=require" ) +# The saved-policy rename and its parent, the Stage 12A revision. +SAVED_POLICY_RENAME_REVISION = "fa7fa1035444" +SAVED_POLICY_RENAME_PARENT_REVISION = "fbb7488ebe8a" +# The only hand-written statements in any v2 revision: the approved temporary +# view under the saved-policy table's earlier name, and its removal on +# downgrade. +SAVED_POLICY_VIEW_CREATION = ( + "CREATE VIEW user_policies AS SELECT id, created_at, updated_at, user_id, " + "policy_id, country_id, name, description, legacy_policy_id, baseline, " + "provenance FROM user_policy_associations" +) +SAVED_POLICY_VIEW_REMOVAL = "DROP VIEW user_policies" DISPOSABLE_URL = ( "postgresql+psycopg://postgres:test-password@127.0.0.1:5432/" f"{DISPOSABLE_DATABASE_NAME}" @@ -235,6 +247,7 @@ def test_v2_revision_chain_has_generated_policy_and_user_identity_changes() -> N assert script.get_heads() == [V2_ALEMBIC_HEAD_REVISION] assert [revision.revision for revision in script.walk_revisions()] == [ V2_ALEMBIC_HEAD_REVISION, + SAVED_POLICY_RENAME_PARENT_REVISION, "91af4fc8d8c1", "60d6518b6a98", "439303be14fe", @@ -495,7 +508,6 @@ def test_stage_12a_user_identity_and_association_revision_is_generated() -> None / "migrations/v2/versions/fbb7488ebe8a_link_signed_in_users_to_v1_anchored_.py" ).read_text(encoding="utf-8") - assert V2_ALEMBIC_HEAD_REVISION == "fbb7488ebe8a" assert ( "Generation: uv run alembic -c alembic-v2.ini revision --autogenerate" in revision @@ -739,3 +751,61 @@ def test_v2_environment_commits_after_persistent_target_qualification() -> None: assert "current_heads != previous_heads" in env_source assert "current_heads == script_heads" in env_source assert "validate_v2_head_schema(connection, target_metadata)" in env_source + + +def _executed_statements(body: str) -> list[str]: + """Return the SQL each ``op.execute`` call in a revision body runs.""" + return [ + "".join(re.findall(r'"([^"]*)"', call)) + for call in re.findall(r"op\.execute\((.*?)\)\n", body, flags=re.DOTALL) + ] + + +def test_saved_policy_rename_revision_renames_the_table_in_place() -> None: + """The rename keeps every saved reform: no table is created or dropped. + + Autogeneration replaces a renamed table with a new empty one; the revision + records its corrections, and every other operation is generated except the + approved temporary view under the old name, created after the rename and + dropped before the downgrade renames the table back. + """ + script = ScriptDirectory.from_config(Config(str(REPO / "alembic-v2.ini"))) + rename = script.get_revision(SAVED_POLICY_RENAME_REVISION) + revision = Path(rename.path).read_text(encoding="utf-8") + upgrade = revision[ + revision.index("def upgrade()") : revision.index("def downgrade()") + ] + downgrade = revision[revision.index("def downgrade()") :] + rename_up = 'op.rename_table("user_policies", "user_policy_associations")' + rename_down = 'op.rename_table("user_policy_associations", "user_policies")' + + assert rename.down_revision == SAVED_POLICY_RENAME_PARENT_REVISION + assert ( + "Generation: uv run alembic -c alembic-v2.ini revision --autogenerate" + in revision + ) + assert rename_up in upgrade + assert rename_down in downgrade + for body in (upgrade, downgrade): + assert "op.create_table(" not in body + assert "op.drop_table(" not in body + assert "op.bulk_insert(" not in body + assert body.count("Approved hand-written exception:") == 1 + assert _executed_statements(upgrade) == [SAVED_POLICY_VIEW_CREATION] + assert _executed_statements(downgrade) == [SAVED_POLICY_VIEW_REMOVAL] + assert upgrade.index(rename_up) < upgrade.index("op.execute(") + assert downgrade.index("op.execute(") < downgrade.index(rename_down) + assert upgrade.count("Post-generation correction:") == 1 + assert downgrade.count("Post-generation correction:") == 1 + + +def test_no_other_v2_revision_runs_hand_written_sql() -> None: + """Only the saved-policy rename's approved view uses ``op.execute``.""" + script = ScriptDirectory.from_config(Config(str(REPO / "alembic-v2.ini"))) + revisions_with_sql = { + revision.revision + for revision in script.walk_revisions() + if "op.execute(" in Path(revision.path).read_text(encoding="utf-8") + } + + assert revisions_with_sql == {SAVED_POLICY_RENAME_REVISION} diff --git a/tests/unit/v2/test_models.py b/tests/unit/v2/test_models.py index 20947ffcb..17b87ec03 100644 --- a/tests/unit/v2/test_models.py +++ b/tests/unit/v2/test_models.py @@ -130,7 +130,7 @@ def test_user_owned_associations_have_relational_integrity() -> None: "user_simulation_associations", "simulation_associations", ), - (UserPolicy, "user_policies", "policy_associations"), + (UserPolicy, "user_policy_associations", "policy_associations"), (UserReportAssociation, "user_report_associations", "report_associations"), ) @@ -195,7 +195,7 @@ def test_policy_parameter_values_use_jsonb_and_enforce_period_identity() -> None def test_user_policy_is_an_independent_country_scoped_association() -> None: - associations = V2_METADATA.tables["user_policies"] + associations = V2_METADATA.tables["user_policy_associations"] assert isinstance(associations.c.user_id.type, sa.Uuid) user_foreign_key = next(iter(associations.c.user_id.foreign_keys)) @@ -215,7 +215,7 @@ def test_user_policy_is_an_independent_country_scoped_association() -> None: policy_country = next( constraint for constraint in associations.foreign_key_constraints - if constraint.name == "fk_user_policies_policy_country" + if constraint.name == "fk_user_policy_associations_policy_country" ) assert [column.name for column in policy_country.columns] == [ "policy_id", @@ -571,7 +571,7 @@ def test_user_identity_maps_one_issuer_subject_to_one_user() -> None: def test_associations_are_v1_anchored_without_client_refs() -> None: for table_name, reference in ( - ("user_policies", "policy"), + ("user_policy_associations", "policy"), ("user_household_associations", "household"), ("user_simulation_associations", "simulation"), ("user_report_associations", "report"), @@ -610,7 +610,7 @@ def test_simulation_and_report_associations_have_no_cross_service_keys() -> None } == {"users.id"} assert {foreign_key.target_fullname for foreign_key in reports.foreign_keys} == { "users.id", - "user_policies.id", + "user_policy_associations.id", } user_policy_key = next(iter(reports.c.user_policy_id.foreign_keys)) assert user_policy_key.ondelete == "SET NULL" @@ -623,15 +623,56 @@ def test_simulation_and_report_associations_have_no_cross_service_keys() -> None def test_user_policy_records_v3_reform_fields() -> None: - policies = V2_METADATA.tables["user_policies"] + policies = V2_METADATA.tables["user_policy_associations"] assert policies.c.baseline.type.length == 16 baseline_check = next( constraint for constraint in policies.constraints - if constraint.name == "ck_user_policies_baseline" + if constraint.name == "ck_user_policy_associations_baseline" ) assert "current-law" in str(baseline_check.sqltext) assert "current-policy" in str(baseline_check.sqltext) postgres_type = policies.c.provenance.type.dialect_impl(postgresql.dialect()) assert isinstance(postgres_type, postgresql.JSONB) + + +def test_saved_policy_table_is_named_like_the_other_association_tables() -> None: + """The saved-reform table and its constraints follow the association pattern. + + Composite indexes keep their short ``ix_user_policies_*`` names, as the + household, simulation and report associations keep ``ix_user_households_*`` + and the like; constraints and convention-named indexes take the table name. + """ + assert UserPolicy.__tablename__ == "user_policy_associations" + assert "user_policies" not in V2_METADATA.tables + table = V2_METADATA.tables["user_policy_associations"] + + assert {constraint.name for constraint in table.constraints} == { + "pk_user_policy_associations", + "uq_user_policy_associations_id_country", + "ck_user_policy_associations_country", + "ck_user_policy_associations_policy_ref", + "ck_user_policy_associations_baseline", + "fk_user_policy_associations_policy_country", + "fk_user_policy_associations_user_id_users", + } + assert {index.name for index in table.indexes} == { + "ix_user_policy_associations_user_id", + "ix_user_policy_associations_policy_id", + "ix_user_policies_country_user_created_id", + "ix_user_policies_country_policy", + "ix_user_policies_country_legacy_policy", + } + referring_keys = { + (foreign_key.parent.table.name, foreign_key.parent.name) + for referring_table in V2_METADATA.tables.values() + for foreign_key in referring_table.foreign_keys + if foreign_key.column.table is table + } + assert referring_keys == { + ("legacy_user_policy_mappings", "user_policy_id"), + ("legacy_user_policy_mappings", "country_id"), + ("user_report_associations", "user_policy_id"), + ("user_report_associations", "baseline_user_policy_id"), + } diff --git a/tests/unit/v2/test_policy_migration_qualification.py b/tests/unit/v2/test_policy_migration_qualification.py index 54a711ef4..dc28306a5 100644 --- a/tests/unit/v2/test_policy_migration_qualification.py +++ b/tests/unit/v2/test_policy_migration_qualification.py @@ -175,3 +175,29 @@ def fail() -> None: assert "qualification failed unexpectedly" in captured.err assert "secret" not in captured.err assert "private-host" not in captured.err + + +class RecordingExecutor: + def __init__(self) -> None: + self.statements: list[str] = [] + + def scalar(self, statement: object) -> int: + self.statements.append(str(statement)) + return 0 + + +def test_saved_policies_are_counted_under_the_name_of_the_pending_revision() -> None: + """The policy revision is pending only before the rename of ``user_policies``. + + Counting through the current model would query ``user_policy_associations``, + which a database still waiting for the policy revision does not have yet. + """ + executor = RecordingExecutor() + + qualification.read_policy_data_counts( + executor, existing_tables={"policies", "parameter_values", "user_policies"} + ) + + saved_policy_count = executor.statements[2] + assert "FROM user_policies" in saved_policy_count + assert "user_policy_associations" not in saved_policy_count diff --git a/tests/unit/v2/test_privilege_manifest.py b/tests/unit/v2/test_privilege_manifest.py index 7a3a76b44..380d9a81e 100644 --- a/tests/unit/v2/test_privilege_manifest.py +++ b/tests/unit/v2/test_privilege_manifest.py @@ -22,6 +22,7 @@ READ_WRITE, RUNTIME_ROLE, TABLE_ACCESS, + TEMPORARY_VIEW_TABLES, InvalidDeploymentEnvironmentError, TablePrivilege, declared_privileges, @@ -72,7 +73,28 @@ def test_every_v2_table_has_an_entry_for_every_managed_role() -> None: assert set(TABLE_ACCESS) == metadata_tables | {"alembic_version"} for role, privileges_by_table in declared_privileges(TEST_ENVIRONMENT).items(): - assert set(privileges_by_table) == set(TABLE_ACCESS), role + assert set(privileges_by_table) == set(TABLE_ACCESS) | set( + TEMPORARY_VIEW_TABLES + ), role + + +def test_each_temporary_view_grants_every_role_what_its_table_grants() -> None: + """A view under a renamed table's earlier name serves the earlier release. + + That release reads and writes the table through the view, so each role + needs on the view exactly what it holds on the table. + """ + metadata_tables = {table.name for table in V2_METADATA.tables.values()} + + assert TEMPORARY_VIEW_TABLES == {"user_policies": "user_policy_associations"} + for view, table in TEMPORARY_VIEW_TABLES.items(): + assert view not in metadata_tables + assert table in metadata_tables + for role, privileges_by_table in declared_privileges(TEST_ENVIRONMENT).items(): + assert privileges_by_table[view] == privileges_by_table[table], role + assert declared_privileges(TEST_ENVIRONMENT)[RUNTIME_ROLE]["user_policies"] == ( + READ_WRITE + ) def test_v2_schema_has_no_sequences_the_manifest_would_need_to_cover() -> None: @@ -133,7 +155,7 @@ def test_rule_proof_runtime_privileges_declared() -> None: reads ``legacy_policy_mappings``, which the runtime role had no grant on. """ runtime_before_3977 = dict.fromkeys(TABLE_ACCESS, NO_ACCESS) | { - "user_policies": READ_WRITE + "user_policy_associations": READ_WRITE } table_by_model = _table_by_model() diff --git a/tests/unit/v2/test_privilege_reconciliation.py b/tests/unit/v2/test_privilege_reconciliation.py index 73ca30e19..a6d9590dc 100644 --- a/tests/unit/v2/test_privilege_reconciliation.py +++ b/tests/unit/v2/test_privilege_reconciliation.py @@ -11,6 +11,7 @@ from policyengine_api.data.v2.privilege_reconciliation import ( DefaultPrivilege, PrivilegeEntry, + PublicRelations, ReconciliationPlan, grant_statements, plan_reconciliation, @@ -48,12 +49,16 @@ def _declared() -> dict[str, dict[str, frozenset[TablePrivilege]]]: } +def _tables(names: set[str]) -> PublicRelations: + return PublicRelations(tables=frozenset(names), views=frozenset()) + + def test_missing_declared_privileges_become_grants() -> None: plan = plan_reconciliation( _declared(), granted={RUNTIME: {"users": {"SELECT", "INSERT"}}}, existing_roles={RUNTIME, STAGE12}, - existing_tables={"users", "legacy_policy_mappings", "reports"}, + existing_relations=_tables({"users", "legacy_policy_mappings", "reports"}), ) assert plan.grants == ( @@ -78,7 +83,7 @@ def test_privileges_beyond_the_manifest_are_reported_not_granted_or_revoked() -> STAGE12: {"users": {"SELECT"}}, }, existing_roles={RUNTIME, STAGE12}, - existing_tables={"users", "legacy_policy_mappings", "reports"}, + existing_relations=_tables({"users", "legacy_policy_mappings", "reports"}), ) assert plan.grants == () @@ -96,7 +101,9 @@ def test_privileges_on_tables_outside_the_manifest_are_reported() -> None: _declared(), granted={RUNTIME: {"scratch_table": {"SELECT"}}}, existing_roles={RUNTIME, STAGE12}, - existing_tables={"users", "legacy_policy_mappings", "reports", "scratch_table"}, + existing_relations=_tables( + {"users", "legacy_policy_mappings", "reports", "scratch_table"} + ), ) assert PrivilegeEntry(RUNTIME, "scratch_table", "SELECT") in plan.undeclared @@ -107,7 +114,7 @@ def test_missing_roles_and_tables_are_recorded_and_block_the_plan() -> None: _declared(), granted={}, existing_roles={RUNTIME}, - existing_tables={"users", "reports"}, + existing_relations=_tables({"users", "reports"}), ) assert plan.missing_roles == (STAGE12,) @@ -120,6 +127,71 @@ def test_missing_roles_and_tables_are_recorded_and_block_the_plan() -> None: ) +def test_a_relation_of_the_other_kind_does_not_satisfy_a_declaration() -> None: + """A declared table must be a table, and a declared view must be a view. + + Before the rename, ``user_policies`` is a table, which must not satisfy the + temporary view of that name; a view named like a declared table must not + satisfy the table either. + """ + declared = { + RUNTIME: {"users": READ_WRITE, "user_policies": READ_WRITE, "reports": READ} + } + + plan = plan_reconciliation( + declared, + granted={}, + existing_roles={RUNTIME}, + existing_relations=PublicRelations( + tables=frozenset({"user_policies", "reports"}), views=frozenset({"users"}) + ), + declared_views={"user_policies"}, + ) + + assert plan.missing_tables == ("users",) + assert plan.missing_views == ("user_policies",) + assert not plan.can_apply + + +def test_declared_views_that_exist_as_views_are_granted() -> None: + declared = {RUNTIME: {"users": READ, "user_policies": READ_WRITE}} + + plan = plan_reconciliation( + declared, + granted={}, + existing_roles={RUNTIME}, + existing_relations=PublicRelations( + tables=frozenset({"users"}), views=frozenset({"user_policies"}) + ), + declared_views={"user_policies"}, + ) + + assert plan.can_apply + assert plan.missing_tables == plan.missing_views == () + assert {entry.table for entry in plan.grants} == {"users", "user_policies"} + + +def test_summary_names_missing_views_separately_from_missing_tables() -> None: + plan = ReconciliationPlan( + grants=(), + undeclared=(), + missing_roles=(), + missing_tables=("users",), + missing_views=("user_policies",), + ) + + summary = render_summary(plan, (), deployment_environment="staging", applied=False) + + assert ( + "Declared tables missing from the database (nothing was granted): `users`" + in summary + ) + assert ( + "Declared views missing from the database (nothing was granted): " + "`user_policies`" in summary + ) + + def test_grant_statements_group_privileges_per_role_and_table() -> None: plan = ReconciliationPlan( grants=( diff --git a/tests/unit/v2/test_privilege_requirements.py b/tests/unit/v2/test_privilege_requirements.py index 76ba0b5eb..314a85aff 100644 --- a/tests/unit/v2/test_privilege_requirements.py +++ b/tests/unit/v2/test_privilege_requirements.py @@ -20,7 +20,7 @@ TABLE_BY_MODEL = { "User": "users", - "UserPolicy": "user_policies", + "UserPolicy": "user_policy_associations", "LegacyPolicyMapping": "legacy_policy_mappings", } @@ -72,8 +72,8 @@ def create(session, values): ) assert _needed(path) == { - ("user_policies", TablePrivilege.INSERT), - ("user_policies", TablePrivilege.SELECT), + ("user_policy_associations", TablePrivilege.INSERT), + ("user_policy_associations", TablePrivilege.SELECT), } @@ -169,5 +169,5 @@ def read_with_owner(session): """, ) - assert ("user_policies", TablePrivilege.UPDATE) in _needed(path) + assert ("user_policy_associations", TablePrivilege.UPDATE) in _needed(path) assert ("users", TablePrivilege.UPDATE) not in _needed(path) diff --git a/tests/unit/v2/test_user_association_migration_qualification.py b/tests/unit/v2/test_user_association_migration_qualification.py index 463a073f9..ac3997cac 100644 --- a/tests/unit/v2/test_user_association_migration_qualification.py +++ b/tests/unit/v2/test_user_association_migration_qualification.py @@ -143,3 +143,48 @@ def fail(): payload = json.loads(capsys.readouterr().err) assert payload["outcome"] == "error" assert "secret" not in json.dumps(payload) + + +def test_saved_policies_are_counted_under_the_name_of_the_pending_revision() -> None: + """The reshape is pending only before the later rename of ``user_policies``. + + Counting through the current model would query ``user_policy_associations``, + which a database still waiting for the reshape does not have yet. + """ + _evidence, connection, _engine = _qualify((0, 0, 0, 0), pending=True) + + saved_policy_count = connection.statements[0] + assert "FROM user_policies" in saved_policy_count + assert "user_policy_associations" not in saved_policy_count + + +class RecordingExecutor: + def __init__(self) -> None: + self.statements: list[str] = [] + + def scalar(self, statement: object) -> int: + self.statements.append(str(statement)) + return 3 + + +def test_saved_policies_are_counted_under_their_name_after_the_rename() -> None: + """Once ``user_policies`` is renamed, its rows are counted under the new name. + + The command runs before every upgrade, so it meets the table under either + name; reading only the old name would report no saved policies. + """ + executor = RecordingExecutor() + + counts = qualification.read_user_association_counts( + executor, + existing_tables={ + "user_policy_associations", + "user_household_associations", + "user_simulation_associations", + "user_report_associations", + }, + ) + + assert counts.user_policies == 3 + saved_policy_count = executor.statements[0] + assert "FROM user_policy_associations" in saved_policy_count diff --git a/tests/unit/v2/test_user_policy_legacy_persistence.py b/tests/unit/v2/test_user_policy_legacy_persistence.py index e52134e67..a6911a0db 100644 --- a/tests/unit/v2/test_user_policy_legacy_persistence.py +++ b/tests/unit/v2/test_user_policy_legacy_persistence.py @@ -44,13 +44,15 @@ SELECT_MAPPING_FOR_UPDATE = f"{SELECT_MAPPING} FOR UPDATE" MAPPING_LOOKUP = {"country_id_1": "us", "legacy_user_policy_id_1": 10} SELECT_MAPPED_ASSOCIATION = ( - "SELECT user_policies.id, user_policies.created_at, user_policies.updated_at, " - "user_policies.user_id, user_policies.policy_id, " - "user_policies.legacy_policy_id, user_policies.country_id, " - "user_policies.name, user_policies.description, user_policies.baseline, " - "user_policies.provenance \nFROM user_policies \n" - "WHERE user_policies.id = %(id_1)s::UUID AND " - "user_policies.country_id = %(country_id_1)s::VARCHAR" + "SELECT user_policy_associations.id, user_policy_associations.created_at, " + "user_policy_associations.updated_at, user_policy_associations.user_id, " + "user_policy_associations.policy_id, " + "user_policy_associations.legacy_policy_id, " + "user_policy_associations.country_id, user_policy_associations.name, " + "user_policy_associations.description, user_policy_associations.baseline, " + "user_policy_associations.provenance \nFROM user_policy_associations \n" + "WHERE user_policy_associations.id = %(id_1)s::UUID AND " + "user_policy_associations.country_id = %(country_id_1)s::VARCHAR" ) INSERT_MAPPING = ( "INSERT INTO legacy_user_policy_mappings (id, created_at, updated_at, "