From 0d324767834f7d352af51d02c589e3ca036ff2b0 Mon Sep 17 00:00:00 2001 From: Yuri Gorshenin Date: Fri, 12 Feb 2016 17:24:30 +0300 Subject: [PATCH] [search] Fixed search in world only. --- map/framework.cpp | 3 +- search/mode.cpp | 15 +++++++ search/mode.hpp | 15 +++++++ search/params.cpp | 12 ++---- search/params.hpp | 20 +++------- search/search.pro | 2 + search/search_engine.cpp | 9 ++++- .../retrieval_test.cpp | 6 +-- .../search_query_v2_test.cpp | 40 ++++++++++++++----- .../search_integration_tests/smoke_test.cpp | 10 ++--- .../search_quality_tests.cpp | 2 +- search/search_query.cpp | 1 + search/search_query.hpp | 3 ++ .../test_search_request.cpp | 5 +-- .../test_search_request.hpp | 2 +- search/v2/geocoder.cpp | 5 ++- search/v2/geocoder.hpp | 2 + search/v2/search_query_v2.cpp | 1 + 18 files changed, 103 insertions(+), 50 deletions(-) create mode 100644 search/mode.cpp create mode 100644 search/mode.hpp diff --git a/map/framework.cpp b/map/framework.cpp index 8320bf4818..7c0d1d6df6 100644 --- a/map/framework.cpp +++ b/map/framework.cpp @@ -886,8 +886,7 @@ void Framework::StartInteractiveSearch(search::SearchParams const & params) m_lastInteractiveSearchParams = params; m_lastInteractiveSearchParams.SetForceSearch(false); - m_lastInteractiveSearchParams.SetSearchMode(SearchParams::IN_VIEWPORT_ONLY | - SearchParams::SEARCH_WORLD); + m_lastInteractiveSearchParams.SetMode(Mode::Viewport); m_lastInteractiveSearchParams.m_callback = [this](Results const & results) { if (!results.IsEndMarker()) diff --git a/search/mode.cpp b/search/mode.cpp new file mode 100644 index 0000000000..59c7eb751b --- /dev/null +++ b/search/mode.cpp @@ -0,0 +1,15 @@ +#include "search/mode.hpp" + +namespace search +{ +string DebugPrint(Mode mode) +{ + switch (mode) + { + case Mode::Viewport: return "Viewport"; + case Mode::Everywhere: return "Everywhere"; + case Mode::World: return "World"; + } + return "Unknown"; +} +} // namespace search diff --git a/search/mode.hpp b/search/mode.hpp new file mode 100644 index 0000000000..0b3b00cfbe --- /dev/null +++ b/search/mode.hpp @@ -0,0 +1,15 @@ +#pragma once + +#include "std/string.hpp" + +namespace search +{ +enum class Mode +{ + Viewport, + Everywhere, + World +}; + +string DebugPrint(Mode mode); +} // namespace search diff --git a/search/params.cpp b/search/params.cpp index ff90ef110b..a374bc81ad 100644 --- a/search/params.cpp +++ b/search/params.cpp @@ -7,10 +7,8 @@ namespace search { - SearchParams::SearchParams() - : m_searchRadiusM(-1.0), m_searchMode(ALL), - m_forceSearch(false), m_validPos(false) + : m_searchRadiusM(-1.0), m_mode(Mode::Everywhere), m_forceSearch(false), m_validPos(false) { } @@ -36,17 +34,15 @@ bool SearchParams::IsEqualCommon(SearchParams const & rhs) const return (m_query == rhs.m_query && m_inputLocale == rhs.m_inputLocale && m_validPos == rhs.m_validPos && - m_searchMode == rhs.m_searchMode && + m_mode == rhs.m_mode && m_searchRadiusM == rhs.m_searchRadiusM); } string DebugPrint(SearchParams const & params) { ostringstream ss; - ss << "{ SearchParams: Query = " << params.m_query << - ", Locale = " << params.m_inputLocale << - ", Mode = " << params.m_searchMode << " }"; + ss << "{ SearchParams: Query = " << params.m_query << ", Locale = " << params.m_inputLocale + << ", Mode = " << DebugPrint(params.m_mode) << " }"; return ss.str(); } - } // namespace search diff --git a/search/params.hpp b/search/params.hpp index 344e4ea88f..829404d0ae 100644 --- a/search/params.hpp +++ b/search/params.hpp @@ -1,5 +1,7 @@ #pragma once +#include "search/mode.hpp" + #include "geometry/point2d.hpp" #include "geometry/rect2d.hpp" @@ -23,19 +25,8 @@ namespace search bool IsForceSearch() const { return m_forceSearch; } //@} - /// @name Search modes. - //@{ - enum SearchModeT - { - IN_VIEWPORT_ONLY = 1, - SEARCH_WORLD = 2, - SEARCH_ADDRESS = 4, - ALL = SEARCH_WORLD | SEARCH_ADDRESS - }; - - inline void SetSearchMode(int mode) { m_searchMode = mode; } - inline bool HasSearchMode(SearchModeT mode) const { return ((m_searchMode & mode) != 0); } - //@} + inline void SetMode(Mode mode) { m_mode = mode; } + inline Mode GetMode() const { return m_mode; } void SetPosition(double lat, double lon); bool IsValidPosition() const { return m_validPos; } @@ -66,8 +57,7 @@ namespace search private: double m_searchRadiusM; - int m_searchMode; + Mode m_mode; bool m_forceSearch, m_validPos; }; - } // namespace search diff --git a/search/search.pro b/search/search.pro index 8027ff058b..ee5438acab 100644 --- a/search/search.pro +++ b/search/search.pro @@ -25,6 +25,7 @@ HEADERS += \ latlon_match.hpp \ locality.hpp \ locality_finder.hpp \ + mode.hpp \ params.hpp \ projection_on_street.hpp \ query_saver.hpp \ @@ -72,6 +73,7 @@ SOURCES += \ latlon_match.cpp \ locality.cpp \ locality_finder.cpp \ + mode.cpp \ params.cpp \ projection_on_street.cpp \ query_saver.cpp \ diff --git a/search/search_engine.cpp b/search/search_engine.cpp index db5d59c7cc..01b7cd5fba 100644 --- a/search/search_engine.cpp +++ b/search/search_engine.cpp @@ -221,7 +221,7 @@ void Engine::PostTask(function && task) void Engine::DoSearch(SearchParams const & params, m2::RectD const & viewport, shared_ptr handle) { - bool const viewportSearch = params.HasSearchMode(SearchParams::IN_VIEWPORT_ONLY); + bool const viewportSearch = params.GetMode() == Mode::Viewport; // Initialize query. m_query->Init(viewportSearch); @@ -242,7 +242,12 @@ void Engine::DoSearch(SearchParams const & params, m2::RectD const & viewport, else m_query->SetPosition(viewport.Center()); - m_query->SetSearchInWorld(params.HasSearchMode(SearchParams::SEARCH_WORLD)); + m_query->SetMode(params.GetMode()); + + // This flag is needed for consistency with old search algorithm + // only. It will gone away when we will remove old search code. + m_query->SetSearchInWorld(true); + m_query->SetInputLocale(params.m_inputLocale); ASSERT(!params.m_query.empty(), ()); diff --git a/search/search_integration_tests/retrieval_test.cpp b/search/search_integration_tests/retrieval_test.cpp index 61edb18389..719615941e 100644 --- a/search/search_integration_tests/retrieval_test.cpp +++ b/search/search_integration_tests/retrieval_test.cpp @@ -384,7 +384,7 @@ UNIT_TEST(Retrieval_CafeMTV) auto const testWorldId = engine.GetMwmIdByCountryFile(testWorld.GetCountryFile()); { - TestSearchRequest request(engine, "Moscow ", "en", search::SearchParams::ALL, mskViewport); + TestSearchRequest request(engine, "Moscow ", "en", search::Mode::Everywhere, mskViewport); request.Wait(); initializer_list> mskCityAlts = { @@ -395,7 +395,7 @@ UNIT_TEST(Retrieval_CafeMTV) } { - TestSearchRequest request(engine, "MTV ", "en", search::SearchParams::ALL, mtvViewport); + TestSearchRequest request(engine, "MTV ", "en", search::Mode::Everywhere, mtvViewport); request.Wait(); initializer_list> mtvCityAlts = { @@ -406,7 +406,7 @@ UNIT_TEST(Retrieval_CafeMTV) } { - TestSearchRequest request(engine, "Moscow MTV ", "en", search::SearchParams::ALL, mtvViewport); + TestSearchRequest request(engine, "Moscow MTV ", "en", search::Mode::Everywhere, mtvViewport); request.Wait(); initializer_list> alternatives = { diff --git a/search/search_integration_tests/search_query_v2_test.cpp b/search/search_integration_tests/search_query_v2_test.cpp index 297b007b3d..0cc1492344 100644 --- a/search/search_integration_tests/search_query_v2_test.cpp +++ b/search/search_integration_tests/search_query_v2_test.cpp @@ -94,7 +94,14 @@ public: bool ResultsMatch(string const & query, vector> const & rules) { - TestSearchRequest request(m_engine, query, "en", search::SearchParams::ALL, m_viewport); + TestSearchRequest request(m_engine, query, "en", search::Mode::Everywhere, m_viewport); + request.Wait(); + return MatchResults(m_engine, rules, request.Results()); + } + + bool ResultsMatch(string const & query, search::Mode mode, vector> const & rules) + { + TestSearchRequest request(m_engine, query, "en", mode, m_viewport); request.Wait(); return MatchResults(m_engine, rules, request.Results()); } @@ -299,22 +306,37 @@ UNIT_CLASS_TEST(SearchQueryV2Test, SearchByName) TestPark hydePark(vector{m2::PointD(0.5, 0.5), m2::PointD(1.5, 0.5), m2::PointD(1.5, 1.5), m2::PointD(0.5, 1.5)}, "Hyde Park", "en"); + TestPOI cafe(m2::PointD(1.0, 1.0), "London Cafe", "en"); - BuildMwm("testWorld", feature::DataHeader::world, [&](TestMwmBuilder & builder) - { - builder.Add(london); - }); + auto worldId = BuildMwm("testWorld", feature::DataHeader::world, [&](TestMwmBuilder & builder) + { + builder.Add(london); + }); auto wonderlandId = BuildMwm("wonderland", feature::DataHeader::country, [&](TestMwmBuilder & builder) { builder.Add(hydePark); + builder.Add(cafe); }); RegisterCountry("Wonderland", m2::RectD(m2::PointD(0, 0), m2::PointD(2, 2))); SetViewport(m2::RectD(m2::PointD(-1.0, -1.0), m2::PointD(-0.9, -0.9))); - TRules rules = {make_shared(wonderlandId, hydePark)}; - TEST(ResultsMatch("hyde park", rules), ()); - TEST(ResultsMatch("london hyde park", rules), ()); - TEST(ResultsMatch("hyde london park", TRules()), ()); + { + TRules rules = {make_shared(wonderlandId, hydePark)}; + TEST(ResultsMatch("hyde park", rules), ()); + TEST(ResultsMatch("london hyde park", rules), ()); + TEST(ResultsMatch("hyde london park", TRules()), ()); + } + + SetViewport(m2::RectD(m2::PointD(0.5, 0.5), m2::PointD(1.5, 1.5))); + { + TRules rules = {make_shared(worldId, london)}; + TEST(ResultsMatch("london", search::Mode::World, rules), ()); + } + { + TRules rules = {make_shared(worldId, london), + make_shared(wonderlandId, cafe)}; + TEST(ResultsMatch("london", search::Mode::Everywhere, rules), ()); + } } diff --git a/search/search_integration_tests/smoke_test.cpp b/search/search_integration_tests/smoke_test.cpp index 340359b5a2..9e18f0aca1 100644 --- a/search/search_integration_tests/smoke_test.cpp +++ b/search/search_integration_tests/smoke_test.cpp @@ -80,14 +80,14 @@ UNIT_TEST(GenerateTestMwm_Smoke) TestFeaturesCount(engine, m2::RectD(m2::PointD(-0.5, -0.5), m2::PointD(0.5, 1.5)), 2); { - TestSearchRequest request(engine, "wine ", "en", search::SearchParams::IN_VIEWPORT_ONLY, + TestSearchRequest request(engine, "wine ", "en", search::Mode::Viewport, m2::RectD(m2::PointD(0, 0), m2::PointD(100, 100))); request.Wait(); TEST_EQUAL(1, request.Results().size(), ()); } { - TestSearchRequest request(engine, "shop ", "en", search::SearchParams::IN_VIEWPORT_ONLY, + TestSearchRequest request(engine, "shop ", "en", search::Mode::Viewport, m2::RectD(m2::PointD(0, 0), m2::PointD(100, 100))); request.Wait(); TEST_EQUAL(4, request.Results().size(), ()); @@ -119,19 +119,19 @@ UNIT_TEST(GenerateTestMwm_NotPrefixFreeNames) TestFeaturesCount(engine, m2::RectD(m2::PointD(0, 0), m2::PointD(2, 2)), 6); { - TestSearchRequest request(engine, "a ", "en", search::SearchParams::IN_VIEWPORT_ONLY, + TestSearchRequest request(engine, "a ", "en", search::Mode::Viewport, m2::RectD(m2::PointD(0, 0), m2::PointD(100, 100))); request.Wait(); TEST_EQUAL(1, request.Results().size(), ()); } { - TestSearchRequest request(engine, "aa ", "en", search::SearchParams::IN_VIEWPORT_ONLY, + TestSearchRequest request(engine, "aa ", "en", search::Mode::Viewport, m2::RectD(m2::PointD(0, 0), m2::PointD(100, 100))); request.Wait(); TEST_EQUAL(2, request.Results().size(), ()); } { - TestSearchRequest request(engine, "aaa ", "en", search::SearchParams::IN_VIEWPORT_ONLY, + TestSearchRequest request(engine, "aaa ", "en", search::Mode::Viewport, m2::RectD(m2::PointD(0, 0), m2::PointD(100, 100))); request.Wait(); TEST_EQUAL(3, request.Results().size(), ()); diff --git a/search/search_quality_tests/search_quality_tests.cpp b/search/search_quality_tests/search_quality_tests.cpp index 48efbdebf6..112a4d0441 100644 --- a/search/search_quality_tests/search_quality_tests.cpp +++ b/search/search_quality_tests/search_quality_tests.cpp @@ -231,7 +231,7 @@ int main(int argc, char * argv[]) string const & query = queries[i] + " "; my::Timer timer; // todo(@m) Viewport and position should belong to the query info. - TestSearchRequest request(engine, query, FLAGS_locale, search::SearchParams::ALL, viewport); + TestSearchRequest request(engine, query, FLAGS_locale, search::Mode::Everywhere, viewport); request.Wait(); responseTimes[i] = timer.ElapsedSeconds(); diff --git a/search/search_query.cpp b/search/search_query.cpp index d6482013ea..1f43cfb857 100644 --- a/search/search_query.cpp +++ b/search/search_query.cpp @@ -202,6 +202,7 @@ Query::Query(Index & index, CategoriesHolder const & categories, vector , m_locality(&index) #endif , m_position(0, 0) + , m_mode(Mode::Everywhere) , m_worldSearch(true) , m_keepHouseNumberInQuery(false) { diff --git a/search/search_query.hpp b/search/search_query.hpp index ef2ac2f41f..e815fd322c 100644 --- a/search/search_query.hpp +++ b/search/search_query.hpp @@ -1,6 +1,7 @@ #pragma once #include "search/intermediate_result.hpp" #include "search/keyword_lang_matcher.hpp" +#include "search/mode.hpp" #include "search/retrieval.hpp" #include "search/search_trie.hpp" #include "search/suggest.hpp" @@ -87,6 +88,7 @@ public: inline string const & GetPivotRegion() const { return m_region; } inline void SetPosition(m2::PointD const & position) { m_position = position; } + inline void SetMode(Mode mode) { m_mode = mode; } inline void SetSearchInWorld(bool b) { m_worldSearch = b; } /// Suggestions language code, not the same as we use in mwm data @@ -238,6 +240,7 @@ protected: m2::RectD m_viewport[COUNT_V]; m2::PointD m_pivot; m2::PointD m_position; + Mode m_mode; bool m_worldSearch; Retrieval m_retrieval; diff --git a/search/search_tests_support/test_search_request.cpp b/search/search_tests_support/test_search_request.cpp index 89adf84fec..d218596155 100644 --- a/search/search_tests_support/test_search_request.cpp +++ b/search/search_tests_support/test_search_request.cpp @@ -10,8 +10,7 @@ namespace search namespace tests_support { TestSearchRequest::TestSearchRequest(TestSearchEngine & engine, string const & query, - string const & locale, search::SearchParams::SearchModeT mode, - m2::RectD const & viewport) + string const & locale, Mode mode, m2::RectD const & viewport) : m_done(false) { search::SearchParams params; @@ -21,7 +20,7 @@ TestSearchRequest::TestSearchRequest(TestSearchEngine & engine, string const & q { Done(results); }; - params.SetSearchMode(mode); + params.SetMode(mode); engine.Search(params, viewport); } diff --git a/search/search_tests_support/test_search_request.hpp b/search/search_tests_support/test_search_request.hpp index c46b84f151..ecad10c83e 100644 --- a/search/search_tests_support/test_search_request.hpp +++ b/search/search_tests_support/test_search_request.hpp @@ -23,7 +23,7 @@ class TestSearchRequest { public: TestSearchRequest(TestSearchEngine & engine, string const & query, string const & locale, - search::SearchParams::SearchModeT mode, m2::RectD const & viewport); + Mode mode, m2::RectD const & viewport); void Wait(); diff --git a/search/v2/geocoder.cpp b/search/v2/geocoder.cpp index 8f142ebdad..e00914548f 100644 --- a/search/v2/geocoder.cpp +++ b/search/v2/geocoder.cpp @@ -361,7 +361,7 @@ void UniteCBVs(vector> & cbvs) } // namespace // Geocoder::Params -------------------------------------------------------------------------------- -Geocoder::Params::Params() : m_position(0, 0), m_maxNumResults(0) {} +Geocoder::Params::Params() : m_mode(Mode::Everywhere), m_position(0, 0), m_maxNumResults(0) {} // Geocoder::Geocoder ------------------------------------------------------------------------------ Geocoder::Geocoder(Index & index, storage::CountryInfoGetter const & infoGetter) @@ -795,6 +795,9 @@ void Geocoder::ForEachCountry(vector> const & infos, TFn && auto const & info = infos[i]; if (info->GetType() != MwmInfo::COUNTRY && info->GetType() != MwmInfo::WORLD) continue; + if (info->GetType() == MwmInfo::COUNTRY && m_params.m_mode == Mode::World) + continue; + auto handle = m_index.GetMwmHandleById(MwmSet::MwmId(info)); if (!handle.IsAlive()) continue; diff --git a/search/v2/geocoder.hpp b/search/v2/geocoder.hpp index 18a92b6431..21e2c6c254 100644 --- a/search/v2/geocoder.hpp +++ b/search/v2/geocoder.hpp @@ -1,6 +1,7 @@ #pragma once #include "search/cancel_exception.hpp" +#include "search/mode.hpp" #include "search/search_query_params.hpp" #include "search/v2/features_layer.hpp" #include "search/v2/features_layer_path_finder.hpp" @@ -69,6 +70,7 @@ public: { Params(); + Mode m_mode; m2::RectD m_viewport; /// User's position or viewport center if there is no valid position. m2::PointD m_position; diff --git a/search/v2/search_query_v2.cpp b/search/v2/search_query_v2.cpp index 81489c0f4e..7534b50228 100644 --- a/search/v2/search_query_v2.cpp +++ b/search/v2/search_query_v2.cpp @@ -37,6 +37,7 @@ void SearchQueryV2::Search(Results & res, size_t resCount) Geocoder::Params params; InitParams(false /* localitySearch */, params); + params.m_mode = m_mode; params.m_viewport = m_viewport[CURRENT_V]; params.m_position = m_position; params.m_maxNumResults = max(resCount, kPreResultsCount);