From c969f54d1be34f83c38d765e1697e81a0ade2e8b Mon Sep 17 00:00:00 2001
From: Anatoly Serdtcev <serdtcev@maps.me>
Date: Thu, 15 Aug 2019 13:02:24 +0300
Subject: [PATCH] [geocoder] Fix indexing for subregion in locality

---
 generator/regions/level_region.hpp         |  6 +++--
 geocoder/geocoder_tests/geocoder_tests.cpp | 19 +++++++++++++
 geocoder/hierarchy.cpp                     | 31 +++++++++++++++++++++-
 geocoder/hierarchy.hpp                     |  2 ++
 4 files changed, 55 insertions(+), 3 deletions(-)

diff --git a/generator/regions/level_region.hpp b/generator/regions/level_region.hpp
index 2988223523..a2f2892f13 100644
--- a/generator/regions/level_region.hpp
+++ b/generator/regions/level_region.hpp
@@ -16,8 +16,10 @@ public:
   PlaceLevel GetLevel() const noexcept { return m_level; }
   void SetLevel(PlaceLevel level) { m_level = level; }
 
-  // Absolute rank values do not mean anything. But if the rank of the first object is more than the
-  // rank of the second object, then the first object is considered more nested.
+  // Absolute rank values do not mean anything (except 1-4 values). But if the rank of the first
+  // object is more than the rank of the second object, then the first object is considered
+  // more nested.
+  // Specific values: 1 - country, 2 - region, 3 - subregion, 4 - locality.
   uint8_t GetRank() const { return static_cast<uint8_t>(m_level); }
 
 private:
diff --git a/geocoder/geocoder_tests/geocoder_tests.cpp b/geocoder/geocoder_tests/geocoder_tests.cpp
index b6d9fda2da..5472da3c69 100644
--- a/geocoder/geocoder_tests/geocoder_tests.cpp
+++ b/geocoder/geocoder_tests/geocoder_tests.cpp
@@ -287,6 +287,25 @@ UNIT_TEST(Geocoder_LocalityBuilding)
   TestGeocoder(geocoder, "Zelenograd 2", {{building2, 1.0}});
 }
 
+// Geocoder_Subregion* -----------------------------------------------------------------------------
+UNIT_TEST(Geocoder_SubregionInLocality)
+{
+  string const kData = R"#(
+10 {"properties": {"locales": {"default": {"address": {"region": "Москва"}}}, "rank": 2}}
+11 {"properties": {"locales": {"default": {"address": {"locality": "Москва", "region": "Москва"}}}, "rank": 4}}
+12 {"properties": {"locales": {"default": {"address": {"subregion": "Северный административный округ", "locality": "Москва", "region": "Москва"}}}, "rank": 3}}
+)#";
+
+  ScopedFile const regionsJsonFile("regions.jsonl", kData);
+  Geocoder geocoder(regionsJsonFile.GetFullPath());
+
+  TestGeocoder(geocoder, "Северный административный округ", {{Id{0x12}, 1.0}});
+  TestGeocoder(geocoder, "Москва, Северный административный округ",
+               {{Id{0x12}, 1.0}, {Id{0x10}, 0.294118}, {Id{0x11}, 0.176471}});
+  TestGeocoder(geocoder, "Москва", {{Id{0x10}, 1.0}, {Id{0x11}, 0.6}});
+}
+
+//--------------------------------------------------------------------------------------------------
 UNIT_TEST(Geocoder_EmptyFileConcurrentRead)
 {
   ScopedFile const regionsJsonFile("regions.jsonl", "");
diff --git a/geocoder/hierarchy.cpp b/geocoder/hierarchy.cpp
index 4e65fa6988..a67ff79b18 100644
--- a/geocoder/hierarchy.cpp
+++ b/geocoder/hierarchy.cpp
@@ -64,7 +64,8 @@ bool Hierarchy::Entry::DeserializeAddressFromJSON(
     json_t * const root, NameDictionaryBuilder & normalizedNameDictionaryBuilder,
     ParsingStats & stats)
 {
-  auto const locales = base::GetJSONObligatoryFieldByPath(root, "properties", "locales");
+  auto const properties = base::GetJSONObligatoryField(root, "properties");
+  auto const locales = base::GetJSONObligatoryField(properties, "locales");
   m_normalizedAddress= {};
   for (size_t i = 0; i < static_cast<size_t>(Type::Count); ++i)
   {
@@ -83,6 +84,16 @@ bool Hierarchy::Entry::DeserializeAddressFromJSON(
     }
   }
 
+  if (auto const rank = FromJSONObjectOptional<uint8_t>(properties, "rank"))
+  {
+    auto const type = RankToType(*rank);
+    if (type != Type::Count &&
+        m_normalizedAddress[static_cast<size_t>(type)] != NameDictionary::kUnspecifiedPosition)
+    {
+      m_type = type;
+    }
+  }
+
   auto const & subregion = m_normalizedAddress[static_cast<size_t>(Type::Subregion)];
   auto const & locality = m_normalizedAddress[static_cast<size_t>(Type::Locality)];
   if (m_type == Type::Street && locality == NameDictionary::kUnspecifiedPosition &&
@@ -147,6 +158,24 @@ MultipleNames const & Hierarchy::Entry::GetNormalizedMultipleNames(
   return normalizedNameDictionary.Get(addressField);
 }
 
+// static
+Type Hierarchy::Entry::RankToType(uint8_t rank)
+{
+  switch (rank)
+  {
+  case 1:
+    return Type::Country;
+  case 2:
+    return Type::Region;
+  case 3:
+    return Type::Subregion;
+  case 4:
+    return Type::Locality;
+  }
+
+  return Type::Count;
+}
+
 // Hierarchy ---------------------------------------------------------------------------------------
 Hierarchy::Hierarchy(vector<Entry> && entries, NameDictionary && normalizedNameDictionary)
   : m_entries{move(entries)}
diff --git a/geocoder/hierarchy.hpp b/geocoder/hierarchy.hpp
index a9534e3445..02cfcdb487 100644
--- a/geocoder/hierarchy.hpp
+++ b/geocoder/hierarchy.hpp
@@ -72,6 +72,8 @@ public:
                                        MultipleNames & multipleNames,
                                        NameDictionaryBuilder & normalizedNameDictionaryBuilder,
                                        ParsingStats & stats);
+    // See generator::regions::LevelRegion::GetRank().
+    static Type RankToType(uint8_t rank);
 
     MultipleNames const & GetNormalizedMultipleNames(
         Type type, NameDictionary const & normalizedNameDictionary) const;