-
-
Notifications
You must be signed in to change notification settings - Fork 377
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add in i18n support for validator #161
Changes from all commits
d0a8f96
6713689
26c0775
6586d1b
781030b
9d1a4a5
a296341
5316c67
a6e97bd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,13 +1,44 @@ | ||
const createError = require('http-errors') | ||
const Ajv = require('ajv') | ||
const ajvKeywords = require('ajv-keywords') | ||
const {deepEqual} = require('assert') | ||
const ajvLocalize = require('ajv-i18n') | ||
const { deepEqual } = require('assert') | ||
|
||
let ajv | ||
let previousConstructorOptions | ||
const defaults = {v5: true, $data: true, allErrors: true} | ||
const defaults = { | ||
v5: true, | ||
coerceTypes: 'array', // important for query string params | ||
allErrors: true, | ||
useDefaults: true, | ||
$data: true, // required for ajv-keywords | ||
defaultLanguage: 'en' | ||
} | ||
|
||
const availableLanguages = Object.keys(ajvLocalize) | ||
|
||
/* in ajv-i18n Portuguese is represented as pt-BR */ | ||
const languageNormalizationMap = { | ||
'pt': 'pt-BR', | ||
'pt-br': 'pt-BR', | ||
'pt_BR': 'pt-BR', | ||
'pt_br': 'pt-BR' | ||
} | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Makes Portuguese less ambiguous and it will work with all possible variations. |
||
const normalizePreferredLanguage = (lang) => languageNormalizationMap[lang] || lang | ||
|
||
module.exports = ({inputSchema, outputSchema, ajvOptions}) => { | ||
const chooseLanguage = ({ preferredLanguage }, defaultLanguage) => { | ||
if (preferredLanguage) { | ||
const lang = normalizePreferredLanguage(preferredLanguage) | ||
if (availableLanguages.includes(lang)) { | ||
return lang | ||
} | ||
} | ||
|
||
return defaultLanguage | ||
} | ||
|
||
module.exports = ({ inputSchema, outputSchema, ajvOptions }) => { | ||
const options = Object.assign({}, defaults, ajvOptions) | ||
lazyLoadAjv(options) | ||
|
||
|
@@ -24,14 +55,18 @@ module.exports = ({inputSchema, outputSchema, ajvOptions}) => { | |
|
||
if (!valid) { | ||
const error = new createError.BadRequest('Event object failed validation') | ||
handler.event.headers = Object.assign({}, handler.event.headers) | ||
const language = chooseLanguage(handler.event, options.defaultLanguage) | ||
ajvLocalize[language](validateInput.errors) | ||
|
||
error.details = validateInput.errors | ||
throw error | ||
} | ||
|
||
return next() | ||
}, | ||
after (handler, next) { | ||
if (!outputSchema) { | ||
if (!outputSchema || (!handler.response && handler.error)) { | ||
return next() | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, isn't this a breaking change?
coerceTypes
isfalse
by defaultThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From a code standpoint, yes. From an end-user standpoint, no. Since this middleware is paired with a json schema, having
coerceTypes
enabled would make requests more forgiving when abody
is passed in and enable the ability to parse query strings making them more usable. There are no cases where existing code would fail where they once passed. For example; if someone is using the current defaults their schema would have"type":"string"
for all their query string params, in this case, no coercion would happen.coerceTypes
is only applied if the types don't match. I added this option to support the use case when you expect an array from the query string, but only one item was passed in.I feel including this as our default will be doing a service to younger developers, allowing their APIs to be more forgiving out of the gate. However, I do understand why if you'd like to default to something more strict. I'm good either way due to I'm always passing in custom options anyway. Let me know which way you want to go.
The background behind
coerceTypes: 'array'
can be found here: ajv-validator/ajv#158There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Totally makes sense for me. And thanks for adding this, I see how it will be useful in my code already!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vladgolubev Along these lines, you may also be interested in this PR: ajv-validator/ajv-keywords#64
I just had a new keyword added,
transform
, that allows to you to do some sanitization on strings being passed in. We're drafting up next steps for it here: ajv-validator/ajv-keywords#66