Remove proxyquire from adminField test

Add comments and refactor a bit more for clarity in query/search.js

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.<string>}
+ */
+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;
+module.exports = getAvailableAdminFields;

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);

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();
   });
 };