Implement fields validation in editor.

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<LocalizedStreet> && 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<string> 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

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<LocalizedStreet> && 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<string> 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;

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"), ());
+}

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 \

search/search_tests/postcodes_matcher_tests.cpp

@@ -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<UniString> 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

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<UniString> 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

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

std/cmath.hpp

@@ -13,6 +13,7 @@
 
 using std::abs;
 using std::isfinite;
+using std::log10;
 
 namespace math
 {