diff --git a/routing/async_router.cpp b/routing/async_router.cpp index f8c7fbc884..746a97857d 100644 --- a/routing/async_router.cpp +++ b/routing/async_router.cpp @@ -301,7 +301,7 @@ void AsyncRouter::CalculateRoute() LOG(LINFO, ("Calculating the route,", checkpoints, "startDirection", startDirection)); if (absentFetcher) - absentFetcher->GenerateRequest(checkpoints.GetStart(), checkpoints.GetFinish()); + absentFetcher->GenerateRequest(checkpoints); // Run basic request. code = router->CalculateRoute(checkpoints, startDirection, adjustToPrevRoute, diff --git a/routing/online_absent_fetcher.cpp b/routing/online_absent_fetcher.cpp index c1abe3f6ab..64e90b12f6 100644 --- a/routing/online_absent_fetcher.cpp +++ b/routing/online_absent_fetcher.cpp @@ -6,6 +6,8 @@ #include "platform/country_file.hpp" #include "platform/local_country_file.hpp" +#include "base/stl_helpers.hpp" + #include "std/vector.hpp" #include "private.h" @@ -15,16 +17,17 @@ using platform::LocalCountryFile; namespace routing { -void OnlineAbsentCountriesFetcher::GenerateRequest(const m2::PointD & startPoint, - const m2::PointD & finalPoint) +void OnlineAbsentCountriesFetcher::GenerateRequest(Checkpoints const & checkpoints) { - // Single mwm case. - if (m_countryFileFn(startPoint) == m_countryFileFn(finalPoint) || - GetPlatform().ConnectionStatus() == Platform::EConnectionType::CONNECTION_NONE) + if (GetPlatform().ConnectionStatus() == Platform::EConnectionType::CONNECTION_NONE) return; + + // Single mwm case. + if (AllPointsInSameMwm(checkpoints)) + return; + unique_ptr fetcher = - make_unique(OSRM_ONLINE_SERVER_URL, MercatorBounds::ToLatLon(startPoint), - MercatorBounds::ToLatLon(finalPoint)); + make_unique(m_countryFileFn, OSRM_ONLINE_SERVER_URL, checkpoints); // iOS can't reuse threads. So we need to recreate the thread. m_fetcherThread.reset(new threads::Thread()); m_fetcherThread->Create(move(fetcher)); @@ -32,9 +35,11 @@ void OnlineAbsentCountriesFetcher::GenerateRequest(const m2::PointD & startPoint void OnlineAbsentCountriesFetcher::GetAbsentCountries(vector & countries) { + countries.clear(); // Check whether a request was scheduled to be run on the thread. if (!m_fetcherThread) return; + m_fetcherThread->Join(); for (auto const & point : m_fetcherThread->GetRoutineAs()->GetMwmPoints()) { @@ -43,9 +48,23 @@ void OnlineAbsentCountriesFetcher::GetAbsentCountries(vector & countries if (name.empty() || m_countryLocalFileFn(name)) continue; - LOG(LINFO, ("Needs: ", name)); countries.emplace_back(move(name)); } m_fetcherThread.reset(); + + my::SortUnique(countries); + for (auto const & country : countries) + LOG(LINFO, ("Needs:", country)); +} + +bool OnlineAbsentCountriesFetcher::AllPointsInSameMwm(Checkpoints const & checkpoints) const +{ + for (size_t i = 0; i < checkpoints.GetNumSubroutes(); ++i) + { + if (m_countryFileFn(checkpoints.GetPoint(i)) != m_countryFileFn(checkpoints.GetPoint(i + 1))) + return false; + } + + return true; } } // namespace routing diff --git a/routing/online_absent_fetcher.hpp b/routing/online_absent_fetcher.hpp index 27760041f6..3dfafa9b9e 100644 --- a/routing/online_absent_fetcher.hpp +++ b/routing/online_absent_fetcher.hpp @@ -1,5 +1,6 @@ #pragma once +#include "routing/checkpoints.hpp" #include "routing/routing_mapping.hpp" #include "geometry/point2d.hpp" @@ -18,7 +19,7 @@ class IOnlineFetcher { public: virtual ~IOnlineFetcher() = default; - virtual void GenerateRequest(m2::PointD const & startPoint, m2::PointD const & finalPoint) = 0; + virtual void GenerateRequest(Checkpoints const &) = 0; virtual void GetAbsentCountries(vector & countries) = 0; }; @@ -36,10 +37,12 @@ public: } // IOnlineFetcher overrides: - void GenerateRequest(m2::PointD const & startPoint, m2::PointD const & finalPoint) override; + void GenerateRequest(Checkpoints const &) override; void GetAbsentCountries(vector & countries) override; private: + bool AllPointsInSameMwm(Checkpoints const &) const; + TCountryFileFn const m_countryFileFn; TCountryLocalFileFn const m_countryLocalFileFn; unique_ptr m_fetcherThread; diff --git a/routing/online_cross_fetcher.cpp b/routing/online_cross_fetcher.cpp index 43b4509800..9381eebc6d 100644 --- a/routing/online_cross_fetcher.cpp +++ b/routing/online_cross_fetcher.cpp @@ -21,50 +21,66 @@ inline string LatLonToURLArgs(ms::LatLon const & point) namespace routing { -bool ParseResponse(const string & serverResponse, vector & outPoints) +bool ParseResponse(string const & serverResponse, vector & outPoints) { try { my::Json parser(serverResponse.c_str()); json_t const * countries = json_object_get(parser.get(), "used_mwms"); - size_t pointsCount = json_array_size(countries); - outPoints.reserve(pointsCount); + size_t const pointsCount = json_array_size(countries); for (size_t i = 0; i < pointsCount; ++i) { json_t * pointArray = json_array_get(countries, i); outPoints.push_back({json_number_value(json_array_get(pointArray, 0)), json_number_value(json_array_get(pointArray, 1))}); } - return !outPoints.empty(); + return pointsCount > 0; } - catch (my::Json::Exception&) + catch (my::Json::Exception const & exception) { + LOG(LWARNING, ("Can't parse server response:", exception.what())); + LOG(LWARNING, ("Response body:", serverResponse)); return false; } - return false; } string GenerateOnlineRequest(string const & serverURL, ms::LatLon const & startPoint, ms::LatLon const & finalPoint) { return serverURL + "/mapsme?loc=" + LatLonToURLArgs(startPoint) + "&loc=" + - LatLonToURLArgs(finalPoint); + LatLonToURLArgs(finalPoint); } -OnlineCrossFetcher::OnlineCrossFetcher(string const & serverURL, ms::LatLon const & startPoint, - ms::LatLon const & finalPoint) - : m_request(GenerateOnlineRequest(serverURL, startPoint, finalPoint)) +OnlineCrossFetcher::OnlineCrossFetcher(TCountryFileFn const & countryFileFn, + string const & serverURL, Checkpoints const & checkpoints) + : m_countryFileFn(countryFileFn), m_serverURL(serverURL), m_checkpoints(checkpoints) { - LOG(LINFO, ("Check mwms by URL: ", GenerateOnlineRequest(serverURL, startPoint, finalPoint))); } void OnlineCrossFetcher::Do() { m_mwmPoints.clear(); - if (m_request.RunHttpRequest() && m_request.ErrorCode() == 200 && !m_request.WasRedirected()) - ParseResponse(m_request.ServerResponse(), m_mwmPoints); - else - LOG(LWARNING, ("Can't get OSRM server response. Code: ", m_request.ErrorCode())); + + for (size_t i = 0; i < m_checkpoints.GetNumSubroutes(); ++i) + { + m2::PointD const & pointFrom = m_checkpoints.GetPoint(i); + m2::PointD const & pointTo = m_checkpoints.GetPoint(i + 1); + + string const mwmFrom = m_countryFileFn(pointFrom); + string const mwmTo = m_countryFileFn(pointTo); + if (mwmFrom == mwmTo) + continue; + + string const url = GenerateOnlineRequest(m_serverURL, MercatorBounds::ToLatLon(pointFrom), + MercatorBounds::ToLatLon(pointTo)); + platform::HttpClient request(url); + LOG(LINFO, ("Check mwms by URL: ", url)); + + if (request.RunHttpRequest() && request.ErrorCode() == 200 && !request.WasRedirected()) + ParseResponse(request.ServerResponse(), m_mwmPoints); + else + LOG(LWARNING, ("Can't get OSRM server response. Code: ", request.ErrorCode())); + } } } // namespace routing diff --git a/routing/online_cross_fetcher.hpp b/routing/online_cross_fetcher.hpp index 7543af4efe..e674189abe 100644 --- a/routing/online_cross_fetcher.hpp +++ b/routing/online_cross_fetcher.hpp @@ -1,5 +1,8 @@ #pragma once +#include "routing/checkpoints.hpp" +#include "routing/router.hpp" + #include "platform/http_client.hpp" #include "geometry/point2d.hpp" @@ -33,11 +36,8 @@ class OnlineCrossFetcher : public threads::IRoutine public: /// \brief OnlineCrossFetcher helper class to make request to online OSRM server /// and get mwm names list - /// \param serverURL Server URL - /// \param startPoint Start point coordinates - /// \param finalPoint Finish point coordinates - OnlineCrossFetcher(string const & serverURL, ms::LatLon const & startPoint, - ms::LatLon const & finalPoint); + OnlineCrossFetcher(TCountryFileFn const & countryFileFn, string const & serverURL, + Checkpoints const & checkpoints); /// Overrides threads::IRoutine processing procedure. Calls online OSRM server and parses response. void Do() override; @@ -47,7 +47,9 @@ public: vector const & GetMwmPoints() { return m_mwmPoints; } private: - platform::HttpClient m_request; + TCountryFileFn const m_countryFileFn; + string const m_serverURL; + Checkpoints const m_checkpoints; vector m_mwmPoints; }; } diff --git a/routing/routing_integration_tests/routing_test_tools.cpp b/routing/routing_integration_tests/routing_test_tools.cpp index 6d6cabef6c..39335e4518 100644 --- a/routing/routing_integration_tests/routing_test_tools.cpp +++ b/routing/routing_integration_tests/routing_test_tools.cpp @@ -365,8 +365,8 @@ namespace integration return false; }; routing::OnlineAbsentCountriesFetcher fetcher(countryFileGetter, localFileChecker); - fetcher.GenerateRequest(MercatorBounds::FromLatLon(startPoint), - MercatorBounds::FromLatLon(finalPoint)); + fetcher.GenerateRequest(Checkpoints(MercatorBounds::FromLatLon(startPoint), + MercatorBounds::FromLatLon(finalPoint))); vector absent; fetcher.GetAbsentCountries(absent); if (expected.size() < 2) @@ -384,7 +384,12 @@ namespace integration vector const & expected, IRouterComponents & routerComponents) { - routing::OnlineCrossFetcher fetcher(OSRM_ONLINE_SERVER_URL, startPoint, finalPoint); + TCountryFileFn const countryFileGetter = [&](m2::PointD const & p) { + return routerComponents.GetCountryInfoGetter().GetRegionCountryId(p); + }; + routing::OnlineCrossFetcher fetcher(countryFileGetter, OSRM_ONLINE_SERVER_URL, + Checkpoints(MercatorBounds::FromLatLon(startPoint), + MercatorBounds::FromLatLon(finalPoint))); fetcher.Do(); vector const & points = fetcher.GetMwmPoints(); set foundMwms; diff --git a/routing/routing_tests/async_router_test.cpp b/routing/routing_tests/async_router_test.cpp index 7a4c9a219d..5ca3e475ae 100644 --- a/routing/routing_tests/async_router_test.cpp +++ b/routing/routing_tests/async_router_test.cpp @@ -50,7 +50,7 @@ public: DummyFetcher(vector const & absent) : m_absent(absent) {} // IOnlineFetcher overrides: - void GenerateRequest(m2::PointD const & startPoint, m2::PointD const & finalPoint) override {} + void GenerateRequest(Checkpoints const &) override {} void GetAbsentCountries(vector & countries) override { countries = m_absent; } }; diff --git a/routing/routing_tests/online_cross_fetcher_test.cpp b/routing/routing_tests/online_cross_fetcher_test.cpp index 2dc7725090..f00cfa28c0 100644 --- a/routing/routing_tests/online_cross_fetcher_test.cpp +++ b/routing/routing_tests/online_cross_fetcher_test.cpp @@ -52,7 +52,7 @@ UNIT_TEST(GarbadgeInputToResponseParser) UNIT_TEST(OnlineAbsentFetcherSingleMwmTest) { OnlineAbsentCountriesFetcher fetcher([](m2::PointD const & p){return "A";}, [](string const &){return false;}); - fetcher.GenerateRequest({1, 1}, {2, 2}); + fetcher.GenerateRequest(Checkpoints({1, 1}, {2, 2})); vector countries; fetcher.GetAbsentCountries(countries); TEST(countries.empty(), ());