Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions packages/kbn-config-schema/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,9 @@ __Usage:__
const valueSchema = schema.arrayOf(schema.number());
```

__Notes:__
* The `schema.arrayOf()` also supports a json string as input if it can be safely parsed using `JSON.parse` and if the resulting value is an array.

#### `schema.object()`

Validates input data as an object with a predefined set of properties.
Expand All @@ -249,6 +252,7 @@ const valueSchema = schema.object({
__Notes:__
* Using `allowUnknowns` is discouraged and should only be used in exceptional circumstances. Consider using `schema.recordOf()` instead.
* Currently `schema.object()` always has a default value of `{}`, but this may change in the near future. Try to not rely on this behaviour and specify default value explicitly or use `schema.maybe()` if the value is optional.
* `schema.object()` also supports a json string as input if it can be safely parsed using `JSON.parse` and if the resulting value is a plain object.

#### `schema.recordOf()`

Expand All @@ -267,6 +271,7 @@ const valueSchema = schema.recordOf(schema.string(), schema.number());

__Notes:__
* You can use a union of literal types as a record's key schema to restrict record to a specific set of keys, e.g. `schema.oneOf([schema.literal('isEnabled'), schema.literal('name')])`.
* `schema.recordOf()` also supports a json string as input if it can be safely parsed using `JSON.parse` and if the resulting value is a plain object.

#### `schema.mapOf()`

Expand All @@ -283,6 +288,10 @@ __Usage:__
const valueSchema = schema.mapOf(schema.string(), schema.number());
```

__Notes:__
* You can use a union of literal types as a record's key schema to restrict record to a specific set of keys, e.g. `schema.oneOf([schema.literal('isEnabled'), schema.literal('name')])`.
* `schema.mapOf()` also supports a json string as input if it can be safely parsed using `JSON.parse` and if the resulting value is a plain object.

### Advanced types

#### `schema.oneOf()`
Expand Down
70 changes: 59 additions & 11 deletions packages/kbn-config-schema/src/internals/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -250,22 +250,47 @@ export const internals = Joi.extend([

base: Joi.object(),
coerce(value: any, state: State, options: ValidationOptions) {
// If value isn't defined, let Joi handle default value if it's defined.
if (value !== undefined && !isPlainObject(value)) {
return this.createError('object.base', { value }, state, options);
if (value === undefined || isPlainObject(value)) {
return value;
}

return value;
if (options.convert && typeof value === 'string') {
try {
const parsed = JSON.parse(value);
if (isPlainObject(parsed)) {
return parsed;
}
return this.createError('object.base', { value: parsed }, state, options);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

} catch (e) {
return this.createError('object.parse', { value }, state, options);
}
}

return this.createError('object.base', { value }, state, options);
},
rules: [anyCustomRule],
},
{
name: 'map',

coerce(value: any, state: State, options: ValidationOptions) {
if (value === undefined) {
return value;
}
if (isPlainObject(value)) {
return new Map(Object.entries(value));
}
if (options.convert && typeof value === 'string') {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

try {
const parsed = JSON.parse(value);
if (isPlainObject(parsed)) {
return new Map(Object.entries(parsed));
}
return this.createError('map.base', { value: parsed }, state, options);
} catch (e) {
return this.createError('map.parse', { value }, state, options);
}
}

return value;
},
Expand Down Expand Up @@ -321,11 +346,23 @@ export const internals = Joi.extend([
{
name: 'record',
pre(value: any, state: State, options: ValidationOptions) {
if (!isPlainObject(value)) {
return this.createError('record.base', { value }, state, options);
if (value === undefined || isPlainObject(value)) {
return value;
}

return value as any;
if (options.convert && typeof value === 'string') {
try {
const parsed = JSON.parse(value);
if (isPlainObject(parsed)) {
return parsed;
}
return this.createError('record.base', { value: parsed }, state, options);
} catch (e) {
return this.createError('record.parse', { value }, state, options);
}
}

return this.createError('record.base', { value }, state, options);
},
rules: [
anyCustomRule,
Expand Down Expand Up @@ -371,12 +408,23 @@ export const internals = Joi.extend([

base: Joi.array(),
coerce(value: any, state: State, options: ValidationOptions) {
// If value isn't defined, let Joi handle default value if it's defined.
if (value !== undefined && !Array.isArray(value)) {
return this.createError('array.base', { value }, state, options);
if (value === undefined || Array.isArray(value)) {
return value;
}

return value;
if (options.convert && typeof value === 'string') {
try {
const parsed = JSON.parse(value);
if (Array.isArray(parsed)) {
return parsed;
}
return this.createError('array.base', { value: parsed }, state, options);
} catch (e) {
return this.createError('array.parse', { value }, state, options);
}
}

return this.createError('array.base', { value }, state, options);
},
rules: [anyCustomRule],
},
Expand Down

This file was deleted.

This file was deleted.

This file was deleted.

58 changes: 49 additions & 9 deletions packages/kbn-config-schema/src/types/array_type.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,29 +24,65 @@ test('returns value if it matches the type', () => {
expect(type.validate(['foo', 'bar', 'baz'])).toEqual(['foo', 'bar', 'baz']);
});

test('properly parse the value if input is a string', () => {
const type = schema.arrayOf(schema.string());
expect(type.validate('["foo", "bar", "baz"]')).toEqual(['foo', 'bar', 'baz']);
});

test('fails if wrong input type', () => {
const type = schema.arrayOf(schema.string());
expect(() => type.validate('test')).toThrowErrorMatchingSnapshot();
expect(() => type.validate(12)).toThrowErrorMatchingInlineSnapshot(
`"expected value of type [array] but got [number]"`
);
});

test('fails if string input cannot be parsed', () => {
const type = schema.arrayOf(schema.string());
expect(() => type.validate('test')).toThrowErrorMatchingInlineSnapshot(
`"could not parse array value from [test]"`
);
});

test('fails with correct type if parsed input is not an array', () => {
const type = schema.arrayOf(schema.string());
expect(() => type.validate('{"foo": "bar"}')).toThrowErrorMatchingInlineSnapshot(
`"expected value of type [array] but got [Object]"`
);
});

test('includes namespace in failure when wrong top-level type', () => {
const type = schema.arrayOf(schema.string());
expect(() => type.validate('test', {}, 'foo-namespace')).toThrowErrorMatchingSnapshot();
expect(() => type.validate('test', {}, 'foo-namespace')).toThrowErrorMatchingInlineSnapshot(
`"[foo-namespace]: could not parse array value from [test]"`
);
Comment on lines -34 to +57
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

});

test('includes namespace in failure when wrong item type', () => {
const type = schema.arrayOf(schema.string());
expect(() => type.validate([123], {}, 'foo-namespace')).toThrowErrorMatchingSnapshot();
expect(() => type.validate([123], {}, 'foo-namespace')).toThrowErrorMatchingInlineSnapshot(
`"[foo-namespace.0]: expected value of type [string] but got [number]"`
);
});

test('fails if wrong type of content in array', () => {
const type = schema.arrayOf(schema.string());
expect(() => type.validate([1, 2, 3])).toThrowErrorMatchingSnapshot();
expect(() => type.validate([1, 2, 3])).toThrowErrorMatchingInlineSnapshot(
`"[0]: expected value of type [string] but got [number]"`
);
});

test('fails when parsing if wrong type of content in array', () => {
const type = schema.arrayOf(schema.string());
expect(() => type.validate('[1, 2, 3]')).toThrowErrorMatchingInlineSnapshot(
`"[0]: expected value of type [string] but got [number]"`
);
});

test('fails if mixed types of content in array', () => {
const type = schema.arrayOf(schema.string());
expect(() => type.validate(['foo', 'bar', true, {}])).toThrowErrorMatchingSnapshot();
expect(() => type.validate(['foo', 'bar', true, {}])).toThrowErrorMatchingInlineSnapshot(
`"[2]: expected value of type [string] but got [boolean]"`
);
});

test('returns empty array if input is empty but type has default value', () => {
Expand All @@ -61,7 +97,9 @@ test('returns empty array if input is empty even if type is required', () => {

test('fails for null values if optional', () => {
const type = schema.arrayOf(schema.maybe(schema.string()));
expect(() => type.validate([null])).toThrowErrorMatchingSnapshot();
expect(() => type.validate([null])).toThrowErrorMatchingInlineSnapshot(
`"[0]: expected value of type [string] but got [null]"`
);
});

test('handles default values for undefined values', () => {
Expand Down Expand Up @@ -108,7 +146,9 @@ test('object within array with required', () => {

const value = [{}];

expect(() => type.validate(value)).toThrowErrorMatchingSnapshot();
expect(() => type.validate(value)).toThrowErrorMatchingInlineSnapshot(
`"[0.foo]: expected value of type [string] but got [undefined]"`
);
});

describe('#minSize', () => {
Expand All @@ -119,7 +159,7 @@ describe('#minSize', () => {
test('returns error when fewer items', () => {
expect(() =>
schema.arrayOf(schema.string(), { minSize: 2 }).validate(['foo'])
).toThrowErrorMatchingSnapshot();
).toThrowErrorMatchingInlineSnapshot(`"array size is [1], but cannot be smaller than [2]"`);
});
});

Expand All @@ -131,6 +171,6 @@ describe('#maxSize', () => {
test('returns error when more items', () => {
expect(() =>
schema.arrayOf(schema.string(), { maxSize: 1 }).validate(['foo', 'bar'])
).toThrowErrorMatchingSnapshot();
).toThrowErrorMatchingInlineSnapshot(`"array size is [2], but cannot be greater than [1]"`);
});
});
2 changes: 2 additions & 0 deletions packages/kbn-config-schema/src/types/array_type.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ export class ArrayType<T> extends Type<T[]> {
case 'any.required':
case 'array.base':
return `expected value of type [array] but got [${typeDetect(value)}]`;
case 'array.parse':
return `could not parse array value from [${value}]`;
case 'array.min':
return `array size is [${value.length}], but cannot be smaller than [${limit}]`;
case 'array.max':
Expand Down
Loading