From 31f17ec0268e7a366516d0e9accb6fc5beeafc61 Mon Sep 17 00:00:00 2001 From: Sylvain Wallez Date: Thu, 5 May 2022 18:01:44 +0200 Subject: [PATCH] Add the "es_quirk" annotation to capture snowflakes in ES behavior (#1674) --- compiler/src/model/metamodel.ts | 4 + compiler/src/model/utils.ts | 14 +++- docs/modeling-guide.md | 14 ++++ output/schema/schema.json | 82 ++++++++++--------- output/typescript/types.ts | 2 +- specification/_types/common.ts | 4 +- .../_types/mapping/dynamic-template.ts | 4 + specification/_types/query_dsl/compound.ts | 14 ++-- specification/indices/_types/IndexSettings.ts | 7 +- 9 files changed, 94 insertions(+), 51 deletions(-) diff --git a/compiler/src/model/metamodel.ts b/compiler/src/model/metamodel.ts index 3ddaa58c2e..bbd2c59523 100644 --- a/compiler/src/model/metamodel.ts +++ b/compiler/src/model/metamodel.ts @@ -139,6 +139,8 @@ export class Property { aliases?: string[] /** If the enclosing class is a variants container, is this a property of the container and not a variant? */ containerProperty?: boolean + /** If this property has a quirk that needs special attention, give a short explanation about it */ + esQuirk?: string } // ------------------------------------------------------------------------------------------------ @@ -158,6 +160,8 @@ export abstract class BaseType { docUrl?: string docId?: string deprecation?: Deprecation + /** If this endpoint has a quirk that needs special attention, give a short explanation about it */ + esQuirk?: string kind: string /** Variant name for externally tagged variants */ variantName?: string diff --git a/compiler/src/model/utils.ts b/compiler/src/model/utils.ts index e2f9b01e5b..d2be33d54a 100644 --- a/compiler/src/model/utils.ts +++ b/compiler/src/model/utils.ts @@ -439,6 +439,10 @@ export function modelEnumDeclaration (declaration: EnumDeclaration): model.Enum type.isOpen = true } + if (typeof tags.es_quirk === 'string') { + type.esQuirk = tags.es_quirk + } + return type } @@ -639,7 +643,8 @@ export function hoistTypeAnnotations (type: model.TypeDefinition, jsDocs: JSDoc[ // We want to enforce a single jsDoc block. assert(jsDocs, jsDocs.length < 2, 'Use a single multiline jsDoc block instead of multiple single line blocks') - const validTags = ['class_serializer', 'doc_url', 'doc_id', 'behavior', 'variants', 'variant', 'shortcut_property', 'codegen_names', 'non_exhaustive'] + const validTags = ['class_serializer', 'doc_url', 'doc_id', 'behavior', 'variants', 'variant', 'shortcut_property', + 'codegen_names', 'non_exhaustive', 'es_quirk'] const tags = parseJsDocTags(jsDocs) if (jsDocs.length === 1) { const description = jsDocs[0].getDescription() @@ -671,6 +676,8 @@ export function hoistTypeAnnotations (type: model.TypeDefinition, jsDocs: JSDoc[ type.kind === 'type_alias' && type.type.kind === 'union_of' && type.type.items.length === type.codegenNames.length, '@codegen_names must have the number of items as the union definition' ) + } else if (tag === 'es_quirk') { + type.esQuirk = value } else { assert(jsDocs, false, `Unhandled tag: '${tag}' with value: '${value}' on type ${type.name.name}`) } @@ -684,7 +691,8 @@ function hoistPropertyAnnotations (property: model.Property, jsDocs: JSDoc[]): v // We want to enforce a single jsDoc block. assert(jsDocs, jsDocs.length < 2, 'Use a single multiline jsDoc block instead of multiple single line blocks') - const validTags = ['stability', 'prop_serializer', 'doc_url', 'aliases', 'codegen_name', 'since', 'server_default', 'variant', 'doc_id'] + const validTags = ['stability', 'prop_serializer', 'doc_url', 'aliases', 'codegen_name', 'since', 'server_default', + 'variant', 'doc_id', 'es_quirk'] const tags = parseJsDocTags(jsDocs) if (jsDocs.length === 1) { const description = jsDocs[0].getDescription() @@ -764,6 +772,8 @@ function hoistPropertyAnnotations (property: model.Property, jsDocs: JSDoc[]): v } else if (tag === 'variant') { assert(jsDocs, value === 'container_property', `Unknown 'variant' value '${value}' on property ${property.name}`) property.containerProperty = true + } else if (tag === 'es_quirk') { + property.esQuirk = value } else { assert(jsDocs, false, `Unhandled tag: '${tag}' with value: '${value}' on property ${property.name}`) } diff --git a/docs/modeling-guide.md b/docs/modeling-guide.md index 8363b0cca1..60a37c32cf 100644 --- a/docs/modeling-guide.md +++ b/docs/modeling-guide.md @@ -397,6 +397,20 @@ export class TermQuery extends QueryBase { } ``` +### Tracking Elasticsearch quirks + +There are a few places where Elasticsearch has an uncommon behavior that does not deserve a specific feature in the API specification metamodel. These quirks still have to be captured so that code generators can act on them. The `eq_quirk` jsdoc tag is meant for that, and can be used on type definitions and properties. + +```ts +/** + * @es_quirk This enum is a boolean that evolved into a tri-state enum. True and False have + * to be (de)serialized as JSON booleans. + */ +enum Foo { true, false, bar } +``` + +Code generators should track the `es_quirk` they implement and fail if a new unhandled quirk is present on a type or a property. This behavior allows code generators to be updated whenever a new quirk is identified in the API specification. + ### Additional information If needed, you can specify additional information on each type with the approariate JSDoc tag. diff --git a/output/schema/schema.json b/output/schema/schema.json index 5ef0bfcc5a..9206e3c061 100644 --- a/output/schema/schema.json +++ b/output/schema/schema.json @@ -33663,7 +33663,7 @@ } } ], - "specLocation": "_types/common.ts#L289-L316" + "specLocation": "_types/common.ts#L291-L318" }, { "inherits": { @@ -33802,7 +33802,7 @@ } } ], - "specLocation": "_types/common.ts#L278-L287" + "specLocation": "_types/common.ts#L280-L289" }, { "inherits": { @@ -35153,6 +35153,7 @@ "specLocation": "_types/Stats.ts#L160-L165" }, { + "esQuirk": "This is a boolean that evolved into an enum. ES also accepts plain booleans for true and false.", "kind": "enum", "members": [ { @@ -35169,7 +35170,7 @@ "name": "Refresh", "namespace": "_types" }, - "specLocation": "_types/common.ts#L237-L241" + "specLocation": "_types/common.ts#L236-L243" }, { "kind": "interface", @@ -35922,7 +35923,7 @@ "name": "SearchType", "namespace": "_types" }, - "specLocation": "_types/common.ts#L243-L248" + "specLocation": "_types/common.ts#L245-L250" }, { "kind": "interface", @@ -36841,7 +36842,7 @@ "name": "SuggestMode", "namespace": "_types" }, - "specLocation": "_types/common.ts#L250-L254" + "specLocation": "_types/common.ts#L252-L256" }, { "description": "The suggestion name as returned from the server. Depending whether typed_keys is specified this could come back\nin the form of `name#type` instead of simply `name`", @@ -36903,7 +36904,7 @@ "name": "ThreadType", "namespace": "_types" }, - "specLocation": "_types/common.ts#L256-L260" + "specLocation": "_types/common.ts#L258-L262" }, { "codegenNames": [ @@ -37377,7 +37378,7 @@ "name": "WaitForActiveShardOptions", "namespace": "_types" }, - "specLocation": "_types/common.ts#L263-L266" + "specLocation": "_types/common.ts#L265-L268" }, { "codegenNames": [ @@ -37436,7 +37437,7 @@ "name": "WaitForEvents", "namespace": "_types" }, - "specLocation": "_types/common.ts#L268-L275" + "specLocation": "_types/common.ts#L270-L277" }, { "kind": "interface", @@ -55711,6 +55712,7 @@ "specLocation": "_types/mapping/range.ts#L42-L44" }, { + "esQuirk": "This is a boolean that evolved into an enum. Boolean values should be accepted on reading, and\ntrue and false must be serialized as JSON booleans, or it may break Kibana (see elasticsearch-java#139)", "kind": "enum", "members": [ { @@ -55730,7 +55732,7 @@ "name": "DynamicMapping", "namespace": "_types.mapping" }, - "specLocation": "_types/mapping/dynamic-template.ts#L37-L42" + "specLocation": "_types/mapping/dynamic-template.ts#L37-L46" }, { "kind": "interface", @@ -60042,7 +60044,7 @@ "name": "FieldValueFactorModifier", "namespace": "_types.query_dsl" }, - "specLocation": "_types/query_dsl/compound.ts#L145-L156" + "specLocation": "_types/query_dsl/compound.ts#L147-L158" }, { "kind": "interface", @@ -60124,9 +60126,10 @@ "name": "FunctionBoostMode", "namespace": "_types.query_dsl" }, - "specLocation": "_types/query_dsl/compound.ts#L136-L143" + "specLocation": "_types/query_dsl/compound.ts#L138-L145" }, { + "esQuirk": "this container is valid without a variant. Despite being documented as a function, 'weight'\nis actually a container property that can be combined with a function. Comment in the ES code\n(SearchModule#registerScoreFunctions) says: Weight doesn't have its own parser, so every function\nsupports it out of the box. Can be a single function too when not associated to any other function,\nwhich is why it needs to be registered manually here.", "kind": "interface", "name": { "name": "FunctionScoreContainer", @@ -60224,7 +60227,7 @@ } } ], - "specLocation": "_types/query_dsl/compound.ts#L107-L125", + "specLocation": "_types/query_dsl/compound.ts#L107-L127", "variants": { "kind": "container" } @@ -60255,7 +60258,7 @@ "name": "FunctionScoreMode", "namespace": "_types.query_dsl" }, - "specLocation": "_types/query_dsl/compound.ts#L127-L134" + "specLocation": "_types/query_dsl/compound.ts#L129-L136" }, { "inherits": { @@ -62858,7 +62861,7 @@ "name": "MultiValueMode", "namespace": "_types.query_dsl" }, - "specLocation": "_types/query_dsl/compound.ts#L158-L163" + "specLocation": "_types/query_dsl/compound.ts#L160-L165" }, { "inherits": { @@ -93871,23 +93874,24 @@ "specLocation": "indices/_types/FielddataFrequencyFilter.ts#L22-L26" }, { + "esQuirk": "This is a boolean that evolved into an enum. ES also accepts plain booleans for true and false.", "kind": "enum", "members": [ { - "name": "false" + "name": "true" }, { - "name": "checksum" + "name": "false" }, { - "name": "true" + "name": "checksum" } ], "name": { "name": "IndexCheckOnStartup", "namespace": "indices._types" }, - "specLocation": "indices/_types/IndexSettings.ts#L310-L314" + "specLocation": "indices/_types/IndexSettings.ts#L310-L317" }, { "kind": "interface", @@ -95336,7 +95340,7 @@ } } ], - "specLocation": "indices/_types/IndexSettings.ts#L363-L369" + "specLocation": "indices/_types/IndexSettings.ts#L366-L372" }, { "kind": "interface", @@ -95420,7 +95424,7 @@ } } ], - "specLocation": "indices/_types/IndexSettings.ts#L320-L353" + "specLocation": "indices/_types/IndexSettings.ts#L323-L356" }, { "kind": "interface", @@ -95442,7 +95446,7 @@ } } ], - "specLocation": "indices/_types/IndexSettings.ts#L355-L361" + "specLocation": "indices/_types/IndexSettings.ts#L358-L364" }, { "kind": "interface", @@ -95540,7 +95544,7 @@ } } ], - "specLocation": "indices/_types/IndexSettings.ts#L316-L318" + "specLocation": "indices/_types/IndexSettings.ts#L319-L321" }, { "kind": "interface", @@ -95561,7 +95565,7 @@ } } ], - "specLocation": "indices/_types/IndexSettings.ts#L567-L569" + "specLocation": "indices/_types/IndexSettings.ts#L570-L572" }, { "kind": "interface", @@ -95583,7 +95587,7 @@ } } ], - "specLocation": "indices/_types/IndexSettings.ts#L571-L578" + "specLocation": "indices/_types/IndexSettings.ts#L574-L581" }, { "description": "Mapping Limit Settings", @@ -95672,7 +95676,7 @@ } } ], - "specLocation": "indices/_types/IndexSettings.ts#L433-L445" + "specLocation": "indices/_types/IndexSettings.ts#L436-L448" }, { "kind": "interface", @@ -95695,7 +95699,7 @@ } } ], - "specLocation": "indices/_types/IndexSettings.ts#L457-L464" + "specLocation": "indices/_types/IndexSettings.ts#L460-L467" }, { "kind": "interface", @@ -95717,7 +95721,7 @@ } } ], - "specLocation": "indices/_types/IndexSettings.ts#L494-L500" + "specLocation": "indices/_types/IndexSettings.ts#L497-L503" }, { "kind": "interface", @@ -95739,7 +95743,7 @@ } } ], - "specLocation": "indices/_types/IndexSettings.ts#L485-L492" + "specLocation": "indices/_types/IndexSettings.ts#L488-L495" }, { "kind": "interface", @@ -95762,7 +95766,7 @@ } } ], - "specLocation": "indices/_types/IndexSettings.ts#L466-L474" + "specLocation": "indices/_types/IndexSettings.ts#L469-L477" }, { "kind": "interface", @@ -95785,7 +95789,7 @@ } } ], - "specLocation": "indices/_types/IndexSettings.ts#L476-L483" + "specLocation": "indices/_types/IndexSettings.ts#L479-L486" }, { "kind": "interface", @@ -95808,7 +95812,7 @@ } } ], - "specLocation": "indices/_types/IndexSettings.ts#L447-L455" + "specLocation": "indices/_types/IndexSettings.ts#L450-L458" }, { "kind": "interface", @@ -96005,7 +96009,7 @@ } } ], - "specLocation": "indices/_types/IndexSettings.ts#L502-L507" + "specLocation": "indices/_types/IndexSettings.ts#L505-L510" }, { "kind": "interface", @@ -96059,7 +96063,7 @@ } } ], - "specLocation": "indices/_types/IndexSettings.ts#L520-L525" + "specLocation": "indices/_types/IndexSettings.ts#L523-L528" }, { "kind": "interface", @@ -96104,7 +96108,7 @@ } } ], - "specLocation": "indices/_types/IndexSettings.ts#L509-L518" + "specLocation": "indices/_types/IndexSettings.ts#L512-L521" }, { "kind": "interface", @@ -96172,7 +96176,7 @@ } } ], - "specLocation": "indices/_types/IndexSettings.ts#L527-L536" + "specLocation": "indices/_types/IndexSettings.ts#L530-L539" }, { "kind": "enum", @@ -96201,7 +96205,7 @@ "name": "StorageType", "namespace": "indices._types" }, - "specLocation": "indices/_types/IndexSettings.ts#L538-L565" + "specLocation": "indices/_types/IndexSettings.ts#L541-L568" }, { "kind": "interface", @@ -96358,7 +96362,7 @@ } } ], - "specLocation": "indices/_types/IndexSettings.ts#L371-L393" + "specLocation": "indices/_types/IndexSettings.ts#L374-L396" }, { "kind": "enum", @@ -96382,7 +96386,7 @@ "name": "TranslogDurability", "namespace": "indices._types" }, - "specLocation": "indices/_types/IndexSettings.ts#L395-L410" + "specLocation": "indices/_types/IndexSettings.ts#L398-L413" }, { "kind": "interface", @@ -96418,7 +96422,7 @@ } } ], - "specLocation": "indices/_types/IndexSettings.ts#L412-L431" + "specLocation": "indices/_types/IndexSettings.ts#L415-L434" }, { "kind": "enum", diff --git a/output/typescript/types.ts b/output/typescript/types.ts index b81f1402ee..d4efcc3256 100644 --- a/output/typescript/types.ts +++ b/output/typescript/types.ts @@ -8959,7 +8959,7 @@ export interface IndicesFielddataFrequencyFilter { min_segment_size: integer } -export type IndicesIndexCheckOnStartup = boolean | 'false' | 'checksum' | 'true' +export type IndicesIndexCheckOnStartup = boolean | 'true' | 'false' | 'checksum' export interface IndicesIndexRouting { allocation?: IndicesIndexRoutingAllocation diff --git a/specification/_types/common.ts b/specification/_types/common.ts index 49805d25b5..4ddca772eb 100644 --- a/specification/_types/common.ts +++ b/specification/_types/common.ts @@ -233,7 +233,9 @@ export enum OpType { create = 1 } -// Note: ES also accepts plain booleans for true and false. The TS generator implements this leniency rule. +/** + * @es_quirk This is a boolean that evolved into an enum. ES also accepts plain booleans for true and false. + */ export enum Refresh { true, false, diff --git a/specification/_types/mapping/dynamic-template.ts b/specification/_types/mapping/dynamic-template.ts index 3cc1bbced3..30e005226b 100644 --- a/specification/_types/mapping/dynamic-template.ts +++ b/specification/_types/mapping/dynamic-template.ts @@ -34,6 +34,10 @@ export enum MatchType { regex = 1 } +/** + * @es_quirk This is a boolean that evolved into an enum. Boolean values should be accepted on reading, and + * true and false must be serialized as JSON booleans, or it may break Kibana (see elasticsearch-java#139) + */ export enum DynamicMapping { strict, runtime, diff --git a/specification/_types/query_dsl/compound.ts b/specification/_types/query_dsl/compound.ts index 3b4a44e545..a13af503f8 100644 --- a/specification/_types/query_dsl/compound.ts +++ b/specification/_types/query_dsl/compound.ts @@ -104,12 +104,14 @@ export type DecayFunction = | NumericDecayFunction | GeoDecayFunction -/** @variants container */ -// This container is valid without a variant. Also, despite being documented as a function, 'weight' is actually a -// container property that can be combined with a function. -// From SearchModule#registerScoreFunctions in ES: -// Weight doesn't have its own parser, so every function supports it out of the box. Can be a single function too when -// not associated to any other function, which is why it needs to be registered manually here. +/** + * @variants container + * @es_quirk this container is valid without a variant. Despite being documented as a function, 'weight' + * is actually a container property that can be combined with a function. Comment in the ES code + * (SearchModule#registerScoreFunctions) says: Weight doesn't have its own parser, so every function + * supports it out of the box. Can be a single function too when not associated to any other function, + * which is why it needs to be registered manually here. + */ export class FunctionScoreContainer { exp?: DecayFunction gauss?: DecayFunction diff --git a/specification/indices/_types/IndexSettings.ts b/specification/indices/_types/IndexSettings.ts index f2257aaf84..fc05a9556b 100644 --- a/specification/indices/_types/IndexSettings.ts +++ b/specification/indices/_types/IndexSettings.ts @@ -307,10 +307,13 @@ export class IndexSettingBlocks { metadata?: boolean } +/** + * @es_quirk This is a boolean that evolved into an enum. ES also accepts plain booleans for true and false. + */ export enum IndexCheckOnStartup { + true, false, - checksum, - true + checksum } export class IndexVersioning {