Merge pull request #3736 from ygorshenin/fix-affiliation-name-loading

[search] Fixed affiliation-name loading.

3 files changed, 60 insertions, 13 deletions

diff --git a/search/geocoder.cpp b/search/geocoder.cpp
index 17805f84d2..74ae905ba4 100644
--- a/search/geocoder.cpp
+++ b/search/geocoder.cpp
@@ -232,10 +232,17 @@ void JoinQueryTokens(QueryParams const & params, size_t curToken, size_t endToke
   }
 }
 
-void GetAffiliationName(FeatureType const & ft, string & name)
+WARN_UNUSED_RESULT bool GetAffiliationName(FeatureType const & ft, string & affiliation)
 {
-  VERIFY(ft.GetName(StringUtf8Multilang::kDefaultCode, name), ());
-  ASSERT(!name.empty(), ());
+  affiliation.clear();
+
+  if (ft.GetName(StringUtf8Multilang::kDefaultCode, affiliation) && !affiliation.empty())
+    return true;
+
+  // As a best effort, we try to read an english name if default name is absent.
+  if (ft.GetName(StringUtf8Multilang::kEnglishCode, affiliation) && !affiliation.empty())
+    return true;
+  return false;
 }
 
 // todo(@m) Refactor at least here, or even at indexer/ftypes_matcher.hpp.
@@ -735,16 +742,22 @@ void Geocoder::FillLocalitiesTable(BaseContext const & ctx)
     {
       if (count < maxCount && ft.GetFeatureType() == feature::GEOM_POINT)
       {
+        string affiliation;
+        if (!GetAffiliationName(ft, affiliation))
+          return;
+
         Region region(l, type);
         region.m_center = ft.GetCenter();
 
-        string name;
-        GetAffiliationName(ft, region.m_enName);
-        LOG(LDEBUG, ("Region =", region.m_enName));
+        ft.GetName(StringUtf8Multilang::kDefaultCode, region.m_defaultName);
+        LOG(LDEBUG, ("Region =", region.m_defaultName));
 
-        m_infoGetter.GetMatchedRegions(region.m_enName, region.m_ids);
+        m_infoGetter.GetMatchedRegions(affiliation, region.m_ids);
         if (region.m_ids.empty())
-          LOG(LWARNING, ("Maps not found for region", region.m_enName));
+        {
+          LOG(LWARNING,
+              ("Maps not found for region:", region.m_defaultName, "affiliation:", affiliation));
+        }
 
         ++count;
         m_regions[type][make_pair(l.m_startToken, l.m_endToken)].push_back(region);
diff --git a/search/geocoder.hpp b/search/geocoder.hpp
index 56f5cfcbde..980bae0916 100644
--- a/search/geocoder.hpp
+++ b/search/geocoder.hpp
@@ -113,7 +113,7 @@ public:
     Region(Locality const & l, RegionType type) : Locality(l), m_center(0, 0), m_type(type) {}
 
     storage::CountryInfoGetter::TRegionIdSet m_ids;
-    string m_enName;
+    string m_defaultName;
     m2::PointD m_center;
     RegionType m_type;
   };
diff --git a/search/search_integration_tests/smoke_test.cpp b/search/search_integration_tests/smoke_test.cpp
index 0ece6f1cf0..93eaf018ec 100644
--- a/search/search_integration_tests/smoke_test.cpp
+++ b/search/search_integration_tests/smoke_test.cpp
@@ -4,9 +4,12 @@
 #include "search/search_tests_support/test_results_matching.hpp"
 #include "search/search_tests_support/test_search_request.hpp"
 
+#include "generator/feature_builder.hpp"
 #include "generator/generator_tests_support/test_feature.hpp"
 #include "generator/generator_tests_support/test_mwm_builder.hpp"
 
+#include "indexer/classificator.hpp"
+
 #include "geometry/point2d.hpp"
 #include "geometry/rect2d.hpp"
 
@@ -22,6 +25,28 @@ namespace search
 {
 namespace
 {
+class IncorrectCountry : public TestCountry
+{
+public:
+  IncorrectCountry(m2::PointD const & center, string const & name, string const & lang)
+    : TestCountry(center, name, lang)
+  {
+  }
+
+  // TestFeature overrides:
+  void Serialize(FeatureBuilder1 & fb) const override
+  {
+    fb.SetTestId(m_id);
+    fb.SetCenter(m_center);
+
+    if (!m_name.empty())
+      CHECK(fb.AddName(m_lang, m_name), ("Can't set feature name:", m_name, "(", m_lang, ")"));
+
+    auto const & classificator = classif();
+    fb.AddType(classificator.GetTypeByPath({"place", "country"}));
+  }
+};
+
 class SmokeTest : public SearchTest
 {
 };
@@ -35,8 +60,7 @@ UNIT_CLASS_TEST(SmokeTest, Smoke)
   TestPOI brandyShop(m2::PointD(0, 1), "Brandy shop", "en");
   TestPOI vodkaShop(m2::PointD(1, 1), "Russian vodka shop", "en");
 
-  auto id = BuildMwm(kCountryName, feature::DataHeader::country, [&](TestMwmBuilder & builder)
-  {
+  auto id = BuildMwm(kCountryName, feature::DataHeader::country, [&](TestMwmBuilder & builder) {
     builder.Add(wineShop);
     builder.Add(tequilaShop);
     builder.Add(brandyShop);
@@ -63,8 +87,7 @@ UNIT_CLASS_TEST(SmokeTest, NotPrefixFreeNames)
 {
   char const kCountryName[] = "ATown";
 
-  auto id = BuildMwm(kCountryName, feature::DataHeader::country, [&](TestMwmBuilder & builder)
-  {
+  auto id = BuildMwm(kCountryName, feature::DataHeader::country, [&](TestMwmBuilder & builder) {
     builder.Add(TestPOI(m2::PointD(0, 0), "a", "en"));
     builder.Add(TestPOI(m2::PointD(0, 1), "aa", "en"));
     builder.Add(TestPOI(m2::PointD(1, 1), "aa", "en"));
@@ -92,5 +115,16 @@ UNIT_CLASS_TEST(SmokeTest, NotPrefixFreeNames)
     TEST_EQUAL(3, request.Results().size(), ());
   }
 }
+
+UNIT_CLASS_TEST(SmokeTest, NoDefaultNameTest)
+{
+  char const kCountryName[] = "Wonderland";
+
+  IncorrectCountry wonderland(m2::PointD(0, 0), kCountryName, "en");
+  auto worldId = BuildWorld([&](TestMwmBuilder & builder) { builder.Add(wonderland); });
+
+  SetViewport(m2::RectD(m2::PointD(-0.5, -0.5), m2::PointD(0.5, 0.5)));
+  TEST(ResultsMatch("Wonderland", {ExactMatch(worldId, wonderland)}), ());
+}
 }  // namespace
 }  // namespace search