diff --git a/search/geocoder.cpp b/search/geocoder.cpp index 5316b95306..cf00d45dc3 100644 --- a/search/geocoder.cpp +++ b/search/geocoder.cpp @@ -356,6 +356,14 @@ void Geocoder::SetParams(Params const & params) m_params = params; + auto const MakeRequest = [this](size_t i, auto & request) + { + FillRequestFromToken(m_params.GetToken(i), request); + for (auto const & index : m_params.GetTypeIndices(i)) + request.m_categories.emplace_back(FeatureTypeToString(index)); + request.SetLangs(m_params.GetLangs()); + }; + m_tokenRequests.clear(); m_prefixTokenRequest.Clear(); for (size_t i = 0; i < m_params.GetNumTokens(); ++i) @@ -363,19 +371,11 @@ void Geocoder::SetParams(Params const & params) if (!m_params.IsPrefixToken(i)) { m_tokenRequests.emplace_back(); - auto & request = m_tokenRequests.back(); - FillRequestFromToken(m_params.GetToken(i), request); - for (auto const & index : m_params.GetTypeIndices(i)) - request.m_categories.emplace_back(FeatureTypeToString(index)); - request.SetLangs(m_params.GetLangs()); + MakeRequest(i, m_tokenRequests.back()); } else { - auto & request = m_prefixTokenRequest; - FillRequestFromToken(m_params.GetToken(i), request); - for (auto const & index : m_params.GetTypeIndices(i)) - request.m_categories.emplace_back(FeatureTypeToString(index)); - request.SetLangs(m_params.GetLangs()); + MakeRequest(i, m_prefixTokenRequest); } } @@ -1205,16 +1205,24 @@ void Geocoder::GreedilyMatchStreets(BaseContext & ctx, CentersFilter const & cen { TRACE(GreedilyMatchStreets); - // Match streets without suburbs. - vector predictions; - StreetsMatcher::Go(ctx, ctx.m_streets, *m_filter, m_params, predictions); - - for (auto const & prediction : predictions) - CreateStreetsLayerAndMatchLowerLayers(ctx, prediction, centers); + ProcessStreets(ctx, centers, ctx.m_streets); GreedilyMatchStreetsWithSuburbs(ctx, centers); } +void Geocoder::ProcessStreets(BaseContext & ctx, CentersFilter const & centers, CBV const & streets) +{ + using PredictionT = StreetsMatcher::Prediction; + vector predictions; + StreetsMatcher::Go(ctx, streets, *m_filter, m_params, predictions); + + /// @todo Iterating from best to worst predictions here. + /// Together with street fallback in CreateStreetsLayerAndMatchLowerLayers worst predictions + /// may produce controversial results (like matching streets by very common tokens). + for (size_t i = 0; i < predictions.size(); ++i) + CreateStreetsLayerAndMatchLowerLayers(ctx, predictions[i], centers); +} + void Geocoder::GreedilyMatchStreetsWithSuburbs(BaseContext & ctx, CentersFilter const & centers) { TRACE(GreedilyMatchStreetsWithSuburbs); @@ -1256,11 +1264,7 @@ void Geocoder::GreedilyMatchStreetsWithSuburbs(BaseContext & ctx, CentersFilter auto const suburbCBV = RetrieveGeometryFeatures(*m_context, rect, RectId::Suburb); auto const suburbStreets = ctx.m_streets.Intersect(suburbCBV); - vector predictions; - StreetsMatcher::Go(ctx, suburbStreets, *m_filter, m_params, predictions); - - for (auto const & prediction : predictions) - CreateStreetsLayerAndMatchLowerLayers(ctx, prediction, centers); + ProcessStreets(ctx, centers, suburbStreets); MatchPOIsAndBuildings(ctx, 0 /* curToken */, suburbCBV); }); @@ -1367,6 +1371,7 @@ void Geocoder::CreateStreetsLayerAndMatchLowerLayers(BaseContext & ctx, MatchPOIsAndBuildings(ctx, 0 /* curToken */, CBV::GetFull()); // A relaxed best effort parse: at least show the street if we can find one. + /// @todo Is is very controversial way. Search for streets and skip only house number (postcode) like tokens? if (numEmitted == ctx.m_numEmitted && ctx.SkipUsedTokens(0) != ctx.m_numTokens) { TRACE(Relaxed); diff --git a/search/geocoder.hpp b/search/geocoder.hpp index 9d597f378d..9475d3329c 100644 --- a/search/geocoder.hpp +++ b/search/geocoder.hpp @@ -256,6 +256,8 @@ private: StreetsMatcher::Prediction const & prediction, CentersFilter const & centers); + void ProcessStreets(BaseContext & ctx, CentersFilter const & centers, CBV const & streets); + // Tries to find all paths in a search tree, where each edge is // marked with some substring of the query tokens. These paths are // called "layer sequence" and current path is stored in |m_layers|. diff --git a/search/search_integration_tests/processor_test.cpp b/search/search_integration_tests/processor_test.cpp index 89ee90a425..4f905c95fb 100644 --- a/search/search_integration_tests/processor_test.cpp +++ b/search/search_integration_tests/processor_test.cpp @@ -2222,8 +2222,10 @@ UNIT_CLASS_TEST(ProcessorTest, SynonymMisprintsTest) { Rules rules = {ExactMatch(wonderlandId, bolshaya), ExactMatch(wonderlandId, bolnaya)}; - TEST(ResultsMatch("большая дмитровка", rules), ()); - TEST(ResultsMatch("больная дмитровка", rules), ()); + TEST(OrderedResultsMatch("большая дмитровка", + { ExactMatch(wonderlandId, bolshaya), ExactMatch(wonderlandId, bolnaya) }), ()); + TEST(OrderedResultsMatch("больная дмировка", + { ExactMatch(wonderlandId, bolnaya), ExactMatch(wonderlandId, bolshaya) }), ()); } { Rules rules = {ExactMatch(wonderlandId, bolshaya)}; @@ -2231,10 +2233,12 @@ UNIT_CLASS_TEST(ProcessorTest, SynonymMisprintsTest) TEST(ResultsMatch("б дмитровка", rules), ()); } { + /// @todo Results are controversial here, despite 2-errors matching. // "southeast" and "southwest" len is 9 and Levenstein distance is 2. - Rules rules = {ExactMatch(wonderlandId, sw), ExactMatch(wonderlandId, se)}; - TEST(ResultsMatch("southeast street", rules), ()); - TEST(ResultsMatch("southwest street", rules), ()); + TEST(OrderedResultsMatch("southeast street", + { ExactMatch(wonderlandId, se), ExactMatch(wonderlandId, sw) }), ()); + TEST(OrderedResultsMatch("southwest street", + { ExactMatch(wonderlandId, sw), ExactMatch(wonderlandId, se) }), ()); } { Rules rules = {ExactMatch(wonderlandId, sw)}; @@ -2248,6 +2252,33 @@ UNIT_CLASS_TEST(ProcessorTest, SynonymMisprintsTest) } } +UNIT_CLASS_TEST(ProcessorTest, StreetsFallback) +{ + TestStreet st1({{-0.5, -0.5}, {0.5, -0.5}}, "1st north street", "en"); + TestStreet st2({{-0.5, 0.5}, {0.5, 0.5}}, "2nd north street", "en"); + + auto wonderlandId = BuildCountry("Wonderland", [&](TestMwmBuilder & builder) + { + builder.Add(st1); + builder.Add(st2); + }); + + // Looks ok here: + { + TEST(ResultsMatch("1st street", { ExactMatch(wonderlandId, st1) }), ()); + TEST(ResultsMatch("2nd street", { ExactMatch(wonderlandId, st2) }), ()); + } + + /// @todo Because of controversial fallback in CreateStreetsLayerAndMatchLowerLayers. + // ... but: + { + TEST(OrderedResultsMatch("1st north street", + { ExactMatch(wonderlandId, st1), ExactMatch(wonderlandId, st2) }), ()); + TEST(OrderedResultsMatch("2nd north street", + { ExactMatch(wonderlandId, st2), ExactMatch(wonderlandId, st1) }), ()); + } +} + UNIT_CLASS_TEST(ProcessorTest, VillagePostcodes) { TestVillage marckolsheim({0, 0}, "Marckolsheim", "en", 10 /* rank */); @@ -3186,13 +3217,11 @@ UNIT_CLASS_TEST(ProcessorTest, ComplexPoi_Rank) SetViewport({-0.5, -0.5, 0.5, 0.5}); - auto request = MakeRequest("Telekom shop"); - auto const & results = request->Results(); - - TEST_EQUAL(results.size(), 2, ()); - - TEST(ResultsMatch({results[0]}, {ExactMatch(countryId, telekom)}), ()); - TEST(ResultsMatch({results[1]}, {ExactMatch(countryId, poiInMall)}), ()); + Rules const rules = { + ExactMatch(countryId, telekom), + ExactMatch(countryId, poiInMall) + }; + TEST(OrderedResultsMatch("Telekom shop", rules), ()); } UNIT_CLASS_TEST(ProcessorTest, Place_Region) @@ -3249,4 +3278,56 @@ UNIT_CLASS_TEST(ProcessorTest, FuzzyCategories) } } +UNIT_CLASS_TEST(ProcessorTest, StreetCategories) +{ + std::string const lang = "en"; + + TestStreet street({{-1, -1}, {1, 1}}, "Avenida Santa Fe", lang); + street.SetType({"highway", "secondary"}); + + TestPOI bus({0, 0}, "Avenida Santa Fe", lang); + bus.SetTypes({{"highway", "bus_stop"}}); + + TestPOI shop({-0.5, -0.5}, "Galerías Bond Street", lang); + shop.SetTypes({{"shop", "department_store"}}); + + auto wonderlandId = BuildCountry("Wonderland", [&](TestMwmBuilder & builder) + { + builder.Add(street); + builder.Add(bus); + builder.Add(shop); + }); + + SetViewport(m2::RectD(-0.5, -0.5, 0.5, 0.5)); + + { + Rules const rules = { + ExactMatch(wonderlandId, bus), + ExactMatch(wonderlandId, street) + }; + TEST(OrderedResultsMatch("avenida santa fe ", rules), ()); + } + + /// @todo Should review search::FindStreets logic! Check 2 cases below: + + // 1. |street| (matched by "sante fe" only) has worse rank than |shop| and even more - emitted in the second batch. + { + Rules const rules = { + ExactMatch(wonderlandId, bus), + ExactMatch(wonderlandId, shop), + ExactMatch(wonderlandId, street) + }; + TEST(OrderedResultsMatch("avenida santa fe street ", rules), ()); + } + + // 2. Next sample matches street by "santa fe улица", thus it has low rank! + { + Rules const rules = { + ExactMatch(wonderlandId, bus), + //ExactMatch(wonderlandId, street) + }; + TEST(OrderedResultsMatch(MakeRequest("avenida santa fe улица ", "ru")->Results(), rules), ()); + } +} + } // namespace processor_test diff --git a/search/search_tests/house_numbers_matcher_test.cpp b/search/search_tests/house_numbers_matcher_test.cpp index 29dea8d741..e29e569334 100644 --- a/search/search_tests/house_numbers_matcher_test.cpp +++ b/search/search_tests/house_numbers_matcher_test.cpp @@ -190,4 +190,10 @@ UNIT_TEST(LooksLikeHouseNumber_Smoke) TEST(LooksLikeHouseNumber("д 16", false /* isPrefix */), ()); TEST(LooksLikeHouseNumber("дом 16", false /* isPrefix */), ()); TEST(LooksLikeHouseNumber("дом 14 д 1", false /* isPrefix */), ()); + + TEST(!LooksLikeHouseNumber("улица", false /* isPrefix */), ()); + /// @todo By VNG: Don't know is it supposed or not, but next tokens are _looks like house numbers_! + /// @see g_strings in house_numbers_matching.cpp + TEST(LooksLikeHouseNumber("avenida", false /* isPrefix */), ()); + TEST(LooksLikeHouseNumber("street", false /* isPrefix */), ()); } diff --git a/search/search_tests_support/helpers.cpp b/search/search_tests_support/helpers.cpp index fedf400045..72b036e30c 100644 --- a/search/search_tests_support/helpers.cpp +++ b/search/search_tests_support/helpers.cpp @@ -27,7 +27,8 @@ void SearchTestBase::SetViewport(ms::LatLon const & ll, double radiusM) SetViewport(mercator::MetersToXY(ll.m_lon, ll.m_lat, radiusM)); } -bool SearchTestBase::CategoryMatch(std::string const & query, Rules const & rules, string const & locale) +bool SearchTestBase::CategoryMatch(std::string const & query, Rules const & rules, + string const & locale /* = "en" */) { TestSearchRequest request(m_engine, query, locale, Mode::Everywhere, m_viewport); request.SetCategorial(); @@ -37,19 +38,44 @@ bool SearchTestBase::CategoryMatch(std::string const & query, Rules const & rule } bool SearchTestBase::ResultsMatch(std::string const & query, Rules const & rules, - std::string const & locale /* = "en" */, - Mode mode /* = Mode::Everywhere */) + std::string const & locale /* = "en" */, + Mode mode /* = Mode::Everywhere */) { TestSearchRequest request(m_engine, query, locale, mode, m_viewport); request.Run(); return MatchResults(m_dataSource, rules, request.Results()); } +bool SearchTestBase::OrderedResultsMatch(std::string const & query, Rules const & rules, + std::string const & locale /* = "en" */, + Mode mode /* = Mode::Everywhere */) +{ + TestSearchRequest request(m_engine, query, locale, mode, m_viewport); + request.Run(); + return OrderedResultsMatch(request.Results(), rules); +} + bool SearchTestBase::ResultsMatch(vector const & results, Rules const & rules) { return MatchResults(m_dataSource, rules, results); } +bool SearchTestBase::OrderedResultsMatch(std::vector const & results, Rules const & rules) +{ + if (results.size() != rules.size()) + return false; + + for (size_t i = 0; i < results.size(); ++i) + { + if (!ResultMatches(m_dataSource, rules[i], results[i])) + { + LOG(LWARNING, ("Not matched:", rules[i], results[i])); + return false; + } + } + return true; +} + bool SearchTestBase::ResultsMatch(SearchParams const & params, Rules const & rules) { TestSearchRequest request(m_engine, params); diff --git a/search/search_tests_support/helpers.hpp b/search/search_tests_support/helpers.hpp index 2c7378a9e6..2e24fba2ab 100644 --- a/search/search_tests_support/helpers.hpp +++ b/search/search_tests_support/helpers.hpp @@ -35,7 +35,14 @@ public: bool ResultsMatch(std::string const & query, Rules const & rules, std::string const & locale = "en", Mode mode = Mode::Everywhere); + + bool OrderedResultsMatch(std::string const & query, Rules const & rules, + std::string const & locale = "en", + Mode mode = Mode::Everywhere); + bool ResultsMatch(std::vector const & results, Rules const & rules); + bool OrderedResultsMatch(std::vector const & results, Rules const & rules); + bool ResultsMatch(SearchParams const & params, Rules const & rules); bool IsResultMatches(Result const & result, Rule const & rule);