From 13961d454b8c5026ed08fe7b8b2df645b5be7324 Mon Sep 17 00:00:00 2001 From: jukkap Date: Wed, 7 Feb 2024 14:07:47 +0200 Subject: [PATCH 01/21] Feat: return all available observations from a station if parameter names are not given --- api/routers/edr.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/api/routers/edr.py b/api/routers/edr.py index 1194f8f2..f2fbc919 100644 --- a/api/routers/edr.py +++ b/api/routers/edr.py @@ -56,12 +56,12 @@ async def get_locations(bbox: str = Query(..., example="5.0,52.0,6.0,52.1")) -> @router.get( "/locations/{location_id}", tags=["Collection data queries"], - response_model=Coverage, + response_model=Coverage | CoverageCollection, response_model_exclude_none=True, ) async def get_data_location_id( location_id: str = Path(..., example="06260"), - parameter_name: str = Query(..., alias="parameter-name", example="dd,ff,rh,pp,tn"), + parameter_name: str = Query(None, alias="parameter-name", example="dd,ff,rh,pp,tn"), datetime: str | None = None, f: str = Query(default="covjson", alias="f", description="Specify return format."), ): @@ -71,7 +71,9 @@ async def get_data_location_id( get_obs_request = dstore.GetObsRequest( filter=dict( platform=dstore.Strings(values=[location_id]), - instrument=dstore.Strings(values=list(map(str.strip, parameter_name.split(",")))), + instrument=dstore.Strings( + values=list(map(str.strip, parameter_name.split(","))) if parameter_name else "*" + ), ), interval=dstore.TimeInterval(start=range[0], end=range[1]) if range else None, ) From 5cee99e605926ed93186485342eb6d496f6dc80e Mon Sep 17 00:00:00 2001 From: jukkap Date: Wed, 7 Feb 2024 14:21:31 +0200 Subject: [PATCH 02/21] Feat: return all available observations from an area or position if parameter names are not given --- api/routers/edr.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/api/routers/edr.py b/api/routers/edr.py index f2fbc919..9beef8f0 100644 --- a/api/routers/edr.py +++ b/api/routers/edr.py @@ -89,7 +89,7 @@ async def get_data_location_id( ) async def get_data_position( coords: str = Query(..., example="POINT(5.179705 52.0988218)"), - parameter_name: str = Query(..., alias="parameter-name", example="dd,ff,rh,pp,tn"), + parameter_name: str = Query(None, alias="parameter-name", example="dd,ff,rh,pp,tn"), datetime: str | None = None, f: str = Query(default="covjson", alias="f", description="Specify return format."), ): @@ -107,7 +107,7 @@ async def get_data_position( ) async def get_data_area( coords: str = Query(..., example="POLYGON((5.0 52.0, 6.0 52.0,6.0 52.1,5.0 52.1, 5.0 52.0))"), - parameter_name: str = Query(..., alias="parameter-name", example="dd,ff,rh,pp,tn"), + parameter_name: str = Query(None, alias="parameter-name", example="dd,ff,rh,pp,tn"), datetime: str | None = None, f: str = Query(default="covjson", alias="f", description="Specify return format."), ): @@ -115,7 +115,9 @@ async def get_data_area( assert poly.geom_type == "Polygon" range = get_datetime_range(datetime) get_obs_request = dstore.GetObsRequest( - filter=dict(instrument=dstore.Strings(values=list(map(str.strip, parameter_name.split(","))))), + filter=dict( + instrument=dstore.Strings(values=list(map(str.strip, parameter_name.split(","))) if parameter_name else "*") + ), inside=dstore.Polygon(points=[dstore.Point(lat=coord[1], lon=coord[0]) for coord in poly.exterior.coords]), interval=dstore.TimeInterval(start=range[0], end=range[1]) if range else None, ) From 2fb606a1e4505791273dcc4670cd2a43c790b5d5 Mon Sep 17 00:00:00 2001 From: jukkap Date: Wed, 7 Feb 2024 14:27:15 +0200 Subject: [PATCH 03/21] Refactor: Add examples to API docs for datetime --- api/routers/edr.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/api/routers/edr.py b/api/routers/edr.py index 9beef8f0..dfddc52c 100644 --- a/api/routers/edr.py +++ b/api/routers/edr.py @@ -62,7 +62,7 @@ async def get_locations(bbox: str = Query(..., example="5.0,52.0,6.0,52.1")) -> async def get_data_location_id( location_id: str = Path(..., example="06260"), parameter_name: str = Query(None, alias="parameter-name", example="dd,ff,rh,pp,tn"), - datetime: str | None = None, + datetime: str = Query(None, example="2022-12-31T00:00Z/2023-01-01T00:00Z"), f: str = Query(default="covjson", alias="f", description="Specify return format."), ): # TODO: There is no error handling of any kind at the moment! @@ -90,7 +90,7 @@ async def get_data_location_id( async def get_data_position( coords: str = Query(..., example="POINT(5.179705 52.0988218)"), parameter_name: str = Query(None, alias="parameter-name", example="dd,ff,rh,pp,tn"), - datetime: str | None = None, + datetime: str = Query(None, example="2022-12-31T00:00Z/2023-01-01T00:00Z"), f: str = Query(default="covjson", alias="f", description="Specify return format."), ): point = wkt.loads(coords) @@ -108,7 +108,7 @@ async def get_data_position( async def get_data_area( coords: str = Query(..., example="POLYGON((5.0 52.0, 6.0 52.0,6.0 52.1,5.0 52.1, 5.0 52.0))"), parameter_name: str = Query(None, alias="parameter-name", example="dd,ff,rh,pp,tn"), - datetime: str | None = None, + datetime: str = Query(None, example="2022-12-31T00:00Z/2023-01-01T00:00Z"), f: str = Query(default="covjson", alias="f", description="Specify return format."), ): poly = wkt.loads(coords) From 334c49ed5d39fa43f230e17f18f548dccaf86e23 Mon Sep 17 00:00:00 2001 From: jukkap Date: Wed, 7 Feb 2024 15:02:31 +0200 Subject: [PATCH 04/21] Refactor: Error handling to get_datetime_range --- api/dependencies.py | 44 ++++++++++++++++++++++++++++++-------------- 1 file changed, 30 insertions(+), 14 deletions(-) diff --git a/api/dependencies.py b/api/dependencies.py index a5924f9c..b3e5b44e 100644 --- a/api/dependencies.py +++ b/api/dependencies.py @@ -2,6 +2,7 @@ from datetime import timedelta from typing import Tuple +from fastapi import HTTPException from google.protobuf.timestamp_pb2 import Timestamp from pydantic import AwareDatetime from pydantic import TypeAdapter @@ -11,23 +12,38 @@ def get_datetime_range(datetime_string: str | None) -> Tuple[Timestamp, Timestam if not datetime_string: return None + errors = {} + start_datetime, end_datetime = Timestamp(), Timestamp() aware_datetime_type_adapter = TypeAdapter(AwareDatetime) - datetimes = tuple(value.strip() for value in datetime_string.split("/")) - if len(datetimes) == 1: - start_datetime.FromDatetime(aware_datetime_type_adapter.validate_python(datetimes[0])) - end_datetime.FromDatetime( - aware_datetime_type_adapter.validate_python(datetimes[0]) + timedelta(seconds=1) - ) # HACK: Add one second so we get some data, as the store returns [start, end) - else: - if datetimes[0] != "..": + + try: + datetimes = tuple(value.strip() for value in datetime_string.split("/")) + if len(datetimes) == 1: start_datetime.FromDatetime(aware_datetime_type_adapter.validate_python(datetimes[0])) + end_datetime.FromDatetime( + aware_datetime_type_adapter.validate_python(datetimes[0]) + timedelta(seconds=1) + ) # HACK: Add one second so we get some data, as the store returns [start, end) else: - start_datetime.FromDatetime(datetime.min) - if datetimes[1] != "..": - # HACK add one second so that the end_datetime is included in the interval. - end_datetime.FromDatetime(aware_datetime_type_adapter.validate_python(datetimes[1]) + timedelta(seconds=1)) - else: - end_datetime.FromDatetime(datetime.max) + if datetimes[0] != "..": + start_datetime.FromDatetime(aware_datetime_type_adapter.validate_python(datetimes[0])) + else: + start_datetime.FromDatetime(datetime.min) + if datetimes[1] != "..": + # HACK add one second so that the end_datetime is included in the interval. + end_datetime.FromDatetime( + aware_datetime_type_adapter.validate_python(datetimes[1]) + timedelta(seconds=1) + ) + else: + end_datetime.FromDatetime(datetime.max) + + if start_datetime.seconds > end_datetime.seconds: + errors["datetime"] = f"Invalid range: {datetimes[0]} > {datetimes[1]}" + + except ValueError: + errors["datetime"] = f"Invalid format: {datetime_string}" + + if errors: + raise HTTPException(status_code=422, detail=errors) return start_datetime, end_datetime From 78744e8f38f99748cfa997d6cac2f127012c02f4 Mon Sep 17 00:00:00 2001 From: jukkap Date: Wed, 7 Feb 2024 16:07:13 +0200 Subject: [PATCH 05/21] Test: Add tests for /locations/{id} endpoint --- api/test/test_edr.py | 77 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 77 insertions(+) create mode 100644 api/test/test_edr.py diff --git a/api/test/test_edr.py b/api/test/test_edr.py new file mode 100644 index 00000000..745202a2 --- /dev/null +++ b/api/test/test_edr.py @@ -0,0 +1,77 @@ +import json +from unittest.mock import patch + +import datastore_pb2 as dstore +from fastapi.testclient import TestClient +from google.protobuf.json_format import Parse +from main import app + + +client = TestClient(app) + + +def test_get_locations_id_with_single_parameter_query_without_format(): + with patch("routers.edr.getObsRequest") as mock_getObsRequest: + test_data = load_json("test/test_data/test_single_proto.json") + compare_data = load_json("test/test_data/test_single_covjson.json") + + mock_getObsRequest.return_value = create_mock_obs_response(test_data) + + response = client.get( + "/collections/observations/locations/06260?" + + "parameter-name=ff&datetime=2022-12-31T00:00:00Z/2022-12-31T01:00:00Z" + ) + + # Check that getObsRequest gets called with correct arguments given in query + mock_getObsRequest.assert_called_once() + assert "ff" in mock_getObsRequest.call_args[0][0].filter["instrument"].values + assert "06260" in mock_getObsRequest.call_args[0][0].filter["platform"].values + + assert response.status_code == 200 + assert response.json()["type"] == "Coverage" + assert response.json() == compare_data + + +def test_get_locations_id_without_parameter_names_query(): + with patch("routers.edr.getObsRequest") as mock_getObsRequest: + test_data = load_json("test/test_data/test_multiple_proto.json") + compare_data = load_json("test/test_data/test_multiple_covjson.json") + + mock_getObsRequest.return_value = create_mock_obs_response(test_data) + + response = client.get("/collections/observations/locations/06260?f=covjson") + + # Check that getObsRequest gets called with correct arguments when no parameter names are given + # in query + mock_getObsRequest.assert_called_once() + assert "*" in mock_getObsRequest.call_args[0][0].filter["instrument"].values + + assert response.status_code == 200 + assert response.json() == compare_data + + +def test_get_locations_id_with_incorrect_datetime_format(): + response = client.get("/collections/observations/locations/06260?datetime=20221231T000000Z/20221231T010000Z") + + assert response.status_code == 422 + assert response.json() == {"detail": {"datetime": "Invalid format: 20221231T000000Z/20221231T010000Z"}} + + +def test_get_locations_id_with_incorrect_datetime_range(): + response = client.get( + "/collections/observations/locations/06260?datetime=2024-12-31T00:00:00Z/2022-12-31T01:00:00Z" + ) + + assert response.status_code == 422 + assert response.json() == {"detail": {"datetime": "Invalid range: 2024-12-31T00:00:00Z > 2022-12-31T01:00:00Z"}} + + +def create_mock_obs_response(json_data): + response = dstore.GetObsResponse() + Parse(json.dumps(json_data), response) + return response + + +def load_json(file_path): + with open(file_path, "r") as file: + return json.load(file) From 8d71fe02ffd8a914495b15674289d9d97562190d Mon Sep 17 00:00:00 2001 From: jukkap Date: Wed, 7 Feb 2024 16:11:07 +0200 Subject: [PATCH 06/21] CI: update GitHub actions with testing depencies --- .github/workflows/ci.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 555538e9..dcbcfa3b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -129,6 +129,7 @@ jobs: pip install --upgrade pip pip install pytest-timeout pip install pytest-cov + pip install httpx pip install -r ./api/requirements.txt - name: Copy Protobuf file to api directory and build From 89eaaf12d3d807cdc69eb5added0631f59349df2 Mon Sep 17 00:00:00 2001 From: jukkap Date: Thu, 8 Feb 2024 13:32:18 +0200 Subject: [PATCH 07/21] Refactor: Add error catching to coord conversions --- api/routers/edr.py | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/api/routers/edr.py b/api/routers/edr.py index dfddc52c..f9b86105 100644 --- a/api/routers/edr.py +++ b/api/routers/edr.py @@ -5,6 +5,7 @@ from covjson_pydantic.coverage import CoverageCollection from dependencies import get_datetime_range from fastapi import APIRouter +from fastapi import HTTPException from fastapi import Path from fastapi import Query from geojson_pydantic import Feature @@ -93,9 +94,13 @@ async def get_data_position( datetime: str = Query(None, example="2022-12-31T00:00Z/2023-01-01T00:00Z"), f: str = Query(default="covjson", alias="f", description="Specify return format."), ): - point = wkt.loads(coords) - assert point.geom_type == "Point" - poly = buffer(point, 0.0001, quad_segs=1) # Roughly 10 meters around the point + try: + point = wkt.loads(coords) + assert point.geom_type == "Point" + poly = buffer(point, 0.0001, quad_segs=1) # Roughly 10 meters around the point + except Exception: + raise HTTPException(status_code=422, detail={"coords": "Invalid coordinates: {}".format(coords)}) + return await get_data_area(poly.wkt, parameter_name, datetime, f) @@ -111,8 +116,12 @@ async def get_data_area( datetime: str = Query(None, example="2022-12-31T00:00Z/2023-01-01T00:00Z"), f: str = Query(default="covjson", alias="f", description="Specify return format."), ): - poly = wkt.loads(coords) - assert poly.geom_type == "Polygon" + try: + poly = wkt.loads(coords) + assert poly.geom_type == "Polygon" + except Exception: + raise HTTPException(status_code=422, detail={"coords": "Invalid coordinates: {}".format(coords)}) + range = get_datetime_range(datetime) get_obs_request = dstore.GetObsRequest( filter=dict( From a0653f2cf2f3c3ca93888393c512d48a03a6b9a1 Mon Sep 17 00:00:00 2001 From: jukkap Date: Thu, 8 Feb 2024 13:34:58 +0200 Subject: [PATCH 08/21] Test: add tests for area & position ednpoints --- api/test/test_edr.py | 109 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 109 insertions(+) diff --git a/api/test/test_edr.py b/api/test/test_edr.py index 745202a2..d8a2bf20 100644 --- a/api/test/test_edr.py +++ b/api/test/test_edr.py @@ -2,6 +2,7 @@ from unittest.mock import patch import datastore_pb2 as dstore +import routers.edr as edr from fastapi.testclient import TestClient from google.protobuf.json_format import Parse from main import app @@ -26,6 +27,12 @@ def test_get_locations_id_with_single_parameter_query_without_format(): mock_getObsRequest.assert_called_once() assert "ff" in mock_getObsRequest.call_args[0][0].filter["instrument"].values assert "06260" in mock_getObsRequest.call_args[0][0].filter["platform"].values + assert "2022-12-31 00:00:00" == mock_getObsRequest.call_args[0][0].interval.start.ToDatetime().strftime( + "%Y-%m-%d %H:%M:%S" + ) + assert "2022-12-31 01:00:01" == mock_getObsRequest.call_args[0][0].interval.end.ToDatetime().strftime( + "%Y-%m-%d %H:%M:%S" + ) assert response.status_code == 200 assert response.json()["type"] == "Coverage" @@ -66,6 +73,108 @@ def test_get_locations_id_with_incorrect_datetime_range(): assert response.json() == {"detail": {"datetime": "Invalid range: 2024-12-31T00:00:00Z > 2022-12-31T01:00:00Z"}} +def test_get_locations_id_with_empty_response(): + with patch("routers.edr.getObsRequest") as mock_getObsRequest: + test_data = load_json("test/test_data/test_empty_proto.json") + + mock_getObsRequest.return_value = create_mock_obs_response(test_data) + + response = client.get("/collections/observations/locations/10000?f=covjson") + + assert response.status_code == 404 + assert response.json() == {"detail": "No data found"} + + +def test_get_area_with_normal_query(): + with patch("routers.edr.getObsRequest") as mock_getObsRequest: + test_data = load_json("test/test_data/test_coverages_proto.json") + compare_data = load_json("test/test_data/test_coverages_covjson.json") + + mock_getObsRequest.return_value = create_mock_obs_response(test_data) + + response = client.get( + "/collections/observations/area?coords=POLYGON((22.12 59.86, 24.39 60.41, " + " 24.39 60.41, 24.39 59.86, 22.12 59.86))" + "¶meter-name=TA_P1D_AVG&datetime=2022-12-31T00:00:00Z/2022-12-31T01:00:00Z" + ) + + # Check that getObsRequest gets called with correct arguments given in query + mock_getObsRequest.assert_called_once() + assert "TA_P1D_AVG" in mock_getObsRequest.call_args[0][0].filter["instrument"].values + assert len(mock_getObsRequest.call_args[0][0].inside.points) == 5 + assert 22.12 == mock_getObsRequest.call_args[0][0].inside.points[0].lon + assert "2022-12-31 00:00:00" == mock_getObsRequest.call_args[0][0].interval.start.ToDatetime().strftime( + "%Y-%m-%d %H:%M:%S" + ) + assert "2022-12-31 01:00:01" == mock_getObsRequest.call_args[0][0].interval.end.ToDatetime().strftime( + "%Y-%m-%d %H:%M:%S" + ) + + assert response.status_code == 200 + assert response.json() == compare_data + + +def test_get_area_with_incorrect_coords(): + response = client.get("/collections/observations/area?coords=POLYGON((22.12 59.86, 24.39 60.41))") + + assert response.status_code == 422 + + +def test_get_area_with_incorrect_geometry_type(): + response = client.get("/collections/observations/area?coords=POINT(22.12 59.86)") + + assert response.status_code == 422 + + +def test_get_position_with_normal_query(): + # Wrap the original get_data_area to a mock so we can assert against the call values + with patch("routers.edr.get_data_area", wraps=edr.get_data_area) as mock_get_data_area, patch( + "routers.edr.getObsRequest" + ) as mock_getObsRequest: + test_data = load_json("test/test_data/test_coverages_proto.json") + compare_data = load_json("test/test_data/test_coverages_covjson.json") + + mock_getObsRequest.return_value = create_mock_obs_response(test_data) + + response = client.get( + "/collections/observations/position?coords=POINT(5.179705 52.0988218)" + "¶meter-name=TA_P1D_AVG&datetime=2022-12-31T00:00Z/2022-12-31T00:00Z" + ) + + mock_get_data_area.assert_called_once() + mock_get_data_area.assert_called_with( + "POLYGON ((5.179805 52.0988218, 5.179705 52.0987218, 5.1796050000000005 52.0988218, " + "5.179705 52.09892180000001, 5.179805 52.0988218))", + "TA_P1D_AVG", + "2022-12-31T00:00Z/2022-12-31T00:00Z", + "covjson", + ) + + assert response.status_code == 200 + assert response.json() == compare_data + + +def test_get_position_with_incorrect_coords(): + response = client.get("/collections/observations/position?coords=POINT(60.41)") + + assert response.status_code == 422 + assert response.json() == {"detail": {"coords": "Invalid coordinates: POINT(60.41)"}} + + +def test_get_position_with_incorrect_geometry_type(): + response = client.get( + "/collections/observations/position?coords=POLYGON((22.12 59.86, 24.39 60.41, " + "24.39 60.41, 24.39 59.86, 22.12 59.86))" + ) + + assert response.status_code == 422 + assert response.json() == { + "detail": { + "coords": "Invalid coordinates: POLYGON((22.12 59.86, 24.39 60.41, 24.39 60.41, 24.39 59.86, 22.12 59.86))" + } + } + + def create_mock_obs_response(json_data): response = dstore.GetObsResponse() Parse(json.dumps(json_data), response) From e87e51a68f562bbddc0f3b06d948082c9ee5c4e9 Mon Sep 17 00:00:00 2001 From: jukkap Date: Thu, 8 Feb 2024 14:10:26 +0200 Subject: [PATCH 09/21] Test: fix assertions with corrected attribute names --- api/test/test_edr.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/api/test/test_edr.py b/api/test/test_edr.py index d8a2bf20..3f3fe888 100644 --- a/api/test/test_edr.py +++ b/api/test/test_edr.py @@ -27,10 +27,10 @@ def test_get_locations_id_with_single_parameter_query_without_format(): mock_getObsRequest.assert_called_once() assert "ff" in mock_getObsRequest.call_args[0][0].filter["instrument"].values assert "06260" in mock_getObsRequest.call_args[0][0].filter["platform"].values - assert "2022-12-31 00:00:00" == mock_getObsRequest.call_args[0][0].interval.start.ToDatetime().strftime( - "%Y-%m-%d %H:%M:%S" - ) - assert "2022-12-31 01:00:01" == mock_getObsRequest.call_args[0][0].interval.end.ToDatetime().strftime( + assert "2022-12-31 00:00:00" == mock_getObsRequest.call_args[0][ + 0 + ].temporal_interval.start.ToDatetime().strftime("%Y-%m-%d %H:%M:%S") + assert "2022-12-31 01:00:01" == mock_getObsRequest.call_args[0][0].temporal_interval.end.ToDatetime().strftime( "%Y-%m-%d %H:%M:%S" ) @@ -101,12 +101,12 @@ def test_get_area_with_normal_query(): # Check that getObsRequest gets called with correct arguments given in query mock_getObsRequest.assert_called_once() assert "TA_P1D_AVG" in mock_getObsRequest.call_args[0][0].filter["instrument"].values - assert len(mock_getObsRequest.call_args[0][0].inside.points) == 5 - assert 22.12 == mock_getObsRequest.call_args[0][0].inside.points[0].lon - assert "2022-12-31 00:00:00" == mock_getObsRequest.call_args[0][0].interval.start.ToDatetime().strftime( - "%Y-%m-%d %H:%M:%S" - ) - assert "2022-12-31 01:00:01" == mock_getObsRequest.call_args[0][0].interval.end.ToDatetime().strftime( + assert len(mock_getObsRequest.call_args[0][0].spatial_area.points) == 5 + assert 22.12 == mock_getObsRequest.call_args[0][0].spatial_area.points[0].lon + assert "2022-12-31 00:00:00" == mock_getObsRequest.call_args[0][ + 0 + ].temporal_interval.start.ToDatetime().strftime("%Y-%m-%d %H:%M:%S") + assert "2022-12-31 01:00:01" == mock_getObsRequest.call_args[0][0].temporal_interval.end.ToDatetime().strftime( "%Y-%m-%d %H:%M:%S" ) From 14e9bc1c2f33801a3b3e76787744ce8925665de7 Mon Sep 17 00:00:00 2001 From: jukkap Date: Thu, 8 Feb 2024 15:10:32 +0200 Subject: [PATCH 10/21] Refactor: Raise explicit errors with coordinates --- api/routers/edr.py | 17 +++++++++++++++-- api/test/test_edr.py | 12 ++++++------ 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/api/routers/edr.py b/api/routers/edr.py index db6ee006..6724b7af 100644 --- a/api/routers/edr.py +++ b/api/routers/edr.py @@ -15,6 +15,7 @@ from shapely import buffer from shapely import geometry from shapely import wkt +from shapely.errors import GEOSException router = APIRouter(prefix="/collections/observations") @@ -100,8 +101,14 @@ async def get_data_position( point = wkt.loads(coords) assert point.geom_type == "Point" poly = buffer(point, 0.0001, quad_segs=1) # Roughly 10 meters around the point + except GEOSException: + raise HTTPException(status_code=422, detail={"coords": f"Invalid or unparseable wkt provided: {coords}"}) + except AssertionError: + raise HTTPException(status_code=422, detail={"coords": f"Invalid geometric type: {point.geom_type}"}) except Exception: - raise HTTPException(status_code=422, detail={"coords": "Invalid coordinates: {}".format(coords)}) + raise HTTPException( + status_code=422, detail={"coords": f"Unexpected error occurred during wkt parsing: {coords}"} + ) return await get_data_area(poly.wkt, parameter_name, datetime, f) @@ -121,8 +128,14 @@ async def get_data_area( try: poly = wkt.loads(coords) assert poly.geom_type == "Polygon" + except GEOSException: + raise HTTPException(status_code=422, detail={"coords": f"Invalid or unparseable wkt provided: {coords}"}) + except AssertionError: + raise HTTPException(status_code=422, detail={"coords": f"Invalid geometric type: {poly.geom_type}"}) except Exception: - raise HTTPException(status_code=422, detail={"coords": "Invalid coordinates: {}".format(coords)}) + raise HTTPException( + status_code=422, detail={"coords": f"Unexpected error occurred during wkt parsing: {coords}"} + ) range = get_datetime_range(datetime) get_obs_request = dstore.GetObsRequest( diff --git a/api/test/test_edr.py b/api/test/test_edr.py index 3f3fe888..facd1baa 100644 --- a/api/test/test_edr.py +++ b/api/test/test_edr.py @@ -118,12 +118,16 @@ def test_get_area_with_incorrect_coords(): response = client.get("/collections/observations/area?coords=POLYGON((22.12 59.86, 24.39 60.41))") assert response.status_code == 422 + assert response.json() == { + "detail": {"coords": "Invalid or unparseable wkt provided: POLYGON((22.12 59.86, 24.39 60.41))"} + } def test_get_area_with_incorrect_geometry_type(): response = client.get("/collections/observations/area?coords=POINT(22.12 59.86)") assert response.status_code == 422 + assert response.json() == {"detail": {"coords": "Invalid geometric type: Point"}} def test_get_position_with_normal_query(): @@ -158,7 +162,7 @@ def test_get_position_with_incorrect_coords(): response = client.get("/collections/observations/position?coords=POINT(60.41)") assert response.status_code == 422 - assert response.json() == {"detail": {"coords": "Invalid coordinates: POINT(60.41)"}} + assert response.json() == {"detail": {"coords": "Invalid or unparseable wkt provided: POINT(60.41)"}} def test_get_position_with_incorrect_geometry_type(): @@ -168,11 +172,7 @@ def test_get_position_with_incorrect_geometry_type(): ) assert response.status_code == 422 - assert response.json() == { - "detail": { - "coords": "Invalid coordinates: POLYGON((22.12 59.86, 24.39 60.41, 24.39 60.41, 24.39 59.86, 22.12 59.86))" - } - } + assert response.json() == {"detail": {"coords": "Invalid geometric type: Polygon"}} def create_mock_obs_response(json_data): From 1ac4ad6138263bf82b8ca83b91571c18145611b4 Mon Sep 17 00:00:00 2001 From: jukkap Date: Fri, 9 Feb 2024 09:42:24 +0200 Subject: [PATCH 11/21] Refactor: Change assertions to check for single values --- api/test/test_edr.py | 33 +++++++++++++++------------------ 1 file changed, 15 insertions(+), 18 deletions(-) diff --git a/api/test/test_edr.py b/api/test/test_edr.py index facd1baa..9f6ad9ef 100644 --- a/api/test/test_edr.py +++ b/api/test/test_edr.py @@ -25,14 +25,12 @@ def test_get_locations_id_with_single_parameter_query_without_format(): # Check that getObsRequest gets called with correct arguments given in query mock_getObsRequest.assert_called_once() - assert "ff" in mock_getObsRequest.call_args[0][0].filter["instrument"].values - assert "06260" in mock_getObsRequest.call_args[0][0].filter["platform"].values - assert "2022-12-31 00:00:00" == mock_getObsRequest.call_args[0][ - 0 - ].temporal_interval.start.ToDatetime().strftime("%Y-%m-%d %H:%M:%S") - assert "2022-12-31 01:00:01" == mock_getObsRequest.call_args[0][0].temporal_interval.end.ToDatetime().strftime( - "%Y-%m-%d %H:%M:%S" - ) + m_args = mock_getObsRequest.call_args[0][0] + + assert {"ff"} == set(m_args.filter["instrument"].values) + assert {"06260"} == set(m_args.filter["platform"].values) + assert "2022-12-31 00:00:00" == m_args.temporal_interval.start.ToDatetime().strftime("%Y-%m-%d %H:%M:%S") + assert "2022-12-31 01:00:01" == m_args.temporal_interval.end.ToDatetime().strftime("%Y-%m-%d %H:%M:%S") assert response.status_code == 200 assert response.json()["type"] == "Coverage" @@ -51,8 +49,9 @@ def test_get_locations_id_without_parameter_names_query(): # Check that getObsRequest gets called with correct arguments when no parameter names are given # in query mock_getObsRequest.assert_called_once() - assert "*" in mock_getObsRequest.call_args[0][0].filter["instrument"].values + m_args = mock_getObsRequest.call_args[0][0] + assert {"*"} == set(m_args.filter["instrument"].values) assert response.status_code == 200 assert response.json() == compare_data @@ -100,15 +99,13 @@ def test_get_area_with_normal_query(): # Check that getObsRequest gets called with correct arguments given in query mock_getObsRequest.assert_called_once() - assert "TA_P1D_AVG" in mock_getObsRequest.call_args[0][0].filter["instrument"].values - assert len(mock_getObsRequest.call_args[0][0].spatial_area.points) == 5 - assert 22.12 == mock_getObsRequest.call_args[0][0].spatial_area.points[0].lon - assert "2022-12-31 00:00:00" == mock_getObsRequest.call_args[0][ - 0 - ].temporal_interval.start.ToDatetime().strftime("%Y-%m-%d %H:%M:%S") - assert "2022-12-31 01:00:01" == mock_getObsRequest.call_args[0][0].temporal_interval.end.ToDatetime().strftime( - "%Y-%m-%d %H:%M:%S" - ) + m_args = mock_getObsRequest.call_args[0][0] + + assert {"TA_P1D_AVG"} == set(m_args.filter["instrument"].values) + assert len(m_args.spatial_area.points) == 5 + assert 22.12 == m_args.spatial_area.points[0].lon + assert "2022-12-31 00:00:00" == m_args.temporal_interval.start.ToDatetime().strftime("%Y-%m-%d %H:%M:%S") + assert "2022-12-31 01:00:01" == m_args.temporal_interval.end.ToDatetime().strftime("%Y-%m-%d %H:%M:%S") assert response.status_code == 200 assert response.json() == compare_data From b34ed81668c048efaf6160a9fa8df2daf8eb1597 Mon Sep 17 00:00:00 2001 From: jukkap Date: Fri, 9 Feb 2024 09:46:20 +0200 Subject: [PATCH 12/21] Refactor: add utilities.py to tests and remove duplicate functions --- api/test/__init__.py | 0 api/test/test_covjson.py | 15 ++------------- api/test/test_edr.py | 16 ++-------------- api/test/utilities.py | 15 +++++++++++++++ 4 files changed, 19 insertions(+), 27 deletions(-) create mode 100644 api/test/__init__.py create mode 100644 api/test/utilities.py diff --git a/api/test/__init__.py b/api/test/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/api/test/test_covjson.py b/api/test/test_covjson.py index abb44bda..df6eb6ca 100644 --- a/api/test/test_covjson.py +++ b/api/test/test_covjson.py @@ -1,12 +1,12 @@ import json -import datastore_pb2 as dstore import pytest from covjson_pydantic.coverage import Coverage from covjson_pydantic.coverage import CoverageCollection from fastapi import HTTPException from formatters.covjson import Covjson -from google.protobuf.json_format import Parse +from test.utilities import create_mock_obs_response +from test.utilities import load_json def test_single_parameter_convert(): @@ -100,14 +100,3 @@ def test_empty_response_convert(): assert exception_info.value.detail == "No data found" assert exception_info.value.status_code == 404 - - -def create_mock_obs_response(json_data): - response = dstore.GetObsResponse() - Parse(json.dumps(json_data), response) - return response - - -def load_json(file_path): - with open(file_path, "r") as file: - return json.load(file) diff --git a/api/test/test_edr.py b/api/test/test_edr.py index 9f6ad9ef..b3462456 100644 --- a/api/test/test_edr.py +++ b/api/test/test_edr.py @@ -1,11 +1,10 @@ -import json from unittest.mock import patch -import datastore_pb2 as dstore import routers.edr as edr from fastapi.testclient import TestClient -from google.protobuf.json_format import Parse from main import app +from test.utilities import create_mock_obs_response +from test.utilities import load_json client = TestClient(app) @@ -170,14 +169,3 @@ def test_get_position_with_incorrect_geometry_type(): assert response.status_code == 422 assert response.json() == {"detail": {"coords": "Invalid geometric type: Polygon"}} - - -def create_mock_obs_response(json_data): - response = dstore.GetObsResponse() - Parse(json.dumps(json_data), response) - return response - - -def load_json(file_path): - with open(file_path, "r") as file: - return json.load(file) diff --git a/api/test/utilities.py b/api/test/utilities.py new file mode 100644 index 00000000..117bb8c5 --- /dev/null +++ b/api/test/utilities.py @@ -0,0 +1,15 @@ +import json + +import datastore_pb2 as dstore +from google.protobuf.json_format import Parse + + +def create_mock_obs_response(json_data): + response = dstore.GetObsResponse() + Parse(json.dumps(json_data), response) + return response + + +def load_json(file_path): + with open(file_path, "r") as file: + return json.load(file) From b732cdcc1420137f7882a7c52b10e702eef9781d Mon Sep 17 00:00:00 2001 From: jukkap Date: Fri, 9 Feb 2024 09:56:13 +0200 Subject: [PATCH 13/21] Refactor: Raise and except TypeErrors instead of assertions with incorrect geom_types --- api/routers/edr.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/api/routers/edr.py b/api/routers/edr.py index 6724b7af..a3920c3f 100644 --- a/api/routers/edr.py +++ b/api/routers/edr.py @@ -99,11 +99,12 @@ async def get_data_position( ): try: point = wkt.loads(coords) - assert point.geom_type == "Point" + if point.geom_type != "Point": + raise TypeError poly = buffer(point, 0.0001, quad_segs=1) # Roughly 10 meters around the point except GEOSException: raise HTTPException(status_code=422, detail={"coords": f"Invalid or unparseable wkt provided: {coords}"}) - except AssertionError: + except TypeError: raise HTTPException(status_code=422, detail={"coords": f"Invalid geometric type: {point.geom_type}"}) except Exception: raise HTTPException( @@ -127,10 +128,11 @@ async def get_data_area( ): try: poly = wkt.loads(coords) - assert poly.geom_type == "Polygon" + if poly.geom_type != "Polygon": + raise TypeError except GEOSException: raise HTTPException(status_code=422, detail={"coords": f"Invalid or unparseable wkt provided: {coords}"}) - except AssertionError: + except TypeError: raise HTTPException(status_code=422, detail={"coords": f"Invalid geometric type: {poly.geom_type}"}) except Exception: raise HTTPException( From f1b1986323ad0b76c2eac6103f256ad90e201a91 Mon Sep 17 00:00:00 2001 From: jukkap Date: Fri, 9 Feb 2024 10:44:46 +0200 Subject: [PATCH 14/21] Refactor: Use Annotated instead of providing default with Query --- api/routers/edr.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/api/routers/edr.py b/api/routers/edr.py index a3920c3f..0afde484 100644 --- a/api/routers/edr.py +++ b/api/routers/edr.py @@ -1,4 +1,6 @@ # For developing: uvicorn main:app --reload +from typing import Annotated + import datastore_pb2 as dstore import formatters from covjson_pydantic.coverage import Coverage @@ -65,8 +67,8 @@ async def get_locations(bbox: str = Query(..., example="5.0,52.0,6.0,52.1")) -> ) async def get_data_location_id( location_id: str = Path(..., example="06260"), - parameter_name: str = Query(None, alias="parameter-name", example="dd,ff,rh,pp,tn"), - datetime: str = Query(None, example="2022-12-31T00:00Z/2023-01-01T00:00Z"), + parameter_name: Annotated[str | None, Query(alias="parameter-name", example="dd,ff,rh,pp,tn")] = None, + datetime: Annotated[str | None, Query(alias="datetime", example="2022-12-31T00:00Z/2023-01-01T00:00Z")] = None, f: str = Query(default="covjson", alias="f", description="Specify return format."), ): # TODO: There is no error handling of any kind at the moment! @@ -93,8 +95,8 @@ async def get_data_location_id( ) async def get_data_position( coords: str = Query(..., example="POINT(5.179705 52.0988218)"), - parameter_name: str = Query(None, alias="parameter-name", example="dd,ff,rh,pp,tn"), - datetime: str = Query(None, example="2022-12-31T00:00Z/2023-01-01T00:00Z"), + parameter_name: Annotated[str | None, Query(alias="parameter-name", example="dd,ff,rh,pp,tn")] = None, + datetime: Annotated[str | None, Query(alias="datetime", example="2022-12-31T00:00Z/2023-01-01T00:00Z")] = None, f: str = Query(default="covjson", alias="f", description="Specify return format."), ): try: @@ -122,8 +124,8 @@ async def get_data_position( ) async def get_data_area( coords: str = Query(..., example="POLYGON((5.0 52.0, 6.0 52.0,6.0 52.1,5.0 52.1, 5.0 52.0))"), - parameter_name: str = Query(None, alias="parameter-name", example="dd,ff,rh,pp,tn"), - datetime: str = Query(None, example="2022-12-31T00:00Z/2023-01-01T00:00Z"), + parameter_name: Annotated[str | None, Query(alias="parameter-name", example="dd,ff,rh,pp,tn")] = None, + datetime: Annotated[str | None, Query(alias="datetime", example={"2022-12-31T00:00Z/2023-01-01T00:00Z"})] = None, f: str = Query(default="covjson", alias="f", description="Specify return format."), ): try: From f700ccb64c55286be9ef60a7aba6bcb612a0ce51 Mon Sep 17 00:00:00 2001 From: jukkap Date: Fri, 9 Feb 2024 10:47:22 +0200 Subject: [PATCH 15/21] Fix: Return 400 instead of 422 as per EDR specification --- api/dependencies.py | 2 +- api/routers/edr.py | 12 ++++++------ api/test/test_edr.py | 12 ++++++------ 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/api/dependencies.py b/api/dependencies.py index b3e5b44e..ff15b716 100644 --- a/api/dependencies.py +++ b/api/dependencies.py @@ -44,6 +44,6 @@ def get_datetime_range(datetime_string: str | None) -> Tuple[Timestamp, Timestam errors["datetime"] = f"Invalid format: {datetime_string}" if errors: - raise HTTPException(status_code=422, detail=errors) + raise HTTPException(status_code=400, detail=errors) return start_datetime, end_datetime diff --git a/api/routers/edr.py b/api/routers/edr.py index 0afde484..18c64eeb 100644 --- a/api/routers/edr.py +++ b/api/routers/edr.py @@ -105,12 +105,12 @@ async def get_data_position( raise TypeError poly = buffer(point, 0.0001, quad_segs=1) # Roughly 10 meters around the point except GEOSException: - raise HTTPException(status_code=422, detail={"coords": f"Invalid or unparseable wkt provided: {coords}"}) + raise HTTPException(status_code=400, detail={"coords": f"Invalid or unparseable wkt provided: {coords}"}) except TypeError: - raise HTTPException(status_code=422, detail={"coords": f"Invalid geometric type: {point.geom_type}"}) + raise HTTPException(status_code=400, detail={"coords": f"Invalid geometric type: {point.geom_type}"}) except Exception: raise HTTPException( - status_code=422, detail={"coords": f"Unexpected error occurred during wkt parsing: {coords}"} + status_code=400, detail={"coords": f"Unexpected error occurred during wkt parsing: {coords}"} ) return await get_data_area(poly.wkt, parameter_name, datetime, f) @@ -133,12 +133,12 @@ async def get_data_area( if poly.geom_type != "Polygon": raise TypeError except GEOSException: - raise HTTPException(status_code=422, detail={"coords": f"Invalid or unparseable wkt provided: {coords}"}) + raise HTTPException(status_code=400, detail={"coords": f"Invalid or unparseable wkt provided: {coords}"}) except TypeError: - raise HTTPException(status_code=422, detail={"coords": f"Invalid geometric type: {poly.geom_type}"}) + raise HTTPException(status_code=400, detail={"coords": f"Invalid geometric type: {poly.geom_type}"}) except Exception: raise HTTPException( - status_code=422, detail={"coords": f"Unexpected error occurred during wkt parsing: {coords}"} + status_code=400, detail={"coords": f"Unexpected error occurred during wkt parsing: {coords}"} ) range = get_datetime_range(datetime) diff --git a/api/test/test_edr.py b/api/test/test_edr.py index b3462456..6490213a 100644 --- a/api/test/test_edr.py +++ b/api/test/test_edr.py @@ -58,7 +58,7 @@ def test_get_locations_id_without_parameter_names_query(): def test_get_locations_id_with_incorrect_datetime_format(): response = client.get("/collections/observations/locations/06260?datetime=20221231T000000Z/20221231T010000Z") - assert response.status_code == 422 + assert response.status_code == 400 assert response.json() == {"detail": {"datetime": "Invalid format: 20221231T000000Z/20221231T010000Z"}} @@ -67,7 +67,7 @@ def test_get_locations_id_with_incorrect_datetime_range(): "/collections/observations/locations/06260?datetime=2024-12-31T00:00:00Z/2022-12-31T01:00:00Z" ) - assert response.status_code == 422 + assert response.status_code == 400 assert response.json() == {"detail": {"datetime": "Invalid range: 2024-12-31T00:00:00Z > 2022-12-31T01:00:00Z"}} @@ -113,7 +113,7 @@ def test_get_area_with_normal_query(): def test_get_area_with_incorrect_coords(): response = client.get("/collections/observations/area?coords=POLYGON((22.12 59.86, 24.39 60.41))") - assert response.status_code == 422 + assert response.status_code == 400 assert response.json() == { "detail": {"coords": "Invalid or unparseable wkt provided: POLYGON((22.12 59.86, 24.39 60.41))"} } @@ -122,7 +122,7 @@ def test_get_area_with_incorrect_coords(): def test_get_area_with_incorrect_geometry_type(): response = client.get("/collections/observations/area?coords=POINT(22.12 59.86)") - assert response.status_code == 422 + assert response.status_code == 400 assert response.json() == {"detail": {"coords": "Invalid geometric type: Point"}} @@ -157,7 +157,7 @@ def test_get_position_with_normal_query(): def test_get_position_with_incorrect_coords(): response = client.get("/collections/observations/position?coords=POINT(60.41)") - assert response.status_code == 422 + assert response.status_code == 400 assert response.json() == {"detail": {"coords": "Invalid or unparseable wkt provided: POINT(60.41)"}} @@ -167,5 +167,5 @@ def test_get_position_with_incorrect_geometry_type(): "24.39 60.41, 24.39 59.86, 22.12 59.86))" ) - assert response.status_code == 422 + assert response.status_code == 400 assert response.json() == {"detail": {"coords": "Invalid geometric type: Polygon"}} From 36e1a0e93a46b2f9a619f2f29a9eb16278107318 Mon Sep 17 00:00:00 2001 From: jukkap Date: Fri, 9 Feb 2024 11:15:15 +0200 Subject: [PATCH 16/21] Refactor: leave instrument out from obs_request when not needed instead of wildcard --- api/routers/edr.py | 12 +++++++++--- api/test/test_edr.py | 3 ++- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/api/routers/edr.py b/api/routers/edr.py index 18c64eeb..00c99795 100644 --- a/api/routers/edr.py +++ b/api/routers/edr.py @@ -77,8 +77,10 @@ async def get_data_location_id( get_obs_request = dstore.GetObsRequest( filter=dict( platform=dstore.Strings(values=[location_id]), - instrument=dstore.Strings( - values=list(map(str.strip, parameter_name.split(","))) if parameter_name else "*" + **( + {"instrument": dstore.Strings(values=list(map(str.strip, parameter_name.split(","))))} + if parameter_name + else {} ), ), temporal_interval=dstore.TimeInterval(start=range[0], end=range[1]) if range else None, @@ -144,7 +146,11 @@ async def get_data_area( range = get_datetime_range(datetime) get_obs_request = dstore.GetObsRequest( filter=dict( - instrument=dstore.Strings(values=list(map(str.strip, parameter_name.split(","))) if parameter_name else "*") + **( + {"instrument": dstore.Strings(values=list(map(str.strip, parameter_name.split(","))))} + if parameter_name + else {} + ), ), spatial_area=dstore.Polygon( points=[dstore.Point(lat=coord[1], lon=coord[0]) for coord in poly.exterior.coords] diff --git a/api/test/test_edr.py b/api/test/test_edr.py index 6490213a..1d557d8f 100644 --- a/api/test/test_edr.py +++ b/api/test/test_edr.py @@ -50,7 +50,8 @@ def test_get_locations_id_without_parameter_names_query(): mock_getObsRequest.assert_called_once() m_args = mock_getObsRequest.call_args[0][0] - assert {"*"} == set(m_args.filter["instrument"].values) + assert "instrument" not in m_args.filter + assert {"06260"} == set(m_args.filter["platform"].values) assert response.status_code == 200 assert response.json() == compare_data From e71f04980a2e7e28dd9a309014f04ecc267c7034 Mon Sep 17 00:00:00 2001 From: jukkap Date: Fri, 9 Feb 2024 11:24:52 +0200 Subject: [PATCH 17/21] Test: Exclude test from coverage --- .github/workflows/ci.yml | 2 +- api/test/.coveragerc | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) create mode 100644 api/test/.coveragerc diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index dcbcfa3b..820c8ee0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -141,7 +141,7 @@ jobs: - name: Run Tests run: | cd api - python -m pytest --timeout=60 --junitxml=pytest.xml --cov-report=term-missing --cov=. | tee pytest-coverage.txt + python -m pytest --timeout=60 --junitxml=pytest.xml --cov-report=term-missing --cov=. --cov-config=test/.coveragerc | tee pytest-coverage.txt - name: Comment coverage uses: MishaKav/pytest-coverage-comment@main diff --git a/api/test/.coveragerc b/api/test/.coveragerc new file mode 100644 index 00000000..d057a8b5 --- /dev/null +++ b/api/test/.coveragerc @@ -0,0 +1,2 @@ +[run] +omit = test/* From caddad2e3b18a13f74fc991225f64bda1bb72cc4 Mon Sep 17 00:00:00 2001 From: jukkap Date: Fri, 9 Feb 2024 15:35:08 +0200 Subject: [PATCH 18/21] Refactor: simplify get_obs_request filter creation --- api/routers/edr.py | 25 +++++++++---------------- 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/api/routers/edr.py b/api/routers/edr.py index 00c99795..4e4c31cc 100644 --- a/api/routers/edr.py +++ b/api/routers/edr.py @@ -74,15 +74,11 @@ async def get_data_location_id( # TODO: There is no error handling of any kind at the moment! # This is just a quick and dirty demo range = get_datetime_range(datetime) + filter = dict(platform=dstore.Strings(values=[location_id])) + if parameter_name: + filter["instrument"] = dstore.Strings(values=list(map(str.strip, parameter_name.split(",")))) get_obs_request = dstore.GetObsRequest( - filter=dict( - platform=dstore.Strings(values=[location_id]), - **( - {"instrument": dstore.Strings(values=list(map(str.strip, parameter_name.split(","))))} - if parameter_name - else {} - ), - ), + filter=filter, temporal_interval=dstore.TimeInterval(start=range[0], end=range[1]) if range else None, ) response = await getObsRequest(get_obs_request) @@ -98,7 +94,7 @@ async def get_data_location_id( async def get_data_position( coords: str = Query(..., example="POINT(5.179705 52.0988218)"), parameter_name: Annotated[str | None, Query(alias="parameter-name", example="dd,ff,rh,pp,tn")] = None, - datetime: Annotated[str | None, Query(alias="datetime", example="2022-12-31T00:00Z/2023-01-01T00:00Z")] = None, + datetime: Annotated[str | None, Query(example="2022-12-31T00:00Z/2023-01-01T00:00Z")] = None, f: str = Query(default="covjson", alias="f", description="Specify return format."), ): try: @@ -144,14 +140,11 @@ async def get_data_area( ) range = get_datetime_range(datetime) + filter = {} + if parameter_name: + filter = dict(instrument=dstore.Strings(values=list(map(str.strip, parameter_name.split(","))))) get_obs_request = dstore.GetObsRequest( - filter=dict( - **( - {"instrument": dstore.Strings(values=list(map(str.strip, parameter_name.split(","))))} - if parameter_name - else {} - ), - ), + filter=filter, spatial_area=dstore.Polygon( points=[dstore.Point(lat=coord[1], lon=coord[0]) for coord in poly.exterior.coords] ), From 699ade2913e0d653ab8f567b2c4ab70b05809e31 Mon Sep 17 00:00:00 2001 From: jukkap Date: Fri, 9 Feb 2024 15:36:59 +0200 Subject: [PATCH 19/21] Test: add a test for area endpoint without query parameters --- api/test/test_edr.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/api/test/test_edr.py b/api/test/test_edr.py index 1d557d8f..cac854f4 100644 --- a/api/test/test_edr.py +++ b/api/test/test_edr.py @@ -111,6 +111,26 @@ def test_get_area_with_normal_query(): assert response.json() == compare_data +def test_get_area_with_without_parameter_names_query(): + with patch("routers.edr.getObsRequest") as mock_getObsRequest: + test_data = load_json("test/test_data/test_coverages_proto.json") + compare_data = load_json("test/test_data/test_coverages_covjson.json") + + mock_getObsRequest.return_value = create_mock_obs_response(test_data) + + response = client.get( + "/collections/observations/area?coords=POLYGON((22.12 59.86, 24.39 60.41, " + "24.39 60.41, 24.39 59.86, 22.12 59.86))" + ) + + mock_getObsRequest.assert_called_once() + m_args = mock_getObsRequest.call_args[0][0] + + assert "instrument" not in m_args.filter + response.status_code == 200 + response.json() == compare_data + + def test_get_area_with_incorrect_coords(): response = client.get("/collections/observations/area?coords=POLYGON((22.12 59.86, 24.39 60.41))") From f6787791d2654b40dbe37798c10f0fd85358e49c Mon Sep 17 00:00:00 2001 From: jukkap Date: Fri, 9 Feb 2024 16:00:12 +0200 Subject: [PATCH 20/21] Refactor: use Annotated with endpoints for information & validation --- api/routers/edr.py | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/api/routers/edr.py b/api/routers/edr.py index 4e4c31cc..405bb9cd 100644 --- a/api/routers/edr.py +++ b/api/routers/edr.py @@ -32,7 +32,9 @@ ) # We can currently only query data, even if we only need metadata like for this endpoint # Maybe it would be better to only query a limited set of data instead of everything (meaning 24 hours) -async def get_locations(bbox: str = Query(..., example="5.0,52.0,6.0,52.1")) -> FeatureCollection: # Hack to use string +async def get_locations( + bbox: Annotated[str, Query(example="5.0,52.0,6.0,52.1")] +) -> FeatureCollection: # Hack to use string left, bottom, right, top = map(str.strip, bbox.split(",")) print("bbox: {}".format(bbox)) poly = geometry.Polygon([(left, bottom), (right, bottom), (right, top), (left, top)]) @@ -66,10 +68,10 @@ async def get_locations(bbox: str = Query(..., example="5.0,52.0,6.0,52.1")) -> response_model_exclude_none=True, ) async def get_data_location_id( - location_id: str = Path(..., example="06260"), + location_id: Annotated[str, Path(example="06260")], parameter_name: Annotated[str | None, Query(alias="parameter-name", example="dd,ff,rh,pp,tn")] = None, - datetime: Annotated[str | None, Query(alias="datetime", example="2022-12-31T00:00Z/2023-01-01T00:00Z")] = None, - f: str = Query(default="covjson", alias="f", description="Specify return format."), + datetime: Annotated[str | None, Query(example="2022-12-31T00:00Z/2023-01-01T00:00Z")] = None, + f: Annotated[str, Query(description="Specify return format.")] = "covjson", ): # TODO: There is no error handling of any kind at the moment! # This is just a quick and dirty demo @@ -92,10 +94,10 @@ async def get_data_location_id( response_model_exclude_none=True, ) async def get_data_position( - coords: str = Query(..., example="POINT(5.179705 52.0988218)"), + coords: Annotated[str, Query(example="POINT(5.179705 52.0988218)")], parameter_name: Annotated[str | None, Query(alias="parameter-name", example="dd,ff,rh,pp,tn")] = None, datetime: Annotated[str | None, Query(example="2022-12-31T00:00Z/2023-01-01T00:00Z")] = None, - f: str = Query(default="covjson", alias="f", description="Specify return format."), + f: Annotated[str, Query(description="Specify return format.")] = "covjson", ): try: point = wkt.loads(coords) @@ -121,10 +123,10 @@ async def get_data_position( response_model_exclude_none=True, ) async def get_data_area( - coords: str = Query(..., example="POLYGON((5.0 52.0, 6.0 52.0,6.0 52.1,5.0 52.1, 5.0 52.0))"), + coords: Annotated[str, Query(example="POLYGON((5.0 52.0, 6.0 52.0,6.0 52.1,5.0 52.1, 5.0 52.0))")], parameter_name: Annotated[str | None, Query(alias="parameter-name", example="dd,ff,rh,pp,tn")] = None, - datetime: Annotated[str | None, Query(alias="datetime", example={"2022-12-31T00:00Z/2023-01-01T00:00Z"})] = None, - f: str = Query(default="covjson", alias="f", description="Specify return format."), + datetime: Annotated[str | None, Query(example="2022-12-31T00:00Z/2023-01-01T00:00Z")] = None, + f: Annotated[str, Query(description="Specify return format.")] = "covjson", ): try: poly = wkt.loads(coords) From b20fd8d028eb704b01225ced6d56b4b58426ffbb Mon Sep 17 00:00:00 2001 From: Jukka Pelli <132542798+fjugipe@users.noreply.github.com> Date: Mon, 12 Feb 2024 08:07:27 +0200 Subject: [PATCH 21/21] Style: refactor area filter for consistency Co-authored-by: Lukas Phaf --- api/routers/edr.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/routers/edr.py b/api/routers/edr.py index 405bb9cd..910d3b0c 100644 --- a/api/routers/edr.py +++ b/api/routers/edr.py @@ -144,7 +144,7 @@ async def get_data_area( range = get_datetime_range(datetime) filter = {} if parameter_name: - filter = dict(instrument=dstore.Strings(values=list(map(str.strip, parameter_name.split(","))))) + filter["instrument"] = dstore.Strings(values=list(map(str.strip, parameter_name.split(",")))) get_obs_request = dstore.GetObsRequest( filter=filter, spatial_area=dstore.Polygon(