diff --git a/coding/uri.cpp b/coding/uri.cpp index f3b9d9f577..4b98a96839 100644 --- a/coding/uri.cpp +++ b/coding/uri.cpp @@ -47,7 +47,7 @@ bool Uri::Parse() return true; } -bool Uri::ForEachKeyValue(CallbackT const & callback) const +bool Uri::ForEachKeyValue(TCallback const & callback) const { // parse query for keys and values size_t const count = m_url.size(); diff --git a/coding/uri.hpp b/coding/uri.hpp index f0ea0a7824..8d2bae7d20 100644 --- a/coding/uri.hpp +++ b/coding/uri.hpp @@ -13,7 +13,7 @@ namespace url_scheme class Uri { public: - typedef function CallbackT; + using TCallback = function; explicit Uri(string const & uri) : m_url(uri) { Init(); } Uri(char const * uri, size_t size) : m_url(uri, uri + size) { Init(); } @@ -21,7 +21,7 @@ public: string const & GetScheme() const { return m_scheme; } string const & GetPath() const { return m_path; } bool IsValid() const { return !m_scheme.empty(); } - bool ForEachKeyValue(CallbackT const & callback) const; + bool ForEachKeyValue(TCallback const & callback) const; private: void Init(); diff --git a/iphone/Maps/Classes/MapsAppDelegate.mm b/iphone/Maps/Classes/MapsAppDelegate.mm index 597f83a01d..b9d1894514 100644 --- a/iphone/Maps/Classes/MapsAppDelegate.mm +++ b/iphone/Maps/Classes/MapsAppDelegate.mm @@ -254,18 +254,20 @@ using namespace osm_auth_ios; } else if (m_mwmURL) { + using namespace url_scheme; + string const url = m_mwmURL.UTF8String; - auto const parsingType = f.ParseApiURL(url); + auto const parsingType = f.ParseAndSetApiURL(url); switch (parsingType) { - case url_scheme::ParsingResult::Incorrect: + case ParsedMapApi::ParsingResult::Incorrect: LOG(LWARNING, ("Incorrect parsing result for url:", url)); break; - case url_scheme::ParsingResult::Route: + case ParsedMapApi::ParsingResult::Route: { auto const parsedData = f.GetParsedRoutingData(); - f.SetRouter(parsedData.second); - auto const points = parsedData.first; + f.SetRouter(parsedData.m_type); + auto const points = parsedData.m_points; auto const & p1 = points[0]; auto const & p2 = points[1]; [[MWMRouter router] buildFromPoint:MWMRoutePoint(p1.m_org, @(p1.m_name.c_str())) @@ -275,10 +277,12 @@ using namespace osm_auth_ios; [self.mapViewController showAPIBar]; break; } - case url_scheme::ParsingResult::Map: - f.ShowMapForURL(url); - [self showMap]; - [self.mapViewController showAPIBar]; + case ParsedMapApi::ParsingResult::Map: + if (f.ShowMapForURL(url)) + { + [self showMap]; + [self.mapViewController showAPIBar]; + } break; } } diff --git a/map/framework.cpp b/map/framework.cpp index 9fe8f45303..8b1612f5ee 100644 --- a/map/framework.cpp +++ b/map/framework.cpp @@ -1749,16 +1749,13 @@ bool Framework::ShowMapForURL(string const & url) result = NEED_CLICK; } } - else if (StartsWith(url, "mapswithme://") || StartsWith(url, "mwm://") || - StartsWith(url, "mapsme://")) + else if (m_ParsedMapApi.IsValid()) { if (!m_ParsedMapApi.GetViewportRect(rect)) rect = df::GetWorldRect(); - if ((apiMark = m_ParsedMapApi.GetSinglePoint())) - result = NEED_CLICK; - else - result = NO_NEED_CLICK; + apiMark = m_ParsedMapApi.GetSinglePoint(); + result = apiMark ? NEED_CLICK : NO_NEED_CLICK; } else // Actually, we can parse any geo url scheme with correct coordinates. { @@ -1804,12 +1801,11 @@ bool Framework::ShowMapForURL(string const & url) return false; } -url_scheme::ParsingResult Framework::ParseApiURL(string const & url) +url_scheme::ParsedMapApi::ParsingResult Framework::ParseAndSetApiURL(string const & url) { using namespace url_scheme; - using namespace strings; - // clear every current API-mark. + // Clear every current API-mark. { UserMarkControllerGuard guard(m_bmManager, UserMarkType::API_MARK); guard.m_controller.Clear(); @@ -1817,24 +1813,13 @@ url_scheme::ParsingResult Framework::ParseApiURL(string const & url) guard.m_controller.SetIsDrawable(true); } - if (!StartsWith(url, "mapswithme://") && !StartsWith(url, "mwm://") && - !StartsWith(url, "mapsme://")) - return ParsingResult::Incorrect; - - auto const resultType = m_ParsedMapApi.SetUriAndParse(url); - - if (resultType == ParsingResult::Incorrect) - { - LOG(LWARNING, ("Incorrect api url", url)); - UserMarkControllerGuard guard(m_bmManager, UserMarkType::API_MARK); - guard.m_controller.SetIsVisible(false); - } - return resultType; + return m_ParsedMapApi.SetUriAndParse(url); } -Framework::TParsedRoutingPointAndType Framework::GetParsedRoutingData() const +Framework::ParsedRoutingData Framework::GetParsedRoutingData() const { - return {m_ParsedMapApi.GetRoutePoints(), routing::FromString(m_ParsedMapApi.GetRoutingType())}; + return Framework::ParsedRoutingData(m_ParsedMapApi.GetRoutePoints(), + routing::FromString(m_ParsedMapApi.GetRoutingType())); } void Framework::ForEachFeatureAtPoint(TFeatureTypeFn && fn, m2::PointD const & mercator, diff --git a/map/framework.hpp b/map/framework.hpp index ebd0e43306..ab884b26ea 100644 --- a/map/framework.hpp +++ b/map/framework.hpp @@ -503,10 +503,19 @@ public: /// Set correct viewport, parse API, show balloon. bool ShowMapForURL(string const & url); - url_scheme::ParsingResult ParseApiURL(string const & url); + url_scheme::ParsedMapApi::ParsingResult ParseAndSetApiURL(string const & url); - using TParsedRoutingPointAndType = pair, routing::RouterType>; - TParsedRoutingPointAndType GetParsedRoutingData() const; + struct ParsedRoutingData + { + ParsedRoutingData(vector const & points, routing::RouterType type) + : m_points(points), m_type(type) + { + } + vector m_points; + routing::RouterType m_type; + }; + + ParsedRoutingData GetParsedRoutingData() const; private: // TODO(vng): Uncomment when needed. diff --git a/map/geourl_process.cpp b/map/geourl_process.cpp index 9cb16670d8..ef9721c70b 100644 --- a/map/geourl_process.cpp +++ b/map/geourl_process.cpp @@ -123,6 +123,7 @@ namespace url_scheme { return (m_latPriority == m_lonPriority && m_latPriority != -1); } + bool operator()(string const & key, string const & value) { if (key == "z" || key == "zoom") @@ -148,14 +149,16 @@ namespace url_scheme { if (key == "lat") { - if (m_info.SetLat(x)) - m_latPriority = priority; + if (!m_info.SetLat(x)) + return false; + m_latPriority = priority; } else { ASSERT_EQUAL(key, "lon", ()); - if (m_info.SetLon(x)) - m_lonPriority = priority; + if (!m_info.SetLon(x)) + return false; + m_lonPriority = priority; } } } diff --git a/map/map_tests/mwm_url_tests.cpp b/map/map_tests/mwm_url_tests.cpp index 4b42ecf1dc..0d1261bd47 100644 --- a/map/map_tests/mwm_url_tests.cpp +++ b/map/map_tests/mwm_url_tests.cpp @@ -31,8 +31,8 @@ namespace m_m = &m_fm.GetBookmarkManager(); m_api.SetBookmarkManager(m_m); - ParsingResult const res = m_api.SetUriAndParse(uriString); - if (res != ParsingResult::Incorrect) + auto const res = m_api.SetUriAndParse(uriString); + if (res != ParsedMapApi::ParsingResult::Incorrect) { if (!m_api.GetViewportRect(m_viewportRect)) m_viewportRect = df::GetWorldRect(); @@ -69,6 +69,7 @@ namespace { return GetMark(index)->GetID() == id; } + bool TestRouteType(string const & type) const { return m_api.GetRoutingType() == type; } private: ApiMarkPoint const * GetMark(int index) const @@ -88,16 +89,14 @@ namespace bool IsValid(Framework & fm, string const & uriString) { ParsedMapApi api; - bool isValid; api.SetBookmarkManager(&fm.GetBookmarkManager()); api.SetUriAndParse(uriString); - isValid = api.IsValid(); { UserMarkControllerGuard guard(fm.GetBookmarkManager(), UserMarkType::API_MARK); guard.m_controller.Clear(); } - return isValid; + return api.IsValid(); } } @@ -137,26 +136,27 @@ UNIT_TEST(MapApiInvalidUrl) TEST(!IsValid(fm, "mwm://"), ("No parameters")); TEST(!IsValid(fm, "mapswithme://map?"), ("No longtitude")); TEST(!IsValid(fm, "mapswithme://map?ll=1,2,3"), ("Too many values for ll")); + TEST(!IsValid(fm, "mapswithme://fffff://map?ll=1,2"), ()); } UNIT_TEST(RouteApiInvalidUrl) { - Framework fm; - TEST(!IsValid(fm, "mapswithme://route?sll=1,1&saddr=name0&dll=2,2&daddr=name2"), + Framework f; + TEST(!IsValid(f, "mapswithme://route?sll=1,1&saddr=name0&dll=2,2&daddr=name2"), ("Route type doesn't exist")); - TEST(!IsValid(fm, "mapswithme://route?sll=1,1&saddr=name0"), ("Destination doesn't exist")); - TEST(!IsValid(fm, "mapswithme://route?sll=1,1&dll=2,2&type=vehicle"), + TEST(!IsValid(f, "mapswithme://route?sll=1,1&saddr=name0"), ("Destination doesn't exist")); + TEST(!IsValid(f, "mapswithme://route?sll=1,1&dll=2,2&type=vehicle"), ("Source or destination name doesn't exist")); - TEST(!IsValid(fm, "mapswithme://route?saddr=name0&daddr=name1&type=vehicle"), ()); - TEST(!IsValid(fm, "mapswithme://route?sll=1,1&sll=2.2&type=vehicle"), ()); - TEST(!IsValid(fm, "mapswithme://route?sll=1,1&dll=2.2&type=666"), ()); - TEST(!IsValid(fm, "mapswithme://route?sll=1,1&saddr=name0&sll=2,2&saddr=name1&type=vehicle"), ()); - TEST(!IsValid(fm, "mapswithme://route?sll=1,1&type=vehicle"), ()); - TEST(!IsValid(fm, + TEST(!IsValid(f, "mapswithme://route?saddr=name0&daddr=name1&type=vehicle"), ()); + TEST(!IsValid(f, "mapswithme://route?sll=1,1&sll=2.2&type=vehicle"), ()); + TEST(!IsValid(f, "mapswithme://route?sll=1,1&dll=2.2&type=666"), ()); + TEST(!IsValid(f, "mapswithme://route?sll=1,1&saddr=name0&sll=2,2&saddr=name1&type=vehicle"), ()); + TEST(!IsValid(f, "mapswithme://route?sll=1,1&type=vehicle"), ()); + TEST(!IsValid(f, "mapswithme://" "route?sll=1,1&saddr=name0&sll=2,2&saddr=name1&sll=1,1&saddr=name0&type=vehicle"), ()); - TEST(!IsValid(fm, "mapswithme://route?type=vehicle"), ()); + TEST(!IsValid(f, "mapswithme://route?type=vehicle"), ()); } UNIT_TEST(MapApiLatLonLimits) @@ -172,8 +172,7 @@ UNIT_TEST(MapApiPointNameBeforeLatLon) { ApiTest test("mapswithme://map?n=Name&ll=1,2"); TEST(!test.IsValid(), ()); - TEST_EQUAL(test.GetPointCount(), 1, ()); - TEST(test.TestName(0, ""), ()); + TEST_EQUAL(test.GetPointCount(), 0, ()); } UNIT_TEST(MapApiPointNameOverwritten) @@ -201,9 +200,7 @@ UNIT_TEST(MapApiInvalidPointLatLonButValidOtherParts) { ApiTest api("mapswithme://map?ll=1,1,1&n=A&ll=2,2&n=B&ll=3,3,3&n=C"); TEST(!api.IsValid(), ()); - TEST_EQUAL(api.GetPointCount(), 1, ()); - TEST(api.TestLatLon(0, 2, 2), ()); - TEST(api.TestName(0, "B"), ()); + TEST_EQUAL(api.GetPointCount(), 0, ()); } UNIT_TEST(MapApiPointURLEncoded) diff --git a/map/mwm_url.cpp b/map/mwm_url.cpp index 89794d56d9..593f9a3ea8 100644 --- a/map/mwm_url.cpp +++ b/map/mwm_url.cpp @@ -19,7 +19,6 @@ namespace url_scheme { - namespace { string const kLatLon = "ll"; @@ -36,28 +35,23 @@ string const kVersion = "v"; string const kAppName = "appname"; string const kBalloonAction = "balloonaction"; string const kRouteType = "type"; +string const kRouteTypeVehicle = "vehicle"; +string const kRouteTypePedestrian = "pedestrian"; +string const kRouteTypeBicycle = "bicycle"; -static int const INVALID_LAT_VALUE = -1000; - -bool ParseLatLon(double & lat, double & lon, string const & key, string const & value) +bool ParseLatLon(string const & key, string const & value, double & lat, double & lon) { size_t const firstComma = value.find(','); if (firstComma == string::npos) { - LOG(LWARNING, ("Map API: no comma between lat and lon for 'll' key", key, value)); - return false; - } - - if (value.find(',', firstComma + 1) != string::npos) - { - LOG(LWARNING, ("Map API: more than one comma in a value for 'll' key", key, value)); + LOG(LWARNING, ("Map API: no comma between lat and lon for key:", key, " value:", value)); return false; } if (!strings::to_double(value.substr(0, firstComma), lat) || !strings::to_double(value.substr(firstComma + 1), lon)) { - LOG(LWARNING, ("Map API: can't parse lat,lon for 'll' key", key, value)); + LOG(LWARNING, ("Map API: can't parse lat,lon for key:", key, " value:", value)); return false; } @@ -69,41 +63,41 @@ bool ParseLatLon(double & lat, double & lon, string const & key, string const & return true; } -bool IsInvalidApiPoint(ApiPoint const & p) { return p.m_lat == INVALID_LAT_VALUE; } - -} // unnames namespace - -ParsedMapApi::ParsedMapApi() - : m_bmManager(nullptr) - , m_version(0) - , m_zoomLevel(0.0) - , m_goBackOnBalloonClick(false) -{ -} +} // namespace void ParsedMapApi::SetBookmarkManager(BookmarkManager * manager) { m_bmManager = manager; } -ParsingResult ParsedMapApi::SetUriAndParse(string const & url) +ParsedMapApi::ParsingResult ParsedMapApi::SetUriAndParse(string const & url) { Reset(); + + if (!strings::StartsWith(url, "mapswithme://") && !strings::StartsWith(url, "mwm://") && + !strings::StartsWith(url, "mapsme://")) + { + return ParsingResult::Incorrect; + } + ParsingResult const res = Parse(url_scheme::Uri(url)); m_isValid = res != ParsingResult::Incorrect; return res; } -ParsingResult ParsedMapApi::Parse(Uri const & uri) +ParsedMapApi::ParsingResult ParsedMapApi::Parse(Uri const & uri) { string const & scheme = uri.GetScheme(); string const & path = uri.GetPath(); bool const isRoutePath = path == "route"; if ((scheme != "mapswithme" && scheme != "mwm" && scheme != "mapsme") || (path != "map" && !isRoutePath)) + { return ParsingResult::Incorrect; + } if (isRoutePath) { + m_routePoints.clear(); vector pattern{kSourceLatLon, kSourceName, kDestLatLon, kDestName, kRouteType}; if (!uri.ForEachKeyValue(bind(&ParsedMapApi::RouteKeyValue, this, _1, _2, ref(pattern)))) return ParsingResult::Incorrect; @@ -119,41 +113,38 @@ ParsingResult ParsedMapApi::Parse(Uri const & uri) return ParsingResult::Route; } - else + + vector points; + if (!uri.ForEachKeyValue(bind(&ParsedMapApi::AddKeyValue, this, _1, _2, ref(points)))) + return ParsingResult::Incorrect; + + if (points.empty()) + return ParsingResult::Incorrect; + + ASSERT(m_bmManager != nullptr, ()); + UserMarkControllerGuard guard(*m_bmManager, UserMarkType::API_MARK); + for (auto const & p : points) { - ASSERT(m_bmManager != nullptr, ()); - UserMarkControllerGuard guard(*m_bmManager, UserMarkType::API_MARK); - vector points; - if (!uri.ForEachKeyValue(bind(&ParsedMapApi::AddKeyValue, this, _1, _2, ref(points)))) - return ParsingResult::Incorrect; - - points.erase(remove_if(points.begin(), points.end(), &IsInvalidApiPoint), points.end()); - if ((isRoutePath && (points.size() < 2 || m_routingType.empty())) || points.empty()) - return ParsingResult::Incorrect; - - for (auto const & p : points) - { - m2::PointD glPoint(MercatorBounds::FromLatLon(p.m_lat, p.m_lon)); - ApiMarkPoint * mark = static_cast(guard.m_controller.CreateUserMark(glPoint)); - mark->SetName(p.m_name); - mark->SetID(p.m_id); - mark->SetStyle(style::GetSupportedStyle(p.m_style, p.m_name, "")); - } - - return ParsingResult::Map; + m2::PointD glPoint(MercatorBounds::FromLatLon(p.m_lat, p.m_lon)); + ApiMarkPoint * mark = static_cast(guard.m_controller.CreateUserMark(glPoint)); + mark->SetName(p.m_name); + mark->SetID(p.m_id); + mark->SetStyle(style::GetSupportedStyle(p.m_style, p.m_name, "")); } + + return ParsingResult::Map; } bool ParsedMapApi::RouteKeyValue(string key, string const & value, vector & pattern) { - if (key != pattern.front()) + if (pattern.empty() || key != pattern.front()) return false; if (key == kSourceLatLon || key == kDestLatLon) { double lat = 0.0; double lon = 0.0; - if (!ParseLatLon(lat, lon, key, value)) + if (!ParseLatLon(key, value, lat, lon)) return false; RoutePoint p; @@ -167,7 +158,7 @@ bool ParsedMapApi::RouteKeyValue(string key, string const & value, vector GetRoutePoints() const { return m_routePoints; } - string GetRoutingType() const { return m_routingType; } + vector const & GetRoutePoints() const { return m_routePoints; } + string const & GetRoutingType() const { return m_routingType; } private: ParsingResult Parse(Uri const & uri); bool AddKeyValue(string key, string const & value, vector & points); bool RouteKeyValue(string key, string const & value, vector & pattern); - BookmarkManager * m_bmManager; + BookmarkManager * m_bmManager = nullptr; vector m_routePoints; string m_globalBackUrl; string m_appTitle; string m_routingType; - int m_version; + int m_version = 0; /// Zoom level in OSM format (e.g. from 1.0 to 20.0) /// Taken into an account when calculating viewport rect, but only if points count is == 1 - double m_zoomLevel; + double m_zoomLevel = 0.0; bool m_goBackOnBalloonClick = false; bool m_isValid = false; };