Merge pull request #4020 from kad-link/fix/add-admin-level-in-extratags

fix: add admin_level in extratags for boundary=administrative

src/nominatim_api/v1/format.py

@@ -19,6 +19,7 @@ from ..localization import Locales
 from ..result_formatting import FormatDispatcher
 from .classtypes import ICONS
 from . import format_json, format_xml
+from .helpers import _add_admin_level
 from .. import logging as loglib
 from ..server import content_types as ct
 
@@ -157,7 +158,7 @@ def _format_details_json(result: DetailedResult, options: Mapping[str, Any]) ->
        .keyval_not_none('indexed_date', result.indexed_date, lambda v: v.isoformat())\
        .keyval_not_none('importance', result.importance)\
        .keyval('calculated_importance', result.calculated_importance())\
-       .keyval('extratags', result.extratags or {})\
+       .keyval('extratags', _add_admin_level(result) or {})\
        .keyval_not_none('calculated_wikipedia', result.wikipedia)\
        .keyval('rank_address', result.rank_address)\
        .keyval('rank_search', result.rank_search)\

src/nominatim_api/v1/format_json.py

@@ -12,6 +12,7 @@ from typing import Mapping, Any, Optional, Tuple, Union, List
 from ..utils.json_writer import JsonWriter
 from ..results import AddressLines, ReverseResults, SearchResults
 from . import classtypes as cl
+from .helpers import _add_admin_level
 from ..types import EntranceDetails
 
 
@@ -134,7 +135,7 @@ def format_base_json(results: Union[ReverseResults, SearchResults],
             write_entrances(out, result.entrances)
 
         if options.get('extratags', False):
-            out.keyval('extratags', result.extratags)
+            out.keyval('extratags', _add_admin_level(result))
 
         if options.get('namedetails', False):
             out.keyval('namedetails', result.names)
@@ -210,7 +211,7 @@ def format_base_geojson(results: Union[ReverseResults, SearchResults],
             write_entrances(out, result.entrances)
 
         if options.get('extratags', False):
-            out.keyval('extratags', result.extratags)
+            out.keyval('extratags', _add_admin_level(result))
 
         if options.get('namedetails', False):
             out.keyval('namedetails', result.names)
@@ -284,7 +285,7 @@ def format_base_geocodejson(results: Union[ReverseResults, SearchResults],
             write_entrances(out, result.entrances)
 
         if options.get('extratags', False):
-            out.keyval('extra', result.extratags)
+            out.keyval('extra', _add_admin_level(result))
 
         out.end_object().next().end_object().next()
 

src/nominatim_api/v1/format_xml.py

@@ -14,6 +14,7 @@ import xml.etree.ElementTree as ET
 from ..results import AddressLines, ReverseResult, ReverseResults, \
                       SearchResult, SearchResults
 from . import classtypes as cl
+from .helpers import _add_admin_level
 from ..types import EntranceDetails
 
 
@@ -125,8 +126,9 @@ def format_base_xml(results: Union[ReverseResults, SearchResults],
 
         if options.get('extratags', False):
             eroot = ET.SubElement(root if simple else place, 'extratags')
-            if result.extratags:
+            tags = _add_admin_level(result)
-                for k, v in result.extratags.items():
+            if tags:
+                for k, v in tags.items():
                     ET.SubElement(eroot, 'tag', attrib={'key': k, 'value': v})
 
         if options.get('namedetails', False):

src/nominatim_api/v1/helpers.py

@@ -12,10 +12,20 @@ from typing import Tuple, Optional, Any, Dict, Iterable
 from itertools import chain
 import re
 
-from ..results import SearchResults, SourceTable
+from ..results import SearchResults, SourceTable, BaseResult
 from ..types import SearchDetails, GeometryFormat
 
 
+def _add_admin_level(result: BaseResult) -> Optional[Dict[str, str]]:
+    """ Inject admin_level into extratags for boundary=administrative results.
+    """
+    tags = result.extratags
+    if result.category == ('boundary', 'administrative') and result.admin_level < 15:
+        tags = dict(tags) if tags else {}
+        tags['admin_level'] = str(result.admin_level)
+    return tags
+
+
 REVERSE_MAX_RANKS = [2, 2, 2,   # 0-2   Continent/Sea
                      4, 4,      # 3-4   Country
                      8,         # 5     State

test/bdd/features/api/search/params.feature

@@ -318,6 +318,28 @@ Feature: Search queries
           | jsonv2 | json |
           | geojson | geojson |
 
+    Scenario Outline: Search boundary=administrative with extratags=1 returns admin_level
+        When sending v1/search with format <format>
+          | q           | featureType | extratags |
+          | Triesenberg | city        | 1         |
+        Then a HTTP 200 is returned
+        And the result is valid <outformat>
+        And more than 0 results are returned
+        And result 0 contains
+          | <cname>  | <tname>        |
+          | boundary | administrative |
+        And result 0 contains in field <ename>
+          | param       | value |
+          | admin_level | 8     |
+
+        Examples:
+          | format      | outformat   | cname    | tname     | ename     |
+          | xml         | xml         | class    | type      | extratags |
+          | json        | json        | class    | type      | extratags |
+          | jsonv2      | json        | category | type      | extratags |
+          | geojson     | geojson     | category | type      | extratags |
+          | geocodejson | geocodejson | osm_key  | osm_value | extra     |
+
     Scenario Outline: Search with namedetails
         When sending v1/search with format <format>
           | q       | namedetails |

test/python/api/test_result_formatting_v1.py

@@ -12,6 +12,7 @@ For functional tests see BDD test suite.
 """
 import datetime as dt
 import json
+import xml.etree.ElementTree as ET
 
 import pytest
 
@@ -332,3 +333,98 @@ def test_search_details_keywords_address():
     assert js['keywords'] == {'address': [{'id': 23, 'token': 'foo'},
                                           {'id': 24, 'token': 'foo'}],
                               'name': []}
+
+
+# admin_level injection into extratags
+
+SEARCH_FORMATS = ['json', 'jsonv2', 'geojson', 'geocodejson', 'xml']
+
+
+@pytest.mark.parametrize('fmt', SEARCH_FORMATS)
+def test_search_extratags_boundary_administrative_injects_admin_level(fmt):
+    search = napi.SearchResult(napi.SourceTable.PLACEX,
+                               ('boundary', 'administrative'),
+                               napi.Point(1.0, 2.0),
+                               admin_level=6,
+                               extratags={'place': 'city'})
+
+    raw = v1_format.format_result(napi.SearchResults([search]), fmt,
+                                  {'extratags': True})
+
+    if fmt == 'xml':
+        root = ET.fromstring(raw)
+        tags = {tag.attrib['key']: tag.attrib['value']
+                for tag in root.find('.//extratags').findall('tag')}
+        assert tags['admin_level'] == '6'
+        assert tags['place'] == 'city'
+    else:
+        result = json.loads(raw)
+        if fmt == 'geocodejson':
+            extra = result['features'][0]['properties']['geocoding']['extra']
+        elif fmt == 'geojson':
+            extra = result['features'][0]['properties']['extratags']
+        else:
+            extra = result[0]['extratags']
+
+        assert extra['admin_level'] == '6'
+        assert extra['place'] == 'city'
+
+
+@pytest.mark.parametrize('fmt', SEARCH_FORMATS)
+def test_search_extratags_non_boundary_no_admin_level_injection(fmt):
+    search = napi.SearchResult(napi.SourceTable.PLACEX,
+                               ('place', 'city'),
+                               napi.Point(1.0, 2.0),
+                               admin_level=8,
+                               extratags={'place': 'city'})
+
+    raw = v1_format.format_result(napi.SearchResults([search]), fmt,
+                                  {'extratags': True})
+
+    if fmt == 'xml':
+        root = ET.fromstring(raw)
+        tags = {tag.attrib['key']: tag.attrib['value']
+                for tag in root.find('.//extratags').findall('tag')}
+        assert 'admin_level' not in tags
+        assert tags['place'] == 'city'
+    else:
+        result = json.loads(raw)
+        if fmt == 'geocodejson':
+            extra = result['features'][0]['properties']['geocoding']['extra']
+        elif fmt == 'geojson':
+            extra = result['features'][0]['properties']['extratags']
+        else:
+            extra = result[0]['extratags']
+
+        assert 'admin_level' not in extra
+        assert extra['place'] == 'city'
+
+
+@pytest.mark.parametrize('fmt', SEARCH_FORMATS)
+def test_search_extratags_boundary_admin_level_15_no_injection(fmt):
+    search = napi.SearchResult(napi.SourceTable.PLACEX,
+                               ('boundary', 'administrative'),
+                               napi.Point(1.0, 2.0),
+                               admin_level=15,
+                               extratags={'place': 'city'})
+
+    raw = v1_format.format_result(napi.SearchResults([search]), fmt,
+                                  {'extratags': True})
+
+    if fmt == 'xml':
+        root = ET.fromstring(raw)
+        tags = {tag.attrib['key']: tag.attrib['value']
+                for tag in root.find('.//extratags').findall('tag')}
+        assert 'admin_level' not in tags
+        assert tags['place'] == 'city'
+    else:
+        result = json.loads(raw)
+        if fmt == 'geocodejson':
+            extra = result['features'][0]['properties']['geocoding']['extra']
+        elif fmt == 'geojson':
+            extra = result['features'][0]['properties']['extratags']
+        else:
+            extra = result[0]['extratags']
+
+        assert 'admin_level' not in extra
+        assert extra['place'] == 'city'