From 867e4c285d80faa692193904fa10e343173f5301 Mon Sep 17 00:00:00 2001 From: bk86a Date: Fri, 3 Jul 2026 18:41:37 +0200 Subject: [PATCH] fix: harden Photon client + consistent lifespan settings (#125, #126, #127) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - _candidates no longer combines street+postcode when city is empty, matching the documented invariant; drops a wasted Photon round-trip (#125) - _query parses coordinates inside the try/except and catches (TypeError, IndexError), so a non-Point feature yields None instead of propagating a 500 — honouring the 'never raises' contract (#126) - lifespan reads _config.settings.* for the NUTS-polygon block, matching the adjacent Photon/token blocks so a monkeypatch can't desync PIP vs geocoder (#127) Adds no-city and non-Point-geometry regression tests. --- app/main.py | 8 ++++---- app/photon_client.py | 20 ++++++++++---------- tests/test_photon_client.py | 26 ++++++++++++++++++++++++++ 3 files changed, 40 insertions(+), 14 deletions(-) diff --git a/app/main.py b/app/main.py index f921f85..cb45c2c 100644 --- a/app/main.py +++ b/app/main.py @@ -123,13 +123,13 @@ async def lifespan(app: FastAPI): # PIP is only useful with a geocoder to produce coordinates, so the ~160 MB # polygon load is skipped entirely in the Lite tier (no PC2NUTS_PHOTON_URL). global _nuts_pip, _photon_client, _geo_http - if settings.photon_url: + if _config.settings.photon_url: try: with _httpx.Client() as _dl_client: _nuts_pip = load_nuts_pip( - url=settings.nuts_geojson_url, - path=settings.nuts_geojson_path, - cache_dir=settings.data_dir, + url=_config.settings.nuts_geojson_url, + path=_config.settings.nuts_geojson_path, + cache_dir=_config.settings.data_dir, client=_dl_client, ) logger.info("NUTS polygons loaded — /resolve PIP ready.") diff --git a/app/photon_client.py b/app/photon_client.py index c7d8b14..97c92cd 100644 --- a/app/photon_client.py +++ b/app/photon_client.py @@ -35,8 +35,6 @@ def _candidates(street: str, city: str, postal_code: str) -> list[str]: out.append(f"{postal_code} {city}") if city: out.append(city) - if street and postal_code and not city: - out.append(f"{street}, {postal_code}") if postal_code: out.append(postal_code) if street and not city and not postal_code: @@ -53,15 +51,17 @@ def _query(self, q: str) -> tuple[float, float] | None: ) resp.raise_for_status() data = resp.json() - except (httpx.HTTPError, ValueError): + feats = data.get("features") or [] + if not feats: + return None + coords = (feats[0].get("geometry") or {}).get("coordinates") or [] + if len(coords) < 2: + return None + # (lat, lon) from [lon, lat]; a non-Point feature nests coordinates, + # so float() would raise TypeError — caught here to honour "never raises". + return (float(coords[1]), float(coords[0])) + except (httpx.HTTPError, ValueError, TypeError, IndexError): return None - feats = data.get("features") or [] - if not feats: - return None - coords = (feats[0].get("geometry") or {}).get("coordinates") or [] - if len(coords) < 2: - return None - return (float(coords[1]), float(coords[0])) # (lat, lon) from [lon, lat] def geocode( self, street: str | None, city: str | None, postal_code: str | None diff --git a/tests/test_photon_client.py b/tests/test_photon_client.py index dbf8a88..f4cd5b9 100644 --- a/tests/test_photon_client.py +++ b/tests/test_photon_client.py @@ -62,6 +62,32 @@ def handler(req): assert all(not ("Museumpark 25" in q and "3000 AE" in q) for q in seen) +def test_never_combines_street_and_postcode_without_city(): + # The no-city path (reachable via the offline enrich path) must also never + # put street and postcode in one query — Photon returns nothing for that pair. + seen = [] + + def handler(req): + seen.append(req.url.params["q"]) + return httpx.Response(200, json={"features": []}) + + pc = PhotonClient("http://photon", _client(handler)) + pc.geocode("Museumpark 25", "", "3000 AE") + assert seen, "expected at least one query" + assert all(not ("Museumpark 25" in q and "3000 AE" in q) for q in seen) + + +def test_non_point_geometry_returns_none(): + # A non-Point feature has nested coordinates (e.g. [[lon,lat],...]); parsing + # coords[1] as a float would raise TypeError. The contract is "never raises". + def handler(req): + geom = {"type": "LineString", "coordinates": [[4.5, 50.8], [4.6, 50.9]]} + return httpx.Response(200, json={"features": [{"geometry": geom}]}) + + pc = PhotonClient("http://photon", _client(handler)) + assert pc.geocode("Markt", "Tervuren", "3080") is None + + def test_all_queries_empty_returns_none(): pc = PhotonClient("http://photon", _client(lambda r: httpx.Response(200, json={"features": []}))) assert pc.geocode("X", "Y", "0000") is None