From c47f4813a7a4e47aad9b149a70631ec727ee8b1c Mon Sep 17 00:00:00 2001 From: jakeross Date: Sat, 22 Aug 2026 17:38:38 -0700 Subject: [PATCH] fix(ogc): gate ogc_waterlevels on the well's release status ogc_waterlevels filtered on the reading's own release_status only, so a well whose own release_status was 'draft' or 'private' still published its public readings through OGC API - EDR -- with the well's name and coordinates attached to every one of them. ogc_water_chemistry has required the parent thing to be public since d9e0f1a2b3c4; this brings water levels onto the same rule, which is what made the inconsistency visible in the first place. ogc_internal_waterlevels is deliberately untouched. It carries non-public records by design for authenticated staff clients, exactly as ogc_internal_water_chemistry does. Downgrade restores the definition by importing z9a0b1c2d3e4 rather than copying its SQL, so the reverted view cannot drift from the definition of record -- the same approach b7c8d9e0f1a2 took. No rows change on the dev database: it has no non-public well carrying readings today, so this closes the hole rather than retracting published data. Whether that also holds in production is worth checking before deploy. Co-Authored-By: Claude Opus 5 --- ...3_gate_ogc_waterlevels_on_thing_release.py | 122 ++++++++++++++++++ tests/test_ogc.py | 51 ++++++++ 2 files changed, 173 insertions(+) create mode 100644 alembic/versions/baba91fe5e83_gate_ogc_waterlevels_on_thing_release.py diff --git a/alembic/versions/baba91fe5e83_gate_ogc_waterlevels_on_thing_release.py b/alembic/versions/baba91fe5e83_gate_ogc_waterlevels_on_thing_release.py new file mode 100644 index 00000000..2eee5c2a --- /dev/null +++ b/alembic/versions/baba91fe5e83_gate_ogc_waterlevels_on_thing_release.py @@ -0,0 +1,122 @@ +"""gate ogc_waterlevels on the thing's release status + +ogc_waterlevels filtered on the reading's own release_status only, so a well +whose release_status is 'draft' or 'private' still published its public +readings through OGC API - EDR -- with the well's name and coordinates +attached. ogc_water_chemistry (d9e0f1a2b3c4) already required the parent thing +to be public; this brings water levels onto the same rule. + +The internal mirror, ogc_internal_waterlevels, is deliberately left alone: it +carries non-public records by design for authenticated staff clients, the same +way ogc_internal_water_chemistry does. + +Revision ID: baba91fe5e83 +Revises: 986e0eb85ab3 +Create Date: 2026-08-22 18:35:00.000000 + +""" + +import importlib.util +from pathlib import Path +from typing import Sequence, Union + +from alembic import op +from sqlalchemy import text + +# revision identifiers, used by Alembic. +revision: str = "baba91fe5e83" +down_revision: Union[str, Sequence[str], None] = "986e0eb85ab3" +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + +_ORIGINAL_REVISION = "z9a0b1c2d3e4_add_edr_water_views.py" + +# Shared join from a thing to its current location point. Mirrors the join in +# z9a0b1c2d3e4, which is the definition of record for this view. +_LOCATION_JOIN = """ + JOIN location_thing_association lta + ON lta.thing_id = t.id AND lta.effective_end IS NULL + JOIN location l ON l.id = lta.location_id +""" + + +def _load_original_module(): + """Import z9a0b1c2d3e4 so downgrade restores its SQL rather than a copy.""" + path = Path(__file__).with_name(_ORIGINAL_REVISION) + if not path.exists(): + raise RuntimeError( + "Cannot restore the previous ogc_waterlevels definition: " + f"{_ORIGINAL_REVISION} is missing from alembic/versions." + ) + spec = importlib.util.spec_from_file_location("_edr_water_views", path) + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +def _create_waterlevels_view() -> str: + return f""" + CREATE VIEW ogc_waterlevels AS + -- manual water-level readings + SELECT + 'm-' || o.id AS id, + t.id AS thing_id, + t.name AS station_name, + ST_X(l.point) AS longitude, + ST_Y(l.point) AS latitude, + o.observation_datetime AS datetime, + o.value AS value, + o.unit AS unit, + 'groundwater level' AS parameter_name, + 'manual' AS source, + NULL::integer AS deployment_id, + o.release_status AS release_status + FROM observation o + JOIN parameter p + ON p.id = o.parameter_id AND p.parameter_name = 'groundwater level' + JOIN sample sm ON sm.id = o.sample_id + JOIN field_activity fa ON fa.id = sm.field_activity_id + JOIN field_event fe ON fe.id = fa.field_event_id + JOIN thing t ON t.id = fe.thing_id + {_LOCATION_JOIN} + WHERE o.release_status = 'public' + AND t.release_status = 'public' + AND o.value IS NOT NULL + + UNION ALL + + -- transducer (instrument) water-level readings + SELECT + 't-' || tobs.id AS id, + t.id AS thing_id, + t.name AS station_name, + ST_X(l.point) AS longitude, + ST_Y(l.point) AS latitude, + tobs.observation_datetime AS datetime, + tobs.value AS value, + p.default_unit AS unit, + 'groundwater level' AS parameter_name, + 'transducer' AS source, + tobs.deployment_id AS deployment_id, + tobs.release_status AS release_status + FROM transducer_observation tobs + JOIN parameter p + ON p.id = tobs.parameter_id AND p.parameter_name = 'groundwater level' + JOIN deployment d ON d.id = tobs.deployment_id + JOIN thing t ON t.id = d.thing_id + {_LOCATION_JOIN} + WHERE tobs.release_status = 'public' + AND t.release_status = 'public' + AND tobs.value IS NOT NULL + """ + + +def upgrade() -> None: + op.execute(text("DROP VIEW IF EXISTS ogc_waterlevels")) + op.execute(text(_create_waterlevels_view())) + + +def downgrade() -> None: + original = _load_original_module() + op.execute(text("DROP VIEW IF EXISTS ogc_waterlevels")) + op.execute(text(original._create_waterlevels_view())) diff --git a/tests/test_ogc.py b/tests/test_ogc.py index 35ed69ac..86265e5a 100644 --- a/tests/test_ogc.py +++ b/tests/test_ogc.py @@ -779,3 +779,54 @@ def test_ogc_polygon_within_filter(location): assert response.status_code == 200 payload = response.json() assert payload["numberReturned"] >= 1 + + +def test_ogc_waterlevels_excludes_readings_from_a_non_public_well( + water_well_thing, groundwater_level_observation +): + """A well that is not public publishes no water levels, even public ones. + + ogc_waterlevels used to filter on the reading's own release_status alone, + so a draft or private well still published its public readings -- with the + well's name and coordinates attached. ogc_water_chemistry already required + the parent thing to be public; baba91fe5e83 brought water levels onto the + same rule. + """ + with session_ctx() as session: + session.execute( + text("UPDATE observation SET release_status = 'public' WHERE id = :oid"), + {"oid": groundwater_level_observation.id}, + ) + session.execute( + text("UPDATE thing SET release_status = 'draft' WHERE id = :tid"), + {"tid": water_well_thing.id}, + ) + session.commit() + + reading_id = f"m-{groundwater_level_observation.id}" + + public_rows = session.execute( + text("SELECT id FROM ogc_waterlevels WHERE id = :rid"), + {"rid": reading_id}, + ).all() + assert not public_rows, "a non-public well leaked a reading to /ogcapi" + + # The internal mount is where staff see non-public records; it must + # keep carrying them. + internal_rows = session.execute( + text("SELECT id FROM ogc_internal_waterlevels WHERE id = :rid"), + {"rid": reading_id}, + ).all() + assert internal_rows, "the internal mirror stopped carrying the reading" + + # Public well, public reading: published again. + session.execute( + text("UPDATE thing SET release_status = 'public' WHERE id = :tid"), + {"tid": water_well_thing.id}, + ) + session.commit() + + assert session.execute( + text("SELECT id FROM ogc_waterlevels WHERE id = :rid"), + {"rid": reading_id}, + ).all()