updated changeLanguage to injection model

middleware/changeLanguage.js

@@ -1,6 +1,5 @@
 
 var logger = require( 'pelias-logger' ).get( 'api' );
-var service = require('../service/language');
 const _ = require('lodash');
 
 /**
@@ -28,84 +27,34 @@ example response from language web service:
 }
 **/
 
-function setup() {
+function setup(service, should_execute) {
-  var transport = service.findById();
+  return function controller(req, res, next) {
-  var middleware = function(req, res, next) {
+    if (!should_execute(req, res)) {
-
-    // no-op, request did not require a language change
-    if( !isLanguageChangeRequired( req, res ) ){
       return next();
     }
 
-    // collect a list of parent ids to fetch translations for
+    service(req, res, (err, translations) => {
-    var ids = extractIds( res );
+      // if there's an error, log it and bail
-
+      if (err) {
-    // perform language lookup for all relevant ids
+        logger.info(`[middleware:language][error]`);
-    var timer = (new Date()).getTime();
+        logger.error(err);
-    transport.query( ids, function( err, translations ){
+        return next();
-
-      // update documents using a translation map
-      if( err ){
-        logger.error( '[language] [error]', err );
-      } else {
-        updateDocs( req, res, translations );
       }
 
-      logger.info( '[language] [took]', (new Date()).getTime() - timer, 'ms' );
+      // otherwise, update all the docs with translations
+      updateDocs(req, res, _.defaultTo(translations, []));
       next();
-    });
-  };
-
-  middleware.transport = transport;
-  return middleware;
-}
-
-// collect a list of parent ids to fetch translations for
-function extractIds( res ){
 
-  // store ids in an object in order to avoid duplicates
+    });
-  var ids = {};
-
-  // convenience function for adding a new id to the object
-  function addId(id) {
-    ids[id] = true;
-  }
-
-  // extract all parent ids from documents
-  res.data.forEach( function( doc ){
-
-    // skip invalid records
-    if( !doc || !doc.parent ){ return; }
-
-    // iterate over doc.parent.* attributes
-    for( var attr in doc.parent ){
-
-      // match only attributes ending with '_id'
-      var match = attr.match(/_id$/);
-      if( !match ){ continue; }
-
-      // skip invalid/empty arrays
-      if( !Array.isArray( doc.parent[attr] ) || !doc.parent[attr].length ){
-        continue;
-      }
 
-      // add each id as a key in the ids object
+  };
-      doc.parent[attr].forEach( addId );
-    }
-  });
 
-  // return a deduplicated array of ids
-  return Object.keys( ids );
 }
 
 // update documents using a translation map
 function updateDocs( req, res, translations ){
-
-  // sanity check arguments
-  if( !req || !res || !res.data || !translations ){ return; }
-
   // this is the target language we will be translating to
-  var requestLanguage = req.language.iso6393;
+  var requestLanguage = req.clean.lang.iso6393;
 
   // iterate over response documents
   res.data.forEach( function( doc, p ){
@@ -136,17 +85,14 @@ function updateDocs( req, res, translations ){
         if( !id ){ continue; }
 
         // id not found in translation service response
-        if( !translations.hasOwnProperty( id ) ){
+        if( !_.has(translations, id)){
-          logger.error( '[language] [error]', 'failed to find translations for', id );
+          logger.debug( `[language] [debug] failed to find translations for ${id}` );
           continue;
         }
 
-        // skip invalid records
-        if( !translations[id].hasOwnProperty( 'names' ) ){ continue; }
-
         // requested language is not available
         if (_.isEmpty(_.get(translations[id].names, requestLanguage, [] ))) {
-          logger.debug( '[language] [debug]', 'missing translation', requestLanguage, id );
+          logger.debug( `[language] [debug] missing translation ${requestLanguage} ${id}` );
           continue;
         }
 

routes/v1.js

@@ -80,6 +80,7 @@ const hasRequestCategories = require('../controller/predicates/has_request_param
 const isOnlyNonAdminLayers = require('../controller/predicates/is_only_non_admin_layers');
 // this can probably be more generalized
 const isRequestSourcesOnlyWhosOnFirst = require('../controller/predicates/is_request_sources_only_whosonfirst');
+const hasRequestParameter = require('../controller/predicates/has_request_parameter');
 
 // shorthand for standard early-exit conditions
 const hasResponseDataOrRequestErrors = any(hasResponseData, hasRequestErrors);
@@ -93,6 +94,7 @@ const hasNumberButNotStreet = all(
 const serviceWrapper = require('pelias-microservice-wrapper').service;
 const PlaceHolder = require('../service/configurations/PlaceHolder');
 const PointInPolygon = require('../service/configurations/PointInPolygon');
+const Language = require('../service/configurations/Language');
 
 /**
  * Append routes to app
@@ -111,6 +113,10 @@ function addRoutes(app, peliasConfig) {
   const placeholderService = serviceWrapper(placeholderConfiguration);
   const isPlaceholderServiceEnabled = _.constant(placeholderConfiguration.isEnabled());
 
+  const changeLanguageConfiguration = new Language(_.defaultTo(peliasConfig.api.services.language, {}));
+  const changeLanguageService = serviceWrapper(changeLanguageConfiguration);
+  const isChangeLanguageEnabled = _.constant(changeLanguageConfiguration.isEnabled());
+
   // fallback to coarse reverse when regular reverse didn't return anything
   const coarseReverseShouldExecute = all(
     isPipServiceEnabled, not(hasRequestErrors), not(hasResponseData)
@@ -195,6 +201,13 @@ function addRoutes(app, peliasConfig) {
     isAddressItParse
   );
 
+  const changeLanguageShouldExecute = all(
+    hasResponseData,
+    not(hasRequestErrors),
+    isChangeLanguageEnabled,
+    hasRequestParameter('lang')
+  );
+
   // execute under the following conditions:
   // - there are no errors or data
   // - request is not coarse OR pip service is disabled
@@ -246,7 +259,7 @@ function addRoutes(app, peliasConfig) {
       postProc.renamePlacenames(),
       postProc.parseBoundingBox(),
       postProc.normalizeParentIds(),
-      postProc.changeLanguage(),
+      postProc.changeLanguage(changeLanguageService, changeLanguageShouldExecute),
       postProc.assignLabels(),
       postProc.geocodeJSON(peliasConfig.api, base),
       postProc.sendJSON

test/unit/middleware/changeLanguage.js

@@ -1,256 +1,235 @@
+'use strict';
 
-var fs = require('fs'),
+const setup = require('../../../middleware/changeLanguage');
-    tmp = require('tmp'),
+const proxyquire =  require('proxyquire').noCallThru();
-    setup = require('../../../middleware/changeLanguage');
+const _  = require('lodash');
-const proxyquire = require('proxyquire').noCallThru();
-
-// load middleware using the default pelias config
-var load = function(){
-  // adapter is driven by config
-  var tmpfile = tmp.tmpNameSync({ postfix: '.json' });
-  fs.writeFileSync( tmpfile, '{}', { encoding: 'utf8' } );
-  process.env.PELIAS_CONFIG = tmpfile;
-  var middleware = setup();
-  delete process.env.PELIAS_CONFIG;
-  return middleware;
-};
 
 module.exports.tests = {};
 
-module.exports.tests.interface = function(test, common) {
+module.exports.tests.interface = (test, common) => {
-  test('valid interface', function(t) {
+  test('valid interface', t => {
-    var middleware = load();
+    t.equal(typeof setup, 'function', 'setup is a function');
-    t.equal(typeof middleware, 'function', 'middleware is a function');
+    t.equal(typeof setup(), 'function', 'setup returns a controller');
-    t.equal(middleware.length, 3, 'middleware is a function');
     t.end();
   });
 };
 
-module.exports.tests.isLanguageChangeRequired = function(test, common) {
+module.exports.tests.early_exit_conditions = (test, common) => {
-  test('invalid query - null req/res', function(t) {
+  test('should_execute returning false should not call service', t => {
-    var middleware = load();
+    t.plan(2, 'should_execute will assert 2 things');
-    middleware(null, null, t.end);
-  });
 
-  test('invalid query - no results', function(t) {
+    const service = () => {
-    var req = { language: { iso6393: 'spa' } };
+      t.fail('service should not have been called');
-    var res = {};
+    };
 
-    var middleware = load();
+    const should_execute = (req, res) => {
-    middleware(req, res, function(){
+      t.deepEquals(req, { a: 1 });
-      t.deepEqual( req, { language: { iso6393: 'spa' } } );
+      t.deepEquals(res, { b: 2 });
-      t.deepEqual( res, {} );
+      return false;
-      t.end();
+    };
-    });
-  });
 
-  test('invalid query - empty results', function(t) {
+    const controller = setup(service, should_execute);
-    var req = { language: { iso6393: 'spa' } };
-    var res = { data: [] };
 
-    var middleware = load();
+    controller({ a: 1 }, { b: 2 }, () => { });
-    middleware(req, res, function(){
-      t.deepEqual( req, { language: { iso6393: 'spa' } } );
-      t.deepEqual( res, { data: [] } );
-      t.end();
-    });
-  });
 
-  test('invalid query - no target language', function(t) {
-    var req = {};
-    var res = { data: [] };
-
-    var middleware = load();
-    middleware(req, res, function(){
-      t.deepEqual( req, {} );
-      t.deepEqual( res, { data: [] } );
-      t.end();
-    });
   });
+
 };
 
-// check the service is called and response mapped correctly
+module.exports.tests.error_conditions = (test, common) => {
-module.exports.tests.miss = function(test, common) {
+  test('service error should log and call next', t => {
-  test('miss', function(t) {
+    // (2) req/res were passed to service
-
+    // (1) error was logged
-    var req = { language: { iso6393: 'spa' } };
+    // (1) res was not modified
-    var res = { data: [
+    t.plan(4);
-      {
+
-        layer: 'locality',
+    const service = (req, res, callback) => {
-        name: { default: 'London' },
+      t.deepEquals(req, { a: 1 } );
-        parent: {
+      t.deepEquals(res, { b: 2 } );
-          locality_id: [ 101750367 ],
+      callback('this is an error');
-          locality: [ 'London' ]
+    };
-        }
-      },
-      {
-        layer: 'example',
-        name: { default: 'London' },
-        parent: {
-          locality_id: [ 101735809 ],
-          locaity: [ 'London' ]
-        }
-      }
-    ]};
 
-    var middleware = load();
+    const logger = require('pelias-mock-logger')();
 
-    // mock out the transport
+    const controller = proxyquire('../../../middleware/changeLanguage', {
-    middleware.transport.query = function mock( ids, cb ){
+      'pelias-logger': logger
-      t.deepEqual( ids, [ '101735809', '101750367' ] );
+    })(service, () => true);
-      t.equal( typeof cb, 'function' );
-      cb( 'error' );
-    };
 
-    middleware(req, res, function(){
+    const req = { a: 1 };
-      t.deepEqual( res, { data: [
+    const res = { b: 2 };
-        {
+
-          layer: 'locality',
+    controller(req, res, () => {
-          name: { default: 'London' },
+      t.ok(logger.isErrorMessage('this is an error'));
-          parent: {
+      t.deepEquals(res, { b: 2 }, 'res should not have been modified');
-            locality_id: [ 101750367 ],
-            locality: [ 'London' ]
-          }
-        },
-        {
-          layer: 'example',
-          name: { default: 'London' },
-          parent: {
-            locality_id: [ 101735809 ],
-            locaity: [ 'London' ]
-          }
-        }
-      ]});
-      t.end();
     });
+
   });
+
 };
 
-// check the service is called and response mapped correctly
+module.exports.tests.success_conditions = (test, common) => {
-module.exports.tests.hit = function(test, common) {
+  test('translations should be mapped in', t => {
-  test('hit', function(t) {
+    // (2) req/res were passed to service
-
+    // (1) error was logged
-    var req = { language: { iso6393: 'spa' } };
+    // (1) res was not modified
-    var res = { data: [
+    // t.plan(4);
-      {
+
-        layer: 'locality',
+    const service = (req, res, callback) => {
-        name: { default: 'London' },
+      const response = {
-        parent: {
+        '1': {
-          locality_id: [ 101750367 ],
+          names: {
-          locality: [ 'London' ]
+            'requested language': [
-        }
+              'replacement name for layer1'
-      },
+            ],
-      {
+            // this should be ignored
-        layer: 'example',
+            'another language': [
-        name: { default: 'London' },
+              'name in another language'
-        parent: {
+            ]
-          locality_id: [ 101735809 ],
-          locality: [ 'London' ]
-        }
-      }
-    ]};
-
-    var middleware = load();
-
-    // mock out the transport
-    middleware.transport.query = function mock( ids, cb ){
-      t.deepEqual( ids, [ '101735809', '101750367' ] );
-      t.equal( typeof cb, 'function' );
-      cb( null, {
-        '101750367': {
-          'names': {
-            'default':['London'],
-            'chi':['倫敦'],
-            'spa':['Londres'],
-            'eng':['London'],
-            'hin':['लंदन'],
-            'ara':['لندن'],
-            'por':['Londres'],
-            'ben':['লন্ডন'],
-            'rus':['Лондон'],
-            'jpn':['ロンドン'],
-            'kor':['런던']
           }
         },
-        '101735809': {
+        '2': {
-          'names':{
+          names: {
-            'default':['London'],
+            'requested language': [
-            'eng':['London']
+              'replacement name for layer2',
+              // this should be ignored
+              'another replacement name for layer2'
+            ]
           }
-        }
+        },
-      });
+        '3': {
-    };
+          names: {
-
+            'requested language': [
-    middleware(req, res, function(){
+              'replacement name 1 for layer3'
-      t.deepEqual( res, { data: [
+            ]
-        {
-          layer: 'locality',
-          name: { default: 'Londres' },
-          parent: {
-            locality_id: [ 101750367 ],
-            locality: [ 'Londres' ]
           }
         },
-        {
+        '4': {
-          layer: 'example',
+          names: {
-          name: { default: 'London' },
+            'requested language': [
-          parent: {
+              'replacement name 2 for layer3'
-            locality_id: [ 101735809 ],
+            ]
-            locality: [ 'London' ]
+          }
+        },
+        '10': {
+          // has names but not in the requested language
+          names: {
+            'another language': [
+              'replacement name for layer4'
+            ]
           }
+        },
+        '11': {
+          // no names
         }
-      ]});
+      };
-      t.end();
+
-    });
+      callback(null, response);
-  });
+    };
+
+    const logger = require('pelias-mock-logger')();
 
-  test('empty array name translation should not change the value', t => {
+    const controller = proxyquire('../../../middleware/changeLanguage', {
-    t.plan(2);
+      'pelias-logger': logger
+    })(service, () => true);
+
+    const req = {
+      clean: {
+        lang: {
+          iso6393: 'requested language'
+        }
+      }
+    };
 
-    const req = { language: { iso6393: 'ISO3 value' } };
     const res = {
       data: [
+        // doc with 2 layer names that will be changed
         {
-          layer: 'locality',
+          name: {
-          name: { default: 'original name' },
+            default: 'original name for 1st result'
+          },
+          layer: 'layer1',
           parent: {
-            locality_id: [ 123 ],
+            layer1_id: ['1'],
-            locality: [ 'original name' ]
+            layer1: ['original name for layer1'],
+            layer2_id: ['2'],
+            layer2: ['original name for layer2']
           }
-        }
+        },
-      ]
+        // not sure how this would sneak in but check anyway
-    };
+        undefined,
-
+        // doc w/o parent
-    const changeLanguage = proxyquire('../../../middleware/changeLanguage', {
+        {},
-      '../service/language': {
+        // doc with only 1 layer name that will be changed and no default name change
-        findById: () => ({
-          query: (ids, callback) => {
-            t.deepEquals(ids, ['123']);
-            callback(null, {
-              '123': {
-                'names': {
-                  'ISO3 value':[]
-                }
-              }
-            });
-          }
-        })
-      }
-    })();
-
-    changeLanguage(req, res, () => {
-      t.deepEqual( res, { data: [
         {
-          layer: 'locality',
           name: {
-            default: 'original name'
+            default: 'original name for 2nd result'
           },
+          layer: 'layer10',
           parent: {
-            locality_id: [ 123 ],
+            layer3_id: ['3', '4'],
-            locality: [ 'original name' ]
+            layer3: ['original name 1 for layer3', 'original name 2 for layer3'],
+            // requested language not found for this id
+            layer10_id: ['10'],
+            layer10: ['original name for layer10'],
+            // no names for this id
+            layer11_id: ['11'],
+            layer11: ['original name for layer11'],
+            // no translations for this id
+            layer12_id: ['12'],
+            layer12: ['original name for layer12'],
+            // undefined id, will be skipped
+            layer13_id: [undefined],
+            layer13: ['original name for layer13']
           }
         }
-      ]});
+      ]
+    };
+
+    controller(req, res, () => {
+      t.ok(logger.isDebugMessage('[language] [debug] missing translation requested language 10'));
+      t.ok(logger.isDebugMessage('[language] [debug] missing translation requested language 11'));
+      t.ok(logger.isDebugMessage('[language] [debug] failed to find translations for 12'));
+
+      t.notOk(logger.hasErrorMessages(), 'there shouldn\'t be any error messages');
+
+      t.deepEquals(res, {
+        data: [
+          {
+            name: {
+              default: 'replacement name for layer1'
+            },
+            layer: 'layer1',
+            parent: {
+              layer1_id: ['1'],
+              layer1: ['replacement name for layer1'],
+              layer2_id: ['2'],
+              layer2: ['replacement name for layer2']
+            }
+          },
+          undefined,
+          {},
+          {
+            name: {
+              default: 'original name for 2nd result'
+            },
+            layer: 'layer10',
+            parent: {
+              layer3_id: ['3', '4'],
+              layer3: ['replacement name 1 for layer3', 'replacement name 2 for layer3'],
+              layer10_id: ['10'],
+              layer10: ['original name for layer10'],
+              layer11_id: ['11'],
+              layer11: ['original name for layer11'],
+              layer12_id: ['12'],
+              layer12: ['original name for layer12'],
+              layer13_id: [undefined],
+              layer13: ['original name for layer13']
+            }
+          }
+        ]
+      });
+
+      t.end();
 
     });
 
@@ -258,13 +237,13 @@ module.exports.tests.hit = function(test, common) {
 
 };
 
-module.exports.all = function (tape, common) {
+module.exports.all = (tape, common) => {
 
   function test(name, testFunction) {
-    return tape('[middleware] changeLanguage: ' + name, testFunction);
+    return tape(`GET /changeLanguage ${name}`, testFunction);
   }
 
-  for( var testCase in module.exports.tests ){
+  for( const testCase in module.exports.tests ){
     module.exports.tests[testCase](test, common);
   }
 };