added guard clauses for bbox and centroid

controller/placeholder.js

@@ -6,47 +6,35 @@ const logger = require('pelias-logger').get('api');
 const logging = require( '../helper/logging' );
 const Document = require('pelias-model').Document;
 
-function createDocWithoutLineage(result) {
-  const doc = new Document('whosonfirst', result.placetype, result.id.toString());
-  doc.setName('default', result.name);
-  doc.setCentroid( { lat: result.geom.lat, lon: result.geom.lon } );
-
-  const parsedBoundingBox = result.geom.bbox.split(',').map(parseFloat);
-  doc.setBoundingBox({
-    upperLeft: {
-      lat: parsedBoundingBox[3],
-      lon: parsedBoundingBox[0]
-    },
-    lowerRight: {
-      lat: parsedBoundingBox[1],
-      lon: parsedBoundingBox[2]
-    }
-  });
-
-  const esDoc = doc.toESDocument();
-  esDoc.data._id = esDoc._id;
-  esDoc.data._type = esDoc._type;
-
-  return esDoc.data;
+const boundingBoxRegex = /^-?\d+\.\d+,-?\d+\.\d+,-?\d+\.\d+,-?\d+\.\d+$/;
 
+function validBoundingBox(bbox) {
+  return boundingBoxRegex.test(bbox);
 }
 
 function synthesizeDocs(result) {
   const doc = new Document('whosonfirst', result.placetype, result.id.toString());
   doc.setName('default', result.name);
-  doc.setCentroid( { lat: result.geom.lat, lon: result.geom.lon } );
-
-  const parsedBoundingBox = result.geom.bbox.split(',').map(parseFloat);
-  doc.setBoundingBox({
-    upperLeft: {
-      lat: parsedBoundingBox[3],
-      lon: parsedBoundingBox[0]
-    },
-    lowerRight: {
-      lat: parsedBoundingBox[1],
-      lon: parsedBoundingBox[2]
-    }
-  });
+
+  // only assign centroid if both lat and lon are finite numbers
+  if (_.conformsTo(result.geom, { 'lat': _.isFinite, 'lon': _.isFinite } )) {
+    doc.setCentroid( { lat: result.geom.lat, lon: result.geom.lon } );
+  }
+
+  // lodash conformsTo verifies that an object has a property with a certain format
+  if (_.conformsTo(result.geom, { 'bbox': validBoundingBox } )) {
+    const parsedBoundingBox = result.geom.bbox.split(',').map(parseFloat);
+    doc.setBoundingBox({
+      upperLeft: {
+        lat: parsedBoundingBox[3],
+        lon: parsedBoundingBox[0]
+      },
+      lowerRight: {
+        lat: parsedBoundingBox[1],
+        lon: parsedBoundingBox[2]
+      }
+    });
+  }
 
   if (_.isEmpty(result.lineage)) {
     // there are no hierarchies so just return what's been assembled so far
@@ -106,7 +94,7 @@ function setup(placeholderService, should_execute) {
         res.meta = {};
         res.data = _.flatten(results.map((result) => {
           if (_.isEmpty(result.lineage)) {
-            return createDocWithoutLineage(result);
+            return synthesizeDocs(result);
           }
 
           return synthesizeDocs(result);

test/unit/controller/placeholder.js

@@ -437,6 +437,140 @@ module.exports.tests.success = function(test, common) {
 
   });
 
+  test('result without geom.bbox should leave bounding_box undefined', (t) => {
+    const placeholder_response = [
+      {
+        id: 123,
+        name: 'name 1',
+        placetype: 'neighbourhood',
+        geom: {
+          area: 12.34,
+          lat: 14.141414,
+          lon: 41.414141
+        }
+      }
+    ];
+
+    const placeholderService = {
+      search: (text, language, do_not_track, callback) => {
+        t.equals(text, 'query value');
+        t.equals(language, 'language value');
+        callback(null, placeholder_response);
+      }
+    };
+
+    const should_execute = (req, res) => {
+      return true;
+    };
+
+    const controller = placeholder(placeholderService, should_execute);
+
+    const req = {
+      clean: {
+        text: 'query value',
+        lang: {
+          iso6393: 'language value'
+        }
+      }
+    };
+    const res = { };
+
+    const expected_res = {
+      meta: {},
+      data: [
+        {
+          _id: '123',
+          _type: 'neighbourhood',
+          layer: 'neighbourhood',
+          source: 'whosonfirst',
+          source_id: '123',
+          center_point: {
+            lat: 14.141414,
+            lon: 41.414141
+          },
+          name: {
+            'default': 'name 1'
+          },
+          phrase: {
+            'default': 'name 1'
+          },
+          parent: { }
+        }
+      ]
+    };
+
+    controller(req, res, () => {
+      t.deepEquals(res, expected_res);
+      t.end();
+    });
+
+  });
+
+  test('result without geom.lat/geom.lon should leave centroid undefined', (t) => {
+    const placeholder_response = [
+      {
+        id: 123,
+        name: 'name 1',
+        placetype: 'neighbourhood',
+        geom: {
+          area: 12.34,
+          bbox: '21.212121,12.121212,31.313131,13.131313'
+        }
+      }
+    ];
+
+    const placeholderService = {
+      search: (text, language, do_not_track, callback) => {
+        t.equals(text, 'query value');
+        t.equals(language, 'language value');
+        callback(null, placeholder_response);
+      }
+    };
+
+    const should_execute = (req, res) => {
+      return true;
+    };
+
+    const controller = placeholder(placeholderService, should_execute);
+
+    const req = {
+      clean: {
+        text: 'query value',
+        lang: {
+          iso6393: 'language value'
+        }
+      }
+    };
+    const res = { };
+
+    const expected_res = {
+      meta: {},
+      data: [
+        {
+          _id: '123',
+          _type: 'neighbourhood',
+          layer: 'neighbourhood',
+          source: 'whosonfirst',
+          source_id: '123',
+          bounding_box: '{"min_lat":12.121212,"max_lat":13.131313,"min_lon":21.212121,"max_lon":31.313131}',
+          name: {
+            'default': 'name 1'
+          },
+          phrase: {
+            'default': 'name 1'
+          },
+          parent: { }
+        }
+      ]
+    };
+
+    controller(req, res, () => {
+      t.deepEquals(res, expected_res);
+      t.end();
+    });
+
+  });
+
 };
 
 module.exports.tests.do_not_track = function(test, common) {