[routing] Fetch required mwms from online server for all checkpoints

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,

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<OnlineCrossFetcher> fetcher =
-      make_unique<OnlineCrossFetcher>(OSRM_ONLINE_SERVER_URL, MercatorBounds::ToLatLon(startPoint),
-                                      MercatorBounds::ToLatLon(finalPoint));
+      make_unique<OnlineCrossFetcher>(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<string> & 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<OnlineCrossFetcher>()->GetMwmPoints())
   {
@@ -43,9 +48,23 @@ void OnlineAbsentCountriesFetcher::GetAbsentCountries(vector<string> & 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

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<string> & 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<string> & countries) override;
 
 private:
+  bool AllPointsInSameMwm(Checkpoints const &) const;
+
   TCountryFileFn const m_countryFileFn;
   TCountryLocalFileFn const m_countryLocalFileFn;
   unique_ptr<threads::Thread> m_fetcherThread;

routing/online_cross_fetcher.cpp

@@ -21,50 +21,66 @@ inline string LatLonToURLArgs(ms::LatLon const & point)
 
 namespace routing
 {
-bool ParseResponse(const string & serverResponse, vector<m2::PointD> & outPoints)
+bool ParseResponse(string const & serverResponse, vector<m2::PointD> & 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

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<m2::PointD> const & GetMwmPoints() { return m_mwmPoints; }
 
 private:
-  platform::HttpClient m_request;
+  TCountryFileFn const m_countryFileFn;
+  string const m_serverURL;
+  Checkpoints const m_checkpoints;
   vector<m2::PointD> m_mwmPoints;
 };
 }

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<string> absent;
     fetcher.GetAbsentCountries(absent);
     if (expected.size() < 2)
@@ -384,7 +384,12 @@ namespace integration
                          vector<string> 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<m2::PointD> const & points = fetcher.GetMwmPoints();
     set<string> foundMwms;

routing/routing_tests/async_router_test.cpp

@@ -50,7 +50,7 @@ public:
   DummyFetcher(vector<string> 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<string> & countries) override { countries = m_absent; }
 };
 

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<string> countries;
   fetcher.GetAbsentCountries(countries);
   TEST(countries.empty(), ());