5289 hootjs error messages (#5307)

* add some test scripts for nodejs calls to hoot merge * make executable * This commit will fix an issue that was occuring when the script scripts/core/MergeNodes.js is executed from within the translations directory. In particular, ConfPath::search(...) was looking in the local directory, before checking the specified searchDir. This was a problem for script execution in the translations dir, because translations contains a Poi.js file collides with the (proper) rules/Poi.js file * update tests so assertions actually invoked adds two tests that are currently failing will pass when #5289 is fixed * MergeElements Update * Fix HootExceptionJs to allow detailed error messages to bubble up to Node, instead of just always reporting "UnsupportedException" * Update copyright headers * handle some hoot settings bleeding over from other test runs * this seems like a typo * format the new HootJS exception better in the http response Co-authored-by: Brian Hatchl <[email protected]> Co-authored-by: Micah Schicker <[email protected]> Co-authored-by: Ben Marchant <[email protected]>
ngageoint · Apr 5, 2022 · 130fc8d · 130fc8d
1 parent 194e294
commit 130fc8d
Show file tree

Hide file tree

Showing 10 changed files with 319 additions and 242 deletions.
diff --git a/hoot-core/src/main/cpp/hoot/core/util/ConfPath.cpp b/hoot-core/src/main/cpp/hoot/core/util/ConfPath.cpp
@@ -22,7 +22,7 @@
  * This will properly maintain the copyright information. Maxar
  * copyrights will be updated automatically.
  *
- * @copyright Copyright (C) 2015, 2016, 2017, 2018, 2019, 2021 Maxar (http://www.maxar.com/)
+ * @copyright Copyright (C) 2015, 2016, 2017, 2018, 2019, 2021, 2022 Maxar (http://www.maxar.com/)
  */
 
 #include "ConfPath.h"
@@ -112,16 +112,13 @@ QString ConfPath::_subDirSearch(QString baseName, QString searchDir)
 
 QString ConfPath::search(QString baseName, QString searchDir)
 {
-  if (QFileInfo(baseName).isFile())
-  {
-    return QFileInfo(baseName).absoluteFilePath();
-  }
-
+  // Try searchDir first
   if (QFileInfo(searchDir + "/" + baseName).isFile())
   {
     return QFileInfo(searchDir + "/" + baseName).absoluteFilePath();
   }
 
+  // Now try HOOT_HOME + / + searchDir
   QString hootHome = getHootHome();
 
   if (hootHome.isEmpty() == false && QFileInfo(hootHome + "/" + searchDir + "/" + baseName).
@@ -130,6 +127,12 @@ QString ConfPath::search(QString baseName, QString searchDir)
     return QFileInfo(hootHome + "/" + searchDir + "/" + baseName).absoluteFilePath();
   }
 
+  // Now try local dir
+  if (QFileInfo(baseName).isFile())
+  {
+    return QFileInfo(baseName).absoluteFilePath();
+  }
+
   // If we still can't find it, try searching subdirectories.
   // Not sure if this should also have a go with "hootHome/searchDir" as well as "searchDir"
 

diff --git a/hoot-js/src/main/cpp/hoot/js/conflate/merging/ElementMergerJs.cpp b/hoot-js/src/main/cpp/hoot/js/conflate/merging/ElementMergerJs.cpp
@@ -77,9 +77,9 @@ void ElementMergerJs::Init(Local<Object> exports)
   Isolate* current = exports->GetIsolate();
   HandleScope scope(current);
   Local<Context> context = current->GetCurrentContext();
-  exports->Set(
-    context, toV8("merge"),
-    FunctionTemplate::New(current, merge)->GetFunction(context).ToLocalChecked());
+  Maybe<bool> success = exports->Set(context, toV8("merge"),
+                                 FunctionTemplate::New(current, merge)->GetFunction(context).ToLocalChecked());
+  (void) success; // unused variable
 }
 
 void ElementMergerJs::merge(const FunctionCallbackInfo<Value>& args)
@@ -110,9 +110,6 @@ void ElementMergerJs::merge(const FunctionCallbackInfo<Value>& args)
     Local<Object> returnMap = OsmMapJs::create(map);
     args.GetReturnValue().Set(returnMap);
   }
-  // This error handling has been proven to not work, as it never returns the error message to the
-  // nodejs calling service....makes debugging very difficult. Need to fix: #2231. As a workaround,
-  // use scripts/core/MergeElements.js to see log output during merging.
   catch (const HootException& e)
   {
     LOG_ERROR(e.getWhat());
@@ -125,7 +122,7 @@ QString ElementMergerJs::_mergeTypeToString(const MergeType& mergeType)
   switch (mergeType)
   {
   case MergeType::Building:
-    return "Buildingg";
+    return "Building";
   case MergeType::PoiToPolygon:
     return "PoiToPolygon";
   case MergeType::Poi:
@@ -141,7 +138,7 @@ QString ElementMergerJs::_mergeTypeToString(const MergeType& mergeType)
 }
 
 void ElementMergerJs::_merge(OsmMapPtr map, Isolate* current)
-{  
+{
   const MergeType mergeType = _determineMergeType(map);
   LOG_VART(_mergeTypeToString(mergeType));
 

diff --git a/hoot-js/src/main/cpp/hoot/js/conflate/merging/LinearMergerJs.cpp b/hoot-js/src/main/cpp/hoot/js/conflate/merging/LinearMergerJs.cpp
@@ -22,7 +22,7 @@
  * This will properly maintain the copyright information. Maxar
  * copyrights will be updated automatically.
  *
- * @copyright Copyright (C) 2015, 2016, 2017, 2018, 2019, 2020, 2021 Maxar (http://www.maxar.com/)
+ * @copyright Copyright (C) 2015, 2016, 2017, 2018, 2019, 2020, 2021, 2022 Maxar (http://www.maxar.com/)
  */
 #include "LinearMergerJs.h"
 
@@ -66,7 +66,8 @@ void LinearMergerJs::Init(Local<Object> target)
   tpl->PrototypeTemplate()->Set(current, "apply", FunctionTemplate::New(current, apply));
 
   _constructor.Reset(current, tpl->GetFunction(context).ToLocalChecked());
-  target->Set(context, toV8(name), ToLocal(&_constructor));
+  Maybe<bool> success = target->Set(context, toV8(name), ToLocal(&_constructor));
+  (void) success; // unused variable
 }
 
 void LinearMergerJs::New(const FunctionCallbackInfo<Value>& args)
@@ -117,10 +118,10 @@ void LinearMergerJs::apply(const FunctionCallbackInfo<Value>& args)
   // Modify the parameter that was passed in.
   Local<Array> newArr = Local<Array>::Cast(toV8(replaced));
   Local<Array> arr = Local<Array>::Cast(args[3]);
-  arr->Set(context, toV8("length"), Integer::New(current, newArr->Length()));
+  Maybe<bool> success = arr->Set(context, toV8("length"), Integer::New(current, newArr->Length()));
   for (uint32_t i = 0; i < newArr->Length(); i++)
   {
-    arr->Set(context, i, newArr->Get(context, i).ToLocalChecked());
+    success = arr->Set(context, i, newArr->Get(context, i).ToLocalChecked());
   }
 
   args.GetReturnValue().SetUndefined();

diff --git a/hoot-js/src/main/cpp/hoot/js/util/HootExceptionJs.cpp b/hoot-js/src/main/cpp/hoot/js/util/HootExceptionJs.cpp
@@ -22,7 +22,7 @@
  * This will properly maintain the copyright information. Maxar
  * copyrights will be updated automatically.
  *
- * @copyright Copyright (C) 2015, 2016, 2017, 2018, 2019, 2020, 2021 Maxar (http://www.maxar.com/)
+ * @copyright Copyright (C) 2015, 2016, 2017, 2018, 2019, 2020, 2021, 2022 Maxar (http://www.maxar.com/)
  */
 #include "HootExceptionJs.h"
 
@@ -48,7 +48,14 @@ Local<Object> HootExceptionJs::create(const std::shared_ptr<HootException>& e)
   EscapableHandleScope scope(current);
   Local<Context> context = current->GetCurrentContext();
 
+  // Construct our exception object
   Local<Object> result = ToLocal(&_constructor)->NewInstance(context).ToLocalChecked();
+
+  // Set the values that will be reported by log(...) functions
+  Maybe<bool> success = result->Set(context, Local<String>::Cast(toV8("Exception")), Local<String>::Cast(toV8(e->getName())));
+  success = result->Set(context, Local<String>::Cast(toV8("What")), Local<String>::Cast(toV8(e->what())));
+
+  // Unwrap & set our pointer to the underlying hoot exception
   HootExceptionJs* from = ObjectWrap::Unwrap<HootExceptionJs>(result);
   from->_e = e;
 
@@ -57,27 +64,25 @@ Local<Object> HootExceptionJs::create(const std::shared_ptr<HootException>& e)
 
 void HootExceptionJs::Init(Local<Object> target)
 {
+  QString className = "HootExceptionJS";
   Isolate* current = target->GetIsolate();
   HandleScope scope(current);
   Local<Context> context = current->GetCurrentContext();
-  vector<QString> opNames =
-    Factory::getInstance().getObjectNamesByBase(HootException::className());
 
-  for (size_t i = 0; i < opNames.size(); i++)
-  {
-    QString opName = opNames[i];
-
-    // Prepare constructor template
-    Local<FunctionTemplate> tpl = FunctionTemplate::New(current, New);
-    tpl->SetClassName(Local<String>::Cast(toV8(opNames[i])));
-    tpl->InstanceTemplate()->SetInternalFieldCount(2);
-    // Prototype
-    tpl->PrototypeTemplate()->Set(
-      PopulateConsumersJs::baseClass(), toV8(HootException::className()));
-
-    _constructor.Reset(current, tpl->GetFunction(context).ToLocalChecked());
-    target->Set(context, toV8(opName), ToLocal(&_constructor));
-  }
+  // Prepare constructor template
+  Local<FunctionTemplate> tpl = FunctionTemplate::New(current, New);
+  tpl->SetClassName(Local<String>::Cast(toV8(className.toStdString())));
+  tpl->InstanceTemplate()->SetInternalFieldCount(2);
+
+  // Wire up functions
+  tpl->PrototypeTemplate()->Set(PopulateConsumersJs::baseClass(), toV8(HootException::className()));
+  tpl->PrototypeTemplate()->Set(current, "toString", FunctionTemplate::New(current, toString));
+  tpl->PrototypeTemplate()->Set(current, "toJSON", FunctionTemplate::New(current, toString));
+  tpl->PrototypeTemplate()->Set(current, "stringify", FunctionTemplate::New(current, toString));
+
+  _constructor.Reset(current, tpl->GetFunction(context).ToLocalChecked());
+  Maybe<bool> success = target->Set(context, toV8(className.toStdString()), ToLocal(&_constructor));
+  (void) success; // unused var
 }
 
 bool HootExceptionJs::isHootException(Local<Value> v)
@@ -188,4 +193,14 @@ void HootExceptionJs::throwAsHootException(const TryCatch& tc)
   }
 }
 
+void HootExceptionJs::toString(const FunctionCallbackInfo<Value>& args)
+{
+  HandleScope scope(args.GetIsolate());
+
+  std::shared_ptr<HootException> e = ObjectWrap::Unwrap<HootExceptionJs>(args.This())->getException();
+  QString msg = QString("Exception %1 : %2").arg(e->getName(), e->what());
+
+  args.GetReturnValue().Set(toV8(msg.toStdString()));
+}
+
 }
diff --git a/hoot-js/src/main/cpp/hoot/js/util/HootExceptionJs.h b/hoot-js/src/main/cpp/hoot/js/util/HootExceptionJs.h
@@ -22,7 +22,7 @@
  * This will properly maintain the copyright information. Maxar
  * copyrights will be updated automatically.
  *
- * @copyright Copyright (C) 2015, 2017, 2018, 2019, 2020, 2021 Maxar (http://www.maxar.com/)
+ * @copyright Copyright (C) 2015, 2017, 2018, 2019, 2020, 2021, 2022 Maxar (http://www.maxar.com/)
  */
 #ifndef HOOTEXCEPTIONJS_H
 #define HOOTEXCEPTIONJS_H
@@ -74,6 +74,7 @@ class HootExceptionJs : public HootBaseJs
   static v8::Persistent<v8::Function> _constructor;
 
   static void New(const v8::FunctionCallbackInfo<v8::Value>& args);
+  static void toString(const v8::FunctionCallbackInfo<v8::Value>& args);
 };
 
 inline void toCpp(v8::Local<v8::Value> v, std::shared_ptr<HootException>& e)

diff --git a/scripts/core/MergeElements.js b/scripts/core/MergeElements.js
@@ -4,14 +4,18 @@
 
 var HOOT_HOME = process.env.HOOT_HOME
 var hoot = require(HOOT_HOME + '/lib/HootJs');
-hoot.Log.setLogLevel('trace');
+//hoot.Log.setLogLevel('trace');
 hoot.Settings.set({"log.class.include.filter":"PoiPolygonMerger;ElementMergerJs;PreserveTypesTagMerger"});
 
 // Don't forget to replace " with '
-var data = "<osm version='0.6' upload='true' generator='hootenanny'><way id='167' version='2'><nd ref='1524'/><nd ref='1525'/><nd ref='1526'/><nd ref='1527'/><nd ref='1528'/><nd ref='1529'/><nd ref='1530'/><nd ref='1531'/><nd ref='1532'/><nd ref='1533'/><nd ref='1534'/><nd ref='1535'/><nd ref='1536'/><nd ref='1524'/><tag k='addr:full' v='Al-Rabwah, Jeddah 23448, Saudi Arabia'/><tag k='amenity' v='place_of_worship'/><tag k='attribution' v='osm'/><tag k='building' v='yes'/><tag k='error:circular' v='15'/><tag k='hoot:poipolygon:poismerged' v='1'/><tag k='hoot:status' v='3'/><tag k='license' v='This data is made available under the Open Database License: http://opendatacommons.org/licenses/odbl/1.0/.'/><tag k='name' v='Sherbalty Mosque'/><tag k='religion' v='muslim'/><tag k='security:classification' v='UNCLASSIFIED'/><tag k='source' v='DigitalGlobe;osm'/><tag k='source:datetime' v='2017-04-15T22:17:51Z'/><tag k='source:imagery:datetime' v='2017-03-21 08:14:26'/><tag k='source:imagery:id' v='15b60aa1243d556e0232bc21a85aa3f9'/><tag k='source:imagery:sensor' v='WV03_VNIR'/><tag k='source:ingest:datetime' v='2019-04-15T18:06:46.626Z'/><tag k='uuid' v='{b0654ffa-b479-41bb-a3b9-5140230350c4}'/><tag k='hoot:merge:target' v='yes'/></way><node id='-103' lon='39.190346' lat='21.58882699999999' version='1'><tag k='addr:full' v='Al-Rabwah, Jeddah 23448, Saudi Arabia'/><tag k='amenity' v='place_of_worship'/><tag k='building' v='mosque'/><tag k='error:circular' v='15'/><tag k='hoot:status' v='Input2'/><tag k='name' v='Sherbatly mosque'/><tag k='name:ar' v='جامع الشربتلي'/><tag k='religion' v='muslim'/><tag k='source:ingest:datetime' v='2019-04-15T18:06:46.630Z'/></node></osm>";
+var data = "<osm version='0.6' upload='true' generator='hootenanny'><way id='167' version='2'><nd ref='1524'/><nd ref='1525'/><nd ref='1526'/><nd ref='1527'/><nd ref='1528'/><nd ref='1529'/><nd ref='1530'/><nd ref='1531'/><nd ref='1532'/><nd ref='1533'/><nd ref='1534'/><nd ref='1535'/><nd ref='1536'/><nd ref='1524'/><tag k='addr:full' v='Al-Rabwah, Jeddah 23448, Saudi Arabia'/><tag k='amenity' v='place_of_worship'/><tag k='attribution' v='osm'/><tag k='building' v='yes'/><tag k='error:circular' v='15'/><tag k='hoot:poipolygon:poismerged' v='1'/><tag k='hoot:status' v='3'/><tag k='license' v='This data is made available under the Open Database License: http://opendatacommons.org/licenses/odbl/1.0/.'/><tag k='name' v='Sherbalty Mosque'/><tag k='religion' v='muslim'/><tag k='security:classification' v='UNCLASSIFIED'/><tag k='source' v='DigitalGlobe;osm'/><tag k='source:datetime' v='2017-04-15T22:17:51Z'/><tag k='source:imagery:datetime' v='2017-03-21 08:14:26'/><tag k='source:imagery:id' v='15b60aa1243d556e0232bc21a85aa3f9'/><tag k='source:imagery:sensor' v='WV03_VNIR'/><tag k='source:ingest:datetime' v='2019-04-15T18:06:46.626Z'/><tag k='uuid' v='{b0654ffa-b479-41bb-a3b9-5140230350c4}'/><tag k='hoot:merge:target' v='yes'/></way></osm>";
 
-var map = new hoot.OsmMap();
-hoot.loadMapFromStringPreserveIdAndStatus(map, data);
-var mergedMap = hoot.mergeElements(map);
-var mergedMapStr = hoot.OsmWriter.toString(mergedMap);
-console.log("\n\nOutput:\n\n" + mergedMapStr);
+try {
+    var map = new hoot.OsmMap();
+    hoot.loadMapFromStringPreserveIdAndStatus(map, data);
+    var mergedMap = hoot.merge(map);
+    var mergedMapStr = hoot.OsmWriter.toString(mergedMap);
+    console.log("\n\nOutput:\n\n" + mergedMapStr);
+} catch (ex) {
+    console.log("\n\nTEST:\n\n" + ex.toString());
+}
diff --git a/scripts/core/MergeNodes.js b/scripts/core/MergeNodes.js
@@ -0,0 +1,31 @@
+#!/usr/bin/node
+
+var HOOT_HOME = process.env.HOOT_HOME
+var hoot = require(HOOT_HOME + '/lib/HootJs');
+hoot.Log.setLogLevel('debug');
+
+var data = '<?xml version="1.0" encoding="UTF-8"?>\
+<osm version="0.6" upload="true" generator="hootenanny">\
+<node id="-1559816" lon="-104.71755649299132" lat="38.888511927381124" version="1">\
+    <tag k="amenity" v="cafe" />\
+    <tag k="error:circular" v="1000" />\
+    <tag k="hoot:status" v="Input1" />\
+    <tag k="name" v="Starbucks" />\
+    <tag k="note" v="1-c" />\
+    <tag k="poi" v="yes" />\
+    <tag k="hoot:merge:target" v="yes" />\
+</node>\
+<node id="-1559910" lon="-104.71877032224545" lat="38.88870088854899" version="1">\
+    <tag k="amenity" v="cafe" />\
+    <tag k="error:circular" v="1000" />\
+    <tag k="hoot:status" v="Input2" />\
+    <tag k="name" v="Starbucks" />\
+    <tag k="note" v="2-c" />\
+    <tag k="poi" v="yes" />\
+</node>\
+</osm>';
+var map = new hoot.OsmMap();
+hoot.loadMapFromStringPreserveIdAndStatus(map, data);
+var mergedMap = hoot.merge(map);
+var mergedMapStr = hoot.OsmWriter.toString(mergedMap);
+console.log("\n\nOutput:\n\n" + mergedMapStr);
diff --git a/translations/ElementMergeServer.js b/translations/ElementMergeServer.js
@@ -1,12 +1,13 @@
 /************************************************************************
-This is a nodejs integration with Hootenanny element merging logic. See 
+This is a nodejs integration with Hootenanny element merging logic. See
 docs/developer/ElementMergeService.asciidoc
 ************************************************************************/
 var http = require('http');
 var url = require('url');
 var serverPort = 8096;
 var HOOT_HOME = process.env.HOOT_HOME;
 var hoot = require(HOOT_HOME + '/lib/HootJs');
+const util = require('util');
 
 if (require.main === module) {
     //I'm a running server
@@ -80,18 +81,17 @@ function ElementMergeserver(request, response) {
 
 
             request.on('end', function() {
-
-                var result;
                 try {
-                    result = postHandler(payload);
+                    // console.log(payload);
+                    var result = postHandler(payload);
+                    // console.log(result);
+                    response.writeHead(200, header);
+                    response.end(result);
                 } catch (err) {
-                    var status = 400;
-                    response.writeHead(status, header);
-                    response.end(JSON.stringify({error: err}));
+                    // console.error(err.What);
+                    response.writeHead(400, err.Exception, header);
+                    response.end(err.What);
                 }
-
-                response.writeHead(200, header);
-                response.end(result);
             });
 
         } else if (request.method === 'OPTIONS') {
@@ -119,14 +119,11 @@ function ElementMergeserver(request, response) {
 
 var postHandler = function(data)
 {
-  // can't seem to get this to work
-  //hoot.Log.setLogLevel('trace');
-
-  var map = new hoot.OsmMap();
-  hoot.loadMapFromStringPreserveIdAndStatus(map, data);
-  var mergedMap = hoot.merge(map);
-  var xml = hoot.OsmWriter.toString(mergedMap);
-  return xml;
+    var map = new hoot.OsmMap();
+    hoot.loadMapFromStringPreserveIdAndStatus(map, data);
+    var mergedMap = hoot.merge(map);
+    var xml = hoot.OsmWriter.toString(mergedMap);
+    return xml;
 }
 
 if (typeof exports !== 'undefined') {