From 1e1db9145d780fcd22585dedd66d29c44a50a2ef Mon Sep 17 00:00:00 2001 From: Maxim Pimenov Date: Wed, 17 Aug 2016 17:46:35 +0300 Subject: [PATCH] Another fix and a test. --- map/framework.cpp | 4 +- map/framework.hpp | 5 +- search/downloader_search_callback.cpp | 12 +- search/downloader_search_callback.hpp | 12 +- search/processor.cpp | 5 +- search/ranker.hpp | 1 - .../downloader_search_test.cpp | 231 ++++++++++++++++++ .../search_integration_tests.pro | 1 + storage/downloader_search_params.hpp | 5 + 9 files changed, 264 insertions(+), 12 deletions(-) create mode 100644 search/search_integration_tests/downloader_search_test.cpp diff --git a/map/framework.cpp b/map/framework.cpp index 430f63239e..a6b5268384 100644 --- a/map/framework.cpp +++ b/map/framework.cpp @@ -1144,7 +1144,9 @@ bool Framework::SearchInDownloader(DownloaderSearchParams const & params) p.SetMode(search::Mode::Downloader); p.SetSuggestsEnabled(false); p.SetForceSearch(true); - p.m_onResults = search::DownloaderSearchCallback(m_model.GetIndex(), GetCountryInfoGetter(), GetStorage(), params); + p.m_onResults = search::DownloaderSearchCallback( + static_cast(*this), m_model.GetIndex(), + GetCountryInfoGetter(), GetStorage(), params); return Search(p); } diff --git a/map/framework.hpp b/map/framework.hpp index 44738886a3..3f9f1fe970 100644 --- a/map/framework.hpp +++ b/map/framework.hpp @@ -24,6 +24,7 @@ #include "editor/user_stats.hpp" +#include "search/downloader_search_callback.hpp" #include "search/engine.hpp" #include "search/mode.hpp" #include "search/query_saver.hpp" @@ -71,6 +72,7 @@ struct ViewportSearchParams; namespace storage { class CountryInfoGetter; +struct DownloaderSearchParams; } namespace routing { namespace turns{ class Settings; } } @@ -87,7 +89,8 @@ namespace df /// build version for screenshots. //#define FIXED_LOCATION -class Framework : public search::ViewportSearchCallback::Delegate +class Framework : public search::ViewportSearchCallback::Delegate, + public search::DownloaderSearchCallback::Delegate { DISALLOW_COPY(Framework); diff --git a/search/downloader_search_callback.cpp b/search/downloader_search_callback.cpp index daab4d63e4..9a421139c1 100644 --- a/search/downloader_search_callback.cpp +++ b/search/downloader_search_callback.cpp @@ -7,6 +7,7 @@ #include "indexer/index.hpp" +#include "base/logging.hpp" #include "base/string_utils.hpp" namespace @@ -30,11 +31,15 @@ bool GetGroupCountryIdFromFeature(storage::Storage const & storage, FeatureType namespace search { -DownloaderSearchCallback::DownloaderSearchCallback(Index const & index, +DownloaderSearchCallback::DownloaderSearchCallback(Delegate & delegate, Index const & index, storage::CountryInfoGetter const & infoGetter, storage::Storage const & storage, storage::DownloaderSearchParams params) - : m_index(index), m_infoGetter(infoGetter), m_storage(storage), m_params(move(params)) + : m_delegate(delegate) + , m_index(index) + , m_infoGetter(infoGetter) + , m_storage(storage) + , m_params(move(params)) { } @@ -76,7 +81,6 @@ void DownloaderSearchCallback::operator()(search::Results const & results) } } } - auto const & mercator = result.GetFeatureCenter(); storage::TCountryId const & countryId = m_infoGetter.GetRegionCountryId(mercator); if (countryId == storage::kInvalidCountryId) @@ -97,7 +101,7 @@ void DownloaderSearchCallback::operator()(search::Results const & results) if (m_params.m_onResults) { auto onResults = m_params.m_onResults; - GetPlatform().RunOnGuiThread( + m_delegate.RunUITask( [onResults, downloaderSearchResults]() { onResults(downloaderSearchResults); }); } } diff --git a/search/downloader_search_callback.hpp b/search/downloader_search_callback.hpp index b60181e981..7c64a91f1a 100644 --- a/search/downloader_search_callback.hpp +++ b/search/downloader_search_callback.hpp @@ -23,9 +23,18 @@ class Results; class DownloaderSearchCallback { public: + class Delegate + { + public: + virtual ~Delegate() = default; + + virtual void RunUITask(function fn) = 0; + }; + using TOnResults = storage::DownloaderSearchParams::TOnResults; - DownloaderSearchCallback(Index const & index, storage::CountryInfoGetter const & infoGetter, + DownloaderSearchCallback(Delegate & delegate, Index const & index, + storage::CountryInfoGetter const & infoGetter, storage::Storage const & storage, storage::DownloaderSearchParams params); @@ -34,6 +43,7 @@ public: private: set m_uniqueResults; + Delegate & m_delegate; Index const & m_index; storage::CountryInfoGetter const & m_infoGetter; storage::Storage const & m_storage; diff --git a/search/processor.cpp b/search/processor.cpp index 4b016ab9c0..7f19f6ba7e 100644 --- a/search/processor.cpp +++ b/search/processor.cpp @@ -437,10 +437,7 @@ void Processor::Search(SearchParams const & params, m2::RectD const & viewport) m_geocoder.GoEverywhere(); } - m_ranker.FlushResults(); - - if (!IsCancelled()) - params.m_onResults(m_ranker.GetResults()); + m_ranker.UpdateResults(true /* lastUpdate */); } catch (CancelException const &) { diff --git a/search/ranker.hpp b/search/ranker.hpp index 9f81147177..ec3abbdd2b 100644 --- a/search/ranker.hpp +++ b/search/ranker.hpp @@ -93,7 +93,6 @@ public: virtual void UpdateResults(bool lastUpdate); inline Results & GetResults() { return m_results; } - inline void FlushResults() { UpdateResults(true /* lastUpdate */); } void ClearCaches(); diff --git a/search/search_integration_tests/downloader_search_test.cpp b/search/search_integration_tests/downloader_search_test.cpp new file mode 100644 index 0000000000..f0bec12e69 --- /dev/null +++ b/search/search_integration_tests/downloader_search_test.cpp @@ -0,0 +1,231 @@ +#include "testing/testing.hpp" + +#include "generator/generator_tests_support/test_feature.hpp" + +#include "search/downloader_search_callback.hpp" +#include "search/mode.hpp" +#include "search/result.hpp" +#include "search/search_integration_tests/helpers.hpp" +#include "search/search_tests_support/test_results_matching.hpp" +#include "search/search_tests_support/test_search_request.hpp" + +#include "storage/downloader_search_params.hpp" +#include "storage/http_map_files_downloader.hpp" +#include "storage/storage.hpp" + +#include "geometry/rect2d.hpp" + +#include "base/logging.hpp" +#include "base/macros.hpp" + +#include "std/algorithm.hpp" +#include "std/string.hpp" + +using namespace generator::tests_support; +using namespace search::tests_support; + +using TRules = vector>; + +namespace storage +{ +class TestMapFilesDownloader : public HttpMapFilesDownloader +{ +public: + // MapFilesDownloader overrides: + void GetServersList(int64_t const mapVersion, string const & mapFileName, + TServersListCallback const & callback) override + { + } +}; +} // namespace storage + +namespace search +{ +namespace +{ +string const kCountriesTxt = string(R"({ + "id": "Countries", + "v": )" + strings::to_string(0 /* version */) + + R"(, + "g": [ + { + "id": "Flatland", + "g": [ + { + "id": "Square One", + "s": 123, + "old": [ + "Flatland" + ] + }, + { + "id": "Square Two", + "s": 456, + "old": [ + "Flatland" + ] + } + ] + }, + { + "id": "Wonderland", + "g": [ + { + "id": "Shortpondville", + "s": 789, + "old": [ + "Wonderland" + ] + }, + { + "id": "Longpondville", + "s": 100, + "old": [ + "Wonderland" + ] + } + ] + } + ]})"); + +class TestDelegate : public DownloaderSearchCallback::Delegate +{ +public: + // DownloaderSearchCallback::Delegate overrides: + void RunUITask(function fn) override { fn(); } +}; + +class DownloaderSearchRequest : public TestSearchRequest, public TestDelegate +{ +public: + DownloaderSearchRequest(TestSearchEngine & engine, string const & query) + : TestSearchRequest(engine, MakeSearchParams(query), m2::RectD(0, 0, 1, 1) /* viewport */) + , m_storage(kCountriesTxt, make_unique()) + , m_downloaderCallback(static_cast(*this), + m_engine /* index */, m_engine.GetCountryInfoGetter(), m_storage, + MakeDownloaderParams(query)) + { + SetCustomOnResults(bind(&DownloaderSearchRequest::OnResultsDownloader, this, _1)); + } + + void OnResultsDownloader(search::Results const & results) + { + OnResults(results); + m_downloaderCallback(results); + } + + vector const & GetResults() const { return m_downloaderResults; } +private: + search::SearchParams MakeSearchParams(string const & query) + { + search::SearchParams p; + p.m_query = query; + p.m_inputLocale = "en"; + p.SetMode(search::Mode::Downloader); + p.SetSuggestsEnabled(false); + p.SetForceSearch(true); + return p; + } + + storage::DownloaderSearchParams MakeDownloaderParams(string const & query) + { + storage::DownloaderSearchParams p; + p.m_query = query; + p.m_inputLocale = "en"; + p.m_onResults = [this](storage::DownloaderSearchResults const & r) + { + CHECK(!m_endMarker, ()); + if (r.m_endMarker) + { + m_endMarker = true; + return; + } + + m_downloaderResults.insert(m_downloaderResults.end(), r.m_results.begin(), r.m_results.end()); + }; + return p; + } + + vector m_downloaderResults; + bool m_endMarker = false; + + storage::Storage m_storage; + + DownloaderSearchCallback m_downloaderCallback; +}; + +class DownloaderSearchTest : public SearchTest +{ +}; + +template +void TestResults(vector received, vector expected) +{ + sort(received.begin(), received.end()); + sort(expected.begin(), expected.end()); + TEST_EQUAL(expected, received, ()); +} + +UNIT_CLASS_TEST(DownloaderSearchTest, Smoke) +{ + vector worldCountries; + vector worldCities; + auto addRegion = [&](string const & countryName, string const & regionName, m2::PointD const & p1, + m2::PointD const & p2) { + TestPOI cornerPost1(p1, regionName + " corner post 1", "en"); + TestPOI cornerPost2(p2, regionName + " corner post 2", "en"); + TestCity capital((p1 + p2) * 0.5, regionName + " capital", "en", 0 /* rank */); + TestCountry country(p1 * 0.3 + p2 * 0.7, countryName, "en"); + BuildCountry(regionName, [&](TestMwmBuilder & builder) + { + builder.Add(cornerPost1); + builder.Add(cornerPost2); + builder.Add(capital); + if (!countryName.empty()) + { + // Add the country feature to one region only. + builder.Add(country); + worldCountries.push_back(country); + } + }); + worldCities.push_back(capital); + }; + + addRegion("Flatland", "Square One", m2::PointD(0.0, 0.0), m2::PointD(1.0, 1.0)); + addRegion("", "Square Two", m2::PointD(1.0, 1.0), m2::PointD(3.0, 3.0)); + addRegion("Wonderland", "Shortpondland", m2::PointD(-1.0, -1.0), m2::PointD(0.0, 0.0)); + addRegion("", "Longpondland", m2::PointD(-3.0, -3.0), m2::PointD(-1.0, -1.0)); + + auto const worldId = BuildWorld([&](TestMwmBuilder & builder) + { + for (auto const & ft : worldCountries) + builder.Add(ft); + for (auto const & ft : worldCities) + builder.Add(ft); + }); + + { + DownloaderSearchRequest request(m_engine, "square one"); + request.Run(); + + TestResults(request.GetResults(), + {storage::DownloaderSearchResult("Square One", "Square One capital")}); + } + + { + DownloaderSearchRequest request(m_engine, "shortpondland"); + request.Run(); + + TestResults(request.GetResults(), + {storage::DownloaderSearchResult("Shortpondland", "Shortpondland capital")}); + } + + { + DownloaderSearchRequest request(m_engine, "flatland"); + request.Run(); + + TestResults(request.GetResults(), {storage::DownloaderSearchResult("Flatland", "Flatland")}); + } +} +} // namespace +} // namespace search diff --git a/search/search_integration_tests/search_integration_tests.pro b/search/search_integration_tests/search_integration_tests.pro index b05b0a5645..e8ae629a85 100644 --- a/search/search_integration_tests/search_integration_tests.pro +++ b/search/search_integration_tests/search_integration_tests.pro @@ -19,6 +19,7 @@ macx-*: LIBS *= "-framework IOKit" SOURCES += \ ../../testing/testingmain.cpp \ + downloader_search_test.cpp \ generate_tests.cpp \ helpers.cpp \ interactive_search_test.cpp \ diff --git a/storage/downloader_search_params.hpp b/storage/downloader_search_params.hpp index fee97e081f..1b7d4cc981 100644 --- a/storage/downloader_search_params.hpp +++ b/storage/downloader_search_params.hpp @@ -52,4 +52,9 @@ struct DownloaderSearchParams string m_query; string m_inputLocale; }; + +inline string DebugPrint(DownloaderSearchResult const & r) +{ + return "(" + r.m_countryId + " " + r.m_matchedName + ")"; +} } // namespace storage