From 97a55c9e76058c7369b17cdd608a07ff083b2686 Mon Sep 17 00:00:00 2001 From: Aleh Zasypkin Date: Mon, 28 Oct 2019 10:29:39 +0100 Subject: [PATCH] Correctly process paths in `map_of` and `record_of`. Do not swallow and use indent for nested `one_of` error messages. --- .../src/errors/validation_error.ts | 7 ++++--- .../src/types/map_of_type.test.ts | 20 +++++++++++++++++++ .../kbn-config-schema/src/types/map_type.ts | 2 +- .../src/types/one_of_type.test.ts | 17 ++++++++++++++++ .../src/types/record_of_type.test.ts | 20 +++++++++++++++++++ .../src/types/record_type.ts | 2 +- .../kbn-config-schema/src/types/union_type.ts | 4 +++- .../routes/api/external/copy_to_space.test.ts | 2 +- 8 files changed, 67 insertions(+), 7 deletions(-) diff --git a/packages/kbn-config-schema/src/errors/validation_error.ts b/packages/kbn-config-schema/src/errors/validation_error.ts index 39aac67c208f57..d688d022da85cd 100644 --- a/packages/kbn-config-schema/src/errors/validation_error.ts +++ b/packages/kbn-config-schema/src/errors/validation_error.ts @@ -20,17 +20,18 @@ import { SchemaError, SchemaTypeError, SchemaTypesError } from '.'; export class ValidationError extends SchemaError { - public static extractMessage(error: SchemaTypeError, namespace?: string) { + private static extractMessage(error: SchemaTypeError, namespace?: string, level?: number) { const path = typeof namespace === 'string' ? [namespace, ...error.path] : error.path; let message = error.message; if (error instanceof SchemaTypesError) { + const indentLevel = level || 0; const childErrorMessages = error.errors.map(childError => - ValidationError.extractMessage(childError, namespace) + ValidationError.extractMessage(childError, namespace, indentLevel + 1) ); message = `${message}\n${childErrorMessages - .map(childErrorMessage => `- ${childErrorMessage}`) + .map(childErrorMessage => `${' '.repeat(indentLevel)}- ${childErrorMessage}`) .join('\n')}`; } diff --git a/packages/kbn-config-schema/src/types/map_of_type.test.ts b/packages/kbn-config-schema/src/types/map_of_type.test.ts index 1b72d39fcec263..6b9b700efdc3c3 100644 --- a/packages/kbn-config-schema/src/types/map_of_type.test.ts +++ b/packages/kbn-config-schema/src/types/map_of_type.test.ts @@ -108,3 +108,23 @@ test('object within mapOf', () => { expect(type.validate(value)).toEqual(expected); }); + +test('error preserves full path', () => { + const type = schema.object({ + grandParentKey: schema.object({ + parentKey: schema.mapOf(schema.string({ minLength: 2 }), schema.number()), + }), + }); + + expect(() => + type.validate({ grandParentKey: { parentKey: { a: 'some-value' } } }) + ).toThrowErrorMatchingInlineSnapshot( + `"[grandParentKey.parentKey.key(\\"a\\")]: value is [a] but it must have a minimum length of [2]."` + ); + + expect(() => + type.validate({ grandParentKey: { parentKey: { ab: 'some-value' } } }) + ).toThrowErrorMatchingInlineSnapshot( + `"[grandParentKey.parentKey.ab]: expected value of type [number] but got [string]"` + ); +}); diff --git a/packages/kbn-config-schema/src/types/map_type.ts b/packages/kbn-config-schema/src/types/map_type.ts index 3acf14a69125e1..c637eccb795715 100644 --- a/packages/kbn-config-schema/src/types/map_type.ts +++ b/packages/kbn-config-schema/src/types/map_type.ts @@ -50,7 +50,7 @@ export class MapOfType extends Type> { return `expected value of type [Map] or [object] but got [${typeDetect(value)}]`; case 'map.key': case 'map.value': - const childPathWithIndex = reason.path.slice(); + const childPathWithIndex = path.slice(); childPathWithIndex.splice( path.length, 0, diff --git a/packages/kbn-config-schema/src/types/one_of_type.test.ts b/packages/kbn-config-schema/src/types/one_of_type.test.ts index 72119e761590b1..c84ae49df7aefb 100644 --- a/packages/kbn-config-schema/src/types/one_of_type.test.ts +++ b/packages/kbn-config-schema/src/types/one_of_type.test.ts @@ -125,3 +125,20 @@ test('fails if not matching literal', () => { expect(() => type.validate('bar')).toThrowErrorMatchingSnapshot(); }); + +test('fails if nested union type fail', () => { + const type = schema.oneOf([ + schema.oneOf([schema.boolean()]), + schema.oneOf([schema.oneOf([schema.object({}), schema.number()])]), + ]); + + expect(() => type.validate('aaa')).toThrowErrorMatchingInlineSnapshot(` +"types that failed validation: +- [0]: types that failed validation: + - [0]: expected value of type [boolean] but got [string] +- [1]: types that failed validation: + - [0]: types that failed validation: + - [0]: expected a plain object value, but found [string] instead. + - [1]: expected value of type [number] but got [string]" +`); +}); diff --git a/packages/kbn-config-schema/src/types/record_of_type.test.ts b/packages/kbn-config-schema/src/types/record_of_type.test.ts index 036e34213411fd..2172160e8d1810 100644 --- a/packages/kbn-config-schema/src/types/record_of_type.test.ts +++ b/packages/kbn-config-schema/src/types/record_of_type.test.ts @@ -120,3 +120,23 @@ test('object within recordOf', () => { expect(type.validate(value)).toEqual({ foo: { bar: 123 } }); }); + +test('error preserves full path', () => { + const type = schema.object({ + grandParentKey: schema.object({ + parentKey: schema.recordOf(schema.string({ minLength: 2 }), schema.number()), + }), + }); + + expect(() => + type.validate({ grandParentKey: { parentKey: { a: 'some-value' } } }) + ).toThrowErrorMatchingInlineSnapshot( + `"[grandParentKey.parentKey.key(\\"a\\")]: value is [a] but it must have a minimum length of [2]."` + ); + + expect(() => + type.validate({ grandParentKey: { parentKey: { ab: 'some-value' } } }) + ).toThrowErrorMatchingInlineSnapshot( + `"[grandParentKey.parentKey.ab]: expected value of type [number] but got [string]"` + ); +}); diff --git a/packages/kbn-config-schema/src/types/record_type.ts b/packages/kbn-config-schema/src/types/record_type.ts index cefb0a7921f4d4..82e585f685c569 100644 --- a/packages/kbn-config-schema/src/types/record_type.ts +++ b/packages/kbn-config-schema/src/types/record_type.ts @@ -42,7 +42,7 @@ export class RecordOfType extends Type> { return `expected value of type [object] but got [${typeDetect(value)}]`; case 'record.key': case 'record.value': - const childPathWithIndex = reason.path.slice(); + const childPathWithIndex = path.slice(); childPathWithIndex.splice( path.length, 0, diff --git a/packages/kbn-config-schema/src/types/union_type.ts b/packages/kbn-config-schema/src/types/union_type.ts index e6efb4afb66aa9..f4de829204e80b 100644 --- a/packages/kbn-config-schema/src/types/union_type.ts +++ b/packages/kbn-config-schema/src/types/union_type.ts @@ -41,7 +41,9 @@ export class UnionType>, T> extends Type { const childPathWithIndex = e.path.slice(); childPathWithIndex.splice(path.length, 0, index.toString()); - return new SchemaTypeError(e.message, childPathWithIndex); + return e instanceof SchemaTypesError + ? new SchemaTypesError(e.message, childPathWithIndex, e.errors) + : new SchemaTypeError(e.message, childPathWithIndex); }) ); } diff --git a/x-pack/plugins/spaces/server/routes/api/external/copy_to_space.test.ts b/x-pack/plugins/spaces/server/routes/api/external/copy_to_space.test.ts index 92744aa9f0fc50..54d9654005f893 100644 --- a/x-pack/plugins/spaces/server/routes/api/external/copy_to_space.test.ts +++ b/x-pack/plugins/spaces/server/routes/api/external/copy_to_space.test.ts @@ -337,7 +337,7 @@ describe('copy to space', () => { expect(() => resolveConflicts.routeValidation.body!.validate(payload) ).toThrowErrorMatchingInlineSnapshot( - `"[key(\\"invalid-space-id!@#$%^&*()\\")]: Invalid space id: invalid-space-id!@#$%^&*()"` + `"[retries.key(\\"invalid-space-id!@#$%^&*()\\")]: Invalid space id: invalid-space-id!@#$%^&*()"` ); });