From 8f1c8f4d890e7bd76a6897b12cbc75f6452c6ee0 Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Tue, 13 Oct 2015 15:51:00 -0400 Subject: [PATCH 1/4] Add helpful comments in various query parser locations --- helper/query_parser.js | 4 ++++ helper/types.js | 4 +++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/helper/query_parser.js b/helper/query_parser.js index cb5dc7a7..a561aef9 100644 --- a/helper/query_parser.js +++ b/helper/query_parser.js @@ -8,6 +8,10 @@ var logger = require('pelias-logger').get('api'); module.exports = {}; +/* + * For performance, and to prefer POI and admin records, express a preference + * to only search coarse layers on very short text inputs. + */ module.exports.get_layers = function get_layers(query) { if (query.length <= 3 ) { // no address parsing required diff --git a/helper/types.js b/helper/types.js index 610d1aba..bea2db6f 100644 --- a/helper/types.js +++ b/helper/types.js @@ -2,12 +2,14 @@ var type_mapping = require( '../helper/type_mapping' ); var _ = require('lodash'); /** - * Combine all types and determine the unique subset + * Different parts of the code express "preferences" for which Elasticsearch types are going to be searched + * This method decides how to combine all the preferences. * * @param {Array} clean_types * @returns {Array} */ module.exports = function calculate_types(clean_types) { + //Check that at least one preference of types is defined if (!clean_types || !(clean_types.from_layers || clean_types.from_sources || clean_types.from_address_parser)) { throw new Error('clean_types should not be null or undefined'); } From 59b70f7c7eb22b91829f01e0141a83090ed229d3 Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Tue, 13 Oct 2015 15:55:02 -0400 Subject: [PATCH 2/4] Rename helper/query_parser to helper/text_parser --- helper/{query_parser.js => text_parser.js} | 0 sanitiser/_text.js | 6 +++--- test/unit/helper/{query_parser.js => text_parser.js} | 2 +- test/unit/query/autocomplete.js | 2 +- test/unit/query/search.js | 2 +- test/unit/run.js | 2 +- test/unit/sanitiser/search.js | 2 +- 7 files changed, 8 insertions(+), 8 deletions(-) rename helper/{query_parser.js => text_parser.js} (100%) rename test/unit/helper/{query_parser.js => text_parser.js} (98%) diff --git a/helper/query_parser.js b/helper/text_parser.js similarity index 100% rename from helper/query_parser.js rename to helper/text_parser.js diff --git a/sanitiser/_text.js b/sanitiser/_text.js index d0cc0986..0aa5c26c 100644 --- a/sanitiser/_text.js +++ b/sanitiser/_text.js @@ -1,5 +1,5 @@ var check = require('check-types'), - query_parser = require('../helper/query_parser'); + text_parser = require('../helper/text_parser'); // validate texts, convert types and apply defaults function sanitize( raw, clean ){ @@ -19,14 +19,14 @@ function sanitize( raw, clean ){ clean.text = raw.text; // parse text with query parser - var parsed_text = query_parser.get_parsed_address(clean.text); + var parsed_text = text_parser.get_parsed_address(clean.text); if (check.assigned(parsed_text)) { clean.parsed_text = parsed_text; } // try to set layers from query parser results clean.types = clean.layers || {}; - clean.types.from_address_parsing = query_parser.get_layers(clean.text); + clean.types.from_address_parsing = text_parser.get_layers(clean.text); } return messages; diff --git a/test/unit/helper/query_parser.js b/test/unit/helper/text_parser.js similarity index 98% rename from test/unit/helper/query_parser.js rename to test/unit/helper/text_parser.js index e7771ca4..a4ce64e3 100644 --- a/test/unit/helper/query_parser.js +++ b/test/unit/helper/text_parser.js @@ -1,4 +1,4 @@ -var parser = require('../../../helper/query_parser'); +var parser = require('../../../helper/text_parser'); var type_mapping = require('../../../helper/type_mapping'); var layers_map = type_mapping.layer_with_aliases_to_type; diff --git a/test/unit/query/autocomplete.js b/test/unit/query/autocomplete.js index ecdf9553..c3f620c0 100644 --- a/test/unit/query/autocomplete.js +++ b/test/unit/query/autocomplete.js @@ -1,6 +1,6 @@ var generate = require('../../../query/autocomplete'); -var parser = require('../../../helper/query_parser'); +var parser = require('../../../helper/text_parser'); module.exports.tests = {}; diff --git a/test/unit/query/search.js b/test/unit/query/search.js index 3a11e054..15527222 100644 --- a/test/unit/query/search.js +++ b/test/unit/query/search.js @@ -1,5 +1,5 @@ var generate = require('../../../query/search'); -var parser = require('../../../helper/query_parser'); +var parser = require('../../../helper/text_parser'); module.exports.tests = {}; diff --git a/test/unit/run.js b/test/unit/run.js index 7e0fe272..fb8b1728 100644 --- a/test/unit/run.js +++ b/test/unit/run.js @@ -9,7 +9,7 @@ var tests = [ require('./helper/geojsonify'), require('./helper/labelGenerator'), require('./helper/labelSchema'), - require('./helper/query_parser'), + require('./helper/text_parser'), require('./helper/type_mapping'), require('./helper/types'), require('./middleware/confidenceScore'), diff --git a/test/unit/sanitiser/search.js b/test/unit/sanitiser/search.js index 4f4e2880..b00160dd 100644 --- a/test/unit/sanitiser/search.js +++ b/test/unit/sanitiser/search.js @@ -1,6 +1,6 @@ var extend = require('extend'), search = require('../../../sanitiser/search'), - parser = require('../../../helper/query_parser'), + parser = require('../../../helper/text_parser'), sanitize = search.sanitize, middleware = search.middleware, defaultError = 'invalid param \'text\': text length, must be >0'; From b188a2046c876e124d8f6fda4fc7ce16b1a04a8c Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Tue, 13 Oct 2015 16:03:00 -0400 Subject: [PATCH 3/4] Fix mismatching name of types preference parameter The parameter in clean.types being set by `helpers/text_parser.js` was "from_address_parsing", but the code in `helper/types.js` was expecting "from_address_parser". This commit makes both use "from_address_parser" and adds a test. --- sanitiser/_text.js | 2 +- test/unit/run.js | 1 + test/unit/sanitiser/_text.js | 32 ++++++++++++++++++++++++++++++++ 3 files changed, 34 insertions(+), 1 deletion(-) create mode 100644 test/unit/sanitiser/_text.js diff --git a/sanitiser/_text.js b/sanitiser/_text.js index 0aa5c26c..e8b055f0 100644 --- a/sanitiser/_text.js +++ b/sanitiser/_text.js @@ -26,7 +26,7 @@ function sanitize( raw, clean ){ // try to set layers from query parser results clean.types = clean.layers || {}; - clean.types.from_address_parsing = text_parser.get_layers(clean.text); + clean.types.from_address_parser = text_parser.get_layers(clean.text); } return messages; diff --git a/test/unit/run.js b/test/unit/run.js index fb8b1728..b79f9aca 100644 --- a/test/unit/run.js +++ b/test/unit/run.js @@ -29,6 +29,7 @@ var tests = [ require('./sanitiser/_single_scalar_parameters'), require('./sanitiser/_size'), require('./sanitiser/_sources'), + require('./sanitiser/_text'), require('./sanitiser/autocomplete'), require('./sanitiser/place'), require('./sanitiser/reverse'), diff --git a/test/unit/sanitiser/_text.js b/test/unit/sanitiser/_text.js new file mode 100644 index 00000000..3ccfbbb2 --- /dev/null +++ b/test/unit/sanitiser/_text.js @@ -0,0 +1,32 @@ +var sanitiser = require('../../../sanitiser/_text'); +var type_mapping = require('../../../helper/type_mapping'); + +module.exports.tests = {}; + +module.exports.tests.text_parser = function(test, common) { + test('short input text has admin layers set ', function(t) { + var raw = { + text: 'emp' //start of empire state building + }; + var clean = { + }; + + var messages = sanitiser(raw, clean); + + t.deepEquals(messages.errors, [], 'no errors'); + t.deepEquals(messages.warnings, [], 'no warnings'); + t.equal(clean.types.from_address_parser, type_mapping.layer_with_aliases_to_type.coarse, 'coarse layers preferred'); + + t.end(); + }); +}; + +module.exports.all = function (tape, common) { + function test(name, testFunction) { + return tape('SANITISER _text: ' + name, testFunction); + } + + for( var testCase in module.exports.tests ){ + module.exports.tests[testCase](test, common); + } +}; From db895c05b71650971a98540a62908239b263312b Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Tue, 13 Oct 2015 16:08:05 -0400 Subject: [PATCH 4/4] Rename `from_address_parser` key to `from_text_parser` --- helper/types.js | 6 +++--- sanitiser/_text.js | 2 +- test/unit/helper/types.js | 4 ++-- test/unit/sanitiser/_text.js | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/helper/types.js b/helper/types.js index bea2db6f..d9dec59d 100644 --- a/helper/types.js +++ b/helper/types.js @@ -10,7 +10,7 @@ var _ = require('lodash'); */ module.exports = function calculate_types(clean_types) { //Check that at least one preference of types is defined - if (!clean_types || !(clean_types.from_layers || clean_types.from_sources || clean_types.from_address_parser)) { + if (!clean_types || !(clean_types.from_layers || clean_types.from_sources || clean_types.from_text_parser)) { throw new Error('clean_types should not be null or undefined'); } @@ -35,8 +35,8 @@ module.exports = function calculate_types(clean_types) { * Type restrictions requested by the address parser should only be used * if both the source and layers parameters are empty, so do this last */ - if (clean_types.from_address_parser) { - return clean_types.from_address_parser; + if (clean_types.from_text_parser) { + return clean_types.from_text_parser; } throw new Error('no types specified'); diff --git a/sanitiser/_text.js b/sanitiser/_text.js index e8b055f0..824c7a87 100644 --- a/sanitiser/_text.js +++ b/sanitiser/_text.js @@ -26,7 +26,7 @@ function sanitize( raw, clean ){ // try to set layers from query parser results clean.types = clean.layers || {}; - clean.types.from_address_parser = text_parser.get_layers(clean.text); + clean.types.from_text_parser = text_parser.get_layers(clean.text); } return messages; diff --git a/test/unit/helper/types.js b/test/unit/helper/types.js index 4638ff12..92848d46 100644 --- a/test/unit/helper/types.js +++ b/test/unit/helper/types.js @@ -33,7 +33,7 @@ module.exports.tests.no_cleaned_types = function(test, common) { module.exports.tests.address_parser = function(test, common) { test('address parser specifies only admin layers', function(t) { var cleaned_types = { - from_address_parser: ['admin0'] // simplified return value from address parser + from_text_parser: ['admin0'] // simplified return value from address parser }; var actual = types(cleaned_types); var expected = ['admin0']; // simplified expected value for all admin layers @@ -58,7 +58,7 @@ module.exports.tests.layers_parameter_and_address_parser = function(test, common test('layers parameter and address parser present', function(t) { var cleaned_types = { from_layers: ['geoname'], - from_address_parser: ['admin0'] // simplified return value from address parse + from_text_parser: ['admin0'] // simplified return value from address parse }; var actual = types(cleaned_types); var expected = ['geoname']; diff --git a/test/unit/sanitiser/_text.js b/test/unit/sanitiser/_text.js index 3ccfbbb2..8bc0710f 100644 --- a/test/unit/sanitiser/_text.js +++ b/test/unit/sanitiser/_text.js @@ -15,7 +15,7 @@ module.exports.tests.text_parser = function(test, common) { t.deepEquals(messages.errors, [], 'no errors'); t.deepEquals(messages.warnings, [], 'no warnings'); - t.equal(clean.types.from_address_parser, type_mapping.layer_with_aliases_to_type.coarse, 'coarse layers preferred'); + t.equal(clean.types.from_text_parser, type_mapping.layer_with_aliases_to_type.coarse, 'coarse layers preferred'); t.end(); });