Merge pull request #1102 from pelias/name_aliases

feat: support aliases for name fields

helper/fieldValue.js

@@ -0,0 +1,47 @@
+'use strict';
+
+const _ = require('lodash');
+
+function getStringValue(property) {
+  // numeric value, cast to string
+  if (_.isNumber(property)) {
+    return _.toString(property);
+  }
+
+  // isEmpty check works for all types of values: strings, arrays, objects
+  if (_.isEmpty(property)) {
+    return '';
+  }
+
+  if (_.isString(property)) {
+    return property;
+  }
+
+  // array value, take first item in array (at this time only used for admin & name values)
+  if (_.isArray(property)) {
+    return property[0];
+  }
+
+  return _.toString(property);
+}
+
+function getArrayValue(property) {
+  // numeric value, cast to array
+  if (_.isNumber(property)) {
+    return [property];
+  }
+
+  // isEmpty check works for all types of values: strings, arrays, objects
+  if (_.isEmpty(property)) {
+    return [];
+  }
+
+  if (_.isArray(property)) {
+    return property;
+  }
+
+  return [property];
+}
+
+module.exports.getStringValue = getStringValue;
+module.exports.getArrayValue = getArrayValue;

helper/geojsonify.js

@@ -5,6 +5,7 @@ const logger = require('pelias-logger').get('geojsonify');
 const collectDetails = require('./geojsonify_place_details');
 const _ = require('lodash');
 const Document = require('pelias-model').Document;
+const field = require('./fieldValue');
 
 function geojsonifyPlaces( params, docs ){
 
@@ -53,7 +54,7 @@ function geojsonifyPlace(params, place) {
 
   // assign name, logging a warning if it doesn't exist
   if (_.has(place, 'name.default')) {
-    doc.name = place.name.default;
+    doc.name = field.getStringValue(place.name.default);
   } else {
     logger.warn(`doc ${doc.gid} does not contain name.default`);
   }

helper/geojsonify_place_details.js

@@ -1,6 +1,7 @@
 'use strict';
 
 const _ = require('lodash');
+const field = require('./fieldValue');
 
 // Properties to be copied
 // If a property is identified as a single string, assume it should be presented as a string in response
@@ -89,10 +90,10 @@ function collectProperties( params, source ) {
 
       switch (prop.type) {
         case 'string':
-          value = getStringValue(source[prop.name]);
+          value = field.getStringValue(source[prop.name]);
           break;
         case 'array':
-          value = getArrayValue(source[prop.name]);
+          value = field.getArrayValue(source[prop.name]);
           break;
         // default behavior is to copy property exactly as is
         default:
@@ -110,35 +111,4 @@ function collectProperties( params, source ) {
 
 }
 
-function getStringValue(property) {
-  // isEmpty check works for all types of values: strings, arrays, objects
-  if (_.isEmpty(property)) {
-    return '';
-  }
-
-  if (_.isString(property)) {
-    return property;
-  }
-
-  // array value, take first item in array (at this time only used for admin values)
-  if (_.isArray(property)) {
-    return property[0];
-  }
-
-  return _.toString(property);
-}
-
-function getArrayValue(property) {
-  // isEmpty check works for all types of values: strings, arrays, objects
-  if (_.isEmpty(property)) {
-    return '';
-  }
-
-  if (_.isArray(property)) {
-    return property;
-  }
-
-  return [property];
-}
-
 module.exports = collectProperties;

middleware/assignLabels.js

@@ -1,4 +1,4 @@
-var defaultLabelGenerator = require('pelias-labels');
+const defaultLabelGenerator = require('pelias-labels');
 
 function setup(labelGenerator) {
   function middleware(req, res, next) {

middleware/confidenceScore.js

@@ -11,9 +11,10 @@
  * - detection (or specification) of query type. i.e. an address shouldn't match an admin address.
  */
 
-var stats = require('stats-lite');
+const stats = require('stats-lite');
-var logger = require('pelias-logger').get('api');
+const logger = require('pelias-logger').get('api');
-var check = require('check-types');
+const check = require('check-types');
+const field = require('../helper/fieldValue');
 
 var RELATIVE_SCORES = true;
 
@@ -131,12 +132,12 @@ function checkDistanceFromMean(score, mean, stdev) {
 function checkName(text, parsed_text, hit) {
   // parsed_text name should take precedence if available since it's the cleaner name property
   if (check.assigned(parsed_text) && check.assigned(parsed_text.name) &&
-    hit.name.default.toLowerCase() === parsed_text.name.toLowerCase()) {
+    field.getStringValue(hit.name.default).toLowerCase() === parsed_text.name.toLowerCase()) {
     return 1;
   }
 
   // if no parsed_text check the text value as provided against result's default name
-  if (hit.name.default.toLowerCase() === text.toLowerCase()) {
+  if (field.getStringValue(hit.name.default).toLowerCase() === text.toLowerCase()) {
     return 1;
   }
 

middleware/dedupe.js

@@ -1,6 +1,7 @@
-var logger = require('pelias-logger').get('api');
+const logger = require('pelias-logger').get('api');
-var _ = require('lodash');
+const _ = require('lodash');
-var isDifferent = require('../helper/diffPlaces').isDifferent;
+const isDifferent = require('../helper/diffPlaces').isDifferent;
+const field = require('../helper/fieldValue');
 
 function setup() {
   return dedupeResults;
@@ -38,7 +39,7 @@ function dedupeResults(req, res, next) {
         logger.info('[dupe][replacing]', {
           query: req.clean.text,
           previous: uniqueResults[dupeIndex].source,
-          hit: hit.name.default + ' ' + hit.source + ':' + hit._id
+          hit: field.getStringValue(hit.name.default) + ' ' + hit.source + ':' + hit._id
         });
         // replace previous dupe item with current hit
         uniqueResults[dupeIndex] = hit;
@@ -48,7 +49,7 @@ function dedupeResults(req, res, next) {
         logger.info('[dupe][skipping]', {
           query: req.clean.text,
           previous: uniqueResults[dupeIndex].source,
-          hit: hit.name.default + ' ' + hit.source + ':' + hit._id
+          hit: field.getStringValue(hit.name.default) + ' ' + hit.source + ':' + hit._id
         });
       }
     }

middleware/localNamingConventions.js

@@ -1,5 +1,6 @@
-var check = require('check-types');
+const check = require('check-types');
-var _ = require('lodash');
+const _ = require('lodash');
+const field = require('../helper/fieldValue');
 
 var flipNumberAndStreetCountries = ['DEU', 'FIN', 'SWE', 'NOR', 'DNK', 'ISL', 'CZE'];
 
@@ -49,7 +50,7 @@ function flipNumberAndStreet(place) {
       flipped  = ( place.address_parts.street + ' ' + place.address_parts.number );
 
   // flip street name and housenumber
-  if( place.name.default === standard ){
+  if( field.getStringValue(place.name.default) === standard ){
     place.name.default = flipped;
   }
 }

package.json

@@ -56,7 +56,7 @@
     "morgan": "^1.8.2",
     "pelias-categories": "1.2.0",
     "pelias-config": "2.14.0",
-    "pelias-labels": "1.7.0",
+    "pelias-labels": "1.8.0",
     "pelias-logger": "0.3.1",
     "pelias-microservice-wrapper": "1.3.0",
     "pelias-model": "5.3.2",

test/unit/helper/diffPlaces.js

@@ -166,6 +166,22 @@ module.exports.tests.dedupe = function(test, common) {
     t.false(isDifferent(item1, item2), 'should be the same');
     t.end();
   });
+
+  test('works with name aliases', function(t) {
+    var item1 = {
+      'name': {
+        'default': ['a','b'] // note the array
+      }
+    };
+    var item2 = {
+      'name': {
+        'default': 'a'
+      }
+    };
+
+    t.false(isDifferent(item1, item2), 'should be the same');
+    t.end();
+  });
 };
 
 module.exports.all = function (tape, common) {

test/unit/helper/fieldValue.js

@@ -0,0 +1,44 @@
+const field = require('../../../helper/fieldValue');
+
+module.exports.tests = {};
+
+module.exports.tests.getStringValue = function(test) {
+  test('getStringValue', function(t) {
+    t.equal(field.getStringValue([]), '', 'empty array');
+    t.equal(field.getStringValue(''), '', 'empty string');
+    t.equal(field.getStringValue('foo'), 'foo', 'string');
+    t.equal(field.getStringValue(['foo','bar']), 'foo', 'array');
+    t.equal(field.getStringValue(['']), '', 'array with empty string');
+    t.equal(field.getStringValue(-0), '-0', 'number');
+    t.equal(field.getStringValue(+0), '0', 'number');
+
+    // note: this behaviour is not desirable, it was inherited during a refactor
+    // see: https://github.com/pelias/api/pull/1102
+    t.equal(field.getStringValue({foo: 'bar'}), '[object Object]', '_.toString');
+    t.end();
+  });
+};
+
+module.exports.tests.getArrayValue = function(test) {
+  test('getArrayValue', function(t) {
+    t.deepEqual(field.getArrayValue([]), [], 'empty array');
+    t.deepEqual(field.getArrayValue(''), [], 'empty string');
+    t.deepEqual(field.getArrayValue('foo'), ['foo'], 'string');
+    t.deepEqual(field.getArrayValue(['foo','bar']), ['foo','bar'], 'array');
+    t.deepEqual(field.getArrayValue(['']), [''], 'array with empty string');
+    t.deepEqual(field.getArrayValue(-0), [0], 'number');
+    t.deepEqual(field.getArrayValue(+0), [0], 'number');
+    t.deepEqual(field.getArrayValue({foo: 'bar'}), [{foo: 'bar'}], '[*]');
+    t.end();
+  });
+};
+
+module.exports.all = function (tape, common) {
+  function test(name, testFunction) {
+    return tape('fieldValue: ' + name, testFunction);
+  }
+
+  for( var testCase in module.exports.tests ){
+    module.exports.tests[testCase](test, common);
+  }
+};

test/unit/helper/geojsonify.js

@@ -638,6 +638,51 @@ module.exports.tests.non_optimal_conditions = (test, common) => {
 
 };
 
+// ensure that if elasticsearch returns an array of values for name.default
+// .. that we handle this case and select the first element for the label.
+module.exports.tests.nameAliases = function(test, common) {
+  test('name aliases', function(t) {
+    var aliases = [{
+      '_type': 'example',
+      '_id': '1',
+      'source': 'example',
+      'layer': 'example',
+      'name': {
+        'default': ['Example1', 'Example2'] // note the array
+      },
+      'center_point': {
+        'lon': 0,
+        'lat': 0
+      }
+    }];
+
+    const expected = {
+      type: 'FeatureCollection',
+      features: [{
+        type: 'Feature',
+        geometry: {
+          type: 'Point',
+          coordinates: [ 0, 0 ]
+        },
+        properties: {
+          id: '1',
+          gid: 'example:example:1',
+          layer: 'example',
+          source: 'example',
+          source_id: undefined,
+          name: 'Example1'
+        }
+      }],
+      bbox: [ 0, 0, 0, 0 ]
+    };
+
+    var actual = geojsonify( {}, aliases );
+    t.deepEquals(actual, expected);
+    t.end();
+  });
+
+};
+
 module.exports.all = (tape, common) => {
   function test(name, testFunction) {
     return tape(`geojsonify: ${name}`, testFunction);

test/unit/helper/geojsonify_place_details.js

@@ -108,7 +108,7 @@ module.exports.tests.geojsonify_place_details = (test, common) => {
   });
 
   test('\'empty\' string-type values should be output as \'\'', t => {
-    [ [], {}, '', 17, true, null, undefined ].forEach(empty_value => {
+    [ [], {}, '', true, null, undefined ].forEach(empty_value => {
       const source = {
         housenumber: empty_value,
         street: empty_value,

test/unit/middleware/assignLabels.js

@@ -104,6 +104,33 @@ module.exports.tests.serialization = function(test, common) {
 
   });
 
+  test('support name aliases', function(t) {
+    var assignLabels = require('../../../middleware/assignLabels')();
+
+    var res = {
+      data: [{
+        name: {
+          default: ['name1','name2']
+        }
+      }]
+    };
+
+    var expected = {
+      data: [{
+        name: {
+          default: ['name1','name2']
+        },
+        label: 'name1'
+      }]
+    };
+
+    assignLabels({}, res, function () {
+      t.deepEqual(res, expected);
+      t.end();
+    });
+
+  });
+
 };
 
 module.exports.all = function (tape, common) {

test/unit/middleware/confidenceScore.js

@@ -201,6 +201,38 @@ module.exports.tests.confidenceScore = function(test, common) {
     t.equal(res.data[0].confidence, 0.28, 'score was set');
     t.end();
   });
+
+  test('works with name aliases', function(t) {
+    var req = {
+      clean: {
+        text: 'example',
+        parsed_text: {
+          number: 123,
+          street: 'example',
+          state: 'EG'
+        }
+      }
+    };
+    var res = {
+      data: [{
+        _score: 10,
+        found: true,
+        value: 1,
+        center_point: { lat: 100.1, lon: -50.5 },
+        name: { default: ['test name1', 'test name2'] }, // note the array
+      }],
+      meta: {
+        scores: [10],
+        query_type: 'original'
+      }
+    };
+
+    t.doesNotThrow(() => {
+      confidenceScore(req, res, () => {});
+    });
+    t.equal(res.data[0].confidence, 0.28, 'score was set');
+    t.end();
+  });
 };
 
 module.exports.all = function (tape, common) {

test/unit/middleware/dedupe.js

@@ -223,7 +223,7 @@ module.exports.tests.trump = function(test, common) {
     });
   });
 
-test('osm with zip trumps openaddresses without zip', function (t) {
+  test('osm with zip trumps openaddresses without zip', function (t) {
     var req = {
       clean: {
         text: '100 Main St',
@@ -257,7 +257,37 @@ test('osm with zip trumps openaddresses without zip', function (t) {
       t.deepEqual(res.data[0].source_id, '654321', 'openstreetmap result with zip won');
       t.end();
     });
-});
+  });
+
+  test('works with name aliases', function (t) {
+    var req = {
+      clean: {
+        text: '100 Main St',
+        size: 100
+      }
+    };
+    var res = {
+      data:  [
+        {
+          'name': { 'default': ['100 Main St'] }, // note the array
+          'source': 'openaddresses',
+          'source_id': '123456'
+        },
+        {
+          'name': { 'default': '100 Main St' },
+          'source': 'openstreetmap',
+          'source_id': '654321'
+        }
+      ]
+    };
+
+    t.doesNotThrow(() => {
+      dedupe(req, res, () => {});
+    });
+
+    t.equal(res.data.length, 1, 'results have fewer items than before');
+    t.end();
+  });
 };
 
 module.exports.all = function (tape, common) {

test/unit/middleware/localNamingConventions.js

@@ -1,5 +1,5 @@
 
-var proxyquire = require('proxyquire');
+const proxyquire = require('proxyquire');
 
 var customConfig = {
   generate: function generate() {
@@ -40,7 +40,7 @@ module.exports.tests.flipNumberAndStreet = function(test, common) {
   var deAddress = {
     '_id': 'test2',
     '_type': 'test',
-    'name': { 'default': '23 Grolmanstraße' },
+    'name': { 'default': ['23 Grolmanstraße'] },
     'center_point': { 'lon': 13.321487, 'lat': 52.506781 },
     'address_parts': {
        'zip': '10623',

test/unit/run.js

@@ -32,6 +32,7 @@ var tests = [
   require('./controller/predicates/is_request_sources_only_whosonfirst'),
   require('./helper/debug'),
   require('./helper/diffPlaces'),
+  require('./helper/fieldValue'),
   require('./helper/geojsonify_place_details'),
   require('./helper/geojsonify'),
   require('./helper/logging'),