Switch to source:layer:id ids format in /place

sanitiser/_ids.js

@@ -1,39 +1,56 @@
 var _ = require('lodash'),
     check = require('check-types'),
-    type_mapping = require('../helper/type_mapping'),
-    types = type_mapping.types_list;
+    type_mapping = require('../helper/type_mapping');
 
 var ID_DELIM = ':';
 
-// validate inputs, convert types and apply defaults
-// id generally looks like 'geoname:4163334' (type:id)
-// so, both type and id are required fields.
+// validate inputs, convert types and apply defaults id generally looks like
+// 'geonames:venue:4163334' (source:layer:id) so, all three are required
 
 var lengthError = 'invalid param \'ids\': length must be >0';
 
 var formatError = function(input) {
-  return 'id `' + input + ' is invalid: must be of the format type:id for ex: \'geoname:4163334\'';
+  return 'id `' + input + ' is invalid: must be of the format source:layer:id for ex: \'geonames:venue:4163334\'';
+};
+
+var targetError = function(target, target_list) {
+  return target + ' is invalid. It must be one of these values - [' + target_list.join(', ') + ']';
 };
 
 function sanitizeId(rawId, messages) {
-  var param_index = rawId.indexOf(ID_DELIM);
-  var type = rawId.substring(0, param_index );
-  var id   = rawId.substring(param_index + 1);
+  var parts = rawId.split(ID_DELIM);
+
+  if ( parts.length < 3 ) {
+    messages.errors.push( formatError(rawId) );
+    return;
+  }
 
-  // check id format
-  if(!check.contains(rawId, ID_DELIM) || !check.unemptyString( id ) || !check.unemptyString( type )) {
+  var source = parts[0];
+  var layer = parts[1];
+  var id = parts.slice(2).join(ID_DELIM);
+
+  // check if any parts of the gid are empty
+  if (_.contains([source, layer, id], '')) {
     messages.errors.push( formatError(rawId) );
+    return;
   }
-  // type text must be one of the types
-  else if( !_.contains( types, type ) ){
-    messages.errors.push( type + ' is invalid. It must be one of these values - [' + types.join(', ') + ']' );
+
+  if (!_.contains(type_mapping.sources, source)) {
+    messages.errors.push( targetError(source, type_mapping.sources) );
+    return;
   }
-  else {
-    return {
-      id: id,
-      types: [type]
-    };
+
+  if (!_.contains(type_mapping.layers, layer)) {
+    messages.errors.push( targetError(layer, type_mapping.layers) );
+    return;
   }
+
+  var types = type_mapping.source_and_layer_to_type(source, layer);
+
+  return {
+    id: id,
+    types: types
+  };
 }
 
 function sanitize( raw, clean ){

test/ciao/place/basic_place.coffee

@@ -1,6 +1,6 @@
 
 #> basic place
-path: '/v1/place?ids=geoname:1'
+path: '/v1/place?ids=geonames:venue:1'
 
 #? 200 ok
 response.statusCode.should.be.equal 200
@@ -29,5 +29,5 @@ should.not.exist json.geocoding.errors
 should.not.exist json.geocoding.warnings
 
 #? inputs
-json.geocoding.query['ids'].should.eql [{ id: '1', type: 'geoname' }]
+json.geocoding.query['ids'].should.eql [{ id: '1', types: [ 'geoname' ] }]
 should.not.exist json.geocoding.query['size']

test/ciao/reverse/layers_invalid.coffee

@@ -24,7 +24,7 @@ json.features.should.be.instanceof Array
 
 #? expected errors
 should.exist json.geocoding.errors
-json.geocoding.errors.should.eql [ '\'notlayer\' is an invalid layers parameter. Valid options: venue,address,country,region,county,locality,localadmin,neighbourhood,coarse' ]
+json.geocoding.errors.should.eql [ '\'notlayer\' is an invalid layers parameter. Valid options: coarse,venue,address,country,region,county,locality,localadmin,neighbourhood' ]
 
 #? expected warnings
 should.not.exist json.geocoding.warnings
@@ -32,4 +32,4 @@ should.not.exist json.geocoding.warnings
 #? inputs
 json.geocoding.query['size'].should.eql 10
 should.not.exist json.geocoding.query['types']
-should.not.exist json.geocoding.query['type']
+should.not.exist json.geocoding.query['type']

test/ciao/reverse/layers_mix_invalid_valid.coffee

@@ -24,7 +24,7 @@ json.features.should.be.instanceof Array
 
 #? expected errors
 should.exist json.geocoding.errors
-json.geocoding.errors.should.eql [ '\'notlayer\' is an invalid layers parameter. Valid options: venue,address,country,region,county,locality,localadmin,neighbourhood,coarse' ]
+json.geocoding.errors.should.eql [ '\'notlayer\' is an invalid layers parameter. Valid options: coarse,venue,address,country,region,county,locality,localadmin,neighbourhood' ]
 
 #? expected warnings
 should.not.exist json.geocoding.warnings
@@ -32,4 +32,4 @@ should.not.exist json.geocoding.warnings
 #? inputs
 json.geocoding.query['size'].should.eql 10
 should.not.exist json.geocoding.query['types']
-should.not.exist json.geocoding.query['type']
+should.not.exist json.geocoding.query['type']

test/ciao/reverse/layers_multiple.coffee

@@ -30,5 +30,5 @@ should.not.exist json.geocoding.warnings
 
 #? inputs
 json.geocoding.query['size'].should.eql 10
-json.geocoding.query.types['from_layers'].should.eql ["admin0","admin1"]
-json.geocoding.query['type'].should.eql ["admin0","admin1"]
+json.geocoding.query.types['from_layers'].should.eql ["admin0","geoname","admin1"]
+json.geocoding.query['type'].should.eql ["admin0","geoname","admin1"]

test/ciao/reverse/layers_single.coffee

@@ -30,5 +30,5 @@ should.not.exist json.geocoding.warnings
 
 #? inputs
 json.geocoding.query['size'].should.eql 10
-json.geocoding.query.types['from_layers'].should.eql ["admin0"]
-json.geocoding.query['type'].should.eql ["admin0"]
+json.geocoding.query.types['from_layers'].should.eql ["admin0","geoname"]
+json.geocoding.query['type'].should.eql ["admin0","geoname"]

test/ciao/reverse/sources_layers_invalid_combo.coffee

@@ -31,6 +31,6 @@ should.not.exist json.geocoding.warnings
 
 #? inputs
 json.geocoding.query['size'].should.eql 10
-json.geocoding.query.types['from_layers'].should.eql ["osmaddress","openaddresses"]
+json.geocoding.query.types['from_layers'].should.eql ["osmaddress","openaddresses","geoname"]
 json.geocoding.query.types['from_sources'].should.eql ["admin0","admin1","admin2","neighborhood","locality","local_admin"]
-should.not.exist json.geocoding.query['type']
+should.not.exist json.geocoding.query['type']

test/ciao/reverse/sources_layers_valid_combo.coffee

@@ -30,5 +30,5 @@ should.not.exist json.geocoding.warnings
 
 #? inputs
 json.geocoding.query['size'].should.eql 10
-json.geocoding.query.types['from_layers'].should.eql ["osmaddress","openaddresses"]
-json.geocoding.query['type'].should.eql ["openaddresses"]
+json.geocoding.query.types['from_layers'].should.eql ["osmaddress","openaddresses","geoname"]
+json.geocoding.query['type'].should.eql ["openaddresses"]

test/ciao/search/layers_alias_address.coffee

@@ -31,5 +31,5 @@ should.not.exist json.geocoding.warnings
 #? inputs
 json.geocoding.query['text'].should.eql 'a'
 json.geocoding.query['size'].should.eql 10
-json.geocoding.query.types['from_layers'].should.eql ["osmaddress","openaddresses"]
-json.geocoding.query['type'].should.eql ["osmaddress","openaddresses"]
+json.geocoding.query.types['from_layers'].should.eql ["osmaddress","openaddresses","geoname"]
+json.geocoding.query['type'].should.eql ["osmaddress","openaddresses","geoname"]

test/ciao/search/layers_invalid.coffee

@@ -24,7 +24,7 @@ json.features.should.be.instanceof Array
 
 #? expected errors
 should.exist json.geocoding.errors
-json.geocoding.errors.should.eql [ '\'notlayer\' is an invalid layers parameter. Valid options: venue,address,country,region,county,locality,localadmin,neighbourhood,coarse' ]
+json.geocoding.errors.should.eql [ '\'notlayer\' is an invalid layers parameter. Valid options: coarse,venue,address,country,region,county,locality,localadmin,neighbourhood' ]
 
 #? expected warnings
 should.not.exist json.geocoding.warnings
@@ -33,4 +33,4 @@ should.not.exist json.geocoding.warnings
 json.geocoding.query['text'].should.eql 'a'
 json.geocoding.query['size'].should.eql 10
 should.not.exist json.geocoding.query['types']
-should.not.exist json.geocoding.query['type']
+should.not.exist json.geocoding.query['type']

test/ciao/search/layers_mix_invalid_valid.coffee

@@ -24,7 +24,7 @@ json.features.should.be.instanceof Array
 
 #? expected errors
 should.exist json.geocoding.errors
-json.geocoding.errors.should.eql [ '\'notlayer\' is an invalid layers parameter. Valid options: venue,address,country,region,county,locality,localadmin,neighbourhood,coarse' ]
+json.geocoding.errors.should.eql [ '\'notlayer\' is an invalid layers parameter. Valid options: coarse,venue,address,country,region,county,locality,localadmin,neighbourhood' ]
 
 #? expected warnings
 should.not.exist json.geocoding.warnings
@@ -33,4 +33,4 @@ should.not.exist json.geocoding.warnings
 json.geocoding.query['text'].should.eql 'a'
 json.geocoding.query['size'].should.eql 10
 should.not.exist json.geocoding.query['types']
-should.not.exist json.geocoding.query['type']
+should.not.exist json.geocoding.query['type']

test/ciao/search/layers_multiple.coffee

@@ -31,5 +31,5 @@ should.not.exist json.geocoding.warnings
 #? inputs
 json.geocoding.query['text'].should.eql 'a'
 json.geocoding.query['size'].should.eql 10
-json.geocoding.query.types['from_layers'].should.eql ["admin0","admin1"]
-json.geocoding.query['type'].should.eql ["admin0","admin1"]
+json.geocoding.query.types['from_layers'].should.eql ["admin0","geoname","admin1"]
+json.geocoding.query['type'].should.eql ["admin0","geoname","admin1"]

test/ciao/search/layers_single.coffee

@@ -31,5 +31,5 @@ should.not.exist json.geocoding.warnings
 #? inputs
 json.geocoding.query['text'].should.eql 'a'
 json.geocoding.query['size'].should.eql 10
-json.geocoding.query.types['from_layers'].should.eql ["admin0"]
-json.geocoding.query['type'].should.eql ["admin0"]
+json.geocoding.query.types['from_layers'].should.eql ["admin0","geoname"]
+json.geocoding.query['type'].should.eql ["admin0","geoname"]

test/ciao/search/sources_layers_invalid_combo.coffee

@@ -32,6 +32,6 @@ should.not.exist json.geocoding.warnings
 #? inputs
 json.geocoding.query['text'].should.eql 'a'
 json.geocoding.query['size'].should.eql 10
-json.geocoding.query.types['from_layers'].should.eql ["osmaddress","openaddresses"]
+json.geocoding.query.types['from_layers'].should.eql ["osmaddress","openaddresses","geoname"]
 json.geocoding.query.types['from_sources'].should.eql ["admin0","admin1","admin2","neighborhood","locality","local_admin"]
-should.not.exist json.geocoding.query['type']
+should.not.exist json.geocoding.query['type']

test/ciao/search/sources_layers_valid_combo.coffee

@@ -31,5 +31,5 @@ should.not.exist json.geocoding.warnings
 #? inputs
 json.geocoding.query['text'].should.eql 'a'
 json.geocoding.query['size'].should.eql 10
-json.geocoding.query.types['from_layers'].should.eql ["osmaddress","openaddresses"]
-json.geocoding.query['type'].should.eql ["openaddresses"]
+json.geocoding.query.types['from_layers'].should.eql ["osmaddress","openaddresses","geoname"]
+json.geocoding.query['type'].should.eql ["openaddresses"]

test/unit/sanitiser/_ids.js

@@ -8,13 +8,9 @@ var inputs = {
 };
 
 var formatError = function(input) {
-  return 'id `' + input + ' is invalid: must be of the format type:id for ex: \'geoname:4163334\'';
+  return 'id `' + input + ' is invalid: must be of the format source:layer:id for ex: \'geonames:venue:4163334\'';
 };
 var lengthError = 'invalid param \'ids\': length must be >0';
-var defaultMissingTypeError = function(input) {
-    var type = input.split(delimiter)[0];
-    return type + ' is invalid. It must be one of these values - [' + type_mapping.types_list.join(', ') + ']';
-  };
 
 module.exports.tests = {};
 
@@ -74,50 +70,58 @@ module.exports.tests.invalid_ids = function(test, common) {
     t.end();
   });
 
-  test('invalid id: type name invalid', function(t) {
-    var raw = { ids: 'gibberish:23' };
+  test('invalid id: source name invalid', function(t) {
+    var raw = { ids: 'invalidsource:venue:23' };
     var clean = {};
+    var expected_error = 'invalidsource is invalid. It must be one of these values - [' + type_mapping.sources.join(', ') + ']';
 
     var messages = sanitize(raw, clean);
 
-    t.equal(messages.errors[0], defaultMissingTypeError('gibberish:23'), 'format error returned');
+    t.equal(messages.errors[0], expected_error, 'format error returned');
+    t.equal(clean.ids, undefined, 'ids unset in clean object');
+    t.end();
+  });
+
+  test('invalid id: old style 2 part id', function(t) {
+    var raw = { ids: 'geonames:23' };
+    var clean = {};
+
+    var messages = sanitize(raw, clean);
+
+    t.equal(messages.errors[0], formatError('geonames:23'), 'format error returned');
     t.equal(clean.ids, undefined, 'ids unset in clean object');
     t.end();
   });
 };
 
 module.exports.tests.valid_ids = function(test, common) {
-  test('ids: valid input', function(t) {
-    inputs.valid.forEach( function( input ){
-      var input_parts = input.split(delimiter);
-      var expected_clean = { ids: [ { id: input_parts[1], types: [ input_parts[0] ]} ]};
-      var raw = {  ids: input };
-      var clean = {};
-
-      var messages = sanitize( raw, clean );
-
-      t.deepEqual( messages.errors, [], 'no error (' + input + ')' );
-      t.deepEqual( clean, expected_clean, 'clean set correctly (' + input + ')');
-    });
+  test('ids: valid input (openaddresses)', function(t) {
+    var raw = { ids: 'openaddresses:address:20' };
+    var clean = {};
+    var expected_ids = [{
+      id: '20',
+      types: [ 'openaddresses' ]
+    }];
+
+    var messages = sanitize( raw, clean );
+
+    t.deepEqual( messages.errors, [], ' no errors');
+    t.deepEqual( clean.ids, expected_ids, 'single type value returned');
     t.end();
   });
 
-  test('ids: valid input with multiple values' , function(t) {
-    var raw = { ids: inputs.valid.join(',') };
+  test('ids: valid input (osm)', function(t) {
+    var raw = { ids: 'osm:venue:500' };
     var clean = {};
-    var expected_clean={
-      ids: [],
-    };
-    // construct the expected id and type for each valid input
-    inputs.valid.forEach( function( input ){
-      var input_parts = input.split(delimiter);
-      expected_clean.ids.push({ id: input_parts[1], types: [ input_parts[0] ]});
-    });
+    var expected_ids = [{
+      id: '500',
+      types: [ 'osmnode', 'osmway' ]
+    }];
 
     var messages = sanitize( raw, clean );
 
-    t.deepEqual( messages.errors, [], 'no errors' );
-    t.deepEqual( clean, expected_clean, 'clean set correctly' );
+    t.deepEqual( messages.errors, [], ' no errors');
+    t.deepEqual( clean.ids, expected_ids, 'osm could be two types, but that\'s ok');
     t.end();
   });
 };
@@ -137,10 +141,10 @@ module.exports.tests.array_of_ids = function(test, common) {
 };
 
 module.exports.tests.multiple_ids = function(test, common) {
-  test('duplicate ids', function(t) {
-    var expected_clean = { ids: [ { id: '1', types: [ 'geoname' ] }, { id: '2', types: [ 'osmnode' ] } ] };
-    var raw = { ids: 'geoname:1,osmnode:2' };
+  test('multiple ids', function(t) {
+    var raw = { ids: 'geonames:venue:1,osm:venue:2' };
     var clean = {};
+    var expected_clean = { ids: [ { id: '1', types: [ 'geoname' ] }, { id: '2', types: [ 'osmnode', 'osmway' ] } ] };
 
     var messages = sanitize( raw, clean);
 
@@ -153,8 +157,8 @@ module.exports.tests.multiple_ids = function(test, common) {
 
 module.exports.tests.de_dupe = function(test, common) {
   test('duplicate ids', function(t) {
-    var expected_clean = { ids: [ { id: '1', types: [ 'geoname' ] }, { id: '2', types: [ 'osmnode' ] } ] };
-    var raw = { ids: 'geoname:1,osmnode:2,geoname:1' };
+    var expected_clean = { ids: [ { id: '1', types: [ 'geoname' ] }, { id: '2', types: [ 'osmnode', 'osmway' ] } ] };
+    var raw = { ids: 'geonames:venue:1,osm:venue:2,geonames:venue:1' };
     var clean = {};
 
     var messages = sanitize( raw, clean );

test/unit/sanitiser/place.js

@@ -31,7 +31,7 @@ module.exports.tests.sanitize_private = function(test, common) {
   var invalid_values = [null, -1, 123, NaN, 'abc'];
   invalid_values.forEach(function(value) {
     test('invalid private param ' + value, function(t) {
-      var req = { query: { ids:'geoname:123', 'private': value } };
+      var req = { query: { ids:'geonames:venue:123', 'private': value } };
       sanitize(req, function(){
         t.deepEqual( req.errors, [], 'no errors' );
         t.deepEqual( req.warnings, [], 'no warnings' );
@@ -44,7 +44,7 @@ module.exports.tests.sanitize_private = function(test, common) {
   var valid_values = ['true', true, 1];
   valid_values.forEach(function(value) {
     test('valid private param ' + value, function(t) {
-      var req = { query: { ids:'geoname:123', 'private': value } };
+      var req = { query: { ids:'geonames:venue:123', 'private': value } };
       sanitize(req, function(){
         t.deepEqual( req.errors, [], 'no errors' );
         t.deepEqual( req.warnings, [], 'no warnings' );
@@ -57,7 +57,7 @@ module.exports.tests.sanitize_private = function(test, common) {
   var valid_false_values = ['false', false, 0];
   valid_false_values.forEach(function(value) {
     test('test setting false explicitly ' + value, function(t) {
-      var req = { query: { ids:'geoname:123', 'private': value } };
+      var req = { query: { ids:'geonames:venue:123', 'private': value } };
       sanitize(req, function(){
         t.deepEqual( req.errors, [], 'no errors' );
         t.deepEqual( req.warnings, [], 'no warnings' );
@@ -68,7 +68,7 @@ module.exports.tests.sanitize_private = function(test, common) {
   });
 
   test('test default behavior', function(t) {
-    var req = { query: { ids:'geoname:123' } };
+    var req = { query: { ids:'geonames:venue:123' } };
     sanitize(req, function(){
       t.deepEqual( req.errors, [], 'no errors' );
       t.deepEqual( req.warnings, [], 'no warnings' );
@@ -91,7 +91,7 @@ module.exports.tests.invalid_params = function(test, common) {
 
 module.exports.tests.middleware_success = function(test, common) {
   test('middleware success', function(t) {
-    var req = { query: { ids: 'geoname:123' }};
+    var req = { query: { ids: 'geonames:venue:123' }};
     var next = function(){
       t.deepEqual( req.errors, [], 'no errors' );
       t.deepEqual( req.warnings, [], 'no warnings' );