Skip to content

Commit

Permalink
fix(auto_discover_type_mapping): bugfix to allow existing references …
Browse files Browse the repository at this point in the history
…to the type mapping to continue functioning after update
  • Loading branch information
missinglink committed Jun 26, 2019
1 parent a221117 commit d751225
Show file tree
Hide file tree
Showing 7 changed files with 68 additions and 49 deletions.
45 changes: 35 additions & 10 deletions helper/TypeMapping.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,32 +57,45 @@ TypeMapping.addStandardTargetsToAliases = function(standard, aliases) {

// source alias setter
TypeMapping.prototype.setSourceAliases = function( aliases ){
this.source_aliases = aliases;
safeReplace(this.source_aliases, aliases);
};

// layers-by-source alias setter
TypeMapping.prototype.setLayersBySource = function( lbs ){
this.layers_by_source = lbs;
safeReplace(this.layers_by_source, lbs);
};

// layer alias setter
TypeMapping.prototype.setLayerAliases = function( aliases ){
this.layer_aliases = aliases;
safeReplace(this.layer_aliases, aliases);
};

// canonical sources setter
TypeMapping.prototype.setCanonicalSources = function( sources ){
this.canonical_sources = sources;
safeReplace(this.canonical_sources, sources);
};

// generate mappings after setters have been run
TypeMapping.prototype.generateMappings = function(){
this.sources = Object.keys( this.layers_by_source );
this.source_mapping = TypeMapping.addStandardTargetsToAliases(this.sources, this.source_aliases);
this.layers = _.uniq(Object.keys(this.layers_by_source).reduce(function(acc, key) {
return acc.concat(this.layers_by_source[key]);
}.bind(this), []));
this.layer_mapping = TypeMapping.addStandardTargetsToAliases(this.layers, this.layer_aliases);

safeReplace(this.sources, Object.keys(this.layers_by_source) );

safeReplace(
this.source_mapping,
TypeMapping.addStandardTargetsToAliases(this.sources, this.source_aliases)
);

safeReplace(
this.layers,
_.uniq(Object.keys(this.layers_by_source).reduce(function (acc, key) {
return acc.concat(this.layers_by_source[key]);
}.bind(this), []))
);

safeReplace(
this.layer_mapping,
TypeMapping.addStandardTargetsToAliases(this.layers, this.layer_aliases)
);
};

// generate a list of all layers which are part of the canonical Pelias configuration
Expand Down Expand Up @@ -133,4 +146,16 @@ TypeMapping.prototype.load = function( done ){
loadFromElasticsearch(this, done);
};

// replace the contents of an object or array
// while maintaining the original pointer reference
function safeReplace(reference, replacement){
if (_.isObject(reference) && _.isObject(replacement) ){
for( let attr in reference ){ delete reference[attr]; }
for (let attr in replacement) { reference[attr] = replacement[attr]; }
} else if (_.isArray(reference) && _.isArray(replacement)) {
reference.length = 0;
replacement.forEach(el => reference.push(el));
}
}

module.exports = TypeMapping;
3 changes: 3 additions & 0 deletions helper/type_mapping_discovery.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,9 @@ module.exports = (tm, done) => {
logger.info( 'total sources', sources.length );
logger.info( 'successfully discovered type mapping from elasticsearch' );
tm.setLayersBySource( layersBySource );

// (re)generate the mappings
tm.generateMappings();
}
}

Expand Down
12 changes: 3 additions & 9 deletions sanitizer/_address_layer_filter.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,9 @@ const check = require('check-types');

const ADDRESS_FILTER_WARNING = 'performance optimization: excluding \'address\' layer';

function _setup(layersBySource) {

// generate a deduplicated list of all layers except 'address'
let layers = Object.keys(layersBySource).reduce((l, key) => l.concat(layersBySource[key]), []);
layers = _.uniq(layers); // dedupe
layers = layers.filter(item => item !== 'address'); // exclude 'address'
function _setup(tm) {

return {
layers: layers,
sanitize: function _sanitize(__, clean) {

// error & warning messages
Expand Down Expand Up @@ -81,15 +75,15 @@ function _setup(layersBySource) {

// handle the common case where neither source nor layers were specified
if (!check.array(clean.sources) || !check.nonEmptyArray(clean.sources)) {
clean.layers = layers;
clean.layers = tm.layers.filter(item => item !== 'address'); // exclude 'address'
messages.warnings.push(ADDRESS_FILTER_WARNING);
}

// handle the case where 'sources' were explicitly specified
else if (check.array(clean.sources)) {

// we need to create a list of layers for the specified sources
let sourceLayers = clean.sources.reduce((l, key) => l.concat(layersBySource[key] || []), []);
let sourceLayers = clean.sources.reduce((l, key) => l.concat(tm.layers_by_source[key] || []), []);
sourceLayers = _.uniq(sourceLayers); // dedupe

// if the sources specified do not have any addresses or if removing the
Expand Down
9 changes: 4 additions & 5 deletions sanitizer/_targets.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,7 @@ function getValidKeys(mapping) {
function _setup( paramName, targetMap ) {
const opts = {
paramName: paramName,
targetMap: targetMap,
targetMapKeysString: getValidKeys(targetMap)
targetMap: targetMap
};

return {
Expand All @@ -27,7 +26,7 @@ function _setup( paramName, targetMap ) {
// param must be a valid non-empty string
if( !check.nonEmptyString( targetsString ) ){
messages.errors.push(
opts.paramName + ' parameter cannot be an empty string. Valid options: ' + opts.targetMapKeysString
opts.paramName + ' parameter cannot be an empty string. Valid options: ' + getValidKeys(opts.targetMap)
);
}
else {
Expand All @@ -42,7 +41,7 @@ function _setup( paramName, targetMap ) {
return !opts.targetMap.hasOwnProperty(target);
}).forEach( function( target ){
messages.errors.push(
'\'' + target + '\' is an invalid ' + opts.paramName + ' parameter. Valid options: ' + opts.targetMapKeysString
'\'' + target + '\' is an invalid ' + opts.paramName + ' parameter. Valid options: ' + getValidKeys(opts.targetMap)
);
});

Expand All @@ -61,7 +60,7 @@ function _setup( paramName, targetMap ) {
// string is empty
else if( check.string( targetsString ) ){
messages.errors.push(
opts.paramName + ' parameter cannot be an empty string. Valid options: ' + opts.targetMapKeysString
opts.paramName + ' parameter cannot be an empty string. Valid options: ' + getValidKeys(opts.targetMap)
);
}

Expand Down
2 changes: 1 addition & 1 deletion sanitizer/autocomplete.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ module.exports.middleware = (_api_pelias_config) => {
size: require('../sanitizer/_size')(/* use defaults*/),
layers: require('../sanitizer/_targets')('layers', type_mapping.layer_mapping),
sources: require('../sanitizer/_targets')('sources', type_mapping.source_mapping),
address_layer_filter: require('../sanitizer/_address_layer_filter')(type_mapping.layers_by_source),
address_layer_filter: require('../sanitizer/_address_layer_filter')(type_mapping),
// depends on the layers and sources sanitizers, must be run after them
sources_and_layers: require('../sanitizer/_sources_and_layers')(),
private: require('../sanitizer/_flag_bool')('private', false),
Expand Down
2 changes: 1 addition & 1 deletion sanitizer/search.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ module.exports.middleware = (_api_pelias_config) => {
size: require('../sanitizer/_size')(/* use defaults*/),
layers: require('../sanitizer/_targets')('layers', type_mapping.layer_mapping),
sources: require('../sanitizer/_targets')('sources', type_mapping.source_mapping),
address_layer_filter: require('../sanitizer/_address_layer_filter')(type_mapping.layers_by_source),
address_layer_filter: require('../sanitizer/_address_layer_filter')(type_mapping),
// depends on the layers and sources sanitizers, must be run after them
sources_and_layers: require('../sanitizer/_sources_and_layers')(),
private: require('../sanitizer/_flag_bool')('private', false),
Expand Down
44 changes: 21 additions & 23 deletions test/unit/sanitizer/_address_layer_filter.js
Original file line number Diff line number Diff line change
@@ -1,25 +1,15 @@
const sanitizer = require('../../../sanitizer/_address_layer_filter');
const TypeMapping = require('../../../helper/TypeMapping');
const NO_MESSAGES = { errors: [], warnings: [] };
const STD_MESSAGES = { errors: [], warnings: ['performance optimization: excluding \'address\' layer'] };

module.exports.tests = {};

module.exports.tests.enumerate_layers = function (test, common) {
test('enumerate layers', (t) => {
let m = { A: ['A', 'B'], B: ['B'], C: ['B', 'C'] };
t.deepEqual(sanitizer(m).layers, ['A','B','C']);
t.end();
});
test('enumerate layers - exclude "address"', (t) => {
let m = { A: ['A', 'B', 'address'], B: ['B'], C: ['B', 'address', 'C'] };
t.deepEqual(sanitizer(m).layers, ['A', 'B', 'C']);
t.end();
});
};

module.exports.tests.sanitize = function (test, common) {
let m = { A: ['A'], B: ['B'], C: ['C'] };
let s = sanitizer(m);
let tm = new TypeMapping();
tm.setLayersBySource({ A: ['A'], B: ['B'], C: ['C'] });
tm.generateMappings();
let s = sanitizer(tm);

test('sanitize - do nothing if clean.layers already specified', (t) => {
let clean = { text: '1 example', layers: ['not empty'] };
Expand Down Expand Up @@ -71,8 +61,10 @@ module.exports.tests.sanitize = function (test, common) {
});

test('sanitize - reduce target layers when clean.sources specified', (t) => {
let m = { A: ['A', 'address'], B: ['B', 'address'], C: ['C'] };
let s = sanitizer(m);
let tm = new TypeMapping();
tm.setLayersBySource({ A: ['A', 'address'], B: ['B', 'address'], C: ['C'] });
tm.generateMappings();
let s = sanitizer(tm);

let clean = { text: 'foo', sources: [ 'A', 'C' ] };
t.deepEqual(s.sanitize(null, clean), STD_MESSAGES);
Expand All @@ -81,8 +73,10 @@ module.exports.tests.sanitize = function (test, common) {
});

test('sanitize - do nothing for sources which do not have any addresses', (t) => {
let m = { A: ['address'], B: ['address'] };
let s = sanitizer(m);
let tm = new TypeMapping();
tm.setLayersBySource({ A: ['address'], B: ['address'] });
tm.generateMappings();
let s = sanitizer(tm);

let clean = { text: 'foo', sources: ['A', 'B'] };
t.deepEqual(s.sanitize(null, clean), NO_MESSAGES);
Expand All @@ -92,8 +86,10 @@ module.exports.tests.sanitize = function (test, common) {
};

module.exports.tests.tricky_inputs = function (test, common) {
let m = { A: ['A'], B: ['B'], C: ['C'] };
let s = sanitizer(m);
let tm = new TypeMapping();
tm.setLayersBySource({ A: ['A'], B: ['B'], C: ['C'] });
tm.generateMappings();
let s = sanitizer(tm);

test('tricky inputs - extra whitespace', (t) => {
let clean = { text: ' \t\n 12 \n\t ' };
Expand All @@ -112,8 +108,10 @@ module.exports.tests.tricky_inputs = function (test, common) {

// handle cases where a parser has run and removed admin tokens
module.exports.tests.parsed_text = function (test, common) {
let m = { A: ['A'], B: ['B'], C: ['C'] };
let s = sanitizer(m);
let tm = new TypeMapping();
tm.setLayersBySource({ A: ['A'], B: ['B'], C: ['C'] });
tm.generateMappings();
let s = sanitizer(tm);

test('naive parser - apply filter due to comma being present', (t) => {
let clean = { text: 'A', parsed_text: { name: '1', admin_parts: 'Avenue' } };
Expand Down

0 comments on commit d751225

Please sign in to comment.