kbn-config-schema: allow parsing of array and object types from string#57189
Conversation
pgayvallet
added
Team:Core
|
Pinging @elastic/kibana-platform (Team:Platform) |
pgayvallet
changed the title
| if (isPlainObject(parsed)) { | ||
| return parsed; | ||
| } | ||
| return this.createError('object.base', { value: parsed }, state, options); |
There was a problem hiding this comment.
I'm using the parsed value to create the error message here. "[1,2,3]" will fails with an error message stating that it received an array instead of an object, even if the actual raw input was a string.I think using the parsed value type makes more sense / is more clear, as least for route validation where the parsing is implicit for query and or multipart/form-data data.
| expect(() => type.validate('test', {}, 'foo-namespace')).toThrowErrorMatchingSnapshot(); | ||
| expect(() => type.validate('test', {}, 'foo-namespace')).toThrowErrorMatchingInlineSnapshot( | ||
| `"[foo-namespace]: could not parse array value from [test]"` | ||
| ); |
There was a problem hiding this comment.
I switched to inline snapshots. Should probably have done this in a separate commit though. No snapshot did change except the ones that were using string data as invalid type, as the error changed to could not parse.... I did adapt the data on these tests though.
| if (isPlainObject(value)) { | ||
| return new Map(Object.entries(value)); | ||
| } | ||
| if (options.convert && typeof value === 'string') { |
There was a problem hiding this comment.
Note: I'm checking options.convert here to see if we should try to convert the value, However this option is never set in our library, and will always default to the default joi value which is true. It was more like a futur-proof thing if we want to implement this option at some point.
| ).toThrowErrorMatchingInlineSnapshot( | ||
| `"[disabledFeatures]: expected value of type [array] but got [string]"` | ||
| `"[disabledFeatures]: could not parse array value from [foo]"` | ||
| ); |
joshdover
left a comment
joshdover
left a comment
There was a problem hiding this comment.
Can we add some details about this feature to the README.md for this package?
💚 Build SucceededHistory
To update your PR or re-run it, just comment with: |
elastic#57189) * allow parsing from string for object-ish and array types * update snapshots * fix FTR assertion * add documentation note about using a json string as input
5 participants
Summary
Fix #56951
Allow
array,object,mapandrecordtypes to accept string representation of their value and parse themThis was already the case for other primitive types such as boolean and number, and allows object-type parameters to be properly parsed / validated when used in query string or in
multiparttype payloads.Checklist
Delete any items that are not applicable to this PR.
For maintainers