From 6bd09d616ffb93124d9fd36db77da11c883eb9e1 Mon Sep 17 00:00:00 2001 From: Josh Dover Date: Fri, 14 Feb 2020 12:06:59 -0700 Subject: [PATCH] Fix maybe behavior with object type (#55932) --- .../src/types/duration_type.test.ts | 6 +-- .../src/types/maybe_type.test.ts | 38 +++++++++++++++++++ .../kbn-config-schema/src/types/maybe_type.ts | 2 +- .../src/types/object_type.test.ts | 22 ++++++++++- .../src/types/object_type.ts | 6 +-- .../event_log/server/event_logger.test.ts | 2 - x-pack/plugins/security/server/config.ts | 2 +- 7 files changed, 67 insertions(+), 11 deletions(-) diff --git a/packages/kbn-config-schema/src/types/duration_type.test.ts b/packages/kbn-config-schema/src/types/duration_type.test.ts index 09e92ce727f2a2..57e917dc99b2b3 100644 --- a/packages/kbn-config-schema/src/types/duration_type.test.ts +++ b/packages/kbn-config-schema/src/types/duration_type.test.ts @@ -101,7 +101,7 @@ describe('#defaultValue', () => { source: duration({ defaultValue: 600 }), target: duration({ defaultValue: siblingRef('source') }), fromContext: duration({ defaultValue: contextRef('val') }), - }).validate(undefined, { val: momentDuration(700, 'ms') }) + }).validate({}, { val: momentDuration(700, 'ms') }) ).toMatchInlineSnapshot(` Object { "fromContext": "PT0.7S", @@ -115,7 +115,7 @@ Object { source: duration({ defaultValue: '1h' }), target: duration({ defaultValue: siblingRef('source') }), fromContext: duration({ defaultValue: contextRef('val') }), - }).validate(undefined, { val: momentDuration(2, 'hour') }) + }).validate({}, { val: momentDuration(2, 'hour') }) ).toMatchInlineSnapshot(` Object { "fromContext": "PT2H", @@ -129,7 +129,7 @@ Object { source: duration({ defaultValue: momentDuration(1, 'hour') }), target: duration({ defaultValue: siblingRef('source') }), fromContext: duration({ defaultValue: contextRef('val') }), - }).validate(undefined, { val: momentDuration(2, 'hour') }) + }).validate({}, { val: momentDuration(2, 'hour') }) ).toMatchInlineSnapshot(` Object { "fromContext": "PT2H", diff --git a/packages/kbn-config-schema/src/types/maybe_type.test.ts b/packages/kbn-config-schema/src/types/maybe_type.test.ts index ecc1d218e186d4..c35fa18593520a 100644 --- a/packages/kbn-config-schema/src/types/maybe_type.test.ts +++ b/packages/kbn-config-schema/src/types/maybe_type.test.ts @@ -60,3 +60,41 @@ test('includes namespace in failure', () => { const type = schema.maybe(schema.string()); expect(() => type.validate(null, {}, 'foo-namespace')).toThrowErrorMatchingSnapshot(); }); + +describe('maybe + object', () => { + test('returns undefined if undefined object', () => { + const type = schema.maybe(schema.object({})); + expect(type.validate(undefined)).toEqual(undefined); + }); + + test('returns undefined if undefined object with no defaults', () => { + const type = schema.maybe( + schema.object({ + type: schema.string(), + id: schema.string(), + }) + ); + + expect(type.validate(undefined)).toEqual(undefined); + }); + + test('returns empty object if maybe keys', () => { + const type = schema.object({ + name: schema.maybe(schema.string()), + }); + expect(type.validate({})).toEqual({}); + }); + + test('returns empty object if maybe nested object', () => { + const type = schema.object({ + name: schema.maybe( + schema.object({ + type: schema.string(), + id: schema.string(), + }) + ), + }); + + expect(type.validate({})).toEqual({}); + }); +}); diff --git a/packages/kbn-config-schema/src/types/maybe_type.ts b/packages/kbn-config-schema/src/types/maybe_type.ts index 06a93691102036..415f6315c57231 100644 --- a/packages/kbn-config-schema/src/types/maybe_type.ts +++ b/packages/kbn-config-schema/src/types/maybe_type.ts @@ -25,7 +25,7 @@ export class MaybeType extends Type { type .getSchema() .optional() - .default() + .default(() => undefined, 'undefined') ); } } diff --git a/packages/kbn-config-schema/src/types/object_type.test.ts b/packages/kbn-config-schema/src/types/object_type.test.ts index 5786984cf7ebdc..64739d7a4c4daa 100644 --- a/packages/kbn-config-schema/src/types/object_type.test.ts +++ b/packages/kbn-config-schema/src/types/object_type.test.ts @@ -30,6 +30,11 @@ test('returns value by default', () => { expect(type.validate(value)).toEqual({ name: 'test' }); }); +test('returns empty object if undefined', () => { + const type = schema.object({}); + expect(type.validate(undefined)).toEqual({}); +}); + test('properly parse the value if input is a string', () => { const type = schema.object({ name: schema.string(), @@ -112,14 +117,26 @@ test('undefined object within object', () => { }), }); + expect(type.validate(undefined)).toEqual({ + foo: { + bar: 'hello world', + }, + }); + expect(type.validate({})).toEqual({ foo: { bar: 'hello world', }, }); + + expect(type.validate({ foo: {} })).toEqual({ + foo: { + bar: 'hello world', + }, + }); }); -test('object within object with required', () => { +test('object within object with key without defaultValue', () => { const type = schema.object({ foo: schema.object({ bar: schema.string(), @@ -127,6 +144,9 @@ test('object within object with required', () => { }); const value = { foo: {} }; + expect(() => type.validate(undefined)).toThrowErrorMatchingInlineSnapshot( + `"[foo.bar]: expected value of type [string] but got [undefined]"` + ); expect(() => type.validate(value)).toThrowErrorMatchingInlineSnapshot( `"[foo.bar]: expected value of type [string] but got [undefined]"` ); diff --git a/packages/kbn-config-schema/src/types/object_type.ts b/packages/kbn-config-schema/src/types/object_type.ts index d2e6c708c263ca..4f3d68a6bac97d 100644 --- a/packages/kbn-config-schema/src/types/object_type.ts +++ b/packages/kbn-config-schema/src/types/object_type.ts @@ -33,23 +33,23 @@ export type ObjectResultType

= Readonly<{ [K in keyof P]: TypeO export type ObjectTypeOptions

= TypeOptions< { [K in keyof P]: TypeOf } > & { + /** Should uknown keys not be defined in the schema be allowed. Defaults to `false` */ allowUnknowns?: boolean; }; export class ObjectType

extends Type> { private props: Record; - constructor(props: P, options: ObjectTypeOptions

= {}) { + constructor(props: P, { allowUnknowns = false, ...typeOptions }: ObjectTypeOptions

= {}) { const schemaKeys = {} as Record; for (const [key, value] of Object.entries(props)) { schemaKeys[key] = value.getSchema(); } - const { allowUnknowns, ...typeOptions } = options; const schema = internals .object() .keys(schemaKeys) - .optional() .default() + .optional() .unknown(Boolean(allowUnknowns)); super(schema, typeOptions); diff --git a/x-pack/plugins/event_log/server/event_logger.test.ts b/x-pack/plugins/event_log/server/event_logger.test.ts index c2de8d4dfd12bd..97e52ad04dd08a 100644 --- a/x-pack/plugins/event_log/server/event_logger.test.ts +++ b/x-pack/plugins/event_log/server/event_logger.test.ts @@ -57,8 +57,6 @@ describe('EventLogger', () => { kibana: { server_uuid: '424-24-2424', }, - error: {}, - user: {}, }); const $timeStamp = event!['@timestamp']!; diff --git a/x-pack/plugins/security/server/config.ts b/x-pack/plugins/security/server/config.ts index 4f1c25702ae974..db8c48f314d7ce 100644 --- a/x-pack/plugins/security/server/config.ts +++ b/x-pack/plugins/security/server/config.ts @@ -41,7 +41,7 @@ export const ConfigSchema = schema.object( secureCookies: schema.boolean({ defaultValue: false }), authc: schema.object({ providers: schema.arrayOf(schema.string(), { defaultValue: ['basic'], minSize: 1 }), - oidc: providerOptionsSchema('oidc', schema.maybe(schema.object({ realm: schema.string() }))), + oidc: providerOptionsSchema('oidc', schema.object({ realm: schema.string() })), saml: providerOptionsSchema( 'saml', schema.object({