]> git.openstreetmap.org Git - nominatim.git/commitdiff
make sure PHP and Python reverse code does the same
authorSarah Hoffmann <lonvia@denofr.de>
Sun, 26 Mar 2023 10:22:34 +0000 (12:22 +0200)
committerSarah Hoffmann <lonvia@denofr.de>
Sun, 26 Mar 2023 14:21:43 +0000 (16:21 +0200)
The only allowable difference is precision of coordinates. Python uses
a precision of 7 digits where possible, which corresponds to the
precision of OSM data.

Also fixes some smaller bugs found by the BDD tests.

20 files changed:
nominatim/api/lookup.py
nominatim/api/results.py
nominatim/api/reverse.py
nominatim/api/v1/classtypes.py
nominatim/api/v1/constants.py [deleted file]
nominatim/api/v1/format_json.py
nominatim/api/v1/format_xml.py
nominatim/api/v1/server_glue.py
nominatim/server/falcon/server.py
nominatim/server/sanic/server.py
nominatim/utils/json_writer.py
test/bdd/api/reverse/v1_geocodejson.feature
test/bdd/api/reverse/v1_geojson.feature
test/bdd/api/reverse/v1_json.feature
test/bdd/api/reverse/v1_xml.feature
test/bdd/steps/check_functions.py
test/bdd/steps/http_responses.py
test/bdd/steps/steps_api_queries.py
test/python/api/test_api_lookup.py
test/python/api/test_results.py

index 3952d4b805bc33011cf367f104795a7b460fa0c9..1b0ee87f29607fe66c48a9237b253861d6ae52e4 100644 (file)
@@ -102,14 +102,17 @@ async def find_in_tiger(conn: SearchConnection, place: ntyp.PlaceRef,
     """
     log().section("Find in TIGER table")
     t = conn.t.tiger
+    parent = conn.t.placex
     sql = sa.select(t.c.place_id, t.c.parent_place_id,
+                    parent.c.osm_type, parent.c.osm_id,
                     t.c.startnumber, t.c.endnumber, t.c.step,
                     t.c.postcode,
                     t.c.linegeo.ST_Centroid().label('centroid'),
                     _select_column_geometry(t.c.linegeo, details.geometry_output))
 
     if isinstance(place, ntyp.PlaceID):
-        sql = sql.where(t.c.place_id == place.place_id)
+        sql = sql.where(t.c.place_id == place.place_id)\
+                 .join(parent, t.c.parent_place_id == parent.c.place_id, isouter=True)
     else:
         return None
 
index 0e3ddeda778bea988b74ea95f5ecd5fe41f687cc..098851ef1b8a39da145eba2b7ba5968f45e84f34 100644 (file)
@@ -255,6 +255,7 @@ def create_from_tiger_row(row: Optional[SaRow],
 
     res = class_type(source_table=SourceTable.TIGER,
                      place_id=row.place_id,
+                     osm_object=(row.osm_type, row.osm_id),
                      category=('place', 'houses' if hnr is None else 'house'),
                      postcode=row.postcode,
                      country_code='us',
@@ -315,8 +316,8 @@ def _result_row_to_address_row(row: SaRow) -> AddressLine:
     """ Create a new AddressLine from the results of a datbase query.
     """
     extratags: Dict[str, str] = getattr(row, 'extratags', {})
-    if 'place_type' in row:
-        extratags['place_type'] = row.place_type
+    if hasattr(row, 'place_type') and row.place_type:
+        extratags['place'] = row.place_type
 
     names = row.name
     if getattr(row, 'housenumber', None) is not None:
index eadb63fb9886c4ba9dcde427291b031397623423..60b24fdc0923b99c92245c59fa1bc98325f4a214 100644 (file)
@@ -7,7 +7,7 @@
 """
 Implementation of reverse geocoding.
 """
-from typing import Optional, List
+from typing import Optional, List, Callable, Type, Tuple
 
 import sqlalchemy as sa
 from geoalchemy2 import WKTElement
@@ -16,12 +16,14 @@ from nominatim.typing import SaColumn, SaSelect, SaFromClause, SaLabel, SaRow
 from nominatim.api.connection import SearchConnection
 import nominatim.api.results as nres
 from nominatim.api.logging import log
-from nominatim.api.types import AnyPoint, DataLayer, LookupDetails, GeometryFormat
+from nominatim.api.types import AnyPoint, DataLayer, LookupDetails, GeometryFormat, Bbox
 
 # In SQLAlchemy expression which compare with NULL need to be expressed with
 # the equal sign.
 # pylint: disable=singleton-comparison
 
+RowFunc = Callable[[Optional[SaRow], Type[nres.ReverseResult]], Optional[nres.ReverseResult]]
+
 def _select_from_placex(t: SaFromClause, wkt: Optional[str] = None) -> SaSelect:
     """ Create a select statement with the columns relevant for reverse
         results.
@@ -37,7 +39,11 @@ def _select_from_placex(t: SaFromClause, wkt: Optional[str] = None) -> SaSelect:
                      t.c.housenumber, t.c.postcode, t.c.country_code,
                      t.c.importance, t.c.wikipedia,
                      t.c.parent_place_id, t.c.rank_address, t.c.rank_search,
-                     t.c.centroid,
+                     sa.case(
+                       (t.c.geometry.ST_GeometryType().in_(('ST_LineString',
+                                                           'ST_MultiLineString')),
+                        t.c.geometry.ST_ClosestPoint(wkt)),
+                       else_=t.c.centroid).label('centroid'),
                      distance.label('distance'),
                      t.c.geometry.ST_Expand(0).label('bbox'))
 
@@ -49,6 +55,14 @@ def _interpolated_housenumber(table: SaFromClause) -> SaLabel:
                    sa.Integer).label('housenumber')
 
 
+def _interpolated_position(table: SaFromClause) -> SaLabel:
+    fac = sa.cast(table.c.step, sa.Float) / (table.c.endnumber - table.c.startnumber)
+    rounded_pos = sa.func.round(table.c.position / fac) * fac
+    return sa.case(
+             (table.c.endnumber == table.c.startnumber, table.c.linegeo.ST_Centroid()),
+              else_=table.c.linegeo.ST_LineInterpolatePoint(rounded_pos)).label('centroid')
+
+
 def _is_address_point(table: SaFromClause) -> SaColumn:
     return sa.and_(table.c.rank_address == 30,
                    sa.or_(table.c.housenumber != None,
@@ -203,8 +217,8 @@ class ReverseGeocoder:
         sql = sa.select(inner.c.place_id, inner.c.osm_id,
                         inner.c.parent_place_id, inner.c.address,
                         _interpolated_housenumber(inner),
+                        _interpolated_position(inner),
                         inner.c.postcode, inner.c.country_code,
-                        inner.c.linegeo.ST_LineInterpolatePoint(inner.c.position).label('centroid'),
                         inner.c.distance)
 
         if self.details.geometry_output:
@@ -215,6 +229,7 @@ class ReverseGeocoder:
 
 
     async def _find_tiger_number_for_street(self, parent_place_id: int,
+                                            parent_type: str, parent_id: int,
                                             wkt: WKTElement) -> Optional[SaRow]:
         t = self.conn.t.tiger
 
@@ -229,9 +244,11 @@ class ReverseGeocoder:
 
         sql = sa.select(inner.c.place_id,
                         inner.c.parent_place_id,
+                        sa.literal(parent_type).label('osm_type'),
+                        sa.literal(parent_id).label('osm_id'),
                         _interpolated_housenumber(inner),
+                        _interpolated_position(inner),
                         inner.c.postcode,
-                        inner.c.linegeo.ST_LineInterpolatePoint(inner.c.position).label('centroid'),
                         inner.c.distance)
 
         if self.details.geometry_output:
@@ -241,15 +258,16 @@ class ReverseGeocoder:
         return (await self.conn.execute(sql)).one_or_none()
 
 
-    async def lookup_street_poi(self, wkt: WKTElement) -> Optional[nres.ReverseResult]:
+    async def lookup_street_poi(self,
+                                wkt: WKTElement) -> Tuple[Optional[SaRow], RowFunc]:
         """ Find a street or POI/address for the given WKT point.
         """
         log().section('Reverse lookup on street/address level')
-        result = None
         distance = 0.006
         parent_place_id = None
 
         row = await self._find_closest_street_or_poi(wkt, distance)
+        row_func: RowFunc = nres.create_from_placex_row
         log().var_dump('Result (street/building)', row)
 
         # If the closest result was a street, but an address was requested,
@@ -266,14 +284,19 @@ class ReverseGeocoder:
 
                 if addr_row is not None:
                     row = addr_row
+                    row_func = nres.create_from_placex_row
                     distance = addr_row.distance
                 elif row.country_code == 'us' and parent_place_id is not None:
                     log().comment('Find TIGER housenumber for street')
-                    addr_row = await self._find_tiger_number_for_street(parent_place_id, wkt)
+                    addr_row = await self._find_tiger_number_for_street(parent_place_id,
+                                                                        row.osm_type,
+                                                                        row.osm_id,
+                                                                        wkt)
                     log().var_dump('Result (street Tiger housenumber)', addr_row)
 
                     if addr_row is not None:
-                        result = nres.create_from_tiger_row(addr_row, nres.ReverseResult)
+                        row = addr_row
+                        row_func = nres.create_from_tiger_row
             else:
                 distance = row.distance
 
@@ -285,9 +308,10 @@ class ReverseGeocoder:
                                                                  wkt, distance)
             log().var_dump('Result (street interpolation)', addr_row)
             if addr_row is not None:
-                result = nres.create_from_osmline_row(addr_row, nres.ReverseResult)
+                row = addr_row
+                row_func = nres.create_from_osmline_row
 
-        return result or nres.create_from_placex_row(row, nres.ReverseResult)
+        return row, row_func
 
 
     async def _lookup_area_address(self, wkt: WKTElement) -> Optional[SaRow]:
@@ -391,7 +415,7 @@ class ReverseGeocoder:
         return row
 
 
-    async def lookup_area(self, wkt: WKTElement) -> Optional[nres.ReverseResult]:
+    async def lookup_area(self, wkt: WKTElement) -> Optional[SaRow]:
         """ Lookup large areas for the given WKT point.
         """
         log().section('Reverse lookup by larger area features')
@@ -406,10 +430,10 @@ class ReverseGeocoder:
         else:
             other_row = None
 
-        return nres.create_from_placex_row(_get_closest(address_row, other_row), nres.ReverseResult)
+        return _get_closest(address_row, other_row)
 
 
-    async def lookup_country(self, wkt: WKTElement) -> Optional[nres.ReverseResult]:
+    async def lookup_country(self, wkt: WKTElement) -> Optional[SaRow]:
         """ Lookup the country for the given WKT point.
         """
         log().section('Reverse lookup by country code')
@@ -470,7 +494,7 @@ class ReverseGeocoder:
 
             address_row = (await self.conn.execute(sql)).one_or_none()
 
-        return nres.create_from_placex_row(address_row, nres.ReverseResult)
+        return address_row
 
 
     async def lookup(self, coord: AnyPoint) -> Optional[nres.ReverseResult]:
@@ -484,15 +508,24 @@ class ReverseGeocoder:
 
         wkt = WKTElement(f'POINT({coord[0]} {coord[1]})', srid=4326)
 
-        result: Optional[nres.ReverseResult] = None
+        row: Optional[SaRow] = None
+        row_func: RowFunc = nres.create_from_placex_row
 
         if self.max_rank >= 26:
-            result = await self.lookup_street_poi(wkt)
-        if result is None and self.max_rank > 4:
-            result = await self.lookup_area(wkt)
-        if result is None and self.layer_enabled(DataLayer.ADDRESS):
-            result = await self.lookup_country(wkt)
+            row, tmp_row_func = await self.lookup_street_poi(wkt)
+            if row is not None:
+                row_func = tmp_row_func
+        if row is None and self.max_rank > 4:
+            row = await self.lookup_area(wkt)
+        if row is None and self.layer_enabled(DataLayer.ADDRESS):
+            row = await self.lookup_country(wkt)
+
+        result = row_func(row, nres.ReverseResult)
         if result is not None:
+            assert row is not None
+            result.distance = row.distance
+            if hasattr(row, 'bbox'):
+                result.bbox = Bbox.from_wkb(row.bbox.data)
             await nres.add_result_details(self.conn, result, self.details)
 
         return result
index b8ed8a9cd4ce7c47a6f1a047a3dbbc798899d5b1..27faa1746c3a105a946b7eae4194f23f6660cb41 100644 (file)
@@ -12,12 +12,16 @@ version a more flexible formatting is required.
 """
 from typing import Tuple, Optional, Mapping
 
+import nominatim.api as napi
+
 def get_label_tag(category: Tuple[str, str], extratags: Optional[Mapping[str, str]],
                   rank: int, country: Optional[str]) -> str:
     """ Create a label tag for the given place that can be used as an XML name.
     """
-    if rank < 26 and extratags and 'place'in extratags:
+    if rank < 26 and extratags and 'place' in extratags:
         label = extratags['place']
+    elif rank < 26 and extratags and 'linked_place' in extratags:
+        label = extratags['linked_place']
     elif category == ('boundary', 'administrative'):
         label = ADMIN_LABELS.get((country or '', int(rank/2)))\
                 or ADMIN_LABELS.get(('', int(rank/2)))\
@@ -37,6 +41,30 @@ def get_label_tag(category: Tuple[str, str], extratags: Optional[Mapping[str, st
     return label.lower().replace(' ', '_')
 
 
+def bbox_from_result(result: napi.ReverseResult) -> napi.Bbox:
+    """ Compute a bounding box for the result. For ways and relations
+        a given boundingbox is used. For all other object, a box is computed
+        around the centroid according to dimensions dereived from the
+        search rank.
+    """
+    if (result.osm_object and result.osm_object[0] == 'N') or result.bbox is None:
+        extent = NODE_EXTENT.get(result.category, 0.00005)
+        return napi.Bbox.from_point(result.centroid, extent)
+
+    return result.bbox
+
+
+# pylint: disable=line-too-long
+OSM_ATTRIBUTION = 'Data © OpenStreetMap contributors, ODbL 1.0. http://osm.org/copyright'
+
+
+OSM_TYPE_NAME = {
+    'N': 'node',
+    'W': 'way',
+    'R': 'relation'
+}
+
+
 ADMIN_LABELS = {
   ('', 1): 'Continent',
   ('', 2): 'Country',
@@ -142,3 +170,31 @@ ICONS = {
     ('amenity', 'prison'): 'amenity_prison',
     ('highway', 'bus_stop'): 'transport_bus_stop2'
 }
+
+NODE_EXTENT = {
+    ('place', 'continent'): 25,
+    ('place', 'country'): 7,
+    ('place', 'state'): 2.6,
+    ('place', 'province'): 2.6,
+    ('place', 'region'): 1.0,
+    ('place', 'county'): 0.7,
+    ('place', 'city'): 0.16,
+    ('place', 'municipality'): 0.16,
+    ('place', 'island'): 0.32,
+    ('place', 'postcode'): 0.16,
+    ('place', 'town'): 0.04,
+    ('place', 'village'): 0.02,
+    ('place', 'hamlet'): 0.02,
+    ('place', 'district'): 0.02,
+    ('place', 'borough'): 0.02,
+    ('place', 'suburb'): 0.02,
+    ('place', 'locality'): 0.01,
+    ('place', 'neighbourhood'): 0.01,
+    ('place', 'quarter'): 0.01,
+    ('place', 'city_block'): 0.01,
+    ('landuse', 'farm'): 0.01,
+    ('place', 'farm'): 0.01,
+    ('place', 'airport'): 0.015,
+    ('aeroway', 'aerodrome'): 0.015,
+    ('railway', 'station'): 0.005
+}
diff --git a/nominatim/api/v1/constants.py b/nominatim/api/v1/constants.py
deleted file mode 100644 (file)
index 68150a4..0000000
+++ /dev/null
@@ -1,43 +0,0 @@
-# SPDX-License-Identifier: GPL-3.0-or-later
-#
-# This file is part of Nominatim. (https://nominatim.org)
-#
-# Copyright (C) 2023 by the Nominatim developer community.
-# For a full list of authors see the git log.
-"""
-Constants shared by all formats.
-"""
-
-import nominatim.api as napi
-
-# pylint: disable=line-too-long
-OSM_ATTRIBUTION = 'Data © OpenStreetMap contributors, ODbL 1.0. http://www.openstreetmap.org/copyright'
-
-OSM_TYPE_NAME = {
-    'N': 'node',
-    'W': 'way',
-    'R': 'relation'
-}
-
-NODE_EXTENT = [25, 25, 25, 25,
-               7,
-               2.6, 2.6, 2.0, 1.0, 1.0,
-               0.7, 0.7, 0.7,
-               0.16, 0.16, 0.16, 0.16,
-               0.04, 0.04,
-               0.02, 0.02,
-               0.01, 0.01, 0.01, 0.01, 0.01,
-               0.015, 0.015, 0.015, 0.015,
-               0.005]
-
-
-def bbox_from_result(result: napi.ReverseResult) -> napi.Bbox:
-    """ Compute a bounding box for the result. For ways and relations
-        a given boundingbox is used. For all other object, a box is computed
-        around the centroid according to dimensions dereived from the
-        search rank.
-    """
-    if (result.osm_object and result.osm_object[0] == 'N') or result.bbox is None:
-        return napi.Bbox.from_point(result.centroid, NODE_EXTENT[result.rank_search])
-
-    return result.bbox
index 898e621377abebd999b1070d3b457e39ecf294b3..7387c89b8037ce0490707e7dc49cce592e899f61 100644 (file)
@@ -10,13 +10,12 @@ Helper functions for output of results in json formats.
 from typing import Mapping, Any, Optional, Tuple
 
 import nominatim.api as napi
-from nominatim.api.v1.constants import OSM_ATTRIBUTION, OSM_TYPE_NAME, bbox_from_result
-from nominatim.api.v1.classtypes import ICONS, get_label_tag
+import nominatim.api.v1.classtypes as cl
 from nominatim.utils.json_writer import JsonWriter
 
 def _write_osm_id(out: JsonWriter, osm_object: Optional[Tuple[str, int]]) -> None:
     if osm_object is not None:
-        out.keyval_not_none('osm_type', OSM_TYPE_NAME.get(osm_object[0], None))\
+        out.keyval_not_none('osm_type', cl.OSM_TYPE_NAME.get(osm_object[0], None))\
            .keyval('osm_id', osm_object[1])
 
 
@@ -24,11 +23,15 @@ def _write_typed_address(out: JsonWriter, address: Optional[napi.AddressLines],
                                country_code: Optional[str]) -> None:
     parts = {}
     for line in (address or []):
-        if line.isaddress and line.local_name:
-            label = get_label_tag(line.category, line.extratags,
-                                  line.rank_address, country_code)
-            if label not in parts:
-                parts[label] = line.local_name
+        if line.isaddress:
+            if line.local_name:
+                label = cl.get_label_tag(line.category, line.extratags,
+                                         line.rank_address, country_code)
+                if label not in parts:
+                    print(label)
+                    parts[label] = line.local_name
+            if line.names and 'ISO3166-2' in line.names and line.admin_level:
+                parts[f"ISO3166-2-lvl{line.admin_level}"] = line.names['ISO3166-2']
 
     for k, v in parts.items():
         out.keyval(k, v)
@@ -79,7 +82,7 @@ def format_base_json(results: napi.ReverseResults, #pylint: disable=too-many-bra
 
         out.start_object()\
              .keyval_not_none('place_id', result.place_id)\
-             .keyval('licence', OSM_ATTRIBUTION)\
+             .keyval('licence', cl.OSM_ATTRIBUTION)\
 
         _write_osm_id(out, result.osm_object)
 
@@ -89,15 +92,15 @@ def format_base_json(results: napi.ReverseResults, #pylint: disable=too-many-bra
              .keyval('type', result.category[1])\
              .keyval('place_rank', result.rank_search)\
              .keyval('importance', result.calculated_importance())\
-             .keyval('addresstype', get_label_tag(result.category, result.extratags,
-                                                  result.rank_address,
-                                                  result.country_code))\
+             .keyval('addresstype', cl.get_label_tag(result.category, result.extratags,
+                                                     result.rank_address,
+                                                     result.country_code))\
              .keyval('name', locales.display_name(result.names))\
              .keyval('display_name', ', '.join(label_parts))
 
 
         if options.get('icon_base_url', None):
-            icon = ICONS.get(result.category)
+            icon = cl.ICONS.get(result.category)
             if icon:
                 out.keyval('icon', f"{options['icon_base_url']}/{icon}.p.20.png")
 
@@ -112,12 +115,12 @@ def format_base_json(results: napi.ReverseResults, #pylint: disable=too-many-bra
         if options.get('namedetails', False):
             out.keyval('namedetails', result.names)
 
-        bbox = bbox_from_result(result)
+        bbox = cl.bbox_from_result(result)
         out.key('boundingbox').start_array()\
-             .value(bbox.minlat).next()\
-             .value(bbox.maxlat).next()\
-             .value(bbox.minlon).next()\
-             .value(bbox.maxlon).next()\
+             .value(f"{bbox.minlat:0.7f}").next()\
+             .value(f"{bbox.maxlat:0.7f}").next()\
+             .value(f"{bbox.minlon:0.7f}").next()\
+             .value(f"{bbox.maxlon:0.7f}").next()\
            .end_array().next()
 
         if result.geometry:
@@ -153,7 +156,7 @@ def format_base_geojson(results: napi.ReverseResults,
 
     out.start_object()\
          .keyval('type', 'FeatureCollection')\
-         .keyval('licence', OSM_ATTRIBUTION)\
+         .keyval('licence', cl.OSM_ATTRIBUTION)\
          .key('features').start_array()
 
     for result in results:
@@ -174,9 +177,9 @@ def format_base_geojson(results: napi.ReverseResults,
            .keyval('category', result.category[0])\
            .keyval('type', result.category[1])\
            .keyval('importance', result.calculated_importance())\
-           .keyval('addresstype', get_label_tag(result.category, result.extratags,
-                                                result.rank_address,
-                                                result.country_code))\
+           .keyval('addresstype', cl.get_label_tag(result.category, result.extratags,
+                                                   result.rank_address,
+                                                   result.country_code))\
            .keyval('name', locales.display_name(result.names))\
            .keyval('display_name', ', '.join(label_parts))
 
@@ -193,8 +196,10 @@ def format_base_geojson(results: napi.ReverseResults,
 
         out.end_object().next() # properties
 
-        bbox = bbox_from_result(result)
-        out.keyval('bbox', bbox.coords)
+        out.key('bbox').start_array()
+        for coord in cl.bbox_from_result(result).coords:
+            out.float(coord, 7).next()
+        out.end_array().next()
 
         out.key('geometry').raw(result.geometry.get('geojson')
                                 or result.centroid.to_geojson()).next()
@@ -221,7 +226,7 @@ def format_base_geocodejson(results: napi.ReverseResults,
          .keyval('type', 'FeatureCollection')\
          .key('geocoding').start_object()\
            .keyval('version', '0.1.0')\
-           .keyval('attribution', OSM_ATTRIBUTION)\
+           .keyval('attribution', cl.OSM_ATTRIBUTION)\
            .keyval('licence', 'ODbL')\
            .keyval_not_none('query', options.get('query'))\
            .end_object().next()\
@@ -245,9 +250,9 @@ def format_base_geocodejson(results: napi.ReverseResults,
         out.keyval('osm_key', result.category[0])\
            .keyval('osm_value', result.category[1])\
            .keyval('type', GEOCODEJSON_RANKS[max(3, min(28, result.rank_address))])\
-           .keyval_not_none('accuracy', result.distance)\
+           .keyval_not_none('accuracy', result.distance, transform=int)\
            .keyval('label', ', '.join(label_parts))\
-           .keyval_not_none('name', locales.display_name(result.names))\
+           .keyval_not_none('name', result.names, transform=locales.display_name)\
 
         if options.get('addressdetails', False):
             _write_geocodejson_address(out, result.address_rows, result.place_id,
index b1159f939949187ab024fe4224fd85018b765de3..3fe3b7fe7771a428b57062c97a59e9c8f797c79f 100644 (file)
@@ -12,18 +12,20 @@ import datetime as dt
 import xml.etree.ElementTree as ET
 
 import nominatim.api as napi
-from nominatim.api.v1.constants import OSM_ATTRIBUTION, OSM_TYPE_NAME, bbox_from_result
-from nominatim.api.v1.classtypes import ICONS, get_label_tag
+import nominatim.api.v1.classtypes as cl
 
 def _write_xml_address(root: ET.Element, address: napi.AddressLines,
                        country_code: Optional[str]) -> None:
     parts = {}
     for line in address:
-        if line.isaddress and line.local_name:
-            label = get_label_tag(line.category, line.extratags,
-                                  line.rank_address, country_code)
-            if label not in parts:
-                parts[label] = line.local_name
+        if line.isaddress:
+            if line.local_name:
+                label = cl.get_label_tag(line.category, line.extratags,
+                                         line.rank_address, country_code)
+                if label not in parts:
+                    parts[label] = line.local_name
+            if line.names and 'ISO3166-2' in line.names and line.admin_level:
+                parts[f"ISO3166-2-lvl{line.admin_level}"] = line.names['ISO3166-2']
 
     for k,v in parts.items():
         ET.SubElement(root, k).text = v
@@ -44,18 +46,21 @@ def _create_base_entry(result: napi.ReverseResult, #pylint: disable=too-many-bra
     if result.place_id is not None:
         place.set('place_id', str(result.place_id))
     if result.osm_object:
-        osm_type = OSM_TYPE_NAME.get(result.osm_object[0], None)
+        osm_type = cl.OSM_TYPE_NAME.get(result.osm_object[0], None)
         if osm_type is not None:
             place.set('osm_type', osm_type)
         place.set('osm_id', str(result.osm_object[1]))
     if result.names and 'ref' in result.names:
-        place.set('place_id', result.names['ref'])
-    place.set('lat', str(result.centroid.lat))
-    place.set('lon', str(result.centroid.lon))
+        place.set('ref', result.names['ref'])
+    elif label_parts:
+        # bug reproduced from PHP
+        place.set('ref', label_parts[0])
+    place.set('lat', f"{result.centroid.lat:.7f}")
+    place.set('lon', f"{result.centroid.lon:.7f}")
 
-    bbox = bbox_from_result(result)
-    place.set('boundingbox', ','.join(map(str, [bbox.minlat, bbox.maxlat,
-                                                bbox.minlon, bbox.maxlon])))
+    bbox = cl.bbox_from_result(result)
+    place.set('boundingbox',
+              f"{bbox.minlat:.7f},{bbox.maxlat:.7f},{bbox.minlon:.7f},{bbox.maxlon:.7f}")
 
     place.set('place_rank', str(result.rank_search))
     place.set('address_rank', str(result.rank_address))
@@ -92,7 +97,7 @@ def format_base_xml(results: napi.ReverseResults,
 
     root = ET.Element(xml_root_tag)
     root.set('timestamp', dt.datetime.utcnow().strftime('%a, %d %b %Y %H:%M:%S +00:00'))
-    root.set('attribution', OSM_ATTRIBUTION)
+    root.set('attribution', cl.OSM_ATTRIBUTION)
     for k, v in xml_extra_info.items():
         root.set(k, v)
 
@@ -103,7 +108,7 @@ def format_base_xml(results: napi.ReverseResults,
         place = _create_base_entry(result, root, simple, locales)
 
         if not simple and options.get('icon_base_url', None):
-            icon = ICONS.get(result.category)
+            icon = cl.ICONS.get(result.category)
             if icon:
                 place.set('icon', icon)
 
index 3c0c58b01e4ba31e98465545fc4266c30c32a586..b5a4a3cacf636cb3f47a21eded4843f7bc03d722 100644 (file)
@@ -8,8 +8,10 @@
 Generic part of the server implementation of the v1 API.
 Combine with the scaffolding provided for the various Python ASGI frameworks.
 """
-from typing import Optional, Any, Type, Callable, NoReturn, TypeVar
+from typing import Optional, Any, Type, Callable, NoReturn, cast
+from functools import reduce
 import abc
+import math
 
 from nominatim.config import Configuration
 import nominatim.api as napi
@@ -22,8 +24,6 @@ CONTENT_TYPE = {
   'debug': 'text/html; charset=utf-8'
 }
 
-ConvT = TypeVar('ConvT', int, float)
-
 class ASGIAdaptor(abc.ABC):
     """ Adapter class for the different ASGI frameworks.
         Wraps functionality over concrete requests and responses.
@@ -107,10 +107,9 @@ class ASGIAdaptor(abc.ABC):
         raise self.error(msg, status)
 
 
-    def _get_typed(self, name: str, dest_type: Type[ConvT], type_name: str,
-                   default: Optional[ConvT] = None) -> ConvT:
-        """ Return an input parameter as the type 'dest_type'. Raises an
-            exception if the parameter is given but not in the given format.
+    def get_int(self, name: str, default: Optional[int] = None) -> int:
+        """ Return an input parameter as an int. Raises an exception if
+            the parameter is given but not in an integer format.
 
             If 'default' is given, then it will be returned when the parameter
             is missing completely. When 'default' is None, an error will be
@@ -125,33 +124,38 @@ class ASGIAdaptor(abc.ABC):
             self.raise_error(f"Parameter '{name}' missing.")
 
         try:
-            intval = dest_type(value)
+            intval = int(value)
         except ValueError:
-            self.raise_error(f"Parameter '{name}' must be a {type_name}.")
+            self.raise_error(f"Parameter '{name}' must be a number.")
 
         return intval
 
 
-    def get_int(self, name: str, default: Optional[int] = None) -> int:
-        """ Return an input parameter as an int. Raises an exception if
-            the parameter is given but not in an integer format.
+    def get_float(self, name: str, default: Optional[float] = None) -> float:
+        """ Return an input parameter as a flaoting-point number. Raises an
+            exception if the parameter is given but not in an float format.
 
             If 'default' is given, then it will be returned when the parameter
             is missing completely. When 'default' is None, an error will be
             raised on a missing parameter.
         """
-        return self._get_typed(name, int, 'number', default)
+        value = self.get(name)
 
+        if value is None:
+            if default is not None:
+                return default
 
-    def get_float(self, name: str, default: Optional[float] = None) -> int:
-        """ Return an input parameter as a flaoting-point number. Raises an
-            exception if the parameter is given but not in an float format.
+            self.raise_error(f"Parameter '{name}' missing.")
 
-            If 'default' is given, then it will be returned when the parameter
-            is missing completely. When 'default' is None, an error will be
-            raised on a missing parameter.
-        """
-        return self._get_typed(name, float, 'number', default)
+        try:
+            fval = float(value)
+        except ValueError:
+            self.raise_error(f"Parameter '{name}' must be a number.")
+
+        if math.isnan(fval) or math.isinf(fval):
+            self.raise_error(f"Parameter '{name}' must be a number.")
+
+        return fval
 
 
     def get_bool(self, name: str, default: Optional[bool] = None) -> bool:
@@ -194,15 +198,16 @@ class ASGIAdaptor(abc.ABC):
         return False
 
 
-    def get_layers(self) -> napi.DataLayer:
+    def get_layers(self) -> Optional[napi.DataLayer]:
         """ Return a parsed version of the layer parameter.
         """
         param = self.get('layer', None)
         if param is None:
             return None
 
-        return reduce(napi.DataLayer.__or__,
-                      (getattr(napi.DataLayer, s.upper()) for s in param.split(',')))
+        return cast(napi.DataLayer,
+                    reduce(napi.DataLayer.__or__,
+                           (getattr(napi.DataLayer, s.upper()) for s in param.split(','))))
 
 
     def parse_format(self, result_type: Type[Any], default: str) -> str:
@@ -289,10 +294,6 @@ async def reverse_endpoint(api: napi.NominatimAPIAsync, params: ASGIAdaptor) ->
 
     zoom = max(0, min(18, params.get_int('zoom', 18)))
 
-    # Negation makes sure that NaN is handled. Don't change.
-    if not abs(coord[0]) <= 180 or not abs(coord[1]) <= 90:
-        params.raise_error('Invalid coordinates.')
-
     details = napi.LookupDetails(address_details=True,
                                  geometry_simplification=params.get_float('polygon_threshold', 0.0))
     numgeoms = 0
@@ -311,7 +312,7 @@ async def reverse_endpoint(api: napi.NominatimAPIAsync, params: ASGIAdaptor) ->
             numgeoms += 1
 
     if numgeoms > params.config().get_int('POLYGON_OUTPUT_MAX_TYPES'):
-        params.raise_error(f'Too many polgyon output options selected.')
+        params.raise_error('Too many polgyon output options selected.')
 
     result = await api.reverse(coord, REVERSE_MAX_RANKS[zoom],
                                params.get_layers() or
index 1885330748960de27e6d56f17155c7e4dddf1889..7478ec39c1275b9e51f896e1c753eed0a8b08e16 100644 (file)
@@ -10,7 +10,6 @@ Server implementation using the falcon webserver framework.
 from typing import Optional, Mapping, cast, Any
 from pathlib import Path
 
-import falcon
 from falcon.asgi import App, Request, Response
 
 from nominatim.api import NominatimAPIAsync
@@ -26,9 +25,12 @@ class HTTPNominatimError(Exception):
         self.content_type = content_type
 
 
-async def nominatim_error_handler(req: Request, resp: Response,
+async def nominatim_error_handler(req: Request, resp: Response, #pylint: disable=unused-argument
                                   exception: HTTPNominatimError,
                                   _: Any) -> None:
+    """ Special error handler that passes message and content type as
+        per exception info.
+    """
     resp.status = exception.status
     resp.text = exception.msg
     resp.content_type = exception.content_type
index cf1ef4ce2859ab352bec841fe591c8a2969d5af1..15887eefd71b04d2d519f77352a54b5758ab2d05 100644 (file)
@@ -36,7 +36,6 @@ class ParamWrapper(api_impl.ASGIAdaptor):
 
     def error(self, msg: str, status: int = 400) -> SanicException:
         exception = SanicException(msg, status_code=status)
-        exception.headers = {'content-type': self.content_type}
 
         return exception
 
index e2e5b9e6c8bfd1644c7ca7c10e87052b5bd3f218..bb642233e78d8c4234afb7e3bc54c4cd0f69cd8b 100644 (file)
@@ -101,6 +101,11 @@ class JsonWriter:
         return self.raw(json.dumps(value, ensure_ascii=False))
 
 
+    def float(self, value: float, precision: int) -> 'JsonWriter':
+        """ Write out a float value with the given precision.
+        """
+        return self.raw(f"{value:0.{precision}f}")
+
     def next(self) -> 'JsonWriter':
         """ Write out a delimiter comma between JSON object or array elements.
         """
index b7ea03549d87282368e53cc35b0dc011d826c5f6..c9112b94c591439779163a0b4f8626b71b125326 100644 (file)
@@ -19,7 +19,7 @@ Feature: Geocodejson for Reverse API
           | Point | [9.5036065, 47.0660892] |
         And results contain in field __geocoding
           | version | licence | attribution |
-          | 0.1.0   | ODbL    | Data © OpenStreetMap contributors, ODbL 1.0. https://osm.org/copyright |
+          | 0.1.0   | ODbL    | ^Data © OpenStreetMap contributors, ODbL 1.0. https?://osm.org/copyright$ |
 
         Examples:
           | has_address | attributes     |
index 8acf067f1f80d9214bca229465c49e61322701d5..0b6ad0d3a3ff6744a277855abcec1a88d7a1b577 100644 (file)
@@ -42,7 +42,7 @@ Feature: Geojson for Reverse API
           | way      | 1      | 30         | place    | house   | place       |
         And results contain
           | boundingbox |
-          | [47.118495392, 47.118595392, 9.57049676, 9.57059676] |
+          | ^\[47.118495\d*, 47.118595\d*, 9.570496\d*, 9.570596\d*\] |
         And results contain
           | display_name |
           | 1019, Grosssteg, Sücka, Triesenberg, Oberland, 9497, Liechtenstein |
index 155e02b0ab45457a8f8e6f0515c931ae9652ea80..ac3c799ed8d71c9c9956ad6bc450a1f8fa39f57a 100644 (file)
@@ -17,12 +17,12 @@ Feature: Json output for Reverse API
           | 1           | attributes     |
           | 0           | not attributes |
 
-    Scenario Outline: Siple OSM result
+    Scenario Outline: Simple OSM result
         When sending v1/reverse at 47.066,9.504 with format <format>
         Then result has attributes place_id
         And results contain
           | licence |
-          | Data © OpenStreetMap contributors, ODbL 1.0. https://osm.org/copyright |
+          | ^Data © OpenStreetMap contributors, ODbL 1.0. https?://osm.org/copyright$ |
         And results contain
           | osm_type | osm_id     |
           | node     | 6522627624 |
@@ -62,7 +62,7 @@ Feature: Json output for Reverse API
           | way      | 1      |
         And results contain
           | centroid                | boundingbox |
-          | 9.57054676 47.118545392 | ['47.118495392', '47.118595392', '9.57049676', '9.57059676'] |
+          | 9.57054676 47.118545392 | ^\['47.118495\d*', '47.118595\d*', '9.570496\d*', '9.570596\d*'\] |
         And results contain
           | display_name |
           | 1019, Grosssteg, Sücka, Triesenberg, Oberland, 9497, Liechtenstein |
index fd6e150120b9f523f2e43d165c79a48eacee0df4..75f27220497009eb65c2d7e0daee3b0b426eed50 100644 (file)
@@ -32,7 +32,7 @@ Feature: XML output for Reverse API
          | way      | 396009653 | 30          | 30           |
         And results contain
           | centroid                     | boundingbox |
-          | -86.4808553258 32.4753580256 | ^32.475308025\d*,32.475408025\d*,-86.480905325\d*,-86.480805325\d* |
+          | -86.4808553 32.4753580 | ^32.4753080\d*,32.4754080\d*,-86.4809053\d*,-86.4808053\d* |
         And results contain
           | display_name |
           | 707, Upper Kingston Road, Upper Kingston, Prattville, Autauga County, 36067, United States |
@@ -45,7 +45,7 @@ Feature: XML output for Reverse API
           | way      | 1      | 30         | 30           |
         And results contain
           | centroid                | boundingbox |
-          | 9.57054676 47.118545392 | 47.118495392,47.118595392,9.57049676,9.57059676 |
+          | 9.57054676 47.118545392 | ^47.118495\d*,47.118595\d*,9.570496\d*,9.570596\d* |
         And results contain
           | display_name |
           | 1019, Grosssteg, Sücka, Triesenberg, Oberland, 9497, Liechtenstein |
index 58d6c1f2a481ddf61453df633d90e725ef457036..49676896f4ff4487d5bc3e5ec815a339c3b8043b 100644 (file)
@@ -47,15 +47,16 @@ class Field:
     """ Generic comparator for fields, which looks at the type of the
         value compared.
     """
-    def __init__(self, value):
+    def __init__(self, value, **extra_args):
         self.value = value
+        self.extra_args = extra_args
 
     def __eq__(self, other):
         if isinstance(self.value, float):
-            return math.isclose(self.value, float(other))
+            return math.isclose(self.value, float(other), **self.extra_args)
 
         if self.value.startswith('^'):
-            return re.fullmatch(self.value, other)
+            return re.fullmatch(self.value, str(other))
 
         if isinstance(other, dict):
             return other == eval('{' + self.value + '}')
index 22fcb01018b4a92b382aadf5cc77bb05a50b5b7c..2e24ed5043bddf87bcfde92e897a6f95e241674e 100644 (file)
@@ -134,8 +134,8 @@ class GenericResponse:
                         lon, lat = context.osm.grid_node(int(value))
                     else:
                         raise RuntimeError("Context needed when using grid coordinates")
-                    self.check_row_field(i, 'lat', Field(float(lat)), base=subdict)
-                    self.check_row_field(i, 'lon', Field(float(lon)), base=subdict)
+                    self.check_row_field(i, 'lat', Field(float(lat), abs_tol=1e-07), base=subdict)
+                    self.check_row_field(i, 'lon', Field(float(lon), abs_tol=1e-07), base=subdict)
                 else:
                     self.check_row_field(i, name, Field(value), base=subdict)
 
index 1c6fac693f9d494c0d6b308155c86f1d612d3acc..550cf5311c105b3c5cb2d842f564546d08170719 100644 (file)
@@ -229,7 +229,8 @@ def validate_result_number(context, operator, number):
 @then(u'a HTTP (?P<status>\d+) is returned')
 def check_http_return_status(context, status):
     assert context.response.errorcode == int(status), \
-           f"Return HTTP status is {context.response.errorcode}."
+           f"Return HTTP status is {context.response.errorcode}."\
+           f" Full response:\n{context.response.page}"
 
 @then(u'the page contents equals "(?P<text>.+)"')
 def check_page_content_equals(context, text):
index f8e89930ca2fe143a7fdc14ee7c98da6111091db..6939ddb9ba28c19ef59656ea9652511fb9272f11 100644 (file)
@@ -378,6 +378,10 @@ def test_lookup_in_tiger(apiobj):
                      startnumber=1, endnumber=4, step=1,
                      postcode='34425',
                      geometry='LINESTRING(23 34, 23 35)')
+    apiobj.add_placex(place_id=12,
+                      category=('highway', 'residential'),
+                      osm_type='W', osm_id=6601223,
+                      geometry='LINESTRING(23 34, 23 35)')
 
     result = apiobj.api.lookup(napi.PlaceID(4924), napi.LookupDetails())
 
@@ -390,7 +394,7 @@ def test_lookup_in_tiger(apiobj):
     assert result.place_id == 4924
     assert result.parent_place_id == 12
     assert result.linked_place_id is None
-    assert result.osm_object is None
+    assert result.osm_object == ('W', 6601223)
     assert result.admin_level == 15
 
     assert result.names is None
index 7ea1fb1fe1d52a64bebe214f23b2c75dfbf29ffd..97d95ac0a65fc81b15e97c7fa371ca8dff587126 100644 (file)
@@ -58,9 +58,9 @@ def test_create_row_none(func):
 @pytest.mark.parametrize('func', (nresults.create_from_osmline_row,
                                   nresults.create_from_tiger_row))
 def test_create_row_with_housenumber(func):
-    row = FakeRow(place_id = 2345, osm_id = 111, housenumber = 4,
-                  address = None, postcode = '99900', country_code = 'xd',
-                  centroid = FakeCentroid(0, 0))
+    row = FakeRow(place_id=2345, osm_type='W', osm_id=111, housenumber=4,
+                  address=None, postcode='99900', country_code='xd',
+                  centroid=FakeCentroid(0, 0))
 
     res = func(row, DetailedResult)
 
@@ -72,7 +72,7 @@ def test_create_row_with_housenumber(func):
 @pytest.mark.parametrize('func', (nresults.create_from_osmline_row,
                                   nresults.create_from_tiger_row))
 def test_create_row_without_housenumber(func):
-    row = FakeRow(place_id=2345, osm_id=111,
+    row = FakeRow(place_id=2345, osm_type='W', osm_id=111,
                   startnumber=1, endnumber=11, step=2,
                   address=None, postcode='99900', country_code='xd',
                   centroid=FakeCentroid(0, 0))