From 743d8efa54dac6e745382b5164762dc64393e6c5 Mon Sep 17 00:00:00 2001 From: Diana Shkolnikov Date: Thu, 13 Aug 2015 12:00:14 -0400 Subject: [PATCH] Remove proxyquire from adminField test Add comments and refactor a bit more for clarity in query/search.js --- helper/adminFields.js | 24 ++++++++--- query/search.js | 76 +++++++++++++++++++++++++++------ test/unit/helper/adminFields.js | 72 +++++++++++++++---------------- 3 files changed, 118 insertions(+), 54 deletions(-) diff --git a/helper/adminFields.js b/helper/adminFields.js index aa96c203..de1dd009 100644 --- a/helper/adminFields.js +++ b/helper/adminFields.js @@ -1,5 +1,5 @@ -var schema = require('pelias-schema'); -var logger = require( 'pelias-logger' ).get( 'api' ); +var peliasSchema = require('pelias-schema'); +var peliasLogger = require( 'pelias-logger' ).get( 'api' ); var ADMIN_FIELDS = [ 'admin0', @@ -11,11 +11,24 @@ var ADMIN_FIELDS = [ 'neighborhood' ]; -function getAvailableAdminFields() { +/** + * Get all admin fields that were expected and also found in schema + * + * @param {Object} [schema] optional: for testing only + * @param {Array} [expectedFields] optional: for testing only + * @param {Object} [logger] optional: for testing only + * @returns {Array.} + */ +function getAvailableAdminFields(schema, expectedFields, logger) { + + schema = schema || peliasSchema; + expectedFields = expectedFields || ADMIN_FIELDS; + logger = logger || peliasLogger; + var actualFields = Object.keys(schema.mappings._default_.properties); // check if expected fields are actually in current schema - var available = ADMIN_FIELDS.filter(function (field) { + var available = expectedFields.filter(function (field) { return (actualFields.indexOf(field) !== -1); }); @@ -26,5 +39,4 @@ function getAvailableAdminFields() { return available; } -module.exports.availableFields = getAvailableAdminFields(); -module.exports.expectedFields = ADMIN_FIELDS; \ No newline at end of file +module.exports = getAvailableAdminFields; \ No newline at end of file diff --git a/query/search.js b/query/search.js index 41a4bff6..f11b472a 100644 --- a/query/search.js +++ b/query/search.js @@ -1,7 +1,7 @@ var queries = require('geopipes-elasticsearch-backend').queries, sort = require('../query/sort'), - adminFields = require('../helper/adminFields').availableFields, + adminFields = require('../helper/adminFields')(), addressWeights = require('../helper/address_weights'); @@ -60,8 +60,16 @@ function generate( params ){ return query; } +/** + * Traverse the parsed input object, containing all the address parts detected in query string. + * Add matches to query for each identifiable component. + * + * @param {Object} query + * @param {string} defaultInput + * @param {Object} parsedInput + */ function addParsedMatch(query, defaultInput, parsedInput) { - query.query.filtered.query.bool.should = []; + query.query.filtered.query.bool.should = query.query.filtered.query.bool.should || []; // copy expected admin fields so we can remove them as we parse the address var unmatchedAdminFields = adminFields.slice(); @@ -74,7 +82,7 @@ function addParsedMatch(query, defaultInput, parsedInput) { // city // admin2, locality, local_admin, neighborhood - addMatch(query, unmatchedAdminFields, 'admin2', parsedInput.admin2, addressWeights.admin2); + addMatch(query, unmatchedAdminFields, 'admin2', parsedInput.city, addressWeights.admin2); // state // admin1, admin1_abbr @@ -84,20 +92,54 @@ function addParsedMatch(query, defaultInput, parsedInput) { // admin0, alpha3 addMatch(query, unmatchedAdminFields, 'alpha3', parsedInput.country, addressWeights.alpha3); - var inputRegions = parsedInput.regions ? parsedInput.regions.join(' ') : undefined; - // if no address was identified and input suggests some admin info in it - if (unmatchedAdminFields.length > 0 && inputRegions !== defaultInput) { + addUnmatchedAdminFieldsToQuery(query, unmatchedAdminFields, parsedInput, defaultInput); +} + +/** + * Check for additional admin fields in the parsed input, and if any was found + * combine into single string and match against all unmatched admin fields. + * + * @param {Object} query + * @param {Array} unmatchedAdminFields + * @param {Object} parsedInput + * @param {string} defaultInput + */ +function addUnmatchedAdminFieldsToQuery(query, unmatchedAdminFields, parsedInput, defaultInput) { + if (unmatchedAdminFields.length === 0 ) { + return; + } + + var leftovers = []; + + if (parsedInput.admin_parts) { + leftovers.push(parsedInput.admin_parts); + } + else if (parsedInput.regions) { + leftovers.push(parsedInput.regions); + } + + if (leftovers.length === 0) { + return; + } + + // if there are additional regions/admin_parts found + if (leftovers !== defaultInput) { unmatchedAdminFields.forEach(function (key) { - if (parsedInput.admin_parts) { - addMatch(query, [], key, parsedInput.admin_parts); - } - else { - addMatch(query, [], key, inputRegions); - } + // combine all the leftover parts into one string + addMatch(query, [], key, leftovers.join(' ')); }); } } +/** + * Add key:value match to query. Apply boost if specified. + * + * @param {Object} query + * @param {Array} unmatched + * @param {string} key + * @param {string|number|undefined} value + * @param {number|undefined} [boost] optional + */ function addMatch(query, unmatched, key, value, boost) { // jshint ignore:line if (typeof value === 'undefined') { return; @@ -117,6 +159,16 @@ function addMatch(query, unmatched, key, value, boost) { // jshint ignore:line query.query.filtered.query.bool.should.push({ 'match': match }); + removeFromUnmatched(unmatched, key); +} + +/** + * If key is found in unmatched list, remove it from the array + * + * @param {Array} unmatched + * @param {string} key + */ +function removeFromUnmatched(unmatched, key) { var index = unmatched.indexOf(key); if (index !== -1) { unmatched.splice(index, 1); diff --git a/test/unit/helper/adminFields.js b/test/unit/helper/adminFields.js index 9d72b934..8780b267 100644 --- a/test/unit/helper/adminFields.js +++ b/test/unit/helper/adminFields.js @@ -1,18 +1,12 @@ -var proxyquire = require('proxyquire'); +var adminFields = require('../../../helper/adminFields'); module.exports.tests = {}; module.exports.tests.interface = function(test, common) { test('validate fields', function(t) { - var adminFields = require('../../../helper/adminFields').availableFields; - t.assert(adminFields instanceof Array, 'adminFields is an array'); - t.assert(adminFields.length > 0, 'adminFields array is not empty'); - t.end(); - }); - test('validate fields', function(t) { - var adminFields = require('../../../helper/adminFields').expectedFields; - t.assert(adminFields instanceof Array, 'adminFields is an array'); - t.assert(adminFields.length > 0, 'adminFields array is not empty'); + t.assert(adminFields instanceof Function, 'adminFields is a function'); + t.assert(adminFields() instanceof Array, 'adminFields() returns an array'); + t.assert(adminFields().length > 0, 'adminFields array is not empty'); t.end(); }); }; @@ -20,54 +14,60 @@ module.exports.tests.interface = function(test, common) { module.exports.tests.lookupExistance = function(test, common) { test('all expected fields in schema', function(t) { - var expectedFields = require('../../../helper/adminFields').expectedFields; - var schemaMock = { mappings: { _default_: { properties: {} } } }; + var expectedFields = [ + 'one', + 'two', + 'three', + 'four' + ]; + var schema = { mappings: { _default_: { properties: {} } } }; // inject all expected fields into schema mock expectedFields.forEach(function (field) { - schemaMock.mappings._default_.properties[field] = {}; + schema.mappings._default_.properties[field] = {}; }); - var adminFields = proxyquire('../../../helper/adminFields', {'pelias-schema': schemaMock}); + var res = adminFields(schema, expectedFields); - t.deepEquals(adminFields.availableFields, adminFields.expectedFields, 'all expected fields are returned'); + t.deepEquals(res, expectedFields, 'all expected fields are returned'); t.end(); }); test('some expected fields in schema', function(t) { - var expectedFields = require('../../../helper/adminFields').expectedFields.slice(0, 3); - var schemaMock = { mappings: { _default_: { properties: {} } } }; - - // inject all expected fields into schema mock - expectedFields.forEach(function (field) { - schemaMock.mappings._default_.properties[field] = {}; + var expectedFields = [ + 'one', + 'two', + 'three', + 'four' + ]; + var schema = { mappings: { _default_: { properties: {} } } }; + + // inject only some of the expected fields into schema mock + expectedFields.slice(0, 3).forEach(function (field) { + schema.mappings._default_.properties[field] = {}; }); - var adminFields = proxyquire('../../../helper/adminFields', {'pelias-schema': schemaMock}); + var res = adminFields(schema, expectedFields); - t.deepEquals(adminFields.availableFields, expectedFields, 'only matching expected fields are returned'); + t.deepEquals(res, expectedFields.slice(0, 3), 'only matching expected fields are returned'); t.end(); }); test('no expected fields in schema', function(t) { - var schemaMock = { mappings: { _default_: { properties: { foo: {} } } } }; + var schema = { mappings: { _default_: { properties: { foo: {} } } } }; - var loggerMock = { get: function (name) { - t.equal(name, 'api'); - return { - error: function () {} - }; - }}; + var logErrorCalled = false; + var logger = { + error: function () { + logErrorCalled = true; + }}; - var adminFields = proxyquire('../../../helper/adminFields', - { - 'pelias-schema': schemaMock, - 'pelias-logger': loggerMock - }); + var res = adminFields(schema, undefined, logger); - t.deepEquals([], adminFields.availableFields, 'no admin fields found'); + t.deepEquals(res, [], 'no admin fields found'); + t.assert(logErrorCalled, 'log error called'); t.end(); }); };