feat(spp_api_v2_gis): promote to Beta with validation and exception fixes#147
feat(spp_api_v2_gis): promote to Beta with validation and exception fixes#147
Conversation
…ixes Promote spp_api_v2_gis from Alpha to Beta after code review. - Add GeoJSON geometry validation to spatial query and geofence schemas (type must be Polygon/MultiPolygon, coordinates must be non-empty) - Type geofence_type as Literal enum instead of open string - Fix exception chaining: use `from e` instead of `from None` in proximity, statistics, and spatial_query routers for better debugging - Add SPP module icon - Bump version 19.0.2.0.0 -> 19.0.2.0.1 - Update HISTORY.md
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## 19.0 #147 +/- ##
==========================================
- Coverage 71.06% 71.06% -0.01%
==========================================
Files 925 925
Lines 54704 54731 +27
==========================================
+ Hits 38876 38895 +19
- Misses 15828 15836 +8
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request promotes the spp_api_v2_gis module to Beta status (v19.0.2.0.1), removing alpha-related warnings and updating documentation. Key improvements include the implementation of GeoJSON geometry validation for spatial queries and geofences, the addition of a module icon, and the use of proper exception chaining in routers for better debugging. Feedback suggests refining the error message formatting for invalid geometry types and enhancing coordinate validation to verify the structural nesting required for Polygon and MultiPolygon types.
| if geo_type not in _VALID_GEOMETRY_TYPES: | ||
| raise ValueError(f"geometry type must be one of {_VALID_GEOMETRY_TYPES}, got '{geo_type}'") |
There was a problem hiding this comment.
The error message for invalid geometry types uses the default string representation of a Python set, which can look a bit unpolished (e.g., {'Polygon', 'MultiPolygon'}). It's better to format it as a sorted, comma-separated string for better readability.
| if geo_type not in _VALID_GEOMETRY_TYPES: | |
| raise ValueError(f"geometry type must be one of {_VALID_GEOMETRY_TYPES}, got '{geo_type}'") | |
| if geo_type not in _VALID_GEOMETRY_TYPES: | |
| valid_types = ", ".join(sorted(_VALID_GEOMETRY_TYPES)) | |
| raise ValueError(f"geometry type must be one of {valid_types}, got '{geo_type}'") |
| if not coords or not isinstance(coords, list): | ||
| raise ValueError("geometry must have a non-empty 'coordinates' array") |
There was a problem hiding this comment.
The validation for the coordinates field is quite basic. While it correctly checks for a non-empty list, it doesn't verify that the structure matches what's expected for a Polygon (list of lists of lists) or MultiPolygon (list of lists of lists of lists). While a full GeoJSON validator might be overkill, consider adding a check for minimum nesting depth to catch obviously malformed inputs like [0, 0] for a Polygon.
|
Increase the test coverage by at least 90% |
2 participants
Why is this change needed?
The
spp_api_v2_gismodule has been reviewed for Alpha to Beta promotion. Code review identified two issues: unvalidated GeoJSON geometry inputs in schemas, and suppressed exception chains in routers that make debugging difficult.How was the change implemented?
Schema Validation
_validate_geojson_geometry()validator that checks geometrytypeis Polygon or MultiPolygon andcoordinatesis a non-empty arraySpatialQueryRequest,GeometryItem(batch), andGeofenceCreateRequestgeofence_typeasLiteral["hazard_zone", "service_area", "targeting_area", "custom"]instead of open stringException Chaining
from Nonetofrom ein 5 places across 3 routers (proximity, statistics, spatial_query)Beta Promotion
New unit tests
N/A — existing 191 tests all pass (0 failures)
Unit tests executed by the author
How to test manually
POST /api/v2/spp/oauth/token{"type": "Point", "coordinates": [0,0]}should fail)Related links