From c65920ee68d1d9b6a9028b7f16995b2de561a309 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Mon, 18 Aug 2025 16:10:40 -0600 Subject: [PATCH 01/45] refactor: update observation model --- db/observation.py | 24 ++++-------------------- 1 file changed, 4 insertions(+), 20 deletions(-) diff --git a/db/observation.py b/db/observation.py index a37c31101..4d8678ee6 100644 --- a/db/observation.py +++ b/db/observation.py @@ -16,7 +16,6 @@ from sqlalchemy import ( ForeignKey, Integer, - TIMESTAMP, PrimaryKeyConstraint, Float, DateTime, @@ -59,15 +58,13 @@ class Observation(Base, AuditMixin, ReleaseMixin): DateTime(timezone=True), nullable=False, doc="Timestamp of the observation" ) observed_property = lexicon_term() - - # groundwater - depth_to_water = mapped_column( + value = mapped_column( Float, nullable=True, - doc="Depth to water level in ft below measuring point", - info={"unit": "ft"}, ) + unit = lexicon_term() + # groundwater measuring_point_height = mapped_column( Float, nullable=True, @@ -78,25 +75,12 @@ class Observation(Base, AuditMixin, ReleaseMixin): level_status = lexicon_term() # geothermal - depth = mapped_column( + observation_depth = mapped_column( Float, nullable=True, info={"unit": "feet"}, doc="Depth of the geothermal observation in feet", ) - temperature = mapped_column( - Float, - nullable=True, - info={"unit": "degC"}, - doc="Temperature of the geothermal observation in degrees Celsius", - ) - - # general observations - value = mapped_column( - Float, - nullable=True, - ) - unit = lexicon_term() sensor = relationship("Sensor") sample = relationship("Sample") From a29cf20b996b4faba4e8db7ee01c936fb44abeba Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Mon, 18 Aug 2025 16:11:22 -0600 Subject: [PATCH 02/45] feat: udpate observation schemas --- schemas/observation.py | 73 ++++++++++++++++++++++++++++-------------- 1 file changed, 49 insertions(+), 24 deletions(-) diff --git a/schemas/observation.py b/schemas/observation.py index fe6bb065b..4bb97b872 100644 --- a/schemas/observation.py +++ b/schemas/observation.py @@ -52,35 +52,48 @@ class CreateBaseObservation(ValidateObservation): sensor_id: int observed_property: str release_status: str + value: float | None + unit: str | None class CreateGroundwaterLevelObservation(CreateBaseObservation): - depth_to_water: float measuring_point_height: float level_status: str class CreateWaterChemistryObservation(CreateBaseObservation): - value: float - unit: str + pass -# -# -# class CreateGroundwaterLevelObservation(ChildObservationModel, GroundwaterLevelMixin): -# pass -# -# -# class CreateGeothermalObservation(ChildObservationModel, GeothermalMixin): -# pass -# -# -# class CreateGroundwaterLevelObservationDirect(CreateObservation, GroundwaterLevelMixin): -# pass -# -# -# class CreateGeothermalObservationDirect(CreateObservation, GeothermalMixin): -# pass +class CreateGeothermalObservation(CreateBaseObservation): + observation_depth: float + + +# -------- UPDATE ------------ + + +class UpdateBaseObservation(ValidateObservation): + observation_datetime: Annotated[AwareDatetime, PastDatetime()] + sample_id: int | None = None + field_sample_id: str | None = None + sensor_id: int | None = None + observed_property: str | None = None + release_status: str | None = None + value: float | None | None = None + unit: str | None = None + + +class UpdateGroundwaterLevelObservation(UpdateBaseObservation): + measuring_point_height: float | None = None + level_status: str | None = None + + +class UpdateWaterChemistryObservation(UpdateBaseObservation): + pass + + +class UpdateGeothermalObservation(UpdateBaseObservation): + observation_depth: float | None = None # -------- RESPONSE ---------- @@ -92,17 +105,30 @@ class BaseObservationResponse(BaseModel): observed_property: str created_at: AwareDatetime release_status: str + value: float | None class GroundwaterLevelObservationResponse(BaseObservationResponse): - depth_to_water: float + depth_to_water_bgs: float | None + measuring_point_height: float level_status: str | None + @field_validator("depth_to_water_bgs") + def calculate_depth_to_water_bgs(cls, depth_to_water_bgs, data): + depth_to_water = data.values.get("value") + measuring_point_height = data.values.get("measuring_point_height") + if depth_to_water is not None: + return depth_to_water - measuring_point_height + else: + return depth_to_water -class GeothermalObservationResponse(BaseObservationResponse): - temperature: float - depth: float +class WaterChemistryObservationResponse(BaseObservationResponse): + pass + + +class GeothermalObservationResponse(BaseObservationResponse): + observation_depth: float class ObservationResponse( @@ -114,5 +140,4 @@ class ObservationResponse( """ -# -------- UPDATE ---------- # ============= EOF ============================================= From 14a535175dd98001513d158849aba62e4811b190 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Mon, 18 Aug 2025 16:33:40 -0600 Subject: [PATCH 03/45] refactor: simplify adding observations the sample table is no longer queried when adding observations, so the simplified adder is now used --- api/observation.py | 43 +++++++++++++------------ services/observation_helper.py | 58 ---------------------------------- 2 files changed, 22 insertions(+), 79 deletions(-) delete mode 100644 services/observation_helper.py diff --git a/api/observation.py b/api/observation.py index 177433053..5a3a50cc3 100644 --- a/api/observation.py +++ b/api/observation.py @@ -21,15 +21,17 @@ from starlette.status import HTTP_201_CREATED from api.pagination import CustomPage -from core.dependencies import session_dependency -from db import Sample +from core.dependencies import session_dependency, amp_admin_dependency, admin_dependency +from db import Sample, Observation, adder from db.observation import Observation from schemas.observation import ( CreateGroundwaterLevelObservation, GroundwaterLevelObservationResponse, CreateWaterChemistryObservation, + WaterChemistryObservationResponse, + CreateGeothermalObservation, + GeothermalObservationResponse, ) -from services.observation_helper import add_observation from services.query_helper import order_sort_filter router = APIRouter(prefix="/observation", tags=["observation"]) @@ -40,39 +42,38 @@ def add_groundwater_level_observation( obs_data: CreateGroundwaterLevelObservation, session: session_dependency, -): + user: amp_admin_dependency, +) -> GroundwaterLevelObservationResponse: """ Add a new groundwater observation to the database. """ - return add_observation(session, obs_data) + return adder(session, Observation, obs_data, user=user) @router.post("/water-chemistry", status_code=HTTP_201_CREATED) def add_water_chemistry_observation( obs_data: CreateWaterChemistryObservation, session: session_dependency, -): + user: amp_admin_dependency, +) -> WaterChemistryObservationResponse: """ Add a new water chemistry observation to the database. This endpoint is currently a placeholder and does not implement any functionality. """ - return add_observation(session, obs_data) + return adder(session, Observation, obs_data, user=user) -# -# @router.post("/geothermal", status_code=HTTP_201_CREATED) -# def add_geothermal_observation( -# obs_data: CreateGeothermalObservation | CreateGeothermalObservationDirect, -# session: session_dependency, -# ): -# """ -# Add a new geothermal observation to the database. -# This endpoint is currently a placeholder and does not implement any functionality. -# """ -# if isinstance(obs_data, CreateGeothermalObservationDirect): -# return direct_adder(session, GeothermalObservation, obs_data) -# else: -# return adder(session, GeothermalObservation, obs_data) +@router.post("/geothermal", status_code=HTTP_201_CREATED) +def add_geothermal_observation( + obs_data: CreateGeothermalObservation, + session: session_dependency, + user: admin_dependency, +) -> GeothermalObservationResponse: + """ + Add a new geothermal observation to the database. + This endpoint is currently a placeholder and does not implement any functionality. + """ + return adder(session, Observation, obs_data, user=user) # ============= Get ============================================== diff --git a/services/observation_helper.py b/services/observation_helper.py deleted file mode 100644 index 43b46abd4..000000000 --- a/services/observation_helper.py +++ /dev/null @@ -1,58 +0,0 @@ -# =============================================================================== -# Copyright 2025 ross -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. -# =============================================================================== -from pydantic import BaseModel -from sqlalchemy import select -from sqlalchemy.orm import Session - -from db import Base, Observation, Sample - - -def add_observation(session: Session, data: BaseModel) -> Base: - - if isinstance(data, BaseModel): - data = data.model_dump(exclude_unset=True) - - # if 'thing_id' in data: - # thing_id = data.pop('thing_id') - # if 'sample_id' not in data: - # sample = Sample(thing_id=thing_id, - # collection_method=data.get('collection_method', 'manual'), - # collection_timestamp=data.get('observation_datetime')) - # session.add(sample) - # data['sample'] = sample - # else: - # raise ValueError('Cannot specify both thing_id and sample_id') - if "field_sample_id" in data: - field_sample_id = data.pop("field_sample_id") - data.pop( - "sample_id", None - ) # Ensure sample_id is not set if field_sample_id is used - - sql = select(Sample).where(Sample.field_sample_id == field_sample_id) - sample = session.scalar(sql) - if not sample: - raise ValueError(f"Sample with id {field_sample_id} does not exist") - data["sample"] = sample - obj = Observation(**data) - - session.add(obj) - session.commit() - session.refresh(obj) - - return obj - - -# ============= EOF ============================================= From eefc32d7cdde1b47dc83d3c01393c1337839782e Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Mon, 18 Aug 2025 16:45:36 -0600 Subject: [PATCH 04/45] fix: remove field_sample_id from observation cannot have two foreign key constraints to the same (sample) table --- schemas/observation.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/schemas/observation.py b/schemas/observation.py index 4bb97b872..cbe1ae81c 100644 --- a/schemas/observation.py +++ b/schemas/observation.py @@ -28,6 +28,8 @@ class ValidateObservation(BaseModel): + observation_datetime: AwareDatetime + @field_validator("observation_datetime", check_fields=False) def convert_observation_datetime_to_utc( observation_datetime: AwareDatetime, @@ -48,7 +50,6 @@ def convert_observation_datetime_to_utc( class CreateBaseObservation(ValidateObservation): observation_datetime: Annotated[AwareDatetime, PastDatetime()] sample_id: int | None = None - field_sample_id: str | None = None sensor_id: int observed_property: str release_status: str @@ -75,7 +76,6 @@ class CreateGeothermalObservation(CreateBaseObservation): class UpdateBaseObservation(ValidateObservation): observation_datetime: Annotated[AwareDatetime, PastDatetime()] sample_id: int | None = None - field_sample_id: str | None = None sensor_id: int | None = None observed_property: str | None = None release_status: str | None = None From de3ad5c60db5ec47ff51f27f9ff04bf588a5d9a6 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Mon, 18 Aug 2025 16:56:27 -0600 Subject: [PATCH 05/45] refactor: make only the id the primary key --- db/observation.py | 9 --------- 1 file changed, 9 deletions(-) diff --git a/db/observation.py b/db/observation.py index 4d8678ee6..58bf5f036 100644 --- a/db/observation.py +++ b/db/observation.py @@ -16,7 +16,6 @@ from sqlalchemy import ( ForeignKey, Integer, - PrimaryKeyConstraint, Float, DateTime, ) @@ -30,14 +29,6 @@ class Observation(Base, AuditMixin, ReleaseMixin): __versioned__ = {} - __table_args__ = ( - PrimaryKeyConstraint( - "id", - "observation_datetime", - ), - {}, - ) - id = mapped_column( Integer, autoincrement=True, From 3560028cf6f41912671c60759ead2471132476b9 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Mon, 18 Aug 2025 17:00:59 -0600 Subject: [PATCH 06/45] refactor: inherit from AutBaseMixin --- db/observation.py | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/db/observation.py b/db/observation.py index 58bf5f036..1b49196ef 100644 --- a/db/observation.py +++ b/db/observation.py @@ -21,19 +21,12 @@ ) from sqlalchemy.orm import mapped_column, relationship -from db.base import Base, AuditMixin, ReleaseMixin, lexicon_term +from db.base import Base, AutoBaseMixin, ReleaseMixin, lexicon_term -class Observation(Base, AuditMixin, ReleaseMixin): - __tablename__ = "observation" - +class Observation(Base, AutoBaseMixin, ReleaseMixin): __versioned__ = {} - id = mapped_column( - Integer, - autoincrement=True, - ) - sample_id = mapped_column( Integer, ForeignKey("sample.id", ondelete="CASCADE"), From 544b910cce6f2f49c1832939962924c79371df39 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Mon, 18 Aug 2025 17:06:45 -0600 Subject: [PATCH 07/45] feat: add temperature and Calcius to lexicon --- core/lexicon.json | 3 +++ schemas/observation.py | 23 +++++++++++++++-------- 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/core/lexicon.json b/core/lexicon.json index 8ff039b51..08deb24b0 100644 --- a/core/lexicon.json +++ b/core/lexicon.json @@ -11,10 +11,13 @@ {"category": "unit", "term": "W/m²", "definition": "watts per square meter"}, {"category": "unit", "term": "W/m·K", "definition": "watts per meter Kelvin"}, {"category": "unit", "term": "m²/s", "definition": "square meters per second"}, + {"category": "unit", "term": "C", "definition": "Celsius"}, {"category": "observed_property", "term": "groundwater level", "definition": "groundwater level measurement" }, {"category": "observed_property", "term": "pH", "definition": "pH"}, {"category": "observed_property", "term": "Alkalinity as CaCO3", "definition": "Alkalinity as CaCO3"}, + {"category": "observed_property", "term": "temperature", "definition": "Temperature measurement"}, + {"category": "release_status", "term": "draft", "definition": "draft version"}, {"category": "release_status", "term": "provisional", "definition": "provisional version"}, diff --git a/schemas/observation.py b/schemas/observation.py index cbe1ae81c..5ba0e40be 100644 --- a/schemas/observation.py +++ b/schemas/observation.py @@ -14,8 +14,15 @@ # limitations under the License. # =============================================================================== from datetime import timezone -from pydantic import BaseModel, AwareDatetime, PastDatetime, field_validator +from pydantic import ( + BaseModel, + AwareDatetime, + PastDatetime, + field_validator, + model_validator, +) from typing import Annotated +from typing_extensions import Self # class GeothermalMixin: @@ -106,6 +113,7 @@ class BaseObservationResponse(BaseModel): created_at: AwareDatetime release_status: str value: float | None + unit: str class GroundwaterLevelObservationResponse(BaseObservationResponse): @@ -113,14 +121,13 @@ class GroundwaterLevelObservationResponse(BaseObservationResponse): measuring_point_height: float level_status: str | None - @field_validator("depth_to_water_bgs") - def calculate_depth_to_water_bgs(cls, depth_to_water_bgs, data): - depth_to_water = data.values.get("value") - measuring_point_height = data.values.get("measuring_point_height") + @model_validator(mode="before") + def calculate_depth_to_water_bgs(self: Self) -> Self: + depth_to_water = self.value + measuring_point_height = self.measuring_point_height if depth_to_water is not None: - return depth_to_water - measuring_point_height - else: - return depth_to_water + self.depth_to_water_bgs = depth_to_water - measuring_point_height + return self class WaterChemistryObservationResponse(BaseObservationResponse): From 9b697a4024f4bfff7519b2a6dbb66f3d1ae09a00 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Mon, 18 Aug 2025 17:07:57 -0600 Subject: [PATCH 08/45] refactor: update C to deg C for clarity --- core/lexicon.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/lexicon.json b/core/lexicon.json index 08deb24b0..b2487697c 100644 --- a/core/lexicon.json +++ b/core/lexicon.json @@ -11,7 +11,7 @@ {"category": "unit", "term": "W/m²", "definition": "watts per square meter"}, {"category": "unit", "term": "W/m·K", "definition": "watts per meter Kelvin"}, {"category": "unit", "term": "m²/s", "definition": "square meters per second"}, - {"category": "unit", "term": "C", "definition": "Celsius"}, + {"category": "unit", "term": "deg C", "definition": "degree Celsius"}, {"category": "observed_property", "term": "groundwater level", "definition": "groundwater level measurement" }, {"category": "observed_property", "term": "pH", "definition": "pH"}, From 79ae468842d1f8ca110dee85ffbc65661a088e8b Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Mon, 18 Aug 2025 17:08:27 -0600 Subject: [PATCH 09/45] refactor: update observation POST tests --- tests/test_observation.py | 145 +++++++++++++++++++++++--------------- 1 file changed, 90 insertions(+), 55 deletions(-) diff --git a/tests/test_observation.py b/tests/test_observation.py index 5a8172e65..aad871fd8 100644 --- a/tests/test_observation.py +++ b/tests/test_observation.py @@ -13,74 +13,109 @@ # See the License for the specific language governing permissions and # limitations under the License. # =============================================================================== - -from tests import client +from db import Observation +from core.dependencies import amp_admin_function, admin_function +from main import app +from tests import client, cleanup_post_test, override_authentication import pytest -# ============= Post tests ================= -def test_add_water_chemistry_observation(location, thing, sample, sensor): - response = client.post( - "/observation/water-chemistry", - json={ - "observation_datetime": "2025-01-01T00:00:00Z", - "release_status": "draft", - "value": 7.5, - "unit": "dimensionless", - "sample_id": sample.id, - "sensor_id": sensor.id, - "observed_property": "pH", - }, +@pytest.fixture(scope="module", autouse=True) +def override_authentication_dependency_fixture(): + app.dependency_overrides[amp_admin_function] = override_authentication( + default={"name": "foobar", "sub": "1234567890"} + ) + app.dependency_overrides[admin_function] = override_authentication( + default={"name": "foobar", "sub": "1234567890"} ) - data = response.json() - assert response.status_code == 201 - assert data["value"] == 7.5 - assert data["unit"] == "dimensionless" + yield + app.dependency_overrides = {} -def test_add_groundwater_observation(location, thing, sample, sensor): - response = client.post( - "/observation/groundwater-level", - json={ - "observation_datetime": "2025-01-01T00:00:00Z", - "release_status": "draft", - "depth_to_water": 101, - "measuring_point_height": 53, - "sample_id": sample.id, - "sensor_id": sensor.id, - "level_status": "normal", - "observed_property": "groundwater level", - }, - ) + +# ============= Post tests ================= +def test_add_water_chemistry_observation(sample, sensor): + payload = { + "observation_datetime": "2025-01-01T00:00:00Z", + "release_status": "draft", + "value": 7.5, + "unit": "dimensionless", + "sample_id": sample.id, + "sensor_id": sensor.id, + "observed_property": "pH", + } + response = client.post("/observation/water-chemistry", json=payload) data = response.json() assert response.status_code == 201 - assert data["depth_to_water"] == 101 - assert data["measuring_point_height"] == 53 - - -# def test_add_geothermal_observation(): -# response = client.post( -# "/observation/geothermal", -# json={ -# "observation_id": 1, -# "observation_datetime": "2025-01-01T00:00:00Z", -# "depth": 100, -# "temperature": 25.5, -# }, -# ) -# assert response.status_code == 201 -# data = response.json() -# assert data["observation_id"] == 1 + assert data["observation_datetime"] == payload["observation_datetime"] + assert data["release_status"] == payload["release_status"] + assert data["value"] == payload["value"] + assert data["unit"] == payload["unit"] + assert data["sample_id"] == payload["sample_id"] + assert data["sensor_id"] == payload["sensor_id"] + assert data["observed_property"] == payload["observed_property"] + + cleanup_post_test(Observation, data["id"]) + + +def test_add_groundwater_level_observation(sample, sensor): + payload = { + "observation_datetime": "2025-01-01T00:00:00Z", + "release_status": "draft", + "value": 101, + "measuring_point_height": 53, + "sample_id": sample.id, + "sensor_id": sensor.id, + "level_status": "normal", + "observed_property": "groundwater level", + "unit": "ft", + } + response = client.post("/observation/groundwater-level", json=payload) + data = response.json() + assert response.status_code == 201 + assert data["observation_datetime"] == payload["observation_datetime"] + assert data["release_status"] == payload["release_status"] + assert data["value"] == payload["value"] + assert data["measuring_point_height"] == payload["measuring_point_height"] + assert data["sensor_id"] == payload["sensor_id"] + assert data["level_status"] == payload["level_status"] + assert data["observed_property"] == payload["observed_property"] + assert ( + data["depth_to_water_bgs"] + == payload["value"] - payload["measuring_point_height"] + ) -@pytest.mark.skip(reason="not implemented yet") -def test_add_geochemical_observation(): - response = client.post("/observation/geochemical", json={"observation_id": 1}) - assert response.status_code == 201 + cleanup_post_test(Observation, data["id"]) + + +def test_add_geothermal_observation(sample, sensor): + payload = { + "observation_datetime": "2025-01-01T00:00:00Z", + "release_status": "draft", + "observation_depth": 100, + "value": 25.5, + "sample_id": sample.id, + "sensor_id": sensor.id, + "observed_property": "temperature", + "unit": "C", + } + response = client.post("/observation/geothermal", json=payload) data = response.json() - assert data["observation_id"] == 1 + assert response.status_code == 201 + + assert data["observation_datetime"] == payload["observation_datetime"] + assert data["release_status"] == payload["release_status"] + assert data["observation_depth"] == payload["observation_depth"] + assert data["value"] == payload["value"] + assert data["sample_id"] == payload["sample_id"] + assert data["sensor_id"] == payload["sensor_id"] + assert data["observed_property"] == payload["observed_property"] + assert data["unit"] == payload["unit"] + + cleanup_post_test(Observation, data["id"]) # ============= Get tests ================= From 153fc119227580e1ac2dcb5d80b6e8c6906b28ff Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Tue, 19 Aug 2025 09:04:10 -0600 Subject: [PATCH 10/45] note: noting thoughts on query parameters --- tests/test_observation.py | 68 +++++++++++++-------------------------- 1 file changed, 22 insertions(+), 46 deletions(-) diff --git a/tests/test_observation.py b/tests/test_observation.py index aad871fd8..92d57b2d0 100644 --- a/tests/test_observation.py +++ b/tests/test_observation.py @@ -119,36 +119,6 @@ def test_add_geothermal_observation(sample, sensor): # ============= Get tests ================= -# def test_get_observation_by_series_id(): -# response = client.get("/observation", params={"series_id": 1}) -# assert response.status_code == 200 -# data = response.json() -# assert "items" in data, "Expected 'items' in response" -# items = data["items"] -# assert len(items) > 0, "Expected at least one observation for the series" -# # assert isinstance(data, list), "Expected a list of observations" -# # assert len(data) == 1, "Expected at least one observation for the series" - - -# def test_get_groundwater_observation_by_thing_id(): -# response = client.get("/observation/groundwater-level", params={"thing_id": 1, -# "observed_property": "groundwater level"}) -# assert response.status_code == 200 -# data = response.json() -# assert "items" in data, "Expected 'items' in response" -# items = data["items"] -# assert ( -# len(items) > 0 -# ), "Expected at least one groundwater observation for the series" - - -# def test_get_geothermal_observation_by_series_id(): -# response = client.get("/observation/geothermal", params={"series_id": 1}) -# assert response.status_code == 200 -# data = response.json() -# assert "items" in data, "Expected 'items' in response" -# items = data["items"] -# assert len(items) > 0, "Expected at least one geothermal observation for the series" def test_get_groundwater_observation_by_sample(sample): @@ -186,12 +156,12 @@ def test_get_groundwater_observation_by_thing_nonexistent(): ), "Expected no groundwater observations for a non-existent thing" -@pytest.mark.skip(reason="unclear why not working. is it necessary functionality?") -def test_get_groundwater_observation_by_polygon(): +def test_get_groundwater_observation_by_time_range(): response = client.get( "/observation/groundwater-level", params={ - "polygon": "POLYGON((-10.0 -10.0, 20.0 10.0, 20.0 20.0, 10.0 20.0, -10.0 -10.0))", + "start_time": "2025-01-01T00:00:00Z", + "end_time": "2025-01-02T00:00:00Z", }, ) assert response.status_code == 200 @@ -200,30 +170,33 @@ def test_get_groundwater_observation_by_polygon(): items = data["items"] assert ( len(items) > 0 - ), "Expected at least one groundwater observation within the polygon" + ), "Expected at least one groundwater observation in the time range" -@pytest.mark.skip(reason="unclear why not working. is it necessary functionality?") -def test_get_groundwater_observation_by_polygon_nonexistent(): +def test_get_groundwater_observation_by_time_range_nonexistent(): response = client.get( "/observation/groundwater-level", params={ - "polygon": "POLYGON((-100.0 -100.0, -90.0 -90.0, -90.0 -80.0, -100.0 -80.0, -100.0 -100.0))", + "start_time": "2020-01-01T00:00:00Z", + "end_time": "2020-01-02T00:00:00Z", }, ) assert response.status_code == 200 data = response.json() assert "items" in data, "Expected 'items' in response" items = data["items"] - assert len(items) == 0, "Expected no groundwater observations within the polygon" + assert len(items) == 0, "Expected no groundwater observations in the time range" -def test_get_groundwater_observation_by_time_range(): +# JB's comment: I don't think that geographic filters are necessary for +# observations. I think that they should only be applicable to finding Things +# and locations. Then the user can proceed from there to find observations. +@pytest.mark.skip(reason="unclear why not working. is it necessary functionality?") +def test_get_groundwater_observation_by_polygon(): response = client.get( "/observation/groundwater-level", params={ - "start_time": "2025-01-01T00:00:00Z", - "end_time": "2025-01-02T00:00:00Z", + "polygon": "POLYGON((-10.0 -10.0, 20.0 10.0, 20.0 20.0, 10.0 20.0, -10.0 -10.0))", }, ) assert response.status_code == 200 @@ -232,22 +205,25 @@ def test_get_groundwater_observation_by_time_range(): items = data["items"] assert ( len(items) > 0 - ), "Expected at least one groundwater observation in the time range" + ), "Expected at least one groundwater observation within the polygon" -def test_get_groundwater_observation_by_time_range_nonexistent(): +# JB's comment: I don't think that geographic filters are necessary for +# observations. I think that they should only be applicable to finding Things +# and locations. Then the user can proceed from there to find observations +@pytest.mark.skip(reason="unclear why not working. is it necessary functionality?") +def test_get_groundwater_observation_by_polygon_nonexistent(): response = client.get( "/observation/groundwater-level", params={ - "start_time": "2020-01-01T00:00:00Z", - "end_time": "2020-01-02T00:00:00Z", + "polygon": "POLYGON((-100.0 -100.0, -90.0 -90.0, -90.0 -80.0, -100.0 -80.0, -100.0 -100.0))", }, ) assert response.status_code == 200 data = response.json() assert "items" in data, "Expected 'items' in response" items = data["items"] - assert len(items) == 0, "Expected no groundwater observations in the time range" + assert len(items) == 0, "Expected no groundwater observations within the polygon" # ============= EOF ============================================= From c86e2ba9c6358ea611f3e3b300805e9945912a59 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Tue, 19 Aug 2025 10:04:57 -0600 Subject: [PATCH 11/45] fix: fix observation fixtures --- tests/conftest.py | 60 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/tests/conftest.py b/tests/conftest.py index 146b60fab..c0469508e 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -172,3 +172,63 @@ def asset(): yield asset session.close() + + +@pytest.fixture(scope="session") +def groundwater_level_observation(sensor, sample): + with session_ctx() as session: + observation = Observation( + observation_datetime="2025-01-01T00:00:00Z", + sample_id=sample.id, + sensor_id=sensor.id, + observed_property="groundwater level", + release_status="draft", + value=10.0, + unit="ft", + measuring_point_height=5.0, + level_status="normal", + ) + session.add(observation) + session.commit() + yield observation + + session.close() + + +@pytest.fixture(scope="session") +def water_chemistry_observation(sensor, sample): + with session_ctx() as session: + observation = Observation( + observation_datetime="2025-01-01T00:00:00Z", + sample_id=sample.id, + sensor_id=sensor.id, + observed_property="pH", + release_status="draft", + value=4.0, + unit="dimensionless", + ) + session.add(observation) + session.commit() + yield observation + + session.close() + + +@pytest.fixture(scope="session") +def geothermal_observation(sensor, sample): + with session_ctx() as session: + observation = Observation( + observation_datetime="2025-01-01T00:00:00Z", + sample_id=sample.id, + sensor_id=sensor.id, + observed_property="temperature", + release_status="draft", + value=20.0, + unit="deg C", + observation_depth="200", + ) + session.add(observation) + session.commit() + yield observation + + session.close() From d9f58bce1f8f88e10c4468ce45e9a5af4e68edbe Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Tue, 19 Aug 2025 10:41:30 -0600 Subject: [PATCH 12/45] refactor: order observed properties by observation type --- core/lexicon.json | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/core/lexicon.json b/core/lexicon.json index b2487697c..f9b3709f3 100644 --- a/core/lexicon.json +++ b/core/lexicon.json @@ -14,9 +14,12 @@ {"category": "unit", "term": "deg C", "definition": "degree Celsius"}, {"category": "observed_property", "term": "groundwater level", "definition": "groundwater level measurement" }, + + {"category": "observed_property", "term": "temperature", "definition": "Temperature measurement"}, + {"category": "observed_property", "term": "pH", "definition": "pH"}, {"category": "observed_property", "term": "Alkalinity as CaCO3", "definition": "Alkalinity as CaCO3"}, - {"category": "observed_property", "term": "temperature", "definition": "Temperature measurement"}, + {"category": "release_status", "term": "draft", "definition": "draft version"}, From d0fe51e069a6d10bac06a1c389bbd77d27e3dae5 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Tue, 19 Aug 2025 11:01:02 -0600 Subject: [PATCH 13/45] feat: implement get groundwater level observations --- api/observation.py | 121 ++++++++++++++++++++++++++++++++++---- tests/test_observation.py | 105 ++++++++++++++++++++++++++++++++- 2 files changed, 211 insertions(+), 15 deletions(-) diff --git a/api/observation.py b/api/observation.py index 5a3a50cc3..c77e0d493 100644 --- a/api/observation.py +++ b/api/observation.py @@ -14,16 +14,16 @@ # limitations under the License. # =============================================================================== from datetime import datetime +import functools from fastapi import APIRouter, Query from fastapi_pagination.ext.sqlalchemy import paginate from sqlalchemy import select -from starlette.status import HTTP_201_CREATED +from starlette.status import HTTP_201_CREATED, HTTP_404_NOT_FOUND from api.pagination import CustomPage from core.dependencies import session_dependency, amp_admin_dependency, admin_dependency from db import Sample, Observation, adder -from db.observation import Observation from schemas.observation import ( CreateGroundwaterLevelObservation, GroundwaterLevelObservationResponse, @@ -32,7 +32,8 @@ CreateGeothermalObservation, GeothermalObservationResponse, ) -from services.query_helper import order_sort_filter +from services.exceptions_helper import PydanticStyleException +from services.query_helper import order_sort_filter, simple_get_by_id router = APIRouter(prefix="/observation", tags=["observation"]) @@ -77,26 +78,40 @@ def add_geothermal_observation( # ============= Get ============================================== -@router.get( - "/groundwater-level", -) -def get_groundwater_level_observations( + + +def specify_observation_type(observation_class: str): + def decorator(func): + @functools.wraps(func) + def wrapper(*args, **kwargs): + sql, session = func(*args, **kwargs) + if observation_class == "groundwater level": + sql = sql.where(Observation.observed_property == "groundwater level") + elif observation_class == "water chemistry": + sql = sql.where(Observation.observed_property != "groundwater level") + sql = sql.where(Observation.observed_property != "temperature") + return paginate(query=sql, conn=session) + + return wrapper + + return decorator + + +def get_observations( session: session_dependency, thing_id: int | None = None, sensor_id: int | None = None, sample_id: int | None = None, - polygon: str | None = None, start_time: datetime | None = None, end_time: datetime | None = None, sort: str | None = None, order: str | None = None, filter_: str = Query(alias="filter", default=None), -) -> CustomPage[GroundwaterLevelObservationResponse]: +) -> tuple: """ - Retrieve all groundwater level observations from the database. + Retrieve all observations """ sql = select(Observation) - sql = sql.where(Observation.observed_property == "groundwater level") if thing_id is not None: sql = sql.join(Sample) sql = sql.where(Sample.thing_id == thing_id) @@ -115,7 +130,89 @@ def get_groundwater_level_observations( if not order: sql = sql.order_by(Observation.observation_datetime.desc()) - return paginate(query=sql, conn=session) + return sql, session +@router.get( + "/groundwater-level", +) +@specify_observation_type(observation_class="groundwater level") +def get_groundwater_level_observations( + session: session_dependency, + thing_id: int | None = None, + sensor_id: int | None = None, + sample_id: int | None = None, + start_time: datetime | None = None, + end_time: datetime | None = None, + sort: str | None = None, + order: str | None = None, + filter_: str = Query(alias="filter", default=None), +) -> CustomPage[GroundwaterLevelObservationResponse]: + """ + Retrieve all groundwater level observations from the database. + """ + return get_observations( + session=session, + thing_id=thing_id, + sensor_id=sensor_id, + sample_id=sample_id, + start_time=start_time, + end_time=end_time, + sort=sort, + order=order, + filter_=filter_, + ) + + +@router.get("/groundwater-level/{observation_id}") +def get_groundwater_level_observation_by_id( + session: session_dependency, observation_id: int +) -> GroundwaterLevelObservationResponse: + observation = simple_get_by_id(session, Observation, observation_id) + if observation.observed_property != "groundwater level": + raise PydanticStyleException( + status_code=HTTP_404_NOT_FOUND, + detail=[ + { + "loc": ["path", "observation_id"], + "msg": f"Observation with ID {observation_id} is not a groundwater level observation.", + "type": "value_error", + "input": {"observation_id": observation_id}, + } + ], + ) + else: + return observation + + +# @router.get( +# "/water-chemistry" +# ) +# @specify_observation_type(observation_class="water chemistry") +# def get_water_chemistry_observations( +# session: session_dependency, +# thing_id: int | None = None, +# sensor_id: int | None = None, +# sample_id: int | None = None, +# start_time: datetime | None = None, +# end_time: datetime | None = None, +# sort: str | None = None, +# order: str | None = None, +# filter_: str = Query(alias="filter", default=None), +# ) -> CustomPage[WaterChemistryObservationResponse]: +# """ +# Retrieve all water chemistry observations from the database. +# """ +# return get_observations( +# session=session, +# thing_id=thing_id, +# sensor_id=sensor_id, +# sample_id=sample_id, +# start_time=start_time, +# end_time=end_time, +# sort=sort, +# order=order, +# filter_=filter_, +# ) + # ============= EOF ============================================= diff --git a/tests/test_observation.py b/tests/test_observation.py index 92d57b2d0..c7a434500 100644 --- a/tests/test_observation.py +++ b/tests/test_observation.py @@ -100,7 +100,7 @@ def test_add_geothermal_observation(sample, sensor): "sample_id": sample.id, "sensor_id": sensor.id, "observed_property": "temperature", - "unit": "C", + "unit": "deg C", } response = client.post("/observation/geothermal", json=payload) data = response.json() @@ -119,6 +119,105 @@ def test_add_geothermal_observation(sample, sensor): # ============= Get tests ================= +def test_get_groundwater_level_observations( + groundwater_level_observation, water_chemistry_observation, geothermal_observation +): + response = client.get("/observation/groundwater-level") + assert response.status_code == 200 + data = response.json() + assert data["total"] == 1 + assert data["items"][0]["id"] == groundwater_level_observation.id + assert data["items"][0]["sample_id"] == groundwater_level_observation.sample_id + assert data["items"][0]["sensor_id"] == groundwater_level_observation.sensor_id + assert ( + data["items"][0]["observation_datetime"] + == groundwater_level_observation.observation_datetime + ) + assert ( + data["items"][0]["observed_property"] + == groundwater_level_observation.observed_property + ) + assert ( + data["items"][0]["release_status"] + == groundwater_level_observation.release_status + ) + assert ( + data["items"][0]["level_status"] == groundwater_level_observation.level_status + ) + assert data["items"][0]["value"] == groundwater_level_observation.value + assert data["items"][0]["unit"] == groundwater_level_observation.unit + assert ( + data["items"][0]["depth_to_water_bgs"] + == groundwater_level_observation.value + - groundwater_level_observation.measuring_point_height + ) + assert ( + data["items"][0]["measuring_point_height"] + == groundwater_level_observation.measuring_point_height + ) + assert ( + data["items"][0]["level_status"] == groundwater_level_observation.level_status + ) + + +def test_get_groundwater_level_observation_by_id(groundwater_level_observation): + response = client.get( + f"/observation/groundwater-level/{groundwater_level_observation.id}" + ) + assert response.status_code == 200 + data = response.json() + assert data["id"] == groundwater_level_observation.id + assert data["sample_id"] == groundwater_level_observation.sample_id + assert data["sensor_id"] == groundwater_level_observation.sensor_id + assert ( + data["observation_datetime"] + == groundwater_level_observation.observation_datetime + ) + assert data["observed_property"] == groundwater_level_observation.observed_property + assert data["release_status"] == groundwater_level_observation.release_status + assert data["level_status"] == groundwater_level_observation.level_status + assert data["value"] == groundwater_level_observation.value + assert data["unit"] == groundwater_level_observation.unit + assert ( + data["depth_to_water_bgs"] + == groundwater_level_observation.value + - groundwater_level_observation.measuring_point_height + ) + assert ( + data["measuring_point_height"] + == groundwater_level_observation.measuring_point_height + ) + assert data["level_status"] == groundwater_level_observation.level_status + + +def test_get_groundwater_level_observation_by_id_404_not_found( + groundwater_level_observation, +): + bad_id = 99999 + response = client.get(f"/observation/groundwater-level/{bad_id}") + assert response.status_code == 404 + data = response.json() + assert "detail" in data, "Expected 'detail' in response" + assert data["detail"] == f"Observation with ID {bad_id} not found." + + +def test_get_groundwater_level_observation_by_id_404_wrong_observed_property( + water_chemistry_observation, +): + response = client.get( + f"/observation/groundwater-level/{water_chemistry_observation.id}" + ) + assert response.status_code == 404 + data = response.json() + assert ( + data["detail"][0]["msg"] + == f"Observation with ID {water_chemistry_observation.id} is not a groundwater level observation." + ) + assert data["detail"][0]["type"] == "value_error" + assert data["detail"][0]["input"] == { + "observation_id": water_chemistry_observation.id + } + assert data["detail"][0]["loc"] == ["path", "observation_id"] def test_get_groundwater_observation_by_sample(sample): @@ -133,10 +232,10 @@ def test_get_groundwater_observation_by_sample(sample): assert len(items) > 0, "Expected at least one groundwater observation for the thing" -def test_get_groundwater_observation_by_thing(sample): +def test_get_groundwater_observation_by_thing(thing): response = client.get( "/observation/groundwater-level", - params={"thing_id": sample.thing_id, "observed_property": "groundwater level"}, + params={"thing_id": thing.id, "observed_property": "groundwater level"}, ) assert response.status_code == 200 data = response.json() From bd450f4bfacc4872ea6c51ef7813406b1d405d42 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Tue, 19 Aug 2025 11:53:37 -0600 Subject: [PATCH 14/45] refactor: observation responses should inherit from ORMBaseModel --- schemas/observation.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/schemas/observation.py b/schemas/observation.py index 5ba0e40be..39bb029f6 100644 --- a/schemas/observation.py +++ b/schemas/observation.py @@ -24,6 +24,8 @@ from typing import Annotated from typing_extensions import Self +from schemas import ORMBaseModel + # class GeothermalMixin: # depth: float @@ -104,13 +106,11 @@ class UpdateGeothermalObservation(UpdateBaseObservation): # -------- RESPONSE ---------- -class BaseObservationResponse(BaseModel): - id: int +class BaseObservationResponse(ORMBaseModel): sample_id: int sensor_id: int observation_datetime: AwareDatetime observed_property: str - created_at: AwareDatetime release_status: str value: float | None unit: str From c39d12071a775011d7c5fbd2e7db1d2e44b3cfb7 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Tue, 19 Aug 2025 11:54:11 -0600 Subject: [PATCH 15/45] feat: implement GET water chemistry observations --- api/observation.py | 93 ++++++++++++++++++++++++--------------- tests/test_observation.py | 74 ++++++++++++++++++++++++++++++- 2 files changed, 131 insertions(+), 36 deletions(-) diff --git a/api/observation.py b/api/observation.py index c77e0d493..d1396e81e 100644 --- a/api/observation.py +++ b/api/observation.py @@ -80,7 +80,7 @@ def add_geothermal_observation( # ============= Get ============================================== -def specify_observation_type(observation_class: str): +def specify_observation_class(observation_class: str): def decorator(func): @functools.wraps(func) def wrapper(*args, **kwargs): @@ -133,10 +133,8 @@ def get_observations( return sql, session -@router.get( - "/groundwater-level", -) -@specify_observation_type(observation_class="groundwater level") +@router.get("/groundwater-level", summary="Get groundwater level observations") +@specify_observation_class(observation_class="groundwater level") def get_groundwater_level_observations( session: session_dependency, thing_id: int | None = None, @@ -164,7 +162,10 @@ def get_groundwater_level_observations( ) -@router.get("/groundwater-level/{observation_id}") +@router.get( + "/groundwater-level/{observation_id}", + summary="Get groundwater level observation by ID", +) def get_groundwater_level_observation_by_id( session: session_dependency, observation_id: int ) -> GroundwaterLevelObservationResponse: @@ -185,34 +186,56 @@ def get_groundwater_level_observation_by_id( return observation -# @router.get( -# "/water-chemistry" -# ) -# @specify_observation_type(observation_class="water chemistry") -# def get_water_chemistry_observations( -# session: session_dependency, -# thing_id: int | None = None, -# sensor_id: int | None = None, -# sample_id: int | None = None, -# start_time: datetime | None = None, -# end_time: datetime | None = None, -# sort: str | None = None, -# order: str | None = None, -# filter_: str = Query(alias="filter", default=None), -# ) -> CustomPage[WaterChemistryObservationResponse]: -# """ -# Retrieve all water chemistry observations from the database. -# """ -# return get_observations( -# session=session, -# thing_id=thing_id, -# sensor_id=sensor_id, -# sample_id=sample_id, -# start_time=start_time, -# end_time=end_time, -# sort=sort, -# order=order, -# filter_=filter_, -# ) +@router.get("/water-chemistry", summary="Get water chemistry observations") +@specify_observation_class(observation_class="water chemistry") +def get_water_chemistry_observations( + session: session_dependency, + thing_id: int | None = None, + sensor_id: int | None = None, + sample_id: int | None = None, + start_time: datetime | None = None, + end_time: datetime | None = None, + sort: str | None = None, + order: str | None = None, + filter_: str = Query(alias="filter", default=None), +) -> CustomPage[WaterChemistryObservationResponse]: + """ + Retrieve all water chemistry observations from the database. + """ + return get_observations( + session=session, + thing_id=thing_id, + sensor_id=sensor_id, + sample_id=sample_id, + start_time=start_time, + end_time=end_time, + sort=sort, + order=order, + filter_=filter_, + ) + + +@router.get( + "/water-chemistry/{observation_id}", summary="Get water chemistry observation by ID" +) +def get_water_chemistry_observation_by_id( + session: session_dependency, observation_id: int +) -> WaterChemistryObservationResponse: + observation = simple_get_by_id(session, Observation, observation_id) + if observation.observed_property in ("groundwater level", "temperature"): + raise PydanticStyleException( + status_code=HTTP_404_NOT_FOUND, + detail=[ + { + "loc": ["path", "observation_id"], + "msg": f"Observation with ID {observation_id} is not a water chemistry observation.", + "type": "value_error", + "input": {"observation_id": observation_id}, + } + ], + ) + else: + return observation + # ============= EOF ============================================= diff --git a/tests/test_observation.py b/tests/test_observation.py index c7a434500..7abf00150 100644 --- a/tests/test_observation.py +++ b/tests/test_observation.py @@ -201,7 +201,7 @@ def test_get_groundwater_level_observation_by_id_404_not_found( assert data["detail"] == f"Observation with ID {bad_id} not found." -def test_get_groundwater_level_observation_by_id_404_wrong_observed_property( +def test_get_groundwater_level_observation_by_id_404_wrong_observation_class( water_chemistry_observation, ): response = client.get( @@ -287,6 +287,78 @@ def test_get_groundwater_observation_by_time_range_nonexistent(): assert len(items) == 0, "Expected no groundwater observations in the time range" +def test_get_water_chemistry_observations(water_chemistry_observation): + response = client.get("/observation/water-chemistry") + assert response.status_code == 200 + data = response.json() + assert data["total"] == 1 + assert data["items"][0]["id"] == water_chemistry_observation.id + assert data["items"][0][ + "created_at" + ] == water_chemistry_observation.created_at.isoformat().replace("+00:00", "Z") + assert data["items"][0]["sample_id"] == water_chemistry_observation.sample_id + assert data["items"][0]["sensor_id"] == water_chemistry_observation.sensor_id + assert ( + data["items"][0]["observation_datetime"] + == water_chemistry_observation.observation_datetime + ) + assert ( + data["items"][0]["observed_property"] + == water_chemistry_observation.observed_property + ) + assert data["items"][0]["value"] == water_chemistry_observation.value + assert data["items"][0]["unit"] == water_chemistry_observation.unit + + +def test_get_water_chemistry_observation_by_id(water_chemistry_observation): + response = client.get( + f"/observation/water-chemistry/{water_chemistry_observation.id}" + ) + assert response.status_code == 200 + data = response.json() + assert data["id"] == water_chemistry_observation.id + assert data[ + "created_at" + ] == water_chemistry_observation.created_at.isoformat().replace("+00:00", "Z") + assert data["sample_id"] == water_chemistry_observation.sample_id + assert data["sensor_id"] == water_chemistry_observation.sensor_id + assert ( + data["observation_datetime"] == water_chemistry_observation.observation_datetime + ) + assert data["observed_property"] == water_chemistry_observation.observed_property + assert data["value"] == water_chemistry_observation.value + assert data["unit"] == water_chemistry_observation.unit + + +def test_get_water_chemistry_observation_by_id_404_not_found( + water_chemistry_observation, +): + bad_id = 99999 + response = client.get(f"/observation/water-chemistry/{bad_id}") + assert response.status_code == 404 + data = response.json() + assert data["detail"] == f"Observation with ID {bad_id} not found." + + +def test_get_water_chemistry_observation_by_id_404_wrong_observation_class( + groundwater_level_observation, +): + response = client.get( + f"/observation/water-chemistry/{groundwater_level_observation.id}" + ) + assert response.status_code == 404 + data = response.json() + assert ( + data["detail"][0]["msg"] + == f"Observation with ID {groundwater_level_observation.id} is not a water chemistry observation." + ) + assert data["detail"][0]["type"] == "value_error" + assert data["detail"][0]["input"] == { + "observation_id": groundwater_level_observation.id + } + assert data["detail"][0]["loc"] == ["path", "observation_id"] + + # JB's comment: I don't think that geographic filters are necessary for # observations. I think that they should only be applicable to finding Things # and locations. Then the user can proceed from there to find observations. From 3ded37696acb8f972b09509b0c1c96bb16d35733 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Tue, 19 Aug 2025 12:02:56 -0600 Subject: [PATCH 16/45] refactor: specify URL for 404 wrong observation class --- api/observation.py | 12 +++++-- tests/test_observation.py | 68 ++++++++++++++++++++++----------------- 2 files changed, 48 insertions(+), 32 deletions(-) diff --git a/api/observation.py b/api/observation.py index d1396e81e..1c985713b 100644 --- a/api/observation.py +++ b/api/observation.py @@ -171,12 +171,16 @@ def get_groundwater_level_observation_by_id( ) -> GroundwaterLevelObservationResponse: observation = simple_get_by_id(session, Observation, observation_id) if observation.observed_property != "groundwater level": + if observation.observed_property == "temperature": + url = f"/observation/geothermal/{observation_id}" + else: + url = f"/observation/water-chemistry/{observation_id}" raise PydanticStyleException( status_code=HTTP_404_NOT_FOUND, detail=[ { "loc": ["path", "observation_id"], - "msg": f"Observation with ID {observation_id} is not a groundwater level observation.", + "msg": f"Observation with ID {observation_id} is not a groundwater level observation. To retrieve it, use the following URL: {url}", "type": "value_error", "input": {"observation_id": observation_id}, } @@ -223,12 +227,16 @@ def get_water_chemistry_observation_by_id( ) -> WaterChemistryObservationResponse: observation = simple_get_by_id(session, Observation, observation_id) if observation.observed_property in ("groundwater level", "temperature"): + if observation.observed_property == "groundwater level": + url = f"/observation/groundwater-level/{observation_id}" + else: + url = f"/observation/geothermal/{observation_id}" raise PydanticStyleException( status_code=HTTP_404_NOT_FOUND, detail=[ { "loc": ["path", "observation_id"], - "msg": f"Observation with ID {observation_id} is not a water chemistry observation.", + "msg": f"Observation with ID {observation_id} is not a water chemistry observation. To retrieve it, use the following URL: {url}", "type": "value_error", "input": {"observation_id": observation_id}, } diff --git a/tests/test_observation.py b/tests/test_observation.py index 7abf00150..917b014d0 100644 --- a/tests/test_observation.py +++ b/tests/test_observation.py @@ -202,22 +202,25 @@ def test_get_groundwater_level_observation_by_id_404_not_found( def test_get_groundwater_level_observation_by_id_404_wrong_observation_class( - water_chemistry_observation, + water_chemistry_observation, geothermal_observation ): - response = client.get( - f"/observation/groundwater-level/{water_chemistry_observation.id}" - ) - assert response.status_code == 404 - data = response.json() - assert ( - data["detail"][0]["msg"] - == f"Observation with ID {water_chemistry_observation.id} is not a groundwater level observation." - ) - assert data["detail"][0]["type"] == "value_error" - assert data["detail"][0]["input"] == { - "observation_id": water_chemistry_observation.id - } - assert data["detail"][0]["loc"] == ["path", "observation_id"] + for obs in water_chemistry_observation, geothermal_observation: + response = client.get(f"/observation/groundwater-level/{obs.id}") + assert response.status_code == 404 + data = response.json() + + if obs.observed_property == "temperature": + url = f"/observation/geothermal/{obs.id}" + else: + url = f"/observation/water-chemistry/{obs.id}" + + assert ( + data["detail"][0]["msg"] + == f"Observation with ID {obs.id} is not a groundwater level observation. To retrieve it, use the following URL: {url}" + ) + assert data["detail"][0]["type"] == "value_error" + assert data["detail"][0]["input"] == {"observation_id": obs.id} + assert data["detail"][0]["loc"] == ["path", "observation_id"] def test_get_groundwater_observation_by_sample(sample): @@ -341,22 +344,27 @@ def test_get_water_chemistry_observation_by_id_404_not_found( def test_get_water_chemistry_observation_by_id_404_wrong_observation_class( - groundwater_level_observation, + groundwater_level_observation, geothermal_observation ): - response = client.get( - f"/observation/water-chemistry/{groundwater_level_observation.id}" - ) - assert response.status_code == 404 - data = response.json() - assert ( - data["detail"][0]["msg"] - == f"Observation with ID {groundwater_level_observation.id} is not a water chemistry observation." - ) - assert data["detail"][0]["type"] == "value_error" - assert data["detail"][0]["input"] == { - "observation_id": groundwater_level_observation.id - } - assert data["detail"][0]["loc"] == ["path", "observation_id"] + for obs in groundwater_level_observation, geothermal_observation: + response = client.get(f"/observation/water-chemistry/{obs.id}") + assert response.status_code == 404 + data = response.json() + + if obs.observed_property == "groundwater level": + url = f"/observation/groundwater-level/{obs.id}" + elif obs.observed_property == "temperature": + url = f"/observation/geothermal/{obs.id}" + else: + url = f"/observation/water-chemistry/{obs.id}" + + assert ( + data["detail"][0]["msg"] + == f"Observation with ID {obs.id} is not a water chemistry observation. To retrieve it, use the following URL: {url}" + ) + assert data["detail"][0]["type"] == "value_error" + assert data["detail"][0]["input"] == {"observation_id": obs.id} + assert data["detail"][0]["loc"] == ["path", "observation_id"] # JB's comment: I don't think that geographic filters are necessary for From dc4f5a1138232c4d6931b7bbdd7fb993318912df Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Tue, 19 Aug 2025 13:39:57 -0600 Subject: [PATCH 17/45] feat: implement get observations for observation classes --- api/observation.py | 56 ++++++++++++++++++++++++++++++ tests/conftest.py | 2 +- tests/test_observation.py | 71 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 128 insertions(+), 1 deletion(-) diff --git a/api/observation.py b/api/observation.py index 1c985713b..f5f8b83fd 100644 --- a/api/observation.py +++ b/api/observation.py @@ -90,6 +90,8 @@ def wrapper(*args, **kwargs): elif observation_class == "water chemistry": sql = sql.where(Observation.observed_property != "groundwater level") sql = sql.where(Observation.observed_property != "temperature") + elif observation_class == "geothermal": + sql = sql.where(Observation.observed_property == "temperature") return paginate(query=sql, conn=session) return wrapper @@ -246,4 +248,58 @@ def get_water_chemistry_observation_by_id( return observation +@router.get("/geothermal", summary="Get geothermal observations") +@specify_observation_class(observation_class="geothermal") +def get_geothermal_observations( + session: session_dependency, + thing_id: int | None = None, + sensor_id: int | None = None, + sample_id: int | None = None, + start_time: datetime | None = None, + end_time: datetime | None = None, + sort: str | None = None, + order: str | None = None, + filter_: str = Query(alias="filter", default=None), +) -> CustomPage[GeothermalObservationResponse]: + """ + Retrieve all geothermal observations from the database. + """ + return get_observations( + session=session, + thing_id=thing_id, + sensor_id=sensor_id, + sample_id=sample_id, + start_time=start_time, + end_time=end_time, + sort=sort, + order=order, + filter_=filter_, + ) + + +@router.get("/geothermal/{observation_id}", summary="Get geothermal observation by ID") +def get_geothermal_observation_by_id( + session: session_dependency, observation_id: int +) -> GeothermalObservationResponse: + observation = simple_get_by_id(session, Observation, observation_id) + if observation.observed_property != "temperature": + if observation.observed_property == "groundwater level": + url = f"/observation/groundwater-level/{observation_id}" + else: + url = f"/observation/water-chemistry/{observation_id}" + raise PydanticStyleException( + status_code=HTTP_404_NOT_FOUND, + detail=[ + { + "loc": ["path", "observation_id"], + "msg": f"Observation with ID {observation_id} is not a geothermal observation. To retrieve it, use the following URL: {url}", + "type": "value_error", + "input": {"observation_id": observation_id}, + } + ], + ) + else: + return observation + + # ============= EOF ============================================= diff --git a/tests/conftest.py b/tests/conftest.py index c0469508e..c24d4134c 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -225,7 +225,7 @@ def geothermal_observation(sensor, sample): release_status="draft", value=20.0, unit="deg C", - observation_depth="200", + observation_depth=200.0, ) session.add(observation) session.commit() diff --git a/tests/test_observation.py b/tests/test_observation.py index 917b014d0..e9060aa6e 100644 --- a/tests/test_observation.py +++ b/tests/test_observation.py @@ -367,6 +367,77 @@ def test_get_water_chemistry_observation_by_id_404_wrong_observation_class( assert data["detail"][0]["loc"] == ["path", "observation_id"] +def test_get_geothermal_observations(geothermal_observation): + response = client.get("/observation/geothermal") + assert response.status_code == 200 + data = response.json() + assert data["total"] == 1 + assert data["items"][0]["id"] == geothermal_observation.id + assert data["items"][0]["sample_id"] == geothermal_observation.sample_id + assert data["items"][0]["sensor_id"] == geothermal_observation.sensor_id + assert ( + data["items"][0]["observation_datetime"] + == geothermal_observation.observation_datetime + ) + assert ( + data["items"][0]["observed_property"] + == geothermal_observation.observed_property + ) + assert data["items"][0]["value"] == geothermal_observation.value + assert data["items"][0]["unit"] == geothermal_observation.unit + assert ( + data["items"][0]["observation_depth"] + == geothermal_observation.observation_depth + ) + + +def test_get_geothermal_observation_by_id(geothermal_observation): + response = client.get(f"/observation/geothermal/{geothermal_observation.id}") + assert response.status_code == 200 + data = response.json() + assert data["id"] == geothermal_observation.id + assert data["created_at"] == geothermal_observation.created_at.isoformat().replace( + "+00:00", "Z" + ) + assert data["sample_id"] == geothermal_observation.sample_id + assert data["sensor_id"] == geothermal_observation.sensor_id + assert data["observation_datetime"] == geothermal_observation.observation_datetime + assert data["observed_property"] == geothermal_observation.observed_property + assert data["value"] == geothermal_observation.value + assert data["unit"] == geothermal_observation.unit + assert data["observation_depth"] == geothermal_observation.observation_depth + + +def test_get_geothermal_observation_by_id_404_not_found(geothermal_observation): + bad_id = 99999 + response = client.get(f"/observation/geothermal/{bad_id}") + assert response.status_code == 404 + data = response.json() + assert data["detail"] == f"Observation with ID {bad_id} not found." + + +def test_get_geothermal_observation_by_id_404_wrong_observation_class( + water_chemistry_observation, groundwater_level_observation +): + for obs in water_chemistry_observation, groundwater_level_observation: + response = client.get(f"/observation/geothermal/{obs.id}") + assert response.status_code == 404 + data = response.json() + + if obs.observed_property == "groundwater level": + url = f"/observation/groundwater-level/{obs.id}" + else: + url = f"/observation/water-chemistry/{obs.id}" + + assert ( + data["detail"][0]["msg"] + == f"Observation with ID {obs.id} is not a geothermal observation. To retrieve it, use the following URL: {url}" + ) + assert data["detail"][0]["type"] == "value_error" + assert data["detail"][0]["input"] == {"observation_id": obs.id} + assert data["detail"][0]["loc"] == ["path", "observation_id"] + + # JB's comment: I don't think that geographic filters are necessary for # observations. I think that they should only be applicable to finding Things # and locations. Then the user can proceed from there to find observations. From 4db3473e6212c48af20c8c246e0854e33e199c00 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Tue, 19 Aug 2025 14:46:54 -0600 Subject: [PATCH 18/45] feat: implement general GET for /observation --- api/observation.py | 36 ++++++++++++++++++++++++++++- schemas/observation.py | 8 ++++--- tests/test_observation.py | 48 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 88 insertions(+), 4 deletions(-) diff --git a/api/observation.py b/api/observation.py index f5f8b83fd..ad5b48bf9 100644 --- a/api/observation.py +++ b/api/observation.py @@ -31,6 +31,7 @@ WaterChemistryObservationResponse, CreateGeothermalObservation, GeothermalObservationResponse, + ObservationResponse, ) from services.exceptions_helper import PydanticStyleException from services.query_helper import order_sort_filter, simple_get_by_id @@ -80,7 +81,7 @@ def add_geothermal_observation( # ============= Get ============================================== -def specify_observation_class(observation_class: str): +def specify_observation_class(observation_class: str | None): def decorator(func): @functools.wraps(func) def wrapper(*args, **kwargs): @@ -302,4 +303,37 @@ def get_geothermal_observation_by_id( return observation +@router.get("", summary="Get all observations") +@specify_observation_class(observation_class=None) +def get_all_observations( + session: session_dependency, + thing_id: int | None = None, + sensor_id: int | None = None, + sample_id: int | None = None, + start_time: datetime | None = None, + end_time: datetime | None = None, + sort: str | None = None, + order: str | None = None, + filter_: str = Query(alias="filter", default=None), +) -> CustomPage[ObservationResponse]: + return get_observations( + session=session, + thing_id=thing_id, + sensor_id=sensor_id, + sample_id=sample_id, + start_time=start_time, + end_time=end_time, + sort=sort, + order=order, + filter_=filter_, + ) + + +@router.get("/{observation_id}", summary="Get an observation by its ID") +def get_observation_by_id( + session: session_dependency, observation_id: int +) -> ObservationResponse: + return simple_get_by_id(session, Observation, observation_id) + + # ============= EOF ============================================= diff --git a/schemas/observation.py b/schemas/observation.py index 39bb029f6..9118d3022 100644 --- a/schemas/observation.py +++ b/schemas/observation.py @@ -118,15 +118,17 @@ class BaseObservationResponse(ORMBaseModel): class GroundwaterLevelObservationResponse(BaseObservationResponse): depth_to_water_bgs: float | None - measuring_point_height: float + measuring_point_height: float | None level_status: str | None @model_validator(mode="before") def calculate_depth_to_water_bgs(self: Self) -> Self: depth_to_water = self.value measuring_point_height = self.measuring_point_height - if depth_to_water is not None: + if depth_to_water is not None and measuring_point_height is not None: self.depth_to_water_bgs = depth_to_water - measuring_point_height + else: + self.depth_to_water_bgs = None return self @@ -135,7 +137,7 @@ class WaterChemistryObservationResponse(BaseObservationResponse): class GeothermalObservationResponse(BaseObservationResponse): - observation_depth: float + observation_depth: float | None class ObservationResponse( diff --git a/tests/test_observation.py b/tests/test_observation.py index e9060aa6e..11d3d8d5c 100644 --- a/tests/test_observation.py +++ b/tests/test_observation.py @@ -119,6 +119,54 @@ def test_add_geothermal_observation(sample, sensor): # ============= Get tests ================= + + +def test_get_all_observations( + groundwater_level_observation, water_chemistry_observation, geothermal_observation +): + response = client.get("/observation") + assert response.status_code == 200 + data = response.json() + assert data["total"] == 3 + assert data["items"][0]["id"] == groundwater_level_observation.id + assert data["items"][1]["id"] == water_chemistry_observation.id + assert data["items"][2]["id"] == geothermal_observation.id + + +def test_get_observation_by_id( + groundwater_level_observation, water_chemistry_observation, geothermal_observation +): + for obs in ( + groundwater_level_observation, + water_chemistry_observation, + geothermal_observation, + ): + response = client.get(f"/observation/{obs.id}") + assert response.status_code == 200 + data = response.json() + + assert data["id"] == obs.id + if obs.observed_property == "groundwater level": + assert data["depth_to_water_bgs"] == obs.value - obs.measuring_point_height + assert data["observation_depth"] is None + elif obs.observed_property == "temperature": + assert data["depth_to_water_bgs"] is None + assert data["observation_depth"] == obs.observation_depth + else: + assert data["depth_to_water_bgs"] is None + assert data["observation_depth"] is None + + +def test_get_observation_by_id_404_not_found( + groundwater_level_observation, water_chemistry_observation, geothermal_observation +): + bad_id = 999999 + response = client.get(f"/observation/{bad_id}") + assert response.status_code == 404 + data = response.json() + assert data["detail"] == f"Observation with ID {bad_id} not found." + + def test_get_groundwater_level_observations( groundwater_level_observation, water_chemistry_observation, geothermal_observation ): From 75a54c431c24778b61be01fe095a3e2d90779602 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Tue, 19 Aug 2025 15:01:44 -0600 Subject: [PATCH 19/45] feat: add auth to GET endpoints --- api/observation.py | 20 +++++++++++++++----- tests/test_observation.py | 9 ++++++++- 2 files changed, 23 insertions(+), 6 deletions(-) diff --git a/api/observation.py b/api/observation.py index ad5b48bf9..eaf65ebfe 100644 --- a/api/observation.py +++ b/api/observation.py @@ -22,7 +22,13 @@ from starlette.status import HTTP_201_CREATED, HTTP_404_NOT_FOUND from api.pagination import CustomPage -from core.dependencies import session_dependency, amp_admin_dependency, admin_dependency +from core.dependencies import ( + session_dependency, + amp_admin_dependency, + admin_dependency, + amp_viewer_dependency, + viewer_dependency, +) from db import Sample, Observation, adder from schemas.observation import ( CreateGroundwaterLevelObservation, @@ -140,6 +146,7 @@ def get_observations( @specify_observation_class(observation_class="groundwater level") def get_groundwater_level_observations( session: session_dependency, + user: amp_viewer_dependency, thing_id: int | None = None, sensor_id: int | None = None, sample_id: int | None = None, @@ -170,7 +177,7 @@ def get_groundwater_level_observations( summary="Get groundwater level observation by ID", ) def get_groundwater_level_observation_by_id( - session: session_dependency, observation_id: int + session: session_dependency, user: amp_viewer_dependency, observation_id: int ) -> GroundwaterLevelObservationResponse: observation = simple_get_by_id(session, Observation, observation_id) if observation.observed_property != "groundwater level": @@ -197,6 +204,7 @@ def get_groundwater_level_observation_by_id( @specify_observation_class(observation_class="water chemistry") def get_water_chemistry_observations( session: session_dependency, + user: amp_viewer_dependency, thing_id: int | None = None, sensor_id: int | None = None, sample_id: int | None = None, @@ -226,7 +234,7 @@ def get_water_chemistry_observations( "/water-chemistry/{observation_id}", summary="Get water chemistry observation by ID" ) def get_water_chemistry_observation_by_id( - session: session_dependency, observation_id: int + session: session_dependency, user: amp_viewer_dependency, observation_id: int ) -> WaterChemistryObservationResponse: observation = simple_get_by_id(session, Observation, observation_id) if observation.observed_property in ("groundwater level", "temperature"): @@ -253,6 +261,7 @@ def get_water_chemistry_observation_by_id( @specify_observation_class(observation_class="geothermal") def get_geothermal_observations( session: session_dependency, + user: viewer_dependency, thing_id: int | None = None, sensor_id: int | None = None, sample_id: int | None = None, @@ -280,7 +289,7 @@ def get_geothermal_observations( @router.get("/geothermal/{observation_id}", summary="Get geothermal observation by ID") def get_geothermal_observation_by_id( - session: session_dependency, observation_id: int + session: session_dependency, user: amp_viewer_dependency, observation_id: int ) -> GeothermalObservationResponse: observation = simple_get_by_id(session, Observation, observation_id) if observation.observed_property != "temperature": @@ -307,6 +316,7 @@ def get_geothermal_observation_by_id( @specify_observation_class(observation_class=None) def get_all_observations( session: session_dependency, + user: amp_viewer_dependency, thing_id: int | None = None, sensor_id: int | None = None, sample_id: int | None = None, @@ -331,7 +341,7 @@ def get_all_observations( @router.get("/{observation_id}", summary="Get an observation by its ID") def get_observation_by_id( - session: session_dependency, observation_id: int + session: session_dependency, user: amp_viewer_dependency, observation_id: int ) -> ObservationResponse: return simple_get_by_id(session, Observation, observation_id) diff --git a/tests/test_observation.py b/tests/test_observation.py index 11d3d8d5c..2b6169ba9 100644 --- a/tests/test_observation.py +++ b/tests/test_observation.py @@ -14,7 +14,12 @@ # limitations under the License. # =============================================================================== from db import Observation -from core.dependencies import amp_admin_function, admin_function +from core.dependencies import ( + amp_admin_function, + admin_function, + amp_viewer_function, + viewer_function, +) from main import app from tests import client, cleanup_post_test, override_authentication import pytest @@ -28,6 +33,8 @@ def override_authentication_dependency_fixture(): app.dependency_overrides[admin_function] = override_authentication( default={"name": "foobar", "sub": "1234567890"} ) + app.dependency_overrides[amp_viewer_function] = override_authentication() + app.dependency_overrides[viewer_function] = override_authentication() yield From edda50b64082646ee32d4fe91fcb319e85f642f1 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Tue, 19 Aug 2025 15:57:42 -0600 Subject: [PATCH 20/45] feat: implement PATCH for groundwater level observations --- api/observation.py | 22 +++++++++++- schemas/observation.py | 2 +- services/observation_helper.py | 63 ++++++++++++++++++++++++++++++++++ tests/test_observation.py | 52 +++++++++++++++++++++++++++- 4 files changed, 136 insertions(+), 3 deletions(-) create mode 100644 services/observation_helper.py diff --git a/api/observation.py b/api/observation.py index eaf65ebfe..0745c01d5 100644 --- a/api/observation.py +++ b/api/observation.py @@ -19,7 +19,7 @@ from fastapi import APIRouter, Query from fastapi_pagination.ext.sqlalchemy import paginate from sqlalchemy import select -from starlette.status import HTTP_201_CREATED, HTTP_404_NOT_FOUND +from starlette.status import HTTP_200_OK, HTTP_201_CREATED, HTTP_404_NOT_FOUND from api.pagination import CustomPage from core.dependencies import ( @@ -38,9 +38,11 @@ CreateGeothermalObservation, GeothermalObservationResponse, ObservationResponse, + UpdateGroundwaterLevelObservation, ) from services.exceptions_helper import PydanticStyleException from services.query_helper import order_sort_filter, simple_get_by_id +from services.observation_helper import observation_model_patcher router = APIRouter(prefix="/observation", tags=["observation"]) @@ -84,6 +86,24 @@ def add_geothermal_observation( return adder(session, Observation, obs_data, user=user) +# PATCH ======================================================================== + + +@router.patch("/groundwater-level/{observation_id}", status_code=HTTP_200_OK) +def update_groundwater_level_observation( + observation_id: int, + obs_data: UpdateGroundwaterLevelObservation, + session: session_dependency, + user: amp_admin_dependency, +) -> GroundwaterLevelObservationResponse: + """ + Update an existing groundwater level observation in the database. + """ + return observation_model_patcher( + session, Observation, observation_id, obs_data, "groundwater level", user + ) + + # ============= Get ============================================== diff --git a/schemas/observation.py b/schemas/observation.py index 9118d3022..664fe0baa 100644 --- a/schemas/observation.py +++ b/schemas/observation.py @@ -83,7 +83,7 @@ class CreateGeothermalObservation(CreateBaseObservation): class UpdateBaseObservation(ValidateObservation): - observation_datetime: Annotated[AwareDatetime, PastDatetime()] + observation_datetime: Annotated[AwareDatetime, PastDatetime()] | None = None sample_id: int | None = None sensor_id: int | None = None observed_property: str | None = None diff --git a/services/observation_helper.py b/services/observation_helper.py new file mode 100644 index 000000000..635cff344 --- /dev/null +++ b/services/observation_helper.py @@ -0,0 +1,63 @@ +from pydantic import BaseModel +from sqlalchemy.orm import Session, DeclarativeBase +from starlette.status import HTTP_404_NOT_FOUND + +from services.exceptions_helper import PydanticStyleException +from services.query_helper import simple_get_by_id + +observation_class_to_observed_properties = { + "groundwater level": ["groundwater level"], + "geothermal": ["temperature"], + "water chemistry": ["pH", "Alkalinity as CaCO3"], +} + +observation_property_to_class = {} +for key, value in observation_class_to_observed_properties.items(): + for prop in value: + observation_property_to_class[prop] = key + + +def observation_model_patcher( + session: Session, + model: DeclarativeBase, + item_id: int, + payload: BaseModel, + observation_class: str, + user: dict, +) -> object: + """ + Patch an observation model with the provided payload. + """ + # simple_get_by_id raises HTTP_404_NOT_FOUND if the item is not found + item = simple_get_by_id(session, model, item_id) + + observed_property = item.observed_property + + if ( + observed_property + not in observation_class_to_observed_properties[observation_class] + ): + actual_observation_class = observation_property_to_class[observed_property] + + raise PydanticStyleException( + status_code=HTTP_404_NOT_FOUND, + detail=[ + { + "loc": ["path", "observation_id"], + "type": "value_error", + "input": {"observation_id": item_id}, + "msg": f"{observation_class.capitalize()} observation with ID {item_id} not found. It is a {actual_observation_class} observation.", + } + ], + ) + + for key, value in payload.model_dump(exclude_unset=True).items(): + setattr(item, key, value) + + if user: + item.updated_by_id = user["sub"] + item.updated_by_name = user["name"] + + session.commit() + session.refresh(item) + return item diff --git a/tests/test_observation.py b/tests/test_observation.py index 2b6169ba9..bdbde439e 100644 --- a/tests/test_observation.py +++ b/tests/test_observation.py @@ -21,7 +21,7 @@ viewer_function, ) from main import app -from tests import client, cleanup_post_test, override_authentication +from tests import client, cleanup_post_test, override_authentication, cleanup_patch_test import pytest @@ -125,6 +125,56 @@ def test_add_geothermal_observation(sample, sensor): cleanup_post_test(Observation, data["id"]) +# PATCH tests ================================================================== + + +def test_patch_groundwater_level_observation(groundwater_level_observation): + payload = {"measuring_point_height": 3} + response = client.patch( + f"/observation/groundwater-level/{groundwater_level_observation.id}", + json=payload, + ) + data = response.json() + assert response.status_code == 200 + + assert data["measuring_point_height"] == payload["measuring_point_height"] + + cleanup_patch_test(Observation, payload, groundwater_level_observation) + + +def test_patch_groundwater_level_observation_404_not_found( + groundwater_level_observation, +): + bad_id = 99999 + payload = {"measuring_point_height": 3} + response = client.patch(f"/observation/groundwater-level/{bad_id}", json=payload) + assert response.status_code == 404 + data = response.json() + assert data["detail"] == f"Observation with ID {bad_id} not found." + + +def test_patch_groundwater_level_observation_404_wrong_observation_class( + water_chemistry_observation, geothermal_observation +): + for obs in water_chemistry_observation, geothermal_observation: + payload = {"measuring_point_height": 3} + response = client.patch( + f"/observation/groundwater-level/{obs.id}", json=payload + ) + assert response.status_code == 404 + data = response.json() + + if obs.observed_property == "temperature": + observation_class = "geothermal" + else: + observation_class = "water chemistry" + + assert ( + data["detail"][0]["msg"] + == f"Groundwater level observation with ID {obs.id} not found. It is a {observation_class} observation." + ) + + # ============= Get tests ================= From 1ab5c4999e88e802cca7b84db3f99898fb0e65e4 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Tue, 19 Aug 2025 16:11:03 -0600 Subject: [PATCH 21/45] feat: create observation helpers for sub tables in observation table --- services/observation_helper.py | 73 ++++++++++++++++++++++++---------- 1 file changed, 52 insertions(+), 21 deletions(-) diff --git a/services/observation_helper.py b/services/observation_helper.py index 635cff344..abbef9584 100644 --- a/services/observation_helper.py +++ b/services/observation_helper.py @@ -1,7 +1,8 @@ from pydantic import BaseModel -from sqlalchemy.orm import Session, DeclarativeBase +from sqlalchemy.orm import Session from starlette.status import HTTP_404_NOT_FOUND +from db import Observation from services.exceptions_helper import PydanticStyleException from services.query_helper import simple_get_by_id @@ -17,21 +18,16 @@ observation_property_to_class[prop] = key -def observation_model_patcher( - session: Session, - model: DeclarativeBase, - item_id: int, - payload: BaseModel, - observation_class: str, - user: dict, -) -> object: +def verify_observed_property_corresponds_with_observation_class( + observation: Observation, observation_class: str +) -> None: """ - Patch an observation model with the provided payload. + Verify that the observed property of the retrieved Observation corresponds + with the observation class as defined by the path + (e.g. /observation/water-chemistry). Raise an error if they do not + correspond. """ - # simple_get_by_id raises HTTP_404_NOT_FOUND if the item is not found - item = simple_get_by_id(session, model, item_id) - - observed_property = item.observed_property + observed_property = observation.observed_property if ( observed_property @@ -45,19 +41,54 @@ def observation_model_patcher( { "loc": ["path", "observation_id"], "type": "value_error", - "input": {"observation_id": item_id}, - "msg": f"{observation_class.capitalize()} observation with ID {item_id} not found. It is a {actual_observation_class} observation.", + "input": {"observation_id": observation.id}, + "msg": f"{observation_class.capitalize()} observation with ID {observation.id} not found. It is a {actual_observation_class} observation.", } ], ) + else: + return True + + +def get_observation_of_an_observation_class_by_id( + session: Session, observation_id: int, observation_class: str +) -> Observation: + """ + Retrieve an observation by its ID. + """ + observation = simple_get_by_id(session, Observation, observation_id) + + verify_observed_property_corresponds_with_observation_class( + observation, observation_class + ) + + return observation + + +def observation_model_patcher( + session: Session, + observation_id: int, + payload: BaseModel, + observation_class: str, + user: dict, +) -> Observation: + """ + Patch an observation model with the provided payload. + """ + # simple_get_by_id raises HTTP_404_NOT_FOUND if the item is not found + observation = simple_get_by_id(session, Observation, observation_id) + + verify_observed_property_corresponds_with_observation_class( + observation, observation_class + ) for key, value in payload.model_dump(exclude_unset=True).items(): - setattr(item, key, value) + setattr(observation, key, value) if user: - item.updated_by_id = user["sub"] - item.updated_by_name = user["name"] + observation.updated_by_id = user["sub"] + observation.updated_by_name = user["name"] session.commit() - session.refresh(item) - return item + session.refresh(observation) + return observation From 494487161ee1d054deaba4c70102eed1a717ea10 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Tue, 19 Aug 2025 16:20:47 -0600 Subject: [PATCH 22/45] refactor: update error msg for wrong observation class ok id --- api/observation.py | 80 ++++++++-------------------------- services/observation_helper.py | 4 +- tests/test_observation.py | 20 ++++----- 3 files changed, 30 insertions(+), 74 deletions(-) diff --git a/api/observation.py b/api/observation.py index 0745c01d5..7c6926aec 100644 --- a/api/observation.py +++ b/api/observation.py @@ -19,7 +19,7 @@ from fastapi import APIRouter, Query from fastapi_pagination.ext.sqlalchemy import paginate from sqlalchemy import select -from starlette.status import HTTP_200_OK, HTTP_201_CREATED, HTTP_404_NOT_FOUND +from starlette.status import HTTP_200_OK, HTTP_201_CREATED from api.pagination import CustomPage from core.dependencies import ( @@ -40,9 +40,11 @@ ObservationResponse, UpdateGroundwaterLevelObservation, ) -from services.exceptions_helper import PydanticStyleException from services.query_helper import order_sort_filter, simple_get_by_id -from services.observation_helper import observation_model_patcher +from services.observation_helper import ( + observation_model_patcher, + get_observation_of_an_observation_class_by_id, +) router = APIRouter(prefix="/observation", tags=["observation"]) @@ -100,7 +102,7 @@ def update_groundwater_level_observation( Update an existing groundwater level observation in the database. """ return observation_model_patcher( - session, Observation, observation_id, obs_data, "groundwater level", user + session, observation_id, obs_data, "groundwater level", user ) @@ -199,25 +201,11 @@ def get_groundwater_level_observations( def get_groundwater_level_observation_by_id( session: session_dependency, user: amp_viewer_dependency, observation_id: int ) -> GroundwaterLevelObservationResponse: - observation = simple_get_by_id(session, Observation, observation_id) - if observation.observed_property != "groundwater level": - if observation.observed_property == "temperature": - url = f"/observation/geothermal/{observation_id}" - else: - url = f"/observation/water-chemistry/{observation_id}" - raise PydanticStyleException( - status_code=HTTP_404_NOT_FOUND, - detail=[ - { - "loc": ["path", "observation_id"], - "msg": f"Observation with ID {observation_id} is not a groundwater level observation. To retrieve it, use the following URL: {url}", - "type": "value_error", - "input": {"observation_id": observation_id}, - } - ], - ) - else: - return observation + return get_observation_of_an_observation_class_by_id( + session=session, + observation_id=observation_id, + observation_class="groundwater level", + ) @router.get("/water-chemistry", summary="Get water chemistry observations") @@ -256,25 +244,11 @@ def get_water_chemistry_observations( def get_water_chemistry_observation_by_id( session: session_dependency, user: amp_viewer_dependency, observation_id: int ) -> WaterChemistryObservationResponse: - observation = simple_get_by_id(session, Observation, observation_id) - if observation.observed_property in ("groundwater level", "temperature"): - if observation.observed_property == "groundwater level": - url = f"/observation/groundwater-level/{observation_id}" - else: - url = f"/observation/geothermal/{observation_id}" - raise PydanticStyleException( - status_code=HTTP_404_NOT_FOUND, - detail=[ - { - "loc": ["path", "observation_id"], - "msg": f"Observation with ID {observation_id} is not a water chemistry observation. To retrieve it, use the following URL: {url}", - "type": "value_error", - "input": {"observation_id": observation_id}, - } - ], - ) - else: - return observation + return get_observation_of_an_observation_class_by_id( + session=session, + observation_id=observation_id, + observation_class="water chemistry", + ) @router.get("/geothermal", summary="Get geothermal observations") @@ -311,25 +285,9 @@ def get_geothermal_observations( def get_geothermal_observation_by_id( session: session_dependency, user: amp_viewer_dependency, observation_id: int ) -> GeothermalObservationResponse: - observation = simple_get_by_id(session, Observation, observation_id) - if observation.observed_property != "temperature": - if observation.observed_property == "groundwater level": - url = f"/observation/groundwater-level/{observation_id}" - else: - url = f"/observation/water-chemistry/{observation_id}" - raise PydanticStyleException( - status_code=HTTP_404_NOT_FOUND, - detail=[ - { - "loc": ["path", "observation_id"], - "msg": f"Observation with ID {observation_id} is not a geothermal observation. To retrieve it, use the following URL: {url}", - "type": "value_error", - "input": {"observation_id": observation_id}, - } - ], - ) - else: - return observation + return get_observation_of_an_observation_class_by_id( + session=session, observation_id=observation_id, observation_class="geothermal" + ) @router.get("", summary="Get all observations") diff --git a/services/observation_helper.py b/services/observation_helper.py index abbef9584..b7b47a87a 100644 --- a/services/observation_helper.py +++ b/services/observation_helper.py @@ -42,12 +42,10 @@ def verify_observed_property_corresponds_with_observation_class( "loc": ["path", "observation_id"], "type": "value_error", "input": {"observation_id": observation.id}, - "msg": f"{observation_class.capitalize()} observation with ID {observation.id} not found. It is a {actual_observation_class} observation.", + "msg": f"Observation with ID {observation.id} is not a {observation_class} observation. It is a {actual_observation_class} observation.", } ], ) - else: - return True def get_observation_of_an_observation_class_by_id( diff --git a/tests/test_observation.py b/tests/test_observation.py index bdbde439e..6dbb55b43 100644 --- a/tests/test_observation.py +++ b/tests/test_observation.py @@ -171,7 +171,7 @@ def test_patch_groundwater_level_observation_404_wrong_observation_class( assert ( data["detail"][0]["msg"] - == f"Groundwater level observation with ID {obs.id} not found. It is a {observation_class} observation." + == f"Observation with ID {obs.id} is not a groundwater level observation. It is a {observation_class} observation." ) @@ -315,13 +315,13 @@ def test_get_groundwater_level_observation_by_id_404_wrong_observation_class( data = response.json() if obs.observed_property == "temperature": - url = f"/observation/geothermal/{obs.id}" + actual_observation_class = "geothermal" else: - url = f"/observation/water-chemistry/{obs.id}" + actual_observation_class = "water chemistry" assert ( data["detail"][0]["msg"] - == f"Observation with ID {obs.id} is not a groundwater level observation. To retrieve it, use the following URL: {url}" + == f"Observation with ID {obs.id} is not a groundwater level observation. It is a {actual_observation_class} observation." ) assert data["detail"][0]["type"] == "value_error" assert data["detail"][0]["input"] == {"observation_id": obs.id} @@ -457,15 +457,15 @@ def test_get_water_chemistry_observation_by_id_404_wrong_observation_class( data = response.json() if obs.observed_property == "groundwater level": - url = f"/observation/groundwater-level/{obs.id}" + actual_observation_class = "groundwater level" elif obs.observed_property == "temperature": - url = f"/observation/geothermal/{obs.id}" + actual_observation_class = "geothermal" else: url = f"/observation/water-chemistry/{obs.id}" assert ( data["detail"][0]["msg"] - == f"Observation with ID {obs.id} is not a water chemistry observation. To retrieve it, use the following URL: {url}" + == f"Observation with ID {obs.id} is not a water chemistry observation. It is a {actual_observation_class} observation." ) assert data["detail"][0]["type"] == "value_error" assert data["detail"][0]["input"] == {"observation_id": obs.id} @@ -530,13 +530,13 @@ def test_get_geothermal_observation_by_id_404_wrong_observation_class( data = response.json() if obs.observed_property == "groundwater level": - url = f"/observation/groundwater-level/{obs.id}" + actual_observation_class = "groundwater level" else: - url = f"/observation/water-chemistry/{obs.id}" + actual_observation_class = "water chemistry" assert ( data["detail"][0]["msg"] - == f"Observation with ID {obs.id} is not a geothermal observation. To retrieve it, use the following URL: {url}" + == f"Observation with ID {obs.id} is not a geothermal observation. It is a {actual_observation_class} observation." ) assert data["detail"][0]["type"] == "value_error" assert data["detail"][0]["input"] == {"observation_id": obs.id} From c626ea36fb4fe569f204b15103c8527b64b7bc18 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Tue, 19 Aug 2025 16:44:39 -0600 Subject: [PATCH 23/45] refactor: update observation fixtures dt for testing order --- tests/conftest.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index c24d4134c..84e60f4b6 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -178,7 +178,7 @@ def asset(): def groundwater_level_observation(sensor, sample): with session_ctx() as session: observation = Observation( - observation_datetime="2025-01-01T00:00:00Z", + observation_datetime="2025-01-01T00:04:00Z", sample_id=sample.id, sensor_id=sensor.id, observed_property="groundwater level", @@ -199,7 +199,7 @@ def groundwater_level_observation(sensor, sample): def water_chemistry_observation(sensor, sample): with session_ctx() as session: observation = Observation( - observation_datetime="2025-01-01T00:00:00Z", + observation_datetime="2025-01-01T00:03:00Z", sample_id=sample.id, sensor_id=sensor.id, observed_property="pH", @@ -218,7 +218,7 @@ def water_chemistry_observation(sensor, sample): def geothermal_observation(sensor, sample): with session_ctx() as session: observation = Observation( - observation_datetime="2025-01-01T00:00:00Z", + observation_datetime="2025-01-01T00:02:00Z", sample_id=sample.id, sensor_id=sensor.id, observed_property="temperature", From 202a63ee8491967ef077348461cbefb342e76fde Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Tue, 19 Aug 2025 16:45:20 -0600 Subject: [PATCH 24/45] feat: implement PATCH for water chemistry observations --- api/observation.py | 16 ++++++++++++++ tests/test_observation.py | 44 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+) diff --git a/api/observation.py b/api/observation.py index 7c6926aec..fd72ad7c6 100644 --- a/api/observation.py +++ b/api/observation.py @@ -39,6 +39,7 @@ GeothermalObservationResponse, ObservationResponse, UpdateGroundwaterLevelObservation, + UpdateWaterChemistryObservation, ) from services.query_helper import order_sort_filter, simple_get_by_id from services.observation_helper import ( @@ -106,6 +107,21 @@ def update_groundwater_level_observation( ) +@router.patch("/water-chemistry/{observation_id}", status_code=HTTP_200_OK) +def update_water_chemistry_observation( + observation_id: int, + obs_data: UpdateWaterChemistryObservation, + session: session_dependency, + user: amp_admin_dependency, +) -> WaterChemistryObservationResponse: + """ + Update an existing water chemistry observation in the database. + """ + return observation_model_patcher( + session, observation_id, obs_data, "water chemistry", user + ) + + # ============= Get ============================================== diff --git a/tests/test_observation.py b/tests/test_observation.py index 6dbb55b43..84f1432f3 100644 --- a/tests/test_observation.py +++ b/tests/test_observation.py @@ -175,6 +175,49 @@ def test_patch_groundwater_level_observation_404_wrong_observation_class( ) +def test_patch_water_chemistry_observation(water_chemistry_observation): + payload = {"value": 8} + response = client.patch( + f"/observation/water-chemistry/{water_chemistry_observation.id}", + json=payload, + ) + data = response.json() + assert response.status_code == 200 + + assert data["value"] == payload["value"] + + cleanup_patch_test(Observation, payload, water_chemistry_observation) + + +def test_patch_water_chemistry_observation_404_not_found(water_chemistry_observation): + bad_id = 999999 + payload = {"value": 8} + response = client.patch(f"/observation/water-chemistry/{bad_id}", json=payload) + assert response.status_code == 404 + data = response.json() + assert data["detail"] == f"Observation with ID {bad_id} not found." + + +def test_patch_water_chemistry_observation_404_wrong_observation_class( + groundwater_level_observation, geothermal_observation +): + for obs in groundwater_level_observation, geothermal_observation: + payload = {"value": 8} + response = client.patch(f"/observation/water-chemistry/{obs.id}", json=payload) + assert response.status_code == 404 + data = response.json() + + if obs.observed_property == "temperature": + observation_class = "geothermal" + else: + observation_class = "groundwater level" + + assert ( + data["detail"][0]["msg"] + == f"Observation with ID {obs.id} is not a water chemistry observation. It is a {observation_class} observation." + ) + + # ============= Get tests ================= @@ -184,6 +227,7 @@ def test_get_all_observations( response = client.get("/observation") assert response.status_code == 200 data = response.json() + print(data) assert data["total"] == 3 assert data["items"][0]["id"] == groundwater_level_observation.id assert data["items"][1]["id"] == water_chemistry_observation.id From caf541ab7e3f164ab78da3ec77dfca3f6d5b97c2 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Tue, 19 Aug 2025 16:53:57 -0600 Subject: [PATCH 25/45] feat: implement PATCH for geothermal observations --- api/observation.py | 16 +++++++++++++++ tests/test_observation.py | 42 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 57 insertions(+), 1 deletion(-) diff --git a/api/observation.py b/api/observation.py index fd72ad7c6..c7a9aacf9 100644 --- a/api/observation.py +++ b/api/observation.py @@ -40,6 +40,7 @@ ObservationResponse, UpdateGroundwaterLevelObservation, UpdateWaterChemistryObservation, + UpdateGeothermalObservation, ) from services.query_helper import order_sort_filter, simple_get_by_id from services.observation_helper import ( @@ -122,6 +123,21 @@ def update_water_chemistry_observation( ) +@router.patch("/geothermal/{observation_id}", status_code=HTTP_200_OK) +def update_geothermal_observation( + observation_id: int, + obs_data: UpdateGeothermalObservation, + session: session_dependency, + user: admin_dependency, +) -> GeothermalObservationResponse: + """ + Update an existing geothermal observation in the database. + """ + return observation_model_patcher( + session, observation_id, obs_data, "geothermal", user + ) + + # ============= Get ============================================== diff --git a/tests/test_observation.py b/tests/test_observation.py index 84f1432f3..26753727f 100644 --- a/tests/test_observation.py +++ b/tests/test_observation.py @@ -218,6 +218,47 @@ def test_patch_water_chemistry_observation_404_wrong_observation_class( ) +def test_patch_geothermal_observation(geothermal_observation): + payload = {"observation_depth": 4} + response = client.patch( + f"/observation/geothermal/{geothermal_observation.id}", json=payload + ) + assert response.status_code == 200 + data = response.json() + assert data["observation_depth"] == payload["observation_depth"] + + cleanup_patch_test(Observation, payload, geothermal_observation) + + +def test_patch_geothermal_observation_404_not_found(geothermal_observation): + bad_id = 999999 + payload = {"observation_depth": 8} + response = client.patch(f"/observation/geothermal/{bad_id}", json=payload) + assert response.status_code == 404 + data = response.json() + assert data["detail"] == f"Observation with ID {bad_id} not found." + + +def test_patch_geothermal_observation_404_wrong_observation_class( + groundwater_level_observation, water_chemistry_observation +): + for obs in groundwater_level_observation, water_chemistry_observation: + payload = {"value": 8} + response = client.patch(f"/observation/geothermal/{obs.id}", json=payload) + assert response.status_code == 404 + data = response.json() + + if obs.observed_property == "groundwater level": + observation_class = "groundwater level" + else: + observation_class = "water chemistry" + + assert ( + data["detail"][0]["msg"] + == f"Observation with ID {obs.id} is not a geothermal observation. It is a {observation_class} observation." + ) + + # ============= Get tests ================= @@ -227,7 +268,6 @@ def test_get_all_observations( response = client.get("/observation") assert response.status_code == 200 data = response.json() - print(data) assert data["total"] == 3 assert data["items"][0]["id"] == groundwater_level_observation.id assert data["items"][1]["id"] == water_chemistry_observation.id From 39be19f7a0b928144823d5d35e914c85f0cda7ef Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Tue, 19 Aug 2025 17:01:45 -0600 Subject: [PATCH 26/45] feat: implement DELETE observation by ID --- api/observation.py | 17 +++++++++++++++- tests/test_observation.py | 42 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 1 deletion(-) diff --git a/api/observation.py b/api/observation.py index c7a9aacf9..4ceae7380 100644 --- a/api/observation.py +++ b/api/observation.py @@ -19,7 +19,7 @@ from fastapi import APIRouter, Query from fastapi_pagination.ext.sqlalchemy import paginate from sqlalchemy import select -from starlette.status import HTTP_200_OK, HTTP_201_CREATED +from starlette.status import HTTP_200_OK, HTTP_201_CREATED, HTTP_204_NO_CONTENT from api.pagination import CustomPage from core.dependencies import ( @@ -42,6 +42,7 @@ UpdateWaterChemistryObservation, UpdateGeothermalObservation, ) +from services.crud_helper import model_deleter from services.query_helper import order_sort_filter, simple_get_by_id from services.observation_helper import ( observation_model_patcher, @@ -356,4 +357,18 @@ def get_observation_by_id( return simple_get_by_id(session, Observation, observation_id) +# DELETE ======================================================================= + + +@router.delete( + "/{observation_id}", + summary="Delete an observation", + status_code=HTTP_204_NO_CONTENT, +) +def delete_observation( + session: session_dependency, user: amp_admin_dependency, observation_id: int +) -> None: + return model_deleter(session, Observation, observation_id) + + # ============= EOF ============================================= diff --git a/tests/test_observation.py b/tests/test_observation.py index 26753727f..e81cb8f80 100644 --- a/tests/test_observation.py +++ b/tests/test_observation.py @@ -14,6 +14,7 @@ # limitations under the License. # =============================================================================== from db import Observation +from db.engine import session_ctx from core.dependencies import ( amp_admin_function, admin_function, @@ -41,6 +42,23 @@ def override_authentication_dependency_fixture(): app.dependency_overrides = {} +@pytest.fixture(scope="function") +def observation_to_delete(sample, sensor): + with session_ctx() as session: + observation = Observation( + observation_datetime="2019-01-01T00:03:00Z", + sample_id=sample.id, + sensor_id=sensor.id, + observed_property="pH", + release_status="draft", + value=4.0, + unit="dimensionless", + ) + session.add(observation) + session.commit() + yield observation + + # ============= Post tests ================= def test_add_water_chemistry_observation(sample, sensor): payload = { @@ -665,4 +683,28 @@ def test_get_groundwater_observation_by_polygon_nonexistent(): assert len(items) == 0, "Expected no groundwater observations within the polygon" +# DELETE tests ================================================================= + + +def test_delete_observation_by_id(observation_to_delete): + response = client.delete(f"/observation/{observation_to_delete.id}") + assert response.status_code == 204 + + # Verify that the observation was deleted + get_response = client.get(f"/observation/{observation_to_delete.id}") + assert get_response.status_code == 404 + data = get_response.json() + assert ( + data["detail"] == f"Observation with ID {observation_to_delete.id} not found." + ) + + +def test_delete_observation_by_id_404_not_found(observation_to_delete): + bad_id = 99999 + response = client.delete(f"/observation/{bad_id}") + assert response.status_code == 404 + data = response.json() + assert data["detail"] == f"Observation with ID {bad_id} not found." + + # ============= EOF ============================================= From c4458c6682c76c6035855cceb10cbc5fd62af862 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Wed, 20 Aug 2025 14:58:55 -0600 Subject: [PATCH 27/45] refactor: use simple_get_by id for /asset/{asset_id} --- api/asset.py | 19 +++---------------- 1 file changed, 3 insertions(+), 16 deletions(-) diff --git a/api/asset.py b/api/asset.py index 66c89d318..fe1369f2b 100644 --- a/api/asset.py +++ b/api/asset.py @@ -14,7 +14,7 @@ # limitations under the License. # =============================================================================== -from fastapi import APIRouter, Depends, UploadFile, File, HTTPException +from fastapi import APIRouter, Depends, UploadFile, File from fastapi_pagination.ext.sqlalchemy import paginate from sqlalchemy import select from starlette.status import HTTP_201_CREATED @@ -32,6 +32,7 @@ from schemas.asset import AssetResponse, CreateAsset, UpdateAsset from services.audit_helper import audit_add from services.crud_helper import model_patcher +from services.query_helper import simple_get_by_id from services.gcs_helper import ( get_storage_bucket, gcs_upload, @@ -118,26 +119,12 @@ def transformer(a): async def get_asset( asset_id: int, session: session_dependency, - thing_id: int = None, bucket=Depends(get_storage_bucket), ) -> AssetResponse: """ Retrieve an asset by its ID. """ - sql = select(Asset) - if thing_id: - sql = sql.join(AssetThingAssociation).where( - AssetThingAssociation.thing_id == thing_id - ) - else: - sql = sql.where(Asset.id == asset_id) - - asset = session.scalars(sql).one_or_none() - if not asset: - raise HTTPException(status_code=404, detail="Asset not found") - - add_signed_url(asset, bucket) - return asset + return simple_get_by_id(session, Asset, asset_id) # ======= Update ========= From c8fdc3e5b1a03953f47e0bdb321dce73134e8597 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Wed, 20 Aug 2025 15:32:29 -0600 Subject: [PATCH 28/45] refactor: update database error handler --- api/contact.py | 24 +----------------------- 1 file changed, 1 insertion(+), 23 deletions(-) diff --git a/api/contact.py b/api/contact.py index 0525e4126..f4c7c1cc0 100644 --- a/api/contact.py +++ b/api/contact.py @@ -33,7 +33,6 @@ CreateAddress, CreateEmail, CreatePhone, - CreateThingAssociation, PhoneResponse, EmailResponse, AddressResponse, @@ -60,7 +59,7 @@ def database_error_handler( - payload: CreateThingAssociation, error: ProgrammingError + payload: CreateEmail | CreateContact | CreatePhone, error: ProgrammingError ) -> None: """ Handle errors raised by the database when adding or updating a sample. @@ -69,27 +68,6 @@ def database_error_handler( error_message = error.orig.args[0]["M"] if ( - error_message - == 'insert or update on table "thing_contact_association" violates foreign key constraint "thing_contact_association_thing_id_fkey"' - ): - detail = { - "loc": ["body", "thing_id"], - "msg": f"Thing with ID {payload.thing_id} not found.", - "type": "value_error", - "input": {"thing_id": payload.thing_id}, - } - - elif ( - error_message - == 'insert or update on table "thing_contact_association" violates foreign key constraint "thing_contact_association_contact_id_fkey"' - ): - detail = { - "loc": ["body", "contact_id"], - "msg": f"Contact with ID {payload.contact_id} not found.", - "type": "value_error", - "input": {"contact_id": payload.contact_id}, - } - elif ( error_message == 'insert or update on table "email" violates foreign key constraint "email_contact_id_fkey"' ): From cd4fdbeec089ba089bae3d288fb4aec9fe982a07 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Wed, 20 Aug 2025 16:27:07 -0600 Subject: [PATCH 29/45] feat: implement POST endpoints for asset --- api/asset.py | 92 +++++++++++++++++++++++++++++++-------------- schemas/asset.py | 19 +++------- tests/test_asset.py | 76 +++++++++++++++++++++---------------- 3 files changed, 113 insertions(+), 74 deletions(-) diff --git a/api/asset.py b/api/asset.py index fe1369f2b..a4d475b3b 100644 --- a/api/asset.py +++ b/api/asset.py @@ -17,7 +17,8 @@ from fastapi import APIRouter, Depends, UploadFile, File from fastapi_pagination.ext.sqlalchemy import paginate from sqlalchemy import select -from starlette.status import HTTP_201_CREATED +from sqlalchemy.exc import ProgrammingError +from starlette.status import HTTP_201_CREATED, HTTP_409_CONFLICT from api.pagination import CustomPage from core.dependencies import ( @@ -39,12 +40,42 @@ check_asset_exists, add_signed_url, ) +from services.exceptions_helper import PydanticStyleException router = APIRouter( prefix="/asset", tags=["asset"], dependencies=[Depends(viewer_function)] ) +def database_error_handler(payload: CreateAsset, error: ProgrammingError) -> None: + """ + Handle errors raised by the database when adding or updating a sample. + """ + + error_message = error.orig.args[0]["M"] + print(error_message) + + if ( + error_message + == 'null value in column "thing_id" of relation "asset_thing_association" violates not-null constraint' + ): + """ + Developer's notes + + this error occurs because the thing_id is set by the Thing record that + is retrieved, so if there is no Thing with thing_id it tries to set + thing_id to None in the AssetThingAssociation table + """ + detail = { + "loc": ["body", "thing_id"], + "msg": f"Thing with ID {payload.thing_id} not found.", + "type": "value_error", + "input": {"thing_id": payload.thing_id}, + } + + raise PydanticStyleException(status_code=HTTP_409_CONFLICT, detail=[detail]) + + # ======= Create ========= @router.post( "/upload", status_code=HTTP_201_CREATED, dependencies=[Depends(admin_function)] @@ -64,33 +95,38 @@ async def add_asset( user: admin_dependency, session: session_dependency, asset_data: CreateAsset ) -> AssetResponse: - data = asset_data.model_dump() - thing_id = data.pop("thing_id", None) - storage_path = data["storage_path"] - - # check to see if an asset entry already exists for - # this storage path and thing_id - existing_asset = check_asset_exists(session, storage_path, thing_id=thing_id) - if existing_asset: - # If an asset already exists, return it - return existing_asset - - data["storage_service"] = "gcs" - asset = Asset(**data) - audit_add(user, asset) - - if thing_id: - assoc = AssetThingAssociation() - audit_add(user, assoc) - thing = session.get(Thing, thing_id) - assoc.thing = thing - assoc.asset = asset - session.add(assoc) - - session.add(asset) - session.commit() - session.refresh(asset) - return asset + try: + data = asset_data.model_dump() + print(data) + thing_id = data.pop("thing_id", None) + print(thing_id) + storage_path = data["storage_path"] + + # check to see if an asset entry already exists for + # this storage path and thing_id + existing_asset = check_asset_exists(session, storage_path, thing_id=thing_id) + if existing_asset: + # If an asset already exists, return it + return existing_asset + + data["storage_service"] = "gcs" + asset = Asset(**data) + audit_add(user, asset) + + if thing_id: + assoc = AssetThingAssociation() + audit_add(user, assoc) + thing = session.get(Thing, thing_id) + assoc.thing = thing + assoc.asset = asset + session.add(assoc) + + session.add(asset) + session.commit() + session.refresh(asset) + return asset + except ProgrammingError as e: + database_error_handler(asset_data, e) # ======= Read ========= diff --git a/schemas/asset.py b/schemas/asset.py index d30038a94..a957b3131 100644 --- a/schemas/asset.py +++ b/schemas/asset.py @@ -13,7 +13,9 @@ # See the License for the specific language governing permissions and # limitations under the License. # =============================================================================== -from pydantic import BaseModel, AwareDatetime +from pydantic import BaseModel + +from schemas import ORMBaseModel class BaseAsset(BaseModel): @@ -23,26 +25,17 @@ class BaseAsset(BaseModel): mime_type: str size: int uri: str - thing_id: int | None = None # -------- CREATE ---------- class CreateAsset(BaseAsset): - pass + thing_id: int | None = None # -------- RESPONSE -------- -class AssetResponse(BaseAsset): - id: int - # name: str - # label: str - # storage_service: str - # storage_path: str - # mime_type: str - # size: int - created_at: AwareDatetime +class AssetResponse(ORMBaseModel, BaseAsset): + storage_service: str storage_service: str - uri: str signed_url: str | None = None diff --git a/tests/test_asset.py b/tests/test_asset.py index 8c146a17d..b3d0ae1d3 100644 --- a/tests/test_asset.py +++ b/tests/test_asset.py @@ -21,6 +21,8 @@ import pytest +# CLASSES, FIXTURES, AND FUNCTIONS ============================================= + class MockBlob: def upload_from_file(self, *args, **kwargs): @@ -63,6 +65,9 @@ def override_dependency_fixture(): app.dependency_overrides = {} +# POST & UPLOAD tests ========================================================== + + def test_upload_asset(): path = "tests/data/riochama.png" @@ -78,46 +83,51 @@ def test_upload_asset(): def test_add_asset(thing): - resp = client.post( - "/asset", - json={ - "thing_id": thing.id, - "name": "riochama.png", - "storage_service": "mock_service", - "storage_path": "mock/path/to/asset", - "uri": "https://storage.googleapis.com/mock-bucket/mock-asset", - "mime_type": "image/png", - "size": 12345, - }, - ) - + payload = { + "thing_id": thing.id, + "name": "test_asset.png", + "label": "Test Asset", + "uri": "https://storage.googleapis.com/mock-bucket/mock-asset", + "storage_service": "mock_service", + "storage_path": "mock/path/to/asset/test_asset.png", + "mime_type": "image/png", + "size": 12345, + } + resp = client.post("/asset", json=payload) assert resp.status_code == 201 data = resp.json() - assert data["name"] == "riochama.png" + assert "id" in data + assert "created_at" in data + assert data["name"] == payload["name"] + assert data["label"] == payload["label"] + assert data["uri"] == payload["uri"] + assert data["storage_service"] == "gcs" + assert data["storage_path"] == payload["storage_path"] + assert data["mime_type"] == payload["mime_type"] + assert data["size"] == payload["size"] cleanup_post_test(Asset, data["id"]) -def test_add_asset_with_label(thing): - resp = client.post( - "/asset", - json={ - "thing_id": thing.id, - "name": "test_asset.png", - "label": "Test Asset", - "uri": "https://storage.googleapis.com/mock-bucket/mock-asset", - "storage_service": "mock_service", - "storage_path": "mock/path/to/asset/test_asset.png", - "mime_type": "image/png", - "size": 12345, - }, - ) - assert resp.status_code == 201 +def test_add_asset_409_bad_thing_id(thing): + bad_thing_id = 99999 + payload = { + "thing_id": bad_thing_id, + "name": "test_asset.png", + "label": "Test Asset", + "uri": "https://storage.googleapis.com/mock-bucket/mock-asset", + "storage_service": "mock_service", + "storage_path": "mock/path/to/asset/test_asset.png", + "mime_type": "image/png", + "size": 12345, + } + resp = client.post("/asset", json=payload) + assert resp.status_code == 409 data = resp.json() - assert data["name"] == "test_asset.png" - assert data["label"] == "Test Asset" - - cleanup_post_test(Asset, data["id"]) + assert data["detail"][0]["loc"] == ["body", "thing_id"] + assert data["detail"][0]["msg"] == f"Thing with ID {bad_thing_id} not found." + assert data["detail"][0]["type"] == "value_error" + assert data["detail"][0]["input"] == {"thing_id": bad_thing_id} def test_get_asset(asset): From 511341c6e5e7f191eeba7cae402095dbf5921705 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Wed, 20 Aug 2025 17:15:58 -0600 Subject: [PATCH 30/45] feat: endpoint and tests for GET asset by ID --- api/asset.py | 4 +++- schemas/asset.py | 1 - tests/test_asset.py | 26 +++++++++++++++++++------- 3 files changed, 22 insertions(+), 9 deletions(-) diff --git a/api/asset.py b/api/asset.py index a4d475b3b..39dbafd9f 100644 --- a/api/asset.py +++ b/api/asset.py @@ -160,7 +160,9 @@ async def get_asset( """ Retrieve an asset by its ID. """ - return simple_get_by_id(session, Asset, asset_id) + asset = simple_get_by_id(session, Asset, asset_id) + add_signed_url(asset, bucket) + return asset # ======= Update ========= diff --git a/schemas/asset.py b/schemas/asset.py index a957b3131..89fa0e976 100644 --- a/schemas/asset.py +++ b/schemas/asset.py @@ -34,7 +34,6 @@ class CreateAsset(BaseAsset): # -------- RESPONSE -------- class AssetResponse(ORMBaseModel, BaseAsset): - storage_service: str storage_service: str signed_url: str | None = None diff --git a/tests/test_asset.py b/tests/test_asset.py index b3d0ae1d3..d3070467a 100644 --- a/tests/test_asset.py +++ b/tests/test_asset.py @@ -130,19 +130,31 @@ def test_add_asset_409_bad_thing_id(thing): assert data["detail"][0]["input"] == {"thing_id": bad_thing_id} -def test_get_asset(asset): +# GET tests ==================================================================== + + +def test_get_asset_by_id(asset): response = client.get(f"/asset/{asset.id}") assert response.status_code == 200 data = response.json() assert data["id"] == asset.id + assert data["created_at"] == asset.created_at.isoformat().replace("+00:00", "Z") assert data["name"] == asset.name - assert data["uri"] == MockBlob().generate_signed_url() - - -def test_get_asset_not_found(): - response = client.get("/asset/9999") + assert data["label"] == asset.label + assert data["storage_path"] == asset.storage_path + assert data["mime_type"] == asset.mime_type + assert data["size"] == asset.size + assert data["uri"] == asset.uri + assert data["storage_service"] == asset.storage_service + assert data["signed_url"] == MockBlob().generate_signed_url() + + +def test_get_asset_by_id_404_not_found(asset): + bad_id = 99999 + response = client.get(f"/asset/{bad_id}") assert response.status_code == 404 - assert response.json() == {"detail": "Asset not found"} + data = response.json() + assert data["detail"] == f"Asset with ID {bad_id} not found." # ============= EOF ============================================= From 6c779d9a49c51e44567193fac624e34ff9a3b9db Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Wed, 20 Aug 2025 17:45:31 -0600 Subject: [PATCH 31/45] refactor: apply transformer to every item in list if asset is list of items --- api/asset.py | 3 +-- services/gcs_helper.py | 18 +++++++++++++----- tests/test_asset.py | 22 ++++++++++++++++++++++ 3 files changed, 36 insertions(+), 7 deletions(-) diff --git a/api/asset.py b/api/asset.py index 39dbafd9f..de36e4967 100644 --- a/api/asset.py +++ b/api/asset.py @@ -144,8 +144,7 @@ async def list_assets( ) def transformer(a): - if thing_id is not None: - add_signed_url(a, get_storage_bucket()) + add_signed_url(a, get_storage_bucket()) return a return paginate(query=sql, conn=session, transformer=transformer) diff --git a/services/gcs_helper.py b/services/gcs_helper.py index 4a3002c56..bfd220f11 100644 --- a/services/gcs_helper.py +++ b/services/gcs_helper.py @@ -77,11 +77,19 @@ def gcs_upload(file: UploadFile, bucket: storage.Bucket = None): def add_signed_url(asset, bucket): - asset.signed_url = bucket.blob(asset.storage_path).generate_signed_url( - version="v4", - expiration=datetime.timedelta(minutes=15), - method="GET", - ) + if isinstance(asset, list): + for a in asset: + a.signed_url = bucket.blob(a.storage_path).generate_signed_url( + version="v4", + expiration=datetime.timedelta(minutes=15), + method="GET", + ) + else: + asset.signed_url = bucket.blob(asset.storage_path).generate_signed_url( + version="v4", + expiration=datetime.timedelta(minutes=15), + method="GET", + ) def check_asset_exists(session, blob_name, thing_id=None): diff --git a/tests/test_asset.py b/tests/test_asset.py index d3070467a..e7daf25a4 100644 --- a/tests/test_asset.py +++ b/tests/test_asset.py @@ -20,6 +20,7 @@ from tests import client, cleanup_post_test, override_authentication import pytest +from unittest.mock import patch # CLASSES, FIXTURES, AND FUNCTIONS ============================================= @@ -133,6 +134,27 @@ def test_add_asset_409_bad_thing_id(thing): # GET tests ==================================================================== +def test_get_assets(asset): + with patch("api.asset.get_storage_bucket") as mock_get_storage_bucket: + mock_get_storage_bucket.return_value = mock_storage_bucket() + response = client.get("/asset") + assert response.status_code == 200 + data = response.json() + assert data["total"] == 1 + assert data["items"][0]["id"] == asset.id + assert data["items"][0]["created_at"] == asset.created_at.isoformat().replace( + "+00:00", "Z" + ) + assert data["items"][0]["name"] == asset.name + assert data["items"][0]["label"] == asset.label + assert data["items"][0]["storage_path"] == asset.storage_path + assert data["items"][0]["mime_type"] == asset.mime_type + assert data["items"][0]["size"] == asset.size + assert data["items"][0]["uri"] == asset.uri + assert data["items"][0]["storage_service"] == asset.storage_service + assert data["items"][0]["signed_url"] == MockBlob().generate_signed_url() + + def test_get_asset_by_id(asset): response = client.get(f"/asset/{asset.id}") assert response.status_code == 200 From eac0c317cd11d3e37808c064d517b69d44a2b766 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Wed, 20 Aug 2025 17:46:25 -0600 Subject: [PATCH 32/45] feat: type hint add_signed_url --- services/gcs_helper.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/services/gcs_helper.py b/services/gcs_helper.py index bfd220f11..0e347e036 100644 --- a/services/gcs_helper.py +++ b/services/gcs_helper.py @@ -27,6 +27,7 @@ GCS_BUCKET_BASE_URL = f"https://storage.cloud.google.com/{GCS_BUCKET_NAME}/uploads" from google.cloud import storage +from typing import List def get_storage_bucket() -> storage.Bucket: @@ -76,7 +77,7 @@ def gcs_upload(file: UploadFile, bucket: storage.Bucket = None): return url, blob_name -def add_signed_url(asset, bucket): +def add_signed_url(asset: Asset | List[Asset], bucket): if isinstance(asset, list): for a in asset: a.signed_url = bucket.blob(a.storage_path).generate_signed_url( From e357b0ecb11f326430fed7fe1c6b81b21b1fdd9c Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Thu, 21 Aug 2025 09:15:55 -0600 Subject: [PATCH 33/45] refactor: simplify method to specify observation class --- api/observation.py | 47 +++++++++++++++++++++------------------------- 1 file changed, 21 insertions(+), 26 deletions(-) diff --git a/api/observation.py b/api/observation.py index 4ceae7380..18d75b99f 100644 --- a/api/observation.py +++ b/api/observation.py @@ -14,12 +14,12 @@ # limitations under the License. # =============================================================================== from datetime import datetime -import functools from fastapi import APIRouter, Query from fastapi_pagination.ext.sqlalchemy import paginate from sqlalchemy import select from starlette.status import HTTP_200_OK, HTTP_201_CREATED, HTTP_204_NO_CONTENT +from typing import List from api.pagination import CustomPage from core.dependencies import ( @@ -142,25 +142,6 @@ def update_geothermal_observation( # ============= Get ============================================== -def specify_observation_class(observation_class: str | None): - def decorator(func): - @functools.wraps(func) - def wrapper(*args, **kwargs): - sql, session = func(*args, **kwargs) - if observation_class == "groundwater level": - sql = sql.where(Observation.observed_property == "groundwater level") - elif observation_class == "water chemistry": - sql = sql.where(Observation.observed_property != "groundwater level") - sql = sql.where(Observation.observed_property != "temperature") - elif observation_class == "geothermal": - sql = sql.where(Observation.observed_property == "temperature") - return paginate(query=sql, conn=session) - - return wrapper - - return decorator - - def get_observations( session: session_dependency, thing_id: int | None = None, @@ -171,7 +152,13 @@ def get_observations( sort: str | None = None, order: str | None = None, filter_: str = Query(alias="filter", default=None), -) -> tuple: + observation_class: str | None = None, +) -> ( + List[ObservationResponse] + | List[WaterChemistryObservationResponse] + | List[GeothermalObservationResponse] + | List[GroundwaterLevelObservationResponse] +): """ Retrieve all observations """ @@ -189,16 +176,23 @@ def get_observations( if end_time: sql = sql.where(Observation.observation_datetime <= end_time) + if observation_class == "groundwater level": + sql = sql.where(Observation.observed_property == "groundwater level") + elif observation_class == "water chemistry": + sql = sql.where(Observation.observed_property != "groundwater level") + sql = sql.where(Observation.observed_property != "temperature") + elif observation_class == "geothermal": + sql = sql.where(Observation.observed_property == "temperature") + sql = order_sort_filter(sql, Observation, sort, order, filter_) if not order: sql = sql.order_by(Observation.observation_datetime.desc()) - return sql, session + return paginate(query=sql, conn=session) @router.get("/groundwater-level", summary="Get groundwater level observations") -@specify_observation_class(observation_class="groundwater level") def get_groundwater_level_observations( session: session_dependency, user: amp_viewer_dependency, @@ -224,6 +218,7 @@ def get_groundwater_level_observations( sort=sort, order=order, filter_=filter_, + observation_class="groundwater level", ) @@ -242,7 +237,6 @@ def get_groundwater_level_observation_by_id( @router.get("/water-chemistry", summary="Get water chemistry observations") -@specify_observation_class(observation_class="water chemistry") def get_water_chemistry_observations( session: session_dependency, user: amp_viewer_dependency, @@ -268,6 +262,7 @@ def get_water_chemistry_observations( sort=sort, order=order, filter_=filter_, + observation_class="water chemistry", ) @@ -285,7 +280,6 @@ def get_water_chemistry_observation_by_id( @router.get("/geothermal", summary="Get geothermal observations") -@specify_observation_class(observation_class="geothermal") def get_geothermal_observations( session: session_dependency, user: viewer_dependency, @@ -311,6 +305,7 @@ def get_geothermal_observations( sort=sort, order=order, filter_=filter_, + observation_class="geothermal", ) @@ -324,7 +319,6 @@ def get_geothermal_observation_by_id( @router.get("", summary="Get all observations") -@specify_observation_class(observation_class=None) def get_all_observations( session: session_dependency, user: amp_viewer_dependency, @@ -347,6 +341,7 @@ def get_all_observations( sort=sort, order=order, filter_=filter_, + observation_class=None, ) From 4f46bb7b4597f4f59bf383eb0800e93d10efcc68 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Thu, 21 Aug 2025 15:09:15 -0600 Subject: [PATCH 34/45] refactor: use : in observed property --- api/observation.py | 7 +++-- core/lexicon.json | 10 +++---- schemas/observation.py | 26 +++++++++++++++--- services/observation_helper.py | 28 +++---------------- tests/conftest.py | 6 ++--- tests/test_observation.py | 49 +++++++++++++++++++++------------- 6 files changed, 69 insertions(+), 57 deletions(-) diff --git a/api/observation.py b/api/observation.py index 18d75b99f..aeb8f110e 100644 --- a/api/observation.py +++ b/api/observation.py @@ -177,12 +177,11 @@ def get_observations( sql = sql.where(Observation.observation_datetime <= end_time) if observation_class == "groundwater level": - sql = sql.where(Observation.observed_property == "groundwater level") + sql = sql.where(Observation.observed_property.like("groundwater level:%")) elif observation_class == "water chemistry": - sql = sql.where(Observation.observed_property != "groundwater level") - sql = sql.where(Observation.observed_property != "temperature") + sql = sql.where(Observation.observed_property.like("water chemistry:%")) elif observation_class == "geothermal": - sql = sql.where(Observation.observed_property == "temperature") + sql = sql.where(Observation.observed_property.like("geothermal:%")) sql = order_sort_filter(sql, Observation, sort, order, filter_) diff --git a/core/lexicon.json b/core/lexicon.json index 61405f739..6113a7f72 100644 --- a/core/lexicon.json +++ b/core/lexicon.json @@ -13,12 +13,12 @@ {"category": "unit", "term": "m²/s", "definition": "square meters per second"}, {"category": "unit", "term": "deg C", "definition": "degree Celsius"}, - {"category": "observed_property", "term": "groundwater level", "definition": "groundwater level measurement" }, + {"category": "observed_property", "term": "groundwater level:groundwater level", "definition": "groundwater level measurement" }, - {"category": "observed_property", "term": "temperature", "definition": "Temperature measurement"}, - - {"category": "observed_property", "term": "pH", "definition": "pH"}, - {"category": "observed_property", "term": "Alkalinity as CaCO3", "definition": "Alkalinity as CaCO3"}, + {"category": "observed_property", "term": "geothermal:temperature", "definition": "Temperature measurement"}, + + {"category": "observed_property", "term": "water chemistry:pH", "definition": "pH"}, + {"category": "observed_property", "term": "water chemistry:Alkalinity as CaCO3", "definition": "Alkalinity as CaCO3"}, diff --git a/schemas/observation.py b/schemas/observation.py index 664fe0baa..4fb0474f3 100644 --- a/schemas/observation.py +++ b/schemas/observation.py @@ -36,7 +36,8 @@ class ValidateObservation(BaseModel): - + _observation_class: str + observed_property: str observation_datetime: AwareDatetime @field_validator("observation_datetime", check_fields=False) @@ -54,6 +55,16 @@ def convert_observation_datetime_to_utc( return observation_datetime.astimezone(timezone.utc) return observation_datetime + @model_validator(mode="after") + def prepend_observed_property(self: Self) -> Self: + observed_property = self.observed_property + observation_class = self._observation_class + if observed_property is not None: + observation_class = self._observation_class + if not observed_property.startswith(f"{observation_class}:"): + self.observed_property = f"{observation_class}:{observed_property}" + return self + # -------- CREATE ---------- class CreateBaseObservation(ValidateObservation): @@ -67,15 +78,17 @@ class CreateBaseObservation(ValidateObservation): class CreateGroundwaterLevelObservation(CreateBaseObservation): + _observation_class: str = "groundwater level" measuring_point_height: float level_status: str class CreateWaterChemistryObservation(CreateBaseObservation): - pass + _observation_class: str = "water chemistry" class CreateGeothermalObservation(CreateBaseObservation): + _observation_class: str = "geothermal" observation_depth: float @@ -93,15 +106,17 @@ class UpdateBaseObservation(ValidateObservation): class UpdateGroundwaterLevelObservation(UpdateBaseObservation): + _observation_class: str = "groundwater level" measuring_point_height: float | None = None level_status: str | None = None class UpdateWaterChemistryObservation(UpdateBaseObservation): - pass + _observation_class: str = "water chemistry" class UpdateGeothermalObservation(UpdateBaseObservation): + _observation_class: str = "geothermal" observation_depth: float | None = None @@ -115,6 +130,11 @@ class BaseObservationResponse(ORMBaseModel): value: float | None unit: str + @field_validator("observed_property") + def remove_observed_property_prefix(cls, v: str) -> str: + colon_index = v.find(":") + return v[colon_index + 1 :] + class GroundwaterLevelObservationResponse(BaseObservationResponse): depth_to_water_bgs: float | None diff --git a/services/observation_helper.py b/services/observation_helper.py index b7b47a87a..b2a543ed6 100644 --- a/services/observation_helper.py +++ b/services/observation_helper.py @@ -6,35 +6,15 @@ from services.exceptions_helper import PydanticStyleException from services.query_helper import simple_get_by_id -observation_class_to_observed_properties = { - "groundwater level": ["groundwater level"], - "geothermal": ["temperature"], - "water chemistry": ["pH", "Alkalinity as CaCO3"], -} - -observation_property_to_class = {} -for key, value in observation_class_to_observed_properties.items(): - for prop in value: - observation_property_to_class[prop] = key - def verify_observed_property_corresponds_with_observation_class( observation: Observation, observation_class: str -) -> None: - """ - Verify that the observed property of the retrieved Observation corresponds - with the observation class as defined by the path - (e.g. /observation/water-chemistry). Raise an error if they do not - correspond. - """ +): observed_property = observation.observed_property + colon_index = observed_property.find(":") + actual_observation_class = observed_property[:colon_index] - if ( - observed_property - not in observation_class_to_observed_properties[observation_class] - ): - actual_observation_class = observation_property_to_class[observed_property] - + if actual_observation_class != observation_class: raise PydanticStyleException( status_code=HTTP_404_NOT_FOUND, detail=[ diff --git a/tests/conftest.py b/tests/conftest.py index 9fba83455..68acbabfb 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -175,7 +175,7 @@ def groundwater_level_observation(sensor, sample): observation_datetime="2025-01-01T00:04:00Z", sample_id=sample.id, sensor_id=sensor.id, - observed_property="groundwater level", + observed_property="groundwater level:groundwater level", release_status="draft", value=10.0, unit="ft", @@ -196,7 +196,7 @@ def water_chemistry_observation(sensor, sample): observation_datetime="2025-01-01T00:03:00Z", sample_id=sample.id, sensor_id=sensor.id, - observed_property="pH", + observed_property="water chemistry:pH", release_status="draft", value=4.0, unit="dimensionless", @@ -215,7 +215,7 @@ def geothermal_observation(sensor, sample): observation_datetime="2025-01-01T00:02:00Z", sample_id=sample.id, sensor_id=sensor.id, - observed_property="temperature", + observed_property="geothermal:temperature", release_status="draft", value=20.0, unit="deg C", diff --git a/tests/test_observation.py b/tests/test_observation.py index e81cb8f80..f1c94483c 100644 --- a/tests/test_observation.py +++ b/tests/test_observation.py @@ -49,7 +49,7 @@ def observation_to_delete(sample, sensor): observation_datetime="2019-01-01T00:03:00Z", sample_id=sample.id, sensor_id=sensor.id, - observed_property="pH", + observed_property="water chemistry:pH", release_status="draft", value=4.0, unit="dimensionless", @@ -182,7 +182,7 @@ def test_patch_groundwater_level_observation_404_wrong_observation_class( assert response.status_code == 404 data = response.json() - if obs.observed_property == "temperature": + if obs.observed_property == "geothermal:temperature": observation_class = "geothermal" else: observation_class = "water chemistry" @@ -225,7 +225,7 @@ def test_patch_water_chemistry_observation_404_wrong_observation_class( assert response.status_code == 404 data = response.json() - if obs.observed_property == "temperature": + if obs.observed_property == "geothermal:temperature": observation_class = "geothermal" else: observation_class = "groundwater level" @@ -266,7 +266,7 @@ def test_patch_geothermal_observation_404_wrong_observation_class( assert response.status_code == 404 data = response.json() - if obs.observed_property == "groundwater level": + if obs.observed_property == "groundwater level:groundwater level": observation_class = "groundwater level" else: observation_class = "water chemistry" @@ -305,10 +305,10 @@ def test_get_observation_by_id( data = response.json() assert data["id"] == obs.id - if obs.observed_property == "groundwater level": + if obs.observed_property == "groundwater level:groundwater level": assert data["depth_to_water_bgs"] == obs.value - obs.measuring_point_height assert data["observation_depth"] is None - elif obs.observed_property == "temperature": + elif obs.observed_property == "geothermal:temperature": assert data["depth_to_water_bgs"] is None assert data["observation_depth"] == obs.observation_depth else: @@ -340,9 +340,10 @@ def test_get_groundwater_level_observations( data["items"][0]["observation_datetime"] == groundwater_level_observation.observation_datetime ) + colon_index = groundwater_level_observation.observed_property.find(":") assert ( data["items"][0]["observed_property"] - == groundwater_level_observation.observed_property + == groundwater_level_observation.observed_property[colon_index + 1 :] ) assert ( data["items"][0]["release_status"] @@ -380,7 +381,11 @@ def test_get_groundwater_level_observation_by_id(groundwater_level_observation): data["observation_datetime"] == groundwater_level_observation.observation_datetime ) - assert data["observed_property"] == groundwater_level_observation.observed_property + colon_index = groundwater_level_observation.observed_property.find(":") + assert ( + data["observed_property"] + == groundwater_level_observation.observed_property[colon_index + 1 :] + ) assert data["release_status"] == groundwater_level_observation.release_status assert data["level_status"] == groundwater_level_observation.level_status assert data["value"] == groundwater_level_observation.value @@ -416,7 +421,7 @@ def test_get_groundwater_level_observation_by_id_404_wrong_observation_class( assert response.status_code == 404 data = response.json() - if obs.observed_property == "temperature": + if obs.observed_property == "geothermal:temperature": actual_observation_class = "geothermal" else: actual_observation_class = "water chemistry" @@ -512,9 +517,10 @@ def test_get_water_chemistry_observations(water_chemistry_observation): data["items"][0]["observation_datetime"] == water_chemistry_observation.observation_datetime ) + colon_index = water_chemistry_observation.observed_property.find(":") assert ( data["items"][0]["observed_property"] - == water_chemistry_observation.observed_property + == water_chemistry_observation.observed_property[colon_index + 1 :] ) assert data["items"][0]["value"] == water_chemistry_observation.value assert data["items"][0]["unit"] == water_chemistry_observation.unit @@ -535,7 +541,11 @@ def test_get_water_chemistry_observation_by_id(water_chemistry_observation): assert ( data["observation_datetime"] == water_chemistry_observation.observation_datetime ) - assert data["observed_property"] == water_chemistry_observation.observed_property + colon_index = water_chemistry_observation.observed_property.find(":") + assert ( + data["observed_property"] + == water_chemistry_observation.observed_property[colon_index + 1 :] + ) assert data["value"] == water_chemistry_observation.value assert data["unit"] == water_chemistry_observation.unit @@ -558,12 +568,10 @@ def test_get_water_chemistry_observation_by_id_404_wrong_observation_class( assert response.status_code == 404 data = response.json() - if obs.observed_property == "groundwater level": + if obs.observed_property == "groundwater level:groundwater level": actual_observation_class = "groundwater level" - elif obs.observed_property == "temperature": - actual_observation_class = "geothermal" else: - url = f"/observation/water-chemistry/{obs.id}" + actual_observation_class = "geothermal" assert ( data["detail"][0]["msg"] @@ -586,9 +594,10 @@ def test_get_geothermal_observations(geothermal_observation): data["items"][0]["observation_datetime"] == geothermal_observation.observation_datetime ) + colon_index = geothermal_observation.observed_property.find(":") assert ( data["items"][0]["observed_property"] - == geothermal_observation.observed_property + == geothermal_observation.observed_property[colon_index + 1 :] ) assert data["items"][0]["value"] == geothermal_observation.value assert data["items"][0]["unit"] == geothermal_observation.unit @@ -609,7 +618,11 @@ def test_get_geothermal_observation_by_id(geothermal_observation): assert data["sample_id"] == geothermal_observation.sample_id assert data["sensor_id"] == geothermal_observation.sensor_id assert data["observation_datetime"] == geothermal_observation.observation_datetime - assert data["observed_property"] == geothermal_observation.observed_property + colon_index = geothermal_observation.observed_property.find(":") + assert ( + data["observed_property"] + == geothermal_observation.observed_property[colon_index + 1 :] + ) assert data["value"] == geothermal_observation.value assert data["unit"] == geothermal_observation.unit assert data["observation_depth"] == geothermal_observation.observation_depth @@ -631,7 +644,7 @@ def test_get_geothermal_observation_by_id_404_wrong_observation_class( assert response.status_code == 404 data = response.json() - if obs.observed_property == "groundwater level": + if obs.observed_property == "groundwater level:groundwater level": actual_observation_class = "groundwater level" else: actual_observation_class = "water chemistry" From e63b04e1be0d5ec1de5e45144704fb39d9006ae4 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Thu, 21 Aug 2025 16:06:07 -0600 Subject: [PATCH 35/45] fix: add signed urls to all retrieved records --- api/asset.py | 9 ++++----- tests/test_asset.py | 35 ++++++++++++++++------------------- 2 files changed, 20 insertions(+), 24 deletions(-) diff --git a/api/asset.py b/api/asset.py index 70d63e62e..d245288c0 100644 --- a/api/asset.py +++ b/api/asset.py @@ -132,7 +132,9 @@ async def add_asset( # ======= Read ========= @router.get("") async def list_assets( - session: session_dependency, thing_id: int = None + session: session_dependency, + thing_id: int = None, + bucket=Depends(get_storage_bucket), ) -> CustomPage[AssetResponse]: """ List all assets or assets associated with a specific thing. @@ -144,10 +146,7 @@ async def list_assets( ) def transformer(records: list[Asset]): - if thing_id is not None: - bucket = get_storage_bucket() - records = [add_signed_url(ai, bucket) for ai in records] - + records = [add_signed_url(ai, bucket) for ai in records] return records return paginate(query=sql, conn=session, transformer=transformer) diff --git a/tests/test_asset.py b/tests/test_asset.py index e7daf25a4..79ce207de 100644 --- a/tests/test_asset.py +++ b/tests/test_asset.py @@ -20,7 +20,6 @@ from tests import client, cleanup_post_test, override_authentication import pytest -from unittest.mock import patch # CLASSES, FIXTURES, AND FUNCTIONS ============================================= @@ -135,24 +134,22 @@ def test_add_asset_409_bad_thing_id(thing): def test_get_assets(asset): - with patch("api.asset.get_storage_bucket") as mock_get_storage_bucket: - mock_get_storage_bucket.return_value = mock_storage_bucket() - response = client.get("/asset") - assert response.status_code == 200 - data = response.json() - assert data["total"] == 1 - assert data["items"][0]["id"] == asset.id - assert data["items"][0]["created_at"] == asset.created_at.isoformat().replace( - "+00:00", "Z" - ) - assert data["items"][0]["name"] == asset.name - assert data["items"][0]["label"] == asset.label - assert data["items"][0]["storage_path"] == asset.storage_path - assert data["items"][0]["mime_type"] == asset.mime_type - assert data["items"][0]["size"] == asset.size - assert data["items"][0]["uri"] == asset.uri - assert data["items"][0]["storage_service"] == asset.storage_service - assert data["items"][0]["signed_url"] == MockBlob().generate_signed_url() + response = client.get("/asset") + assert response.status_code == 200 + data = response.json() + assert data["total"] == 1 + assert data["items"][0]["id"] == asset.id + assert data["items"][0]["created_at"] == asset.created_at.isoformat().replace( + "+00:00", "Z" + ) + assert data["items"][0]["name"] == asset.name + assert data["items"][0]["label"] == asset.label + assert data["items"][0]["storage_path"] == asset.storage_path + assert data["items"][0]["mime_type"] == asset.mime_type + assert data["items"][0]["size"] == asset.size + assert data["items"][0]["uri"] == asset.uri + assert data["items"][0]["storage_service"] == asset.storage_service + assert data["items"][0]["signed_url"] == MockBlob().generate_signed_url() def test_get_asset_by_id(asset): From 600d3bf657bda873afdd7835768726f9e7d1d9ec Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Thu, 21 Aug 2025 16:50:24 -0600 Subject: [PATCH 36/45] refactor: get observation class from path --- api/observation.py | 108 ++++++++++----------------------- services/observation_helper.py | 94 ++++++++++++++++++++++++---- 2 files changed, 116 insertions(+), 86 deletions(-) diff --git a/api/observation.py b/api/observation.py index aeb8f110e..888783836 100644 --- a/api/observation.py +++ b/api/observation.py @@ -14,12 +14,8 @@ # limitations under the License. # =============================================================================== from datetime import datetime - -from fastapi import APIRouter, Query -from fastapi_pagination.ext.sqlalchemy import paginate -from sqlalchemy import select +from fastapi import APIRouter, Query, Request from starlette.status import HTTP_200_OK, HTTP_201_CREATED, HTTP_204_NO_CONTENT -from typing import List from api.pagination import CustomPage from core.dependencies import ( @@ -29,7 +25,7 @@ amp_viewer_dependency, viewer_dependency, ) -from db import Sample, Observation, adder +from db import Observation, adder from schemas.observation import ( CreateGroundwaterLevelObservation, GroundwaterLevelObservationResponse, @@ -43,8 +39,9 @@ UpdateGeothermalObservation, ) from services.crud_helper import model_deleter -from services.query_helper import order_sort_filter, simple_get_by_id +from services.query_helper import simple_get_by_id from services.observation_helper import ( + get_observations, observation_model_patcher, get_observation_of_an_observation_class_by_id, ) @@ -100,13 +97,12 @@ def update_groundwater_level_observation( obs_data: UpdateGroundwaterLevelObservation, session: session_dependency, user: amp_admin_dependency, + request: Request, ) -> GroundwaterLevelObservationResponse: """ Update an existing groundwater level observation in the database. """ - return observation_model_patcher( - session, observation_id, obs_data, "groundwater level", user - ) + return observation_model_patcher(session, request, observation_id, obs_data, user) @router.patch("/water-chemistry/{observation_id}", status_code=HTTP_200_OK) @@ -115,13 +111,12 @@ def update_water_chemistry_observation( obs_data: UpdateWaterChemistryObservation, session: session_dependency, user: amp_admin_dependency, + request: Request, ) -> WaterChemistryObservationResponse: """ Update an existing water chemistry observation in the database. """ - return observation_model_patcher( - session, observation_id, obs_data, "water chemistry", user - ) + return observation_model_patcher(session, request, observation_id, obs_data, user) @router.patch("/geothermal/{observation_id}", status_code=HTTP_200_OK) @@ -130,69 +125,20 @@ def update_geothermal_observation( obs_data: UpdateGeothermalObservation, session: session_dependency, user: admin_dependency, + request: Request, ) -> GeothermalObservationResponse: """ Update an existing geothermal observation in the database. """ - return observation_model_patcher( - session, observation_id, obs_data, "geothermal", user - ) + return observation_model_patcher(session, request, observation_id, obs_data, user) # ============= Get ============================================== -def get_observations( - session: session_dependency, - thing_id: int | None = None, - sensor_id: int | None = None, - sample_id: int | None = None, - start_time: datetime | None = None, - end_time: datetime | None = None, - sort: str | None = None, - order: str | None = None, - filter_: str = Query(alias="filter", default=None), - observation_class: str | None = None, -) -> ( - List[ObservationResponse] - | List[WaterChemistryObservationResponse] - | List[GeothermalObservationResponse] - | List[GroundwaterLevelObservationResponse] -): - """ - Retrieve all observations - """ - sql = select(Observation) - if thing_id is not None: - sql = sql.join(Sample) - sql = sql.where(Sample.thing_id == thing_id) - if sample_id is not None: - sql = sql.where(Observation.sample_id == sample_id) - if sensor_id is not None: - sql = sql.where(Observation.sensor_id == sensor_id) - - if start_time: - sql = sql.where(Observation.observation_datetime >= start_time) - if end_time: - sql = sql.where(Observation.observation_datetime <= end_time) - - if observation_class == "groundwater level": - sql = sql.where(Observation.observed_property.like("groundwater level:%")) - elif observation_class == "water chemistry": - sql = sql.where(Observation.observed_property.like("water chemistry:%")) - elif observation_class == "geothermal": - sql = sql.where(Observation.observed_property.like("geothermal:%")) - - sql = order_sort_filter(sql, Observation, sort, order, filter_) - - if not order: - sql = sql.order_by(Observation.observation_datetime.desc()) - - return paginate(query=sql, conn=session) - - @router.get("/groundwater-level", summary="Get groundwater level observations") def get_groundwater_level_observations( + request: Request, session: session_dependency, user: amp_viewer_dependency, thing_id: int | None = None, @@ -208,6 +154,7 @@ def get_groundwater_level_observations( Retrieve all groundwater level observations from the database. """ return get_observations( + request=request, session=session, thing_id=thing_id, sensor_id=sensor_id, @@ -217,7 +164,6 @@ def get_groundwater_level_observations( sort=sort, order=order, filter_=filter_, - observation_class="groundwater level", ) @@ -226,17 +172,21 @@ def get_groundwater_level_observations( summary="Get groundwater level observation by ID", ) def get_groundwater_level_observation_by_id( - session: session_dependency, user: amp_viewer_dependency, observation_id: int + session: session_dependency, + request: Request, + user: amp_viewer_dependency, + observation_id: int, ) -> GroundwaterLevelObservationResponse: return get_observation_of_an_observation_class_by_id( session=session, + request=request, observation_id=observation_id, - observation_class="groundwater level", ) @router.get("/water-chemistry", summary="Get water chemistry observations") def get_water_chemistry_observations( + request: Request, session: session_dependency, user: amp_viewer_dependency, thing_id: int | None = None, @@ -252,6 +202,7 @@ def get_water_chemistry_observations( Retrieve all water chemistry observations from the database. """ return get_observations( + request=request, session=session, thing_id=thing_id, sensor_id=sensor_id, @@ -261,7 +212,6 @@ def get_water_chemistry_observations( sort=sort, order=order, filter_=filter_, - observation_class="water chemistry", ) @@ -269,17 +219,21 @@ def get_water_chemistry_observations( "/water-chemistry/{observation_id}", summary="Get water chemistry observation by ID" ) def get_water_chemistry_observation_by_id( - session: session_dependency, user: amp_viewer_dependency, observation_id: int + session: session_dependency, + request: Request, + user: amp_viewer_dependency, + observation_id: int, ) -> WaterChemistryObservationResponse: return get_observation_of_an_observation_class_by_id( session=session, + request=request, observation_id=observation_id, - observation_class="water chemistry", ) @router.get("/geothermal", summary="Get geothermal observations") def get_geothermal_observations( + request: Request, session: session_dependency, user: viewer_dependency, thing_id: int | None = None, @@ -295,6 +249,7 @@ def get_geothermal_observations( Retrieve all geothermal observations from the database. """ return get_observations( + request=request, session=session, thing_id=thing_id, sensor_id=sensor_id, @@ -304,21 +259,24 @@ def get_geothermal_observations( sort=sort, order=order, filter_=filter_, - observation_class="geothermal", ) @router.get("/geothermal/{observation_id}", summary="Get geothermal observation by ID") def get_geothermal_observation_by_id( - session: session_dependency, user: amp_viewer_dependency, observation_id: int + session: session_dependency, + request: Request, + user: amp_viewer_dependency, + observation_id: int, ) -> GeothermalObservationResponse: return get_observation_of_an_observation_class_by_id( - session=session, observation_id=observation_id, observation_class="geothermal" + session=session, request=request, observation_id=observation_id ) @router.get("", summary="Get all observations") def get_all_observations( + request: Request, session: session_dependency, user: amp_viewer_dependency, thing_id: int | None = None, @@ -331,6 +289,7 @@ def get_all_observations( filter_: str = Query(alias="filter", default=None), ) -> CustomPage[ObservationResponse]: return get_observations( + request=request, session=session, thing_id=thing_id, sensor_id=sensor_id, @@ -340,7 +299,6 @@ def get_all_observations( sort=sort, order=order, filter_=filter_, - observation_class=None, ) diff --git a/services/observation_helper.py b/services/observation_helper.py index b2a543ed6..8f4561c32 100644 --- a/services/observation_helper.py +++ b/services/observation_helper.py @@ -1,15 +1,91 @@ from pydantic import BaseModel from sqlalchemy.orm import Session from starlette.status import HTTP_404_NOT_FOUND +from fastapi_pagination.ext.sqlalchemy import paginate +from sqlalchemy import select +from typing import List +from fastapi import Request, Query +from datetime import datetime -from db import Observation +from core.dependencies import session_dependency +from db import Observation, Sample +from schemas.observation import ( + ObservationResponse, + WaterChemistryObservationResponse, + GeothermalObservationResponse, + GroundwaterLevelObservationResponse, +) from services.exceptions_helper import PydanticStyleException -from services.query_helper import simple_get_by_id +from services.query_helper import simple_get_by_id, order_sort_filter + + +def get_observation_class_from_request(request: Request) -> str: + path = request.url.path + path_components = path.split("/") + if len(path_components) == 2: + # no observation class specified in path + observation_class_in_path = path_components[1] + if len(path_components) >= 3: + # observation class specified in path + observation_class_in_path = path_components[2] + + observation_class = observation_class_in_path.replace("-", " ") + return observation_class + + +def get_observations( + request: Request, + session: session_dependency, + thing_id: int | None = None, + sensor_id: int | None = None, + sample_id: int | None = None, + start_time: datetime | None = None, + end_time: datetime | None = None, + sort: str | None = None, + order: str | None = None, + filter_: str = Query(alias="filter", default=None), +) -> ( + List[ObservationResponse] + | List[WaterChemistryObservationResponse] + | List[GeothermalObservationResponse] + | List[GroundwaterLevelObservationResponse] +): + """ + Retrieve all observations + """ + observation_class = get_observation_class_from_request(request) + + sql = select(Observation) + if thing_id is not None: + sql = sql.join(Sample) + sql = sql.where(Sample.thing_id == thing_id) + if sample_id is not None: + sql = sql.where(Observation.sample_id == sample_id) + if sensor_id is not None: + sql = sql.where(Observation.sensor_id == sensor_id) + + if start_time: + sql = sql.where(Observation.observation_datetime >= start_time) + if end_time: + sql = sql.where(Observation.observation_datetime <= end_time) + + # root of path is /observation + if observation_class != "observation": + sql = sql.where(Observation.observed_property.like(f"{observation_class}:%")) + + sql = order_sort_filter(sql, Observation, sort, order, filter_) + + if not order: + sql = sql.order_by(Observation.observation_datetime.desc()) + + return paginate(query=sql, conn=session) def verify_observed_property_corresponds_with_observation_class( - observation: Observation, observation_class: str + observation: Observation, request: Request ): + observation_class = get_observation_class_from_request(request) + observed_property = observation.observed_property colon_index = observed_property.find(":") actual_observation_class = observed_property[:colon_index] @@ -29,25 +105,23 @@ def verify_observed_property_corresponds_with_observation_class( def get_observation_of_an_observation_class_by_id( - session: Session, observation_id: int, observation_class: str + session: Session, request: Request, observation_id: int ) -> Observation: """ Retrieve an observation by its ID. """ observation = simple_get_by_id(session, Observation, observation_id) - verify_observed_property_corresponds_with_observation_class( - observation, observation_class - ) + verify_observed_property_corresponds_with_observation_class(observation, request) return observation def observation_model_patcher( session: Session, + request: Request, observation_id: int, payload: BaseModel, - observation_class: str, user: dict, ) -> Observation: """ @@ -56,9 +130,7 @@ def observation_model_patcher( # simple_get_by_id raises HTTP_404_NOT_FOUND if the item is not found observation = simple_get_by_id(session, Observation, observation_id) - verify_observed_property_corresponds_with_observation_class( - observation, observation_class - ) + verify_observed_property_corresponds_with_observation_class(observation, request) for key, value in payload.model_dump(exclude_unset=True).items(): setattr(observation, key, value) From e397d1bb4a10a491292c1b171c61b33f455afc0d Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Fri, 22 Aug 2025 08:35:58 -0600 Subject: [PATCH 37/45] feat: implement patch asset schema and test --- schemas/asset.py | 7 ++++--- tests/test_asset.py | 26 +++++++++++++++++++++++++- 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/schemas/asset.py b/schemas/asset.py index 89fa0e976..e7d1d4bd1 100644 --- a/schemas/asset.py +++ b/schemas/asset.py @@ -35,12 +35,13 @@ class CreateAsset(BaseAsset): # -------- RESPONSE -------- class AssetResponse(ORMBaseModel, BaseAsset): storage_service: str - signed_url: str | None = None + signed_url: str # -------- UPDATE ---------- -class UpdateAsset(BaseAsset): - pass +class UpdateAsset(BaseModel): + name: str | None = None + label: str | None = None # ============= EOF ============================================= diff --git a/tests/test_asset.py b/tests/test_asset.py index 79ce207de..c49adc842 100644 --- a/tests/test_asset.py +++ b/tests/test_asset.py @@ -17,7 +17,7 @@ from core.app import app from core.dependencies import viewer_function, admin_function, editor_function from db import Asset -from tests import client, cleanup_post_test, override_authentication +from tests import client, cleanup_post_test, override_authentication, cleanup_patch_test import pytest @@ -176,4 +176,28 @@ def test_get_asset_by_id_404_not_found(asset): assert data["detail"] == f"Asset with ID {bad_id} not found." +# PATCH tests ================================================================== + + +def test_patch_asset(asset): + payload = {"name": "patched name", "label": "patched label"} + response = client.patch(f"/asset/{asset.id}", json=payload) + assert response.status_code == 200 + data = response.json() + assert data["id"] == asset.id + assert data["name"] == payload["name"] + assert data["label"] == payload["label"] + + cleanup_patch_test(Asset, payload, asset) + + +def test_patch_asset_404_not_found(asset): + bad_id = 99999 + payload = {"name": "patched name", "label": "patched label"} + response = client.patch(f"/asset/{bad_id}", json=payload) + assert response.status_code == 404 + data = response.json() + assert data["detail"] == f"Asset with ID {bad_id} not found." + + # ============= EOF ============================================= From 52e4d06a15f9b0411a190510b4ee8ffa2e6ce78c Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Fri, 22 Aug 2025 10:05:25 -0600 Subject: [PATCH 38/45] feat: implemetn DELETE asset and remove from gcs --- api/asset.py | 45 ++++++++++++++++++++++++++++++++++-------- services/gcs_helper.py | 5 +++++ tests/conftest.py | 20 +++++++++++++++++++ tests/test_asset.py | 30 ++++++++++++++++++++++++++++ 4 files changed, 92 insertions(+), 8 deletions(-) diff --git a/api/asset.py b/api/asset.py index d245288c0..8cf577d81 100644 --- a/api/asset.py +++ b/api/asset.py @@ -18,7 +18,7 @@ from fastapi_pagination.ext.sqlalchemy import paginate from sqlalchemy import select from sqlalchemy.exc import ProgrammingError -from starlette.status import HTTP_201_CREATED, HTTP_409_CONFLICT +from starlette.status import HTTP_201_CREATED, HTTP_409_CONFLICT, HTTP_204_NO_CONTENT from api.pagination import CustomPage from core.dependencies import ( @@ -32,11 +32,12 @@ from db.asset import Asset, AssetThingAssociation from schemas.asset import AssetResponse, CreateAsset, UpdateAsset from services.audit_helper import audit_add -from services.crud_helper import model_patcher +from services.crud_helper import model_patcher, model_deleter from services.query_helper import simple_get_by_id from services.gcs_helper import ( get_storage_bucket, gcs_upload, + gcs_remove, check_asset_exists, add_signed_url, ) @@ -76,7 +77,7 @@ def database_error_handler(payload: CreateAsset, error: ProgrammingError) -> Non raise PydanticStyleException(status_code=HTTP_409_CONFLICT, detail=[detail]) -# ======= Create ========= +# POST ========================================================================= @router.post( "/upload", status_code=HTTP_201_CREATED, dependencies=[Depends(admin_function)] ) @@ -92,14 +93,16 @@ async def upload_asset( @router.post("", status_code=HTTP_201_CREATED) async def add_asset( - user: admin_dependency, session: session_dependency, asset_data: CreateAsset + user: admin_dependency, + session: session_dependency, + asset_data: CreateAsset, + bucket=Depends(get_storage_bucket), ) -> AssetResponse: try: data = asset_data.model_dump() - print(data) thing_id = data.pop("thing_id", None) - print(thing_id) + storage_path = data["storage_path"] # check to see if an asset entry already exists for @@ -124,12 +127,13 @@ async def add_asset( session.add(asset) session.commit() session.refresh(asset) + add_signed_url(asset, bucket) return asset except ProgrammingError as e: database_error_handler(asset_data, e) -# ======= Read ========= +# GET ========================================================================== @router.get("") async def list_assets( session: session_dependency, @@ -166,7 +170,7 @@ async def get_asset( return asset -# ======= Update ========= +# PATCH ======================================================================== @router.patch("/{asset_id}") async def update_asset( asset_id: int, @@ -180,4 +184,29 @@ async def update_asset( return model_patcher(session, Asset, asset_id, asset_data, user=user) +# DELETE ======================================================================= + + +@router.delete("/{asset_id}", status_code=HTTP_204_NO_CONTENT) +async def delete_asset( + asset_id: int, session: session_dependency, user: admin_dependency +): + return model_deleter(session, Asset, asset_id) + + +@router.delete( + "/{asset_id}/remove", + status_code=HTTP_204_NO_CONTENT, + dependencies=[Depends(admin_function)], +) +async def remove_asset( + asset_id: int, + session: session_dependency, + user: admin_dependency, + bucket=Depends(get_storage_bucket), +): + asset = simple_get_by_id(session, Asset, asset_id) + gcs_remove(asset.uri, bucket) + + # ============= EOF ============================================= diff --git a/services/gcs_helper.py b/services/gcs_helper.py index 184e41267..1ef8661c6 100644 --- a/services/gcs_helper.py +++ b/services/gcs_helper.py @@ -76,6 +76,11 @@ def gcs_upload(file: UploadFile, bucket: storage.Bucket = None): return url, blob_name +def gcs_remove(uri: str, bucket: storage.Bucket): + blob = bucket.blob(uri) + blob.delete() + + def add_signed_url(asset: Asset, bucket: storage.Bucket): asset.signed_url = bucket.blob(asset.storage_path).generate_signed_url( version="v4", diff --git a/tests/conftest.py b/tests/conftest.py index 68acbabfb..f193441aa 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -168,6 +168,26 @@ def asset(): session.close() +@pytest.fixture(scope="function") +def second_asset(): + with session_ctx() as session: + asset = Asset( + name="Second test asset", + label="Second test label", + mime_type="application/pdf", + size=2468, + storage_service="mock_service", + storage_path="mock/path/to/asset", + uri="https://storage.googleapis.com/mock-bucket/second-mock-asset", + ) + session.add(asset) + session.commit() + session.refresh(asset) + yield asset + session.delete(asset) + session.close() + + @pytest.fixture(scope="session") def groundwater_level_observation(sensor, sample): with session_ctx() as session: diff --git a/tests/test_asset.py b/tests/test_asset.py index c49adc842..054d2dea8 100644 --- a/tests/test_asset.py +++ b/tests/test_asset.py @@ -31,6 +31,9 @@ def upload_from_file(self, *args, **kwargs): def generate_signed_url(self, *args, **kwargs): return "https://storage.googleapis.com/mock-bucket/mock-asset" + def delete(self, *args, **kwargs): + pass + class MockStorageBucket: name = "mock-bucket" @@ -200,4 +203,31 @@ def test_patch_asset_404_not_found(asset): assert data["detail"] == f"Asset with ID {bad_id} not found." +# DELETE tests ================================================================= + + +def test_delete_asset(second_asset): + response = client.delete(f"/asset/{second_asset.id}") + assert response.status_code == 204 + + # verify deletion + response = client.get(f"/asset/{second_asset.id}") + assert response.status_code == 404 + data = response.json() + assert data["detail"] == f"Asset with ID {second_asset.id} not found." + + +def test_delete_asset_404_not_found(second_asset): + bad_id = 99999 + response = client.delete(f"/asset/{bad_id}/remove") + assert response.status_code == 404 + data = response.json() + assert data["detail"] == f"Asset with ID {bad_id} not found." + + +def test_remove_asset(second_asset): + response = client.delete(f"/asset/{second_asset.id}/remove") + assert response.status_code == 204 + + # ============= EOF ============================================= From b619f1aac2243d34f0f25933ac525d270b19a037 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Fri, 22 Aug 2025 10:16:30 -0600 Subject: [PATCH 39/45] fix: add back thing_id fk error handler for post/patch contact --- api/contact.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/api/contact.py b/api/contact.py index f4c7c1cc0..00dec32c7 100644 --- a/api/contact.py +++ b/api/contact.py @@ -68,6 +68,16 @@ def database_error_handler( error_message = error.orig.args[0]["M"] if ( + error_message + == 'insert or update on table "thing_contact_association" violates foreign key constraint "thing_contact_association_thing_id_fkey"' + ): + detail = { + "loc": ["body", "thing_id"], + "msg": f"Thing with ID {payload.thing_id} not found.", + "type": "value_error", + "input": {"thing_id": payload.thing_id}, + } + elif ( error_message == 'insert or update on table "email" violates foreign key constraint "email_contact_id_fkey"' ): From 1a6bbe5e6ef402997be594a59ab246bbc57e205d Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Fri, 22 Aug 2025 17:45:50 -0600 Subject: [PATCH 40/45] refactor: don't add signed url to asset without existing thing --- api/asset.py | 14 ++++++++++---- schemas/asset.py | 2 +- tests/test_asset.py | 4 ++-- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/api/asset.py b/api/asset.py index 8cf577d81..12b4adabd 100644 --- a/api/asset.py +++ b/api/asset.py @@ -50,11 +50,10 @@ def database_error_handler(payload: CreateAsset, error: ProgrammingError) -> None: """ - Handle errors raised by the database when adding or updating a sample. + Handle errors raised by the database when adding or updating a asset. """ error_message = error.orig.args[0]["M"] - print(error_message) if ( error_message @@ -150,7 +149,8 @@ async def list_assets( ) def transformer(records: list[Asset]): - records = [add_signed_url(ai, bucket) for ai in records] + if thing_id is not None: + records = [add_signed_url(ai, bucket) for ai in records] return records return paginate(query=sql, conn=session, transformer=transformer) @@ -166,7 +166,13 @@ async def get_asset( Retrieve an asset by its ID. """ asset = simple_get_by_id(session, Asset, asset_id) - add_signed_url(asset, bucket) + + sql = select(AssetThingAssociation).where( + AssetThingAssociation.asset_id == asset_id + ) + + if session.execute(sql).scalars().first() is not None: + add_signed_url(asset, bucket) return asset diff --git a/schemas/asset.py b/schemas/asset.py index e7d1d4bd1..de8b37c10 100644 --- a/schemas/asset.py +++ b/schemas/asset.py @@ -35,7 +35,7 @@ class CreateAsset(BaseAsset): # -------- RESPONSE -------- class AssetResponse(ORMBaseModel, BaseAsset): storage_service: str - signed_url: str + signed_url: str | None = None # -------- UPDATE ---------- diff --git a/tests/test_asset.py b/tests/test_asset.py index 054d2dea8..6144178e6 100644 --- a/tests/test_asset.py +++ b/tests/test_asset.py @@ -152,7 +152,7 @@ def test_get_assets(asset): assert data["items"][0]["size"] == asset.size assert data["items"][0]["uri"] == asset.uri assert data["items"][0]["storage_service"] == asset.storage_service - assert data["items"][0]["signed_url"] == MockBlob().generate_signed_url() + assert data["items"][0]["signed_url"] == None def test_get_asset_by_id(asset): @@ -168,7 +168,7 @@ def test_get_asset_by_id(asset): assert data["size"] == asset.size assert data["uri"] == asset.uri assert data["storage_service"] == asset.storage_service - assert data["signed_url"] == MockBlob().generate_signed_url() + assert data["signed_url"] == None def test_get_asset_by_id_404_not_found(asset): From a3ac5d092acf822a93e1af93d6e900170066a8fa Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Fri, 22 Aug 2025 18:07:12 -0600 Subject: [PATCH 41/45] refactor: only add signed url for assets with thing associations --- api/asset.py | 15 +++++++-------- tests/conftest.py | 27 +++++++++++++++++++++++++++ tests/test_asset.py | 27 ++++++++++++++++++++++++--- 3 files changed, 58 insertions(+), 11 deletions(-) diff --git a/api/asset.py b/api/asset.py index 12b4adabd..a3da27348 100644 --- a/api/asset.py +++ b/api/asset.py @@ -126,7 +126,9 @@ async def add_asset( session.add(asset) session.commit() session.refresh(asset) - add_signed_url(asset, bucket) + + if thing_id: + add_signed_url(asset, bucket) return asset except ProgrammingError as e: database_error_handler(asset_data, e) @@ -149,8 +151,9 @@ async def list_assets( ) def transformer(records: list[Asset]): - if thing_id is not None: - records = [add_signed_url(ai, bucket) for ai in records] + records = [ + add_signed_url(ai, bucket) if ai.things != [] else ai for ai in records + ] return records return paginate(query=sql, conn=session, transformer=transformer) @@ -167,11 +170,7 @@ async def get_asset( """ asset = simple_get_by_id(session, Asset, asset_id) - sql = select(AssetThingAssociation).where( - AssetThingAssociation.asset_id == asset_id - ) - - if session.execute(sql).scalars().first() is not None: + if asset.things != []: add_signed_url(asset, bucket) return asset diff --git a/tests/conftest.py b/tests/conftest.py index f193441aa..de49b0763 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -168,6 +168,33 @@ def asset(): session.close() +@pytest.fixture(scope="function") +def asset_with_associated_thing(thing): + with session_ctx() as session: + asset = Asset( + name="Test Asset", + label="test label", + mime_type="application/pdf", + size=12345, + storage_service="mock_service", + storage_path="mock/path/to/asset", + uri="https://storage.googleapis.com/mock-bucket/mock-asset", + ) + session.add(asset) + session.commit() + session.refresh(asset) + + association = AssetThingAssociation(asset_id=asset.id, thing_id=thing.id) + session.add(association) + session.commit() + session.refresh(association) + + yield asset + session.delete(asset) + session.delete(association) + session.close() + + @pytest.fixture(scope="function") def second_asset(): with session_ctx() as session: diff --git a/tests/test_asset.py b/tests/test_asset.py index 6144178e6..a917bf950 100644 --- a/tests/test_asset.py +++ b/tests/test_asset.py @@ -136,11 +136,11 @@ def test_add_asset_409_bad_thing_id(thing): # GET tests ==================================================================== -def test_get_assets(asset): +def test_get_assets(asset, asset_with_associated_thing): response = client.get("/asset") assert response.status_code == 200 data = response.json() - assert data["total"] == 1 + assert data["total"] == 2 assert data["items"][0]["id"] == asset.id assert data["items"][0]["created_at"] == asset.created_at.isoformat().replace( "+00:00", "Z" @@ -154,8 +154,11 @@ def test_get_assets(asset): assert data["items"][0]["storage_service"] == asset.storage_service assert data["items"][0]["signed_url"] == None + assert data["items"][1]["id"] == asset_with_associated_thing.id + assert data["items"][1]["signed_url"] == MockBlob().generate_signed_url() -def test_get_asset_by_id(asset): + +def test_get_asset_by_id_no_associated_thing(asset): response = client.get(f"/asset/{asset.id}") assert response.status_code == 200 data = response.json() @@ -171,6 +174,24 @@ def test_get_asset_by_id(asset): assert data["signed_url"] == None +def test_get_asset_by_id_with_associated_thing(asset_with_associated_thing): + response = client.get(f"/asset/{asset_with_associated_thing.id}") + assert response.status_code == 200 + data = response.json() + assert data["id"] == asset_with_associated_thing.id + assert data[ + "created_at" + ] == asset_with_associated_thing.created_at.isoformat().replace("+00:00", "Z") + assert data["name"] == asset_with_associated_thing.name + assert data["label"] == asset_with_associated_thing.label + assert data["storage_path"] == asset_with_associated_thing.storage_path + assert data["mime_type"] == asset_with_associated_thing.mime_type + assert data["size"] == asset_with_associated_thing.size + assert data["uri"] == asset_with_associated_thing.uri + assert data["storage_service"] == asset_with_associated_thing.storage_service + assert data["signed_url"] == MockBlob().generate_signed_url() + + def test_get_asset_by_id_404_not_found(asset): bad_id = 99999 response = client.get(f"/asset/{bad_id}") From 71aba6738b18bde558265e78728fb273c72c4ec3 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Fri, 22 Aug 2025 18:14:00 -0600 Subject: [PATCH 42/45] fix: delete fixtures --- tests/conftest.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/conftest.py b/tests/conftest.py index de49b0763..0fc57fb1f 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -172,7 +172,7 @@ def asset(): def asset_with_associated_thing(thing): with session_ctx() as session: asset = Asset( - name="Test Asset", + name="Test Asset with thing", label="test label", mime_type="application/pdf", size=12345, @@ -192,6 +192,7 @@ def asset_with_associated_thing(thing): yield asset session.delete(asset) session.delete(association) + session.commit() session.close() From d48ee3e3e5f3a1b2a58564461d05b9f0166aa39a Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Mon, 25 Aug 2025 09:31:58 -0600 Subject: [PATCH 43/45] refactor: address PR 97 feedback --- api/asset.py | 10 +++------- tests/test_asset.py | 36 ++++++++++++++++-------------------- 2 files changed, 19 insertions(+), 27 deletions(-) diff --git a/api/asset.py b/api/asset.py index a3da27348..646c13595 100644 --- a/api/asset.py +++ b/api/asset.py @@ -127,8 +127,6 @@ async def add_asset( session.commit() session.refresh(asset) - if thing_id: - add_signed_url(asset, bucket) return asset except ProgrammingError as e: database_error_handler(asset_data, e) @@ -151,9 +149,8 @@ async def list_assets( ) def transformer(records: list[Asset]): - records = [ - add_signed_url(ai, bucket) if ai.things != [] else ai for ai in records - ] + if thing_id is not None: + records = [add_signed_url(ai, bucket) for ai in records] return records return paginate(query=sql, conn=session, transformer=transformer) @@ -170,8 +167,7 @@ async def get_asset( """ asset = simple_get_by_id(session, Asset, asset_id) - if asset.things != []: - add_signed_url(asset, bucket) + add_signed_url(asset, bucket) return asset diff --git a/tests/test_asset.py b/tests/test_asset.py index a917bf950..7d941145e 100644 --- a/tests/test_asset.py +++ b/tests/test_asset.py @@ -108,6 +108,7 @@ def test_add_asset(thing): assert data["storage_path"] == payload["storage_path"] assert data["mime_type"] == payload["mime_type"] assert data["size"] == payload["size"] + assert data["signed_url"] == None cleanup_post_test(Asset, data["id"]) @@ -155,10 +156,23 @@ def test_get_assets(asset, asset_with_associated_thing): assert data["items"][0]["signed_url"] == None assert data["items"][1]["id"] == asset_with_associated_thing.id - assert data["items"][1]["signed_url"] == MockBlob().generate_signed_url() + assert data["items"][1]["signed_url"] == None + + +def test_get_assets_thing_id(asset_with_associated_thing, thing): + query_parameters = {"thing_id": thing.id} + response = client.get("/asset", params=query_parameters) + assert response.status_code == 200 + data = response.json() + assert data["total"] == 1 + assert data["items"][0]["id"] == asset_with_associated_thing.id + assert ( + data["items"][0]["signed_url"] + == mock_storage_bucket().blob().generate_signed_url() + ) -def test_get_asset_by_id_no_associated_thing(asset): +def test_get_asset_by_id(asset): response = client.get(f"/asset/{asset.id}") assert response.status_code == 200 data = response.json() @@ -171,24 +185,6 @@ def test_get_asset_by_id_no_associated_thing(asset): assert data["size"] == asset.size assert data["uri"] == asset.uri assert data["storage_service"] == asset.storage_service - assert data["signed_url"] == None - - -def test_get_asset_by_id_with_associated_thing(asset_with_associated_thing): - response = client.get(f"/asset/{asset_with_associated_thing.id}") - assert response.status_code == 200 - data = response.json() - assert data["id"] == asset_with_associated_thing.id - assert data[ - "created_at" - ] == asset_with_associated_thing.created_at.isoformat().replace("+00:00", "Z") - assert data["name"] == asset_with_associated_thing.name - assert data["label"] == asset_with_associated_thing.label - assert data["storage_path"] == asset_with_associated_thing.storage_path - assert data["mime_type"] == asset_with_associated_thing.mime_type - assert data["size"] == asset_with_associated_thing.size - assert data["uri"] == asset_with_associated_thing.uri - assert data["storage_service"] == asset_with_associated_thing.storage_service assert data["signed_url"] == MockBlob().generate_signed_url() From 90cee54ec5def5fe34280b56e7804cbf012d6cad Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Mon, 25 Aug 2025 09:40:15 -0600 Subject: [PATCH 44/45] refactor: get_storage_bucket should not be dependency in GET /asset --- api/asset.py | 2 +- tests/test_asset.py | 22 ++++++++++++---------- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/api/asset.py b/api/asset.py index 646c13595..df5f516d0 100644 --- a/api/asset.py +++ b/api/asset.py @@ -137,7 +137,6 @@ async def add_asset( async def list_assets( session: session_dependency, thing_id: int = None, - bucket=Depends(get_storage_bucket), ) -> CustomPage[AssetResponse]: """ List all assets or assets associated with a specific thing. @@ -150,6 +149,7 @@ async def list_assets( def transformer(records: list[Asset]): if thing_id is not None: + bucket = get_storage_bucket() records = [add_signed_url(ai, bucket) for ai in records] return records diff --git a/tests/test_asset.py b/tests/test_asset.py index 7d941145e..e56b463a4 100644 --- a/tests/test_asset.py +++ b/tests/test_asset.py @@ -20,6 +20,7 @@ from tests import client, cleanup_post_test, override_authentication, cleanup_patch_test import pytest +from unittest.mock import patch # CLASSES, FIXTURES, AND FUNCTIONS ============================================= @@ -160,16 +161,17 @@ def test_get_assets(asset, asset_with_associated_thing): def test_get_assets_thing_id(asset_with_associated_thing, thing): - query_parameters = {"thing_id": thing.id} - response = client.get("/asset", params=query_parameters) - assert response.status_code == 200 - data = response.json() - assert data["total"] == 1 - assert data["items"][0]["id"] == asset_with_associated_thing.id - assert ( - data["items"][0]["signed_url"] - == mock_storage_bucket().blob().generate_signed_url() - ) + with patch("api.asset.get_storage_bucket", return_value=MockStorageBucket()): + query_parameters = {"thing_id": thing.id} + response = client.get("/asset", params=query_parameters) + assert response.status_code == 200 + data = response.json() + assert data["total"] == 1 + assert data["items"][0]["id"] == asset_with_associated_thing.id + assert ( + data["items"][0]["signed_url"] + == mock_storage_bucket().blob().generate_signed_url() + ) def test_get_asset_by_id(asset): From 611ca3d5c342b123e816fb1440a5a1d9cb654fe6 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Mon, 25 Aug 2025 10:58:57 -0600 Subject: [PATCH 45/45] feat: add developer's note about signed urls for assets --- api/asset.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/api/asset.py b/api/asset.py index df5f516d0..db93a5d2b 100644 --- a/api/asset.py +++ b/api/asset.py @@ -133,6 +133,18 @@ async def add_asset( # GET ========================================================================== + +""" +Developer's notes + +Do not generate signed urls when listing ALL assets. There is a reason to +generate signed urls when listing assets for a given `thing_id` because this +is used by the front end to display a gallery of images all at once. This is +the only case in which signed urls should be generated for a list of assets. A +signed url is always generated when retrieving assets individually +""" + + @router.get("") async def list_assets( session: session_dependency,