From 46b0728df00db943fe5f94e18bd8ce65bef9c5cf Mon Sep 17 00:00:00 2001 From: Sergey Magidovich Date: Tue, 17 May 2016 17:11:17 +0300 Subject: [PATCH] Implement fields validation in editor. --- indexer/editable_map_object.cpp | 176 +++++++++++++++--- indexer/editable_map_object.hpp | 21 ++- .../editable_map_object_test.cpp | 113 +++++++++++ indexer/indexer_tests/indexer_tests.pro | 1 + .../search_tests/postcodes_matcher_tests.cpp | 72 ------- search/v2/postcodes_matcher.cpp | 24 +++ search/v2/postcodes_matcher.hpp | 5 + std/cmath.hpp | 1 + 8 files changed, 305 insertions(+), 108 deletions(-) create mode 100644 indexer/indexer_tests/editable_map_object_test.cpp delete mode 100644 search/search_tests/postcodes_matcher_tests.cpp diff --git a/indexer/editable_map_object.cpp b/indexer/editable_map_object.cpp index b8cd947c09..e193decb50 100644 --- a/indexer/editable_map_object.cpp +++ b/indexer/editable_map_object.cpp @@ -1,3 +1,5 @@ +#include "search/v2/postcodes_matcher.hpp" + #include "indexer/classificator.hpp" #include "indexer/cuisines.hpp" #include "indexer/editable_map_object.hpp" @@ -6,6 +8,7 @@ #include "base/string_utils.hpp" #include "std/cctype.hpp" +#include "std/cmath.hpp" namespace osm { @@ -95,29 +98,6 @@ void EditableMapObject::SetNearbyStreets(vector && streets) m_nearbyStreets = move(streets); } -// static -bool EditableMapObject::ValidateHouseNumber(string const & houseNumber) -{ - if (houseNumber.empty()) - return true; - - strings::UniString us = strings::MakeUniString(houseNumber); - // TODO: Improve this basic limit - it was choosen by @Zverik. - auto constexpr kMaxHouseNumberLength = 15; - if (us.size() > kMaxHouseNumberLength) - return false; - - // TODO: Should we allow arabic numbers like U+0661 ١ Arabic-Indic Digit One? - strings::NormalizeDigits(us); - for (strings::UniChar const c : us) - { - // Valid house numbers contain at least one number. - if (c >= '0' && c <= '9') - return true; - } - return false; -} - void EditableMapObject::SetHouseNumber(string const & houseNumber) { m_houseNumber = houseNumber; @@ -143,8 +123,14 @@ void EditableMapObject::SetEmail(string const & email) m_metadata.Set(feature::Metadata::FMD_EMAIL, email); } -void EditableMapObject::SetWebsite(string const & website) +void EditableMapObject::SetWebsite(string website) { + if (!website.empty() && + !strings::StartsWith(website, "http://") && + !strings::StartsWith(website, "https://")) + { + website = "http://" + website; + } m_metadata.Set(feature::Metadata::FMD_WEBSITE, website); m_metadata.Drop(feature::Metadata::FMD_URL); } @@ -188,13 +174,6 @@ void EditableMapObject::SetFlats(string const & flats) m_metadata.Set(feature::Metadata::FMD_FLATS, flats); } -// static -bool EditableMapObject::ValidateBuildingLevels(string const & buildingLevels) -{ - uint64_t levels; - return strings::to_uint64(buildingLevels, levels) && levels <= kMaximumLevelsEditableByUsers; -} - void EditableMapObject::SetBuildingLevels(string const & buildingLevels) { m_metadata.Set(feature::Metadata::FMD_BUILDING_LEVELS, buildingLevels); @@ -213,4 +192,139 @@ void EditableMapObject::SetOpeningHours(string const & openingHours) } void EditableMapObject::SetPointType() { m_geomType = feature::EGeomType::GEOM_POINT; } + +// static +bool EditableMapObject::ValidateBuildingLevels(string const & buildingLevels) +{ + if (buildingLevels.size() > log10(kMaximumLevelsEditableByUsers) + 1) + return false; + + uint64_t levels; + return strings::to_uint64(buildingLevels, levels) && levels <= kMaximumLevelsEditableByUsers; +} + +// static +bool EditableMapObject::ValidateHouseNumber(string const & houseNumber) +{ + if (houseNumber.empty()) + return true; + + strings::UniString us = strings::MakeUniString(houseNumber); + // TODO: Improve this basic limit - it was choosen by @Zverik. + auto constexpr kMaxHouseNumberLength = 15; + if (us.size() > kMaxHouseNumberLength) + return false; + + // TODO: Should we allow arabic numbers like U+0661 ١ Arabic-Indic Digit One? + strings::NormalizeDigits(us); + for (auto const c : us) + { + // Valid house numbers contain at least one digit. + if (c >= '0' && c <= '9') + return true; + } + return false; +} + +// static +bool EditableMapObject::ValidateFlats(string const & flats) +{ + auto it = strings::SimpleTokenizer(flats, ";"); + for (; it != strings::SimpleTokenizer(); ++it) + { + auto token = *it; + strings::Trim(token); + vector range(strings::SimpleTokenizer(token, "-"), strings::SimpleTokenizer()); + if (range.empty() || range.size() > 2) + return false; + + for (auto const & rangeBorder : range) + { + if (!all_of(begin(rangeBorder), end(rangeBorder), isalnum)) + return false; + } + } + return true; +} + +// static +bool EditableMapObject::ValidatePostCode(string const & postCode) +{ + return search::LooksLikePostcode(postCode, false /* check whole matching */); +} + +// static +bool EditableMapObject::ValidatePhone(string const & phone) +{ + if (phone.empty()) + return true; + + auto start = begin(phone); + auto const stop = end(phone); + + auto const kMaxNumberLen = 15; + auto const kMinNumberLen = 5; + + if (*start == '+') + ++start; + + auto digitsCount = 0; + for (; start != stop; ++start) + { + auto const isCharValid = isdigit(*start) || *start == '(' || + *start == ')' || *start == ' ' || *start == '-'; + if (!isCharValid) + return false; + + if (isdigit(*start)) + ++digitsCount; + } + + return kMinNumberLen <= digitsCount && digitsCount <= kMaxNumberLen; +} + +// static +bool EditableMapObject::ValidateFax(string const & fax) +{ + return ValidatePhone(fax); +} + +// static +bool EditableMapObject::ValidateWebsite(string const & site) +{ + if (site.empty()) + return true; + + auto const dotPos = find(begin(site), end(site), '.'); + // Site should contain at least one dot but not at the begining and not at the and. + if (dotPos == end(site) || site.front() == '.' || site.back() == '.') + return false; + + return true; +} + +// static +bool EditableMapObject::ValidateEmail(string const & email) +{ + if (email.empty()) + return true; + + auto const atPos = find(begin(email), end(email), '@'); + if (atPos == end(email)) + return false; + + // There should be only one '@' sign. + if (find(next(atPos), end(email), '@') != end(email)) + return false; + + // There should be at least one '.' sign after '@' ... + if (find(next(atPos), end(email), '.') == end(email)) + return false; + + // ... not in the end. + if (email.back() == '.') + return false; + + return true; +} } // namespace osm diff --git a/indexer/editable_map_object.hpp b/indexer/editable_map_object.hpp index eef0ddb55d..ac7b0db100 100644 --- a/indexer/editable_map_object.hpp +++ b/indexer/editable_map_object.hpp @@ -78,24 +78,26 @@ public: void SetMercator(m2::PointD const & center); void SetType(uint32_t featureType); void SetID(FeatureID const & fid); + // void SetTypes(feature::TypesHolder const & types); void SetStreet(LocalizedStreet const & st); void SetNearbyStreets(vector && streets); - /// @returns false if house number fails validation. - static bool ValidateHouseNumber(string const & houseNumber); void SetHouseNumber(string const & houseNumber); void SetPostcode(string const & postcode); void SetPhone(string const & phone); void SetFax(string const & fax); + void SetEmail(string const & email); - void SetWebsite(string const & website); + void SetWebsite(string website); + void SetWikipedia(string const & wikipedia); + void SetInternet(Internet internet); void SetStars(int stars); void SetOperator(string const & op); + void SetElevation(double ele); - void SetWikipedia(string const & wikipedia); void SetFlats(string const & flats); - static bool ValidateBuildingLevels(string const & buildingLevels); + void SetBuildingLevels(string const & buildingLevels); /// @param[in] cuisine is a vector of osm cuisine ids. void SetCuisines(vector const & cuisine); @@ -104,6 +106,15 @@ public: /// Special mark that it's a point feature, not area or line. void SetPointType(); + static bool ValidateBuildingLevels(string const & buildingLevels); + static bool ValidateHouseNumber(string const & houseNumber); + static bool ValidateFlats(string const & flats); + static bool ValidatePostCode(string const & postCode); + static bool ValidatePhone(string const & phone); + static bool ValidateFax(string const & fax); + static bool ValidateWebsite(string const & site); + static bool ValidateEmail(string const & email); + private: string m_houseNumber; LocalizedStreet m_street; diff --git a/indexer/indexer_tests/editable_map_object_test.cpp b/indexer/indexer_tests/editable_map_object_test.cpp new file mode 100644 index 0000000000..2f808d84ae --- /dev/null +++ b/indexer/indexer_tests/editable_map_object_test.cpp @@ -0,0 +1,113 @@ +#include "testing/testing.hpp" + +#include "indexer/editable_map_object.hpp" + +using osm::EditableMapObject; + +UNIT_TEST(EditableMapObject_SetWebsite) +{ + EditableMapObject emo; + emo.SetWebsite("https://some.thing.org"); + TEST_EQUAL(emo.GetWebsite(), "https://some.thing.org", ()); + + emo.SetWebsite("some.thing.org"); + TEST_EQUAL(emo.GetWebsite(), "http://some.thing.org", ()); + + emo.SetWebsite("some.thing.org"); + TEST_EQUAL(emo.GetWebsite(), "http://some.thing.org", ()); + + emo.SetWebsite(""); + TEST_EQUAL(emo.GetWebsite(), "", ()); + +} + +UNIT_TEST(EditableMapObject_ValidateBuildingLevels) +{ + TEST(EditableMapObject::ValidateBuildingLevels(""), ()); + TEST(EditableMapObject::ValidateBuildingLevels("7"), ()); + TEST(EditableMapObject::ValidateBuildingLevels("17"), ()); + TEST(EditableMapObject::ValidateBuildingLevels("25"), ()); + TEST(!EditableMapObject::ValidateBuildingLevels("26"), ()); + TEST(!EditableMapObject::ValidateBuildingLevels("ab"), ()); + TEST(!EditableMapObject::ValidateBuildingLevels( + "2345534564564453645534545345534564564453645"), ()); +} + +UNIT_TEST(EditableMapObject_ValidateHouseNumber) +{ + TEST(EditableMapObject::ValidateHouseNumber(""), ()); + TEST(EditableMapObject::ValidateHouseNumber("qwer7ty"), ()); + TEST(EditableMapObject::ValidateHouseNumber("12345678"), ()); + + // House number must contain at least one number. + TEST(!EditableMapObject::ValidateHouseNumber("qwerty"), ()); + // House number is too long. + TEST(!EditableMapObject::ValidateHouseNumber("1234567890123456"), ()); +} + +UNIT_TEST(EditableMapObject_ValidateFlats) +{ + TEST(EditableMapObject::ValidateFlats(""), ()); + TEST(EditableMapObject::ValidateFlats("123"), ()); + TEST(EditableMapObject::ValidateFlats("123a"), ()); + TEST(EditableMapObject::ValidateFlats("a"), ()); + TEST(EditableMapObject::ValidateFlats("123-456;a-e"), ()); + TEST(EditableMapObject::ValidateFlats("123-456"), ()); + TEST(EditableMapObject::ValidateFlats("123-456; 43-45"), ()); + TEST(!EditableMapObject::ValidateFlats("234-234 124"), ()); + TEST(!EditableMapObject::ValidateFlats("123-345-567"), ()); + TEST(!EditableMapObject::ValidateFlats("234-234;234("), ()); + TEST(!EditableMapObject::ValidateFlats("-;"), ()); +} + +// See search_tests/postcodes_matcher_test.cpp +// UNIT_TEST(EditableMapObject_ValidatePostCode) +// { +// } + +UNIT_TEST(EditableMapObject_ValidatePhone) +{ + TEST(EditableMapObject::ValidatePhone(""), ()); + TEST(EditableMapObject::ValidatePhone("+7 000 000 00 00"), ()); + TEST(EditableMapObject::ValidatePhone("+7 (000) 000 00 00"), ()); + TEST(EditableMapObject::ValidatePhone("+7 0000000000"), ()); + TEST(EditableMapObject::ValidatePhone("+7 0000 000 000"), ()); + TEST(EditableMapObject::ValidatePhone("8 0000-000-000"), ()); + + TEST(EditableMapObject::ValidatePhone("000 00 00"), ()); + TEST(EditableMapObject::ValidatePhone("000 000 00"), ()); + TEST(EditableMapObject::ValidatePhone("+00 0000 000 000"), ()); + + TEST(!EditableMapObject::ValidatePhone("+00 0000 000 0000 000"), ()); + TEST(!EditableMapObject::ValidatePhone("00 00"), ()); + TEST(!EditableMapObject::ValidatePhone("acb"), ()); + TEST(!EditableMapObject::ValidatePhone("000 000 00b"), ()); +} + +// See validate phone. +// UNIT_TEST(EditableMapObject_ValidateFax) +// { +// } + +UNIT_TEST(EditableMapObject_ValidateWebsite) +{ + TEST(EditableMapObject::ValidateWebsite(""), ()); + TEST(EditableMapObject::ValidateWebsite("qwe.rty"), ()); + + TEST(!EditableMapObject::ValidateWebsite("qwerty"), ()); + TEST(!EditableMapObject::ValidateWebsite(".qwerty"), ()); + TEST(!EditableMapObject::ValidateWebsite("qwerty."), ()); + TEST(!EditableMapObject::ValidateWebsite(".qwerty."), ()); +} + +UNIT_TEST(EditableMapObject_ValidateEmail) +{ + TEST(EditableMapObject::ValidateEmail(""), ()); + TEST(EditableMapObject::ValidateEmail("e@ma.il"), ()); + TEST(EditableMapObject::ValidateEmail("e@ma.i.l"), ()); + + TEST(!EditableMapObject::ValidateEmail("e.ma.il"), ()); + TEST(!EditableMapObject::ValidateEmail("e@ma@il"), ()); + TEST(!EditableMapObject::ValidateEmail("e@ma@i.l"), ()); + TEST(!EditableMapObject::ValidateEmail("e@mail"), ()); +} diff --git a/indexer/indexer_tests/indexer_tests.pro b/indexer/indexer_tests/indexer_tests.pro index 35f2f070e3..1b007199a9 100644 --- a/indexer/indexer_tests/indexer_tests.pro +++ b/indexer/indexer_tests/indexer_tests.pro @@ -22,6 +22,7 @@ SOURCES += \ cell_id_test.cpp \ checker_test.cpp \ drules_selector_parser_test.cpp \ + editable_map_object_test.cpp \ feature_metadata_test.cpp \ feature_xml_test.cpp \ features_offsets_table_test.cpp \ diff --git a/search/search_tests/postcodes_matcher_tests.cpp b/search/search_tests/postcodes_matcher_tests.cpp deleted file mode 100644 index 161aa42b64..0000000000 --- a/search/search_tests/postcodes_matcher_tests.cpp +++ /dev/null @@ -1,72 +0,0 @@ -#include "../../testing/testing.hpp" - -#include "search/query_params.hpp" -#include "search/v2/postcodes_matcher.hpp" -#include "search/v2/token_slice.hpp" - -#include "indexer/search_delimiters.hpp" -#include "indexer/search_string_utils.hpp" - -#include "base/stl_add.hpp" -#include "base/string_utils.hpp" - -#include "std/string.hpp" -#include "std/vector.hpp" - -using namespace strings; - -namespace search -{ -namespace v2 -{ -namespace -{ -bool LooksLikePostcode(string const & s, bool checkPrefix) -{ - vector tokens; - bool const lastTokenIsPrefix = - TokenizeStringAndCheckIfLastTokenIsPrefix(s, tokens, search::Delimiters()); - - size_t const numTokens = tokens.size(); - - QueryParams params; - if (checkPrefix && lastTokenIsPrefix) - { - params.m_prefixTokens.push_back(tokens.back()); - tokens.pop_back(); - } - - for (auto const & token : tokens) - { - params.m_tokens.emplace_back(); - params.m_tokens.back().push_back(token); - } - - return LooksLikePostcode(TokenSlice(params, 0, numTokens)); -} - -UNIT_TEST(PostcodesMatcher_Smoke) -{ - TEST(LooksLikePostcode("141701", false /* checkPrefix */), ()); - TEST(LooksLikePostcode("141", true /* checkPrefix */), ()); - TEST(LooksLikePostcode("BA6 8JP", true /* checkPrefix */), ()); - TEST(LooksLikePostcode("BA6 8JP", true /* checkPrefix */), ()); - TEST(LooksLikePostcode("BA22 9HR", true /* checkPrefix */), ()); - TEST(LooksLikePostcode("BA22", true /* checkPrefix */), ()); - TEST(LooksLikePostcode("DE56 4FW", true /* checkPrefix */), ()); - TEST(LooksLikePostcode("NY 1000", true /* checkPrefix */), ()); - TEST(LooksLikePostcode("AZ 85203", true /* checkPrefix */), ()); - TEST(LooksLikePostcode("AZ", true /* checkPrefix */), ()); - - TEST(LooksLikePostcode("803 0271", true /* checkPrefix */), ()); - TEST(LooksLikePostcode("803-0271", true /* checkPrefix */), ()); - TEST(LooksLikePostcode("〒803-0271", true /* checkPrefix */), ()); - - TEST(!LooksLikePostcode("1 мая", true /* checkPrefix */), ()); - TEST(!LooksLikePostcode("1 мая улица", true /* checkPrefix */), ()); - TEST(!LooksLikePostcode("москва", true /* checkPrefix */), ()); - TEST(!LooksLikePostcode("39 с 79", true /* checkPrefix */), ()); -} -} // namespace -} // namespace v2 -} // namespace search diff --git a/search/v2/postcodes_matcher.cpp b/search/v2/postcodes_matcher.cpp index 8f2e41f9b8..e5112ad148 100644 --- a/search/v2/postcodes_matcher.cpp +++ b/search/v2/postcodes_matcher.cpp @@ -170,6 +170,30 @@ PostcodesMatcher const & GetPostcodesMatcher() bool LooksLikePostcode(TokenSlice const & slice) { return GetPostcodesMatcher().HasString(slice); } +bool LooksLikePostcode(string const & s, bool checkPrefix) +{ + vector tokens; + bool const lastTokenIsPrefix = + TokenizeStringAndCheckIfLastTokenIsPrefix(s, tokens, search::Delimiters()); + + size_t const numTokens = tokens.size(); + + SearchQueryParams params; + if (checkPrefix && lastTokenIsPrefix) + { + params.m_prefixTokens.push_back(tokens.back()); + tokens.pop_back(); + } + + for (auto const & token : tokens) + { + params.m_tokens.emplace_back(); + params.m_tokens.back().push_back(token); + } + + return LooksLikePostcode(TokenSlice(params, 0, numTokens)); +} + size_t GetMaxNumTokensInPostcode() { return GetPostcodesMatcher().GetMaxNumTokensInPostcode(); } } // namespace v2 } // namespace search diff --git a/search/v2/postcodes_matcher.hpp b/search/v2/postcodes_matcher.hpp index b0e2398e8e..f30b712b9b 100644 --- a/search/v2/postcodes_matcher.hpp +++ b/search/v2/postcodes_matcher.hpp @@ -1,6 +1,7 @@ #pragma once #include "std/cstdint.hpp" +#include "std/string.hpp" namespace search { @@ -9,6 +10,10 @@ namespace v2 class TokenSlice; bool LooksLikePostcode(TokenSlice const & slice); +/// Splits s into tokens and call LooksLikePostcode(TokenSlice) on the result. +/// If checkPrefix is true returns true if some postcode starts with s. +/// If checkPrefix is false returns true if s equals to some postcode. +bool LooksLikePostcode(string const & s, bool checkPrefix); size_t GetMaxNumTokensInPostcode(); } // namespace v2 diff --git a/std/cmath.hpp b/std/cmath.hpp index e2dcffaf63..b58de179db 100644 --- a/std/cmath.hpp +++ b/std/cmath.hpp @@ -13,6 +13,7 @@ using std::abs; using std::isfinite; +using std::log10; namespace math {