diff --git a/helper/geojsonify.js b/helper/geojsonify.js index 3a97e047a..b049d1a51 100644 --- a/helper/geojsonify.js +++ b/helper/geojsonify.js @@ -6,6 +6,7 @@ const _ = require('lodash'); const Document = require('pelias-model').Document; const codec = require('pelias-model').codec; const field = require('./fieldValue'); +const decode_gid = require('./decode_gid'); function geojsonifyPlaces( params, docs ){ @@ -40,13 +41,14 @@ function geojsonifyPlaces( params, docs ){ } function geojsonifyPlace(params, place) { + const gid_components = decode_gid(place._id); // setup the base doc const doc = { - id: place.source_id, - gid: new Document(place.source, place.layer, place.source_id).getGid(), + id: gid_components.id, + gid: new Document(place.source, place.layer, gid_components.id).getGid(), layer: place.layer, source: place.source, - source_id: place.source_id, + source_id: gid_components.id, bounding_box: place.bounding_box, lat: parseFloat(place.center_point.lat), lng: parseFloat(place.center_point.lon) diff --git a/sanitizer/_ids.js b/sanitizer/_ids.js index 89590ac35..873e45e3f 100644 --- a/sanitizer/_ids.js +++ b/sanitizer/_ids.js @@ -1,8 +1,7 @@ -var _ = require('lodash'), - check = require('check-types'), - type_mapping = require('../helper/type_mapping'); - -var ID_DELIM = ':'; +const _ = require('lodash'); +const check = require('check-types'); +const type_mapping = require('../helper/type_mapping'); +const decode_gid = require('../helper/decode_gid'); // validate inputs, convert types and apply defaults id generally looks like // 'geonames:venue:4163334' (source:layer:id) so, all three are required @@ -18,23 +17,22 @@ var targetError = function(target, target_list) { }; function sanitizeId(rawId, messages) { - var parts = rawId.split(ID_DELIM); + const gid = decode_gid(rawId); - if ( parts.length < 3 ) { + if (!gid ) { messages.errors.push( formatError(rawId) ); return; } - var source = parts[0]; - var layer = parts[1]; - var id = parts.slice(2).join(ID_DELIM); + const { source, layer, id } = gid; // check if any parts of the gid are empty if (_.includes([source, layer, id], '')) { messages.errors.push( formatError(rawId) ); return; } - var valid_values = Object.keys(type_mapping.source_mapping); + + const valid_values = Object.keys(type_mapping.source_mapping); if (!_.includes(valid_values, source)) { messages.errors.push( targetError(source, valid_values) ); return; diff --git a/test/unit/fixture/dedupe_elasticsearch_results.js b/test/unit/fixture/dedupe_elasticsearch_results.js index 656730521..eb0c85e67 100644 --- a/test/unit/fixture/dedupe_elasticsearch_results.js +++ b/test/unit/fixture/dedupe_elasticsearch_results.js @@ -57,7 +57,6 @@ module.exports = [ 'address_parts': {}, 'alpha3': 'USA', 'source': 'openstreetmap', - 'source_id': 'node:357289197', 'category': [ 'education' ], @@ -118,7 +117,6 @@ module.exports = [ 'address_parts': {}, 'alpha3': 'USA', 'source': 'geonames', - 'source_id': '5219083', 'category': [ 'education' ], @@ -197,7 +195,6 @@ module.exports = [ 'address_parts': {}, 'alpha3': 'USA', 'source': 'geonames', - 'source_id': '5183465', 'category': [ 'entertainment', 'recreation' @@ -277,7 +274,6 @@ module.exports = [ 'address_parts': {}, 'alpha3': 'USA', 'source': 'openstreetmap', - 'source_id': 'node:368338500', 'category': [ 'education' ], @@ -339,7 +335,6 @@ module.exports = [ 'address_parts': {}, 'alpha3': 'USA', 'source': 'openstreetmap', - 'source_id': 'way:84969670', 'category': [ 'education' ], @@ -409,7 +404,6 @@ module.exports = [ 'address_parts': {}, 'alpha3': 'USA', 'source': 'geonames', - 'source_id': '5192545', 'category': [ 'education' ], @@ -488,7 +482,6 @@ module.exports = [ 'address_parts': {}, 'alpha3': 'USA', 'source': 'geonames', - 'source_id': '5198085', 'category': [ 'education' ], @@ -558,7 +551,6 @@ module.exports = [ 'address_parts': {}, 'alpha3': 'USA', 'source': 'geonames', - 'source_id': '5208101', 'category': [ 'education' ], @@ -638,7 +630,6 @@ module.exports = [ 'address_parts': {}, 'alpha3': 'USA', 'source': 'openstreetmap', - 'source_id': 'way:161088588', 'category': [ 'education' ], @@ -717,7 +708,6 @@ module.exports = [ 'address_parts': {}, 'alpha3': 'USA', 'source': 'geonames', - 'source_id': '5200263', 'category': [ 'education' ], @@ -788,7 +778,6 @@ module.exports = [ 'address_parts': {}, 'alpha3': 'USA', 'source': 'openstreetmap', - 'source_id': 'way:34212977', 'category': [ 'education' ], @@ -867,7 +856,6 @@ module.exports = [ 'address_parts': {}, 'alpha3': 'USA', 'source': 'openstreetmap', - 'source_id': 'node:357330916', 'category': [ 'education' ], @@ -937,7 +925,6 @@ module.exports = [ 'address_parts': {}, 'alpha3': 'USA', 'source': 'openstreetmap', - 'source_id': 'node:357330919', 'category': [ 'education' ], @@ -1007,7 +994,6 @@ module.exports = [ 'address_parts': {}, 'alpha3': 'USA', 'source': 'geonames', - 'source_id': '5197082', 'category': [ 'education' ], diff --git a/test/unit/helper/geojsonify.js b/test/unit/helper/geojsonify.js index 8e3ebe414..1f1523f0e 100644 --- a/test/unit/helper/geojsonify.js +++ b/test/unit/helper/geojsonify.js @@ -12,14 +12,15 @@ module.exports.tests.interface = function(test, common) { }); }; +// ensure null island coordinates work // ref: https://github.com/pelias/pelias/issues/84 module.exports.tests.earth = function(test, common) { test('earth', function(t) { var earth = [{ - '_id': '6295630', + '_type': 'doc', + '_id': 'whosonfirst:continent:6295630', 'source': 'whosonfirst', 'layer': 'continent', - 'source_id': '6295630', 'name': { 'default': 'Earth' }, @@ -41,9 +42,8 @@ module.exports.tests.all = (test, common) => { test('bounding_box should be calculated using points when avaiable', t => { const input = [ { - _id: 'id 1', + _id: 'source 1:layer 1:id 1', source: 'source 1', - source_id: 'id 1', layer: 'layer 1', name: { default: 'name 1', @@ -54,9 +54,8 @@ module.exports.tests.all = (test, common) => { } }, { - _id: 'id 2', + _id: 'source 2:layer 2:id 2', source: 'source 2', - source_id: 'id 2', layer: 'layer 2', name: { default: 'name 2', @@ -70,12 +69,12 @@ module.exports.tests.all = (test, common) => { const geojsonify = proxyquire('../../../helper/geojsonify', { './geojsonify_place_details': (params, source, dst) => { - if (source._id === 'id 1') { + if (source._id === 'source 1:layer 1:id 1') { return { property1: 'property 1', property2: 'property 2' }; - } else if (source._id === 'id 2') { + } else if (source._id === 'source 2:layer 2:id 2') { return { property3: 'property 3', property4: 'property 4' @@ -136,9 +135,8 @@ module.exports.tests.all = (test, common) => { test('bounding_box should be calculated using polygons when avaiable', t => { const input = [ { - _id: 'id 1', + _id: 'source 1:layer 1:id 1', source: 'source 1', - source_id: 'id 1', layer: 'layer 1', name: { default: 'name 1', @@ -155,9 +153,8 @@ module.exports.tests.all = (test, common) => { } }, { - _id: 'id 2', + _id: 'source 2:layer 2:id 2', source: 'source 2', - source_id: 'id 2', layer: 'layer 2', name: { default: 'name 2', @@ -177,12 +174,12 @@ module.exports.tests.all = (test, common) => { const geojsonify = proxyquire('../../../helper/geojsonify', { './geojsonify_place_details': (params, source, dst) => { - if (source._id === 'id 1') { + if (source._id === 'source 1:layer 1:id 1') { return { property1: 'property 1', property2: 'property 2' }; - } else if (source._id === 'id 2') { + } else if (source._id === 'source 2:layer 2:id 2') { return { property3: 'property 3', property4: 'property 4' @@ -245,7 +242,7 @@ module.exports.tests.all = (test, common) => { test('bounding_box should be calculated using polygons AND points when avaiable', t => { const input = [ { - _id: 'id 1', + _id: 'source 1:layer 1:id 1', source: 'source 1', source_id: 'id 1', layer: 'layer 1', @@ -258,9 +255,8 @@ module.exports.tests.all = (test, common) => { } }, { - _id: 'id 2', + _id: 'source 2:layer 2:id 2', source: 'source 2', - source_id: 'id 2', layer: 'layer 2', name: { default: 'name 2', @@ -280,12 +276,12 @@ module.exports.tests.all = (test, common) => { const geojsonify = proxyquire('../../../helper/geojsonify', { './geojsonify_place_details': (params, source, dst) => { - if (source._id === 'id 1') { + if (source._id === 'source 1:layer 1:id 1') { return { property1: 'property 1', property2: 'property 2' }; - } else if (source._id === 'id 2') { + } else if (source._id === 'source 2:layer 2:id 2') { return { property3: 'property 3', property4: 'property 4' @@ -354,9 +350,8 @@ module.exports.tests.non_optimal_conditions = (test, common) => { null, undefined, { - _id: 'id 1', + _id: 'source 1:layer 1:id 1', source: 'source 1', - source_id: 'id 1', layer: 'layer 1', name: { default: 'name 1', @@ -370,7 +365,7 @@ module.exports.tests.non_optimal_conditions = (test, common) => { const geojsonify = proxyquire('../../../helper/geojsonify', { './geojsonify_place_details': (params, source, dst) => { - if (source._id === 'id 1') { + if (source._id === 'source 1:layer 1:id 1') { return { property1: 'property 1', property2: 'property 2' @@ -417,18 +412,16 @@ module.exports.tests.non_optimal_conditions = (test, common) => { const input = [ { - _id: 'id 1', + _id: 'source 1:layer 1:id 1', source: 'source 1', - source_id: 'id 1', layer: 'layer 1', name: { default: 'name 1', } }, { - _id: 'id 2', + _id: 'source 2:layer 2:id 2', source: 'source 2', - source_id: 'id 2', layer: 'layer 2', name: { default: 'name 2', @@ -442,7 +435,7 @@ module.exports.tests.non_optimal_conditions = (test, common) => { const geojsonify = proxyquire('../../../helper/geojsonify', { './geojsonify_place_details': (params, source, dst) => { - if (source._id === 'id 2') { + if (source._id === 'source 2:layer 2:id 2') { return { property3: 'property 3', property4: 'property 4' @@ -489,9 +482,8 @@ module.exports.tests.non_optimal_conditions = (test, common) => { const input = [ { - _id: 'id 1', + _id: 'source 1:layer 1:id 1', source: 'source 1', - source_id: 'id 1', layer: 'layer 1', center_point: { lat: 12.121212, @@ -499,9 +491,8 @@ module.exports.tests.non_optimal_conditions = (test, common) => { } }, { - _id: 'id 2', + _id: 'source 2:layer 2:id 2', source: 'source 2', - source_id: 'id 2', layer: 'layer 2', name: {}, center_point: { @@ -510,9 +501,8 @@ module.exports.tests.non_optimal_conditions = (test, common) => { } }, { - _id: 'id 3', + _id: 'source 3:layer 3:id 3', source: 'source 3', - source_id: 'id 3', layer: 'layer 3', name: { default: 'name 3', @@ -526,17 +516,17 @@ module.exports.tests.non_optimal_conditions = (test, common) => { const geojsonify = proxyquire('../../../helper/geojsonify', { './geojsonify_place_details': (params, source, dst) => { - if (source._id === 'id 1') { + if (source._id === 'source 1:layer 1:id 1') { return { property1: 'property 1', property2: 'property 2' }; - } else if (source._id === 'id 2') { + } else if (source._id === 'source 2:layer 2:id 2') { return { property3: 'property 3', property4: 'property 4' }; - } else if (source._id === 'id 3') { + } else if (source._id === 'source 3:layer 3:id 3') { return { property5: 'property 5', property6: 'property 6' @@ -643,10 +633,9 @@ module.exports.tests.non_optimal_conditions = (test, common) => { module.exports.tests.nameAliases = function(test, common) { test('name aliases', function(t) { var aliases = [{ - '_id': '1', + '_id': 'example:example:1', 'source': 'example', 'layer': 'example', - 'source_id': '1', 'name': { 'default': ['Example1', 'Example2'] // note the array }, @@ -687,10 +676,9 @@ module.exports.tests.nameAliases = function(test, common) { module.exports.tests.addendum = function(test, common) { test('addendum: not set in source', function(t) { var example = [{ - '_id': '6295630', + '_id': 'whosonfirst:continent:6295630', 'source': 'whosonfirst', 'layer': 'continent', - 'source_id': '6295630', 'name': { 'default': 'Earth' }, @@ -704,12 +692,12 @@ module.exports.tests.addendum = function(test, common) { t.false(collection.features[0].properties.addendum); t.end(); }); + test('addendum: set in source', function(t) { var example = [{ - '_id': '6295630', + '_id': 'whosonfirst:continent:6295630', 'source': 'whosonfirst', 'layer': 'continent', - 'source_id': '6295630', 'name': { 'default': 'Earth' }, @@ -730,12 +718,12 @@ module.exports.tests.addendum = function(test, common) { }); t.end(); }); + test('addendum: partially corrupted', function(t) { var example = [{ - '_id': '6295630', + '_id': 'whosonfirst:continent:6295630', 'source': 'whosonfirst', 'layer': 'continent', - 'source_id': '6295630', 'name': { 'default': 'Earth' }, @@ -757,7 +745,7 @@ module.exports.tests.addendum = function(test, common) { }); test('addendum: all corrupted', function(t) { var example = [{ - '_id': '6295630', + '_id': 'whosonfirst:continent:6295630', 'source': 'whosonfirst', 'layer': 'continent', 'source_id': '6295630',