Merge pull request #1693 from pelias/fix-layer-and-source-combination

Handle sources and layers param with address filter

Author: orangejulius

sanitizer/_address_layer_filter.js

@@ -25,6 +25,51 @@ const nonEmptyString = (v) => _.isString(v) && !_.isEmpty(v);
 
 const ADDRESS_FILTER_WARNING = 'performance optimization: excluding \'address\' layer';
 
+function can_remove_addresses(clean) {
+  // default to using the full 'clean.text'
+  // note: this should already have superfluous characters removed
+  let input = clean.text;
+
+  // if a parser has removed tokens, use the parsed text instead, this
+  // is the text which will be queried against the 'name.default' field.
+  // @todo: this logic is duplicated from 'query/text_parser.js' and may
+  // be subject to change.
+  if (_.isObject(clean.parsed_text) && !_.isEmpty(clean.parsed_text)) {
+
+    var isStreetAddress = clean.parsed_text.hasOwnProperty('housenumber') && clean.parsed_text.hasOwnProperty('street');
+
+    // use $subject where available (pelias parser)
+    if (_.has(clean, 'parsed_text.subject')) {
+      input = clean.parsed_text.subject;
+    }
+
+    // if 'pelias_parser' or 'libpostal' identified input as a street address
+    else if (isStreetAddress) {
+      input = clean.parsed_text.housenumber + ' ' + clean.parsed_text.street;
+    }
+
+    // else if the 'naive parser' was used, input is equal to 'name'
+    else if (nonEmptyString(clean.parsed_text.admin_parts) && nonEmptyString(clean.parsed_text.name)) {
+      input = clean.parsed_text.name;
+    }
+  }
+
+  // count the number of words specified
+  let totalWords = input.split(/\s+/).filter(nonEmptyString).length;
+
+  // check that at least one numeral was specified
+  let hasNumeral = /\d/.test(input);
+
+  // do not consider numeric street names, such as '26 st' in numeric check.
+  if( _.has(clean, 'parsed_text.street') ){
+    hasNumeral = /\d/.test(input.replace(clean.parsed_text.street, ''));
+  }
+
+  // if less than two words were specified /or no numeral is present
+  // then it is safe to apply the layer filter
+  return totalWords < 2 || !hasNumeral;
+}
+
 function _setup(tm) {
 
   return {
@@ -38,54 +83,12 @@ function _setup(tm) {
         return messages;
       }
 
-      // default to using the full 'clean.text'
-      // note: this should already have superfluous characters removed
-      let input = clean.text;
-
       // do nothing if no input text specified in the request
-      if (!nonEmptyString(input)) {
+      if (!nonEmptyString(clean.text)) {
         return messages;
       }
 
-      // if a parser has removed tokens, use the parsed text instead, this
-      // is the text which will be queried against the 'name.default' field.
-      // @todo: this logic is duplicated from 'query/text_parser.js' and may
-      // be subject to change.
-      if (_.isObject(clean.parsed_text) && !_.isEmpty(clean.parsed_text)) {
-
-        var isStreetAddress = clean.parsed_text.hasOwnProperty('housenumber') && clean.parsed_text.hasOwnProperty('street');
-
-        // use $subject where available (pelias parser)
-        if (_.has(clean, 'parsed_text.subject')) {
-          input = clean.parsed_text.subject;
-        }
-
-        // if 'pelias_parser' or 'libpostal' identified input as a street address
-        else if (isStreetAddress) {
-          input = clean.parsed_text.housenumber + ' ' + clean.parsed_text.street;
-        }
-
-        // else if the 'naive parser' was used, input is equal to 'name'
-        else if (nonEmptyString(clean.parsed_text.admin_parts) && nonEmptyString(clean.parsed_text.name)) {
-          input = clean.parsed_text.name;
-        }
-      }
-
-      // count the number of words specified
-      let totalWords = input.split(/\s+/).filter(nonEmptyString).length;
-
-      // check that at least one numeral was specified
-      let hasNumeral = /\d/.test(input);
-
-      // do not consider numeric street names, such as '26 st' in numeric check.
-      if( _.has(clean, 'parsed_text.street') ){
-        hasNumeral = /\d/.test(input.replace(clean.parsed_text.street, ''));
-      }
-
-      // if less than two words were specified /or no numeral is present
-      // then it is safe to apply the layer filter
-      if (totalWords < 2 || !hasNumeral) {
-
+      if (can_remove_addresses(clean)) {
         // handle the common case where neither sources nor (positive) layers were specified
         if (!_.isArray(clean.sources) || _.isEmpty(clean.sources)) {
           // if there are no layers already set, start with the list of all of them
@@ -100,7 +103,6 @@ function _setup(tm) {
 
         // handle the case where 'sources' were explicitly specified
         else if (_.isArray(clean.sources)) {
-
           // we need to create a list of layers for the specified sources
           let sourceLayers = clean.sources.reduce((l, key) => l.concat(tm.layers_by_source[key] || []), []);
           sourceLayers = _.uniq(sourceLayers); // dedupe
@@ -111,8 +113,18 @@ function _setup(tm) {
             return messages;
           }
 
-          // target all layers for the sources specified except 'address'
-          clean.layers = sourceLayers.filter(item => item !== 'address'); // exclude 'address'
+          // create a list of all "possible layers": layers from the specified sources, minus address layer
+          const possibleLayers = sourceLayers.filter(item => item !== 'address');
+
+          // intersect the possible layers with any already specified layer preferences
+          if (_.isArray(clean.layers) && clean.layers.length > 1) {
+            // layers already exist, intersect
+            clean.layers = _.intersection(clean.layers, possibleLayers);
+          } else {
+            // no layers already, use all possible layers
+            clean.layers = possibleLayers;
+          }
+
           messages.warnings.push(ADDRESS_FILTER_WARNING);
         }
       }

test/unit/sanitizer/_address_layer_filter.js

@@ -117,6 +117,26 @@ module.exports.tests.sanitize = function (test, common) {
     t.end();
   });
 
+  test('sanitize - exclude addresses when negative layers and sources are specified', (t) => {
+    // select all layers except venue to simulate value of clean.layers from targets sanitizer
+    const clean_layers = real_type_mapping.getCanonicalLayers().filter(layer => layer !== 'venue').sort();
+
+    let clean = { text: 'foo',
+      layers: clean_layers,
+      negative_layers: ['venue'],
+      positive_layers: [],
+      sources: ['openstreetmap', 'openaddresses','whosonfirst'],
+      negative_sources: ['geonames'],
+      positive_sources: []
+    };
+
+    const expected_layers = clean_layers.filter(layer => layer !== 'address').sort();
+
+    t.deepEqual(real_sanitizer.sanitize(null, clean), STD_MESSAGES);
+    t.deepEqual(clean.layers.sort(), expected_layers, 'layer list is reduced to exclude addresses');
+    t.end();
+  });
+
   test('sanitize - exclude addresses when negative layers other than address are specified', (t) => {
     // select all layers except venue to simulate value of clean.layers from targets sanitizer
     const clean_layers = real_type_mapping.getCanonicalLayers().filter(layer => layer !== 'venue').sort();