diff --git a/coding/coding_tests/url_tests.cpp b/coding/coding_tests/url_tests.cpp index 0d573fdede..c0f18da886 100644 --- a/coding/coding_tests/url_tests.cpp +++ b/coding/coding_tests/url_tests.cpp @@ -35,13 +35,12 @@ public: } private: - bool AddTestValue(url::Param const & param) + void AddTestValue(url::Param const & param) { TEST(!m_keyValuePairs.empty(), ("Failed for url = ", m_url, "Passed KV = ", param)); TEST_EQUAL(m_keyValuePairs.front().first, param.m_name, ()); TEST_EQUAL(m_keyValuePairs.front().second, param.m_value, ()); m_keyValuePairs.pop(); - return true; } string m_url; diff --git a/coding/url.cpp b/coding/url.cpp index 9f3e09c767..6e7c361d9c 100644 --- a/coding/url.cpp +++ b/coding/url.cpp @@ -224,19 +224,10 @@ bool Url::Parse(std::string const & url) return true; } -bool Url::ForEachParam(Callback const & callback) const +void Url::ForEachParam(Callback const & callback) const { - // todo(@m) Looks strange but old code worked this way. - if (m_params.empty()) - return false; - for (auto const & param : m_params) - { - if (!callback(param)) - return false; - } - - return true; + callback(param); } string Make(string const & baseUrl, Params const & params) diff --git a/coding/url.hpp b/coding/url.hpp index 13c0d29ece..982a4595da 100644 --- a/coding/url.hpp +++ b/coding/url.hpp @@ -26,14 +26,14 @@ using Params = std::vector; class Url { public: - using Callback = std::function; + using Callback = std::function; explicit Url(std::string const & url); std::string const & GetScheme() const { return m_scheme; } std::string const & GetPath() const { return m_path; } bool IsValid() const { return !m_scheme.empty(); } - bool ForEachParam(Callback const & callback) const; + void ForEachParam(Callback const & callback) const; private: bool Parse(std::string const & url); diff --git a/map/map_tests/mwm_url_tests.cpp b/map/map_tests/mwm_url_tests.cpp index a452cc8c4c..5b3efd4b17 100644 --- a/map/map_tests/mwm_url_tests.cpp +++ b/map/map_tests/mwm_url_tests.cpp @@ -39,9 +39,11 @@ class ApiTest { public: explicit ApiTest(string const & urlString) - : m_fm(kFrameworkParams) + : m_framework(kFrameworkParams) { - m_m = &m_fm.GetBookmarkManager(); + df::VisualParams::Init(1.0, 1024); + + m_m = &m_framework.GetBookmarkManager(); m_api.SetBookmarkManager(m_m); auto const res = m_api.SetUrlAndParse(urlString); @@ -103,7 +105,7 @@ private: } private: - Framework m_fm; + Framework m_framework; ParsedMapApi m_api; m2::RectD m_viewportRect; BookmarkManager * m_m; @@ -118,7 +120,7 @@ bool IsValid(Framework & fm, string const & urlString) return api.IsValid(); } -} +} // namespace UNIT_TEST(MapApiSmoke) { diff --git a/map/mwm_url.cpp b/map/mwm_url.cpp index 8a9c0ae273..9b147d50c8 100644 --- a/map/mwm_url.cpp +++ b/map/mwm_url.cpp @@ -11,8 +11,6 @@ #include "platform/marketing_service.hpp" #include "platform/settings.hpp" -#include "coding/url.hpp" - #include "base/logging.hpp" #include "base/string_utils.hpp" @@ -127,12 +125,12 @@ enum class ApiURLType std::array const kAvailableSchemes = {{"mapswithme", "mwm", "mapsme"}}; -ApiURLType URLType(url::Url const & uri) +ApiURLType URLType(url::Url const & url) { - if (std::find(kAvailableSchemes.begin(), kAvailableSchemes.end(), uri.GetScheme()) == kAvailableSchemes.end()) + if (std::find(kAvailableSchemes.begin(), kAvailableSchemes.end(), url.GetScheme()) == kAvailableSchemes.end()) return ApiURLType::Incorrect; - auto const path = uri.GetPath(); + auto const path = url.GetPath(); if (path == "map") return ApiURLType::Map; if (path == "route") @@ -151,8 +149,11 @@ ApiURLType URLType(url::Url const & uri) return ApiURLType::Incorrect; } -bool ParseLatLon(string const & key, string const & value, double & lat, double & lon) +bool ParseLatLon(url::Param const & param, double & lat, double & lon) { + string const & key = param.m_name; + string const & value = param.m_value; + size_t const firstComma = value.find(','); if (firstComma == string::npos) { @@ -196,20 +197,21 @@ ParsedMapApi::ParsingResult ParsedMapApi::SetUrlAndParse(string const & url) return res; } -ParsedMapApi::ParsingResult ParsedMapApi::Parse(url::Url const & uri) +ParsedMapApi::ParsingResult ParsedMapApi::Parse(url::Url const & url) { - switch (URLType(uri)) + switch (URLType(url)) { case ApiURLType::Incorrect: return ParsingResult::Incorrect; case ApiURLType::Map: { vector points; - auto const result = uri.ForEachParam([&points, this](url::Param const & param) { - return AddKeyValue(param.m_name, param.m_value, points); + bool correctOrder = true; + url.ForEachParam([&points, &correctOrder, this](url::Param const & param) { + ParseMapParam(param, points, correctOrder); }); - if (!result || points.empty()) + if (points.empty() || !correctOrder) return ParsingResult::Incorrect; ASSERT(m_bmManager != nullptr, ()); @@ -230,11 +232,11 @@ ParsedMapApi::ParsingResult ParsedMapApi::Parse(url::Url const & uri) m_routePoints.clear(); using namespace route; vector pattern{kSourceLatLon, kSourceName, kDestLatLon, kDestName, kRouteType}; - auto const result = uri.ForEachParam([&pattern, this](url::Param const & param) { - return RouteKeyValue(param.m_name, param.m_value, pattern); + url.ForEachParam([&pattern, this](url::Param const & param) { + ParseRouteParam(param, pattern); }); - if (!result || pattern.size() != 0) + if (pattern.size() != 0) return ParsingResult::Incorrect; if (m_routePoints.size() != 2) @@ -248,23 +250,23 @@ ParsedMapApi::ParsingResult ParsedMapApi::Parse(url::Url const & uri) case ApiURLType::Search: { SearchRequest request; - auto const result = uri.ForEachParam([&request, this](url::Param const & param) { - return SearchKeyValue(param.m_name, param.m_value, request); + url.ForEachParam([&request, this](url::Param const & param) { + ParseSearchParam(param, request); }); - if (!result) + if (request.m_query.empty()) return ParsingResult::Incorrect; m_request = request; - return request.m_query.empty() ? ParsingResult::Incorrect : ParsingResult::Search; + return ParsingResult::Search; } case ApiURLType::Lead: { lead::CampaignDescription description; - auto result = uri.ForEachParam([&description, this](url::Param const & param) { - return LeadKeyValue(param.m_name, param.m_value, description); + url.ForEachParam([&description, this](url::Param const & param) { + ParseLeadParam(param, description); }); - if (!result || !description.IsValid()) + if (!description.IsValid()) return ParsingResult::Incorrect; description.Write(); @@ -273,11 +275,11 @@ ParsedMapApi::ParsingResult ParsedMapApi::Parse(url::Url const & uri) case ApiURLType::Catalogue: { Catalog item; - auto const result = uri.ForEachParam([&item, this](url::Param const & param) { - return CatalogKeyValue(param.m_name, param.m_value, item); + url.ForEachParam([&item, this](url::Param const & param) { + ParseCatalogParam(param, item); }); - if (!result || item.m_id.empty()) + if (item.m_id.empty()) return ParsingResult::Incorrect; m_catalog = item; @@ -286,11 +288,11 @@ ParsedMapApi::ParsingResult ParsedMapApi::Parse(url::Url const & uri) case ApiURLType::CataloguePath: { CatalogPath item; - auto const result = uri.ForEachParam([&item, this](url::Param const & param) { - return CatalogPathKeyValue(param.m_name, param.m_value, item); + url.ForEachParam([&item, this](url::Param const & param) { + ParseCatalogPathParam(param, item); }); - if (!result || item.m_url.empty()) + if (item.m_url.empty()) return ParsingResult::Incorrect; m_catalogPath = item; @@ -299,11 +301,11 @@ ParsedMapApi::ParsingResult ParsedMapApi::Parse(url::Url const & uri) case ApiURLType::Subscription: { Subscription item; - auto const result = uri.ForEachParam([&item, this](url::Param const & param) { - return SubscriptionKeyValue(param.m_name, param.m_value, item); + url.ForEachParam([&item, this](url::Param const & param) { + ParseSubscriptionParam(param, item); }); - if (!result || item.m_groups.empty()) + if (item.m_groups.empty()) return ParsingResult::Incorrect; m_subscription = item; @@ -313,60 +315,19 @@ ParsedMapApi::ParsingResult ParsedMapApi::Parse(url::Url const & uri) UNREACHABLE(); } -bool ParsedMapApi::RouteKeyValue(string const & key, string const & value, vector & pattern) -{ - using namespace route; - - if (pattern.empty()) - return true; - - if (key != pattern.front()) - return false; - - if (key == kSourceLatLon || key == kDestLatLon) - { - double lat = 0.0; - double lon = 0.0; - if (!ParseLatLon(key, value, lat, lon)) - return false; - - RoutePoint p; - p.m_org = mercator::FromLatLon(lat, lon); - m_routePoints.push_back(p); - } - else if (key == kSourceName || key == kDestName) - { - m_routePoints.back().m_name = value; - } - else if (key == kRouteType) - { - string const lowerValue = strings::MakeLowerCase(value); - if (lowerValue == kRouteTypePedestrian || lowerValue == kRouteTypeVehicle || - lowerValue == kRouteTypeBicycle || lowerValue == kRouteTypeTransit) - { - m_routingType = lowerValue; - } - else - { - LOG(LWARNING, ("Incorrect routing type:", value)); - return false; - } - } - - pattern.erase(pattern.begin()); - return true; -} - -bool ParsedMapApi::AddKeyValue(string const & key, string const & value, vector & points) +void ParsedMapApi::ParseMapParam(url::Param const & param, vector & points, bool & correctOrder) { using namespace map; + string const & key = param.m_name; + string const & value = param.m_value; + if (key == kLatLon) { double lat = 0.0; double lon = 0.0; - if (!ParseLatLon(key, value, lat, lon)) - return false; + if (!ParseLatLon(param, lat, lon)) + return; ApiPoint pt{.m_lat = lat, .m_lon = lon}; points.push_back(pt); @@ -385,7 +346,8 @@ bool ParsedMapApi::AddKeyValue(string const & key, string const & value, vector< else { LOG(LWARNING, ("Map API: Point name with no point. 'll' should come first!")); - return false; + correctOrder = false; + return; } } else if (key == kId) @@ -397,7 +359,8 @@ bool ParsedMapApi::AddKeyValue(string const & key, string const & value, vector< else { LOG(LWARNING, ("Map API: Point url with no point. 'll' should come first!")); - return false; + correctOrder = false; + return; } } else if (key == kStyle) @@ -409,7 +372,7 @@ bool ParsedMapApi::AddKeyValue(string const & key, string const & value, vector< else { LOG(LWARNING, ("Map API: Point style with no point. 'll' should come first!")); - return false; + return; } } else if (key == kBackUrl) @@ -433,25 +396,67 @@ bool ParsedMapApi::AddKeyValue(string const & key, string const & value, vector< { m_goBackOnBalloonClick = true; } - return true; } -bool ParsedMapApi::SearchKeyValue(string const & key, string const & value, SearchRequest & request) const +void ParsedMapApi::ParseRouteParam(url::Param const & param, vector & pattern) +{ + using namespace route; + + string const & key = param.m_name; + string const & value = param.m_value; + + if (pattern.empty() || key != pattern.front()) + return; + + if (key == kSourceLatLon || key == kDestLatLon) + { + double lat = 0.0; + double lon = 0.0; + if (!ParseLatLon(param, lat, lon)) + return; + + RoutePoint p; + p.m_org = mercator::FromLatLon(lat, lon); + m_routePoints.push_back(p); + } + else if (key == kSourceName || key == kDestName) + { + m_routePoints.back().m_name = value; + } + else if (key == kRouteType) + { + string const lowerValue = strings::MakeLowerCase(value); + if (lowerValue == kRouteTypePedestrian || lowerValue == kRouteTypeVehicle || + lowerValue == kRouteTypeBicycle || lowerValue == kRouteTypeTransit) + { + m_routingType = lowerValue; + } + else + { + LOG(LWARNING, ("Incorrect routing type:", value)); + return; + } + } + + pattern.erase(pattern.begin()); +} + +void ParsedMapApi::ParseSearchParam(url::Param const & param, SearchRequest & request) const { using namespace search; + string const & key = param.m_name; + string const & value = param.m_value; + if (key == kQuery) { - if (value.empty()) - return false; - request.m_query = value; } else if (key == kCenterLatLon) { double lat = 0.0; double lon = 0.0; - if (ParseLatLon(key, value, lat, lon)) + if (ParseLatLon(param, lat, lon)) { request.m_centerLat = lat; request.m_centerLon = lon; @@ -465,14 +470,15 @@ bool ParsedMapApi::SearchKeyValue(string const & key, string const & value, Sear { request.m_isSearchOnMap = true; } - - return true; } -bool ParsedMapApi::LeadKeyValue(string const & key, string const & value, lead::CampaignDescription & description) const +void ParsedMapApi::ParseLeadParam(url::Param const & param, lead::CampaignDescription & description) const { using namespace lead; + string const & key = param.m_name; + string const & value = param.m_value; + if (key == kFrom) description.m_from = value; else if (key == kType) @@ -483,38 +489,31 @@ bool ParsedMapApi::LeadKeyValue(string const & key, string const & value, lead:: description.m_content = value; else if (key == kKeyword) description.m_keyword = value; - /* - We have to support parsing the uri which contains unregistred parameters. - */ - return true; } -bool ParsedMapApi::CatalogKeyValue(string const & key, string const & value, Catalog & item) const +void ParsedMapApi::ParseCatalogParam(url::Param const & param, Catalog & item) const { using namespace catalogue; + string const & key = param.m_name; + string const & value = param.m_value; + if (key == kName) item.m_name = value; else if (key == kId) item.m_id = value; - - return true; } -bool ParsedMapApi::CatalogPathKeyValue(string const & key, string const & value, CatalogPath & item) const +void ParsedMapApi::ParseCatalogPathParam(url::Param const & param, CatalogPath & item) const { - if (key == catalogue_path::kUrl) - item.m_url = value; - - return true; + if (param.m_name == catalogue_path::kUrl) + item.m_url = param.m_value; } -bool ParsedMapApi::SubscriptionKeyValue(string const & key, string const & value, Subscription & item) const +void ParsedMapApi::ParseSubscriptionParam(url::Param const & param, Subscription & item) const { - if (key == subscription::kGroups) - item.m_groups = value; - - return true; + if (param.m_name == subscription::kGroups) + item.m_groups = param.m_value; } void ParsedMapApi::Reset() diff --git a/map/mwm_url.hpp b/map/mwm_url.hpp index e801941041..bc4b4ff9eb 100644 --- a/map/mwm_url.hpp +++ b/map/mwm_url.hpp @@ -1,5 +1,7 @@ #pragma once +#include "coding/url.hpp" + #include "geometry/point2d.hpp" #include "geometry/rect2d.hpp" @@ -102,13 +104,13 @@ public: Subscription const & GetSubscription() const { return m_subscription; } private: ParsingResult Parse(url::Url const & url); - bool AddKeyValue(std::string const & key, std::string const & value, std::vector & points); - bool RouteKeyValue(std::string const & key, std::string const & value, std::vector & pattern); - bool SearchKeyValue(std::string const & key, std::string const & value, SearchRequest & request) const; - bool LeadKeyValue(std::string const & key, std::string const & value, lead::CampaignDescription & description) const; - bool CatalogKeyValue(std::string const & key, std::string const & value, Catalog & item) const; - bool CatalogPathKeyValue(std::string const & key, std::string const & value, CatalogPath & item) const; - bool SubscriptionKeyValue(std::string const & key, std::string const & value, Subscription & item) const; + void ParseMapParam(url::Param const & param, std::vector & points, bool & correctOrder); + void ParseRouteParam(url::Param const & param, std::vector & pattern); + void ParseSearchParam(url::Param const & param, SearchRequest & request) const; + void ParseLeadParam(url::Param const & param, lead::CampaignDescription & description) const; + void ParseCatalogParam(url::Param const & param, Catalog & item) const; + void ParseCatalogPathParam(url::Param const & param, CatalogPath & item) const; + void ParseSubscriptionParam(url::Param const & param, Subscription & item) const; BookmarkManager * m_bmManager = nullptr; std::vector m_routePoints;