From f461aa86a3f60813f6edca650f059009269d47e7 Mon Sep 17 00:00:00 2001 From: Sylvain Wallez Date: Tue, 3 May 2022 18:09:21 +0200 Subject: [PATCH 1/3] Add the "es_quirk" annotation to capture snowflakes in ES behavior --- compiler/src/model/metamodel.ts | 4 + compiler/src/model/utils.ts | 14 ++- output/schema/schema.json | 92 ++++++++++--------- 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 +- 8 files changed, 85 insertions(+), 56 deletions(-) diff --git a/compiler/src/model/metamodel.ts b/compiler/src/model/metamodel.ts index f3a65a44f0..fe0b1f8b2a 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 02131864ac..53fd54a656 100644 --- a/compiler/src/model/utils.ts +++ b/compiler/src/model/utils.ts @@ -444,6 +444,10 @@ export function modelEnumDeclaration (declaration: EnumDeclaration): model.Enum type.isOpen = true } + if (typeof tags.es_quirk === 'string') { + type.esQuirk = tags.es_quirk + } + return type } @@ -647,7 +651,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() @@ -682,6 +687,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}`) } @@ -695,7 +702,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() @@ -787,6 +795,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/output/schema/schema.json b/output/schema/schema.json index c9401015cc..cfcfd20501 100644 --- a/output/schema/schema.json +++ b/output/schema/schema.json @@ -34181,7 +34181,7 @@ } } ], - "specLocation": "_types/common.ts#L288-L315" + "specLocation": "_types/common.ts#L290-L317" }, { "inherits": { @@ -34320,7 +34320,7 @@ } } ], - "specLocation": "_types/common.ts#L277-L286" + "specLocation": "_types/common.ts#L279-L288" }, { "inherits": { @@ -35700,6 +35700,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": [ { @@ -35716,7 +35717,7 @@ "name": "Refresh", "namespace": "_types" }, - "specLocation": "_types/common.ts#L234-L238" + "specLocation": "_types/common.ts#L233-L240" }, { "kind": "interface", @@ -36483,7 +36484,7 @@ "name": "SearchType", "namespace": "_types" }, - "specLocation": "_types/common.ts#L240-L245" + "specLocation": "_types/common.ts#L242-L247" }, { "kind": "interface", @@ -37403,7 +37404,7 @@ "name": "SuggestMode", "namespace": "_types" }, - "specLocation": "_types/common.ts#L247-L251" + "specLocation": "_types/common.ts#L249-L253" }, { "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`", @@ -37525,7 +37526,7 @@ "name": "ThreadType", "namespace": "_types" }, - "specLocation": "_types/common.ts#L253-L259" + "specLocation": "_types/common.ts#L255-L261" }, { "codegenNames": [ @@ -37955,7 +37956,7 @@ "name": "WaitForActiveShardOptions", "namespace": "_types" }, - "specLocation": "_types/common.ts#L261-L265" + "specLocation": "_types/common.ts#L263-L267" }, { "codegenNames": [ @@ -38014,7 +38015,7 @@ "name": "WaitForEvents", "namespace": "_types" }, - "specLocation": "_types/common.ts#L267-L274" + "specLocation": "_types/common.ts#L269-L276" }, { "kind": "interface", @@ -56816,6 +56817,7 @@ "specLocation": "_types/mapping/range.ts#L42-L44" }, { + "esQuirk": "This is a boolean that evolved into an enum. ES also accepts plain booleans for true and false.", "kind": "enum", "members": [ { @@ -56835,7 +56837,7 @@ "name": "DynamicMapping", "namespace": "_types.mapping" }, - "specLocation": "_types/mapping/dynamic-template.ts#L37-L42" + "specLocation": "_types/mapping/dynamic-template.ts#L37-L45" }, { "kind": "interface", @@ -61226,7 +61228,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", @@ -61308,9 +61310,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", @@ -61408,7 +61411,7 @@ } } ], - "specLocation": "_types/query_dsl/compound.ts#L107-L125", + "specLocation": "_types/query_dsl/compound.ts#L107-L127", "variants": { "kind": "container" } @@ -61439,7 +61442,7 @@ "name": "FunctionScoreMode", "namespace": "_types.query_dsl" }, - "specLocation": "_types/query_dsl/compound.ts#L127-L134" + "specLocation": "_types/query_dsl/compound.ts#L129-L136" }, { "inherits": { @@ -64085,7 +64088,7 @@ "name": "MultiValueMode", "namespace": "_types.query_dsl" }, - "specLocation": "_types/query_dsl/compound.ts#L158-L163" + "specLocation": "_types/query_dsl/compound.ts#L160-L165" }, { "inherits": { @@ -96804,7 +96807,7 @@ } } ], - "specLocation": "indices/_types/IndexSettings.ts#L390-L392" + "specLocation": "indices/_types/IndexSettings.ts#L393-L395" }, { "kind": "interface", @@ -97072,23 +97075,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#L248-L252" + "specLocation": "indices/_types/IndexSettings.ts#L248-L255" }, { "kind": "interface", @@ -98373,7 +98377,7 @@ } } ], - "specLocation": "indices/_types/IndexSettings.ts#L302-L308" + "specLocation": "indices/_types/IndexSettings.ts#L305-L311" }, { "kind": "interface", @@ -98457,7 +98461,7 @@ } } ], - "specLocation": "indices/_types/IndexSettings.ts#L259-L292" + "specLocation": "indices/_types/IndexSettings.ts#L262-L295" }, { "kind": "interface", @@ -98479,7 +98483,7 @@ } } ], - "specLocation": "indices/_types/IndexSettings.ts#L294-L300" + "specLocation": "indices/_types/IndexSettings.ts#L297-L303" }, { "kind": "interface", @@ -98511,7 +98515,7 @@ } } ], - "specLocation": "indices/_types/IndexSettings.ts#L310-L313" + "specLocation": "indices/_types/IndexSettings.ts#L313-L316" }, { "kind": "interface", @@ -98813,7 +98817,7 @@ } } ], - "specLocation": "indices/_types/IndexSettings.ts#L254-L257" + "specLocation": "indices/_types/IndexSettings.ts#L257-L260" }, { "kind": "interface", @@ -98834,7 +98838,7 @@ } } ], - "specLocation": "indices/_types/IndexSettings.ts#L528-L530" + "specLocation": "indices/_types/IndexSettings.ts#L531-L533" }, { "kind": "interface", @@ -98856,7 +98860,7 @@ } } ], - "specLocation": "indices/_types/IndexSettings.ts#L532-L539" + "specLocation": "indices/_types/IndexSettings.ts#L535-L542" }, { "description": "Mapping Limit Settings", @@ -98946,7 +98950,7 @@ } } ], - "specLocation": "indices/_types/IndexSettings.ts#L394-L406" + "specLocation": "indices/_types/IndexSettings.ts#L397-L409" }, { "kind": "interface", @@ -98969,7 +98973,7 @@ } } ], - "specLocation": "indices/_types/IndexSettings.ts#L418-L425" + "specLocation": "indices/_types/IndexSettings.ts#L421-L428" }, { "kind": "interface", @@ -98991,7 +98995,7 @@ } } ], - "specLocation": "indices/_types/IndexSettings.ts#L455-L461" + "specLocation": "indices/_types/IndexSettings.ts#L458-L464" }, { "kind": "interface", @@ -99013,7 +99017,7 @@ } } ], - "specLocation": "indices/_types/IndexSettings.ts#L446-L453" + "specLocation": "indices/_types/IndexSettings.ts#L449-L456" }, { "kind": "interface", @@ -99036,7 +99040,7 @@ } } ], - "specLocation": "indices/_types/IndexSettings.ts#L427-L435" + "specLocation": "indices/_types/IndexSettings.ts#L430-L438" }, { "kind": "interface", @@ -99059,7 +99063,7 @@ } } ], - "specLocation": "indices/_types/IndexSettings.ts#L437-L444" + "specLocation": "indices/_types/IndexSettings.ts#L440-L447" }, { "kind": "interface", @@ -99082,7 +99086,7 @@ } } ], - "specLocation": "indices/_types/IndexSettings.ts#L408-L416" + "specLocation": "indices/_types/IndexSettings.ts#L411-L419" }, { "kind": "interface", @@ -99103,7 +99107,7 @@ } } ], - "specLocation": "indices/_types/IndexSettings.ts#L315-L317" + "specLocation": "indices/_types/IndexSettings.ts#L318-L320" }, { "kind": "interface", @@ -99135,7 +99139,7 @@ } } ], - "specLocation": "indices/_types/IndexSettings.ts#L319-L322" + "specLocation": "indices/_types/IndexSettings.ts#L322-L325" }, { "kind": "interface", @@ -99193,7 +99197,7 @@ } } ], - "specLocation": "indices/_types/IndexSettings.ts#L386-L388" + "specLocation": "indices/_types/IndexSettings.ts#L389-L391" }, { "kind": "interface", @@ -99805,7 +99809,7 @@ } } ], - "specLocation": "indices/_types/IndexSettings.ts#L463-L468" + "specLocation": "indices/_types/IndexSettings.ts#L466-L471" }, { "kind": "interface", @@ -99859,7 +99863,7 @@ } } ], - "specLocation": "indices/_types/IndexSettings.ts#L481-L486" + "specLocation": "indices/_types/IndexSettings.ts#L484-L489" }, { "kind": "interface", @@ -99905,7 +99909,7 @@ } } ], - "specLocation": "indices/_types/IndexSettings.ts#L470-L479" + "specLocation": "indices/_types/IndexSettings.ts#L473-L482" }, { "kind": "interface", @@ -99973,7 +99977,7 @@ } } ], - "specLocation": "indices/_types/IndexSettings.ts#L488-L497" + "specLocation": "indices/_types/IndexSettings.ts#L491-L500" }, { "kind": "enum", @@ -100002,7 +100006,7 @@ "name": "StorageType", "namespace": "indices._types" }, - "specLocation": "indices/_types/IndexSettings.ts#L499-L526" + "specLocation": "indices/_types/IndexSettings.ts#L502-L529" }, { "kind": "interface", @@ -100159,7 +100163,7 @@ } } ], - "specLocation": "indices/_types/IndexSettings.ts#L324-L346" + "specLocation": "indices/_types/IndexSettings.ts#L327-L349" }, { "kind": "enum", @@ -100183,7 +100187,7 @@ "name": "TranslogDurability", "namespace": "indices._types" }, - "specLocation": "indices/_types/IndexSettings.ts#L348-L363" + "specLocation": "indices/_types/IndexSettings.ts#L351-L366" }, { "kind": "interface", @@ -100219,7 +100223,7 @@ } } ], - "specLocation": "indices/_types/IndexSettings.ts#L365-L384" + "specLocation": "indices/_types/IndexSettings.ts#L368-L387" }, { "kind": "enum", diff --git a/output/typescript/types.ts b/output/typescript/types.ts index 3511773ef0..f62d891660 100644 --- a/output/typescript/types.ts +++ b/output/typescript/types.ts @@ -9195,7 +9195,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 bfd702ec16..32e79a0bc9 100644 --- a/specification/_types/common.ts +++ b/specification/_types/common.ts @@ -230,7 +230,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 c5bf9a22e3..ee23a4c311 100644 --- a/specification/indices/_types/IndexSettings.ts +++ b/specification/indices/_types/IndexSettings.ts @@ -245,10 +245,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 { From d4fcbc49b12e2539d40b7d655909a45511d09746 Mon Sep 17 00:00:00 2001 From: Sylvain Wallez Date: Tue, 3 May 2022 18:58:28 +0200 Subject: [PATCH 2/3] update output --- output/schema/schema.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/output/schema/schema.json b/output/schema/schema.json index cfcfd20501..ca92f45e2e 100644 --- a/output/schema/schema.json +++ b/output/schema/schema.json @@ -56817,7 +56817,7 @@ "specLocation": "_types/mapping/range.ts#L42-L44" }, { - "esQuirk": "This is a boolean that evolved into an enum. ES also accepts plain booleans for true and false.", + "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": [ { @@ -56837,7 +56837,7 @@ "name": "DynamicMapping", "namespace": "_types.mapping" }, - "specLocation": "_types/mapping/dynamic-template.ts#L37-L45" + "specLocation": "_types/mapping/dynamic-template.ts#L37-L46" }, { "kind": "interface", From 053e4510418b74cadb413b096cb1166df21f95f7 Mon Sep 17 00:00:00 2001 From: Sylvain Wallez Date: Thu, 5 May 2022 17:29:49 +0200 Subject: [PATCH 3/3] Add docs --- docs/known-issues.md | 4 ++-- docs/modeling-guide.md | 14 ++++++++++++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/docs/known-issues.md b/docs/known-issues.md index 34929c7587..56634b3302 100644 --- a/docs/known-issues.md +++ b/docs/known-issues.md @@ -34,7 +34,7 @@ There is nothing that can be done here, it's a security limitation imposed by Gi #### Current solution -Eitgher performa cherry-pick once merged or create a branch directly in this repository. +Either perform a cherry-pick once merged or create a branch directly in this repository. Nit: if you do the latter, name the branch like this: `{username}/{feature_name}` ## Boolean in specific enum shouldn't be sent as string @@ -45,4 +45,4 @@ Nit: if you do the latter, name the branch like this: `{username}/{feature_name} #### Current solution -Handle the enum `true` and `false` to be serialized as booleans and not string. +Handle the enum `true` and `false` to be serialized as booleans and not string. These enums have an `es_quirk` annotation. diff --git a/docs/modeling-guide.md b/docs/modeling-guide.md index 65ae87f1cc..a60a266049 100644 --- a/docs/modeling-guide.md +++ b/docs/modeling-guide.md @@ -399,6 +399,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.