diff --git a/controller/place.js b/controller/place.js index 101430c5..e24be2fc 100644 --- a/controller/place.js +++ b/controller/place.js @@ -19,13 +19,8 @@ function setup( backend ){ /* * some gids aren't resolvable to a single type (ex: osmnode and osmway * both have source osm and layer venue), so expect an array of - * possible values. It's important to use `type` here instead of - * `_type`, as the former actually queries against the type, and thus - * can accept multiple match values. `_type`, on the other hand, - * simply changes the actual URL of the query sent to Elasticsearch to - * contain a type, which obviously can only take a single type. - */ - type: id.types, + * possible values. */ + _type: id.types, _id: id.id }; }); diff --git a/helper/type_mapping.js b/helper/type_mapping.js index 27fce81c..06a8ec2d 100644 --- a/helper/type_mapping.js +++ b/helper/type_mapping.js @@ -48,13 +48,13 @@ var SOURCE_TO_TYPE = { */ var LAYER_TO_TYPE = { 'venue': ['geoname','osmnode','osmway'], - 'address': ['osmaddress','openaddresses', 'geoname'], - 'country': ['admin0', 'geoname'], - 'region': ['admin1', 'geoname'], - 'county': ['admin2', 'geoname'], - 'locality': ['locality', 'geoname'], + 'address': ['osmaddress','openaddresses'], + 'country': ['admin0'], + 'region': ['admin1'], + 'county': ['admin2'], + 'locality': ['locality'], 'localadmin': ['local_admin'], - 'neighbourhood': ['neighborhood', 'geoname'] + 'neighbourhood': ['neighborhood'] }; var LAYER_ALIASES = { diff --git a/middleware/404.js b/middleware/404.js index e90ed3c3..85d81c70 100644 --- a/middleware/404.js +++ b/middleware/404.js @@ -1,7 +1,7 @@ // handle not found errors function middleware(req, res) { - res.header('Cache-Control','public,max-age=300'); // 5 minute cache + res.header('Cache-Control','public'); res.status(404).json({ error: 'not found: invalid path' }); } diff --git a/middleware/408.js b/middleware/408.js index ffbb6066..8e57eb28 100644 --- a/middleware/408.js +++ b/middleware/408.js @@ -1,7 +1,7 @@ // handle time out errors function middleware(err, req, res, next) { - res.header('Cache-Control','no-cache'); + res.header('Cache-Control','public'); var error = (err && err.message) ? err.message : err; if( res.statusCode === 408 || (error.toLowerCase().indexOf('request timeout') !== -1) ){ diff --git a/middleware/500.js b/middleware/500.js index cc3f8325..92acea60 100644 --- a/middleware/500.js +++ b/middleware/500.js @@ -3,7 +3,7 @@ var logger = require( 'pelias-logger' ).get( 'middleware-500' ); // handle application errors function middleware(err, req, res, next) { logger.error( 'Error: `%s`. Stack trace: `%s`.', err, err.stack ); - res.header('Cache-Control','no-cache'); + res.header('Cache-Control','public'); var error = (err && err.message) ? err.message : err; if( res.statusCode < 400 ){ res.status(500); } diff --git a/middleware/headers.js b/middleware/headers.js index 4b40dd21..a747be33 100644 --- a/middleware/headers.js +++ b/middleware/headers.js @@ -3,7 +3,7 @@ var pkg = require('../package'); function middleware(req, res, next){ res.header('Charset','utf8'); - res.header('Cache-Control','public,max-age=60'); + res.header('Cache-Control','public'); res.header('Server', 'Pelias/'+pkg.version); res.header('X-Powered-By', 'mapzen'); next(); diff --git a/package.json b/package.json index d949fdb3..abca960e 100644 --- a/package.json +++ b/package.json @@ -47,7 +47,6 @@ "iso3166-1": "^0.2.3", "lodash": "^3.10.1", "markdown": "0.5.0", - "microtime": "1.4.0", "morgan": "1.5.2", "pelias-config": "^1.0.1", "pelias-esclient": "0.0.25", diff --git a/query/defaults.js b/query/defaults.js index cf9422d0..83ddb4d9 100644 --- a/query/defaults.js +++ b/query/defaults.js @@ -39,35 +39,35 @@ module.exports = extend( false, peliasQuery.defaults, { 'address:housenumber:analyzer': 'standard', 'address:housenumber:field': 'address.number', - 'address:housenumber:boost': 1, + 'address:housenumber:boost': 2, 'address:street:analyzer': 'standard', 'address:street:field': 'address.street', - 'address:street:boost': 1, + 'address:street:boost': 5, 'address:postcode:analyzer': 'standard', 'address:postcode:field': 'address.zip', - 'address:postcode:boost': 1, + 'address:postcode:boost': 3, 'admin:alpha3:analyzer': 'standard', 'admin:alpha3:field': 'alpha3', - 'admin:alpha3:boost': 1, + 'admin:alpha3:boost': 5, 'admin:admin0:analyzer': 'peliasAdmin', 'admin:admin0:field': 'admin0', - 'admin:admin0:boost': 1, + 'admin:admin0:boost': 4, 'admin:admin1:analyzer': 'peliasAdmin', 'admin:admin1:field': 'admin1', - 'admin:admin1:boost': 1, + 'admin:admin1:boost': 3, 'admin:admin1_abbr:analyzer': 'peliasAdmin', 'admin:admin1_abbr:field': 'admin1_abbr', - 'admin:admin1_abbr:boost': 1, + 'admin:admin1_abbr:boost': 3, 'admin:admin2:analyzer': 'peliasAdmin', 'admin:admin2:field': 'admin2', - 'admin:admin2:boost': 1, + 'admin:admin2:boost': 2, 'admin:local_admin:analyzer': 'peliasAdmin', 'admin:local_admin:field': 'local_admin', diff --git a/sanitiser/_geo_common.js b/sanitiser/_geo_common.js index bc5a4bcc..0ca1184e 100644 --- a/sanitiser/_geo_common.js +++ b/sanitiser/_geo_common.js @@ -1,7 +1,8 @@ /** * helper sanitiser methods for geo parameters */ -var util = require('util'), +var groups = require('./_groups'), + util = require('util'), check = require('check-types'), _ = require('lodash'); @@ -14,37 +15,29 @@ var util = require('util'), * @param {bool} bbox_is_required */ function sanitize_rect( key_prefix, clean, raw, bbox_is_required ) { - - // the names we use to define the corners of the rect - var mandatoryProps = [ 'min_lat', 'max_lat', 'min_lon', 'max_lon' ]; - - // count up how many fields the user actually specified - var totalFieldsSpecified = 0; - mandatoryProps.forEach( function( prop ){ - if( raw.hasOwnProperty( key_prefix + '.' + prop ) ){ - totalFieldsSpecified++; - } + // calculate full property names from the key_prefix + var properties = [ 'min_lat', 'max_lat', 'min_lon', 'max_lon' ].map(function(prop) { + return key_prefix + '.' + prop; }); - // all fields specified - if( 4 === totalFieldsSpecified ) { - // reuse the coord sanitizer and set required:true so we get a fatal error if - // any one of the corners is not specified. - sanitize_coord( key_prefix + '.min_lat', clean, raw[ key_prefix + '.min_lat' ], true ); - sanitize_coord( key_prefix + '.max_lat', clean, raw[ key_prefix + '.max_lat' ], true ); - sanitize_coord( key_prefix + '.min_lon', clean, raw[ key_prefix + '.min_lon' ], true ); - sanitize_coord( key_prefix + '.max_lon', clean, raw[ key_prefix + '.max_lon' ], true ); - } - // fields only partially specified - else if( totalFieldsSpecified > 0 ){ - var format1 = 'missing rect param \'%s\' requires all of: \'%s\' to be present'; - throw new Error( util.format( format1, key_prefix, mandatoryProps.join('\',\'') ) ); - } - // fields required, eg. ( totalFieldsSpecified === 0 && bbox_is_required === true ) - else if( bbox_is_required ){ - var format2 = 'missing rect param \'%s\' requires all of: \'%s\' to be present'; - throw new Error( util.format( format2, key_prefix, mandatoryProps.join('\',\'') ) ); + // sanitize the rect property group, this throws an exception if + // the group is not complete + var bbox_present; + if (bbox_is_required) { + bbox_present = groups.required(raw, properties); + } else { + bbox_present = groups.optional(raw, properties); } + + // don't bother checking individual elements if bbox is not required + // and not present + if (!bbox_present) { return; } + + // check each property individually. now that it is known a bbox is present, + // all properties must exist, so pass the true flag for coord_is_required + properties.forEach(function(prop) { + sanitize_coord(prop, clean, raw[prop], true); + }); } /** @@ -56,43 +49,13 @@ function sanitize_rect( key_prefix, clean, raw, bbox_is_required ) { * @param {bool} circle_is_required */ function sanitize_circle( key_prefix, clean, raw, circle_is_required ) { - - // the names we use to define the centroid - var mandatoryProps = [ 'lat', 'lon' ]; - - // count up how many fields the user actually specified - var totalFieldsSpecified = 0; - mandatoryProps.forEach( function( prop ){ - if( raw.hasOwnProperty( key_prefix + '.' + prop ) ){ - totalFieldsSpecified++; - } - }); - - // all fields specified - if( 2 === totalFieldsSpecified ) { - // reuse the coord sanitizer and set required:true so we get a fatal error if - // any one of the coords is not specified. - sanitize_coord( key_prefix + '.lat', clean, raw[ key_prefix + '.lat' ], true ); - sanitize_coord( key_prefix + '.lon', clean, raw[ key_prefix + '.lon' ], true ); - - if( check.assigned( raw[ key_prefix + '.radius' ] ) ){ - sanitize_coord( key_prefix + '.radius', clean, raw[ key_prefix + '.radius' ], true ); - } - } - // fields only partially specified - else if( totalFieldsSpecified > 0 ){ - var format1 = 'missing circle param \'%s\' requires all of: \'%s\' to be present'; - throw new Error( util.format( format1, key_prefix, mandatoryProps.join('\',\'') ) ); - } - // radius was specified without lat or lon - else if( raw.hasOwnProperty( key_prefix + '.radius' ) ){ - var format2 = 'missing circle param \'%s\' requires all of: \'%s\' to be present'; - throw new Error( util.format( format2, key_prefix, mandatoryProps.join('\',\'') ) ); - } - // fields required, eg. ( totalFieldsSpecified === 0 && bbox_is_required === true ) - else if( circle_is_required ){ - var format3 = 'missing circle param \'%s\' requires all of: \'%s\' to be present'; - throw new Error( util.format( format3, key_prefix, mandatoryProps.join('\',\'') ) ); + // sanitize both a point and a radius if radius is present + // otherwise just sanittize the point + if( check.assigned( raw[ key_prefix + '.radius' ] ) ){ + sanitize_coord( key_prefix + '.radius', clean, raw[ key_prefix + '.radius' ], true ); + sanitize_point( key_prefix, clean, raw, true); + } else { + sanitize_point( key_prefix, clean, raw, circle_is_required); } } @@ -105,35 +68,29 @@ function sanitize_circle( key_prefix, clean, raw, circle_is_required ) { * @param {bool} point_is_required */ function sanitize_point( key_prefix, clean, raw, point_is_required ) { - - // the names we use to define the point - var mandatoryProps = [ 'lat', 'lon' ]; - - // count up how many fields the user actually specified - var totalFieldsSpecified = 0; - mandatoryProps.forEach( function( prop ){ - if( raw.hasOwnProperty( key_prefix + '.' + prop ) ){ - totalFieldsSpecified++; - } + // calculate full property names from the key_prefix + var properties = [ 'lat', 'lon'].map(function(prop) { + return key_prefix + '.' + prop; }); - // all fields specified - if( 2 === totalFieldsSpecified ) { - // reuse the coord sanitizer and set required:true so we get a fatal error if - // any one of the coords is not specified. - sanitize_coord( key_prefix + '.lat', clean, raw[ key_prefix + '.lat' ], true ); - sanitize_coord( key_prefix + '.lon', clean, raw[ key_prefix + '.lon' ], true ); - } - // fields only partially specified - else if( totalFieldsSpecified > 0 ){ - var format1 = 'missing point param \'%s\' requires all of: \'%s\' to be present'; - throw new Error( util.format( format1, key_prefix, mandatoryProps.join('\',\'') ) ); - } - // fields required, eg. ( totalFieldsSpecified === 0 && bbox_is_required === true ) - else if( point_is_required ){ - var format2 = 'missing point param \'%s\' requires all of: \'%s\' to be present'; - throw new Error( util.format( format2, key_prefix, mandatoryProps.join('\',\'') ) ); + // sanitize the rect property group, this throws an exception if + // the group is not complete + var point_present; + if (point_is_required) { + point_present = groups.required(raw, properties); + } else { + point_present = groups.optional(raw, properties); } + + // don't bother checking individual elements if point is not required + // and not present + if (!point_present) { return; } + + // check each property individually. now that it is known a bbox is present, + // all properties must exist, so pass the true flag for coord_is_required + properties.forEach(function(prop) { + sanitize_coord(prop, clean, raw[prop], true); + }); } /** diff --git a/sanitiser/_groups.js b/sanitiser/_groups.js new file mode 100644 index 00000000..a06b839c --- /dev/null +++ b/sanitiser/_groups.js @@ -0,0 +1,57 @@ +var _ = require('lodash'); + +/* + * Specify an object and array of keys to check. + * An error will be thrown if at least one, but not all of them are specified + * + * @param {object} - the object + * @param {array} - keys to check + * + * returns true if all are present, false if none are present, throws an exception otherwise + */ +function optional_group(object, keys) { + var contained_in_object = _.contains.bind(null, Object.keys(object)); + + if (keys.every(contained_in_object)) { + return true; + } else if (!keys.some(contained_in_object)) { + return false; + } else { + throw new Error(error_message(keys)); + } +} + +/* + * Specify an object and array of keys to check. + * An error will be thrown if any of the keys are missing from the object + */ +function required_group(object, keys) { + var contained_in_object = _.contains.bind(null, Object.keys(object)); + + if (keys.every(contained_in_object)) { + return true; + } else { + throw new Error(error_message(keys)); + } +} + +/* + * Create a friendly error message from a list of keys + */ +function error_message(keys) { + var start = 'parameters '; + + var listStart = keys.slice(0, -1).join(', '); + var listEnd = ' and ' + keys[keys.length - 1]; + + var adjective = (keys.length > 2) ? 'all' : 'both'; + + var end = ' must ' + adjective + ' be specified'; + + return start + listStart + listEnd + end; +} + +module.exports = { + optional: optional_group, + required: required_group +}; diff --git a/sanitiser/_ids.js b/sanitiser/_ids.js index 0b4d91c7..73f9d68a 100644 --- a/sanitiser/_ids.js +++ b/sanitiser/_ids.js @@ -57,13 +57,6 @@ function sanitize( raw, clean ){ // error & warning messages var messages = { errors: [], warnings: [] }; - // 'raw.ids' can be an array if ids is specified multiple times - // see https://github.com/pelias/api/issues/272 - if (check.array( raw.ids )) { - messages.errors.push( '`ids` parameter specified multiple times.' ); - return messages; - } - if (!check.unemptyString( raw.ids )) { messages.errors.push( lengthError); return messages; diff --git a/service/mget.js b/service/mget.js index 936ec8bd..a878e8cb 100644 --- a/service/mget.js +++ b/service/mget.js @@ -12,7 +12,6 @@ **/ var peliasLogger = require( 'pelias-logger' ).get( 'service/mget' ); -var microtime = require( 'microtime' ); function service( backend, query, cb ){ @@ -22,11 +21,9 @@ function service( backend, query, cb ){ docs: query } }; - - var startTime = microtime.nowDouble(); + // query new backend backend().client.mget( cmd, function( err, data ){ - peliasLogger.verbose( 'time elasticsearch query took:', microtime.nowDouble() - startTime ); // handle backend errors if( err ){ return cb( err ); } @@ -39,7 +36,7 @@ function service( backend, query, cb ){ // remove docs not actually found return doc.found; - + }).map( function( doc ){ // map metadata in to _source so we diff --git a/service/search.js b/service/search.js index 1e77f69f..c5aad5a9 100644 --- a/service/search.js +++ b/service/search.js @@ -6,14 +6,11 @@ **/ var peliasLogger = require( 'pelias-logger' ).get( 'service/search' ); -var microtime = require( 'microtime' ); function service( backend, cmd, cb ){ - - var startTime = microtime.nowDouble(); + // query new backend backend().client.search( cmd, function( err, data ){ - peliasLogger.verbose( 'time elasticsearch query took:', microtime.nowDouble() - startTime ); // handle backend errors if( err ){ return cb( err ); } diff --git a/test/ciao/404.coffee b/test/ciao/404.coffee index 7a22618d..9f6a9c3c 100644 --- a/test/ciao/404.coffee +++ b/test/ciao/404.coffee @@ -9,7 +9,7 @@ response.statusCode.should.be.equal 404 response.should.have.header 'Content-Type','application/json; charset=utf-8' #? cache-control header correctly set -response.should.have.header 'Cache-Control','public,max-age=300' +response.should.have.header 'Cache-Control','public' #? should respond in json with server info should.exist json diff --git a/test/ciao/index.coffee b/test/ciao/index.coffee index 8dd9da90..aac96fd9 100644 --- a/test/ciao/index.coffee +++ b/test/ciao/index.coffee @@ -12,7 +12,7 @@ response.should.have.header 'Content-Type','text/html; charset=utf-8' response.should.have.header 'Charset','utf8' #? cache-control header correctly set -response.should.have.header 'Cache-Control','public,max-age=60' +response.should.have.header 'Cache-Control','public' #? server header correctly set response.should.have.header 'Server' diff --git a/test/ciao/reverse/layers_multiple.coffee b/test/ciao/reverse/layers_multiple.coffee index 2fd0e1dc..f5f2d7dd 100644 --- a/test/ciao/reverse/layers_multiple.coffee +++ b/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","geoname","admin1"] -json.geocoding.query['type'].should.eql ["admin0","geoname","admin1"] +json.geocoding.query.types['from_layers'].should.eql ["admin0","admin1"] +json.geocoding.query['type'].should.eql ["admin0","admin1"] diff --git a/test/ciao/reverse/layers_single.coffee b/test/ciao/reverse/layers_single.coffee index e6642fb9..4b65c331 100644 --- a/test/ciao/reverse/layers_single.coffee +++ b/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","geoname"] -json.geocoding.query['type'].should.eql ["admin0","geoname"] +json.geocoding.query.types['from_layers'].should.eql ["admin0"] +json.geocoding.query['type'].should.eql ["admin0"] diff --git a/test/ciao/reverse/sources_layers_invalid_combo.coffee b/test/ciao/reverse/sources_layers_invalid_combo.coffee index dc74ba4a..b15fcc88 100644 --- a/test/ciao/reverse/sources_layers_invalid_combo.coffee +++ b/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","geoname"] +json.geocoding.query.types['from_layers'].should.eql ["osmaddress","openaddresses"] json.geocoding.query.types['from_sources'].should.eql ["admin0","admin1","admin2","neighborhood","locality","local_admin"] should.not.exist json.geocoding.query['type'] diff --git a/test/ciao/reverse/sources_layers_valid_combo.coffee b/test/ciao/reverse/sources_layers_valid_combo.coffee index cc593768..a24ea221 100644 --- a/test/ciao/reverse/sources_layers_valid_combo.coffee +++ b/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","geoname"] +json.geocoding.query.types['from_layers'].should.eql ["osmaddress","openaddresses"] json.geocoding.query['type'].should.eql ["openaddresses"] diff --git a/test/ciao/search/layers_alias_address.coffee b/test/ciao/search/layers_alias_address.coffee index 14074bf9..9bf32a04 100644 --- a/test/ciao/search/layers_alias_address.coffee +++ b/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","geoname"] -json.geocoding.query['type'].should.eql ["osmaddress","openaddresses","geoname"] +json.geocoding.query.types['from_layers'].should.eql ["osmaddress","openaddresses"] +json.geocoding.query['type'].should.eql ["osmaddress","openaddresses"] diff --git a/test/ciao/search/layers_multiple.coffee b/test/ciao/search/layers_multiple.coffee index 20e67eba..5f714d4d 100644 --- a/test/ciao/search/layers_multiple.coffee +++ b/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","geoname","admin1"] -json.geocoding.query['type'].should.eql ["admin0","geoname","admin1"] +json.geocoding.query.types['from_layers'].should.eql ["admin0","admin1"] +json.geocoding.query['type'].should.eql ["admin0","admin1"] diff --git a/test/ciao/search/layers_single.coffee b/test/ciao/search/layers_single.coffee index 9dc79a00..227f7bc9 100644 --- a/test/ciao/search/layers_single.coffee +++ b/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","geoname"] -json.geocoding.query['type'].should.eql ["admin0","geoname"] +json.geocoding.query.types['from_layers'].should.eql ["admin0"] +json.geocoding.query['type'].should.eql ["admin0"] diff --git a/test/ciao/search/sources_layers_invalid_combo.coffee b/test/ciao/search/sources_layers_invalid_combo.coffee index 6d9702ed..fb01cdf7 100644 --- a/test/ciao/search/sources_layers_invalid_combo.coffee +++ b/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","geoname"] +json.geocoding.query.types['from_layers'].should.eql ["osmaddress","openaddresses"] json.geocoding.query.types['from_sources'].should.eql ["admin0","admin1","admin2","neighborhood","locality","local_admin"] should.not.exist json.geocoding.query['type'] diff --git a/test/ciao/search/sources_layers_valid_combo.coffee b/test/ciao/search/sources_layers_valid_combo.coffee index d20cc6ca..eeb2a663 100644 --- a/test/ciao/search/sources_layers_valid_combo.coffee +++ b/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","geoname"] +json.geocoding.query.types['from_layers'].should.eql ["osmaddress","openaddresses"] json.geocoding.query['type'].should.eql ["openaddresses"] diff --git a/test/unit/controller/place.js b/test/unit/controller/place.js index 20455e3a..1d38f5d5 100644 --- a/test/unit/controller/place.js +++ b/test/unit/controller/place.js @@ -41,7 +41,7 @@ module.exports.tests.functional_success = function(test, common) { test('functional success', function(t) { var backend = mockBackend( 'client/mget/ok/1', function( cmd ){ - t.deepEqual(cmd, { body: { docs: [ { _id: 123, _index: 'pelias', type: [ 'a' ] } ] } }, 'correct backend command'); + t.deepEqual(cmd, { body: { docs: [ { _id: 123, _index: 'pelias', _type: [ 'a' ] } ] } }, 'correct backend command'); }); var controller = setup( backend ); var res = { @@ -70,7 +70,7 @@ module.exports.tests.functional_success = function(test, common) { module.exports.tests.functional_failure = function(test, common) { test('functional failure', function(t) { var backend = mockBackend( 'client/mget/fail/1', function( cmd ){ - t.deepEqual(cmd, { body: { docs: [ { _id: 123, _index: 'pelias', type: [ 'b' ] } ] } }, 'correct backend command'); + t.deepEqual(cmd, { body: { docs: [ { _id: 123, _index: 'pelias', _type: [ 'b' ] } ] } }, 'correct backend command'); }); var controller = setup( backend ); var req = { clean: { ids: [ {'id' : 123, types: [ 'b' ] } ] }, errors: [], warnings: [] }; diff --git a/test/unit/run.js b/test/unit/run.js index 7da0c978..7e0fe272 100644 --- a/test/unit/run.js +++ b/test/unit/run.js @@ -6,34 +6,35 @@ var tests = [ require('./controller/index'), require('./controller/place'), require('./controller/search'), - require('./service/mget'), - require('./service/search'), - require('./sanitiser/_ids'), - require('./sanitiser/_flag_bool'), - require('./sanitiser/autocomplete'), - require('./sanitiser/_sources'), - require('./sanitiser/_boundary_country'), - require('./sanitiser/search'), - require('./sanitiser/_layers'), - require('./sanitiser/reverse'), - require('./sanitiser/place'), - require('./query/search'), - require('./query/autocomplete'), - require('./query/reverse'), - require('./query/defaults'), - require('./helper/query_parser'), require('./helper/geojsonify'), - require('./helper/labelSchema'), require('./helper/labelGenerator'), - require('./helper/types'), + require('./helper/labelSchema'), + require('./helper/query_parser'), require('./helper/type_mapping'), - require('./sanitiser/_geo_common'), - require('./middleware/distance'), - require('./middleware/confidenceScoreReverse'), + require('./helper/types'), require('./middleware/confidenceScore'), - require('./sanitiser/_size'), - require('./sanitiser/_single_scalar_parameters'), + require('./middleware/confidenceScoreReverse'), + require('./middleware/distance'), + require('./query/autocomplete'), + require('./query/defaults'), + require('./query/reverse'), + require('./query/search'), + require('./sanitiser/_boundary_country'), + require('./sanitiser/_flag_bool'), + require('./sanitiser/_geo_common'), require('./sanitiser/_geo_reverse'), + require('./sanitiser/_groups'), + require('./sanitiser/_ids'), + require('./sanitiser/_layers'), + require('./sanitiser/_single_scalar_parameters'), + require('./sanitiser/_size'), + require('./sanitiser/_sources'), + require('./sanitiser/autocomplete'), + require('./sanitiser/place'), + require('./sanitiser/reverse'), + require('./sanitiser/search'), + require('./service/mget'), + require('./service/search'), ]; tests.map(function(t) { diff --git a/test/unit/sanitiser/_groups.js b/test/unit/sanitiser/_groups.js new file mode 100644 index 00000000..63b86ea7 --- /dev/null +++ b/test/unit/sanitiser/_groups.js @@ -0,0 +1,75 @@ +var groups = require('../../../sanitiser/_groups'); + +module.exports.tests = {}; + +module.exports.tests.optional_group = function(test, common) { + test('optional group none present ', function(t) { + var object = {}; + t.doesNotThrow(function() { + var present = groups.optional(object, [ 'a', 'b' ]); + t.equal(present, false, 'group reported not present'); + }); + t.end(); + }); + + test('optional group all present ', function(t) { + var object = { 'a': 5, 'b': 9 }; + t.doesNotThrow(function() { + var present = groups.optional(object, [ 'a', 'b' ]); + t.equal(present, true, 'group reported present'); + }); + t.end(); + }); + + test('optional group some present ', function(t) { + var object = { 'b': 9 }; + t.throws(function() { + groups.optional(object, [ 'a', 'b' ]); + }, new RegExp('parameters a and b must both be specified')); + t.end(); + }); + + test('optional group some present (larger group) ', function(t) { + var object = { 'b': 9, 'd': 5 }; + t.throws(function() { + groups.optional(object, [ 'a', 'b', 'c', 'd', 'e' ]); + }, new RegExp('parameters a, b, c, d and e must all be specified')); + t.end(); + }); +}; + +module.exports.tests.required_group = function(test, common) { + test('required group none present ', function(t) { + var object = {}; + t.throws(function() { + groups.required(object, [ 'a', 'b' ]); + }, new RegExp('parameters a and b must both be specified')); + t.end(); + }); + + test('required group all present ', function(t) { + var object = { 'a': 5, 'b': 9 }; + t.doesNotThrow(function() { + groups.required(object, [ 'a', 'b' ]); + }); + t.end(); + }); + + test('required group some present ', function(t) { + var object = { 'b': 9 }; + t.throws(function() { + groups.required(object, [ 'a', 'b' ]); + }, new RegExp('parameters a and b must both be specified')); + t.end(); + }); +}; + +module.exports.all = function (tape, common) { + function test(name, testFunction) { + return tape('SANTIZE _groups ' + name, testFunction); + } + + for( var testCase in module.exports.tests ){ + module.exports.tests[testCase](test, common); + } +}; diff --git a/test/unit/sanitiser/_ids.js b/test/unit/sanitiser/_ids.js index 9438f3ee..176c981a 100644 --- a/test/unit/sanitiser/_ids.js +++ b/test/unit/sanitiser/_ids.js @@ -126,20 +126,6 @@ module.exports.tests.valid_ids = function(test, common) { }); }; -module.exports.tests.array_of_ids = function(test, common) { - // see https://github.com/pelias/api/issues/272 - test('array of ids sent by queryparser', function(t) { - var raw = { ids: ['geoname:2', 'oswmay:4'] }; - var clean = {}; - - var messages = sanitize( raw, clean); - - t.deepEqual( messages.errors, ['`ids` parameter specified multiple times.'], 'error sent' ); - t.deepEqual( clean.ids, undefined, 'response is empty due to error' ); - t.end(); - }); -}; - module.exports.tests.multiple_ids = function(test, common) { test('multiple ids', function(t) { var raw = { ids: 'geonames:venue:1,osm:venue:2' }; diff --git a/test/unit/sanitiser/_layers.js b/test/unit/sanitiser/_layers.js index 3264f434..519d95d9 100644 --- a/test/unit/sanitiser/_layers.js +++ b/test/unit/sanitiser/_layers.js @@ -43,7 +43,7 @@ module.exports.tests.sanitize_layers = function(test, common) { t.end(); }); test('address (alias) layer', function(t) { - var address_layers = ['osmaddress','openaddresses','geoname']; + var address_layers = ['osmaddress','openaddresses']; var raw = { layers: 'address' }; var clean = {}; @@ -76,7 +76,7 @@ module.exports.tests.sanitize_layers = function(test, common) { t.end(); }); test('address alias layer plus regular layers', function(t) { - var address_layers = ['osmaddress','openaddresses','geoname']; + var address_layers = ['osmaddress','openaddresses']; var reg_layers = ['admin0', 'locality']; var raw = { layers: 'address,country,locality' }; diff --git a/test/unit/sanitiser/search.js b/test/unit/sanitiser/search.js index f50a46a2..59bc87f3 100644 --- a/test/unit/sanitiser/search.js +++ b/test/unit/sanitiser/search.js @@ -167,7 +167,7 @@ module.exports.tests.sanitize_optional_geo = function(test, common) { var req = { query: { text: 'test', 'focus.point.lon': 0 } }; sanitize(req, function(){ var expected_lon = 0; - t.equal(req.errors[0], 'missing point param \'focus.point\' requires all of: \'lat\',\'lon\' to be present'); + t.equal(req.errors[0], 'parameters focus.point.lat and focus.point.lon must both be specified'); t.equal(req.clean['focus.point.lat'], undefined); t.equal(req.clean['focus.point.lon'], undefined); }); @@ -177,7 +177,7 @@ module.exports.tests.sanitize_optional_geo = function(test, common) { var req = { query: { text: 'test', 'focus.point.lat': 0 } }; sanitize(req, function(){ var expected_lat = 0; - t.equal(req.errors[0], 'missing point param \'focus.point\' requires all of: \'lat\',\'lon\' to be present'); + t.equal(req.errors[0], 'parameters focus.point.lat and focus.point.lon must both be specified'); t.equal(req.clean['focus.point.lat'], undefined); t.equal(req.clean['focus.point.lon'], undefined); });