Precompile error templates. Closes #1894. Closes #1905

hapijs · Jun 17, 2019 · a5ceb30 · a5ceb30
1 parent 96081b6
commit a5ceb30
Show file tree

Hide file tree

Showing 13 changed files with 70 additions and 45 deletions.
diff --git a/API.md b/API.md
@@ -296,17 +296,18 @@ Validates a value using the given schema and options where:
   - `context` - provides an external data set to be used in [references](#refkey-options). Can only be set as an external option to
     `validate()` and not using `any.prefs()`.
   - `convert` - when `true`, attempts to cast values to the required types (e.g. a string to a number). Defaults to `true`.
-  - `dateErrorFormat` - sets the string format used when including dates in error messages. Options are:
-    - `'date'` - date string.
-    - `'iso'` - date time ISO string. This is the default.
-    - `'string'` - JS default date time string.
-    - `'time'` - time string.
-    - `'utc'` - UTC date time string.
-  - `escapeErrors` - when `true`, error message templates will escape special characters to HTML
-    entities, for security purposes. Defaults to `false`.
+  - `error` - error formatting settings:
+    - `dateFormat` - sets the string format used when including dates in error messages. Options are:
+      - `'date'` - date string.
+      - `'iso'` - date time ISO string. This is the default.
+      - `'string'` - JS default date time string.
+      - `'time'` - time string.
+      - `'utc'` - UTC date time string.
+    - `escapeHtml` - when `true`, error message templates will escape special characters to HTML
+      entities, for security purposes. Defaults to `false`.
   - `messages` - overrides individual error messages. Defaults to no override (`{}`). Messages use
     the same rules as [templates](#template-syntax). Variables in double braces `{{var}}` are HTML
-    escaped if the option `escapeErrors` is set to `true`.
+    escaped if the option `errors.escapeHtml` is set to `true`.
   - `noDefaults` - when `true`, do not apply default values. Defaults to `false`.
   - `nonEnumerables` - when `true`, inputs are shallow cloned to include non-enumerables properties. Defaults to `false`.
   - `presence` - sets the default presence requirements. Supported modes: `'optional'`, `'required'`, and `'forbidden'`.
@@ -556,7 +557,7 @@ Generates a template from a string where:
 The template syntax uses `{}` and `{{}}` enclosed variables to indicate that these values should be
 replace with the actual referenced values at rendering time. Single braces `{}` leave the value
 as-is, while double braces `{{}}` HTML-escape the result (unless the template is used for error messages
-and the `escapeErrors` preference flag is set to `false`).
+and the `errors.escapeHtml` preference flag is set to `false`).
 
 The variable names can have one of the following prefixes:
 - `#` - indicates the variable references a local context value. For example, in errors this is the

diff --git a/docs/check-errors-list.js b/docs/check-errors-list.js
@@ -10,7 +10,7 @@ const internals = {
     API: Fs.readFileSync(Path.join(__dirname, '../API.md'), 'utf8'),
     startString: '<!-- errors -->',
     endString: '<!-- errorsstop -->',
-    ignoredCodes: ['root', 'key', 'wrapArrays']
+    ignoredCodes: ['root']
 };
 
 

diff --git a/lib/common.js b/lib/common.js
@@ -12,8 +12,11 @@ exports.defaults = {
     allowUnknown: false,
     // context: null
     convert: true,
-    dateErrorFormat: 'iso',
-    escapeErrors: false,
+    errors: {
+        dateFormat: 'iso',
+        escapeHtml: false,
+        wrapArrays: true
+    },
     messages: {},
     nonEnumerables: false,
     noDefaults: false,

diff --git a/lib/errors.js b/lib/errors.js
@@ -2,17 +2,15 @@
 
 const Hoek = require('@hapi/hoek');
 
-const Common = require('./common');
 const Messages = require('./messages');
 const Template = require('./template');
 
 let Any;
 
 
 const internals = {
-    annotations: Symbol('joi-annotations'),
-
-    templateRx: /{{?[^}]+}}?/
+    annotations: Symbol('annotations'),
+    templates: {}
 };
 
 
@@ -50,17 +48,17 @@ exports.Report = class {
         }
 
         const localized = this.prefs.messages;
-        const format = this.template || localized[this.code] || Messages.errors[this.code];
-        if (typeof format !== 'string') {
+        let template = this.template || localized[this.code] || internals.templates[this.code];
+        if (template === undefined) {
             return `Error code "${this.code}" is not defined, your custom type is missing the correct messages definition`;
         }
 
-        const wrapArrays = Common.default(localized.wrapArrays, Messages.errors.wrapArrays);
-        const template = new Template(format);
-        const message = template.render(this.value, null, this.prefs, this.local, { wrapArrays, escapeHtml: !!this.prefs.escapeErrors });
+        if (typeof template === 'string') {
+            template = new Template(template);
+        }
 
-        this.toString = () => message;  // Cache result
-        return message;
+        this.message = template.render(this.value, null, this.prefs, this.local, this.prefs.errors);        // Cache result
+        return this.message;
     }
 };
 
@@ -333,9 +331,10 @@ exports.messages = function (options) {
     }
 
     if (typeof options.message === 'string') {
-        if (internals.isTemplate(options.message)) {
+        const template = new Template(options.message);
+        if (template.isDynamic()) {
             options = Object.assign({}, options);           // Shallow cloned
-            options.template = options.message;
+            options.template = template;
             delete options.message;
         }
 
@@ -345,7 +344,13 @@ exports.messages = function (options) {
     const sorted = { message: {}, template: {} };
     for (const code in options.message) {
         const message = options.message[code];
-        sorted[internals.isTemplate(message) ? 'template' : 'message'][code] = message;
+        const template = new Template(message);
+        if (template.isDynamic()) {
+            sorted.template[code] = template;
+        }
+        else {
+            sorted.message[code] = message;
+        }
     }
 
     if (!Object.keys(sorted.template).length) {
@@ -366,7 +371,15 @@ exports.messages = function (options) {
 };
 
 
-internals.isTemplate = function (message) {
+internals.cache = function () {
 
-    return internals.templateRx.test(message);
+    for (const code in Messages.errors) {
+        if (code === 'root') {
+            continue;
+        }
+
+        internals.templates[code] = new Template(Messages.errors[code]);
+    }
 };
+
+internals.cache();
diff --git a/lib/messages.js b/lib/messages.js
@@ -5,7 +5,6 @@ const internals = {};
 
 exports.errors = {
     root: 'value',
-    wrapArrays: true,
 
     'any.unknown': '"{{#label}}" is not allowed',
     'any.invalid': '"{{#label}}" contains an invalid value',

diff --git a/lib/schemas.js b/lib/schemas.js
@@ -11,8 +11,11 @@ exports.preferences = Joi.object({
     abortEarly: Joi.boolean(),
     context: Joi.object(),
     convert: Joi.boolean(),
-    dateErrorFormat: Joi.string().only('date', 'iso', 'string', 'time', 'utc'),
-    escapeErrors: Joi.boolean(),
+    errors: {
+        dateFormat: Joi.string().only('date', 'iso', 'string', 'time', 'utc'),
+        escapeHtml: Joi.boolean(),
+        wrapArrays: Joi.boolean()
+    },
     messages: Joi.object(),
     noDefaults: Joi.boolean(),
     nonEnumerables: Joi.boolean(),

diff --git a/lib/template.js b/lib/template.js
@@ -111,6 +111,11 @@ module.exports = exports = internals.Template = class {
 
         return { template: this.source, options: this._settings };
     }
+
+    isDynamic() {
+
+        return !!this._template;
+    }
 };
 
 
@@ -192,7 +197,7 @@ internals.stringify = function (value, prefs, options) {
     }
 
     if (value instanceof Date) {
-        return internals.dateFormat[prefs.dateErrorFormat].call(value);
+        return internals.dateFormat[prefs.errors.dateFormat].call(value);
     }
 
     if (!Array.isArray(value)) {

diff --git a/lib/validator.js b/lib/validator.js
@@ -5,6 +5,7 @@ const Hoek = require('@hapi/hoek');
 const Common = require('./common');
 const Errors = require('./errors');
 const Ref = require('./ref');
+const Template = require('./template');
 
 
 const internals = {};
@@ -214,7 +215,7 @@ internals.error = function (report, { message, template }) {
     }
 
     if (template) {
-        template = typeof template === 'string' ? template : template[report.code];
+        template = template instanceof Template ? template : template[report.code];
         if (template) {
             report.template = template;
             return report;

diff --git a/test/errors.js b/test/errors.js
@@ -174,7 +174,7 @@ describe('errors', () => {
             'a()': Joi.number()
         };
 
-        const err = await expect(Joi.validate({ 'a()': 'x' }, schema, { escapeErrors: true })).to.reject();
+        const err = await expect(Joi.validate({ 'a()': 'x' }, schema, { errors: { escapeHtml: true } })).to.reject();
         expect(err).to.be.an.error('"a&#x28;&#x29;" must be a number');
         expect(err.details).to.equal([{
             message: '"a&#x28;&#x29;" must be a number',
@@ -183,7 +183,7 @@ describe('errors', () => {
             context: { label: 'a()', key: 'a()', value: 'x' }
         }]);
 
-        const err2 = await expect(Joi.validate({ 'b()': 'x' }, schema, { escapeErrors: true })).to.reject();
+        const err2 = await expect(Joi.validate({ 'b()': 'x' }, schema, { errors: { escapeHtml: true } })).to.reject();
         expect(err2).to.be.an.error('"b&#x28;&#x29;" is not allowed');
         expect(err2.details).to.equal([{
             message: '"b&#x28;&#x29;" is not allowed',
@@ -327,7 +327,7 @@ describe('errors', () => {
 
     it('overrides wrapArrays', async () => {
 
-        const schema = Joi.array().items(Joi.boolean()).prefs({ messages: { wrapArrays: false } });
+        const schema = Joi.array().items(Joi.boolean()).prefs({ errors: { wrapArrays: false } });
         const err = await expect(schema.validate([4])).to.reject();
         expect(err).to.be.an.error('"[0]" must be a boolean');
         expect(err.details).to.equal([{

diff --git a/test/template.js b/test/template.js
@@ -20,7 +20,7 @@ describe('Template', () => {
         const template = Joi.template(source);
 
         expect(template.source).to.equal(source);
-        expect(template._template).to.be.null();
+        expect(template.isDynamic()).to.be.false();
         expect(template.render()).to.equal(source);
     });
 
@@ -30,7 +30,7 @@ describe('Template', () => {
         const template = Joi.template(source);
 
         expect(template.source).to.equal(source);
-        expect(template._template).to.be.null();
+        expect(template.isDynamic()).to.be.false();
         expect(template.render()).to.equal(source);
     });
 
@@ -40,7 +40,7 @@ describe('Template', () => {
         const template = Joi.template(source);
 
         expect(template.source).to.equal(source);
-        expect(template._template).to.be.null();
+        expect(template.isDynamic()).to.be.false();
         expect(template.render()).to.equal(source);
     });
 
@@ -50,7 +50,7 @@ describe('Template', () => {
         const template = Joi.template(source);
 
         expect(template.source).to.equal(source);
-        expect(template._template).to.be.null();
+        expect(template.isDynamic()).to.be.false();
         expect(template.render()).to.equal(source);
     });
 
@@ -60,7 +60,7 @@ describe('Template', () => {
         const template = Joi.template(source);
 
         expect(template.source).to.equal(source);
-        expect(template._template).to.be.null();
+        expect(template.isDynamic()).to.be.false();
         expect(template.render()).to.equal('text {{{ without }}} any }} {{escaped}} variables');
     });
 

diff --git a/test/types/alternatives.js b/test/types/alternatives.js
@@ -239,7 +239,7 @@ describe('alternatives', () => {
                         Joi.string().valid('foo', 'bar')
                     ])
                 })
-            ]).prefs({ messages: { wrapArrays: false } });
+            ]).prefs({ errors: { wrapArrays: false } });
 
             Helper.validate(schema, [
                 [{ p: 1 }, false, null, {

diff --git a/test/types/any.js b/test/types/any.js
@@ -1553,7 +1553,7 @@ describe('any', () => {
                     })
                 });
 
-                const err = await expect(Joi.validate({ a: new Date('1973-01-01') }, schema, { dateErrorFormat: 'date', abortEarly: false })).to.reject();
+                const err = await expect(Joi.validate({ a: new Date('1973-01-01') }, schema, { errors: { dateFormat: 'date' }, abortEarly: false })).to.reject();
                 expect(err.isJoi).to.not.exist();
                 expect(err.message).to.equal(`"a" must be larger than or equal to "${min.toDateString()}" and "a" must be greater than "${min.toDateString()}"`);
             });

diff --git a/test/types/date.js b/test/types/date.js
@@ -749,7 +749,7 @@ describe('date', () => {
 
                 const d = new Date('1-1-1970 UTC');
                 const message = `"value" must be less than "${d}"`;
-                Helper.validate(Joi.date().less('1-1-1970 UTC').prefs({ dateErrorFormat: 'string' }), [
+                Helper.validate(Joi.date().less('1-1-1970 UTC').prefs({ errors: { dateFormat: 'string' } }), [
                     ['1-1-1971 UTC', false, null, {
                         message,
                         details: [{