Merge pull request #122 from eseglem/feature/bbox-validation

Additional bbox validation.

Author: vincentsarago

geojson_pydantic/features.py

@@ -7,7 +7,7 @@
 
 from geojson_pydantic.geo_interface import GeoInterfaceMixin
 from geojson_pydantic.geometries import Geometry, GeometryCollection
-from geojson_pydantic.types import BBox
+from geojson_pydantic.types import BBox, validate_bbox
 
 Props = TypeVar("Props", bound=Union[Dict[str, Any], BaseModel])
 Geom = TypeVar("Geom", bound=Union[Geometry, GeometryCollection])
@@ -22,33 +22,7 @@ class Feature(GenericModel, Generic[Geom, Props], GeoInterfaceMixin):
     id: Optional[Union[StrictInt, StrictStr]] = None
     bbox: Optional[BBox] = None
 
-    class Config:
-        """Model configuration."""
-
-        use_enum_values = True
-
-    @validator("bbox", pre=True)
-    def check_bbox(cls, bbox: BBox) -> BBox:
-        """Check that bbox is valid."""
-        if bbox is None:
-            return bbox
-
-        if len(bbox) == 6:
-            if bbox[0] > bbox[3] or bbox[1] > bbox[4] or bbox[2] > bbox[5]:  # type: ignore
-                raise ValueError(
-                    "BBox must be in the form [west, south, bottom, east, north, top]"
-                )
-
-        elif len(bbox) == 4:
-            if bbox[0] > bbox[2] or bbox[1] > bbox[3]:
-                raise ValueError("BBox must be in the form [west, south, east, north]")
-
-        else:
-            raise ValueError(
-                "BBox must be in the form [west, south, east, north] or [west, south, bottom, east, north, top]"
-            )
-
-        return bbox
+    _validate_bbox = validator("bbox", allow_reuse=True)(validate_bbox)
 
     @validator("geometry", pre=True, always=True)
     def set_geometry(cls, geometry: Any) -> Any:
@@ -77,3 +51,5 @@ def __len__(self) -> int:
     def __getitem__(self, index: int) -> Feature:
         """get feature at a given index"""
         return self.features[index]
+
+    _validate_bbox = validator("bbox", allow_reuse=True)(validate_bbox)

geojson_pydantic/geometries.py

@@ -19,6 +19,7 @@
     MultiPolygonCoords,
     PolygonCoords,
     Position,
+    validate_bbox,
 )
 
 
@@ -107,6 +108,8 @@ def wkt(self) -> str:
 
         return wkt
 
+    _validate_bbox = validator("bbox", allow_reuse=True)(validate_bbox)
+
 
 class Point(_GeometryBase):
     """Point Model"""
@@ -287,6 +290,8 @@ def wkt(self) -> str:
         z = " Z " if "Z" in geometries else " "
         return f"{self.type.upper()}{z}{geometries}"
 
+    _validate_bbox = validator("bbox", allow_reuse=True)(validate_bbox)
+
     @validator("geometries")
     def check_geometries(cls, geometries: List) -> List:
         """Add warnings for conditions the spec does not explicitly forbid."""

geojson_pydantic/types.py

@@ -1,13 +1,46 @@
 """Types for geojson_pydantic models"""
 
-from typing import TYPE_CHECKING, List, Tuple, Union
+from typing import TYPE_CHECKING, List, Optional, Tuple, TypeVar, Union
 
 from pydantic import conlist
 
+T = TypeVar("T")
+
 BBox = Union[
     Tuple[float, float, float, float],  # 2D bbox
     Tuple[float, float, float, float, float, float],  # 3D bbox
 ]
+
+
+def validate_bbox(bbox: Optional[BBox]) -> Optional[BBox]:
+    """Validate BBox values are ordered correctly."""
+    # If bbox is None, there is nothing to validate.
+    if bbox is None:
+        return None
+
+    # A list to store any errors found so we can raise them all at once.
+    errors: List[str] = []
+
+    # Determine where the second position starts. 2 for 2D, 3 for 3D.
+    offset = len(bbox) // 2
+
+    # Check X
+    if bbox[0] > bbox[offset]:
+        errors.append(f"Min X ({bbox[0]}) must be <= Max X ({bbox[offset]}).")
+    # Check Y
+    if bbox[1] > bbox[1 + offset]:
+        errors.append(f"Min Y ({bbox[1]}) must be <= Max Y ({bbox[1 + offset]}).")
+    # If 3D, check Z values.
+    if offset > 2 and bbox[2] > bbox[2 + offset]:
+        errors.append(f"Min Z ({bbox[2]}) must be <= Max Z ({bbox[2 + offset]}).")
+
+    # Raise any errors found.
+    if errors:
+        raise ValueError("Invalid BBox. Error(s): " + " ".join(errors))
+
+    return bbox
+
+
 Position = Union[Tuple[float, float], Tuple[float, float, float]]
 
 # Coordinate arrays

tests/test_features.py

@@ -232,21 +232,34 @@ def test_feature_validation():
         Feature(type="Feature", properties=None)
 
     assert Feature(
-        type="Feature", properties=None, bbox=[0, 0, 100, 100], geometry=None
+        type="Feature", properties=None, bbox=(0, 0, 100, 100), geometry=None
     )
     assert Feature(
-        type="Feature", properties=None, bbox=[0, 0, 0, 100, 100, 100], geometry=None
+        type="Feature", properties=None, bbox=(0, 0, 0, 100, 100, 100), geometry=None
     )
 
     with pytest.raises(ValidationError):
         # bad bbox2d
-        Feature(type="Feature", properties=None, bbox=[100, 100, 0, 0], geometry=None)
+        Feature(type="Feature", properties=None, bbox=(100, 100, 0, 0), geometry=None)
 
     with pytest.raises(ValidationError):
         # bad bbox3d
         Feature(
             type="Feature",
             properties=None,
-            bbox=[100, 100, 100, 0, 0, 0],
+            bbox=(100, 100, 100, 0, 0, 0),
+            geometry=None,
+        )
+
+
+def test_bbox_validation():
+    # Some attempts at generic validation did not validate the types within
+    # bbox before passing them to the function and resulted in TypeErrors.
+    # This test exists to ensure that doesn't happen in the future.
+    with pytest.raises(ValidationError):
+        Feature(
+            type="Feature",
+            properties=None,
+            bbox=(0, "a", 0, 1, 1, 1),
             geometry=None,
         )