-
Notifications
You must be signed in to change notification settings - Fork 8.6k
[OAS/config-schema] Implement discriminated union #246216
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
Changes from all commits
4ec74ba
604d0d7
f2dc8c8
248f917
27ccdde
3f2c8d6
c74ecad
ce8cb20
4266cb2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -33,6 +33,7 @@ import type { | |
| TypeOptions, | ||
| URIOptions, | ||
| UnionTypeOptions, | ||
| DiscriminatedUnionTypeOptions, | ||
| } from './src/types'; | ||
| import { | ||
| AnyType, | ||
|
|
@@ -54,6 +55,7 @@ import { | |
| StringType, | ||
| Type, | ||
| UnionType, | ||
| DiscriminatedUnionType, | ||
| URIType, | ||
| StreamType, | ||
| Lazy, | ||
|
|
@@ -224,6 +226,21 @@ function oneOf<RTS extends Array<Type<any>>>( | |
| return new UnionType(types, options); | ||
| } | ||
|
|
||
| type ExtractTypeFromObjectType<T extends Array<ObjectType<any>>> = { | ||
| [K in keyof T]: T[K]['type']; | ||
| }; | ||
|
|
||
| type UnionOfObjectTypes<T extends Array<ObjectType<any>>> = ExtractTypeFromObjectType<T>[number]; | ||
|
|
||
| /** @deprecated This is an experimental feature */ | ||
| function discriminatedOneOf<T extends Array<ObjectType<any>>>( | ||
| discriminator: string, | ||
| types: T, | ||
| options?: DiscriminatedUnionTypeOptions<UnionOfObjectTypes<T>> | ||
| ): Type<UnionOfObjectTypes<T>> { | ||
| return new DiscriminatedUnionType(discriminator, types, options); | ||
| } | ||
|
Comment on lines
+236
to
+242
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This typing does not enforce that the schema objects contain the Enforcing this makes the types harder as I could not find a variadic type solution, so I ended up with this. function oneOfKind<
Discriminator extends string,
A extends PropsWithDiscriminator<Discriminator, Props>,
B extends PropsWithDiscriminator<Discriminator, Props>
>(
discriminator: Discriminator,
types: [ObjectType<A>, ObjectType<B>],
options?: UnionTypeOptions<ObjectResultUnionType<A | B>>
): Type<ObjectResultUnionType<A | B>>;
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I agree that's a nice improvement! |
||
|
|
||
| function allOf< | ||
| A extends Props, | ||
| B extends Props, | ||
|
|
@@ -428,6 +445,7 @@ export const schema = { | |
| number, | ||
| object, | ||
| oneOf, | ||
| discriminatedOneOf, | ||
| recordOf, | ||
| stream, | ||
| siblingRef, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,203 @@ | ||
| /* | ||
| * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
| * or more contributor license agreements. Licensed under the "Elastic License | ||
| * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side | ||
| * Public License v 1"; you may not use this file except in compliance with, at | ||
| * your election, the "Elastic License 2.0", the "GNU Affero General Public | ||
| * License v3.0 only", or the "Server Side Public License, v 1". | ||
| */ | ||
|
|
||
| import { schema } from '../..'; | ||
|
|
||
| test('handles single object', () => { | ||
| const type = schema.discriminatedOneOf('type', [ | ||
| schema.object({ type: schema.literal('foo'), age: schema.number() }), | ||
| ]); | ||
|
|
||
| expect(type.validate({ type: 'foo', age: 24 })).toEqual({ type: 'foo', age: 24 }); | ||
| }); | ||
|
|
||
| test('fails as expected with single object', () => { | ||
| const type = schema.discriminatedOneOf('type', [ | ||
| schema.object({ type: schema.literal('foo'), age: schema.number() }), | ||
| ]); | ||
|
|
||
| expect(() => type.validate({ type: 'foo', age: 'foo' })).toThrowErrorMatchingInlineSnapshot( | ||
| `"[age]: Error: expected value of type [number] but got [string]"` | ||
| ); | ||
| }); | ||
|
|
||
| test('handles multiple objects', () => { | ||
| const type = schema.discriminatedOneOf('type', [ | ||
| schema.object({ type: schema.literal('foo'), foo: schema.string() }), | ||
| schema.object({ type: schema.literal('bar'), bar: schema.number() }), | ||
| ]); | ||
|
|
||
| expect(type.validate({ type: 'foo', foo: 'test' })).toEqual({ type: 'foo', foo: 'test' }); | ||
| }); | ||
|
|
||
| test('handles catch-all pattern', () => { | ||
| const type = schema.discriminatedOneOf('type', [ | ||
| schema.object({ type: schema.literal('foo'), foo: schema.string() }), | ||
| schema.object({ type: schema.literal('bar'), bar: schema.number() }), | ||
| schema.object({ type: schema.string(), bar: schema.literal('catch all!') }), | ||
|
jloleysens marked this conversation as resolved.
|
||
| ]); | ||
|
|
||
| expect(type.validate({ type: 'whathaveyou', bar: 'catch all!' })).toEqual({ | ||
| type: 'whathaveyou', | ||
| bar: 'catch all!', | ||
| }); | ||
| }); | ||
|
|
||
| test('fails with less helpful error message when multiple catch-all patterns are provided', () => { | ||
| // Schema authors should avoid this antipattern | ||
| const type = schema.discriminatedOneOf('type', [ | ||
| schema.object({ type: schema.literal('foo'), foo: schema.string() }), | ||
| schema.object({ type: schema.literal('bar'), bar: schema.number() }), | ||
| schema.object({ type: schema.string(), bar: schema.literal('catch all!') }), | ||
| schema.object({ type: schema.string(), bar: schema.literal('catch all again!') }), | ||
| ]); | ||
|
|
||
| expect(() => type.validate({ type: 123, bar: 'catch all!' })).toThrowErrorMatchingInlineSnapshot( | ||
| `"value [123] did not match any of the allowed values for [type]: [Error: expected value to equal [foo], Error: expected value to equal [bar], Error: expected value of type [string] but got [number], Error: expected value of type [string] but got [number]]"` | ||
| ); | ||
| }); | ||
|
|
||
| test('fails with less helpful error message when multiple discriminators are matched', () => { | ||
| // Schema authors should avoid this antipattern | ||
| const type = schema.discriminatedOneOf('type', [ | ||
| schema.object({ type: schema.literal('foo'), foo: schema.string() }), | ||
| schema.object({ type: schema.literal('bar'), bar: schema.number() }), | ||
| schema.object({ type: schema.string(), bar: schema.number() }), | ||
| schema.object({ type: schema.string(), bar: schema.number() }), | ||
| ]); | ||
|
|
||
| expect(() => type.validate({ type: 'catchall', bar: 123 })).toThrowErrorMatchingInlineSnapshot( | ||
| `"value [catchall] matched more than one allowed type of [type]"` | ||
| ); | ||
| }); | ||
|
|
||
| test('handles multiple objects with the same type', () => { | ||
| // This defeats the purpose of the discriminator, but it will work | ||
| const type = schema.discriminatedOneOf('type', [ | ||
| schema.object({ type: schema.literal('foo'), age: schema.string() }), | ||
| schema.object({ type: schema.literal('foo'), age: schema.number() }), | ||
| ]); | ||
|
|
||
| expect(type.validate({ type: 'foo', age: 'foo' })).toEqual({ type: 'foo', age: 'foo' }); | ||
| }); | ||
|
|
||
| test('includes namespace in failure', () => { | ||
| const type = schema.discriminatedOneOf('type', [ | ||
| schema.object({ type: schema.literal('foo'), age: schema.string() }), | ||
| schema.object({ type: schema.literal('bar'), age: schema.number() }), | ||
| ]); | ||
|
|
||
| expect(() => | ||
| type.validate({ type: 'foo', age: 12 }, {}, 'foo-namespace') | ||
| ).toThrowErrorMatchingInlineSnapshot( | ||
| `"[foo-namespace.age]: Error: expected value of type [string] but got [number]"` | ||
| ); | ||
| }); | ||
|
|
||
| test('fails when no discriminator is provided', () => { | ||
| const type = schema.discriminatedOneOf('type', [ | ||
| schema.object({ type: schema.literal('foo'), foo: schema.string() }), | ||
| schema.object({ type: schema.literal('bar'), bar: schema.number() }), | ||
| ]); | ||
|
|
||
| expect(() => type.validate({ foo: 12 })).toThrowErrorMatchingInlineSnapshot( | ||
| `"value [undefined] did not match any of the allowed values for [type]: [Error: expected value to equal [foo], Error: expected value to equal [bar]]"` | ||
| ); | ||
| }); | ||
|
|
||
| test('fails when discriminator is provided, but is not any allowed value', () => { | ||
| const type = schema.discriminatedOneOf('type', [ | ||
| schema.object({ type: schema.literal('foo'), foo: schema.string() }), | ||
| schema.object({ type: schema.literal('bar'), bar: schema.number() }), | ||
| ]); | ||
|
|
||
| expect(() => type.validate({ type: 'foo1', foo: 12 })).toThrowErrorMatchingInlineSnapshot( | ||
| `"value [foo1] did not match any of the allowed values for [type]: [Error: expected value to equal [foo], Error: expected value to equal [bar]]"` | ||
| ); | ||
| }); | ||
|
|
||
| test('fails when discriminator matches but the rest of the type fails', () => { | ||
| const type = schema.discriminatedOneOf('type', [ | ||
| schema.object({ type: schema.literal('foo'), foo: schema.string() }), | ||
| schema.object({ type: schema.literal('bar'), bar: schema.number() }), | ||
| ]); | ||
|
|
||
| expect(() => type.validate({ type: 'foo', foo: 12 })).toThrowErrorMatchingInlineSnapshot( | ||
| `"[foo]: Error: expected value of type [string] but got [number]"` | ||
| ); | ||
| }); | ||
|
|
||
| test('fails when discriminator matches but the rest of the type fails (nested object)', () => { | ||
| const type = schema.discriminatedOneOf('type', [ | ||
| schema.object({ type: schema.literal('foo'), object: schema.object({ foo: schema.string() }) }), | ||
| schema.object({ type: schema.literal('bar'), bar: schema.number() }), | ||
| ]); | ||
|
|
||
| expect(() => | ||
| type.validate({ type: 'foo', object: { foo: 12 } }) | ||
| ).toThrowErrorMatchingInlineSnapshot( | ||
| `"[object.foo]: Error: expected value of type [string] but got [number]"` | ||
| ); | ||
| }); | ||
|
|
||
| test('fails weirdly if discriminator keys are not ordered consistently', () => { | ||
| // Unfortunately I can't see a good way of handling this case gracefully, | ||
| // so hopefully developers will specify their discriminator key first in the schema. | ||
| // Alternatively, we can try to find a way to not abort validation early for discriminatedOneOf | ||
| // types to introspect the error details better (likely a performance hit). | ||
| const type = schema.discriminatedOneOf('type', [ | ||
| schema.object({ foo: schema.string(), type: schema.literal('foo') }), | ||
| schema.object({ type: schema.literal('bar'), bar: schema.number() }), | ||
| ]); | ||
|
|
||
| expect(() => type.validate({ type: 'foo1', nothing: 12 })).toThrowErrorMatchingInlineSnapshot( | ||
| `"[foo]: Error: expected value of type [string] but got [undefined]"` | ||
| ); | ||
| }); | ||
|
|
||
| test('fails if nested discriminatedOneOf types fail due to discriminator', () => { | ||
| const type = schema.discriminatedOneOf('discriminator1', [ | ||
| schema.object({ | ||
| discriminator1: schema.literal('foo'), | ||
| foo: schema.discriminatedOneOf('discriminator2', [ | ||
| schema.object({ discriminator2: schema.literal('foo'), foo: schema.string() }), | ||
| schema.object({ discriminator2: schema.literal('bar'), foo: schema.number() }), | ||
| ]), | ||
| }), | ||
| schema.object({ discriminator1: schema.literal('foo'), foo: schema.number() }), | ||
| ]); | ||
|
|
||
| expect(() => | ||
| type.validate({ discriminator1: 'foo', foo: { discriminator2: 'baz', foo: 12 } }) | ||
| ).toThrowErrorMatchingInlineSnapshot( | ||
| `"[foo]: Error: value [baz] did not match any of the allowed values for [discriminator2]: [Error: expected value to equal [foo], Error: expected value to equal [bar]]"` | ||
| ); | ||
| }); | ||
|
|
||
| test('fails if nested discriminatedOneOf types fail', () => { | ||
| const type = schema.discriminatedOneOf('discriminator1', [ | ||
| schema.object({ | ||
| discriminator1: schema.literal('1'), | ||
| foo1: schema.discriminatedOneOf('discriminator2', [ | ||
| schema.object({ discriminator2: schema.literal('foo'), foo2: schema.string() }), | ||
| schema.object({ discriminator2: schema.literal('bar'), foo2: schema.number() }), | ||
| ]), | ||
| }), | ||
| schema.object({ discriminator1: schema.literal('2'), foo: schema.number() }), | ||
| ]); | ||
|
|
||
| expect(() => | ||
| type.validate({ | ||
| discriminator1: '1', | ||
| foo1: { discriminator2: 'bar', foo2: 'should be a number' }, | ||
| }) | ||
| ).toThrowErrorMatchingInlineSnapshot( | ||
| `"[foo1.foo2]: Error: Error: expected value of type [number] but got [string]"` | ||
| ); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,118 @@ | ||
| /* | ||
| * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
| * or more contributor license agreements. Licensed under the "Elastic License | ||
| * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side | ||
| * Public License v 1"; you may not use this file except in compliance with, at | ||
| * your election, the "Elastic License 2.0", the "GNU Affero General Public | ||
| * License v3.0 only", or the "Server Side Public License, v 1". | ||
| */ | ||
|
|
||
| import typeDetect from 'type-detect'; | ||
| import { get } from 'lodash'; | ||
| import { SchemaTypeError, SchemaTypesError } from '../errors'; | ||
| import { internals } from '../internals'; | ||
| import { Type, type TypeOptions, type TypeMeta } from './type'; | ||
| import { META_FIELD_X_OAS_DISCRIMINATOR } from '../oas_meta_fields'; | ||
|
|
||
| export type DiscriminatedUnionTypeOptions<T> = TypeOptions<T> & { | ||
| meta?: Omit<TypeMeta, 'id'>; | ||
| }; | ||
|
|
||
| export class DiscriminatedUnionType<RTS extends Array<Type<any>>, T> extends Type<T> { | ||
| private readonly discriminator: string; | ||
|
|
||
| constructor(discriminator: string, types: RTS, options?: DiscriminatedUnionTypeOptions<T>) { | ||
| let schema = internals.alternatives(types.map((type) => type.getSchema())).match('one'); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't see how this is discriminated based on the This approach requires exactly one of the provided branches to match the value. If zero or more than one match, the validation fails. Compared to using See my solution here.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
True, this is an exclusive or hidden under a constructor that will ask for a discriminator key - I believe this is essentially what a discriminated union is.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah but it's not splitting the various schemas by the Thus why I think this internal schema is no different than the current
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also from a performance perspective, the But with
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree, from an internal implementation perspective it could match this behaviour more optimally, but viewed as a black box they'd work the same, but good point regarding performance! |
||
| schema = schema.meta({ [META_FIELD_X_OAS_DISCRIMINATOR]: discriminator }); | ||
|
|
||
| super(schema, options); | ||
| this.discriminator = discriminator; | ||
| } | ||
|
|
||
| protected handleError(type: string, { value, details }: Record<string, any>, path: string[]) { | ||
| switch (type) { | ||
| case 'any.required': | ||
| return `expected at least one defined value but got [${typeDetect(value)}]`; | ||
| case 'alternatives.any': | ||
| if (!this.discriminator) return; | ||
| const nonDiscriminatorDetails: AlternativeAnyErrorDetail[] = []; | ||
| const isDiscriminatorError = details.every((detail: AlternativeAnyErrorDetail) => { | ||
| if (detail.details.length !== 1) { | ||
| nonDiscriminatorDetails.push(detail); | ||
| return false; | ||
| } | ||
| const errorPath = get(detail, 'details.0.context.error.path'); | ||
| if (errorPath[errorPath.length - 1] !== this.discriminator) { | ||
| nonDiscriminatorDetails.push(detail); | ||
| return false; | ||
| } | ||
| return true; | ||
| }); | ||
| if (isDiscriminatorError) { | ||
| return new SchemaTypeError( | ||
| `value [${value?.[this.discriminator]}] did not match any of the allowed values for [${ | ||
| this.discriminator | ||
| }]: [${details.map((detail: AlternativeAnyErrorDetail) => detail.message).join(', ')}]`, | ||
| path | ||
| ); | ||
| } else if ( | ||
| nonDiscriminatorDetails.length === 1 && | ||
| get(nonDiscriminatorDetails, '0.details.length') === 1 | ||
| ) { | ||
| const childPath = get(nonDiscriminatorDetails, '0.details.0.context.error.path'); | ||
| return new SchemaTypeError(nonDiscriminatorDetails[0].message, childPath); | ||
| } | ||
| return errorDetailToSchemaTypeError( | ||
| 'types that failed validation:', | ||
| nonDiscriminatorDetails.flatMap((detail) => detail.details), | ||
| path | ||
| ); | ||
| case 'alternatives.one': | ||
| return new SchemaTypeError( | ||
| `value [${value?.[this.discriminator]}] matched more than one allowed type of [${ | ||
| this.discriminator | ||
| }]`, | ||
| path | ||
| ); | ||
| case 'alternatives.match': | ||
| return errorDetailToSchemaTypeError( | ||
| 'types that failed validation:', | ||
| details as AlternativeMatchErrorDetail[], | ||
| path | ||
| ); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| interface ErrorDetail { | ||
| context: { | ||
| error: SchemaTypeError; | ||
| }; | ||
| } | ||
|
|
||
| interface AlternativeAnyErrorDetail { | ||
| message: string; | ||
| details: ErrorDetail[]; | ||
| } | ||
|
|
||
| type AlternativeMatchErrorDetail = ErrorDetail; | ||
|
|
||
| function errorDetailToSchemaTypeError( | ||
| message: string, | ||
| details: ErrorDetail[], | ||
| path: string[] | ||
| ): SchemaTypeError { | ||
| return new SchemaTypesError( | ||
| message, | ||
| path, | ||
| details.map((detail: AlternativeMatchErrorDetail, index: number) => { | ||
| const e = detail.context.error; | ||
| const childPathWithIndex = e.path.slice(); | ||
| childPathWithIndex.splice(path.length, 0, index.toString()); | ||
|
|
||
| return e instanceof SchemaTypesError | ||
| ? new SchemaTypesError(e.message, childPathWithIndex, e.errors) | ||
| : new SchemaTypeError(e.message, childPathWithIndex); | ||
| }) | ||
| ); | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI: I also put up a PoC for a discriminated union type here #246095