From 77379f841a322ccc56338dd748cd5255b9142805 Mon Sep 17 00:00:00 2001 From: Arsentiy Milchakov Date: Mon, 19 Oct 2020 16:32:01 +0300 Subject: [PATCH] [deep links] Deep link parsing refactoring. Review fixes. --- .../mapswithme/maps/api/ParsingResult.java | 6 +- map/map_tests/mwm_url_tests.cpp | 99 +++++++++++-------- map/mwm_url.cpp | 16 +++ map/mwm_url.hpp | 10 +- 4 files changed, 84 insertions(+), 47 deletions(-) diff --git a/android/src/com/mapswithme/maps/api/ParsingResult.java b/android/src/com/mapswithme/maps/api/ParsingResult.java index a250eae277..6f44493be9 100644 --- a/android/src/com/mapswithme/maps/api/ParsingResult.java +++ b/android/src/com/mapswithme/maps/api/ParsingResult.java @@ -23,13 +23,13 @@ public class ParsingResult public static final int TYPE_SUBSCRIPTION = 7; private final int mUrlType; - private final boolean mIsSuccess; + private final boolean mSuccess; @SuppressWarnings("unused") public ParsingResult(@UrlType int urlType, boolean isSuccess) { mUrlType = urlType; - mIsSuccess = isSuccess; + mSuccess = isSuccess; } @UrlType @@ -40,6 +40,6 @@ public class ParsingResult public boolean isSuccess() { - return mIsSuccess; + return mSuccess; } } diff --git a/map/map_tests/mwm_url_tests.cpp b/map/map_tests/mwm_url_tests.cpp index 65a12a2b0e..80abd7d97b 100644 --- a/map/map_tests/mwm_url_tests.cpp +++ b/map/map_tests/mwm_url_tests.cpp @@ -25,6 +25,7 @@ using namespace url_scheme; namespace { +using UrlType = ParsedMapApi::UrlType; static FrameworkParams const kFrameworkParams(false /* m_enableLocalAds */, false /* m_enableDiffs */); void ToMercatoToLatLon(double & lat, double & lon) @@ -111,11 +112,14 @@ private: BookmarkManager * m_m; }; -bool IsValid(Framework & fm, string const & urlString) +bool IsValid(Framework & fm, string const & urlString, UrlType expectedType) { ParsedMapApi api; api.SetBookmarkManager(&fm.GetBookmarkManager()); - api.SetUrlAndParse(urlString); + + auto [type, success] = api.SetUrlAndParse(urlString); + TEST_EQUAL(type, expectedType, ()); + fm.GetBookmarkManager().GetEditSession().ClearGroup(UserMark::Type::API); return api.IsValid(); @@ -166,13 +170,18 @@ UNIT_TEST(CatalogueApiSmoke) UNIT_TEST(CatalogueApiInvalidUrl) { Framework f(kFrameworkParams); - TEST(!IsValid(f, "mapsme://catalogue?"), ("id parameter is required")); - TEST(IsValid(f, "mapsme://catalogue?id=440f02e5-ff38-45ed-95c0-44587c9a5fc7"), ("Name is optional")); - TEST(IsValid(f, "mapsme://catalogue?id=440f02e5-ff38-45ed-95c0-44587c9a5fc7&name=CatalogGroupName&otherExtraParam=xyz"), + TEST(!IsValid(f, "mapsme://catalogue?", UrlType::Catalogue), ("id parameter is required")); + TEST(!IsValid(f, "mapsme://catalog?id=440f02e5-ff38-45ed-95c0-44587c9a5fc7", UrlType::Incorrect), + ("Incorrect url type")); + TEST(IsValid(f, "mapsme://catalogue?id=440f02e5-ff38-45ed-95c0-44587c9a5fc7", UrlType::Catalogue), + ("Name is optional")); + TEST(IsValid(f, "mapsme://catalogue?id=440f02e5-ff38-45ed-95c0-44587c9a5fc7&name=CatalogGroupName&otherExtraParam=xyz", + UrlType::Catalogue), ("We shouldn't fail on extra params")); - TEST(IsValid(f, "mapsme://catalogue?name=CatalogGroupName&id=440f02e5-ff38-45ed-95c0-44587c9a5fc7"), + TEST(IsValid(f, "mapsme://catalogue?name=CatalogGroupName&id=440f02e5-ff38-45ed-95c0-44587c9a5fc7", + UrlType::Catalogue), ("parameter position doesn't matter")); - TEST(!IsValid(f, "mapsme://catalogue?ID=440f02e5-ff38-45ed-95c0-44587c9a5fc7"), + TEST(!IsValid(f, "mapsme://catalogue?ID=440f02e5-ff38-45ed-95c0-44587c9a5fc7", UrlType::Catalogue), ("The parser is case sensitive")); } @@ -195,12 +204,13 @@ UNIT_TEST(SearchApiSmoke) UNIT_TEST(SearchApiInvalidUrl) { Framework f(kFrameworkParams); - TEST(!IsValid(f, "mapsme://search?"), ("The search query parameter is necessary")); - TEST(!IsValid(f, "mapsme://search?query"), ("Search query can't be empty")); - TEST(IsValid(f, "mapsme://search?query=aaa&cll=1,1,1"), ("If it's wrong lat lon format then just ignore it")); - TEST(IsValid(f, "mapsme://search?query=aaa&ignoreThisParam=sure"), ("We shouldn't fail search request if there are some unsupported parameters")); - TEST(IsValid(f, "mapsme://search?cll=1,1&locale=ru&query=aaa"), ("Query parameter position doesn't matter")); - TEST(!IsValid(f, "mapsme://search?Query=fff"), ("The parser is case sensitive")); + TEST(!IsValid(f, "mapsme://search?", UrlType::Search), ("The search query parameter is necessary")); + TEST(!IsValid(f, "mapsme://search?query", UrlType::Search), ("Search query can't be empty")); + TEST(IsValid(f, "mapsme://search?query=aaa&cll=1,1,1", UrlType::Search), ("If it's wrong lat lon format then just ignore it")); + TEST(IsValid(f, "mapsme://search?query=aaa&ignoreThisParam=sure", UrlType::Search), ("We shouldn't fail search request if there are some unsupported parameters")); + TEST(IsValid(f, "mapsme://search?cll=1,1&locale=ru&query=aaa", UrlType::Search), ("Query parameter position doesn't matter")); + TEST(!IsValid(f, "mapsme://serch?cll=1,1&locale=ru&query=aaa", UrlType::Incorrect), ("Incorrect url type")); + TEST(!IsValid(f, "mapsme://search?Query=fff", UrlType::Search), ("The parser is case sensitive")); } UNIT_TEST(LeadApiSmoke) @@ -232,52 +242,57 @@ UNIT_TEST(LeadApiSmoke) UNIT_TEST(LeadApiInvalid) { Framework f(kFrameworkParams); - TEST(!IsValid(f, "mapsme://lead?"), ("From, type and name parameters are necessary")); - TEST(!IsValid(f, "mapsme://lead?utm_source&utm_medium&utm_campaign"), ("Parameters can't be empty")); - TEST(IsValid(f, "mapsme://lead?utm_source=a&utm_medium=b&utm_campaign=c"), ("These parameters are enough")); - TEST(IsValid(f, "mapsme://lead?utm_source=a&utm_medium=b&utm_campaign=c&smh=smh"), ("If there is an excess parameter just ignore it")); - TEST(!IsValid(f, "mapsme://lead?utm_source=a&UTM_MEDIUM=b&utm_campaign=c&smh=smh"), ("The parser is case sensitive")); + TEST(!IsValid(f, "mapsme://lead?", UrlType::Lead), ("From, type and name parameters are necessary")); + TEST(!IsValid(f, "mapsme://lead?utm_source&utm_medium&utm_campaign", UrlType::Lead), + ("Parameters can't be empty")); + TEST(!IsValid(f, "mapsme://leed?utm_source=a&utm_medium=b&utm_campaign=c", UrlType::Incorrect), + ("Incorrect url type")); + TEST(IsValid(f, "mapsme://lead?utm_source=a&utm_medium=b&utm_campaign=c", UrlType::Lead), + ("These parameters are enough")); + TEST(IsValid(f, "mapsme://lead?utm_source=a&utm_medium=b&utm_campaign=c&smh=smh", UrlType::Lead), + ("If there is an excess parameter just ignore it")); + TEST(!IsValid(f, "mapsme://lead?utm_source=a&UTM_MEDIUM=b&utm_campaign=c&smh=smh", UrlType::Lead), + ("The parser is case sensitive")); } UNIT_TEST(MapApiInvalidUrl) { Framework fm(kFrameworkParams); - TEST(!IsValid(fm, "competitors://map?ll=12.3,34.54"), ()); - TEST(!IsValid(fm, "mapswithme://ggg?ll=12.3,34.54"), ()); - 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"), ()); - TEST(!IsValid(fm, "mapsme://map?LL=1,1"), ("The parser is case sensitive")); + TEST(!IsValid(fm, "competitors://map?ll=12.3,34.54", UrlType::Incorrect), ()); + TEST(!IsValid(fm, "mapswithme://ggg?ll=12.3,34.54", UrlType::Incorrect), ()); + TEST(!IsValid(fm, "mwm://", UrlType::Incorrect), ("No parameters")); + TEST(!IsValid(fm, "mapswithme://map?", UrlType::Map), ("No longtitude")); + TEST(!IsValid(fm, "mapswithme://map?ll=1,2,3", UrlType::Map), ("Too many values for ll")); + TEST(!IsValid(fm, "mapswithme://fffff://map?ll=1,2", UrlType::Incorrect), ()); + TEST(!IsValid(fm, "mapsme://map?LL=1,1", UrlType::Map), ("The parser is case sensitive")); } UNIT_TEST(RouteApiInvalidUrl) { Framework f(kFrameworkParams); - TEST(!IsValid(f, "mapswithme://route?sll=1,1&saddr=name0&dll=2,2&daddr=name2"), + TEST(!IsValid(f, "mapswithme://route?sll=1,1&saddr=name0&dll=2,2&daddr=name2", UrlType::Route), ("Route type doesn't exist")); - 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"), + TEST(!IsValid(f, "mapswithme://route?sll=1,1&saddr=name0", UrlType::Route), ("Destination doesn't exist")); + TEST(!IsValid(f, "mapswithme://route?sll=1,1&dll=2,2&type=vehicle", UrlType::Route), ("Source or destination name doesn't exist")); - 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(f, "mapswithme://route?type=vehicle"), ()); + TEST(!IsValid(f, "mapswithme://route?saddr=name0&daddr=name1&type=vehicle", UrlType::Route), ()); + TEST(!IsValid(f, "mapswithme://route?sll=1,1&sll=2.2&type=vehicle", UrlType::Route), ()); + TEST(!IsValid(f, "mapswithme://route?sll=1,1&dll=2.2&type=666", UrlType::Route), ()); + TEST(!IsValid(f, "mapswithme://route?sll=1,1&saddr=name0&sll=2,2&saddr=name1&type=vehicle", UrlType::Route), ()); + TEST(!IsValid(f, "mapswithme://route?sll=1,1&type=vehicle", UrlType::Route), ()); + TEST(!IsValid(f,"mapswithme://route?sll=1,1&saddr=name0&sll=2,2&saddr=name1&sll=1,1&saddr=name0&type=vehicle", + UrlType::Route), ()); + TEST(!IsValid(f, "mapswithme://route?type=vehicle", UrlType::Route), ()); + TEST(!IsValid(f, "mapswithme://rout?sll=1,1&saddr=name0&dll=2,2&daddr=name1&type=vehicle", UrlType::Incorrect), ()); } UNIT_TEST(MapApiLatLonLimits) { Framework fm(kFrameworkParams); - TEST(!IsValid(fm, "mapswithme://map?ll=-91,10"), ("Invalid latitude")); - TEST(!IsValid(fm, "mwm://map?ll=523.55,10"), ("Invalid latitude")); - TEST(!IsValid(fm, "mapswithme://map?ll=23.55,450"), ("Invalid longtitude")); - TEST(!IsValid(fm, "mapswithme://map?ll=23.55,-450"), ("Invalid longtitude")); + TEST(!IsValid(fm, "mapswithme://map?ll=-91,10", UrlType::Map), ("Invalid latitude")); + TEST(!IsValid(fm, "mwm://map?ll=523.55,10", UrlType::Map), ("Invalid latitude")); + TEST(!IsValid(fm, "mapswithme://map?ll=23.55,450", UrlType::Map), ("Invalid longtitude")); + TEST(!IsValid(fm, "mapswithme://map?ll=23.55,-450", UrlType::Map), ("Invalid longtitude")); } UNIT_TEST(MapApiPointNameBeforeLatLon) diff --git a/map/mwm_url.cpp b/map/mwm_url.cpp index 5d93521839..b3f5089be4 100644 --- a/map/mwm_url.cpp +++ b/map/mwm_url.cpp @@ -566,4 +566,20 @@ ApiMarkPoint const * ParsedMapApi::GetSinglePoint() const return static_cast(m_bmManager->GetUserMark(*markIds.begin())); } + +std::string DebugPrint(ParsedMapApi::UrlType type) +{ + switch(type) + { + case ParsedMapApi::UrlType::Incorrect: return "Incorrect"; + case ParsedMapApi::UrlType::Map: return "Map"; + case ParsedMapApi::UrlType::Route: return "Route"; + case ParsedMapApi::UrlType::Search: return "Search"; + case ParsedMapApi::UrlType::Lead: return "Lead"; + case ParsedMapApi::UrlType::Catalogue: return "Catalogue"; + case ParsedMapApi::UrlType::CataloguePath: return "CataloguePath"; + case ParsedMapApi::UrlType::Subscription: return "Subscription"; + } + UNREACHABLE(); +} } // namespace url_scheme diff --git a/map/mwm_url.hpp b/map/mwm_url.hpp index 40846c5d18..60e01d53c9 100644 --- a/map/mwm_url.hpp +++ b/map/mwm_url.hpp @@ -83,8 +83,8 @@ public: struct ParsingResult { - UrlType m_type; - bool m_isSuccess; + UrlType m_type = UrlType::Incorrect; + bool m_isSuccess = false; }; ParsedMapApi() = default; @@ -111,6 +111,10 @@ public: std::string const & GetAffiliateId() const { return m_affiliateId; } private: + /// Returns true when all statements are true: + /// - url parsed correctly; + /// - all mandatory parameters for url type |type| are provided; + /// - the order of params is correct (for UrlType::Map) bool Parse(url::Url const & url, UrlType type); void ParseAdditional(url::Url const & url); void ParseMapParam(url::Param const & param, std::vector & points, bool & correctOrder); @@ -138,4 +142,6 @@ private: bool m_goBackOnBalloonClick = false; bool m_isValid = false; }; + +std::string DebugPrint(ParsedMapApi::UrlType type); } // namespace url_scheme