Browse Source

converted geojsonify_place_details to return instead of modify

pull/1002/head
Stephen Hess 7 years ago
parent
commit
9d54735e41
  1. 2
      helper/geojsonify.js
  2. 22
      helper/geojsonify_place_details.js
  3. 73
      test/unit/helper/geojsonify.js
  4. 106
      test/unit/helper/geojsonify_place_details.js

2
helper/geojsonify.js

@ -58,7 +58,7 @@ function geojsonifyPlace(params, place) {
logger.warn(`doc ${doc.gid} does not contain name.default`); logger.warn(`doc ${doc.gid} does not contain name.default`);
} }
addDetails(params, place, doc); _.assign(doc, addDetails(params, place));
return doc; return doc;
} }

22
helper/geojsonify_place_details.js

@ -53,19 +53,18 @@ function checkCategoryParam(params) {
} }
/** /**
* Copy specified properties from source to dest. * Collect the specified properties from source into an object and return it
* Ignore missing properties. * Ignore missing properties.
* *
* @param {object} params clean query params * @param {object} params clean query params
* @param {object} source * @param {object} source
* @param {object} dst * @param {object} dst
*/ */
function copyProperties( params, source, dst ) { function collectProperties( params, source ) {
DETAILS_PROPS.forEach( function ( prop ) { return DETAILS_PROPS.reduce((result, prop) => {
// if condition isn't met, don't set the property
// if condition isn't met, just return without setting the property
if (_.isFunction(prop.condition) && !prop.condition(params)) { if (_.isFunction(prop.condition) && !prop.condition(params)) {
return; return result;
} }
if ( source.hasOwnProperty( prop.name ) ) { if ( source.hasOwnProperty( prop.name ) ) {
@ -84,10 +83,14 @@ function copyProperties( params, source, dst ) {
} }
if (_.isNumber(value) || (value && !_.isEmpty(value))) { if (_.isNumber(value) || (value && !_.isEmpty(value))) {
dst[prop.name] = value; result[prop.name] = value;
} }
} }
});
return result;
}, {});
} }
function getStringValue(property) { function getStringValue(property) {
@ -108,7 +111,6 @@ function getStringValue(property) {
return _.toString(property); return _.toString(property);
} }
function getArrayValue(property) { function getArrayValue(property) {
// isEmpty check works for all types of values: strings, arrays, objects // isEmpty check works for all types of values: strings, arrays, objects
if (_.isEmpty(property)) { if (_.isEmpty(property)) {
@ -122,4 +124,4 @@ function getArrayValue(property) {
return [property]; return [property];
} }
module.exports = copyProperties; module.exports = collectProperties;

73
test/unit/helper/geojsonify.js

@ -71,11 +71,15 @@ module.exports.tests.all = (test, common) => {
const geojsonify = proxyquire('../../../helper/geojsonify', { const geojsonify = proxyquire('../../../helper/geojsonify', {
'./geojsonify_place_details': (params, source, dst) => { './geojsonify_place_details': (params, source, dst) => {
if (source._id === 'id 1') { if (source._id === 'id 1') {
dst.property1 = 'property 1'; return {
dst.property2 = 'property 2'; property1: 'property 1',
property2: 'property 2'
};
} else if (source._id === 'id 2') { } else if (source._id === 'id 2') {
dst.property3 = 'property 3'; return {
dst.property4 = 'property 4'; property3: 'property 3',
property4: 'property 4'
};
} }
} }
@ -174,11 +178,15 @@ module.exports.tests.all = (test, common) => {
const geojsonify = proxyquire('../../../helper/geojsonify', { const geojsonify = proxyquire('../../../helper/geojsonify', {
'./geojsonify_place_details': (params, source, dst) => { './geojsonify_place_details': (params, source, dst) => {
if (source._id === 'id 1') { if (source._id === 'id 1') {
dst.property1 = 'property 1'; return {
dst.property2 = 'property 2'; property1: 'property 1',
property2: 'property 2'
};
} else if (source._id === 'id 2') { } else if (source._id === 'id 2') {
dst.property3 = 'property 3'; return {
dst.property4 = 'property 4'; property3: 'property 3',
property4: 'property 4'
};
} }
} }
@ -273,11 +281,15 @@ module.exports.tests.all = (test, common) => {
const geojsonify = proxyquire('../../../helper/geojsonify', { const geojsonify = proxyquire('../../../helper/geojsonify', {
'./geojsonify_place_details': (params, source, dst) => { './geojsonify_place_details': (params, source, dst) => {
if (source._id === 'id 1') { if (source._id === 'id 1') {
dst.property1 = 'property 1'; return {
dst.property2 = 'property 2'; property1: 'property 1',
property2: 'property 2'
};
} else if (source._id === 'id 2') { } else if (source._id === 'id 2') {
dst.property3 = 'property 3'; return {
dst.property4 = 'property 4'; property3: 'property 3',
property4: 'property 4'
};
} }
} }
@ -359,8 +371,10 @@ module.exports.tests.non_optimal_conditions = (test, common) => {
const geojsonify = proxyquire('../../../helper/geojsonify', { const geojsonify = proxyquire('../../../helper/geojsonify', {
'./geojsonify_place_details': (params, source, dst) => { './geojsonify_place_details': (params, source, dst) => {
if (source._id === 'id 1') { if (source._id === 'id 1') {
dst.property1 = 'property 1'; return {
dst.property2 = 'property 2'; property1: 'property 1',
property2: 'property 2'
};
} }
}, },
'pelias-logger': logger 'pelias-logger': logger
@ -429,8 +443,10 @@ module.exports.tests.non_optimal_conditions = (test, common) => {
const geojsonify = proxyquire('../../../helper/geojsonify', { const geojsonify = proxyquire('../../../helper/geojsonify', {
'./geojsonify_place_details': (params, source, dst) => { './geojsonify_place_details': (params, source, dst) => {
if (source._id === 'id 2') { if (source._id === 'id 2') {
dst.property3 = 'property 3'; return {
dst.property4 = 'property 4'; property3: 'property 3',
property4: 'property 4'
};
} }
}, },
'pelias-logger': logger 'pelias-logger': logger
@ -511,16 +527,21 @@ module.exports.tests.non_optimal_conditions = (test, common) => {
const geojsonify = proxyquire('../../../helper/geojsonify', { const geojsonify = proxyquire('../../../helper/geojsonify', {
'./geojsonify_place_details': (params, source, dst) => { './geojsonify_place_details': (params, source, dst) => {
if (source._id === 'id 1') { if (source._id === 'id 1') {
dst.property1 = 'property 1'; return {
dst.property2 = 'property 2'; property1: 'property 1',
} property2: 'property 2'
else if (source._id === 'id 2') { };
dst.property3 = 'property 3'; } else if (source._id === 'id 2') {
dst.property4 = 'property 4'; return {
} property3: 'property 3',
else if (source._id === 'id 3') { property4: 'property 4'
dst.property5 = 'property 5'; };
dst.property6 = 'property 6'; } else if (source._id === 'id 3') {
return {
property5: 'property 5',
property6: 'property 6'
};
} }
}, },
'pelias-logger': logger 'pelias-logger': logger

106
test/unit/helper/geojsonify_place_details.js

@ -5,7 +5,6 @@ module.exports.tests = {};
module.exports.tests.geojsonify_place_details = (test, common) => { module.exports.tests.geojsonify_place_details = (test, common) => {
test('plain old string values should be copied verbatim, replacing old values', t => { test('plain old string values should be copied verbatim, replacing old values', t => {
const source = { const source = {
unknown_field: 'original unknown_field value',
housenumber: 'housenumber value', housenumber: 'housenumber value',
street: 'street value', street: 'street value',
postalcode: 'postalcode value', postalcode: 'postalcode value',
@ -43,48 +42,8 @@ module.exports.tests.geojsonify_place_details = (test, common) => {
neighbourhood_gid: 'neighbourhood_gid value', neighbourhood_gid: 'neighbourhood_gid value',
label: 'label value' label: 'label value'
}; };
const destination = {
unknown_field: 'original unknown_field value',
housenumber: 'original housenumber value',
street: 'original street value',
postalcode: 'original postalcode value',
postalcode_gid: 'original postalcode_gid value',
match_type: 'original match_type value',
accuracy: 'original accuracy value',
country: 'original country value',
country_gid: 'original country_gid value',
country_a: 'original country_a value',
dependency: 'original dependency value',
dependency_gid: 'original dependency_gid value',
dependency_a: 'original dependency_a value',
macroregion: 'original macroregion value',
macroregion_gid: 'original macroregion_gid value',
macroregion_a: 'original macroregion_a value',
region: 'original region value',
region_gid: 'original region_gid value',
region_a: 'original region_a value',
macrocounty: 'original macrocounty value',
macrocounty_gid: 'original macrocounty_gid value',
macrocounty_a: 'original macrocounty_a value',
county: 'original county value',
county_gid: 'original county_gid value',
county_a: 'original county_a value',
localadmin: 'original localadmin value',
localadmin_gid: 'original localadmin_gid value',
localadmin_a: 'original localadmin_a value',
locality: 'original locality value',
locality_gid: 'original locality_gid value',
locality_a: 'original locality_a value',
borough: 'original borough value',
borough_gid: 'original borough_gid value',
borough_a: 'original borough_a value',
neighbourhood: 'original neighbourhood value',
neighbourhood_gid: 'original neighbourhood_gid value',
label: 'original label value'
};
const expected = { const expected = {
unknown_field: 'original unknown_field value',
housenumber: 'housenumber value', housenumber: 'housenumber value',
street: 'street value', street: 'street value',
postalcode: 'postalcode value', postalcode: 'postalcode value',
@ -123,9 +82,9 @@ module.exports.tests.geojsonify_place_details = (test, common) => {
label: 'label value' label: 'label value'
}; };
geojsonify({}, source, destination); const actual = geojsonify({}, source);
t.deepEqual(destination, expected); t.deepEqual(actual, expected);
t.end(); t.end();
}); });
@ -170,12 +129,11 @@ module.exports.tests.geojsonify_place_details = (test, common) => {
neighbourhood_gid: empty_value, neighbourhood_gid: empty_value,
label: empty_value label: empty_value
}; };
const destination = {};
const expected = {}; const expected = {};
geojsonify({}, source, destination); const actual = geojsonify({}, source);
t.deepEqual(destination, expected); t.deepEqual(actual, expected);
}); });
@ -222,7 +180,6 @@ module.exports.tests.geojsonify_place_details = (test, common) => {
neighbourhood_gid: ['neighbourhood_gid value 1', 'neighbourhood_gid value 2'], neighbourhood_gid: ['neighbourhood_gid value 1', 'neighbourhood_gid value 2'],
label: ['label value 1', 'label value 2'] label: ['label value 1', 'label value 2']
}; };
const destination = { };
const expected = { const expected = {
housenumber: 'housenumber value 1', housenumber: 'housenumber value 1',
@ -263,9 +220,9 @@ module.exports.tests.geojsonify_place_details = (test, common) => {
label: 'label value 1' label: 'label value 1'
}; };
geojsonify({}, source, destination); const actual = geojsonify({}, source);
t.deepEqual(destination, expected); t.deepEqual(actual, expected);
t.end(); t.end();
}); });
@ -310,7 +267,6 @@ module.exports.tests.geojsonify_place_details = (test, common) => {
neighbourhood_gid: { neighbourhood_gid: 'neighbourhood_gid value'}, neighbourhood_gid: { neighbourhood_gid: 'neighbourhood_gid value'},
label: { label: 'label value'} label: { label: 'label value'}
}; };
const destination = { };
const expected = { const expected = {
housenumber: '[object Object]', housenumber: '[object Object]',
@ -351,9 +307,9 @@ module.exports.tests.geojsonify_place_details = (test, common) => {
label: '[object Object]' label: '[object Object]'
}; };
geojsonify({}, source, destination); const actual = geojsonify({}, source);
t.deepEqual(destination, expected); t.deepEqual(actual, expected);
t.end(); t.end();
}); });
@ -365,11 +321,6 @@ module.exports.tests.geojsonify_place_details = (test, common) => {
distance: value, distance: value,
bounding_box: value bounding_box: value
}; };
const destination = {
confidence: 'original confidence value',
distance: 'original distance value',
bounding_box: 'original bounding_box value'
};
const expected = { const expected = {
confidence: value, confidence: value,
@ -377,9 +328,9 @@ module.exports.tests.geojsonify_place_details = (test, common) => {
bounding_box: value bounding_box: value
}; };
geojsonify({}, source, destination); const actual = geojsonify({}, source);
t.deepEqual(destination, expected); t.deepEqual(actual, expected);
}); });
@ -394,16 +345,15 @@ module.exports.tests.geojsonify_place_details = (test, common) => {
distance: value, distance: value,
bounding_box: value bounding_box: value
}; };
const destination = {};
const expected = { const expected = {
confidence: value, confidence: value,
distance: value, distance: value,
bounding_box: value bounding_box: value
}; };
geojsonify({}, source, destination); const actual = geojsonify({}, source);
t.deepEqual(destination, expected); t.deepEqual(actual, expected);
}); });
@ -418,12 +368,11 @@ module.exports.tests.geojsonify_place_details = (test, common) => {
distance: value, distance: value,
bounding_box: value bounding_box: value
}; };
const destination = {};
const expected = {}; const expected = {};
geojsonify({}, source, destination); const actual = geojsonify({}, source);
t.deepEqual(destination, expected); t.deepEqual(actual, expected);
}); });
@ -435,12 +384,11 @@ module.exports.tests.geojsonify_place_details = (test, common) => {
const source = { const source = {
category: [] category: []
}; };
const destination = {};
const expected = {}; const expected = {};
geojsonify({}, source, destination); const actual = geojsonify({}, source);
t.deepEqual(destination, expected); t.deepEqual(actual, expected);
t.end(); t.end();
}); });
@ -449,7 +397,6 @@ module.exports.tests.geojsonify_place_details = (test, common) => {
const source = { const source = {
category: [ 1, 2 ] category: [ 1, 2 ]
}; };
const destination = {};
const expected = { const expected = {
category: [ 1, 2 ] category: [ 1, 2 ]
}; };
@ -458,9 +405,9 @@ module.exports.tests.geojsonify_place_details = (test, common) => {
categories: true categories: true
}; };
geojsonify(clean, source, destination); const actual = geojsonify(clean, source);
t.deepEqual(destination, expected); t.deepEqual(actual, expected);
t.end(); t.end();
}); });
@ -470,7 +417,6 @@ module.exports.tests.geojsonify_place_details = (test, common) => {
const source = { const source = {
category: value category: value
}; };
const destination = {};
const expected = { const expected = {
category: [ value ] category: [ value ]
}; };
@ -479,9 +425,9 @@ module.exports.tests.geojsonify_place_details = (test, common) => {
categories: true categories: true
}; };
geojsonify(clean, source, destination); const actual = geojsonify(clean, source);
t.deepEqual(destination, expected); t.deepEqual(actual, expected);
}); });
@ -493,15 +439,14 @@ module.exports.tests.geojsonify_place_details = (test, common) => {
const source = { const source = {
category: [ 1, 2 ] category: [ 1, 2 ]
}; };
const destination = {};
const expected = { const expected = {
}; };
const clean = {}; const clean = {};
geojsonify(clean, source, destination); const actual = geojsonify(clean, source);
t.deepEqual(destination, expected); t.deepEqual(actual, expected);
t.end(); t.end();
}); });
@ -510,19 +455,18 @@ module.exports.tests.geojsonify_place_details = (test, common) => {
const source = { const source = {
category: [ 1, 2 ] category: [ 1, 2 ]
}; };
const destination = {};
const expected = { const expected = {
}; };
const clean = 'this is not an object'; const clean = 'this is not an object';
geojsonify(clean, source, destination); const actual = geojsonify(clean, source);
t.deepEqual(destination, expected); t.deepEqual(actual, expected);
t.end(); t.end();
}); });
}; };
module.exports.all = (tape, common) => { module.exports.all = (tape, common) => {

Loading…
Cancel
Save