Update schemas boolean, byteSize, and duration to coerce strings (#54177

)

* Update Duration to coerce number strings to numbers (in millis)

* Coerce in a way that's consistent with kbn-config-schema

* Update ByteSizeValue to coerce strings to numbers

* Update Boolean to coerce strings to boolean values

* Fix Jest test

* Address PR review feedback

* Whoops

* Whoops 2

* Whoops 3

packages/kbn-config-schema/README.md

@@ -156,6 +156,9 @@ __Usage:__
 const valueSchema = schema.boolean({ defaultValue: false });
 ```
 
+__Notes:__
+* The `schema.boolean()` also supports a string as input if it equals `'true'` or `'false'` (case-insensitive).
+
 #### `schema.literal()`
 
 Validates input data as a [string](https://www.typescriptlang.org/docs/handbook/advanced-types.html#string-literal-types), [numeric](https://www.typescriptlang.org/docs/handbook/advanced-types.html#numeric-literal-types) or boolean literal.
@@ -397,7 +400,7 @@ const valueSchema = schema.byteSize({ min: '3kb' });
 ```
 
 __Notes:__
-* The string value for `schema.byteSize()` and its options supports the following prefixes: `b`, `kb`, `mb`, `gb` and `tb`.
+* The string value for `schema.byteSize()` and its options supports the following optional suffixes: `b`, `kb`, `mb`, `gb` and `tb`. The default suffix is `b`.
 * The number value is treated as a number of bytes and hence should be a positive integer, e.g. `100` is equal to `'100b'`.
 * Currently you cannot specify zero bytes with a string format and should use number `0` instead.
 
@@ -417,7 +420,7 @@ const valueSchema = schema.duration({ defaultValue: '70ms' });
 ```
 
 __Notes:__
-* The string value for `schema.duration()` supports the following prefixes: `ms`, `s`, `m`, `h`, `d`, `w`, `M` and `Y`.
+* The string value for `schema.duration()` supports the following optional suffixes: `ms`, `s`, `m`, `h`, `d`, `w`, `M` and `Y`. The default suffix is `ms`.
 * The number value is treated as a number of milliseconds and hence should be a positive integer, e.g. `100` is equal to `'100ms'`.
 
 #### `schema.conditional()`

packages/kbn-config-schema/src/byte_size_value/index.test.ts

@@ -20,6 +20,10 @@
 import { ByteSizeValue } from '.';
 
 describe('parsing units', () => {
+  test('number string (bytes)', () => {
+    expect(ByteSizeValue.parse('123').getValueInBytes()).toBe(123);
+  });
+
   test('bytes', () => {
     expect(ByteSizeValue.parse('123b').getValueInBytes()).toBe(123);
   });
@@ -37,12 +41,8 @@ describe('parsing units', () => {
     expect(ByteSizeValue.parse('1gb').getValueInBytes()).toBe(1073741824);
   });
 
-  test('throws an error when no unit specified', () => {
-    expect(() => ByteSizeValue.parse('123')).toThrowError('could not parse byte size value');
-  });
-
   test('throws an error when unsupported unit specified', () => {
-    expect(() => ByteSizeValue.parse('1tb')).toThrowError('could not parse byte size value');
+    expect(() => ByteSizeValue.parse('1tb')).toThrowErrorMatchingSnapshot();
   });
 });
 

packages/kbn-config-schema/src/byte_size_value/index.ts

@@ -35,9 +35,14 @@ export class ByteSizeValue {
   public static parse(text: string): ByteSizeValue {
     const match = /([1-9][0-9]*)(b|kb|mb|gb)/.exec(text);
     if (!match) {
-      throw new Error(
-        `could not parse byte size value [${text}]. Value must be a safe positive integer.`
-      );
+      const number = Number(text);
+      if (typeof number !== 'number' || isNaN(number)) {
+        throw new Error(
+          `Failed to parse [${text}] as byte value. Value must be either number of bytes, or follow the format <count>[b|kb|mb|gb] ` +
+            `(e.g., '1024kb', '200mb', '1gb'), where the number is a safe positive integer.`
+        );
+      }
+      return new ByteSizeValue(number);
     }
 
     const value = parseInt(match[1], 0);
@@ -49,8 +54,7 @@ export class ByteSizeValue {
   constructor(private readonly valueInBytes: number) {
     if (!Number.isSafeInteger(valueInBytes) || valueInBytes < 0) {
       throw new Error(
-        `Value in bytes is expected to be a safe positive integer, ` +
-          `but provided [${valueInBytes}]`
+        `Value in bytes is expected to be a safe positive integer, but provided [${valueInBytes}].`
       );
     }
   }

packages/kbn-config-schema/src/duration/index.ts

@@ -25,10 +25,14 @@ const timeFormatRegex = /^(0|[1-9][0-9]*)(ms|s|m|h|d|w|M|Y)$/;
 function stringToDuration(text: string) {
   const result = timeFormatRegex.exec(text);
   if (!result) {
-    throw new Error(
-      `Failed to parse [${text}] as time value. ` +
-        `Format must be <count>[ms|s|m|h|d|w|M|Y] (e.g. '70ms', '5s', '3d', '1Y')`
-    );
+    const number = Number(text);
+    if (typeof number !== 'number' || isNaN(number)) {
+      throw new Error(
+        `Failed to parse [${text}] as time value. Value must be a duration in milliseconds, or follow the format ` +
+          `<count>[ms|s|m|h|d|w|M|Y] (e.g. '70ms', '5s', '3d', '1Y'), where the duration is a safe positive integer.`
+      );
+    }
+    return numberToDuration(number);
   }
 
   const count = parseInt(result[1], 0);
@@ -40,8 +44,7 @@ function stringToDuration(text: string) {
 function numberToDuration(numberMs: number) {
   if (!Number.isSafeInteger(numberMs) || numberMs < 0) {
     throw new Error(
-      `Failed to parse [${numberMs}] as time value. ` +
-        `Value should be a safe positive integer number.`
+      `Value in milliseconds is expected to be a safe positive integer, but provided [${numberMs}].`
     );
   }
 

packages/kbn-config-schema/src/internals/index.ts

@@ -82,7 +82,23 @@ export const internals = Joi.extend([
     base: Joi.boolean(),
     coerce(value: any, state: State, options: ValidationOptions) {
       // If value isn't defined, let Joi handle default value if it's defined.
-      if (value !== undefined && typeof value !== 'boolean') {
+      if (value === undefined) {
+        return value;
+      }
+
+      // Allow strings 'true' and 'false' to be coerced to booleans (case-insensitive).
+
+      // From Joi docs on `Joi.boolean`:
+      // > Generates a schema object that matches a boolean data type. Can also
+      // >  be called via bool(). If the validation convert option is on
+      // > (enabled by default), a string (either "true" or "false") will be
+      // converted to a boolean if specified.
+      if (typeof value === 'string') {
+        const normalized = value.toLowerCase();
+        value = normalized === 'true' ? true : normalized === 'false' ? false : value;
+      }
+
+      if (typeof value !== 'boolean') {
         return this.createError('boolean.base', { value }, state, options);
       }
 

packages/kbn-config-schema/src/types/boolean_type.test.ts

@@ -23,6 +23,17 @@ test('returns value by default', () => {
   expect(schema.boolean().validate(true)).toBe(true);
 });
 
+test('handles boolean strings', () => {
+  expect(schema.boolean().validate('true')).toBe(true);
+  expect(schema.boolean().validate('TRUE')).toBe(true);
+  expect(schema.boolean().validate('True')).toBe(true);
+  expect(schema.boolean().validate('TrUe')).toBe(true);
+  expect(schema.boolean().validate('false')).toBe(false);
+  expect(schema.boolean().validate('FALSE')).toBe(false);
+  expect(schema.boolean().validate('False')).toBe(false);
+  expect(schema.boolean().validate('FaLse')).toBe(false);
+});
+
 test('is required by default', () => {
   expect(() => schema.boolean().validate(undefined)).toThrowErrorMatchingSnapshot();
 });
@@ -49,4 +60,8 @@ test('returns error when not boolean', () => {
   expect(() => schema.boolean().validate([1, 2, 3])).toThrowErrorMatchingSnapshot();
 
   expect(() => schema.boolean().validate('abc')).toThrowErrorMatchingSnapshot();
+
+  expect(() => schema.boolean().validate(0)).toThrowErrorMatchingSnapshot();
+
+  expect(() => schema.boolean().validate('no')).toThrowErrorMatchingSnapshot();
 });

packages/kbn-config-schema/src/types/byte_size_type.test.ts

@@ -23,7 +23,15 @@ import { ByteSizeValue } from '../byte_size_value';
 const { byteSize } = schema;
 
 test('returns value by default', () => {
-  expect(byteSize().validate('123b')).toMatchSnapshot();
+  expect(byteSize().validate('123b')).toEqual(new ByteSizeValue(123));
+});
+
+test('handles numeric strings', () => {
+  expect(byteSize().validate('123')).toEqual(new ByteSizeValue(123));
+});
+
+test('handles numbers', () => {
+  expect(byteSize().validate(123)).toEqual(new ByteSizeValue(123));
 });
 
 test('is required by default', () => {
@@ -51,6 +59,14 @@ describe('#defaultValue', () => {
     ).toMatchSnapshot();
   });
 
+  test('can be a string-formatted number', () => {
+    expect(
+      byteSize({
+        defaultValue: '1024',
+      }).validate(undefined)
+    ).toMatchSnapshot();
+  });
+
   test('can be a number', () => {
     expect(
       byteSize({
@@ -88,7 +104,7 @@ describe('#max', () => {
   });
 });
 
-test('returns error when not string or positive safe integer', () => {
+test('returns error when not valid string or positive safe integer', () => {
   expect(() => byteSize().validate(-123)).toThrowErrorMatchingSnapshot();
 
   expect(() => byteSize().validate(NaN)).toThrowErrorMatchingSnapshot();
@@ -100,4 +116,8 @@ test('returns error when not string or positive safe integer', () => {
   expect(() => byteSize().validate([1, 2, 3])).toThrowErrorMatchingSnapshot();
 
   expect(() => byteSize().validate(/abc/)).toThrowErrorMatchingSnapshot();
+
+  expect(() => byteSize().validate('123foo')).toThrowErrorMatchingSnapshot();
+
+  expect(() => byteSize().validate('123 456')).toThrowErrorMatchingSnapshot();
 });

packages/kbn-config-schema/src/types/duration_type.test.ts

@@ -23,7 +23,15 @@ import { schema } from '..';
 const { duration, object, contextRef, siblingRef } = schema;
 
 test('returns value by default', () => {
-  expect(duration().validate('123s')).toMatchSnapshot();
+  expect(duration().validate('123s')).toEqual(momentDuration(123000));
+});
+
+test('handles numeric string', () => {
+  expect(duration().validate('123000')).toEqual(momentDuration(123000));
+});
+
+test('handles number', () => {
+  expect(duration().validate(123000)).toEqual(momentDuration(123000));
 });
 
 test('is required by default', () => {
@@ -51,6 +59,14 @@ describe('#defaultValue', () => {
     ).toMatchSnapshot();
   });
 
+  test('can be a string-formatted number', () => {
+    expect(
+      duration({
+        defaultValue: '600',
+      }).validate(undefined)
+    ).toMatchSnapshot();
+  });
+
   test('can be a number', () => {
     expect(
       duration({
@@ -124,7 +140,7 @@ Object {
   });
 });
 
-test('returns error when not string or non-safe positive integer', () => {
+test('returns error when not valid string or non-safe positive integer', () => {
   expect(() => duration().validate(-123)).toThrowErrorMatchingSnapshot();
 
   expect(() => duration().validate(NaN)).toThrowErrorMatchingSnapshot();
@@ -136,4 +152,8 @@ test('returns error when not string or non-safe positive integer', () => {
   expect(() => duration().validate([1, 2, 3])).toThrowErrorMatchingSnapshot();
 
   expect(() => duration().validate(/abc/)).toThrowErrorMatchingSnapshot();
+
+  expect(() => duration().validate('123foo')).toThrowErrorMatchingSnapshot();
+
+  expect(() => duration().validate('123 456')).toThrowErrorMatchingSnapshot();
 });

x-pack/legacy/plugins/actions/server/builtin_action_types/es_index.test.ts

@@ -142,7 +142,7 @@ describe('params validation', () => {
     );
 
     expect(() => {
-      validateParams(actionType, { refresh: 'true' });
+      validateParams(actionType, { refresh: 'foo' });
     }).toThrowErrorMatchingInlineSnapshot(
       `"error validating action params: [refresh]: expected value of type [boolean] but got [string]"`
     );