v0.2.47..v0.2.48 changeset PoiPolygonMerger.cpp

diff --git a/hoot-core/src/main/cpp/hoot/core/conflate/poi-polygon/PoiPolygonMerger.cpp b/hoot-core/src/main/cpp/hoot/core/conflate/poi-polygon/PoiPolygonMerger.cpp
index 7098c3d..c7c97b2 100644
--- a/hoot-core/src/main/cpp/hoot/core/conflate/poi-polygon/PoiPolygonMerger.cpp
+++ b/hoot-core/src/main/cpp/hoot/core/conflate/poi-polygon/PoiPolygonMerger.cpp
@@ -52,17 +52,54 @@ HOOT_FACTORY_REGISTER(Merger, PoiPolygonMerger)
 int PoiPolygonMerger::logWarnCount = 0;
 
 PoiPolygonMerger::PoiPolygonMerger() :
-MergerBase()
+MergerBase(),
+_autoMergeManyPoiToOnePolyMatches(ConfigOptions().getPoiPolygonAutoMergeManyPoiToOnePolyMatches()),
+_tagMergerClass("")
 {
 }
 
 PoiPolygonMerger::PoiPolygonMerger(const set<pair<ElementId, ElementId>>& pairs) :
 _pairs(pairs),
-_autoMergeManyPoiToOnePolyMatches(ConfigOptions().getPoiPolygonAutoMergeManyPoiToOnePolyMatches())
+_autoMergeManyPoiToOnePolyMatches(ConfigOptions().getPoiPolygonAutoMergeManyPoiToOnePolyMatches()),
+_tagMergerClass("")
 {
   assert(_pairs.size() >= 1);
 }
 
+std::shared_ptr<const TagMerger> PoiPolygonMerger::_getTagMerger()
+{
+  std::shared_ptr<const TagMerger> tagMerger;
+  std::string tagMergerClass;
+  // Always preserve types when merging many POIs in. This can go away if we stay with defaulting
+  // poi/poly merging to use PreserveTypesTagMerger.
+  if (_autoMergeManyPoiToOnePolyMatches)
+  {
+    tagMergerClass = PreserveTypesTagMerger::className();
+  }
+  // Otherwise, allow for calling class to specify the tag merger outside of Configurable.
+  else if (!_tagMergerClass.trimmed().isEmpty())
+  {
+    tagMergerClass = _tagMergerClass.toStdString();
+  }
+  // Otherwise, let's see if the tag merger was set specifically for poi/poly.
+  else if (!ConfigOptions().getPoiPolygonTagMerger().trimmed().isEmpty())
+  {
+    tagMergerClass = ConfigOptions().getPoiPolygonTagMerger().trimmed().toStdString();
+  }
+  // Otherwise, let's try the global tag merger.
+  else if (!ConfigOptions().getTagMergerDefault().trimmed().isEmpty())
+  {
+    tagMergerClass = ConfigOptions().getTagMergerDefault().trimmed().toStdString();
+  }
+  else
+  {
+    throw IllegalArgumentException("No tag merger specified for POI/Polygon conflation.");
+  }
+  LOG_VART(tagMergerClass);
+  tagMerger.reset(Factory::getInstance().constructObject<TagMerger>(tagMergerClass));
+  return tagMerger;
+}
+
 void PoiPolygonMerger::apply(const OsmMapPtr& map, vector<pair<ElementId, ElementId>>& replaced)
 {
   /// See
@@ -105,16 +142,11 @@ void PoiPolygonMerger::apply(const OsmMapPtr& map, vector<pair<ElementId, Elemen
   }
   assert(finalBuilding.get());
 
+  // select a tag merging strategy
+  std::shared_ptr<const TagMerger> tagMerger = _getTagMerger();
+
+  // merge the tags
   Tags finalBuildingTags = finalBuilding->getTags();
-  std::shared_ptr<const TagMerger> tagMerger;
-  if (_autoMergeManyPoiToOnePolyMatches)
-  {
-    tagMerger.reset(new PreserveTypesTagMerger());
-  }
-  else
-  {
-    tagMerger = TagMergerFactory::getInstance().getDefaultPtr();
-  }
   if (poiTags1.size())
   {
     LOG_TRACE("Merging POI tags with building tags for POI status Unknown1...");
@@ -162,6 +194,13 @@ void PoiPolygonMerger::apply(const OsmMapPtr& map, vector<pair<ElementId, Elemen
 
   if (poisMerged > 0)
   {
+    // If the poly merge target was previously conflated, let's add to the total number of POIs that
+    // have been merged.
+    if (finalBuildingTags.contains(MetadataTags::HootPoiPolygonPoisMerged()))
+    {
+      poisMerged += finalBuildingTags[MetadataTags::HootPoiPolygonPoisMerged()].toLong();
+    }
+    LOG_VART(poisMerged);
     finalBuildingTags.set(MetadataTags::HootPoiPolygonPoisMerged(), QString::number(poisMerged));
     finalBuilding->setStatus(Status::Conflated);
   }
@@ -289,54 +328,87 @@ ElementId PoiPolygonMerger::_mergeBuildings(const OsmMapPtr& map,
   return *newElement.begin();
 }
 
-ElementId PoiPolygonMerger::mergePoiAndPolygon(OsmMapPtr map)
+ElementId PoiPolygonMerger::_getElementIdByType(OsmMapPtr map, const ElementCriterion& typeCrit)
 {
-  //Trying to merge more than one POI into the polygon has proven problematic due to the building
-  //merging logic.  Merging more than one POI isn't a requirement, so only supporting 1:1 merging
-  //at this time.
-
-  LOG_INFO("Merging one POI and one polygon...");
-
-  StatusUpdateVisitor statusVisitor(Status::Unknown1, true);
-  PoiPolygonPoiCriterion poiCrit;
-  PoiPolygonPolyCriterion polyCrit;
-
-  //If the features involved in the merging don't have statuses, let's arbitrarily set them to
-  //Unknown1, since poi/poly requires it.  Setting them artificially *shouldn't* have a negative
-  //effect on the poi/poly merging, though.
+  UniqueElementIdVisitor idSetVis;
+  FilteredVisitor filteredVis(typeCrit, idSetVis);
+  map->visitRo(filteredVis);
+  const std::set<ElementId>& ids = idSetVis.getElementSet();
+  if (ids.size() != 1)
+  {
+    throw IllegalArgumentException(
+      "Exactly one POI and one polygon should be passed to POI/Polygon merging.");
+  }
+  return *ids.begin();
+}
 
-  FilteredVisitor filteredVis1(poiCrit, statusVisitor);
+void PoiPolygonMerger::_fixStatuses(OsmMapPtr map, const ElementId& poiId, const ElementId& polyId)
+{
+  // If the features involved in the merging don't have statuses, let's arbitrarily set them to
+  // Unknown1, since the building merging code used by poi/poly requires it.  Setting them
+  // artificially *shouldn't* have a negative effect on the poi/poly merging, though. Below we are
+  // failing when seeing a conflated status...we could rework to update that status as well.
+
+  // If the poly has an invalid status, we'll give it an Unknown1 status instead.
+  StatusUpdateVisitor status1Vis(Status::Unknown1, true);
+  FilteredVisitor filteredVis1(PoiPolygonPoiCriterion(), status1Vis);
   map->visitRw(filteredVis1);
 
-  FilteredVisitor filteredVis2(polyCrit, statusVisitor);
+  // If the POI has an invalid status, we'll give it an Unknown2 status instead.
+  StatusUpdateVisitor status2Vis(Status::Unknown2, true);
+  FilteredVisitor filteredVis2(PoiPolygonPolyCriterion(), status2Vis);
   map->visitRw(filteredVis2);
 
-  //get our poi id
-  UniqueElementIdVisitor idSetVis1;
-  FilteredVisitor filteredVis3(poiCrit, idSetVis1);
-  map->visitRo(filteredVis3);
-  const std::set<ElementId>& poiIds = idSetVis1.getElementSet();
-  if (poiIds.size() != 1)
+  // The BuildingMerger (used by both poi/poly and building merging) assumes that all input elements
+  // will either have an Unknown1 or Unknown2 status. If we are merging an element that was
+  // previously merged and given a conflated status, we need to replace that status with the
+  // appropriate unconflated status, if we can determine it. This logic could possibly be combined
+  // with the invalid status related logic above.
+  ElementPtr poi = map->getElement(poiId);
+  LOG_VART(poi);
+  ElementPtr poly = map->getElement(polyId);
+  LOG_VART(poly);
+  // pois always get merged into polys
+  LOG_VART(poi->getStatus());
+  LOG_VART(poly->getStatus());
+  if (poi->getStatus() == Status::Conflated)
   {
+    // don't think this should ever occur
     throw IllegalArgumentException(
-      "Exactly one POI should be passed to PoiPolygonMerger::mergePoiAndPolygon.");
+      "POI being merged with polygon cannot must have an Unknown1 or Unknown2 status.");
   }
-  const ElementId poiId = *poiIds.begin();
-  LOG_VART(poiId);
-
-  //get our poly id
-  UniqueElementIdVisitor idSetVis2;
-  FilteredVisitor filteredVis4(polyCrit, idSetVis2);
-  map->visitRo(filteredVis4);
-  const std::set<ElementId>& polyIds = idSetVis2.getElementSet();
-  if (polyIds.size() != 1)
+  else if (poly->getStatus() == Status::Conflated)
   {
-    throw IllegalArgumentException(
-      "Exactly one polygon should be passed to PoiPolygonMerger::mergePoiAndPolygon.");
+    // we've already handled invalid statuses above
+    if (poi->getStatus() == Status::Unknown1)
+    {
+      poly->setStatus(Status::Unknown2);
+    }
+    else if (poi->getStatus() == Status::Unknown2)
+    {
+      poly->setStatus(Status::Unknown1);
+    }
   }
-  const ElementId polyId = *polyIds.begin();
+}
+
+ElementId PoiPolygonMerger::mergeOnePoiAndOnePolygon(OsmMapPtr map)
+{
+  // Trying to merge more than one POI into the polygon has proven problematic due to the building
+  // merging logic. Merging more than one POI isn't a requirement, so only supporting 1:1 merging
+  // at this time.
+
+  LOG_INFO("Merging one POI and one polygon...");
+
+  // determine which element is which
+  const ElementId poiId = _getElementIdByType(map, PoiPolygonPoiCriterion());
+  LOG_VART(poiId);
+  const ElementId polyId = _getElementIdByType(map, PoiPolygonPolyCriterion());
   LOG_VART(polyId);
 
+  // handle invalid and conflated statuses
+  _fixStatuses(map, poiId, polyId);
+
+  // do the merging
   std::set<std::pair<ElementId, ElementId>> pairs;
   //Ordering doesn't matter here, since the poi is always merged into the poly.
   pairs.insert(std::pair<ElementId, ElementId>(polyId, poiId));
@@ -344,7 +416,8 @@ ElementId PoiPolygonMerger::mergePoiAndPolygon(OsmMapPtr map)
   std::vector<std::pair<ElementId, ElementId>> replacedElements;
   merger.apply(map, replacedElements);
 
-  LOG_INFO("Merged the POI into the polygon.");
+  LOG_INFO("Merged POI: " << poiId << " into polygon: " << polyId);
+  LOG_DEBUG("Merged feature: " << map->getElement(polyId));
 
   return polyId;
 }