Merge branch 'master' into country-names-yaml-configuration

docs/admin/Installation.md

@@ -129,7 +129,7 @@ If you want to install latest development version from github, make sure to
 also check out the osm2pgsql subproject:
 
 ```
-git clone --recursive git://github.com/openstreetmap/Nominatim.git
+git clone --recursive https://github.com/openstreetmap/Nominatim.git
 ```
 
 The development version does not include the country grid. Download it separately:

lib-php/ParameterParser.php

@@ -114,22 +114,28 @@ class ParameterParser
         }
 
         foreach ($aLanguages as $sLanguage => $fLanguagePref) {
-            $aLangPrefOrder[$sLanguage] = $sLanguage;
+            $this->addNameTag($aLangPrefOrder, $sLanguage);
         }
- 
-        $aLangPrefOrder['default'] = 'default';
-        $aLangPrefOrder['brand'] = 'brand';
+        $this->addNameTag($aLangPrefOrder, 'default');
+        $this->addNameTag($aLangPrefOrder, 'brand');
+
         foreach ($aLanguages as $sLanguage => $fLanguagePref) {
-            $aLangPrefOrder['official_name:'.$sLanguage] = 'official_name:'.$sLanguage;
-            $aLangPrefOrder['short_name:'.$sLanguage] = 'short_name:'.$sLanguage;
+            $this->addNameTag($aLangPrefOrder, 'official_name:'.$sLanguage);
+            $this->addNameTag($aLangPrefOrder, 'short_name:'.$sLanguage);
         }
-        $aLangPrefOrder['official_name'] = 'official_name';
-        $aLangPrefOrder['short_name'] = 'short_name';
-        $aLangPrefOrder['ref'] = 'ref';
-        $aLangPrefOrder['type'] = 'type';
+        $this->addNameTag($aLangPrefOrder, 'official_name');
+        $this->addNameTag($aLangPrefOrder, 'short_name');
+        $this->addNameTag($aLangPrefOrder, 'ref');
+        $this->addNameTag($aLangPrefOrder, 'type');
         return $aLangPrefOrder;
     }
 
+    private function addNameTag(&$aLangPrefOrder, $sTag)
+    {
+        $aLangPrefOrder[$sTag] = $sTag;
+        $aLangPrefOrder['_place_'.$sTag] = '_place_'.$sTag;
+    }
+
     public function hasSetAny($aParamNames)
     {
         foreach ($aParamNames as $sName) {

lib-php/PlaceLookup.php

@@ -452,11 +452,7 @@ class PlaceLookup
             }
 
             if ($this->bNameDetails) {
-                if ($aPlace['names']) {
-                    $aPlace['sNameDetails'] = json_decode($aPlace['names']);
-                } else {
-                    $aPlace['sNameDetails'] = (object) array();
-                }
+                $aPlace['sNameDetails'] = $this->extractNames($aPlace['names']);
             }
 
             $aPlace['addresstype'] = ClassTypes\getLabelTag(
@@ -479,6 +475,33 @@ class PlaceLookup
         return $aResults;
     }
 
+
+    private function extractNames($sNames)
+    {
+        if (!$sNames) {
+            return (object) array();
+        }
+
+        $aFullNames = json_decode($sNames);
+        $aNames = array();
+
+        foreach ($aFullNames as $sKey => $sValue) {
+            if (strpos($sKey, '_place_') === 0) {
+                $sSubKey = substr($sKey, 7);
+                if (array_key_exists($sSubKey, $aFullNames)) {
+                    $aNames[$sKey] = $sValue;
+                } else {
+                    $aNames[$sSubKey] = $sValue;
+                }
+            } else {
+                $aNames[$sKey] = $sValue;
+            }
+        }
+
+        return $aNames;
+    }
+
+
     /* returns an array which will contain the keys
      *   aBoundingBox
      * and may also contain one or more of the keys
@@ -489,8 +512,6 @@ class PlaceLookup
      *   lat
      *   lon
      */
-
-
     public function getOutlines($iPlaceID, $fLon = null, $fLat = null, $fRadius = null, $fLonReverse = null, $fLatReverse = null)
     {
 

lib-php/ReverseGeocode.php

@@ -64,7 +64,9 @@ class ReverseGeocode
     {
         Debug::newFunction('lookupInterpolation');
         $sSQL = 'SELECT place_id, parent_place_id, 30 as rank_search,';
-        $sSQL .= '  (endnumber - startnumber) * ST_LineLocatePoint(linegeo,'.$sPointSQL.') as fhnr,';
+        $sSQL .= '  (CASE WHEN endnumber != startnumber';
+        $sSQL .= '        THEN (endnumber - startnumber) * ST_LineLocatePoint(linegeo,'.$sPointSQL.')';
+        $sSQL .= '        ELSE startnumber END) as fhnr,';
         $sSQL .= '  startnumber, endnumber, step,';
         $sSQL .= '  ST_Distance(linegeo,'.$sPointSQL.') as distance';
         $sSQL .= ' FROM location_property_osmline';

lib-php/init-website.php

@@ -26,7 +26,7 @@ function userError($sMsg)
 
 function exception_handler_json($exception)
 {
-    http_response_code($exception->getCode());
+    http_response_code($exception->getCode() == 0 ? 500 : $exception->getCode());
     header('Content-type: application/json; charset=utf-8');
     include(CONST_LibDir.'/template/error-json.php');
     exit();
@@ -34,7 +34,7 @@ function exception_handler_json($exception)
 
 function exception_handler_xml($exception)
 {
-    http_response_code($exception->getCode());
+    http_response_code($exception->getCode() == 0 ? 500 : $exception->getCode());
     header('Content-type: text/xml; charset=utf-8');
     echo '<?xml version="1.0" encoding="UTF-8" ?>'."\n";
     include(CONST_LibDir.'/template/error-xml.php');

lib-sql/functions/placex_triggers.sql

@@ -26,6 +26,7 @@ CREATE OR REPLACE FUNCTION placex_indexing_prepare(p placex)
 DECLARE
   location RECORD;
   result prepare_update_info;
+  extra_names HSTORE;
 BEGIN
   -- For POI nodes, check if the address should be derived from a surrounding
   -- building.
@@ -58,8 +59,11 @@ BEGIN
     END LOOP;
   END IF;
 
+  -- remove internal and derived names
   result.address := result.address - '_unlisted_place'::TEXT;
-  result.name := p.name;
+  SELECT hstore(array_agg(key), array_agg(value)) INTO result.name
+    FROM each(p.name) WHERE key not like '\_%';
+
   result.class := p.class;
   result.type := p.type;
   result.country_code := p.country_code;
@@ -72,8 +76,20 @@ BEGIN
   IF location.place_id is not NULL THEN
     result.linked_place_id := location.place_id;
 
-    IF NOT location.name IS NULL THEN
-      result.name := location.name || result.name;
+    IF location.name is not NULL THEN
+      {% if debug %}RAISE WARNING 'Names original: %, location: %', result.name, location.name;{% endif %}
+      -- Add all names from the place nodes that deviate from the name
+      -- in the relation with the prefix '_place_'. Deviation means that
+      -- either the value is different or a given key is missing completely
+      SELECT hstore(array_agg('_place_' || key), array_agg(value)) INTO extra_names
+        FROM each(location.name - result.name);
+      {% if debug %}RAISE WARNING 'Extra names: %', extra_names;{% endif %}
+
+      IF extra_names is not null THEN
+          result.name := result.name || extra_names;
+      END IF;
+
+      {% if debug %}RAISE WARNING 'Final names: %', result.name;{% endif %}
     END IF;
   END IF;
 

nominatim/tools/replication.py

@@ -47,7 +47,7 @@ def init_replication(conn, base_url):
 
     status.set_status(conn, date=date, seq=seq)
 
-    LOG.warning("Updates intialised at sequence %s (%s)", seq, date)
+    LOG.warning("Updates initialised at sequence %s (%s)", seq, date)
 
 
 def check_for_updates(conn, base_url):

test/bdd/db/query/interpolation.feature

@@ -0,0 +1,58 @@
+@DB
+Feature: Query of address interpolations
+    Tests that interpolated addresses can be queried correctly
+
+    Background:
+        Given the grid
+          | 1  |  | 2  |  | 3  |
+          | 10 |  | 12 |  | 13 |
+          | 7  |  | 8  |  | 9  |
+
+    Scenario: Find interpolations with single number
+        Given the places
+          | osm | class   | type    | name    | geometry |
+          | W10 | highway | primary | Nickway | 10,12,13 |
+        And the places
+          | osm | class | type   | addr+interpolation | geometry |
+          | W1  | place | houses | odd                | 1,3      |
+        And the places
+          | osm | class | type  | housenr | geometry |
+          | N1  | place | house | 1       | 1        |
+          | N3  | place | house | 5       | 3        |
+        And the ways
+          | id | nodes |
+          | 1  | 1,3   |
+        When importing
+        When sending jsonv2 reverse point 2
+        Then results contain
+          | ID | display_name |
+          | 0  | 3, Nickway   |
+        When sending search query "Nickway 3"
+        Then results contain
+          | osm | display_name |
+          | W1  | 3, Nickway   |
+
+
+    Scenario: Find interpolations with multiple numbers
+        Given the places
+          | osm | class   | type    | name    | geometry |
+          | W10 | highway | primary | Nickway | 10,12,13 |
+        And the places
+          | osm | class | type   | addr+interpolation | geometry |
+          | W1  | place | houses | even               | 1,3      |
+        And the places
+          | osm | class | type  | housenr | geometry |
+          | N1  | place | house | 2       | 1        |
+          | N3  | place | house | 16      | 3        |
+        And the ways
+          | id | nodes |
+          | 1  | 1,3   |
+        When importing
+        When sending jsonv2 reverse point 2
+        Then results contain
+          | ID | display_name | centroid |
+          | 0  | 10, Nickway  | 2 |
+        When sending search query "Nickway 10"
+        Then results contain
+          | osm | display_name  | centroid |
+          | W1  | 10, Nickway   | 2 |

test/bdd/db/query/linking.feature

@@ -17,6 +17,40 @@ Feature: Searching linked places
          | object  | linked_place_id |
          | N2      | R13 |
         When sending search query "Vario"
+         | namedetails |
+         | 1 |
         Then results contain
-         | osm |
-         | R13 |
+         | osm | display_name | namedetails |
+         | R13 | Garbo | "name": "Garbo", "name:it": "Vario" |
+        When sending search query "Vario"
+         | accept-language |
+         | it |
+        Then results contain
+         | osm | display_name |
+         | R13 | Vario |
+
+
+    Scenario: Differing names from linked places are searchable
+        Given the places
+         | osm  | class    | type           | admin | name  | geometry |
+         | R13  | boundary | administrative | 6     | Garbo | poly-area:0.1 |
+        Given the places
+         | osm  | class    | type           | admin | name  | geometry |
+         | N2   | place    | hamlet         | 15    | Vario | 0.006 0.00001 |
+        And the relations
+         | id | members       | tags+type |
+         | 13 | N2:label      | boundary |
+        When importing
+        Then placex contains
+         | object  | linked_place_id |
+         | N2      | R13 |
+        When sending search query "Vario"
+         | namedetails |
+         | 1 |
+        Then results contain
+         | osm | display_name | namedetails |
+         | R13 | Garbo        | "name": "Garbo", "_place_name": "Vario" |
+        When sending search query "Garbo"
+        Then results contain
+         | osm | display_name |
+         | R13 | Garbo |

test/bdd/db/update/linked_places.feature

@@ -117,8 +117,10 @@ Feature: Updates of linked places
          | 1  | N3:label |
         When importing
         Then placex contains
-         | object | linked_place_id | name+name:de |
+         | object | linked_place_id | name+_place_name:de |
          | R1     | -               | pnt  |
+        And placex contains
+         | object | linked_place_id | name+name:de |
          | N3     | R1              | pnt  |
         When updating places
          | osm | class    | type        | name+name:de | admin | geometry |
@@ -126,8 +128,43 @@ Feature: Updates of linked places
         Then placex contains
          | object | linked_place_id | name+name:de |
          | N3     | R1              | newname  |
+        And placex contains
+         | object | linked_place_id | name+_place_name:de |
          | R1     | -               | newname  |
 
+    Scenario: Update linking relation when linkee name is deleted
+        Given the places
+         | osm | class    | type           | name | admin | geometry |
+         | R1  | boundary | administrative | rel  | 8     | poly-area:0.1 |
+        And the places
+         | osm | class    | type           | name | admin | geometry |
+         | N3  | place    | city           | pnt  | 30    | 0.00001 0 |
+        And the relations
+         | id | members  |
+         | 1  | N3:label |
+        When importing
+        Then placex contains
+         | object | linked_place_id | name+_place_name | name+name |
+         | R1     | -               | pnt              | rel       |
+        And placex contains
+         | object | linked_place_id | name+name |
+         | N3     | R1              | pnt  |
+        When sending search query "pnt"
+        Then results contain
+          | osm |
+          | R1  |
+        When updating places
+         | osm | class    | type        | name+name:de | admin | geometry |
+         | N3  | place    | city        | depnt        | 30    | 0.00001 0 |
+        Then placex contains
+         | object | linked_place_id | name+name:de |
+         | N3     | R1              | depnt  |
+        And placex contains
+         | object | linked_place_id | name+_place_name:de | name+name |
+         | R1     | -               | depnt               | rel       |
+        When sending search query "pnt"
+        Then exactly 0 results are returned
+
     Scenario: Updating linkee extratags keeps linker's extratags
         Given the named places
          | osm | class    | type           | extra+wikidata | admin | geometry |

test/bdd/steps/http_responses.py

@@ -72,13 +72,14 @@ class GenericResponse:
             self.header['json_func'] = m.group(1)
         self.result = json.JSONDecoder(object_pairs_hook=OrderedDict).decode(code)
         if isinstance(self.result, OrderedDict):
-            self.result = [self.result]
+            if 'error' in self.result:
+                self.result = []
+            else:
+                self.result = [self.result]
 
     def _parse_geojson(self):
         self._parse_json()
-        if 'error' in self.result[0]:
-            self.result = []
-        else:
+        if self.result:
             self.result = list(map(_geojson_result_to_json_result, self.result[0]['features']))
 
     def _parse_geocodejson(self):
@@ -101,6 +102,9 @@ class GenericResponse:
         elif value.startswith("^"):
             assert re.fullmatch(value, self.result[idx][field]), \
                    BadRowValueAssert(self, idx, field, value)
+        elif isinstance(self.result[idx][field], OrderedDict):
+            assert self.result[idx][field] == eval('{' + value + '}'), \
+                   BadRowValueAssert(self, idx, field, value)
         else:
             assert str(self.result[idx][field]) == str(value), \
                    BadRowValueAssert(self, idx, field, value)
@@ -128,7 +132,7 @@ class GenericResponse:
                    "\nBad value for row {} field '{}' in address. Expected: {}, got: {}.\nFull address: {}"""\
                        .format(idx, field, value, address[field], json.dumps(address, indent=4))
 
-    def match_row(self, row):
+    def match_row(self, row, context=None):
         """ Match the result fields against the given behave table row.
         """
         if 'ID' in row.headings:
@@ -151,7 +155,12 @@ class GenericResponse:
                     assert self.result[i]['osm_type'] in (OSM_TYPE[value[0]], value[0]), \
                            BadRowValueAssert(self, i, 'osm_type', value)
                 elif name == 'centroid':
-                    lon, lat = value.split(' ')
+                    if ' ' in value:
+                        lon, lat = value.split(' ')
+                    elif context is not None:
+                        lon, lat = context.osm.grid_node(int(value))
+                    else:
+                        raise RuntimeError("Context needed when using grid coordinates")
                     self.assert_field(i, 'lat', float(lat))
                     self.assert_field(i, 'lon', float(lon))
                 else:

test/bdd/steps/steps_api_queries.py

@@ -153,6 +153,20 @@ def website_reverse_request(context, fmt, lat, lon):
 
     context.response = ReverseResponse(outp, fmt or 'xml', status)
 
+@when(u'sending (?P<fmt>\S+ )?reverse point (?P<nodeid>.+)')
+def website_reverse_request(context, fmt, nodeid):
+    params = {}
+    if fmt and fmt.strip() == 'debug':
+        params['debug'] = '1'
+    params['lon'], params['lat'] = (f'{c:f}' for c in context.osm.grid_node(int(nodeid)))
+
+
+    outp, status = send_api_query('reverse', params, fmt, context)
+
+    context.response = ReverseResponse(outp, fmt or 'xml', status)
+
+
+
 @when(u'sending (?P<fmt>\S+ )?details query for (?P<query>.*)')
 def website_details_request(context, fmt, query):
     params = {}
@@ -238,7 +252,7 @@ def step_impl(context):
     context.execute_steps("then at least 1 result is returned")
 
     for line in context.table:
-        context.response.match_row(line)
+        context.response.match_row(line, context=context)
 
 @then(u'result (?P<lid>\d+ )?has (?P<neg>not )?attributes (?P<attrs>.*)')
 def validate_attributes(context, lid, neg, attrs):

test/bdd/steps/steps_db_ops.py

@@ -93,6 +93,7 @@ def add_data_to_planet_ways(context):
 def import_and_index_data_from_place_table(context):
     """ Import data previously set up in the place table.
     """
+    context.nominatim.run_nominatim('refresh', '--functions')
     context.nominatim.run_nominatim('import', '--continue', 'load-data',
                                               '--index-noanalyse', '-q')
 

test/php/Nominatim/ParameterParserTest.php

@@ -184,52 +184,24 @@ class ParameterParserTest extends \PHPUnit\Framework\TestCase
         $oParams = new ParameterParser(array('accept-language' => ''));
         $this->assertSame(array(
                            'default' => 'default',
-                           'brand' => 'brand',
-                           'official_name:default' => 'official_name:default',
-                           'short_name:default' => 'short_name:default',
-                           'official_name' => 'official_name',
-                           'short_name' => 'short_name',
-                           'ref' => 'ref',
-                           'type' => 'type'
-                          ), $oParams->getPreferredLanguages('default'));
+                          ), array_slice($oParams->getPreferredLanguages('default'), 0, 4));
 
         $oParams = new ParameterParser(array('accept-language' => 'de,en'));
         $this->assertSame(array(
                            'de' => 'de',
                            'en' => 'en',
-                           'name' => 'name',
-                           'brand' => 'brand',
-                           'official_name:de' => 'official_name:de',
-                           'short_name:de' => 'short_name:de',
-                           'official_name:en' => 'official_name:en',
-                           'short_name:en' => 'short_name:en',
-                           'official_name' => 'official_name',
-                           'short_name' => 'short_name',
-                           'ref' => 'ref',
-                           'type' => 'type'
-                          ), $oParams->getPreferredLanguages('default'));
+                           'default' => 'default',
+                          ), array_slice($oParams->getPreferredLanguages('default'), 0, 6));
 
         $oParams = new ParameterParser(array('accept-language' => 'fr-ca,fr;q=0.8,en-ca;q=0.5,en;q=0.3'));
         $this->assertSame(array(
                            'fr-ca' => 'fr-ca',
                            'fr' => 'fr',
+                           'fr' => 'fr',
                            'en-ca' => 'en-ca',
                            'en' => 'en',
-                           'name' => 'name',
-                           'brand' => 'brand',
-                           'official_name:fr-ca' => 'official_name:fr-ca',
-                           'short_name:fr-ca' => 'short_name:fr-ca',
-                           'official_name:fr' => 'official_name:fr',
-                           'short_name:fr' => 'short_name:fr',
-                           'official_name:en-ca' => 'official_name:en-ca',
-                           'short_name:en-ca' => 'short_name:en-ca',
-                           'official_name:en' => 'official_name:en',
-                           'short_name:en' => 'short_name:en',
-                           'official_name' => 'official_name',
-                           'short_name' => 'short_name',
-                           'ref' => 'ref',
-                           'type' => 'type',
-                          ), $oParams->getPreferredLanguages('default'));
+                           'default' => 'default',
+                          ), array_slice($oParams->getPreferredLanguages('default'), 0, 10));
 
         $oParams = new ParameterParser(array('accept-language' => 'ja_rm,zh_pinyin'));
         $this->assertSame(array(
@@ -237,21 +209,8 @@ class ParameterParserTest extends \PHPUnit\Framework\TestCase
                            'zh_pinyin' => 'zh_pinyin',
                            'ja' => 'ja',
                            'zh' => 'zh',
-                           'name' => 'name',
-                           'brand' => 'brand',
-                           'official_name:ja_rm' => 'official_name:ja_rm',
-                           'short_name:ja_rm' => 'short_name:ja_rm',
-                           'official_name:zh_pinyin' => 'official_name:zh_pinyin',
-                           'short_name:zh_pinyin' => 'short_name:zh_pinyin',
-                           'official_name:ja' => 'official_name:ja',
-                           'short_name:ja' => 'short_name:ja',
-                           'official_name:zh' => 'official_name:zh',
-                           'short_name:zh' => 'short_name:zh',
-                           'official_name' => 'official_name',
-                           'short_name' => 'short_name',
-                           'ref' => 'ref',
-                           'type' => 'type',
-                          ), $oParams->getPreferredLanguages('default'));
+                           'default' => 'default',
+                          ), array_slice($oParams->getPreferredLanguages('default'), 0, 10));
     }
 
     public function testHasSetAny()

vagrant/Install-on-Centos-8.sh

@@ -125,7 +125,7 @@ fi                                    #DOCS:
 #
 if [ "x$1" == "xyes" ]; then  #DOCS:    :::sh
     cd $USERHOME
-    git clone --recursive git://github.com/openstreetmap/Nominatim.git
+    git clone --recursive https://github.com/openstreetmap/Nominatim.git
     cd Nominatim
 else                               #DOCS:
     cd $USERHOME/Nominatim         #DOCS:

vagrant/Install-on-Ubuntu-18.sh

@@ -105,7 +105,7 @@ fi                                    #DOCS:
 #
 if [ "x$1" == "xyes" ]; then  #DOCS:    :::sh
     cd $USERHOME
-    git clone --recursive git://github.com/openstreetmap/Nominatim.git
+    git clone --recursive https://github.com/openstreetmap/Nominatim.git
     cd Nominatim
 else                               #DOCS:
     cd $USERHOME/Nominatim         #DOCS:

vagrant/Install-on-Ubuntu-20.sh

@@ -99,7 +99,7 @@ fi                                    #DOCS:
 #
 if [ "x$1" == "xyes" ]; then  #DOCS:    :::sh
     cd $USERHOME
-    git clone --recursive git://github.com/openstreetmap/Nominatim.git
+    git clone --recursive https://github.com/openstreetmap/Nominatim.git
     cd Nominatim
 else                               #DOCS:
     cd $USERHOME/Nominatim         #DOCS: