Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix maybe behavior with object type #55932

Merged
merged 11 commits into from
Feb 14, 2020
6 changes: 3 additions & 3 deletions packages/kbn-config-schema/src/types/duration_type.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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",
Expand All @@ -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",
Expand Down
38 changes: 38 additions & 0 deletions packages/kbn-config-schema/src/types/maybe_type.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
joshdover marked this conversation as resolved.
Show resolved Hide resolved

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({});
});
});
2 changes: 1 addition & 1 deletion packages/kbn-config-schema/src/types/maybe_type.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export class MaybeType<V> extends Type<V | undefined> {
type
.getSchema()
.optional()
.default()
.default(() => undefined, 'undefined')
);
}
}
22 changes: 21 additions & 1 deletion packages/kbn-config-schema/src/types/object_type.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({});
joshdover marked this conversation as resolved.
Show resolved Hide resolved
});

test('properly parse the value if input is a string', () => {
const type = schema.object({
name: schema.string(),
Expand Down Expand Up @@ -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]"`
);
Expand Down
6 changes: 3 additions & 3 deletions packages/kbn-config-schema/src/types/object_type.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 0 additions & 2 deletions x-pack/plugins/event_log/server/event_logger.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,6 @@ describe('EventLogger', () => {
kibana: {
server_uuid: '424-24-2424',
},
error: {},
user: {},
});

const $timeStamp = event!['@timestamp']!;
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/security/server/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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() })),
joshdover marked this conversation as resolved.
Show resolved Hide resolved
saml: providerOptionsSchema(
'saml',
schema.object({
Expand Down