Fix maybe behavior with object type (#55932)

elastic · Feb 14, 2020 · 6bd09d6 · 6bd09d6
1 parent 0abfd3c
commit 6bd09d6
Show file tree

Hide file tree

Showing 7 changed files with 67 additions and 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
@@ -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
@@ -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
@@ -25,7 +25,7 @@ export class MaybeType<V> extends Type<V | undefined> {
       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
@@ -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,21 +117,36 @@ 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(),
     }),
   });
   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
@@ -33,23 +33,23 @@ export type ObjectResultType<P extends Props> = Readonly<{ [K in keyof P]: TypeO
 export type ObjectTypeOptions<P extends Props = any> = TypeOptions<
   { [K in keyof P]: TypeOf<P[K]> }
 > & {
+  /** Should uknown keys not be defined in the schema be allowed. Defaults to `false` */
   allowUnknowns?: boolean;
 };
 
 export class ObjectType<P extends Props = any> extends Type<ObjectResultType<P>> {
   private props: Record<string, AnySchema>;
 
-  constructor(props: P, options: ObjectTypeOptions<P> = {}) {
+  constructor(props: P, { allowUnknowns = false, ...typeOptions }: ObjectTypeOptions<P> = {}) {
     const schemaKeys = {} as Record<string, AnySchema>;
     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
@@ -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
@@ -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({