fix: validate bbox input and guard empty geocoder results

rapida/util/bbox_param_type.py

@@ -14,6 +14,10 @@ def buffer_bbox(bbox: tuple[float, float, float, float], meters: float) -> tuple
     Useful for coarse rasters (e.g. NTL ~500m pixels) where a small AOI would otherwise
     contain too few pixels.
 
+    Latitude is clamped to the valid range so a bbox that already touches a pole is not
+    pushed past it. Longitude is left alone because a bbox crossing the antimeridian is
+    a legitimate AOI.
+
     :param bbox: (min_lon, min_lat, max_lon, max_lat) in EPSG:4326
     :param meters: buffer distance in meters; 0/None returns the bbox unchanged
     :return: the enlarged bbox
@@ -24,7 +28,7 @@ def buffer_bbox(bbox: tuple[float, float, float, float], meters: float) -> tuple
     center_lat = (minlat + maxlat) / 2.0
     dlat = meters / 111320.0
     dlon = meters / (111320.0 * max(math.cos(math.radians(center_lat)), 1e-6))
-    return (minlon - dlon, minlat - dlat, maxlon + dlon, maxlat + dlat)
+    return (minlon - dlon, max(-90.0, minlat - dlat), maxlon + dlon, min(90.0, maxlat + dlat))
 
 
 class BboxParamType(click.ParamType):
@@ -41,16 +45,36 @@ def convert(self, value, param, ctx):
                 f"bbox must be 4 floating point numbers separated by commas. Got '{value}'"
             )
 
+        # nan compares False against everything, so this has to come before the range checks
+        if not all(math.isfinite(x) for x in bbox):
+            self.fail(f"bbox must contain finite numbers. Got '{value}'")
+
+        minlon, minlat, maxlon, maxlat = bbox
+
+        if not (-180 <= minlon <= 180 and -180 <= maxlon <= 180):
+            self.fail(f"bbox longitude must be between -180 and 180. Got '{value}'")
+
+        if not (-90 <= minlat <= 90 and -90 <= maxlat <= 90):
+            self.fail(f"bbox latitude must be between -90 and 90. Got '{value}'")
+
+        if minlon >= maxlon:
+            self.fail(f"bbox min longitude must be smaller than max longitude. Got '{value}'")
+
+        if minlat >= maxlat:
+            self.fail(f"bbox min latitude must be smaller than max latitude. Got '{value}'")
+
         return bbox
 
 
-def get_bbox_label(bbox: tuple[float, float, float, float])->str:
+def get_bbox_label(bbox: tuple[float, float, float, float])->dict:
     minlon, minlat, maxlon, maxlat = bbox
 
     lon = (minlon + maxlon) * .5
     lat = (minlat + maxlat) * .5
-    result = rg.search((lat,lon))[0]
-    return result
+    results = rg.search((lat,lon))
+    if not results:
+        raise click.BadParameter(f"no geocoding result for the center of bbox {bbox}")
+    return results[0]
 
 
 def get_best_semantic_label(bbox: tuple[float, float, float, float]):
@@ -61,7 +85,10 @@ def get_best_semantic_label(bbox: tuple[float, float, float, float]):
 
     # 2. Get the offline geocode result
     # rg.search expects a list/tuple of tuples
-    result = rg.search((lat_center, lon_center))[0]
+    results = rg.search((lat_center, lon_center))
+    if not results:
+        raise click.BadParameter(f"no geocoding result for the center of bbox {bbox}")
+    result = results[0]
 
     country = result.get('cc', '')
     admin1 = result.get('admin1', '').strip()

tests/rapida/test_bbox_param_type.py

@@ -0,0 +1,288 @@
+import math
+import pytest
+from typing import Dict, Tuple
+import click
+from rapida.util import bbox_param_type
+from rapida.util.bbox_param_type import (
+    BboxParamType,
+    buffer_bbox,
+    get_bbox_label,
+    get_best_semantic_label,
+)
+
+
+@pytest.mark.parametrize(
+    "value, expected",
+    [
+        # a plain bbox over Kampala
+        ("32.5,0.2,32.7,0.4", (32.5, 0.2, 32.7, 0.4)),
+        # whitespace around each ordinate is stripped
+        (" 32.5 , 0.2 , 32.7 , 0.4 ", (32.5, 0.2, 32.7, 0.4)),
+        # southern/western hemisphere, negative ordinates
+        ("-58.5,-34.7,-58.3,-34.5", (-58.5, -34.7, -58.3, -34.5)),
+        # a bbox anchored on the null island corner
+        ("0,0,1,1", (0.0, 0.0, 1.0, 1.0)),
+        # integers are promoted to float
+        ("1,2,3,4", (1.0, 2.0, 3.0, 4.0)),
+        # the antimeridian/pole corners are inside the valid domain
+        ("-180,-90,180,90", (-180.0, -90.0, 180.0, 90.0)),
+    ]
+)
+def test_convert_accepts_valid_bbox(value: str, expected: Tuple[float, float, float, float]) -> None:
+    """
+    Test BboxParamType returns a 4 float tuple for well formed input.
+    Args:
+        value (str): the raw --bbox string as typed on the command line.
+        expected (Tuple[float, float, float, float]): the parsed bbox.
+    """
+    param_type = BboxParamType()
+    assert param_type.convert(value, None, None) == expected
+
+
+@pytest.mark.parametrize(
+    "value",
+    [
+        # too few ordinates
+        "1,2,3",
+        # too many ordinates
+        "1,2,3,4,5",
+        # not numbers at all
+        "a,b,c,d",
+        # empty string
+        "",
+        # a lone separator
+        ",,,",
+        # one bad ordinate among three good ones
+        "1,2,three,4",
+        # semicolons instead of commas
+        "1;2;3;4",
+    ]
+)
+def test_convert_rejects_malformed_bbox(value: str) -> None:
+    """
+    Test BboxParamType fails with a click error when the string is not 4 numbers.
+    Args:
+        value (str): a malformed --bbox string.
+    """
+    param_type = BboxParamType()
+    with pytest.raises(click.BadParameter):
+        param_type.convert(value, None, None)
+
+
+def test_nan_input_rejection() -> None:
+    """
+    Test a nan ordinate is refused. float() accepts it, so convert has to check.
+    """
+    with pytest.raises(click.BadParameter):
+        BboxParamType().convert("nan,nan,nan,nan", None, None)
+
+
+def test_inf_input_rejection() -> None:
+    """
+    Test a positive inf ordinate is refused.
+    """
+    with pytest.raises(click.BadParameter):
+        BboxParamType().convert("1,2,inf,4", None, None)
+
+
+def test_negative_inf_input_rejection() -> None:
+    """
+    Test a negative inf ordinate is refused.
+    """
+    with pytest.raises(click.BadParameter):
+        BboxParamType().convert("-inf,2,3,4", None, None)
+
+
+def test_longitude_above_maximum_rejection() -> None:
+    """
+    Test a longitude beyond 180 degrees is refused.
+    """
+    with pytest.raises(click.BadParameter):
+        BboxParamType().convert("200,10,210,20", None, None)
+
+
+def test_latitude_above_maximum_rejection() -> None:
+    """
+    Test a latitude beyond 90 degrees is refused.
+    """
+    with pytest.raises(click.BadParameter):
+        BboxParamType().convert("10,100,20,110", None, None)
+
+
+def test_ordinates_below_minimum_rejection() -> None:
+    """
+    Test ordinates below -180/-90 are refused.
+    """
+    with pytest.raises(click.BadParameter):
+        BboxParamType().convert("-200,-100,-190,-95", None, None)
+
+
+def test_inverted_longitude_rejection() -> None:
+    """
+    Test a bbox whose minimum longitude exceeds its maximum is refused.
+    """
+    with pytest.raises(click.BadParameter):
+        BboxParamType().convert("10,0,5,1", None, None)
+
+
+def test_inverted_latitude_rejection() -> None:
+    """
+    Test a bbox whose minimum latitude exceeds its maximum is refused.
+    """
+    with pytest.raises(click.BadParameter):
+        BboxParamType().convert("0,10,1,5", None, None)
+
+
+def test_zero_area_bbox_rejection() -> None:
+    """
+    Test a bbox with identical corners is refused before it reaches GDAL.
+    """
+    with pytest.raises(click.BadParameter):
+        BboxParamType().convert("5,5,5,5", None, None)
+
+
+@pytest.mark.parametrize("meters", [0, 0.0, None])
+def test_buffer_bbox_returns_input_when_no_buffer(meters: float) -> None:
+    """
+    Test buffer_bbox is a no op for a falsy buffer distance.
+    Args:
+        meters (float): the buffer distance in meters.
+    """
+    bbox = (32.5, 0.2, 32.7, 0.4)
+    assert buffer_bbox(bbox, meters) == bbox
+
+
+def test_buffer_bbox_grows_on_all_sides() -> None:
+    """
+    Test buffer_bbox enlarges the bbox in every direction.
+    """
+    minlon, minlat, maxlon, maxlat = buffer_bbox((0.0, 0.0, 1.0, 1.0), 1000)
+    assert minlon < 0.0
+    assert minlat < 0.0
+    assert maxlon > 1.0
+    assert maxlat > 1.0
+
+
+def test_buffer_bbox_latitude_buffer_is_constant() -> None:
+    """
+    Test the latitude buffer only depends on the distance, not on the latitude.
+    """
+    _, low_minlat, _, _ = buffer_bbox((0.0, 0.0, 1.0, 1.0), 1000)
+    _, high_minlat, _, _ = buffer_bbox((0.0, 60.0, 1.0, 61.0), 1000)
+    assert low_minlat == pytest.approx(0.0 - 1000 / 111320.0)
+    assert high_minlat == pytest.approx(60.0 - 1000 / 111320.0)
+
+
+def test_buffer_bbox_longitude_buffer_widens_towards_the_pole() -> None:
+    """
+    Test the longitude buffer grows with latitude, since a degree of longitude
+    covers less ground away from the equator.
+    """
+    equator = buffer_bbox((0.0, 0.0, 1.0, 1.0), 1000)
+    high_lat = buffer_bbox((0.0, 60.0, 1.0, 61.0), 1000)
+    assert (1.0 - equator[0]) < (1.0 - high_lat[0])
+
+
+def test_buffer_bbox_survives_a_polar_bbox() -> None:
+    """
+    Test a bbox touching the pole does not divide by zero. cos(90) is clamped,
+    so the call returns instead of raising.
+    """
+    minlon, _, maxlon, _ = buffer_bbox((0.0, 89.9, 1.0, 90.0), 1000)
+    assert math.isfinite(minlon)
+    assert math.isfinite(maxlon)
+
+
+def test_buffer_bbox_keeps_latitude_within_range() -> None:
+    """
+    Test buffering a polar bbox does not push latitude past 90 degrees.
+    """
+    _, _, _, maxlat = buffer_bbox((0.0, 89.9, 1.0, 90.0), 1000)
+    assert maxlat <= 90.0
+
+
+def test_get_bbox_label_uses_the_bbox_centre(monkeypatch: pytest.MonkeyPatch) -> None:
+    """
+    Test get_bbox_label geocodes the centre of the bbox rather than a corner.
+    Args:
+        monkeypatch (pytest.MonkeyPatch): used to capture the geocoder call.
+    """
+    seen = {}
+
+    def fake_search(coords):
+        seen["coords"] = coords
+        return [{"cc": "UG", "admin1": "Central Region", "admin2": "", "name": "Kampala"}]
+
+    monkeypatch.setattr(bbox_param_type.rg, "search", fake_search)
+    result = get_bbox_label((32.5, 0.2, 32.7, 0.4))
+    assert seen["coords"] == (pytest.approx(0.3), pytest.approx(32.6))
+    assert result["name"] == "Kampala"
+
+
+@pytest.mark.parametrize(
+    "record, expected",
+    [
+        # full hierarchy, every part contributes
+        (
+            {"cc": "UG", "admin1": "Central Region", "admin2": "Kampala District", "name": "Kampala"},
+            "UG_Central_Region_Kampala_District_Kampala",
+        ),
+        # admin2 repeats admin1 and is dropped
+        (
+            {"cc": "KE", "admin1": "Nairobi", "admin2": "Nairobi", "name": "Westlands"},
+            "KE_Nairobi_Westlands",
+        ),
+        # the settlement name repeats admin2 and is dropped
+        (
+            {"cc": "RW", "admin1": "Kigali", "admin2": "Nyarugenge", "name": "Nyarugenge"},
+            "RW_Kigali_Nyarugenge",
+        ),
+        # empty administrative levels are skipped
+        (
+            {"cc": "ZM", "admin1": "", "admin2": "", "name": "Lusaka"},
+            "ZM_Lusaka",
+        ),
+        # only the country code is known
+        (
+            {"cc": "NG", "admin1": "", "admin2": "", "name": ""},
+            "NG",
+        ),
+    ]
+)
+def test_get_best_semantic_label_builds_the_hierarchy(
+    record: Dict[str, str], expected: str, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    """
+    Test get_best_semantic_label joins the administrative hierarchy without
+    repeating a level and without leaving spaces in the label.
+    Args:
+        record (Dict[str, str]): a single reverse geocoder result.
+        expected (str): the label the record should produce.
+        monkeypatch (pytest.MonkeyPatch): used to stub the geocoder.
+    """
+    monkeypatch.setattr(bbox_param_type.rg, "search", lambda coords: [record])
+    assert get_best_semantic_label((32.5, 0.2, 32.7, 0.4)) == expected
+
+
+def test_get_bbox_label_empty_geocoder_result(monkeypatch: pytest.MonkeyPatch) -> None:
+    """
+    Test get_bbox_label reports an empty geocoder result as a click error rather
+    than indexing an empty list.
+    Args:
+        monkeypatch (pytest.MonkeyPatch): used to stub the geocoder.
+    """
+    monkeypatch.setattr(bbox_param_type.rg, "search", lambda coords: [])
+    with pytest.raises(click.BadParameter):
+        get_bbox_label((32.5, 0.2, 32.7, 0.4))
+
+
+def test_get_best_semantic_label_empty_geocoder_result(monkeypatch: pytest.MonkeyPatch) -> None:
+    """
+    Test get_best_semantic_label reports an empty geocoder result as a click
+    error rather than indexing an empty list.
+    Args:
+        monkeypatch (pytest.MonkeyPatch): used to stub the geocoder.
+    """
+    monkeypatch.setattr(bbox_param_type.rg, "search", lambda coords: [])
+    with pytest.raises(click.BadParameter):
+        get_best_semantic_label((32.5, 0.2, 32.7, 0.4))