-
Notifications
You must be signed in to change notification settings - Fork 1
fix: empty reconstruction children during granted retention purge #177
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -76,6 +76,14 @@ operators who purge from `psql`. Do not expose purge on a public HTTP | |
| route. Split the application login from the migration owner so the | ||
| product role cannot execute the function even as table owner. | ||
|
|
||
| Start reconstruction (ADR 0021) adds `analysis_run_lineage_edge`, | ||
| `analysis_run_reconstruction`, and `analysis_source_snapshot_member` | ||
| with delete-reject triggers. This procedure already disables those | ||
| user triggers when `to_regclass` finds the tables, deletes children | ||
| before the 0018 rows, then re-enables the triggers. A 0.87.0-only | ||
| database without those relations still purges. Do not require a | ||
| superuser `DISABLE TRIGGER` after a Succeeded start. | ||
|
Comment on lines
+79
to
+85
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same numbering collision. Describe the three optional relations and the |
||
|
|
||
| ## References — APA 7th | ||
|
|
||
| American Institute of Certified Public Accountants. (2017). *SOC 2®: SOC | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -295,6 +295,10 @@ def test_registry_contract_is_normalized_and_has_one_temporal_authority() -> Non | |
| assert "analysis_run_retention_not_approved" in retention | ||
| assert "analysis_run_retention_not_granted" in retention | ||
| assert "analysis_run_retention_not_admin" in retention | ||
| assert "analysis_run_lineage_edge" in retention | ||
| assert "analysis_run_reconstruction" in retention | ||
| assert "analysis_source_snapshot_member" in retention | ||
| assert "to_regclass" in retention | ||
| assert "analysis_run_retention_event_not_empty" in retention_rollback | ||
| assert "jsonb" not in retention.casefold() | ||
| for object_name in re.findall( | ||
|
|
@@ -901,6 +905,105 @@ def test_approved_retention_purge_empties_a_run_bearing_registry(registry_db) -> | |
| assert cursor.fetchone()[0] is None | ||
|
|
||
|
|
||
| def test_retention_purge_empties_optional_reconstruction_children( | ||
| registry_db, | ||
| ) -> None: | ||
| """Grant plus admin empties ADR 0021 children despite delete-reject triggers.""" | ||
|
|
||
| with registry_db.cursor() as cursor: | ||
| _insert_run_bearing_registry( | ||
| cursor, | ||
| digest="b" * 64, | ||
| idempotency_key="retention-purge-children", | ||
| ) | ||
| cursor.execute( | ||
| "select analysis_run_id, analysis_source_snapshot_id " | ||
| "from analysis_run" | ||
| ) | ||
| run_id, snapshot_id = cursor.fetchone() | ||
| cursor.execute( | ||
| """ | ||
| create table analysis_run_reconstruction ( | ||
| analysis_run_id uuid primary key | ||
| references analysis_run (analysis_run_id), | ||
| result_sha256 text not null, | ||
| edge_count integer not null, | ||
| reconstructed_at timestamptz not null | ||
| ); | ||
| create table analysis_run_lineage_edge ( | ||
| analysis_run_id uuid not null | ||
| references analysis_run_reconstruction (analysis_run_id), | ||
| child_post_id uuid not null, | ||
| parent_post_id uuid not null, | ||
| fused_score double precision not null, | ||
| primary key (analysis_run_id, child_post_id) | ||
| ); | ||
| create table analysis_source_snapshot_member ( | ||
| analysis_source_snapshot_id uuid not null | ||
| references analysis_source_snapshot | ||
| (analysis_source_snapshot_id), | ||
| source_post_id uuid not null, | ||
| primary key ( | ||
| analysis_source_snapshot_id, source_post_id | ||
| ) | ||
| ); | ||
| create function reject_reconstruction_child_delete() | ||
| returns trigger language plpgsql as $fn$ | ||
| begin | ||
| raise exception 'analysis_run_reconstruction_is_immutable'; | ||
| end | ||
| $fn$; | ||
| create trigger analysis_run_reconstruction_update_reject | ||
| before update or delete on analysis_run_reconstruction | ||
| for each row execute function reject_reconstruction_child_delete(); | ||
| create trigger analysis_run_lineage_edge_update_reject | ||
| before update or delete on analysis_run_lineage_edge | ||
| for each row execute function reject_reconstruction_child_delete(); | ||
| create trigger analysis_source_snapshot_member_update_reject | ||
| before update or delete on analysis_source_snapshot_member | ||
| for each row execute function reject_reconstruction_child_delete(); | ||
| """ | ||
| ) | ||
| cursor.execute( | ||
| "insert into analysis_run_reconstruction " | ||
| "(analysis_run_id, result_sha256, edge_count, reconstructed_at) " | ||
| "values (%s, %s, 1, now())", | ||
| (run_id, "c" * 64), | ||
| ) | ||
| cursor.execute( | ||
| "insert into analysis_run_lineage_edge " | ||
| "(analysis_run_id, child_post_id, parent_post_id, fused_score) " | ||
| "values (%s, %s, %s, 0.91)", | ||
| (run_id, str(uuid.uuid4()), str(uuid.uuid4())), | ||
| ) | ||
| cursor.execute( | ||
| "insert into analysis_source_snapshot_member " | ||
| "(analysis_source_snapshot_id, source_post_id) " | ||
| "values (%s, %s)", | ||
| (snapshot_id, str(uuid.uuid4())), | ||
| ) | ||
| with pytest.raises( | ||
| psycopg2.errors.RaiseException, | ||
| match="analysis_run_reconstruction_is_immutable", | ||
| ): | ||
| cursor.execute("delete from analysis_run_reconstruction") | ||
| _authorize_session_for_purge(cursor) | ||
| cursor.execute( | ||
| "select purge_analysis_run_registry(%s)", | ||
| ("approved-retention-purge",), | ||
| ) | ||
| cursor.execute("select count(*) from analysis_run") | ||
| assert cursor.fetchone()[0] == 0 | ||
| cursor.execute("select count(*) from analysis_source_snapshot") | ||
| assert cursor.fetchone()[0] == 0 | ||
| cursor.execute("select count(*) from analysis_run_reconstruction") | ||
| assert cursor.fetchone()[0] == 0 | ||
| cursor.execute("select count(*) from analysis_run_lineage_edge") | ||
| assert cursor.fetchone()[0] == 0 | ||
| cursor.execute("select count(*) from analysis_source_snapshot_member") | ||
| assert cursor.fetchone()[0] == 0 | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. After the successful purge, assert |
||
|
|
||
|
|
||
| def test_retention_purge_requires_unrevoked_session_grant(registry_db) -> None: | ||
| """Admin membership plus the published token cannot purge without a grant.""" | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Drop the ADR number. Person-catalog already claims 0021, and the current start head still calls these tables ADR 0020 (taken here by retention). Write “when start-reconstruction tables exist” so the next free ADR can be assigned on the start rebase.