Merge pull request #611 from dannycoates/iFUn

step 1 in fixing i18n

bin/key_server.js

@@ -45,8 +45,8 @@ function main() {
 
   // TODO: send to the SMTP server directly. In the future this may change
   // to another process that we send an http request to.
-  require('../mailer')(config, log)
+  require('../mailer')(config.smtp, config.i18n.defaultLanguage, config.templateServer, log)
-    .then(
+    .done(
       function(m) {
         mailer = m
         // server public key
@@ -68,7 +68,7 @@ function main() {
           .done(
             function (db) {
               var routes = require('../routes')(log, error, serverPublicKey, signer, db, mailer, config)
-              server = Server.create(log, error, config, routes, db, i18n)
+              server = Server.create(log, error, config, routes, db)
 
               server.start(
                 function () {

config/config.js

@@ -245,27 +245,9 @@ module.exports = function (fs, path, url, convict) {
       }
     },
     i18n: {
-      defaultLang: {
+      defaultLanguage: {
         format: String,
         default: "en-US"
-      },
-      supportedLanguages: {
-        doc: "List of languages this deployment should detect and display localized strings.",
-        format: Array,
-        default: ['en-US', 'it-CH'],
-        env: 'I18N_SUPPORTED_LANGUAGES'
-      },
-      translationDirectory: {
-        doc: "The directory where per-locale .json files containing translations reside",
-        format: String,
-        default: "resources/i18n/",
-        env: "I18N_TRANSLATION_DIR"
-      },
-      translationType: {
-        doc: "The file format used for the translations",
-        format: String,
-        default: "key-value-json",
-        env: "I18N_TRANSLATION_TYPE"
       }
     },
     tokenLifetimes: {

i18n.js

@@ -2,77 +2,61 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+module.exports = function (supportedLanguages, defaultLanguage) {
 
-/*  Helper API for dealing with internationalization/localization.
+  function qualityCmp(a, b) {
- *
+    if (a.quality === b.quality) {
- *  This is a hacky little wrapper around the i18n-abide module, to give
+      return 0
- *  it a nice API for use outside the context of an express app.  If it
+    } else if (a.quality < b.quality) {
- *  works out OK, we should propse the API changes upstream and get rid
+      return 1
- *  of this module.
+    } else {
- *
+      return -1
- */
-
-
-module.exports = function (config) {
-
-  var abide = require('i18n-abide')
-
-  // Configure i18n-abide for loading gettext templates.
-  // This causes it to process the configuration settings, parse the
-  // message files for each language, etc.
-  //
-  // It actually returns an express application with all that state
-  // bundled into a function; we're going to hide that fact with a
-  // bit of a wrapper API, returning the function as if it were a
-  // stateful object with helper methods.
-  var abideObj = abide.abide(
-    {
-      default_lang: config.defaultLang,
-      supported_languages: config.supportedLanguages,
-      translation_directory: config.translationDirectory,
-      translation_type: config.translationType
     }
-  )
-
-
-  // Export the parseAcceptLanguage() function as-is.
-  abideObj.parseAcceptLanguage = function(header) {
-    return abide.parseAcceptLanguage(header)
   }
 
-
+  function parseAcceptLanguage(header) {
-  // Export the bestLanguage() function, but using defaults from the config.
+    // pl,fr-FR;q=0.3,en-US;q=0.1
-  abideObj.bestLanguage = function(accepted, supported) {
+    if (! header || ! header.split) {
-    if (!supported) {
+      return []
-      supported = config.supportedLanguages
     }
-    return abide.bestLanguage(accepted, supported)
+    var rawLanguages = header.split(',')
+    var languages = rawLanguages.map(
+      function(rawLanguage) {
+        var parts = rawLanguage.split(';')
+        var q = 1
+        if (parts.length > 1 && parts[1].indexOf('q=') === 0) {
+          var qval = parseFloat(parts[1].split('=')[1])
+          if (isNaN(qval) === false) {
+            q = qval
+          }
+        }
+        return { lang: parts[0].trim(), quality: q }
+      }
+    )
+    languages.sort(qualityCmp)
+    return languages
   }
 
-  // A new function to get a stand-alone 'localization context'
+  function bestLanguage(languages, supportedLanguages, defaultLanguage) {
-  // This gives us the properties that i18n-abide attaches to the request
+    var lower = supportedLanguages.map(function(l) { return l.toLowerCase() })
-  // object, without actually having to be an express app.
+    for(var i=0; i < languages.length; i++) {
-  abideObj.localizationContext = function(acceptLang) {
+      var lq = languages[i]
-    var fakeReq = {headers: {}}
+      if (lower.indexOf(lq.lang.toLowerCase()) !== -1) {
-    var fakeResp = {locals: function(){}}
+        return lq.lang
-    if (acceptLang) {
+      } else if (lower.indexOf(lq.lang.split('-')[0].toLowerCase()) !== -1) {
-      fakeReq.headers['accept-language'] = acceptLang
+        return lq.lang.split('-')[0]
+      }
     }
-    var callWasSynchronous = false;
+    return defaultLanguage
-    abideObj(fakeReq, fakeResp, function() { callWasSynchronous = true })
-    if (!callWasSynchronous) {
-      throw new Error('uh-oh, the call to i18n-abide was not synchronous!')
-    }
-    var l10n = {}
-    l10n.lang = fakeReq.lang
-    l10n.lang_dir = fakeReq.lang_dir
-    l10n.locale = fakeReq.locale
-    l10n.gettext = fakeReq.gettext.bind(fakeReq)
-    l10n.format = fakeReq.format.bind(fakeReq)
-    return l10n
   }
 
-  abideObj.defaultLang = config.defaultLang
+  return {
-
+    language: function (header) {
-  return abideObj
+      return bestLanguage(
+        parseAcceptLanguage(header),
+        supportedLanguages,
+        defaultLanguage
+      ).toLowerCase()
+    }
+  }
 }

lockdown.json

@@ -352,9 +352,6 @@
   "http-signature": {
     "0.10.0": "1494e4f5000a83c0f11bcc12d6007c530cb99582"
   },
-  "i18n-abide": {
-    "0.0.16": "09f9110fe40da80373bc282cb043d272baa30578"
-  },
   "iconv": {
     "2.0.7": "a05421a08b0d4247c099b16f65e4767a90aa851f"
   },

log.js

@@ -61,7 +61,7 @@ Overdrive.prototype.summary = function (request, response) {
     errno: response.errno || 0,
     rid: request.id,
     path: request.path,
-    lang: request.app.preferredLang,
+    lang: request.app.acceptLanguage,
     agent: request.headers['user-agent'],
     remoteAddressChain: request.app.remoteAddressChain,
     t: Date.now() - request.info.received

mailer.js

@@ -10,7 +10,19 @@ var P = require('./promise')
 var handlebars = require("handlebars")
 var request = require('request')
 
-module.exports = function (config, log) {
+module.exports = function (smtpConfig, defaultLanguage, templateServer, log) {
+
+  function templateLanguages() {
+    var dir = fs.readdirSync(path.join(__dirname, 'templates'))
+    var languages = {}
+    for (var i = 0; i < dir.length; i++) {
+      var languageMatch = /(\S+)_(?:reset|verify)\.json$/.exec(dir[i])
+      if (languageMatch) {
+        languages[languageMatch[1]] = true
+      }
+    }
+    return Object.keys(languages)
+  }
   // A map of all the different emails we send. The templates are retrieved from
   // (1) the local `templates/`
   // (2) the 'fxa-content-server'
@@ -20,25 +32,19 @@ module.exports = function (config, log) {
   //     templates['en']['reset']
   //     templates['it-CH']['verify']
   //
-  // We read the languages from config.i18n.supportedLanguages and
-  // the types are currently 'verify' and 'reset'.
   var templates = {}
   var types = [ 'verify', 'reset' ]
+  var supportedLanguages = templateLanguages()
+  var i18n = require('./i18n')(supportedLanguages, defaultLanguage)
 
-  // This function reads the local templates first so that we can startup
-  // without depending on an external service. Once read, we will resolve
-  // the promise so the caller can continue.
   function readLocalTemplates() {
     var p = P.defer()
-
+    var remaining = supportedLanguages.length * types.length
-    var remaining = config.i18n.supportedLanguages.length * types.length;
+    supportedLanguages.forEach(function(language) {
-    config.i18n.supportedLanguages.forEach(function(lang) {
+      var lang = language.toLowerCase()
-      // somewhere to store the templates
       templates[lang] = templates[lang] || {}
-
       types.forEach(function(type) {
-        // read the *.json file
+        var filename = path.join(__dirname, 'templates', language + '_' + type + '.json')
-        var filename = path.join(__dirname, 'templates', lang + '_' + type + '.json')
         fs.readFile(filename, { encoding : 'utf8' }, function(err, data) {
           if (err) {
             log.warn({ op: 'mailer.readLocalTemplates', err: err })
@@ -46,12 +52,10 @@ module.exports = function (config, log) {
           else {
             templates[lang][type] = JSON.parse(data)
 
-            // compile the templates
             templates[lang][type].html = handlebars.compile(templates[lang][type].html)
             templates[lang][type].text = handlebars.compile(templates[lang][type].text)
           }
 
-          // whether we errored or not, count down so we know when to resolve
           remaining -= 1
           if ( remaining === 0 ) {
             p.resolve()
@@ -66,7 +70,7 @@ module.exports = function (config, log) {
   function fetchTemplates() {
     var p = P.defer()
 
-    var remaining = config.i18n.supportedLanguages.length * types.length;
+    var remaining = supportedLanguages.length * types.length
 
     function checkRemaining() {
       if ( remaining === 0 ) {
@@ -74,13 +78,14 @@ module.exports = function (config, log) {
       }
     }
 
-    config.i18n.supportedLanguages.forEach(function(lang) {
+    supportedLanguages.forEach(function(language) {
+      var lang = language.toLowerCase()
       // somewhere to store the templates
       templates[lang] = templates[lang] || {}
 
       types.forEach(function(type) {
         var opts = {
-          uri : config.templateServer.url + '/template/' + lang + '/' + type,
+          uri : templateServer.url + '/template/' + language + '/' + type,
           json : true,
         }
         request(opts, function(err, res, body) {
@@ -118,22 +123,22 @@ module.exports = function (config, log) {
   }
 
 
-  function Mailer(smtp) {
+  function Mailer(config) {
     var options = {
-      host: config.smtp.host,
+      host: config.host,
-      secureConnection: config.smtp.secure,
+      secureConnection: config.secure,
-      port: config.smtp.port
+      port: config.port
     }
-    if (config.smtp.user && config.smtp.password) {
+    if (config.user && config.password) {
       options.auth = {
-        user: config.smtp.user,
+        user: config.user,
-        pass: config.smtp.password
+        pass: config.password
       }
     }
     this.mailer = nodemailer.createTransport('SMTP', options)
-    this.sender = config.smtp.sender
+    this.sender = config.sender
-    this.verificationUrl = config.smtp.verificationUrl
+    this.verificationUrl = config.verificationUrl
-    this.passwordResetUrl = config.smtp.passwordResetUrl
+    this.passwordResetUrl = config.passwordResetUrl
   }
 
   Mailer.prototype.stop = function () {
@@ -168,16 +173,16 @@ module.exports = function (config, log) {
   // - opts : object of options:
   //   - service : the service we came from
   //   - redirectTo : where to redirect the user once clicked
-  //   - preferredLang : the preferred language of the user
+  //   - acceptLanguage : the preferred language of the user
   Mailer.prototype.sendVerifyCode = function (account, code, opts) {
     log.trace({ op: 'mailer.sendVerifyCode', email: account.email, uid: account.uid })
     code = code.toString('hex')
     opts = opts || {}
 
-    var lang = opts.preferredLang || config.defaultLang
+    var lang = i18n.language(opts.acceptLanguage)
-    lang = lang in templates ? lang : 'en-US'
+    lang = lang in templates ? lang : 'en-us'
 
-    var template = templates[lang].verify || templates[config.defaultLang].verify
+    var template = templates[lang].verify || templates[defaultLanguage].verify
     var query = {
       uid: account.uid.toString('hex'),
       code: code
@@ -215,14 +220,14 @@ module.exports = function (config, log) {
   // - opts : object of options:
   //   - service : the service we came from
   //   - redirectTo : where to redirect the user once clicked
-  //   - preferredLang : the preferred language of the user
+  //   - acceptLanguage : the preferred language of the user
   Mailer.prototype.sendRecoveryCode = function (token, code, opts) {
     log.trace({ op: 'mailer.sendRecoveryCode', email: token.email })
     code = code.toString('hex')
     opts = opts || {}
-    var lang = opts.preferredLang || config.defaultLang
+    var lang = i18n.language(opts.acceptLanguage)
-    lang = lang in templates ? lang : 'en-US'
+    lang = lang in templates ? lang : 'en-us'
-    var template = templates[lang].reset || templates[config.defaultLang].reset
+    var template = templates[lang].reset || templates[defaultLanguage].reset
     var query = {
       token: token.data.toString('hex'),
       code: code,
@@ -253,11 +258,10 @@ module.exports = function (config, log) {
   }
 
   // fetch the templates first, then resolve the new Mailer
-  var p = P.defer()
+  return readLocalTemplates().then(function() {
-  readLocalTemplates().then(function() {
     // Now that the local templates have been read in, we can now read them from the
     // fxa-content-server for any updates.
-    fetchTemplates().then(
+    fetchTemplates().done(
       function() {
         log.info({ op: 'mailer.fetchTemplates', msg : 'All ok' })
       },
@@ -265,8 +269,6 @@ module.exports = function (config, log) {
         log.error({ op: 'mailer.fetchTemplates', err: err })
       }
     )
-
+    return new Mailer(smtpConfig)
-    return p.resolve(new Mailer(config.smtp))
   })
-  return p.promise
 }

package.json

@@ -45,7 +45,6 @@
     "toobusy": "0.2.4",
     "nodemailer": "0.6.0",
     "then-redis": "0.3.10",
-    "i18n-abide": "0.0.16",
     "scrypt-hash": "1.1.8",
     "lockdown": "0.0.5",
     "request": "2.31.0"

routes/account.js

@@ -136,7 +136,7 @@ module.exports = function (
                 mailer.sendVerifyCode(response.account, response.account.emailCode, {
                   service: form.service,
                   redirectTo: form.redirectTo,
-                  preferredLang: request.app.preferredLang
+                  acceptLanguage: request.app.acceptLanguage
                 })
                 .fail(
                   function (err) {
@@ -372,7 +372,7 @@ module.exports = function (
         mailer.sendVerifyCode(sessionToken, sessionToken.emailCode, {
           service: request.payload.service,
           redirectTo: request.payload.redirectTo,
-          preferredLang: request.app.preferredLang
+          acceptLanguage: request.app.acceptLanguage
         }).done(
           function () {
             reply({})

routes/password.js

@@ -196,7 +196,7 @@ module.exports = function (
                 {
                   service: request.payload.service,
                   redirectTo: request.payload.redirectTo,
-                  preferredLang: request.app.preferredLang
+                  acceptLanguage: request.app.acceptLanguage
                 }
               )
               .then(
@@ -256,7 +256,7 @@ module.exports = function (
           {
             service: request.payload.service,
             redirectTo: request.payload.redirectTo,
-            preferredLang: request.app.preferredLang
+            acceptLanguage: request.app.acceptLanguage
           }
         )
         .done(

server/server.js

@@ -6,7 +6,7 @@ var HEX_STRING = require('../routes/validators').HEX_STRING
 
 module.exports = function (path, url, Hapi, toobusy) {
 
-  function create(log, error, config, routes, db, i18n) {
+  function create(log, error, config, routes, db) {
 
     // Hawk needs to calculate request signatures based on public URL,
     // not the local URL to which it is bound.
@@ -147,14 +147,10 @@ module.exports = function (path, url, Hapi, toobusy) {
         var xff = (request.headers['x-forwarded-for'] || '').split(/\s*,\s*/)
         xff.push(request.info.remoteAddress)
         // Remove empty items from the list, in case of badly-formed header.
-        request.app.remoteAddressChain = xff.filter(function(x){ return x});
+        request.app.remoteAddressChain = xff.filter(function(x){ return x })
+
+        request.app.acceptLanguage = request.headers['accept-language']
 
-        // Select user's preferred language via the accept-language header.
-        var acceptLanguage = request.headers['accept-language']
-        if (acceptLanguage) {
-          var accepted = i18n.parseAcceptLanguage(acceptLanguage)
-          request.app.preferredLang = i18n.bestLanguage(accepted)
-        }
         if (request.headers.authorization) {
           // Log some helpful details for debugging authentication problems.
           log.trace(