From: Sarah Hoffmann Date: Mon, 7 May 2018 19:48:29 +0000 (+0200) Subject: Merge remote-tracking branch 'upstream/master' X-Git-Tag: deploy~328 X-Git-Url: https://git.openstreetmap.org/nominatim.git/commitdiff_plain/473bbb7234dc21a74b3d7ed0115a02ca3379014b?hp=275f3b26560a44099dff762776277812ef0f122f Merge remote-tracking branch 'upstream/master' --- diff --git a/lib/Geocode.php b/lib/Geocode.php index 1c84f14b..e9b304d2 100644 --- a/lib/Geocode.php +++ b/lib/Geocode.php @@ -658,7 +658,6 @@ class Geocode $this->oDB->getAll($sSQL), 'Could not get word tokens.' ); - $aWordFrequencyScores = array(); foreach ($aDatabaseWords as $aToken) { // Filter country tokens that do not match restricted countries. if ($this->aCountryCodes @@ -681,7 +680,6 @@ class Geocode } else { $aValidTokens[$aToken['word_token']] = array($aToken); } - $aWordFrequencyScores[$aToken['word_id']] = $aToken['search_name_count'] + 1; } // US ZIP+4 codes - if there is no token, merge in the 5-digit ZIP code @@ -781,7 +779,6 @@ class Geocode $aResults += $oSearch->query( $this->oDB, - $aWordFrequencyScores, $this->iMinAddressRank, $this->iMaxAddressRank, $this->iLimit diff --git a/lib/SearchDescription.php b/lib/SearchDescription.php index 6345f50f..5f01e01b 100644 --- a/lib/SearchDescription.php +++ b/lib/SearchDescription.php @@ -17,6 +17,8 @@ class SearchDescription private $sCountryCode = ''; /// List of word ids making up the name of the object. private $aName = array(); + /// True if the name is rare enough to force index use on name. + private $bRareName = false; /// List of word ids making up the address of the object. private $aAddress = array(); /// Subset of word ids of full words making up the address. @@ -292,6 +294,11 @@ class SearchDescription $oSearch = clone $this; $oSearch->iSearchRank++; $oSearch->aName = array($iWordID => $iWordID); + if (CONST_Search_NameOnlySearchFrequencyThreshold) { + $oSearch->bRareName = + $aSearchTerm['search_name_count'] + 1 + < CONST_Search_NameOnlySearchFrequencyThreshold; + } $aNewSearches[] = $oSearch; } } @@ -368,6 +375,13 @@ class SearchDescription $oSearch->iSearchRank += 2; } if ($aSearchTerm['search_name_count'] + 1 < CONST_Max_Word_Frequency) { + if (empty($this->aName) && CONST_Search_NameOnlySearchFrequencyThreshold) { + $oSearch->bRareName = + $aSearchTerm['search_name_count'] + 1 + < CONST_Search_NameOnlySearchFrequencyThreshold; + } else { + $oSearch->bRareName = false; + } $oSearch->aName[$iWordID] = $iWordID; } else { $oSearch->aNameNonSearch[$iWordID] = $iWordID; @@ -385,20 +399,16 @@ class SearchDescription /** * Query database for places that match this search. * - * @param object $oDB Database connection to use. - * @param mixed[] $aWordFrequencyScores Number of times tokens appears - * overall in a planet database. - * @param integer $iMinRank Minimum address rank to restrict - * search to. - * @param integer $iMaxRank Maximum address rank to restrict - * search to. - * @param integer $iLimit Maximum number of results. + * @param object $oDB Database connection to use. + * @param integer $iMinRank Minimum address rank to restrict search to. + * @param integer $iMaxRank Maximum address rank to restrict search to. + * @param integer $iLimit Maximum number of results. * * @return mixed[] An array with two fields: IDs contains the list of * matching place IDs and houseNumber the houseNumber * if appicable or -1 if not. */ - public function query(&$oDB, &$aWordFrequencyScores, $iMinRank, $iMaxRank, $iLimit) + public function query(&$oDB, $iMinRank, $iMaxRank, $iLimit) { $aResults = array(); $iHousenumber = -1; @@ -427,7 +437,6 @@ class SearchDescription // First search for places according to name and address. $aResults = $this->queryNamedPlace( $oDB, - $aWordFrequencyScores, $iMinRank, $iMaxRank, $iLimit @@ -579,12 +588,16 @@ class SearchDescription return $aResults; } - private function queryNamedPlace(&$oDB, $aWordFrequencyScores, $iMinAddressRank, $iMaxAddressRank, $iLimit) + private function queryNamedPlace(&$oDB, $iMinAddressRank, $iMaxAddressRank, $iLimit) { $aTerms = array(); $aOrder = array(); - if ($this->sHouseNumber && !empty($this->aAddress)) { + // Sort by existence of the requested house number but only if not + // too many results are expected for the street, i.e. if the result + // will be narrowed down by an address. Remeber that with ordering + // every single result has to be checked. + if ($this->sHouseNumber && (!empty($this->aAddress) || $this->sPostcode)) { $sHouseNumberRegex = '\\\\m'.$this->sHouseNumber.'\\\\M'; $aOrder[] = ' ('; $aOrder[0] .= 'EXISTS('; @@ -615,11 +628,7 @@ class SearchDescription } if (!empty($this->aAddress)) { // For infrequent name terms disable index usage for address - if (CONST_Search_NameOnlySearchFrequencyThreshold - && count($this->aName) == 1 - && $aWordFrequencyScores[$this->aName[reset($this->aName)]] - < CONST_Search_NameOnlySearchFrequencyThreshold - ) { + if ($this->bRareName) { $aTerms[] = 'array_cat(nameaddress_vector,ARRAY[]::integer[]) @> '.getArraySQL($this->aAddress); } else { $aTerms[] = 'nameaddress_vector @> '.getArraySQL($this->aAddress); diff --git a/lib/Status.php b/lib/Status.php new file mode 100644 index 00000000..86f5cac3 --- /dev/null +++ b/lib/Status.php @@ -0,0 +1,54 @@ +oDB =& $oDB; + } + + public function status() + { + if (!$this->oDB || PEAR::isError($this->oDB)) { + throw new Exception('No database', 700); + } + + $sStandardWord = $this->oDB->getOne("SELECT make_standard_name('a')"); + if (PEAR::isError($sStandardWord)) { + throw new Exception('Module failed', 701); + } + + if ($sStandardWord != 'a') { + throw new Exception('Module call failed', 702); + } + + $sSQL = 'SELECT word_id, word_token, word, class, type, country_code, '; + $sSQL .= "operator, search_name_count FROM word WHERE word_token IN (' a')"; + $iWordID = $this->oDB->getOne($sSQL); + if (PEAR::isError($iWordID)) { + throw new Exception('Query failed', 703); + } + if (!$iWordID) { + throw new Exception('No value', 704); + } + } + + public function dataDate() + { + $sSQL = 'SELECT EXTRACT(EPOCH FROM lastimportdate) FROM import_status LIMIT 1'; + $iDataDateEpoch = $this->oDB->getOne($sSQL); + + if (PEAR::isError($iDataDateEpoch)) { + throw Exception('Data date query failed '.$iDataDateEpoch->getMessage(), 705); + } + + return $iDataDateEpoch; + } +} diff --git a/test/bdd/api/status/failures.feature b/test/bdd/api/status/failures.feature new file mode 100644 index 00000000..9ef06532 --- /dev/null +++ b/test/bdd/api/status/failures.feature @@ -0,0 +1,17 @@ +@UNKNOWNDB +Feature: Status queries against unknown database + Testing status query + + Scenario: Failed status as text + When sending text status query + Then a HTTP 500 is returned + And the page contents equals "ERROR: No database" + + Scenario: Failed status as json + When sending json status query + Then a HTTP 200 is returned + And the result is valid json + And results contain + | status | message | + | 700 | No database | + And result has not attributes data_updated diff --git a/test/bdd/api/status/simple.feature b/test/bdd/api/status/simple.feature new file mode 100644 index 00000000..1323caa1 --- /dev/null +++ b/test/bdd/api/status/simple.feature @@ -0,0 +1,16 @@ +@APIDB +Feature: Status queries + Testing status query + + Scenario: Status as text + When sending status query + Then a HTTP 200 is returned + And the page contents equals "OK" + + Scenario: Status as json + When sending json status query + Then the result is valid json + And results contain + | status | message | + | 0 | OK | + And result has attributes data_updated diff --git a/test/bdd/environment.py b/test/bdd/environment.py index 2bea3180..162346de 100644 --- a/test/bdd/environment.py +++ b/test/bdd/environment.py @@ -122,6 +122,9 @@ class NominatimEnvironment(object): def setup_api_db(self, context): self.write_nominatim_config(self.api_test_db) + def setup_unknown_db(self, context): + self.write_nominatim_config('UNKNOWN_DATABASE_NAME') + def setup_db(self, context): self.setup_template_db() self.write_nominatim_config(self.test_db) @@ -261,6 +264,8 @@ def before_scenario(context, scenario): context.nominatim.setup_db(context) elif 'APIDB' in context.tags: context.nominatim.setup_api_db(context) + elif 'UNKNOWNDB' in context.tags: + context.nominatim.setup_unknown_db(context) context.scene = None def after_scenario(context, scenario): diff --git a/test/bdd/steps/queries.py b/test/bdd/steps/queries.py index 5779c763..7266be55 100644 --- a/test/bdd/steps/queries.py +++ b/test/bdd/steps/queries.py @@ -236,6 +236,20 @@ class DetailsResponse(GenericResponse): self.result = [json.JSONDecoder(object_pairs_hook=OrderedDict).decode(self.page)] +class StatusResponse(GenericResponse): + + def __init__(self, page, fmt='text', errorcode=200): + self.page = page + self.format = fmt + self.errorcode = errorcode + + if errorcode == 200 and fmt != 'text': + getattr(self, 'parse_' + fmt)() + + def parse_json(self): + self.result = [json.JSONDecoder(object_pairs_hook=OrderedDict).decode(self.page)] + + @when(u'searching for "(?P.*)"(?P with dups)?') def query_cmd(context, query, dups): """ Query directly via PHP script. @@ -402,6 +416,18 @@ def website_lookup_request(context, fmt, query): context.response = SearchResponse(outp, outfmt, status) +@when(u'sending (?P\S+ )?status query') +def website_status_request(context, fmt): + params = {} + outp, status = send_api_query('status', params, fmt, context) + + if fmt is None: + outfmt = 'text' + else: + outfmt = fmt.strip() + + context.response = StatusResponse(outp, outfmt, status) + @step(u'(?Pless than|more than|exactly|at least|at most) (?P\d+) results? (?:is|are) returned') def validate_result_number(context, operator, number): eq_(context.response.errorcode, 200) @@ -413,6 +439,10 @@ def validate_result_number(context, operator, number): def check_http_return_status(context, status): eq_(context.response.errorcode, int(status)) +@then(u'the page contents equals "(?P.+)"') +def check_page_content_equals(context, text): + eq_(context.response.page, text) + @then(u'the result is valid (?P\w+)') def step_impl(context, fmt): context.execute_steps("Then a HTTP 200 is returned") diff --git a/test/php/Nominatim/StatusTest.php b/test/php/Nominatim/StatusTest.php new file mode 100644 index 00000000..06823720 --- /dev/null +++ b/test/php/Nominatim/StatusTest.php @@ -0,0 +1,95 @@ +setExpectedException(Exception::class, 'No database', 700); + + $oDB = null; + $oStatus = new Status($oDB); + $this->assertEquals('No database', $oStatus->status()); + } + + public function testNoDatabaseConnectionFail() + { + $this->setExpectedException(Exception::class, 'No database', 700); + + // causes 'Non-static method should not be called statically, assuming $this from incompatible context' + // failure on travis + // $oDB = \DB::connect('', false); // returns a DB_Error instance + + $oDB = new \DB_Error; + $oStatus = new Status($oDB); + $this->assertEquals('No database', $oStatus->status()); + + $oDB = null; + $oStatus = new Status($oDB); + $this->assertEquals('No database', $oStatus->status()); + } + + + public function testModuleFail() + { + $this->setExpectedException(Exception::class, 'Module call failed', 702); + + // stub has getOne method but doesn't return anything + $oDbStub = $this->getMock(\DB::class, array('getOne')); + + $oStatus = new Status($oDbStub); + $this->assertNull($oStatus->status()); + } + + + public function testWordIdQueryFail() + { + $this->setExpectedException(Exception::class, 'No value', 704); + + $oDbStub = $this->getMock(\DB::class, array('getOne')); + + // return no word_id + $oDbStub->method('getOne') + ->will($this->returnCallback(function ($sql) { + if (preg_match("/make_standard_name\('a'\)/", $sql)) return 'a'; + if (preg_match('/SELECT word_id, word_token/', $sql)) return null; + })); + + $oStatus = new Status($oDbStub); + $this->assertNull($oStatus->status()); + } + + + public function testOK() + { + $oDbStub = $this->getMock(\DB::class, array('getOne')); + + $oDbStub->method('getOne') + ->will($this->returnCallback(function ($sql) { + if (preg_match("/make_standard_name\('(\w+)'\)/", $sql, $aMatch)) return $aMatch[1]; + if (preg_match('/SELECT word_id, word_token/', $sql)) return 1234; + })); + + $oStatus = new Status($oDbStub); + $this->assertNull($oStatus->status()); + } + + public function testDataDate() + { + $oDbStub = $this->getMock(\DB::class, array('getOne')); + + $oDbStub->method('getOne') + ->willReturn(1519430221); + + $oStatus = new Status($oDbStub); + $this->assertEquals(1519430221, $oStatus->dataDate()); + } +} diff --git a/website/status.php b/website/status.php index 124dc507..262fe8e0 100644 --- a/website/status.php +++ b/website/status.php @@ -1,37 +1,51 @@ getSet('format', array('text', 'json'), 'text'); -function statusError($sMsg) -{ - header('HTTP/1.0 500 Internal Server Error'); - echo 'ERROR: '.$sMsg; - exit; -} +$oDB = DB::connect(CONST_Database_DSN, false); +$oStatus = new Nominatim\Status($oDB); -$oDB =& DB::connect(CONST_Database_DSN, false); -if (!$oDB || PEAR::isError($oDB)) { - statusError('No database'); -} -$sStandardWord = $oDB->getOne("select make_standard_name('a')"); -if (PEAR::isError($sStandardWord)) { - statusError('Module failed'); -} -if ($sStandardWord != 'a') { - statusError('Module call failed'); +if ($sOutputFormat == 'json') { + header('content-type: application/json; charset=UTF-8'); } -$iWordID = $oDB->getOne("select word_id,word_token, word, class, type, country_code, operator, search_name_count from word where word_token in (' a')"); -if (PEAR::isError($iWordID)) { - statusError('Query failed'); + +try { + $oStatus->status(); +} catch (Exception $oErr) { + if ($sOutputFormat == 'json') { + $aResponse = array( + 'status' => $oErr->getCode(), + 'message' => $oErr->getMessage() + ); + javascript_renderData($aResponse); + } else { + header('HTTP/1.0 500 Internal Server Error'); + echo 'ERROR: '.$oErr->getMessage(); + } + exit; } -if (!$iWordID) { - statusError('No value'); + + +if ($sOutputFormat == 'json') { + $epoch = $oStatus->dataDate(); + $aResponse = array( + 'status' => 0, + 'message' => 'OK', + 'data_updated' => (new DateTime('@'.$epoch))->format(DateTime::RFC3339) + ); + javascript_renderData($aResponse); +} else { + echo 'OK'; } -echo 'OK'; exit;