Skip to content

Commit 131b49b

Browse files
pquentinswallez
pquentin
and
swallez
authored
[8.x] Fix spec validator bugs (#3295) (#3306)
* Fix open generics definition in type alias verification * Add void builtin type * A tagged union can have generic parameters * Add binary builtin type * Fix lint --------- Co-authored-by: Quentin Pradet <quentin.pradet@elastic.co> (cherry picked from commit 9dd5b47) # Conflicts: # output/schema/validation-errors.json Co-authored-by: Sylvain Wallez <sylvain@elastic.co>
1 parent 6a72285 commit 131b49b

File tree

2 files changed

+19
-65
lines changed

2 files changed

+19
-65
lines changed

‎compiler/src/steps/validate-model.ts

Lines changed: 16 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ export default async function validateModel (apiModel: model.Model, restSpec: Ma
149149

150150
// Register builtin types
151151
for (const name of [
152-
'string', 'boolean', 'number', 'null'
152+
'string', 'boolean', 'number', 'null', 'void', 'binary'
153153
]) {
154154
const typeName = {
155155
namespace: '_builtins',
@@ -546,33 +546,29 @@ export default async function validateModel (apiModel: model.Model, restSpec: Ma
546546
}
547547

548548
function validateTypeAlias (typeDef: model.TypeAlias): void {
549-
const openGenerics = new Set(typeDef.generics?.map(t => t.name))
549+
const openGenerics = openGenericSet(typeDef)
550550

551551
if (typeDef.variants != null) {
552-
if (typeDef.generics != null && typeDef.generics.length !== 0) {
553-
modelError('A tagged union should not have generic parameters')
554-
}
555-
556552
if (typeDef.type.kind !== 'union_of') {
557553
modelError('The "variants" tag only applies to unions')
558554
} else {
559-
validateTaggedUnion(typeDef.name, typeDef.type, typeDef.variants)
555+
validateTaggedUnion(typeDef.name, typeDef.type, typeDef.variants, openGenerics)
560556
}
561557
} else {
562558
validateValueOf(typeDef.type, openGenerics)
563559
}
564560
}
565561

566-
function validateTaggedUnion (parentName: TypeName, valueOf: model.UnionOf, variants: model.InternalTag | model.ExternalTag | model.Untagged): void {
562+
function validateTaggedUnion (parentName: TypeName, valueOf: model.UnionOf, variants: model.InternalTag | model.ExternalTag | model.Untagged, openGenerics: Set<string>): void {
567563
if (variants.kind === 'external_tag') {
568564
// All items must have a 'variant' attribute
569-
const items = flattenUnionMembers(valueOf)
565+
const items = flattenUnionMembers(valueOf, openGenerics)
570566

571567
for (const item of items) {
572568
if (item.kind !== 'instance_of') {
573569
modelError('Items of externally tagged unions must be types with a "variant_tag" annotation')
574570
} else {
575-
validateTypeRef(item.type, undefined, new Set<string>())
571+
validateTypeRef(item.type, item.generics, openGenerics)
576572
const type = getTypeDef(item.type)
577573
if (type == null) {
578574
modelError(`Type ${fqn(item.type)} not found`)
@@ -587,13 +583,13 @@ export default async function validateModel (apiModel: model.Model, restSpec: Ma
587583
}
588584
} else if (variants.kind === 'internal_tag') {
589585
const tagName = variants.tag
590-
const items = flattenUnionMembers(valueOf)
586+
const items = flattenUnionMembers(valueOf, openGenerics)
591587

592588
for (const item of items) {
593589
if (item.kind !== 'instance_of') {
594590
modelError('Items of internally tagged unions must be type references')
595591
} else {
596-
validateTypeRef(item.type, undefined, new Set<string>())
592+
validateTypeRef(item.type, item.generics, openGenerics)
597593
const type = getTypeDef(item.type)
598594
if (type == null) {
599595
modelError(`Type ${fqn(item.type)} not found`)
@@ -606,7 +602,7 @@ export default async function validateModel (apiModel: model.Model, restSpec: Ma
606602
}
607603
}
608604

609-
validateValueOf(valueOf, new Set())
605+
validateValueOf(valueOf, openGenerics)
610606
} else if (variants.kind === 'untagged') {
611607
if (fqn(parentName) !== '_types.query_dsl:DecayFunction' &&
612608
fqn(parentName) !== '_types.query_dsl:DistanceFeatureQuery' &&
@@ -619,15 +615,15 @@ export default async function validateModel (apiModel: model.Model, restSpec: Ma
619615
modelError(`Type ${fqn(variants.untypedVariant)} not found`)
620616
}
621617

622-
const items = flattenUnionMembers(valueOf)
618+
const items = flattenUnionMembers(valueOf, openGenerics)
623619
const baseTypes = new Set<string>()
624620
let foundUntyped = false
625621

626622
for (const item of items) {
627623
if (item.kind !== 'instance_of') {
628624
modelError('Items of type untagged unions must be type references')
629625
} else {
630-
validateTypeRef(item.type, undefined, new Set<string>())
626+
validateTypeRef(item.type, item.generics, openGenerics)
631627
const type = getTypeDef(item.type)
632628
if (type == null) {
633629
modelError(`Type ${fqn(item.type)} not found`)
@@ -679,7 +675,7 @@ export default async function validateModel (apiModel: model.Model, restSpec: Ma
679675
// -----------------------------------------------------------------------------------------------
680676
// Constituents of type definitions
681677

682-
function openGenericSet (typeDef: model.Request | model.Response | model.Interface): Set<string> {
678+
function openGenericSet (typeDef: model.Request | model.Response | model.Interface | model.TypeAlias): Set<string> {
683679
return new Set((typeDef.generics ?? []).map(name => fqn(name)))
684680
}
685681

@@ -884,7 +880,7 @@ export default async function validateModel (apiModel: model.Model, restSpec: Ma
884880
} else {
885881
// tagged union: the discriminant tells us what to look for, check each member in isolation
886882
assert(typeDef.type.kind === 'union_of', 'Variants are only allowed on union_of type aliases')
887-
for (const item of flattenUnionMembers(typeDef.type)) {
883+
for (const item of flattenUnionMembers(typeDef.type, new Set())) {
888884
validateValueOfJsonEvents(item)
889885
}
890886

@@ -899,13 +895,13 @@ export default async function validateModel (apiModel: model.Model, restSpec: Ma
899895
}
900896

901897
/** Build the flattened item list of potentially nested unions (this is used for large unions) */
902-
function flattenUnionMembers (union: model.UnionOf): model.ValueOf[] {
898+
function flattenUnionMembers (union: model.UnionOf, openGenerics: Set<string>): model.ValueOf[] {
903899
const allItems = new Array<model.ValueOf>()
904900

905901
function collectItems (items: model.ValueOf[]): void {
906902
for (const item of items) {
907903
if (item.kind !== 'instance_of') {
908-
validateValueOf(item, new Set<string>())
904+
validateValueOf(item, openGenerics)
909905
allItems.push(item)
910906
} else {
911907
const itemType = getTypeDef(item.type)
@@ -915,7 +911,7 @@ export default async function validateModel (apiModel: model.Model, restSpec: Ma
915911
// Recurse in nested union
916912
collectItems(itemType.type.items)
917913
} else {
918-
validateValueOf(item, new Set<string>())
914+
validateValueOf(item, openGenerics)
919915
allItems.push(item)
920916
}
921917
}

‎output/schema/validation-errors.json

Lines changed: 3 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -36,24 +36,10 @@
3636
],
3737
"response": []
3838
},
39-
"async_search.get": {
40-
"request": [],
41-
"response": [
42-
"type_alias definition _types:EpochTime / instance_of - No type definition for '_types.EpochTime:Unit'",
43-
"type_alias definition _types.aggregations:Buckets / union_of / dictionary_of / instance_of - No type definition for '_types.aggregations.Buckets:TBucket'",
44-
"type_alias definition _types.aggregations:Buckets / union_of / array_of / instance_of - No type definition for '_types.aggregations.Buckets:TBucket'",
45-
"type_alias definition _spec_utils:Void / instance_of - No type definition for '_builtins:void'",
46-
"type_alias definition _types:DurationValue / instance_of - No type definition for '_types.DurationValue:Unit'",
47-
"type_alias definition _global.search._types:Suggest - A tagged union should not have generic parameters",
48-
"type_alias definition _global.search._types:Suggest / instance_of / Generics / instance_of - No type definition for '_global.search._types.Suggest:TDocument'",
49-
"type_alias definition _global.search._types:Suggest - Expected 1 generic parameters but got 0"
50-
]
51-
},
5239
"async_search.submit": {
5340
"request": [
5441
"Request: query parameter 'min_compatible_shard_node' does not exist in the json spec",
55-
"interface definition _types:QueryVectorBuilder - Property text_embedding is a single-variant and must be required",
56-
"type_alias definition _spec_utils:PipeSeparatedFlags / union_of / instance_of - No type definition for '_spec_utils.PipeSeparatedFlags:T'"
42+
"interface definition _types:QueryVectorBuilder - Property text_embedding is a single-variant and must be required"
5743
],
5844
"response": []
5945
},
@@ -74,9 +60,7 @@
7460
"request": [
7561
"request definition cat.allocation:Request / body - A request with inherited properties must have a PropertyBody"
7662
],
77-
"response": [
78-
"type_alias definition _spec_utils:Stringified / union_of / instance_of - No type definition for '_spec_utils.Stringified:T'"
79-
]
63+
"response": []
8064
},
8165
"cat.component_templates": {
8266
"request": [
@@ -431,12 +415,6 @@
431415
],
432416
"response": []
433417
},
434-
"connector.update_error": {
435-
"request": [
436-
"type_alias definition _spec_utils:WithNullValue / union_of / instance_of - No type definition for '_spec_utils.WithNullValue:T'"
437-
],
438-
"response": []
439-
},
440418
"connector.update_features": {
441419
"request": [
442420
"Missing request & response"
@@ -493,12 +471,6 @@
493471
],
494472
"response": []
495473
},
496-
"esql.query": {
497-
"request": [],
498-
"response": [
499-
"type_alias definition _types:EsqlColumns / instance_of - No type definition for '_builtins:binary'"
500-
]
501-
},
502474
"features.get_features": {
503475
"request": [
504476
"Request: missing json spec query parameter 'master_timeout'"
@@ -523,12 +495,6 @@
523495
],
524496
"response": []
525497
},
526-
"fleet.msearch": {
527-
"request": [],
528-
"response": [
529-
"type_alias definition _global.msearch:ResponseItem / union_of / instance_of / Generics / instance_of - No type definition for '_global.msearch.ResponseItem:TDocument'"
530-
]
531-
},
532498
"fleet.post_secret": {
533499
"request": [
534500
"Missing request & response"
@@ -728,12 +694,6 @@
728694
],
729695
"response": []
730696
},
731-
"mget": {
732-
"request": [],
733-
"response": [
734-
"type_alias definition _global.mget:ResponseItem / union_of / instance_of / Generics / instance_of - No type definition for '_global.mget.ResponseItem:TDocument'"
735-
]
736-
},
737697
"ml.delete_trained_model": {
738698
"request": [
739699
"Request: missing json spec query parameter 'timeout'"
@@ -842,9 +802,7 @@
842802
"Request: query parameter 'grid_agg' does not exist in the json spec",
843803
"Request: missing json spec query parameter 'track_total_hits'"
844804
],
845-
"response": [
846-
"type_alias definition _types:MapboxVectorTiles / instance_of - No type definition for '_builtins:binary'"
847-
]
805+
"response": []
848806
},
849807
"search_shards": {
850808
"request": [

0 commit comments

Comments
 (0)