v0.2.55..v0.2.56 changeset ApiDbReader.cpp

diff --git a/hoot-core/src/main/cpp/hoot/core/io/ApiDbReader.cpp b/hoot-core/src/main/cpp/hoot/core/io/ApiDbReader.cpp
index c806d50..f2e75d0 100644
--- a/hoot-core/src/main/cpp/hoot/core/io/ApiDbReader.cpp
+++ b/hoot-core/src/main/cpp/hoot/core/io/ApiDbReader.cpp
@@ -22,7 +22,7 @@
  * This will properly maintain the copyright information. DigitalGlobe
  * copyrights will be updated automatically.
  *
- * @copyright Copyright (C) 2016, 2017, 2018, 2019 DigitalGlobe (http://www.digitalglobe.com/)
+ * @copyright Copyright (C) 2016, 2017, 2018, 2019, 2020 DigitalGlobe (http://www.digitalglobe.com/)
  */
 #include "ApiDbReader.h"
 
@@ -56,6 +56,7 @@ _open(false),
 _defaultCircularError(ConfigOptions().getCircularErrorDefaultValue()),
 _returnNodesOnly(false),
 _keepStatusTag(ConfigOptions().getReaderKeepStatusTag()),
+_circularErrorTagKeys(ConfigOptions().getCircularErrorTagKeys()),
 _statusUpdateInterval(ConfigOptions().getTaskStatusUpdateInterval()),
 _totalNumMapNodes(0),
 _totalNumMapWays(0),
@@ -193,8 +194,8 @@ void ApiDbReader::_updateMetadataOnElement(ElementPtr element)
         logWarnCount++;
       }
     }
-    //We don't need to carry this tag around once the value is set on the element...it will
-    //be reinstated by some writers, though.
+    // We don't need to carry this tag around once the value is set on the element...it will
+    // be reinstated by some writers, though.
     if (!_keepStatusTag) { tags.remove(MetadataTags::HootStatus()); }
   }
 
@@ -208,14 +209,17 @@ void ApiDbReader::_updateMetadataOnElement(ElementPtr element)
     }
   }
 
-  if (tags.contains(MetadataTags::ErrorCircular()))
+  // Arbitrarily pick the first error tag found. If the element has both, the last one parsed will
+  // be used. We're not expecting elements to have more than one CE tag.
+  const QString ceKey = tags.getFirstKey(_circularErrorTagKeys);
+  if (!ceKey.isEmpty())
   {
-    element->setCircularError(tags.get(MetadataTags::ErrorCircular()).toDouble(&ok));
+    element->setCircularError(tags.get(ceKey).toDouble(&ok));
     if (!ok)
     {
       try
       {
-        double tv = tags.getLength(MetadataTags::ErrorCircular()).value();
+        double tv = tags.getLength(ceKey).value();
         element->setCircularError(tv);
         ok = true;
       }
@@ -228,7 +232,7 @@ void ApiDbReader::_updateMetadataOnElement(ElementPtr element)
       {
         if (logWarnCount < Log::getWarnMessageLimit())
         {
-          LOG_WARN("Error parsing " + MetadataTags::ErrorCircular() + ".");
+          LOG_WARN("Error parsing " + ceKey + ".");
         }
         else if (logWarnCount == Log::getWarnMessageLimit())
         {
@@ -237,39 +241,14 @@ void ApiDbReader::_updateMetadataOnElement(ElementPtr element)
         logWarnCount++;
       }
     }
-    //We don't need to carry this tag around once the value is set on the element...it will
-    //be reinstated by some writers, though.
-    tags.remove(MetadataTags::ErrorCircular());
-  }
-  else if (tags.contains(MetadataTags::Accuracy()))
-  {
-    element->setCircularError(tags.get(MetadataTags::Accuracy()).toDouble(&ok));
-
-    if (!ok)
+    // Preserving original behavior of the reader here. Not all readers remove this key, so we may
+    // want to unify their behavior. It may be ok, since its a hoot specific key. We wouldn't,
+    // however, want to remove other error tags.
+    if (ceKey == MetadataTags::ErrorCircular())
     {
-      try
-      {
-        double tv = tags.getLength(MetadataTags::Accuracy()).value();
-        element->setCircularError(tv);
-        ok = true;
-      }
-      catch (const HootException&)
-      {
-        ok = false;
-      }
-
-      if (!ok)
-      {
-        if (logWarnCount < Log::getWarnMessageLimit())
-        {
-          LOG_WARN("Error parsing " + MetadataTags::Accuracy() + ".");
-        }
-        else if (logWarnCount == Log::getWarnMessageLimit())
-        {
-          LOG_WARN(className() << ": " << Log::LOG_WARN_LIMIT_REACHED_MESSAGE);
-        }
-        logWarnCount++;
-      }
+      // We don't need to carry this tag around once the value is set on the element...it will
+      // be reinstated by some writers, though.
+      tags.remove(ceKey);
     }
   }
 }
@@ -290,6 +269,7 @@ void ApiDbReader::_readWaysByNodeIds(OsmMapPtr map, const QSet<QString>& nodeIds
                                      long& nodeCount, long& wayCount)
 {
   LOG_DEBUG("Retrieving way IDs referenced by the selected nodes...");
+  // TODO: This is extremely slow for large numbers of node IDs.
   std::shared_ptr<QSqlQuery> wayIdItr = _getDatabase()->selectWayIdsByWayNodeIds(nodeIds);
   while (wayIdItr->next())
   {
@@ -372,6 +352,8 @@ void ApiDbReader::_readWaysByNodeIds(OsmMapPtr map, const QSet<QString>& nodeIds
 
 void ApiDbReader::_readByBounds(OsmMapPtr map, const Envelope& bounds)
 {
+  // TODO: This proves very slow for AOI's with a lot of data. Need something better.
+
   long boundedNodeCount = 0;
   long boundedWayCount = 0;
   long boundedRelationCount = 0;
@@ -406,7 +388,6 @@ void ApiDbReader::_readByBounds(OsmMapPtr map, const Envelope& bounds)
         map, nodeIds, wayIds, additionalWayNodeIds, boundedNodeCount, boundedWayCount);
       nodeIds.unite(additionalWayNodeIds);
       LOG_VARD(nodeIds.size());
-      LOG_VARD(wayIds);
       LOG_VARD(wayIds.size());
 
       LOG_DEBUG("Retrieving relation IDs referenced by the selected ways and nodes...");