diff --git a/CHANGELOG.md b/CHANGELOG.md index 24d85ea9..3e5c948d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,7 @@ and start a new "In Progress" section above it. - Include `job_options` as top-level properties in `GET /jobs/{job_id}` response ([#470](https://github.com/Open-EO/openeo-python-driver/issues/470)) - Bump STAC version from `0.9.0` to `1.0.0` in capabilities endpoint, collection metadata, job results and ML model metadata ([#363](https://github.com/Open-EO/openeo-python-driver/issues/363)) +- `load_collection`: add check on malformed `spatial_extent`. ([#284](https://github.com/Open-EO/openeo-python-driver/issues/284)) ## 0.139.0 diff --git a/openeo_driver/processgraph/process_implementations/io.py b/openeo_driver/processgraph/process_implementations/io.py index 82092f3a..66756704 100644 --- a/openeo_driver/processgraph/process_implementations/io.py +++ b/openeo_driver/processgraph/process_implementations/io.py @@ -61,7 +61,7 @@ def _extract_temporal_extent(args: ProcessArgs, field="extent", process_id="filt def _extract_bbox_extent(args: ProcessArgs, field="extent", process_id="filter_bbox", handle_geojson=False) -> dict: - extent = args.get_required(name=field) + extent = args.get_required(name=field, expected_type=dict) if handle_geojson and extent.get("type") in [ "Polygon", "MultiPolygon", "GeometryCollection", "Feature", "FeatureCollection", ]: @@ -79,8 +79,44 @@ def _extract_bbox_extent(args: ProcessArgs, field="extent", process_id="filter_b ) d = {"west": w, "south": s, "east": e, "north": n, "crs": "EPSG:4326"} elif all(k in extent for k in ["west", "south", "east", "north"]): - d = {k: extent[k] for k in ["west", "south", "east", "north"]} - crs = extent.get("crs") or "EPSG:4326" + d = {} + for key in ["west", "south", "east", "north"]: + value = extent[key] + if isinstance(value, bool) or not isinstance(value, (int, float)): + raise ProcessParameterInvalidException( + parameter=field, + process=process_id, + reason=f"'{key}' must be a number, but got {value!r}.", + ) + d[key] = value + + if d["west"] >= d["east"]: + raise ProcessParameterInvalidException( + parameter=field, + process=process_id, + reason=f"'west' must be smaller than 'east', but got west={d['west']!r} and east={d['east']!r}.", + ) + if d["south"] >= d["north"]: + raise ProcessParameterInvalidException( + parameter=field, + process=process_id, + reason=f"'south' must be smaller than 'north', but got south={d['south']!r} and north={d['north']!r}.", + ) + + crs = extent.get("crs") + if crs is None and ( + d["west"] < -360 or d["west"] > 360 or d["east"] < -360 or d["east"] > 360 + or d["south"] < -100 or d["south"] > 100 or d["north"] < -100 or d["north"] > 100 + ): + raise ProcessParameterInvalidException( + parameter=field, + process=process_id, + reason=( + "coordinates are outside the valid EPSG:4326 range while no 'crs' was specified. " + "Specify the correct 'crs' explicitly." + ), + ) + crs = crs or "EPSG:4326" if isinstance(crs, int): crs = "EPSG:{crs}".format(crs=crs) d["crs"] = crs diff --git a/tests/test_views_validation.py b/tests/test_views_validation.py index a0000f91..46e4c354 100644 --- a/tests/test_views_validation.py +++ b/tests/test_views_validation.py @@ -49,3 +49,65 @@ def test_load_collection_basic(api100, backend_implementation): res = api100.validation(pg) errors = res.json["errors"] assert errors == [{"code": "MissingProduct", "message": "Tile 4322 not available"}] + + +@pytest.mark.parametrize( + ["spatial_extent", "expected_message_part"], + [ + ([1, 2, 3, 4], "Expected dictionary/mapping but got"), + ( + {"west": [0], "south": 60.11, "east": 25.24, "north": 60.35}, + "'west' must be a number, but got [0].", + ), + ( + {"west": 5, "south": 51.215, "east": 4, "north": 51.22}, + "'west' must be smaller than 'east'", + ), + ( + {"west": 4, "south": 51.22, "east": 5, "north": 51.215}, + "'south' must be smaller than 'north'", + ), + ( + { + "west": 4329317.717132108, + "east": 4330615.2810456185, + "north": 3005295.0854642093, + "south": 3003997.791438847, + }, + "outside the valid EPSG:4326 range while no 'crs' was specified", + ), + ], +) +def test_validation_load_collection_invalid_spatial_extent(api100, spatial_extent, expected_message_part): + pg = { + "lc": { + "process_id": "load_collection", + "arguments": {"id": "S2_FOOBAR", "spatial_extent": spatial_extent}, + "result": True, + } + } + res = api100.validation(pg) + errors = res.json["errors"] + assert len(errors) == 1 + assert errors[0]["code"] == "ProcessParameterInvalid" + assert expected_message_part in errors[0]["message"] + + +@pytest.mark.parametrize( + "spatial_extent", + [ + {"west": -200, "south": 51.215, "east": -190, "north": 51.22}, + {"west": 4, "south": 95, "east": 5, "north": 96}, + ], +) +def test_validation_load_collection_spatial_extent_lenient_epsg4326_default_bounds(api100, spatial_extent): + pg = { + "lc": { + "process_id": "load_collection", + "arguments": {"id": "S2_FOOBAR", "spatial_extent": spatial_extent}, + "result": True, + } + } + res = api100.validation(pg) + errors = res.json["errors"] + assert all(error["code"] != "ProcessParameterInvalid" for error in errors)