review fixes

5 files changed, 108 insertions, 127 deletions

diff --git a/indexer/editable_map_object.cpp b/indexer/editable_map_object.cpp
index 3515027f97..e87414f626 100644
--- a/indexer/editable_map_object.cpp
+++ b/indexer/editable_map_object.cpp
@@ -38,25 +38,21 @@ bool ExtractName(StringUtf8Multilang const & names, int8_t const langCode,
   return true;
 }
 
-size_t PushMwmLanguages(StringUtf8Multilang const & names, vector<string> const & mwmLanguages, vector<osm::LocalizedName> & result)
+size_t PushMwmLanguages(StringUtf8Multilang const & names, vector<int8_t> const & mwmLanguages,
+                        vector<osm::LocalizedName> & result)
 {
   size_t count = 0;
-  auto langCode = StringUtf8Multilang::kUnsupportedLanguageCode;
   static size_t const kMaxCountMwmLanguages = 2;
-  
-  for (auto const & language : mwmLanguages)
+
+  for (size_t i = 0; i < mwmLanguages.size() && count < kMaxCountMwmLanguages; ++i)
   {
-    langCode = StringUtf8Multilang::GetLangIndex(language);
-    if (ExtractName(names, langCode, result))
+    if (ExtractName(names, mwmLanguages[i], result))
       ++count;
-    
-    if (count >= kMaxCountMwmLanguages)
-      return count;
   }
   
   return count;
 }
-}
+}  // namespace
 
 namespace osm
 {
@@ -100,37 +96,42 @@ StringUtf8Multilang const & EditableMapObject::GetName() const { return m_name;
 
 NamesDataSource EditableMapObject::GetNamesDataSource() const
 {
-  return GetNamesDataSource(m_name, GetMwmLanguages(), languages::GetCurrentNorm());
+  const auto mwmInfo = GetID().m_mwmId.GetInfo();
+
+  if (!mwmInfo)
+    return NamesDataSource();
+
+  vector<int8_t> mwmLanguages;
+  mwmInfo->GetRegionData().GetLanguages(mwmLanguages);
+
+  auto const userLangCode = StringUtf8Multilang::GetLangIndex(languages::GetCurrentNorm());
+
+  return GetNamesDataSource(m_name, mwmLanguages, userLangCode);
 }
 
 // static
 NamesDataSource EditableMapObject::GetNamesDataSource(StringUtf8Multilang const & source,
-                                                      vector<string> const & nativeMwmLanguages,
-                                                      string const & userLanguage)
+                                                      vector<int8_t> const & mwmLanguages,
+                                                      int8_t const userLangCode)
 {
   NamesDataSource result;
   auto & names = result.names;
   auto & mandatoryCount = result.mandatoryNamesCount;
   // Push Mwm languages.
-  mandatoryCount = PushMwmLanguages(source, nativeMwmLanguages, names);
+  mandatoryCount = PushMwmLanguages(source, mwmLanguages, names);
 
   // Push user's language.
-  auto const langCode = StringUtf8Multilang::GetLangIndex(userLanguage);
-  if (ExtractName(source, langCode, names))
-    ++mandatoryCount;
-
-  // Push international language.
-  if (ExtractName(source, StringUtf8Multilang::kInternationalCode, names))
+  if (ExtractName(source, userLangCode, names))
     ++mandatoryCount;
 
   // Push other languages.
-  source.ForEach([&names, mandatoryCount](int8_t code, string const & name) -> bool {
+  source.ForEach([&names, mandatoryCount](int8_t const code, string const & name) -> bool {
     // Exclude default name.
     if (StringUtf8Multilang::kDefaultCode == code)
       return true;
     
     auto const mandatoryNamesEnd = names.begin() + mandatoryCount;
-    // Exclude languages which already in container (languages with top priority).
+    // Exclude languages which are already in container (languages with top priority).
     auto const it = find_if(
         names.begin(), mandatoryNamesEnd,
         [code](LocalizedName const & localizedName) { return localizedName.m_code == code; });
@@ -171,44 +172,37 @@ void EditableMapObject::SetName(string name, int8_t langCode)
     return;
 
   ASSERT_NOT_EQUAL(StringUtf8Multilang::kDefaultCode, langCode,
-                   ("You trying to set ", name,
-                    " as default, but direct editing of default name is deprecated."));
+                   ("Direct editing of default name is deprecated."));
 
-  if (!Editor::Instance().WasDefaultNameSaved(GetID()) &&
-      CanUseAsDefaultName(langCode, m_name, GetMwmLanguages()))
+  if (!Editor::Instance().OriginalFeatureHasDefaultName(GetID()))
   {
-    m_name.AddString(StringUtf8Multilang::kDefaultCode, name);
+    const auto mwmInfo = GetID().m_mwmId.GetInfo();
+
+    if (mwmInfo)
+    {
+      vector<int8_t> mwmLanguages;
+      mwmInfo->GetRegionData().GetLanguages(mwmLanguages);
+
+      if (CanUseAsDefaultName(langCode, mwmLanguages))
+        m_name.AddString(StringUtf8Multilang::kDefaultCode, name);
+    }
   }
 
   m_name.AddString(langCode, name);
 }
 
 // static
-bool EditableMapObject::CanUseAsDefaultName(int8_t const langCode, StringUtf8Multilang const & name,
-                                            vector<string> const & nativeMwmLanguages)
+bool EditableMapObject::CanUseAsDefaultName(int8_t const lang, vector<int8_t> const & mwmLanguages)
 {
-  auto index = StringUtf8Multilang::kUnsupportedLanguageCode;
-  string unused;
-
-  // Languages priority: 1. Mwm languages 2. International language.
-  for (auto const & language : nativeMwmLanguages)
+  for (auto const & mwmLang : mwmLanguages)
   {
-    index = StringUtf8Multilang::GetLangIndex(language);
-
-    if (StringUtf8Multilang::kUnsupportedLanguageCode == index)
-      return false;
+    if (StringUtf8Multilang::kUnsupportedLanguageCode == mwmLang)
+      continue;
 
-    if (langCode == index)
+    if (lang == mwmLang)
       return true;
-
-    // A name with a higher priority was added already.
-    if (name.GetString(index, unused))
-      return false;
   }
 
-  if (langCode == StringUtf8Multilang::kInternationalCode)
-    return true;
-
   return false;
 }
 
diff --git a/indexer/editable_map_object.hpp b/indexer/editable_map_object.hpp
index e89aa543ad..d033bc3d04 100644
--- a/indexer/editable_map_object.hpp
+++ b/indexer/editable_map_object.hpp
@@ -49,8 +49,7 @@ struct LocalizedName
 // Class which contains vector of localized names with following priority:
 //  1. Names for Mwm languages
 //  2. User`s language name
-//  3. International name
-//  4. Other names
+//  3. Other names
 // and mandatoryNamesCount - count of names which should be always shown.
 struct NamesDataSource
 {
@@ -130,23 +129,13 @@ public:
   static bool ValidateWebsite(string const & site);
   static bool ValidateEmail(string const & email);
 
-  // TODO dummy, should be removed.
-  static vector<string> const & GetMwmLanguages()
-  {
-    static vector<string> const kNativelanguagesForMwm = {"de", "fr"};
-
-    return kNativelanguagesForMwm;
-  }
-  // TODO dummy, should be removed.
-
   // Check whether langCode can be used as default name.
-  static bool CanUseAsDefaultName(int8_t const langCode, StringUtf8Multilang const & name,
-                                  vector<string> const & nativeMwmLanguages);
+  static bool CanUseAsDefaultName(int8_t const langCode, vector<int8_t> const & nativeMwmLanguages);
 
   // See comment for NamesDataSource class.
   static NamesDataSource GetNamesDataSource(StringUtf8Multilang const & source,
-                                            vector<string> const & nativeMwmLanguages,
-                                            string const & userLanguage);
+                                            vector<int8_t> const & nativeMwmLanguages,
+                                            int8_t const userLanguage);
 
 private:
   string m_houseNumber;
diff --git a/indexer/indexer_tests/editable_map_object_test.cpp b/indexer/indexer_tests/editable_map_object_test.cpp
index d281221726..cfad66817d 100644
--- a/indexer/indexer_tests/editable_map_object_test.cpp
+++ b/indexer/indexer_tests/editable_map_object_test.cpp
@@ -6,6 +6,11 @@ namespace
 {
 using osm::EditableMapObject;
 
+int8_t const GetLangCode(char const * ch)
+{
+  return StringUtf8Multilang::GetLangIndex(ch);
+}
+  
 UNIT_TEST(EditableMapObject_SetWebsite)
 {
   EditableMapObject emo;
@@ -129,100 +134,94 @@ UNIT_TEST(EditableMapObject_ValidateEmail)
 UNIT_TEST(EditableMapObject_CanUseAsDefaultName)
 {
   EditableMapObject emo;
-  
-  TEST(EditableMapObject::CanUseAsDefaultName(StringUtf8Multilang::GetLangIndex("de"), emo.GetName(), {"de", "fr"}),
+  vector<int8_t> const nativeMwmLanguages {GetLangCode("de"), GetLangCode("fr")};
+
+  TEST(EditableMapObject::CanUseAsDefaultName(GetLangCode("de"), nativeMwmLanguages),
        ("Check possibility to use Mwm language code"));
-  TEST(EditableMapObject::CanUseAsDefaultName(StringUtf8Multilang::GetLangIndex("fr"), emo.GetName(), {"de", "fr"}),
+  TEST(EditableMapObject::CanUseAsDefaultName(GetLangCode("fr"), nativeMwmLanguages),
        ("Check possibility to use Mwm language code"));
-  TEST(EditableMapObject::CanUseAsDefaultName(StringUtf8Multilang::GetLangIndex("int_name"), emo.GetName(), {"de", "fr"}),
+  TEST(!EditableMapObject::CanUseAsDefaultName(GetLangCode("int_name"), nativeMwmLanguages),
        ("Check possibility to use international language code"));
-
-  TEST(!EditableMapObject::CanUseAsDefaultName(100, emo.GetName(), {"de", "fr"}),
+  TEST(!EditableMapObject::CanUseAsDefaultName(100, nativeMwmLanguages),
        ("Incorrect language code is not supported"));
-  TEST(!EditableMapObject::CanUseAsDefaultName(StringUtf8Multilang::GetLangIndex("en"), emo.GetName(), {"abcd"}),
+  TEST(!EditableMapObject::CanUseAsDefaultName(GetLangCode("en"), {GetLangCode("abcd")}),
        ("Incorrect Mwm language name is not supported"));
-  TEST(!EditableMapObject::CanUseAsDefaultName(StringUtf8Multilang::GetLangIndex("en"), emo.GetName(), {"de", "fr"}),
+  TEST(!EditableMapObject::CanUseAsDefaultName(GetLangCode("en"), nativeMwmLanguages),
        ("Can not to use language which not Mwm language or international"));
-  TEST(!EditableMapObject::CanUseAsDefaultName(StringUtf8Multilang::GetLangIndex("ru"), emo.GetName(), {"de", "fr"}),
+  TEST(!EditableMapObject::CanUseAsDefaultName(GetLangCode("ru"), nativeMwmLanguages),
        ("Check possibility to use user`s language code"));
 
   // Trying to use language codes in reverse priority.
   StringUtf8Multilang names;
-  names.AddString(StringUtf8Multilang::GetLangIndex("int_name"), "international name");
-  emo.SetName(names);
-
-  TEST(EditableMapObject::CanUseAsDefaultName(StringUtf8Multilang::GetLangIndex("int_name"), emo.GetName(), {"de", "fr"}),
-       ("It is possible to fix typo for international language"));
-
-  names.AddString(StringUtf8Multilang::GetLangIndex("fr"), "second mwm language");
+  names.AddString(GetLangCode("fr"), "second mwm language");
   emo.SetName(names);
 
-  TEST(EditableMapObject::CanUseAsDefaultName(StringUtf8Multilang::GetLangIndex("fr"), emo.GetName(), {"de", "fr"}),
+  TEST(EditableMapObject::CanUseAsDefaultName(GetLangCode("fr"), nativeMwmLanguages),
        ("It is possible to fix typo"));
-  TEST(!EditableMapObject::CanUseAsDefaultName(StringUtf8Multilang::GetLangIndex("int_name"), emo.GetName(), {"de", "fr"}),
-       ("Name on language with high priority was already entered"));
 
-  names.AddString(StringUtf8Multilang::GetLangIndex("de"), "first mwm language");
+  names.AddString(GetLangCode("de"), "first mwm language");
   emo.SetName(names);
 
-  TEST(EditableMapObject::CanUseAsDefaultName(StringUtf8Multilang::GetLangIndex("de"), emo.GetName(), {"de", "fr"}),
+  TEST(EditableMapObject::CanUseAsDefaultName(GetLangCode("de"), nativeMwmLanguages),
+       ("It is possible to fix typo"));
+  TEST(EditableMapObject::CanUseAsDefaultName(GetLangCode("fr"), nativeMwmLanguages),
        ("It is possible to fix typo"));
-  TEST(!EditableMapObject::CanUseAsDefaultName(StringUtf8Multilang::GetLangIndex("fr"), emo.GetName(), {"de", "fr"}),
-       ("Name on language with high priority was already entered"));
-  TEST(!EditableMapObject::CanUseAsDefaultName(StringUtf8Multilang::GetLangIndex("int_name"), emo.GetName(), {"de", "fr"}),
-       ("Name on language with high priority was already entered"));
 }
 
 UNIT_TEST(EditableMapObject_GetNamesDataSource)
 {
   EditableMapObject emo;
   StringUtf8Multilang names;
-  names.AddString(0, "Default name");
-  names.AddString(1, "Eng name");
-  names.AddString(7, "Int name");
-  names.AddString(6, "De name");
-  names.AddString(8, "Ru name");
-  names.AddString(9, "Sv name");
-  names.AddString(12, "By name");
-  names.AddString(14, "Ko name");
-  names.AddString(20, "It name");
+
+  names.AddString(GetLangCode("default"), "Default name");
+  names.AddString(GetLangCode("en"), "Eng name");
+  names.AddString(GetLangCode("int_name"), "Int name");
+  names.AddString(GetLangCode("de"), "De name");
+  names.AddString(GetLangCode("ru"), "Ru name");
+  names.AddString(GetLangCode("sv"), "Sv name");
+  names.AddString(GetLangCode("be"), "By name");
+  names.AddString(GetLangCode("ko"), "Ko name");
+  names.AddString(GetLangCode("it"), "It name");
   emo.SetName(names);
 
+  vector<int8_t> nativeMwmLanguages = {GetLangCode("de"), GetLangCode("fr")};
+
   auto const namesDataSource =
-      EditableMapObject::GetNamesDataSource(emo.GetName(), {"de", "fr"}, "ko");
+      EditableMapObject::GetNamesDataSource(emo.GetName(), nativeMwmLanguages, GetLangCode("ko"));
 
-  TEST(namesDataSource.names.size() == 9, ("All names except the default should be pushed into "
+  TEST_EQUAL(namesDataSource.names.size(), 9, ("All names except the default should be pushed into "
                                            "data source plus empty mandatory names"));
-  TEST(namesDataSource.mandatoryNamesCount == 4,
-       ("Mandatory names count should be equal == Mwm languages + user`s language + international "
-        "language"));
-  TEST(namesDataSource.names[0].m_code == 6 /*de*/,
-       ("Deutsch name should be first as first language in Mwm"));
-  TEST(namesDataSource.names[1].m_code == 3 /*fr*/,
-       ("French name should be second as second language in Mwm"));
-  TEST(namesDataSource.names[2].m_code == 14 /*ko*/,
-       ("Korean name should be third because user`s language should be followed by Mwm languages"));
-  TEST(namesDataSource.names[3].m_code == 7 /*int*/,
-       ("International name should be fourth because International language should be followed by "
-        "user`s language"));
+  TEST_EQUAL(namesDataSource.mandatoryNamesCount, 3,
+             ("Mandatory names count should be equal == Mwm languages + user`s language"));
+  TEST_EQUAL(namesDataSource.names[0].m_code, GetLangCode("de"),
+             ("Deutsch name should be first as first language in Mwm"));
+  TEST_EQUAL(namesDataSource.names[1].m_code, GetLangCode("fr"),
+             ("French name should be second as second language in Mwm"));
+  TEST_EQUAL(namesDataSource.names[2].m_code, GetLangCode("ko"),
+             ("Korean name should be third because user`s language should be followed by Mwm languages"));
 
   {
+    vector<int8_t> nativeMwmLanguages = {GetLangCode("de"), GetLangCode("fr")};
+
     auto const namesDataSource =
-        EditableMapObject::GetNamesDataSource(emo.GetName(), {"int_name"}, "int_name");
-    TEST(namesDataSource.names.size() == 8,
-         ("All names except the default should be pushed into data source"));
-    TEST(namesDataSource.mandatoryNamesCount == 1,
-         ("Mandatory names count should be equal == Mwm languages + user`s language + "
-          "international language. Excluding repetiton"));
+        EditableMapObject::GetNamesDataSource(emo.GetName(), nativeMwmLanguages, GetLangCode("fr"));
+    TEST_EQUAL(namesDataSource.names.size(), 9,
+               ("All names except the default should be pushed into data source"));
+    TEST_EQUAL(namesDataSource.mandatoryNamesCount, 2,
+               ("Mandatory names count should be equal == Mwm languages + user`s language. "
+                "Excluding repetiton"));
   }
   {
+    vector<int8_t> nativeMwmLanguages = {GetLangCode("fr"), GetLangCode("fr")};
+
     auto const namesDataSource =
-        EditableMapObject::GetNamesDataSource(emo.GetName(), {"fr", "fr"}, "fr");
-    TEST(namesDataSource.names.size() == 9, ("All names except the default should be pushed into "
+        EditableMapObject::GetNamesDataSource(emo.GetName(), nativeMwmLanguages, GetLangCode("fr"));
+    TEST_EQUAL(namesDataSource.names.size(), 9,
+               ("All names except the default should be pushed into "
                                              "data source plus empty mandatory names"));
-    TEST(namesDataSource.mandatoryNamesCount == 2,
-         ("Mandatory names count should be equal == Mwm languages + user`s language + "
-          "international language. Excluding repetiton"));
+    TEST_EQUAL(namesDataSource.mandatoryNamesCount, 1,
+               ("Mandatory names count should be equal == Mwm languages + user`s language. "
+                "Excluding repetiton"));
   }
 }
 }  // namespace
diff --git a/indexer/osm_editor.cpp b/indexer/osm_editor.cpp
index 6c9ac30912..549cef66ea 100644
--- a/indexer/osm_editor.cpp
+++ b/indexer/osm_editor.cpp
@@ -426,14 +426,11 @@ bool Editor::IsCreatedFeature(FeatureID const & fid)
   return fid.m_index >= kStartIndexForCreatedFeatures;
 }
 
-bool Editor::WasDefaultNameSaved(FeatureID const & fid) const
+bool Editor::OriginalFeatureHasDefaultName(FeatureID const & fid) const
 {
   if (IsCreatedFeature(fid))
     return false;
 
-  if (FeatureStatus::Created == GetFeatureStatus(fid))
-    return false;
-
   auto const originalFeaturePtr = m_getOriginalFeatureFn(fid);
   if (!originalFeaturePtr)
   {
@@ -442,7 +439,9 @@ bool Editor::WasDefaultNameSaved(FeatureID const & fid) const
     return false;
   }
 
-  return originalFeaturePtr->HasName();
+  auto const & names = originalFeaturePtr->GetNames();
+  
+  return names.HasString(StringUtf8Multilang::kDefaultCode);
 }
 
 /// Several cases should be handled while saving changes:
diff --git a/indexer/osm_editor.hpp b/indexer/osm_editor.hpp
index d11ff810be..d107e05333 100644
--- a/indexer/osm_editor.hpp
+++ b/indexer/osm_editor.hpp
@@ -167,8 +167,8 @@ public:
   // Use GetFeatureStatus(fid) instead. This function is used when a feature is
   // not yet saved and we have to know if it was modified or created.
   static bool IsCreatedFeature(FeatureID const & fid);
-  // Returns true in case when original feature from mwm has default name
-  bool WasDefaultNameSaved(FeatureID const & fid) const;
+  // Returns true if the original feature has default name.
+  bool OriginalFeatureHasDefaultName(FeatureID const & fid) const;
 
 private:
   // TODO(AlexZ): Synchronize Save call/make it on a separate thread.