From 1a33c9b777e35bf11b9a590f8e189edca3135fe1 Mon Sep 17 00:00:00 2001
From: Anatoly Serdtcev <serdtcev@maps.me>
Date: Wed, 17 Apr 2019 16:40:17 +0300
Subject: [PATCH] [generator:geo_objects] Fix region find: border check

---
 3party/jansson/myjansson.hpp                 | 22 ++++++++
 generator/geo_objects/region_info_getter.cpp | 59 ++++++++++++--------
 generator/geo_objects/region_info_getter.hpp |  6 +-
 3 files changed, 64 insertions(+), 23 deletions(-)

diff --git a/3party/jansson/myjansson.hpp b/3party/jansson/myjansson.hpp
index 1963f4bb48..6f580671b0 100644
--- a/3party/jansson/myjansson.hpp
+++ b/3party/jansson/myjansson.hpp
@@ -12,6 +12,8 @@
 #include <string>
 #include <vector>
 
+#include <boost/optional.hpp>
+
 #include "3party/jansson/src/jansson.h"
 
 namespace base
@@ -68,6 +70,14 @@ json_t * GetJSONOptionalField(json_t * root, char const * field);
 bool JSONIsNull(json_t * root);
 }  // namespace base
 
+template <typename T>
+T FromJSON(json_t * root)
+{
+  T result{};
+  FromJSON(root, result);
+  return result;
+}
+
 inline void FromJSON(json_t * root, json_t *& value) { value = root; }
 
 void FromJSON(json_t * root, double & result);
@@ -98,6 +108,18 @@ void FromJSONObject(json_t * root, std::string const & field, T & result)
   }
 }
 
+template <typename T>
+boost::optional<T> FromJSONObjectOptional(json_t * root, char const * field)
+{
+  auto * json = base::GetJSONOptionalField(root, field);
+  if (!json)
+    return {};
+
+  boost::optional<T> result{T{}};
+  FromJSON(json, *result);
+  return result;
+}
+
 template <typename T>
 void FromJSONObjectOptionalField(json_t * root, std::string const & field, T & result)
 {
diff --git a/generator/geo_objects/region_info_getter.cpp b/generator/geo_objects/region_info_getter.cpp
index 39650551d8..9ebf4dcef5 100644
--- a/generator/geo_objects/region_info_getter.cpp
+++ b/generator/geo_objects/region_info_getter.cpp
@@ -11,7 +11,9 @@ namespace geo_objects
 RegionInfoGetter::RegionInfoGetter(std::string const & indexPath, std::string const & kvPath)
     : m_index{indexer::ReadIndex<indexer::RegionsIndexBox<IndexReader>, MmapReader>(indexPath)}
     , m_storage(kvPath)
-{ }
+{
+  m_borders.Deserialize(indexPath);
+}
 
 boost::optional<KeyValue> RegionInfoGetter::FindDeepest(m2::PointD const & point) const
 {
@@ -22,7 +24,7 @@ boost::optional<KeyValue> RegionInfoGetter::FindDeepest(
     m2::PointD const & point, Selector const & selector) const
 {
   auto const ids = SearchObjectsInIndex(point);
-  return GetDeepest(ids, selector);
+  return GetDeepest(point, ids, selector);
 }
 
 std::vector<base::GeoObjectId> RegionInfoGetter::SearchObjectsInIndex(m2::PointD const & point) const
@@ -33,37 +35,41 @@ std::vector<base::GeoObjectId> RegionInfoGetter::SearchObjectsInIndex(m2::PointD
   return ids;
 }
 
-boost::optional<KeyValue> RegionInfoGetter::GetDeepest(
+boost::optional<KeyValue> RegionInfoGetter::GetDeepest(m2::PointD const & point,
     std::vector<base::GeoObjectId> const & ids, Selector const & selector) const
 {
-  boost::optional<KeyValue> deepest;
-  int deepestRank = 0;
+  // Minimize CPU consumption by minimizing the number of calls to heavy m_borders.IsPointInside().
+  std::multimap<int, KeyValue> regionsByRank;
   for (auto const & id : ids)
   {
-    base::Json temp;
-    auto const res = m_storage.Find(id.GetEncodedId());
-    if (!res)
+    auto const region = m_storage.Find(id.GetEncodedId());
+    if (!region)
     {
       LOG(LWARNING, ("Id not found in region key-value storage:", id));
       continue;
     }
 
-    temp = *res;
-    if (!json_is_object(temp.get()))
-    {
-      LOG(LWARNING, ("Value is not a json object in region key-value storage:", id));
-      continue;
-    }
-
-    int tempRank = GetRank(temp);
-    if ((!deepest || deepestRank < tempRank) && selector(temp))
-    {
-      deepestRank = tempRank;
-      deepest = KeyValue(static_cast<int64_t>(id.GetEncodedId()), temp);
-    }
+    auto rank = GetRank(*region);
+    regionsByRank.emplace(rank, KeyValue{id.GetEncodedId(), std::move(*region)});
   }
 
-  return deepest;
+  boost::optional<uint64_t> borderCheckSkipRegionId;
+  for (auto i = regionsByRank.rbegin(); i != regionsByRank.rend(); ++i)
+  {
+    auto & kv = i->second;
+    auto regionId = kv.first;
+    if (regionId != borderCheckSkipRegionId && !m_borders.IsPointInside(regionId, point))
+      continue;
+
+    if (selector(kv.second))
+      return std::move(kv);
+
+    // Skip border check for parent region.
+    if (auto pid = GetPid(kv.second))
+      borderCheckSkipRegionId = pid;
+  }
+
+  return {};
 }
 
 int RegionInfoGetter::GetRank(base::Json const & json) const
@@ -75,6 +81,15 @@ int RegionInfoGetter::GetRank(base::Json const & json) const
   return rank;
 }
 
+boost::optional<uint64_t> RegionInfoGetter::GetPid(base::Json const & json) const
+{
+  auto && properties = base::GetJSONObligatoryField(json.get(), "properties");
+  auto && pid = base::GetJSONOptionalField(json.get(), "pid");
+  if (!pid || base::JSONIsNull(pid))
+    return {};
+  return static_cast<uint64_t>(FromJSON<int64_t>(pid));
+}
+
 KeyValueStorage const & RegionInfoGetter::GetStorage() const noexcept
 {
   return m_storage;
diff --git a/generator/geo_objects/region_info_getter.hpp b/generator/geo_objects/region_info_getter.hpp
index 3c0d4d9a56..3c62304294 100644
--- a/generator/geo_objects/region_info_getter.hpp
+++ b/generator/geo_objects/region_info_getter.hpp
@@ -2,6 +2,7 @@
 
 #include "generator/geo_objects/key_value_storage.hpp"
 
+#include "indexer/borders.hpp"
 #include "indexer/locality_index.hpp"
 
 #include "coding/reader.hpp"
@@ -36,11 +37,14 @@ private:
   using IndexReader = ReaderPtr<Reader>;
 
   std::vector<base::GeoObjectId> SearchObjectsInIndex(m2::PointD const & point) const;
-  boost::optional<KeyValue> GetDeepest(std::vector<base::GeoObjectId> const & ids,
+  boost::optional<KeyValue> GetDeepest(m2::PointD const & point, std::vector<base::GeoObjectId> const & ids,
                                        Selector const & selector) const;
   int GetRank(base::Json const & json) const;
+  // Get parent id of object: optional field `properties.pid` in JSON.
+  boost::optional<uint64_t> GetPid(base::Json const & json) const;
 
   indexer::RegionsIndex<IndexReader> m_index;
+  indexer::Borders m_borders;
   KeyValueStorage m_storage;
 };
 }  // namespace geo_objects