From 8fd73bd6a6ab334e10feb47057aa94ad10328e2b Mon Sep 17 00:00:00 2001
From: Mikhail Gorbushin <m.gorbushin@corp.mail.ru>
Date: Fri, 22 Nov 2019 19:21:59 +0300
Subject: [PATCH] [generator] Add filtering of osmelements for
 RoutingCityBoundariesCollector

---
 .../collector_routing_city_boundaries.cpp     | 45 +++++++++++++++++--
 .../collector_routing_city_boundaries.hpp     |  1 +
 ...ollector_routing_city_boundaries_tests.cpp |  4 +-
 3 files changed, 45 insertions(+), 5 deletions(-)

diff --git a/generator/collector_routing_city_boundaries.cpp b/generator/collector_routing_city_boundaries.cpp
index d1b90bbadd..c9a247fca8 100644
--- a/generator/collector_routing_city_boundaries.cpp
+++ b/generator/collector_routing_city_boundaries.cpp
@@ -75,6 +75,39 @@ namespace generator
 {
 // RoutingCityBoundariesCollector::LocalityData ----------------------------------------------------
 
+// static
+bool RoutingCityBoundariesCollector::FilterOsmElement(OsmElement const & osmElement)
+{
+  static std::set<std::string> const kSuitablePlaceValues = {"city", "town", "village", "hamlet"};
+  if (osmElement.IsNode())
+  {
+    if (!osmElement.HasTag("population"))
+      return false;
+
+    auto const place = osmElement.GetTag("place");
+    if (place.empty())
+      return false;
+
+    return kSuitablePlaceValues.count(place) != 0;
+  }
+
+  auto const place = osmElement.GetTag("place");
+  if (!place.empty() && kSuitablePlaceValues.count(place) != 0)
+    return true;
+
+  if (osmElement.IsWay())
+    return false;
+
+  // We don't check here "type=boundary" and "boundary=administrative" because some OSM'ers prefer
+  // map boundaries without such tags in some countries (for example in Russia).
+  for (auto const & member : osmElement.m_members)
+  {
+    if (member.m_role == "admin_centre" || member.m_role == "label")
+      return true;
+  }
+  return false;
+}
+
 void RoutingCityBoundariesCollector::LocalityData::Serialize(FileWriter & writer,
                                                              LocalityData const & localityData)
 {
@@ -128,6 +161,9 @@ std::shared_ptr<CollectorInterface> RoutingCityBoundariesCollector::Clone(
 
 void RoutingCityBoundariesCollector::Collect(OsmElement const & osmElement)
 {
+  if (!FilterOsmElement(osmElement))
+    return;
+
   auto osmElementCopy = osmElement;
   feature::FeatureBuilder feature;
   m_featureMakerSimple.Add(osmElementCopy);
@@ -142,6 +178,8 @@ void RoutingCityBoundariesCollector::Collect(OsmElement const & osmElement)
 void RoutingCityBoundariesCollector::Process(feature::FeatureBuilder & feature,
                                              OsmElement const & osmElement)
 {
+  ASSERT(FilterOsmElement(osmElement), ());
+
   if (feature.IsArea() && IsSuitablePlaceType(GetPlaceType(feature)))
   {
     if (feature.PreSerialize())
@@ -152,8 +190,7 @@ void RoutingCityBoundariesCollector::Process(feature::FeatureBuilder & feature,
   if (feature.IsArea())
   {
     auto const placeOsmIdOp = GetPlaceNodeFromMembers(osmElement);
-    if (!placeOsmIdOp)
-      return;
+    ASSERT(placeOsmIdOp, ("FilterOsmElement() should filtered such elements:", osmElement));
 
     auto const placeOsmId = *placeOsmIdOp;
 
@@ -164,8 +201,8 @@ void RoutingCityBoundariesCollector::Process(feature::FeatureBuilder & feature,
   else if (feature.IsPoint())
   {
     auto const placeType = GetPlaceType(feature);
-    if (!IsSuitablePlaceType(placeType))
-      return;
+    ASSERT(IsSuitablePlaceType(placeType),
+           ("FilterOsmElement() should filtered such elements:", osmElement));
 
     uint64_t const population = osm_element::GetPopulation(osmElement);
     if (population == 0)
diff --git a/generator/collector_routing_city_boundaries.hpp b/generator/collector_routing_city_boundaries.hpp
index f791b3c63b..14ee32f6b9 100644
--- a/generator/collector_routing_city_boundaries.hpp
+++ b/generator/collector_routing_city_boundaries.hpp
@@ -55,6 +55,7 @@ public:
   void Merge(generator::CollectorInterface const & collector) override;
   void MergeInto(RoutingCityBoundariesCollector & collector) const override;
 
+  static bool FilterOsmElement(OsmElement const & osmElement);
   void Process(feature::FeatureBuilder & feature, OsmElement const & osmElement);
 
 private:
diff --git a/generator/generator_tests/collector_routing_city_boundaries_tests.cpp b/generator/generator_tests/collector_routing_city_boundaries_tests.cpp
index 8e274682fd..f9d2a6f169 100644
--- a/generator/generator_tests/collector_routing_city_boundaries_tests.cpp
+++ b/generator/generator_tests/collector_routing_city_boundaries_tests.cpp
@@ -91,7 +91,9 @@ void Collect(BoundariesCollector & collector, std::vector<OsmElement> const & el
     auto const & element = elements[i];
     auto featureBuilder = element.IsNode() ? MakeNodeFeatureBuilder(element)
                                            : MakeAreaFeatureBuilder(element, geometries[i]);
-    collector.Process(featureBuilder, element);
+
+    if (BoundariesCollector::FilterOsmElement(element))
+      collector.Process(featureBuilder, element);
   }
 }