diff --git a/lib-php/ParameterParser.php b/lib-php/ParameterParser.php index dd637722..98b95388 100644 --- a/lib-php/ParameterParser.php +++ b/lib-php/ParameterParser.php @@ -114,21 +114,27 @@ class ParameterParser } foreach ($aLanguages as $sLanguage => $fLanguagePref) { - $aLangPrefOrder['name:'.$sLanguage] = 'name:'.$sLanguage; + $this->addNameTag($aLangPrefOrder, 'name:'.$sLanguage); } - $aLangPrefOrder['name'] = 'name'; - $aLangPrefOrder['brand'] = 'brand'; + $this->addNameTag($aLangPrefOrder, 'name'); + $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/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/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 fa841d25..3b9f59eb 100644 --- a/test/bdd/steps/http_responses.py +++ b/test/bdd/steps/http_responses.py @@ -62,8 +62,6 @@ class GenericResponse: if errorcode == 200 and fmt != 'debug': getattr(self, '_parse_' + fmt)() - else: - print("Bad response: ", page) def _parse_json(self): m = re.fullmatch(r'([\w$][^(]*)\((.*)\)', self.page) @@ -74,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): @@ -103,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) 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 57cf5f35..1488c987 100644 --- a/test/php/Nominatim/ParameterParserTest.php +++ b/test/php/Nominatim/ParameterParserTest.php @@ -184,75 +184,48 @@ class ParameterParserTest extends \PHPUnit\Framework\TestCase $oParams = new ParameterParser(array('accept-language' => '')); $this->assertSame(array( 'name:default' => 'name:default', + '_place_name:default' => '_place_name:default', 'name' => 'name', - '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')); + '_place_name' => '_place_name' + ), array_slice($oParams->getPreferredLanguages('default'), 0, 4)); $oParams = new ParameterParser(array('accept-language' => 'de,en')); $this->assertSame(array( 'name:de' => 'name:de', + '_place_name:de' => '_place_name:de', 'name:en' => 'name:en', + '_place_name:en' => '_place_name: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')); + '_place_name' => '_place_name' + ), 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( 'name:fr-ca' => 'name:fr-ca', + '_place_name:fr-ca' => '_place_name:fr-ca', 'name:fr' => 'name:fr', + '_place_name:fr' => '_place_name:fr', 'name:en-ca' => 'name:en-ca', + '_place_name:en-ca' => '_place_name:en-ca', 'name:en' => 'name:en', + '_place_name:en' => '_place_name: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')); + '_place_name' => '_place_name' + ), array_slice($oParams->getPreferredLanguages('default'), 0, 10)); $oParams = new ParameterParser(array('accept-language' => 'ja_rm,zh_pinyin')); $this->assertSame(array( 'name:ja_rm' => 'name:ja_rm', + '_place_name:ja_rm' => '_place_name:ja_rm', 'name:zh_pinyin' => 'name:zh_pinyin', + '_place_name:zh_pinyin' => '_place_name:zh_pinyin', 'name:ja' => 'name:ja', + '_place_name:ja' => '_place_name:ja', 'name:zh' => 'name:zh', + '_place_name:zh' => '_place_name: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')); + '_place_name' => '_place_name' + ), array_slice($oParams->getPreferredLanguages('default'), 0, 10)); } public function testHasSetAny()