diff --git a/docs/admin/Installation.md b/docs/admin/Installation.md index 19ad2dbb..00c7ca29 100644 --- a/docs/admin/Installation.md +++ b/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: diff --git a/lib-php/ParameterParser.php b/lib-php/ParameterParser.php index 46e69501..90e0cee3 100644 --- a/lib-php/ParameterParser.php +++ b/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) { diff --git a/lib-php/PlaceLookup.php b/lib-php/PlaceLookup.php index 120f5543..715f1ced 100644 --- a/lib-php/PlaceLookup.php +++ b/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) { diff --git a/lib-php/ReverseGeocode.php b/lib-php/ReverseGeocode.php index 646c592b..35103aeb 100644 --- a/lib-php/ReverseGeocode.php +++ b/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'; diff --git a/lib-php/init-website.php b/lib-php/init-website.php index 770c245d..60367503 100644 --- a/lib-php/init-website.php +++ b/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 ''."\n"; include(CONST_LibDir.'/template/error-xml.php'); diff --git a/lib-sql/functions/placex_triggers.sql b/lib-sql/functions/placex_triggers.sql index 1eae353e..9463bb27 100644 --- a/lib-sql/functions/placex_triggers.sql +++ b/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; diff --git a/nominatim/tools/replication.py b/nominatim/tools/replication.py index cbbf0585..fbd33e39 100644 --- a/nominatim/tools/replication.py +++ b/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): diff --git a/test/bdd/db/query/interpolation.feature b/test/bdd/db/query/interpolation.feature new file mode 100644 index 00000000..602ac434 --- /dev/null +++ b/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 | diff --git a/test/bdd/db/query/linking.feature b/test/bdd/db/query/linking.feature index 4e6c47d8..bd8e1da0 100644 --- a/test/bdd/db/query/linking.feature +++ b/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 | diff --git a/test/bdd/db/update/linked_places.feature b/test/bdd/db/update/linked_places.feature index 7a0fa21a..99614b7f 100644 --- a/test/bdd/db/update/linked_places.feature +++ b/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 | diff --git a/test/bdd/steps/http_responses.py b/test/bdd/steps/http_responses.py index fa6ab7fb..3b9f59eb 100644 --- a/test/bdd/steps/http_responses.py +++ b/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: diff --git a/test/bdd/steps/steps_api_queries.py b/test/bdd/steps/steps_api_queries.py index 0fda8f08..22517338 100644 --- a/test/bdd/steps/steps_api_queries.py +++ b/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\S+ )?reverse point (?P.+)') +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\S+ )?details query for (?P.*)') 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\d+ )?has (?Pnot )?attributes (?P.*)') def validate_attributes(context, lid, neg, attrs): diff --git a/test/bdd/steps/steps_db_ops.py b/test/bdd/steps/steps_db_ops.py index 8df5d617..e02cad8f 100644 --- a/test/bdd/steps/steps_db_ops.py +++ b/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') diff --git a/test/php/Nominatim/ParameterParserTest.php b/test/php/Nominatim/ParameterParserTest.php index cb7b0e81..2239b844 100644 --- a/test/php/Nominatim/ParameterParserTest.php +++ b/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() diff --git a/vagrant/Install-on-Centos-8.sh b/vagrant/Install-on-Centos-8.sh index 4877e0ad..c9278f9e 100755 --- a/vagrant/Install-on-Centos-8.sh +++ b/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: diff --git a/vagrant/Install-on-Ubuntu-18.sh b/vagrant/Install-on-Ubuntu-18.sh index a1a1fe30..40ee7ba8 100755 --- a/vagrant/Install-on-Ubuntu-18.sh +++ b/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: diff --git a/vagrant/Install-on-Ubuntu-20.sh b/vagrant/Install-on-Ubuntu-20.sh index 1fbabf24..68bd6b04 100755 --- a/vagrant/Install-on-Ubuntu-20.sh +++ b/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: